morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #7076] [MERGED] fix(artifacts): preserve integrity contradictions for one-sided members #7624

New issue

Closed

opened 2026-11-03 13:39:11 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:11 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7076
Author: @WenyuChiou
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/25/2026
Merged by: @morluto

Base: main ← Head: fix/one-sided-artifact-integrity-20261025

📝 Commits (1)

8e963d9 fix(artifacts): preserve integrity contradictions for one-sided members

📊 Changes

2 files changed (+60 additions, -15 deletions)

View changed files

📝 src/domain/artifactComparison.ts (+9 -5)
📝 tests/boundary/mcp/artifactMcp.test.ts (+51 -10)

📄 Description

Summary

Preserve an explicit ASAR member integrity mismatch when that member is present on only one side of compare_artifacts. A contradictory input must not become ordinary added/removed evidence that can support a static/runtime hypothesis.

This is a newly reproduced comparator defect, not a duplicate of the already-fixed correlation issue #6217. Related PR #6307 currently adds test-only coverage for shared-path contradictions; this change fixes the earlier classification of one-sided members. Both extend the existing ASAR boundary test, so that test may need reconciliation if the other PR lands first.

Problem and expected behavior

Pack an ASAR containing main.js, then change its payload from console.log('ok'); to equal-length console.log('no'); without changing its declared integrity. Inspect it with integrity_policy: "record-and-continue" and compare it with an archive that lacks main.js.

Before this fix, presence classification returned added/removed before checking the known hash mismatch. The comparison retained integrity_mismatch_evidence_ids, but its selected member was not classified as contradictory; correlate_static_and_runtime could therefore return a supporting hypothesis for those contradictory bytes. The expected contract already states that contradictory source findings cannot support hypotheses (docs/mcp-contracts.md).

Compared member	Before	After	Downstream after
Absent → mismatched	`added`, contradictions 0	`contradiction`, contradictions 1	`unknown`, hypotheses 0
Mismatched → absent	`removed`, contradictions 0	`contradiction`, contradictions 1	`unknown`, hypotheses 0
Absent → valid	`added`	`added`	`correlated`, hypotheses 1
Valid → absent	`removed`	`removed`	`correlated`, hypotheses 1

The downstream controls select unchanged terminal output and the caller's static_only expected pattern. This is analytical correctness, not an authentication or security-bypass claim.

Change and scope

Check the existing integrity classifier before presence classification, sharing the same change base. Extend the existing real-ASAR MCP regression with corrupt and valid one-sided pairs in both directions; keep its four shared-path pairs.

Coverage/truncation precedence, ambiguous duplicate-path handling, root-hash policy, schemas, and correlation policy are unchanged. The fix does not turn ordinary additions/removals into contradictions: an explicit declared/observed mismatch is required.

Contract and boundary impact

Semantic owner and earliest changed stage: src/domain/artifactComparison.ts, selecting per-path comparison findings before downstream correlation consumes them.
Contribution scope fit and linked maintainer decision: Ordinary correctness fix inside existing offline artifact inspection/comparison; new-capability scope decision not applicable.
CLI and MCP/tool-catalog contract: No schema/catalog changes. Corrects existing compare_artifacts MCP results. The CLI exposes artifact inspection, not a one-shot compare-artifacts command.
Provider, bridge, target-format, or platform compatibility: Real ASAR archives and the production ArtifactProvider on Linux; no new provider or platform capability.
Evidence, artifact, provenance, or reconstruction contract: Preserve present-side occurrence/artifact IDs, null absent-side IDs, actual mismatch source IDs, residual-unknown attribution, and downstream refusal of hypothesis support.
Target selection, authority, upstream behavior, and cleanup: Owned temporary archives only; static inspection, no target execution/attachment/injection. Existing test-session cleanup retained.
Generated metadata, package, or installation impact: None; no dependency, lockfile, generated-output, or catalog changes.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

On unmodified main 8e6a95788688439baeeec8e54c9b3e845226f79d, an exploratory four-case real-ASAR MCP test produced two failures (corrupt added/removed) and two passing valid controls; both wrong downstream hypotheses were observed.
The final compact regression also failed on that base at the new one-sided contradiction assertion. After the production fix, all 49 selected tests passed.
Eight real-ASAR pair cases verify classification, contradiction count, identity, mismatch attribution, residual unknowns, and correlation outcomes. Existing shared-path cases remain covered.
A separate compiled stdio MCP check used production artifact inspections, imported canonical retained process-capture fixtures, and advertised tool schemas: all four one-sided cases passed with the outputs above. Three production CLI inspect-artifact calls passed. No target was executed.
Boundary coverage follows evidence identity and consumer classification through inspection → comparison → retained Evidence → correlation. Lifecycle behavior was not changed; no new cancellation or native-process coverage is claimed.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Linux VM, Node 24.22.1, exact hash-verified superproject source at 8e6a95788688439baeeec8e54c9b3e845226f79d, plus this two-file patch. Submodule commit IDs were verified; submodule contents were not initialized. Checks were serial and bounded.

