morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #1575] [CLOSED] fix(artifacts): reject Windows alternate stream output paths #2983

New issue

Closed

opened 2026-10-17 21:12:32 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:12:32 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1575
Author: @dakjdakd
Created: 10/10/2026
Status: ❌ Closed

Base: main ← Head: fix/windows-artifact-stream-paths

📝 Commits (1)

1ba9222 fix(artifacts): reject Windows alternate stream output paths

📊 Changes

4 files changed (+96 additions, -3 deletions)

View changed files

📝 src/artifacts/SafeOutputTree.ts (+6 -0)
📝 src/domain/analysisErrorPresentation.ts (+6 -3)
📝 tests/boundary/filesystem/safeOutputTree.test.ts (+33 -0)
📝 tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts (+51 -0)

📄 Description

Summary

Fixes #1574

Reject alternate data stream syntax when materializing artifact files on Windows. A regular ZIP entry named hello.txt:hidden previously produced a zero-byte hello.txt with its payload in an NTFS stream, while REA reported the original logical entry as successfully extracted. The operation now returns an explicit typed path failure and rolls back its owned output tree.

The original logical name remains available in inventory, and POSIX materialization keeps its existing literal-colon behavior. The fix lives in the shared output-tree boundary, so callers cannot accidentally obtain a successful regular-file result from a Windows stream path.

Problem and expected behavior

A ZIP filename belongs to the archive's logical namespace. The : in hello.txt:hidden is an ordinary filename character there. Passing the same string to Windows file APIs changes its interpretation: hello.txt becomes the base file and hidden becomes an alternate data stream.

The reduced fixture contains secret payload, exactly 14 bytes. Before this change, the public CLI returned relative_path: "hello.txt:hidden", bytes_written: 14, created: true, and the statement that all regular files had been materialized. Independent PowerShell inspection showed an empty unnamed data stream on hello.txt and a separate 14-byte hidden stream.

The existing digest verification could not catch that mismatch. The writer and readback both opened the same colon path, so both accessed the same alternate stream. Their bytes and hashes agreed, but ordinary-file identity had already been lost. Users enumerating or copying the ordinary extracted tree could receive an empty file while the extraction manifest claimed complete materialization.

The expected invariant is that each successfully published ordinary artifact file retains its logical identity and bytes at the destination. If the destination cannot represent that identity, extraction must fail with a useful reason and remove its uncommitted output. Sanitizing or silently renaming the entry is not a substitute: it changes caller-visible identity and can collapse distinct archive names.

Change and scope

This changes four files, with the production correction concentrated in the owning materialization boundary and its error presentation.

In src/artifacts/SafeOutputTree.ts, a six-line guard checks the normalized path when platform === "win32". A colon anywhere in the relative path is refused before the path is registered or parent directories and destination handles are created. This covers the leaf stream name, explicit ::$DATA syntax, and stream syntax in an intermediate component. The existing ArtifactReaderFailure("path", ...) carries the original logical path and explains the Windows destination limitation.

The guard runs inside the writer's existing failure boundary, which destroys the supplied source stream before returning the path error. Archive extraction opens an entry stream before calling the output writer; for ZIP entries, that can already start asynchronous decompression. Destroying the rejected consumer triggers the ZIP stream's existing close-to-abort handling, allowing its producer to settle before rollback closes the reader. The guard reuses that shared cleanup path and preserves the writer's inventoried-size validation and bounded writes; the ZIP adapter itself is unchanged.

In src/domain/analysisErrorPresentation.ts, the explicit Windows destination diagnostic is preserved as the user-facing path message. It remains a destination representation failure rather than being reduced to a generic conflicting-path message.

The remaining changes are observable-behavior regressions in tests/boundary/filesystem/safeOutputTree.test.ts and tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts. No archive-name normalization, provider-specific parser, filename rewriting, or new dependency is introduced. Reserved Windows device names, trailing-dot aliases, and output-root spelling are outside this focused correction.

Contract and boundary impact

Semantic owner and earliest changed stage: SafeOutputTree.write, after logical path normalization and before filesystem materialization. Inventory retains its platform-neutral responsibility.
CLI and MCP/tool-catalog contract: no argument, command, operation, or schema changes. The artifact provider translates the existing typed path failure; CLI output now explains why this destination cannot represent the requested file.
Provider, bridge, target-format, or platform compatibility: the guard applies to Windows output. It is shared by output-tree publishers rather than being added only to the ZIP parser. Ordinary Windows names and POSIX literal-colon names keep their existing behavior. No bridge changes.
Evidence, artifact, provenance, or reconstruction contract: successful extraction no longer claims ordinary-file materialization for an NTFS stream. Inventory preserves the source logical name; no name mapping or provenance rewrite occurs.
Process execution, authorization, cleanup, or containment impact: no new execution or authority. The rejected entry stream is destroyed, and the extraction workflow's existing rollback removes the operation-owned tree before the failure returns.
Generated metadata, package, or installation impact: none. No public schema or catalog structure changes, dependency changes, installation changes, or generated artifacts are included.

