From b93ae852e05548f5fb5063b4f80137cbf8c235eb Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Wed, 26 Aug 2026 18:53:55 +0200 Subject: [PATCH] Reuse native macOS Git credentials --- AGENTS.md | 12 ++- crates/storage/src/authentication.rs | 14 +++ crates/storage/src/config.rs | 64 ++++++++++---- crates/storage/src/git.rs | 20 +++-- crates/storage/src/secret_store.rs | 43 +++++++-- crates/storage/src/secret_store/platform.rs | 98 +++++++++++++++++++++ crates/storage/tests/config_contract.rs | 43 +++++++++ crates/storage/tests/secret_store.rs | 61 +++++++++++++ docs/git-synchronization.md | 3 +- docs/secure-secret-storage.md | 5 +- 10 files changed, 327 insertions(+), 36 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c871247..db12367 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,13 +11,17 @@ secure-secret-storage orchestration. Git remotes are typed HTTPS or optional SSH endpoints; reject local, helper, executable, and unknown transports before entering Git transport code. Builds without the `ssh` feature must still parse SSH endpoints and return a typed unsupported-transport error before connection -or repository mutation. +or repository mutation. When a password-store repository contains Git remotes, +those names and URLs are authoritative; use application configuration only as +the fallback for a repository without remotes. The SSH feature must keep its algorithm allowlist, strict known-host checking, single configured identity, bounded channel/diagnostic limits, and ambiguous -push outcome rules in `crates/storage`. Do not add OpenSSH configuration, -proxy commands, password or keyboard-interactive authentication, host-key -bypasses, or frontend transport policy. +push outcome rules in `crates/storage`. The in-process SSH configuration reader +may honor only the connection directives `Host`, `HostName`, `User`, `Port`, +and `IdentityFile`. Do not add `Include`, `Match`, proxy commands, password or +keyboard-interactive authentication, host-key bypasses, or frontend transport +policy. The CLI, Ratatui, Iced, Swift, SwiftUI, AutoFill, and watchOS code may collect input, invoke the Rust API, and present Rust-provided state. They must not diff --git a/crates/storage/src/authentication.rs b/crates/storage/src/authentication.rs index a1934ad..8cb9cfc 100644 --- a/crates/storage/src/authentication.rs +++ b/crates/storage/src/authentication.rs @@ -455,6 +455,20 @@ impl GitCredentialProvider .map_err(|_| GitError::CredentialsUnavailable)?; self.shared.store.credential(server, application) } + + fn credential_for(&self, remote: &crate::config::GitRemote) -> Result { + let _operation = self + .shared + .operation() + .map_err(|_| GitError::CredentialsUnavailable)?; + self.shared + .expire_if_needed() + .map_err(|_| GitError::CredentialsUnavailable)?; + self.shared + .with_active(self.generation, |_| ()) + .map_err(|_| GitError::CredentialsUnavailable)?; + self.shared.store.credential_for(remote) + } } impl SshPassphraseProvider diff --git a/crates/storage/src/config.rs b/crates/storage/src/config.rs index b5ec17d..77a1995 100644 --- a/crates/storage/src/config.rs +++ b/crates/storage/src/config.rs @@ -1587,7 +1587,7 @@ struct RawGit { remotes: Vec, } -#[derive(Deserialize)] +#[derive(Clone, Deserialize)] #[serde(deny_unknown_fields)] struct RawGitRemote { name: String, @@ -1700,7 +1700,7 @@ fn validate_config( }; let configured_git_remotes = raw.git.remotes; #[cfg(not(any(target_os = "ios", target_os = "watchos")))] - let git_remotes = match repository_git_remotes(&vault)? { + let git_remotes = match repository_git_remotes(&vault, &configured_git_remotes, base)? { Some(remotes) => remotes, None => validate_remotes(configured_git_remotes, Some(base))?, }; @@ -1727,7 +1727,11 @@ fn validate_config( } #[cfg(not(any(target_os = "ios", target_os = "watchos")))] -fn repository_git_remotes(vault: &Path) -> Result>, ConfigError> { +fn repository_git_remotes( + vault: &Path, + configured_remotes: &[RawGitRemote], + config_base: &Path, +) -> Result>, ConfigError> { let path = vault.join(".git/config"); if !path.is_file() { return Ok(None); @@ -1760,21 +1764,45 @@ fn repository_git_remotes(vault: &Path) -> Result>, Config name: name.to_owned(), })?; let remote = match endpoint { - RemoteEndpoint::Https(ref endpoint) => GitRemote::https( - name, - &url, - stable_identifier( - "server", - format!( - "{}://{}:{}", - endpoint.scheme(), - endpoint.host_str().unwrap_or_default(), - endpoint.port_or_known_default().unwrap_or(443) - ) - .as_bytes(), - ), - stable_identifier("repository", url.as_bytes()), - )?, + RemoteEndpoint::Https(ref https_endpoint) => { + let credential_ids = configured_remotes + .iter() + .find(|configured| { + RemoteEndpoint::parse(&configured.url) + .is_ok_and(|configured| configured == endpoint) + }) + .map(|configured| { + let mut validated = + validate_remotes(vec![configured.clone()], Some(config_base))?; + let configured = validated.remove(0); + let (server, application) = configured.https_credentials().ok_or( + ConfigError::InvalidField { + field: "git.remotes.https_credentials", + }, + )?; + Ok::<_, ConfigError>(( + server.as_str().to_owned(), + application.as_str().to_owned(), + )) + }) + .transpose()? + .unwrap_or_else(|| { + ( + stable_identifier( + "server", + format!( + "{}://{}:{}", + https_endpoint.scheme(), + https_endpoint.host_str().unwrap_or_default(), + https_endpoint.port_or_known_default().unwrap_or(443) + ) + .as_bytes(), + ), + stable_identifier("repository", https_endpoint.as_str().as_bytes()), + ) + }); + GitRemote::https(name, &url, credential_ids.0, credential_ids.1)? + } RemoteEndpoint::Ssh(_) => GitRemote::ssh(name, &url)?, }; remotes.push(remote); diff --git a/crates/storage/src/git.rs b/crates/storage/src/git.rs index 6db37bc..6513650 100644 --- a/crates/storage/src/git.rs +++ b/crates/storage/src/git.rs @@ -828,6 +828,11 @@ pub trait GitCredentialProvider { server: &ServerId, application: &ApplicationId, ) -> Result; + + fn credential_for(&self, remote: &GitRemote) -> Result { + let (_, server, application) = require_https_remote(remote)?; + self.credential(server, application) + } } pub trait SshPassphraseProvider { @@ -873,6 +878,10 @@ impl GitCredentialProvider for GitRemoteCredentialOver ) -> Result { self.provider.credential(server, application) } + + fn credential_for(&self, remote: &GitRemote) -> Result { + self.provider.credential_for(remote) + } } impl SshPassphraseProvider for GitRemoteCredentialOverride<'_, P> { @@ -1856,8 +1865,7 @@ impl GitRepository { ) -> Result { match configured.endpoint() { RemoteEndpoint::Https(_) => { - let (_, server_id, application_id) = require_https_remote(configured)?; - let credential = credentials.credential(server_id, application_id)?; + let credential = credentials.credential_for(configured)?; self.fetch_embedded_https(configured, &credential, control) } RemoteEndpoint::Ssh(_) => { @@ -2171,8 +2179,8 @@ impl GitRepository { control.checkpoint(GitProgressPhase::Authenticating)?; match configured.endpoint() { RemoteEndpoint::Https(_) => { - let (url, server_id, application_id) = require_https_remote(configured)?; - let credential = credentials.credential(server_id, application_id)?; + let (url, _, _) = require_https_remote(configured)?; + let credential = credentials.credential_for(configured)?; let transport = ReqwestGitTransport; let mut client = ReceivePackClient::Https { url, @@ -2228,9 +2236,9 @@ impl GitRepository { control: &GitOperationControl, ) -> Result { let context = self.prepare_push(configured, branch, control)?; - let (url, server_id, application_id) = require_https_remote(configured)?; + let (url, _, _) = require_https_remote(configured)?; control.checkpoint(GitProgressPhase::Authenticating)?; - let credential = credentials.credential(server_id, application_id)?; + let credential = credentials.credential_for(configured)?; let mut client = ReceivePackClient::Https { url, credential, diff --git a/crates/storage/src/secret_store.rs b/crates/storage/src/secret_store.rs index 6e104ff..9b23964 100644 --- a/crates/storage/src/secret_store.rs +++ b/crates/storage/src/secret_store.rs @@ -355,6 +355,12 @@ pub trait SecretStoreBackend: Send + Sync { locator: &SecretLocator, protection: SecretProtection, ) -> Result<(), SecretStoreError>; + fn platform_https_git_credential( + &self, + _endpoint: &url::Url, + ) -> Result, SecretStoreError> { + Ok(None) + } fn lock(&self) -> Result<(), SecretStoreError> { Ok(()) } @@ -848,19 +854,44 @@ impl GitCredentialProvider for SecretStore { let record = self .retrieve_git_record(server, application) .map_err(git_provider_error)?; - let Some(account) = record.reference.account() else { + git_credential_from_record(&record) + } + + fn credential_for(&self, remote: &crate::config::GitRemote) -> Result { + let Some((server, application)) = remote.https_credentials() else { return Err(GitError::CredentialsUnavailable); }; - if record.value.expose().contains(&b'\n') - || record.value.expose().contains(&b'\r') - || record.value.expose().contains(&0) + let endpoint = remote + .endpoint() + .as_https() + .ok_or(GitError::CredentialsUnavailable)?; + match self + .backend + .platform_https_git_credential(endpoint) + .map_err(git_provider_error)? { - return Err(GitError::CredentialsUnavailable); + Some((account, password)) => GitCredential::new(account, password.expose().to_vec()), + None => self + .retrieve_git_record(server, application) + .map_err(git_provider_error) + .and_then(|record| git_credential_from_record(&record)), } - GitCredential::new(account, record.value.expose().to_vec()) } } +fn git_credential_from_record(record: &SecretRecord) -> Result { + let Some(account) = record.reference.account() else { + return Err(GitError::CredentialsUnavailable); + }; + if record.value.expose().contains(&b'\n') + || record.value.expose().contains(&b'\r') + || record.value.expose().contains(&0) + { + return Err(GitError::CredentialsUnavailable); + } + GitCredential::new(account, record.value.expose().to_vec()) +} + impl SshPassphraseProvider for SecretStore { fn ssh_key_passphrase(&self, fingerprint: &SshFingerprint) -> Result { self.retrieve(&SecretReference::ssh_key_passphrase(fingerprint.clone())) diff --git a/crates/storage/src/secret_store/platform.rs b/crates/storage/src/secret_store/platform.rs index 05c3285..1d8a1b4 100644 --- a/crates/storage/src/secret_store/platform.rs +++ b/crates/storage/src/secret_store/platform.rs @@ -20,6 +20,15 @@ use security_framework::{ }, }; +#[cfg(target_os = "macos")] +use security_framework::{ + item::{ItemClass, ItemSearchOptions, Limit}, + os::macos::{ + keychain::SecKeychain, + passwords::{SecAuthenticationType, SecProtocolType}, + }, +}; + use keyring_core::{CredentialStore, Entry}; use super::{SecretLocator, SecretProtection, SecretStoreBackend, SecretStoreError}; @@ -196,6 +205,95 @@ impl SecretStoreBackend for NativeSecretBackend { .delete_credential() .map_err(map_error) } + + fn platform_https_git_credential( + &self, + endpoint: &url::Url, + ) -> Result, SecretStoreError> { + #[cfg(target_os = "macos")] + { + macos_https_git_credential(endpoint) + } + #[cfg(not(target_os = "macos"))] + { + let _ = endpoint; + Ok(None) + } + } +} + +#[cfg(target_os = "macos")] +fn macos_https_git_credential( + endpoint: &url::Url, +) -> Result, SecretStoreError> { + let host = endpoint + .host_str() + .ok_or(SecretStoreError::InvalidReference)?; + let path = endpoint.path(); + let mut search = ItemSearchOptions::new(); + search + .class(ItemClass::internet_password()) + .load_attributes(true) + .limit(Limit::All); + let mut candidates = search + .search() + .map_err(map_security_error)? + .into_iter() + .filter_map(|item| item.simplify_dict()) + .filter_map(|attributes| { + let stored_path = attributes.get("path").map_or("", String::as_str); + if attributes.get("srvr").map(String::as_str) == Some(host) + && attributes.get("ptcl").map(String::as_str) == Some("htps") + && (stored_path.is_empty() || stored_path == path) + { + attributes + .get("acct") + .map(|account| (account.clone(), stored_path.to_owned())) + } else { + None + } + }) + .collect::>(); + candidates.sort_unstable(); + candidates.dedup(); + let [(account, credential_path)] = candidates.as_slice() else { + return if candidates.is_empty() { + Err(SecretStoreError::Missing) + } else { + Err(SecretStoreError::Corrupted) + }; + }; + let keychain = SecKeychain::default().map_err(map_security_error)?; + let (password, _) = keychain + .find_internet_password( + host, + None, + account, + credential_path, + endpoint.port(), + SecProtocolType::HTTPS, + SecAuthenticationType::Default, + ) + .or_else(|error| { + if endpoint.port().is_some() && error.code() == -25300 { + keychain.find_internet_password( + host, + None, + account, + credential_path, + None, + SecProtocolType::HTTPS, + SecAuthenticationType::Default, + ) + } else { + Err(error) + } + }) + .map_err(map_security_error)?; + Ok(Some(( + account.clone(), + SecretBytes::new(password.as_ref().to_vec()), + ))) } #[cfg(any(target_os = "ios", target_os = "macos"))] diff --git a/crates/storage/tests/config_contract.rs b/crates/storage/tests/config_contract.rs index cf573a3..68b3640 100644 --- a/crates/storage/tests/config_contract.rs +++ b/crates/storage/tests/config_contract.rs @@ -133,6 +133,49 @@ key_material = "keys" Ok(()) } +#[cfg(not(any(target_os = "ios", target_os = "watchos")))] +#[test] +fn repository_https_remote_reuses_credentials_only_for_the_same_url() -> TestResult { + let fixture = ConfigurationFixture::new()?; + let vault = fixture.temporary.path().join("cwd/vault"); + fs::create_dir_all(vault.join(".git"))?; + fs::write( + vault.join(".git/config"), + r#"[remote "origin"] + url = https://git.example.test/team/store.git +[remote "mirror"] + url = https://mirror.example.test/team/store.git +"#, + )?; + fixture.write_explicit( + r#" +vault = "../vault" +default_key = "0123456789ABCDEF0123456789ABCDEF01234567" +key_material = "keys" + +[[git.remotes]] +name = "legacy-name" +url = "https://git.example.test/team/store.git" +server_id = "existing-server" +application_id = "existing-application" +"#, + )?; + + let config = fixture + .loader() + .load(Some(Path::new("config/config.toml")))?; + let origin = config.git_remote(Some("origin")).expect("origin remote"); + let (server, application) = origin.https_credentials().expect("HTTPS credentials"); + assert_eq!(server.as_str(), "existing-server"); + assert_eq!(application.as_str(), "existing-application"); + + let mirror = config.git_remote(Some("mirror")).expect("mirror remote"); + let (server, application) = mirror.https_credentials().expect("HTTPS credentials"); + assert_ne!(server.as_str(), "existing-server"); + assert_ne!(application.as_str(), "existing-application"); + Ok(()) +} + #[test] fn git_commit_identity_defaults_validates_and_persists() -> TestResult { let fixture = ConfigurationFixture::new()?; diff --git a/crates/storage/tests/secret_store.rs b/crates/storage/tests/secret_store.rs index 03960bc..28c4f12 100644 --- a/crates/storage/tests/secret_store.rs +++ b/crates/storage/tests/secret_store.rs @@ -32,6 +32,8 @@ struct MemoryState { create_fault: Option, retrieves: usize, protections: Vec, + platform_https: Option<(String, SecretBytes)>, + platform_https_requests: Vec, } #[derive(Clone, Default)] @@ -60,6 +62,13 @@ impl MemoryBackend { self.0.lock().expect("test mutex").protections.clone() } + fn with_platform_https(account: &str, password: &[u8]) -> Self { + let backend = Self::default(); + backend.0.lock().expect("test mutex").platform_https = + Some((account.to_owned(), SecretBytes::new(password.to_vec()))); + backend + } + fn take_fault(state: &mut MemoryState) -> Result<(), SecretStoreError> { match state.fault.take() { Some(error) => Err(error), @@ -138,6 +147,22 @@ impl SecretStoreBackend for MemoryBackend { .ok_or(SecretStoreError::Missing) } + fn platform_https_git_credential( + &self, + endpoint: &url::Url, + ) -> Result, SecretStoreError> { + let mut state = self.0.lock().map_err(|_| SecretStoreError::Unavailable)?; + state + .platform_https_requests + .push(endpoint.as_str().to_owned()); + Ok(state.platform_https.as_ref().map(|(account, password)| { + ( + account.clone(), + SecretBytes::new(password.expose().to_vec()), + ) + })) + } + fn lock(&self) -> Result<(), SecretStoreError> { let mut state = self.0.lock().map_err(|_| SecretStoreError::Unavailable)?; Self::take_fault(&mut state) @@ -149,6 +174,42 @@ impl SecretStoreBackend for MemoryBackend { } } +#[test] +fn endpoint_lookup_uses_the_native_https_credential_before_private_storage() -> TestResult { + let backend = MemoryBackend::with_platform_https("alice", b"native-token"); + let store = store(backend.clone()); + store.unlock()?; + let remote = GitRemote::https( + "origin", + "https://example.test/team/store.git", + "server", + "application", + )?; + let (server, application) = remote.https_credentials().expect("HTTPS credentials"); + store.store_https_git_credential( + server, + application, + "private-account", + SecretBytes::new(b"private-token".to_vec()), + )?; + let retrieves_before = backend.retrieves(); + + let credential = store.credential_for(&remote)?; + + assert_eq!(credential.username(), "alice"); + assert_eq!(credential.password(), b"native-token"); + assert_eq!(backend.retrieves(), retrieves_before); + assert_eq!( + backend + .0 + .lock() + .expect("test mutex") + .platform_https_requests, + ["https://example.test/team/store.git".to_owned()] + ); + Ok(()) +} + #[derive(Clone, Default)] struct MemoryPrompt(Arc>); diff --git a/docs/git-synchronization.md b/docs/git-synchronization.md index 1560c0f..b03fdb2 100644 --- a/docs/git-synchronization.md +++ b/docs/git-synchronization.md @@ -32,7 +32,8 @@ repository mutation instead of treating its configuration as malformed. ## HTTPS transport HTTPS credentials are requested with the configured server ID and application -ID and remain outside Git configuration. +ID and remain outside Git configuration. macOS resolves them solely through +the standard Internet-password credential for the same HTTPS endpoint. Fetch uses the embedded Rust smart-HTTP client with an explicit credential callback, so Git's credential cascade is never entered. Push implements the diff --git a/docs/secure-secret-storage.md b/docs/secure-secret-storage.md index 8c22d19..9cb9613 100644 --- a/docs/secure-secret-storage.md +++ b/docs/secure-secret-storage.md @@ -29,7 +29,10 @@ The platform-selection code is isolated in - macOS uses legacy Keychain for command-line-compatible device-unlocked credentials and Protected Data for `RequireUserPresence`; iOS uses Protected - Data. User cancellation is mapped from the native Security Framework status. + Data. macOS HTTPS synchronization uses only the unique standard + Internet-password credential matching the remote endpoint; it does not use + an IronStorage-specific Git credential locator. User cancellation is mapped + from the native Security Framework status. - Windows uses Credential Manager. Store operations are serialized because the upstream adapter documents unreliable same-entry sequencing across threads. - Linux uses Secret Service through zbus with the Rust cryptography feature. It