Fix resolveLatestTag() using an unauthenticated GitHub API call - #120
Conversation
GITHUB_TOKEN/GH_TOKEN are not automatically injected into a custom JS action's process environment - a calling workflow has to set them explicitly via env:, which essentially no consumer had reason to do. So resolving cliVersion: latest hit the GitHub API unauthenticated for effectively every consumer, exhausting the shared 60 req/hour rate limit under any real concurrency (e.g. a multi-version test matrix). Confirmed live via a Mirror Networking Actions run and the identical bug already fixed in game-ci/unity-test-runner#332 and game-ci/unity-builder#852. Adds a githubToken input (default: ${{ github.token }}, populated by Actions on every run with no consumer action needed) and threads it through downloadCli -> resolveLatestTag ahead of the env-var fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe action adds an optional ChangesGitHub token resolution
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant GitHubAction
participant run
participant downloadCli
participant resolveLatestTag
participant GitHubAPI
GitHubAction->>run: Read githubToken input
run->>downloadCli: Pass cliVersion and githubToken
downloadCli->>resolveLatestTag: Resolve latest tag
resolveLatestTag->>GitHubAPI: Send request with optional Bearer token
GitHubAPI-->>resolveLatestTag: Return latest tag
Merge Risk: ⚪ Minimal · up to The token input and latest-version lookup changes have no identified merge-blocking risk in the reviewed scope. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use 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 |
Summary
resolveLatestTag()(used to resolvecliVersion: latestto a concrete release tag) hit the GitHub API unauthenticated for effectively every consumer, becauseGITHUB_TOKEN/GH_TOKENare not automatically injected into a custom JS action's process environment — a calling workflow has to set them explicitly viaenv:, which essentially no consumer had reason to do. This exhausts the shared unauthenticated rate limit (60 req/hour per runner IP) under any real concurrency, e.g. a multi-version test matrix all resolving "latest" at once.Confirmed live via a Mirror Networking Actions run failing with "GitHub API returned 403", and this is the identical root cause already fixed in unity-test-runner#332 and unity-builder#852 — this PR applies the same fix to unity-activate, the third and final thin wrapper.
Changes
githubTokeninput toaction.yml, defaulting to${{ github.token }}(populated by GitHub Actions on every run — no consumer action needed).index.ts→downloadCli()→resolveLatestTag(), sent asAuthorization: Bearer <token>, ahead of theGITHUB_TOKEN/GH_TOKENenv-var fallback.src/download-cli.test.tswith regression tests covering: noAuthorizationheader when nothing is set, the header from thegithubTokenparameter, the env-var fallback, and thatdownloadCli()actually forwards the parameter through toresolveLatestTag()(the production wiring, not just the isolated function).Test plan
tsc --noEmitcleanyarn lintclean (only pre-existing, unrelatedanywarnings)yarn build(ncc) succeeds;dist/index.jsverified to contain the fixaction.ymlvalidated as well-formed YAML🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests