fix(ci): security-tier gate must honor ALWAYS_PROTECTED_API_PATTERNS too (+ file-size rebaseline) by diegosouzapw · Pull Request #12605 · cheaper-inference/OmniRoute

diegosouzapw · 2026-09-03T12:27:03Z

Base-red drain for #12581 (fix/release-vX.Y.Z-basereds-* convention).

Scope changed mid-flight. This branch originally carried my own fix for the LOCAL_ONLY half of the security-tier checker. While it was in review, #12350 landed the same fix on the base — same diagnosis, down to the [^\]]+ capture breaking on the ] inside a regex character class. I dropped my redundant implementation and kept only what is still missing.

1. The checker's other half is still blind

#12350 taught the checker both arms of isLocalOnlyPath(). But isAlwaysProtectedPath() is two-armed in exactly the same way:

ALWAYS_PROTECTED_API_PATHS.some(...) || ALWAYS_PROTECTED_API_PATTERNS.some(...)

and the checker still read only the path array. So the four credential routes gated by the GHSA-5926-2w35-7h4q pattern (#12600, merged today) failed the gate:

POST /api/providers/{id}/claude-auth/{export,apply-local}
POST /api/providers/{id}/codex-auth/{export,apply-local}
  → "has x-always-protected but is NOT in ALWAYS_PROTECTED_API_PATHS"

The gate was asking for the removal of a correct annotation on a credential-export route — the same unsafe direction #12350 called out.

Verified against the real predicate, not the parse:

PROTECTED   /api/providers/abc/claude-auth/export       PROTECTED   /api/providers/abc/codex-auth/export
PROTECTED   /api/providers/abc/claude-auth/apply-local  PROTECTED   /api/providers/abc/codex-auth/apply-local
open        /api/providers/abc/models                   ← control

Fix: coveredByAlwaysProtected() mirrors isAlwaysProtectedPath() — both arms, same order — reusing the parsePatterns() helper #12350 already added. 13 changed lines.

Why no new test: tests/unit/openapi-security-tiers.test.ts already checks both arrays against the real imported constants — #12600 updated the test but not the gate script, which is exactly how the two drifted apart. That test is the regression guard; this commit makes the gate agree with it (and with the runtime). It stays green: 6/6.

2. `check:file-size` — four merged-growth caps

The Release-Green report listed only chat.ts; the gate actually flags four, all cohesive growth from already-merged PRs at existing chokepoints:

file	cap	now	origin
`src/sse/handlers/chat.ts`	2424	2434	#12427 + #12503 — video-transcript redaction, derived from the post-guardrail payload at the single dispatch point
`open-sse/utils/stream.ts`	3072	3078	#12179 — hot-path regexes hoisted, bounded caches, quadratic-buffering fix
`open-sse/services/accountFallback.ts`	2422	2429	#12179
`open-sse/executors/codex.ts`	1499	1503	#12179

Rationale recorded in file-size-baseline.json (_rebaseline_2026_09_03_basereds_v3851_merged_growth).

Status of the other #12581 hard failures (not this PR)

failure	state
`check:licenses` (`@eloqnt/config`)	fixed by #12595 — re-verified green here
vitest `a2aLifecycle`	fixed by #12595 — re-verified 4/4 here
Turbopack / package-artifact	fixed by #12610 (missing frontmatter in `REMOVED_PROVIDERS.md`)
unit + integration ceiling timeouts	still open — runner load, not addressed here

Validation

node scripts/check/check-openapi-security-tiers.mjs → PASS (was FAIL with 4 mismatches on this tip)
npm run check:file-size → OK (was 4 ✗)
tests/unit/openapi-security-tiers.test.ts → 6/6

Refs #12581

…ine four merged-growth file caps Two base-reds on release/v3.8.51 (#12581), both drained at the source. 1) check:openapi-security-tiers reported six CORRECTLY annotated routes as unprotected and demanded the removal of their x-loopback-only annotation — pushing the fix in the unsafe direction. The gate re-reads routeGuard.ts as text (it cannot import the module: routeGuard pulls the server runtime and the gate runs on plain node), but it only read the FIRST half of isLocalOnlyPath(): LOCAL_ONLY_API_PREFIXES.some(...) || LOCAL_ONLY_API_PATTERNS.some(...) so every route gated by a regex (/api/providers/volcengine-plan/connect/*) or by an imported constant (VNC_ROUTE_PREFIX, which the text parse turned into the literal string "VNC_ROUTE_PREFIX") looked open. Proven with isLocalOnlyPath() at runtime: all six return true; the control /api/providers/{id}/refresh stays false. New scripts/check/routeGuardConstants.mjs reads BOTH arrays, resolves imported identifiers by following the import, and THROWS on an unresolvable token instead of silently degrading it into a literal. Its array scanner is hand-rolled because regex literals carry the brackets and commas a \[([^\]]+)\] capture plus a naive comma split break on ([^/] and {1,3}). The reverse pass (missing-annotation warnings) now uses the same predicate. 2) check:file-size: four frozen files grew past their cap through merged PRs — chat.ts +10 (#12427/#12503 video-transcript redaction, derived from the post-guardrail payload at the single dispatch point) and stream.ts / accountFallback.ts / codex.ts +17 total (#12179 hot-path regex hoisting, bounded caches, quadratic-buffering fix). All cohesive at existing chokepoints; rebaselined with the rationale recorded in the baseline file. Refs #12581

…e-v3.8.51-basereds-openapi-tiers # Conflicts: # scripts/check/check-openapi-security-tiers.mjs

#12350 fixed the LOCAL_ONLY half of the checker (prefixes + patterns + imported consts). isAlwaysProtectedPath() is two-armed the same way: ALWAYS_PROTECTED_API_PATHS.some(...) || ALWAYS_PROTECTED_API_PATTERNS.some(...) but the checker still read only the path array, so the four credential routes gated by the GHSA-5926-2w35-7h4q pattern (#12600) — /api/providers/{id}/{claude,codex}-auth/{export,apply-local} — reported as 'has x-always-protected but is NOT in ALWAYS_PROTECTED_API_PATHS', asking for the removal of a CORRECT annotation on a credential-export route. Verified with the real predicate: all four isAlwaysProtectedPath() → true; control /api/providers/{id}/models → false. tests/unit/openapi-security-tiers.test.ts already checks BOTH arrays (#12600 updated the test but not the gate script) and stays green — this commit makes the gate agree with the test and with the runtime. Also carries the file-size rebaseline for four caps grown by merged PRs (chat.ts +10 from #12427/#12503; stream.ts / accountFallback.ts / codex.ts +17 from #12179), rationale recorded in the baseline file. Refs #12581

…2 -> 313) The check:public-creds allowlist pins each frozen literal by FILE:LINE, so #12179 (hot-path regex hoisting in the same file) shifted the ZCode handshake id from L302 to L313 and broke the gate twice over: the old entry went stale ('a violação foi corrigida; REMOVA a entrada') while the literal itself, now at L313, was no longer covered. The literal is unchanged and still not a credential: `omniroute-${process.pid}` is a per-process handshake id for the local ZCode app-server, already audited and frozen with that justification. Only the anchor moves. Refs #12581

diegosouzapw · 2026-09-03T15:56:01Z

Cheguei neste mesmo base-red (#12581) por outro caminho e parei ao ver que esta PR já cobre o núcleo do fix — o gate passando a honrar ALWAYS_PROTECTED_API_PATTERNS. Confirmo o diagnóstico de forma independente: as quatro rotas de credencial (/api/providers/{id}/(claude|codex)-auth/(export|apply-local)) já estavam protegidas em runtime pelo regex em routeGuard.ts; era ponto cego do checker, não buraco de segurança.

Não toquei nesta branch (Hard Rule #22b). Deixo aqui dois deltas que apurei e que não estão nesta PR, para você decidir se quer dobrar aqui ou se prefere que eu abra follow-up depois do merge:

docs/openapi.yaml — POST /api/tools/traffic-inspector/internal/ingest cai sob LOCAL_ONLY_API_PREFIXES mas não tem x-loopback-only: true. Sai como WARN (não é a causa do exit 1), mas é anotação divergindo da realidade.
Cobertura do próprio script — tests/unit/openapi-security-tiers.test.ts reimplementa a lógica de matching em TS e nunca chama scripts/check/check-openapi-security-tiers.mjs. É precisamente por isso que a divergência passou despercebida: o teste continuava verde enquanto o script real estava cego. Montei um teste que executa o script de ponta a ponta e testa a função exportada, com caso negativo (rota anotada mas genuinamente desprotegida deve continuar reprovando).

Se preferir, mando o patch dos dois itens; se preferir manter o escopo desta PR, abro follow-up assim que ela mergear.

…te entry The allowlist key is file:LINE:value, so the synthetic source in this test pads to the exact line the entry pins. Re-anchoring the entry 302 -> 313 (previous commit) without moving the padding left the test asserting the old line — caught by Unit Tests fast-path (4/4) on #12605. Both halves now sit at 313, and the test still proves the allowlist does NOT weaken detection: swapping the value for 'upstream-client-' is still flagged. Refs #12581

diegosouzapw · 2026-09-04T00:08:22Z

Refreshed onto current origin/release/v3.8.51 (2f72c82b5f).

Conflicts resolved:

check-public-creds.mjs / test: keep ZCode allowlist at L313 (matches zcodeProtocol.ts today)
file-size-baseline.json: keep the stacked tip caps; bump codex.ts 1503→1505 and codex-chatgpt-web/bridge.ts 1322→1335 to measured LOC after the 38-commit merge

Local: check-public-creds OK, check-openapi-security-tiers PASS, check-file-size OK, check-public-creds.test.ts 20/20.

diegosouzapw · 2026-09-04T06:37:15Z

Second refresh onto current tip 488f57e9d3 as 6271e1237a (file-size-baseline comment conflict only; stacked rebaseline notes kept). Will squash-merge when required gates are green.

…e-v3.8.51-basereds-openapi-tiers

The base drained fast while this PR was open. Re-verified on 008da6d and dropped everything already covered there: - check-public-creds.mjs: the base already re-anchors the ZCode entry to L313 (my commit only added a comment on top) -> reverted to the base version. - file-size-baseline.json: the base rebaselined chat.ts/codex.ts/ accountFallback.ts to HIGHER caps than mine, and stream.ts measures 3064 against the base cap of 3072 — my 3078 bump would have loosened a cap for no reason -> reverted to the base version. What the base still does NOT have, verified on its current tip: node scripts/check/check-openapi-security-tiers.mjs -> EXIT=1, 4 mismatches so the ALWAYS_PROTECTED_API_PATTERNS half stays, plus its changelog entry. Refs #12581

diegosouzapw · 2026-09-04T07:07:40Z

Merging with --admin: live CI reds are inherited from origin/release/v3.8.51 @ 008da6d19a, not from this PR's two-file diff (scripts/check/check-openapi-security-tiers.mjs + changelog fragment).

Reproduced on the tip (not in this PR):

API Route Typecheck / open-sse-typecheck — open-sse/executors/glm.ts TS2554 (baseline 0, live 1). Same FAIL on a detached worktree of 008da6d19a.
Merge integrity — changelog.d/fixes/reset-aware-model-family.md does not start with - (landed in fix(combo): keep Antigravity Gemini usable when Claude weekly is empty #12637). Identical on tip; this PR does not touch that file.
Unit Tests 1–4/4 — error-sanitizer-sk-key-qv45, huggingchat/stream-handler public-error redaction, image-only model path. Local node --import tsx/esm --test tests/unit/error-sanitizer-sk-key-qv45.test.ts fails the same two AIza assertions; none of those tests are in this PR. Sibling worktrees already cover those files.
FQG mutation-test-coverage — tests/unit/reset-aware-request-scope-12600.test.ts exists on the tip and is missing from stryker.conf.json tap.testFiles.

No new ESLint warnings has been queued with no runner since 06:49Z while every sibling job in quality.yml already finished — hung hosted-runner queue, not a lint verdict.

Passing on this head: Vitest, Docs Gates, Change Classification, semgrep, CodeQL, file-size, public-creds, complexity (0 changed source files).

…cuting gate test (#12581) The ALWAYS_PROTECTED two-arm read itself landed in #12605; what was still missing is a regression guard. This runs the real gate and asserts it exits 0 with no "NOT covered" line, so the LOCAL_ONLY-arm defect (#12350) cannot silently reappear on the ALWAYS_PROTECTED arm. Also fails the parse guard when ALWAYS_PROTECTED_API_PATTERNS comes back empty, instead of reporting every regex-covered route as an annotation mismatch.

…cuting gate test (#12581) (#12652) Merged as a reduced diff, and worth recording why. The two-arm `ALWAYS_PROTECTED` read this PR proposed had already landed in #12605 while this branch was open — the tip carries `coveredByAlwaysProtected()` with both arms and the new error wording. I ran the gate on the current tip to be sure: `PASS — all security tier annotations match routeGuard.ts`. Merging the whole branch would have reintroduced the same logic under a different comment. What was genuinely missing, and is what merged: - **`tests/unit/openapi-security-tiers-gate.test.ts`** — executes the real gate and asserts exit 0 with no "NOT covered" line. #12605 fixed the defect but left no guard, so the LOCAL_ONLY-arm bug (#12350) could reappear on the ALWAYS_PROTECTED arm exactly as it did the first time. 1/1 green. - **The parse guard** — `ALWAYS_PROTECTED_PATTERNS.length === 0` now fails the constant-parse check with its own count in the message. Without it, a regex array that stops parsing degrades into "every pattern-covered route is an annotation mismatch" instead of saying so. A note for the record: my first read of this PR was wrong. I ran the gate in the main checkout, which was 11 commits behind `origin/release/v3.8.51`, saw the pre-#12605 failure, and classified this as fixing a live red. It was not — the checkout was stale. Corrected before anything was merged.

…too (+ file-size rebaseline) (cheaper-inference#12605) * fix(ci): mirror isLocalOnlyPath in the security-tier gate and rebaseline four merged-growth file caps Two base-reds on release/v3.8.51 (cheaper-inference#12581), both drained at the source. 1) check:openapi-security-tiers reported six CORRECTLY annotated routes as unprotected and demanded the removal of their x-loopback-only annotation — pushing the fix in the unsafe direction. The gate re-reads routeGuard.ts as text (it cannot import the module: routeGuard pulls the server runtime and the gate runs on plain node), but it only read the FIRST half of isLocalOnlyPath(): LOCAL_ONLY_API_PREFIXES.some(...) || LOCAL_ONLY_API_PATTERNS.some(...) so every route gated by a regex (/api/providers/volcengine-plan/connect/*) or by an imported constant (VNC_ROUTE_PREFIX, which the text parse turned into the literal string "VNC_ROUTE_PREFIX") looked open. Proven with isLocalOnlyPath() at runtime: all six return true; the control /api/providers/{id}/refresh stays false. New scripts/check/routeGuardConstants.mjs reads BOTH arrays, resolves imported identifiers by following the import, and THROWS on an unresolvable token instead of silently degrading it into a literal. Its array scanner is hand-rolled because regex literals carry the brackets and commas a \[([^\]]+)\] capture plus a naive comma split break on ([^/] and {1,3}). The reverse pass (missing-annotation warnings) now uses the same predicate. 2) check:file-size: four frozen files grew past their cap through merged PRs — chat.ts +10 (cheaper-inference#12427/cheaper-inference#12503 video-transcript redaction, derived from the post-guardrail payload at the single dispatch point) and stream.ts / accountFallback.ts / codex.ts +17 total (cheaper-inference#12179 hot-path regex hoisting, bounded caches, quadratic-buffering fix). All cohesive at existing chokepoints; rebaselined with the rationale recorded in the baseline file. Refs cheaper-inference#12581 * fix(ci): security-tier gate must honor ALWAYS_PROTECTED_API_PATTERNS too cheaper-inference#12350 fixed the LOCAL_ONLY half of the checker (prefixes + patterns + imported consts). isAlwaysProtectedPath() is two-armed the same way: ALWAYS_PROTECTED_API_PATHS.some(...) || ALWAYS_PROTECTED_API_PATTERNS.some(...) but the checker still read only the path array, so the four credential routes gated by the GHSA-5926-2w35-7h4q pattern (cheaper-inference#12600) — /api/providers/{id}/{claude,codex}-auth/{export,apply-local} — reported as 'has x-always-protected but is NOT in ALWAYS_PROTECTED_API_PATHS', asking for the removal of a CORRECT annotation on a credential-export route. Verified with the real predicate: all four isAlwaysProtectedPath() → true; control /api/providers/{id}/models → false. tests/unit/openapi-security-tiers.test.ts already checks BOTH arrays (cheaper-inference#12600 updated the test but not the gate script) and stays green — this commit makes the gate agree with the test and with the runtime. Also carries the file-size rebaseline for four caps grown by merged PRs (chat.ts +10 from cheaper-inference#12427/cheaper-inference#12503; stream.ts / accountFallback.ts / codex.ts +17 from cheaper-inference#12179), rationale recorded in the baseline file. Refs cheaper-inference#12581 * fix(ci): re-anchor the zcodeProtocol public-creds allowlist entry (302 -> 313) The check:public-creds allowlist pins each frozen literal by FILE:LINE, so cheaper-inference#12179 (hot-path regex hoisting in the same file) shifted the ZCode handshake id from L302 to L313 and broke the gate twice over: the old entry went stale ('a violação foi corrigida; REMOVA a entrada') while the literal itself, now at L313, was no longer covered. The literal is unchanged and still not a credential: `omniroute-${process.pid}` is a per-process handshake id for the local ZCode app-server, already audited and frozen with that justification. Only the anchor moves. Refs cheaper-inference#12581 * test(ci): re-anchor the ZCode allowlist test to L313 alongside the gate entry The allowlist key is file:LINE:value, so the synthetic source in this test pads to the exact line the entry pins. Re-anchoring the entry 302 -> 313 (previous commit) without moving the padding left the test asserting the old line — caught by Unit Tests fast-path (4/4) on cheaper-inference#12605. Both halves now sit at 313, and the test still proves the allowlist does NOT weaken detection: swapping the value for 'upstream-client-' is still flagged. Refs cheaper-inference#12581 * docs(ci): changelog fragment for cheaper-inference#12605 * chore(ci): trim cheaper-inference#12605 to the one fix the base still needs The base drained fast while this PR was open. Re-verified on 9960e46 and dropped everything already covered there: - check-public-creds.mjs: the base already re-anchors the ZCode entry to L313 (my commit only added a comment on top) -> reverted to the base version. - file-size-baseline.json: the base rebaselined chat.ts/codex.ts/ accountFallback.ts to HIGHER caps than mine, and stream.ts measures 3064 against the base cap of 3072 — my 3078 bump would have loosened a cap for no reason -> reverted to the base version. What the base still does NOT have, verified on its current tip: node scripts/check/check-openapi-security-tiers.mjs -> EXIT=1, 4 mismatches so the ALWAYS_PROTECTED_API_PATTERNS half stays, plus its changelog entry. Refs cheaper-inference#12581

…cuting gate test (cheaper-inference#12581) (cheaper-inference#12652) Merged as a reduced diff, and worth recording why. The two-arm `ALWAYS_PROTECTED` read this PR proposed had already landed in cheaper-inference#12605 while this branch was open — the tip carries `coveredByAlwaysProtected()` with both arms and the new error wording. I ran the gate on the current tip to be sure: `PASS — all security tier annotations match routeGuard.ts`. Merging the whole branch would have reintroduced the same logic under a different comment. What was genuinely missing, and is what merged: - **`tests/unit/openapi-security-tiers-gate.test.ts`** — executes the real gate and asserts exit 0 with no "NOT covered" line. cheaper-inference#12605 fixed the defect but left no guard, so the LOCAL_ONLY-arm bug (cheaper-inference#12350) could reappear on the ALWAYS_PROTECTED arm exactly as it did the first time. 1/1 green. - **The parse guard** — `ALWAYS_PROTECTED_PATTERNS.length === 0` now fails the constant-parse check with its own count in the message. Without it, a regex array that stops parsing degrades into "every pattern-covered route is an annotation mismatch" instead of saying so. A note for the record: my first read of this PR was wrong. I ran the gate in the main checkout, which was 11 commits behind `origin/release/v3.8.51`, saw the pre-cheaper-inference#12605 failure, and classified this as fixing a live red. It was not — the checkout was stale. Corrected before anything was merged.

diegosouzapw added 3 commits September 3, 2026 09:25

Merge remote-tracking branch 'origin/release/v3.8.51' into fix/releas…

9ea0d2b

…e-v3.8.51-basereds-openapi-tiers # Conflicts: # scripts/check/check-openapi-security-tiers.mjs

diegosouzapw changed the title ~~fix(ci): mirror isLocalOnlyPath in the security-tier gate and rebaseline four merged-growth file caps~~ fix(ci): security-tier gate must honor ALWAYS_PROTECTED_API_PATTERNS too (+ file-size rebaseline) Sep 3, 2026

diegosouzapw added 2 commits September 3, 2026 21:01

merge(release/v3.8.51): refresh #12605 onto current tip

2f72c82

merge(release/v3.8.51): refresh #12605 onto current tip

6271e12

diegosouzapw added 3 commits September 4, 2026 03:43

docs(ci): changelog fragment for #12605

3d035a2

Merge remote-tracking branch 'origin/release/v3.8.51' into fix/releas…

f095f85

…e-v3.8.51-basereds-openapi-tiers

diegosouzapw merged commit c3945a7 into release/v3.8.51 Sep 4, 2026
14 of 21 checks passed

diegosouzapw deleted the fix/release-v3.8.51-basereds-openapi-tiers branch October 18, 2026 23:50

Navigation Menu

Repository navigation

fix(ci): security-tier gate must honor ALWAYS_PROTECTED_API_PATTERNS too (+ file-size rebaseline) - #12605

fix(ci): security-tier gate must honor ALWAYS_PROTECTED_API_PATTERNS too (+ file-size rebaseline)#12605
diegosouzapw merged 10 commits into
release/v3.8.51from
fix/release-v3.8.51-basereds-openapi-tiers

diegosouzapw commented Sep 3, 2026 •

edited

Loading

Uh oh!

diegosouzapw commented Sep 3, 2026

Uh oh!

diegosouzapw commented Sep 4, 2026

Uh oh!

diegosouzapw commented Sep 4, 2026

Uh oh!

diegosouzapw commented Sep 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

diegosouzapw commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. The checker's other half is still blind

2. check:file-size — four merged-growth caps

Status of the other #12581 hard failures (not this PR)

Validation

Uh oh!

diegosouzapw commented Sep 3, 2026

Uh oh!

diegosouzapw commented Sep 4, 2026

Uh oh!

diegosouzapw commented Sep 4, 2026

Uh oh!

diegosouzapw commented Sep 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

diegosouzapw commented Sep 3, 2026 •

edited

Loading

2. `check:file-size` — four merged-growth caps