Skip to content

Fix command injection in shell.remove_directory via FUZZ_INPUTS - #5540

Open
a1itkd wants to merge 1 commit into
google:masterfrom
a1itkd:fix-fuzz-inputs-command-injection
Open

a1itkd wants to merge 1 commit into
google:masterfrom
a1itkd:fix-fuzz-inputs-command-injection

Conversation

@a1itkd

@a1itkd a1itkd commented Oct 10, 2026

Copy link
Copy Markdown

project.yaml's additional_vars can set FUZZ_INPUTS, which was previously interpolated directly into a shell string via os.system(). Command substitution ($(...)) inside the directory path executes on the trusted task coordinator, not a sandboxed worker, since this runs before any worker RPC in the cleanup path.

Switch to subprocess.run() with an argv list instead of a shell string, so the path can never be interpreted as shell syntax regardless of its contents. Preserves existing behavior (including the Windows branch and the shutil.rmtree fallback for files native deletion can't remove).

Adds a regression test asserting a path containing shell metacharacters reaches subprocess.run() as a literal argv element.

Ref: issuetracker.google.com/issues/569090261 (reported via OSS VRP, ruled out of scope due to the Oct 1 2026 policy change restricting product-vulnerability submissions - this patch is submitted via the contribution path instead)

@a1itkd
a1itkd requested a review from a team as a code owner October 10, 2026 21:46
@google-cla

google-cla Bot commented Oct 10, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

project.yaml's additional_vars can set FUZZ_INPUTS, which was previously
interpolated directly into a shell string via os.system(). Command
substitution ($(...)) inside the directory path executes on the trusted
task coordinator, not a sandboxed worker, since this runs before any
worker RPC in the cleanup path.

Switch to subprocess.run() with an argv list instead of a shell string,
so the path can never be interpreted as shell syntax regardless of its
contents. Preserves existing behavior (including the Windows branch and
the shutil.rmtree fallback for files native deletion can't remove).

Adds a regression test asserting a path containing shell metacharacters
reaches subprocess.run() as a literal argv element.

Ref: issuetracker.google.com/issues/569090261 (reported via OSS VRP,
ruled out of scope due to the Oct 1 2026 policy change restricting
product-vulnerability submissions - this patch is submitted via the
contribution path instead)
@a1itkd
a1itkd force-pushed the fix-fuzz-inputs-command-injection branch from 08a4490 to f9286f6 Compare October 11, 2026 15:27

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.

2 participants