From 4a2495843b206e0c3cb73949b1befaf65d777451 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sat, 22 Aug 2026 11:53:10 +0200 Subject: [PATCH 1/2] Prevent ambiguous server destructuring from reaching browsers Classify binding positions separately from evaluated pattern positions, prune declarations only for trusted browser-droppable initializer sources, and fail the browser build when safe removal cannot be proved. Constraint: Preserve arbitrary project-local client initialization and client-live sibling bindings. Rejected: Relax every-in-closure globally | deletes unproven initializer side effects. Rejected: Statement-wide pattern eligibility | unrelated hazardous co-declarators pin removable server values. Rejected: General JavaScript purity analysis | too broad for this transform boundary. Confidence: high Scope-risk: moderate Directive: Keep unknown pattern syntax and effectful initializer arguments on the fail-closed path. Tested: Deno 2.7.7 focused stage test (165 steps), full transforms suite (161 suites, 2674 steps), fmt, lint, check. Not-tested: Full repository unit and integration suites before the commit hook. Related: veryfront/veryfront-issue-inbox#607 --- .../browser-server-exports-strip.test.ts | 272 +++++++++++-- .../stages/browser-server-exports-strip.ts | 357 +++++++++++++----- 2 files changed, 518 insertions(+), 111 deletions(-) diff --git a/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts b/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts index a9b069a6f5..a59171ff30 100644 --- a/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts +++ b/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts @@ -13,6 +13,7 @@ import { register, tryResolve, unregister } from "#veryfront/extensions/contract import type { CodeParser } from "#veryfront/extensions/parser/index.ts"; import { getErrorCollector, resetErrorCollector } from "#veryfront/observability"; import { + browserServerExportsStripInternals, browserServerExportsStripPlugin, moduleReferenceWalkers, stripServerOnlyExports, @@ -30,11 +31,75 @@ function occurrences(haystack: string, name: string): number { return haystack.match(new RegExp(`\\b${name}\\b`, "g"))?.length ?? 0; } +async function assertUnsafeServerDestructuring(code: string): Promise { + const error = await assertRejects( + () => stripServerOnlyExports(code, "pages/unsafe-destructuring.tsx"), + VeryfrontError, + ); + assertInstanceOf(error, VeryfrontError); + assertEquals(error.slug, "server-only-in-client"); + assertStringIncludes(error.message, "Move the destructuring into getServerData"); + assertStringIncludes(error.message, "Declare client initialization separately"); + assertStringIncludes(error.message, 'import { getEnv } from "veryfront"'); + assertStringIncludes(error.message, "export default function Page()"); + return error; +} + +async function bindingPatternAnalysis(source: string) { + const parser = tryResolve("CodeParser"); + if (!parser) throw new Error("CodeParser extension is not registered"); + const ast = await parser.parse({ code: source, filePath: "pattern.ts" }); + const statement = (ast as unknown as { program: { body: Array> } }) + .program.body[0]; + if (!statement) throw new Error(`No statement parsed from: ${source}`); + const declarator = (statement.declarations as Array> | undefined)?.[0]; + if (!declarator) throw new Error(`No declarator parsed from: ${source}`); + return browserServerExportsStripInternals.analyzeBindingPattern(declarator.id); +} + describe("browser-server-exports-strip", () => { afterAll(async () => { await stopEsbuild(); }); + describe("binding-pattern classification", () => { + it("collects nested object, array, hole, rest, and renamed binding positions", async () => { + const analysis = await bindingPatternAnalysis( + "const { plain, renamed: alias, nested: { value }, list: [first, , ...tail], ...rest } = source;", + ); + + assertEquals(analysis.bindingNames, ["plain", "alias", "value", "first", "tail", "rest"]); + assertEquals(analysis.hazards, []); + }); + + it("separates a default expression from its binding position", async () => { + const analysis = await bindingPatternAnalysis("const { source: value = fallback } = input;"); + + assertEquals(analysis.bindingNames, ["value"]); + assertEquals(analysis.possibleNames, ["value"]); + assertEquals(analysis.hazards, ["default-value"]); + }); + + it("separates a computed key from its binding position", async () => { + const analysis = await bindingPatternAnalysis("const { [keyName]: value } = input;"); + + assertEquals(analysis.bindingNames, ["value"]); + assertEquals(analysis.possibleNames, ["value"]); + assertEquals(analysis.hazards, ["computed-key"]); + }); + + it("classifies unknown pattern syntax without trusting possible bindings", () => { + const analysis = browserServerExportsStripInternals.analyzeBindingPattern({ + type: "FutureBindingPattern", + child: { type: "Identifier", name: "possibleBinding" }, + }); + + assertEquals(analysis.bindingNames, []); + assertEquals(analysis.possibleNames, ["possibleBinding"]); + assertEquals(analysis.hazards, ["unknown-syntax"]); + }); + }); + describe("emptying server-only hooks", () => { it("runs before custom pre-compile browser plugins see server-only code", async () => { const source = [ @@ -931,7 +996,48 @@ describe("browser-server-exports-strip", () => { assertEquals(occurrences(result, "rest"), 0); }); - it("conservatively keeps a pattern with a default value", async () => { + it("drops a known-framework rest pattern when an unused sibling is outside the hook closure", async () => { + const code = [ + `import { getEnv } from "veryfront";`, + `const { unusedByEveryone, ...rest } = getEnv("SERVER_ONLY");`, + `export async function getServerData() { return { props: { rest } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await stripServerOnlyExports(code); + + assertNotIncludes(result, "SERVER_ONLY"); + assertNotIncludes(result, "getEnv"); + assertEquals(occurrences(result, "unusedByEveryone"), 0); + assertEquals(occurrences(result, "rest"), 0); + }); + + it("recognizes aliased and namespace framework imports as known server initializers", async () => { + const variants = [ + [ + `import { getEnv as readEnv } from "veryfront";`, + `const { unused, ...rest } = readEnv("SERVER_ONLY");`, + ], + [ + `import * as vf from "veryfront";`, + `const { unused, ...rest } = vf.getEnv("SERVER_ONLY");`, + ], + ]; + + for (const [importLine, declaration] of variants) { + const result = await stripServerOnlyExports([ + importLine, + declaration, + `export async function getServerData() { return { props: { rest } }; }`, + `export default function Page() { return null; }`, + ].join("\n")); + + assertNotIncludes(result, "SERVER_ONLY"); + assertNotIncludes(result, "veryfront"); + } + }); + + it("rejects a server-hook destructuring default instead of shipping it", async () => { const code = [ `import { getEnv } from "veryfront";`, `const DEFAULT = getEnv("CLIENT_FALLBACK");`, @@ -940,15 +1046,10 @@ describe("browser-server-exports-strip", () => { `export default function Page() { return DEFAULT; }`, ].join("\n"); - const result = await stripServerOnlyExports(code); - - assertStringIncludes(result, "CLIENT_FALLBACK"); - assertStringIncludes(result, "DEFAULT"); - assertStringIncludes(result, "SERVER_ONLY"); - assertStringIncludes(result, "a = DEFAULT"); + await assertUnsafeServerDestructuring(code); }); - it("conservatively keeps a pattern with a computed key", async () => { + it("rejects a computed server-hook pattern instead of shipping it", async () => { const code = [ `import { getEnv } from "veryfront";`, `const KEY = getEnv("CLIENT_KEY");`, @@ -957,12 +1058,7 @@ describe("browser-server-exports-strip", () => { `export default function Page() { return KEY; }`, ].join("\n"); - const result = await stripServerOnlyExports(code); - - assertStringIncludes(result, "CLIENT_KEY"); - assertStringIncludes(result, "KEY"); - assertStringIncludes(result, "SERVER_ONLY"); - assertStringIncludes(result, "[KEY]: value"); + await assertUnsafeServerDestructuring(code); }); it("removes one destructuring declarator without dropping its client sibling", async () => { @@ -981,44 +1077,131 @@ describe("browser-server-exports-strip", () => { assertStringIncludes(result, "return client"); }); - it("keeps a destructuring default with an unrelated client effect", async () => { + it("does not let a hazardous co-declarator pin a known server rest pattern", async () => { const code = [ - `const { token, client = startClient() } = loadSecret();`, + `import { getEnv } from "veryfront";`, + `const { unused, ...rest } = getEnv("SERVER_REST"), { client = startClient() } = loadClient();`, + `export async function getServerData() { return { props: { rest } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await stripServerOnlyExports(code); + + assertNotIncludes(result, "SERVER_REST"); + assertNotIncludes(result, "getEnv"); + assertStringIncludes(result, "client = startClient()"); + assertStringIncludes(result, "loadClient()"); + }); + + it("does not let a hazardous co-declarator pin a simple server value", async () => { + const code = [ + `import { getEnv } from "veryfront";`, + `const token = getEnv("SERVER_SIMPLE"), { client = startClient() } = loadClient();`, `export async function getServerData() { return { props: { token } }; }`, `export default function Page() { return null; }`, ].join("\n"); const result = await stripServerOnlyExports(code); + assertNotIncludes(result, "SERVER_SIMPLE"); + assertNotIncludes(result, "getEnv"); assertStringIncludes(result, "client = startClient()"); - assertStringIncludes(result, "loadSecret()"); + assertStringIncludes(result, "loadClient()"); }); - it("keeps a computed pattern key with an unrelated client effect", async () => { + it("keeps a server declarator a hazardous client co-declarator still reads", async () => { const code = [ - `const { [startClient()]: token } = loadSecret();`, + `import { getEnv } from "veryfront";`, + `const token = getEnv("CLIENT_FALLBACK"), { client = token } = loadClient();`, `export async function getServerData() { return { props: { token } }; }`, `export default function Page() { return null; }`, ].join("\n"); const result = await stripServerOnlyExports(code); - assertStringIncludes(result, "[startClient()]: token"); - assertStringIncludes(result, "loadSecret()"); + assertStringIncludes(result, "CLIENT_FALLBACK"); + assertStringIncludes(result, "getEnv"); + assertStringIncludes(result, "client = token"); + assertStringIncludes(result, "loadClient()"); + }); + + it("rejects a destructuring default with an unrelated client effect", async () => { + const code = [ + `const { token, client = startClient() } = loadSecret();`, + `export async function getServerData() { return { props: { token } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + await assertUnsafeServerDestructuring(code); + }); + + it("rejects a computed pattern key with an unrelated client effect", async () => { + const code = [ + `const { [startClient()]: token } = loadSecret();`, + `export async function getServerData() { return { props: { token } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + await assertUnsafeServerDestructuring(code); + }); + + it("rejects an unknown side-effecting initializer with a sibling outside the hook closure", async () => { + const code = [ + `const { token, client } = loadSecret();`, + `export async function getServerData() { return { props: { token } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + await assertUnsafeServerDestructuring(code); }); - it("keeps a destructuring declarator with a sibling outside the hook closure", async () => { + it("rejects a project-local initializer with a sibling outside the hook closure", async () => { const code = [ + `import { loadSecret } from "./server.ts";`, `const { token, client } = loadSecret();`, `export async function getServerData() { return { props: { token } }; }`, `export default function Page() { return null; }`, ].join("\n"); + await assertUnsafeServerDestructuring(code); + }); + + it("rejects an effectful argument to a known-framework initializer", async () => { + const code = [ + `import { getEnv } from "veryfront";`, + `const { token, client } = getEnv(startClient());`, + `export async function getServerData() { return { props: { token } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + await assertUnsafeServerDestructuring(code); + }); + + it("keeps a destructuring initializer when the browser reads a sibling binding", async () => { + const code = [ + `const { token, client } = loadSecret();`, + `export async function getServerData() { return { props: { token } }; }`, + `export default function Page() { return client; }`, + ].join("\n"); + + const result = await stripServerOnlyExports(code); + + assertStringIncludes(result, "loadSecret()"); + assertStringIncludes(result, "return client"); + }); + + it("keeps an evaluated pattern when the browser reads a sibling binding", async () => { + const code = [ + `const { token, client = startClient() } = loadSecret();`, + `export async function getServerData() { return { props: { token } }; }`, + `export default function Page() { return client; }`, + ].join("\n"); + const result = await stripServerOnlyExports(code); assertStringIncludes(result, "loadSecret()"); - assertEquals(occurrences(result, "token"), 1); - assertEquals(occurrences(result, "client"), 1); + assertStringIncludes(result, "client = startClient()"); + assertStringIncludes(result, "return client"); }); it("keeps an import that the client still references", async () => { @@ -1305,6 +1488,47 @@ describe("browser-server-exports-strip", () => { assertEquals(occurrences(code, "initClientMetrics"), 0); } + it("removes a known server destructuring dependency from the browser pipeline", async () => { + const source = [ + `import { getEnv } from "veryfront";`, + `const { unused, ...rest } = getEnv("SERVER_ONLY_PIPELINE_VALUE");`, + `export async function getServerData() { return { props: { rest } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline( + source, + "/project/pages/destructuring.tsx", + "/project", + { projectId: "server-destructuring-strip", dev: false, ssr: false }, + ); + + assertNotIncludes(result.code, "SERVER_ONLY_PIPELINE_VALUE"); + assertNotIncludes(result.code, "getEnv"); + }); + + it("returns a stable boundary error for ambiguous browser destructuring", async () => { + const source = [ + `const { token, unused } = loadSecret();`, + `export async function getServerData() { return { props: { token } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const error = await assertRejects( + () => + runPipeline( + source, + "/project/pages/ambiguous.tsx", + "/project", + { projectId: "ambiguous-server-destructuring", dev: false, ssr: false }, + ), + VeryfrontError, + ); + + assertInstanceOf(error, VeryfrontError); + assertEquals(error.slug, "server-only-in-client"); + }); + it("accepts decorators after export before compiling browser modules", async () => { const source = [ `function logged(value: unknown) { return value; }`, diff --git a/src/transforms/pipeline/stages/browser-server-exports-strip.ts b/src/transforms/pipeline/stages/browser-server-exports-strip.ts index 41ac73d1ea..6653cbe93c 100644 --- a/src/transforms/pipeline/stages/browser-server-exports-strip.ts +++ b/src/transforms/pipeline/stages/browser-server-exports-strip.ts @@ -57,7 +57,7 @@ import { tryResolve } from "#veryfront/extensions/contracts.ts"; import type { ASTNode, CodeParser } from "#veryfront/extensions/parser/index.ts"; -import { COMPILATION_ERROR } from "#veryfront/errors"; +import { COMPILATION_ERROR, SERVER_ONLY_IN_CLIENT } from "#veryfront/errors"; import { getErrorCollector } from "#veryfront/observability"; import { getLoaderFromPath, isGeneratedContentOutput } from "../../esm/transform-utils.ts"; import { isTypeScript } from "../context.ts"; @@ -388,90 +388,112 @@ async function parseStubs(parser: CodeParser): Promise<{ body: Node; init: Node return { body, init }; } -/** Every binding identifier a variable or parameter pattern introduces. */ -function patternBindingIds(pattern: Node): Node[] { - const bindings: Node[] = []; +type BindingPatternHazard = "computed-key" | "default-value" | "unknown-syntax"; - const collect = (node: Node): void => { - if (node.type === "Identifier") { - bindings.push(node); - return; - } - - if (node.type === "AssignmentPattern") { - if (isNode(node.left)) collect(node.left); - return; - } - - if (node.type === "RestElement") { - if (isNode(node.argument)) collect(node.argument); - return; - } - - // `constructor(private dep: Dep)` binds `dep` as a parameter and assigns - // it to `this` at runtime. - if (node.type === "TSParameterProperty") { - if (isNode(node.parameter)) collect(node.parameter); - return; - } +interface BindingPatternAnalysis { + bindingIds: Node[]; + possibleNames: string[]; + hazards: Set; +} - if (node.type === "ArrayPattern") { - for (const element of Array.isArray(node.elements) ? node.elements : []) { - if (isNode(element)) collect(element); - } - return; - } +function mergePatternAnalysis( + target: BindingPatternAnalysis, + nested: BindingPatternAnalysis, +): void { + target.bindingIds.push(...nested.bindingIds); + target.possibleNames.push(...nested.possibleNames); + for (const hazard of nested.hazards) target.hazards.add(hazard); +} - if (node.type === "ObjectPattern") { - for (const property of Array.isArray(node.properties) ? node.properties : []) { - if (!isNode(property)) continue; - if (property.type === "RestElement") { - if (isNode(property.argument)) collect(property.argument); - continue; - } - if (property.type === "ObjectProperty" && isNode(property.value)) { - collect(property.value); - } - } - } +/** + * Classify one binding pattern without confusing its bindings with expressions + * evaluated while destructuring. Unknown syntax stays visible as a hazard so a + * server-hook dependency can fail closed instead of silently leaving code in + * the browser artifact. + */ +function analyzeBindingPattern(value: unknown): BindingPatternAnalysis { + const analysis: BindingPatternAnalysis = { + bindingIds: [], + possibleNames: [], + hazards: new Set(), }; + if (!isNode(value)) { + analysis.hazards.add("unknown-syntax"); + return analysis; + } - collect(pattern); + if (value.type === "Identifier") { + const name = nodeName(value); + analysis.bindingIds.push(value); + if (name) analysis.possibleNames.push(name); + else analysis.hazards.add("unknown-syntax"); + return analysis; + } - return bindings; -} + if (value.type === "AssignmentPattern") { + analysis.hazards.add("default-value"); + mergePatternAnalysis(analysis, analyzeBindingPattern(value.left)); + return analysis; + } -/** Whether evaluating a pattern can run code outside its binding positions. */ -function patternHasEvaluatedValuePosition(pattern: Node): boolean { - if (pattern.type === "Identifier") return false; - if (pattern.type === "AssignmentPattern") return true; - if (pattern.type === "RestElement") { - return !isNode(pattern.argument) || patternHasEvaluatedValuePosition(pattern.argument); + if (value.type === "RestElement") { + mergePatternAnalysis(analysis, analyzeBindingPattern(value.argument)); + return analysis; } - if (pattern.type === "TSParameterProperty") { - return !isNode(pattern.parameter) || patternHasEvaluatedValuePosition(pattern.parameter); + + // `constructor(private dep: Dep)` binds `dep` as a parameter and assigns + // it to `this` at runtime. + if (value.type === "TSParameterProperty") { + mergePatternAnalysis(analysis, analyzeBindingPattern(value.parameter)); + return analysis; } - if (pattern.type === "ArrayPattern") { - return (Array.isArray(pattern.elements) ? pattern.elements : []).some((element) => - isNode(element) && patternHasEvaluatedValuePosition(element) - ); + + if (value.type === "ArrayPattern") { + for (const element of Array.isArray(value.elements) ? value.elements : []) { + if (element === null || element === undefined) continue; + mergePatternAnalysis(analysis, analyzeBindingPattern(element)); + } + return analysis; } - if (pattern.type === "ObjectPattern") { - return (Array.isArray(pattern.properties) ? pattern.properties : []).some((property) => { - if (!isNode(property)) return false; + + if (value.type === "ObjectPattern") { + for (const property of Array.isArray(value.properties) ? value.properties : []) { + if (!isNode(property)) { + analysis.hazards.add("unknown-syntax"); + continue; + } if (property.type === "RestElement") { - return patternHasEvaluatedValuePosition(property); + mergePatternAnalysis(analysis, analyzeBindingPattern(property.argument)); + continue; } - return property.type !== "ObjectProperty" || property.computed === true || - !isNode(property.value) || patternHasEvaluatedValuePosition(property.value); - }); + if (property.type !== "ObjectProperty") { + mergePatternAnalysis(analysis, analyzeBindingPattern(property)); + analysis.hazards.add("unknown-syntax"); + continue; + } + if (property.computed === true) analysis.hazards.add("computed-key"); + mergePatternAnalysis(analysis, analyzeBindingPattern(property.value)); + } + return analysis; } - return true; + + // Preserve every identifier as a possible binding for the conservative + // error decision, but never add it to bindingIds: its position is unknown. + walk(value, (node) => { + if (node.type !== "Identifier") return true; + const name = nodeName(node); + if (name) analysis.possibleNames.push(name); + return true; + }); + analysis.hazards.add("unknown-syntax"); + return analysis; } /** Every binding name a destructuring pattern introduces. */ function patternBoundNames(pattern: Node): string[] { - return patternBindingIds(pattern).map(nodeName).filter((name): name is string => Boolean(name)); + return analyzeBindingPattern(pattern).bindingIds.map(nodeName).filter( + (name): name is string => Boolean(name), + ); } /** @@ -691,16 +713,28 @@ interface ModuleScopeDecl { bindingIds: Node[]; } +interface DestructuredModuleScopeDecl { + declarator: Node; + bindingIds: Node[]; + analysis: BindingPatternAnalysis; +} + +interface ModuleScopeDeclarationAnalysis { + declarations: ModuleScopeDecl[]; + destructured: DestructuredModuleScopeDecl[]; +} + /** * Non-exported top-level `const`/`let`/`var`/`function`/`class` declarations * whose bindings we could safely drop if nothing references them. Exported * declarations are part of the module's contract and are never candidates. - * Patterns with defaults or computed keys stay fail-closed because evaluating - * them can run unrelated client code. In supported patterns, only binding - * positions are excluded from liveness analysis. + * Patterns with defaults or computed keys are classified separately so an + * unsafe hook-related declaration can fail closed. In supported patterns, + * only binding positions are excluded from liveness analysis. */ -function moduleScopeDeclarations(body: Node[]): ModuleScopeDecl[] { +function analyzeModuleScopeDeclarations(body: Node[]): ModuleScopeDeclarationAnalysis { const decls: ModuleScopeDecl[] = []; + const destructured: DestructuredModuleScopeDecl[] = []; for (const statement of body) { if (statement.type === "FunctionDeclaration" || statement.type === "ClassDeclaration") { @@ -728,16 +762,19 @@ function moduleScopeDeclarations(body: Node[]): ModuleScopeDecl[] { ) { if (!isNode(declarator)) continue; const id = declarator.id; - if (!isNode(id) || patternHasEvaluatedValuePosition(id)) { - variableDecls.length = 0; - break; + if (!isNode(id)) continue; + const analysis = analyzeBindingPattern(id); + if (id.type !== "Identifier") { + destructured.push({ + declarator, + bindingIds: analysis.bindingIds, + analysis, + }); } - const bindingIds = patternBindingIds(id); + if (analysis.hazards.size > 0) continue; + const bindingIds = analysis.bindingIds; const names = bindingIds.map(nodeName).filter((name): name is string => Boolean(name)); - if (names.length === 0 || names.length !== bindingIds.length) { - variableDecls.length = 0; - break; - } + if (names.length === 0 || names.length !== bindingIds.length) continue; variableDecls.push({ statement, declarator, names, bindingIds }); } @@ -745,7 +782,7 @@ function moduleScopeDeclarations(body: Node[]): ModuleScopeDecl[] { } } - return decls; + return { declarations: decls, destructured }; } /** Whether a name is bound in the current lexical stack. */ @@ -1433,7 +1470,11 @@ function failOnAmbiguousCompilerNameHelperDuplicates( if (candidates.length === 0) return; const excluded = new WeakSet(); - for (const decl of moduleScopeDeclarations(body)) { + const scopeDeclarations = analyzeModuleScopeDeclarations(body); + for (const decl of scopeDeclarations.declarations) { + for (const id of decl.bindingIds) excluded.add(id); + } + for (const decl of scopeDeclarations.destructured) { for (const id of decl.bindingIds) excluded.add(id); } for (const registration of compilerNameRegistrations(body)) excluded.add(registration.target); @@ -1511,6 +1552,125 @@ function jsxPragmaBindings(ast: ASTNode): Set { return pinned; } +function isBrowserDroppableImportSource(source: unknown): source is string { + return typeof source === "string" && + (source.startsWith("node:") || source === "veryfront" || source.startsWith("veryfront/")); +} + +function knownServerImportBindings(body: Node[]): Set { + const bindings = new Set(); + for (const statement of body) { + if (statement.type !== "ImportDeclaration" || statement.importKind === "type") continue; + const source = isNode(statement.source) ? statement.source.value : undefined; + if (!isBrowserDroppableImportSource(source)) continue; + for (const binding of importedBindings(statement)) bindings.add(binding); + } + return bindings; +} + +function staticCalleeRoot(node: Node | undefined): string | null { + if (!node) return null; + if (node.type === "Identifier") return nodeName(node); + if ( + (node.type === "MemberExpression" || node.type === "OptionalMemberExpression") && + node.computed !== true && isNode(node.object) + ) { + return staticCalleeRoot(node.object); + } + return null; +} + +/** A deliberately small data-only subset, not a general purity analysis. */ +function isStaticInitializerArgument(node: Node): boolean { + if (node.type === "TemplateLiteral") { + return !Array.isArray(node.expressions) || node.expressions.length === 0; + } + return node.type === "Literal" || node.type.endsWith("Literal"); +} + +function isKnownServerInitializer(node: Node | undefined, trustedBindings: Set): boolean { + if (!node) return false; + if (node.type === "Identifier") return trustedBindings.has(nodeName(node) ?? ""); + if (node.type === "MemberExpression" || node.type === "OptionalMemberExpression") { + const root = staticCalleeRoot(node); + return root !== null && trustedBindings.has(root); + } + if (node.type !== "CallExpression") return false; + const root = staticCalleeRoot(isNode(node.callee) ? node.callee : undefined); + if (root === null || !trustedBindings.has(root)) return false; + return (Array.isArray(node.arguments) ? node.arguments : []).every((argument) => + isNode(argument) && isStaticInitializerArgument(argument) + ); +} + +type DestructuringDisposition = + | "keep-client-live" + | "not-hook-related" + | "reject-initializer" + | "reject-pattern" + | "remove"; + +interface DestructuringDecisionInput { + analysis: BindingPatternAnalysis; + clientReferences: Set; + hookClosure: Set; + initializerIsKnownServer: boolean; + pinned: Set; +} + +function decideDestructuringDisposition( + input: DestructuringDecisionInput, +): DestructuringDisposition { + const { analysis, clientReferences, hookClosure, initializerIsKnownServer, pinned } = input; + if (!analysis.possibleNames.some((name) => hookClosure.has(name))) return "not-hook-related"; + if (analysis.hazards.has("unknown-syntax")) return "reject-pattern"; + + const names = analysis.bindingIds.map(nodeName).filter((name): name is string => Boolean(name)); + if (names.some((name) => clientReferences.has(name) || pinned.has(name))) { + return "keep-client-live"; + } + if (analysis.hazards.size > 0) return "reject-pattern"; + if (names.length > 0 && names.every((name) => hookClosure.has(name))) return "remove"; + return initializerIsKnownServer ? "remove" : "reject-initializer"; +} + +function throwUnsafeServerDestructuring(disposition: DestructuringDisposition): never { + const reason = disposition === "reject-pattern" + ? "the binding pattern evaluates a default value, a computed key, or unsupported syntax" + : "the initializer or one of its arguments may run unproven client side effects"; + throw SERVER_ONLY_IN_CLIENT.create({ + message: + `Cannot safely remove module-scope destructuring used by a server-only hook because ${reason}. ` + + "Move the destructuring into getServerData or the server-only hook that uses it. " + + "Declare client initialization separately.\n\n" + + 'import { getEnv } from "veryfront";\n\n' + + "export async function getServerData() {\n" + + ' const { serverValue } = getEnv("");\n' + + " return { props: { serverValue } };\n" + + "}\n\n" + + 'const clientValue = "";\n\n' + + "export default function Page() {\n" + + " return clientValue;\n" + + "}", + detail: "Ambiguous module-scope destructuring crosses the server and browser boundary", + context: { reason: disposition }, + }); +} + +/** @internal Stable test seams for the browser server-export safety policy. */ +export const browserServerExportsStripInternals = Object.freeze({ + analyzeBindingPattern(value: unknown) { + const analysis = analyzeBindingPattern(value); + return { + bindingNames: analysis.bindingIds.map(nodeName).filter( + (name): name is string => Boolean(name), + ), + possibleNames: [...analysis.possibleNames], + hazards: [...analysis.hazards].sort(), + }; + }, +}); + /** * Drop the top-level declarations the emptied server-only hooks closed over. * @@ -1534,12 +1694,13 @@ function dropUnusedModuleScopeBindings( let current = body; for (;;) { - const decls = moduleScopeDeclarations(current); - if (decls.length === 0) return current; + const { declarations: decls, destructured } = analyzeModuleScopeDeclarations(current); + if (decls.length === 0 && destructured.length === 0) return current; failOnAmbiguousCompilerNameHelperDuplicates(current, hookClosure, filePath); const excluded = new WeakSet(); for (const decl of decls) for (const id of decl.bindingIds) excluded.add(id); + for (const decl of destructured) for (const id of decl.bindingIds) excluded.add(id); // Esbuild's generated name-registration call is metadata for a declaration, // not an independent browser consumer of it. Ignore that target reference @@ -1549,12 +1710,35 @@ function dropUnusedModuleScopeBindings( for (const registration of nameRegistrations) excluded.add(registration.target); const referenced = referencedIdentifiers(current, excluded); + const trustedBindings = knownServerImportBindings(current); + const destructuringDecisions = new Map(); + for (const decl of destructured) { + const disposition = decideDestructuringDisposition({ + analysis: decl.analysis, + clientReferences: referenced, + hookClosure, + initializerIsKnownServer: isKnownServerInitializer( + isNode(decl.declarator.init) ? decl.declarator.init : undefined, + trustedBindings, + ), + pinned, + }); + if (disposition === "reject-initializer" || disposition === "reject-pattern") { + throwUnsafeServerDestructuring(disposition); + } + destructuringDecisions.set(decl.declarator, disposition); + } const removableStatements = new Set(); const removableDeclarators = new Map>(); const removedDecls: ModuleScopeDecl[] = []; for (const decl of decls) { - const inClosure = decl.names.every((name) => hookClosure.has(name)); + const destructuringDecision = decl.declarator === undefined + ? undefined + : destructuringDecisions.get(decl.declarator); + const inClosure = destructuringDecision === undefined + ? decl.names.every((name) => hookClosure.has(name)) + : destructuringDecision === "remove"; const unused = decl.names.every((name) => !referenced.has(name) && !pinned.has(name)); if (!inClosure || !unused) continue; @@ -1657,8 +1841,7 @@ function dropUnusedImportBindings( if (bindings.some((binding) => referenced.has(binding) || pinned.has(binding))) return true; const source = isNode(statement.source) ? statement.source.value : undefined; - const isKnownDroppableSource = typeof source === "string" && - (source.startsWith("node:") || source === "veryfront" || source.startsWith("veryfront/")); + const isKnownDroppableSource = isBrowserDroppableImportSource(source); // A node: or veryfront source is unsafe or pointless as a browser // side-effect import whatever used it. Any other source is deleted when the // stripped hook owned at least one binding and no surviving code reads any From 6bf608b2ce218468341563868e20a3447a73d430 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sat, 22 Aug 2026 12:15:20 +0200 Subject: [PATCH 2/2] Make server-boundary failures actionable Ambiguous destructuring already failed closed, but its diagnostic did not identify the module or bindings that need relocation. Add that context and avoid recomputing the immutable trusted-import set on each pruning iteration. Constraint: Preserve the stable server-only-in-client slug and existing remediation example. Rejected: Add blanket docstrings to touched internals | repository checks do not require them and they would add noise without clarifying the policy boundary. Confidence: high Scope-risk: narrow Directive: Keep trusted import collection outside this loop only while this pass does not mutate imports. Tested: Focused transform suite 165 steps; full transform suite 161 suites and 2674 steps; fmt, lint, and typecheck on touched files. Not-tested: Full repository suite before commit; pre-push gate will run before publishing. Related: veryfront/veryfront-issue-inbox#607 Related: veryfront/veryfront-code#3967 --- .../browser-server-exports-strip.test.ts | 4 +++- .../stages/browser-server-exports-strip.ts | 18 +++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts b/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts index a59171ff30..e4db36b3e8 100644 --- a/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts +++ b/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts @@ -1046,7 +1046,9 @@ describe("browser-server-exports-strip", () => { `export default function Page() { return DEFAULT; }`, ].join("\n"); - await assertUnsafeServerDestructuring(code); + const error = await assertUnsafeServerDestructuring(code); + assertStringIncludes(error.message, "Module: pages/unsafe-destructuring.tsx"); + assertStringIncludes(error.message, "Bindings: a"); }); it("rejects a computed server-hook pattern instead of shipping it", async () => { diff --git a/src/transforms/pipeline/stages/browser-server-exports-strip.ts b/src/transforms/pipeline/stages/browser-server-exports-strip.ts index 6653cbe93c..89de7c3421 100644 --- a/src/transforms/pipeline/stages/browser-server-exports-strip.ts +++ b/src/transforms/pipeline/stages/browser-server-exports-strip.ts @@ -1634,13 +1634,19 @@ function decideDestructuringDisposition( return initializerIsKnownServer ? "remove" : "reject-initializer"; } -function throwUnsafeServerDestructuring(disposition: DestructuringDisposition): never { +function throwUnsafeServerDestructuring( + disposition: DestructuringDisposition, + filePath: string | undefined, + bindingNames: string[], +): never { const reason = disposition === "reject-pattern" ? "the binding pattern evaluates a default value, a computed key, or unsupported syntax" : "the initializer or one of its arguments may run unproven client side effects"; throw SERVER_ONLY_IN_CLIENT.create({ message: `Cannot safely remove module-scope destructuring used by a server-only hook because ${reason}. ` + + `Module: ${filePath ?? "this module"}. ` + + `Bindings: ${bindingNames.join(", ") || "unknown"}. ` + "Move the destructuring into getServerData or the server-only hook that uses it. " + "Declare client initialization separately.\n\n" + 'import { getEnv } from "veryfront";\n\n' + @@ -1692,6 +1698,7 @@ function dropUnusedModuleScopeBindings( pinned: Set, ): Node[] { let current = body; + const trustedBindings = knownServerImportBindings(body); for (;;) { const { declarations: decls, destructured } = analyzeModuleScopeDeclarations(current); @@ -1710,7 +1717,6 @@ function dropUnusedModuleScopeBindings( for (const registration of nameRegistrations) excluded.add(registration.target); const referenced = referencedIdentifiers(current, excluded); - const trustedBindings = knownServerImportBindings(current); const destructuringDecisions = new Map(); for (const decl of destructured) { const disposition = decideDestructuringDisposition({ @@ -1724,7 +1730,13 @@ function dropUnusedModuleScopeBindings( pinned, }); if (disposition === "reject-initializer" || disposition === "reject-pattern") { - throwUnsafeServerDestructuring(disposition); + throwUnsafeServerDestructuring( + disposition, + filePath, + decl.analysis.bindingIds.map(nodeName).filter( + (name): name is string => Boolean(name), + ), + ); } destructuringDecisions.set(decl.declarator, disposition); }