diff --git a/app/src-tauri/src/commands/settings_export_commands.rs b/app/src-tauri/src/commands/settings_export_commands.rs index b219389..0dac02f 100644 --- a/app/src-tauri/src/commands/settings_export_commands.rs +++ b/app/src-tauri/src/commands/settings_export_commands.rs @@ -43,6 +43,7 @@ use zeroize::Zeroizing; use std::collections::BTreeMap; +use crate::models::host_connection::{AwsAuth, HostConnection, HostKind}; use crate::models::marketplace::{AccountMethod, MarketplaceAccount}; use crate::models::{ AppSettings, ExportedSecrets, SettingsExportPayload, SettingsImportOutcome, @@ -138,6 +139,7 @@ fn split_settings_and_secrets(current: AppSettings) -> (AppSettings, ExportedSec &settings.marketplace_accounts, secure::get_marketplace_token, ), + aws_connection_keys: exported_aws_connection_keys(&settings.hosts, secure::get_aws_keys), }; (settings, secrets) @@ -184,6 +186,107 @@ fn restore_marketplace_tokens( warnings } +/// The stored keys of every `Keys` host connection, by connection id, as the +/// `AwsKeys` JSON. A `Profile` connection holds none. A missing or unreadable +/// entry is left out, like the other keychain secrets above. +fn exported_aws_connection_keys( + hosts: &[HostConnection], + get: impl Fn(&str) -> Result, String>, +) -> BTreeMap { + hosts + .iter() + .filter(|h| matches!(&h.kind, HostKind::Aws { auth: AwsAuth::Keys { .. }, .. })) + .filter_map(|h| { + let keys = get(&h.id).unwrap_or_default()?; + Some((h.id.clone(), serde_json::to_string(&keys).ok()?)) + }) + .collect() +} + +/// Write each imported set of AWS keys to the keychain, returning a warning +/// (naming the connection id, never the value) for each that could not be. +fn restore_aws_connection_keys( + keys: &BTreeMap, + mut store: impl FnMut(&str, &secure::AwsKeys) -> Result<(), String>, +) -> Vec { + let mut warnings = Vec::new(); + for (id, json) in keys { + if json.trim().is_empty() { + continue; + } + let Ok(parsed) = serde_json::from_str::(json) else { + log::warn!("Settings import: the AWS keys of connection {} are malformed", id); + warnings.push(format!( + "The AWS access keys of connection {} in the file are malformed; enter them again.", + id + )); + continue; + }; + if let Err(e) = store(id, &parsed) { + log::warn!( + "Settings import: could not restore the AWS keys of connection {}: {}", + id, + e + ); + warnings.push(format!( + "Could not restore the AWS access keys of connection {} ({}); enter them again.", + id, e + )); + } + } + warnings +} + +/// Checks the imported host connections with the rules the connection +/// commands use, before anything is written; refuses the import (like +/// `validate_imported_marketplace_state`) rather than applying a bad entry. +/// A `Keys` connection's `keyring_ref` must be its own (valid) id, and keys +/// may only be carried for a `Keys` connection in the file. +fn validate_imported_hosts( + settings: &mut AppSettings, + keys: &BTreeMap, +) -> Result<(), String> { + use crate::models::host_connection::{ + validate_connection_name, validate_profile_name, validate_region, + }; + let wrap = |e: String| format!("The file's host connections were refused: {e}"); + + let mut seen = std::collections::HashSet::new(); + for h in &mut settings.hosts { + if !secure::is_valid_connection_id(&h.id) { + return Err(wrap("a connection has an invalid id.".into())); + } + if !seen.insert(h.id.clone()) { + return Err(wrap("a connection appears twice.".into())); + } + h.name = validate_connection_name(&h.name).map_err(wrap)?; + let HostKind::Aws { auth, default_region } = &h.kind; + validate_region(default_region).map_err(wrap)?; + match auth { + AwsAuth::Profile { name } => { + validate_profile_name(name).map_err(wrap)?; + } + AwsAuth::Keys { keyring_ref } => { + if !secure::is_valid_connection_id(keyring_ref) || *keyring_ref != h.id { + return Err(wrap(format!( + "the connection \"{}\" has an invalid key reference.", + h.name + ))); + } + } + } + } + for id in keys.keys() { + let is_keys_connection = settings.hosts.iter().any(|h| { + h.id == *id && matches!(&h.kind, HostKind::Aws { auth: AwsAuth::Keys { .. }, .. }) + }); + if !is_keys_connection { + return Err(wrap("access keys are present for an unknown connection.".into())); + } + } + Ok(()) +} + /// Export the current global settings and secrets to a password-encrypted /// file. `Ok(false)` means the save dialog was dismissed — not an error, and /// deliberately distinguishable from one so the frontend shows nothing @@ -376,6 +479,9 @@ pub async fn apply_settings_import( &marketplace_tokens, )?; + let aws_keys = payload.secrets.aws_connection_keys; + validate_imported_hosts(&mut settings, &aws_keys)?; + let mut secret_restore_warnings = Vec::new(); let mut gateway_secret_changed = false; @@ -423,6 +529,11 @@ pub async fn apply_settings_import( secure::store_marketplace_token, )); + secret_restore_warnings.extend(restore_aws_connection_keys( + &aws_keys, + secure::store_aws_keys, + )); + let imported_marketplace = ( settings.marketplace_accounts.clone(), settings.marketplaces.clone(), @@ -827,4 +938,109 @@ mod tests { assert_eq!(warnings.len(), 1); assert!(!warnings[0].contains("test-token-not-real")); } + + fn keys_host(id: &str) -> HostConnection { + HostConnection { + id: id.into(), + name: "Prod".into(), + kind: HostKind::Aws { + auth: AwsAuth::Keys { keyring_ref: id.into() }, + default_region: "us-west-2".into(), + }, + } + } + + fn profile_host(id: &str) -> HostConnection { + HostConnection { + id: id.into(), + name: "Dev".into(), + kind: HostKind::Aws { + auth: AwsAuth::Profile { name: "default".into() }, + default_region: "us-west-2".into(), + }, + } + } + + fn test_keys() -> secure::AwsKeys { + secure::AwsKeys { + access_key_id: "AKIATESTNOTREAL".into(), + secret_access_key: "test-secret-not-real".into(), + session_token: None, + } + } + + #[test] + fn aws_connection_keys_round_trip_through_export() { + let hosts = vec![keys_host("k1"), profile_host("p1")]; + let exported = exported_aws_connection_keys(&hosts, |id| match id { + "k1" => Ok(Some(test_keys())), + other => panic!("only a Keys connection is read, got {other}"), + }); + assert_eq!(exported.keys().collect::>(), vec!["k1"]); + assert_eq!(serde_json::from_str::(&exported["k1"]).unwrap(), test_keys()); + + let mut stored = Vec::new(); + let warnings = restore_aws_connection_keys(&exported, |id, keys| { + stored.push((id.to_string(), keys.clone())); + Ok(()) + }); + assert!(warnings.is_empty()); + assert_eq!(stored, vec![("k1".to_string(), test_keys())]); + } + + #[test] + fn aws_connection_keys_restore_failures_name_the_id_not_the_value() { + let keys = BTreeMap::from([ + ("bad".to_string(), "not json test-secret-not-real".to_string()), + ("locked".to_string(), serde_json::to_string(&test_keys()).unwrap()), + ]); + let warnings = restore_aws_connection_keys(&keys, |_, _| Err("keychain locked".into())); + assert_eq!(warnings.len(), 2); + assert!(warnings[0].contains("bad")); + assert!(warnings[1].contains("locked")); + assert!(warnings.iter().all(|w| !w.contains("test-secret-not-real"))); + } + + #[test] + fn imported_hosts_are_validated() { + let check = |hosts: Vec, keys: BTreeMap| { + let mut s = AppSettings::default(); + s.hosts = hosts; + validate_imported_hosts(&mut s, &keys) + }; + let none = BTreeMap::new(); + assert!(check(vec![keys_host("k1"), profile_host("p1")], none.clone()).is_ok()); + assert!(check(vec![keys_host("k1")], BTreeMap::from([("k1".into(), "{}".into())])).is_ok()); + + let mut bad_name = profile_host("p1"); + bad_name.name = " ".into(); + assert!(check(vec![bad_name], none.clone()).is_err()); + + let mut bad_region = profile_host("p1"); + bad_region.kind = HostKind::Aws { + auth: AwsAuth::Profile { name: "default".into() }, + default_region: "nowhere".into(), + }; + assert!(check(vec![bad_region], none.clone()).is_err()); + + let mut bad_profile = profile_host("p1"); + bad_profile.kind = HostKind::Aws { + auth: AwsAuth::Profile { name: "a b".into() }, + default_region: "us-west-2".into(), + }; + assert!(check(vec![bad_profile], none.clone()).is_err()); + + let mut other_ref = keys_host("k1"); + other_ref.kind = HostKind::Aws { + auth: AwsAuth::Keys { keyring_ref: "someone-else".into() }, + default_region: "us-west-2".into(), + }; + assert!(check(vec![other_ref], none.clone()).is_err()); + + assert!(check(vec![profile_host("../x")], none.clone()).is_err()); + assert!(check(vec![profile_host("p1"), profile_host("p1")], none.clone()).is_err()); + // Keys carried for a connection that is not a Keys connection. + assert!(check(vec![profile_host("p1")], BTreeMap::from([("p1".into(), "{}".into())])).is_err()); + assert!(check(vec![], BTreeMap::from([("ghost".into(), "{}".into())])).is_err()); + } } diff --git a/app/src-tauri/src/models/settings_export.rs b/app/src-tauri/src/models/settings_export.rs index 85396cd..0631e95 100644 --- a/app/src-tauri/src/models/settings_export.rs +++ b/app/src-tauri/src/models/settings_export.rs @@ -64,6 +64,11 @@ pub struct ExportedSecrets { /// here or an imported account could never fetch. #[serde(default)] pub marketplace_account_tokens: BTreeMap, + /// Access keys of `AwsAuth::Keys` host connections, keyed by connection + /// id; each value is the `AwsKeys` JSON. Like the tokens above they live + /// in the keychain, not in `AppSettings::hosts`. + #[serde(default)] + pub aws_connection_keys: BTreeMap, } impl ExportedSecrets { @@ -74,6 +79,7 @@ impl ExportedSecrets { && blank(&self.gateway_master_key) && blank(&self.web_terminal_access_token) && self.marketplace_account_tokens.values().all(|v| v.trim().is_empty()) + && self.aws_connection_keys.values().all(|v| v.trim().is_empty()) } } diff --git a/app/src-tauri/src/storage/secure.rs b/app/src-tauri/src/storage/secure.rs index ef68da2..4b29dad 100644 --- a/app/src-tauri/src/storage/secure.rs +++ b/app/src-tauri/src/storage/secure.rs @@ -167,7 +167,12 @@ fn read_entry(service: &str, label: &str) -> Result, String> { fn delete_entry(service: &str, label: &str) -> Result<(), String> { let entry = keyring::Entry::new(service, KEYCHAIN_ACCOUNT) .map_err(|e| format!("Keyring error: {}", e))?; - match entry.delete_credential() { + delete_outcome(entry.delete_credential(), label) +} + +/// "Wasn't there" is success; any other failure names the entry, not its value. +fn delete_outcome(result: Result<(), keyring::Error>, label: &str) -> Result<(), String> { + match result { Ok(()) | Err(keyring::Error::NoEntry) => Ok(()), Err(e) => Err(format!("Failed to delete {}: {}", label, e)), } @@ -408,6 +413,74 @@ pub fn delete_marketplace_token(account_id: &str) -> Result<(), String> { delete_entry(&marketplace_token_service(account_id)?, "the marketplace account token") } +// ───────────────────────────────────────────────────────────────────────────── +// AWS connection access keys (global, one entry per connection) +// ───────────────────────────────────────────────────────────────────────────── + +/// Keychain service prefix; the connection id completes it. +const AWS_CONNECTION_SERVICE_PREFIX: &str = "triple-c-aws-connection-"; + +/// Static AWS credentials for a `HostConnection` using `AwsAuth::Keys`. Held in +/// the keychain as one JSON value. `Debug` redacts the secret and the token. +#[derive(Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq)] +pub struct AwsKeys { + pub access_key_id: String, + pub secret_access_key: String, + pub session_token: Option, +} + +impl std::fmt::Debug for AwsKeys { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("AwsKeys") + .field("access_key_id", &self.access_key_id) + .field("secret_access_key", &"") + .field("session_token", &self.session_token.as_ref().map(|_| "")) + .finish() + } +} + +/// Whether `id` may be used as a connection id / keyring reference: 1..=64 of +/// `[A-Za-z0-9-]`. The same rule as marketplace account ids. +pub fn is_valid_connection_id(id: &str) -> bool { + !id.is_empty() + && id.len() <= 64 + && id.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-') +} + +fn aws_connection_service(connection_id: &str) -> Result { + if !is_valid_connection_id(connection_id) { + return Err(format!("Invalid AWS connection id {:?}", connection_id)); + } + Ok(format!("{}{}", AWS_CONNECTION_SERVICE_PREFIX, connection_id)) +} + +pub fn store_aws_keys(connection_id: &str, keys: &AwsKeys) -> Result<(), String> { + let service = aws_connection_service(connection_id)?; + let json = serde_json::to_string(keys) + .map_err(|_| "Failed to encode the AWS access keys".to_string())?; + let entry = keyring::Entry::new(&service, KEYCHAIN_ACCOUNT) + .map_err(|e| format!("Keyring error: {}", e))?; + entry + .set_password(&json) + .map_err(|e| format!("Failed to store the AWS access keys: {}", e)) +} + +pub fn get_aws_keys(connection_id: &str) -> Result, String> { + let Some(json) = read_entry(&aws_connection_service(connection_id)?, "the AWS access keys")? + else { + return Ok(None); + }; + serde_json::from_str(&json) + .map(Some) + .map_err(|_| "The stored AWS access keys are unreadable".to_string()) +} + +/// Missing entry is success. Unused until connection removal lands. +#[allow(dead_code)] +pub fn delete_aws_keys(connection_id: &str) -> Result<(), String> { + delete_entry(&aws_connection_service(connection_id)?, "the AWS access keys") +} + #[cfg(test)] mod tests { use super::*; @@ -496,4 +569,39 @@ mod tests { // Pasted credentials routinely carry a trailing newline. assert_eq!(secret_to_store(Some(" ghp_abc123\n")), Some("ghp_abc123")); } + + #[test] + fn aws_keys_debug_redacts_secrets() { + let keys = AwsKeys { + access_key_id: "AKIATESTNOTREAL".into(), + secret_access_key: "test-secret-not-real".into(), + session_token: Some("test-session-token-not-real".into()), + }; + let shown = format!("{:?}", keys); + assert!(!shown.contains("test-secret-not-real"), "{shown}"); + assert!(!shown.contains("test-session-token-not-real"), "{shown}"); + } + + #[test] + fn aws_key_service_names_reject_bad_ids() { + for bad in ["../x", "", &"a".repeat(65)] { + assert!(aws_connection_service(bad).is_err(), "{bad:?}"); + assert!(get_aws_keys(bad).is_err()); + assert!(delete_aws_keys(bad).is_err()); + } + assert_eq!( + aws_connection_service("0b9e6a2c-1111").unwrap(), + "triple-c-aws-connection-0b9e6a2c-1111" + ); + } + + /// There is no mock keyring here, so the "no entry is fine" rule is tested + /// as the pure mapping `delete_entry` applies. + #[test] + fn removing_a_keys_connection_with_no_keyring_entry_succeeds() { + assert_eq!(delete_outcome(Err(keyring::Error::NoEntry), "x"), Ok(())); + assert_eq!(delete_outcome(Ok(()), "x"), Ok(())); + let err = delete_outcome(Err(keyring::Error::NoStorageAccess("denied".into())), "x"); + assert!(err.is_err()); + } }