fix(ci): openapi-security-tiers checker must honor routeGuard patterns + imported prefixes (#12350)

Validado numa worktree sobre o tip de `release/v3.8.51`, medindo o gate dos dois lados: **red no tip** (dezenas de rotas `volcengine-plan`/`vnc-session` reportadas como "has x-loopback-only but is NOT covered") e **PASS com este PR**, exit 0. Como é um gate de segurança, confirmei que o fix torna o checker *preciso* e não *frouxo*. A afirmação central do PR — que uma rota é coberta se casar com um prefixo resolvido **ou** com um pattern — bate exatamente com o runtime (`src/server/authz/routeGuard.ts:252-255`): ```ts return ( LOCAL_ONLY_API_PREFIXES.some((p) => path === p || path.startsWith(p)) || LOCAL_ONLY_API_PATTERNS.some((re) => re.test(path)) ); ``` O checker antigo enxergava só o primeiro braço, e nem isso por completo: a captura `[^\]]+` quebrava no `]` dentro de classes de regex, então `LOCAL_ONLY_API_PATTERNS` não era parseado, e `VNC_ROUTE_PREFIX` (const importada, não literal) não era resolvido. Resultado: rotas efetivamente protegidas em runtime apareciam como desprotegidas. Nenhum achado real foi silenciado — as 95 linhas de `WARN — missing x-loopback-only annotation` continuam saindo, são explicitamente não-fatais e pré-existentes. Fecha um dos HARDs do base-red #12335. Obrigado, @ggiak.
2026-09-13 18:32:12 +03:00 · 2026-09-03 15:10:45 +03:00
parent e1cf542378
commit 3f3d27e264
1 changed files with 127 additions and 47 deletions
--- 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}`));
 }