Skip to content

fix(security): prevent Windows command injection and improve RFC 7009 compliance#30

Merged
appleboy merged 4 commits intomainfrom
fix/security-hardening-and-rfc7009
Apr 2, 2026
Merged

fix(security): prevent Windows command injection and improve RFC 7009 compliance#30
appleboy merged 4 commits intomainfrom
fix/security-hardening-and-rfc7009

Conversation

@appleboy
Copy link
Copy Markdown
Member

@appleboy appleboy commented Apr 2, 2026

Summary

  • Use rundll32 url.dll,FileProtocolHandler instead of cmd /c start on Windows to prevent shell metacharacter injection in URLs
  • Add token_type_hint parameter to token revocation requests per RFC 7009
  • Replace panic() with fmt.Fprintf(os.Stderr, ...) + os.Exit(1) for consistent error handling in loadConfig
  • Write token save warning to stderr instead of stdout in refreshAccessToken

Test plan

  • make test — all existing tests pass
  • make lint — 0 issues
  • Manual verification on Windows for rundll32 browser launch

🤖 Generated with Claude Code

… compliance

- Use rundll32 instead of cmd /c start to prevent shell metacharacter injection in URLs on Windows
- Replace panic with fmt.Fprintf + os.Exit(1) for consistent error handling in loadConfig
- Write token save warning to stderr instead of stdout in refreshAccessToken
- Add token_type_hint parameter to revocation requests per RFC 7009
Copilot AI review requested due to automatic review settings April 2, 2026 10:29
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 75.86207% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
browser.go 62.50% 3 Missing ⚠️
config.go 0.00% 2 Missing ⚠️
auth.go 0.00% 1 Missing ⚠️
token_cmd.go 94.44% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

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

This PR hardens browser launching on Windows to avoid shell-based command injection and improves OAuth token revocation compliance with RFC 7009.

Changes:

  • Switch Windows browser launch from cmd /c start to rundll32 url.dll,FileProtocolHandler.
  • Add token_type_hint to token revocation requests (refresh vs access token).
  • Standardize CLI error/warning output by writing errors to stderr and avoiding panic() in loadConfig.

Reviewed changes

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

File Description
token_cmd.go Adds token_type_hint to revocation requests and updates call sites accordingly.
config.go Replaces a panic() with stderr output + os.Exit(1) for retry client creation failure.
browser.go Uses rundll32 for Windows URL opening to avoid shell metacharacter injection.
auth.go Writes token-save warnings to stderr instead of stdout.

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

- Align error message wording to say "retry HTTP client" in loadConfig
- Add token_type_hint assertions to revocation tests for RFC 7009 compliance
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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


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

- Track revocation call count to ensure exactly one request is made
  when only an access token exists (no refresh token)
@appleboy appleboy requested a review from Copilot April 2, 2026 10:49
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

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


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

- Conditionally include token_type_hint only when non-empty for server compatibility
- Extract browserCommand helper for testability and add unit tests
  covering darwin, windows, and linux command construction
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

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.


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

@appleboy appleboy merged commit a5b37a3 into main Apr 2, 2026
20 checks passed
@appleboy appleboy deleted the fix/security-hardening-and-rfc7009 branch April 2, 2026 14:10
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