Repository navigation
Conversation
|
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)
08a4490 to
f9286f6
Compare
|
Hi, friendly ping. This fixes command injection in shell.remove_directory: FUZZ_INPUTS can be overridden via additional_vars in project.yaml and reaches os.system() in the trusted coordinator cleanup path, not just the sandboxed worker. CLA is passing, but the PR has been open about a month and three checks are blocked on maintainer action: Cloud Build needs /gcbrun, and the CodeQL and “Run basic tests” workflows are awaiting approval. Could someone approve and trigger them, and take a look when you have time? Happy to adjust anything. |
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)