[PR #1879] [MERGED] fix(firmware): name missing extractor dependencies in partial extraction #4874

Closed
opened 2026-10-31 00:56:15 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1879
Author: @DeryFerd
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/11/2026
Merged by: @morluto

Base: main ← Head: fix/firmware-dependency-diagnostics


📝 Commits (1)

  • 150a56e fix(firmware): name missing extractor dependencies in partial extraction

📊 Changes

3 files changed (+110 additions, -3 deletions)

View changed files

📝 src/firmware/FirmwareReports.test.ts (+47 -1)
📝 src/firmware/FirmwareReports.ts (+61 -2)
📝 tests/boundary/firmware/firmwareIntegration.test.ts (+2 -0)

📄 Description

Summary

Fixes #1853. When Unblob reaches partial extraction because an external extractor is absent, it reports an ExtractorDependencyNotFoundReport that already names the missing dependencies, but normalizeUnblobReport forwarded that value verbatim into diagnostics as { input_path, report }. A caller that landed on partial coverage therefore had to decode the raw producer report to learn which extractor was missing and at which stage. This PR normalizes those entries: the missing dependency names are collected from the report and any nested value, the producing handler is recorded as the stage, and the original report stays retained for provenance.

Problem and expected behavior

Smallest trigger: run extract_firmware on a firmware image whose format needs an extractor that is not on PATH. Unblob finishes its configured extraction, retains the carved region, and reports the missing extractor as a nested ExtractorDependencyNotFoundReport with dependencies: ["sasquatch"]. Prior behavior: the normalized result carried coverage: "partial" and a diagnostics entry of { input_path, report }, where report held the raw producer report, so the dependency name and the stage existed only inside an unparsed producer value. The violated invariant: a partial result should explain the exact missing dependency and extraction stage, and distinguish carving from content extraction, without making the caller reverse-engineer the producer's report shape. Expected behavior: the diagnostic names the missing dependencies and the producing stage inline, the raw report is still retained, and coverage stays partial rather than implying a corrupt target or an empty filesystem.

Change and scope

src/firmware/FirmwareReports.ts gains one normalization helper used at both places where nested extraction problems are collected: the chunk-report branch (which carries a handler_name and an extraction_reports array) and the multi-file branch. The helper walks the report and any nested value for ExtractorDependencyNotFoundReport entries, collects their dependencies in first-seen order, records the handler as stage, and keeps the original report under report. Entries that carry no dependency report keep their previous { input_path, report } shape, so unrelated diagnostics such as unknown chunk types are unchanged. Intentionally not included: no change to the extract_firmware result schema, no change to how coverage is derived, no attempt to install or resolve extractors, and no reinterpretation of a missing dependency as a corrupt target.

Contract and boundary impact

  • Semantic owner and earliest changed stage: firmware producer-report normalization in src/firmware/FirmwareReports.ts, which interprets the pinned Unblob report before it is projected into the extract_firmware result.
  • CLI and MCP/tool-catalog contract: no schema change. diagnostics is an open array of JSON values in the result contract, so the added stage and missing_dependencies fields are additive and the tool catalog does not move.
  • Provider, bridge, target-format, or platform compatibility: none beyond Unblob's own report shape; Binwalk inspection and the region-scan path are untouched, and the change is host-independent.
  • Evidence, artifact, provenance, or reconstruction contract: the original producer report is retained alongside the normalized fields, so provenance and the exact reported dependencies both remain available; carved region offsets and digests are unchanged.
  • Process execution, authorization, cleanup, or containment impact: none; this reads an already-produced report and does not change how the extractor runs, which dependencies are present, or how output is published.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in skill-src/ and never commit build outputs): none committed.

