Skip to content

fix: replace OAuth callback server with copy/paste flow#35

Merged
rianjs merged 1 commit intomainfrom
fix/34-remove-oauth-callback-server
Feb 7, 2026
Merged

fix: replace OAuth callback server with copy/paste flow#35
rianjs merged 1 commit intomainfrom
fix/34-remove-oauth-callback-server

Conversation

@rianjs
Copy link
Contributor

@rianjs rianjs commented Feb 7, 2026

Summary

  • Remove the localhost HTTP callback server (internal/auth/callback.go) from the OAuth init flow
  • Replace with a manual copy/paste approach matching the pattern used by google-readonly (gro)
  • Remove openBrowser() function and --no-browser flag
  • Redirect URL stays http://localhost:8080/callback — existing Connected App configs unaffected

Why

The callback server, browser-opening code (os/exec), and local HTTP server (net.Listen, http.Server) triggered Windows SmartScreen/Defender heuristic detection, causing winget validation to repeatedly fail with "Installer failed security check" (microsoft/winget-pkgs#335819).

Other CLIs (jtk, cfl, slack-chat-cli) pass winget validation clean because they don't run a local server.

Changes

File Change
internal/auth/callback.go Deleted — removes StartCallbackServer, CallbackResult, all server code
internal/auth/auth.go DefaultCallbackPortCallbackURL constant (no more fmt.Sprintf)
internal/cmd/initcmd/init.go Copy/paste flow replaces server + browser opening. Removed os/exec, runtime, time imports
internal/cmd/initcmd/init_test.go Removed --no-browser flag test, added URL parsing edge cases

Net: -151 lines (39 added, 190 removed)

Test plan

  • go build ./... — compiles clean
  • go test ./... — all 25 packages pass
  • go vet ./... — no issues
  • golangci-lint run — 0 issues
  • Manual: sfdc init prints URL, accepts pasted code, exchanges token

Closes #34

Remove the localhost HTTP callback server from the OAuth init flow
and replace it with a manual copy/paste approach (matching the pattern
used by google-readonly).

The callback server, browser-opening code, and --no-browser flag
triggered Windows SmartScreen/Defender heuristic detection, causing
winget validation to fail with "Installer failed security check".

The redirect URL remains http://localhost:8080/callback so existing
Connected App configurations are unaffected.

Closes #34
@rianjs
Copy link
Contributor Author

rianjs commented Feb 7, 2026

Test Coverage Assessment

Summary

This PR replaces the OAuth callback server with a manual copy/paste flow. The test coverage for the changed code is adequate. The changes are primarily deletion of code (net -151 lines), so the main risk surface is small, and the tests that exist target it well.

What's covered

extractAuthCode() — well covered. This is the most critical new code path: it parses user input that could be a raw auth code or a full redirect URL. The existing 6 test cases plus the 4 new ones cover:

  • Raw code strings (with and without whitespace)
  • http://localhost:8080/callback?code=... (the standard redirect)
  • URLs with additional query params (&state=...)
  • Error URLs (no code param)
  • Empty input
  • Localhost URLs without a port
  • HTTPS localhost URLs
  • Codes with special characters (/, -, _, ., ~)
  • URL-encoded codes (%2F/)

This is the function most likely to see unexpected user input, and the edge cases are solid.

NewCommand() — covered. The test verifies the command structure and flag registration, including a negative assertion that --no-browser is no longer registered. This is a good regression guard.

auth.go changes — covered indirectly. TestGetOAuthConfig already asserts config.RedirectURL == "http://localhost:8080/callback", which validates the DefaultCallbackPortCallbackURL constant refactor. No new test needed here.

What's not covered (and whether it matters)

runInit() itself is not unit-tested. This is the main orchestration function: it reads stdin, calls extractAuthCode, calls auth.ExchangeAuthCode, saves tokens, etc. However, this function is essentially a TUI workflow (huh forms + stdin + network calls). It is difficult to unit-test without significant mocking infrastructure, and the individual functions it calls are tested. The PR description notes manual testing is planned. This is a reasonable tradeoff for a CLI init flow.

Deleted code (callback.go, openBrowser()) had no tests either. There is no coverage regression from the deletion — the old callback server and browser-opener were also untested. Removing them is a net positive for maintainability.

Verdict

No test gaps that warrant blocking this PR. The critical parsing logic (extractAuthCode) has thorough coverage including the new edge cases added by this PR. The orchestration code (runInit) is inherently hard to unit-test and is appropriately validated via manual testing. The auth.go constant change is validated by existing tests.

@rianjs rianjs merged commit 55d8523 into main Feb 7, 2026
3 checks passed
@rianjs rianjs deleted the fix/34-remove-oauth-callback-server branch February 7, 2026 17:37
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.

Remove localhost callback server from OAuth flow

1 participant

Comments