zhangning21 commented on code in PR #19075:
URL: https://github.com/apache/nuttx/pull/19075#discussion_r3385497668
##########
.github/workflows/build.yml:
##########
@@ -45,9 +45,12 @@ jobs:
- name: Determine Target Branches
id: gittargets
shell: bash
+ env:
+ PR_BODY: ${{ github.event.pull_request.body }}
Review Comment:
Thanks for raising this. Yes, the PR body is untrusted input, so we need to
be careful here.
The reason we pass the body through an env: variable instead of inlining ${{
github.event.pull_request.body }} directly into the run: script is precisely to
avoid script injection. This is the mitigation GitHub documents in Security
hardening for GitHub Actions
(https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-an-intermediate-environment-variable):
│
│ - With env:, the body is delivered to the step as the value of an
environment variable. It is never substituted into the script source, so shell
metacharacters in the body (`, $(
), ;, &&, …) are treated as literal data, not executed.
│ - The dangerous form would be inlining it directly, e.g. run: echo "${{
github.event.pull_request.body }}", where a body like "; rm -rf / # would be
injected into the script text.
We deliberately do not do that.
│
│ On top of the env: indirection, the body is only ever consumed as quoted
data by text-processing tools — echo "$PR_BODY" | grep -oE … — never eval'd or
executed. The values we extract are further constrained:
│ - dependencies must match a fixed regex
(\?:https://github.com/)?apache/nuttx(\?:-apps)?/pull/[0-9]+;
│ - the repo is checked against a 2-entry allow-list (apache/nuttx,
apache/nuttx-apps);
│ - the PR number is [0-9]+ only, so the later git fetch origin
"pull/${DEP_PR_NUM}/head:…" can't be abused.
│
│ Finally, this workflow runs on pull_request (not pull_request_target),
so the job has a read-only GITHUB_TOKEN and no secrets — the blast radius is
minimal even in the worst case.
│
│ If helpful, I can switch the echo "$PR_BODY" calls to printf '%s\n'
"$PR_BODY" for slightly more robust handling of arbitrary text (purely a
robustness nicety, not a security fix).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