node scripts/run-exact-tests.mjs run --maxWorkers 1 tests/boundary/mcp/artifactMcp.test.ts tests/boundary/filesystem/artifactComparison.test.ts src/domain/staticRuntimeCorrelation.test.ts — 49 passed, 3 files.
node scripts/run-typescript.mjs --noEmit --singleThreaded --checkers 1 --maxMemory 1G — passed.
node scripts/ci/static-check-report.mjs typed-lint src/domain/artifactComparison.ts tests/boundary/mcp/artifactMcp.test.ts — 0 errors, 6 warnings.
node node_modules/oxfmt/bin/oxfmt --check src/domain/artifactComparison.ts tests/boundary/mcp/artifactMcp.test.ts — passed.
Production TypeScript build (-p tsconfig.build.json, single-threaded), catalog digest/MCP catalog recording, and node scripts/bundle-runtime.mjs — passed; existing ineffective-dynamic-import warnings remain.
Compiled CLI inspection and stdio MCP acceptance described above — passed. Initial private probe attempts used a nonexistent CLI comparison command and then omitted transitive retained Evidence; those attempts were not counted as passes. The corrected probe uses advertised MCP tools and session-produced artifact Evidence.
All 6,272 tracked blob paths and modes rechecked after validation: exactly the intended two files changed; no generated outputs included. Added patch lines have no trailing whitespace.

Not run: complete test suite, aggregate npm run check, real native engines, live process capture, other operating systems. Process comparison uses canonical recorded fixtures, not a claim of real runtime capture. Sampled process-family RSS is only a lower bound, not an independently established total peak.

Compatibility, safety, and release

Breaking changes or migration steps: None; restores explicit integrity semantics without changing schemas.
Real Hopper/Ghidra, browser, or OS coverage: None required or claimed; Linux static artifact workflow only.
Package or release metadata impact: None.
Security, privacy, process, or containment review: No external target artifacts, credentials, generated archives, local caches, binaries, or built outputs committed. No change to execution authority.

Author readiness

