feat(cli): add optional GitHub token support for higher API rate limits#294
Conversation
Automated security fix generated by Orbis Security AI
mrgoonie
left a comment
There was a problem hiding this comment.
Summary: This PR tries to add optional GitHub API authentication for release fetch/download calls, but it reintroduces a narrower and weaker version of prior closed PR #186 without the CLI/docs wiring or token-safety guidance.
Risk level: Medium
Mandatory gates:
- Duplicate/prior implementation: overlap found — closed PR #186 already implemented a broader optional-token design for the same GitHub release helper path and received maintainer review feedback.
- Project standards: issue found — changing CLI auth behavior should include documented user-facing configuration and validation, not an implicit environment-only behavior.
- Strategic necessity: questionable as submitted — avoiding rate limits is useful, but the stated hardcoded-token security issue is not present in current
cli/src/utils/github.ts. - CI/checks: missing / locally blocked — no PR checks are reported and
bunis unavailable in this cron environment, so I could not run the TypeScript build.
Findings:
- Important: The PR claims to fix possible hardcoded GitHub token exposure, but current
origin/main:cli/src/utils/github.tshas no hardcoded token and only sends unauthenticated fetch requests. The actual change silently starts reading a GitHub token from the environment and attaching it to every release/download request, without README/CLI documentation, token naming guidance, or warning about secret exposure. That makes this a rate-limit feature, not the described security fix, and it needs product/UX documentation before merge. - Important: This overlaps with PR #186, which already proposed optional GitHub token support plus docs and command wiring, and was closed after review feedback around secret handling,
--tokenUX, empty-token guards, and auth/rate-limit messaging. Please either reopen/supersede that design deliberately or explain why this narrower env-only version is the preferred approach and incorporate the relevant safeguards.
Verdict: REQUEST_CHANGES
- Rename env var from GITHUB_TOKEN to UI_PRO_MAX_GITHUB_TOKEN (primary), with GITHUB_TOKEN as fallback — avoids silently attaching CI workflow credentials that GitHub Actions injects automatically - Add whitespace trim guard to prevent malformed Authorization headers - Add getGitHubTokenGuidance() helper exported from github.ts so rate-limit errors and spinner warnings tell users exactly how to fix the problem - Thread optional token param through fetchReleases, getLatestRelease, downloadRelease signatures - Add --token flag to init, versions, update commands (Commander.js wiring) - Update rate-limit catch in tryGitHubInstall to show token guidance - Document token options (flag, env var, fallback) in cli/README.md with CI warning about GITHUB_TOKEN scope Supersedes the narrower env-only approach in PR nextlevelbuilder#294 and incorporates the safeguards requested during review of closed PR nextlevelbuilder#186. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addressed. Pls review. |
mrgoonie
left a comment
There was a problem hiding this comment.
Summary: The updated PR now addresses the prior token-support blockers: CLI flag wiring, project-scoped env var precedence, empty-token trimming, rate-limit guidance, and user-facing docs are present.
Risk level: Medium
Mandatory gates:
- Duplicate/prior implementation: overlap found with closed PR #186, but this PR now deliberately supersedes it and incorporates the relevant safeguards.
- Project standards: passed — CLI behavior is wired through command options and documented in cli/README.md.
- Strategic necessity: clear value — optional auth helps users avoid GitHub unauthenticated API rate limits without changing default unauthenticated behavior.
- CI/checks: missing — no PR checks are reported for this branch, so merge should still rely on branch protection/manual build if required.
Findings:
- Critical: none.
- Important: none.
- Suggestion: consider avoiding direct --token examples with realistic token-looking strings in future docs, since env vars are safer for shell history; the current warning is enough for this PR.
Verdict: APPROVE
Summary
Adds optional GitHub Personal Access Token support to the CLI so users can raise the GitHub API rate limit from 60 to 5,000 requests/hour. This is a rate-limit convenience feature, not a security fix — the original
cli/src/utils/github.tshad no hardcoded token.This supersedes closed PR #186 and incorporates the safeguards requested during its review.
Changes
cli/src/utils/github.tsgetAuthHeaders()now accepts an optionaltokenparam; precedence: explicit arg →UI_PRO_MAX_GITHUB_TOKENenv var →GITHUB_TOKENenv varAuthorizationheaders from empty env varsgetGitHubTokenGuidance()helper used in rate-limit error messagesfetchReleases,getLatestRelease,downloadReleaseaccept optionaltokenparamcli/src/index.ts-t, --token <token>option toinit,versions,updatecommandscli/src/commands/init.ts/update.ts/versions.tstokenthrough options interfaces and GitHub helper callstryGitHubInstallnow showsgetGitHubTokenGuidance()before falling backcli/README.md--tokenflag,UI_PRO_MAX_GITHUB_TOKEN,GITHUB_TOKENfallback, PAT creation steps, and a CI warning aboutGITHUB_TOKENscopeWhy
UI_PRO_MAX_GITHUB_TOKENinstead ofGITHUB_TOKENGitHub Actions automatically injects
GITHUB_TOKENwith broad repo permissions into every workflow. Using that as the primary name would silently attach those credentials to every release fetch, which is the actual token-exposure risk.UI_PRO_MAX_GITHUB_TOKENis project-scoped and explicit;GITHUB_TOKENis kept as an opt-in fallback.Verification
bun run buildpasses (bun required)uipro init --token ghp_fakeattachesAuthorization: Bearer ghp_fakeheaderuipro initwith no token makes unauthenticated requests (noAuthorizationheader)UI_PRO_MAX_GITHUB_TOKEN=x uipro versionspicks up env token