fix: reject generated directory symlinks - #169
Open
sylvesterkaczmarek wants to merge 5 commits into
Open
Conversation
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
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
Make Fence's generated-path helpers fail closed instead of traversing symbolic-link components below their approved generated roots.
Problem
Fence restricts generated-directory management to reviewed repo-generated paths,
RUNNER_TEMP, orTMPDIR, but the base helper validates only the literal path string. A path can therefore satisfy the approved prefix check while one of its filesystem components is a symlink to a different location.For
clear_generated_dir, this is especially subtle. If the final managed path is itself a symlink, GNUfinddoes not follow that command-line symlink in the current invocation, so the clear can return successfully. Later writes such ascp "$binary" "$dist_dir/$name"or coverage output creation then follow the symlink and land outside the approved generated root. The same escape is possible when an intermediate component is a symlink.remove_generated_pathhas a related intermediate-component case: removing a final symlink is safe becauserm -rfremoves the link itself, but a path such as<symlink-parent>/childtraverses the parent symlink beforermreaches the child.Evidence / reproduction
script/lib/common.bash::require_generated_pathchecks the literal path against approved prefixes but does not inspect filesystem components.script/buildcallsclear_generated_dirfor its distribution directory before copying release artifacts into that path.script/test --coveragecalls the same helper before writing coverage outputs.$DIR/target/generated-linkas a symlink to another directory, callclear_generated_diron it, then writegenerated-link/file. The clear can return without clearing the target, while the subsequent write lands in the symlink target.$DIR/target/generated-link/nestedas the managed path. The literal path remains under$DIR/target, but filesystem resolution crossesgenerated-linkbefore reachingnested.remove_generated_path "$link/victim"can otherwise address a file in the symlink target, even thoughremove_generated_path "$link"itself is safe.script/test-generated-pathsnow covers all three boundaries: final-link clearing is rejected, intermediate-link clearing/removal is rejected, and deleting the final symlink itself remains permitted while its target remains untouched.Change
clear_generated_dirreject both final and intermediate symlink componentsremove_generated_pathreject symlink ancestors while still allowing safe removal of the final symlink itselfscript/testentrypointNo normal generated-directory layout, artifact location, or cleanup behavior changes for ordinary non-symlink paths.