Skip to content

fix(artifacts): make extraction output tree work on Windows - #753

Merged
N0zoM1z0 merged 1 commit into
morluto:mainfrom
DeryFerd:fix/safe-output-tree-windows
Oct 6, 2026
Merged

N0zoM1z0 merged 1 commit into
morluto:mainfrom
DeryFerd:fix/safe-output-tree-windows

Conversation

@DeryFerd

@DeryFerd DeryFerd commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Summary

extract_artifact fails on every Windows host. SafeOutputTree.create() opens the freshly created output directory and calls chmod(0o700) on the directory handle, and commit() calls sync() on a directory handle. Both are POSIX-only operations, and Windows rejects each with EPERM: operation not permitted. The provider catches the failure and reports it as a generic io reason, so the caller sees artifact_operation_failed with no output directory and no hint about the cause. This change skips the two directory-only operations on Windows, where they have no effect to begin with, and adds a regression test plus Windows CI coverage so the behavior cannot silently break again.

Problem and expected behavior

The smallest trigger is a single extraction on Windows. Given any extractable archive target (zip, ipa, apk, msix, appx, or a macho slice set), ArtifactProvider.execute("extract_artifact", { output_root }) calls extractArtifact, which calls SafeOutputTree.create(output_root). On Windows that call throws EPERM: operation not permitted, fchmod before any file is written, extractArtifact rolls back the unsealed tree, and the provider returns { ok: false, reason: "io" }. The tool is advertised as available on Windows, and the operation is a first-class workflow, so the effect is that a supported capability is entirely broken rather than partially degraded.

The violated invariant is narrower than it looks. The directory chmod was meant to force 0o700 on the new output root, but mkdir(canonicalOutput, { mode: 0o700 }) on the line above already applies the requested mode, so the handle chmod only re-asserts a mode that is already set. The directory sync in commit() was meant to make the tree durable, but each file's bytes are already flushed and read back inside write(), and Windows has no directory fsync at all. Neither operation contributes anything on Windows, and both are the reason the whole operation aborts there.

The expected behavior is that extraction produces the same files with the same digests on Windows as it does on Linux and macOS, and that the POSIX-only durability steps remain in place on the hosts that support them. That is what this change does.

Change and scope

SafeOutputTree now takes the platform as a constructor value with a process.platform default, matching the seam already used elsewhere in the artifact layer (for example ArtifactProvider(platform = process.platform)). create() only opens the directory handle and applies 0o700 when the platform is not win32; on Windows it relies on the mkdir mode, which is what actually sets the mode on that host. commit() marks the tree published and returns immediately on Windows, because there is no directory fsync there and the file-level durability guarantee is already established in write(). The POSIX path is byte-for-byte unchanged, including the descriptor close on the failure path and the parent/output sync ordering.

Two regression surfaces are added. tests/boundary/filesystem/safeOutputTree.test.ts gains a case that builds and commits a tree through the win32 seam and asserts the written file is present with the expected content, which is exactly the path that used to throw. The curated Windows CI lane in .github/workflows/ci.yml now runs safeOutputTree.test.ts and artifactProvider.archiveSafety.test.ts, so the Windows-only behavior is exercised on a Windows runner instead of only on developer machines.

Intentionally not included: no change to extraction containment, path normalization, digest verification, rollback, or the public tool contract; no new capability flag or approval step; and no attempt to give Windows a directory-level durability guarantee it does not have. The Windows behavior is now "the same result, minus the two no-op POSIX steps", not "a reduced guarantee that used to exist".

Contract and boundary impact

  • Semantic owner and earliest changed stage: artifact extraction (src/artifacts/SafeOutputTree.ts), reached from src/application/ArtifactExtraction.ts and src/firmware/FirmwarePublication.ts.
  • CLI and MCP/tool-catalog contract: none. extract_artifact input, output, and result schemas are unchanged.
  • Provider, bridge, target-format, or platform compatibility: Windows x64 now produces output for extraction instead of failing; Linux and macOS behavior is unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: none. Extraction results, digests, and cleanup reports keep their existing shape.
  • Process execution, authorization, cleanup, or containment impact: none. Rollback, O_EXCL | O_NOFOLLOW, symlink-resistant parent creation, and readback verification are untouched.
  • Generated metadata (docs/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

  • Tests added or updated: a new Windows-path case in tests/boundary/filesystem/safeOutputTree.test.ts, and the Windows CI lane now includes safeOutputTree.test.ts and artifactProvider.archiveSafety.test.ts.
  • Base reproduction or other evidence: on unchanged main, safeOutputTree.test.ts fails three cases with EPERM: operation not permitted, fchmod, and artifactProvider.archiveSafety.test.ts fails the two msix/appxbundle extraction cases with expected false to be true. The new case fails on base with the same fchmod error and passes after the fix, which is the red/green check that the test actually covers the change.
  • User-visible CLI/MCP output (if applicable): not applicable; the fix removes a failure rather than changing a result shape.
  • Remaining proof gaps: the full deterministic suite could not be completed in the authoring environment because the test runner exhausts the sandbox heap, so verification here is limited to the focused files listed below; hosted CI covers the complete gate. No real Hopper, Ghidra, or browser workflow is involved in this path.

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 tsc --noEmit — passed, exit 0.
  • npm run build:cached — passed, 1 task successful.
  • npx vitest run tests/boundary/filesystem/safeOutputTree.test.ts tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts tests/boundary/providers/native/artifactProvider.extraction.test.ts tests/process-global/curatedWindowsLane.test.ts — 10 passed, 1 skipped (4 files).
  • npx oxlint src/artifacts/SafeOutputTree.ts tests/boundary/filesystem/safeOutputTree.test.ts — 0 warnings, 0 errors.
  • npx oxfmt --check on the changed files — clean (LF, committed form).

The focused runs above were executed on Windows x64 with Node.js 24, which is the affected host. The Windows CI lane will re-run the two boundary files on a hosted Windows runner.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The change is additive and scoped to a host where the previous code path always failed.
  • Real Hopper/Ghidra, browser, or OS coverage: no real-provider claim is made. The change is in a filesystem helper shared by artifact extraction and firmware publication; firmware publication remains Linux-only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no containment or authorization behavior changes. The Windows path still creates the output root exclusively with mkdir and still writes each file with O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW under a symlink-checked parent. The only thing skipped on Windows is a redundant mode re-assertion and a directory sync that Windows does not implement.

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: this is a reproduced platform gap with a deterministic reproduction and a regression test, so it does not need separate triage.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed. No generated metadata changes are required.
  • User-visible CLI/MCP changes include representative output. Not applicable; this fixes a failure rather than changing output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

SafeOutputTree.create() called fchmod(0o700) on a directory handle and
commit() fsynced a directory handle. Both are POSIX-only: Windows rejects
them with EPERM ("operation not permitted, fchmod"/"fsync"), so every
extract_artifact call failed with a generic io error and produced no output.

Skip the redundant directory chmod and directory fsync on Windows. mkdir
already applies the requested mode, and file contents are synced in write().
Add a Windows regression test and run it, plus the archive-safety test, in
the curated Windows CI lane so the behavior is covered.
@N0zoM1z0
N0zoM1z0 merged commit c8e66c7 into morluto:main Oct 6, 2026
19 checks passed
@DeryFerd
DeryFerd deleted the fix/safe-output-tree-windows branch October 6, 2026 17:57
@morluto

morluto commented Oct 7, 2026

Copy link
Copy Markdown
Owner

Thanks, @DeryFerd, for your work on “fix(artifacts): make extraction output tree work on Windows.” I appreciate your contribution to REA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants