fix: throttle invalid api key attempts

[Justinabox]

Summary

• Fixes Rate-limit invalid API key attempts #120 by applying a privacy-safe auth-failure rate-limit bucket before credential validation once failed attempts reach the configured limit.
• Keeps existing response semantics before threshold: missing/malformed auth returns 401, invalid bearer returns 403, valid keys use the existing per-key rate limit.
• Prunes stale rate-limit buckets on later requests so unauthenticated peer buckets do not accumulate indefinitely.

Tests

• PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/test_api_auth.py -q -> 20 passed in 0.06s
• git diff --check -> passed
• PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q -> 564 passed in 5.68s

Risks

• After a client exceeds the invalid-auth threshold, valid credentials from the same remote are blocked until the rate window clears. This is intentional fail-closed behavior for brute-force resistance.

[Justinabox]

Review/merge-manager classification: needs-fix / needs human security review before autonomous landing.

Diff review found the main throttling behavior is in the right direction, but two privacy/security-policy blockers remain:

• The auth-failure bucket uses the raw peer value in _request_log (auth-failure:<peer>). Please switch to a privacy-safe internal peer bucket and add a regression that raw peer/IP-like values are not present in any rate-limit bucket names.
• The valid-key rate-limit path still keys _request_log by the raw API key. Since this PR is touching auth throttling, please move valid credentials to a non-overlapping privacy-safe bucket format too, and cover that raw bearer/API-key values and raw Authorization headers never appear as bucket names/substrings.
• Avoid namespace collisions between valid-key buckets and auth-failure buckets.

Also needs explicit human/security signoff for the policy behavior where repeated invalid attempts from one peer can temporarily block a later valid credential from the same peer (shared IP/NAT/proxy impact). Local gate evidence was not used to approve/merge because these are security/privacy review blockers.

[Justinabox]

Review/merge-manager classification: needs-human.

I reviewed the diff and it looks technically narrow, but I am not merging it autonomously because it changes API-auth/security policy and the PR is labeled question.

Human decision needed: confirm the intended product/security behavior for per-peer invalid-auth throttling, especially that once a peer crosses the invalid-attempt threshold, otherwise valid credentials from that peer are denied until the rate-limit window/reset path allows recovery. If that policy is approved, this can come back through the merge lane with fresh gates.

[Justinabox]

Human reviewer:

approved, this greatly adds onto the security policy of the application. for the issue mentioned, we include clear .env variables for disabling such behavior when needed and adjusting the rate limit to a higher number.

[Justinabox]

Review-manager classification remains needs-human / needs-fix, not an autonomous merge candidate in this lane.

Acknowledged the human approval for the general invalid-auth throttling policy. I am still holding this PR because it changes API-auth/security policy, and the approval comment also adds an implementation requirement: clear environment/config variables for disabling or tuning the rate limit when deployments need a different policy.

Before this returns as a merge candidate, please update the branch to include that operator-config surface and tests/docs, then rerun:

git diff --check
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q

The review/merge lane will not land security-policy changes without that explicit completed implementation and fresh gates.

[Justinabox]

Release review: needs-human.

This draft is held from autonomous merge because it changes a security-sensitive, credential/SIM-recovery, production-deployment, or public product/API policy surface. Please provide an explicit product/security decision and a refreshed review-ready head (this PR is also stale/dirty where applicable) before it can re-enter the landing queue. No behavior was approved or merged in this pass.

[Justinabox]

Release-captain classification: needs human decision.

This changes authentication/rate-limit policy so a client that has exceeded invalid-auth limits is denied even when it later presents valid credentials from the same remote until the window expires. That security/usability trade-off needs explicit owner approval before an autonomous merge. Please confirm the intended policy, duration, and operational escape-hatch/observability expectations.

[Justinabox]

Release lane update: needs-human.

This change affects a sensitive security/privacy, production-deployment, modem/SIM, audio/hardware, or public realtime product boundary. It will not be autonomously merged under this lane. Please obtain an explicit Korgo decision on the intended policy/scope and, where applicable, rebase on current main with current gate evidence before requesting a new release review.

[Justinabox]

Release review: needs-human.

This PR changes a sensitive credential, security, or modem-safety policy. Per the autonomous release-lane policy it is intentionally held for an explicit Korgo decision and will not be merged on local-test evidence alone. After a decision, refresh it over current main and rerun the required gates before a new review.

[Justinabox]

Release review: needs-human. This changes authentication/rate-limit policy by blocking valid credentials from a peer after invalid attempts. The security trade-off (brute-force resistance versus shared-NAT/operator lockout), trusted-proxy/client identity policy, and rollout expectations require product/security ownership. It is also an old branch; do not rebase or merge autonomously until that decision is recorded.

[Justinabox]

Release review classification: needs-human. This changes security/credential or externally reachable policy behavior and requires Korgo’s explicit product/security decision before release. It will not be auto-merged in this lane. Please retain the branch and provide the requested policy/operational rationale plus updated gates for a later human-approved review.

[Justinabox]

Release-lane classification: needs-human.

This changes API-key failure/rate-limit policy, including deliberate fail-closed blocking of valid credentials after nearby invalid attempts. That is a security and operator-availability policy decision, not a narrow autonomous landing. No merge action taken in this lane.

[Justinabox]

Release-lane classification: needs human decision.

This changes a security-sensitive or externally exposed policy surface. It requires Korgo's explicit product/security decision and a focused security review before an autonomous merge; local test success alone is not authorization to land it.

[Justinabox]

Release classification: needs-human.

This change affects an externally visible security/default-exposure policy. It should not be merged autonomously without Korgo’s explicit decision on the intended compatibility and threat-model behavior. Please retain as a draft pending that decision; any follow-up must be rebased on current main and revalidated with the prescribed full suite.

[Justinabox]

Release review: needs-human. Throttling a peer after invalid API-key attempts creates an availability/security trade-off. Korgo approval is required before merge.

[Justinabox]

Release classification: needs-human decision.

This changes API authentication/rate-limit policy. The scheduled release lane does not make credential or externally visible auth-policy decisions. Please confirm the intended policy (including behavior for valid credentials from a peer that has crossed an invalid-auth threshold) before a dedicated security review and landing pass.