From 23f03ee1b088a200ff0c1fc819a57c16108b442c Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 12 Sep 2026 06:52:04 +0800 Subject: [PATCH] fix(exports): keep private output outside source checkouts --- .gitignore | 2 + CHANGELOG.md | 1 + README.md | 5 +- docs/npm-comparison.md | 7 ++- scripts/lib/npm-quality.mjs | 8 +-- scripts/lib/private-export-path.mjs | 74 +++++++++++++++++++++++ scripts/lib/telemetry-history.mjs | 10 ++- test/helpers/private-export-checkouts.mjs | 52 ++++++++++++++++ test/npm-quality.test.mjs | 57 ++++++++++++++++- test/private-export-path.test.mjs | 74 +++++++++++++++++++++++ test/telemetry-history.test.mjs | 53 +++++++++++++++- 11 files changed, 324 insertions(+), 19 deletions(-) create mode 100644 scripts/lib/private-export-path.mjs create mode 100644 test/helpers/private-export-checkouts.mjs create mode 100644 test/private-export-path.test.mjs diff --git a/.gitignore b/.gitignore index d833692..b85a956 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ node_modules/ .wrangler/ .dev.vars dist/ +/captures/ +/results/ diff --git a/CHANGELOG.md b/CHANGELOG.md index d95ea25..8209aaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Keep offline exports outside their executing source checkout, linked worktrees, and input archives, including differently cased paths on case-insensitive filesystems. - Remove the public statistics dashboard and `/api/stats` endpoint while preserving the privacy page, update checks, analytics recording, and data retention. **Highlights:** Public update checks with optional feature statistics and no stored install identifiers. diff --git a/README.md b/README.md index 7b797ce..3dc9e76 100644 --- a/README.md +++ b/README.md @@ -262,8 +262,9 @@ npm run telemetry:history -- \ --output /private/exports/hourly-history ``` -The output parent must already exist. Output must be outside the archive, with no symlinks or -path traversal. New directories are mode `0700`; files are `0600`. A rerun returns `unchanged` +The output parent must already exist. Output must be outside the archive and this source checkout +or its linked worktrees, with no symlinks or path traversal. Unrelated private repositories remain +supported output destinations. New directories are mode `0700`; files are `0600`. A rerun returns `unchanged` only after verifying every existing artifact byte-for-byte. Conflicting, incomplete, or non-private destinations fail without overwrite. Source files are never changed. diff --git a/docs/npm-comparison.md b/docs/npm-comparison.md index 0ed49b2..104b522 100644 --- a/docs/npm-comparison.md +++ b/docs/npm-comparison.md @@ -3,13 +3,14 @@ Use Node.js 24 and the repository's existing npm dependencies: ```sh -npm run npm:quality -- --manifest ./captures/manifest.json --output ./results/run-01 +npm run npm:quality -- --manifest /private/captures/manifest.json --output /private/results/run-01 ``` This command reads saved JSON only. It does not collect data, fall back to a network service, schedule work, publish results, or load the Worker. The output -directory must not exist, must be outside the input archive, and must have an -existing parent. Resolve filesystem aliases before passing paths: symlink files +directory must not exist, must be outside the input archive and this source checkout +or its linked worktrees, and must have an existing parent. Unrelated private repositories +remain supported output destinations. Resolve filesystem aliases before passing paths: symlink files and symlink parent directories are rejected. Each successful run creates `quality.json`, `quality.md`, and `input-hashes.json` diff --git a/scripts/lib/npm-quality.mjs b/scripts/lib/npm-quality.mjs index 00b0a91..51aceb2 100644 --- a/scripts/lib/npm-quality.mjs +++ b/scripts/lib/npm-quality.mjs @@ -11,6 +11,7 @@ import { } from "node:fs"; import { dirname, isAbsolute, join, parse, relative, resolve, sep } from "node:path"; import { TextDecoder } from "node:util"; +import { isTelemetryCheckoutPath, isWithinDirectory } from "./private-export-path.mjs"; const DAY = 86_400_000; const SHA = /^[a-f0-9]{64}$/u; @@ -1031,11 +1032,8 @@ function writeOutputs(output, files, inputRoot) { noTraversal(output); const absolute = resolve(output); const parent = realDirectory(dirname(absolute)); - const rel = relative(inputRoot, absolute); - requireValue( - rel === ".." || rel.startsWith(`..${sep}`) || isAbsolute(rel), - "output must be outside the input archive", - ); + requireValue(!isTelemetryCheckoutPath(absolute), "output must be outside telemetry source checkouts"); + requireValue(!isWithinDirectory(absolute, inputRoot), "output must be outside the input archive"); mkdirSync(absolute, { mode: 0o700 }); const claimed = lstatSync(absolute); requireValue( diff --git a/scripts/lib/private-export-path.mjs b/scripts/lib/private-export-path.mjs new file mode 100644 index 0000000..d8ad51b --- /dev/null +++ b/scripts/lib/private-export-path.mjs @@ -0,0 +1,74 @@ +import { lstatSync, readFileSync, realpathSync, statSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const SOURCE_ROOT = fileURLToPath(new URL("../../", import.meta.url)); + +function gitPathFile(file) { + const stat = statSync(file); + if (!stat.isFile() || stat.size > 1024 * 1024) throw new Error("invalid Git metadata"); + const value = readFileSync(file, "utf8").replace(/[\r\n]+$/, ""); + if (!value || value.includes("\0")) throw new Error("invalid Git metadata"); + return value; +} + +function gitCommonDirectory(directory) { + const marker = join(directory, ".git"); + try { + lstatSync(marker); + } catch (error) { + if (error.code === "ENOENT" || error.code === "ENOTDIR") return undefined; + throw error; + } + // Read Git's on-disk markers so offline exports need no Git executable or env overrides. + let gitDirectory = marker; + if (!statSync(marker).isDirectory()) { + const pointer = gitPathFile(marker); + if (!pointer.startsWith("gitdir: ") || pointer.length <= 8) throw new Error("invalid Git metadata"); + gitDirectory = resolve(directory, pointer.slice(8)); + } + gitDirectory = realpathSync(gitDirectory); + const commonFile = join(gitDirectory, "commondir"); + try { + lstatSync(commonFile); + } catch (error) { + if (error.code === "ENOENT") return gitDirectory; + throw error; + } + return realpathSync(resolve(gitDirectory, gitPathFile(commonFile))); +} + +function* ancestors(path) { + for (let current = resolve(path);; current = dirname(current)) { + yield current; + if (current === dirname(current)) return; + } +} + +function sameDirectory(path, expected) { + const actual = statSync(path, { bigint: true, throwIfNoEntry: false }); + // realpath can preserve case aliases; lowering case would conflate distinct Unix directories. + return actual?.isDirectory() && actual.dev === expected.dev && actual.ino === expected.ino; +} + +export function isWithinDirectory(output, root) { + const expected = statSync(root, { bigint: true }); + for (const current of ancestors(output)) { + if (sameDirectory(current, expected)) return true; + } + return false; +} + +/** Private results must not enter the executing source checkout or its linked worktrees. */ +export function isTelemetryCheckoutPath(output, sourceRoot = SOURCE_ROOT) { + if (isWithinDirectory(output, sourceRoot)) return true; + const common = gitCommonDirectory(sourceRoot); + if (!common) return false; + const expected = statSync(common, { bigint: true }); + for (const current of ancestors(output)) { + const candidate = gitCommonDirectory(current); + if (candidate && sameDirectory(candidate, expected)) return true; + // A nested foreign repository cannot hide a containing telemetry checkout. + } + return false; +} diff --git a/scripts/lib/telemetry-history.mjs b/scripts/lib/telemetry-history.mjs index 50ae2b0..ad74079 100644 --- a/scripts/lib/telemetry-history.mjs +++ b/scripts/lib/telemetry-history.mjs @@ -10,8 +10,9 @@ import { readSync, writeFileSync, } from "node:fs"; -import { dirname, isAbsolute, join, parse, relative, resolve, sep } from "node:path"; +import { dirname, join, parse, relative, resolve, sep } from "node:path"; import { TextDecoder } from "node:util"; +import { isTelemetryCheckoutPath, isWithinDirectory } from "./private-export-path.mjs"; const HOUR = 3_600_000; const DAY = 24 * HOUR; @@ -405,11 +406,8 @@ function writeOutputs(output, files, root) { noTraversal(output); const absolute = resolve(output); realDirectory(dirname(absolute)); - const rel = relative(root, absolute); - requireValue( - rel === ".." || rel.startsWith(`..${sep}`) || isAbsolute(rel), - "output must be outside the input archive", - ); + requireValue(!isTelemetryCheckoutPath(absolute), "output must be outside telemetry source checkouts"); + requireValue(!isWithinDirectory(absolute, root), "output must be outside the input archive"); let existing = false; try { mkdirSync(absolute, { mode: 0o700 }); diff --git a/test/helpers/private-export-checkouts.mjs b/test/helpers/private-export-checkouts.mjs new file mode 100644 index 0000000..d26bafa --- /dev/null +++ b/test/helpers/private-export-checkouts.mjs @@ -0,0 +1,52 @@ +import { execFileSync } from "node:child_process"; +import { copyFileSync, mkdtempSync, mkdirSync, realpathSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +/** Real Git fixtures keep checkout detection independent of the developer's repository. */ +export function privateExportCheckouts() { + const root = realpathSync(mkdtempSync(join(tmpdir(), "telemetry-export-checkouts-"))); + const source = join(root, "source"); + const linked = join(root, "linked"); + const unrelated = join(root, "private-repository"); + const env = Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_")), + ); + function git(directory, ...args) { + execFileSync( + "git", + [ + "-C", directory, + "-c", "user.name=Telemetry Test", + "-c", "user.email=test@example.invalid", + "-c", "commit.gpgsign=false", + "-c", "core.hooksPath=/dev/null", + ...args, + ], + { env, stdio: ["ignore", "pipe", "pipe"] }, + ); + } + function repository(directory) { + mkdirSync(directory); + git(directory, "init", "--initial-branch=main"); + } + try { + repository(source); + mkdirSync(join(source, "scripts", "lib"), { recursive: true }); + for (const file of ["private-export-path.mjs", "npm-quality.mjs", "telemetry-history.mjs"]) { + copyFileSync( + new URL(`../../scripts/lib/${file}`, import.meta.url), + join(source, "scripts", "lib", file), + ); + } + git(source, "add", "scripts"); + git(source, "commit", "-m", "test fixture"); + git(source, "worktree", "add", "--detach", linked, "HEAD"); + repository(unrelated); + for (const checkout of [source, linked]) repository(join(checkout, "nested-repository")); + return { root, source, linked, unrelated }; + } catch (error) { + rmSync(root, { recursive: true, force: true }); + throw error; + } +} diff --git a/test/npm-quality.test.mjs b/test/npm-quality.test.mjs index bd10ab0..ed49d2c 100644 --- a/test/npm-quality.test.mjs +++ b/test/npm-quality.test.mjs @@ -1,6 +1,7 @@ import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; import { + existsSync, mkdtempSync, mkdirSync, readFileSync, @@ -16,9 +17,10 @@ import https from "node:https"; import net from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; import { afterEach, describe, expect, it, vi } from "vitest"; import { runNpmQuality } from "../scripts/lib/npm-quality.mjs"; +import { privateExportCheckouts } from "./helpers/private-export-checkouts.mjs"; const CLI = fileURLToPath(new URL("../scripts/npm-quality.mjs", import.meta.url)); const AT = "2026-09-07T04:54:09.343Z"; @@ -1195,6 +1197,55 @@ describe("manifest and resource boundaries", () => { }); describe("offline filesystem and CLI contract", () => { + it.each(["source", "linked"])("protects every checkout when running from %s", async (executing) => { + const checkouts = privateExportCheckouts(); + owned.push(checkouts.root); + const { runNpmQuality: runExport } = await import( + pathToFileURL(join(checkouts[executing], "scripts/lib/npm-quality.mjs")) + ); + const files = save(); + for (const checkout of [ + checkouts.source, checkouts.linked, + checkouts.source.toUpperCase(), checkouts.linked.toUpperCase(), + ].filter(existsSync)) { + for (const parent of [checkout, join(checkout, "nested-repository")]) { + files.output = join(parent, "results"); + expect(() => runExport(files)).toThrow(/outside telemetry source checkouts/); + expect(existsSync(files.output)).toBe(false); + } + } + for (const parent of [checkouts.root, checkouts.unrelated]) { + files.output = join(parent, "results"); + expect(runExport(files).summary.identities).toBe(1); + expect(statSync(files.output).mode & 0o777).toBe(0o700); + for (const name of readdirSync(files.output)) { + expect(statSync(join(files.output, name)).mode & 0o777).toBe(0o600); + } + } + }); + + it("preserves the archive boundary with filesystem case semantics", () => { + const files = save(); + const alias = join(dirname(files.archive), "ARCHIVE"); + const shared = existsSync(alias); + if (!shared) mkdirSync(alias); + files.output = join(alias, "results"); + if (shared) { + expect(() => runNpmQuality(files)).toThrow(/outside the input archive/); + expect(existsSync(files.output)).toBe(false); + } else { + expect(runNpmQuality(files).summary.identities).toBe(1); + } + }); + + it("rejects results inside the executing source checkout before creating files", () => { + const files = save(); + files.output = join(dirname(dirname(CLI)), `npm-quality-test-${digest(files.dir).slice(0, 16)}`); + owned.push(files.output); + expect(() => runNpmQuality(files)).toThrow(/outside telemetry source checkouts/); + expect(existsSync(files.output)).toBe(false); + }); + it("writes exact-byte digests and exclusive private outputs without network access", () => { const files = save(); const forbidden = vi.fn(() => { @@ -1250,7 +1301,9 @@ describe("offline filesystem and CLI contract", () => { symlinkSync(files.dir, join(files.dir, "linked")); files.output = join(files.dir, "linked", "result"); } else symlinkSync(files.archive, files.output); - expect(() => runNpmQuality(files)).toThrow(/symlink|already exists/); + expect(() => runNpmQuality(files)).toThrow( + kind === "output" ? /outside the input archive/ : /symlink|already exists/, + ); expect(readdirSync(files.archive)).not.toContain("quality.json"); }, ); diff --git a/test/private-export-path.test.mjs b/test/private-export-path.test.mjs new file mode 100644 index 0000000..6334b7b --- /dev/null +++ b/test/private-export-path.test.mjs @@ -0,0 +1,74 @@ +import { existsSync, mkdirSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { isTelemetryCheckoutPath, isWithinDirectory } from "../scripts/lib/private-export-path.mjs"; +import { privateExportCheckouts } from "./helpers/private-export-checkouts.mjs"; + +describe("private export destinations", () => { + let temporary; + let source; + let linked; + let unrelated; + + beforeAll(() => { + ({ root: temporary, source, linked, unrelated } = privateExportCheckouts()); + }); + afterAll(() => rmSync(temporary, { recursive: true, force: true })); + afterEach(() => vi.unstubAllEnvs()); + + it.each(["root", "child", "nested foreign repository"])( + "rejects the source checkout's %s destination", + (kind) => { + const output = kind === "root" + ? source + : join(source, kind === "child" ? "results" : "nested-repository/results"); + expect(isTelemetryCheckoutPath(output, source)).toBe(true); + }, + ); + + it.each(["root", "child", "nested foreign repository"])( + "rejects a linked worktree's %s destination from either source checkout", + (kind) => { + const output = kind === "root" + ? linked + : join(linked, kind === "child" ? "results" : "nested-repository/results"); + expect(isTelemetryCheckoutPath(output, source)).toBe(true); + expect(isTelemetryCheckoutPath(join(source, "results"), linked)).toBe(true); + }, + ); + + it("permits private output and unrelated private repositories", () => { + for (const output of [join(temporary, "private-output"), unrelated, join(unrelated, "results")]) { + expect(isTelemetryCheckoutPath(output, source)).toBe(false); + expect(isTelemetryCheckoutPath(output, linked)).toBe(false); + } + }); + + it.each(["source", "linked"])("respects filesystem case semantics for %s", (name) => { + const alias = join(temporary, name.toUpperCase()); + const shared = existsSync(alias); + if (!shared) mkdirSync(alias); + for (const root of [source, linked]) { + expect(isTelemetryCheckoutPath(join(alias, "results"), root)).toBe(shared); + } + }); + + it("protects unpacked source without Git metadata, including physical aliases", () => { + const unpacked = join(temporary, "unpacked"); + const alias = join(temporary, "UNPACKED"); + mkdirSync(unpacked); + const shared = existsSync(alias); + if (!shared) mkdirSync(alias); + expect(isTelemetryCheckoutPath(join(unpacked, "results"), unpacked)).toBe(true); + expect(isTelemetryCheckoutPath(join(alias, "results"), unpacked)).toBe(shared); + expect(isWithinDirectory(join(alias, "results"), unpacked)).toBe(shared); + }); + + it("works without Git on PATH and ignores inherited Git overrides", () => { + vi.stubEnv("PATH", ""); + vi.stubEnv("GIT_DIR", join(unrelated, ".git")); + vi.stubEnv("GIT_COMMON_DIR", join(unrelated, ".git")); + expect(isTelemetryCheckoutPath(join(linked, "results"), source)).toBe(true); + expect(isTelemetryCheckoutPath(join(unrelated, "results"), linked)).toBe(false); + }); +}); diff --git a/test/telemetry-history.test.mjs b/test/telemetry-history.test.mjs index d710896..c37072c 100644 --- a/test/telemetry-history.test.mjs +++ b/test/telemetry-history.test.mjs @@ -16,9 +16,10 @@ import { import net from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; import { afterEach, describe, expect, it, vi } from "vitest"; import { runTelemetryHistory } from "../scripts/lib/telemetry-history.mjs"; +import { privateExportCheckouts } from "./helpers/private-export-checkouts.mjs"; const CLI = fileURLToPath(new URL("../scripts/telemetry-history.mjs", import.meta.url)); const HOUR = 3_600_000; @@ -513,6 +514,56 @@ describe("capture validation", () => { }); describe("offline private output and CLI", () => { + it.each(["source", "linked"])("protects every checkout when running from %s", async (executing) => { + const checkouts = privateExportCheckouts(); + owned.push(checkouts.root); + const { runTelemetryHistory: runExport } = await import( + pathToFileURL(join(checkouts[executing], "scripts/lib/telemetry-history.mjs")) + ); + const files = save(); + for (const checkout of [ + checkouts.source, checkouts.linked, + checkouts.source.toUpperCase(), checkouts.linked.toUpperCase(), + ].filter(existsSync)) { + for (const parent of [checkout, join(checkout, "nested-repository")]) { + files.output = join(parent, "results"); + expect(() => runExport(files)).toThrow(/outside telemetry source checkouts/); + expect(existsSync(files.output)).toBe(false); + } + } + for (const parent of [checkouts.root, checkouts.unrelated]) { + files.output = join(parent, "results"); + expect(runExport(files).status).toBe("written"); + expect(runExport(files).status).toBe("unchanged"); + expect(statSync(files.output).mode & 0o777).toBe(0o700); + for (const name of readdirSync(files.output)) { + expect(statSync(join(files.output, name)).mode & 0o777).toBe(0o600); + } + } + }); + + it("preserves the archive boundary with filesystem case semantics", () => { + const files = save(); + const alias = join(dirname(files.archive), "ARCHIVE"); + const shared = existsSync(alias); + if (!shared) mkdirSync(alias); + files.output = join(alias, "results"); + if (shared) { + expect(() => runTelemetryHistory(files)).toThrow(/outside the input archive/); + expect(existsSync(files.output)).toBe(false); + } else { + expect(runTelemetryHistory(files).status).toBe("written"); + } + }); + + it("rejects results inside the executing source checkout before creating files", () => { + const files = save(); + files.output = join(dirname(dirname(CLI)), `telemetry-history-test-${digest(files.root).slice(0, 16)}`); + owned.push(files.output); + expect(() => runTelemetryHistory(files)).toThrow(/outside telemetry source checkouts/); + expect(existsSync(files.output)).toBe(false); + }); + it("writes deterministic private artifacts, omits freeform metadata, and verifies idempotent reuse", () => { const files = save(); const forbidden = vi.fn(() => {