Skip to content

Commit c3945a7

Browse files
authored
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 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
1 parent 008da6d commit c3945a7

2 files changed

Lines changed: 19 additions & 9 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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)

scripts/check/check-openapi-security-tiers.mjs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,11 @@ function parsePatterns(name) {
106106
const LOCAL_ONLY_PREFIXES = parsePrefixes("LOCAL_ONLY_API_PREFIXES");
107107
const LOCAL_ONLY_PATTERNS = parsePatterns("LOCAL_ONLY_API_PATTERNS");
108108
const ALWAYS_PROTECTED_PATHS = parsePrefixes("ALWAYS_PROTECTED_API_PATHS");
109+
// isAlwaysProtectedPath() is ALSO two-armed (paths || patterns) — reading only the
110+
// path array repeated, on this half, the very bug #12350 fixed on the LOCAL_ONLY
111+
// half: the pattern-gated credential routes (…/{claude,codex}-auth/{export,
112+
// apply-local}, #12600) read as unannotated even though they are protected.
113+
const ALWAYS_PROTECTED_PATTERNS = parsePatterns("ALWAYS_PROTECTED_API_PATTERNS");
109114

110115
if (
111116
LOCAL_ONLY_PREFIXES.length === 0 ||
@@ -135,6 +140,15 @@ function coveredByLocalOnly(pathStr) {
135140
return matchesPrefix(concrete) || LOCAL_ONLY_PATTERNS.some((re) => re.test(concrete));
136141
}
137142

143+
/** Mirror of routeGuard.isAlwaysProtectedPath() — both arms, same order. */
144+
function coveredByAlwaysProtected(pathStr) {
145+
const concrete = concretize(pathStr);
146+
return (
147+
ALWAYS_PROTECTED_PATHS.some((p) => concrete === p || concrete.startsWith(`${p}/`)) ||
148+
ALWAYS_PROTECTED_PATTERNS.some((re) => re.test(concrete))
149+
);
150+
}
151+
138152
const raw = yaml.load(fs.readFileSync(OPENAPI_PATH, "utf-8"));
139153
const paths = raw.paths || {};
140154
const errors = [];
@@ -151,16 +165,11 @@ for (const [pathStr, methods] of Object.entries(paths)) {
151165
);
152166
}
153167

154-
if (spec["x-always-protected"] === true) {
155-
const matchesPath = ALWAYS_PROTECTED_PATHS.some(
156-
(p) => pathStr === p || pathStr.startsWith(`${p}/`)
168+
if (spec["x-always-protected"] === true && !coveredByAlwaysProtected(pathStr)) {
169+
errors.push(
170+
`${method.toUpperCase()} ${pathStr}: has x-always-protected but is NOT covered by ` +
171+
`ALWAYS_PROTECTED_API_PATHS or ALWAYS_PROTECTED_API_PATTERNS`
157172
);
158-
if (!matchesPath) {
159-
errors.push(
160-
`${method.toUpperCase()} ${pathStr}: has x-always-protected but is NOT in ` +
161-
`ALWAYS_PROTECTED_API_PATHS [${ALWAYS_PROTECTED_PATHS.join(", ")}]`
162-
);
163-
}
164173
}
165174
}
166175
}

0 commit comments

Comments
 (0)