diff --git a/src/permissions/__tests__/deferred-queue.test.ts b/src/permissions/__tests__/deferred-queue.test.ts index 62e0fa104..74e012a0f 100644 --- a/src/permissions/__tests__/deferred-queue.test.ts +++ b/src/permissions/__tests__/deferred-queue.test.ts @@ -1,14 +1,20 @@ -import { describe, it, expect, vi } from "vitest"; -import { mkdtempSync, readFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, it, expect, vi } from "vitest"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; import { DeferredQueue } from "../reviewer/deferred-queue.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); function tmpQueuePath(): string { - const dir = mkdtempSync(join(tmpdir(), "lvis-deferred-queue-")); + const dir = resources.makeTmpDir("lvis-deferred-queue-"); return join(dir, "deferred-queue.jsonl"); } +afterEach(async () => { + await resources.cleanup(); +}); + const SAMPLE = { toolName: "fs_write", source: "builtin" as const, diff --git a/src/permissions/__tests__/host-fetch-verb-snapshot.test.ts b/src/permissions/__tests__/host-fetch-verb-snapshot.test.ts index 19ee1bab2..0de4a2c44 100644 --- a/src/permissions/__tests__/host-fetch-verb-snapshot.test.ts +++ b/src/permissions/__tests__/host-fetch-verb-snapshot.test.ts @@ -24,10 +24,7 @@ * verb-snapshot flow from the DNS/SSRF/allow-list gate (which is unchanged and * covered by host-fetch-guard.test.ts). */ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { mkdtempSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const harness = vi.hoisted(() => ({ capturedRuntimeOptions: null as Record | null, @@ -117,6 +114,13 @@ import { type EffectEntry, } from "../effect-ledger.js"; import type { PluginHostApi } from "../../plugins/types.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); + +afterEach(async () => { + await resources.cleanup(); +}); type CreateHostApi = ( pluginId: string, @@ -186,7 +190,7 @@ async function buildRealHostApi(): Promise<{ createHostApi, "initPluginRuntime must register a createHostApi factory", ).toBeDefined(); - const pluginDataDir = mkdtempSync(join(tmpdir(), "lvis-hostfetch-verb-")); + const pluginDataDir = resources.makeTmpDir("lvis-hostfetch-verb-"); const hostApi = createHostApi!( "verb-snapshot-plugin", { diff --git a/src/permissions/__tests__/hostapi-effect-completeness.test.ts b/src/permissions/__tests__/hostapi-effect-completeness.test.ts index 42eb8cbb0..6dcca95e2 100644 --- a/src/permissions/__tests__/hostapi-effect-completeness.test.ts +++ b/src/permissions/__tests__/hostapi-effect-completeness.test.ts @@ -19,10 +19,7 @@ * fail-closed `unclassifiedHostApiMethod` WRITE for an unmapped path, and is a * PURE side-effect (it never alters the wrapped method's behavior). */ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { mkdtempSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const harness = vi.hoisted(() => ({ capturedRuntimeOptions: null as Record | null, @@ -102,6 +99,13 @@ import { type EffectLedger, } from "../effect-ledger.js"; import type { PluginHostApi } from "../../plugins/types.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); + +afterEach(async () => { + await resources.cleanup(); +}); type CreateHostApi = ( pluginId: string, @@ -164,9 +168,7 @@ async function buildRealHostApi(): Promise { createHostApi, "initPluginRuntime must register a createHostApi factory", ).toBeDefined(); - const pluginDataDir = mkdtempSync( - join(tmpdir(), "lvis-hostapi-completeness-"), - ); + const pluginDataDir = resources.makeTmpDir("lvis-hostapi-completeness-"); // Build with the FULL capability vocabulary, not a sampled subset. A // namespace/method wired ONLY under a capability ABSENT from the fixture would // escape BOTH the non-plain-namespace assertion AND the SOT-coverage assertion diff --git a/src/permissions/__tests__/permission-manager-legacy-null-verdict.test.ts b/src/permissions/__tests__/permission-manager-legacy-null-verdict.test.ts index 763c4f4d4..4d8a23fa8 100644 --- a/src/permissions/__tests__/permission-manager-legacy-null-verdict.test.ts +++ b/src/permissions/__tests__/permission-manager-legacy-null-verdict.test.ts @@ -11,8 +11,6 @@ * guard for that gate. */ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { mkdtempSync } from "node:fs"; -import { tmpdir } from "node:os"; import { join } from "node:path"; // vi.mock must be at top level (hoisted). We feed the mock from a @@ -42,26 +40,32 @@ vi.mock("../../audit/sandbox-audit-sink.js", async () => { }); import { PermissionManager } from "../permission-manager.js"; -import { VerdictCache } from "../reviewer/verdict-cache.js"; import { DeferredQueue } from "../reviewer/deferred-queue.js"; import { LlmRiskClassifier, RuleBasedRiskClassifier, type RiskClassifier, } from "../reviewer/risk-classifier.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); function tmpFile(name: string): string { - const dir = mkdtempSync(join(tmpdir(), "lvis-pm-legacy-null-")); + const dir = resources.makeTmpDir("lvis-pm-legacy-null-"); return join(dir, name); } +afterEach(async () => { + await resources.cleanup(); +}); + function makeManager(): { pm: PermissionManager; classifier: RiskClassifier; } { const pm = new PermissionManager(tmpFile("permissions.json")); const classifier = new RuleBasedRiskClassifier(); - const cache = new VerdictCache(tmpFile("reviewer-cache.jsonl")); + const cache = resources.makeVerdictCache(tmpFile("reviewer-cache.jsonl")); const queue = new DeferredQueue(tmpFile("deferred-queue.jsonl")); pm.setReviewer({ classifier, cache, deferredQueue: queue }); return { pm, classifier }; @@ -215,7 +219,7 @@ describe("PermissionManager — fail-closed gate against legacy null-verdict ent }, "gpt-4o-mini", ); - const cache = new VerdictCache(tmpFile("reviewer-cache.jsonl")); + const cache = resources.makeVerdictCache(tmpFile("reviewer-cache.jsonl")); const queue = new DeferredQueue(tmpFile("deferred-queue.jsonl")); pm.setReviewer({ classifier, cache, deferredQueue: queue }); diff --git a/src/permissions/__tests__/permission-manager-reviewer.test.ts b/src/permissions/__tests__/permission-manager-reviewer.test.ts index edf4f13b4..c19f03ea9 100644 --- a/src/permissions/__tests__/permission-manager-reviewer.test.ts +++ b/src/permissions/__tests__/permission-manager-reviewer.test.ts @@ -2,7 +2,6 @@ * Permission policy Phase 3 — PermissionManager.dispatchReviewer + setReviewer wiring. */ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -29,12 +28,19 @@ import { unmarkPluginWorkerWrapped, setActiveSandboxCapability, } from "../sandbox-capability.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); function tmpFile(name: string): string { - const dir = mkdtempSync(join(tmpdir(), "lvis-pm-reviewer-")); + const dir = resources.makeTmpDir("lvis-pm-reviewer-"); return join(dir, name); } +afterEach(async () => { + await resources.cleanup(); +}); + function allowedDir(path: string): string { return caseFoldForMatch(canonicalizePathForMatch(path)); } @@ -46,7 +52,7 @@ function makeManager(): { classifier: RiskClassifier; } { const pm = new PermissionManager(tmpFile("permissions.json")); - const cache = new VerdictCache(tmpFile("reviewer-cache.jsonl")); + const cache = resources.makeVerdictCache(tmpFile("reviewer-cache.jsonl")); const queue = new DeferredQueue(tmpFile("deferred-queue.jsonl")); const classifier = new RuleBasedRiskClassifier(); pm.setReviewer({ classifier, cache, deferredQueue: queue }); @@ -670,7 +676,7 @@ describe("MAJOR-1 R2: dispatchReviewer threads abortSignal to LlmRiskClassifier. }, ); - const cache = new VerdictCache(tmpFile("reviewer-cache.jsonl")); + const cache = resources.makeVerdictCache(tmpFile("reviewer-cache.jsonl")); const queue = new DeferredQueue(tmpFile("deferred-queue.jsonl")); pm.setReviewer({ classifier: llmClassifier, cache, deferredQueue: queue }); @@ -708,7 +714,7 @@ describe("MAJOR-1 R2: dispatchReviewer threads abortSignal to LlmRiskClassifier. }), }; const llmClassifier = new LlmRiskClassifier(providerStub, "gpt-4o-mini", "deny"); - const cache = new VerdictCache(tmpFile("reviewer-cache.jsonl")); + const cache = resources.makeVerdictCache(tmpFile("reviewer-cache.jsonl")); const queue = new DeferredQueue(tmpFile("deferred-queue.jsonl")); pm.setReviewer({ classifier: llmClassifier, cache, deferredQueue: queue }); @@ -761,6 +767,8 @@ describe("#664 flood guard — degraded rule reviewer does not over-defer headle verdictCachePath: tmpFile("flood-cache.jsonl"), deferredQueuePath: tmpFile("flood-queue.jsonl"), }); + const wiredCache = pm.getVerdictCache(); + if (wiredCache) resources.trackFlushable(wiredCache); expect(wiring.runtimeMode).toBe("llm-degraded-to-rule"); expect(pm.isReviewerDegradedToRule()).toBe(true); @@ -830,7 +838,7 @@ describe("reviewer outcome provenance and base-cache safety", () => { complete: () => Promise, ) { const pm = new PermissionManager(tmpFile("permissions.json")); - const cache = new VerdictCache(tmpFile("reviewer-cache.jsonl")); + const cache = resources.makeVerdictCache(tmpFile("reviewer-cache.jsonl")); const queue = new DeferredQueue(tmpFile("deferred-queue.jsonl")); const provider = { complete: vi.fn(complete) }; pm.setReviewer({ @@ -933,7 +941,7 @@ describe("reviewer outcome provenance and base-cache safety", () => { const pm = new PermissionManager(tmpFile("permissions.json")); pm.setReviewer({ classifier: { classify }, - cache: new VerdictCache(tmpFile("reviewer-cache.jsonl")), + cache: resources.makeVerdictCache(tmpFile("reviewer-cache.jsonl")), deferredQueue: queue, }); diff --git a/src/permissions/__tests__/permission-review-scenario-board.test.ts b/src/permissions/__tests__/permission-review-scenario-board.test.ts index fcab7e947..fe2fdc49a 100644 --- a/src/permissions/__tests__/permission-review-scenario-board.test.ts +++ b/src/permissions/__tests__/permission-review-scenario-board.test.ts @@ -6,14 +6,12 @@ * permission policy path. The board is a PR artifact, but these tests keep it * tied to executable behavior instead of letting it drift into a static mockup. */ -import { describe, expect, it, vi } from "vitest"; -import { readFileSync, mkdtempSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { readFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { PermissionManager } from "../permission-manager.js"; import { DeferredQueue } from "../reviewer/deferred-queue.js"; -import { VerdictCache } from "../reviewer/verdict-cache.js"; import { RuleBasedRiskClassifier, type RiskClassifier, @@ -26,20 +24,25 @@ import type { ToolCategory, ToolSource } from "../../tools/types.js"; import { buildPluginToolsForTest } from "../../plugins/__tests__/plugin-tool-test-fixture.js"; import type { PluginManifest } from "../../plugins/types.js"; import type { PluginRuntime } from "../../plugins/runtime.js"; -import { cleanupTmpDir } from "../../testing/tmp-dir-teardown.js"; +import { PermissionTestResources } from "./test-resources.js"; const BOARD_PATH = resolve(process.cwd(), "docs/design/permission-review-scenario-board-v2.html"); +const resources = new PermissionTestResources(); function tmpFile(name: string): string { - const dir = mkdtempSync(join(tmpdir(), "lvis-permission-scenarios-")); + const dir = resources.makeTmpDir("lvis-permission-scenarios-"); return join(dir, name); } +afterEach(async () => { + await resources.cleanup(); +}); + function makeManager( mode: "default" | "strict" | "auto" | "allow" = "default", classifier: RiskClassifier = new RuleBasedRiskClassifier(), ): { pm: PermissionManager; queue: DeferredQueue; cleanup: () => Promise } { - const dir = mkdtempSync(join(tmpdir(), "lvis-permission-scenarios-")); + const dir = resources.makeTmpDir("lvis-permission-scenarios-"); const pm = new PermissionManager(join(dir, "permissions.json")); const queue = new DeferredQueue(join(dir, "deferred-queue.jsonl")); pm.setMode(mode); @@ -51,10 +54,10 @@ function makeManager( } pm.setReviewer({ classifier, - cache: new VerdictCache(join(dir, "reviewer-cache.jsonl")), + cache: resources.makeVerdictCache(join(dir, "reviewer-cache.jsonl")), deferredQueue: queue, }); - return { pm, queue, cleanup: () => cleanupTmpDir(dir) }; + return { pm, queue, cleanup: () => resources.cleanup() }; } function fixedClassifier(verdict: RiskVerdict): RiskClassifier { diff --git a/src/permissions/__tests__/permission-settings-store-664-migration.test.ts b/src/permissions/__tests__/permission-settings-store-664-migration.test.ts index 55d4ca766..70d1e46ef 100644 --- a/src/permissions/__tests__/permission-settings-store-664-migration.test.ts +++ b/src/permissions/__tests__/permission-settings-store-664-migration.test.ts @@ -13,14 +13,20 @@ * Defends the migration against silently flipping a fail-closed user to * the new pass-through-LOW semantic at upgrade time. */ -import { describe, it, expect } from "vitest"; -import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, it, expect } from "vitest"; +import { readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { readPermissionSettings, migrateLegacyDisabledMode, } from "../permission-settings-store.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); + +afterEach(async () => { + await resources.cleanup(); +}); function writeRaw(dir: string, body: object): string { const p = join(dir, "settings.json"); @@ -119,7 +125,7 @@ describe("migrateLegacyDisabledMode — issue #664 idempotency", () => { describe("readPermissionSettings — issue #664 migration end-to-end", () => { it("persists the migrated file on first read", () => { - const dir = mkdtempSync(join(tmpdir(), "lvis-664-mig-")); + const dir = resources.makeTmpDir("lvis-664-mig-"); const filePath = writeRaw(dir, { permissions: { reviewer: { @@ -149,7 +155,7 @@ describe("readPermissionSettings — issue #664 migration end-to-end", () => { }); it("preserves user-chosen disabled after migration marker present", () => { - const dir = mkdtempSync(join(tmpdir(), "lvis-664-userpick-")); + const dir = resources.makeTmpDir("lvis-664-userpick-"); const filePath = writeRaw(dir, { permissions: { reviewer: { diff --git a/src/permissions/__tests__/permission-slash-reviewer.test.ts b/src/permissions/__tests__/permission-slash-reviewer.test.ts index b08c4c7f4..25e93ea4d 100644 --- a/src/permissions/__tests__/permission-slash-reviewer.test.ts +++ b/src/permissions/__tests__/permission-slash-reviewer.test.ts @@ -1,9 +1,8 @@ /** * Permission policy Phase 3 — `/permission reviewer` slash + settings persistence tests. */ -import { describe, it, expect, vi } from "vitest"; -import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, it, expect, vi } from "vitest"; +import { readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { parsePermissionReviewerCommand, @@ -16,12 +15,19 @@ import { normalizePermissionSettings, writePermissionSettings, } from "../permission-settings-store.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); function tmpSettingsPath(): string { - const dir = mkdtempSync(join(tmpdir(), "lvis-perm-reviewer-")); + const dir = resources.makeTmpDir("lvis-perm-reviewer-"); return join(dir, "settings.json"); } +afterEach(async () => { + await resources.cleanup(); +}); + describe("parsePermissionReviewerCommand", () => { it("parses 'show'", () => { expect(parsePermissionReviewerCommand("show")).toEqual({ verb: "show", value: "" }); diff --git a/src/permissions/__tests__/permission-slash.test.ts b/src/permissions/__tests__/permission-slash.test.ts index bd8347c99..4246d18fb 100644 --- a/src/permissions/__tests__/permission-slash.test.ts +++ b/src/permissions/__tests__/permission-slash.test.ts @@ -1,6 +1,5 @@ -import { describe, it, expect, vi } from "vitest"; -import { mkdtempSync, mkdirSync, readFileSync, writeFileSync, chmodSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, it, expect, vi } from "vitest"; +import { mkdirSync, readFileSync, writeFileSync, chmodSync } from "node:fs"; import { join } from "node:path"; import { parsePermissionDirCommand, @@ -15,12 +14,19 @@ import { normalizePermissionSettings, } from "../permission-settings-store.js"; import { validateDirectoryAddition } from "../allowed-directories.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); function tmpSettingsPath(): string { - const dir = mkdtempSync(join(tmpdir(), "lvis-perm-slash-")); + const dir = resources.makeTmpDir("lvis-perm-slash-"); return join(dir, "settings.json"); } +afterEach(async () => { + await resources.cleanup(); +}); + describe("parsePermissionDirCommand", () => { it("parses 'allow '", () => { const r = parsePermissionDirCommand("allow /Users/ken/work"); @@ -190,7 +196,7 @@ describe("dispatchPermissionDirCommand — deny", () => { describe("dispatchPermissionDirCommand — injected workspace lifecycle", () => { it("delegates allow with the permission-slash source and returns lifecycle persistence", async () => { - const directory = mkdtempSync(join(tmpdir(), "lvis-perm-lifecycle-allow-")); + const directory = resources.makeTmpDir("lvis-perm-lifecycle-allow-"); const validation = validateDirectoryAddition(directory); expect(validation.ok).toBe(true); const persisted = [directory, join(directory, "other")]; @@ -217,7 +223,7 @@ describe("dispatchPermissionDirCommand — injected workspace lifecycle", () => }); it("delegates deny with the permission-slash source and returns lifecycle persistence", async () => { - const directory = mkdtempSync(join(tmpdir(), "lvis-perm-lifecycle-deny-")); + const directory = resources.makeTmpDir("lvis-perm-lifecycle-deny-"); const persisted = [join(directory, "remaining")]; const allowDirectory = vi.fn(async () => [] as string[]); const denyDirectory = vi.fn(async () => persisted); @@ -233,7 +239,7 @@ describe("dispatchPermissionDirCommand — injected workspace lifecycle", () => }); it("returns a stable structured error when lifecycle allow rejects", async () => { - const directory = mkdtempSync(join(tmpdir(), "lvis-perm-lifecycle-allow-fail-")); + const directory = resources.makeTmpDir("lvis-perm-lifecycle-allow-fail-"); const allowDirectory = vi.fn(async () => { throw new Error("private allow failure"); }); @@ -253,7 +259,7 @@ describe("dispatchPermissionDirCommand — injected workspace lifecycle", () => }); it("returns a stable structured error when lifecycle deny rejects", async () => { - const directory = mkdtempSync(join(tmpdir(), "lvis-perm-lifecycle-deny-fail-")); + const directory = resources.makeTmpDir("lvis-perm-lifecycle-deny-fail-"); const allowDirectory = vi.fn(async () => [] as string[]); const denyDirectory = vi.fn(async () => { throw new Error("private deny failure"); @@ -270,7 +276,7 @@ describe("dispatchPermissionDirCommand — injected workspace lifecycle", () => }); it("fails closed for persistent mutations when the host lifecycle is unavailable", async () => { - const directory = mkdtempSync(join(tmpdir(), "lvis-perm-lifecycle-missing-")); + const directory = resources.makeTmpDir("lvis-perm-lifecycle-missing-"); const allowResult = await dispatchPermissionDirCommand({ verb: "allow", @@ -413,7 +419,7 @@ describe("writePermissionSettings — alias is dropped on write", () => { */ describe("dispatchPermissionHooksCommand — renderer broadcast gating (FU2)", () => { function hooksFixture() { - const tmpDir = mkdtempSync(join(tmpdir(), "lvis-hook-trust-bcast-")); + const tmpDir = resources.makeTmpDir("lvis-hook-trust-bcast-"); const hooksDir = join(tmpDir, "hooks"); const disabledDir = join(hooksDir, ".disabled"); const lockfilePath = join(hooksDir, ".lockfile.json"); diff --git a/src/permissions/__tests__/sandbox-write-jail.test.ts b/src/permissions/__tests__/sandbox-write-jail.test.ts index 78603c7d6..1a2dc56be 100644 --- a/src/permissions/__tests__/sandbox-write-jail.test.ts +++ b/src/permissions/__tests__/sandbox-write-jail.test.ts @@ -4,23 +4,28 @@ * (union of owner plugin sandbox root + allowed directories, canonicalized * and de-duplicated) without invoking any OS sandbox primitive. */ -import { describe, it, expect } from "vitest"; -import { mkdtempSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, it, expect } from "vitest"; import { join } from "node:path"; import { deriveSandboxWritePaths } from "../sandbox-write-jail.js"; import { canonicalizePathForMatch } from "../sensitive-paths.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); + +afterEach(async () => { + await resources.cleanup(); +}); describe("deriveSandboxWritePaths", () => { it("jails to the allowed directories when there is no owner plugin (builtin shell)", () => { - const cwd = mkdtempSync(join(tmpdir(), "lvis-jail-cwd-")); + const cwd = resources.makeTmpDir("lvis-jail-cwd-"); const result = deriveSandboxWritePaths({ allowedDirectories: [cwd] }); expect(result).toEqual([canonicalizePathForMatch(cwd)]); }); it("includes the owner plugin sandbox root when the tool is plugin-owned", () => { - const cwd = mkdtempSync(join(tmpdir(), "lvis-jail-cwd-")); - const pluginRoot = mkdtempSync(join(tmpdir(), "lvis-jail-plugin-")); + const cwd = resources.makeTmpDir("lvis-jail-cwd-"); + const pluginRoot = resources.makeTmpDir("lvis-jail-plugin-"); const result = deriveSandboxWritePaths({ ownerPluginSandboxRoot: pluginRoot, allowedDirectories: [cwd], @@ -31,9 +36,9 @@ describe("deriveSandboxWritePaths", () => { }); it("unions the owner plugin root with all in-scope allowed directories", () => { - const cwd = mkdtempSync(join(tmpdir(), "lvis-jail-cwd-")); - const extra = mkdtempSync(join(tmpdir(), "lvis-jail-extra-")); - const pluginRoot = mkdtempSync(join(tmpdir(), "lvis-jail-plugin-")); + const cwd = resources.makeTmpDir("lvis-jail-cwd-"); + const extra = resources.makeTmpDir("lvis-jail-extra-"); + const pluginRoot = resources.makeTmpDir("lvis-jail-plugin-"); const result = deriveSandboxWritePaths({ ownerPluginSandboxRoot: pluginRoot, allowedDirectories: [cwd, extra], @@ -48,14 +53,14 @@ describe("deriveSandboxWritePaths", () => { }); it("de-duplicates paths that canonicalize to the same location", () => { - const cwd = mkdtempSync(join(tmpdir(), "lvis-jail-cwd-")); + const cwd = resources.makeTmpDir("lvis-jail-cwd-"); // Same dir passed twice (e.g. cwd also listed as an extra) collapses to one. const result = deriveSandboxWritePaths({ allowedDirectories: [cwd, cwd] }); expect(result).toEqual([canonicalizePathForMatch(cwd)]); }); it("does not treat the owner plugin root as writable when it is undefined", () => { - const cwd = mkdtempSync(join(tmpdir(), "lvis-jail-cwd-")); + const cwd = resources.makeTmpDir("lvis-jail-cwd-"); const result = deriveSandboxWritePaths({ ownerPluginSandboxRoot: undefined, allowedDirectories: [cwd], @@ -64,7 +69,7 @@ describe("deriveSandboxWritePaths", () => { }); it("drops empty-string entries from both sources", () => { - const cwd = mkdtempSync(join(tmpdir(), "lvis-jail-cwd-")); + const cwd = resources.makeTmpDir("lvis-jail-cwd-"); const result = deriveSandboxWritePaths({ ownerPluginSandboxRoot: "", allowedDirectories: ["", cwd], @@ -77,7 +82,7 @@ describe("deriveSandboxWritePaths", () => { }); it("canonicalizes paths (the OS jail and the reviewer see identical strings)", () => { - const base = mkdtempSync(join(tmpdir(), "lvis-jail-canon-")); + const base = resources.makeTmpDir("lvis-jail-canon-"); // A path with a redundant '.' segment must canonicalize to the same // string the reviewer's sensitive-path layer produces. const dotted = join(base, ".", ""); diff --git a/src/permissions/__tests__/sensitive-paths-canonicalize.test.ts b/src/permissions/__tests__/sensitive-paths-canonicalize.test.ts index edf727748..f422d3df6 100644 --- a/src/permissions/__tests__/sensitive-paths-canonicalize.test.ts +++ b/src/permissions/__tests__/sensitive-paths-canonicalize.test.ts @@ -16,15 +16,21 @@ * These tests use `realpath`'d tmpdir paths to keep the test * fs-independent (darwin /var → /private/var symlink). */ -import { describe, it, expect } from "vitest"; -import { mkdtempSync, mkdirSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, it, expect } from "vitest"; +import { mkdirSync } from "node:fs"; import { isAbsolute, join } from "node:path"; import { canonicalizePathForMatch } from "../sensitive-paths.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); + +afterEach(async () => { + await resources.cleanup(); +}); describe("canonicalizePathForMatch — security MAJOR-3 bypass vectors", () => { it("collapses `..` segments", () => { - const root = mkdtempSync(join(tmpdir(), "lvis-canon-dot-")); + const root = resources.makeTmpDir("lvis-canon-dot-"); mkdirSync(join(root, "a/b/c"), { recursive: true }); // //a/b/c/../../b → //a/b const traversed = join(root, "a/b/c/../../b"); @@ -34,7 +40,7 @@ describe("canonicalizePathForMatch — security MAJOR-3 bypass vectors", () => { }); it("collapses duplicate slashes", () => { - const root = mkdtempSync(join(tmpdir(), "lvis-canon-slash-")); + const root = resources.makeTmpDir("lvis-canon-slash-"); mkdirSync(join(root, "x"), { recursive: true }); const dup = `${root}///x`; const canonical = canonicalizePathForMatch(dup); @@ -44,7 +50,7 @@ describe("canonicalizePathForMatch — security MAJOR-3 bypass vectors", () => { }); it("trailing slash does not survive resolve", () => { - const root = mkdtempSync(join(tmpdir(), "lvis-canon-trail-")); + const root = resources.makeTmpDir("lvis-canon-trail-"); mkdirSync(join(root, "leaf"), { recursive: true }); const trailed = `${root}/leaf/`; const canonical = canonicalizePathForMatch(trailed); @@ -57,7 +63,7 @@ describe("canonicalizePathForMatch — security MAJOR-3 bypass vectors", () => { // "café" — composed (NFC) e + ́ and decomposed (NFD). const nfc = "café"; // 4 code points (composed) const nfd = "café"; // 5 code points (decomposed) - const root = mkdtempSync(join(tmpdir(), "lvis-canon-nfd-")); + const root = resources.makeTmpDir("lvis-canon-nfd-"); const composed = canonicalizePathForMatch(`${root}/${nfc}`); const decomposed = canonicalizePathForMatch(`${root}/${nfd}`); // After NFC normalization both forms collapse to the same string. @@ -79,7 +85,7 @@ describe("canonicalizePathForMatch — security MAJOR-3 bypass vectors", () => { }); it("repeated canonicalize is idempotent (frozen-canonical contract)", () => { - const root = mkdtempSync(join(tmpdir(), "lvis-canon-idem-")); + const root = resources.makeTmpDir("lvis-canon-idem-"); mkdirSync(join(root, "deep/nest/path"), { recursive: true }); const raw = `${root}//deep/./nest/../nest/path/`; const once = canonicalizePathForMatch(raw); diff --git a/src/permissions/__tests__/sensitive-paths.test.ts b/src/permissions/__tests__/sensitive-paths.test.ts index 0f316c767..61e723a2d 100644 --- a/src/permissions/__tests__/sensitive-paths.test.ts +++ b/src/permissions/__tests__/sensitive-paths.test.ts @@ -5,9 +5,8 @@ * canonicalizePathForMatch (frozen-canonical + bounded walk-up), and * caseFoldForMatch. */ -import { describe, it, expect } from "vitest"; -import { mkdtempSync, symlinkSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { afterEach, describe, it, expect } from "vitest"; +import { symlinkSync } from "node:fs"; import { isAbsolute, join } from "node:path"; import { SENSITIVE_PATH_PATTERNS, @@ -17,6 +16,13 @@ import { caseFoldForMatch, MAX_WALK_UP, } from "../sensitive-paths.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); + +afterEach(async () => { + await resources.cleanup(); +}); describe("SENSITIVE_PATH_PATTERNS", () => { it("is a non-empty readonly list", () => { @@ -339,7 +345,7 @@ describe("canonicalizePathForMatch", () => { // Stage a self-symlink in a temp dir; canonicalize a child path and // confirm we get a string back (no hang, no throw). On platforms that // forbid self-symlinks, the test still passes via the bounded cap. - const root = mkdtempSync(join(tmpdir(), "lvis-canonical-")); + const root = resources.makeTmpDir("lvis-canonical-"); const linkDir = join(root, "loop"); try { symlinkSync(linkDir, linkDir); // self-loop diff --git a/src/permissions/__tests__/test-resources.test.ts b/src/permissions/__tests__/test-resources.test.ts new file mode 100644 index 000000000..699dedb50 --- /dev/null +++ b/src/permissions/__tests__/test-resources.test.ts @@ -0,0 +1,97 @@ +import { existsSync } from "node:fs"; +import { describe, expect, it, vi } from "vitest"; + +import { cleanupTmpDir } from "../../testing/tmp-dir-teardown.js"; +import { PermissionTestResources } from "./test-resources.js"; + +describe("PermissionTestResources cleanup failures", () => { + it("settles every flushable and directory before reporting aggregate failures", async () => { + const flushError = new Error("flush failed"); + const directoryError = new Error("directory cleanup failed"); + let rejectFlush = true; + let rejectDirectory = true; + let failedDir = ""; + const cleanupDir = vi.fn(async (dir: string) => { + if (dir === failedDir && rejectDirectory) { + rejectDirectory = false; + throw directoryError; + } + await cleanupTmpDir(dir); + }); + const resources = new PermissionTestResources(cleanupDir); + failedDir = resources.makeTmpDir("lvis-permission-resources-failed-"); + const cleanedDir = resources.makeTmpDir("lvis-permission-resources-cleaned-"); + const retryingFlush = vi.fn(async () => { + if (rejectFlush) { + rejectFlush = false; + throw flushError; + } + }); + const successfulFlush = vi.fn(async () => {}); + resources.trackFlushable({ flush: retryingFlush }); + resources.trackFlushable({ flush: successfulFlush }); + + try { + const error = await resources.cleanup().catch((failure: unknown) => failure); + + expect(error).toBeInstanceOf(AggregateError); + expect((error as AggregateError).errors).toEqual([flushError, directoryError]); + expect(retryingFlush).toHaveBeenCalledTimes(1); + expect(successfulFlush).toHaveBeenCalledTimes(1); + expect(cleanupDir.mock.calls.map(([dir]) => dir)).toEqual([failedDir, cleanedDir]); + expect(existsSync(failedDir)).toBe(true); + expect(existsSync(cleanedDir)).toBe(false); + + cleanupDir.mockClear(); + await resources.cleanup(); + + expect(retryingFlush).toHaveBeenCalledTimes(2); + expect(successfulFlush).toHaveBeenCalledTimes(1); + expect(cleanupDir.mock.calls.map(([dir]) => dir)).toEqual([failedDir, cleanedDir]); + expect(existsSync(failedDir)).toBe(false); + expect(existsSync(cleanedDir)).toBe(false); + } finally { + rejectFlush = false; + rejectDirectory = false; + await resources.cleanup(); + } + }); + + it("retries only failed directories after every flush succeeds", async () => { + const directoryError = new Error("directory cleanup failed"); + let rejectDirectory = true; + let failedDir = ""; + const cleanupDir = vi.fn(async (dir: string) => { + if (dir === failedDir && rejectDirectory) { + rejectDirectory = false; + throw directoryError; + } + await cleanupTmpDir(dir); + }); + const resources = new PermissionTestResources(cleanupDir); + failedDir = resources.makeTmpDir("lvis-permission-resources-retry-"); + const cleanedDir = resources.makeTmpDir("lvis-permission-resources-done-"); + const flush = vi.fn(async () => {}); + resources.trackFlushable({ flush }); + + try { + const error = await resources.cleanup().catch((failure: unknown) => failure); + + expect(error).toBeInstanceOf(AggregateError); + expect((error as AggregateError).errors).toEqual([directoryError]); + expect(flush).toHaveBeenCalledTimes(1); + expect(cleanupDir.mock.calls.map(([dir]) => dir)).toEqual([failedDir, cleanedDir]); + + cleanupDir.mockClear(); + await resources.cleanup(); + + expect(flush).toHaveBeenCalledTimes(1); + expect(cleanupDir.mock.calls.map(([dir]) => dir)).toEqual([failedDir]); + expect(existsSync(failedDir)).toBe(false); + expect(existsSync(cleanedDir)).toBe(false); + } finally { + rejectDirectory = false; + await resources.cleanup(); + } + }); +}); diff --git a/src/permissions/__tests__/test-resources.ts b/src/permissions/__tests__/test-resources.ts new file mode 100644 index 000000000..e2536eb83 --- /dev/null +++ b/src/permissions/__tests__/test-resources.ts @@ -0,0 +1,70 @@ +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { cleanupTmpDir } from "../../testing/tmp-dir-teardown.js"; +import { VerdictCache } from "../reviewer/verdict-cache.js"; + +interface FlushableResource { + flush(): Promise; +} + +export class PermissionTestResources { + private readonly tmpDirs: string[] = []; + private readonly flushables = new Set(); + + constructor( + private readonly cleanupDir: (dir: string) => Promise = cleanupTmpDir, + ) {} + + makeTmpDir(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + this.tmpDirs.push(dir); + return dir; + } + + trackFlushable(resource: T): T { + this.flushables.add(resource); + return resource; + } + + makeVerdictCache(path: string): VerdictCache { + return this.trackFlushable(new VerdictCache(path)); + } + + async cleanup(): Promise { + const errors: unknown[] = []; + let flushFailed = false; + + for (const resource of [...this.flushables]) { + try { + await resource.flush(); + this.flushables.delete(resource); + } catch (error) { + flushFailed = true; + errors.push(error); + } + } + + const cleanedDirs: string[] = []; + for (const dir of [...this.tmpDirs]) { + try { + await this.cleanupDir(dir); + cleanedDirs.push(dir); + } catch (error) { + errors.push(error); + } + } + + if (!flushFailed) { + for (const dir of cleanedDirs) { + const index = this.tmpDirs.indexOf(dir); + if (index >= 0) this.tmpDirs.splice(index, 1); + } + } + + if (errors.length > 0) { + throw new AggregateError(errors, "Failed to clean up permission test resources"); + } + } +} diff --git a/src/permissions/__tests__/verdict-cache.test.ts b/src/permissions/__tests__/verdict-cache.test.ts index 2b8016e69..69ba02ffb 100644 --- a/src/permissions/__tests__/verdict-cache.test.ts +++ b/src/permissions/__tests__/verdict-cache.test.ts @@ -9,10 +9,9 @@ * - HIGH verdicts are cached too * - Persistence to file across instances */ -import { describe, it, expect, beforeEach } from "vitest"; -import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { afterEach, describe, it, expect, beforeEach } from "vitest"; +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; import { MAX_VERDICT_CACHE_ENTRIES, VerdictCache, @@ -23,12 +22,19 @@ import { type VerdictCacheContext, } from "../reviewer/verdict-cache.js"; import type { RiskVerdict } from "../reviewer/risk-classifier.js"; +import { PermissionTestResources } from "./test-resources.js"; + +const resources = new PermissionTestResources(); function tmpCachePath(): string { - const dir = mkdtempSync(join(tmpdir(), "lvis-verdict-cache-")); + const dir = resources.makeTmpDir("lvis-verdict-cache-"); return join(dir, "reviewer-cache.jsonl"); } +afterEach(async () => { + await resources.cleanup(); +}); + const CTX: VerdictCacheContext = { allowedDirectories: ["/Users/ken/work", "/Users/ken/.lvis"], scope: { mode: "deny-all" }, @@ -42,6 +48,15 @@ const LOOKUP: VerdictCacheLookupKey = { finalInput: { path: "/Users/ken/work/a.md", count: 5 }, }; +function expiredCacheLine(lookup: VerdictCacheLookupKey = LOOKUP): string { + return JSON.stringify({ + key: computeCacheKey(lookup), + verdict: { level: "low", reason: "ok" }, + expiresAt: Date.now() - 1000, + invalidationKey: computeInvalidationKey(CTX), + }); +} + describe("canonicalInputShape", () => { it("replaces values with type names", () => { expect(canonicalInputShape({ path: "/a", count: 5 })).toBe( @@ -205,7 +220,7 @@ describe("VerdictCache lookup states", () => { beforeEach(() => { path = tmpCachePath(); - cache = new VerdictCache(path); + cache = resources.makeVerdictCache(path); }); it("returns miss-not-found for empty cache", () => { @@ -244,19 +259,55 @@ describe("VerdictCache lookup states", () => { }); it("miss-expired when expiresAt < now", () => { - const expired = JSON.stringify({ - key: computeCacheKey(LOOKUP), - verdict: { level: "low", reason: "ok" }, - expiresAt: Date.now() - 1000, - invalidationKey: computeInvalidationKey(CTX), - }); - writeFileSync(path, expired + "\n", "utf-8"); - cache = new VerdictCache(path); + writeFileSync(path, expiredCacheLine() + "\n", "utf-8"); + cache = resources.makeVerdictCache(path); const r = cache.lookup(LOOKUP, CTX); expect(r.hit).toBe(false); expect(r.reason).toBe("miss-expired"); }); + it("flush waits for the expired-entry rewrite", async () => { + writeFileSync(path, expiredCacheLine() + "\n", "utf-8"); + cache = resources.makeVerdictCache(path); + + expect(cache.lookup(LOOKUP, CTX).reason).toBe("miss-expired"); + await cache.flush(); + + expect(readFileSync(path, "utf-8")).toBe(""); + }); + + it("flush drains consecutive expired-entry rewrites", async () => { + const secondLookup: VerdictCacheLookupKey = { + ...LOOKUP, + toolName: "fs_delete", + finalInput: { path: "/Users/ken/work/b.md" }, + }; + writeFileSync( + path, + `${expiredCacheLine(LOOKUP)}\n${expiredCacheLine(secondLookup)}\n`, + "utf-8", + ); + cache = resources.makeVerdictCache(path); + + expect(cache.lookup(LOOKUP, CTX).reason).toBe("miss-expired"); + expect(cache.lookup(secondLookup, CTX).reason).toBe("miss-expired"); + await cache.flush(); + + expect(readFileSync(path, "utf-8")).toBe(""); + }); + + it("does not recreate the cache directory after flush and cleanup", async () => { + writeFileSync(path, expiredCacheLine() + "\n", "utf-8"); + cache = resources.makeVerdictCache(path); + + expect(cache.lookup(LOOKUP, CTX).reason).toBe("miss-expired"); + await cache.flush(); + await resources.cleanup(); + await new Promise((resolve) => setImmediate(resolve)); + + expect(existsSync(dirname(path))).toBe(false); + }); + it("prunes stale entries and can still hit an older current entry", async () => { const staleCtx: VerdictCacheContext = { allowedDirectories: ["/stale"], @@ -274,9 +325,9 @@ describe("VerdictCache lookup states", () => { describe("VerdictCache persistence", () => { it("entry survives across cache instances", async () => { const path = tmpCachePath(); - const a = new VerdictCache(path); + const a = resources.makeVerdictCache(path); await a.store(LOOKUP, CTX, { level: "medium", reason: "x" }); - const b = new VerdictCache(path); + const b = resources.makeVerdictCache(path); const r = b.lookup(LOOKUP, CTX); expect(r.hit).toBe(true); expect(r.verdict?.level).toBe("medium"); @@ -284,7 +335,7 @@ describe("VerdictCache persistence", () => { it("file format is JSONL", async () => { const path = tmpCachePath(); - const cache = new VerdictCache(path); + const cache = resources.makeVerdictCache(path); await cache.store(LOOKUP, CTX, { level: "low", reason: "a" }); await cache.store({ ...LOOKUP, toolName: "other" }, CTX, { level: "high", reason: "b" }); const lines = readFileSync(path, "utf-8").trim().split("\n"); @@ -300,7 +351,7 @@ describe("VerdictCache persistence", () => { it("caps stored entries to the newest cache window", async () => { const path = tmpCachePath(); - const cache = new VerdictCache(path); + const cache = resources.makeVerdictCache(path); for (let i = 0; i < MAX_VERDICT_CACHE_ENTRIES + 3; i += 1) { await cache.store({ ...LOOKUP, toolName: `tool_${i}` }, CTX, { level: "low", @@ -317,7 +368,7 @@ describe("VerdictCache persistence", () => { describe("VerdictCache invalidateMismatching (selective by invalidationKey)", () => { it("drops only entries with mismatching invalidationKey", async () => { const path = tmpCachePath(); - const cache = new VerdictCache(path); + const cache = resources.makeVerdictCache(path); const ctxA: VerdictCacheContext = { allowedDirectories: ["/A"], scope: { mode: "deny-all" }, @@ -340,7 +391,7 @@ describe("VerdictCache invalidateMismatching (selective by invalidationKey)", () it("returns 0 when no entries are stale", async () => { const path = tmpCachePath(); - const cache = new VerdictCache(path); + const cache = resources.makeVerdictCache(path); await cache.store(LOOKUP, CTX, { level: "low", reason: "a" }); const dropped = await cache.invalidateMismatching(CTX); expect(dropped).toBe(0); @@ -349,7 +400,7 @@ describe("VerdictCache invalidateMismatching (selective by invalidationKey)", () it("settings change invalidates only mismatching entries (cache integrity)", async () => { const path = tmpCachePath(); - const cache = new VerdictCache(path); + const cache = resources.makeVerdictCache(path); const old: VerdictCacheContext = { allowedDirectories: ["/old"], scope: { x: 1 }, diff --git a/src/permissions/reviewer/verdict-cache.ts b/src/permissions/reviewer/verdict-cache.ts index 28f73d8cb..f774abf40 100644 --- a/src/permissions/reviewer/verdict-cache.ts +++ b/src/permissions/reviewer/verdict-cache.ts @@ -200,6 +200,8 @@ export class VerdictCache { private readonly filePath: string; /** In-memory mirror of the file. Loaded lazily on first read. */ private entries: VerdictCacheEntry[] | null = null; + /** Best-effort rewrites scheduled by synchronous lookup pruning. */ + private readonly pendingRewrites = new Set>(); constructor(filePath?: string) { this.filePath = filePath ?? defaultPath(); @@ -330,6 +332,19 @@ export class VerdictCache { return dropped; } + /** + * Wait for background rewrites scheduled by lookup pruning to finish. + * + * Lookup stays synchronous, so stale and expired entries are rewritten on a + * best-effort background path. Lifecycle owners can await this boundary + * before removing the cache directory or otherwise retiring the instance. + */ + async flush(): Promise { + while (this.pendingRewrites.size > 0) { + await Promise.all([...this.pendingRewrites]); + } + } + /** Reset in-memory mirror (test helper). */ resetForTests(): void { this.entries = null; @@ -345,9 +360,15 @@ export class VerdictCache { } private scheduleRewrite(): void { - void this.rewriteFromMemory().catch((err) => { - log.warn(`failed to rewrite pruned cache: %s`, (err as Error).message); - }); + let pending: Promise; + pending = this.rewriteFromMemory() + .catch((err) => { + log.warn(`failed to rewrite pruned cache: %s`, (err as Error).message); + }) + .finally(() => { + this.pendingRewrites.delete(pending); + }); + this.pendingRewrites.add(pending); } private async appendLine(entry: VerdictCacheEntry): Promise {