Draft
actions: harden inputs against injection (ci/request, gcs/sync, env/summary, run, retest)#5185
Conversation
✅ Deploy Preview for nifty-bassi-e26446 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Copilot <copilot@github.com> Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Signed-off-by: Copilot <copilot@github.com> Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Harden GitHub Actions for envoyproxy/toolshed
actions: harden inputs against injection (ci/request, gcs/sync, env/summary, run, retest)
Sep 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of envoyproxy/envoy-ci-staging#2075
These actions run in a
pull_request_target/workflow_runCI system where several inputs are attacker-influenced (fork PR title/body, PR-editable.github/config.yml, comment bodies, artefacts from jobs that ran PR code), while the steps consuming them hold credentials (GitHub App tokens, GCP SA keys). The recurring bug class is splicing${{ }}expressions directly intorun:scripts or jqfilter:programs. Everything here now goes throughenv:(shell:"$VAR", jq:env.VAR) orinput:JSON.actions/gcs/artefact/sync/env:; newvalidate.shgatesbucket/sha/redirectbefore anygcloudcallactions/github/env/summary/action.ymltables/actor/icon/event-name/linked-title/repositoryviaenv:;titleviatoJSON()actions/envoy/ci/request/action.ymlenv:+fromjsonactions/envoy/ci/request/action.yml,jq/validate.jqactions/envoy/ci/request/action.ymltrusted_bots→trusted-botsactions/github/run/action.ymlargs/command/container-commandviaenv:; scopedenvsubstactions/retest/,js/retest/retest.tsargsallow-listed; dropped token loggcs/artefact/sync(P0)bucket/sha/redirectwere interpolated unquoted into thegcloud storage rsync --delete-unmatched-destination-objectsdestination, and in Envoy CI they come fromgcs-metadata.jsoninside an artefact produced by an untrusted job. All inputs now travel viaenv:and are quoted; a newvalidate.shruns first and fails with::error::on anything not matching the expected shapes (defence in depth —_upload_gcs.ymlalso validates).github/env/summaryThe generated table filter and
inputs.actorwere spliced into the jq program, and the PR title/body flows through this step intoGITHUB_STEP_SUMMARYand into theenvartefact every downstream check consumes.titleis passed astoJSON(inputs.title)so it can only ever be a JSON string value.envoy/ci/requestconfig/changed/request/version/build-imagesoutputs are nowenv:+fromjson. These are read from the target-branch checkout, which is what made the old form safe; the invariant is now stated in a comment rather than being load-bearing.build-image.tag/shafrom PR-editable.github/config.ymlbut not the repository, so a PR could pointENVOY_BUILD_IMAGE(and the docker cache job, which runs with the mutex app key) at an arbitrary registry. Every resolved image —default,mobile, and thebuild-image-currentequivalents — is checked against a newbuild-image-reposinput (newline-separated, defaults todocker.io/envoyproxy/andgcr.io/envoy-ci/) via a newvalidate::image_prefixjq helper. Failures name the offending image and the allowed prefixes.trusted_bots: the action declaredtrusted_botsbut readinputs.trusted-bots, so the value was always empty and theNOBOTSfallback always won. Renamed to kebab-case withtrusted_botsretained as a deprecated alias, plus a step that fails if the resolved list is empty.github/runargs/command/container-commandmoved toenv:.envsubstwas expanding the whole environment into the command line; it now takes an explicit shell-format built from a newenvsubst-varsinput (defaulting to the variables Envoy's_run.yml/_mobile_container_ci.ymlactually rely on, e.g.PWD,TMP_ENTRYPOINT,TMP_REPORT), with each name validated as an identifier.retestargs(derived from a/retestcomment body, which anyone can post) was declared but never read. It is now parsed and validated against/^[A-Za-z0-9][A-Za-z0-9._/-]*$/, rejecting anything else with a clear error. Also removes aconsole.log(token)that printed the GitHub token into the job log. Trigger and author-association gating are deliberately untouched.Behaviour change
trusted-botsis now actually honoured, sotrigger-release-envoy[bot]becomes trusted where callers pass it. The new "resolved value must be non-empty" assertion will fail for a caller that intentionally passes an empty string — such callers should passNOBOTSexplicitly.Tests
Negative cases for both new validations plus injection coverage:
actions/gcs/artefact/sync/tests/(with agcloudmock),actions/github/env/summary/tests/injection.test.yml,jq/tests/validate/image-prefix-*.test.yml, andparseArgscases injs/retest/__tests__/retest.test.ts.Open questions for review
bucketregex disallows/; no in-repo caller passes a path, but worth confirming against downstream callers.js/retest/dist/index.jsis regenerated as required by the JS CI check.