Skip to content

ci(smoke-test): pass workflow inputs through env vars#173

Merged
wyattjoh merged 1 commit intomainfrom
security/smoke-test-injection
Apr 16, 2026
Merged

ci(smoke-test): pass workflow inputs through env vars#173
wyattjoh merged 1 commit intomainfrom
security/smoke-test-injection

Conversation

@wyattjoh
Copy link
Copy Markdown
Contributor

@wyattjoh wyattjoh commented Apr 15, 2026

Summary

  • Move inputs.preset, inputs.version, matrix.docker, and matrix.bin from inline ${{ ... }} template substitution in run: blocks to env: bindings, so shell interpolation happens after expansion rather than before.
  • Defense-in-depth: the upstream gate in release.yml already restricts issue_comment triggers to MEMBER/OWNER actors, but the pattern is cheap to follow and eliminates the rule entirely.
  • Closes CodeQL actions/code-injection/critical alert on the smoke-test version check.

Test plan

  • Smoke-test workflow runs green against an existing release (or canary) job

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 15, 2026

⚠️ No Changeset found

Latest commit: ac671af

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@wyattjoh
Copy link
Copy Markdown
Contributor Author

wyattjoh commented Apr 15, 2026

@wyattjoh wyattjoh force-pushed the security/smoke-test-injection branch from 194fa04 to cc1585b Compare April 15, 2026 22:47
@wyattjoh wyattjoh force-pushed the security/workflow-permissions branch from 68c54c5 to 94620e0 Compare April 15, 2026 22:47
@wyattjoh wyattjoh force-pushed the security/smoke-test-injection branch from cc1585b to e2a4f3f Compare April 15, 2026 23:26
@wyattjoh wyattjoh force-pushed the security/workflow-permissions branch from 94620e0 to 7bfbe2a Compare April 15, 2026 23:26
Base automatically changed from security/workflow-permissions to main April 16, 2026 19:46
@wyattjoh wyattjoh force-pushed the security/smoke-test-injection branch from e2a4f3f to 1c2041e Compare April 16, 2026 20:04
@wyattjoh wyattjoh marked this pull request as ready for review April 16, 2026 20:04
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad2a2a50-2acb-4e2a-9d7b-7bd16c509e2d

📥 Commits

Reviewing files that changed from the base of the PR and between 1c2041e and ac671af.

📒 Files selected for processing (1)
  • .github/workflows/smoke-test.yml
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/smoke-test.yml

📝 Walkthrough

Walkthrough

The GitHub Actions workflow .github/workflows/smoke-test.yml was modified to export preset/version/container/bin values into bash via environment variables instead of embedding ${{ ... }} directly in run scripts. In resolve-matrix, PRESET is assigned from inputs.preset, the case switches on "$PRESET", and the unknown-preset error reports $PRESET. In smoke-test, EXPECTED, DOCKER_IMAGE, and BIN are set from inputs/matrix values; script branches either run docker run "$DOCKER_IMAGE" "/work/$BIN --version" or execute "$BIN" --version, and version messages/errors reference $EXPECTED.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: moving workflow inputs from inline template substitution to environment variables in the smoke-test CI workflow.
Description check ✅ Passed The description clearly relates to the changeset, explaining the specific refactoring of how workflow inputs are passed and the security improvement (CodeQL alert closure) it achieves.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Move `inputs.preset`, `inputs.version`, `matrix.docker`, and `matrix.bin`
into `env:` blocks so shell interpolation happens after variable
expansion rather than template substitution. Closes the CodeQL
actions/code-injection/critical finding on the version comparison.

The values are already gated upstream (release.yml requires
author_association in MEMBER/OWNER for issue_comment triggers), so this
is defense-in-depth, but the pattern is cheap to follow and eliminates
the rule entirely.
@wyattjoh wyattjoh force-pushed the security/smoke-test-injection branch from 1c2041e to ac671af Compare April 16, 2026 23:08
@wyattjoh wyattjoh merged commit 2023b5a into main Apr 16, 2026
10 checks passed
@wyattjoh wyattjoh deleted the security/smoke-test-injection branch April 16, 2026 23:14
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.

2 participants