Human owner: @WenyuChiou

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/7076 **Author:** [@WenyuChiou](https://github.com/WenyuChiou) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/25/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/one-sided-artifact-integrity-20261025` --- ### 📝 Commits (1) - [`8e963d9`](https://github.com/morluto/rea/commit/8e963d913f052eb19d6c449e67e8869856e8227f) fix(artifacts): preserve integrity contradictions for one-sided members ### 📊 Changes **2 files changed** (+60 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/artifactComparison.ts` (+9 -5) 📝 `tests/boundary/mcp/artifactMcp.test.ts` (+51 -10) </details> ### 📄 Description ## Summary Preserve an explicit ASAR member integrity mismatch when that member is present on only one side of `compare_artifacts`. A contradictory input must not become ordinary added/removed evidence that can support a static/runtime hypothesis. This is a newly reproduced comparator defect, not a duplicate of the already-fixed correlation issue #6217. Related PR #6307 currently adds test-only coverage for shared-path contradictions; this change fixes the earlier classification of one-sided members. Both extend the existing ASAR boundary test, so that test may need reconciliation if the other PR lands first. ## Problem and expected behavior Pack an ASAR containing `main.js`, then change its payload from `console.log('ok');` to equal-length `console.log('no');` without changing its declared integrity. Inspect it with `integrity_policy: "record-and-continue"` and compare it with an archive that lacks `main.js`. Before this fix, presence classification returned `added`/`removed` before checking the known hash mismatch. The comparison retained `integrity_mismatch_evidence_ids`, but its selected member was not classified as contradictory; `correlate_static_and_runtime` could therefore return a supporting hypothesis for those contradictory bytes. The expected contract already states that contradictory source findings cannot support hypotheses (`docs/mcp-contracts.md`). | Compared member | Before | After | Downstream after | | --- | --- | --- | --- | | Absent → mismatched | `added`, contradictions 0 | `contradiction`, contradictions 1 | `unknown`, hypotheses 0 | | Mismatched → absent | `removed`, contradictions 0 | `contradiction`, contradictions 1 | `unknown`, hypotheses 0 | | Absent → valid | `added` | `added` | `correlated`, hypotheses 1 | | Valid → absent | `removed` | `removed` | `correlated`, hypotheses 1 | The downstream controls select unchanged terminal output and the caller's `static_only` expected pattern. This is analytical correctness, not an authentication or security-bypass claim. ## Change and scope Check the existing integrity classifier before presence classification, sharing the same change base. Extend the existing real-ASAR MCP regression with corrupt and valid one-sided pairs in both directions; keep its four shared-path pairs. Coverage/truncation precedence, ambiguous duplicate-path handling, root-hash policy, schemas, and correlation policy are unchanged. The fix does not turn ordinary additions/removals into contradictions: an explicit declared/observed mismatch is required. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/domain/artifactComparison.ts`, selecting per-path comparison findings before downstream correlation consumes them. - Contribution scope fit and linked maintainer decision: Ordinary correctness fix inside existing offline artifact inspection/comparison; new-capability scope decision not applicable. - CLI and MCP/tool-catalog contract: No schema/catalog changes. Corrects existing `compare_artifacts` MCP results. The CLI exposes artifact inspection, not a one-shot `compare-artifacts` command. - Provider, bridge, target-format, or platform compatibility: Real ASAR archives and the production ArtifactProvider on Linux; no new provider or platform capability. - Evidence, artifact, provenance, or reconstruction contract: Preserve present-side occurrence/artifact IDs, null absent-side IDs, actual mismatch source IDs, residual-unknown attribution, and downstream refusal of hypothesis support. - Target selection, authority, upstream behavior, and cleanup: Owned temporary archives only; static inspection, no target execution/attachment/injection. Existing test-session cleanup retained. - Generated metadata, package, or installation impact: None; no dependency, lockfile, generated-output, or catalog changes. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - On unmodified main `8e6a95788688439baeeec8e54c9b3e845226f79d`, an exploratory four-case real-ASAR MCP test produced two failures (corrupt added/removed) and two passing valid controls; both wrong downstream hypotheses were observed. - The final compact regression also failed on that base at the new one-sided contradiction assertion. After the production fix, all 49 selected tests passed. - Eight real-ASAR pair cases verify classification, contradiction count, identity, mismatch attribution, residual unknowns, and correlation outcomes. Existing shared-path cases remain covered. - A separate compiled stdio MCP check used production artifact inspections, imported canonical retained process-capture fixtures, and advertised tool schemas: all four one-sided cases passed with the outputs above. Three production CLI `inspect-artifact` calls passed. No target was executed. - Boundary coverage follows evidence identity and consumer classification through inspection → comparison → retained Evidence → correlation. Lifecycle behavior was not changed; no new cancellation or native-process coverage is claimed. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Linux VM, Node 24.22.1, exact hash-verified superproject source at `8e6a95788688439baeeec8e54c9b3e845226f79d`, plus this two-file patch. Submodule commit IDs were verified; submodule contents were not initialized. Checks were serial and bounded. - `node scripts/run-exact-tests.mjs run --maxWorkers 1 tests/boundary/mcp/artifactMcp.test.ts tests/boundary/filesystem/artifactComparison.test.ts src/domain/staticRuntimeCorrelation.test.ts` — **49 passed**, 3 files. - `node scripts/run-typescript.mjs --noEmit --singleThreaded --checkers 1 --maxMemory 1G` — passed. - `node scripts/ci/static-check-report.mjs typed-lint src/domain/artifactComparison.ts tests/boundary/mcp/artifactMcp.test.ts` — **0 errors, 6 warnings**. - `node node_modules/oxfmt/bin/oxfmt --check src/domain/artifactComparison.ts tests/boundary/mcp/artifactMcp.test.ts` — passed. - Production TypeScript build (`-p tsconfig.build.json`, single-threaded), catalog digest/MCP catalog recording, and `node scripts/bundle-runtime.mjs` — passed; existing ineffective-dynamic-import warnings remain. - Compiled CLI inspection and stdio MCP acceptance described above — passed. Initial private probe attempts used a nonexistent CLI comparison command and then omitted transitive retained Evidence; those attempts were not counted as passes. The corrected probe uses advertised MCP tools and session-produced artifact Evidence. - All 6,272 tracked blob paths and modes rechecked after validation: exactly the intended two files changed; no generated outputs included. Added patch lines have no trailing whitespace. Not run: complete test suite, aggregate `npm run check`, real native engines, live process capture, other operating systems. Process comparison uses canonical recorded fixtures, not a claim of real runtime capture. Sampled process-family RSS is only a lower bound, not an independently established total peak. ## Compatibility, safety, and release - Breaking changes or migration steps: None; restores explicit integrity semantics without changing schemas. - Real Hopper/Ghidra, browser, or OS coverage: None required or claimed; Linux static artifact workflow only. - Package or release metadata impact: None. - Security, privacy, process, or containment review: No external target artifacts, credentials, generated archives, local caches, binaries, or built outputs committed. No change to execution authority. ## Author readiness Human owner: @WenyuChiou - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-03 13:39:11 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7624

No description provided.

Rows
Columns