fix: harden API key handling and bound agent task claims

[gasparottog80-hash]

Summary

• hardens request logging to avoid query-string exposure
• centralizes timing-safe secret comparison
• adds minimum-length validation for AGENT_BRIDGE_SECRET
• preserves API-key rate-limit errors as HTTP 429 with Retry-After support
• adds API-key rate-limit telemetry and test-only validation infrastructure
• fixes planner-dependent over-claiming in claimDue()
• fixes planner-dependent over-retirement in retireExhausted()
• adds deterministic regression and concurrency coverage

Why

PostgreSQL could choose a Nested Loop plan that re-evaluated limited
FOR UPDATE SKIP LOCKED subqueries, allowing claimDue() and
retireExhausted() to affect more rows than their requested limit.

The affected selections now use MATERIALIZED CTEs so the bounded set of
rows is evaluated once per statement.

Validation

• bun run check-types — PASS
• bun run lint — PASS
• bun run lint:slop — PASS
• bun run test — PASS

Test totals:

• auth: 43
• db: 115
• env: 17
• telemetry: 61
• validation: 5
• agent: 371
• api: 404
• app: 169

Security

• production API-key rate limiting remains disabled
• no production rate-limit values were introduced
• no API keys or secrets were committed
• no migrations were added
• no deployment was performed

Note

Median task ID was not added because no configured Median binding or
recoverable task ID exists in this checkout; no ID was fabricated.

---

Summary by cubic

Fixes agent task claim/retire operations exceeding their row limit and hardens API key handling so rate-limit errors surface correctly.

Agent task limits

• claimDue() and retireExhausted() now build a MATERIALIZED CTE so the bound of selected rows is evaluated once, preventing planner-dependent over-claiming and over-retirement.
• retireExhausted() now accepts an optional kinds filter to act on only targeted task kinds.
• Adds deterministic regression and concurrency integration tests, isolated per run via a TEST_RUN_ID suffix.

API key handling

• Rate-limit rejections from the auth layer now return HTTP 429 with Retry-After instead of being treated as unauthorized; production rate limiting remains disabled.
• Centralizes timing-safe comparison in node:crypto, and AGENT_BRIDGE_SECRET must now be at least 32 characters; empty values are treated as unset, and absent secrets remain optional.
• Request logging switches to request.path to avoid exposing query strings, records whether an API key was presented, and logs once per request on finish at a status-dependent level.
• Adds a telemetry counter for API key rate-limits plus a backfill helper for rate-limit policies, with @better-auth/api-key as a dev dependency for test coverage.

^{Written for commit 79f443c. Summary will update on new commits.}

[vercel]

@gasparottog80-hash is attempting to deploy a commit to the Comp AI - PoC Team on Vercel.

A member of the Team first needs to authorize it.

[cubic-dev-ai]

1 issue found across 25 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/logging/request-logger.middleware.ts">

<violation number="1" location="apps/api/src/logging/request-logger.middleware.ts:60">
P3: The hardening is untested: `logging.spec.ts` exercises only request-id behavior and never asserts the logged payload, so nothing guards the new query-string stripping (`request.path` vs `request.originalUrl`) or the `apiKeyPresented` field. Add a test that drives `logCompleted` output (capture the logger as in the existing spec) and asserts the payload's `path` excludes the query string and `apiKeyPresented` is true/false with/without `x-api-key`.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The hardening is untested: logging.spec.ts exercises only request-id behavior and never asserts the logged payload, so nothing guards the new query-string stripping (request.path vs request.originalUrl) or the apiKeyPresented field. Add a test that drives logCompleted output (capture the logger as in the existing spec) and asserts the payload's path excludes the query string and apiKeyPresented is true/false with/without x-api-key.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/logging/request-logger.middleware.ts, line 60:

<comment>The hardening is untested: `logging.spec.ts` exercises only request-id behavior and never asserts the logged payload, so nothing guards the new query-string stripping (`request.path` vs `request.originalUrl`) or the `apiKeyPresented` field. Add a test that drives `logCompleted` output (capture the logger as in the existing spec) and asserts the payload's `path` excludes the query string and `apiKeyPresented` is true/false with/without `x-api-key`.</comment>

<file context>
@@ -56,6 +57,7 @@ export class RequestLoggerMiddleware implements NestMiddleware {
 			durationMs: Number(durationMs.toFixed(1)),
 			ip: request.ip,
 			userAgent: request.get("user-agent"),
+			apiKeyPresented: Boolean(request.get(API_KEY_HEADER)),
 		};
 
</file context>

[cubic-dev-ai]

All reported issues were addressed across 7 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[gasparottog80-hash]

Validation complete for current head d79e7238b503fcf6deccb5667fb81ea7d1af1b3d.

• bun run check-types — pass
• bun run lint — pass
• bun run lint:slop — pass
• bun run test — pass (10/10 tasks)
• Cubic review check — success; latest review reports all raised issues addressed
• Production API-key rate limiting remains disabled
• No secrets, migrations, or deployment changes were introduced
• Planner-safety fixes retain MATERIALIZED CTEs

The PR is currently mergeable. Vercel deployment authorization was intentionally not granted because it requires Comp AI - PoC team permission.

Ready for maintainer merge/review.

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[gasparottog80-hash]

Hi maintainers — the current PR head is 79f443cb1d25766ed1dbf17a82a6d93651ee58b0 and is mergeable with no conflicts.

Validation completed successfully:

• bun run check-types — PASS
• bun run lint — PASS
• bun run lint:slop — PASS
• bun run test — PASS
• Cubic review — SUCCESS, 0 issues on the latest changes

The remaining Vercel deployment authorization requires a member of the Comp AI - PoC team; I do not have that permission and have not attempted to bypass it.

Could a maintainer please review and merge PR #235 when appropriate, and authorize the Vercel deployments if they are required by the project workflow?