Skip to content

fix(realfs): use no-follow resolver for stat/read_link/remove#1584

Merged
chaliy merged 1 commit into
mainfrom
claude/fix-1578-realfs-no-follow
May 7, 2026
Merged

fix(realfs): use no-follow resolver for stat/read_link/remove#1584
chaliy merged 1 commit into
mainfrom
claude/fix-1578-realfs-no-follow

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented May 7, 2026

Closes #1578.

Summary

RealFs::resolve canonicalizes any existing path, so a final symlink component was always followed before the operation ran. The resulting bugs:

  • stat('/link') returned the target's metadata, not the symlink's.
  • read_link('/link') tried readlink on the canonicalized target and failed.
  • remove('/link', recursive=true) reached remove_dir_all on the target — on read-write mounts that could wipe an arbitrary directory tree (the dangling -> /outside-dir case is the worst).

Fix

  • Add resolve_no_follow that canonicalizes only the parent (so symlink hops in any parent component are still verified), checks containment under the mount root, then appends the basename verbatim.
  • Switch stat, read_link, and remove to the new resolver.
  • remove now branches on symlink_metadata so a symlink is unlinked with remove_file even when recursive=true.

Tests

Four new regression tests in realfs::tests:

  • stat_describes_symlink_not_target
  • read_link_returns_target_for_link_path
  • remove_unlinks_symlink_without_following (verifies the target tree is intact)
  • remove_symlink_with_recursive_flag_outside_target_intact (link points outside root, target stays untouched)

Test plan

  • cargo test -p bashkit --lib --features realfs fs::realfs (32/32 passing)
  • cargo test -p bashkit --features realfs --test realfs_tests (35/35 passing)
  • cargo fmt --all -- --check
  • cargo clippy -p bashkit --features realfs,ssh --all-targets -- -D warnings

Generated by Claude Code

`RealFs::resolve` canonicalizes any existing path, so a final symlink
component was always followed. As a result:

- stat('/link') returned the target's metadata
- read_link('/link') tried readlink on the target and failed
- remove('/link', recursive=true) called remove_dir_all on the target,
  which on read-write mounts could wipe an arbitrary directory tree

Add `resolve_no_follow` that canonicalizes only the parent, validates
containment under the mount root, and appends the basename verbatim.
Switch stat/read_link/remove to it. `remove` now also branches on
symlink_metadata so a symlink is unlinked even when `recursive=true`.

Add four regression tests in realfs.rs covering each operation,
including a check that removing a symlink to an outside path leaves
the outside tree untouched.

Closes #1578
@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 0cadb87 Commit Preview URL

Branch Preview URL
May 07 2026, 04:10 AM

@chaliy chaliy merged commit dee455e into main May 7, 2026
29 checks passed
@chaliy chaliy deleted the claude/fix-1578-realfs-no-follow branch May 7, 2026 06:50
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.

DeepSec: RealFs dereferences final symlinks for stat/readlink/remove

1 participant