Skip to content

ci: read the !build comment body from env instead of interpolating it - #4389

Open
kobihikri wants to merge 1 commit into
GraphiteEditor:masterfrom
kobihikri:ci/comment-body-via-env
Open

ci: read the !build comment body from env instead of interpolating it#4389
kobihikri wants to merge 1 commit into
GraphiteEditor:masterfrom
kobihikri:ci/comment-body-via-env

Conversation

@kobihikri

Copy link
Copy Markdown

Hi, and thanks for Graphite.

In .github/workflows/comment-!build-commands.yml, the !build comment is read into the shell like this:

run: |
  COMMENT="${{ github.event.comment.body }}"

  # Split into space-separated words
  read -ra WORDS <<< "$COMMENT"

Actions expands ${{ ... }} into the script text before bash runs, so the comment body becomes part of the script rather than a string assigned to COMMENT. A comment body is completely free text, and $(...) and backticks still run inside double quotes — so !build $(...) would execute on the runner instead of being parsed as a target argument.

You have already thought about the security of this workflow, and I want to acknowledge that rather than talk past it. The job is gated on:

github.event.comment.author_association == 'MEMBER'

with a comment explaining exactly why. That is a real control and it means this is not open to the public — it needs an org member. So I am raising this as defence in depth, not as an open door: it matters if a member's account is compromised, and it removes a sharp edge where a member's comment can do something they did not intend.

The change binds the body to an environment variable, which is the fix from GitHub's own hardening guidance:

env:
  COMMENT_BODY: ${{ github.event.comment.body }}
run: |
  COMMENT="$COMMENT_BODY"

Everything downstream is untouched — read -ra WORDS <<< "$COMMENT" and the !build / target / profile parsing all behave exactly as they do today. Two lines added, one changed.

Disclosure: I used AI assistance to help spot this and prepare the change, and I read the workflow and its membership gate myself.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant