fix(advisor): complete local review on macOS

test/automation/pull-requests/pr-review-advisor-local.test.ts

@@ -174,7 +174,7 @@ describe("local PR review advisor", () => {
     expect(ADVISOR_PI_IMAGE).toMatch(/@sha256:[0-9a-f]{64}$/u);
   });
 
-  it("installs origin/main dependencies without executing contributor node_modules (#10611)", () => {
+  it("installs trusted dependencies and runs the canonical entrypoint through a temporary symlink (#10611)", () => {
     const source = temporaryDirectory();
     git(source, ["init", "--initial-branch=main"]);
     git(source, ["config", "user.name", "Test"]);
@@ -198,13 +198,16 @@ describe("local PR review advisor", () => {
         'import fs from "node:fs";',
         'import path from "node:path";',
         'import { execFileSync } from "node:child_process";',
+        'import { pathToFileURL } from "node:url";',
         'import { hostValue } from "./trusted-host.mts";',
+        "if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {",
         "const source = process.argv[2];",
         'const gitHead = execFileSync("git", ["rev-parse", "HEAD"], { encoding: "utf8" }).trim();',
         'let detached = false; try { execFileSync("git", ["symbolic-ref", "-q", "HEAD"], { stdio: "ignore" }); } catch { detached = true; }',
         'const policy = fs.readFileSync(path.join(source, "tools/pr-review-advisor/policy.txt"), "utf8").trim();',
         'fs.writeFileSync(path.join(source, "bootstrap-result.txt"), [hostValue, policy].join("|") + "\\n");',
         'fs.writeFileSync(path.join(source, "trusted-child.json"), JSON.stringify({ pid: process.pid, nodeOptions: process.env.NODE_OPTIONS, nodePath: process.env.NODE_PATH, git: fs.existsSync(".git"), gitHead, detached }));',
+        "}",
       ].join("\n"),
     );
     const npmBin = installFakeNpm(source);
@@ -265,6 +268,9 @@ describe("local PR review advisor", () => {
       path.join(source, "node_modules", "malicious", "index.js"),
       'require("node:fs").writeFileSync("contributor-module-executed", "yes")\n',
     );
+    const trustedTemporaryDirectory = temporaryDirectory();
+    const temporaryAlias = path.join(temporaryDirectory(), "temporary-alias");
+    fs.symlinkSync(trustedTemporaryDirectory, temporaryAlias, "dir");
 
     const result = spawnSync(
       process.execPath,
@@ -280,6 +286,7 @@ describe("local PR review advisor", () => {
           NODE_OPTIONS: "--require=" + preload,
           NODE_PATH: maliciousBin,
           SECRET_TOKEN: "must-not-reach-npm",
+          TMPDIR: temporaryAlias,
           npm_config_cache: path.join(source, "npm-cache"),
         },
       },
@@ -546,6 +553,17 @@ describe("local PR review advisor", () => {
     ],
   ])("removes its temporary root after %s (#10611)", async (_case, lifecycle, expected) => {
     const source = repository();
+    const external = temporaryDirectory();
+    const externalMode = fs.statSync(external).mode & 0o777;
+    const originalPrepare = lifecycle.prepare;
+    lifecycle.prepare = async (env) => {
+      await originalPrepare(env);
+      const readOnly = path.join(env.RUNNER_TEMP as string, "read-only");
+      fs.mkdirSync(readOnly);
+      fs.writeFileSync(path.join(readOnly, "artifact"), "review\n");
+      fs.symlinkSync(external, path.join(readOnly, "external"), "dir");
+      fs.chmodSync(readOnly, 0o500);
+    };
     let removedRoot = "";
     const [result] = await Promise.allSettled([
       runLocalReview({
@@ -562,6 +580,7 @@ describe("local PR review advisor", () => {
     expect(result).toMatchObject(expected);
     expect(path.basename(removedRoot)).toMatch(/^nemoclaw-local-review-/u);
     expect(fs.existsSync(removedRoot)).toBe(false);
+    expect(fs.statSync(external).mode & 0o777).toBe(externalMode);
   });
 
   it("stops between specialists and restores a received signal after cleanup (#10611)", async () => {

tools/pr-review-advisor/local-review-implementation.mts

@@ -71,6 +71,13 @@ function combineFailures(first: unknown, next: unknown): unknown {
   });
 }
 
+function makeOwnedTemporaryDirectoriesWritable(root: string): void {
+  for (const entry of fs.readdirSync(root, { recursive: true, withFileTypes: true })) {
+    if (entry.isDirectory()) fs.chmodSync(path.join(entry.parentPath, entry.name), 0o700);
+  }
+  fs.chmodSync(root, 0o700);
+}
+
 export const defaultLocalReviewLifecycle: LocalReviewLifecycle = {
   ...defaultAdvisorSpecialistLifecycle,
   prepare: (env) => prepareAdvisorSandboxInputs(env, { collectContext: async () => null }),
@@ -430,7 +437,10 @@ export async function runLocalReview(input: {
   try {
     await activeCleanup?.();
     activeCleanup = undefined;
-    if (ownsRoot) (input.removeTemporaryRoot ?? fs.rmSync)(root, { recursive: true, force: true });
+    if (ownsRoot) {
+      makeOwnedTemporaryDirectoriesWritable(root);
+      (input.removeTemporaryRoot ?? fs.rmSync)(root, { recursive: true, force: true });
+    }
   } catch (error) {
     cleanup = contextualError(
       `Local review failed during cleanup for temporary root ${root}`,

tools/pr-review-advisor/local-review.mts

@@ -10,7 +10,14 @@ const IMPLEMENTATION = "tools/pr-review-advisor/local-review-implementation.mts"
 const SIGNALS = ["SIGINT", "SIGTERM", "SIGHUP"] as const;
 function hostEnv(source: string): NodeJS.ProcessEnv {
   const homeBin = process.env.HOME && path.join(process.env.HOME, ".local", "bin");
-  const entries = [homeBin, path.dirname(process.execPath), "/usr/local/bin", "/usr/bin", "/bin"]
+  const entries = [
+    homeBin,
+    path.dirname(process.execPath),
+    "/opt/homebrew/bin",
+    "/usr/local/bin",
+    "/usr/bin",
+    "/bin",
+  ]
     .filter((value): value is string => typeof value === "string" && fs.existsSync(value))
     .map((value) => fs.realpathSync(value))
     .filter((value) => path.relative(source, value).startsWith(".."));
@@ -176,7 +183,7 @@ async function main(): Promise<{ code: number | null; signal: NodeJS.Signals | n
       throw new Error("npm failed while preparing the trusted local review checkout");
     result = await run(
       process.execPath,
-      ["--no-warnings", path.join(checkout, IMPLEMENTATION), source],
+      ["--no-warnings", fs.realpathSync(path.join(checkout, IMPLEMENTATION)), source],
       { cwd: checkout, env, inherit: true },
     );
     return { code: result.code, signal: received ?? result.signal };