fix: harden dbt review action version lookup and credential write#910
Conversation
Follow-up hardening for the `github/review` composite action from the #900 review. Stacks on #900 (refines its new semver version step). - Authenticate the release-version lookup with `${{ github.token }}` (lifts the unauthenticated 60 req/hr IP limit to 1,000 req/hr) so busy runners aren't throttled into the `latest` fallback. - Skip the binary cache when the version resolves to `latest` (`if: steps.version.outputs.version != 'latest'`), so one rate-limited/offline lookup can't pin a stale binary across all later runs. - Read the hosted API key from the environment inside the `jq` program (`$ENV.IN_ALT_KEY`) instead of passing it via `--arg`, keeping it out of `argv` (visible to other processes; printed under `ACTIONS_STEP_DEBUG`). - Add 4 adversarial tests: auth header present with a token, omitted+safe without one (bash-3.2 empty-array idiom), cache gated on `!= 'latest'`, and the API key absent from the `jq` argv. Closes #909 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Heads-up: this PR was merged into its base branch |
What does this PR do?
Follow-up hardening for the
github/reviewcomposite action, surfaced by the multi-model code review of #900. Three non-blocking refinements to binary-version resolution and the hosted-credential write:github.action_ref— a branch, SHA, ormain) callsreleases/latestunauthenticated (IP-limited to 60 req/hr). It now passes${{ github.token }}, lifting the limit to 1,000 req/hr so busy runners aren't throttled into thelatestfallback.latest. When the version resolves tolatestit was used as a static cache key, so one rate-limited/offline lookup pinned that binary forever. The cache step is now gated withif: steps.version.outputs.version != 'latest'; a resolved semver caches normally,latestfalls through to a fresh install.jqargv. The credential write read the key via--arg key "$IN_ALT_KEY"(visible inargv; printed underACTIONS_STEP_DEBUG→set -x). It now reads from the environment inside the jq program ($ENV.IN_ALT_KEY).Stacks on #900 — it refines the new semver version step that #900 introduces, so the base is
fix/v0.8.4-dbt-review-launch. Merge after #900 (GitHub will retarget tomainwhen #900 merges). None of these are regressions; the golden@vX.Y.Zpath never hits the release-API lookup and is unaffected.Type of change
Issue for this PR
Closes #909
How did you verify your code works?
Added 4 adversarial tests to
release-v0.8.5-adversarial.test.ts(run the action's real shell with fakecurl/jqonPATH):Authorization: Bearer <token>whenGITHUB_TOKENis present;${AUTH[@]+"${AUTH[@]}"}empty-array idiom is safe underset -u, back to bash 3.2);Cache altimate-codestep is gated onsteps.version.outputs.version != 'latest';jqargv (asserts the secret is absent from captured args and the program uses$ENV.IN_ALT_KEY).Local checks (all green):
bun test test/skill/release-v0.8.5-adversarial.test.ts→ 13 pass; plusreview-runner+github-actionsuites → 32 pass total.bun run script/upstream/analyze.ts --markers --base main --strict→ ok (no unmarked changes in upstream-shared files).prettier --checkon the changed test file → clean. Action YAML parses.Checklist
🤖 Generated with Claude Code
Summary by cubic
Hardened the
github/reviewaction for reliable version resolution and safer credential handling. Authenticates the release lookup, avoids cachinglatest, and keeps the hosted API key out of process args.Bug Fixes
GITHUB_TOKENto avoid the 60 req/hr cap and prevent unintendedlatestfallback.latestto avoid pinning a stale binary.Security
$ENV.IN_ALT_KEYinsidejqinstead of--arg, keeping the secret out of argv and debug logs.Written for commit 862f7c8. Summary will update on new commits.