Skip to content

fix(scan): surface GitHub rate-limit errors in bulk repo scan#1235

Open
John-David Dalton (jdalton) wants to merge 1 commit intomainfrom
fix/github-rate-limit-visibility
Open

fix(scan): surface GitHub rate-limit errors in bulk repo scan#1235
John-David Dalton (jdalton) wants to merge 1 commit intomainfrom
fix/github-rate-limit-visibility

Conversation

@jdalton
Copy link
Copy Markdown
Contributor

@jdalton John-David Dalton (jdalton) commented Apr 18, 2026

Summary

  • The bulk-scan loop in createScanFromGithub silently swallowed per-repo failures. A rate-limited GitHub token made every repo fail, yet the outer function returned ok:true with "N repos / 0 manifests", misleading users into thinking the scan worked.
  • Short-circuit the loop on known blocking errors (rate limit, GraphQL rate limit, abuse detection, auth failure) so subsequent repos don't burn more quota re-hitting the same wall.
  • Return an error CResult when every repo fails, so scripts can tell the run did not succeed.

Test plan

  • pnpm --filter @socketsecurity/cli run test:unit test/unit/commands/scan/create-scan-from-github.test.mts — 16 passed (3 new regression tests)
  • pnpm --filter @socketsecurity/cli run test:unit test/unit/commands/scan/ — 634 passed
  • pnpm run type — passes
  • pnpm run lint — passes

Note

Medium Risk
Changes bulk-scan control flow to short-circuit and return failures on rate-limit/auth conditions, which can alter CLI exit behavior and automation expectations.

Overview
Fixes createScanFromGithub bulk scanning to stop early and return ok:false when hitting blocking GitHub errors (rate limit, GraphQL rate limit, abuse detection, or auth failure) instead of continuing and reporting misleading "0 manifests" success.

Also treats the run as an error when all repos fail for non-blocking reasons, and updates the summary logging to report repos actually processed. Adds unit regression tests covering rate-limit short-circuiting, auth failure short-circuiting, and the "all repos failed" case (ASK-167).

Reviewed by Cursor Bugbot for commit 7f7f649. Configure here.

The bulk-scan loop in `createScanFromGithub` silently swallowed
per-repo failures. A rate-limited token made every repo fail, yet
the outer function returned ok:true with "N repos / 0 manifests",
misleading users into thinking the scan worked.

- Track per-repo failures and short-circuit the loop on known
  blocking errors (rate limit, GraphQL rate limit, abuse detection,
  auth failure) so subsequent repos don't burn more quota for the
  same error
- Return an error CResult when all repos fail, so scripts can tell
  the run did not succeed
- Add regression tests covering the short-circuit and the
  all-repos-failed fallback
@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7f7f649. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7f7f649. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7f7f649. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7f7f649. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7f7f649. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7f7f649. Configure here.

Copy link
Copy Markdown

@m74278803-cmyk MAN (m74278803-cmyk) left a comment

Choose a reason for hiding this comment

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants