Compare commits

...

1 Commits

Author SHA1 Message Date
Diego Rodrigues de Sa e Souza
c3945a724c fix(ci): security-tier gate must honor ALWAYS_PROTECTED_API_PATTERNS too (+ file-size rebaseline) (#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 (#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

* fix(ci): security-tier gate must honor ALWAYS_PROTECTED_API_PATTERNS too

#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

* 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
#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

* 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 #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

* docs(ci): changelog fragment for #12605

* chore(ci): trim #12605 to the one fix the base still needs

The base drained fast while this PR was open. Re-verified on 008da6d19a 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
2026-09-04 04:07:43 -03:00
2 changed files with 19 additions and 9 deletions

View File

@@ -0,0 +1 @@
- **CI:** the OpenAPI security-tier gate now mirrors `isAlwaysProtectedPath()` in full — it also reads `ALWAYS_PROTECTED_API_PATTERNS`, so the pattern-gated credential routes (`/api/providers/{id}/{claude,codex}-auth/{export,apply-local}`, GHSA-5926-2w35-7h4q) no longer report as unannotated. (#12605)

View File

@@ -106,6 +106,11 @@ function parsePatterns(name) {
const LOCAL_ONLY_PREFIXES = parsePrefixes("LOCAL_ONLY_API_PREFIXES");
const LOCAL_ONLY_PATTERNS = parsePatterns("LOCAL_ONLY_API_PATTERNS");
const ALWAYS_PROTECTED_PATHS = parsePrefixes("ALWAYS_PROTECTED_API_PATHS");
// isAlwaysProtectedPath() is ALSO two-armed (paths || patterns) — reading only the
// path array repeated, on this half, the very bug #12350 fixed on the LOCAL_ONLY
// half: the pattern-gated credential routes (…/{claude,codex}-auth/{export,
// apply-local}, #12600) read as unannotated even though they are protected.
const ALWAYS_PROTECTED_PATTERNS = parsePatterns("ALWAYS_PROTECTED_API_PATTERNS");
if (
LOCAL_ONLY_PREFIXES.length === 0 ||
@@ -135,6 +140,15 @@ function coveredByLocalOnly(pathStr) {
return matchesPrefix(concrete) || LOCAL_ONLY_PATTERNS.some((re) => re.test(concrete));
}
/** Mirror of routeGuard.isAlwaysProtectedPath() — both arms, same order. */
function coveredByAlwaysProtected(pathStr) {
const concrete = concretize(pathStr);
return (
ALWAYS_PROTECTED_PATHS.some((p) => concrete === p || concrete.startsWith(`${p}/`)) ||
ALWAYS_PROTECTED_PATTERNS.some((re) => re.test(concrete))
);
}
const raw = yaml.load(fs.readFileSync(OPENAPI_PATH, "utf-8"));
const paths = raw.paths || {};
const errors = [];
@@ -151,16 +165,11 @@ for (const [pathStr, methods] of Object.entries(paths)) {
);
}
if (spec["x-always-protected"] === true) {
const matchesPath = ALWAYS_PROTECTED_PATHS.some(
(p) => pathStr === p || pathStr.startsWith(`${p}/`)
if (spec["x-always-protected"] === true && !coveredByAlwaysProtected(pathStr)) {
errors.push(
`${method.toUpperCase()} ${pathStr}: has x-always-protected but is NOT covered by ` +
`ALWAYS_PROTECTED_API_PATHS or ALWAYS_PROTECTED_API_PATTERNS`
);
if (!matchesPath) {
errors.push(
`${method.toUpperCase()} ${pathStr}: has x-always-protected but is NOT in ` +
`ALWAYS_PROTECTED_API_PATHS [${ALWAYS_PROTECTED_PATHS.join(", ")}]`
);
}
}
}
}