Evidence and regression coverage

The evidence includes executed Windows filesystem behavior and the compiled public CLI. The fixture is inert synthetic text; no archive contents are executed.

The shared-tree regression exercises hello.txt:hidden, nested/hello.txt::$DATA, and folder:stream/file.txt. Each must reject with reason path, destroy the source stream, leave the newly created output root empty, and permit verified rollback with no remaining root. Checking the directory before rollback matters: cleanup alone could otherwise hide an empty base file or partially created parent.

The ZIP provider regression first inventories an archive containing 0-ordinary.txt and hello.txt:hidden. Inventory must retain the colon name. On Windows, extraction must return the explicit destination path error and remove its entire output tree, including the ordinary entry that precedes the offending name. Attempting to read the rejected stream path after rollback must return ENOENT.

On POSIX, the same provider test instead requires successful extraction of both entries, verifies their bytes, and confirms that a separate hello.txt base file does not appear. This branch records the compatibility requirement without mocking Windows behavior. The existing ordinary-name Windows test and normal ZIP CLI control verify that regular output still succeeds.

The regressions verify the corrected destination boundary through direct tree writes and real ZIP extraction. Rejected sources settle cleanly, and the final run confirms the explicit path diagnostic and complete rollback.

Tests added or updated: the two existing boundary suites above; no additional test framework or binary fixture is added.
Base reproduction: real NTFS stream observations for the Python-produced ZIP, plus the base regression scenarios.
User-visible CLI output: extract-artifact ads.zip --format json exits 1 with the existing error code and the following representative fields:

{
  "code": "artifact_operation_failed",
  "message": "Windows destination cannot materialize artifact path as a regular file: hello.txt:hidden; ':' denotes alternate data stream syntax. Inventory retains the logical name.",
  "details": {
    "operation": "extract_artifact",
    "reason": "path"
  }
}

Remaining proof gaps: the POSIX success branch is included but was not executed on this Windows host. No claim is made about every Windows filename alias or unrelated provider workflow.

For evidence-bearing changes:

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

Validation performed

Validation used the pinned Node.js 24.18.0 / npm 11.16.0 toolchain on Windows x64 with NTFS. The focused fix is a single commit above 603d04ee3ac6ed496f84478164f832a9b5ce3b91; the focused tests, full repository check, and real CLI controls were rerun after rebasing onto that revision.

npm run test:focused -- tests/boundary/filesystem/safeOutputTree.test.ts tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts tests/boundary/providers/native/artifactProvider.extraction.test.ts tests/boundary/providers/native/artifactProvider.failureDetail.test.ts src/domain/analysisErrorProjection.test.ts — 56 passed; two existing POSIX-only cases skipped on Windows.
npm run test:focused -- tests/boundary/providers/native/zipBackpressureCancellation.test.ts tests/boundary/providers/native/artifactProvider.extractionIdentity.test.ts tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts — seven additional stream-ownership, identity, and extraction-completeness tests passed.
npm run build:cached — passed.
npm run check — passed, including typecheck, lint, formatting, module boundaries, knip, metadata freshness, and architecture guards.
node scripts/verify-test-discovery.mjs — all test files discovered exactly once.
git diff --check — passed.
node scripts/rea.mjs extract-artifact ads.zip --format json — typed path refusal, exit 1, no residual extraction tree.
Public CLI normal-ZIP control — exit 0; nested/hello.txt verified independently as an ordinary 16-byte file containing ordinary payload.

Compatibility, safety, and release

Breaking changes or migration steps: no input/schema migration. Windows colon entries now fail explicitly because their previous successful result misrepresented the filesystem output. Callers can inspect the retained inventory or choose a destination that supports the literal filename.
Real Hopper/Ghidra, browser, or OS coverage: real Windows/NTFS extraction and compiled CLI verified. No external analysis engine is required for this artifact operation. Actual POSIX execution remains unverified locally.
Package or release metadata impact: none; the Conventional Commit title describes the focused fix.
Security, privacy, process, or containment review: the guard runs before destination creation and preserves existing rollback ownership. The observed defect concerns file identity and evidence completeness; this PR does not claim directory escape or code execution.

Review checklist

