Skip to content

fix: harden dbt review action version lookup and credential write#910

Merged
anandgupta42 merged 1 commit into
fix/v0.8.4-dbt-review-launchfrom
fix/dbt-review-action-version-hardening
Jun 6, 2026
Merged

fix: harden dbt review action version lookup and credential write#910
anandgupta42 merged 1 commit into
fix/v0.8.4-dbt-review-launchfrom
fix/dbt-review-action-version-hardening

Conversation

@anandgupta42

@anandgupta42 anandgupta42 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Follow-up hardening for the github/review composite action, surfaced by the multi-model code review of #900. Three non-blocking refinements to binary-version resolution and the hosted-credential write:

  • Authenticate the release-version lookup. The fallback path (non-semver github.action_ref — a branch, SHA, or main) calls releases/latest unauthenticated (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 the latest fallback.
  • Never cache a floating latest. When the version resolves to latest it was used as a static cache key, so one rate-limited/offline lookup pinned that binary forever. The cache step is now gated with if: steps.version.outputs.version != 'latest'; a resolved semver caches normally, latest falls through to a fresh install.
  • Keep the hosted API key out of jq argv. The credential write read the key via --arg key "$IN_ALT_KEY" (visible in argv; printed under ACTIONS_STEP_DEBUGset -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 to main when #900 merges). None of these are regressions; the golden @vX.Y.Z path never hits the release-API lookup and is unaffected.

Type of change

  • Bug fix / hardening (non-breaking change)
  • New feature
  • Breaking change
  • Documentation

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 fake curl/jq on PATH):

  • the release lookup attaches Authorization: Bearer <token> when GITHUB_TOKEN is present;
  • it omits the header and still resolves the version when no token is set (verifies the ${AUTH[@]+"${AUTH[@]}"} empty-array idiom is safe under set -u, back to bash 3.2);
  • the Cache altimate-code step is gated on steps.version.outputs.version != 'latest';
  • the hosted-credential write keeps the API key out of the jq argv (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; plus review-runner + github-action suites → 32 pass total.
  • bun run script/upstream/analyze.ts --markers --base main --strict → ok (no unmarked changes in upstream-shared files).
  • prettier --check on the changed test file → clean. Action YAML parses.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (CHANGELOG)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code


Summary by cubic

Hardened the github/review action for reliable version resolution and safer credential handling. Authenticates the release lookup, avoids caching latest, and keeps the hosted API key out of process args.

  • Bug Fixes

    • Authenticate the release lookup with GITHUB_TOKEN to avoid the 60 req/hr cap and prevent unintended latest fallback.
    • Skip caching when the resolved version is latest to avoid pinning a stale binary.
  • Security

    • Read the hosted API key via $ENV.IN_ALT_KEY inside jq instead of --arg, keeping the secret out of argv and debug logs.

Written for commit 862f7c8. Summary will update on new commits.

Review in cubic

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>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b67a19a4-117d-4667-b1b8-2e687aa9f06e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dbt-review-action-version-hardening

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.

❤️ Share

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Re-trigger cubic

@anandgupta42 anandgupta42 merged commit 7318c3a into fix/v0.8.4-dbt-review-launch Jun 6, 2026
10 checks passed
@anandgupta42

Copy link
Copy Markdown
Contributor Author

Heads-up: this PR was merged into its base branch fix/v0.8.4-dbt-review-launch rather than main. By that point #900 had already squash-merged to main, so these hardening changes never reached main — they're stranded on the now-orphaned #900 branch. Re-landed against main in #911 (cherry-picked cleanly, same commit + tests). Tracking issue #909 reopened until #911 merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant