Skip to content

feat: support native artifact downloads on Windows - #346

Open
signal-forge-lab wants to merge 1 commit into
Waishnav:mainfrom
signal-forge-lab:feat/artifact-download-windows
Open

feat: support native artifact downloads on Windows#346
signal-forge-lab wants to merge 1 commit into
Waishnav:mainfrom
signal-forge-lab:feat/artifact-download-windows

Conversation

@signal-forge-lab

@signal-forge-lab signal-forge-lab commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

download_artifact is currently Linux-only because secure traversal relies on descriptor-anchored filesystem operations. This PR adds Windows support while keeping the current Linux implementation unchanged. On Windows, the workspace root and each destination directory are pinned with native CreateFileW handles via Koffi; reparse points are rejected and delete sharing is omitted so path components cannot be renamed or replaced during transfer. Existing streaming, size/hash verification, same-directory partial files, and hard-link no-overwrite publication remain shared. Windows-specific validation rejects drive-relative or absolute paths, UNC and device namespaces, ADS, reserved device names, invalid characters, and trailing dot or space segments. Koffi is lazy-loaded only on Windows and is included in the pnpm build-script allowlist. This is intentionally narrower than #103 and based on current main: it carries forward the native Windows handle-pinning approach without refactoring the Linux path or adding macOS support. Verified on Windows with pnpm typecheck, the full test suite (131 passed, 6 skipped, 0 failed), and pnpm build. pnpm test:package-install currently fails on Windows with the same devspace.cmd path-not-found error on an untouched current upstream/main checkout, so that failure is pre-existing and not introduced by this branch.

Summary by CodeRabbit

  • New Features

    • Native artifact downloads are now supported on Windows.
    • Windows downloads validate paths and protect destination directories against unsafe links and replacement during transfers.
    • Windows file permissions follow the destination directory’s access rules.
  • Bug Fixes

    • Improved validation rejects unsafe Windows paths, reserved names, invalid characters, and traversal attempts.
  • Documentation

    • Updated artifact download and security documentation to describe Linux and Windows behavior, while macOS and BSD remain unsupported.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: cd4bbd0d-16dd-4fdb-9ed0-32fd9acc80f0

📥 Commits

Reviewing files that changed from the base of the PR and between 3ee2b1e and 3486a73.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • docs/artifact-exchange.md
  • docs/configuration.md
  • docs/security.md
  • package.json
  • pnpm-workspace.yaml
  • src/artifact-destination-windows.ts
  • src/artifact-download.test.ts
  • src/artifact-path-windows.ts
  • src/artifact-tools.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Windows now supports native artifact downloads. The change adds Windows path validation, native directory-handle protection, platform-specific integration, tests, runtime bindings, and documentation updates. Linux behavior remains descriptor-based. macOS and BSD remain unsupported.

Changes

Windows artifact downloads

Layer / File(s) Summary
Windows path validation
src/artifact-path-windows.ts, src/artifact-download.test.ts
Windows-relative artifact paths reject traversal, device names, streams, invalid characters, and unsafe separators. Valid paths are normalized and tested.
Windows destination handles
src/artifact-destination-windows.ts, package.json, pnpm-workspace.yaml
Destination directories are created and pinned with kernel32.dll handles. Reparse points and non-directories are rejected. koffi provides the native bindings.
Artifact download integration
src/artifact-tools.ts
Windows uses the new path parser and destination preparation flow. Linux continues to use descriptor-anchored paths.
Windows coverage and documentation
src/artifact-download.test.ts, docs/artifact-exchange.md, docs/configuration.md, docs/security.md
Tests cover Windows junctions, validation, regular files, and platform permissions. Documentation describes Windows and Linux behavior separately.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ArtifactDownload
  participant ArtifactTools
  participant WindowsDestination
  participant WindowsFilesystem
  ArtifactDownload->>ArtifactTools: request artifact download
  ArtifactTools->>WindowsDestination: prepare validated destination
  WindowsDestination->>WindowsFilesystem: pin directory handles and reject reparse points
  WindowsFilesystem-->>WindowsDestination: validated destination handle
  WindowsDestination-->>ArtifactTools: destination handle and anchor path
  ArtifactTools-->>ArtifactDownload: write and publish artifact
Loading

Suggested reviewers: waishnav

Merge Risk: ⚪ Minimal · up to 3486a

Windows artifact downloads add validated paths and protected destination handling without an identified merge-blocking regression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. (5 skipped: 5… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: adding native artifact-download support on Windows.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks each path with care
Native handles guard the lair
Junctions fail, safe files remain
Windows joins the download train
Linux keeps its anchored way
Secure artifacts hop today

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown

Greptile Summary

This change enables native artifact downloads on Windows, adds Windows-specific safe-path validation, and pins workspace and destination directories before using the existing streaming, integrity verification, stale-partial cleanup, and hard-link publication flow.

Confidence Score: 5/5

Safe to merge: no blocking or non-blocking defects were established.

The focused artifact checks passed, including safe-path rejection, real filesystem publication, traversal rejection, type checking, and the artifact download regression test.

Files Needing Attention: None.

T-Rex T-Rex Logs

What T-Rex did

  • Executed a focused artifact-validation harness covering portable Windows path validation, safe artifact publication into a temporary workspace, and rejection of traversal paths.
  • Confirmed Windows directory-pinning rejects reparse points and omits delete sharing while path-based operations run below the pinned anchor.
  • Ran the TypeScript type check and focused artifact-download regression test and verified success.
  • Reviewed the Windows-specific validation assets and outputs, including the before/after validation scripts and regression logs, and noted the exact source lines examined.
  • Noted that native kernel32.dll handle calls require a Windows host and were not executed in the Linux environment.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "feat: support native artifact downloads ..." | Re-trigger Greptile

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.

1 participant