fix(api): reject revoked, deactivated, banned or expired keys on DELETE /v1/batches/delete-completed (omni-code-sec on #12969)

[diegosouzapw]

Summary

Second pass of the review batteries on #12969, this time /omni-code-sec (4 security lenses: built-in security-review, claude-security, ToB differential-review, ToB insecure-defaults). Closes the two findings the owner approved for an immediate fix; the third one is an auth-architecture decision tracked separately.

• SEC-B (high, CWE-613, 3 reviewers) — the fail-closed gate added in fix(api): explicit scope, audit log and atomic sweep for DELETE /v1/batches/delete-completed (omni-code-review battery on #12969) #13262 authorized a presented key by row existence (getApiKeyMetadata), not validity: a revoked, deactivated, banned or expired key still resolved an apiKeyId and ran the sweep. The route now also requires validateApiKey() (the one lifecycle gate: is_active, revoked_at, is_banned, expires_at) before choosing a scope. Neither an unresolved nor an invalid key falls through to the session branch.
• SEC-E (nit, Hard Rule fix(ui): fix Select dropdown dark theme inconsistency #12) — both 401 bodies now go through buildErrorBody(401, …) (type: authentication_error, code: invalid_api_key) instead of a hand-built object.
• SEC-F (low) — 4 negative route tests: revoked, deactivated, banned, expired (the last one alongside a session cookie, proving the session branch is never reached); the unauthenticated case now asserts the buildErrorBody shape.

Not in this PR (owner decisions):

• SEC-A (high, CWE-287): a dashboard session is any JWT signed with JWT_SECRET, and the public Cursor CLI key-exchange endpoint mints JWTs with the same secret → forgeable auth_token → instance-wide sweep. Auth architecture, tracked in security(auth): any JWT signed with JWT_SECRET is a dashboard session — Cursor CLI key-exchange tokens can forge auth_token #13298.
• SEC-C (medium): the file half of the sweep is not scoped by owner — deferred by the owner in the previous round, still open.
• SEC-D (medium): single write-lock for a large instance-wide sweep — follow-up (chunking).

Validation

node --import tsx/esm --test tests/unit/batch-deletion.test.ts \
  tests/unit/batches-delete-completed-ownership-wvxc.test.ts \
  tests/unit/batches-delete-completed-route-scope.test.ts tests/unit/api-keys*.test.ts
# pass 32 / fail 0  (route-scope: 5 red before the fix → 10 green after)
npm run typecheck:core                      # OK
npx eslint <changed files>                  # 0 errors
CHANGELOG_BASE_REF=origin/fix/batches-delete-completed-authz npm run check:changelog-integrity  # OK

Full suite on the .113 box: see the last comment.

⚠️ base-red inherited: #12732 (ESLint ×3, README migration count) — none touched here.

Refs #12969 · review record: _tasks/reviews/omni-code-sec/2026-09-10_fix-batches-delete-completed-authz_vs_release-v3.8.51/

[diegosouzapw]

Full suite on the .113 box (b6d2f0f, own worktree, npm ci)

suite	result
test:unit	37 731 pass / 43 fail / 27 skipped — 0 failures in batches or api-keys; the 4 new negative tests pass in the full run
test:vitest	51 files / 465 tests passed
build	standalone OK

The 43 unit failures: 36 are the same inherited files recorded on #13262; image-generation-handler ×3 fails identically on the base 150ca009 in isolation; the remaining 7 (models-catalog-combo-metadata, specialty-model-catalog-routes, sanitizers.property, vscode-token-routes, conversationTracker-reconnect-7847, quota-weighted-strategy) only failed while vitest and build ran concurrently (load average 77) and pass on both the fix and the base with the box idle. No failure is introduced by this diff.

Focused: batches-delete-completed-route-scope 10/10 · batches-delete-completed-ownership-wvxc 8/8 · batch-deletion 8/8.