Close setup path escape and verify packed upgrades

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.
 

src/cli.ts

@@ -544,7 +544,7 @@ export function createProgram(customIo?: Partial<Io>): 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<Io>): 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;
         }

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,15 +31,15 @@ function writableProposal(
 export async function applyProposals(preview: PreviewResult): Promise<ApplyResult> {
   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);
   const references = preview.proposedFiles
     .filter((proposal) => 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<ApplyResul
   const temporary: string[] = [];
   try {
     for (const proposal of writable) {
-      const target = resolveWithin(preview.root, proposal.path);
+      const target = await setupDestination(preview.root, proposal.path);
       const createdDirectory = await mkdir(path.dirname(target), { recursive: true });
       if (createdDirectory) {
         let directory = path.dirname(target);
@@ -78,28 +78,41 @@ export async function applyProposals(preview: PreviewResult): Promise<ApplyResul
           directory = path.dirname(directory);
         }
       }
+      await setupDestination(preview.root, proposal.path);
       if (proposal.action === "patch") originals.set(target, await readFile(target, "utf8"));
       const temp = path.join(path.dirname(target), `.noxroot-${randomUUID()}.tmp`);
       temporary.push(temp);
       await writeFile(temp, proposal.content, { encoding: "utf8", flag: "wx", mode: 0o600 });
+      await setupDestination(preview.root, proposal.path);
       await rename(temp, target);
       temporary.splice(temporary.indexOf(temp), 1);
       if (proposal.action === "create") created.push(proposal.path);
       else patched.push(proposal.path);
     }
     return { created, patched, referenced: references };
   } catch (error) {
-    await Promise.allSettled(temporary.map((file) => 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;
   }

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<PreviewRe
   const changedProposals = proposedFiles.filter(
     (file) => file.action === "create" || file.action === "patch",
   );
+  const destinationConflicts = new Set<string>();
+  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<PreviewRe
     modules,
     proposedFiles,
     capabilities: adoption.capabilities,
-    initializationAllowed: adoption.initializationAllowed,
+    initializationAllowed: adoption.initializationAllowed && destinationConflicts.size === 0,
     existingSetup,
     conflicts,
     unknowns,

src/security/paths.ts

@@ -45,3 +45,35 @@ export async function canonicalDirectory(candidate: string): Promise<string> {
   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<string> {
+  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;
+}

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.

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": []
+}

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);
+  });
+});