Validate module name and version derived from PR paths in bcr_presubmit.py (#2759)
`module_name` and `module_version` in `bcr_presubmit.py` are derived
from a pull request's changed file paths (`modules/<name>/<version>/`)
via `get_target_modules()`, using a regex whose only constraint is "no
slash" (`modules\/([^\/]+)\/([^\/]+)\/`). Both values can therefore
contain arbitrary shell and Starlark metacharacters.
Two sinks interpolate these values unescaped:
- The shell command string built in `add_presubmit_jobs()` (partially
addressed by #2739's `shlex.quote` fix at that one call site).
- The generated `MODULE.bazel` content in `create_anonymous_repo()`:
```python
scratch_file(root, "MODULE.bazel", ["bazel_dep(name = '%s', version =
'%s')" % (module_name, module_version)])
```
A `module_name`/`module_version` containing a single quote closes the
Starlark string literal early, letting the rest of the payload be parsed
as new top-level `MODULE.bazel` statements (e.g. `load(...)` + a
repository rule invocation), evaluated when `bazel vendor` runs during
`anonymous_module_runner`/`test_module_runner`. This sink is unaffected
by #2739, since `shlex.quote` only protects shell-argument parsing, not
the raw string value that later reaches this `%`-format call.
This patch rejects any `module_name`/`module_version` that doesn't match
the character set Bzlmod itself accepts
(https://bazel.build/external/module#module_name, `#version`) at the
point they're derived from the PR path in
`get_target_modules()`/`get_modules_with_metadata_change()`. Validating
at the source closes both the already-partially-fixed shell sink and the
still-open `MODULE.bazel` sink (and any future sink in this file) at a
single choke point, rather than escaping per call site.
Normal module names/versions (e.g. `rules_foo` / `1.2.3`, `protobuf` /
`27.0-rc1`) are unaffected. Anything containing quotes, backticks,
semicolons, dollar signs, spaces, or uppercase letters in the name is
now rejected before it reaches any downstream sink.
1 file changed