Quality hardening: shell safety, injection hardening, functional fixes

[dcbickfo]

Summary

• S0 — Functional fixes: Add if: always() to artifact upload steps so test reports and coverage upload even when tests fail; fix restore-keys prefixes in Go and Ruby dep caches (were using full hash, defeating the fallback); fix test.yml workflow to pass job_status instead of bogus milliseconds input
• S1 — Shell safety: Replace eval-based command construction in deploy/eks with a bash array (cmd=(...) + "${cmd[@]}") — eliminates word-splitting and shell injection risk
• S2 — Injection hardening: Move all ${{ inputs.* }} and ${{ github.* }} expressions out of run: blocks and into env: vars across all actions — prevents expression injection if untrusted input reaches a run step; also updates test.sh to read TEST_TAGS/TEST_TIMEOUT/TEST_PARALLEL from env instead of positional args

Test plan

Validated against consumer repos on test/quality-hardening-actions branches with internal refs pointing at this branch:

• ✅ geo-admin (Ruby): all jobs passed — rspec, lint, validate-schemas, validate-topics, one-for-all; artifacts uploaded (test-report, coverage, lint-results) — https://github.com/wishabi/geo-admin/actions/runs/23816934559
• ✅ inventory-api (Go): lint and build-and-test passed; artifacts uploaded (coverage, test-report) — https://github.com/wishabi/inventory-api/actions/runs/23816937147

Notes

• No inputs renamed or removed — fully backward compatible
• The TESTING commit (internal refs → branch) and its revert are included in the branch history but net to zero change; consider squashing if desired before merge

[Copilot]

Pull request overview

Hardens CI/CD composite actions and workflows by improving artifact reliability, reducing cache misses, and mitigating shell/expression injection risks.

Changes:

• Ensure key artifacts (test reports, coverage, lint results) upload even when jobs fail (if: always()).
• Fix dependency cache restore-keys prefixes so fallback restores actually work (Go/Ruby).
• Reduce injection/word-splitting risk by moving GitHub expressions into env: and improving shell command construction (notably EKS deploy).

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ruby/validation/topic/action.yml	Move inputs.* usage into env and quote variables in run commands.
ruby/validation/schema/action.yml	Pass ENV via env and quote usage in run.
ruby/test/action.yml	Fix coverage path rewrite to use shell env var; always upload coverage/test artifacts.
ruby/lint/action.yml	Always upload rubocop results artifact.
ruby/deps/action.yml	Fix Bundler cache restore-key prefix for effective fallback restores.
go/test/test.sh	Switch from positional args to env-driven configuration (tags/timeout/parallel).
go/test/action.yml	Pass test configuration via env; always upload test/coverage artifacts.
go/smoke-test/action.yml	Use env var for rollback deploy environment to avoid inline expressions in run.
go/lint/action.yml	Move workspace/version inputs into env before use in shell.
go/deps/action.yml	Fix vendor cache restore-key prefix; move mockery version input into env.
go/configure/action.yml	Move repo token into env for safer shell interpolation.
deploy/lambda/go/action.yml	Use $GITHUB_WORKSPACE and move inputs into env for safer shell usage.
deploy/eks/action.yml	Replace eval command construction with a bash array; move inputs into env.
common/branch-info/action.yml	Move GitHub context expressions into env and use shell vars in run.
.github/workflows/test.yml	Fix workflow to pass required job_status input.
.github/workflows/main-version-update.yml	Move workflow-dispatch inputs into env and quote in git commands.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

go/test/test.sh still builds a shell command as a single string and later executes it via bash -c. Since TAGS/TIMEOUT/PARALLEL come from action inputs, this leaves room for shell injection/word-splitting (e.g., if tags contain spaces or shell metacharacters). Prefer constructing the go test invocation as a bash array and executing it directly (similar to the EKS deploy change), so input values are passed as arguments rather than re-parsed by the shell.

[Copilot]

This PR’s description says all ${{ inputs.* }} / ${{ github.* }} expressions were moved out of run: blocks for injection hardening, but this composite action still contains such expressions in run: steps (e.g., the later summary steps referencing ${{ job.status }} / ${{ env.* }}). To fully achieve the stated hardening goal, move those contexts into env: entries and reference them as shell variables inside the scripts.

[Copilot]

This PR’s description calls out moving ${{ inputs.* }} / ${{ github.* }} expressions out of run: blocks, but this action still has a run: step earlier (“Write smoke test summary”) that embeds ${{ inputs.* }} and ${{ job.status }} directly in the shell script. Consider applying the same pattern used elsewhere in this PR: pass these values via env: for that step and reference them as $VARS inside the run: script.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chriskarlin]

LGTM

claude came back with these mintor suggestion:

Should Fix

  - go/test/test.sh:6 — The default for PARALLEL changed from empty (${3:-}) to "true" (${TEST_PARALLEL:-true}). This is safe today because action.yml declares default: "true" for the input, so TEST_PARALLEL is never unset when called through the action. But if someone runs test.sh standalone (or another action
  sets TEST_PARALLEL=""), the behavior silently changes — previously no -p flag, now it skips -p 1. Consider using ${TEST_PARALLEL:-} to keep the script's standalone default consistent with the old behavior, and let the action.yml default drive the "true" value. Minor, but reduces surprise.
  - go/deps/action.yml:63-64 — The env var is named MOCKERY_VERSION but the local variable inside the script is MOCKERY_INSTALL_VERSION. This works, but the shadowing is confusing — you read MOCKERY_VERSION from env, then assign it to a local with a different name. Consider naming the env var
  MOCKERY_INSTALL_VERSION to match, or simplify the script to just use MOCKERY_VERSION directly.

  Nits

  - deploy/eks/action.yml:74 — $GITHUB_WORKSPACE is used directly (not via env: mapping) while every other expression in this PR is explicitly mapped. Consistent since GITHUB_WORKSPACE is a built-in env var set by the runner, but worth a comment or being consistent with the pattern if you want to be thorough. Same
  in deploy/lambda/go/action.yml:19.