refactor(review): remove security receipt ledger

internal/security-reviews/openshell-0.0.101-migration-review.md

@@ -127,7 +127,7 @@ are `a2704babbb468fd0a359bfdd9844de71095b730758541b4ca8cbab77d4018920` for linux
 `88300e35f153123e4dc3021c537834dd6c0a09665a4a6d3974cd285d512345c4` for linux-aarch64.
 
 The correction commit has a raw SSH signature and exact contributor `Signed-off-by` trailer. Its
-independent exact-commit review passed all nine security categories, 90 focused trust tests, the
+independent exact-commit review passed every security category, 90 focused trust tests, the
 repository integrity checks, and type-checking. Both findings are closed with no new blocker.
 Because the formula asset remains mutable upstream, a replacement now causes a fail-closed
 availability failure instead of silently changing trusted identity. The dormant v0.0.101 sandbox

test/helpers/pr-review-advisor-test-fixtures.ts

@@ -5,7 +5,10 @@ import fs from "node:fs";
 import path from "node:path";
 
 import { buildRiskPlan } from "../../tools/advisors/risk-plan.mts";
-import type { ReviewAdvisorResult, ReviewMetadata } from "../../tools/pr-review-advisor/analyze.mts";
+import type {
+  ReviewAdvisorResult,
+  ReviewMetadata,
+} from "../../tools/pr-review-advisor/analyze.mts";
 
 export const ROOT = path.resolve(import.meta.dirname, "../..");
 
@@ -26,7 +29,7 @@ export function metadata(overrides: Partial<ReviewMetadata> = {}): ReviewMetadat
       candidateExistingCoverage: [],
     },
     simplificationSignals: [],
-        workflowSignals: [],
+    workflowSignals: [],
     localizedPatchSignals: [],
     driftEvidence: [],
     github: null,
@@ -46,9 +49,7 @@ export function loadAdvisorSchema(): Record<string, unknown> {
   return JSON.parse(fs.readFileSync(schemaPath, "utf-8")) as Record<string, unknown>;
 }
 
