feat(aws): fail-closed overwrite check, name local connections, disclose removals in import preview

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

app/src-tauri/src/commands/settings_export_commands.rs

@@ -364,7 +364,7 @@ pub async fn preview_settings_import(
     let encrypted = std::fs::read(&path).map_err(|e| format!("Failed to read export file: {}", e))?;
     let payload = read_and_decrypt_bytes(&encrypted, &password)?;
     let local_hosts = state.settings_store.get().hosts;
-    let preview = SettingsImportPreview::from_payload_against(&payload, &local_hosts, secure::aws_keys_entry_exists);
+    let preview = SettingsImportPreview::from_payload_against(&payload, &local_hosts, secure::aws_keys_entry_state);
 
     *state.pending_settings_import.lock().await = Some(PendingSettingsImport {
         path,

app/src-tauri/src/models/settings_export.rs

@@ -185,6 +185,37 @@ pub struct SettingsImportPreview {
     /// Non-blank AWS access-key entries the import restores to the keychain.
     #[serde(default)]
     pub aws_connection_key_count: usize,
+    /// Local connections the import drops, because it replaces the whole list.
+    /// Their keychain entries, if any, are left behind.
+    #[serde(default)]
+    pub removed_aws_connections: Vec<AwsConnectionSummary>,
+}
+
+/// Cleaned, non-secret description of one connection.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct AwsConnectionSummary {
+    pub name: String,
+    /// `"profile"` or `"keys"`.
+    pub auth: String,
+    pub profile_name: Option<String>,
+    pub default_region: String,
+}
+
+impl AwsConnectionSummary {
+    fn of(h: &crate::models::host_connection::HostConnection) -> Self {
+        use crate::models::host_connection::{AwsAuth, HostKind};
+        let HostKind::Aws { auth, default_region } = &h.kind;
+        let (auth_kind, profile_name) = match auth {
+            AwsAuth::Profile { name } => ("profile", Some(sanitize_for_preview(name))),
+            AwsAuth::Keys { .. } => ("keys", None),
+        };
+        Self {
+            name: sanitize_for_preview(&h.name),
+            auth: auth_kind.to_string(),
+            profile_name,
+            default_region: sanitize_for_preview(default_region),
+        }
+    }
 }
 
 /// One imported AWS connection as the confirmation dialog shows it.
@@ -201,6 +232,14 @@ pub struct AwsConnectionPreview {
     /// The import carries keys for this id and a keychain entry for it
     /// already exists here (its value is never read into the preview).
     pub overwrites_keys: bool,
+    /// The import carries keys for this id and the keychain could not be
+    /// asked whether some are stored already.
+    #[serde(default)]
+    pub keys_check_failed: bool,
+    /// The local connection with this id, when the import replaces it, so an
+    /// imported name cannot hide which connection is being replaced.
+    #[serde(default)]
+    pub local: Option<AwsConnectionSummary>,
 }
 
 /// A cap on how much of a decrypted, not-yet-trusted string gets echoed back
@@ -225,19 +264,20 @@ fn sanitize_for_preview(value: &str) -> String {
 }
 
 impl SettingsImportPreview {
+    #[cfg(test)]
     pub fn from_payload(payload: &SettingsExportPayload) -> Self {
-        Self::from_payload_against(payload, &[], |_| false)
+        Self::from_payload_against(payload, &[], |_| Some(false))
     }
 
-    /// As [`Self::from_payload`], also flagging what the import would
-    /// overwrite: `local_hosts` are the connections held now and
-    /// `local_has_keys` says whether a keychain entry exists for an id.
+    /// Builds the preview, also flagging what the import would overwrite or
+    /// drop: `local_hosts` are the connections held now and `local_has_keys`
+    /// says whether a keychain entry exists for an id (`None`: could not be
+    /// determined).
     pub fn from_payload_against(
         payload: &SettingsExportPayload,
         local_hosts: &[crate::models::host_connection::HostConnection],
-        local_has_keys: impl Fn(&str) -> bool,
+        local_has_keys: impl Fn(&str) -> Option<bool>,
     ) -> Self {
-        use crate::models::host_connection::{AwsAuth, HostKind};
         let carried_keys = |id: &str| {
             payload
                 .secrets
@@ -250,21 +290,29 @@ impl SettingsImportPreview {
             .hosts
             .iter()
             .map(|h| {
-                let HostKind::Aws { auth, default_region } = &h.kind;
-                let (auth_kind, profile_name) = match auth {
-                    AwsAuth::Profile { name } => ("profile", Some(sanitize_for_preview(name))),
-                    AwsAuth::Keys { .. } => ("keys", None),
-                };
+                let summary = AwsConnectionSummary::of(h);
+                let local = local_hosts.iter().find(|l| l.id == h.id);
+                let overwrites_existing = local.is_some_and(|l| l != h);
+                let key_state = if carried_keys(&h.id) { local_has_keys(&h.id) } else { Some(false) };
                 AwsConnectionPreview {
-                    name: sanitize_for_preview(&h.name),
-                    auth: auth_kind.to_string(),
-                    profile_name,
-                    default_region: sanitize_for_preview(default_region),
-                    overwrites_existing: local_hosts.iter().any(|l| l.id == h.id && l != h),
-                    overwrites_keys: carried_keys(&h.id) && local_has_keys(&h.id),
+                    name: summary.name,
+                    auth: summary.auth,
+                    profile_name: summary.profile_name,
+                    default_region: summary.default_region,
+                    overwrites_existing,
+                    overwrites_keys: key_state == Some(true),
+                    keys_check_failed: key_state.is_none(),
+                    local: local
+                        .filter(|_| overwrites_existing || key_state != Some(false))
+                        .map(AwsConnectionSummary::of),
                 }
             })
             .collect();
+        let removed_aws_connections = local_hosts
+            .iter()
+            .filter(|l| !payload.settings.hosts.iter().any(|h| h.id == l.id))
+            .map(AwsConnectionSummary::of)
+            .collect();
         let non_blank = |s: &Option<String>| s.as_deref().is_some_and(|v| !v.trim().is_empty());
         let sanitized_non_blank = |s: &Option<String>| {
             s.as_deref()
@@ -316,6 +364,7 @@ impl SettingsImportPreview {
                 .values()
                 .filter(|v| !v.trim().is_empty())
                 .count(),
+            removed_aws_connections,
         }
     }
 }
@@ -605,7 +654,7 @@ mod tests {
         ];
         let local = vec![aws_host("same", "Same", false), aws_host("changed", "Old name", false)];
         let preview =
-            SettingsImportPreview::from_payload_against(&payload, &local, |id| id == "k1");
+            SettingsImportPreview::from_payload_against(&payload, &local, |id| Some(id == "k1"));
         let flags: Vec<_> = preview
             .aws_connections
             .iter()
@@ -613,4 +662,38 @@ mod tests {
             .collect();
         assert_eq!(flags, vec![(false, true), (false, false), (true, false), (false, false)]);
     }
+
+    #[test]
+    fn aws_preview_names_the_local_connection_and_discloses_removals_and_failed_checks() {
+        let mut payload = payload_with(ExportedSecrets {
+            aws_connection_keys: BTreeMap::from([
+                ("k1".to_string(), "{}".to_string()),
+                ("k2".to_string(), "{}".to_string()),
+            ]),
+            ..Default::default()
+        });
+        payload.settings.hosts = vec![
+            aws_host("k1", "Innocent", true),
+            aws_host("k2", "Other", true),
+            aws_host("p1", "Fresh Name", false),
+        ];
+        let local = vec![
+            aws_host("k1", "Real Prod", true),
+            aws_host("p1", "Real Dev", false),
+            aws_host("gone", "Dropped\n", false),
+        ];
+        let preview = SettingsImportPreview::from_payload_against(&payload, &local, |id| match id {
+            "k1" => Some(true),
+            _ => None,
+        });
+        let c = &preview.aws_connections;
+        assert!(c[0].overwrites_keys && !c[0].keys_check_failed);
+        assert_eq!(c[0].local.as_ref().unwrap().name, "Real Prod");
+        assert!(c[1].keys_check_failed && !c[1].overwrites_keys && c[1].local.is_none());
+        assert!(c[2].overwrites_existing);
+        assert_eq!(c[2].local.as_ref().unwrap().name, "Real Dev");
+        assert_eq!(c[2].local.as_ref().unwrap().profile_name.as_deref(), Some("prod-profile"));
+        assert_eq!(preview.removed_aws_connections.len(), 1);
+        assert_eq!(preview.removed_aws_connections[0].name, "Dropped");
+    }
 }

app/src-tauri/src/storage/secure.rs

@@ -474,12 +474,19 @@ pub fn get_aws_keys(connection_id: &str) -> Result<Option<AwsKeys>, String> {
         .map_err(|_| "The stored AWS access keys are unreadable".to_string())
 }
 
-/// Whether any entry exists for the connection, readable or not. Never
-/// returns or logs a value.
-pub fn aws_keys_entry_exists(connection_id: &str) -> bool {
-    aws_connection_service(connection_id)
-        .and_then(|svc| read_entry(&svc, "the AWS access keys"))
-        .is_ok_and(|v| v.is_some())
+/// Whether an entry exists for the connection, readable or not. `None` means
+/// the keychain could not be asked, so callers must not read it as "absent".
+/// Never returns or logs a value.
+pub fn aws_keys_entry_state(connection_id: &str) -> Option<bool> {
+    let svc = aws_connection_service(connection_id).ok()?;
+    let entry = keyring::Entry::new(&svc, KEYCHAIN_ACCOUNT).ok()?;
+    match entry.get_password() {
+        Ok(_) => Some(true),
+        Err(keyring::Error::NoEntry) => Some(false),
+        // Present but not a plain string, etc.: something is there.
+        Err(keyring::Error::BadEncoding(_)) => Some(true),
+        Err(_) => None,
+    }
 }
 
 /// Missing entry is success. Unused until connection removal lands.

app/src/components/settings/ImportSettingsModal.test.tsx

@@ -38,6 +38,7 @@ const samplePreview: SettingsImportPreview = {
   marketplace_account_token_count: 0,
   aws_connections: [],
   aws_connection_key_count: 0,
+  removed_aws_connections: [],
 };
 
 function outcome(settings: AppSettings, secretRestoreWarnings: string[] = []): SettingsImportOutcome {
@@ -93,6 +94,8 @@ describe("ImportSettingsModal", () => {
           default_region: "us-west-2",
           overwrites_existing: true,
           overwrites_keys: true,
+          keys_check_failed: false,
+          local: { name: "Real Prod", auth: "keys", profile_name: null, default_region: "us-east-1" },
         },
         {
           name: "Dev",
@@ -101,9 +104,14 @@ describe("ImportSettingsModal", () => {
           default_region: "eu-west-1",
           overwrites_existing: false,
           overwrites_keys: false,
+          keys_check_failed: true,
+          local: null,
         },
       ],
       aws_connection_key_count: 1,
+      removed_aws_connections: [
+        { name: "Old", auth: "profile", profile_name: "old-p", default_region: "ap-south-1" },
+      ],
     });
     render(<ImportSettingsModal onClose={vi.fn()} onImported={vi.fn()} />);
 
@@ -113,8 +121,10 @@ describe("ImportSettingsModal", () => {
     expect(await screen.findByText(/AWS connection "Prod" \(access keys, us-west-2\)/)).toBeInTheDocument();
     expect(screen.getByText(/AWS connection "Dev" \(profile dev-profile, eu-west-1\)/)).toBeInTheDocument();
     expect(screen.getByText(/1 AWS access key set$/)).toBeInTheDocument();
-    expect(screen.getByText(/Overwrites the AWS access keys stored for "Prod"/)).toBeInTheDocument();
-    expect(screen.getByText(/Replaces your existing AWS connection.*"Prod"/)).toBeInTheDocument();
+    expect(screen.getByText(/Overwrites the AWS access keys stored for your connection "Real Prod"/)).toBeInTheDocument();
+    expect(screen.getByText(/Replaces your connection "Real Prod" \(access keys, us-east-1\) with "Prod"/)).toBeInTheDocument();
+    expect(screen.getByText(/Could not check whether keys are stored for "Dev".*may overwrite them/)).toBeInTheDocument();
+    expect(screen.getByText(/Removes your AWS connection "Old" \(profile old-p, ap-south-1\)/)).toBeInTheDocument();
   });
 
   it("warns about a custom Docker image every time, not just on change", async () => {

app/src/lib/settingsImportPreview.test.ts

@@ -26,6 +26,7 @@ function preview(overrides: Partial<SettingsImportPreview> = {}): SettingsImport
     marketplace_account_token_count: 0,
     aws_connections: [],
     aws_connection_key_count: 0,
+    removed_aws_connections: [],
     ...overrides,
   };
 }

app/src/lib/settingsImportPreview.ts

@@ -1,4 +1,4 @@
-import type { SettingsImportPreview } from "./types";
+import type { AwsConnectionSummary, SettingsImportPreview } from "./types";
 
 /** Named things a `SettingsImportPreview` says an import will change, for
  *  `ImportSettingsModal`'s confirmation list. Does not include anything
@@ -91,12 +91,27 @@ export function describeImportWarnings(preview: SettingsImportPreview): string[]
     );
   }
   for (const c of preview.aws_connections) {
-    if (c.overwrites_existing) {
-      warnings.push(`Replaces your existing AWS connection with different settings: "${c.name}".`);
+    const localDesc = c.local ? describeAws(c.local) : null;
+    if (c.overwrites_existing && c.local) {
+      warnings.push(`Replaces your connection ${localDesc} with "${c.name}" (${c.auth === "keys" ? "access keys" : "profile " + (c.profile_name ?? "")}, ${c.default_region}).`);
     }
     if (c.overwrites_keys) {
-      warnings.push(`Overwrites the AWS access keys stored for "${c.name}".`);
+      warnings.push(
+        c.local
+          ? `Overwrites the AWS access keys stored for your connection ${localDesc} (imported as "${c.name}").`
+          : `Overwrites AWS access keys already stored for the imported connection "${c.name}".`,
+      );
     }
+    if (c.keys_check_failed) {
+      warnings.push(
+        `Could not check whether keys are stored for "${c.name}"${c.local ? ` (your connection ${localDesc})` : ""}; importing may overwrite them.`,
+      );
+    }
+  }
+  for (const r of preview.removed_aws_connections) {
+    warnings.push(
+      `Removes your AWS connection ${describeAws(r)}, which is not in the file. Any access keys stored for it stay in your keychain.`,
+    );
   }
   if (preview.image_source === "custom") {
     warnings.push(
@@ -105,3 +120,8 @@ export function describeImportWarnings(preview: SettingsImportPreview): string[]
   }
   return warnings;
 }
+
+function describeAws(c: AwsConnectionSummary): string {
+  const auth = c.auth === "keys" ? "access keys" : `profile ${c.profile_name ?? ""}`.trim();
+  return `"${c.name}" (${auth}, ${c.default_region})`;
+}

app/src/lib/types.ts

@@ -429,6 +429,15 @@ export interface SettingsImportPreview {
   aws_connections: AwsConnectionPreview[];
   /** AWS access-key entries the import restores to the keychain. */
   aws_connection_key_count: number;
+  /** Local AWS connections the import drops (it replaces the whole list). */
+  removed_aws_connections: AwsConnectionSummary[];
+}
+
+export interface AwsConnectionSummary {
+  name: string;
+  auth: "profile" | "keys";
+  profile_name: string | null;
+  default_region: string;
 }
 
 export interface AwsConnectionPreview {
@@ -440,6 +449,10 @@ export interface AwsConnectionPreview {
   overwrites_existing: boolean;
   /** The import carries keys for an id that already has keys stored here. */
   overwrites_keys: boolean;
+  /** The keychain could not be asked whether keys are already stored. */
+  keys_check_failed: boolean;
+  /** The local connection with this id, when the import replaces it. */
+  local: AwsConnectionSummary | null;
 }
 
 /** What `apply_settings_import` returns: the settings that were actually