From 9f5f9c1289f453afbb462bc9d38106b0e3a2a1e8 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:36:47 +0800 Subject: [PATCH] fix(desktop): preserve capture evidence and output ownership (#9899) * fix(desktop): retain capture identity and cleanup ownership * fix(desktop): validate destinations and finish inputs before export * fix(desktop): preserve package ownership and apt trust boundaries * fix(sqlite): preserve observed facts and distinguish special real values * fix(desktop): bind computer files to data-object metadata * test(desktop): count the tagged negative-zero cell spelling SQLite view cells encode negative zero as a real object so it stays distinct from the text "-0". The byte budget follows that spelling. * fix(desktop): keep unresolved discovered state directories Canonicalizing an unreadable path or dangling symlink aborted capture before the inventory could record that the path was unknown. Those discovered directories now keep their absolute spelling in the scope. * test(desktop): keep the capture scope suite within its line limit The unresolved state-directory case lives beside the suite so the existing callback stays within the lint limit. * fix(desktop): retain cancellation through capture input cleanup * fix(desktop): avoid reserved Pydantic settings properties --------- Co-authored-by: Cursor Agent --- docs/desktop-application-capture.md | 15 +- docs/desktop-application-export.md | 3 + .../desktop/DesktopApplicationCapture.test.ts | 64 ++++++ .../desktop/DesktopApplicationCapture.ts | 28 +-- .../desktop/DesktopApplicationExport.test.ts | 113 ++++++++++ .../desktop/DesktopApplicationExport.ts | 18 +- .../desktop/DesktopApplicationState.test.ts | 206 +++++++++++++++++- .../desktop/DesktopApplicationState.ts | 126 ++++++++--- src/application/desktop/DesktopBuildExport.ts | 3 +- .../desktop/DesktopBuildWitnessRecord.ts | 3 +- .../desktop/DesktopCaptureFailure.test.ts | 72 ++++++ .../desktop/DesktopCaptureOutput.test.ts | 15 +- .../desktop/DesktopCaptureOutput.ts | 30 +-- .../desktop/DesktopCaptureRefusals.ts | 14 +- .../desktop/DesktopCaptureScope.test.ts | 62 ++++++ .../desktop/DesktopCaptureScope.ts | 48 +++- .../desktop/DesktopExportWithheld.test.ts | 62 +++++- .../desktop/DesktopExportWithheld.ts | 68 +++--- .../desktop/DesktopOutputTree.test.ts | 30 +++ src/application/desktop/DesktopOutputTree.ts | 79 ++++++- .../desktop/DesktopSqliteState.test.ts | 77 ++++++- src/application/desktop/DesktopSqliteState.ts | 4 +- .../sqlite/SqliteDatabaseService.test.ts | 75 ++++--- .../sqlite/SqliteDatabaseService.ts | 31 ++- src/desktop/AgentEnvComputer.test.ts | 158 ++++++++++++++ src/desktop/AgentEnvComputer.ts | 24 +- .../ApplicationStateDirectories.test.ts | 54 +++++ src/desktop/ApplicationStateDirectories.ts | 20 +- src/desktop/AptRepositoryInventory.test.ts | 82 ++++++- src/desktop/AptRepositoryInventory.ts | 8 +- src/desktop/ComputerHomeFiles.ts | 4 +- src/desktop/DebianPackageInventory.test.ts | 49 +++++ src/desktop/DebianPackageInventory.ts | 21 +- src/desktop/DesktopApplicationSession.test.ts | 44 ++++ src/desktop/DesktopApplicationSession.ts | 5 +- src/desktop/DesktopContentStore.test.ts | 61 +++++- src/desktop/DesktopContentStore.ts | 35 ++- src/desktop/GpgvKeyCheck.test.ts | 43 ++++ src/desktop/GpgvKeyCheck.ts | 1 + src/desktop/GuestFileAnswer.test.ts | 38 +++- src/desktop/LibraryPackageInventory.test.ts | 4 +- src/desktop/LinuxApplicationInventory.test.ts | 192 +++++++++++++++- src/desktop/LinuxApplicationInventory.ts | 21 +- src/desktop/ProgramLookup.ts | 4 +- src/domain/desktop/appExportPython.ts | 4 +- src/domain/desktop/appWithheld.test.ts | 39 +++- src/domain/desktop/appWithheld.ts | 17 +- src/domain/desktop/desktopCapture.test.ts | 15 ++ src/domain/desktop/desktopCapture.ts | 7 +- src/domain/desktop/desktopEntry.ts | 38 ++-- src/domain/desktop/desktopEntryValues.test.ts | 13 ++ src/domain/desktop/linuxPackages.test.ts | 37 ++++ src/domain/desktop/linuxPackages.ts | 12 +- src/filesystem/canonicalCallerPath.ts | 39 ++++ .../providers/desktopExportPython.test.ts | 81 ++++++- .../providers/desktopSqliteViewBudget.test.ts | 4 +- 56 files changed, 2153 insertions(+), 267 deletions(-) create mode 100644 src/application/desktop/DesktopOutputTree.test.ts create mode 100644 src/desktop/AgentEnvComputer.test.ts create mode 100644 src/desktop/ApplicationStateDirectories.test.ts create mode 100644 src/filesystem/canonicalCallerPath.ts diff --git a/docs/desktop-application-capture.md b/docs/desktop-application-capture.md index 40498d160a..f7f0209382 100644 --- a/docs/desktop-application-capture.md +++ b/docs/desktop-application-capture.md @@ -185,8 +185,9 @@ package that installed it. A Debian package records its name, version, architecture and the apt sources that list the installed version, with the origin (`o=`) apt reports for each source in `source_origins`, such as `Ubuntu` for the Ubuntu archive and its mirrors or `LP-PPA-` for a PPA; an empty -source list means the version came from a local `.deb` or the package lists -are missing, which `package_lists_present` tells apart. apt's lists folder, +source list means apt reports no source for the installed version. +`package_lists_present` reports whether the lists are present. It is `null` +when REA cannot read the lists directory. apt's lists folder, and the trusted keys below, are where `apt-config` resolves `Dir::State::Lists`, `Dir::Etc::TrustedParts` and `Dir::Etc::Trusted` under `Dir`, so an `APT_CONFIG` file that moves them is followed: on a computer @@ -214,9 +215,9 @@ does not bring it. A program link that dpkg does not own counts as owned when dpkg owns the file it resolves to, as an alternatives link does. An `--executable` launch, or an entry no package owns, such as one in `~/.local/share/applications`, records the executable's package. When -`dpkg-query` does not finish or exits with an error for the executable or the -entry, the package is `unknown` and the limitations say which path, since a -failed lookup is not proof that no package owns it. The sources, vendor +`dpkg-query` fails for the executable or entry, or the selected path contains +characters `dpkg-query -S` would interpret as a pattern, package ownership is +`unknown` and the limitations name the path. The sources, vendor repositories and `--package-file` match below are the recorded package's; the library walk still reads the executable. @@ -243,7 +244,9 @@ writes it on Ubuntu 24.04, has that key as its only candidate, as apt trusts no other for it: REA reads the field's continuation lines as deb822 writes them, a `.` line standing for an empty one, from the key block's `BEGIN` line to its last `END` line, and records the key's path as the entry's sources -file. A key counts when gpgv reports a valid signature by it and no bad, +file. A fingerprint restriction in `Signed-By` leaves `signing_key` null. +The reason names the restriction that REA could not verify. A key counts +when gpgv reports a valid signature by it and no bad, expired, revoked or unverifiable signature by it, whatever gpgv's exit status: a release also signed by a key the host lacks makes gpgv fail. Each candidate is copied, an armored key decoded to a binary keyring, into a private diff --git a/docs/desktop-application-export.md b/docs/desktop-application-export.md index 1aac6f86e9..0dba52387d 100644 --- a/docs/desktop-application-export.md +++ b/docs/desktop-application-export.md @@ -336,6 +336,9 @@ A withheld file larger than 48 KiB is refused, since its base64 must fit one environment variable. `--withheld-copy ` copies the withheld files, as captured, into a new or empty folder outside the bundle. The result lists them as `withheld_files`, each with its `setting`, `variable` and `handle`. +REA resolves both destinations before writing either. A path that cannot be +resolved, including a dangling link, is refused; the copy and bundle cannot +contain one another, including through symbolic links. `--consent ` ships the person's consents, a `consent.json` that `record_build_consent` records, as the bundle's own `consent.json`. diff --git a/src/application/desktop/DesktopApplicationCapture.test.ts b/src/application/desktop/DesktopApplicationCapture.test.ts index 6ac6d439bf..8b1565f31e 100644 --- a/src/application/desktop/DesktopApplicationCapture.test.ts +++ b/src/application/desktop/DesktopApplicationCapture.test.ts @@ -14,17 +14,22 @@ import { join } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; +import { AnalysisCleanupLease } from "../../artifacts/AnalysisCleanupLease.js"; import { desktopApplicationCaptureSchema, desktopCaptureEventSchema, } from "../../domain/desktop/desktopCapture.js"; import type { SessionRequest } from "../../desktop/DesktopApplicationSession.js"; +import * as desktopSession from "../../desktop/DesktopApplicationSession.js"; +import * as linuxInventory from "../../desktop/LinuxApplicationInventory.js"; +import { X11WindowObserver } from "../../desktop/X11WindowObserver.js"; import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js"; import { CaptureFile } from "./DesktopCaptureOutput.js"; import { captureDesktopApplication, type DesktopCaptureRequest, } from "./DesktopApplicationCapture.js"; +import { capture as captureFixture } from "./desktopCapture.fixture.js"; const sha256 = (text: string) => createHash("sha256").update(text).digest("hex"); @@ -170,6 +175,15 @@ describe("captureDesktopApplication refusals", () => { await captureDesktopApplication(request(output), host(environment)), ), ).toContain("is not empty"); + const file = join(root, "existing-file"); + await writeFile(file, "keep"); + expect( + await captureDesktopApplication(request(file), host(environment)), + ).toMatchObject({ + code: "invalid_request", + details: { issues: [{ path: ["output"], reason: "invalid_value" }] }, + }); + expect(await readFile(file, "utf8")).toBe("keep"); }); it("refuses a window manager that does not publish its client list", async () => { @@ -226,6 +240,56 @@ describe("captureDesktopApplication refusals", () => { }); }); +it.each([false, true])( + "preserves unresolved session ownership when publication fails (unreleased: %s)", + async (unreleased) => { + const { root, home, environment } = await fixture(); + const output = join(root, "out"); + const captured = captureFixture(); + const lease = new AnalysisCleanupLease(async () => {}, ["run-pending"]); + vi.spyOn(linuxInventory, "inspectLinuxApplication").mockResolvedValue({ + ok: true, + value: { + inventory: { ...captured.application, state_directories: [] }, + limitations: [], + signingKeys: new Map(), + }, + }); + vi.spyOn(X11WindowObserver.prototype, "sample").mockResolvedValue({ + state: "listed", + windows: [], + }); + vi.spyOn(desktopSession, "runDesktopSession").mockResolvedValue({ + session: { + ...captured.session, + cleanup: { verified: !unreleased, reason: unreleased ? "busy" : null }, + }, + unreleased: unreleased ? { reason: "busy", leases: [lease] } : undefined, + unreadableWindowSamples: 0, + }); + vi.spyOn(CaptureFile.prototype, "publish").mockRejectedValue( + new Error("publication denied"), + ); + try { + const result = await captureDesktopApplication(request(output), { + ...host(environment), + callerDirectory: home, + }); + expect(result).toMatchObject({ + code: unreleased ? "cleanup_incomplete" : "execution_failure", + message: expect.stringContaining("publication denied"), + ...(unreleased + ? { details: { cleanup: "incomplete", resources: ["run-pending"] } } + : {}), + }); + expect(await readdir(output)).toEqual([]); + } finally { + vi.restoreAllMocks(); + await lease.release(); + } + }, +); + it.skipIf(process.platform !== "linux")( "returns the published evidence when cancellation arrives during publication", async () => { diff --git a/src/application/desktop/DesktopApplicationCapture.ts b/src/application/desktop/DesktopApplicationCapture.ts index 1a6de5f776..c5baabc311 100644 --- a/src/application/desktop/DesktopApplicationCapture.ts +++ b/src/application/desktop/DesktopApplicationCapture.ts @@ -19,11 +19,13 @@ import { import { createEvidence, type Evidence } from "../../domain/evidence.js"; import { DesktopContentStore } from "../../desktop/DesktopContentStore.js"; import { + DesktopSessionCleanupError, runDesktopSession, runningApplicationWindows, type SessionRequest, } from "../../desktop/DesktopApplicationSession.js"; import { inspectLinuxApplication } from "../../desktop/LinuxApplicationInventory.js"; +import { canonicalCallerPath } from "../../filesystem/canonicalCallerPath.js"; import { X11WindowObserver } from "../../desktop/X11WindowObserver.js"; import { discardFailedCapture, @@ -31,11 +33,7 @@ import { projectFailure, publishedCaptureCleanup, } from "./DesktopCaptureFailure.js"; -import { - CaptureFile, - canonicalPath, - prepareOutputDirectory, -} from "./DesktopCaptureOutput.js"; +import { CaptureFile, prepareOutputDirectory } from "./DesktopCaptureOutput.js"; import { DESKTOP_OPERATION, DESKTOP_PROVIDER, @@ -232,7 +230,7 @@ const admit = async ( throw outputRefusal(request.output, prepared.refusal); return { display, - home: await canonicalPath(home), + home: await canonicalCallerPath(home), output: prepared.path, inventory, limitations, @@ -244,7 +242,7 @@ const admit = async ( type Admitted = Awaited>; -/** Record the baseline, run the session and publish `capture.json`. */ +/** Record the baseline, run the session and assemble its Evidence. */ const recordCapture = async ( request: DesktopCaptureRequest, host: DesktopCaptureHost, @@ -252,7 +250,6 @@ const recordCapture = async ( readonly admitted: Admitted; readonly scope: CaptureScope; readonly store: DesktopContentStore; - readonly captureFile: CaptureFile; }, signal: AbortSignal | undefined, ) => { @@ -341,8 +338,7 @@ const recordCapture = async ( ], }; const evidence = captureEvidence(request, capture); - const published = await context.captureFile.publish(evidence); - return { evidence, published, session, unreleased }; + return { evidence, session, unreleased }; }; const runCapture = async ( @@ -360,22 +356,26 @@ const runCapture = async ( ); await store.prepare(); const captureFile = new CaptureFile(output); - let recorded: Awaited>; + let recorded: Awaited> | undefined; + let published: string; try { recorded = await recordCapture( request, host, - { admitted, scope, store, captureFile }, + { admitted, scope, store }, signal, ); + published = await captureFile.publish(recorded.evidence); } catch (cause: unknown) { // Nothing was published, so a failure or cancellation removes what was written. throw failedCaptureError( - cause, + recorded?.unreleased === undefined + ? cause + : new DesktopSessionCleanupError(cause, recorded.unreleased), await discardFailedCapture(store, captureFile), ); } - const { evidence, published, session, unreleased } = recorded; + const { evidence, session, unreleased } = recorded; const cleanup = await publishedCaptureCleanup(published, store, unreleased); if (cleanup !== undefined) throw cleanup; // A cancellation that arrives during publication must not read as success. diff --git a/src/application/desktop/DesktopApplicationExport.test.ts b/src/application/desktop/DesktopApplicationExport.test.ts index 69153edb09..c5af4edb50 100644 --- a/src/application/desktop/DesktopApplicationExport.test.ts +++ b/src/application/desktop/DesktopApplicationExport.test.ts @@ -9,6 +9,11 @@ import { z } from "zod"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; import { SafeOutputTree } from "../../artifacts/SafeOutputTree.js"; +import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; +import type { AnalysisCleanupLease } from "../../artifacts/AnalysisCleanupLease.js"; +import { AnalysisError } from "../../domain/analysisErrorBase.js"; +import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js"; +import { RetainedContent } from "./DesktopRetainedContent.js"; import { VARICAD_FILE, VARICAD_URL, @@ -2217,6 +2222,114 @@ describe("exportDesktopApplication launch document beside its lock", () => { }); }); +describe("exportDesktopApplication retained input finalization", () => { + it("honors cancellation during retained input finalization before publication", async () => { + const controller = new AbortController(); + const reason = new Error("cancel during seed finalization"); + let writing = false; + let finalized = false; + const write = unspied("write"); + vi.spyOn(SafeOutputTree.prototype, "write").mockImplementation( + async function (this: SafeOutputTree, ...args) { + const written = await write.apply(this, args); + writing = true; + return written; + }, + ); + const release = RetainedContent.prototype.release; + vi.spyOn(RetainedContent.prototype, "release").mockImplementation( + async function (this: RetainedContent, resources) { + const result = await release.call(this, resources); + if (writing) { + finalized = true; + controller.abort(reason); + } + return result; + }, + ); + const { result, directory } = await exportCapture( + capture(), + controller.signal, + ); + expect(finalized).toBe(true); + expect(result).toMatchObject({ _tag: "AnalysisCancelledError" }); + expect((result as Error).cause).toBe(reason); + expect(await exists(directory)).toBe(false); + }); + + it.each([false, true])( + "finalizes retained seed inputs before publication and retains cleanup retry (rollback fails: %s)", + async (rollbackFails) => { + let writing = false; + let reading = false; + const blocked = new Set(); + const leases: AnalysisCleanupLease[] = []; + if (rollbackFails) + vi.spyOn( + SafeOutputTree.prototype, + "closeHandles", + ).mockRejectedValueOnce(new Error("output close failed")); + const write = unspied("write"); + vi.spyOn(SafeOutputTree.prototype, "write").mockImplementation( + async function (this: SafeOutputTree, ...args) { + const written = await write.apply(this, args); + writing = true; + return written; + }, + ); + const read = RetainedContent.prototype.read; + vi.spyOn(RetainedContent.prototype, "read").mockImplementation( + async function (this: RetainedContent, ...args) { + reading = writing; + try { + return await read.apply(this, args); + } finally { + reading = false; + } + }, + ); + const close = OwnedFileHandle.prototype.close; + const closing = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(function (this: OwnedFileHandle) { + if (reading && blocked.size === 0) blocked.add(this); + return blocked.has(this) + ? Promise.reject(new Error("retained seed close failed")) + : close.call(this); + }); + const release = ArtifactResourceScope.prototype.release; + vi.spyOn(ArtifactResourceScope.prototype, "release").mockImplementation( + async function (this: ArtifactResourceScope, owner) { + const result = await release.call(this, owner); + if (result.kind === "failed") leases.push(result.lease); + return result; + }, + ); + try { + const { result, directory } = await exportCapture(capture()); + expect(blocked.size).toBe(1); + expect(result).toBeInstanceOf(AnalysisError); + if (!(result instanceof AnalysisError)) + throw new Error("expected cleanup failure"); + expect(result.cleanupIncomplete).toBe(true); + expect(result.cleanupResources).toHaveLength(rollbackFails ? 2 : 1); + if (rollbackFails) expect(result.cleanupResources).toContain(directory); + expect(await exists(directory)).toBe(rollbackFails); + expect(leases).toHaveLength(rollbackFails ? 2 : 1); + closing.mockRestore(); + await leases[0]!.release(); + expect(result.cleanupIncomplete).toBe(rollbackFails); + for (const lease of leases.slice(1)) await lease.release(); + expect(result.cleanupIncomplete).toBe(false); + expect(await exists(directory)).toBe(false); + } finally { + closing.mockRestore(); + for (const lease of leases) await lease.release(); + } + }, + ); +}); + describe("exportDesktopApplication bundle writes", () => { it.each(["cancelled", "out of space"])( "removes the unpublished bundle when its write fails (%s)", diff --git a/src/application/desktop/DesktopApplicationExport.ts b/src/application/desktop/DesktopApplicationExport.ts index e1c5f5430b..9b0cf690ee 100644 --- a/src/application/desktop/DesktopApplicationExport.ts +++ b/src/application/desktop/DesktopApplicationExport.ts @@ -403,19 +403,19 @@ const runExport = async ( const text = bundleText(plan, admitted, dconf.holdsIdentity, held.consent); const { generated, readme } = text; let scrub = NO_SCRUB; - const unclosed = await withRetainedContent( - request.capture, - (content) => - writeOutputTree( - request.directory, - async (tree) => { + const unclosed = await writeOutputTree( + copy.bundle, + (tree) => + withRetainedContent( + request.capture, + async (content) => { const bundle = { generated, copies, identity, readme }; scrub = await writeFiles(tree, content, bundle, signal); }, - { operation: OPERATION, field: "directory" }, - signal, + SEED_MAX_BYTES, ), - SEED_MAX_BYTES, + { operation: OPERATION, field: "directory" }, + signal, ); const limitations = exportLimitations(capture, plan, { install: install.limitations, diff --git a/src/application/desktop/DesktopApplicationState.test.ts b/src/application/desktop/DesktopApplicationState.test.ts index 2be605bd5e..6469b8994a 100644 --- a/src/application/desktop/DesktopApplicationState.test.ts +++ b/src/application/desktop/DesktopApplicationState.test.ts @@ -1,10 +1,17 @@ -import { readFile } from "node:fs/promises"; +import { open, readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { DatabaseSync } from "node:sqlite"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; +import { + AnalysisError, + primaryAnalysisError, +} from "../../domain/analysisErrorBase.js"; +import { EvidenceFileError } from "../../domain/evidenceErrors.js"; +import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; +import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js"; import type { DesktopApplicationCapture, DesktopCheckpoint, @@ -12,6 +19,7 @@ import type { import { desktopApplicationStateSchema } from "../../domain/desktop/desktopState.js"; import { inspectDesktopApplicationState, + readDesktopCapture, type DesktopStateReaders, } from "./DesktopApplicationState.js"; import { draftDesktopApplicationTask } from "./DesktopTaskDraft.js"; @@ -27,6 +35,49 @@ import { writeCaptureFolder, } from "./desktopCapture.fixture.js"; +it("retains invalid capture JSON and the descriptor owner through failed final cleanup", async () => { + const root = await createTestTempDirectory("rea-capture-read-cleanup-"); + await writeFile(join(root, "capture.json"), '{"broken":'); + await expect(readDesktopCapture(root, undefined)).rejects.toMatchObject({ + _tag: "EvidenceFileError", + reason: "invalid-json", + }); + const scopes = new Set(); + const originalClose = ArtifactResourceScope.prototype.close; + const scopeClose = vi + .spyOn(ArtifactResourceScope.prototype, "close") + .mockImplementation(function (this: ArtifactResourceScope) { + scopes.add(this); + return originalClose.call(this); + }); + const fileClose = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockRejectedValue(new Error("descriptor close remains unavailable")); + let failure: AnalysisError | undefined; + try { + const reported = await readDesktopCapture(root, undefined).catch( + (cause: unknown) => cause, + ); + expect(reported).toBeInstanceOf(AnalysisError); + if (!(reported instanceof AnalysisError)) throw reported; + failure = reported; + expect(primaryAnalysisError(failure)).toBeInstanceOf(EvidenceFileError); + expect(primaryAnalysisError(failure)).toMatchObject({ + reason: "invalid-json", + }); + expect(failure.cleanupIncomplete).toBe(true); + expect(failure.cleanupResources).toEqual([join(root, "capture.json")]); + } finally { + fileClose.mockRestore(); + scopeClose.mockRestore(); + for (const scope of scopes) await scope.close(); + } + expect(failure?.cleanupIncomplete).toBe(false); + expect(primaryAnalysisError(failure!)).toMatchObject({ + reason: "invalid-json", + }); +}); + /** The fixture capture with extra files at the baseline and the second checkpoint. */ const extended = (extra: { readonly baseline?: DesktopCheckpoint["retained"]; @@ -1080,3 +1131,154 @@ describe("inspectDesktopApplicationState view bounds", () => { expect(names).toEqual(expect.arrayContaining(["notes", "noteswindow"])); }); }); + +it("preserves failure and late cancellation when capture input cleanup finishes", async () => { + const root = await createTestTempDirectory("rea-capture-late-cancel-"); + await writeFile(join(root, "capture.json"), '{"broken":'); + const controller = new AbortController(); + const reason = { cause: "during close" }; + const close = ArtifactResourceScope.prototype.close; + const spy = vi + .spyOn(ArtifactResourceScope.prototype, "close") + .mockImplementation(async function (this: ArtifactResourceScope) { + await close.call(this); + controller.abort(reason); + }); + try { + const failure: unknown = await readDesktopCapture( + root, + controller.signal, + ).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(AnalysisError); + if (!(failure instanceof AnalysisError)) throw failure; + expect(failure).toMatchObject({ + _tag: "EvidenceFileError", + reason: "invalid-json", + }); + expect(failure.secondaryFailures).toContainEqual( + expect.objectContaining({ + _tag: "AnalysisCancelledError", + cause: reason, + }), + ); + } finally { + spy.mockRestore(); + } +}); + +it.each([false, true])( + "retains completed capture and cancellation after cleanup (close fails: %s)", + async (closeFails) => { + const folder = await writeCaptureFolder(capture()); + const controller = new AbortController(); + const reason = { cause: "during final close" }; + const scopes = new Set(); + const originalClose = ArtifactResourceScope.prototype.close; + const extra = closeFails + ? new OwnedFileHandle(await open(join(folder, "capture.json"), "r")) + : undefined; + const originalFileClose = OwnedFileHandle.prototype.close; + const fileClose = closeFails + ? vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(function (this: OwnedFileHandle) { + return this === extra + ? Promise.reject(new Error("descriptor close failed")) + : originalFileClose.call(this); + }) + : undefined; + const scopeClose = vi + .spyOn(ArtifactResourceScope.prototype, "close") + .mockImplementation(async function (this: ArtifactResourceScope) { + scopes.add(this); + if (extra !== undefined) + this.retain({ + kind: "file-handle", + resource: "final descriptor", + handle: extra, + }); + try { + await originalClose.call(this); + } finally { + controller.abort(reason); + } + }); + let failure: AnalysisError | undefined; + try { + const result: unknown = await readDesktopCapture( + folder, + controller.signal, + ).catch((cause: unknown) => cause); + expect(result).toBeInstanceOf(AnalysisError); + if (!(result instanceof AnalysisError)) throw result; + failure = result; + expect(primaryAnalysisError(result)).toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + }); + expect(result.partialObservation).toMatchObject({ + operation: "capture_desktop_application", + }); + expect(result.cleanupIncomplete).toBe(closeFails); + if (extra !== undefined) { + expect(result.cleanupResources).toEqual(["final descriptor"]); + expect(extra.handle.fd).toBeGreaterThanOrEqual(0); + } + } finally { + fileClose?.mockRestore(); + scopeClose.mockRestore(); + for (const scope of scopes) await scope.close(); + } + expect(failure?.cleanupIncomplete).toBe(false); + if (extra !== undefined) expect(extra.handle.fd).toBe(-1); + }, +); + +it("retains a failed input, cancellation and failed descriptor cleanup together", async () => { + const folder = await createTestTempDirectory("rea-capture-mixed-final-"); + await writeFile(join(folder, "capture.json"), '{"broken":'); + const controller = new AbortController(); + const reason = { cause: "during failed close" }; + const scopes = new Set(); + const originalClose = ArtifactResourceScope.prototype.close; + const scopeClose = vi + .spyOn(ArtifactResourceScope.prototype, "close") + .mockImplementation(async function (this: ArtifactResourceScope) { + scopes.add(this); + try { + await originalClose.call(this); + } finally { + controller.abort(reason); + } + }); + const fileClose = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockRejectedValue(new Error("descriptor close failed")); + let failure: AnalysisError | undefined; + try { + const result: unknown = await readDesktopCapture( + folder, + controller.signal, + ).catch((cause: unknown) => cause); + expect(result).toBeInstanceOf(AnalysisError); + if (!(result instanceof AnalysisError)) throw result; + failure = result; + const primary = primaryAnalysisError(result); + expect(primary).toMatchObject({ + _tag: "EvidenceFileError", + reason: "invalid-json", + }); + expect(primary.secondaryFailures).toContainEqual( + expect.objectContaining({ + _tag: "AnalysisCancelledError", + cause: reason, + }), + ); + expect(result.cleanupIncomplete).toBe(true); + } finally { + fileClose.mockRestore(); + scopeClose.mockRestore(); + for (const scope of scopes) await scope.close(); + } + expect(failure?.cleanupIncomplete).toBe(false); +}); diff --git a/src/application/desktop/DesktopApplicationState.ts b/src/application/desktop/DesktopApplicationState.ts index 0b1dcf517d..574d983e26 100644 --- a/src/application/desktop/DesktopApplicationState.ts +++ b/src/application/desktop/DesktopApplicationState.ts @@ -9,7 +9,12 @@ import { join } from "node:path"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; -import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; +import { + AnalysisCancelledError, + AnalysisInputError, +} from "../../domain/analysisErrorCore.js"; +import { analysisErrorWithCancellation } from "../../domain/analysisErrorCancellation.js"; +import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js"; import { analysisInputErrorFromIssues } from "../../domain/inputIssueProjection.js"; import { desktopApplicationCaptureSchema, @@ -92,44 +97,93 @@ export const readDesktopCapture = async ( signal: AbortSignal | undefined, ): Promise<{ evidence: Evidence; capture: DesktopApplicationCapture }> => { const resources = new ArtifactResourceScope(); + const outcome = await resources + .run(async () => { + const loaded = await readJsonFile( + join(folder, "capture.json"), + signal, + resources, + OPERATION, + ); + if (!loaded.ok) { + if (loaded.error instanceof AnalysisError) throw loaded.error; + throw inputError( + `${join(folder, "capture.json")} could not be read as JSON.`, + "Pass a folder written by capture-desktop-application.", + ); + } + const evidence = safeParseEvidence(loaded.value); + if ( + !evidence.ok || + evidence.value.operation !== DESKTOP_OPERATION || + evidence.value.predicate_type !== "rea.desktop-application-capture" || + evidence.value.provider.id !== DESKTOP_PROVIDER.id || + evidence.value.provider.version !== DESKTOP_PROVIDER.version + ) + throw inputError( + "capture.json is not desktop application capture Evidence.", + "Pass a folder written by capture-desktop-application.", + ); + const capture = desktopApplicationCaptureSchema.safeParse( + evidence.value.normalized_result, + ); + if (!capture.success) + throw inputError( + `The capture result is malformed (${describeValidationFailure(capture.error)}).`, + "Capture the application again.", + ); + return { evidence: evidence.value, capture: capture.data }; + }) + .then(ok, (cause: unknown) => err(cause)); try { - const loaded = await readJsonFile( - join(folder, "capture.json"), - signal, - resources, - OPERATION, - ); - if (!loaded.ok) { - if (loaded.error instanceof AnalysisError) throw loaded.error; - throw inputError( - `${join(folder, "capture.json")} could not be read as JSON.`, - "Pass a folder written by capture-desktop-application.", - ); - } - const evidence = safeParseEvidence(loaded.value); - if ( - !evidence.ok || - evidence.value.operation !== DESKTOP_OPERATION || - evidence.value.predicate_type !== "rea.desktop-application-capture" || - evidence.value.provider.id !== DESKTOP_PROVIDER.id || - evidence.value.provider.version !== DESKTOP_PROVIDER.version - ) - throw inputError( - "capture.json is not desktop application capture Evidence.", - "Pass a folder written by capture-desktop-application.", - ); - const capture = desktopApplicationCaptureSchema.safeParse( - evidence.value.normalized_result, - ); - if (!capture.success) - throw inputError( - `The capture result is malformed (${describeValidationFailure(capture.error)}).`, - "Capture the application again.", - ); - return { evidence: evidence.value, capture: capture.data }; - } finally { await resources.close(); + } catch (cause: unknown) { + const cleanup = desktopOperationFailure( + cause, + signal, + OPERATION, + "Desktop capture input", + ); + if (outcome.ok && !signal?.aborted) + throw cleanup.retainPartialObservation(outcome.value.evidence); + const primary = outcome.ok + ? new AnalysisCancelledError(OPERATION, { + cause: signal?.reason, + }).retainPartialObservation(outcome.value.evidence) + : desktopOperationFailure( + outcome.error, + undefined, + OPERATION, + "Desktop capture input", + ); + if (!outcome.ok && signal?.aborted) + analysisErrorWithCancellation( + primary, + new AnalysisCancelledError(OPERATION, { cause: signal.reason }), + ); + throw analysisErrorWithCleanupFailure( + primary, + cleanup, + OPERATION, + ).bindCleanupDependencies([primary, cleanup]); } + if (!outcome.ok) { + if (!signal?.aborted) throw outcome.error; + throw analysisErrorWithCancellation( + desktopOperationFailure( + outcome.error, + undefined, + OPERATION, + "Desktop capture input", + ), + new AnalysisCancelledError(OPERATION, { cause: signal.reason }), + ); + } + if (signal?.aborted) + throw new AnalysisCancelledError(OPERATION, { + cause: signal.reason, + }).retainPartialObservation(outcome.value.evidence); + return outcome.value; }; interface ViewContext { diff --git a/src/application/desktop/DesktopBuildExport.ts b/src/application/desktop/DesktopBuildExport.ts index 67c9588298..c70c7b929c 100644 --- a/src/application/desktop/DesktopBuildExport.ts +++ b/src/application/desktop/DesktopBuildExport.ts @@ -46,6 +46,7 @@ import { WORK, } from "./DesktopBuildCommands.js"; import { + planOutputTree, writeOutputTree, writtenWithOpenHandles, } from "./DesktopOutputTree.js"; @@ -183,7 +184,7 @@ const writeBundle = async ( unchecked: boolean; }[] = []; const unclosed = await writeOutputTree( - directory, + await planOutputTree(directory, { operation: EXPORT, field: "directory" }), async (tree) => { fetched = await fetchHomeFolder( computer, diff --git a/src/application/desktop/DesktopBuildWitnessRecord.ts b/src/application/desktop/DesktopBuildWitnessRecord.ts index c9dd67ae35..612cf6dea1 100644 --- a/src/application/desktop/DesktopBuildWitnessRecord.ts +++ b/src/application/desktop/DesktopBuildWitnessRecord.ts @@ -28,6 +28,7 @@ import { import { errorText } from "./DesktopBuildCommands.js"; import { DESKTOP_PROVIDER } from "./DesktopCaptureRefusals.js"; import { + planOutputTree, writeOutputTree, writtenWithOpenHandles, } from "./DesktopOutputTree.js"; @@ -153,7 +154,7 @@ const writeRecordFolder = async ( > => { try { const unclosed = await writeOutputTree( - directory, + await planOutputTree(directory, { operation: CHECK, field: "directory" }), async (tree) => { for (const file of files) await tree.write( diff --git a/src/application/desktop/DesktopCaptureFailure.test.ts b/src/application/desktop/DesktopCaptureFailure.test.ts index 4a9aeec08a..f1f4efd8b0 100644 --- a/src/application/desktop/DesktopCaptureFailure.test.ts +++ b/src/application/desktop/DesktopCaptureFailure.test.ts @@ -13,6 +13,7 @@ import { DesktopSessionCleanupError } from "../../desktop/DesktopApplicationSess import { DesktopContentStore } from "../../desktop/DesktopContentStore.js"; import { walkHomeState } from "../../desktop/HomeStateWalker.js"; import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js"; +import { captureDesktopApplication } from "./DesktopApplicationCapture.js"; import { CaptureFile } from "./DesktopCaptureOutput.js"; import { discardFailedCapture, @@ -236,6 +237,77 @@ describe("failedCaptureError", () => { }); describe("projectFailure", () => { + it.each([ + { oversized: false, category: "cancelled" }, + { oversized: true, category: "execution_failure" }, + ])( + "preserves the physical read outcome beside final cancellation and cleanup (oversized: $oversized)", + async ({ oversized, category }) => { + const root = await createTestTempDirectory("desktop-inventory-cancel-"); + const entry = join(root, "notes.desktop"); + await writeFile( + entry, + oversized + ? "x".repeat(1024 * 1024 + 1) + : "[Desktop Entry]\nType=Application\nName=Notes\nExec=notes\n", + ); + const controller = new AbortController(); + const reason = new Error("cancelled while closing the desktop entry"); + let owner: OwnedFileHandle | undefined; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(function (this: OwnedFileHandle) { + // oxlint-disable-next-line typescript/no-this-alias -- Release the exact denied descriptor after the public call. + owner = this; + controller.abort(reason); + return Promise.reject(new Error("close denied")); + }); + try { + const result = await captureDesktopApplication( + { + desktopEntry: entry, + executable: undefined, + arguments: [], + output: join(root, "capture"), + statePaths: [], + excludes: [], + maxDurationSeconds: 1, + maxEntries: 10, + maxFileBytes: 1024, + maxContentBytes: 1024, + packageFile: undefined, + }, + { + platform: "linux", + callerDirectory: root, + environment: { + HOME: root, + DISPLAY: ":controlled", + REA_XPROP_COMMAND: process.execPath, + }, + nextRequest: () => undefined, + report: async () => { + throw new Error( + "inventory failure must precede target execution", + ); + }, + }, + controller.signal, + ); + expect(owner).toBeDefined(); + expect(result).toMatchObject({ + error: "Desktop capture failed", + code: "cleanup_incomplete", + category, + details: { resources: [entry] }, + }); + } finally { + close.mockRestore(); + await owner?.close(); + } + }, + ); + it("keeps the owner of a resource the inventory could not release", async () => { const resources = new ArtifactResourceScope(); let attempts = 0; diff --git a/src/application/desktop/DesktopCaptureOutput.test.ts b/src/application/desktop/DesktopCaptureOutput.test.ts index f402cae929..6c6b8c483f 100644 --- a/src/application/desktop/DesktopCaptureOutput.test.ts +++ b/src/application/desktop/DesktopCaptureOutput.test.ts @@ -14,10 +14,23 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; import type { Evidence } from "../../domain/evidence.js"; import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js"; -import { CaptureFile } from "./DesktopCaptureOutput.js"; +import { CaptureFile, prepareOutputDirectory } from "./DesktopCaptureOutput.js"; const evidence = { id: "evidence" } as unknown as Evidence; +it.each(["", "/child"])( + "refuses an output path blocked by a regular file (%s)", + async (suffix) => { + const root = await createTestTempDirectory("desktop-output-file-"); + const file = join(root, "existing"); + await writeFile(file, "keep"); + await expect(prepareOutputDirectory(`${file}${suffix}`)).resolves.toEqual({ + refusal: "not_a_directory", + }); + expect(await readFile(file, "utf8")).toBe("keep"); + }, +); + describe("CaptureFile", () => { it("publishes through its partial file and leaves nothing to discard", async () => { const output = await createTestTempDirectory("desktop-output-"); diff --git a/src/application/desktop/DesktopCaptureOutput.ts b/src/application/desktop/DesktopCaptureOutput.ts index fcf7da8cb2..bf04913e8a 100644 --- a/src/application/desktop/DesktopCaptureOutput.ts +++ b/src/application/desktop/DesktopCaptureOutput.ts @@ -8,31 +8,12 @@ import { rename, rm, } from "node:fs/promises"; -import { basename, dirname, join, resolve } from "node:path"; +import { join } from "node:path"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import type { Evidence } from "../../domain/evidence.js"; import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js"; -/** - * The physical spelling of a path that may not exist yet: the real path of - * its longest existing ancestor followed by the rest. Walks, exclusions and - * state directories compare in this spelling, so a symbolic link in the - * caller's spelling cannot make one directory look like two. - */ -export const canonicalPath = async (path: string): Promise => { - const absolute = resolve(path); - const rest: string[] = []; - for (let current = absolute; ; current = dirname(current)) { - try { - return join(await realpath(current), ...rest.reverse()); - } catch { - if (dirname(current) === current) return absolute; - rest.push(basename(current)); - } - } -}; - /** Why the output folder cannot hold a private capture. */ export type OutputRefusal = "not_a_directory" | "foreign_owner" | "not_empty"; @@ -43,7 +24,14 @@ export type OutputRefusal = "not_a_directory" | "foreign_owner" | "not_empty"; export const prepareOutputDirectory = async ( output: string, ): Promise<{ readonly path: string } | { readonly refusal: OutputRefusal }> => { - await mkdir(output, { recursive: true, mode: 0o700 }); + try { + await mkdir(output, { recursive: true, mode: 0o700 }); + } catch (cause: unknown) { + const code = (cause as NodeJS.ErrnoException).code; + if (code === "EEXIST" || code === "ENOTDIR") + return { refusal: "not_a_directory" }; + throw cause; + } const stats = await lstat(output); if (!stats.isDirectory()) return { refusal: "not_a_directory" }; const uid = process.getuid?.(); diff --git a/src/application/desktop/DesktopCaptureRefusals.ts b/src/application/desktop/DesktopCaptureRefusals.ts index 811bbe6cdd..6c2085baf5 100644 --- a/src/application/desktop/DesktopCaptureRefusals.ts +++ b/src/application/desktop/DesktopCaptureRefusals.ts @@ -2,7 +2,10 @@ import { readdir } from "node:fs/promises"; import { isAbsolute } from "node:path"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; -import { bindArtifactFileCleanup } from "../../artifacts/ArtifactFileOperation.js"; +import { + artifactFileCancellationCause, + bindArtifactFileCleanup, +} from "../../artifacts/ArtifactFileOperation.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { AnalysisCancelledError, @@ -193,6 +196,8 @@ export const checkDesktopCaptureHost = async ( ); const existing = await readdir(request.output).catch((cause: unknown) => { if ((cause as NodeJS.ErrnoException).code === "ENOENT") return []; + if ((cause as NodeJS.ErrnoException).code === "ENOTDIR") + throw outputRefusal(request.output, "not_a_directory"); throw cause; }); if (existing.length > 0) throw outputRefusal(request.output, "not_empty"); @@ -232,8 +237,15 @@ export const desktopOperationFailure = ( } // A resource that could not be released stays reported, with its owner. if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) { + const primary = + cause.reason === "cancelled" + ? new AnalysisCancelledError(operation, { + cause: artifactFileCancellationCause(cause), + }) + : undefined; const error = new ProviderAdapterError(DESKTOP_PROVIDER.id, operation, { cause, + ...(primary === undefined ? {} : { cleanupPrimary: primary }), cleanup: cause.cleanup, diagnostics: { phase: operation }, userMessage: `${label} could not release a resource it used: ${describeValidationFailure(cause)}`, diff --git a/src/application/desktop/DesktopCaptureScope.test.ts b/src/application/desktop/DesktopCaptureScope.test.ts index 9f71ab792a..76767aba19 100644 --- a/src/application/desktop/DesktopCaptureScope.test.ts +++ b/src/application/desktop/DesktopCaptureScope.test.ts @@ -141,6 +141,68 @@ describe("captureScope", () => { ); expect(scope.contentRoots).not.toContain(linked); }); + + it("follows a selected symlink before its parent segment for capture and exclusion", async () => { + const root = await realpath( + await createTestTempDirectory("desktop-scope-parent-"), + ); + const home = join(root, "home"); + await mkdir(home); + await mkdir(join(root, "physical", "child"), { recursive: true }); + await mkdir(join(root, "named")); + await symlink(join(root, "physical", "child"), join(root, "named", "link")); + await writeFile(join(root, "physical", "selected"), "selected"); + await writeFile(join(root, "named", "selected"), "decoy"); + const selected = `${root}/named/link/../selected`; + const { scope, selectedPaths } = await captureScope( + { ...request([]), statePaths: [selected], excludes: [selected] }, + inventory(), + home, + join(root, "out"), + ); + expect(selectedPaths).toEqual([ + { + path: selected, + physical: join(root, "physical", "selected"), + kind: "file", + }, + ]); + expect(scope.contentRoots).toContain(join(root, "physical", "selected")); + expect(scope.excluded).toContain(join(root, "physical", "selected")); + expect(scope.excluded).not.toContain(join(root, "named", "selected")); + }); +}); + +it("keeps discovered state directories whose physical path cannot be established", async () => { + const root = await realpath( + await createTestTempDirectory("desktop-scope-unknown-"), + ); + const home = join(root, "home"); + const dangling = join(home, ".config/notes-link"); + const file = join(root, "outside/not-a-directory"); + const throughFile = join(file, "child"); + await mkdir(dirname(dangling), { recursive: true }); + await symlink(join(home, "missing-notes"), dangling); + await mkdir(dirname(file), { recursive: true }); + await writeFile(file, "not a directory"); + const { scope } = await captureScope( + request([]), + inventory({ + state_directories: [ + { path: dangling, exists_at_start: true }, + { + path: throughFile, + exists_at_start: null, + existence_error: "ENOTDIR", + }, + ], + }), + home, + join(root, "out"), + ); + expect(scope.stateDirectories).toEqual([dangling, throughFile]); + expect(scope.contentRoots).toEqual([dangling, throughFile]); + expect(scope.roots).toEqual([home, throughFile]); }); describe("applicationWindowMatching", () => { diff --git a/src/application/desktop/DesktopCaptureScope.ts b/src/application/desktop/DesktopCaptureScope.ts index e06b22f65a..11ce301688 100644 --- a/src/application/desktop/DesktopCaptureScope.ts +++ b/src/application/desktop/DesktopCaptureScope.ts @@ -1,20 +1,50 @@ import { lstat } from "node:fs/promises"; -import { basename, isAbsolute, resolve } from "node:path"; +import { basename, isAbsolute } from "node:path"; import type { DesktopApplicationInventory } from "../../domain/desktop/desktopCapture.js"; import { isWithinAny } from "../../domain/desktop/desktopStateDiff.js"; import { SQLITE_SIDECARS } from "../../domain/desktop/stateFormats.js"; import type { WindowMatching } from "../../desktop/DesktopSessionObservations.js"; -import { canonicalPath } from "./DesktopCaptureOutput.js"; +import { absoluteCallerPath } from "../../filesystem/callerPath.js"; +import { canonicalCallerPath } from "../../filesystem/canonicalCallerPath.js"; import type { CheckpointScope } from "./DesktopCheckpointRecorder.js"; const unique = (paths: readonly string[]) => [...new Set(paths)]; +/** A discovered state path whose physical file cannot be established. */ +const unresolvedStateDirectory = (cause: unknown): boolean => { + const code = (cause as NodeJS.ErrnoException).code; + return ( + code === "ENOENT" || + code === "EACCES" || + code === "EPERM" || + code === "ENOTDIR" || + code === "ELOOP" + ); +}; + +/** + * Canonicalize a discovered state directory when the filesystem can. An + * unreadable path or dangling symlink stays the discovered absolute path so + * the capture can publish that unknown instead of aborting. + */ +const canonicalStateDirectory = async (path: string): Promise => { + try { + return await canonicalCallerPath(path); + } catch (cause: unknown) { + if (!unresolvedStateDirectory(cause)) throw cause; + return absoluteCallerPath(path); + } +}; + /** Whether the walked path is a directory. A missing path is not one, matching the sidecar decision. */ const pathKind = async (path: string): Promise<"file" | "directory"> => (await lstat(path).then( (stats) => stats.isDirectory(), - () => false, + (cause: unknown) => { + if ((cause as NodeJS.ErrnoException).code === "ENOENT") return false; + throw cause; + }, )) ? "directory" : "file"; @@ -55,16 +85,18 @@ export const captureScope = async ( output: string, ): Promise => { const canonical = (paths: readonly string[]) => - Promise.all(paths.map((path) => canonicalPath(path))); + Promise.all(paths.map((path) => canonicalCallerPath(path))); const named = (paths: readonly string[]) => Promise.all( paths.map(async (path) => ({ - path: resolve(path), - physical: await canonicalPath(path), + path: absoluteCallerPath(path), + physical: await canonicalCallerPath(path), })), ); - const stateDirectories = await canonical( - inventory.state_directories.map((entry) => entry.path), + const stateDirectories = await Promise.all( + inventory.state_directories.map((entry) => + canonicalStateDirectory(entry.path), + ), ); const argumentPaths = await named( request.arguments.filter((value) => isAbsolute(value)), diff --git a/src/application/desktop/DesktopExportWithheld.test.ts b/src/application/desktop/DesktopExportWithheld.test.ts index 9db3988dd4..4c7ef44311 100644 --- a/src/application/desktop/DesktopExportWithheld.test.ts +++ b/src/application/desktop/DesktopExportWithheld.test.ts @@ -164,7 +164,7 @@ const evidence = (result: Evidence | Error): Evidence => { return result; }; -it("reports the published bundle when the withheld copy destination is occupied", async () => { +it("refuses an occupied withheld copy destination before writing the bundle", async () => { const root = await createTestTempDirectory("desktop-copy-refusal-"); const occupied = join(root, "occupied"); await mkdir(occupied); @@ -173,21 +173,31 @@ it("reports the published bundle when the withheld copy destination is occupied" withhold: { paths: [SETTINGS] }, withheldCopy: occupied, }); - expect(result).toMatchObject({ - partialObservation: { - operation: "export_desktop_application", - normalized_result: { directory }, - }, - }); - expect(await exists(join(directory, "README.md"))).toBe(true); + expect(result).toBeInstanceOf(AnalysisError); + expect(await exists(directory)).toBe(false); expect(await readFile(join(occupied, "keep"), "utf8")).toBe("existing"); + const occupiedFile = join(root, "file"); + await writeFile(occupiedFile, "existing file"); + const file = await exportWithheld({ + withhold: { paths: [SETTINGS] }, + withheldCopy: occupiedFile, + }); + expect(file.result).toBeInstanceOf(AnalysisError); + expect(await exists(file.directory)).toBe(false); + expect(await readFile(occupiedFile, "utf8")).toBe("existing file"); }); it("retains the bundle cleanup owner when a later withheld copy fails", async () => { const root = await createTestTempDirectory("desktop-copy-close-"); const occupied = join(root, "occupied"); await mkdir(occupied); - await writeFile(join(occupied, "keep"), "existing"); + const commit = SafeOutputTree.prototype.commit; + vi.spyOn(SafeOutputTree.prototype, "commit").mockImplementationOnce( + async function (this: SafeOutputTree, signal) { + await commit.call(this, signal); + await writeFile(join(occupied, "keep"), "existing"); + }, + ); const original = SafeOutputTree.prototype.closeHandles; const release = ArtifactResourceScope.prototype.release; const failures: Extract[] = []; @@ -562,7 +572,9 @@ describe("exportDesktopApplication of withheld paths and copies", () => { ).toContain("49152"); expect(await exists(directory)).toBe(false); }); +}); +describe("exportDesktopApplication withheld copy destinations", () => { it("refuses a withheld copy inside the bundle, also through a link, and writes neither", async () => { const root = await createTestTempDirectory("desktop-withheld-inside-"); const directory = join(root, "bundle"); @@ -582,6 +594,38 @@ describe("exportDesktopApplication of withheld paths and copies", () => { }); expect(linked.result).toBeInstanceOf(Error); expect(await readdir(directory)).toEqual([]); + await mkdir(join(root, "outside")); + await symlink(directory, join(root, "outside", "link")); + const parentTraversal = await exportWithheld({ + withhold: { paths: [SETTINGS] }, + directory, + withheldCopy: `${root}/outside/link/../bundle/private`, + }); + expect(parentTraversal.result).toBeInstanceOf(Error); + expect(await readdir(directory)).toEqual([]); + const unresolved = await exportWithheld({ + withhold: { paths: [SETTINGS] }, + directory: join(root, "new-bundle"), + withheldCopy: `${root}/missing/../private`, + }); + expect(unresolved.result).toBeInstanceOf(Error); + expect(await exists(join(root, "missing"))).toBe(false); + expect(await exists(join(root, "new-bundle"))).toBe(false); + await symlink(join(root, "absent"), join(root, "dangling")); + const dangling = await exportWithheld({ + withhold: { paths: [SETTINGS] }, + directory: join(root, "new-bundle"), + withheldCopy: join(root, "dangling", "private"), + }); + expect(dangling.result).toBeInstanceOf(AnalysisError); + expect(await exists(join(root, "new-bundle"))).toBe(false); + const leafLink = await exportWithheld({ + withhold: { paths: [SETTINGS] }, + directory: `${root}/alias//`, + withheldCopy: join(root, "outside-copy"), + }); + expect(leafLink.result).toBeInstanceOf(Error); + expect(await readdir(directory)).toEqual([]); }); it("reports a written copy whose handles cannot be closed, with the export's Evidence", async () => { diff --git a/src/application/desktop/DesktopExportWithheld.ts b/src/application/desktop/DesktopExportWithheld.ts index 55ca7eb83e..554a71c96d 100644 --- a/src/application/desktop/DesktopExportWithheld.ts +++ b/src/application/desktop/DesktopExportWithheld.ts @@ -1,9 +1,9 @@ import { createHash } from "node:crypto"; -import { readFile, realpath } from "node:fs/promises"; -import { basename, dirname, join, resolve, sep } from "node:path"; +import { readFile } from "node:fs/promises"; import { Readable } from "node:stream"; import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; +import { isPathWithinRoot } from "../../domain/localPath.js"; import { consentFileSchema, MAX_WITHHELD_BYTES, @@ -24,7 +24,12 @@ import { import { isWithinAny } from "../../domain/desktop/desktopStateDiff.js"; import { safeParseUtf8Json } from "../../domain/safeJson.js"; import type { SeedVersion } from "./DesktopExportFiles.js"; -import { writeOutputTree, type FailedRelease } from "./DesktopOutputTree.js"; +import { + planOutputTree, + writeOutputTree, + type DesktopOutputPlan, + type FailedRelease, +} from "./DesktopOutputTree.js"; import { withRetainedContent, type RetainedContent, @@ -262,7 +267,7 @@ const readWithheld = async ( /** Write the withheld files, by home path, into a new or empty folder outside the bundle; a failed release of a written copy is returned. */ const writeWithheldCopy = ( - directory: string, + directory: DesktopOutputPlan, files: ReadonlyMap, signal: AbortSignal | undefined, ): Promise => @@ -284,32 +289,18 @@ const writeWithheldCopy = ( signal, ); -/** A path with every link in its existing part resolved, so two spellings of one folder compare equal. */ -const resolvedPath = async (path: string): Promise => { - const rest: string[] = []; - for (let current = resolve(path); ; current = dirname(current)) { - try { - return join(await realpath(current), ...rest); - } catch { - if (dirname(current) === current) return resolve(path); - rest.unshift(basename(current)); - } - } -}; - -const isWithin = (path: string, root: string) => - path === root || path.startsWith(root.endsWith(sep) ? root : `${root}${sep}`); - /** Refuse a copy folder that is the bundle's, inside it or holding it, also through a link, before either is written. */ -const refuseCopyInBundle = async (copy: string, bundle: string) => { - const [copied, bundled] = await Promise.all([ - resolvedPath(copy), - resolvedPath(bundle), - ]); - if (isWithin(copied, bundled) || isWithin(bundled, copied)) +const refuseCopyInBundle = ( + copy: DesktopOutputPlan, + bundle: DesktopOutputPlan, +) => { + if ( + isPathWithinRoot(bundle.physical, copy.physical) || + isPathWithinRoot(copy.physical, bundle.physical) + ) throw refused( "withheld_copy", - `The withheld copy ${copy} lies in the bundle ${bundle}, or holds it, so the withheld files would ship with it.`, + `The withheld copy ${copy.requested} lies in the bundle ${bundle.requested}, or holds it, so the withheld files would ship with it.`, "Name a copy folder outside the bundle's folder.", ); }; @@ -325,22 +316,35 @@ export const withheldCopy = async ( captured: ReadonlyMap, signal: AbortSignal | undefined, ): Promise<{ + readonly bundle: DesktopOutputPlan; readonly write: () => Promise; }> => { const directory = request.withheldCopy; const withheld = plan.withheld ?? []; - if (directory !== undefined) - await refuseCopyInBundle(directory, request.directory); - if (withheld.length === 0) return { write: () => Promise.resolve(undefined) }; + const bundle = await planOutputTree(request.directory, { + operation: OPERATION, + field: "directory", + }); + const copy = + directory === undefined + ? undefined + : await planOutputTree(directory, { + operation: OPERATION, + field: "withheld_copy", + }); + if (copy !== undefined) refuseCopyInBundle(copy, bundle); + if (withheld.length === 0) + return { bundle, write: () => Promise.resolve(undefined) }; const files = await withRetainedContent( request.capture, (content) => readWithheld(content, withheld, captured, signal), SEED_MAX_BYTES, ); return { + bundle, write: () => - directory === undefined + copy === undefined ? Promise.resolve(undefined) - : writeWithheldCopy(directory, files, signal), + : writeWithheldCopy(copy, files, signal), }; }; diff --git a/src/application/desktop/DesktopOutputTree.test.ts b/src/application/desktop/DesktopOutputTree.test.ts new file mode 100644 index 0000000000..a6ef3d1188 --- /dev/null +++ b/src/application/desktop/DesktopOutputTree.test.ts @@ -0,0 +1,30 @@ +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; +import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; +import { planOutputTree, writeOutputTree } from "./DesktopOutputTree.js"; + +it("does not create output parents or invoke the writer after cancellation", async () => { + const root = await createTestTempDirectory("rea-output-cancel-"); + const directory = join(root, "new-parent", "output"); + const refusal = { + operation: "export_desktop_application", + field: "directory", + }; + const plan = await planOutputTree(directory, refusal); + const controller = new AbortController(); + const reason = new Error("cancelled before output acquisition"); + controller.abort(reason); + let written = false; + const failure = await writeOutputTree( + plan, + async () => { + written = true; + }, + refusal, + controller.signal, + ).catch((cause: unknown) => cause); + expect(written).toBe(false); + expect(await readdir(root)).toEqual([]); + expect(failure).toBe(reason); +}); diff --git a/src/application/desktop/DesktopOutputTree.ts b/src/application/desktop/DesktopOutputTree.ts index 6b33ac41d8..09053bc5c8 100644 --- a/src/application/desktop/DesktopOutputTree.ts +++ b/src/application/desktop/DesktopOutputTree.ts @@ -1,13 +1,18 @@ -import { mkdir } from "node:fs/promises"; -import { dirname } from "node:path"; +import { lstat, mkdir, readdir } from "node:fs/promises"; +import { basename, dirname, isAbsolute } from "node:path"; import { ArtifactResourceScope, type ArtifactCleanupAttempt, } from "../../artifacts/ArtifactResourceScope.js"; import { SafeOutputTree } from "../../artifacts/SafeOutputTree.js"; -import { outputDirectoryAdmissionRefusal } from "../../artifacts/SafeOutputTreeAdmission.js"; +import { + outputDirectoryAdmissionRefusal, + SafeOutputTreeDestinationRefusal, +} from "../../artifacts/SafeOutputTreeAdmission.js"; import { SafeOutputTreeCreationFailure } from "../../artifacts/SafeOutputTreeCreationFailure.js"; +import { canonicalCallerPath } from "../../filesystem/canonicalCallerPath.js"; +import { absoluteCallerPath } from "../../filesystem/callerPath.js"; import type { Evidence } from "../../domain/evidence.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; @@ -17,6 +22,65 @@ export type FailedRelease = Extract< { readonly kind: "failed" } >; +/** A caller-selected destination and the filesystem path admitted for writing. */ +export interface DesktopOutputPlan { + readonly requested: string; + readonly physical: string; +} + +/** Establish destination identity before either output tree is created. */ +export const planOutputTree = async ( + requested: string, + refusal: { readonly operation: string; readonly field: string }, +): Promise => { + try { + if (!isAbsolute(requested)) + throw new SafeOutputTreeDestinationRefusal( + "Output root must be absolute", + ); + if (basename(requested) === ".." || basename(requested) === ".") + throw new SafeOutputTreeDestinationRefusal("Invalid output directory"); + const physical = await canonicalCallerPath(requested); + const absolute = absoluteCallerPath(requested); + const selected = + absolute.length > 1 ? absolute.replace(/\/$/u, "") : absolute; + const existing = await lstat(selected).then( + (value) => value, + (cause: unknown) => { + if ((cause as NodeJS.ErrnoException).code === "ENOENT") + return undefined; + throw cause; + }, + ); + if (existing?.isSymbolicLink()) + throw new SafeOutputTreeDestinationRefusal( + "Output root cannot be a symbolic link", + ); + if (existing !== undefined && !existing.isDirectory()) + throw new SafeOutputTreeDestinationRefusal( + "Output root must be a directory", + ); + if (existing !== undefined && (await readdir(selected)).length > 0) + throw new SafeOutputTreeDestinationRefusal( + "Output directory is not empty", + ); + return Object.freeze({ requested, physical }); + } catch (cause: unknown) { + const error = + cause instanceof SafeOutputTreeDestinationRefusal + ? cause + : new SafeOutputTreeDestinationRefusal( + "Output destination cannot be resolved", + { cause }, + ); + throw outputDirectoryAdmissionRefusal( + refusal.operation, + refusal.field, + error, + ); + } +}; + /** * Write a new or empty folder through the safe output tree, rolling an * unpublished tree back on any failure so no partial copy remains. A @@ -24,12 +88,13 @@ export type FailedRelease = Extract< * refused destination names the caller's `field` under `operation`. */ export const writeOutputTree = async ( - directory: string, + plan: DesktopOutputPlan, write: (tree: SafeOutputTree) => Promise, refusal: { readonly operation: string; readonly field: string }, signal: AbortSignal | undefined, ): Promise => { - await mkdir(dirname(directory), { recursive: true, mode: 0o700 }); + signal?.throwIfAborted(); + await mkdir(dirname(plan.physical), { recursive: true, mode: 0o700 }); const resources = new ArtifactResourceScope(); let tree: SafeOutputTree | undefined; let outcome: @@ -37,7 +102,7 @@ export const writeOutputTree = async ( | { readonly kind: "failed"; readonly cause: unknown }; try { try { - tree = await SafeOutputTree.create(directory, process.platform, { + tree = await SafeOutputTree.create(plan.physical, process.platform, { acceptExistingEmpty: true, }); } catch (cause: unknown) { @@ -53,7 +118,7 @@ export const writeOutputTree = async ( if (tree !== undefined) { const released = await resources.release({ kind: "output-tree", - resource: directory, + resource: plan.requested, tree, }); if (released.kind === "failed") { diff --git a/src/application/desktop/DesktopSqliteState.test.ts b/src/application/desktop/DesktopSqliteState.test.ts index 4829b037f2..0de2ce6575 100644 --- a/src/application/desktop/DesktopSqliteState.test.ts +++ b/src/application/desktop/DesktopSqliteState.test.ts @@ -7,6 +7,8 @@ import { } from "../../domain/analysisErrorCore.js"; import { createEvidence } from "../../domain/evidence.js"; import { err, ok } from "../../domain/result.js"; +import type { SqliteDatabase } from "../../domain/sqlite/sqliteDatabase.js"; +import { diffStateViews } from "../../domain/desktop/stateChanges.js"; import { readSqliteStateView } from "./DesktopSqliteState.js"; const OPERATION = "inspect_sqlite_database"; @@ -34,7 +36,7 @@ const table = (name: string) => ({ }); /** A SQLite report with two tables, and one row of the selected table. */ -const report = (selected: string | undefined) => ({ +const report = (selected: string | undefined): SqliteDatabase => ({ artifact: { path: "/staged/state.db", sha256: "a".repeat(64), bytes: 4096 }, wal: null, engine: { name: "SQLite", version: "3.50.2" }, @@ -86,6 +88,79 @@ const service = (failure: (table: string) => AnalysisError | undefined) => ({ }); describe("readSqliteStateView", () => { + it("distinguishes special REAL values from text when comparing captured rows", async () => { + const read = async (type: "real" | "text") => + readSqliteStateView( + { + inspect: async (input: unknown) => { + const selected = (input as { table?: string }).table; + const result: SqliteDatabase = report(selected); + result.schema.tables = [table("notes")]; + if (result.rows !== null) { + result.rows.values = ( + ["Infinity", "-Infinity", "-0"] as const + ).map((value) => [ + type === "real" + ? { type: "real", value } + : { + type: "text", + value, + bytes_hex: Buffer.from(value).toString("hex"), + encoding: "UTF-8", + }, + ]); + result.rows.returned_rows = 3; + } + return ok( + createEvidence( + undefined, + { id: "sqlite", name: "SQLite", version: "1" }, + { + operation: OPERATION, + parameters: {}, + result, + confidence: "observed", + }, + ), + ); + }, + }, + "/staged/state.db", + ); + const real = await read("real"); + const text = await read("text"); + expect(real).toMatchObject({ + ok: true, + view: { + tables: { + notes: { + rows: [ + [{ real: "Infinity" }], + [{ real: "-Infinity" }], + [{ real: "-0" }], + ], + }, + }, + }, + }); + expect(text).toMatchObject({ + ok: true, + view: { + tables: { + notes: { + rows: [["Infinity"], ["-Infinity"], ["-0"]], + }, + }, + }, + }); + if (!real.ok || !text.ok) throw new Error("Expected readable SQLite views"); + expect(diffStateViews("sqlite", real.view, text.view)).toMatchObject({ + added_count: 3, + removed_count: 3, + complete: true, + }); + }); + it("keeps the tables that read and records the error of one that did not", async () => { const read = await readSqliteStateView( service((name) => diff --git a/src/application/desktop/DesktopSqliteState.ts b/src/application/desktop/DesktopSqliteState.ts index 7b85929262..61a8ce6696 100644 --- a/src/application/desktop/DesktopSqliteState.ts +++ b/src/application/desktop/DesktopSqliteState.ts @@ -27,7 +27,7 @@ type Cell = z.infer; /** * SQLite cells as JSON. Values that plain JSON would conflate keep a tagged * form: integers beyond the safe range, text whose bytes did not decode - * cleanly, and blobs. + * cleanly, special REAL values, and blobs. */ const cellValue = (cell: Cell): JsonValue => { switch (cell.type) { @@ -38,7 +38,7 @@ const cellValue = (cell: Cell): JsonValue => { return Number.isSafeInteger(value) ? value : { integer: cell.value }; } case "real": - return cell.value; + return typeof cell.value === "string" ? { real: cell.value } : cell.value; case "text": return cell.value.includes("\uFFFD") ? { text: cell.value, bytes_hex: cell.bytes_hex } diff --git a/src/application/sqlite/SqliteDatabaseService.test.ts b/src/application/sqlite/SqliteDatabaseService.test.ts index 5637ac53ef..9926f2fa1c 100644 --- a/src/application/sqlite/SqliteDatabaseService.test.ts +++ b/src/application/sqlite/SqliteDatabaseService.test.ts @@ -4,6 +4,7 @@ import { AnalysisCapabilityUnavailableError } from "../../domain/analysisErrorCo import { err, ok } from "../../domain/result.js"; import { createAnalysisExecution } from "../AnalysisProvider.js"; import { sqliteDatabaseSchema } from "../../domain/sqlite/sqliteDatabase.js"; +import { createAnalysisProfile } from "../../domain/analysisProfile.js"; const fixture = () => ({ artifact: { path: "/selected.db", sha256: "a".repeat(64), bytes: 4096 }, @@ -236,39 +237,59 @@ it.each(["path", "digest", "subject", "wal", "table"])( }, ); -it("preserves completed observed evidence when cancellation arrives after provider completion", async () => { - const value = fixture(); - const service = new SqliteDatabaseService({ - inspect: () => Promise.resolve(ok(execution(value))), - }); - expect(await service.inspect({ path: "/selected.db" })).toMatchObject({ - ok: true, - value: { +it.each([false, true])( + "retains completed SQLite facts with late cancellation=%s", + async (cancelled) => { + const value = fixture(); + const observed = execution(value); + const profile = createAnalysisProfile( + { ...observed.provider, version: value.engine.version }, + { + schema: "inventory", + }, + ); + const relationshipCoverage = { + status: "unknown", + reason: "Relationships were not inspected", + } as const; + const completed = { + ...observed, + analysisProfile: profile, + relationshipCoverage, + }; + const controller = new AbortController(); + const reason = Object.freeze({ + requested: "cancel after database inspection", + }); + const service = new SqliteDatabaseService({ + inspect: () => { + if (cancelled) controller.abort(reason); + return Promise.resolve(ok(completed)); + }, + }); + const result = await service.inspect( + { path: "/selected.db" }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(!cancelled); + if (!result.ok) { + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + } + const evidence = result.ok ? result.value : result.error.partialObservation; + expect(evidence).toMatchObject({ + operation: "inspect_sqlite_database", confidence: "observed", subject: { local_path: "/selected.db", digest: { sha256: "a".repeat(64) }, }, normalized_result: value, - }, - }); - const controller = new AbortController(); - const cancelled = new SqliteDatabaseService({ - inspect: () => { - controller.abort(); - return Promise.resolve(ok(execution(value))); - }, - }); - expect( - await cancelled.inspect( - { path: "/selected.db" }, - { signal: controller.signal }, - ), - ).toMatchObject({ - ok: true, - value: { normalized_result: value }, - }); -}); + analysis_profile: profile, + relationship_coverage: relationshipCoverage, + }); + }, +); it("preserves the exact caller reason when inspection is already aborted", async () => { const reason = Object.freeze({ requested: "stop" }); diff --git a/src/application/sqlite/SqliteDatabaseService.ts b/src/application/sqlite/SqliteDatabaseService.ts index bb83c7c25b..bed99f8527 100644 --- a/src/application/sqlite/SqliteDatabaseService.ts +++ b/src/application/sqlite/SqliteDatabaseService.ts @@ -3,11 +3,10 @@ import type { ExecutionOptions } from "../AnalysisProvider.js"; import type { SqliteDatabasePort } from "./SqliteDatabasePort.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import { AnalysisOutputError } from "../../domain/analysisErrorCore.js"; -import { - createEvidence, - type Evidence, - type EvidenceSubjectTarget, -} from "../../domain/evidence.js"; +import type { Evidence, EvidenceSubjectTarget } from "../../domain/evidence.js"; +import { createAnalysisExecutionEvidence } from "../../domain/analysisObservation.js"; +import { ownedJsonValueSchema } from "../../domain/jsonValue.js"; +import { completeProviderEvidence } from "../ProviderOperationExecution.js"; import { inspectSqliteDatabaseInputSchema, sqliteDatabaseSchema, @@ -19,7 +18,7 @@ import { sqliteSchemaScope, sqliteInspectionParameters, } from "../../domain/sqlite/sqliteDatabase.js"; -import { err, ok, type Result } from "../../domain/result.js"; +import { err, type Result } from "../../domain/result.js"; const OPERATION = "inspect_sqlite_database"; @@ -102,18 +101,14 @@ export class SqliteDatabaseService { "SQLite adapter returned malformed data or changed the selected database identity/table/row limit/worker budget.", ), ); - return ok( - createEvidence(execution.subject, execution.provider, { - operation: OPERATION, - parameters: sqliteInspectionParameters(input.value), - result: report.data, - ...(execution.rawResult === null - ? {} - : { rawResult: execution.rawResult }), - confidence: "observed", - locations: execution.locations, - limitations: execution.limitations, - }), + return completeProviderEvidence( + createAnalysisExecutionEvidence( + OPERATION, + sqliteInspectionParameters(input.value), + { ...execution, result: ownedJsonValueSchema.parse(report.data) }, + "observed", + ), + options?.signal, ); } } diff --git a/src/desktop/AgentEnvComputer.test.ts b/src/desktop/AgentEnvComputer.test.ts new file mode 100644 index 0000000000..bbcb5e06f3 --- /dev/null +++ b/src/desktop/AgentEnvComputer.test.ts @@ -0,0 +1,158 @@ +import { createHash } from "node:crypto"; + +import { expect, it, vi } from "vitest"; + +import { StructuredContentMcpClient } from "../providers/StructuredContentMcpClient.js"; +import { AgentEnvComputer } from "./AgentEnvComputer.js"; +import type { ComputerHelper } from "./AgentEnvComputerHelper.js"; + +it.each(["http", "invalid_json", "readiness"] as const)( + "redacts the complete control token in %s failure diagnostics", + async (failure) => { + const token = "fake-control-token-keep-private-123456789"; + const status = failure === "http" ? 403 : 200; + const ordinary = + failure === "http" ? "x".repeat(500) : "ordinary target text"; + const body = + failure === "readiness" + ? JSON.stringify({ + ready: false, + checks: [ + { + name: "app", + ok: false, + final: true, + detail: `${token} ${ordinary}`, + }, + ], + }) + : failure === "http" + ? `${ordinary}${token}` + : `${token} ${ordinary}`; + const connect = vi + .spyOn(StructuredContentMcpClient.prototype, "connect") + .mockResolvedValue(undefined); + const close = vi + .spyOn(StructuredContentMcpClient.prototype, "close") + .mockResolvedValue(undefined); + const fetch = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response(body, { status })); + try { + const error = await AgentEnvComputer.connect( + { + handshake: async () => ({ + sandbox_id: "controlled-sandbox", + base_url: "http://127.0.0.1:1", + mcp_url: "http://127.0.0.1:1/mcp", + control_token: token, + }), + } as unknown as ComputerHelper, + new AbortController().signal, + ).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(Error); + if (!(error instanceof Error)) throw error; + expect(error.message).not.toContain(token.slice(0, 10)); + expect(error.message).toContain("[control tok"); + expect(error.message).toContain(ordinary.slice(0, 100)); + if (failure !== "readiness") { + expect(error.cause).toBeInstanceOf(Error); + expect(String(error.cause)).not.toContain(token.slice(0, 10)); + } + } finally { + fetch.mockRestore(); + close.mockRestore(); + connect.mockRestore(); + } + }, +); +it.each(["canonical", "missing", "rpc_error"] as const)( + "validates recording replies and redacts their errors (%s)", + async (answer) => { + const present = answer === "canonical"; + const token = "fake-recording-control-token-123456789"; + const path = "rec-0123/0000.mp4"; + const homePath = `.agentenv-recordings/${path}`; + const bytes = Buffer.from("controlled recording"); + const connect = vi + .spyOn(StructuredContentMcpClient.prototype, "connect") + .mockResolvedValue(undefined); + const close = vi + .spyOn(StructuredContentMcpClient.prototype, "close") + .mockResolvedValue(undefined); + const call = vi + .spyOn(StructuredContentMcpClient.prototype, "callTool") + .mockResolvedValue({ content: [] }); + const fetch = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce(Response.json({ ready: true })) + .mockResolvedValueOnce( + Response.json( + answer === "rpc_error" + ? { error: { message: `${token} ordinary target text` } } + : { + result: { + parts: [ + { + kind: "file", + file: { bytes: bytes.toString("base64") }, + metadata: { + "urn:unrelated": { + path: present ? "wrong-path" : homePath, + }, + ...(present + ? { + "urn:agentenv:data-objects/v1": { + path: homePath, + }, + } + : {}), + }, + }, + ], + }, + }, + ), + ); + try { + const { computer } = await AgentEnvComputer.connect( + { + handshake: async () => ({ + sandbox_id: "controlled-sandbox", + base_url: "http://127.0.0.1:1", + mcp_url: "http://127.0.0.1:1/mcp", + control_token: token, + }), + release: async () => ({ verified: true, teardown: "agentenv" }), + } as unknown as ComputerHelper, + new AbortController().signal, + ); + const read = computer.recordingSegments({ + recording_id: "rec-0123", + segments: [ + { + index: 0, + path, + bytes: bytes.length, + sha256: createHash("sha256").update(bytes).digest("hex"), + }, + ], + }); + if (present) await expect(read).resolves.toEqual([{ path, bytes }]); + else if (answer === "rpc_error") { + const failure = await read.catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(Error); + if (!(failure instanceof Error)) throw failure; + expect(failure.message).toContain("ordinary target text"); + expect(failure.message).toContain("[control token]"); + expect(failure.message).not.toContain(token); + } else await expect(read).rejects.toThrow("missing or differs"); + await computer.close(); + } finally { + fetch.mockRestore(); + call.mockRestore(); + close.mockRestore(); + connect.mockRestore(); + } + }, +); diff --git a/src/desktop/AgentEnvComputer.ts b/src/desktop/AgentEnvComputer.ts index 0a6a621c3f..ccf5231eff 100644 --- a/src/desktop/AgentEnvComputer.ts +++ b/src/desktop/AgentEnvComputer.ts @@ -14,6 +14,7 @@ import type { ComputerRelease, } from "./AgentEnvComputerHelper.js"; import { + DATA_OBJECTS, dataGetSchema, readHomeFiles, writeHomeFiles, @@ -498,10 +499,10 @@ export class AgentEnvComputer { ); if ("error" in answer) throw new Error( - `data/get of recording segment ${segment.path} failed: ${answer.error.message}`, + `data/get of recording segment ${segment.path} failed: ${this.redacted(answer.error.message)}`, ); const encoded = answer.result.parts.find( - (part) => Object.values(part.metadata ?? {})[0]?.path === path, + (part) => part.metadata?.[DATA_OBJECTS]?.path === path, )?.file?.bytes; const bytes = encoded === undefined ? undefined : Buffer.from(encoded, "base64"); @@ -653,9 +654,11 @@ export class AgentEnvComputer { const failed = waiting.find((check) => check.final === true); if (failed !== undefined || Date.now() > deadline) throw new ComputerStartFailure( - failed === undefined - ? `The computer was not ready within ${String(READY_TIMEOUT_MS / 1000)} s; waiting on ${waiting.map((check) => check.name).join(", ")}.` - : `The computer's ${failed.name} check failed: ${failed.detail ?? "no detail"}.`, + this.redacted( + failed === undefined + ? `The computer was not ready within ${String(READY_TIMEOUT_MS / 1000)} s; waiting on ${waiting.map((check) => check.name).join(", ")}.` + : `The computer's ${failed.name} check failed: ${failed.detail ?? "no detail"}.`, + ), "Read the helper's output; `agent-env run --task smoke --no-emulate` reproduces the deploy.", { reason: "not_ready" }, ); @@ -702,8 +705,15 @@ export class AgentEnvComputer { const text = await readBounded(response, limit.bytes, limit.what); if (response.status < 200 || response.status > 299) throw new Error( - `The computer answered ${path} with HTTP ${String(response.status)}: ${this.redacted(text.slice(0, 512))}`, + `The computer answered ${path} with HTTP ${String(response.status)}: ${this.redacted(text).slice(0, 512)}`, ); - return JSON.parse(text) as unknown; + try { + return JSON.parse(text) as unknown; + } catch { + // Parser diagnostics can quote a token fragment that exact redaction cannot match. + throw new Error( + `The computer answered ${path} with invalid JSON: ${this.redacted(text).slice(0, 512)}`, + ); + } } } diff --git a/src/desktop/ApplicationStateDirectories.test.ts b/src/desktop/ApplicationStateDirectories.test.ts new file mode 100644 index 0000000000..6d40a09f91 --- /dev/null +++ b/src/desktop/ApplicationStateDirectories.test.ts @@ -0,0 +1,54 @@ +import { chmod, lstat, mkdir } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, it } from "vitest"; + +import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; +import { desktopApplicationInventorySchema } from "../domain/desktop/desktopCapture.js"; +import { stateDirectories } from "./ApplicationStateDirectories.js"; + +it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "preserves unknown state-path existence when permission denies the probe", + async () => { + const home = await createTestTempDirectory("state-path-existence-"); + const sealed = join(home, "sealed"); + const denied = join(sealed, "notes"); + const present = join(home, ".notes"); + const absent = join(home, ".local/share/notes"); + await mkdir(denied, { recursive: true }); + await mkdir(present); + await chmod(sealed, 0o000); + try { + await expect(lstat(denied)).rejects.toMatchObject({ code: "EACCES" }); + const directories = await stateDirectories( + ["notes"], + { + home, + environment: { XDG_CONFIG_HOME: sealed }, + }, + [], + ); + const facts = new Map(directories.map((entry) => [entry.path, entry])); + expect(facts.get(present)).toEqual({ + path: present, + exists_at_start: true, + }); + expect(facts.get(absent)).toEqual({ + path: absent, + exists_at_start: false, + }); + expect(facts.get(denied)).toMatchObject({ + path: denied, + exists_at_start: null, + existence_error: expect.stringContaining("EACCES"), + }); + expect( + desktopApplicationInventorySchema.shape.state_directories.parse( + directories, + ), + ).toEqual(directories); + } finally { + await chmod(sealed, 0o700); + } + }, +); diff --git a/src/desktop/ApplicationStateDirectories.ts b/src/desktop/ApplicationStateDirectories.ts index 301e6b3599..5860848f25 100644 --- a/src/desktop/ApplicationStateDirectories.ts +++ b/src/desktop/ApplicationStateDirectories.ts @@ -102,9 +102,21 @@ export const stateDirectories = async ( ...extra, ]); return Promise.all( - [...paths].map(async (path) => ({ - path, - exists_at_start: (await lstat(path).catch(() => undefined)) !== undefined, - })), + [...paths].map(async (path) => { + try { + await lstat(path); + return { path, exists_at_start: true }; + } catch (cause: unknown) { + const code = (cause as NodeJS.ErrnoException).code; + return code === "ENOENT" || code === "ENOTDIR" + ? { path, exists_at_start: false } + : { + path, + exists_at_start: null, + existence_error: + cause instanceof Error ? cause.message : String(cause), + }; + } + }), ); }; diff --git a/src/desktop/AptRepositoryInventory.test.ts b/src/desktop/AptRepositoryInventory.test.ts index 9d11b52837..8c1f5df2cd 100644 --- a/src/desktop/AptRepositoryInventory.test.ts +++ b/src/desktop/AptRepositoryInventory.test.ts @@ -1,5 +1,5 @@ import { existsSync } from "node:fs"; -import { writeFile } from "node:fs/promises"; +import { mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -7,9 +7,89 @@ import { describe, expect, it } from "vitest"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { APT_LOCATIONS, + inspectAptRepositories, resolveAptLocations, } from "./AptRepositoryInventory.js"; import { runHostQuery, type QueryRunner } from "./HostQuery.js"; +import { + GPGV_A_ONLY, + VENDOR_A, + VENDOR_B, +} from "../domain/desktop/aptKeys.fixture.js"; + +it.each(["list", "sources"] as const)( + "does not broaden %s Signed-By fingerprint restrictions to every key in a keyring", + async (format) => { + const root = await createTestTempDirectory("apt-signed-by-"); + const sources = join(root, `vendor.${format}`); + const key = join(root, "vendor.gpg"); + const trustedParts = join(root, "trusted.gpg.d"); + await mkdir(trustedParts); + await writeFile(key, VENDOR_A.key); + await writeFile( + join(root, "vendor.example_repo_InRelease"), + "controlled release", + ); + const source = "https://vendor.example/repo stable/main amd64 Packages"; + const targets = [ + `Description: ${source}`, + `Sourcesentry: ${sources}:1`, + "Repo-URI: https://vendor.example/repo/", + "Release: stable", + "Component: main", + "Base-URI: https://vendor.example/repo/", + `Filename: ${join(root, "Packages")}`, + ].join("\n"); + let verifications = 0; + const runQuery: QueryRunner = async (command) => { + if (command === "apt-get") return { stdout: targets, exitCode: 0 }; + if (command !== "gpgv") throw new Error(`Unexpected query ${command}`); + verifications += 1; + return { stdout: GPGV_A_ONLY, exitCode: 1 }; + }; + const inspect = async (signedBy: string) => { + await writeFile( + sources, + format === "list" + ? `deb [signed-by=${signedBy.replaceAll(" ", ",")}] https://vendor.example/repo stable main\n` + : `Types: deb\nURIs: https://vendor.example/repo\nSuites: stable\nComponents: main\nSigned-By: ${signedBy}\n`, + ); + return inspectAptRepositories( + { sources: [source], origins: ["Vendor"] }, + { + runQuery, + apt: { lists: root, trustedParts, trusted: key }, + }, + undefined, + ); + }; + const pathOnly = await inspect(key); + expect(pathOnly.repositories[0]?.signing_key).toMatchObject({ + path: key, + fingerprint: VENDOR_A.fingerprint, + }); + expect(pathOnly.keys.size).toBe(1); + for (const selector of [VENDOR_B.fingerprint, `${VENDOR_B.fingerprint}!`]) { + for (const signedBy of [selector, `${key} ${selector}`]) { + const restricted = await inspect(signedBy); + expect(restricted.repositories[0]).toMatchObject({ + source, + sources_file: sources, + uri: "https://vendor.example/repo", + suite: "stable", + components: ["main"], + signing_key: null, + signing_key_reason: expect.stringContaining(selector), + }); + expect(restricted.repositories[0]?.signing_key_reason).toContain( + "not supported", + ); + expect(restricted.keys.size).toBe(0); + } + } + expect(verifications).toBe(1); + }, +); describe.skipIf( process.platform !== "linux" || !existsSync("/usr/bin/apt-config"), diff --git a/src/desktop/AptRepositoryInventory.ts b/src/desktop/AptRepositoryInventory.ts index 5452d5c9a9..1317542960 100644 --- a/src/desktop/AptRepositoryInventory.ts +++ b/src/desktop/AptRepositoryInventory.ts @@ -138,8 +138,7 @@ const candidateKeys = async ( entry: AptSourceEntry, apt: AptLocations, ): Promise => { - const signedBy = entry.signedBy.filter((value) => value.startsWith("/")); - if (signedBy.length > 0) return signedBy; + if (entry.signedBy.length > 0) return entry.signedBy; return [ ...(await fileNames(apt.trustedParts)) .filter((name) => name.endsWith(".gpg") || name.endsWith(".asc")) @@ -199,6 +198,11 @@ const keyCheck = async ( return { reason: `no sources entry at ${target.sourcesFile}:${String(target.position)} configures it`, }; + const selectors = entry.signedBy.filter((value) => !value.startsWith("/")); + if (selectors.length > 0) + return { + reason: `Signed-By selectors ${selectors.map((value) => JSON.stringify(value)).join(", ")} are not supported; REA cannot verify this repository's fingerprint restrictions, so no signing key is retained`, + }; const release = await signedRelease(target); if (release === undefined) return { diff --git a/src/desktop/ComputerHomeFiles.ts b/src/desktop/ComputerHomeFiles.ts index ebe9db541b..9c6bf6e53f 100644 --- a/src/desktop/ComputerHomeFiles.ts +++ b/src/desktop/ComputerHomeFiles.ts @@ -1,7 +1,7 @@ import { z } from "zod"; /** Where a data-plane file part names its path in the home. */ -const DATA_OBJECTS = "urn:agentenv:data-objects/v1"; +export const DATA_OBJECTS = "urn:agentenv:data-objects/v1"; /** A `data/get` answer: file parts, each naming its home path, or an error. */ export const dataGetSchema = z.union([ z.object({ @@ -74,7 +74,7 @@ export const readHomeFiles = async ( ? [] : [ { - path: Object.values(part.metadata ?? {})[0]?.path, + path: part.metadata?.[DATA_OBJECTS]?.path, bytes: Buffer.from(encoded, "base64"), }, ]; diff --git a/src/desktop/DebianPackageInventory.test.ts b/src/desktop/DebianPackageInventory.test.ts index ef5d14f72c..0ace10d99e 100644 --- a/src/desktop/DebianPackageInventory.test.ts +++ b/src/desktop/DebianPackageInventory.test.ts @@ -1066,3 +1066,52 @@ describe.skipIf(process.platform === "win32")( }); }, ); + +it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "keeps apt list presence unknown when its directory cannot be read", + async () => { + const { root, inspect } = await writerFixture(); + const lists = join(root, "lists"); + await chmod(lists, 0o000); + try { + const { installed, limitations } = await inspect({ + executable: "libreoffice", + }); + expect(installed.package_lists_present).toBeNull(); + expect(limitations.join("\n")).toContain("could not be read"); + expect(limitations.join("\n")).toContain("EACCES"); + } finally { + await chmod(lists, 0o755); + } + }, +); + +it.skipIf(process.platform === "win32")( + "keeps package ownership unknown for a literal path dpkg would treat as a pattern", + async () => { + const { root, runQuery, inspect } = await writerFixture(); + const executable = join(root, "bin/libreoffice[1]"); + await writeFile(executable, "#!/bin/sh\n"); + await chmod(executable, 0o755); + const result = await inspect( + { executable }, + { + run: async (command, argv, signal) => { + if ( + command === "dpkg-query" && + argv[0] === "-S" && + argv.includes(executable) + ) + // A glob search can name another file without establishing who owns the literal brackets. + return { + stdout: `other-package: ${executable.replace("[1]", "1")}`, + exitCode: 0, + }; + return runQuery(command, argv, signal); + }, + }, + ); + expect(result.installed).toMatchObject({ kind: "unknown", name: null }); + expect(result.limitations.join("\n")).toContain("pattern characters"); + }, +); diff --git a/src/desktop/DebianPackageInventory.ts b/src/desktop/DebianPackageInventory.ts index 204d40996a..e89f106550 100644 --- a/src/desktop/DebianPackageInventory.ts +++ b/src/desktop/DebianPackageInventory.ts @@ -1,4 +1,5 @@ import type { DesktopLibraryPolicy } from "../domain/desktop/libraryPolicy.js"; +import { describeValidationFailure } from "../domain/validationFailure.js"; import { readdir } from "node:fs/promises"; import { @@ -203,6 +204,11 @@ const dpkgOwner = async ( { host, signal }: InventoryContext, ): Promise => { for (const path of ownershipCandidates(paths)) { + if (/[*?[\\]/u.test(path)) + return { + kind: "unknown", + reason: `dpkg-query cannot safely query ${path} containing pattern characters`, + }; const outcome = await host.runQuery("dpkg-query", ["-S", path], signal); if (outcome === "missing") return { kind: "unknown", reason: "dpkg-query is not installed" }; @@ -283,10 +289,17 @@ export const debianPackage = async ( await host.runQuery("apt-cache", ["policy", name], signal), ); const apt = await resolveAptLocations(host.runQuery, signal, host.apt); - const listsPresent = ( - await readdir(apt.lists).catch(() => [] as string[]) - ).some((list) => /_Packages(?:\.[a-z0-9]+)?$/u.test(list)); - if (!listsPresent) + const listsPresent = await readdir(apt.lists).then( + (lists) => lists.some((list) => /_Packages(?:\.[a-z0-9]+)?$/u.test(list)), + (cause: unknown) => { + if ((cause as NodeJS.ErrnoException).code === "ENOENT") return false; + limitations.push( + `The apt package lists directory ${apt.lists} could not be read, so whether package lists are present is unknown: ${describeValidationFailure(cause)}`, + ); + return null; + }, + ); + if (listsPresent === false) limitations.push( "No apt package lists are present, so whether the installed version comes from a package archive is unknown.", ); diff --git a/src/desktop/DesktopApplicationSession.test.ts b/src/desktop/DesktopApplicationSession.test.ts index 797e77760d..f06d8353d5 100644 --- a/src/desktop/DesktopApplicationSession.test.ts +++ b/src/desktop/DesktopApplicationSession.test.ts @@ -143,6 +143,50 @@ describe.skipIf(process.platform !== "linux")( describe.skipIf(process.platform === "win32")( "runDesktopSession stderr", () => { + it.each([ + { fails: false, cancelAt: "cleanup" }, + { fails: true, cancelAt: "cleanup" }, + { fails: false, cancelAt: "exit" }, + ])( + "preserves the session outcome when cancellation arrives during $cancelAt (checkpoint fails: $fails)", + async ({ fails, cancelAt }) => { + const root = await createTestTempDirectory( + "desktop-session-final-cancel-", + ); + const controller = new AbortController(); + const reason = new Error("cancelled during cleanup"); + const failure = new Error("stop checkpoint failed"); + const terminate = vi.mocked(terminateTokenOwnedProcesses); + terminate.mockImplementationOnce(async (...args) => { + if (cancelAt === "cleanup") controller.abort(reason); + const original = await vi.importActual< + typeof import("../process/TokenOwnedTermination.js") + >("../process/TokenOwnedTermination.js"); + return original.terminateTokenOwnedProcesses(...args); + }); + const checkpoints: string[] = []; + const result = await runDesktopSession({ + ...shellPlan(root, "exit 0"), + signal: controller.signal, + nextRequest: () => ({ kind: "stop" }), + checkpoint: async (name) => { + checkpoints.push(name); + if (cancelAt === "exit" && name === "exit") + controller.abort(reason); + if (fails) throw failure; + }, + }).catch((cause: unknown) => cause); + if (fails) expect(result).toBe(failure); + else + expect(result).toMatchObject({ + session: { end_reason: "cancelled", cleanup: { verified: true } }, + }); + expect(checkpoints).toEqual( + cancelAt === "exit" ? ["stop", "exit"] : ["stop"], + ); + }, + ); + it("keeps the end of an app's stderr when it writes more than the session keeps", async () => { const root = await createTestTempDirectory("desktop-session-stderr-"); const cause = diff --git a/src/desktop/DesktopApplicationSession.ts b/src/desktop/DesktopApplicationSession.ts index 28493702c8..8035bc2d89 100644 --- a/src/desktop/DesktopApplicationSession.ts +++ b/src/desktop/DesktopApplicationSession.ts @@ -364,10 +364,13 @@ export const runDesktopSession = async ( "stop", ); cleanup = await releaseOnce(); + signal.throwIfAborted(); await checkpointNow(plan, observations, "exit", "exit"); + signal.throwIfAborted(); } catch (cause: unknown) { + const cancelled = signal.aborted; cleanup = await releaseOnce(); - if (!signal.aborted) { + if (!cancelled) { const left = unreleased(spawned, supervisor, cleanup); if (left === undefined) throw cause; throw new DesktopSessionCleanupError(cause, left); diff --git a/src/desktop/DesktopContentStore.test.ts b/src/desktop/DesktopContentStore.test.ts index 05e36ebc04..e61299ab1b 100644 --- a/src/desktop/DesktopContentStore.test.ts +++ b/src/desktop/DesktopContentStore.test.ts @@ -2,12 +2,14 @@ import { createHash } from "node:crypto"; import { mkdir, readFile, readdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import type { DesktopPathState } from "../domain/desktop/desktopStateDiff.js"; import { DesktopContentStore } from "./DesktopContentStore.js"; import { walkHomeState } from "./HomeStateWalker.js"; +import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; +import { RegularFileCleanupFailure } from "../filesystem/OwnedRegularFile.js"; const sha256 = (text: string) => createHash("sha256").update(text).digest("hex"); @@ -33,6 +35,63 @@ const store = async (root: string, maxFileBytes: number, maxBytes: number) => { return created; }; +it("DesktopContentStore leaves an occupied partial filename unowned", async () => { + const root = await createTestTempDirectory("desktop-content-occupied-"); + const content = await store(root, 1024, 1024); + const bytes = Buffer.from("new content"); + const partial = join(content.directory, `${sha256("new content")}.partial`); + await writeFile(partial, "foreign content"); + await expect(content.retainBytes(bytes)).rejects.toMatchObject({ + code: "EEXIST", + }); + await expect(content.discard()).rejects.toThrow(); + expect(await readFile(partial, "utf8")).toBe("foreign content"); +}); + +it("DesktopContentStore retains every unpublished file until discard", async () => { + const root = await createTestTempDirectory("desktop-content-unpublished-"); + const content = await store(root, 1024, 1024); + const digests = [sha256("first"), sha256("second")]; + for (const [index, text] of ["first", "second"].entries()) { + await mkdir(join(content.directory, digests[index]!)); + await expect(content.retainBytes(Buffer.from(text))).rejects.toThrow(); + } + await expect(content.discard()).rejects.toThrow(); + expect((await readdir(content.directory)).sort()).toEqual(digests.sort()); +}); + +it("DesktopContentStore retains a failed output close until cleanup retry", async () => { + const root = await createTestTempDirectory("desktop-content-close-"); + const content = await store(root, 1024, 1024); + const closeFailure = new Error("output descriptor close failed"); + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockRejectedValue(closeFailure); + try { + const failure = await content + .retainBytes(Buffer.from("pending")) + .catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(RegularFileCleanupFailure); + if (!(failure instanceof RegularFileCleanupFailure)) + throw new Error("Expected retained output close failure"); + await expect(content.close()).rejects.toMatchObject({ + cleanup: { + resources: [join(content.directory, `${sha256("pending")}.partial`)], + }, + }); + close.mockRestore(); + await content.close(); + await expect(failure.owner.handle.stat()).rejects.toMatchObject({ + code: "EBADF", + }); + await content.discard(); + expect(await readdir(join(root, "capture"))).toEqual([]); + } finally { + close.mockRestore(); + await content.close(); + } +}); + describe("DesktopContentStore", () => { it("stores each distinct content once and charges the budget once", async () => { const root = await createTestTempDirectory("desktop-content-"); diff --git a/src/desktop/DesktopContentStore.ts b/src/desktop/DesktopContentStore.ts index 44ad9c2762..e337ce3bfc 100644 --- a/src/desktop/DesktopContentStore.ts +++ b/src/desktop/DesktopContentStore.ts @@ -1,13 +1,14 @@ import { createHash } from "node:crypto"; -import { mkdir, rename, rm, rmdir, writeFile } from "node:fs/promises"; +import { mkdir, open, rename, rm, rmdir } from "node:fs/promises"; import { join } from "node:path"; import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import type { DesktopApplicationCapture } from "../domain/desktop/desktopCapture.js"; import type { DesktopPathState } from "../domain/desktop/desktopStateDiff.js"; -import type { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; +import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { RegularFileCleanupFailure, + withOwnedFileHandle, withOwnedRegularFile, } from "../filesystem/OwnedRegularFile.js"; import { @@ -45,8 +46,8 @@ export class DesktopContentStore { readonly #omissions: Omission[] = []; /** Handles whose close failed; kept owned so `close` can retry them. */ readonly #unclosed = new ArtifactResourceScope(); - /** The partial file being written, removed by `discard` if it was left. */ - #writing: string | undefined; + /** Every partial file this store created but has not published or removed. */ + readonly #unpublished = new Set(); #remainingBytes: number; constructor( @@ -94,8 +95,10 @@ export class DesktopContentStore { * when the directory holds anything else or a removal fails. */ async discard(): Promise { - if (this.#writing !== undefined) await rm(this.#writing, { force: true }); - this.#writing = undefined; + for (const path of this.#unpublished) { + await rm(path, { force: true }); + this.#unpublished.delete(path); + } for (const sha256 of this.#stored.keys()) { await rm(join(this.#directory, sha256), { force: true }); this.#stored.delete(sha256); @@ -139,10 +142,24 @@ export class DesktopContentStore { return { kind: "omitted", reason: "budget_exhausted" }; const target = join(this.#directory, sha256); const partial = `${target}.partial`; - this.#writing = partial; - await writeFile(partial, bytes, { mode: 0o600, flag: "wx" }); + const owner = new OwnedFileHandle(await open(partial, "wx", 0o600)); + this.#unpublished.add(partial); + await withOwnedFileHandle( + partial, + owner, + { + retainFailedClose: (handle) => { + this.#unclosed.retain({ + kind: "file-handle", + resource: partial, + handle, + }); + }, + }, + (file) => file.writeFile(bytes), + ); await rename(partial, target); - this.#writing = undefined; + this.#unpublished.delete(partial); this.#stored.set(sha256, size); this.#storedSizes.add(size); this.#remainingBytes -= size; diff --git a/src/desktop/GpgvKeyCheck.test.ts b/src/desktop/GpgvKeyCheck.test.ts index 6cd53855fc..a48b839ed9 100644 --- a/src/desktop/GpgvKeyCheck.test.ts +++ b/src/desktop/GpgvKeyCheck.test.ts @@ -85,6 +85,49 @@ describe.skipIf(!hasGpgv)("withGpgvKeyCheck with gpgv", () => { }); describe("withGpgvKeyCheck cleanup", () => { + it.each(["completed", "failed"])( + "observes cancellation during final keyring cleanup after %s work", + async (completion) => { + const cancel = new AbortController(); + const reason = new Error("cancelled during keyring cleanup"); + const primary = new Error("verification failed"); + const create = PrivateRuntimeRoot.create.bind(PrivateRuntimeRoot); + let path: string | undefined; + vi.spyOn(PrivateRuntimeRoot, "create").mockImplementationOnce( + async (options) => { + const root = await create(options); + path = root.path; + const close = root.close.bind(root); + vi.spyOn(root, "close").mockImplementationOnce(async () => { + cancel.abort(reason); + await close(); + }); + return root; + }, + ); + const { release, paths } = await files({ "a.gpg": VENDOR_A.key }); + const result = withGpgvKeyCheck( + async () => ({ stdout: "", exitCode: 2 }), + cancel.signal, + async (check) => { + const verified = await check.firstVerifying( + [paths["a.gpg"] ?? ""], + [release], + ); + if (completion === "failed") throw primary; + return verified; + }, + ); + await expect(result).rejects.toBe( + completion === "failed" ? primary : reason, + ); + expect(path).toBeDefined(); + await expect(access(path ?? "")).rejects.toMatchObject({ + code: "ENOENT", + }); + }, + ); + it("keeps the private keyring folder's owner when its removal fails", async () => { const create = PrivateRuntimeRoot.create.bind(PrivateRuntimeRoot); let owned: PrivateRuntimeRoot | undefined; diff --git a/src/desktop/GpgvKeyCheck.ts b/src/desktop/GpgvKeyCheck.ts index e234703caf..a813344308 100644 --- a/src/desktop/GpgvKeyCheck.ts +++ b/src/desktop/GpgvKeyCheck.ts @@ -174,5 +174,6 @@ export const withGpgvKeyCheck = async ( throw resources.cleanupFailure(outcome, [removed]); } if (outcome.kind === "failed") throw outcome.cause; + signal?.throwIfAborted(); return outcome.value; }; diff --git a/src/desktop/GuestFileAnswer.test.ts b/src/desktop/GuestFileAnswer.test.ts index b9aafdf3a9..c381163eb0 100644 --- a/src/desktop/GuestFileAnswer.test.ts +++ b/src/desktop/GuestFileAnswer.test.ts @@ -1,18 +1,54 @@ import { describe, expect, it } from "vitest"; import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js"; +import { readHomeFiles } from "./ComputerHomeFiles.js"; import { AgentEnvComputer, GUEST_FILE_ANSWER_COPIES, GUEST_FILE_ANSWER_SOURCE, guestFileAnswerBytes, - guestFileAnswerCeiling, guestFileAnswerRefusal, guestFileAnswerWorkingBytes, retainGuestFiles, } from "./AgentEnvComputer.js"; describe("guest file answers", () => { + it.each([true, false])( + "uses only the named data-object metadata for file identity (present: %s)", + async (present) => { + const files = await readHomeFiles( + async () => ({ + result: { + parts: [ + { + kind: "file", + file: { bytes: Buffer.from("source bytes").toString("base64") }, + metadata: { + "urn:unrelated": { path: "other.txt" }, + ...(present + ? { + "urn:agentenv:data-objects/v1": { + path: "selected.txt", + }, + } + : {}), + }, + }, + ], + }, + }), + (text) => text, + ["selected.txt"], + ); + expect(files).toEqual([ + { + path: present ? "selected.txt" : undefined, + bytes: Buffer.from("source bytes"), + }, + ]); + }, + ); + it("counts base64, the envelope, and the copies held while the answer is parsed", () => { expect(guestFileAnswerBytes(3, 1)).toBeGreaterThan(3); expect(guestFileAnswerBytes(3, 2)).toBeGreaterThan( diff --git a/src/desktop/LibraryPackageInventory.test.ts b/src/desktop/LibraryPackageInventory.test.ts index 4d5313bc68..5f72a72202 100644 --- a/src/desktop/LibraryPackageInventory.test.ts +++ b/src/desktop/LibraryPackageInventory.test.ts @@ -147,7 +147,7 @@ it.skipIf(process.platform === "win32")( const bin = join(root, "bin"); const lib = join(root, "lib"); await Promise.all([mkdir(bin), mkdir(lib)]); - const paths = Array.from({ length: 102 }, (_, index) => + const paths = Array.from({ length: 201 }, (_, index) => join(lib, `lib${index}.so`), ); const executable = join(bin, "app"); @@ -170,7 +170,7 @@ it.skipIf(process.platform === "win32")( expect(batches).toBe(2); expect(result.library_packages).toEqual(["owner"]); expect(result.unpackaged_libraries).toContain(paths[1]); - expect(result.unpackaged_libraries).not.toContain(paths[101]); + expect(result.unpackaged_libraries).not.toContain(paths[200]); expect(result.library_packages_reason).toContain("dpkg-query"); }, ); diff --git a/src/desktop/LinuxApplicationInventory.test.ts b/src/desktop/LinuxApplicationInventory.test.ts index 9e6725e0f8..6d549174ca 100644 --- a/src/desktop/LinuxApplicationInventory.test.ts +++ b/src/desktop/LinuxApplicationInventory.test.ts @@ -1,10 +1,20 @@ -import { chmod, mkdir, stat, symlink, writeFile } from "node:fs/promises"; +import { + chmod, + mkdir, + realpath, + stat, + symlink, + writeFile, +} from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { inspectLinuxApplication } from "./LinuxApplicationInventory.js"; +import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; +import { ArtifactFileCleanupFailure } from "../artifacts/ArtifactFileOperation.js"; +import { desktopOperationFailure } from "../application/desktop/DesktopCaptureRefusals.js"; const fixture = async () => { const root = await createTestTempDirectory("desktop-inventory-"); @@ -37,6 +47,58 @@ const fixture = async () => { }; }; +describe.skipIf(process.platform === "win32")( + "explicit executable traversal", + () => { + it("refuses a directory symlink as an executable while accepting a file symlink", async () => { + const { home, bin, environment } = await fixture(); + const directoryAlias = join(bin, "directory-alias"); + const fileAlias = join(bin, "file-alias"); + await symlink(home, directoryAlias); + await symlink(join(bin, "notes-app"), fileAlias); + const inspect = (executable: string) => + inspectLinuxApplication({ + desktopEntry: undefined, + executable, + arguments: [], + environment, + home, + callerDirectory: home, + }); + expect(await inspect(fileAlias)).toMatchObject({ ok: true }); + expect(await inspect(directoryAlias)).toEqual({ + ok: false, + error: { reason: "executable_not_found", program: directoryAlias }, + }); + }); + + it("preserves symlink and parent traversal for the selected executable", async () => { + const { home, environment } = await fixture(); + const physical = join(home, "physical"); + const named = join(home, "named"); + await mkdir(join(physical, "child"), { recursive: true }); + await mkdir(named); + await symlink(join(physical, "child"), join(named, "link")); + await writeFile(join(physical, "app"), "#!/bin/sh\n", { mode: 0o755 }); + await writeFile(join(named, "app"), "#!/bin/sh\n", { mode: 0o755 }); + const selected = `${named}/link/../app`; + const result = await inspectLinuxApplication({ + desktopEntry: undefined, + executable: selected, + arguments: [], + environment, + home, + callerDirectory: home, + }); + if (!result.ok) throw new Error(result.error.reason); + expect(result.value.inventory.argv[0]).toBe(selected); + expect(result.value.inventory.executable.resolved_path).toBe( + await realpath(join(physical, "app")), + ); + }); + }, +); + describe.skipIf(process.platform === "win32")("inspectLinuxApplication", () => { it("resolves a desktop entry id into a launch command and state directories", async () => { const { home, bin, environment } = await fixture(); @@ -726,3 +788,129 @@ describe.skipIf(process.platform === "win32")( }); }, ); + +it.skipIf(process.platform === "win32").each([ + { stage: "desktop entry", closeAt: 1, oversized: false, cancel: false }, + { + stage: "desktop entry capacity failure", + closeAt: 1, + oversized: true, + cancel: false, + }, + { stage: "executable header", closeAt: 2, oversized: false, cancel: false }, + { + stage: "optional Electron probe", + closeAt: 3, + oversized: false, + cancel: false, + }, + { + stage: "desktop entry final cancellation", + closeAt: 1, + oversized: false, + cancel: true, + }, + { + stage: "capacity failure before final cancellation", + closeAt: 1, + oversized: true, + cancel: true, + }, +])( + "retains and retries a failed close during $stage", + async ({ closeAt, oversized, cancel }) => { + const { home, bin, environment } = await fixture(); + const entry = join( + environment.XDG_DATA_DIRS, + "applications/org.example.notes.desktop", + ); + if (oversized) await writeFile(entry, "x".repeat(1024 * 1024 + 1)); + const controller = new AbortController(); + const reason = new Error("cancelled during close"); + const denied = new Error("descriptor close denied"); + const originalClose = OwnedFileHandle.prototype.close; + let closes = 0; + let retained: OwnedFileHandle | undefined; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(function (this: OwnedFileHandle) { + closes += 1; + if (closes === closeAt || this === retained) { + // oxlint-disable-next-line typescript/no-this-alias -- Retain the exact failed descriptor owner for retry. + retained = this; + if (cancel) controller.abort(reason); + return Promise.reject(denied); + } + return originalClose.call(this); + }); + try { + const failure = await inspectLinuxApplication( + { + desktopEntry: "org.example.notes", + executable: undefined, + arguments: [], + environment, + home, + callerDirectory: home, + }, + controller.signal, + { + runQuery: async () => "missing", + apt: { + trusted: join(home, "trusted"), + trustedParts: join(home, "trusted.d"), + lists: join(home, "lists"), + }, + }, + ).catch((cause: unknown) => cause); + expect(retained).toBeDefined(); + if (!(failure instanceof ArtifactFileCleanupFailure)) + throw new Error("expected transferable descriptor cleanup", { + cause: failure, + }); + const expectedPath = + closeAt === 1 ? entry : await realpath(join(bin, "notes-app")); + expect(failure.cleanup?.resources).toEqual([expectedPath]); + expect(failure.fileCleanupFailure.owner).toBe(retained); + expect(failure.fileCleanupFailure.cleanupCause).toBe(denied); + const outcome = failure.fileCleanupFailure.outcome; + if (oversized) { + expect(outcome).toMatchObject({ + kind: "failed", + cause: { + maximumBytes: 1024 * 1024, + observedBytes: BigInt(1024 * 1024 + 1), + }, + }); + } else if (closeAt === 3) { + expect(outcome).toMatchObject({ + kind: "failed", + cause: { kind: "unsupported" }, + }); + } else { + expect(outcome.kind).toBe("completed"); + } + if (cancel && !oversized) + expect(failure.cancellation?.reason).toBe(reason); + else expect(failure.reason).not.toBe("cancelled"); + const projected = desktopOperationFailure( + failure, + controller.signal, + "capture_desktop_application", + "Desktop capture", + ); + expect(projected.cleanupIncomplete).toBe(true); + expect(projected.cleanupResources).toEqual([expectedPath]); + await expect(failure.cleanupLease.release()).rejects.toBe(denied); + expect(projected.cleanupIncomplete).toBe(true); + close.mockRestore(); + await failure.cleanupLease.release(); + expect(projected.cleanupIncomplete).toBe(false); + expect(projected.cleanupResources).toEqual([]); + expect(retained?.handle.fd).toBe(-1); + } finally { + close.mockRestore(); + await retained?.close(); + } + }, +); diff --git a/src/desktop/LinuxApplicationInventory.ts b/src/desktop/LinuxApplicationInventory.ts index dd388294de..94b23fd234 100644 --- a/src/desktop/LinuxApplicationInventory.ts +++ b/src/desktop/LinuxApplicationInventory.ts @@ -3,7 +3,7 @@ import { type DesktopLibraryPolicyInput, } from "../domain/desktop/libraryPolicy.js"; import { lstat, realpath, stat } from "node:fs/promises"; -import { basename, dirname, isAbsolute, join, resolve } from "node:path"; +import { basename, dirname, isAbsolute, join } from "node:path"; import type { DesktopApplicationInventory } from "../domain/desktop/desktopCapture.js"; import { @@ -19,7 +19,12 @@ import { } from "../domain/desktop/linuxPackages.js"; import { err, ok, type Result } from "../domain/result.js"; import { readElectronFuseImage } from "../electron/ElectronFuseReader.js"; -import { withOwnedRegularFile } from "../filesystem/OwnedRegularFile.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { + RegularFileCleanupFailure, + withOwnedRegularFile, +} from "../filesystem/OwnedRegularFile.js"; +import { absoluteCallerPath } from "../filesystem/callerPath.js"; import { RegularFileCapacityError, readStableFileBytes, @@ -33,7 +38,7 @@ import { type DebianPackageHost, type InventoryContext, } from "./DebianPackageInventory.js"; -import { isRefusedRead } from "./HostFileRead.js"; +import { hostCloseFailure, isRefusedRead } from "./HostFileRead.js"; import { runHostQuery } from "./HostQuery.js"; import { resolveProgram } from "./ProgramLookup.js"; @@ -230,6 +235,8 @@ const readDesktopEntry = async ( ) ).toString("utf8"); } catch (cause: unknown) { + if (cause instanceof RegularFileCleanupFailure) + throw hostCloseFailure(cause); signal?.throwIfAborted(); if (!isRefusedRead(cause)) throw cause; return err({ @@ -267,6 +274,8 @@ const readHeader = async ( (await readStableFilePrefix(handle, path, 16, options)).bytes, ); } catch (cause: unknown) { + if (cause instanceof RegularFileCleanupFailure) + throw hostCloseFailure(cause); signal?.throwIfAborted(); if (!isRefusedRead(cause)) throw cause; return new Uint8Array(); @@ -328,7 +337,9 @@ const electronMarkers = async ( ); if (image.slices.some((slice) => slice.sentinel_count > 0)) markers.push("fuse-sentinel"); - } catch { + } catch (cause: unknown) { + if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) + throw cause; signal?.throwIfAborted(); // Scripts and other non-binary launchers carry no fuse wire. } @@ -358,7 +369,7 @@ const launchCommand = async ( return ok({ argv: [ executable.includes("/") - ? resolve(request.callerDirectory, executable) + ? absoluteCallerPath(executable, { cwd: request.callerDirectory }) : executable, ...request.arguments, ], diff --git a/src/desktop/ProgramLookup.ts b/src/desktop/ProgramLookup.ts index acd4d53f23..b8fbe7c09f 100644 --- a/src/desktop/ProgramLookup.ts +++ b/src/desktop/ProgramLookup.ts @@ -1,11 +1,11 @@ import { constants } from "node:fs"; -import { access, lstat } from "node:fs/promises"; +import { access, stat } from "node:fs/promises"; import { delimiter, isAbsolute } from "node:path"; const isExecutable = async (path: string): Promise => { try { await access(path, constants.X_OK); - return (await lstat(path)).isDirectory() === false; + return (await stat(path)).isFile(); } catch { return false; } diff --git a/src/domain/desktop/appExportPython.ts b/src/domain/desktop/appExportPython.ts index fe5d134a8e..004baa4542 100644 --- a/src/domain/desktop/appExportPython.ts +++ b/src/domain/desktop/appExportPython.ts @@ -218,9 +218,9 @@ def _cell(value, encoding: str): return _sqlite_text(data, encoding) if value[0] == "t" else {"blob_hex": data.hex()} if isinstance(value, int) and abs(value) > MAX_SAFE_INTEGER: return {"integer": str(value)} - # JSON has no infinities or negative zero; REA's SQLite reader spells them as strings. + # Keep JSON's missing REAL values distinct from literal TEXT spellings. if isinstance(value, float) and (math.isinf(value) or (value == 0 and math.copysign(1, value) < 0)): - return "Infinity" if value == math.inf else "-Infinity" if value == -math.inf else "-0" + return {"real": "Infinity" if value == math.inf else "-Infinity" if value == -math.inf else "-0"} return value diff --git a/src/domain/desktop/appWithheld.test.ts b/src/domain/desktop/appWithheld.test.ts index 41f5eba530..383aea979e 100644 --- a/src/domain/desktop/appWithheld.test.ts +++ b/src/domain/desktop/appWithheld.test.ts @@ -118,7 +118,9 @@ describe("withheld files", () => { expect(gapped(4, { truncated: true })).toMatchObject({ ok: true }); expect(gapped(1, { unconfirmed_changes: 1 })).toMatchObject({ ok: true }); }); +}); +describe("withheld file settings", () => { it("names each withheld file's setting, environment variable and handle", () => { expect(withheldSettings("typora", [".config/Typora/mAxvdiPNmi"])).toEqual([ { @@ -140,8 +142,27 @@ describe("withheld files", () => { expect(other?.variable).not.toBe(again?.variable); }); - it("names settings Python takes as arguments, distinct whatever the files' order", () => { - const files = [".pass", "1license", "file_1license", "class"]; + it("names settings Python and Pydantic accept, distinct whatever the files' order", () => { + const files = [ + ".pass", + "1license", + "file_1license", + "class", + "model_config", + "model_extra", + "model_fields_set", + "file_model_extra", + "model_post_init", + "model_dump", + "model_dump_json", + "model_validate", + "model_validate_json", + "model_validate_strings", + "file_model_config", + "model_id", + "model_copy", + "model_fields", + ]; for (const order of [files, [...files].reverse()]) { const settings = withheldSettings("an-app", order).map( ({ setting }) => setting, @@ -150,7 +171,21 @@ describe("withheld files", () => { for (const setting of settings) { expect(setting).toMatch(/^[a-z]\w*$/u); expect(["pass", "class"]).not.toContain(setting); + expect([ + "model_config", + "model_extra", + "model_fields_set", + "model_post_init", + "model_dump", + "model_dump_json", + "model_validate", + "model_validate_json", + "model_validate_strings", + ]).not.toContain(setting); } + expect(settings[order.indexOf("model_id")]).toBe("model_id"); + expect(settings[order.indexOf("model_copy")]).toBe("model_copy"); + expect(settings[order.indexOf("model_fields")]).toBe("model_fields"); } }); diff --git a/src/domain/desktop/appWithheld.ts b/src/domain/desktop/appWithheld.ts index c75138ba6d..751c823bf5 100644 --- a/src/domain/desktop/appWithheld.ts +++ b/src/domain/desktop/appWithheld.ts @@ -169,10 +169,25 @@ const PYTHON_KEYWORDS = new Set( ), ); +/** Pydantic >=2.12: configuration, its instance initialization hook and protected inherited methods. */ +const PYDANTIC_RESERVED_FIELDS = new Set([ + "model_config", + "model_extra", + "model_fields_set", + "model_post_init", + "model_dump", + "model_dump_json", + "model_validate", + "model_validate_json", + "model_validate_strings", +]); + /** A setting Python takes as an argument's name, distinct from those `settings` holds, which it joins. */ const distinctSetting = (named: string, settings: Set): string => { const valid = - /^[a-z]/u.test(named) && !PYTHON_KEYWORDS.has(named) + /^[a-z]/u.test(named) && + !PYTHON_KEYWORDS.has(named) && + !PYDANTIC_RESERVED_FIELDS.has(named) ? named : `file_${named}`; let setting = valid; diff --git a/src/domain/desktop/desktopCapture.test.ts b/src/domain/desktop/desktopCapture.test.ts index 71adf97858..e6637f58df 100644 --- a/src/domain/desktop/desktopCapture.test.ts +++ b/src/domain/desktop/desktopCapture.test.ts @@ -5,10 +5,25 @@ import { checkpointTaken, desktopApplicationInventorySchema, desktopCaptureEventSchema, + desktopFileFactsSchema, type DesktopCheckpoint, type DesktopStateChange, } from "./desktopCapture.js"; +it.each(["-315619200000000000", "0", "1"])( + "retains filesystem modification timestamp %s without changing its spelling", + (mtime_ns) => { + const value = { + type: "file", + size: 3, + mode: 0o644, + mtime_ns, + sha256: null, + }; + expect(desktopFileFactsSchema.parse(value)).toEqual(value); + }, +); + describe("checkpointNameRefusal", () => { it("accepts an omitted name and a new one", () => { expect(checkpointNameRefusal(undefined, new Set(["saved"]))).toBe( diff --git a/src/domain/desktop/desktopCapture.ts b/src/domain/desktop/desktopCapture.ts index 78572b15b1..951a2af0a0 100644 --- a/src/domain/desktop/desktopCapture.ts +++ b/src/domain/desktop/desktopCapture.ts @@ -11,7 +11,7 @@ export const desktopFileFactsSchema = z.strictObject({ type: z.enum(["file", "directory", "symlink", "other"]), size: z.int().min(0), mode: z.int().min(0), - mtime_ns: z.string().regex(/^\d+$/u), + mtime_ns: z.string().regex(/^-?\d+$/u), sha256: sha256.nullable(), }); export type DesktopFileFacts = z.infer; @@ -271,7 +271,10 @@ export const desktopApplicationInventorySchema = z.strictObject({ state_directories: z.array( z.strictObject({ path: z.string().min(1), - exists_at_start: z.boolean(), + /** Null when the filesystem probe could not establish presence or absence. */ + exists_at_start: z.boolean().nullable(), + /** The failed existence probe's reason; absent for confirmed presence or absence. */ + existence_error: z.string().optional(), }), ), }); diff --git a/src/domain/desktop/desktopEntry.ts b/src/domain/desktop/desktopEntry.ts index 6b22e28809..271fce17f9 100644 --- a/src/domain/desktop/desktopEntry.ts +++ b/src/domain/desktop/desktopEntry.ts @@ -22,7 +22,6 @@ const VALUE_ESCAPES: Readonly> = { t: "\t", r: "\r", "\\": "\\", - ";": "\\;", }; /** Undo the escapes the desktop entry format allows in string values. */ @@ -69,13 +68,25 @@ export const escapeDesktopEntryValue = (value: string): string => { return escaped; }; -const listValue = (value: string | undefined): readonly string[] => - value === undefined - ? [] - : value - .split(/(? part.replaceAll("\\;", ";")) - .filter((part) => part.length > 0); +/** Decode list separators and escapes together, before either loses its meaning. */ +const listValue = (value: string | undefined): readonly string[] => { + if (value === undefined) return []; + const entries: string[] = []; + let entry = ""; + for (let index = 0; index < value.length; index += 1) { + const character = value.charAt(index); + const next = value.charAt(index + 1); + if (character === "\\" && next.length > 0) { + entry += next === ";" ? ";" : (VALUE_ESCAPES[next] ?? `\\${next}`); + index += 1; + } else if (character === ";") { + entries.push(entry); + entry = ""; + } else entry += character; + } + if (entry.length > 0) entries.push(entry); + return entries; +}; /** Read the main group of a freedesktop desktop entry, ignoring localized keys. */ export const parseDesktopEntry = ( @@ -101,13 +112,14 @@ export const parseDesktopEntry = ( // Keep the existing enum/boolean token treatment before value unescaping. values.set( key, - unescapeValue( - key === "Type" || key === "Terminal" ? value.trimEnd() : value, - ), + key === "Type" || key === "Terminal" ? value.trimEnd() : value, ); } if (!found) return err("missing_desktop_entry_group"); - const field = (key: string) => values.get(key) ?? null; + const field = (key: string) => { + const value = values.get(key); + return value === undefined ? null : unescapeValue(value); + }; return ok({ type: field("Type"), name: field("Name"), @@ -118,7 +130,7 @@ export const parseDesktopEntry = ( startup_wm_class: field("StartupWMClass"), categories: listValue(values.get("Categories")), mime_types: listValue(values.get("MimeType")), - terminal: values.get("Terminal") === "true", + terminal: field("Terminal") === "true", }); }; diff --git a/src/domain/desktop/desktopEntryValues.test.ts b/src/domain/desktop/desktopEntryValues.test.ts index d4fafe44a6..e73ba28423 100644 --- a/src/domain/desktop/desktopEntryValues.test.ts +++ b/src/domain/desktop/desktopEntryValues.test.ts @@ -82,3 +82,16 @@ it("does not broaden Type or Terminal recognition by trimming decoded escapes", if (!parsed.ok) throw new Error(parsed.error); expect(parsed.value).toMatchObject({ type: "Application ", terminal: false }); }); + +it.each([ + [String.raw`First\\;Second;`, ["First\\", "Second"]], + [String.raw`First\;Second;`, ["First;Second"]], + [String.raw`First\\\;Second;`, ["First\\;Second"]], + ["First;;", ["First", ""]], + ["First;", ["First"]], + ["", []], +])("decodes string-list escapes and terminators once: %s", (wire, expected) => { + const parsed = parseDesktopEntry(`[Desktop Entry]\nCategories=${wire}\n`); + if (!parsed.ok) throw new Error(parsed.error); + expect(parsed.value.categories).toEqual(expected); +}); diff --git a/src/domain/desktop/linuxPackages.test.ts b/src/domain/desktop/linuxPackages.test.ts index 30043eea2f..f787d20019 100644 --- a/src/domain/desktop/linuxPackages.test.ts +++ b/src/domain/desktop/linuxPackages.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { isAppImageHeader, + ownershipCandidates, packageFromPath, parseAptPolicyOrigins, parseAptPolicyPackageSources, @@ -11,6 +12,42 @@ import { } from "./linuxPackages.js"; describe("dpkg output", () => { + it("adds merged-/usr ownership candidates only for merged system directories", () => { + const paths = [ + "/usr/bin/editor", + "/sbin/helper", + "/usr/lib/x86_64-linux-gnu/libexample.so", + "/lib64/ld-linux-x86-64.so.2", + "/usr/lib32/libexample.so", + "/libx32/libexample.so", + "/usr/libo32/libexample.so", + "/opt/vendor/app", + "/home/ada/bin/app", + "/usr/share/applications/app.desktop", + "/usr/local/bin/app", + ]; + expect(ownershipCandidates(paths)).toEqual([ + "/usr/bin/editor", + "/bin/editor", + "/sbin/helper", + "/usr/sbin/helper", + "/usr/lib/x86_64-linux-gnu/libexample.so", + "/lib/x86_64-linux-gnu/libexample.so", + "/lib64/ld-linux-x86-64.so.2", + "/usr/lib64/ld-linux-x86-64.so.2", + "/usr/lib32/libexample.so", + "/lib32/libexample.so", + "/libx32/libexample.so", + "/usr/libx32/libexample.so", + "/usr/libo32/libexample.so", + "/libo32/libexample.so", + "/opt/vendor/app", + "/home/ada/bin/app", + "/usr/share/applications/app.desktop", + "/usr/local/bin/app", + ]); + }); + it("reads every owner of a path and skips diversions", () => { expect( parseDpkgOwners( diff --git a/src/domain/desktop/linuxPackages.ts b/src/domain/desktop/linuxPackages.ts index 1965c87e04..164f1795d9 100644 --- a/src/domain/desktop/linuxPackages.ts +++ b/src/domain/desktop/linuxPackages.ts @@ -26,14 +26,18 @@ export const parseDpkgOwners = ( return [...owners]; }; -/** `/bin/x` and `/usr/bin/x` name the same file on merged-/usr systems, and dpkg may know either. */ +/** Only merged system directories have alternate `/usr` spellings; application, home and shared-data paths retain their own identity. */ export const ownershipCandidates = ( paths: readonly string[], ): readonly string[] => [ ...new Set( - paths.flatMap((path) => - path.startsWith("/usr/") ? [path, path.slice(4)] : [path, `/usr${path}`], - ), + paths.flatMap((path) => { + if (!/^\/(?:usr\/)?(?:bin|sbin|lib(?:32|64|o32|x32)?)\//u.test(path)) + return [path]; + return path.startsWith("/usr/") + ? [path, path.slice(4)] + : [path, `/usr${path}`]; + }), ), ]; diff --git a/src/filesystem/canonicalCallerPath.ts b/src/filesystem/canonicalCallerPath.ts new file mode 100644 index 0000000000..eb30285567 --- /dev/null +++ b/src/filesystem/canonicalCallerPath.ts @@ -0,0 +1,39 @@ +import { lstat, realpath, stat } from "node:fs/promises"; +import { basename, dirname, join } from "node:path"; +import { absoluteCallerPath } from "./callerPath.js"; + +/** Resolve existing path components without changing the caller's traversal. */ +export const canonicalCallerPath = async (path: string): Promise => { + const remaining: string[] = []; + let current = absoluteCallerPath(path); + for (;;) { + try { + const found = await stat(current); + if (remaining.length > 0 && !found.isDirectory()) + throw Object.assign( + new Error(`Path parent is not a directory: ${current}`), + { code: "ENOTDIR" }, + ); + return join(await realpath(current), ...remaining); + } catch (cause: unknown) { + if ((cause as NodeJS.ErrnoException).code !== "ENOENT") throw cause; + // A dangling symlink is not an absent destination: its target cannot be + // established. A missing component followed by .. is likewise unknown. + const existing = await lstat(current).then( + () => true, + (error: unknown) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw error; + }, + ); + if ( + existing || + basename(current) === ".." || + dirname(current) === current + ) + throw cause; + remaining.unshift(basename(current)); + current = dirname(current); + } + } +}; diff --git a/tests/boundary/providers/desktopExportPython.test.ts b/tests/boundary/providers/desktopExportPython.test.ts index 99131c4fdc..c892821daa 100644 --- a/tests/boundary/providers/desktopExportPython.test.ts +++ b/tests/boundary/providers/desktopExportPython.test.ts @@ -5,6 +5,7 @@ import { } from "../../../src/domain/desktop/desktopViewPolicy.js"; import { execFileSync, + spawnSync, type ExecFileSyncOptionsWithStringEncoding, } from "node:child_process"; import { mkdtempSync, writeFileSync, unlinkSync, rmdirSync } from "node:fs"; @@ -34,6 +35,8 @@ import { textualView, } from "../../../src/domain/desktop/stateFormats.js"; import { safeParseJson } from "../../../src/domain/safeJson.js"; +import { withheldSettings } from "../../../src/domain/desktop/appWithheld.js"; +import { withheldSettingsClass } from "../../../src/domain/desktop/appExportWithheld.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; /** Keep the exported Python helpers out of the host command-line size limit. */ @@ -763,7 +766,7 @@ print(json.dumps({"deep": deep, "peak": peak, "edge": _deeper_than(nested, 501), expect(result.peak).toBeLessThan(1024 * 1024); }); -it("spells infinite and negative-zero SQLite reals as REA's SQLite reader does", () => { +it("keeps special SQLite numbers distinct from text in exported table views", () => { const imports = PYTHON_IMPORTS.split("\n") .filter((line) => !line.includes("agentenv_protocol")) .join("\n"); @@ -772,12 +775,33 @@ it("spells infinite and negative-zero SQLite reals as REA's SQLite reader does", "-I", "-B", "-c", - `${imports}${PYTHON_HELPERS}\nprint(json.dumps([_cell(v, "utf-8") for v in (math.inf, -math.inf, -0.0, 0.0, 1.5)]))`, + `${imports}${PYTHON_HELPERS} +with sqlite3.connect(":memory:") as db: + db.execute("create table cells(value)") + db.executemany("insert into cells values(?)", [(v,) for v in (math.inf, -math.inf, "Infinity", "-Infinity", "-0", 9007199254740993, "9007199254740993")]) + print(json.dumps({"table": _table(db, "cells", [], "utf-8"), "reals": [_cell(v, "utf-8") for v in (-0.0, 0.0, 1.5)]})) +`, ], { encoding: "utf8" }, ); - expect(JSON.parse(cells)).toEqual(["Infinity", "-Infinity", "-0", 0, 1.5]); + expect(JSON.parse(cells)).toEqual({ + table: { + columns: ["value"], + primary_key: [], + rows: [ + [{ real: "Infinity" }], + [{ real: "-Infinity" }], + ["Infinity"], + ["-Infinity"], + ["-0"], + [{ integer: "9007199254740993" }], + ["9007199254740993"], + ], + truncated: false, + }, + reals: [{ real: "-0" }, 0, 1.5], + }); }); it("keeps the same keyfile whitespace as the retained state view", async () => { @@ -804,3 +828,54 @@ print(json.dumps([_text_view("settings.ini", text, len(text.encode()))["view"][" ); expect(JSON.parse(output)).toEqual(inputs.map(keyfileView)); }); + +const hasPydantic2 = + spawnSync( + process.platform === "win32" ? "python" : "python3", + [ + "-I", + "-c", + "import pydantic; assert int(pydantic.__version__.split('.')[0]) >= 2", + ], + { stdio: "ignore", timeout: 10_000 }, + ).status === 0; + +it.skipIf(!hasPydantic2)( + "restores withheld files with Pydantic-reserved names", + async () => { + const home = await createTestTempDirectory("rea-pydantic-withheld-"); + const files = ["model_extra", "model_fields_set", "file_model_extra"]; + const withheld = withheldSettings("fixture", files); + const imports = PYTHON_IMPORTS.split("\n") + .filter((line) => !line.includes("agentenv_protocol")) + .join("\n"); + const program = ` +${imports} +from pydantic import BaseModel, SecretStr, create_model +class ComputerAppEnvironment: + Settings = BaseModel +class Exported: +${withheldSettingsClass(withheld)} +${PYTHON_HELPERS} +settings = Exported.Settings(**{name: "YQ==" for name in json.loads(sys.argv[2])}) +_write_withheld(Path(sys.argv[1]), settings, json.loads(sys.argv[3])) +`; + execPython( + [ + "-B", + "-c", + program, + home, + JSON.stringify(withheld.map(({ setting }) => setting)), + JSON.stringify( + Object.fromEntries( + withheld.map(({ setting, file }) => [setting, file]), + ), + ), + ], + { encoding: "utf8", timeout: 10_000 }, + ); + for (const file of files) + expect(await readFile(join(home, file), "utf8")).toBe("a"); + }, +); diff --git a/tests/boundary/providers/desktopSqliteViewBudget.test.ts b/tests/boundary/providers/desktopSqliteViewBudget.test.ts index b259c579c6..5ae57554b5 100644 --- a/tests/boundary/providers/desktopSqliteViewBudget.test.ts +++ b/tests/boundary/providers/desktopSqliteViewBudget.test.ts @@ -222,7 +222,9 @@ print(json.dumps({"size": size, "peak": peak, "negative_zero": negative_zero})) ), ) as { size: number; peak: number; negative_zero: number }; expect(result.size).toBeGreaterThan(1024); - expect(result.negative_zero).toBe(Buffer.byteLength(JSON.stringify("-0"))); + expect(result.negative_zero).toBe( + Buffer.byteLength(JSON.stringify({ real: "-0" })), + ); // Input exists before tracing. Whole-cell quoting previously made a six-MiB // string merely to reject this one-KiB allowance. expect(result.peak).toBeLessThan(1024 * 1024);