fix: stop logging external exchange codes

[khendrikse]

- Summary

- Test plan

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Enhanced OAuth error messages to include the provider type when token exchange fails, providing clearer feedback during authentication issues.

Walkthrough

The change updates an error message in the OAuth token exchange failure handler. The message is modified to include the provider type in the formatted string, changing from "Unable to exchange external code" to "Unable to exchange external code for %s provider". The error message now displays which provider caused the exchange failure. No control flow, error handling logic, or exported declarations are affected.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title claims to 'stop logging external exchange codes', but the actual change only updates an error message format to include provider type—no logging is removed.	Update the title to accurately reflect the change, such as 'fix: include provider type in OAuth token exchange error message' or similar.
Description check	❓ Inconclusive	The PR description contains only the repository's template with no actual content filled in—all sections (Summary, Test plan, Changelog) are empty.	Fill in the PR description with relevant details about the motivation, test plan, and changelog entry for the change.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ex-1926/stop-logging-external-exchange-codes

📝 Coding Plan

• Generate coding plan for human review comments

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.3)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

api/external_oauth.go (1)

46-49: Good security improvement; consider adding a brief comment.

The change correctly stops logging the underlying OAuth error, which could contain sensitive exchange code information. The providerType inclusion provides sufficient context for identifying failures.

However, the err from GetOAuthToken is now silently discarded. A brief comment explaining the intentional omission would help future maintainers understand this is deliberate rather than an oversight.

📝 Suggested comment for clarity

 	tok, err := oAuthProvider.GetOAuthToken(oauthCode)
 	if err != nil {
+		// Intentionally not logging the error to avoid exposing exchange code details
 		return nil, internalServerError("Unable to exchange external code for %s provider", providerType)
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@api/external_oauth.go` around lines 46 - 49, The error returned from
oAuthProvider.GetOAuthToken is currently not logged or propagated (the code
returns internalServerError with providerType), which is intentional for
security; add a short inline comment above this return explaining that the
original err is intentionally omitted to avoid exposing sensitive OAuth exchange
details and that providerType is sufficient context, referencing GetOAuthToken,
oAuthProvider and internalServerError to make the rationale clear for future
maintainers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@api/external_oauth.go`:
- Around line 46-49: The error returned from oAuthProvider.GetOAuthToken is
currently not logged or propagated (the code returns internalServerError with
providerType), which is intentional for security; add a short inline comment
above this return explaining that the original err is intentionally omitted to
avoid exposing sensitive OAuth exchange details and that providerType is
sufficient context, referencing GetOAuthToken, oAuthProvider and
internalServerError to make the rationale clear for future maintainers.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9ad38f73-23b4-41cf-9571-9e28a5a31e81

📥 Commits

Reviewing files that changed from the base of the PR and between fd558f2 and c7f2f39.

📒 Files selected for processing (1)

• api/external_oauth.go