Evidence and regression coverage

  • Tests added or updated: a new unit case in src/firmware/FirmwareReports.test.ts builds a chunk report with a nested ExtractorDependencyNotFoundReport and asserts the normalized diagnostic names the dependency and the gzip stage while retaining the raw report; the existing multi-file case now asserts the same enriched shape for a nested dependency report, and the firmware integration fixture expectation for the dependency mode is updated to the enriched diagnostic.
  • Base reproduction or other evidence: on the base revision the new case fails because the diagnostic is only { input_path, report }; after the change the firmware unit suite passes with the dependency named inline.
  • User-visible CLI/MCP output: a partial extract_firmware result now lists the missing extractor and the stage directly in diagnostics instead of burying them in a raw producer report.
  • Remaining proof gaps: real Unblob extraction was not run locally because it requires the external extractor stack; the integration lane that exercises a real Unblob run is gated on that toolchain and was skipped here, so the fixture-level and unit-level coverage stand in for it.

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

  • npx vitest run src/firmware/FirmwareReports.test.ts — 7 passed, including the new dependency-diagnostic case.
  • npx vitest run src/firmware/ tests/boundary/firmware/ — 23 passed, 30 skipped; the skipped cases are the real-toolchain lanes that need an installed Unblob.
  • npm run typecheck — exit 0.
  • npx oxlint on the changed files — 0 errors; the three warnings (complexity, max-lines-per-function, max-depth) are pre-existing on the base in normalizeUnblobReport and reproduced with the change stashed. oxfmt --check on the committed (LF) form — clean.
  • Real Hopper/Ghidra, browser, or OS lanes: not applicable; no provider, bridge, or platform-specific code is touched.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The change is additive on an open JSON-valued field; existing diagnostics keep their shape, and the enriched entries add fields rather than replacing the retained report.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; the firmware unit and fixture suites cover the changed normalization.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the change surfaces the exact missing extractor and stage for an already-produced partial result; it adds no network, process, installation, or authority surface and does not weaken the extractor sandbox.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • 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/1879 **Author:** [@DeryFerd](https://github.com/DeryFerd) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/firmware-dependency-diagnostics` --- ### 📝 Commits (1) - [`150a56e`](https://github.com/morluto/rea/commit/150a56ea5554a70ff75b0e8ce3ad3ec44f4d1066) fix(firmware): name missing extractor dependencies in partial extraction ### 📊 Changes **3 files changed** (+110 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/firmware/FirmwareReports.test.ts` (+47 -1) 📝 `src/firmware/FirmwareReports.ts` (+61 -2) 📝 `tests/boundary/firmware/firmwareIntegration.test.ts` (+2 -0) </details> ### 📄 Description ## Summary Fixes #1853. When Unblob reaches partial extraction because an external extractor is absent, it reports an `ExtractorDependencyNotFoundReport` that already names the missing dependencies, but `normalizeUnblobReport` forwarded that value verbatim into `diagnostics` as `{ input_path, report }`. A caller that landed on partial coverage therefore had to decode the raw producer report to learn which extractor was missing and at which stage. This PR normalizes those entries: the missing dependency names are collected from the report and any nested value, the producing handler is recorded as the stage, and the original report stays retained for provenance. ## Problem and expected behavior Smallest trigger: run `extract_firmware` on a firmware image whose format needs an extractor that is not on `PATH`. Unblob finishes its configured extraction, retains the carved region, and reports the missing extractor as a nested `ExtractorDependencyNotFoundReport` with `dependencies: ["sasquatch"]`. Prior behavior: the normalized result carried `coverage: "partial"` and a `diagnostics` entry of `{ input_path, report }`, where `report` held the raw producer report, so the dependency name and the stage existed only inside an unparsed producer value. The violated invariant: a partial result should explain the exact missing dependency and extraction stage, and distinguish carving from content extraction, without making the caller reverse-engineer the producer's report shape. Expected behavior: the diagnostic names the missing dependencies and the producing stage inline, the raw report is still retained, and coverage stays `partial` rather than implying a corrupt target or an empty filesystem. ## Change and scope `src/firmware/FirmwareReports.ts` gains one normalization helper used at both places where nested extraction problems are collected: the chunk-report branch (which carries a `handler_name` and an `extraction_reports` array) and the multi-file branch. The helper walks the report and any nested value for `ExtractorDependencyNotFoundReport` entries, collects their `dependencies` in first-seen order, records the handler as `stage`, and keeps the original report under `report`. Entries that carry no dependency report keep their previous `{ input_path, report }` shape, so unrelated diagnostics such as unknown chunk types are unchanged. Intentionally not included: no change to the `extract_firmware` result schema, no change to how `coverage` is derived, no attempt to install or resolve extractors, and no reinterpretation of a missing dependency as a corrupt target. ## Contract and boundary impact - Semantic owner and earliest changed stage: firmware producer-report normalization in `src/firmware/FirmwareReports.ts`, which interprets the pinned Unblob report before it is projected into the `extract_firmware` result. - CLI and MCP/tool-catalog contract: no schema change. `diagnostics` is an open array of JSON values in the result contract, so the added `stage` and `missing_dependencies` fields are additive and the tool catalog does not move. - Provider, bridge, target-format, or platform compatibility: none beyond Unblob's own report shape; Binwalk inspection and the region-scan path are untouched, and the change is host-independent. - Evidence, artifact, provenance, or reconstruction contract: the original producer report is retained alongside the normalized fields, so provenance and the exact reported dependencies both remain available; carved region offsets and digests are unchanged. - Process execution, authorization, cleanup, or containment impact: none; this reads an already-produced report and does not change how the extractor runs, which dependencies are present, or how output is published. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `skill-src/` and never commit build outputs): none committed. ## Evidence and regression coverage - Tests added or updated: a new unit case in `src/firmware/FirmwareReports.test.ts` builds a chunk report with a nested `ExtractorDependencyNotFoundReport` and asserts the normalized diagnostic names the dependency and the `gzip` stage while retaining the raw report; the existing multi-file case now asserts the same enriched shape for a nested dependency report, and the firmware integration fixture expectation for the dependency mode is updated to the enriched diagnostic. - Base reproduction or other evidence: on the base revision the new case fails because the diagnostic is only `{ input_path, report }`; after the change the firmware unit suite passes with the dependency named inline. - User-visible CLI/MCP output: a partial `extract_firmware` result now lists the missing extractor and the stage directly in `diagnostics` instead of burying them in a raw producer report. - Remaining proof gaps: real Unblob extraction was not run locally because it requires the external extractor stack; the integration lane that exercises a real Unblob run is gated on that toolchain and was skipped here, so the fixture-level and unit-level coverage stand in for it. 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 - `npx vitest run src/firmware/FirmwareReports.test.ts` — 7 passed, including the new dependency-diagnostic case. - `npx vitest run src/firmware/ tests/boundary/firmware/` — 23 passed, 30 skipped; the skipped cases are the real-toolchain lanes that need an installed Unblob. - `npm run typecheck` — exit 0. - `npx oxlint` on the changed files — 0 errors; the three warnings (`complexity`, `max-lines-per-function`, `max-depth`) are pre-existing on the base in `normalizeUnblobReport` and reproduced with the change stashed. `oxfmt --check` on the committed (LF) form — clean. - Real Hopper/Ghidra, browser, or OS lanes: not applicable; no provider, bridge, or platform-specific code is touched. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The change is additive on an open JSON-valued field; existing diagnostics keep their shape, and the enriched entries add fields rather than replacing the retained report. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; the firmware unit and fixture suites cover the changed normalization. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the change surfaces the exact missing extractor and stage for an already-produced partial result; it adds no network, process, installation, or authority surface and does not weaken the extractor sandbox. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] 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-10-31 00:56:15 +00:00
Sign in to join this conversation.
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#4874
No description provided.