Skip to content

Commit a219025

Browse files
p1024kclaude
andcommitted
chore: upgrade MSRV to 1.89, fix clippy errors, improve Windows support
## MSRV Upgrade - Upgrade minimum Rust version from 1.75 to 1.89 for fs4::FileExt compatibility ## Windows Support - Implement real Windows clipboard using clipboard-win crate (replaces MOCK) - Fix file lock path construction for vault lock mechanism - Keep Windows TUI tests ignored (KeyBindingManager I/O timeout on CI) ## Code Quality - Fix all clippy warnings (dead code, redundant closures, etc.) - Move test helper functions into test modules - Fix KeyBindingManager::all_bindings() sorting for deterministic output - Refactor SSH handler: add AuthContext struct to reduce parameter count ## Lock Mechanism Fix - Fix VaultLock path construction for file paths - File path (e.g., /path/to/passwords.db) → /path/to/passwords.db.lock - Directory path (e.g., /path/to/vault/) → /path/to/vault/.lock - Enable test_with_read_lock test previously ignored Co-Authored-By: Claude Code <noreply@anthropic.com>
1 parent 16b198c commit a219025

116 files changed

Lines changed: 5098 additions & 126 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.insta.toml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# insta snapshot testing configuration
2+
# See https://insta.rs/docs/configuration/
3+
4+
[insta]
5+
# Auto-accept new snapshots during development
6+
# Set to "no" for CI/strict mode
7+
update = "auto"
8+
9+
# Automatically delete snapshots that are no longer referenced
10+
unreferenced = "delete"
11+
12+
# Snapshot filters - normalize dynamic content
13+
[insta.filters]
14+
# Timestamps in various formats
15+
"\\d{4}-\\d{2}-\\d{2}[T ]\\d{2}:\\d{2}:\\d{2}" = "[TIMESTAMP]"
16+
"\\d{4}-\\d{2}-\\d{2}" = "[DATE]"
17+
"\\d{2}:\\d{2}:\\d{2}" = "[TIME]"
18+
19+
# UUIDs
20+
"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}" = "[UUID]"
21+
"[0-9a-f]{32}" = "[UUID_NO_DASHES]"
22+
23+
# Relative time strings
24+
"\\d+[mhd]\\s+ago" = "[DURATION]"
25+
"\\d+\\s+(seconds?|minutes?|hours?|days?|weeks?|months?|years?)\\s+ago" = "[DURATION]"
26+
27+
# Duration patterns
28+
"\\d+ms" = "[MILLISECONDS]"
29+
"\\d+\\.\\d+s" = "[SECONDS]"
30+
"\\d+s" = "[SECONDS]"
31+
32+
# File sizes
33+
"\\d+\\.?\\d*\\s*(KB|MB|GB|TB)" = "[FILESIZE]"
34+
"\\d+\\s+bytes?" = "[BYTES]"
35+
36+
# Memory addresses
37+
"0x[0-9a-f]+" = "[ADDRESS]"
38+
39+
# Process/thread IDs
40+
"pid:\\s*\\d+" = "[PID]"
41+
"thread:\\s*\\d+" = "[THREAD_ID]"
42+
43+
# Path normalization (optional - uncomment if needed)
44+
# "/[\\w/-.]+" = "[PATH]"

Cargo.lock

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
name = "keyring-cli"
33
version = "0.1.0"
44
edition = "2021"
5-
rust-version = "1.75"
5+
rust-version = "1.89"
66
authors = ["OpenKeyring Team"]
77
license = "MIT"
88
repository = "https://github.com/open-keyring/keyring-cli"
@@ -180,6 +180,8 @@ harness = false
180180
tempfile = "3.10"
181181
criterion = "0.5"
182182
serial_test = "3.1"
183+
insta = "1.34"
184+
regex = "1.10"
183185

184186
[profile.release]
185187
opt-level = 3

