Repository navigation
Harden and simplify the Unity Cloud Build trigger action - #2
Merged
Merged
Conversation
Security:
- Pass inputs through `env:` instead of interpolating `${{ inputs.* }}` into
`run:` blocks. Previously a branch name or header containing shell
metacharacters could execute arbitrary commands on the runner.
- Stop writing the authorization header to `$GITHUB_ENV`, which exposed the
credential to every subsequent step in the calling job. It now lives only in
the environment of the single step that needs it, and is masked in logs.
- Stop echoing the raw API response unconditionally; it is emitted only under
debug logging.
Correctness:
- Drop the internal `actions/checkout@v4`. Nothing reads the repository, and it
clobbered the caller's checkout.
- Wire up `platform` and `machine_type_label`, which were documented but never
sent. They now default to empty and are omitted from the payload unless set,
so build target defaults still apply.
- Validate required inputs and the `clean` flag before issuing the request.
- Fail loudly when a 2xx response carries no build ID instead of reporting
`build_id=null` as success.
- Fall back to the raw response body when an error body is not JSON; `jq -r
'.error'` previously produced an empty message.
- Retry only DNS and connection failures. Triggering a build is not idempotent,
so a request that may have reached Unity is never resent.
- Write the response to `$RUNNER_TEMP` rather than the caller's workspace.
- Skip the `apt-get` install when `curl` and `jq` are already present, and
handle runners without `sudo` or without `apt-get`.
- Add a build summary table to the job summary.
Features:
- Add an `api_url` input so a newer API version can be targeted without
changing the action. The response parser accepts both a single-element array
and a bare object.
Tooling and docs:
- Add `scripts/lint-action.sh` and a CI workflow: validates the manifest, checks
outputs reference real step IDs, rejects inputs interpolated into `run:`
blocks, and runs `bash -n` plus `shellcheck` on every embedded script.
- Rewrite the README: remove the stale `commit_sha` reference and the incorrect
claim that the branch's latest SHA is fetched, and document outputs usage,
API versioning and behaviour.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BbivmM23ZX7xoHxbEN79Pw
There was a problem hiding this comment.
Pull request overview
This PR hardens and simplifies the composite GitHub Action that triggers Unity Build Automation (Unity Cloud Build) by reducing injection risk, improving input/response handling, and adding CI linting plus updated documentation.
Changes:
- Refactors
action.ymlto pass inputs viaenv, validate inputs, improve error handling, and add safer dependency installation and logging/summary behavior. - Adds a lint script + CI workflow to validate
action.ymlstructure and lint embeddedrun:scripts. - Rewrites the README to reflect the action’s behavior, inputs/outputs, and API versioning.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
scripts/lint-action.sh |
Adds an action manifest + embedded-script linting helper. |
README.md |
Updates usage, inputs/outputs, API versioning notes, and behavioral guarantees. |
action.yml |
Hardens the trigger step (env-passing, validation, payload wiring, retries, summaries) and simplifies dependencies. |
.github/workflows/ci.yml |
Adds CI to run the new lint script and workflow linting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ss runners - scripts/lint-action.sh: guard the `import yaml` so a missing PyYAML produces an actionable install hint instead of a ModuleNotFoundError traceback. - ci.yml: install `python3-yaml` alongside shellcheck so the lint job does not depend on the runner image happening to ship PyYAML. - ci.yml: pin actionlint to `1.7.12` instead of `latest`, so CI is reproducible and a moving tag cannot change what runs. - action.yml: a non-root runner without `sudo` no longer falls through to a bare `apt-get` permission error. Elevation is resolved up front and the unsupported case reports which of the two conditions failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BbivmM23ZX7xoHxbEN79Pw
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.
Security:
env:instead of interpolating${{ inputs.* }}intorun:blocks. Previously a branch name or header containing shellmetacharacters could execute arbitrary commands on the runner.
$GITHUB_ENV, which exposed thecredential to every subsequent step in the calling job. It now lives only in
the environment of the single step that needs it, and is masked in logs.
debug logging.
Correctness:
actions/checkout@v4. Nothing reads the repository, and itclobbered the caller's checkout.
platformandmachine_type_label, which were documented but neversent. They now default to empty and are omitted from the payload unless set,
so build target defaults still apply.
cleanflag before issuing the request.build_id=nullas success.jq -r '.error'previously produced an empty message.so a request that may have reached Unity is never resent.
$RUNNER_TEMPrather than the caller's workspace.apt-getinstall whencurlandjqare already present, andhandle runners without
sudoor withoutapt-get.Features:
api_urlinput so a newer API version can be targeted withoutchanging the action. The response parser accepts both a single-element array
and a bare object.
Tooling and docs:
scripts/lint-action.shand a CI workflow: validates the manifest, checksoutputs reference real step IDs, rejects inputs interpolated into
run:blocks, and runs
bash -nplusshellcheckon every embedded script.commit_shareference and the incorrectclaim that the branch's latest SHA is fetched, and document outputs usage,
API versioning and behaviour.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01BbivmM23ZX7xoHxbEN79Pw