Skip to content

fix(services): reject absolute paths in confined_join and prepare_path - #8005

Open
aysha-afrah26 wants to merge 1 commit into
apache:mainfrom
aysha-afrah26:fs-absolute-path-confine
Open

fix(services): reject absolute paths in confined_join and prepare_path#8005
aysha-afrah26 wants to merge 1 commit into
apache:mainfrom
aysha-afrah26:fs-absolute-path-confine

Conversation

@aysha-afrah26

Copy link
Copy Markdown

Which issue does this PR close?

None

Rationale for this change

RFC 7799 specifies the local filesystem root check as rejecting Component::ParentDir, Component::RootDir and Component::Prefix(_), and gives the reason for the last two: they prevent absolute paths from replacing the root via PathBuf::join. The shipped guard matches only the ParentDir arm, so an absolute key walks straight through it and root.join("/etc/passwd") evaluates to /etc/passwd.

Operator::delete_with runs normalize_path first, which strips the leading /, so the single-path API is fine. Deleter does not: Deleter::delete passes the caller's key to the service verbatim. With an Fs operator rooted at a temp dir, op.delete_iter(["<absolute path outside root>"]) deletes the target, and with recursive set it is remove_dir_all. delete_try_iter, delete_stream, delete_try_stream and the Sink impl all reach the same place. On Windows the exposure is not limited to delete: normalize_path only splits on /, so C:/…, a leading \, \\?\… and UNC paths survive it unchanged and are absolute to std::path, which escapes the root on every operation.

What changes are included in this PR?

Adds the two missing component arms to FsCore::confined_join and to the prepare_path copies in compfs and monoiofs, which carry the same guard. The error message loses its parent-dir wording and matches the RFC text, since the check now covers more than ...

Keeping the check inside the join helper means every caller is covered whether or not the path reached it through normalize_path, which is what the Deleter case turns on.

Regression tests go in next to the existing .. ones: test_prepare_path_rejects_absolute_path for compfs and monoiofs, test_confined_join_rejects_escaping_keys for fs, plus an end-to-end fs test that drives delete_iter with an absolute path and asserts the outside file survives. The end-to-end test fails on the current tree.

Are there any user-facing changes?

An absolute key passed to fs, compfs or monoiofs now returns ErrorKind::NotFound instead of resolving outside the configured root. Paths that arrive through the normal Operator methods are unaffected, since normalize_path has already stripped the leading / by then. "/" still resolves to the root itself.

AI Usage Statement

RFC 7799 specifies rejecting RootDir and Prefix alongside ParentDir so an absolute key cannot replace the root through PathBuf::join. Only the ParentDir arm shipped, so a key that skips normalize_path, such as one passed to Deleter, reached the filesystem outside the configured root.
@aysha-afrah26
aysha-afrah26 requested a review from Xuanwo as a code owner July 31, 2026 18:57
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/fix The PR fixes a bug or has a title that begins with "fix" labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant