get-commit-msg: head_commit.author.name/email are interpolated into the run: block while the sibling commit message is env-guarded
- Dominant language
- C++
- Stars
- 20.5k
- Forks
- 2.3k
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 90
Description
`.github/actions/get-commit-msg/action.yml` protects `github.event.head_commit.message` by passing it
through `env:`, and then interpolates four sibling fields of the same object straight into the shell.
```yaml
- name: Find commit message (on push)
if: github.event_name == 'push'
shell: bash
env:
COMMIT_MESSAGE: ${{ github.event.head_commit.message }} # via env — correct
run: |
AUTHOR_NAME="${{ github.event.head_commit.author.name }}" # interpolated
AUTHOR_EMAIL="${{ github.event.head_commit.author.email }}" # interpolated
TSTAMP="${{ github.event.head_commit.timestamp }}" # interpolated
HASH="${{ github.event.head_commit.id }}" # interpolated
```
The `env:` line was added deliberately. #9330 says why:
> "If we assign the message (which might contain quotes) to an environment variable and then echo it, this should prevent the problem of having a double quote (\")."
That reasoning applies to the four lines below it just as much. `author.name` and `author.email` are
git metadata the committer writes themselves — `git commit --author='… <…>'` — and GitHub does not
constrain them. Inside `"…"`, `$( )` and backticks are still evaluated by bash, so a commit authored
as
```
git commit --author='$(id > /tmp/x) ' -m "ordinary looking change"
```
executes during the `push` run of this action rather than being assigned to `AUTHOR_NAME`.
## Why the `push` branch is the one that matters
`postsubmit-main.yml` runs on `push: branches: [main]` and calls this action in three jobs, each of
which then uses `secrets.FILAMENTBOT_TOKEN` — to push to `google/filament-assets`
(`update-sizeguard`), to publish the docs site (`update-docs`), and to merge golden branches
(`update-renderdiff-goldens`). In `update-docs` the token is also passed as a command-line argument:
```yaml
run: |
bash docs_src/build/postsubmit.sh ${COMMIT_HASH} ${GH_TOKEN}
```
`presubmit.yml` calls the same action, but on a pull request it takes the other branch, which reads
the fields with `git log --format=%an` into shell variables — assignment, not evaluation. That half
is fine.
## What I have and have not established
I have **not** run this against Filament's CI, and I am not going to. What I checked is the source on
`main` and the workflow wiring; the reachability question — whether a merged pull request's
`head_commit.author.name` on `main` carries the contributor's raw `--author` string, and how that
differs between squash, rebase and merge-commit — depends on your merge settings and I could not
determine it from outside. If you squash, the author is the PR author and the string is their GitHub
display name; if you rebase, it is the original `--author` text verbatim.
Either way the value is not one your reviewers read as code, because it is not part of the diff.
## A second, smaller thing in the same file
```bash
DELIMITER="EOF_FILE_CONTENT_$(date +%s)"
echo "msg<<$DELIMITER" >> "$GITHUB_OUTPUT"
cat /tmp/commit_msg.txt >> "$GITHUB_OUTPUT"
```
GitHub's guidance is to use a random delimiter here specifically so that the content cannot terminate
the heredoc. A unix timestamp is guessable within a small window, and the content of
`/tmp/commit_msg.txt` is the commit message and author name. The forged outputs would land in two
unquoted interpolations — `presubmit.yml:122`
(`check_bypass.py ${{ steps.get_commit_msg.outputs.hashes }}`) and `:180`
(`presubmit_check.sh ${{ … }}`). I have not demonstrated a working delimiter collision and I am
reporting this as a hardening note rather than as a working attack.
## Suggested fix
Same shape as #9330, applied to the remaining four:
```yaml
env:
COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
AUTHOR_NAME: ${{ github.event.head_commit.author.name }}
AUTHOR_EMAIL: ${{ github.event.head_commit.author.email }}
TSTAMP: ${{ github.event.head_commit.timestamp }}
HASH: ${{ github.event.head_commit.id }}
run: |
echo "commit $HASH" >> /tmp/commit_msg.txt
echo "Author: ${AUTHOR_NAME}<${AUTHOR_EMAIL}>" >> /tmp/commit_msg.txt
…
```
and, for the second item, a random delimiter — `DELIMITER="EOF_$(openssl rand -hex 16)"` or
`$RANDOM$RANDOM` — instead of `$(date +%s)`.
Happy to open a pull request for either or both if that is useful.
Contributor guide
Research direction
Start with .github/actions/get-commit-msg/action.yml and compare its push branch with the env-guarded commit message and the pull-request branch. Review postsubmit-main.yml and presubmit.yml around the cited output uses. Done means the four commit metadata fields are passed through env variables and the output delimiter is unpredictable, without changing the intended commit-message outputs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- bash, github-actions
- Domain
- ci-cd, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100