feat/one-api

Length-based fallback misses exactly-32-char segments (UUID without dashes)

packages/api/src/middleware/internal-auth.ts:57

Condition is `seg.length > 32` (strictly greater). A UUID without dashes is exactly 32 chars and passes through unmasked. No current internal route uses bare 32-char IDs (all use prefixed ULIDs), so no immediate risk. Worth documenting the assumption or using `>= 26` (bare ULID length) to future-proof.

Relative URLs → 'unknown' endpoint — may hollow out migration signal

packages/api/src/middleware/internal-auth.ts:48

In Next.js App Router route handlers, req.url is commonly a relative path (e.g. '/api/internal/bills'). new URL('/api/internal/bills') throws — caught, returns 'unknown'. If the primary call sites pass relative URLs, the telemetry exists but is silent on endpoint shape — the migration-tracking purpose is defeated. Tests only exercise absolute URLs (https://x.test/...). Worth verifying how callers pass the request object, or extracting pathname differently for relative-path inputs (e.g. treating the input as a path if URL parsing fails).

Masking regex requires exactly 3-char lowercase prefix

packages/api/src/middleware/internal-auth.ts:57

^[a-z]{3}_ only catches 3-char prefixes. Future IDs with 2- or 4-char prefixes (if introduced) pass through unmasked. The >32-char length fallback partially mitigates this. No current exposure — all production IDs use 3-char prefixes — but worth documenting the assumption.

Log injection via control characters is properly mitigated

packages/api/src/middleware/internal-auth.ts

URL constructor percent-encodes control characters in the pathname; JSON.stringify further escapes them. A crafted URL like /api/v1/foo%0abar does NOT inject a newline into the log line. No vulnerability.

Tautological instanceof branch — both arms return req.headers

packages/api/src/middleware/internal-auth.ts:104

Pre-existing: `const headers = req instanceof Request ? req.headers : req.headers` — both branches are identical. This commit widened the union type (adding `url?: string`) which was the natural moment to clean it up. Both union members expose `.headers` directly; the check is dead. Safe to simplify to `const headers = req.headers;`.

JSDoc on endpointOf leads with WHAT rather than WHY

packages/api/src/middleware/internal-auth.ts:42

Project convention: comments explain hidden constraints and reasons, not mechanics. The first sentence describes the masking behavior (visible from the function body). The important WHY — 'this line is retained, shipped to third parties, and read by more people than the database is' — is buried. Invert the ratio.

Second telemetry test inlines full AuthContext instead of reusing authOf() helper

packages/api/src/middleware/__tests__/internal-auth.test.ts

The 'names the SERVICE token' test constructs AuthContext inline. The authOf('service') helper at line 148 exists for exactly this purpose and is used everywhere else in the file. The inline literal will drift silently if AuthContext gains new required fields.

No test for malformed non-empty URL string

packages/api/src/middleware/__tests__/internal-auth.test.ts

Test 6 covers url=undefined → 'unknown'. The endpointOf try/catch also handles a non-empty but unparseable string (e.g. 'not-a-url') → 'unknown'. This branch is correctly implemented but has no test exercising it.

No telemetry assertion for the both-valid-headers short-circuit case

packages/api/src/middleware/__tests__/internal-auth.test.ts

When both x-api-key and Authorization: Bearer are valid, the legacy path short-circuits and only 'legacy_static_key' is recorded. The short-circuit itself is tested in the dual-accept suite but the telemetry labeling for that case is not pinned — a future reorder of the two checks would mislabel the credential without a failing test.

emitted() helper throws if non-JSON output reaches console.log

packages/api/src/middleware/__tests__/internal-auth.test.ts

emitted() calls JSON.parse on every captured line. A future debug log added to withInternalAuth would cause the helper to throw rather than return an empty array. Low risk today since recordInternalAuth is the only console.log path, but a try/catch around JSON.parse would make the helper more resilient.

ts field in telemetry record is never asserted in tests

packages/api/src/middleware/__tests__/internal-auth.test.ts

Tests use toMatchObject which permits extra keys — the ts field presence is never checked. Adding expect(emitted()[0]).toHaveProperty('ts') to test 1 would confirm the timestamp is present.

ok handler stub defined twice in test file

packages/api/src/middleware/__tests__/internal-auth.test.ts

`const ok = async () => ({ status: 200 as const, body: 'ok' })` is defined identically in two separate describe blocks. A single module-level const would serve both.

endpointOf try/catch could be collapsed to remove the let declaration

packages/api/src/middleware/internal-auth.ts

Minor readability: try { const { pathname } = new URL(req.url); return pathname.split('/').map(...).join('/'); } catch { return 'unknown'; } eliminates the intermediate let and makes both exit points co-located.

endpointOf is NOT duplicated with public-v1-telemetry.ts

packages/api/src/middleware/internal-auth.ts

The public mount uses the ts-rest route KEY from contract metadata — never the raw URL. endpointOf exists specifically because the internal mount has no metadata registry. The two approaches are genuinely different; no extraction is warranted.