cherry_picker: validate commit IDs parsed from the issue body (#2816)
### What
Validate the commit IDs that `cherrypick_with_commits.py` parses out of
the issue body, accepting only git object names (7–40 hex characters).
### Why
The on-demand cherry-pick flow reads commit IDs from the `### Commit
IDs` section of the issue body:
```python
issue_body_dict["commits"] = get_middle_text(...).replace(" ", "").split(",")
```
That normalisation strips spaces and splits on commas, but it preserves
a leading dash. Each entry is then passed to git as an argv element:
```python
subprocess.run(["git", "diff-tree", "--no-commit-id", "--name-only", commit_id, "-r", "-m"])
```
Because `commit_id` sits in an argument position, a value such as
`--output=<path>` is interpreted by git as an *option* rather than a
revision. `git diff-tree` honours `--output=<file>` and
creates/truncates that file, so a malformed entry can write to a path on
the runner instead of being rejected as an invalid revision. Relative
paths are accepted, so the target is not confined to the checkout
directory.
The commit IDs come from the issue body, which is user-authored content,
so it seems worth constraining them to values git will only ever treat
as revisions.
The existing `subprocess` calls already use list form without
`shell=True`, so there is no shell involved here — this is purely about
git's own option parsing.
### How
Filter empty entries (so a trailing comma stays tolerated) and require
each remaining entry to match `^[0-9a-fA-F]{7,40}$`. Anything else is
reported back on the issue using the existing `issue_comment` helper,
following the error-reporting style already used in this script, and the
run exits cleanly.
Verified that the pattern accepts short (7) and full (40) hashes in
either case, and rejects `--output=...`, bare options such as `-r`,
over-short values, and non-hex strings.diff --git a/actions/cherry_picker/cherrypick_with_commits.py b/actions/cherry_picker/cherrypick_with_commits.py
index 0cc9219..abe7a1c 100644
--- a/actions/cherry_picker/cherrypick_with_commits.py
+++ b/actions/cherry_picker/cherrypick_with_commits.py
@@ -15,6 +15,35 @@
for commit_index in range(len(issue_body_dict["commits"])):
issue_body_dict["commits"][commit_index] = re.sub(r'https://.*/commit/', "", issue_body_dict["commits"][commit_index])
+# The commit IDs are parsed out of the issue body, which any GitHub user can
+# author or edit. Accept only values that look like git object names, so that a
+# value such as "--output=..." is rejected here instead of reaching git, which
+# would otherwise interpret it as a command-line option rather than a revision.
+COMMIT_ID_PATTERN = re.compile(r"^[0-9a-fA-F]{6,40}$")
+
+issue_body_dict["commits"] = [commit.strip() for commit in issue_body_dict["commits"] if commit.strip()]
+
+if not issue_body_dict["commits"]:
+ issue_comment(
+ milestoned_issue_number,
+ "No commit ID(s) provided in the issue body. Please provide commit hashes to cherry-pick.\ncc: @bazelbuild/triage",
+ input_data["api_repo_name"],
+ input_data["is_prod"],
+ )
+ raise SystemExit(0)
+
+invalid_commits = [commit for commit in issue_body_dict["commits"] if not COMMIT_ID_PATTERN.match(commit)]
+if invalid_commits:
+ invalid_commits_str = ", ".join(f"`{commit}`" for commit in invalid_commits)
+ issue_comment(
+ milestoned_issue_number,
+ f"The following commit ID(s) are not valid git commit hashes: {invalid_commits_str}\n"
+ "Please provide 6-40 character hexadecimal commit hashes.\ncc: @bazelbuild/triage",
+ input_data["api_repo_name"],
+ input_data["is_prod"],
+ )
+ raise SystemExit(0)
+
issue_body_dict["labels"] = get_middle_text(issue_body, team_labels_text["left"], team_labels_text["right"]).replace(" ", "").replace("@", "").split(",")
issue_body_dict["reviewers"] = get_middle_text(issue_body, reviewers_text["left"], reviewers_text["right"]).replace(" ", "").replace("@", "").split(",")