From 546b2a6cf6ffbf4ca693799ec6ae587c23157501 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:49:53 +0800 Subject: [PATCH] fix(evidence): preserve authentication failures and diagnose prior commitments --- CHANGELOG.md | 2 +- .../JavaScriptApplicationEvidenceGraph.ts | 41 +++++++++-------- src/domain/evidence.test.ts | 19 ++++++++ src/domain/evidence.ts | 22 ++++++++- src/domain/evidenceBundle.ts | 2 +- src/domain/evidenceLegacy.fixture.ts | 29 ++++++++++++ .../cli/evidenceBundleCommands.test.ts | 45 +++++++++++++++++++ .../cli/workflowInputIdentities.test.ts | 45 ++++++++++++++++--- .../filesystem/sourceToBundlePaths.test.ts | 6 ++- 9 files changed, 182 insertions(+), 29 deletions(-) create mode 100644 src/domain/evidenceLegacy.fixture.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 790f22535f..77a6eac63e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,7 +78,7 @@ Full contribution record: [4 commits and 1 merged PR](https://github.com/morluto ### ⚠ BREAKING CHANGES -- **Saved Evidence:** Evidence and analysis snapshots now require explicit nullable `relationship_coverage` in their identity commitments. Artifact inventories require scoped coverage and explicit `integrity_contradictions`; obsolete `coverage_status` and prose-based coverage inference are removed. Regenerate old Evidence/snapshots and inventory observations with this release: saved files are not migrated and omitted facts are not filled with guessed defaults. ([db8128d](https://github.com/morluto/rea/commit/db8128dc0d1c4f731f3d19877fa00d25cf17f657), [0405c9d](https://github.com/morluto/rea/commit/0405c9d361de6d904dfa16cc0b6d8350407f2c9b), [a9588be](https://github.com/morluto/rea/commit/a9588be4ebdd20eeda975cb63ffcdb1cd868b340)). +- **Saved Evidence:** Evidence and analysis snapshots now require explicit nullable `relationship_coverage` in their identity commitments. Earlier records whose digest omitted null coverage are incompatible even when the serialized field is explicitly null. Artifact inventories require scoped coverage and explicit `integrity_contradictions`; obsolete `coverage_status` and prose-based coverage inference are removed. Regenerate old Evidence/snapshots and inventory observations with this release: saved files are not migrated and omitted facts are not filled with guessed defaults. ([db8128d](https://github.com/morluto/rea/commit/db8128dc0d1c4f731f3d19877fa00d25cf17f657), [0405c9d](https://github.com/morluto/rea/commit/0405c9d361de6d904dfa16cc0b6d8350407f2c9b), [a9588be](https://github.com/morluto/rea/commit/a9588be4ebdd20eeda975cb63ffcdb1cd868b340)). - **Browser capture results:** Consumers must retain resolved capture limits, event families and omission facts. Set desired budgets through the supported capture options; omitted caller inputs still select defaults, but stored observations must report the resolved values. Admitted content reads queue behind bounded concurrency. ([57a23f0](https://github.com/morluto/rea/commit/57a23f0a2da0fdc3835e203718ded8acea5a34e6)). - **Android version codes:** `inspect_adb_package.version_code` is a decimal string rather than a JSON number. Read it as a string, or use `BigInt` for numeric comparison. ([#3485](https://github.com/morluto/rea/pull/3485), [morluto/rea#3368](https://github.com/morluto/rea/issues/3368), [7666a48](https://github.com/morluto/rea/commit/7666a482b69dc7cabdbac3bbaf89a43e32300192)). Thanks @coygeek, @kaoru0822-kitauji. - **Native UI failures:** Failed or cancelled native UI scenarios now return typed errors with the collected report in `partial_observation`. Handle the error path and read that report instead of expecting a successful result containing failed steps. ([d909919](https://github.com/morluto/rea/commit/d9099190ab3a819bd3a9fd62a8714cef398016d5)). diff --git a/src/application/javascript/JavaScriptApplicationEvidenceGraph.ts b/src/application/javascript/JavaScriptApplicationEvidenceGraph.ts index 00afb1123b..ae982cff7f 100644 --- a/src/application/javascript/JavaScriptApplicationEvidenceGraph.ts +++ b/src/application/javascript/JavaScriptApplicationEvidenceGraph.ts @@ -4,7 +4,8 @@ import { MANAGED_WORKFLOW_PROVIDER, } from "../../domain/investigationProviders.js"; import { isImmutableEvidence, type Evidence } from "../../domain/evidence.js"; -import { safeParseEvidence } from "../../domain/evidenceValidation.js"; +import { readEvidence } from "../../domain/evidenceValidation.js"; +import { EvidenceIntegrityError } from "../../domain/evidenceErrors.js"; import { type AnalysisInputIssue, AnalysisInputError, @@ -42,9 +43,11 @@ export interface ApplicationGraphEvidenceInputFailure { /** Convert typed parser issues into this workflow's caller-facing input error. */ export const applicationGraphEvidenceInputError = ( operation: string, - failure: ApplicationGraphEvidenceInputFailure, -): AnalysisInputError => - new AnalysisInputError(operation, undefined, failure.issues); + failure: ApplicationGraphEvidenceInputFailure | EvidenceIntegrityError, +): AnalysisInputError | EvidenceIntegrityError => + failure instanceof EvidenceIntegrityError + ? failure + : new AnalysisInputError(operation, undefined, failure.issues); class InvalidApplicationGraphEvidence extends Error { constructor(readonly issues: readonly AnalysisInputIssue[]) { @@ -84,10 +87,10 @@ export const parseApplicationGraphEvidence = ( path: readonly (string | number)[] = [], ): Result< ApplicationGraphEvidenceSource, - ApplicationGraphEvidenceInputFailure + ApplicationGraphEvidenceInputFailure | EvidenceIntegrityError > => captureInputValidation(() => { - const evidence = parseEvidenceInput(input, path); + const evidence = parseEvidenceInput(input); const owned = ownedApplicationSources.get(evidence); if (owned !== undefined) return owned; if (evidence.operation === "analyze_javascript_application") { @@ -321,11 +324,12 @@ const requireProvider = ( export const parseNativeApplicationEvidence = ( inputs: readonly unknown[], path: readonly (string | number)[] = [], -): Result => +): Result< + readonly Evidence[], + ApplicationGraphEvidenceInputFailure | EvidenceIntegrityError +> => captureInputValidation(() => { - const parsed = inputs.map((input, index) => - parseEvidenceInput(input, [...path, index]), - ); + const parsed = inputs.map((input) => parseEvidenceInput(input)); const withoutSubject = parsed.findIndex(({ subject }) => subject === null); if (withoutSubject !== -1) throw invalidEvidence( @@ -338,23 +342,24 @@ export const parseNativeApplicationEvidence = ( const captureInputValidation = ( operation: () => Value, -): Result => { +): Result< + Value, + ApplicationGraphEvidenceInputFailure | EvidenceIntegrityError +> => { try { return ok(operation()); } catch (cause: unknown) { if (cause instanceof InvalidApplicationGraphEvidence) return err({ issues: cause.issues }); + if (cause instanceof EvidenceIntegrityError) return err(cause); throw cause; } }; -const parseEvidenceInput = ( - input: unknown, - path: readonly (string | number)[], -): Evidence => { - const parsed = safeParseEvidence(input); - if (parsed.ok) return parsed.value; - throw zodInputFailure(parsed.error, input, path); +const parseEvidenceInput = (input: unknown): Evidence => { + const parsed = readEvidence(input); + if (!parsed.ok) throw parsed.error; + return parsed.value; }; const parseCallerValue = ( diff --git a/src/domain/evidence.test.ts b/src/domain/evidence.test.ts index 12f693e0f7..d3561bc3f5 100644 --- a/src/domain/evidence.test.ts +++ b/src/domain/evidence.test.ts @@ -1,3 +1,4 @@ +import { PRE_66_NULL_COVERAGE_EVIDENCE } from "./evidenceLegacy.fixture.js"; import { fc, it } from "@fast-check/vitest"; import { describe, expect } from "vitest"; @@ -625,3 +626,21 @@ it("borrows a previously sealed envelope only after authenticating all semantic }); expect(() => immutableEvidence(changed)).toThrow(); }); + +it("diagnoses a previous explicit-null coverage commitment without admitting or rewriting its ID", () => { + const historical = structuredClone(PRE_66_NULL_COVERAGE_EVIDENCE); + expect(() => parseEvidence(historical)).toThrow( + /earlier relationship_coverage identity commitment/u, + ); + expect(historical).toEqual(PRE_66_NULL_COVERAGE_EVIDENCE); + const corrupted = { ...historical, normalized_result: { observed: false } }; + expect(() => parseEvidence(corrupted)).toThrow( + /semantic identifier does not match/u, + ); + const current = createEvidence(undefined, historical.provider, { + operation: "inspect", + parameters: {}, + result: { observed: true }, + }); + expect(parseEvidence(current)).toEqual(current); +}); diff --git a/src/domain/evidence.ts b/src/domain/evidence.ts index 5c68da4b67..0e453531a6 100644 --- a/src/domain/evidence.ts +++ b/src/domain/evidence.ts @@ -184,7 +184,9 @@ const authenticatedEvidenceSchema = evidenceSchema.superRefine( context.addIssue({ code: "custom", path: ["evidence_id"], - message: "Evidence semantic identifier does not match its record", + message: usesPre66NullCoverageIdentity(withoutId, evidenceId) + ? "Evidence uses an earlier relationship_coverage identity commitment: explicit null was omitted from that digest. Recreate this observation with the current REA release; re-exporting the old bundle cannot update its identity." + : "Evidence semantic identifier does not match its record", }); }, ); @@ -249,7 +251,9 @@ export interface EvidenceObservation { type EvidenceWithoutId = Omit; -const semanticProjection = (evidence: EvidenceWithoutId): JsonValue => ({ +const semanticProjection = ( + evidence: EvidenceWithoutId, +): Record => ({ subject: evidence.subject === null ? null @@ -278,6 +282,20 @@ const semanticProjection = (evidence: EvidenceWithoutId): JsonValue => ({ const computeEvidenceId = (evidence: EvidenceWithoutId): string => `ev_${digestCanonicalValue(semanticProjection(evidence), "Evidence")}`; +/** Recognize an obsolete commitment for diagnosis only; never admit or rewrite it. */ +const usesPre66NullCoverageIdentity = ( + evidence: EvidenceWithoutId, + evidenceId: string, +): boolean => { + if (evidence.relationship_coverage !== null) return false; + const legacyProjection = semanticProjection(evidence); + delete legacyProjection.relationship_coverage; + return ( + evidenceId === + `ev_${digestCanonicalValue(legacyProjection, "Historical Evidence identity")}` + ); +}; + /** Parse evidence and reject a syntactically valid but tampered semantic ID. */ export const parseEvidence = (input: unknown): Evidence => { const immutable = diff --git a/src/domain/evidenceBundle.ts b/src/domain/evidenceBundle.ts index 9cb5878298..82b7146fdb 100644 --- a/src/domain/evidenceBundle.ts +++ b/src/domain/evidenceBundle.ts @@ -269,7 +269,7 @@ const describeEvidenceBundleFailure = ( cause, message: cause instanceof z.ZodError - ? `Evidence bundle does not match the bundle schema ${describeValidationFailure(cause)}. Supply an unmodified bundle written by export_evidence_bundle or rea evidence-export.` + ? `Evidence bundle does not match the bundle schema ${describeValidationFailure(cause)}. Supply an unmodified bundle produced by this REA identity format. Recreate observations from earlier formats; re-exporting an old bundle does not update its identities.` : `Evidence bundle validation failed: ${describeValidationFailure(cause)}. Recreate or re-export the bundle, then try again.`, }; } diff --git a/src/domain/evidenceLegacy.fixture.ts b/src/domain/evidenceLegacy.fixture.ts new file mode 100644 index 0000000000..df9da34538 --- /dev/null +++ b/src/domain/evidenceLegacy.fixture.ts @@ -0,0 +1,29 @@ +/** ASCII golden for the rea-agents-6.5.0 semanticProjection rule in evidence.ts. + * Its independently computed canonical SHA-256 omits explicit null coverage. + * This is a schema/identity fixture, not a claim of a released-provider run. + */ +export const PRE_66_NULL_COVERAGE_EVIDENCE = { + evidence_id: + "ev_4efa1f7816d7017ce464ad524ccdb987d37b163b248d1ffbb14e8f0179a8f4d3", + subject: null, + provider: { + id: "historical-fixture", + name: "Historical fixture", + version: "1", + }, + analysis_profile: null, + predicate_type: "rea.analysis", + operation: "inspect", + parameters: {}, + raw_result: null, + normalized_result: { + observed: true, + }, + confidence: "observed", + authority: "shipped-artifact", + environment: null, + limitations: [], + relationship_coverage: null, + locations: [], + evidence_links: [], +} as const; diff --git a/tests/boundary/cli/evidenceBundleCommands.test.ts b/tests/boundary/cli/evidenceBundleCommands.test.ts index 3883e7dba9..59e36cdb7a 100644 --- a/tests/boundary/cli/evidenceBundleCommands.test.ts +++ b/tests/boundary/cli/evidenceBundleCommands.test.ts @@ -1,3 +1,5 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { PRE_66_NULL_COVERAGE_EVIDENCE } from "../../../src/domain/evidenceLegacy.fixture.js"; import { readdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -220,3 +222,46 @@ cliTest( expect(markdown.stdout).toContain("left\\|right\\\\tail
next"); }, ); + +cliTest( + "diagnoses an obsolete explicit-null Evidence commitment without claiming the file was modified", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-legacy-coverage-identity-"); + const path = join(root, "legacy.json"); + const record = PRE_66_NULL_COVERAGE_EVIDENCE; + const bundle = { + artifacts: [], + providers: [record.provider], + environments: [], + scenarios: [], + captures: [], + unknowns: [], + records: [record], + }; + await writeFile(path, JSON.stringify(bundle)); + const imported = await cli.run({ + arguments: ["evidence-import", path, "--json"], + }); + expect(imported.exitCode).toBe(1); + expect(imported.json).toMatchObject({ + code: "evidence_integrity_mismatch", + retryable: false, + message: expect.stringContaining( + "earlier relationship_coverage identity commitment", + ), + }); + const call = await cli.sharedMcpCall?.(); + if (call === undefined) throw new Error("Expected shared MCP session"); + const response = await call("import_evidence_bundle", { path }); + expect(response.isError).toBe(true); + expect(parseMcpToolError(response)).toMatchObject({ + error: { + code: "evidence_integrity_mismatch", + retryable: false, + message: expect.stringContaining( + "earlier relationship_coverage identity commitment", + ), + }, + }); + }, +); diff --git a/tests/boundary/cli/workflowInputIdentities.test.ts b/tests/boundary/cli/workflowInputIdentities.test.ts index c59849d639..6ad72d5cc3 100644 --- a/tests/boundary/cli/workflowInputIdentities.test.ts +++ b/tests/boundary/cli/workflowInputIdentities.test.ts @@ -209,35 +209,68 @@ const checkNativeObservationIdentities = async ({ const checkMalformedEvidenceBeforeIdentities = async ({ cli, root, + call, application, }: WorkflowContext) => { const malformed = { ...application, provider: { ...application.provider, name: 3 }, }; - for (const [command, input, path] of [ + const tampered = { ...application, confidence: "observed" }; + for (const [name, command, input, reason] of [ [ + "compare_application_versions", "compare-application-versions", { left: application, right: malformed }, - ["right", "provider", "name"], + "provider.name", ], [ + "compare_javascript_export_shapes", "compare-javascript-export-shapes", { left: application, right: malformed, ...selectors }, - ["right", "provider", "name"], + "provider.name", ], [ + "compare_javascript_export_shapes", + "compare-javascript-export-shapes", + { left: application, right: tampered, ...selectors }, + "semantic identifier", + ], + [ + "trace_application_feature", "trace-application-feature", { application, seed, native_observations: [malformed, application] }, - ["native_observations", 0, "provider", "name"], + "provider.name", + ], + [ + "compare_application_versions", + "compare-application-versions", + { left: application, right: tampered }, + "semantic identifier", + ], + [ + "trace_application_feature", + "trace-application-feature", + { application: tampered, seed }, + "semantic identifier", ], ] as const) { const rejected = await runJson(cli, root, command, input); expect(rejected.exitCode).toBe(1); expect(rejected.json).toMatchObject({ - code: "invalid_request", - details: { issues: [{ path, reason: "invalid_type" }] }, + code: "evidence_integrity_mismatch", + retryable: false, + details: { reason: expect.stringContaining(reason) }, }); + if (reason === "semantic identifier") { + const mcp = await call(name, input); + expect(mcp.isError).toBe(true); + expect(parseMcpToolError(mcp).error).toMatchObject({ + code: "evidence_integrity_mismatch", + retryable: false, + details: { reason: expect.stringContaining(reason) }, + }); + } } const missingLeft = { kind: "retained-evidence", diff --git a/tests/boundary/filesystem/sourceToBundlePaths.test.ts b/tests/boundary/filesystem/sourceToBundlePaths.test.ts index cc00f2b148..85181a7b79 100644 --- a/tests/boundary/filesystem/sourceToBundlePaths.test.ts +++ b/tests/boundary/filesystem/sourceToBundlePaths.test.ts @@ -1,3 +1,4 @@ +import { EvidenceIntegrityError } from "../../../src/domain/evidenceErrors.js"; import { resolveSourceToBundleReference } from "../../../src/application/javascript/SourceToBundleReference.js"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { mkdir, writeFile } from "node:fs/promises"; @@ -711,7 +712,9 @@ it("rejects static application Evidence whose subject path disagrees with its re const parsed = parseApplicationGraphEvidence(inconsistent); expect(parsed.ok).toBe(false); - if (!parsed.ok) + if (!parsed.ok) { + expect(parsed.error).not.toBeInstanceOf(EvidenceIntegrityError); + if (parsed.error instanceof EvidenceIntegrityError) throw parsed.error; expect(parsed.error.issues).toContainEqual( expect.objectContaining({ path: ["subject"], @@ -719,6 +722,7 @@ it("rejects static application Evidence whose subject path disagrees with its re message: expect.stringContaining("subject must match"), }), ); + } }); const compareTrees = async (previous: string, current: string) => {