From 5f2463d91318d12f994718f66d732ba58e20dbb8 Mon Sep 17 00:00:00 2001 From: liolevx <312117550+liolevx@users.noreply.github.com> Date: Thu, 3 Sep 2026 22:00:31 -0400 Subject: [PATCH 1/3] fix: refuse linked setup destinations before mutation --- docs/security.md | 7 ++- src/cli.ts | 4 +- src/core/init.ts | 29 ++++++++---- src/core/preview.ts | 13 +++++- src/security/paths.ts | 32 ++++++++++++++ tests/init-path-safety.test.ts | 80 ++++++++++++++++++++++++++++++++++ 6 files changed, 151 insertions(+), 14 deletions(-) create mode 100644 tests/init-path-safety.test.ts diff --git a/docs/security.md b/docs/security.md index a37770d..4c32d1f 100644 --- a/docs/security.md +++ b/docs/security.md @@ -12,8 +12,11 @@ full fixture tree before and after preview, use scripts that would leave a marke symlink escapes, and assert secret values never reach output. Package-manager retrieval, when a user chooses it after publication, occurs outside the runtime preview boundary. -Mutating setup shows complete patches, rechecks target absence, writes same-directory temporary -files with restrictive modes, renames them into place, and never overwrites existing content. +Mutating setup shows complete patches, rechecks target absence or the reviewed content hash, and +refuses symbolic links and junctions at write destinations or their ancestors. It checks all +destinations before writing, rechecks during application, and guards rollback paths too. Setup uses +same-directory temporary files with restrictive modes and preserves unmanaged content. These checks +are not an atomic filesystem transaction against a concurrent hostile process changing paths. Process execution uses direct executable/argument arrays, repository-contained working directories, timeouts, cancellation, a minimal environment, and output caps. diff --git a/src/cli.ts b/src/cli.ts index 0d17f5d..19521f7 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -544,7 +544,7 @@ export function createProgram(customIo?: Partial): Command { if (common.json) writeJson(io, { preview, applied: { created: [] }, refused: true }); process.exitCode = EXIT.refused; io.stderr( - "Initialization refused; resolve the reported instruction conflict. No files were changed.\n", + "Initialization refused; resolve the reported setup conflict. No files were changed.\n", ); return; } @@ -613,7 +613,7 @@ export function createProgram(customIo?: Partial): Command { writeJson(io, { summary, preview, applied: { created: [] }, refused: true }); process.exitCode = EXIT.refused; io.stderr( - "Synchronization refused; resolve the reported instruction conflict. No files were changed.\n", + "Synchronization refused; resolve the reported setup conflict. No files were changed.\n", ); return; } diff --git a/src/core/init.ts b/src/core/init.ts index d3f0b51..3adb294 100644 --- a/src/core/init.ts +++ b/src/core/init.ts @@ -2,7 +2,7 @@ import { createHash, randomUUID } from "node:crypto"; import { mkdir, readFile, rename, rm, rmdir, stat, writeFile } from "node:fs/promises"; import path from "node:path"; import type { PreviewResult, ProposedFile } from "../model.js"; -import { resolveWithin } from "../security/paths.js"; +import { setupDestination } from "../security/paths.js"; export interface ApplyResult { created: string[]; @@ -31,7 +31,7 @@ function writableProposal( export async function applyProposals(preview: PreviewResult): Promise { if (!preview.initializationAllowed) { throw new Error( - "Initialization stopped because the reviewed preview reported an unresolved instruction conflict.", + "Initialization stopped because the reviewed preview reported an unresolved setup conflict.", ); } const writable = preview.proposedFiles.filter(writableProposal); @@ -39,7 +39,7 @@ export async function applyProposals(preview: PreviewResult): Promise proposal.action === "reference") .map((proposal) => proposal.path); for (const proposal of writable) { - const target = resolveWithin(preview.root, proposal.path); + const target = await setupDestination(preview.root, proposal.path); if (proposal.action === "create" && (await exists(target))) { throw new Error( `Initialization stopped because ${proposal.path} now exists; run preview again.`, @@ -68,7 +68,7 @@ export async function applyProposals(preview: PreviewResult): Promise rm(file, { force: true }))); + // A path may have changed during application. Rollback must not follow it either. + const safeTarget = (file: string) => + setupDestination(preview.root, path.relative(preview.root, file)); await Promise.allSettled( - created.map((relative) => rm(resolveWithin(preview.root, relative), { force: true })), + temporary.map(async (file) => rm(await safeTarget(file), { force: true })), ); await Promise.allSettled( - [...originals.entries()].map(([target, content]) => writeFile(target, content, "utf8")), + created.map(async (relative) => + rm(await setupDestination(preview.root, relative), { force: true }), + ), + ); + await Promise.allSettled( + [...originals.entries()].map(async ([target, content]) => + writeFile(await safeTarget(target), content, "utf8"), + ), ); for (const directory of [...new Set(createdDirectories)].sort( (left, right) => right.length - left.length, )) { - await rmdir(directory).catch(() => undefined); + await safeTarget(directory) + .then((safe) => rmdir(safe)) + .catch(() => undefined); } throw error; } diff --git a/src/core/preview.ts b/src/core/preview.ts index 7db08ff..d6d9d1a 100644 --- a/src/core/preview.ts +++ b/src/core/preview.ts @@ -3,7 +3,7 @@ import { ConfigurationError, loadConfig } from "../config/load.js"; import type { PreviewResult } from "../model.js"; import { scanRepository } from "../detection/scan.js"; import { inspectRepositoryAdoption } from "../detection/adoption.js"; -import { canonicalDirectory } from "../security/paths.js"; +import { canonicalDirectory, setupDestination } from "../security/paths.js"; import { assessModules, buildProposals } from "./proposals.js"; function lineCount(source: string): number { @@ -74,6 +74,15 @@ export async function previewRepository(root = process.cwd()): Promise file.action === "create" || file.action === "patch", ); + const destinationConflicts = new Set(); + for (const proposal of changedProposals) { + try { + await setupDestination(canonicalRoot, proposal.path); + } catch (error) { + destinationConflicts.add((error as Error).message); + } + } + conflicts.push(...destinationConflicts); const proposalGrowth = (file: (typeof changedProposals)[number]): number => lineCount(file.content ?? "") - (file.action === "patch" ? previousLineCount(file.patch) : 0); return { @@ -83,7 +92,7 @@ export async function previewRepository(root = process.cwd()): Promise { if (!stat.isDirectory()) throw new Error(`Repository root is not a directory: ${candidate}`); return resolved; } + +// Setup accepts canonical roots from preview. Refuse links, including in-repository +// links, so the reviewed destination never silently redirects a write. +export async function setupDestination(root: string, relativePath: string): Promise { + const target = resolveWithin(root, relativePath); + if ((await lstat(root)).isSymbolicLink()) { + throw new Error("Setup stopped: repository root is a symbolic link; run preview again."); + } + if (path.relative(root, await realpath(root)) !== "") { + throw new Error("Setup stopped: repository root changed; run preview again."); + } + let current = root; + for (const part of path.relative(root, target).split(path.sep)) { + current = path.join(current, part); + let entry; + try { + entry = await lstat(current); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") break; + throw error; + } + if (entry.isSymbolicLink()) { + throw new Error( + `Setup stopped: ${normalizeRelative(path.relative(root, current))} is a symbolic link; use an unlinked destination and run preview again.`, + ); + } + if (current !== target && !entry.isDirectory()) { + throw new Error(`Setup stopped: ${path.relative(root, current)} is not a directory.`); + } + } + return target; +} diff --git a/tests/init-path-safety.test.ts b/tests/init-path-safety.test.ts new file mode 100644 index 0000000..bef16fc --- /dev/null +++ b/tests/init-path-safety.test.ts @@ -0,0 +1,80 @@ +import { mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { applyProposals } from "../src/core/init.js"; +import { previewRepository } from "../src/core/preview.js"; +import { hashTree, temporaryDirectory } from "./helpers.js"; + +const cleanup: string[] = []; +afterEach(async () => { + await Promise.all(cleanup.splice(0).map((root) => rm(root, { recursive: true, force: true }))); +}); + +async function setup() { + const parent = await temporaryDirectory("noxroot-init-path-"); + cleanup.push(parent); + const root = path.join(parent, "repository"); + const outside = path.join(parent, "outside"); + await mkdir(root); + await mkdir(outside); + await writeFile(path.join(root, "package.json"), '{"name":"path-safety"}\n'); + await writeFile(path.join(outside, "keep.txt"), "Unrelated content.\n"); + return { root, outside }; +} + +describe("initialization destination safety", () => { + it.each([".noxroot", ".noxroot/knowledge", "AGENTS.md"])( + "refuses a linked destination at %s without partial setup", + async (relative) => { + const { root, outside } = await setup(); + const destination = path.join(root, relative); + await mkdir(path.dirname(destination), { recursive: true }); + await symlink(outside, destination, "junction"); + const before = await hashTree(root); + const outsideBefore = await hashTree(outside); + const preview = await previewRepository(root); + expect(preview.initializationAllowed).toBe(false); + expect(preview.conflicts.join("\n")).toMatch(/symbolic link/i); + await expect(applyProposals(preview)).rejects.toThrow(); + // The application boundary must enforce safety even for a caller-supplied preview. + await expect(applyProposals({ ...preview, initializationAllowed: true })).rejects.toThrow( + /symbolic link/i, + ); + expect(await hashTree(root)).toBe(before); + expect(await hashTree(outside)).toBe(outsideBefore); + }, + ); + + it("rechecks every destination when a link is introduced after preview", async () => { + const { root, outside } = await setup(); + const preview = await previewRepository(root); + expect(preview.initializationAllowed).toBe(true); + await mkdir(path.join(root, ".noxroot")); + await symlink(outside, path.join(root, ".noxroot/knowledge"), "junction"); + const before = await hashTree(root); + const outsideBefore = await hashTree(outside); + await expect(applyProposals(preview)).rejects.toThrow(/symbolic link/i); + expect(await hashTree(root)).toBe(before); + expect(await hashTree(outside)).toBe(outsideBefore); + }); + + it("rejects a repository root replaced by a link after preview", async () => { + const { root, outside } = await setup(); + const preview = await previewRepository(root); + await rm(root, { recursive: true }); + await symlink(outside, root, "junction"); + const before = await hashTree(outside); + await expect(applyProposals(preview)).rejects.toThrow(/symbolic link|root changed/i); + expect(await hashTree(outside)).toBe(before); + }); + + it("does not refuse setup merely because an unrelated link exists", async () => { + const { root, outside } = await setup(); + await symlink(outside, path.join(root, "unrelated"), "junction"); + const before = await hashTree(outside); + const preview = await previewRepository(root); + expect(preview.initializationAllowed).toBe(true); + expect((await applyProposals(preview)).created).toContain(".noxroot/config.yml"); + expect(await hashTree(outside)).toBe(before); + }); +}); From fa87c13d6dc0c31a1a23d0c56fef6ca36935ab1f Mon Sep 17 00:00:00 2001 From: liolevx <312117550+liolevx@users.noreply.github.com> Date: Thu, 3 Sep 2026 22:00:31 -0400 Subject: [PATCH 2/3] test: rehearse packed setup upgrades and path refusal --- tests/package-smoke.mjs | 91 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-) diff --git a/tests/package-smoke.mjs b/tests/package-smoke.mjs index 4006ca9..98e994b 100644 --- a/tests/package-smoke.mjs +++ b/tests/package-smoke.mjs @@ -1,5 +1,6 @@ import { spawnSync } from "node:child_process"; -import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; +import assert from "node:assert/strict"; +import { mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -8,6 +9,20 @@ const fixtureRoot = path.join(repositoryRoot, "tests", "fixtures", "typescript") const temporaryRoot = await mkdtemp(path.join(tmpdir(), "noxroot-package-smoke-")); const npmCache = path.join(temporaryRoot, "npm-cache"); +async function snapshot(root) { + const files = {}; + async function visit(directory) { + for (const entry of await readdir(directory, { withFileTypes: true })) { + const file = path.join(directory, entry.name); + if (entry.isDirectory()) await visit(file); + else + files[path.relative(root, file)] = entry.isSymbolicLink() ? "link" : await readFile(file); + } + } + await visit(root); + return files; +} + function run(executable, args, options = {}) { const result = spawnSync(executable, args, { cwd: options.cwd ?? repositoryRoot, @@ -142,8 +157,80 @@ try { if (!generatedInstructions.includes("npx --yes noxroot@0.1.0 finish")) { throw new Error("Packed CLI initialization did not pin its finish command."); } + const initialized = await snapshot(initializedRoot); + invokeBinary(binary, ["init", "--yes", "--root", initializedRoot], installRoot); + assert.deepEqual(await snapshot(initializedRoot), initialized, "Repeated init must be a no-op"); + + // Synthetic older pin, not a claim that an earlier version was published. + const agentsPath = path.join(initializedRoot, "AGENTS.md"); + const userPrefix = "# Repository instructions\n\nKeep changes focused.\n\n"; + const userSuffix = "\n## User-owned notes\n\nUse existing documentation.\n"; + const currentInstructions = userPrefix + generatedInstructions + userSuffix; + await writeFile(agentsPath, currentInstructions.replaceAll("noxroot@0.1.0", "noxroot@0.0.9")); + await writeFile(path.join(initializedRoot, "user-notes.md"), "Preserve this document.\n"); + const beforeSync = await snapshot(initializedRoot); + const dryRun = JSON.parse( + invokeBinary( + binary, + ["sync", "--dry-run", "--diff", "--json", "--root", initializedRoot], + installRoot, + ), + ); + assert.deepEqual(dryRun.summary, { + repositoryVersion: "0.0.9", + runningVersion: "0.1.0", + managedChanges: 1, + }); + const changes = dryRun.preview.proposedFiles.filter((file) => file.action !== "reference"); + assert.equal(changes.length, 1); + assert.equal(changes[0].path, "AGENTS.md"); + assert.equal(changes[0].action, "patch"); + assert.ok(changes[0].patch.includes("noxroot@0.0.9")); + assert.ok(changes[0].patch.includes("noxroot@0.1.0")); + assert.deepEqual(await snapshot(initializedRoot), beforeSync, "Sync preview must not write"); + assert.throws( + () => invokeBinary(binary, ["sync", "--json", "--root", initializedRoot], installRoot), + /requires --yes/, + ); + assert.deepEqual(await snapshot(initializedRoot), beforeSync, "Unconfirmed sync must not write"); + invokeBinary(binary, ["sync", "--yes", "--json", "--root", initializedRoot], installRoot); + assert.deepEqual( + await snapshot(initializedRoot), + { + ...beforeSync, + "AGENTS.md": Buffer.from(currentInstructions), + }, + "Sync must change only the managed pin, preserving user content and knowledge", + ); + const afterSync = await snapshot(initializedRoot); + const repeated = JSON.parse( + invokeBinary( + binary, + ["sync", "--dry-run", "--diff", "--json", "--root", initializedRoot], + installRoot, + ), + ); + assert.equal(repeated.summary.managedChanges, 0); + assert.deepEqual(await snapshot(initializedRoot), afterSync); + + const linkedRoot = path.join(temporaryRoot, "linked-repository"); + const outside = path.join(temporaryRoot, "outside"); + await mkdir(linkedRoot); + await mkdir(outside); + await symlink(outside, path.join(linkedRoot, ".noxroot"), "junction"); + const linkedBefore = await snapshot(linkedRoot); + const linkedPreview = JSON.parse( + invokeBinary(binary, ["preview", "--json", "--root", linkedRoot], installRoot), + ); + assert.equal(linkedPreview.initializationAllowed, false); + assert.throws( + () => invokeBinary(binary, ["init", "--yes", "--json", "--root", linkedRoot], installRoot), + /failed with 3/, + ); + assert.deepEqual(await snapshot(linkedRoot), linkedBefore); + assert.deepEqual(await snapshot(outside), {}); process.stdout.write( - `Packed CLI smoke passed on ${process.platform} with a real tarball install.\n`, + `Packed CLI smoke passed on ${process.platform}: real tarball install, repeated init, managed-pin upgrade, user-content preservation, and linked-destination refusal.\n`, ); } finally { await rm(temporaryRoot, { recursive: true, force: true }); From d1dada4571c0069a4f6213bc0ad77da9806bf45c Mon Sep 17 00:00:00 2001 From: liolevx <312117550+liolevx@users.noreply.github.com> Date: Thu, 3 Sep 2026 22:05:35 -0400 Subject: [PATCH 3/3] docs: record independent release safety acceptance --- .../RELEASE-ACCEPTANCE-2026-09-04.md | 81 +++++++++++++++++++ .../acceptance/release-review-2026-09-04.json | 6 ++ 2 files changed, 87 insertions(+) create mode 100644 tests/acceptance/RELEASE-ACCEPTANCE-2026-09-04.md create mode 100644 tests/acceptance/release-review-2026-09-04.json diff --git a/tests/acceptance/RELEASE-ACCEPTANCE-2026-09-04.md b/tests/acceptance/RELEASE-ACCEPTANCE-2026-09-04.md new file mode 100644 index 0000000..d7e9e15 --- /dev/null +++ b/tests/acceptance/RELEASE-ACCEPTANCE-2026-09-04.md @@ -0,0 +1,81 @@ +# Release acceptance follow-up + +Performed September 3 in America/Toronto (September 4 UTC). Baseline: merged main +`a925fa78a5230c690cc18086bdca14995076638a`. Fix: `5f2463d`; packed regression checks: `fa87c13`. No +additional external repositories, product features, or dependencies. + +## Independent review and fix + +A fresh reviewer inspected the preceding implementation against `f4e906c` and ran 56 focused tests. +The review found a pre-existing release blocker: initialization followed a `.noxroot` junction and +wrote outside the selected repository even though preview reported the link. + +Six regressions were added first. Five failed before the fix; all six pass afterward: + +- Linked `.noxroot`, nested knowledge directory, and `AGENTS.md` destinations are refused. +- Direct application independently rejects unsafe destinations, even with caller-supplied approval. +- Links introduced after preview and a repository root replaced by a link are refused. +- Refusal leaves repository and outside content unchanged, including no partial setup files. +- An unrelated link does not prevent normal setup. + +Preview now refuses unsafe writable destinations. Application checks every destination before its +first write, rechecks during application, and guards rollback paths. CLI refusal text now says +"setup conflict" rather than incorrectly calling every refusal an instruction conflict. + +The reviewer independently rechecked the fix and ran 42 focused tests. The exact approval is in +[the review response](release-review-2026-09-04.json). These checks are not atomic protection +against a hostile concurrent filesystem writer; rollback can leave recovery artifacts rather than +follow a path that became unsafe. This limitation is stated in `docs/security.md`. + +## Packed install and upgrade rehearsal + +The existing package smoke test now installs the actual tarball and exercises its installed binary +on Windows and Linux. Dependencies are packed locally and installed offline with scripts disabled. + +- Repeated initialization is byte-for-byte unchanged. +- A synthetic `0.0.9` managed pin is upgraded to the running `0.1.0` pin. +- `sync --dry-run --diff --json` reports exactly one `AGENTS.md` patch and makes no changes. +- Unconfirmed JSON sync is refused and makes no changes. +- Confirmed sync changes only that pin. User-owned instruction prefixes/suffixes, documentation, + configuration, and project knowledge remain unchanged. +- A subsequent sync reports zero managed changes. +- Packed preview refuses linked setup destinations; packed init exits 3 and writes nothing outside + or inside the test repository. + +The older pin is synthetic, not an older published package. This does not test npm registry +retrieval or an actual migration between published releases. + +## Validation + +- Windows Node 24.13.0: `npm run check` passed, including all 184 unit tests, formatting, lint, + typecheck, build, permission-confined compiled preview, and real package smoke. +- Linux Node 24.19.0: `node tests/acceptance/linux.mjs` passed from committed `fa87c13` in a clean + temporary checkout: 182 tests passed, two Windows-only tests skipped, all other checks passed. +- `git diff --check` passed. +- Final report checks: all five documentation tests and `npm run format:check` passed. +- A fresh `npm audit --audit-level=high --json` stalled. A bounded retry with + `--fetch-retries=0 --fetch-timeout=15000` timed out at the npm advisory endpoint. No fresh audit + pass is claimed; dependencies and lockfile are unchanged from the preceding validated release. +- `npm pack --dry-run --json`: 120,983 packed bytes, 387,801 unpacked bytes. Compared with the + preceding recorded package, +373 packed bytes and +2,553 unpacked bytes. +- Local Noxroot task `20260904-ba4005c6` finished as `approved`, using the actual independent review + JSON. All five approved checks passed; no learning or knowledge document was proposed. + +Runtime source change: four files, 66 added lines and 12 removed. README and visuals are unchanged; +this safety correction does not change the product description. The setup safety documentation was +updated, and the release evidence is retained separately from project knowledge. + +## Remaining gate and cleanup + +Still pending: a real signed-in compatible coding agent taking one task from the user's request +through start, change, verification, and finish, including continuation in a new conversation. +Standalone client authentication remains deferred until the user is available. Neither these tests +nor the review prove universal automatic invocation or long-term knowledge usefulness. The existing +thirty-repository report remains the breadth evidence; it was not expanded or rerun in this slice. + +Repository: `C:/Users/lione/Documents/ChatGPT/noxroot`; branch: `agent/release-safety-acceptance`. +No additional worktrees or workspace-parent artifacts were created. Test-owned temporary fixtures, +package installations/caches, and the isolated Linux checkout were removed. The empty preparation +directory `C:/Users/lione/AppData/Local/Temp/noxroot-release-bef73e6d39d340ef9ac66a2b79933d0e` was +removed. Older unrelated temporary directories were left untouched. Local task evidence is retained +under `.git/noxroot`; no push, merge, npm publication, or deployment occurred in this slice. diff --git a/tests/acceptance/release-review-2026-09-04.json b/tests/acceptance/release-review-2026-09-04.json new file mode 100644 index 0000000..1427179 --- /dev/null +++ b/tests/acceptance/release-review-2026-09-04.json @@ -0,0 +1,6 @@ +{ + "decision": "approved", + "summary": "The bounded setup-path fix closes the reproduced initialization escape. Preview rejects linked writable destinations, direct application independently checks every destination before mutation, and application plus rollback recheck paths instead of blindly following replacements. Root-link replacement and changed canonical ancestry are rejected; unrelated links remain compatible. Independently reran 42 tests across initialization path safety, initialization/context/doctor, and adoption: all passed. git diff --check passed. No remaining release blocker found in this fix. These checks are not race-free filesystem isolation: concurrent replacement between validation and filesystem operations remains a limitation, and rollback may intentionally leave recovery artifacts when a path becomes unsafe. No files were edited during review.", + "findings": [], + "learningCandidates": [] +}