fix-native-edit-shortcuts

[Makisuo]

fixes #283

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
hazel-docs	Skipped		Feb 28, 2026 5:59pm

[claude]

Claude finished @Makisuo's task —— View job

---

Code Review for PR #284: fix-native-edit-shortcuts

I've completed the review of this merged pull request that adds native edit shortcuts to the Tauri desktop application. Here's my analysis:

Todo List:

• Read the modified file (apps/desktop/src-tauri/src/lib.rs)
• Analyze code quality and best practices
• Check for potential bugs or issues
• Review performance considerations
• Examine security concerns
• Assess test coverage
• Provide comprehensive review feedback

---

Review Summary

This PR introduces a complete Tauri desktop application setup with OAuth handling and native menu shortcuts. The entire apps/desktop/src-tauri/src/lib.rs file was newly created (328 lines).

✅ Positive Aspects

1. Well-structured OAuth implementation

   • Dynamic port allocation (17900-17999 range) prevents conflicts
   • Proper nonce-based security validation
   • Clean CORS headers handling
   • Thread-safe nonce storage using OnceLock and Mutex

2. Cross-platform menu support

   • Platform-specific conditional compilation for macOS vs other platforms
   • Comprehensive native menu with standard shortcuts (Cmd/Ctrl+,, Cmd/Ctrl+Alt+N, etc.)
   • Proper event handling and emission

3. Modern Rust patterns

   • Good use of Result types for error handling
   • Pattern matching with guard clauses
   • Appropriate use of OnceLock for static initialization

⚠️ Security & Safety Concerns

1. Weak nonce generation apps/desktop/src-tauri/src/lib.rs:21-30

   fn generate_nonce() -> String {
       let timestamp = SystemTime::now()
           .duration_since(UNIX_EPOCH)
           .unwrap()
           .as_nanos();
       let thread_id = format!("{:?}", std::thread::current().id());
       format!("{:x}{}", timestamp, thread_id.len())
   }

   Issue: This nonce generation is predictable and not cryptographically secure. An attacker could potentially predict nonce values based on timing.
   Recommendation: Use a cryptographically secure random number generator like rand::thread_rng().

2. CORS wildcard origin apps/desktop/src-tauri/src/lib.rs:35

   Header::from_bytes("Access-Control-Allow-Origin", "*").unwrap(),

   Issue: Allows any origin to make requests, which could be exploited by malicious websites.
   Recommendation: Restrict to specific trusted origins.

3. Multiple .unwrap() calls (throughout the code)
   Issue: Could cause panics in production if headers fail to parse.
   Recommendation: Use proper error handling or .expect() with descriptive messages.

🐛 Potential Bugs

1. Resource cleanup issue apps/desktop/src-tauri/src/lib.rs:72-166

   • The spawned thread has no cleanup mechanism if the server fails to start properly
   • Nonces could accumulate in memory if threads exit unexpectedly
   • Recommendation: Add proper cleanup and consider using a bounded thread pool

2. Port exhaustion handling apps/desktop/src-tauri/src/lib.rs:48-60

   • Limited error information when no ports are available
   • Recommendation: Log which ports were attempted and why they failed

🚀 Performance Considerations

1. Thread spawning apps/desktop/src-tauri/src/lib.rs:72

   • Creates a new thread for each OAuth flow, which is acceptable for desktop use
   • Limited to 10 requests per server instance prevents resource exhaustion

2. String allocations

   • Multiple string allocations for responses could be optimized with static strings or string interning
   • Not critical for desktop OAuth flows

📝 Code Quality Suggestions

1. Add error types

   // Instead of returning String errors, consider:
   #[derive(Debug, thiserror::Error)]
   enum OAuthError {
       #[error("No available ports in range {min}-{max}")]
       NoPortsAvailable { min: u16, max: u16 },
       #[error("Server error: {0}")]
       ServerError(String),
   }

2. Extract constants

   const MAX_REQUESTS_PER_SERVER: usize = 10;
   const RESPONSE_DELAY_MS: u64 = 500;

3. Missing documentation

   • Public functions like run() should have doc comments
   • Module-level documentation would be helpful

🧪 Test Coverage

Major Gap: No tests found for this critical OAuth and menu functionality.

Recommendations:

• Unit tests for nonce generation and validation
• Integration tests for OAuth flow
• Menu event handling tests
• Mock server tests for various failure scenarios

📋 Minor Issues

1. Unused import potential: Verify all imports are actually used
2. Magic numbers: The 500ms delay could be made configurable
3. Logging: Consider adding debug/trace logging for OAuth flow debugging

Overall Assessment

This is a solid foundation for desktop OAuth and native menus, but has some security vulnerabilities that should be addressed before production use. The code demonstrates good Rust patterns but lacks proper testing and has some security concerns around nonce generation and CORS configuration.

Priority: Address the cryptographic nonce generation and CORS origin restrictions before release.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	36.9%	3209 / 8695
🔵	Statements	36.83%	3345 / 9081
🔵	Functions	23.46%	686 / 2923
🔵	Branches	27.45%	1013 / 3690

File Coverage

No changed files found.

Generated in workflow #1593 for commit 910b9bd by the Vitest Coverage Report Action