Skip to content

fix: address security and quality issues from code review#2

Open
Dnamb wants to merge 1 commit into
utajum:masterfrom
Dnamb:fix/code-review-security-and-quality
Open

fix: address security and quality issues from code review#2
Dnamb wants to merge 1 commit into
utajum:masterfrom
Dnamb:fix/code-review-security-and-quality

Conversation

@Dnamb
Copy link
Copy Markdown

@Dnamb Dnamb commented Mar 16, 2026

Summary

  • [Critical] Remove plaintext refresh token debug file (NEW_REFRESH_TOKEN_WARNING.txt) that leaked OAuth credentials to disk with world-readable permissions
  • [High] Fix nil dereference crash in self-update binary size check
  • [High] Add fsync in copyFile during self-update to prevent corruption on power loss
  • [High] Fix race condition on apiClient field accessed from multiple goroutines without synchronization
  • [High] Restrict config file permissions from 0644 to 0600
  • [Medium] Replace misleading random percentage fallback with deterministic zero
  • [Medium] Drain HTTP response bodies before close for connection reuse
  • [Medium] Add context.Context propagation to all API HTTP requests
  • [Medium] Update golang.org/x/sys from v0.15.0 to v0.42.0
  • [Low] Remove unused exported FileExists function
  • [Low] Initialize credOrigin default in App constructor

Test plan

  • go build ./... passes
  • go test -v ./... — all 4 tests pass
  • go vet ./... clean
  • Manual test: installed updated binary, launched app, verified tray icon shows correct usage (4%), API call succeeds, no NEW_REFRESH_TOKEN_WARNING.txt created

🤖 Generated with Claude Code

- Remove plaintext refresh token debug file (credential leak)
- Add context.Context propagation to API HTTP requests
- Drain HTTP response bodies for connection reuse
- Fix nil dereference in update binary size check
- Add fsync in copyFile during self-update
- Fix race condition on apiClient with mutex guard
- Restrict config file permissions to 0600
- Replace random percentage fallback with deterministic zero
- Remove unused FileExists from stats/parser
- Initialize credOrigin default in App constructor
- Update golang.org/x/sys v0.15.0 -> v0.42.0

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

1 participant