fix: reject root directory path traversal in builds worker#12460
Draft
abnegate wants to merge 1 commit into
Draft
fix: reject root directory path traversal in builds worker#12460abnegate wants to merge 1 commit into
abnegate wants to merge 1 commit into
Conversation
Reapplies the ".." rejection that was reverted in 6ed5470 to restore stable builds. The earlier hardening switched to trim('/'), which dropped the leading-dot normalization, so "./" roots resolved to "." and broke every VCS/template build (empty source, npm ENOENT on package.json). This keeps the original rtrim/ltrim normalization ("./" -> "", "./src" -> "src") and layers the traversal check on top: a single leading "./" is collapsed before the check, while deeper or embedded ".." segments are rejected. Extracts the three duplicated blocks into Builds::normalizeRootDirectory() and adds unit coverage for the matrix. Co-Authored-By: Claude Opus 4.7 <[email protected]>
✨ Benchmark resultsComparing 1.9.x (before) to fix-builds-root-directory-traversal (after). Before
After
Delta
Top API waits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reapplies the path-traversal protection on build root directories that was reverted in
6ed54706b3("Restore original ./ stripping", part of #12450) — this time without the regression that took builds down.Background
8e02beff58, refactor: tighten internal validation paths #12413) replacedrtrim('/')→ltrim('.')→ltrim('/')withtrim('/')+ a..rejection.trim($x, '/')strips slashes but not dots, so the leading-dot normalization was lost:./resolved to.and./srcstayed./src. Deployments/templates that storeproviderRootDirectory: "./"then pointed the build at the wrong directory → empty source →npm ENOENT ... package.json. All Sites/template builds broke and the change was reverted, which also removed the..rejection — so the traversal weakness is live again on1.9.x.This change
Extracts a single
Builds::normalizeRootDirectory()used by all three build paths (template, VCS, VCS+template) that:rtrim('/'),ltrim('.'),ltrim('/')— so./→''and./src→src(no build regression)...rejection after normalization, so a single leading./collapses before the check while any residual/embedded..segment is rejected.Invariant: the helper can never return a value containing a
..segment or an absolute path, so a craftedproviderRootDirectorycan't escape the build directory when interpolated into the downstreammkdir/rsync/tar/clone paths../,.,..''(repo root)./src,/src/,src/src../etcetc(single../sandboxed in-tree)../../etc/passwdfoo/../bar,foo/..Test plan
tests/unit/Platform/Modules/Functions/Workers/BuildsTest.php(16 cases) covering the normalize/reject matrix —OKcomposer lint(Pint) — passed1.9.x🤖 Generated with Claude Code