[PR #785] [CLOSED] fix(deps): remediate reachable archive-parsing advisories #2241

New issue

Closed

opened 2026-10-16 21:16:09 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:16:09 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/785
Author: @DeryFerd
Created: 10/6/2026
Status: ❌ Closed

Base: main ← Head: fix/reachable-dependency-advisories

📝 Commits (1)

db4872b fix(deps): remediate reachable archive-parsing advisories

📊 Changes

1 file changed (+7 additions, -7 deletions)

View changed files

📝 package-lock.json (+7 -7)

📄 Description

Summary

Two high-severity advisories in the dependency tree are reachable from the untrusted content REA analyzes, so a crafted target can drive the vulnerable code path. @xmldom/xmldom@0.9.10, pulled in through plist, parses XML plist bytes from an analyzed app bundle in the keyed-archive reader. brace-expansion@5.0.7, pulled in through @electron/asar → minimatch, is reached while analyzing an ASAR archive. This change refreshes both to their patched releases and leaves everything else untouched. Both are in-range patch updates, so no direct dependency declaration moves, no API changes, and no source file is edited.

Problem and expected behavior

The smallest trigger is analyzing an attacker-supplied target whose format exercises one of the two paths. An XML plist inside an analyzed bundle reaches plist's parse() at src/artifacts/KeyedArchiveReader.ts, which delegates to @xmldom/xmldom; an ASAR archive reaches @electron/asar, which delegates glob matching to minimatch and brace expansion to brace-expansion. In both cases the dependency is a transitive one, so the vulnerable version arrives without any direct declaration in package.json, and npm audit reports it at high severity.

The violated invariant is that REA's dependency surface should not contain known-vulnerable code on paths that untrusted target content can reach. The expected behavior is that the same inputs are still processed with the same results, but through patched dependency releases. This change produces exactly that: the two advisories are remediated and observable behavior is unchanged.

Change and scope

The change is confined to package-lock.json and moves two transitive packages to their patched releases: @xmldom/xmldom 0.9.10 → 0.9.12, and brace-expansion 5.0.7 → 5.0.12. Both targets sit inside the range each parent already declares (plist declares ^0.9.10, minimatch declares ^5.0.5), so this is a lockfile-only refresh and no parent or direct dependency changes its declared range. The diff is seven lines.

I deliberately did not include the third advisory in the tree. smol-toml@1.7.0 is a direct dependency, but it parses the operator's own client-configuration file rather than untrusted target content, so it is outside the reachable-from-untrusted-input boundary this PR targets. Its fix also moves outside the declared range and changes behavior: 1.9.0 accepts a leading UTF-8 BOM where 1.7.0 rejects it, and an existing test intentionally locks the rejection, so bumping it is a separate decision with its own review. I also did not add a dependency-audit step to CI. A per-PR hard-fail audit gate would red-flag unrelated pull requests whenever a new advisory is published without any code change, which would disrupt the repository's normal contribution flow; that is a policy decision for the maintainer, not something to attach to this fix.

Intentionally not included: no source edits, no test edits, no direct dependency range changes, no CI changes, and no attempt to address the remaining advisory.

Contract and boundary impact

Semantic owner and earliest changed stage: dependency resolution (package-lock.json); no REA source stage changes.
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none. The patched releases keep their public APIs, and brace-expansion@5.0.12 narrows its supported Node range to 20 || >=22, which REA already exceeds (its supported runtime is ^22.19 || ^24.11 || >=26).
Evidence, artifact, provenance, or reconstruction contract: none.
Process execution, authorization, cleanup, or containment impact: none.
Generated metadata (docs/product-catalog.json), package, or installation impact: none. The generated package metadata tracks the MCP SDK versions, not these packages, so npm run metadata:check stays green and no regeneration is required.

Evidence and regression coverage

Tests added or updated: none. This is a lockfile-only change with no source edit, so there is no new behavior to test; the existing suites that exercise the affected dependencies are the regression coverage and were run against the bumped lockfile.
Base reproduction or other evidence: npm audit on the affected base revision reports three high-severity advisories, two of which are @xmldom/xmldom and brace-expansion; after the change, npm audit reports only the smol-toml advisory that is intentionally out of scope.
User-visible CLI/MCP output (if applicable): not applicable.
Remaining proof gaps: I did not execute a real exploit against either advisory, so the claim is that the vulnerable code is reachable and the fix removes it, not that a specific exploit was demonstrated end to end. The remaining smol-toml advisory is left open by design and is noted above.

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

npm ci — passed, lockfile installs consistently.
npm audit — three high advisories before, one after (only the intentionally excluded smol-toml).
npm ls @xmldom/xmldom brace-expansion — confirms the resolved versions are 0.9.12 and 5.0.12.
npx tsc --noEmit — passed, exit 0.
npm run build:cached — passed, one task successful.
npm run deps:check — passed.
npm run metadata:check — passed.
npx vitest run src/artifacts/KeyedArchiveReader.test.ts tests/boundary/filesystem/asarInventoryContainment.test.ts tests/boundary/filesystem/asarInventoryFreshness.test.ts tests/boundary/providers/native/keyedArchiveEncoding.test.ts tests/boundary/providers/native/artifactProvider.archiveInventory.test.ts — 24 passed, 3 skipped (5 files), covering the keyed-archive and ASAR readers that use the bumped packages.

npm run docs:check currently fails on docs/verification/managed-conformance-manifest.json in this checkout. That failure reproduces on the unmodified base revision and is unrelated to this change, so it is not a regression from this PR.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: no real-provider claim. The affected readers are the artifact/keyed-archive and ASAR paths, exercised by the deterministic tests listed above.
Package or release metadata impact: none. No dependency range in package.json changes, so no release note or metadata regeneration is triggered.
Security, privacy, process, or containment review: this PR improves the dependency posture on two untrusted-input paths and changes no runtime control, authorization, or containment behavior. It removes known-vulnerable transitive code from paths reachable by analyzed targets.

Review checklist

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. There is no linked issue: the advisories are reported by the package manager, and this is a scoped remediation rather than a tracked feature or bug.
Tests cover changed observable behavior and meaningful failure paths. No behavior changed; the existing readers' suites cover the affected packages and were run.
Owning docs, contracts, and generated metadata are updated where needed. No generated artifact depends on these two packages, confirmed by a green metadata:check.
User-visible CLI/MCP changes include representative output. Not applicable.
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/785 **Author:** [@DeryFerd](https://github.com/DeryFerd) **Created:** 10/6/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/reachable-dependency-advisories` --- ### 📝 Commits (1) - [`db4872b`](https://github.com/morluto/rea/commit/db4872b8f9730827f66713557a10d1b8f65bbe2f) fix(deps): remediate reachable archive-parsing advisories ### 📊 Changes **1 file changed** (+7 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `package-lock.json` (+7 -7) </details> ### 📄 Description ## Summary Two high-severity advisories in the dependency tree are reachable from the untrusted content REA analyzes, so a crafted target can drive the vulnerable code path. `@xmldom/xmldom@0.9.10`, pulled in through `plist`, parses XML plist bytes from an analyzed app bundle in the keyed-archive reader. `brace-expansion@5.0.7`, pulled in through `@electron/asar` → `minimatch`, is reached while analyzing an ASAR archive. This change refreshes both to their patched releases and leaves everything else untouched. Both are in-range patch updates, so no direct dependency declaration moves, no API changes, and no source file is edited. ## Problem and expected behavior The smallest trigger is analyzing an attacker-supplied target whose format exercises one of the two paths. An XML plist inside an analyzed bundle reaches `plist`'s `parse()` at `src/artifacts/KeyedArchiveReader.ts`, which delegates to `@xmldom/xmldom`; an ASAR archive reaches `@electron/asar`, which delegates glob matching to `minimatch` and brace expansion to `brace-expansion`. In both cases the dependency is a transitive one, so the vulnerable version arrives without any direct declaration in `package.json`, and `npm audit` reports it at high severity. The violated invariant is that REA's dependency surface should not contain known-vulnerable code on paths that untrusted target content can reach. The expected behavior is that the same inputs are still processed with the same results, but through patched dependency releases. This change produces exactly that: the two advisories are remediated and observable behavior is unchanged. ## Change and scope The change is confined to `package-lock.json` and moves two transitive packages to their patched releases: `@xmldom/xmldom` `0.9.10` → `0.9.12`, and `brace-expansion` `5.0.7` → `5.0.12`. Both targets sit inside the range each parent already declares (`plist` declares `^0.9.10`, `minimatch` declares `^5.0.5`), so this is a lockfile-only refresh and no parent or direct dependency changes its declared range. The diff is seven lines. I deliberately did not include the third advisory in the tree. `smol-toml@1.7.0` is a direct dependency, but it parses the operator's own client-configuration file rather than untrusted target content, so it is outside the reachable-from-untrusted-input boundary this PR targets. Its fix also moves outside the declared range and changes behavior: `1.9.0` accepts a leading UTF-8 BOM where `1.7.0` rejects it, and an existing test intentionally locks the rejection, so bumping it is a separate decision with its own review. I also did not add a dependency-audit step to CI. A per-PR hard-fail audit gate would red-flag unrelated pull requests whenever a new advisory is published without any code change, which would disrupt the repository's normal contribution flow; that is a policy decision for the maintainer, not something to attach to this fix. Intentionally not included: no source edits, no test edits, no direct dependency range changes, no CI changes, and no attempt to address the remaining advisory. ## Contract and boundary impact - Semantic owner and earliest changed stage: dependency resolution (`package-lock.json`); no REA source stage changes. - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. The patched releases keep their public APIs, and `brace-expansion@5.0.12` narrows its supported Node range to `20 || >=22`, which REA already exceeds (its supported runtime is `^22.19 || ^24.11 || >=26`). - Evidence, artifact, provenance, or reconstruction contract: none. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata (`docs/product-catalog.json`), package, or installation impact: none. The generated package metadata tracks the MCP SDK versions, not these packages, so `npm run metadata:check` stays green and no regeneration is required. ## Evidence and regression coverage - Tests added or updated: none. This is a lockfile-only change with no source edit, so there is no new behavior to test; the existing suites that exercise the affected dependencies are the regression coverage and were run against the bumped lockfile. - Base reproduction or other evidence: `npm audit` on the affected base revision reports three high-severity advisories, two of which are `@xmldom/xmldom` and `brace-expansion`; after the change, `npm audit` reports only the `smol-toml` advisory that is intentionally out of scope. - User-visible CLI/MCP output (if applicable): not applicable. - Remaining proof gaps: I did not execute a real exploit against either advisory, so the claim is that the vulnerable code is reachable and the fix removes it, not that a specific exploit was demonstrated end to end. The remaining `smol-toml` advisory is left open by design and is noted above. 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 - `npm ci` — passed, lockfile installs consistently. - `npm audit` — three high advisories before, one after (only the intentionally excluded `smol-toml`). - `npm ls @xmldom/xmldom brace-expansion` — confirms the resolved versions are `0.9.12` and `5.0.12`. - `npx tsc --noEmit` — passed, exit 0. - `npm run build:cached` — passed, one task successful. - `npm run deps:check` — passed. - `npm run metadata:check` — passed. - `npx vitest run src/artifacts/KeyedArchiveReader.test.ts tests/boundary/filesystem/asarInventoryContainment.test.ts tests/boundary/filesystem/asarInventoryFreshness.test.ts tests/boundary/providers/native/keyedArchiveEncoding.test.ts tests/boundary/providers/native/artifactProvider.archiveInventory.test.ts` — 24 passed, 3 skipped (5 files), covering the keyed-archive and ASAR readers that use the bumped packages. `npm run docs:check` currently fails on `docs/verification/managed-conformance-manifest.json` in this checkout. That failure reproduces on the unmodified base revision and is unrelated to this change, so it is not a regression from this PR. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: no real-provider claim. The affected readers are the artifact/keyed-archive and ASAR paths, exercised by the deterministic tests listed above. - Package or release metadata impact: none. No dependency range in `package.json` changes, so no release note or metadata regeneration is triggered. - Security, privacy, process, or containment review: this PR improves the dependency posture on two untrusted-input paths and changes no runtime control, authorization, or containment behavior. It removes known-vulnerable transitive code from paths reachable by analyzed targets. ## Review checklist - [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. There is no linked issue: the advisories are reported by the package manager, and this is a scoped remediation rather than a tracked feature or bug. - [x] Tests cover changed observable behavior and meaningful failure paths. No behavior changed; the existing readers' suites cover the affected packages and were run. - [x] Owning docs, contracts, and generated metadata are updated where needed. No generated artifact depends on these two packages, confirmed by a green `metadata:check`. - [x] User-visible CLI/MCP changes include representative output. Not applicable. - [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-16 21:16:09 +00:00