From ccff01a13ab6dc3305ca9dbdc0c5e89028096b97 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 22 Sep 2026 22:52:17 -0700 Subject: [PATCH] fix(acl): keep template-substitution source visible to the namespace-alias scan Fix round 3: re-review found stripCommentsAndStrings collapsed whole backtick template literals, including ${...} substitutions, before the namespace-alias occurrence scan ever saw them. A tagged template hands each substitution's value to the tag function by reference, unstringified, so tag`${X}` smuggled the tauri-commands.ts namespace object past the check exactly like fn(X) does, and neither threw. Replaced the regex-based comment/string stripper with a small hand-rolled scanner (skipQuoted/scanSubstitution/maskTemplate) that drops literal template text but keeps a substitution's source intact, recursively re-stripped for its own comments/strings/nested templates, so an alias referenced only inside ${...} stays visible to (and, when used via member access, correctly counted by) the occurrence scan. Unterminated strings/comments/templates/substitutions now throw (fail-closed) rather than running off the end of the text. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/src/test/capabilities.test.ts | 155 +++++++++++++++++++++++++++--- 1 file changed, 140 insertions(+), 15 deletions(-) diff --git a/app/src/test/capabilities.test.ts b/app/src/test/capabilities.test.ts index c2d841e..64f5fc2 100644 --- a/app/src/test/capabilities.test.ts +++ b/app/src/test/capabilities.test.ts @@ -135,25 +135,150 @@ function importSpecs(file: string): string[] { ]; } -/** Best-effort removal of block/line comments and string/template literals, so the namespace-usage - * scan below doesn't trip on the alias appearing in prose or as text data. Regex-based, not a real - * parser — good enough for this test's purpose, not a general-purpose stripper. */ -function stripCommentsAndStrings(text: string): string { - return text - .replace(/\/\*[\s\S]*?\*\//g, "") - .replace(/\/\/.*$/gm, "") - .replace(/`(?:\\.|[^`\\])*`/g, "``") - .replace(/"(?:\\.|[^"\\])*"/g, '""') - .replace(/'(?:\\.|[^'\\])*'/g, "''"); +/** Scans a double- or single-quoted string starting at `text[start]` (the opening quote) and + * returns the index just past its closing quote, honoring backslash escapes. Throws (fail-closed) + * on an unterminated string rather than silently running to end-of-file. */ +function skipQuoted(text: string, start: number, file: string): number { + const quote = text[start]; + let i = start + 1; + while (i < text.length) { + if (text[i] === "\\") { + i += 2; + continue; + } + if (text[i] === quote) return i + 1; + i++; + } + throw new Error(`${relative(srcDir, file)}: unterminated ${quote === '"' ? "double" : "single"}-quoted string`); +} + +/** Scans a `${...}` substitution's contents starting right after the `${`, tracking nesting of + * `{`/`(`/`[` (and skipping over any string/template/comment inside) so an unrelated `}` doesn't + * end the substitution early. Returns the raw source text of the substitution (not including the + * closing `}`) and the index just past that `}`. Throws (fail-closed) if it runs off the end + * without finding a balancing `}` — an unparseable substitution is not silently ignored. */ +function scanSubstitution(text: string, start: number, file: string): { content: string; endIndex: number } { + let i = start; + let depth = 0; + while (i < text.length) { + const c = text[i]; + if (c === "\\") { + i += 2; + continue; + } + if (c === '"' || c === "'") { + i = skipQuoted(text, i, file); + continue; + } + if (c === "`") { + i = maskTemplate(text, i, file).endIndex; + continue; + } + if (c === "/" && text[i + 1] === "*") { + const end = text.indexOf("*/", i + 2); + if (end === -1) throw new Error(`${relative(srcDir, file)}: unterminated block comment`); + i = end + 2; + continue; + } + if (c === "/" && text[i + 1] === "/") { + const end = text.indexOf("\n", i + 2); + i = end === -1 ? text.length : end; + continue; + } + if (c === "{" || c === "(" || c === "[") { + depth++; + i++; + continue; + } + if (c === ")" || c === "]") { + depth--; + i++; + continue; + } + if (c === "}") { + if (depth === 0) return { content: text.slice(start, i), endIndex: i + 1 }; + depth--; + i++; + continue; + } + i++; + } + throw new Error(`${relative(srcDir, file)}: unterminated template substitution ("\${...}" never closes)`); +} + +/** Masks a backtick template literal starting at `text[start]` (the opening backtick): literal + * template *text* is dropped, but every `${...}` substitution's source is kept — recursively + * stripped of its own comments/strings/nested templates via `stripCommentsAndStrings` — since a + * namespace alias referenced only inside a substitution is passed by reference to whatever + * consumes it (a tagged template hands each substitution to the tag function unstringified, + * exactly like `fn(alias)`) and must stay visible to the occurrence scan. Throws (fail-closed) on + * an unterminated template. */ +function maskTemplate(text: string, start: number, file: string): { result: string; endIndex: number } { + let i = start + 1; + let result = ""; + while (i < text.length) { + const c = text[i]; + if (c === "\\") { + i += 2; + continue; + } + if (c === "`") return { result, endIndex: i + 1 }; + if (c === "$" && text[i + 1] === "{") { + const { content, endIndex } = scanSubstitution(text, i + 2, file); + result += ` ${stripCommentsAndStrings(content, file)} `; + i = endIndex; + continue; + } + i++; + } + throw new Error(`${relative(srcDir, file)}: unterminated template literal`); +} + +/** Best-effort removal of block/line comments and string/template literal *text*, so the + * namespace-usage scan below doesn't trip on the alias appearing in prose or as string data — while + * keeping the source of any `${...}` template substitution intact, recursively, since that's live + * code that can reference the alias. A small hand-rolled scanner, not a full parser: good enough + * for this test's purpose. Throws (fail-closed) on anything it can't make sense of — an + * unterminated string, comment or template — rather than silently truncating. */ +function stripCommentsAndStrings(text: string, file: string): string { + let out = ""; + let i = 0; + while (i < text.length) { + const c = text[i]; + if (c === "/" && text[i + 1] === "*") { + const end = text.indexOf("*/", i + 2); + if (end === -1) throw new Error(`${relative(srcDir, file)}: unterminated block comment`); + i = end + 2; + continue; + } + if (c === "/" && text[i + 1] === "/") { + const end = text.indexOf("\n", i + 2); + i = end === -1 ? text.length : end; + continue; + } + if (c === '"' || c === "'") { + i = skipQuoted(text, i, file); + continue; + } + if (c === "`") { + const masked = maskTemplate(text, i, file); + out += masked.result; + i = masked.endIndex; + continue; + } + out += c; + i++; + } + return out; } /** Every occurrence of a namespace-imported alias, after its import line, must be a plain - * `alias.identifier` member access — `fn(alias)`, `const y = alias`, `alias["name"]` and - * `alias[expr]` all hand the whole namespace object somewhere this scan can't follow, which is - * exactly the silent-smuggling shape this test exists to close. Any such occurrence is fail-closed - * rather than silently accepted. */ + * `alias.identifier` member access — `fn(alias)`, `const y = alias`, `alias["name"]`, + * `alias[expr]`, and a tagged-template substitution like `` tag`${alias}` `` all hand the whole + * namespace object somewhere this scan can't follow, which is exactly the silent-smuggling shape + * this test exists to close. Any such occurrence is fail-closed rather than silently accepted. */ function assertNamespaceUsedAsMemberAccessOnly(file: string, text: string, alias: string, importEnd: number) { - const rest = stripCommentsAndStrings(text.slice(importEnd)); + const rest = stripCommentsAndStrings(text.slice(importEnd), file); const occurrence = new RegExp(`\\b${alias}\\b(\\.[A-Za-z_$][\\w$]*)?`, "g"); for (const m of rest.matchAll(occurrence)) { if (!m[1]) {