Skip to content

pkg/fs: resolve path components in order without pre-cleaning - #7749

Open
myangitzh wants to merge 1 commit into
mainfrom
fix/fs-path-resolve
Open

myangitzh wants to merge 1 commit into
mainfrom
fix/fs-path-resolve

Conversation

@myangitzh

Copy link
Copy Markdown
Contributor

Summary

Previously, doResolve called path.Clean(p) at the start and parentDir used path.Dir, which collapsed ., .., and trailing slashes lexically before walking the filesystem. This caused path resolution to diverge from standard POSIX/OS behavior.

PR Fixed

  1. Symlinks followed by .. resolved to the wrong path: path.Clean eliminated <symlink>/.. lexically instead of following the symlink first and then resolving .. from the target directory (e.g., /ln/../f where ln -> x/y resolved to /f instead of /x/f).
  2. Missing intermediate checks before . and ..:
    • Non-existent components before .. (e.g., /missing/..) were silently collapsed instead of returning ENOENT.
    • Regular files before ., .., or a trailing / (e.g., /file/.., /file/., /file/) were not rejected with ENOTDIR.
    • Search permission (MODE_MASK_X) on intermediate directories before .. was bypassed.
  3. Intermediate regular files returned ENOENT/EACCES instead of ENOTDIR: Traversing through a non-directory component (e.g., /file/sub) now checks TypeDirectory before permission/lookup and consistently returns ENOTDIR.
  4. False ELOOP on valid paths: Tracking visited symlink inodes in a set caused valid paths traversing the same directory symlink multiple times (e.g., /ln/../../ln/../f) to fail with ELOOP. Replaced with a symlink hop counter (limit 40).
  5. Subdir restriction bypasses: subdir checks are now enforced step-by-step on the resolved path (allowing ancestor traversal during the walk and requiring the final resolved target to stay within allowed subdirs), preventing escapes via symlinks, .., or stale dentry cache entries.

Signed-off-by: miyang <myang@juicedata.io>

This branch has not been deployed

No deployments
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