From 6d4f32e81c21d274111b959b329a05b9fd1eab14 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 22 Sep 2026 22:46:18 -0700 Subject: [PATCH] fix(acl): fail closed on namespace-alias value-smuggling and export-* re-export Fix round 2: re-review found the resolve-based namespace-import fix from round 1 unsound for non-dot-access uses. fn(X), const y = X, and X["name"]/X[expr] all hand the whole tauri-commands.ts namespace object somewhere the member-access regex can't follow, and none of them failed closed. In any viewer-closure file that namespace-imports tauri-commands, every occurrence of the alias after its import line (comments and strings stripped, best-effort) must now be a plain alias.identifier member access or the test throws, naming the file and telling the author to use named imports instead. Also fails closed on `export * as ns from ".../tauri-commands"`, which the member-access scan can't audit either. Both checks are scoped to the viewer side of the ACL boundary (wrapperNamesImportedBy's new strict parameter) since that's where a missed case is a real escape; the main-window count stays permissive as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/src/test/capabilities.test.ts | 76 ++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 11 deletions(-) diff --git a/app/src/test/capabilities.test.ts b/app/src/test/capabilities.test.ts index 3e4fba6..c2d841e 100644 --- a/app/src/test/capabilities.test.ts +++ b/app/src/test/capabilities.test.ts @@ -102,8 +102,9 @@ function resolveRelativeImport(from: string, spec: string): string | null { } const IMPORT_SPEC = /^import\b[^;]*?\bfrom\s*["']([^"']+)["']|^import\s*["']([^"']+)["']/gm; -/** `export { x } from "…"` and `export * from "…"` — a re-export is a module-graph edge too. */ -const EXPORT_FROM_SPEC = /^export\s*(?:\*|(?:type\s*)?\{[^}]*\})\s*from\s*["']([^"']+)["']/gm; +/** `export { x } from "…"`, `export * from "…"` and `export * as ns from "…"` — a re-export is a + * module-graph edge too. */ +const EXPORT_FROM_SPEC = /^export\s*(?:\*(?:\s*as\s+\w+)?|(?:type\s*)?\{[^}]*\})\s*from\s*["']([^"']+)["']/gm; /** A dynamic `import("…")` with a plain string literal argument. Template/computed specs aren't walked. */ const DYNAMIC_IMPORT_SPEC = /\bimport\(\s*["']([^"']+)["']\s*\)/g; /** Named or re-exported wrapper identifiers, e.g. `import { x } from ".../tauri-commands"` or @@ -114,6 +115,11 @@ const WRAPPER_IMPORT = /(?:import|export)\s*(?:type\s*)?\{([^}]*)\}\s*from\s*["' * used with `.test()`, and a global regex's `.test()` mutates `lastIndex` across calls, which * would make later files silently skip a real match. */ const STAR_REEXPORT_WRAPPERS = /^export\s*\*\s*from\s*["'][^"']*\/lib\/tauri-commands["']/m; +/** `export * as ns from ".../tauri-commands"` — a namespace re-export. Resolving `ns`'s downstream + * uses (by whoever imports it from *this* file) is out of reach for a per-file regex scan, so this + * form is fail-closed rather than resolved, same as an unhandled path alias. No `g` flag, same + * `.test()`-reuse reason as STAR_REEXPORT_WRAPPERS above. */ +const NAMESPACE_REEXPORT = /^export\s*\*\s*as\s+\w+\s*from\s*["'][^"']*\/lib\/tauri-commands["']/m; /** `import * as NS from ".../tauri-commands"` — NS's member accesses must be resolved, not dropped. */ const NAMESPACE_IMPORT = /^import\s*\*\s*as\s+(\w+)\s*from\s*["']([^"']+)["']/gm; /** Presence of the `@tauri-apps/api/core` specifier, static or dynamic — this file is the only @@ -129,9 +135,44 @@ 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, "''"); +} + +/** 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. */ +function assertNamespaceUsedAsMemberAccessOnly(file: string, text: string, alias: string, importEnd: number) { + const rest = stripCommentsAndStrings(text.slice(importEnd)); + const occurrence = new RegExp(`\\b${alias}\\b(\\.[A-Za-z_$][\\w$]*)?`, "g"); + for (const m of rest.matchAll(occurrence)) { + if (!m[1]) { + throw new Error( + `${relative(srcDir, file)}: "${alias}" (a namespace import of tauri-commands.ts) is used as a ` + + `bare value rather than always as "${alias}.wrapperName" — this test can only audit direct ` + + `member access; rewrite as named imports (import { wrapperName } from "…/tauri-commands") so ` + + `wrapper usage stays auditable.`, + ); + } + } +} + /** Wrapper names a file's code actually reaches: named imports/re-exports, a `export *` of all of - * them, and member accesses through a `import * as NS from ".../tauri-commands"` namespace. */ -function wrapperNamesImportedBy(file: string, wrappers: Map): string[] { + * them, and member accesses through a `import * as NS from ".../tauri-commands"` namespace. + * `strict` additionally fails closed on forms this scan cannot safely resolve — the viewer side of + * the ACL boundary, where a silent miss would be a real escape, uses it; the main-window side (used + * only to build an inclusive usage count, not to bound anything) does not. */ +function wrapperNamesImportedBy(file: string, wrappers: Map, strict: boolean): string[] { const text = readFileSync(file, "utf-8"); const named = [...text.matchAll(WRAPPER_IMPORT)].flatMap((m) => m[1] @@ -140,12 +181,25 @@ function wrapperNamesImportedBy(file: string, wrappers: Map): st .filter((s) => s.length > 0), ); const starReexported = STAR_REEXPORT_WRAPPERS.test(text) ? [...wrappers.keys()] : []; - const namespaceAliases = [...text.matchAll(NAMESPACE_IMPORT)] - .filter((m) => resolveRelativeImport(file, m[2]) === WRAPPERS) - .map((m) => m[1]); - const namespaced = namespaceAliases.flatMap((alias) => { + if (strict && NAMESPACE_REEXPORT.test(text)) { + throw new Error( + `${relative(srcDir, file)}: re-exports tauri-commands.ts via "export * as ... from" — this test ` + + `cannot audit a namespace re-export; use named re-exports (export { wrapperName } from ` + + `"…/tauri-commands") instead.`, + ); + } + const namespaceImports = [...text.matchAll(NAMESPACE_IMPORT)].filter( + (m) => resolveRelativeImport(file, m[2]) === WRAPPERS, + ); + if (strict) { + for (const m of namespaceImports) { + assertNamespaceUsedAsMemberAccessOnly(file, text, m[1], m.index + m[0].length); + } + } + const namespaced = namespaceImports.flatMap((m) => { + const alias = m[1]; const memberAccess = new RegExp(`\\b${alias}\\.(\\w+)\\b`, "g"); - return [...text.matchAll(memberAccess)].map((m) => m[1]); + return [...text.matchAll(memberAccess)].map((mm) => mm[1]); }); return [...named, ...starReexported, ...namespaced]; } @@ -207,7 +261,7 @@ describe("capability files match the code each window runs", () => { expect(closure.has(WRAPPERS), "the viewer reaches tauri-commands.ts").toBe(true); const viewerCommands = new Set(); for (const file of closure) { - for (const name of wrapperNamesImportedBy(file, wrappers)) { + for (const name of wrapperNamesImportedBy(file, wrappers, true)) { const command = wrappers.get(name); expect(command, `${relative(srcDir, file)} imports unknown wrapper ${name}`).toBeDefined(); viewerCommands.add(command!); @@ -223,7 +277,7 @@ describe("capability files match the code each window runs", () => { const mainCommands = new Set(); for (const file of files) { if (closure.has(file)) continue; - for (const name of wrapperNamesImportedBy(file, wrappers)) { + for (const name of wrapperNamesImportedBy(file, wrappers, false)) { const command = wrappers.get(name); expect(command, `${relative(srcDir, file)} imports unknown wrapper ${name}`).toBeDefined(); mainCommands.add(command!);