-export function validResult(
-  overrides: Record<string, unknown> = {},
-): ReviewAdvisorResult {
+export function validResult(overrides: Record<string, unknown> = {}): ReviewAdvisorResult {
   return {
     version: 1,
     baseRef: "origin/main",
@@ -88,13 +89,6 @@ export function validResult(
         evidence: "comment.mts uses marker",
       },
     ],
-    securityCategories: [
-      {
-        category: "Secrets and Credentials",
-        verdict: "pass",
-        justification: "No secrets in diff.",
-      },
-    ],
     sourceOfTruthReview: [
       {
         surface: "trusted-code boundary",

test/pr-review-advisor-openshell.test.ts

@@ -411,6 +411,58 @@ describe("PR review advisor OpenShell wrapper", () => {
     }
   });
 
+  it("prepares specialist diff evidence before the worktree becomes read-only", async () => {
+    const env = advisorEnvironment();
+    const workdir = env.ADVISOR_WORKDIR as string;
+    fs.rmSync(path.join(workdir, ".git"), { recursive: true });
+    execFileSync("git", ["init", "--quiet"], { cwd: workdir });
+    fs.writeFileSync(path.join(workdir, "reviewed.txt"), "base\n");
+    execFileSync("git", ["add", "reviewed.txt"], { cwd: workdir });
+    const commit = (message: string) =>
+      execFileSync(
+        "git",
+        [
+          "-c",
+          "user.name=PR Review Advisor",
+          "-c",
+          "user.email=advisor@example.invalid",
+          "commit",
+          "--quiet",
+          "-m",
+          message,
+        ],
+        { cwd: workdir },
+      );
+    commit("test: add base content");
+    fs.writeFileSync(path.join(workdir, "reviewed.txt"), "changed\n");
+    execFileSync("git", ["add", "reviewed.txt"], { cwd: workdir });
+    commit("test: change reviewed content");
+    env.BASE_REF = "HEAD~1";
+    env.HEAD_REF = "HEAD";
+    env.PR_REVIEW_ADVISOR_INTEREST = "security";
+    const binaries = path.join(temporaryDirectory(), "binaries");
+    fs.mkdirSync(binaries);
+    fs.writeFileSync(path.join(binaries, "rg"), "rg", { mode: 0o755 });
+    fs.writeFileSync(path.join(binaries, "fdfind"), "fdfind", { mode: 0o755 });
+
+    await prepareAdvisorSandboxInputs(env, {
+      collectContext: async () => null,
+      resolveExecutable: (name) => path.join(binaries, name),
+    });
+
+    const diffPath = path.join(
+      env.RUNNER_TEMP as string,
+      "pr-review-advisor-context",
+      "specialist",
+      "diff.patch",
+    );
+    expect(fs.readFileSync(diffPath, "utf8")).toContain("+changed");
+    expect(fs.statSync(diffPath).mode & 0o777).toBe(0o444);
+    expect(fs.existsSync(path.join(workdir, ".pr-review-advisor-context"))).toBe(false);
+    fs.chmodSync(path.dirname(diffPath), 0o700);
+    fs.chmodSync(diffPath, 0o600);
+  });
+
   it("requires repository metadata before placing immutable-boundary proof files", async () => {
     const env = advisorEnvironment();
     fs.rmSync(path.join(env.ADVISOR_WORKDIR as string, ".git"), {

test/pr-review-advisor-quality.test.ts

@@ -7,7 +7,7 @@ import { afterEach, describe, expect, it, vi } from "vitest";
 import { renderSummary } from "../tools/pr-review-advisor/render-result.mts";
 import { reviewQualityIssues } from "../tools/pr-review-advisor/review-quality.mts";
 import {
-  parseSecurityRubric,
+  buildSystemPrompt,
   readTrustedSecurityRubric,
 } from "../tools/pr-review-advisor/trusted-guidance.mts";
 import { buildComment } from "../tools/pr-review-advisor/comment.mts";
@@ -39,46 +39,73 @@ describe("PR review advisor", () => {
     try {
       process.chdir(tmp);
       const rubric = readTrustedSecurityRubric();
-      expect(rubric).toContain("# Security Rubric");
-      expect(rubric).toContain("Category 9: System Security");
+      expect(rubric).toContain("## Category 9: System Security");
       expect(rubric).not.toContain("PR-controlled rubric");
     } finally {
       process.chdir(originalCwd);
       fs.rmSync(tmp, { recursive: true, force: true });
     }
   });
 
-  it("rejects missing and malformed trusted security rubrics", () => {
-    const readSpy = vi.spyOn(fs, "readFileSync").mockImplementationOnce(() => {
+  it("embeds the complete trusted security rubric in the model prompt", () => {
+    const rubric = readTrustedSecurityRubric();
+
+    expect(buildSystemPrompt()).toContain(rubric);
+  });
+
+  it("reports a missing trusted security rubric", () => {
+    vi.spyOn(fs, "readFileSync").mockImplementationOnce(() => {
       throw new Error("missing rubric fixture");
     });
+
     expect(() => readTrustedSecurityRubric()).toThrow("Security rubric unavailable");
-    readSpy.mockRestore();
+  });
 
-    expect(() => parseSecurityRubric("# Security Rubric\n\n## Category 1: Secrets\n")).toThrow(
+  it.each([
+    [
+      "a missing category",
+      (rubric: string) => rubric.replace(/## Category 5:.*?(?=## Category 6:)/su, ""),
       "must define exactly 9 categories",
-    );
-    expect(() =>
-      parseSecurityRubric(
-        readTrustedSecurityRubric().replace("### Expected evidence", "### Evidence"),
-      ),
-    ).toThrow("must define Meaning, Questions, and Expected evidence in order");
-    expect(() =>
-      parseSecurityRubric(
-        readTrustedSecurityRubric().replace(
+    ],
+    [
+      "an out-of-order category",
+      (rubric: string) => rubric.replace("## Category 2:", "## Category 3:"),
+      "category 2 has a malformed heading",
+    ],
+    [
+      "a duplicate category name",
+      (rubric: string) =>
+        rubric.replace("## Category 2: Input Validation and Data Sanitization", "## Category 2: Secrets and Credentials"),
+      "category names must be unique",
+    ],
+    [
+      "an empty category section",
+      (rubric: string) =>
+        rubric.replace(
           /### Meaning\n\nKeep credentials[^\n]*\n/u,
           "### Meaning\n\n",
         ),
-      ),
-    ).toThrow("has empty Meaning");
-    expect(() =>
-      parseSecurityRubric(
-        readTrustedSecurityRubric().replace(
-          "### Meaning\n\nKeep credentials",
-          "### Questions\n\nDuplicate section.\n\n### Meaning\n\nKeep credentials",
-        ),
-      ),
-    ).toThrow("must define Meaning, Questions, and Expected evidence in order");
+      "category 1 has empty Meaning",
+    ],
+    [
+      "a different final category",
+      (rubric: string) => rubric.replace("## Category 9: System Security", "## Category 9: Host Security"),
+      "category 9 must be System Security",
+    ],
+    [
+      "reordered category subsections",
+      (rubric: string) =>
+        rubric
+          .replace("### Meaning", "### Temporary")
+          .replace("### Questions", "### Meaning")
+          .replace("### Temporary", "### Questions"),
+      "must define Meaning, Questions, and Expected evidence in order",
+    ],
+  ])("rejects a trusted security rubric with %s", (_case, mutate, message) => {
+    const malformed = mutate(readTrustedSecurityRubric());
+    vi.spyOn(fs, "readFileSync").mockReturnValueOnce(malformed);
+
+    expect(() => readTrustedSecurityRubric()).toThrow(message);
   });
 
   it("renders summaries and sticky comments with maintainer-review framing", () => {

test/pr-review-advisor-rendering.test.ts

@@ -319,13 +319,13 @@ describe("PR review advisor", () => {
   });
 
   it.each(["needs_rework", "blocked"])(
-    "keeps legacy public recommendation %s schema-compatible",
+    "rejects retired public recommendation %s",
     (recommendation) => {
       const schema = loadAdvisorSchema();
       const validate = new Ajv2020({ strict: false }).compile(schema);
 
       expect(validate(validResult({ summary: { ...validResult().summary, recommendation } }))).toBe(
-        true,
+        false,
       );
     },
   );

test/pr-review-advisor-specialists.test.ts

@@ -7,14 +7,12 @@ import os from "node:os";
 import path from "node:path";
 
 import type { ToolDefinition } from "@earendil-works/pi-coding-agent";
+import { canonicalRepoReadPath } from "../tools/advisors/repo-read-only-tools.mts";
 import { describe, expect, it, onTestFinished, vi } from "vitest";
 
 import { TERMINOLOGY_TRACE_TOOL } from "../tools/pr-review-advisor/terminology.mts";
-import {
-  runSpecialistAdvisor,
-  writeSpecialistDiff,
-  writeSpecialistSummary,
-} from "../tools/pr-review-advisor/run-specialist.mts";
+import { runSpecialistAdvisor, writeSpecialistSummary } from "../tools/pr-review-advisor/run-specialist.mts";
+import { writeSpecialistDiff } from "../tools/pr-review-advisor/specialist-context.mts";
 import type { RunAdvisorResult, RunReadOnlyAdvisorOptions } from "../tools/advisors/session.mts";
 import {
   ADVISOR_INTERESTS,
@@ -49,15 +47,15 @@ const context: InvestigateTurnContext = {
 };
 
 describe("PR review advisor specialist prompts", () => {
-  it("writes diff evidence to a new owner-only runtime path", () => {
-    const configDir = fs.mkdtempSync(path.join(process.cwd(), ".tmp-specialist-config-"));
-    onTestFinished(() => fs.rmSync(configDir, { recursive: true, force: true }));
-    const directory = path.join(configDir, "context");
+  it("writes readable diff evidence in the prepared advisor context", async () => {
+    const directory = fs.mkdtempSync(path.join(os.tmpdir(), "specialist-context-"));
+    onTestFinished(() => fs.rmSync(directory, { recursive: true, force: true }));
     const expected = path.join(directory, "diff.patch");
 
-    const file = writeSpecialistDiff(configDir, "diff evidence");
+    const file = writeSpecialistDiff(directory, "diff evidence");
 
     expect(file).toBe(expected);
+    await expect(canonicalRepoReadPath(directory, "diff.patch")).resolves.toBe(expected);
     expect(fs.readFileSync(file, "utf8")).toBe("diff evidence");
     expect(fs.statSync(directory).mode & 0o777).toBe(0o700);
     expect(fs.statSync(file).mode & 0o777).toBe(0o600);
@@ -66,18 +64,47 @@ describe("PR review advisor specialist prompts", () => {
   it("tightens an existing specialist diff path", () => {
     const configDir = fs.mkdtempSync(path.join(process.cwd(), ".tmp-specialist-config-"));
     onTestFinished(() => fs.rmSync(configDir, { recursive: true, force: true }));
-    const directory = path.join(configDir, "context");
+    const directory = configDir;
     const expected = path.join(directory, "diff.patch");
-    fs.mkdirSync(directory, { mode: 0o755 });
+    fs.chmodSync(directory, 0o755);
     fs.writeFileSync(expected, "stale", { mode: 0o644 });
 
-    writeSpecialistDiff(configDir, "diff evidence");
+    writeSpecialistDiff(directory, "diff evidence");
 
     expect(fs.readFileSync(expected, "utf8")).toBe("diff evidence");
     expect(fs.statSync(directory).mode & 0o777).toBe(0o700);
     expect(fs.statSync(expected).mode & 0o777).toBe(0o600);
   });
 
+  it("rejects a symbolic-link specialist diff file", () => {
+    const directory = fs.mkdtempSync(path.join(os.tmpdir(), "specialist-context-"));
+    const target = path.join(directory, "outside.patch");
+    onTestFinished(() => fs.rmSync(directory, { recursive: true, force: true }));
+    fs.writeFileSync(target, "unchanged");
+    fs.symlinkSync(target, path.join(directory, "diff.patch"));
+
+    expect(() => writeSpecialistDiff(directory, "diff evidence")).toThrow(
+      "Specialist diff file must not be a symbolic link",
+    );
+    expect(fs.readFileSync(target, "utf8")).toBe("unchanged");
+  });
+
+  it("rejects a dangling symbolic-link specialist diff file", () => {
+    const directory = fs.mkdtempSync(path.join(os.tmpdir(), "specialist-context-"));
+    const targetDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "specialist-target-"));
+    const target = path.join(targetDirectory, "missing.patch");
+    onTestFinished(() => {
+      fs.rmSync(directory, { recursive: true, force: true });
+      fs.rmSync(targetDirectory, { recursive: true, force: true });
+    });
+    fs.symlinkSync(target, path.join(directory, "diff.patch"));
+
+    expect(() => writeSpecialistDiff(directory, "diff evidence")).toThrow(
+      "Specialist diff file must not be a symbolic link",
+    );
+    expect(fs.existsSync(target)).toBe(false);
+  });
+
   it("parses every discovered specialist interest (#9949)", () => {
     expect(ADVISOR_INTERESTS.map(parseAdvisorInterest)).toEqual(ADVISOR_INTERESTS);
     expect(() => parseAdvisorInterest("missing-specialist")).toThrowError(