feat/one-api

Tinybird occurred_at captured post-response, not at request time

apps/platform/src/api/utils/public-v1-telemetry.ts

shipToTinybird schedules the append via after(), which runs after the response is sent. occurred_at is then captured as new Date().toISOString() inside the callback — potentially milliseconds to seconds after the actual request. The durationMs in the same record was captured at request time, so occurred_at and durationMs will disagree. Fix: capture occurred_at before calling shipToTinybird and pass it as a field on the record.

Duplicate client_secret in form body: last value wins silently

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

Object.fromEntries(new URLSearchParams(text)) keeps only the last value for duplicate keys. Per RFC 6749 §3.3, duplicate parameters MUST be treated as malformed. Consider detecting duplicates and returning invalid_request (400). URLSearchParams.getAll(key).length > 1 is the check.

oauth/token fire-and-forget catch silently swallows all errors

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

apiKeyQueries.updateLastUsedAt(...).catch(() => {}) discards all failures including connection errors. The parallel call in auth-token.handler.ts logs the error with the apiKeyPublicId for diagnostic traceability. The oauth/token handler should match that pattern.

DUMMY_HASH module-level Promise is correct

apps/platform/src/api/handlers/public-v1/api-key-verify.ts

hashSecret() returns Promise<string>. Awaiting a settled Promise multiple times in JavaScript correctly returns the same value — no correctness issue. The scrypt work is done once at module init.

Sealing key null+non-transient correctly cached

apps/platform/src/lib/secrets/sealing-key.ts

When readFromSecretsManager returns { key: null, transient: false } and SEALING_KEY env is empty, decodeKey('') returns null (0-byte Buffer fails the 32-byte check). Caching proceeds correctly — null returned on every subsequent call for an unconfigured environment.

No rate limiting on OAuth token endpoint

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

The standalone Next.js route at /api/v1/auth/oauth/token is outside the ts-rest mount and does not inherit its IP-based rate limiting. An unauthenticated attacker can send unlimited POST requests to brute-force service key prefixes. Even with scrypt cost, at scale this is a sustained CPU/DB amplification vector. A middleware-level or edge-level rate limit is needed before this endpoint is reachable in production.

Tinybird metrics_runtime token now grants APPEND to api_request_events

infra/tinybird/datasources/api_request_events.datasource

The existing metrics_runtime token was scoped to read operations on metrics datasources. Wiring the telemetry sink through the same token means a token leak now grants APPEND access to api_request_events — recording route, actor, org_public_id, principal, and outcome for every API call. An attacker could inject fabricated records, corrupting analytics and obscuring real usage anomalies. A dedicated write-only APPEND token should be used.

Timing oracle: bk_ fast-rejection leaks key-type information

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

SERVICE_KEY_SECRET_FORMAT.test(clientSecret) rejects customer keys (bk_) immediately before the DB lookup and scrypt verify that run for sk_ keys, creating a measurable timing difference. The endpoint is service-only and undocumented which reduces risk, but routing bk_ keys through verifyPresentedApiKey (or burning a dummy verify before returning invalid_client for format mismatches) would eliminate the oracle.

DB error messages in api-key-verify.ts may leak schema details in logs

apps/platform/src/api/handlers/public-v1/api-key-verify.ts

console.error logs e.message directly. PostgreSQL error messages can include column names, constraint names, or partial query fragments. Consider filtering to error code only (e.g. pgError.code) rather than the full message string if logs are forwarded to a third-party aggregator.

Fire-and-forget updateLastUsedAt using service-role DB is appropriate

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

Service-role DB is appropriate for a last-used-at update keyed by the verified internal matched.id (not user input). Fire-and-forget is appropriate — failure should not block token issuance.

OAuth route telemetry gap undocumented — indistinguishable from accidental miss

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

The route bypasses the ts-rest mount, so recordApiRequest is never invoked. The exemption is intentional but undocumented in the route file itself. Without a comment explaining why it is exempt, the next reviewer cannot distinguish intentional from accidental. Add an inline comment or instrument a manual recordApiRequest call.

H1 grey-area: api-key-verify.ts imports @batu/database (client) in handler layer

apps/platform/src/api/handlers/public-v1/api-key-verify.ts

H1 bans drizzle-orm and @batu/database/schema in handlers. This file imports @batu/database (the client module) — not a literal violation, but the same class of infrastructure coupling. Consider passing db as a parameter or owning it in a query layer.

429 forward-declaration in auth.contract.ts needs declared-status guard verified