This PR addresses a concrete reproducible problem.
The PR has one focused outcome and the title follows type(scope): outcome.
The corresponding issue is linked above.
Tests cover changed observable behavior and meaningful failure paths.
Owning contracts and generated metadata require no changes.
User-visible CLI 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/1575 **Author:** [@dakjdakd](https://github.com/dakjdakd) **Created:** 10/10/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/windows-artifact-stream-paths` --- ### 📝 Commits (1) - [`1ba9222`](https://github.com/morluto/rea/commit/1ba92220596cd36def0a8f382999b1a3a3976134) fix(artifacts): reject Windows alternate stream output paths ### 📊 Changes **4 files changed** (+96 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/SafeOutputTree.ts` (+6 -0) 📝 `src/domain/analysisErrorPresentation.ts` (+6 -3) 📝 `tests/boundary/filesystem/safeOutputTree.test.ts` (+33 -0) 📝 `tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts` (+51 -0) </details> ### 📄 Description ## Summary Fixes #1574 Reject alternate data stream syntax when materializing artifact files on Windows. A regular ZIP entry named `hello.txt:hidden` previously produced a zero-byte `hello.txt` with its payload in an NTFS stream, while REA reported the original logical entry as successfully extracted. The operation now returns an explicit typed path failure and rolls back its owned output tree. The original logical name remains available in inventory, and POSIX materialization keeps its existing literal-colon behavior. The fix lives in the shared output-tree boundary, so callers cannot accidentally obtain a successful regular-file result from a Windows stream path. ## Problem and expected behavior A ZIP filename belongs to the archive's logical namespace. The `:` in `hello.txt:hidden` is an ordinary filename character there. Passing the same string to Windows file APIs changes its interpretation: `hello.txt` becomes the base file and `hidden` becomes an alternate data stream. The reduced fixture contains `secret payload`, exactly 14 bytes. Before this change, the public CLI returned `relative_path: "hello.txt:hidden"`, `bytes_written: 14`, `created: true`, and the statement that all regular files had been materialized. Independent PowerShell inspection showed an empty unnamed data stream on `hello.txt` and a separate 14-byte `hidden` stream. The existing digest verification could not catch that mismatch. The writer and readback both opened the same colon path, so both accessed the same alternate stream. Their bytes and hashes agreed, but ordinary-file identity had already been lost. Users enumerating or copying the ordinary extracted tree could receive an empty file while the extraction manifest claimed complete materialization. The expected invariant is that each successfully published ordinary artifact file retains its logical identity and bytes at the destination. If the destination cannot represent that identity, extraction must fail with a useful reason and remove its uncommitted output. Sanitizing or silently renaming the entry is not a substitute: it changes caller-visible identity and can collapse distinct archive names. ## Change and scope This changes four files, with the production correction concentrated in the owning materialization boundary and its error presentation. In `src/artifacts/SafeOutputTree.ts`, a six-line guard checks the normalized path when `platform === "win32"`. A colon anywhere in the relative path is refused before the path is registered or parent directories and destination handles are created. This covers the leaf stream name, explicit `::$DATA` syntax, and stream syntax in an intermediate component. The existing `ArtifactReaderFailure("path", ...)` carries the original logical path and explains the Windows destination limitation. The guard runs inside the writer's existing failure boundary, which destroys the supplied source stream before returning the path error. Archive extraction opens an entry stream before calling the output writer; for ZIP entries, that can already start asynchronous decompression. Destroying the rejected consumer triggers the ZIP stream's existing close-to-abort handling, allowing its producer to settle before rollback closes the reader. The guard reuses that shared cleanup path and preserves the writer's inventoried-size validation and bounded writes; the ZIP adapter itself is unchanged. In `src/domain/analysisErrorPresentation.ts`, the explicit Windows destination diagnostic is preserved as the user-facing path message. It remains a destination representation failure rather than being reduced to a generic conflicting-path message. The remaining changes are observable-behavior regressions in `tests/boundary/filesystem/safeOutputTree.test.ts` and `tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts`. No archive-name normalization, provider-specific parser, filename rewriting, or new dependency is introduced. Reserved Windows device names, trailing-dot aliases, and output-root spelling are outside this focused correction. ## Contract and boundary impact - Semantic owner and earliest changed stage: `SafeOutputTree.write`, after logical path normalization and before filesystem materialization. Inventory retains its platform-neutral responsibility. - CLI and MCP/tool-catalog contract: no argument, command, operation, or schema changes. The artifact provider translates the existing typed path failure; CLI output now explains why this destination cannot represent the requested file. - Provider, bridge, target-format, or platform compatibility: the guard applies to Windows output. It is shared by output-tree publishers rather than being added only to the ZIP parser. Ordinary Windows names and POSIX literal-colon names keep their existing behavior. No bridge changes. - Evidence, artifact, provenance, or reconstruction contract: successful extraction no longer claims ordinary-file materialization for an NTFS stream. Inventory preserves the source logical name; no name mapping or provenance rewrite occurs. - Process execution, authorization, cleanup, or containment impact: no new execution or authority. The rejected entry stream is destroyed, and the extraction workflow's existing rollback removes the operation-owned tree before the failure returns. - Generated metadata, package, or installation impact: none. No public schema or catalog structure changes, dependency changes, installation changes, or generated artifacts are included. ## Evidence and regression coverage The evidence includes executed Windows filesystem behavior and the compiled public CLI. The fixture is inert synthetic text; no archive contents are executed. The shared-tree regression exercises `hello.txt:hidden`, `nested/hello.txt::$DATA`, and `folder:stream/file.txt`. Each must reject with reason `path`, destroy the source stream, leave the newly created output root empty, and permit verified rollback with no remaining root. Checking the directory before rollback matters: cleanup alone could otherwise hide an empty base file or partially created parent. The ZIP provider regression first inventories an archive containing `0-ordinary.txt` and `hello.txt:hidden`. Inventory must retain the colon name. On Windows, extraction must return the explicit destination path error and remove its entire output tree, including the ordinary entry that precedes the offending name. Attempting to read the rejected stream path after rollback must return `ENOENT`. On POSIX, the same provider test instead requires successful extraction of both entries, verifies their bytes, and confirms that a separate `hello.txt` base file does not appear. This branch records the compatibility requirement without mocking Windows behavior. The existing ordinary-name Windows test and normal ZIP CLI control verify that regular output still succeeds. The regressions verify the corrected destination boundary through direct tree writes and real ZIP extraction. Rejected sources settle cleanly, and the final run confirms the explicit path diagnostic and complete rollback. - Tests added or updated: the two existing boundary suites above; no additional test framework or binary fixture is added. - Base reproduction: real NTFS stream observations for the Python-produced ZIP, plus the base regression scenarios. - User-visible CLI output: `extract-artifact ads.zip --format json` exits 1 with the existing error code and the following representative fields: ```json { "code": "artifact_operation_failed", "message": "Windows destination cannot materialize artifact path as a regular file: hello.txt:hidden; ':' denotes alternate data stream syntax. Inventory retains the logical name.", "details": { "operation": "extract_artifact", "reason": "path" } } ``` - Remaining proof gaps: the POSIX success branch is included but was not executed on this Windows host. No claim is made about every Windows filename alias or unrelated provider workflow. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failure details remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Validation used the pinned Node.js 24.18.0 / npm 11.16.0 toolchain on Windows x64 with NTFS. The focused fix is a single commit above `603d04ee3ac6ed496f84478164f832a9b5ce3b91`; the focused tests, full repository check, and real CLI controls were rerun after rebasing onto that revision. - `npm run test:focused -- tests/boundary/filesystem/safeOutputTree.test.ts tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts tests/boundary/providers/native/artifactProvider.extraction.test.ts tests/boundary/providers/native/artifactProvider.failureDetail.test.ts src/domain/analysisErrorProjection.test.ts` — 56 passed; two existing POSIX-only cases skipped on Windows. - `npm run test:focused -- tests/boundary/providers/native/zipBackpressureCancellation.test.ts tests/boundary/providers/native/artifactProvider.extractionIdentity.test.ts tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts` — seven additional stream-ownership, identity, and extraction-completeness tests passed. - `npm run build:cached` — passed. - `npm run check` — passed, including typecheck, lint, formatting, module boundaries, knip, metadata freshness, and architecture guards. - `node scripts/verify-test-discovery.mjs` — all test files discovered exactly once. - `git diff --check` — passed. - `node scripts/rea.mjs extract-artifact ads.zip --format json` — typed path refusal, exit 1, no residual extraction tree. - Public CLI normal-ZIP control — exit 0; `nested/hello.txt` verified independently as an ordinary 16-byte file containing `ordinary payload`. ## Compatibility, safety, and release - Breaking changes or migration steps: no input/schema migration. Windows colon entries now fail explicitly because their previous successful result misrepresented the filesystem output. Callers can inspect the retained inventory or choose a destination that supports the literal filename. - Real Hopper/Ghidra, browser, or OS coverage: real Windows/NTFS extraction and compiled CLI verified. No external analysis engine is required for this artifact operation. Actual POSIX execution remains unverified locally. - Package or release metadata impact: none; the Conventional Commit title describes the focused fix. - Security, privacy, process, or containment review: the guard runs before destination creation and preserves existing rollback ownership. The observed defect concerns file identity and evidence completeness; this PR does not claim directory escape or code execution. ## Review checklist - [x] This PR addresses a concrete reproducible problem. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] The corresponding issue is linked above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning contracts and generated metadata require no changes. - [x] User-visible CLI 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-17 21:12:32 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:07:24 +00:00

[PR #2983] [CLOSED] fix(setup): carry REA_JADX_MCP_JAR into agent registrations #5536

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#2983

No description provided.

Rows
Columns