From 6c18ff44181d97a10966523cf9d58f245d33a2c0 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 29 Sep 2026 13:46:32 -0500 Subject: [PATCH] fix(runner): parse import gates and pack explicit consumer inputs Use the declared TypeScript parser to distinguish module references from fixture strings. Enforce declared public exports and the ADR0001 private development-only eval harness boundary, including public transitive exposure. Pack absolute dependency inputs from scratch without adopting publisher development toolchains. Co-Authored-By: Paperclip --- .../scripts/check-clean-consumers.mjs | 5 +- .../scripts/check-forbidden-imports.test.mjs | 131 ++++++++++++++++ .../scripts/check-tracked-imports.test.mjs | 26 ++++ .../scripts/lib/forbidden-imports.mjs | 144 +++++++++++++----- .../scripts/lib/tracked-imports.mjs | 11 +- 5 files changed, 276 insertions(+), 41 deletions(-) diff --git a/packages/paperclip-runner/scripts/check-clean-consumers.mjs b/packages/paperclip-runner/scripts/check-clean-consumers.mjs index 65b9310404e..bc4ee1ce9e2 100644 --- a/packages/paperclip-runner/scripts/check-clean-consumers.mjs +++ b/packages/paperclip-runner/scripts/check-clean-consumers.mjs @@ -78,7 +78,10 @@ try { async function pack(packageRoot, destination) { const before = new Set(await readdir(destination)); - run("npm", ["pack", "--ignore-scripts", "--pack-destination", destination], packageRoot, { quiet: true }); + // These installed dependencies are pack inputs, not our development cwd. + // Keep their publisher devEngines intact without adopting their toolchain; + // normal tarball contents and clean-consumer runtime engine checks still apply. + run("npm", ["pack", resolve(packageRoot), "--ignore-scripts", "--pack-destination", destination], destination, { quiet: true }); const created = (await readdir(destination)) .filter((entry) => entry.endsWith(".tgz") && !before.has(entry)) .sort(); diff --git a/packages/paperclip-runner/scripts/check-forbidden-imports.test.mjs b/packages/paperclip-runner/scripts/check-forbidden-imports.test.mjs index f4dc12e447c..59a620f4f93 100644 --- a/packages/paperclip-runner/scripts/check-forbidden-imports.test.mjs +++ b/packages/paperclip-runner/scripts/check-forbidden-imports.test.mjs @@ -1,9 +1,13 @@ import assert from "node:assert/strict"; import { test } from "node:test"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, resolve } from "node:path"; import { checkForbiddenImports, defaultPackageRoot, + findSpecifiers, } from "./lib/forbidden-imports.mjs"; test("the package passes its standalone boundary", async () => { @@ -59,3 +63,130 @@ test("a negative fixture proves that an SDK consumer deep import is rejected", a assert.equal(violations.length, 1); assert.match(violations[0].reason, /may not deep-import/); }); + +test("the parser ignores embedded fixtures but preserves executable imports and types", () => { + const source = [ + '// import "comment";', + '/* export * from "comment-export"; */', + 'const fixture = "import value from \\"fixture\\"";', + 'const template = `require("fixture-require"); import("fixture-dynamic")`;', + 'const pattern = /import\\("fixture-regex"\\)/;', + 'import "side-effect";', + 'import type { T } from "types";', + 'export { value } from "barrel";', + 'export * from "star";', + 'import equal = require("equal");', + 'type Imported = import("import-type").T;', + 'const cjs = require("cjs");', + 'const dynamic = import("dynamic", { with: { type: "json" } });', + 'const literal = import(`static-template`);', + 'const escaped = import("escaped\\u002dname");', + 'const interpolation = `text ${import("executed-interpolation")}`;', + ].join("\n"); + const found = findSpecifiers(source); + assert.deepEqual(found.map(({ specifier }) => specifier), [ + "side-effect", "types", "barrel", "star", "equal", "import-type", "cjs", + "dynamic", "static-template", "escaped-name", "executed-interpolation", + ]); + assert.deepEqual(found.map(({ offset }) => source.slice(0, offset).split("\n").length), + [6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]); +}); + +test("the parser treats JSX fixture text separately from expression imports", () => { + assert.deepEqual(findSpecifiers( + '
require("text") {import("executed")}
', + "fixture.tsx", + ).map(({ specifier }) => specifier), ["executed"]); +}); + +test("Rust include macros and path modules retain their boundary checks", () => { + assert.deepEqual(findSpecifiers([ + 'include!("../included.rs");', + 'include_str!("../text.txt");', + 'include_bytes!("../bytes.bin");', + '#[path = "../module.rs"] mod module;', + ].join("\n"), "source.rs").map(({ specifier }) => specifier), + ["../included.rs", "../text.txt", "../bytes.bin", "../module.rs"]); +}); + +async function fixtureBoundary(t, { manifest = {}, files = {} } = {}) { + const root = await mkdtemp(resolve(tmpdir(), "runner-import-boundary-")); + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(resolve(root, "src"), { recursive: true }); + await writeFile(resolve(root, "package.json"), JSON.stringify(manifest)); + for (const [file, source] of Object.entries(files)) { + await mkdir(dirname(resolve(root, file)), { recursive: true }); + await writeFile(resolve(root, file), source); + } + return checkForbiddenImports({ packageRoot: root, scanRoots: ["src"], cargoRoots: [] }); +} + +const kernel = "@paperclipai/paperclip-eval-kernel"; +const harness = "src/eval/workflow-harness.ts"; +const reviewedManifest = { + exports: { "./evals": "./dist/evals.js" }, + devDependencies: { [kernel]: "workspace:*" }, +}; + +test("only exact declared public subpaths are admitted", async (t) => { + const violations = await fixtureBoundary(t, { + manifest: reviewedManifest, + files: { "src/consumer.ts": [ + 'import "@paperclipai/paperclip-runner/evals";', + 'import "@paperclipai/paperclip-runner/evals/private";', + 'import "@paperclipai/paperclip-runner/undeclared";', + ].join("\n") }, + }); + assert.deepEqual(violations.map(({ specifier }) => specifier), [ + "@paperclipai/paperclip-runner/evals/private", "@paperclipai/paperclip-runner/undeclared", + ]); +}); + +test("ADR0001 permits only the private development harness and reviewed package", async (t) => { + assert.deepEqual(await fixtureBoundary(t, { + manifest: reviewedManifest, + files: { [harness]: `import { scenario } from "${kernel}";`, "src/evals.ts": "export const publicApi = true;" }, + }), []); + for (const [file, specifier] of [ + ["src/other.ts", kernel], [harness, `${kernel}/private`], [harness, "@paperclipai/other"], + ]) { + const violations = await fixtureBoundary(t, { + manifest: reviewedManifest, files: { [file]: `import "${specifier}";` }, + }); + assert.equal(violations.length, 1); + assert.equal(violations[0].specifier, specifier); + } + assert.equal((await fixtureBoundary(t, { + files: { [harness]: `import "${kernel}";` }, + })).length, 1, "the exception requires the reviewed devDependency declaration"); +}); + +test("the reviewed kernel is still forbidden in every production dependency group", async (t) => { + for (const group of ["dependencies", "optionalDependencies", "peerDependencies"]) { + const violations = await fixtureBoundary(t, { + manifest: { ...reviewedManifest, [group]: { [kernel]: "workspace:*" } }, + }); + assert.equal(violations.length, 1, group); + assert.equal(violations[0].specifier, kernel); + } +}); + +test("direct or transitive public harness exposure revokes the dev-only exception", async (t) => { + for (const entry of [ + "./dist/eval/workflow-harness.js", + "./dist/evals.js", + ]) { + for (const imported of ["./eval/workflow-harness.js", "./eval/workflow-harness"]) { + const violations = await fixtureBoundary(t, { + manifest: { ...reviewedManifest, exports: { "./evals": { types: "./dist/evals.d.ts", import: entry } } }, + files: { + [harness]: `import "${kernel}";`, + "src/evals.ts": 'export * from "./barrel.js";', + "src/barrel.ts": `export * from "${imported}";`, + }, + }); + assert.equal(violations.length, 1, `${entry} via ${imported}`); + assert.equal(violations[0].specifier, kernel); + } + } +}); diff --git a/packages/paperclip-runner/scripts/check-tracked-imports.test.mjs b/packages/paperclip-runner/scripts/check-tracked-imports.test.mjs index 79778857de3..66151ddc837 100644 --- a/packages/paperclip-runner/scripts/check-tracked-imports.test.mjs +++ b/packages/paperclip-runner/scripts/check-tracked-imports.test.mjs @@ -1,5 +1,8 @@ import assert from "node:assert/strict"; import { resolve } from "node:path"; +import { spawnSync } from "node:child_process"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { test } from "node:test"; import { @@ -71,3 +74,26 @@ test("the fixture itself is tracked so the package-wide check stays green", asyn assert.ok(tracked.has(resolve(defaultPackageRoot, fixtureRoot, name)), name); } }); + + +test("embedded fixture imports are ignored while real missing imports still fail", async (t) => { + const root = await mkdtemp(resolve(tmpdir(), "runner-tracked-imports-")); + t.after(() => rm(root, { recursive: true, force: true })); + const initialized = spawnSync("git", ["init", "--quiet", root], { encoding: "utf8" }); + assert.equal(initialized.status, 0, initialized.stderr); + await mkdir(resolve(root, "src")); + const consumer = resolve(root, "src/consumer.mts"); + await writeFile(consumer, [ + 'const fixture = `import "./fixture-only.js"; require("./other-fixture.js")`;', + 'const actual = import("./missing.js", { with: { type: "json" } });', + 'export { tracked } from "./tracked.js";', + ].join("\n")); + const tracked = resolve(root, "src/tracked.ts"); + await writeFile(tracked, "export const tracked = true;"); + const violations = await checkTrackedImports({ + packageRoot: root, scanRoots: ["src"], trackedFiles: [consumer, tracked], + }); + assert.deepEqual(violations.map(({ specifier, line, reason }) => ({ specifier, line, reason })), [{ + specifier: "./missing.js", line: 2, reason: "does not resolve to any tracked file", + }]); +}); diff --git a/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs b/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs index 5132fe4519f..2fda4a5377a 100644 --- a/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs +++ b/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs @@ -1,15 +1,17 @@ import { readFile, readdir } from "node:fs/promises"; import { dirname, isAbsolute, relative, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { API } from "typescript/unstable/sync"; +import { createVirtualFileSystem } from "typescript/unstable/fs"; +import { SyntaxKind } from "typescript/unstable/ast"; -const SOURCE_EXTENSIONS = new Set([".cjs", ".js", ".jsx", ".mjs", ".rs", ".ts", ".tsx"]); -const IMPORT_PATTERNS = [ - /\b(?:import|export)\s+(?:type\s+)?(?:[^"'`;]{0,500}?\s+from\s+)?["']([^"']+)["']/g, - /\bimport\s*\(\s*["']([^"']+)["']\s*\)/g, - /\brequire\s*\(\s*["']([^"']+)["']\s*\)/g, +const SOURCE_EXTENSIONS = new Set([".cjs", ".js", ".jsx", ".mjs", ".mts", ".cts", ".rs", ".ts", ".tsx"]); +const RUST_IMPORT_PATTERNS = [ /\b(?:include|include_str|include_bytes)!\s*\(\s*["']([^"']+)["']\s*\)/g, /#\[path\s*=\s*["']([^"']+)["']\]/g, ]; +const REVIEWED_EVAL_KERNEL = "@paperclipai/paperclip-eval-kernel"; +const REVIEWED_EVAL_HARNESS = "src/eval/workflow-harness.ts"; const libraryDirectory = dirname(fileURLToPath(import.meta.url)); export const defaultPackageRoot = resolve(libraryDirectory, "../.."); @@ -44,15 +46,92 @@ async function collectSourceFiles(target) { return files; } -export function findSpecifiers(source) { - const found = []; - for (const pattern of IMPORT_PATTERNS) { - pattern.lastIndex = 0; - for (let match = pattern.exec(source); match !== null; match = pattern.exec(source)) { - found.push({ specifier: match[1], offset: match.index }); +/** Parse a whole scan in one compiler session; never execute scanned source. */ +export function findSpecifiersInFiles(files) { + const results = new Map(files.map(({ file }) => [file, []])); + const javascript = files.filter(({ file }) => extension(file) !== ".rs"); + for (const { file, source } of files.filter(({ file }) => extension(file) === ".rs")) { + for (const pattern of RUST_IMPORT_PATTERNS) { + pattern.lastIndex = 0; + for (const match of source.matchAll(pattern)) { + results.get(file).push({ specifier: match[1], offset: match.index }); + } } } - return found; + if (javascript.length === 0) return results; + const root = "/__paperclip_import_check__"; + const names = javascript.map(({ file }, index) => `input-${index}${extension(file) || ".ts"}`); + const virtualFiles = Object.fromEntries(javascript.map(({ source }, index) => [`${root}/${names[index]}`, source])); + virtualFiles[`${root}/tsconfig.json`] = JSON.stringify({ + compilerOptions: { noLib: true, noResolve: true, allowJs: true }, files: names, + }); + const api = new API({ fs: createVirtualFileSystem(virtualFiles) }); + try { + const snapshot = api.updateSnapshot({ openProjects: [`${root}/tsconfig.json`] }); + const program = snapshot.getProject(`${root}/tsconfig.json`).program; + for (const [index, { file }] of javascript.entries()) { + const sourceFile = program.getSourceFile(`${root}/${names[index]}`); + if (!sourceFile) throw new Error(`Import scanner could not parse ${file}`); + const found = results.get(file); + function add(literal) { + if (literal?.kind === SyntaxKind.StringLiteral || literal?.kind === SyntaxKind.NoSubstitutionTemplateLiteral) { + found.push({ specifier: literal.text, offset: literal.getStart(sourceFile) }); + } + } + function visit(node) { + if (node.kind === SyntaxKind.ImportDeclaration || node.kind === SyntaxKind.ExportDeclaration) add(node.moduleSpecifier); + else if (node.kind === SyntaxKind.ImportEqualsDeclaration && node.moduleReference.kind === SyntaxKind.ExternalModuleReference) add(node.moduleReference.expression); + else if (node.kind === SyntaxKind.ImportType && node.argument.kind === SyntaxKind.LiteralType) add(node.argument.literal); + else if (node.kind === SyntaxKind.CallExpression && ( + node.expression.kind === SyntaxKind.ImportKeyword || + (node.expression.kind === SyntaxKind.Identifier && node.expression.text === "require") + )) add(node.arguments[0]); + node.forEachChild(visit); + } + visit(sourceFile); + } + } finally { + api.close(); + } + return results; +} + +export function findSpecifiers(source, file = "source.ts") { + return findSpecifiersInFiles([{ file, source }]).get(file); +} + +function declaredPublicImports(manifest) { + return new Set(Object.entries(manifest.exports ?? {}) + .filter(([key, target]) => key.startsWith("./") && !key.includes("*") && target !== null) + .map(([key]) => `@paperclipai/paperclip-runner/${key.slice(2)}`)); +} + +function publicSourceGraph(packageRoot, manifest, specifiers) { + const known = new Set(specifiers.keys()); + const sourceCandidates = (path) => { + const source = path.replace(/\/dist\//, "/src/"); + const stem = source.replace(/(?:\.d)?\.(?:[cm]?js|jsx|[cm]?ts|tsx)$/, ""); + // Follow every matching source candidate conservatively, including barrels + // and extensionless imports. A public graph must never hide the dev harness. + return [source, ...[".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs", "/index.ts", "/index.tsx", "/index.js"] + .map((suffix) => `${stem}${suffix}`)].filter((candidate) => known.has(candidate)); + }; + const targets = (value) => typeof value === "string" ? [value] : value && typeof value === "object" ? Object.values(value).flatMap(targets) : []; + const exportSources = (value) => targets(value).flatMap((target) => sourceCandidates(resolve(packageRoot, target))); + const queue = exportSources(manifest.exports); + const reachable = new Set(); + while (queue.length > 0) { + const file = queue.pop(); + if (reachable.has(file)) continue; + reachable.add(file); + for (const { specifier } of specifiers.get(file) ?? []) { + if (specifier.startsWith(".")) queue.push(...sourceCandidates(resolve(dirname(file), specifier))); + else if (specifier.startsWith("@paperclipai/paperclip-runner/")) { + queue.push(...exportSources(manifest.exports?.[`./${specifier.slice("@paperclipai/paperclip-runner/".length)}`])); + } + } + } + return reachable; } // The Live console component decision record adapts shadcn/ui and AI Elements @@ -79,16 +158,9 @@ function isForbiddenBrowserPackage(specifier) { ); } -function violationReason({ file, packageRoot, specifier }) { +function violationReason({ file, packageRoot, specifier, publicRunnerImports, reviewedEvalHarness }) { const relativeFile = relative(packageRoot, file).split(/[\\/]/).join("/"); const isExampleConsumer = relativeFile.startsWith("examples/"); - const publicRunnerImports = new Set([ - "@paperclipai/paperclip-runner/browser", - "@paperclipai/paperclip-runner/react", - "@paperclipai/paperclip-runner/standalone", - "@paperclipai/paperclip-runner/testing", - "@paperclipai/paperclip-runner/styles.css", - ]); if ( specifier.startsWith("@paperclipai/paperclip-runner/") && !publicRunnerImports.has(specifier) @@ -101,7 +173,8 @@ function violationReason({ file, packageRoot, specifier }) { if ( specifier.startsWith("@paperclipai/") && specifier !== "@paperclipai/paperclip-runner" && - !publicRunnerImports.has(specifier) + !publicRunnerImports.has(specifier) && + !(specifier === REVIEWED_EVAL_KERNEL && reviewedEvalHarness && relativeFile === REVIEWED_EVAL_HARNESS) ) { return "Paperclip workspace packages are outside the standalone boundary"; } @@ -152,7 +225,7 @@ async function manifestViolations(packageRoot) { const unreviewedDevelopmentDependencies = Object.keys( manifest.devDependencies ?? {}, ).filter( - (name) => name.startsWith("@paperclipai/"), + (name) => name.startsWith("@paperclipai/") && !(name === REVIEWED_EVAL_KERNEL && manifest.devDependencies[name] === "workspace:*"), ); return [...runtimeDependencies, ...unreviewedDevelopmentDependencies] .filter( @@ -221,19 +294,20 @@ export async function checkForbiddenImports({ files.push(...(await collectSourceFiles(resolve(packageRoot, root)))); } - for (const file of files.sort()) { - const source = await readFile(file, "utf8"); - for (const { specifier, offset } of findSpecifiers(source)) { - const reason = violationReason({ file, packageRoot, specifier }); - if (reason === null) { - continue; - } - violations.push({ - file, - line: source.slice(0, offset).split("\n").length, - specifier, - reason, - }); + const manifest = JSON.parse(await readFile(resolve(packageRoot, "package.json"), "utf8")); + const sources = await Promise.all(files.sort().map(async (file) => ({ file, source: await readFile(file, "utf8") }))); + const specifiers = findSpecifiersInFiles(sources); + const publicRunnerImports = declaredPublicImports(manifest); + const publicSources = publicSourceGraph(packageRoot, manifest, specifiers); + // ADR0001 permits only this private development harness to use the kernel. + // A public export reaching it revokes the exception, including through barrels. + const reviewedEvalHarness = manifest.devDependencies?.[REVIEWED_EVAL_KERNEL] === "workspace:*" + && !publicSources.has(resolve(packageRoot, REVIEWED_EVAL_HARNESS)); + for (const { file, source } of sources) { + for (const { specifier, offset } of specifiers.get(file)) { + const reason = violationReason({ file, packageRoot, specifier, publicRunnerImports, reviewedEvalHarness }); + if (reason === null) continue; + violations.push({ file, line: source.slice(0, offset).split("\n").length, specifier, reason }); } } diff --git a/packages/paperclip-runner/scripts/lib/tracked-imports.mjs b/packages/paperclip-runner/scripts/lib/tracked-imports.mjs index d11dd14e93f..97a98efca86 100644 --- a/packages/paperclip-runner/scripts/lib/tracked-imports.mjs +++ b/packages/paperclip-runner/scripts/lib/tracked-imports.mjs @@ -4,13 +4,13 @@ import { readFile } from "node:fs/promises"; import { dirname, relative, resolve } from "node:path"; import { promisify } from "node:util"; -import { defaultPackageRoot, findSpecifiers } from "./forbidden-imports.mjs"; +import { defaultPackageRoot, findSpecifiersInFiles } from "./forbidden-imports.mjs"; export { defaultPackageRoot }; const execFileAsync = promisify(execFile); -const SCANNED_EXTENSIONS = new Set([".cjs", ".js", ".jsx", ".mjs", ".ts", ".tsx"]); +const SCANNED_EXTENSIONS = new Set([".cjs", ".js", ".jsx", ".mjs", ".mts", ".cts", ".ts", ".tsx"]); // TypeScript resolves an ESM ".js" specifier against its ".ts" source sibling, // so a tracked import may legitimately name a file that never exists on disk. const JS_TO_SOURCE_EXTENSIONS = new Map([ @@ -129,9 +129,10 @@ export async function checkTrackedImports({ .sort(); const suspects = []; - for (const file of files) { - const source = await readFile(file, "utf8"); - for (const { specifier, offset } of findSpecifiers(source)) { + const sources = await Promise.all(files.map(async (file) => ({ file, source: await readFile(file, "utf8") }))); + const specifiers = findSpecifiersInFiles(sources); + for (const { file, source } of sources) { + for (const { specifier, offset } of specifiers.get(file)) { if (!isRelative(specifier) || /[?*]|\$\{/.test(specifier)) { continue; }