Skip to content

fix: detect invalid OAuth credentials at startup#194

Closed
bobbyg603 wants to merge 1 commit into
mainfrom
fix/oauth-credential-validation
Closed

fix: detect invalid OAuth credentials at startup#194
bobbyg603 wants to merge 1 commit into
mainfrom
fix/oauth-credential-validation

Conversation

@bobbyg603
Copy link
Copy Markdown
Member

Summary

  • When client credentials are invalid, the CLI was getting stuck in a retry loop instead of exiting with a clear error.
  • Root cause: the BugSplat server now returns HTTP 400 with { message: "Unknown clientId ..." } for bad OAuth credentials. The @bugsplat/js-api-client OAuth login only treats HTTP 401 or a JSON body containing error: 'invalid_client' as auth failure, so login silently "succeeds" with an undefined access token. Every subsequent /symsrv/uploadUrl call returns HTTP 200 with { error: 'access_denied' } (also unrecognized as auth failure), so getPresignedUrl returns undefined and S3 throws a generic "Failed to parse URL from undefined" that worker.ts retries forever.
  • Fix: validate OAuth credentials explicitly in createBugSplatClient before constructing the library client. Bad creds throw BugSplatAuthenticationError, which the existing top-level handler logs and exits 1 on.

Test plan

  • npx vitest run — full suite passes (49 tests including 5 new auth tests)
  • npm run build — clean TypeScript build
  • Manual: invalid SYMBOL_UPLOAD_CLIENT_ID / SYMBOL_UPLOAD_CLIENT_SECRET now exit in <1s with Could not authenticate, check clientId and clientSecret and try again instead of looping
  • Manual: valid credentials still authenticate and proceed normally
  • Verify in CI

🤖 Generated with Claude Code

The BugSplat server now returns HTTP 400 with `{ message: "Unknown
clientId ..." }` for bad client credentials, which slips past the
js-api-client OAuth login checks (it only treats 401 or
`error: 'invalid_client'` as auth failure). Login silently succeeds with
an undefined access token and every upload then hits
`Failed to parse URL from undefined` in S3 — retried forever.

Validate credentials explicitly in createBugSplatClient via a dedicated
src/auth helper so we fail fast with a BugSplatAuthenticationError and
exit with code 1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 18, 2026 16:12
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an explicit OAuth credential validation step to prevent the CLI from entering an infinite retry loop when BugSplat returns “successful” OAuth responses that lack an access token, ensuring authentication failures are detected and surfaced immediately at startup.

Changes:

  • Introduces validateOAuthCredentials to proactively verify clientId/clientSecret by checking for a returned access_token.
  • Calls the new validation during OAuth-based client creation before constructing the @bugsplat/js-api-client OAuth client.
  • Adds a new Vitest suite covering success and multiple failure modes (400 unknown clientId payload, 200 without token, non-JSON body, fetch failure).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/auth.ts Adds explicit OAuth credential validation and throws BugSplatAuthenticationError on failure.
spec/auth.spec.ts Adds tests for the new credential validation logic.
bin/index.ts Runs OAuth credential validation before creating the OAuth authenticated API client.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/auth.ts
Comment on lines +25 to +26
throw new BugSplatAuthenticationError(
`Could not reach ${url} to authenticate: ${(cause as Error).message}`
Comment thread src/auth.ts
Comment on lines +30 to +41
let payload: { access_token?: string } | null = null;
try {
payload = (await response.json()) as { access_token?: string };
} catch {
// empty/non-JSON body is treated as auth failure below
}

if (!response.ok || !payload?.access_token) {
throw new BugSplatAuthenticationError(
'Could not authenticate, check clientId and clientSecret and try again'
);
}
Comment thread src/auth.ts
try {
payload = (await response.json()) as { access_token?: string };
} catch {
// empty/non-JSON body is treated as auth failure below
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

smelly

@bobbyg603
Copy link
Copy Markdown
Member Author

I think we tried this fix because bugsplat-android was attempting to upload a file using 10.1.7 and getting stuck in a retry loop because that version doesn't support scoped tokens.

@bobbyg603 bobbyg603 closed this May 20, 2026
@bobbyg603 bobbyg603 deleted the fix/oauth-credential-validation branch May 20, 2026 21:32
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.

3 participants