Skip to content

Add support for symlink resolution - #1566

Merged
Jay Bosamiya (Microsoft) (jaybosamiya-ms) merged 1 commit into
mainfrom
jayb/symlink-resolution
Oct 12, 2026
Merged

Jay Bosamiya (Microsoft) (jaybosamiya-ms) merged 1 commit into
mainfrom
jayb/symlink-resolution

Conversation

@jaybosamiya-ms

Copy link
Copy Markdown
Member

This PR introduces initial support for symlinks to our file system. Specifically, it introduces symlink resolution and support for initializing the in-memory file system with symlinks. Future PRs will introduce more support, such as the guest being able to create symlinks, and other backends supporting symlinks.

@jaybosamiya-ms
Jay Bosamiya (Microsoft) (jaybosamiya-ms) marked this pull request as ready for review October 10, 2026 00:13
@wdcui

Copy link
Copy Markdown
Member

Agents found the following issue:

 stat  on  /proc/self/fd/  no longer works for paths written in any non-canonical form

Where:  litebox_shim_linux/src/syscalls/file.rs 

•  do_path_stat , line 1530:  path.starts_with("/proc/self/fd/") 
•  do_readlink , lines 1413–1414:  strip_prefix("/proc/self/fd/")  followed by  parse::() 

What changed: before this PR,  do_path_stat  called  pathname.normalized()  before checking for the hardcoded  /proc/self/fd/  prefix. The PR removes that call and now matches the raw path string. Neither caller normalizes the path either:  resolve_path  (used by  stat  and  lstat ) and  FsPath::new  (used by  newfstatat  and  statx ) only add the cwd prefix to relative paths. So the check now sees the path exactly as the guest wrote it.

Effect: any path that reaches the same entry but isn't written in canonical form misses the special case. It then goes to the real filesystem, which has no  /proc/self/fd  entries, and the call fails with  ENOENT  instead of returning the status of  /dev/stdin ,  /dev/stdout  or  /dev/stderr :

┌─────────────────────────────────────┬────────┬───────────────────────────────────────────────────────────────────────┐
│ Call │ Before │ After │
├─────────────────────────────────────┼────────┼───────────────────────────────────────────────────────────────────────┤
│ stat("/proc/self/fd/0") │ ok │ ok │
├─────────────────────────────────────┼────────┼───────────────────────────────────────────────────────────────────────┤
│ stat("/proc//self/fd/0") │ ok │ ENOENT (prefix check fails) │
├─────────────────────────────────────┼────────┼───────────────────────────────────────────────────────────────────────┤
│ stat("/proc/self/fd/./0") │ ok │ ENOENT (passes the prefix check, but parsing "./0" as a number fails) │
├─────────────────────────────────────┼────────┼───────────────────────────────────────────────────────────────────────┤
│ cwd /tmp, stat("../proc/self/fd/0") │ ok │ ENOENT (the path becomes /tmp/../proc/self/fd/0) │
└─────────────────────────────────────┴────────┴───────────────────────────────────────────────────────────────────────┘

The relative-path case is the one a real program is most likely to hit.

Suggested fix: don't just put the old lexical  normalized()  back. Now that symlinks exist,  ..  has to be resolved against the real directories, so collapsing it as text could give the wrong answer. Two options:

  1. Resolve the parent directory ( /proc/self/fd ) with the new symlink-aware resolver, then match the resolved path plus the last component.
  2. Match by path component: split on  / , drop empty and  .  components, and compare against  ["proc", "self", "fd", ] . This option still needs a decision on how to handle  .. .

Either way, please add regression tests for the equivalent spellings above.

Related, but not introduced by this PR:  do_readlink  hits  _ => unimplemented!()  for any fd above 2 ( file.rs:1420 ). That means a guest calling  stat("/proc/self/fd/5")  can panic the shim. It's worth returning an errno, or looking up the real fd, while this code is being changed.

@wdcui

Copy link
Copy Markdown
Member

Two more issues reported by agents:

  1. Composer: a symlink above a nested mount redirects lookups away from that mount

Location:  litebox/src/fs/composer.rs , in the  AncestorOfMount  branch of  walk_directories  and in  read_link_at  (line 597). I set up a root InMem plus a second InMem mounted at  /a/b , which contains  x :

┌────────────────────────┬─────────────────────────────────────────┬─────────────┬────────────────┐
│ /a in the root backend │ open("/a/b/x") │ lstat("/a") │ readlink("/a") │
├────────────────────────┼─────────────────────────────────────────┼─────────────┼────────────────┤
│ directory │ reads the mounted file ✅ │ Directory │ EINVAL │
├────────────────────────┼─────────────────────────────────────────┼─────────────┼────────────────┤
│ regular file │ ENOTDIR │ ENOENT │ EINVAL │
├────────────────────────┼─────────────────────────────────────────┼─────────────┼────────────────┤
│ symlink to /target │ reads /target/b/x from the root backend │ ENOENT │ Ok("/target") │
└────────────────────────┴─────────────────────────────────────────┴─────────────┴────────────────┘

The symlink silently skips the mount, and  readlink  succeeds on a path that  lstat  says doesn't exist. Today's composers only mount at  /  and  /dev , so nothing hits this yet. Suggested fix: apply  checked_child_path  in  read_link_at , and either don't follow a backend symlink in an ancestor-of-mount position or reject that setup.

  1. Nit: wrong errno for  open("missing/", O_CREAT) 

• Before this PR, it created a regular file named  missing .
• It now returns  ENOENT .
• Linux returns  EISDIR  (checked on the host).

@wdcui Weidong Cui (wdcui) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@jaybosamiya-ms

Copy link
Copy Markdown
Member Author

Thanks Weidong. For the issues your agents found:

  1. I had already documented that the /proc/self/fd/* stuff needs to be moved out of the special casing. It used to be special cased previously because we did not have symlinks at all, now that we have them, it is more of a cleanup that goes into a separate PR, I've explicitly chosen for it to not be part of this PR.
  2. This is a weird case that requires the user of the code to actively set things up to break them at initialization; I'm not convinced it is a real bug. Not important imho.
  3. Wrong errno: yep, there is an errors overhaul needed, doing the "correct" error number is kinda annoying with the existing error system. The overhauled version will fix this up.

Thus, no changes to the code due to these. I'll rebase to account for the conflicts and then merge soon.

@github-actions

Copy link
Copy Markdown

🤖 SemverChecks 🤖 ⚠️ Potential breaking API changes detected ⚠️

Click for details
--- failure copy_impl_added: type now implements Copy ---

Description:
A public type now implements Copy, causing non-move closures to capture it by reference instead of moving it.
        ref: https://github.com/rust-lang/rust/issues/100905
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/copy_impl_added.ron

Failed in:
  litebox::fs::FileType in /home/runner/work/litebox/litebox/litebox/src/fs/mod.rs:75

--- failure enum_no_repr_variant_discriminant_changed: enum variant had its discriminant change value ---

Description:
The enum's variant had its discriminant value change. This breaks downstream code that used its value via a numeric cast like `as isize`.
        ref: https://doc.rust-lang.org/reference/items/enumerations.html#assigning-discriminant-values
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/enum_no_repr_variant_discriminant_changed.ron

Failed in:
  variant PathError::NoSuchFileOrDirectory 0 -> 1 in /home/runner/work/litebox/litebox/litebox/src/fs/errors.rs:239
  variant PathError::NoSearchPerms 1 -> 2 in /home/runner/work/litebox/litebox/litebox/src/fs/errors.rs:241
  variant PathError::InvalidPathname 2 -> 3 in /home/runner/work/litebox/litebox/litebox/src/fs/errors.rs:243
  variant PathError::MissingComponent 3 -> 4 in /home/runner/work/litebox/litebox/litebox/src/fs/errors.rs:245
  variant PathError::ComponentNotADirectory 4 -> 5 in /home/runner/work/litebox/litebox/litebox/src/fs/errors.rs:247

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/enum_variant_added.ron

Failed in:
  variant InitialNode:Symlink in /home/runner/work/litebox/litebox/litebox/src/fs/in_mem.rs:264
  variant PathError:TooManySymlinks in /home/runner/work/litebox/litebox/litebox/src/fs/errors.rs:237

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/inherent_method_missing.ron

Failed in:
  Context::resolve, previously in file /home/runner/work/litebox/litebox/target/semver-checks/git-main/3dc77b9da65f00a4e3749a211c4332da353f1cbc/litebox/src/fs/resolver.rs:102

--- failure method_parameter_count_changed: pub method parameter count changed ---

Description:
A publicly-visible method now takes a different number of parameters, not counting the receiver (self) parameter.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#fn-change-arity
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/method_parameter_count_changed.ron

Failed in:
  litebox::fs::resolver::Resolver::file_status takes 2 parameters in /home/runner/work/litebox/litebox/target/semver-checks/git-main/3dc77b9da65f00a4e3749a211c4332da353f1cbc/litebox/src/fs/resolver.rs:981, but now takes 3 parameters in /home/runner/work/litebox/litebox/litebox/src/fs/resolver.rs:1204

Merged via the queue into main with commit abad331 Oct 12, 2026
16 checks passed
@jaybosamiya-ms
Jay Bosamiya (Microsoft) (jaybosamiya-ms) deleted the jayb/symlink-resolution branch October 12, 2026 20:55
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.

2 participants