src/clipboard/windows.rs

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,29 @@
1+
//! Windows clipboard implementation using clipboard-win crate
2+
//!
3+
//! Provides clipboard functionality on Windows using the clipboard-win crate,
4+
//! which wraps the Windows clipboard API.
5+
16
use crate::clipboard::manager::ClipboardManager;
27
use crate::error::KeyringError;
8+
use clipboard_win::{get_clipboard_string, set_clipboard_string, empty_clipboard};
39
use std::time::Duration;
410

511
pub struct WindowsClipboard;
612

713
impl ClipboardManager for WindowsClipboard {
814
fn set_content(&mut self, content: &str) -> Result<(), KeyringError> {
9-
// In a real implementation, this would use Windows clipboard API
10-
// For now, we'll simulate it
11-
println!("[MOCK] Setting Windows clipboard content: {}", content);
12-
Ok(())
15+
set_clipboard_string(content)
16+
.map_err(|e| KeyringError::Clipboard { context: format!("Failed to set clipboard: {}", e) })
1317
}
1418

1519
fn get_content(&mut self) -> Result<String, KeyringError> {
16-
// In a real implementation, this would read from Windows clipboard
17-
println!("[MOCK] Reading from Windows clipboard");
18-
Ok("mock_clipboard_content".to_string())
20+
get_clipboard_string()
21+
.map_err(|e| KeyringError::Clipboard { context: format!("Failed to get clipboard: {}", e) })
1922
}
2023

2124
fn clear(&mut self) -> Result<(), KeyringError> {
22-
// In a real implementation, this would clear the Windows clipboard
23-
println!("[MOCK] Clearing Windows clipboard");
24-
Ok(())
25+
empty_clipboard()
26+
.map_err(|e| KeyringError::Clipboard { context: format!("Failed to clear clipboard: {}", e) })
2527
}
2628

2729
fn is_supported(&self) -> bool {
@@ -32,3 +34,49 @@ impl ClipboardManager for WindowsClipboard {
3234
Duration::from_secs(30)
3335
}
3436
}
37+
38+
#[cfg(test)]
39+
mod tests {
40+
use super::*;
41+
42+
#[test]
43+
fn test_set_and_get_clipboard() {
44+
let mut clipboard = WindowsClipboard;
45+
46+
// Set clipboard content
47+
let test_content = "Test clipboard content";
48+
clipboard.set_content(test_content).unwrap();
49+
50+
// Get clipboard content
51+
let retrieved = clipboard.get_content().unwrap();
52+
assert_eq!(retrieved, test_content);
53+
}
54+
55+
#[test]
56+
fn test_clear_clipboard() {
57+
let mut clipboard = WindowsClipboard;
58+
59+
// Set content
60+
clipboard.set_content("Temporary content").unwrap();
61+
62+
// Clear
63+
clipboard.clear().unwrap();
64+
65+
// Verify it's empty or different
66+
let content = clipboard.get_content().unwrap();
67+
// After clearing, clipboard might be empty or contain previous content from other apps
68+
// We just verify the operation didn't error
69+
}
70+
71+
#[test]
72+
fn test_is_supported() {
73+
let clipboard = WindowsClipboard;
74+
assert!(clipboard.is_supported());
75+
}
76+
77+
#[test]
78+
fn test_timeout() {
79+
let clipboard = WindowsClipboard;
80+
assert_eq!(clipboard.timeout(), Duration::from_secs(30));
81+
}
82+
}

src/db/lock.rs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ use std::sync::atomic::{AtomicBool, Ordering};
99
///
1010
/// Uses fslock-style file locking with platform-specific implementations.
1111
/// The lock file is created alongside the vault database.
12+
///
13+
/// # Lock Path Construction
14+
///
15+
/// When `vault_path` is a file path (e.g., `/path/to/passwords.db`):
16+
/// - Lock file is created as `/path/to/passwords.db.lock` (replacing extension)
17+
///
18+
/// When `vault_path` is a directory path (e.g., `/path/to/vault/`):
19+
/// - Lock file is created as `/path/to/vault/.lock`
1220
#[allow(dead_code)]
1321
pub struct VaultLock {
1422
#[allow(dead_code)]
@@ -22,7 +30,7 @@ impl VaultLock {
2230
/// Acquire an exclusive write lock
2331
///
2432
/// # Arguments
25-
/// * `vault_path` - Path to the vault directory
33+
/// * `vault_path` - Path to the vault database file
2634
/// * `timeout_ms` - Maximum time to wait for lock acquisition
2735
///
2836
/// # Returns
@@ -33,7 +41,7 @@ impl VaultLock {
3341
/// - **Unix/macOS**: Uses `flock` with exclusive lock
3442
/// - **Windows**: Uses Windows file locking
3543
pub fn acquire_write(vault_path: &Path, timeout_ms: u64) -> Result<Self> {
36-
let lock_path = vault_path.join(".lock");
44+
let lock_path = Self::lock_path_for_vault(vault_path);
3745
let lock_file = Self::open_lock_file(&lock_path)?;
3846

3947
// Try to acquire lock with timeout
@@ -70,10 +78,10 @@ impl VaultLock {
7078
/// Multiple read locks can be held simultaneously, but write locks are exclusive.
7179
///
7280
/// # Arguments
73-
/// * `vault_path` - Path to the vault directory
81+
/// * `vault_path` - Path to the vault database file
7482
/// * `timeout_ms` - Maximum time to wait for lock acquisition
7583
pub fn acquire_read(vault_path: &Path, timeout_ms: u64) -> Result<Self> {
76-
let lock_path = vault_path.join(".lock");
84+
let lock_path = Self::lock_path_for_vault(vault_path);
7785
let lock_file = Self::open_lock_file(&lock_path)?;
7886

7987
// Try to acquire shared lock with timeout
@@ -104,6 +112,25 @@ impl VaultLock {
104112
})
105113
}
106114

115+
/// Determine the lock file path for a given vault path
116+
///
117+
/// Handles both file paths and directory paths:
118+
/// - File path (`/path/to/passwords.db`) → `/path/to/passwords.db.lock`
119+
/// - Directory path (`/path/to/vault/`) → `/path/to/vault/.lock`
120+
fn lock_path_for_vault(vault_path: &Path) -> std::path::PathBuf {
121+
if vault_path.extension().is_some() {
122+
// It's a file path (e.g., /path/to/passwords.db)
123+
// Replace/add .lock extension
124+
let mut lock_path = vault_path.to_path_buf();
125+
lock_path.set_extension("lock");
126+
lock_path
127+
} else {
128+
// It's a directory path (e.g., /path/to/vault/)
129+
// Append .lock
130+
vault_path.join(".lock")
131+
}
132+
}
133+
107134
/// Release the lock
108135
///
109136
/// The lock is also automatically released when VaultLock is dropped.

src/db/vault.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1396,7 +1396,6 @@ mod tests {
13961396
}
13971397

13981398
#[test]
1399-
#[ignore = "Lock path construction needs refactoring - see issue in lock.rs"]
14001399
fn test_with_read_lock() {
14011400
let temp_dir = TempDir::new().unwrap();
14021401
let db_path = temp_dir.path().join("passwords.db");

src/mcp/audit/audit/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ impl Default for AuditQuery {
8484
/// Audit logger for MCP operations
8585
pub struct AuditLogger {
8686
log_path: PathBuf,
87+
/// Signing key for audit log integrity verification
88+
/// Reserved for future implementation of cryptographic signatures
89+
#[allow(dead_code)]
8790
signing_key: Vec<u8>,
8891
}
8992

src/mcp/audit/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ impl SimpleAuditLogger {
159159
}
160160

161161
// Re-export the async audit logger types for tests and external use
162+
// Note: audit subdirectory contains the async audit logger implementation
163+
// The module inception warning is acceptable for cleaner namespace
164+
#[allow(clippy::module_inception)]
162165
pub mod audit;
163166

164167
// Re-export the async logger types for tests

0 commit comments

Comments
 (0)