Skip to content

fix(fs): validate mount paths with POSIX semantics on Windows#1492

Merged
chaliy merged 1 commit intomainfrom
claude/fix-mountable-windows-posix-absolute
Apr 30, 2026
Merged

fix(fs): validate mount paths with POSIX semantics on Windows#1492
chaliy merged 1 commit intomainfrom
claude/fix-mountable-windows-posix-absolute

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented Apr 30, 2026

Summary

MountableFs::mount validated paths with Path::is_absolute, which on Windows requires a drive prefix (C:\…) and rejects POSIX-style absolute paths like /workspace. Bashkit's VFS is always POSIX-style on every host, so this broke bash.mount("/workspace", fs) on Windows.

This surfaced after the v0.2.0 release as the publish-only test failure:

vfs › filesystem external roundtrip mounts into bash
Error: io error: mount path must be absolute
  at Bash.mount (...wrapper.ts:855:19)
  at vfs.spec.ts:159

The interop FS feature in #1353 exercised mount() from JS for the first time, and the JS publish workflow runs the full Windows × Node 20/22/24 matrix that PR CI doesn't.

Why this regressed silently

PR CI runs Rust tests on ubuntu-latest only. Windows coverage exists exclusively in publish-js.yml, which only runs on release: published. So the bug rode all the way to the v0.2.0 GitHub Release before it was observed.

Fix

  • Use Path::has_root instead of Path::is_absolute. has_root returns true for any leading / on both Unix and Windows.
  • Move the validation ahead of normalization so the caller's intent (absolute vs. relative) is checked against the original input — normalize_path always returns a rooted path, which would have masked the check entirely had we left it after.
  • Extract the check into a small is_posix_absolute helper so the platform-portability invariant is unit-testable in isolation.

Test coverage

Added in crates/bashkit/src/fs/mountable.rs:

  • test_is_posix_absolute_accepts_root_paths/, /workspace, /data/sub
  • test_is_posix_absolute_rejects_relative_pathsrelative, relative/path, ./foo, ""
  • test_mount_accepts_posix_absolute_path_on_any_host — regression test asserting mount("/workspace", …) and mount("/data/sub", …) succeed (post-fix this passes on Windows; pre-fix it would fail there)
  • test_mount_rejects_relative_path — error path still rejects relative inputs with an absolute message

PR CI verifies the Linux behavior is unchanged. Windows behavior will be verified by the JS publish workflow's vfs.spec.ts once v0.2.1 is cut (or by re-dispatching publish-js.yml against v0.2.0 after this lands on main).

Test plan

  • cargo fmt --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace (84 suites green)
  • CI green on PR
  • After merge: re-dispatch publish-js.yml for v0.2.0 and confirm Windows tests pass + npm publishes

Generated by Claude Code

`MountableFs::mount` previously called `Path::is_absolute`, which on
Windows requires a drive prefix (`C:\…`) and rejects POSIX-style paths
like `/workspace`. Bashkit's VFS is always POSIX-style on every host,
so the platform-aware predicate broke `bash.mount("/workspace", fs)`
on Windows — surfacing as the publish-only test failure
`vfs > filesystem external roundtrip mounts into bash` after the
interop FS feature shipped in 0.2.0.

Switch to `Path::has_root`, which returns true for any leading `/`
on both Unix and Windows. Move the validation ahead of normalization
so the caller's intent (absolute vs. relative) is checked against
the original input rather than the always-rooted normalized form.

Also extract the check into a small `is_posix_absolute` helper so the
behavior is unit-testable without spinning up a full filesystem.

Tests:
- new helper-level tests cover absolute vs. relative paths and the
  edge cases (`/`, empty, `./foo`)
- new integration test asserts `mount("/workspace", ...)` succeeds on
  the local host (proves Linux behavior is unchanged) and documents
  the regression
- new integration test asserts relative paths are still rejected with
  an error containing `absolute`

PR CI is Linux-only for Rust, so this fix is verified on Windows by
the JS publish workflow's full-matrix `vfs.spec.ts` run.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit 5d62f52 Commit Preview URL

Branch Preview URL
Apr 30 2026, 04:13 PM

@chaliy chaliy merged commit 7af3e8d into main Apr 30, 2026
34 checks passed
@chaliy chaliy deleted the claude/fix-mountable-windows-posix-absolute branch April 30, 2026 17:04
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@chaliy chaliy mentioned this pull request Apr 30, 2026
9 tasks
chaliy added a commit that referenced this pull request Apr 30, 2026
Patch release shipping #1492 (Windows mount path validation) and #1493 (rustls switched to ring) so the v0.2.0 publish-pipeline failures are unblocked.
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