apps/platform/src/api/contracts/auth.contract.ts

Declaring 429 before the rate-limiter is wired is acceptable per canonical form. But if the handler never returns 429, declared-status.guard.test.ts may flag it as unreachable drift. Confirm the guard test is updated or suppressed.

D5 rule in check-fcis-boundaries.mjs correctly aligned with canonical form

scripts/check-fcis-boundaries.mjs

The script bans any @batu/database import except @batu/database/rls in shells, correctly implementing the canonical form's fifth sanctioned exception.

Timing-equalization logic in api-key-verify.ts has no dedicated test

apps/platform/src/api/handlers/public-v1/api-key-verify.ts

The security-critical timing-equalization path (burning a dummy scrypt verify on zero-candidate lookups) is only tested transitively through the OAuth route test, which mocks verifySecret to return a plain boolean. The property that verifySecret is called exactly once on the zero-candidate path is untested. The multi-candidate iteration is also only tested with a single-element mock.

DUMMY_HASH mock hoisting dependency is load-bearing but undocumented

apps/platform/src/app/api/v1/auth/oauth/token/__tests__/route.test.ts

The test mocks hashSecret: async () => 'scrypt:dummy:dummy' inside vi.mock. In api-key-verify.ts, DUMMY_HASH = hashSecret(...) is evaluated at module load. Vitest hoists vi.mock before imports so this works today, but the load-order dependency is unexplained. The sealing-key test explicitly documents its vi.resetModules() rationale; the same discipline is warranted here.

String-search boundary in after()-callback test is anchor-fragile

apps/platform/src/api/utils/__tests__/public-v1-telemetry.test.ts

The test slices source between 'after(async' and '// Not in a request scope'. If a second after(async block is added above the Tinybird one, start points at the wrong block and the assertion would miss any @batu/database import in the actual Tinybird callback. Fragility worth noting — works today given the file is short.

event-publisher.ts singleton has no test and no cache-reset seam

apps/platform/src/api/utils/event-publisher.ts

The module-level publisher singleton lacks a reset seam. Unlike sealing-key.ts which exports __resetSealingKeyCacheForTests(), this module will leak its cached instance across test suites if a future test exercises a handler that calls getImmediatePublisher(). Adding a __resetForTests export proactively matches the sealing-key precedent in this same diff.

OAuth route test does not assert updateLastUsedAt called on success

apps/platform/src/app/api/v1/auth/oauth/token/__tests__/route.test.ts

The success test does not verify that updateLastUsedAt was called with the matched key's internal id. Low-priority gap — the route mirrors auth/token which presumably has this assertion.

Dual-casing Code/code in describeSecretsError likely dead code for SDK v3

apps/platform/src/lib/secrets/sealing-key.ts

In AWS SDK v3, error codes surface as err.name not err.Code (SDK v2 PascalCase). The Code branch appears to be dead code and adds noise. If any SDK v2 compatibility path exists it should be documented; otherwise drop the Code arm.

admin-required security annotation buried inside compound comment

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

The admin-required comment is a CI-checked security gate. Mixing it mid-sentence with the fire-and-forget rationale makes it easy to miss by both CI grep and reviewers. It should be on its own line in the conventional '// admin-required: <reason>' form.

readParams silently defaults missing content-type to form-encoded

apps/platform/src/app/api/v1/auth/oauth/token/route.ts

A missing content-type header silently defaults to form-encoded parsing. If the body is actually JSON without a content-type header, readParams returns {} and produces a confusing downstream error. A comment explaining this is the intentional RFC default would reduce future confusion.

Timing-equalization security comment removed from point of decision

apps/platform/src/api/handlers/public-v1/api-key-verify.ts

The detailed timing-equalization rationale moved from the DUMMY_HASH declaration to the module docblock. A one-liner cross-reference at the DUMMY_HASH line ('// timing-equalization — see module docblock') would preserve in-place discoverability of the security intent.

as any cast in event-publisher.ts should use a narrower escape hatch

apps/platform/src/api/utils/event-publisher.ts

Casting db as any to satisfy createImmediatePublisher's loose config type discards all type safety. A cast to the specific expected type (or a TODO linking to a type-fix issue) is preferable to an open-ended as any.

Shared RATCHET_D5 string for 16 allowlist entries is the right call

scripts/check-fcis-boundaries.mjs

Using one canonical string documents the exit path uniformly, prevents per-entry rationale drift, and the shrink-only enforcement is the real guard. No change recommended.