diff --git a/scripts/check/check-openapi-security-tiers.mjs b/scripts/check/check-openapi-security-tiers.mjs index 812e4a0c1b..89d17c0482 100644 --- a/scripts/check/check-openapi-security-tiers.mjs +++ b/scripts/check/check-openapi-security-tiers.mjs @@ -1,9 +1,23 @@ #!/usr/bin/env node /** * Cross-references openapi.yaml x-loopback-only / x-always-protected annotations - * against the compile-time constants in src/server/authz/routeGuard.ts. + * against the compile-time route-classification constants in + * src/server/authz/routeGuard.ts. * - * Fails if any YAML annotation disagrees with the routeGuard.ts constants. + * routeGuard classifies a loopback-only route through TWO mechanisms, and this + * checker must honor BOTH or it reports false positives (regression #12335): + * + * 1. LOCAL_ONLY_API_PREFIXES — flat string prefixes. One entry + * (VNC_ROUTE_PREFIX) is an imported const rather than a string literal, so + * it is resolved from its source module. + * 2. LOCAL_ONLY_API_PATTERNS — RegExp entries for spawn-capable routes whose + * dynamic path parameter sits BEFORE the gated segment (e.g. + * /api/providers/{id}/login), which a flat prefix cannot target without + * over-broadening the whole /api/providers/ subtree. + * + * A route is "covered" iff it matches a resolved prefix OR a pattern — exactly + * the `isLocalOnlyPath()` runtime contract. Fails if any YAML annotation + * disagrees with the routeGuard.ts constants. */ import fs from "node:fs"; @@ -13,34 +27,116 @@ import * as yaml from "js-yaml"; const ROOT = process.cwd(); const OPENAPI_PATH = path.join(ROOT, "docs", "openapi.yaml"); const ROUTE_GUARD_PATH = path.join(ROOT, "src", "server", "authz", "routeGuard.ts"); +const guardSrc = fs.readFileSync(ROUTE_GUARD_PATH, "utf-8"); -function parseStringArray(match) { - if (!match) return []; - // Strip line comments before splitting — array entries in routeGuard.ts often - // carry inline `// T-XX:` annotations that would otherwise pollute the parsed tokens. - return match[1] - .replace(/\/\/[^\n]*/g, "") - .split(",") - .map((s) => s.trim().replace(/^["']|["']$/g, "")) - .filter(Boolean); +// Capture an exported array's body up to its closing `\n];`. Unlike a `[^\]]+` +// capture, this is immune to `]` characters inside comments or regex character +// classes (e.g. `[^/]`) — the exact footgun documented at routeGuard.ts's +// /api/oauth/cursor/auto-import entry, and the reason regex patterns could not +// be parsed at all before. +function extractArrayBody(name) { + const m = guardSrc.match( + new RegExp(`export const ${name}\\b[\\s\\S]*?=\\s*\\[([\\s\\S]*?)\\n\\];`) + ); + return m ? m[1] : null; } -const guardSrc = fs.readFileSync(ROUTE_GUARD_PATH, "utf-8"); -const LOCAL_ONLY_PREFIXES = parseStringArray( - guardSrc.match(/export const LOCAL_ONLY_API_PREFIXES.*?=\s*\[([^\]]+)\]/s) -); -const ALWAYS_PROTECTED_PATHS = parseStringArray( - guardSrc.match(/export const ALWAYS_PROTECTED_API_PATHS.*?=\s*\[([^\]]+)\]/s) -); +const stripLineComments = (s) => s.replace(/\/\/[^\n]*/g, ""); -if (LOCAL_ONLY_PREFIXES.length === 0 || ALWAYS_PROTECTED_PATHS.length === 0) { - console.error("[openapi-security-tiers] FAIL — could not parse routeGuard.ts constants"); +function resolveModule(spec) { + let base; + if (spec.startsWith("@/")) base = path.join(ROOT, "src", spec.slice(2)); + else if (spec.startsWith(".")) base = path.resolve(path.dirname(ROUTE_GUARD_PATH), spec); + else throw new Error(`openapi-security-tiers: unsupported import specifier '${spec}'`); + for (const cand of [base, `${base}.ts`, `${base}.mts`, path.join(base, "index.ts")]) { + if (fs.existsSync(cand) && fs.statSync(cand).isFile()) return cand; + } + throw new Error(`openapi-security-tiers: cannot resolve module '${spec}' (from ${base})`); +} + +// Resolve a bare identifier used inside a prefix array (e.g. VNC_ROUTE_PREFIX) +// to its string-literal value by following its import in routeGuard.ts. +function resolveIdentifier(ident) { + const imp = guardSrc.match( + new RegExp(`import\\s*(?:type\\s*)?\\{[^}]*\\b${ident}\\b[^}]*\\}\\s*from\\s*["']([^"']+)["']`) + ); + if (!imp) + throw new Error( + `openapi-security-tiers: '${ident}' used in a prefix array has no import in routeGuard.ts` + ); + const modSrc = fs.readFileSync(resolveModule(imp[1]), "utf-8"); + const lit = modSrc.match(new RegExp(`export const ${ident}\\s*=\\s*["']([^"']+)["']`)); + if (!lit) + throw new Error(`openapi-security-tiers: cannot resolve '${ident}' to a string literal`); + return lit[1]; +} + +// String prefixes: quoted entries pass through; bare identifiers are resolved. +function parsePrefixes(name) { + const body = extractArrayBody(name); + if (body == null) + throw new Error(`openapi-security-tiers: could not locate ${name} in routeGuard.ts`); + return stripLineComments(body) + .split(",") + .map((s) => s.trim()) + .filter(Boolean) + .map((tok) => { + const unquoted = tok.replace(/^["']|["']$/g, ""); + return unquoted !== tok ? unquoted : resolveIdentifier(tok); + }); +} + +// RegExp patterns: one `/.../ ` literal per line. +function parsePatterns(name) { + const body = extractArrayBody(name); + if (body == null) + throw new Error(`openapi-security-tiers: could not locate ${name} in routeGuard.ts`); + const out = []; + for (const raw of body.split("\n")) { + const t = raw + .replace(/\/\/.*$/, "") + .trim() + .replace(/,\s*$/, "") + .trim(); + if (t.length > 2 && t.startsWith("/") && t.endsWith("/")) out.push(new RegExp(t.slice(1, -1))); + } + return out; +} + +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"); + +if ( + LOCAL_ONLY_PREFIXES.length === 0 || + LOCAL_ONLY_PATTERNS.length === 0 || + ALWAYS_PROTECTED_PATHS.length === 0 +) { + console.error( + `[openapi-security-tiers] FAIL — could not parse routeGuard.ts constants ` + + `(prefixes=${LOCAL_ONLY_PREFIXES.length}, patterns=${LOCAL_ONLY_PATTERNS.length}, ` + + `alwaysProtected=${ALWAYS_PROTECTED_PATHS.length})` + ); process.exit(1); } +// OpenAPI template params ({id}, {sessionId}, …) → a concrete single non-slash +// segment, so pattern regexes written against resolved paths (`[^/]+`) match. +const concretize = (p) => p.replace(/\{[^}]+\}/g, "x"); + +const matchesPrefix = (concrete) => + LOCAL_ONLY_PREFIXES.some((prefix) => { + const norm = prefix.endsWith("/") ? prefix.slice(0, -1) : prefix; + return concrete === norm || concrete.startsWith(`${norm}/`); + }); + +function coveredByLocalOnly(pathStr) { + const concrete = concretize(pathStr); + return matchesPrefix(concrete) || LOCAL_ONLY_PATTERNS.some((re) => re.test(concrete)); +} + const raw = yaml.load(fs.readFileSync(OPENAPI_PATH, "utf-8")); const paths = raw.paths || {}; - const errors = []; for (const [pathStr, methods] of Object.entries(paths)) { @@ -48,17 +144,11 @@ for (const [pathStr, methods] of Object.entries(paths)) { for (const [method, spec] of Object.entries(methods)) { if (!["get", "post", "put", "patch", "delete"].includes(method) || !spec) continue; - if (spec["x-loopback-only"] === true) { - const matchesPrefix = LOCAL_ONLY_PREFIXES.some((prefix) => { - const norm = prefix.endsWith("/") ? prefix.slice(0, -1) : prefix; - return pathStr === norm || pathStr.startsWith(norm + "/"); - }); - if (!matchesPrefix) { - errors.push( - `${method.toUpperCase()} ${pathStr}: has x-loopback-only but is NOT covered by ` + - `LOCAL_ONLY_API_PREFIXES [${LOCAL_ONLY_PREFIXES.join(", ")}]` - ); - } + if (spec["x-loopback-only"] === true && !coveredByLocalOnly(pathStr)) { + errors.push( + `${method.toUpperCase()} ${pathStr}: has x-loopback-only but is NOT covered by ` + + `LOCAL_ONLY_API_PREFIXES or LOCAL_ONLY_API_PATTERNS` + ); } if (spec["x-always-protected"] === true) { @@ -75,23 +165,13 @@ for (const [pathStr, methods] of Object.entries(paths)) { } } -// Reverse pass: every YAML path that falls under a LOCAL_ONLY prefix should -// carry `x-loopback-only: true` on every method, otherwise external API -// consumers have no signal that the route is loopback-restricted. Closes the -// "new spawn-capable route added without annotation" regression class. -// -// Currently reported as warnings (non-fatal) because the v3.8.4 release ships -// with a known annotation gap on /api/services/* and /api/cli-tools/runtime/* -// that will be patched in a follow-up doc-only PR. Promote to errors once the -// backlog is cleared. +// Reverse pass (non-fatal): every YAML path that falls under a LOCAL_ONLY prefix +// should carry `x-loopback-only`. Pattern-only routes are intentionally excluded +// — they are not "under" a broad prefix. Known annotation gaps stay warnings. const reverseWarnings = []; for (const [pathStr, methods] of Object.entries(paths)) { if (!methods || typeof methods !== "object") continue; - const fallsUnderLocalOnly = LOCAL_ONLY_PREFIXES.some((prefix) => { - const norm = prefix.endsWith("/") ? prefix.slice(0, -1) : prefix; - return pathStr === norm || pathStr.startsWith(norm + "/"); - }); - if (!fallsUnderLocalOnly) continue; + if (!matchesPrefix(concretize(pathStr))) continue; for (const [method, spec] of Object.entries(methods)) { if (!["get", "post", "put", "patch", "delete"].includes(method) || !spec) continue; if (spec["x-loopback-only"] !== true) { @@ -105,7 +185,7 @@ for (const [pathStr, methods] of Object.entries(paths)) { if (reverseWarnings.length > 0) { console.warn( - `[openapi-security-tiers] WARN — ${reverseWarnings.length} LOCAL_ONLY paths missing x-loopback-only annotation (non-fatal, follow-up doc PR):` + `[openapi-security-tiers] WARN — ${reverseWarnings.length} LOCAL_ONLY paths missing x-loopback-only annotation (non-fatal):` ); reverseWarnings.forEach((w) => console.warn(` - ${w}`)); }