Check for dangling symlinks in sandbox before copying outputs #28196
Conversation
github-actions
bot
added
team-Local-Exec
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where dangling symbolic links created as outputs in a sandboxed action could lead to non-deterministic build failures. The fix introduces a validation step in AbstractSandboxSpawnRunner.verifyPostCondition that checks all outputs for dangling symlinks after an action completes but before its outputs are copied from the sandbox. This is implemented in a new helper method, SandboxHelpers.validateOutputs. The change is correctly propagated to LinuxSandboxedSpawnRunner by adding a call to the super method. A comprehensive shell test is added to verify the fix and prevent regressions. The changes appear correct and robust, effectively solving the reported issue.
| */ | ||
| public static void validateOutputs(SandboxOutputs outputs, Path sourceRoot) throws IOException { | ||
| for (Entry<PathFragment, PathFragment> output : | ||
| Iterables.concat(outputs.files().entrySet(), outputs.dirs().entrySet())) { |
There was a problem hiding this comment.
If you move this logic into moveOutputs, you could reuse the stat performed there to avoid an additional stat for all outputs, which can be significant.
| Iterables.concat(outputs.files().entrySet(), outputs.dirs().entrySet())) { | ||
| Path source = sourceRoot.getRelative(output.getValue()); | ||
| if (source.isSymbolicLink() && !source.exists()) { | ||
| throw new IOException("declared output '" + output.getKey() + "' is a dangling symbolic link"); |
There was a problem hiding this comment.
Outputs matched to artifacts declared with ctx.declare_symlink could legitimately be dangling.
2 participants
Fixes #3759