From 722d9aeff14ae9d5885673fab7c937bdab3997dc Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 27 Aug 2026 11:57:16 -0700 Subject: [PATCH 1/4] Add password-encrypted settings export/import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #35. Exports the host environment — global AppSettings (already the non-secret shape persisted to settings.json) plus the global secrets that live in the OS keychain instead (the shared Claude Code OAuth login, the model gateway's provider API key and master key) — to one password-encrypted file, and restores it on another machine. Per-project settings, per-project secrets, and Docker volumes are deliberately out of scope; this is not a project backup. Designed with the user in issue #35's comments: global settings only, no docker volumes, the password is the lock/key, and the export is portable as one file. Crypto (storage/settings_crypto.rs): Argon2id derives a 256-bit key from the password (memory-hard, meaningfully resistant to GPU/ASIC brute-forcing in a way PBKDF2 at any reasonable iteration count is not), AES-256-GCM does the actual encryption. A wrong password fails GCM's authentication tag rather than producing silent garbage. Salt and nonce are random per export and stored in the clear in the file header — their job is uniqueness, not secrecy. The save/open dialogs are opened from Rust, matching the boundary file_commands.rs's pick_save_path/pick_files_to_upload already establish: a frontend-driven dialog handing Rust a host path is the exact shape of bug that produced this app's past criticals. preview_settings_import resolves the chosen import path itself and remembers it (AppState::pending_settings_import) so apply_settings_import re-reads the same file without a path crossing back over IPC. The password is re-entered rather than cached between preview and apply, so nothing here holds decrypted plaintext in memory for longer than one command's execution; the preview returned to the frontend carries counts and presence flags only, never a secret value. Import replaces settings wholesale (an import is "restore this environment"), but only writes secrets actually present in the file — an absent secret means "the source machine never had this configured," not "delete this on import." Added storage::secure::store_gateway_master_key and get_gateway_master_key (read-only, unlike get_or_create_gateway_master_key which mints one as a side effect) since neither existed and import needs to restore an exact captured value rather than mint a new random one. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ --- CLAUDE.md | 34 +++ app/src-tauri/Cargo.lock | 143 +++++++++++ app/src-tauri/Cargo.toml | 2 + app/src-tauri/src/commands/mod.rs | 1 + .../src/commands/settings_export_commands.rs | 232 ++++++++++++++++++ app/src-tauri/src/lib.rs | 14 ++ app/src-tauri/src/models/mod.rs | 2 + app/src-tauri/src/models/settings_export.rs | 180 ++++++++++++++ app/src-tauri/src/storage/mod.rs | 1 + app/src-tauri/src/storage/secure.rs | 38 ++- app/src-tauri/src/storage/settings_crypto.rs | 158 ++++++++++++ .../settings/ExportSettingsModal.test.tsx | 72 ++++++ .../settings/ExportSettingsModal.tsx | 119 +++++++++ .../settings/ImportSettingsModal.test.tsx | 91 +++++++ .../settings/ImportSettingsModal.tsx | 139 +++++++++++ app/src/components/settings/SettingsPanel.tsx | 39 ++- app/src/hooks/useSettings.ts | 4 + app/src/lib/settingsImportPreview.test.ts | 55 +++++ app/src/lib/settingsImportPreview.ts | 20 ++ app/src/lib/tauri-commands.ts | 11 +- app/src/lib/types.ts | 14 ++ 21 files changed, 1360 insertions(+), 9 deletions(-) create mode 100644 app/src-tauri/src/commands/settings_export_commands.rs create mode 100644 app/src-tauri/src/models/settings_export.rs create mode 100644 app/src-tauri/src/storage/settings_crypto.rs create mode 100644 app/src/components/settings/ExportSettingsModal.test.tsx create mode 100644 app/src/components/settings/ExportSettingsModal.tsx create mode 100644 app/src/components/settings/ImportSettingsModal.test.tsx create mode 100644 app/src/components/settings/ImportSettingsModal.tsx create mode 100644 app/src/lib/settingsImportPreview.test.ts create mode 100644 app/src/lib/settingsImportPreview.ts diff --git a/CLAUDE.md b/CLAUDE.md index ab75119..39d3024 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -552,6 +552,40 @@ survived 92 commits and fourteen days in the public GitHub mirror, past five aud independent reviews, because every one of them read the code under change and this sat in a test nobody had reason to open. Fixtures are never live values; there is no case where they need to be. +## Settings export/import + +`commands::settings_export_commands`, `storage::settings_crypto`, `models::settings_export` +(triple-c#35). Exports the *host* environment — global `AppSettings` (already the non-secret +shape persisted to `settings.json`) plus the global secrets that live in the OS keychain instead: +the shared Claude Code OAuth login and the model gateway's two keys. Per-project settings, +per-project secrets, and anything in a project's Docker volumes are deliberately out of scope — +this is not a project backup. + +- **Encrypted because it can carry live credentials, not for appearance's sake.** Argon2id derives + a 256-bit key from the user's password (memory-hard — meaningfully resistant to GPU/ASIC + brute-forcing, unlike PBKDF2 at any reasonable iteration count), AES-256-GCM does the actual + encryption. A wrong password fails GCM's authentication tag rather than producing silent + garbage. The salt and nonce are not secret and are written in the clear in the file's own + header — the salt's job is only to make two exports of the same password derive different keys, + and the nonce's only requirement is per-encryption uniqueness, which a fresh random draw on + every export already gives it. +- **The save/open dialogs are opened from Rust**, the same boundary `file_commands.rs`'s + `pick_save_path`/`pick_files_to_upload` draw and document at length: a frontend-driven dialog + handing Rust a host path string is the exact shape of bug that produced this app's past + criticals. `preview_settings_import` resolves the chosen path itself and remembers it + (`AppState::pending_settings_import`) so `apply_settings_import` re-reads the same file without + a path ever crossing back over IPC. +- **The password is re-entered, not cached, between preview and apply.** Nothing here holds + decrypted plaintext — secrets included — in memory for longer than one command's execution. + `preview_settings_import` returns counts and presence flags only (`SettingsImportPreview`), + never a secret value, so it's safe to hand to the frontend and render directly. +- **Import replaces settings wholesale, but only writes secrets actually present in the file.** + An import is "restore this environment," so the settings half is a full replace, not a + field-by-field merge. Secrets are different on purpose: an absent secret in the export means + "the source machine never had this configured," not "delete this on import" — a user who wants + to clear a secret already has dedicated UI for that (signing out of shared auth, clearing the + gateway key). + ## Testing Frontend tests use Vitest with jsdom environment and React Testing Library. Setup file at `src/test/setup.ts`. Run a single test file: diff --git a/app/src-tauri/Cargo.lock b/app/src-tauri/Cargo.lock index c75619d..b5b92bd 100644 --- a/app/src-tauri/Cargo.lock +++ b/app/src-tauri/Cargo.lock @@ -8,6 +8,41 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "aead" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" +dependencies = [ + "crypto-common", + "generic-array", +] + +[[package]] +name = "aes" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b169f7a6d4742236a0a00c541b845991d0ac43e546831af1249753ab4c3aa3a0" +dependencies = [ + "cfg-if", + "cipher", + "cpufeatures", +] + +[[package]] +name = "aes-gcm" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "831010a0f742e1209b3bcea8fab6a8e149051ba6099432c8cb2cc117dec3ead1" +dependencies = [ + "aead", + "aes", + "cipher", + "ctr", + "ghash", + "subtle", +] + [[package]] name = "aho-corasick" version = "1.1.4" @@ -47,6 +82,18 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "argon2" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072" +dependencies = [ + "base64ct", + "blake2", + "cpufeatures", + "password-hash", +] + [[package]] name = "async-broadcast" version = "0.7.2" @@ -280,6 +327,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64ct" +version = "1.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2af50177e190e07a26ab74f8b1efbfe2ef87da2116221318cb1c2e82baf7de06" + [[package]] name = "bit-set" version = "0.8.0" @@ -310,6 +363,15 @@ dependencies = [ "serde_core", ] +[[package]] +name = "blake2" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" +dependencies = [ + "digest", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -569,6 +631,16 @@ dependencies = [ "windows-link 0.2.1", ] +[[package]] +name = "cipher" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" +dependencies = [ + "crypto-common", + "inout", +] + [[package]] name = "combine" version = "4.6.7" @@ -694,6 +766,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" dependencies = [ "generic-array", + "rand_core 0.6.4", "typenum", ] @@ -753,6 +826,15 @@ version = "0.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "52560adf09603e58c9a7ee1fe1dcb95a16927b17c127f0ac02d6e768a0e25bc1" +[[package]] +name = "ctr" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0369ee1ad671834580515889b80f2ea915f23b8be8d0daa4bbaf2ac5c7590835" +dependencies = [ + "cipher", +] + [[package]] name = "darling" version = "0.20.11" @@ -923,6 +1005,7 @@ checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer", "crypto-common", + "subtle", ] [[package]] @@ -1550,6 +1633,16 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "ghash" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0d8a4362ccb29cb0b265253fb0a2728f592895ee6854fd9bc13f2ffda266ff1" +dependencies = [ + "opaque-debug", + "polyval", +] + [[package]] name = "gio" version = "0.18.4" @@ -2114,6 +2207,15 @@ dependencies = [ "cfb", ] +[[package]] +name = "inout" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "879f10e63c20629ecabbb64a8010319738c66a5cd0c29b02d63d272b03751d01" +dependencies = [ + "generic-array", +] + [[package]] name = "ipnet" version = "2.11.0" @@ -2831,6 +2933,12 @@ version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" +[[package]] +name = "opaque-debug" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" + [[package]] name = "open" version = "5.3.3" @@ -2913,6 +3021,17 @@ dependencies = [ "windows-link 0.2.1", ] +[[package]] +name = "password-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +dependencies = [ + "base64ct", + "rand_core 0.6.4", + "subtle", +] + [[package]] name = "pathdiff" version = "0.2.3" @@ -3194,6 +3313,18 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "polyval" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d1fe60d06143b2430aa532c94cfe9e29783047f06c0d7fd359a9a51b729fa25" +dependencies = [ + "cfg-if", + "cpufeatures", + "opaque-debug", + "universal-hash", +] + [[package]] name = "potential_utf" version = "0.1.4" @@ -5149,6 +5280,8 @@ dependencies = [ name = "triple-c" version = "0.4.0" dependencies = [ + "aes-gcm", + "argon2", "axum", "base64 0.22.1", "bollard", @@ -5287,6 +5420,16 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "universal-hash" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc1de2c688dc15305988b563c3854064043356019f97a4b46276fe734c4f07ea" +dependencies = [ + "crypto-common", + "subtle", +] + [[package]] name = "untrusted" version = "0.9.0" diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index b97b6cb..88bad73 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -36,6 +36,8 @@ tower-http = { version = "0.6", features = ["cors"] } base64 = "0.22" rand = "0.9" local-ip-address = "0.6" +argon2 = "0.5" +aes-gcm = "0.10" [dev-dependencies] # `test-util` (not part of tokio's `full`) lets the auto-start retry tests run diff --git a/app/src-tauri/src/commands/mod.rs b/app/src-tauri/src/commands/mod.rs index bd2ecfc..cf2fb81 100644 --- a/app/src-tauri/src/commands/mod.rs +++ b/app/src-tauri/src/commands/mod.rs @@ -10,6 +10,7 @@ pub mod install_helper_commands; pub mod migration_commands; pub mod project_commands; pub mod settings_commands; +pub mod settings_export_commands; pub mod stt_commands; pub mod terminal_commands; pub mod update_commands; diff --git a/app/src-tauri/src/commands/settings_export_commands.rs b/app/src-tauri/src/commands/settings_export_commands.rs new file mode 100644 index 0000000..ebebc81 --- /dev/null +++ b/app/src-tauri/src/commands/settings_export_commands.rs @@ -0,0 +1,232 @@ +//! Settings export/import — see triple-c#35. +//! +//! Exports the *host* environment (global `AppSettings` plus the global +//! secrets kept in the OS keychain: the shared Claude Code OAuth login and +//! the model gateway's two keys), encrypted with a user-chosen password — +//! see `storage::settings_crypto` for the actual cryptography. Deliberately +//! out of scope: per-project settings, per-project secrets, and anything +//! living in a project's Docker volumes. +//! +//! **The save/open dialogs are opened from Rust**, the same pattern +//! `file_commands.rs`'s `pick_save_path`/`pick_files_to_upload` already +//! establish and document at length: a frontend-driven dialog handing Rust a +//! host path string is the exact shape of bug that produced this app's past +//! criticals, so the boundary here is drawn the same place. The frontend can +//! ask for a picker; it cannot name a host path as an *input*. `preview_ +//! settings_import` resolves the chosen path itself and remembers it +//! (`AppState::pending_settings_import`) so `apply_settings_import` re-reads +//! the same file without the path ever crossing back over IPC. +//! +//! The password is re-entered (not cached) between preview and apply, so +//! that nothing here holds decrypted plaintext — export/import secrets +//! included — in memory for longer than one command's execution. + +use std::path::PathBuf; + +use tauri::State; +use tauri_plugin_dialog::DialogExt; + +use crate::models::{ + ExportedSecrets, SettingsExportPayload, SettingsImportPreview, SETTINGS_EXPORT_FORMAT_VERSION, +}; +use crate::storage::{secure, settings_crypto}; +use crate::AppState; + +const FILE_EXTENSION: &str = "triplec"; + +fn suggested_export_name() -> String { + // Timestamped so exporting more than once doesn't silently overwrite an + // earlier file just because the save dialog defaults to the same name. + format!( + "triple-c-settings-{}.{}", + chrono::Utc::now().format("%Y%m%d-%H%M%S"), + FILE_EXTENSION + ) +} + +async fn pick_export_save_path(window: &tauri::Window, suggested: &str) -> Option { + let (tx, rx) = tokio::sync::oneshot::channel(); + window + .dialog() + .file() + .set_parent(window) + .set_title("Export Triple-C settings") + .set_file_name(suggested) + .add_filter("Triple-C settings export", &[FILE_EXTENSION]) + .save_file(move |picked| { + let _ = tx.send(picked); + }); + rx.await.ok().flatten().and_then(|p| p.into_path().ok()) +} + +async fn pick_import_open_path(window: &tauri::Window) -> Option { + let (tx, rx) = tokio::sync::oneshot::channel(); + window + .dialog() + .file() + .set_parent(window) + .set_title("Import Triple-C settings") + .add_filter("Triple-C settings export", &[FILE_EXTENSION]) + .pick_file(move |picked| { + let _ = tx.send(picked); + }); + rx.await.ok().flatten().and_then(|p| p.into_path().ok()) +} + +/// Gather the current global secrets. A missing secret reads as `None` — a +/// keychain read failure is treated as "nothing to export" for that one +/// entry rather than aborting the whole export, matching how the rest of +/// this app degrades a keychain error to "absent" (`has_claude_oauth_token`, +/// `has_gateway_api_key`) rather than surfacing it as a hard failure. +fn gather_secrets() -> ExportedSecrets { + ExportedSecrets { + claude_oauth_token: secure::get_claude_oauth_token().unwrap_or_default(), + gateway_api_key: secure::get_gateway_api_key().unwrap_or_default(), + gateway_master_key: secure::get_gateway_master_key().unwrap_or_default(), + } +} + +/// 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 +/// rather than a "failed" toast for a plain cancel. +#[tauri::command] +pub async fn export_settings( + password: String, + window: tauri::Window, + state: State<'_, AppState>, +) -> Result { + if password.is_empty() { + return Err("A password is required to export settings.".to_string()); + } + + let Some(dest) = pick_export_save_path(&window, &suggested_export_name()).await else { + return Ok(false); + }; + + let secrets = gather_secrets(); + if secrets.is_empty() { + log::info!("Exporting settings with no global secrets configured on this machine"); + } + + let payload = SettingsExportPayload { + format_version: SETTINGS_EXPORT_FORMAT_VERSION, + exported_at: chrono::Utc::now().to_rfc3339(), + app_version: env!("CARGO_PKG_VERSION").to_string(), + settings: state.settings_store.get(), + secrets, + }; + + let plaintext = serde_json::to_vec(&payload) + .map_err(|e| format!("Failed to prepare settings for export: {}", e))?; + let encrypted = settings_crypto::encrypt(&plaintext, &password)?; + + std::fs::write(&dest, encrypted).map_err(|e| format!("Failed to write export file: {}", e))?; + + Ok(true) +} + +/// Open a file picker, decrypt the chosen file with `password`, and return a +/// preview (counts and presence flags only — never a secret value) for a +/// confirmation UI. `Ok(None)` means the picker was dismissed. +/// +/// Remembers the resolved path in `AppState::pending_settings_import` for +/// `apply_settings_import` to re-read; does **not** remember the decrypted +/// payload itself, so the password must be supplied again to actually apply +/// it — seeing the preview is not the same as committing to it. +#[tauri::command] +pub async fn preview_settings_import( + password: String, + window: tauri::Window, + state: State<'_, AppState>, +) -> Result, String> { + if password.is_empty() { + return Err("A password is required to open a settings export.".to_string()); + } + + let Some(path) = pick_import_open_path(&window).await else { + return Ok(None); + }; + + let payload = read_and_decrypt(&path, &password)?; + let preview = SettingsImportPreview::from_payload(&payload); + + *state.pending_settings_import.lock().await = Some(path); + + Ok(Some(preview)) +} + +/// Apply the import a prior `preview_settings_import` call resolved a path +/// for. Fails if no preview is pending — this is not a general "decrypt and +/// apply this file" entry point, deliberately: seeing the preview first is +/// required, not just encouraged, since it is the only place a user is told +/// what an import is about to touch before it touches it. +/// +/// Global settings are replaced wholesale — an import is "restore this +/// environment," not a field-by-field merge. Global secrets are handled +/// differently and on purpose: **only secrets actually present in the +/// import are written**; a secret the export doesn't have is left alone on +/// this machine rather than cleared, because an absent secret in the export +/// means "the source machine never had this configured," not "delete this +/// on import." A user who wants to clear a secret already has dedicated UI +/// for that (signing out of shared auth, clearing the gateway key). +#[tauri::command] +pub async fn apply_settings_import( + password: String, + state: State<'_, AppState>, +) -> Result { + if password.is_empty() { + return Err("A password is required to import settings.".to_string()); + } + + let path = state + .pending_settings_import + .lock() + .await + .take() + .ok_or_else(|| "No import is pending — choose a file first.".to_string())?; + + let payload = read_and_decrypt(&path, &password)?; + + let saved = crate::commands::settings_commands::update_settings(payload.settings, state).await?; + + if let Some(token) = non_blank(payload.secrets.claude_oauth_token) { + if let Err(e) = secure::store_claude_oauth_token(&token) { + log::warn!("Settings import: could not restore the shared Claude login: {}", e); + } + } + if let Some(key) = non_blank(payload.secrets.gateway_api_key) { + if let Err(e) = secure::store_gateway_api_key(&key) { + log::warn!("Settings import: could not restore the gateway provider API key: {}", e); + } + } + if let Some(key) = non_blank(payload.secrets.gateway_master_key) { + if let Err(e) = secure::store_gateway_master_key(&key) { + log::warn!("Settings import: could not restore the gateway master key: {}", e); + } + } + + Ok(saved) +} + +fn non_blank(value: Option) -> Option { + value.filter(|v| !v.trim().is_empty()) +} + +fn read_and_decrypt(path: &std::path::Path, password: &str) -> Result { + let encrypted = std::fs::read(path).map_err(|e| format!("Failed to read export file: {}", e))?; + let plaintext = settings_crypto::decrypt(&encrypted, password)?; + + let payload: SettingsExportPayload = serde_json::from_slice(&plaintext) + .map_err(|e| format!("This file doesn't look like a valid settings export: {}", e))?; + + if payload.format_version > SETTINGS_EXPORT_FORMAT_VERSION { + return Err(format!( + "This export was made by a newer version of Triple-C (format {}, this app supports up to {}). \ + Update Triple-C before importing it.", + payload.format_version, SETTINGS_EXPORT_FORMAT_VERSION + )); + } + + Ok(payload) +} diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 2bbe8f1..679c3df 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -29,6 +29,15 @@ pub struct AppState { pub auth_bridge: Arc, pub web_terminal_server: Arc>>, pub lifecycle: Arc, + /// The file `preview_settings_import` last decrypted successfully, held + /// so `apply_settings_import` can re-read and re-decrypt the same file + /// without the frontend ever passing a host path back to Rust as an + /// argument — see the doc comment on `commands::settings_export_commands` + /// for why that direction specifically is the one this app treats as + /// dangerous. Deliberately re-decrypted rather than cached in plaintext: + /// nothing here holds a decrypted secret in memory for longer than one + /// command's execution. + pub pending_settings_import: Arc>>, } // ───────────────────────────────────────────────────────────────────────────── @@ -222,6 +231,7 @@ pub fn run() { auth_bridge, web_terminal_server: Arc::new(tokio::sync::Mutex::new(None)), lifecycle, + pending_settings_import: Arc::new(tokio::sync::Mutex::new(None)), }) .setup(move |app| { match tauri::image::Image::from_bytes(include_bytes!("../icons/icon.png")) { @@ -494,6 +504,10 @@ pub fn run() { commands::settings_commands::inspect_ca_cert_path, commands::settings_commands::list_aws_profiles, commands::settings_commands::detect_host_timezone, + // Settings export/import + commands::settings_export_commands::export_settings, + commands::settings_export_commands::preview_settings_import, + commands::settings_export_commands::apply_settings_import, // Terminal commands::terminal_commands::open_terminal_session, commands::terminal_commands::terminal_input, diff --git a/app/src-tauri/src/models/mod.rs b/app/src-tauri/src/models/mod.rs index e442ae7..a868bff 100644 --- a/app/src-tauri/src/models/mod.rs +++ b/app/src-tauri/src/models/mod.rs @@ -3,6 +3,7 @@ pub mod container_config; pub mod app_settings; pub mod gateway_settings; pub mod migration; +pub mod settings_export; pub mod update_info; pub use project::*; @@ -10,4 +11,5 @@ pub use container_config::*; pub use app_settings::*; pub use gateway_settings::*; pub use migration::*; +pub use settings_export::*; pub use update_info::*; diff --git a/app/src-tauri/src/models/settings_export.rs b/app/src-tauri/src/models/settings_export.rs new file mode 100644 index 0000000..0e71df8 --- /dev/null +++ b/app/src-tauri/src/models/settings_export.rs @@ -0,0 +1,180 @@ +//! Settings export/import — see triple-c#35. +//! +//! `SettingsExportPayload` is the whole plaintext export before encryption +//! and after decryption (see `storage::settings_crypto`). It bundles +//! `AppSettings` (already the non-secret shape persisted to `settings.json`) +//! with the global secrets that live in the OS keychain instead — the shared +//! Claude Code OAuth login and the model gateway's two keys. Per-project +//! settings, per-project secrets, and anything living in a project's Docker +//! volumes are deliberately out of scope: this exports the *host* +//! environment, not any one project's. + +use serde::{Deserialize, Serialize}; + +use super::AppSettings; + +/// Bumped when the shape of [`SettingsExportPayload`] changes in a way that +/// isn't just an additive, `#[serde(default)]`-covered field — e.g. if a +/// field is ever removed or its meaning changes. `apply_settings_import` +/// checks this before touching anything. +pub const SETTINGS_EXPORT_FORMAT_VERSION: u32 = 1; + +/// The global secrets bundled into an export. Deliberately a separate struct +/// from `AppSettings`: these live in the OS keychain, never in +/// `settings.json`, and — outside of this export/import flow — the values +/// themselves never cross into the frontend; see the doc comments on +/// `storage::secure::get_gateway_api_key` and +/// `commands::settings_export_commands` for why that boundary matters here +/// too. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct ExportedSecrets { + #[serde(default)] + pub claude_oauth_token: Option, + #[serde(default)] + pub gateway_api_key: Option, + #[serde(default)] + pub gateway_master_key: Option, +} + +impl ExportedSecrets { + pub fn is_empty(&self) -> bool { + self.claude_oauth_token.is_none() + && self.gateway_api_key.is_none() + && self.gateway_master_key.is_none() + } +} + +/// The full plaintext payload — this is what gets encrypted on export and +/// what decryption recovers on import. Never written to disk unencrypted; +/// see `storage::settings_crypto`. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SettingsExportPayload { + pub format_version: u32, + /// RFC3339. Purely informational — shown in the import preview so a user + /// picking between a few old export files has something to go on. + pub exported_at: String, + /// The exporting app's `CARGO_PKG_VERSION`. Also informational: every + /// field below already round-trips through `#[serde(default)]`-covered + /// `AppSettings`, so an older or newer export still deserializes; this is + /// for a human to notice "this is from a much older version" if an import + /// ever looks wrong, not something the code branches on. + pub app_version: String, + pub settings: AppSettings, + #[serde(default)] + pub secrets: ExportedSecrets, +} + +/// What `preview_settings_import` hands the frontend before anything is +/// applied — counts and presence flags only, **never** a secret value itself, +/// so this type is safe to return across the IPC boundary and render +/// directly. The confirmation UI is built from this. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SettingsImportPreview { + pub exported_at: String, + pub app_version: String, + pub custom_env_var_count: usize, + pub gateway_model_count: usize, + pub has_claude_code_settings: bool, + pub has_claude_oauth_token: bool, + pub has_gateway_api_key: bool, + pub has_gateway_master_key: bool, +} + +impl SettingsImportPreview { + pub fn from_payload(payload: &SettingsExportPayload) -> Self { + Self { + exported_at: payload.exported_at.clone(), + app_version: payload.app_version.clone(), + custom_env_var_count: payload.settings.global_custom_env_vars.len(), + gateway_model_count: payload.settings.gateway.models.len(), + has_claude_code_settings: payload.settings.global_claude_code_settings.is_some(), + has_claude_oauth_token: payload + .secrets + .claude_oauth_token + .as_deref() + .is_some_and(|t| !t.trim().is_empty()), + has_gateway_api_key: payload + .secrets + .gateway_api_key + .as_deref() + .is_some_and(|k| !k.trim().is_empty()), + has_gateway_master_key: payload + .secrets + .gateway_master_key + .as_deref() + .is_some_and(|k| !k.trim().is_empty()), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::AppSettings; + + fn payload_with(secrets: ExportedSecrets) -> SettingsExportPayload { + let mut settings = AppSettings::default(); + settings.global_custom_env_vars = vec![ + crate::models::EnvVar { key: "A".to_string(), value: "1".to_string() }, + crate::models::EnvVar { key: "B".to_string(), value: "2".to_string() }, + ]; + SettingsExportPayload { + format_version: SETTINGS_EXPORT_FORMAT_VERSION, + exported_at: "2026-08-27T00:00:00Z".to_string(), + app_version: "0.4.14".to_string(), + settings, + secrets, + } + } + + #[test] + fn the_preview_never_carries_a_secret_value() { + let payload = payload_with(ExportedSecrets { + claude_oauth_token: Some("sk-super-secret-token".to_string()), + gateway_api_key: Some("sk-another-secret".to_string()), + gateway_master_key: Some("sk-triple-c-yet-another".to_string()), + }); + let preview = SettingsImportPreview::from_payload(&payload); + let serialized = serde_json::to_string(&preview).unwrap(); + + assert!(!serialized.contains("sk-super-secret-token")); + assert!(!serialized.contains("sk-another-secret")); + assert!(!serialized.contains("sk-triple-c-yet-another")); + assert!(preview.has_claude_oauth_token); + assert!(preview.has_gateway_api_key); + assert!(preview.has_gateway_master_key); + } + + #[test] + fn a_blank_secret_reads_as_absent_in_the_preview() { + // A keychain entry that exists but holds only whitespace must not + // read as "present" — same "blank counts as absent" rule the + // keychain layer itself applies when storing these. + let payload = payload_with(ExportedSecrets { + claude_oauth_token: Some(" ".to_string()), + gateway_api_key: None, + gateway_master_key: None, + }); + let preview = SettingsImportPreview::from_payload(&payload); + assert!(!preview.has_claude_oauth_token); + assert!(!preview.has_gateway_api_key); + assert!(!preview.has_gateway_master_key); + } + + #[test] + fn counts_reflect_the_real_settings() { + let payload = payload_with(ExportedSecrets::default()); + let preview = SettingsImportPreview::from_payload(&payload); + assert_eq!(preview.custom_env_var_count, 2); + } + + #[test] + fn an_empty_secrets_bundle_reports_itself_as_empty() { + assert!(ExportedSecrets::default().is_empty()); + assert!(!ExportedSecrets { + claude_oauth_token: Some("x".to_string()), + ..Default::default() + } + .is_empty()); + } +} diff --git a/app/src-tauri/src/storage/mod.rs b/app/src-tauri/src/storage/mod.rs index 151759b..fda731f 100644 --- a/app/src-tauri/src/storage/mod.rs +++ b/app/src-tauri/src/storage/mod.rs @@ -2,6 +2,7 @@ pub mod migration_store; pub mod pending_cleanup; pub mod projects_store; pub mod secure; +pub mod settings_crypto; pub mod settings_store; #[allow(unused_imports)] diff --git a/app/src-tauri/src/storage/secure.rs b/app/src-tauri/src/storage/secure.rs index a1c7e86..081dbf6 100644 --- a/app/src-tauri/src/storage/secure.rs +++ b/app/src-tauri/src/storage/secure.rs @@ -321,28 +321,52 @@ pub fn delete_gateway_api_key() -> Result<(), String> { /// only enforces auth when a master key is configured, so Triple-C always /// configures one. pub fn get_or_create_gateway_master_key() -> Result { - if let Some(existing) = read_entry(GATEWAY_MASTER_KEY_SERVICE, "the gateway master key")? { - if !existing.trim().is_empty() { - return Ok(existing); - } + if let Some(existing) = get_gateway_master_key()? { + return Ok(existing); } regenerate_gateway_master_key() } +/// Read the gateway master key without minting one if none exists yet. +/// Distinct from [`get_or_create_gateway_master_key`], which mints as a side +/// effect the read half of that function must not have — settings export +/// (triple-c#35) needs "is there one, and if so what is it", not "make sure +/// one exists". +pub fn get_gateway_master_key() -> Result, String> { + Ok(read_entry(GATEWAY_MASTER_KEY_SERVICE, "the gateway master key")? + .filter(|k| !k.trim().is_empty())) +} + /// Mint a new gateway master key, invalidating the old one. Projects using the /// previous value must be updated. pub fn regenerate_gateway_master_key() -> Result { // LiteLLM requires the master key to start with `sk-`. let key = format!("sk-triple-c-{}", uuid::Uuid::new_v4().simple()); + store_gateway_master_key(&key)?; + Ok(key) +} + +/// Store an exact given gateway master key, replacing any previous one. +/// +/// Distinct from [`regenerate_gateway_master_key`], which always mints a +/// fresh random value: this exists for settings import (triple-c#35), where +/// restoring the *same* key an export captured is the point — projects on +/// the destination machine may not exist yet, but a project migrated or +/// re-added later that still has the old key pasted into its config must +/// keep working against it. Blank input is rejected rather than silently +/// stored, matching every other `store_*` function in this module. +pub fn store_gateway_master_key(key: &str) -> Result<(), String> { + if key.trim().is_empty() { + return Err("Refusing to store an empty gateway master key.".to_string()); + } let entry = keyring::Entry::new(GATEWAY_MASTER_KEY_SERVICE, KEYCHAIN_ACCOUNT) .map_err(|e| format!("Keyring error: {}", e))?; entry - .set_password(&key) + .set_password(key.trim()) .map_err(|e| format!("Failed to store the gateway master key: {}", e))?; - bump_gateway_secret_version()?; - Ok(key) + bump_gateway_secret_version() } diff --git a/app/src-tauri/src/storage/settings_crypto.rs b/app/src-tauri/src/storage/settings_crypto.rs new file mode 100644 index 0000000..499e7d4 --- /dev/null +++ b/app/src-tauri/src/storage/settings_crypto.rs @@ -0,0 +1,158 @@ +//! Password-based encryption for the settings export/import file — see +//! triple-c#35. +//! +//! The exported payload can carry live credentials (the shared Claude OAuth +//! token, the gateway provider/master keys — see +//! `commands::settings_export_commands`), so this is not encryption for its +//! own sake; a wrong or missing key here is a real credential leak, not a +//! cosmetic bug. Argon2id derives a 256-bit key from the password (memory- +//! hard, meaningfully resistant to GPU/ASIC brute-forcing in a way PBKDF2 at +//! any reasonable iteration count is not), and AES-256-GCM is what actually +//! encrypts — authenticated, so a wrong password is detected by a failed tag +//! check rather than producing silent garbage. +//! +//! File format: `MAGIC (4 bytes) | salt (16 bytes) | nonce (12 bytes) | +//! ciphertext+tag`. The salt and nonce are not secret — they are written in +//! the clear right here, on purpose. The salt's only job is to make two +//! exports with the same password derive different keys (defeats a +//! precomputed-table attack against the password alone); the nonce's job is +//! GCM's requirement that a (key, nonce) pair never repeat. Both hold +//! because a fresh random value is drawn for each, on every call to +//! [`encrypt`]. + +use aes_gcm::aead::{Aead, KeyInit}; +use aes_gcm::{Aes256Gcm, Nonce}; +use argon2::{Algorithm, Argon2, Params, Version}; +use rand::RngCore; + +/// Identifies the file as a Triple-C settings export and pins the format — +/// a change to the salt/nonce lengths or the KDF/cipher choice below needs a +/// new magic value, not a silent reinterpretation of old bytes. +const MAGIC: &[u8; 4] = b"TCX1"; +const SALT_LEN: usize = 16; +const NONCE_LEN: usize = 12; +const KEY_LEN: usize = 32; +const HEADER_LEN: usize = MAGIC.len() + SALT_LEN + NONCE_LEN; + +/// Argon2id parameters: memory cost in KiB, time cost (iterations), +/// parallelism. `(19 MiB, 2, 1)` is OWASP's documented minimum recommendation +/// for Argon2id — deliberately heavier than a login-flow KDF would use, since +/// this runs once per export/import rather than on every request, so trading +/// roughly a second of wall time for real brute-force resistance costs +/// nothing a user would notice. +fn argon2_params() -> Params { + Params::new(19 * 1024, 2, 1, Some(KEY_LEN)).expect("hardcoded Argon2 params are valid") +} + +fn derive_key(password: &str, salt: &[u8]) -> Result<[u8; KEY_LEN], String> { + let argon2 = Argon2::new(Algorithm::Argon2id, Version::V0x13, argon2_params()); + let mut key = [0u8; KEY_LEN]; + argon2 + .hash_password_into(password.as_bytes(), salt, &mut key) + .map_err(|e| format!("Failed to derive encryption key: {}", e))?; + Ok(key) +} + +/// Encrypt `plaintext` with a key derived from `password`. Returns the whole +/// file's bytes (header + ciphertext) — see the module doc for the layout. +pub fn encrypt(plaintext: &[u8], password: &str) -> Result, String> { + let mut salt = [0u8; SALT_LEN]; + rand::rng().fill_bytes(&mut salt); + let key = derive_key(password, &salt)?; + + let mut nonce_bytes = [0u8; NONCE_LEN]; + rand::rng().fill_bytes(&mut nonce_bytes); + let nonce = Nonce::from_slice(&nonce_bytes); + + let cipher = Aes256Gcm::new_from_slice(&key) + .map_err(|e| format!("Failed to initialize cipher: {}", e))?; + let ciphertext = cipher + .encrypt(nonce, plaintext) + .map_err(|e| format!("Encryption failed: {}", e))?; + + let mut out = Vec::with_capacity(HEADER_LEN + ciphertext.len()); + out.extend_from_slice(MAGIC); + out.extend_from_slice(&salt); + out.extend_from_slice(&nonce_bytes); + out.extend_from_slice(&ciphertext); + Ok(out) +} + +/// Decrypt a file produced by [`encrypt`]. The one error this returns for a +/// wrong password is deliberately generic ("wrong password, or the file is +/// corrupted") rather than distinguishing the two: GCM's authentication tag +/// fails to verify for the wrong key on essentially any ciphertext, so there +/// is no reliable way to tell "wrong password" from "corrupted file" apart, +/// and guessing would be worse than saying so. +pub fn decrypt(data: &[u8], password: &str) -> Result, String> { + if data.len() < HEADER_LEN { + return Err("This does not look like a Triple-C settings export (file too short).".to_string()); + } + if &data[..MAGIC.len()] != MAGIC { + return Err("This does not look like a Triple-C settings export (unrecognized file).".to_string()); + } + let salt = &data[MAGIC.len()..MAGIC.len() + SALT_LEN]; + let nonce_bytes = &data[MAGIC.len() + SALT_LEN..HEADER_LEN]; + let ciphertext = &data[HEADER_LEN..]; + + let key = derive_key(password, salt)?; + let cipher = Aes256Gcm::new_from_slice(&key) + .map_err(|e| format!("Failed to initialize cipher: {}", e))?; + let nonce = Nonce::from_slice(nonce_bytes); + cipher + .decrypt(nonce, ciphertext) + .map_err(|_| "Wrong password, or the file is corrupted.".to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_round_trip_with_the_right_password_recovers_the_plaintext() { + let plaintext = b"{\"settings\": \"whatever\"}"; + let encrypted = encrypt(plaintext, "correct horse battery staple").unwrap(); + let decrypted = decrypt(&encrypted, "correct horse battery staple").unwrap(); + assert_eq!(decrypted, plaintext); + } + + #[test] + fn the_wrong_password_fails_rather_than_returning_garbage() { + let encrypted = encrypt(b"secret payload", "correct password").unwrap(); + let result = decrypt(&encrypted, "wrong password"); + assert!(result.is_err(), "decrypting with the wrong password must fail, not silently succeed"); + } + + #[test] + fn two_exports_of_the_same_plaintext_and_password_produce_different_files() { + // If this ever failed it would mean the salt or nonce stopped being + // randomized — either one repeating is a real security regression + // (a fixed salt lets an attacker precompute against the password + // alone; a repeated (key, nonce) pair breaks GCM's guarantees + // outright), not just a cosmetic one. + let a = encrypt(b"same plaintext", "same password").unwrap(); + let b = encrypt(b"same plaintext", "same password").unwrap(); + assert_ne!(a, b, "two independent exports must not be byte-identical"); + } + + #[test] + fn corrupting_a_single_byte_of_ciphertext_is_detected() { + let mut encrypted = encrypt(b"tamper-evident payload", "a password").unwrap(); + let last = encrypted.len() - 1; + encrypted[last] ^= 0xFF; + assert!(decrypt(&encrypted, "a password").is_err()); + } + + #[test] + fn a_file_that_is_too_short_is_rejected_cleanly_not_by_panicking() { + assert!(decrypt(b"short", "any password").is_err()); + assert!(decrypt(b"", "any password").is_err()); + } + + #[test] + fn a_file_with_the_wrong_magic_is_rejected() { + let mut encrypted = encrypt(b"payload", "password").unwrap(); + encrypted[0] = b'X'; + assert!(decrypt(&encrypted, "password").is_err()); + } +} diff --git a/app/src/components/settings/ExportSettingsModal.test.tsx b/app/src/components/settings/ExportSettingsModal.test.tsx new file mode 100644 index 0000000..c8e85f7 --- /dev/null +++ b/app/src/components/settings/ExportSettingsModal.test.tsx @@ -0,0 +1,72 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import ExportSettingsModal from "./ExportSettingsModal"; + +const exportSettings = vi.fn(); + +vi.mock("../../lib/tauri-commands", () => ({ + exportSettings: (password: string) => exportSettings(password), +})); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +function fillPasswords(password: string, confirm: string) { + fireEvent.change(screen.getByLabelText("Password"), { target: { value: password } }); + fireEvent.change(screen.getByLabelText("Confirm password"), { target: { value: confirm } }); +} + +describe("ExportSettingsModal", () => { + it("keeps the submit button disabled until the passwords are long enough and match", () => { + render(); + const submit = screen.getByRole("button", { name: /choose where to save/i }); + expect(submit).toBeDisabled(); + + fillPasswords("short", "short"); + expect(submit).toBeDisabled(); + expect(screen.getByText(/use at least 8 characters/i)).toBeInTheDocument(); + + fillPasswords("longenoughpassword", "different"); + expect(submit).toBeDisabled(); + expect(screen.getByText(/don't match/i)).toBeInTheDocument(); + + fillPasswords("longenoughpassword", "longenoughpassword"); + expect(submit).not.toBeDisabled(); + }); + + it("exports with the entered password and shows success", async () => { + exportSettings.mockResolvedValue(true); + render(); + + fillPasswords("longenoughpassword", "longenoughpassword"); + fireEvent.click(screen.getByRole("button", { name: /choose where to save/i })); + + await waitFor(() => expect(exportSettings).toHaveBeenCalledWith("longenoughpassword")); + await waitFor(() => expect(screen.getByText(/settings exported/i)).toBeInTheDocument()); + }); + + it("closes quietly when the save dialog is dismissed", async () => { + exportSettings.mockResolvedValue(false); + const onClose = vi.fn(); + render(); + + fillPasswords("longenoughpassword", "longenoughpassword"); + fireEvent.click(screen.getByRole("button", { name: /choose where to save/i })); + + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(screen.queryByText(/settings exported/i)).not.toBeInTheDocument(); + }); + + it("shows an error rather than closing when the export fails", async () => { + exportSettings.mockRejectedValue("Disk is full"); + const onClose = vi.fn(); + render(); + + fillPasswords("longenoughpassword", "longenoughpassword"); + fireEvent.click(screen.getByRole("button", { name: /choose where to save/i })); + + await waitFor(() => expect(screen.getByText("Disk is full")).toBeInTheDocument()); + expect(onClose).not.toHaveBeenCalled(); + }); +}); diff --git a/app/src/components/settings/ExportSettingsModal.tsx b/app/src/components/settings/ExportSettingsModal.tsx new file mode 100644 index 0000000..cd1d70a --- /dev/null +++ b/app/src/components/settings/ExportSettingsModal.tsx @@ -0,0 +1,119 @@ +import { useState } from "react"; +import Modal from "../ui/Modal"; +import Button from "../ui/Button"; +import Field, { inputClass } from "../ui/Field"; +import { exportSettings } from "../../lib/tauri-commands"; + +interface Props { + onClose: () => void; +} + +const MIN_PASSWORD_LENGTH = 8; + +/** + * Password entry for exporting global settings. The save dialog itself opens + * from Rust once a password is confirmed here — see the doc comment on + * `commands::settings_export_commands` for why the host path never + * round-trips through this component. + */ +export default function ExportSettingsModal({ onClose }: Props) { + const [password, setPassword] = useState(""); + const [confirmPassword, setConfirmPassword] = useState(""); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const [done, setDone] = useState(false); + + const mismatch = confirmPassword.length > 0 && password !== confirmPassword; + const tooShort = password.length > 0 && password.length < MIN_PASSWORD_LENGTH; + const canSubmit = password.length >= MIN_PASSWORD_LENGTH && password === confirmPassword; + + const handleExport = async () => { + setError(null); + setBusy(true); + try { + const saved = await exportSettings(password); + if (saved) setDone(true); + // `false` means the save dialog was dismissed — close quietly, same as + // if the user had cancelled the modal itself. + else onClose(); + } catch (e) { + setError(String(e)); + } finally { + setBusy(false); + } + }; + + return ( + + Done + + ) : ( + <> + + + + ) + } + > + {done ? ( +

+ Settings exported. Keep the password somewhere safe — there is no way to recover + the file without it. +

+ ) : ( +
+ + {(id) => ( + setPassword(e.target.value)} + disabled={busy} + className={inputClass} + /> + )} + + + {(id) => ( + setConfirmPassword(e.target.value)} + disabled={busy} + className={inputClass} + /> + )} + + {tooShort && ( +

+ Use at least {MIN_PASSWORD_LENGTH} characters. +

+ )} + {mismatch &&

Passwords don't match.

} + {error &&

{error}

} +
+ )} + + ); +} diff --git a/app/src/components/settings/ImportSettingsModal.test.tsx b/app/src/components/settings/ImportSettingsModal.test.tsx new file mode 100644 index 0000000..345d916 --- /dev/null +++ b/app/src/components/settings/ImportSettingsModal.test.tsx @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import ImportSettingsModal from "./ImportSettingsModal"; +import type { AppSettings, SettingsImportPreview } from "../../lib/types"; + +const previewSettingsImport = vi.fn(); +const applySettingsImport = vi.fn(); + +vi.mock("../../lib/tauri-commands", () => ({ + previewSettingsImport: (password: string) => previewSettingsImport(password), + applySettingsImport: (password: string) => applySettingsImport(password), +})); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +const samplePreview: SettingsImportPreview = { + exported_at: "2026-08-27T00:00:00Z", + app_version: "0.4.14", + custom_env_var_count: 2, + gateway_model_count: 0, + has_claude_code_settings: false, + has_claude_oauth_token: true, + has_gateway_api_key: false, + has_gateway_master_key: false, +}; + +describe("ImportSettingsModal", () => { + it("keeps 'Choose file' disabled until a password is entered", () => { + render(); + expect(screen.getByRole("button", { name: /choose file/i })).toBeDisabled(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "hunter2" } }); + expect(screen.getByRole("button", { name: /choose file/i })).not.toBeDisabled(); + }); + + it("shows the preview and confirms with the same password used to open it", async () => { + previewSettingsImport.mockResolvedValue(samplePreview); + applySettingsImport.mockResolvedValue({} as AppSettings); + const onImported = vi.fn(); + render(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "hunter2" } }); + fireEvent.click(screen.getByRole("button", { name: /choose file/i })); + + await waitFor(() => expect(previewSettingsImport).toHaveBeenCalledWith("hunter2")); + expect(await screen.findByText(/2 global custom env vars/i)).toBeInTheDocument(); + expect(screen.getByText(/your shared claude login/i)).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: /^import$/i })); + await waitFor(() => expect(applySettingsImport).toHaveBeenCalledWith("hunter2")); + await waitFor(() => expect(onImported).toHaveBeenCalledWith({})); + expect(await screen.findByText(/settings imported/i)).toBeInTheDocument(); + }); + + it("closes quietly when the file picker is dismissed", async () => { + previewSettingsImport.mockResolvedValue(null); + const onClose = vi.fn(); + render(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "hunter2" } }); + fireEvent.click(screen.getByRole("button", { name: /choose file/i })); + + await waitFor(() => expect(onClose).toHaveBeenCalled()); + }); + + it("shows an error when the password is wrong rather than a blank preview", async () => { + previewSettingsImport.mockRejectedValue("Wrong password, or the file is corrupted."); + render(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "wrong" } }); + fireEvent.click(screen.getByRole("button", { name: /choose file/i })); + + expect(await screen.findByText(/wrong password, or the file is corrupted/i)).toBeInTheDocument(); + }); + + it("shows an error if applying the import fails, without claiming success", async () => { + previewSettingsImport.mockResolvedValue(samplePreview); + applySettingsImport.mockRejectedValue("Keychain write failed"); + render(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "hunter2" } }); + fireEvent.click(screen.getByRole("button", { name: /choose file/i })); + await screen.findByText(/2 global custom env vars/i); + + fireEvent.click(screen.getByRole("button", { name: /^import$/i })); + expect(await screen.findByText("Keychain write failed")).toBeInTheDocument(); + expect(screen.queryByText(/settings imported/i)).not.toBeInTheDocument(); + }); +}); diff --git a/app/src/components/settings/ImportSettingsModal.tsx b/app/src/components/settings/ImportSettingsModal.tsx new file mode 100644 index 0000000..c8abb8d --- /dev/null +++ b/app/src/components/settings/ImportSettingsModal.tsx @@ -0,0 +1,139 @@ +import { useState } from "react"; +import Modal from "../ui/Modal"; +import Button from "../ui/Button"; +import Field, { inputClass } from "../ui/Field"; +import { applySettingsImport, previewSettingsImport } from "../../lib/tauri-commands"; +import { describeImport } from "../../lib/settingsImportPreview"; +import type { AppSettings, SettingsImportPreview } from "../../lib/types"; + +interface Props { + onClose: () => void; + /** Fired once the import is actually applied, so the caller can refresh + * whatever reads settings from the store. */ + onImported: (settings: AppSettings) => void; +} + +/** + * Two phases: enter the password and pick the file (backend resolves the + * file dialog itself — see `commands::settings_export_commands`), then + * confirm a preview before anything is actually applied. The same password + * is reused for the second call rather than asking again; nothing about + * that call needs a fresh secret; the backend just doesn't cache the + * *decrypted payload* between the two. + */ +export default function ImportSettingsModal({ onClose, onImported }: Props) { + const [password, setPassword] = useState(""); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const [preview, setPreview] = useState(null); + const [applied, setApplied] = useState(false); + + const handleChooseFile = async () => { + setError(null); + setBusy(true); + try { + const result = await previewSettingsImport(password); + if (result) setPreview(result); + else onClose(); // File picker dismissed. + } catch (e) { + setError(String(e)); + } finally { + setBusy(false); + } + }; + + const handleConfirm = async () => { + setError(null); + setBusy(true); + try { + const settings = await applySettingsImport(password); + setApplied(true); + onImported(settings); + } catch (e) { + setError(String(e)); + } finally { + setBusy(false); + } + }; + + return ( + + Done + + ) : preview ? ( + <> + + + + ) : ( + <> + + + + ) + } + > + {applied ? ( +

Settings imported.

+ ) : preview ? ( +
+

+ Exported {new Date(preview.exported_at).toLocaleString()} from Triple-C{" "} + {preview.app_version}. +

+
+

This will replace:

+
    + {describeImport(preview).map((item) => ( +
  • {item}
    • + ))} +
+
+ {error &&

{error}

} +
+ ) : ( +
+ + {(id) => ( + setPassword(e.target.value)} + disabled={busy} + className={inputClass} + /> + )} + + {error &&

{error}

} +
+ )} + + ); +} diff --git a/app/src/components/settings/SettingsPanel.tsx b/app/src/components/settings/SettingsPanel.tsx index 062a9f4..ea11494 100644 --- a/app/src/components/settings/SettingsPanel.tsx +++ b/app/src/components/settings/SettingsPanel.tsx @@ -19,9 +19,11 @@ import WebTerminalSettings from "./WebTerminalSettings"; import SttSettings from "./SttSettings"; import SharedAuthSettings from "./SharedAuthSettings"; import CertificateSettings from "./CertificateSettings"; +import ExportSettingsModal from "./ExportSettingsModal"; +import ImportSettingsModal from "./ImportSettingsModal"; export default function SettingsPanel() { - const { appSettings, saveSettings } = useSettings(); + const { appSettings, saveSettings, setAppSettings } = useSettings(); const { appVersion, imageUpdateInfo, checkForUpdates, checkImageUpdate } = useUpdates(); const [globalInstructions, setGlobalInstructions] = useState(appSettings?.global_claude_instructions ?? ""); const [globalEnvVars, setGlobalEnvVars] = useState(appSettings?.global_custom_env_vars ?? []); @@ -33,6 +35,8 @@ export default function SettingsPanel() { const [showInstructionsModal, setShowInstructionsModal] = useState(false); const [showEnvVarsModal, setShowEnvVarsModal] = useState(false); const [showClaudeCodeSettingsModal, setShowClaudeCodeSettingsModal] = useState(false); + const [showExportModal, setShowExportModal] = useState(false); + const [showImportModal, setShowImportModal] = useState(false); // Sync local state when appSettings change useEffect(() => { @@ -269,6 +273,39 @@ export default function SettingsPanel() { + +
+

+ Export your global settings and stored credentials (a shared Claude login, + gateway keys) to one password-encrypted file, or restore them on a new machine. + Project-specific settings and container data are never included. +

+
+ + +
+
+ + + {showExportModal && setShowExportModal(false)} />} + + {showImportModal && ( + setShowImportModal(false)} + onImported={(settings) => setAppSettings(settings)} + /> + )} + {showInstructionsModal && ( = {}): SettingsImportPreview { + return { + exported_at: "2026-08-27T00:00:00Z", + app_version: "0.4.14", + custom_env_var_count: 0, + gateway_model_count: 0, + has_claude_code_settings: false, + has_claude_oauth_token: false, + has_gateway_api_key: false, + has_gateway_master_key: false, + ...overrides, + }; +} + +describe("describeImport", () => { + it("always names the settings replacement, even with nothing else set", () => { + expect(describeImport(preview())).toEqual([ + "Your global settings (all of them — this replaces what's here now)", + ]); + }); + + it("singularizes a count of exactly one", () => { + const items = describeImport(preview({ custom_env_var_count: 1, gateway_model_count: 1 })); + expect(items).toContain("1 global custom env var"); + expect(items).toContain("1 gateway model"); + }); + + it("pluralizes counts greater than one", () => { + const items = describeImport(preview({ custom_env_var_count: 3, gateway_model_count: 2 })); + expect(items).toContain("3 global custom env vars"); + expect(items).toContain("2 gateway models"); + }); + + it("names every present secret and setting without naming absent ones", () => { + const items = describeImport( + preview({ + has_claude_code_settings: true, + has_claude_oauth_token: true, + has_gateway_api_key: true, + has_gateway_master_key: true, + }), + ); + expect(items).toContain("Global Claude Code settings"); + expect(items).toContain("Your shared Claude login"); + expect(items).toContain("The gateway provider API key"); + expect(items).toContain("The gateway master key"); + // None of the count-based items, since both counts are 0. + expect(items.some((i) => i.includes("env var"))).toBe(false); + expect(items.some((i) => i.includes("gateway model"))).toBe(false); + }); +}); diff --git a/app/src/lib/settingsImportPreview.ts b/app/src/lib/settingsImportPreview.ts new file mode 100644 index 0000000..dde8d46 --- /dev/null +++ b/app/src/lib/settingsImportPreview.ts @@ -0,0 +1,20 @@ +import type { SettingsImportPreview } from "./types"; + +/** Named things a `SettingsImportPreview` says an import will change, for + * `ImportSettingsModal`'s confirmation list. */ +export function describeImport(preview: SettingsImportPreview): string[] { + const items: string[] = ["Your global settings (all of them — this replaces what's here now)"]; + if (preview.custom_env_var_count > 0) { + items.push( + `${preview.custom_env_var_count} global custom env var${preview.custom_env_var_count === 1 ? "" : "s"}`, + ); + } + if (preview.has_claude_code_settings) items.push("Global Claude Code settings"); + if (preview.gateway_model_count > 0) { + items.push(`${preview.gateway_model_count} gateway model${preview.gateway_model_count === 1 ? "" : "s"}`); + } + if (preview.has_claude_oauth_token) items.push("Your shared Claude login"); + if (preview.has_gateway_api_key) items.push("The gateway provider API key"); + if (preview.has_gateway_master_key) items.push("The gateway master key"); + return items; +} diff --git a/app/src/lib/tauri-commands.ts b/app/src/lib/tauri-commands.ts index 6389a4c..dbceeec 100644 --- a/app/src/lib/tauri-commands.ts +++ b/app/src/lib/tauri-commands.ts @@ -1,5 +1,5 @@ import { invoke } from "@tauri-apps/api/core"; -import type { Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ContainerInfo, AppSettings, UpdateInfo, ImageUpdateInfo, FileEntry, FileContents, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, BrowserViewPopoutState, BrowserPageState, PlaywrightDetection, BrowserSetupOutcome, BrowserInstallTarget, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo, UploadOutcome } from "./types"; +import type { Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ContainerInfo, AppSettings, SettingsImportPreview, UpdateInfo, ImageUpdateInfo, FileEntry, FileContents, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, BrowserViewPopoutState, BrowserPageState, PlaywrightDetection, BrowserSetupOutcome, BrowserInstallTarget, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo, UploadOutcome } from "./types"; // Docker export const checkDocker = () => invoke("check_docker"); @@ -42,6 +42,15 @@ export const inspectCaCertPath = (path: string) => export const detectHostTimezone = () => invoke("detect_host_timezone"); +// Settings export/import — `false`/`null` mean the save/open dialog was +// dismissed, not an error. +export const exportSettings = (password: string) => + invoke("export_settings", { password }); +export const previewSettingsImport = (password: string) => + invoke("preview_settings_import", { password }); +export const applySettingsImport = (password: string) => + invoke("apply_settings_import", { password }); + // AWS export const awsSsoRefresh = (projectId: string) => invoke("aws_sso_refresh", { projectId }); diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index 9a6462c..baadc0b 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -292,6 +292,20 @@ export interface AppSettings { global_claude_code_settings: ClaudeCodeSettings | null; } +/** What `preview_settings_import` returns before anything is applied — + * counts and presence flags only, never a secret value itself. Built from + * this, not from the raw import file, which the frontend never sees. */ +export interface SettingsImportPreview { + exported_at: string; + app_version: string; + custom_env_var_count: number; + gateway_model_count: number; + has_claude_code_settings: boolean; + has_claude_oauth_token: boolean; + has_gateway_api_key: boolean; + has_gateway_master_key: boolean; +} + /** What `inspect_ca_cert_path` reports about a corporate CA path. Errors ride * in the payload rather than rejecting, so the field can render them inline * while the user is still typing. */ From 925e51e43506fba8e5dd2fb6157e48e640dd139e Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 27 Aug 2026 12:16:43 -0700 Subject: [PATCH 2/4] Fix a real credential-leak vector a review found, plus four smaller issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The headline finding: WebTerminalSettings::access_token is a live bearer credential for a server that binds every interface, stored as a plain field on AppSettings — which this feature was exporting and importing wholesale as if it were as inert as a port number. A crafted export file could set web_terminal.enabled and access_token together, and importing it (with no more warning than any other setting change) would silently stand up a LAN-listening terminal server with an attacker-known token on the victim's next launch. Fixed by carving the token out into ExportedSecrets, same as the other three global secrets, with the same "only overwrite what the import actually has" treatment — except that has to be done by hand here, since this one lives inside the AppSettings blob that gets replaced wholesale rather than in the keychain. Added SettingsImportPreview:: enables_web_terminal so "this turns on a listening service" gets its own visible warning in the confirmation modal rather than hiding inside a generic "settings replaced" bullet list. Also fixed: - read_and_decrypt checked format_version only after attempting to parse the full payload, so a future version bump that isn't deserialize-compatible would fail on the shape mismatch before the version check ever ran — and serde's type-mismatch errors quote the offending value inline, which is a real leak path since the plaintext here can hold a live credential. Now probes just the version field first, and neither error path interpolates the underlying serde message into what the user sees. - apply_settings_import cleared the pending-import path before it could fail, so a rejected import (an invalid host path, anything update_settings validates) dead-ended the modal with no way back except cancelling and reopening the file picker. The path is now only cleared on success. - Secrets are restored before the settings replace runs, not after — replacing settings is what triggers reconcile_gateway, and restoring secrets afterward left a real window where a gateway recreation happened against the destination's stale keys. - The 8-character password minimum was frontend-only; export_settings now enforces it too, since that's the actual boundary a weak password has to cross. The derived key and decrypted plaintext are wrapped in zeroize::Zeroizing (already in the tree via aes-gcm). Added test coverage the review named as missing: format-version ordering, the generic-error-message guarantee, non_blank's blank-vs- absent handling, and the new web-terminal preview/warning behavior on both sides of the IPC boundary. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ --- CLAUDE.md | 41 ++- app/src-tauri/Cargo.lock | 1 + app/src-tauri/Cargo.toml | 1 + .../src/commands/settings_export_commands.rs | 236 ++++++++++++++++-- app/src-tauri/src/models/settings_export.rs | 84 +++++-- app/src-tauri/src/storage/settings_crypto.rs | 26 +- .../settings/ImportSettingsModal.test.tsx | 12 + .../settings/ImportSettingsModal.tsx | 10 +- app/src/lib/settingsImportPreview.test.ts | 32 ++- app/src/lib/settingsImportPreview.ts | 21 +- app/src/lib/types.ts | 6 + 11 files changed, 407 insertions(+), 63 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 39d3024..7267b16 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -555,12 +555,26 @@ nobody had reason to open. Fixtures are never live values; there is no case wher ## Settings export/import `commands::settings_export_commands`, `storage::settings_crypto`, `models::settings_export` -(triple-c#35). Exports the *host* environment — global `AppSettings` (already the non-secret -shape persisted to `settings.json`) plus the global secrets that live in the OS keychain instead: -the shared Claude Code OAuth login and the model gateway's two keys. Per-project settings, -per-project secrets, and anything in a project's Docker volumes are deliberately out of scope — -this is not a project backup. +(triple-c#35). Exports the *host* environment — global `AppSettings` plus the global secrets that +live in the OS keychain instead: the shared Claude Code OAuth login and the model gateway's two +keys. Per-project settings, per-project secrets, and anything in a project's Docker volumes are +deliberately out of scope — this is not a project backup. +- **`AppSettings` is not entirely the non-secret shape it looks like, and a review of this feature + caught the one place that isn't.** `WebTerminalSettings::access_token` is a live bearer + credential for a server that binds every interface — exporting `AppSettings` wholesale would + have carried it along as if it were as inert as a port number, and importing it would have + applied `web_terminal.enabled` and the token together with no more warning than any other + setting, letting a crafted export silently stand up a LAN-listening terminal on the next launch. + `export_settings`/`apply_settings_import` carve this one field out into `ExportedSecrets` + instead, with the same "only overwrite what the import actually has" treatment as the other + three secrets — except "leave it alone" has to be done by hand in `apply_settings_import`, since + unlike the keychain secrets this one lives inside the `AppSettings` blob that gets replaced + wholesale. `SettingsImportPreview::enables_web_terminal` also exists because of this: `enabled` + and the token are independent fields, and "this turns on a listening service" must not hide + inside a generic "settings replaced" summary. Read this as the standing example of the class of + thing to keep checking for in this feature, not a one-off fixed bug — any other field that looks + like config but is actually a live credential would have the same problem. - **Encrypted because it can carry live credentials, not for appearance's sake.** Argon2id derives a 256-bit key from the user's password (memory-hard — meaningfully resistant to GPU/ASIC brute-forcing, unlike PBKDF2 at any reasonable iteration count), AES-256-GCM does the actual @@ -584,7 +598,22 @@ this is not a project backup. field-by-field merge. Secrets are different on purpose: an absent secret in the export means "the source machine never had this configured," not "delete this on import" — a user who wants to clear a secret already has dedicated UI for that (signing out of shared auth, clearing the - gateway key). + gateway key). Secrets are restored *before* the settings replace runs, not after — replacing + settings is what triggers `reconcile_gateway`, and restoring the other way round leaves a real + window where a gateway recreation happens against the destination's old keys. +- **`read_and_decrypt` checks `format_version` before attempting to parse the full payload, not + after.** A version bump that isn't deserialize-compatible is exactly the case that check exists + for, and parsing the full struct first would fail on the shape mismatch before the version check + ever ran. Neither error path interpolates what `serde_json` actually says into the message + shown to the user — its type-mismatch errors quote the offending value inline, and the plaintext + here can hold a live credential. +- **The 8-character password minimum is enforced in `export_settings` itself, not only in the + export modal.** The frontend minimum is a UX nudge; the Rust command is the actual boundary a + weak password has to cross, and Argon2id's memory-hardness buys little against an attacker who + can just try a short password directly. The derived key and the decrypted plaintext are both + wrapped in `zeroize::Zeroizing` for the same reason every other secret in this codebase gets + handled carefully — cheap insurance (`zeroize` is already pulled in transitively via `aes-gcm`) + for material that exists only to hold or produce live credentials. ## Testing diff --git a/app/src-tauri/Cargo.lock b/app/src-tauri/Cargo.lock index b5b92bd..d663f61 100644 --- a/app/src-tauri/Cargo.lock +++ b/app/src-tauri/Cargo.lock @@ -5307,6 +5307,7 @@ dependencies = [ "tokio", "tower-http", "uuid", + "zeroize", ] [[package]] diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index 88bad73..ebc9648 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -38,6 +38,7 @@ rand = "0.9" local-ip-address = "0.6" argon2 = "0.5" aes-gcm = "0.10" +zeroize = "1" [dev-dependencies] # `test-util` (not part of tokio's `full`) lets the auto-start retry tests run diff --git a/app/src-tauri/src/commands/settings_export_commands.rs b/app/src-tauri/src/commands/settings_export_commands.rs index ebebc81..dc5c3e3 100644 --- a/app/src-tauri/src/commands/settings_export_commands.rs +++ b/app/src-tauri/src/commands/settings_export_commands.rs @@ -20,20 +20,35 @@ //! The password is re-entered (not cached) between preview and apply, so //! that nothing here holds decrypted plaintext — export/import secrets //! included — in memory for longer than one command's execution. +//! +//! **This is new attack surface**: a settings export is a file one person +//! can hand another and ask them to import, together with a password, and +//! `apply_settings_import` applies whatever `AppSettings` it decrypts to +//! wholesale — see the module doc on `models::settings_export` for the +//! `web_terminal.access_token` carve-out a review of this feature found, +//! and treat that as the standing example of the class of thing to keep +//! checking for here, not a one-off fixed bug. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use tauri::State; use tauri_plugin_dialog::DialogExt; use crate::models::{ - ExportedSecrets, SettingsExportPayload, SettingsImportPreview, SETTINGS_EXPORT_FORMAT_VERSION, + AppSettings, ExportedSecrets, SettingsExportPayload, SettingsImportPreview, + SETTINGS_EXPORT_FORMAT_VERSION, }; use crate::storage::{secure, settings_crypto}; use crate::AppState; const FILE_EXTENSION: &str = "triplec"; +/// Enforced here, not only in the export modal: the frontend's minimum is a +/// UX nudge, but `export_settings` is the actual boundary a weak password +/// has to cross, and Argon2id's memory-hardness buys little against an +/// attacker who can just try a three-character password directly. +const MIN_PASSWORD_LEN: usize = 8; + fn suggested_export_name() -> String { // Timestamped so exporting more than once doesn't silently overwrite an // earlier file just because the save dialog defaults to the same name. @@ -73,17 +88,28 @@ async fn pick_import_open_path(window: &tauri::Window) -> Option { rx.await.ok().flatten().and_then(|p| p.into_path().ok()) } -/// Gather the current global secrets. A missing secret reads as `None` — a -/// keychain read failure is treated as "nothing to export" for that one -/// entry rather than aborting the whole export, matching how the rest of -/// this app degrades a keychain error to "absent" (`has_claude_oauth_token`, -/// `has_gateway_api_key`) rather than surfacing it as a hard failure. -fn gather_secrets() -> ExportedSecrets { - ExportedSecrets { +/// Gather the current global secrets, and hand back the `AppSettings` to +/// export with the web-terminal token blanked out of it — see the module +/// doc comment on `models::settings_export` for why that field cannot +/// travel through `settings` like the rest of this struct. +/// +/// A missing keychain secret reads as `None` — a keychain read failure is +/// treated as "nothing to export" for that one entry rather than aborting +/// the whole export, matching how the rest of this app degrades a keychain +/// error to "absent" (`has_claude_oauth_token`, `has_gateway_api_key`) +/// rather than surfacing it as a hard failure. +fn split_settings_and_secrets(current: AppSettings) -> (AppSettings, ExportedSecrets) { + let mut settings = current; + let web_terminal_access_token = settings.web_terminal.access_token.take(); + + let secrets = ExportedSecrets { claude_oauth_token: secure::get_claude_oauth_token().unwrap_or_default(), gateway_api_key: secure::get_gateway_api_key().unwrap_or_default(), gateway_master_key: secure::get_gateway_master_key().unwrap_or_default(), - } + web_terminal_access_token, + }; + + (settings, secrets) } /// Export the current global settings and secrets to a password-encrypted @@ -96,15 +122,18 @@ pub async fn export_settings( window: tauri::Window, state: State<'_, AppState>, ) -> Result { - if password.is_empty() { - return Err("A password is required to export settings.".to_string()); + if password.len() < MIN_PASSWORD_LEN { + return Err(format!( + "Use a password of at least {} characters.", + MIN_PASSWORD_LEN + )); } let Some(dest) = pick_export_save_path(&window, &suggested_export_name()).await else { return Ok(false); }; - let secrets = gather_secrets(); + let (settings, secrets) = split_settings_and_secrets(state.settings_store.get()); if secrets.is_empty() { log::info!("Exporting settings with no global secrets configured on this machine"); } @@ -113,7 +142,7 @@ pub async fn export_settings( format_version: SETTINGS_EXPORT_FORMAT_VERSION, exported_at: chrono::Utc::now().to_rfc3339(), app_version: env!("CARGO_PKG_VERSION").to_string(), - settings: state.settings_store.get(), + settings, secrets, }; @@ -121,7 +150,7 @@ pub async fn export_settings( .map_err(|e| format!("Failed to prepare settings for export: {}", e))?; let encrypted = settings_crypto::encrypt(&plaintext, &password)?; - std::fs::write(&dest, encrypted).map_err(|e| format!("Failed to write export file: {}", e))?; + std::fs::write(&dest, &encrypted).map_err(|e| format!("Failed to write export file: {}", e))?; Ok(true) } @@ -170,11 +199,24 @@ pub async fn preview_settings_import( /// means "the source machine never had this configured," not "delete this /// on import." A user who wants to clear a secret already has dedicated UI /// for that (signing out of shared auth, clearing the gateway key). +/// +/// Order matters here: secrets are restored **before** the settings replace +/// runs (which is what triggers `reconcile_gateway`), so a gateway +/// recreation that replace provokes sees the final key material rather than +/// racing it — restoring the other way round left a real window where the +/// running gateway and the keychain briefly disagreed. +/// +/// The pending path is only cleared on success. A failure here (a rejected +/// host path, a keychain write failure surfaced some other way) leaves the +/// import pending so the frontend can let the user retry `apply` without +/// making them pick the file and re-enter the password again — the +/// preview's job was confirming *what* to import, not spending the one +/// attempt at applying it. #[tauri::command] pub async fn apply_settings_import( password: String, state: State<'_, AppState>, -) -> Result { +) -> Result { if password.is_empty() { return Err("A password is required to import settings.".to_string()); } @@ -183,13 +225,11 @@ pub async fn apply_settings_import( .pending_settings_import .lock() .await - .take() + .clone() .ok_or_else(|| "No import is pending — choose a file first.".to_string())?; let payload = read_and_decrypt(&path, &password)?; - let saved = crate::commands::settings_commands::update_settings(payload.settings, state).await?; - if let Some(token) = non_blank(payload.secrets.claude_oauth_token) { if let Err(e) = secure::store_claude_oauth_token(&token) { log::warn!("Settings import: could not restore the shared Claude login: {}", e); @@ -206,6 +246,20 @@ pub async fn apply_settings_import( } } + // The web-terminal token lives inside `AppSettings` itself rather than + // the keychain, so "leave an absent secret alone" has to be done by + // hand here: carry the destination's current token forward when the + // import doesn't have one, instead of letting the wholesale replace + // below blank it (every export writes `None` there — see + // `split_settings_and_secrets`). + let mut settings = payload.settings; + settings.web_terminal.access_token = non_blank(payload.secrets.web_terminal_access_token) + .or_else(|| state.settings_store.get().web_terminal.access_token); + + let saved = crate::commands::settings_commands::update_settings(settings, state.clone()).await?; + + state.pending_settings_import.lock().await.take(); + Ok(saved) } @@ -213,20 +267,150 @@ fn non_blank(value: Option) -> Option { value.filter(|v| !v.trim().is_empty()) } -fn read_and_decrypt(path: &std::path::Path, password: &str) -> Result { +/// Only the field `read_and_decrypt` needs before deciding whether the rest +/// of the payload is even worth attempting to parse. +#[derive(serde::Deserialize)] +struct FormatVersionProbe { + format_version: u32, +} + +/// Decrypt and parse an export file, checking the format version **before** +/// attempting to deserialize the full payload. +/// +/// That ordering is not just tidiness: a version bump that isn't +/// deserialize-compatible (a field's type changes, not just a new +/// `#[serde(default)]`-covered one) is exactly the case this check exists +/// for, and parsing the full struct first would fail on the shape mismatch +/// before the version check ever ran, surfacing a raw parse error instead +/// of "update Triple-C" — and, more seriously, `serde_json`'s type-mismatch +/// errors quote the offending value inline. This file is not attacker +/// content in the usual sense (it must still decrypt under the right +/// password), but the plaintext it decrypts to can hold a live credential, +/// so neither error path below ever interpolates what `serde_json` +/// actually says — only a fixed, generic message. +fn read_and_decrypt(path: &Path, password: &str) -> Result { let encrypted = std::fs::read(path).map_err(|e| format!("Failed to read export file: {}", e))?; let plaintext = settings_crypto::decrypt(&encrypted, password)?; - let payload: SettingsExportPayload = serde_json::from_slice(&plaintext) - .map_err(|e| format!("This file doesn't look like a valid settings export: {}", e))?; - - if payload.format_version > SETTINGS_EXPORT_FORMAT_VERSION { + let probe: FormatVersionProbe = serde_json::from_slice(&plaintext) + .map_err(|_| "This file doesn't look like a valid settings export.".to_string())?; + if probe.format_version > SETTINGS_EXPORT_FORMAT_VERSION { return Err(format!( "This export was made by a newer version of Triple-C (format {}, this app supports up to {}). \ Update Triple-C before importing it.", - payload.format_version, SETTINGS_EXPORT_FORMAT_VERSION + probe.format_version, SETTINGS_EXPORT_FORMAT_VERSION )); } - Ok(payload) + serde_json::from_slice(&plaintext) + .map_err(|_| "This file doesn't look like a valid settings export (unexpected shape).".to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn non_blank_treats_whitespace_only_as_absent() { + assert_eq!(non_blank(Some(" ".to_string())), None); + assert_eq!(non_blank(Some("".to_string())), None); + assert_eq!(non_blank(None), None); + assert_eq!(non_blank(Some(" a ".to_string())), Some(" a ".to_string())); + } + + fn write_export(dir: &std::path::Path, name: &str, payload: &SettingsExportPayload, password: &str) -> PathBuf { + let plaintext = serde_json::to_vec(payload).unwrap(); + let encrypted = settings_crypto::encrypt(&plaintext, password).unwrap(); + let path = dir.join(name); + std::fs::write(&path, &encrypted).unwrap(); + path + } + + fn sample_payload(format_version: u32) -> SettingsExportPayload { + SettingsExportPayload { + format_version, + exported_at: "2026-08-27T00:00:00Z".to_string(), + app_version: "0.4.14".to_string(), + settings: AppSettings::default(), + secrets: ExportedSecrets::default(), + } + } + + fn temp_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "triple-c-settings-export-test-{}-{}", + name, + uuid::Uuid::new_v4().simple() + )); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + #[test] + fn a_file_from_a_newer_format_is_refused_before_the_full_shape_is_parsed() { + let dir = temp_dir("newer-format"); + let path = write_export( + &dir, + "export.triplec", + &sample_payload(SETTINGS_EXPORT_FORMAT_VERSION + 1), + "correct password", + ); + + let err = read_and_decrypt(&path, "correct password").unwrap_err(); + assert!(err.contains("newer version"), "unexpected message: {}", err); + assert!(err.contains("Update Triple-C")); + + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn a_file_at_the_current_format_is_accepted() { + let dir = temp_dir("current-format"); + let path = write_export( + &dir, + "export.triplec", + &sample_payload(SETTINGS_EXPORT_FORMAT_VERSION), + "correct password", + ); + + let payload = read_and_decrypt(&path, "correct password").unwrap(); + assert_eq!(payload.format_version, SETTINGS_EXPORT_FORMAT_VERSION); + + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn a_malformed_payload_produces_a_generic_error_not_a_raw_serde_message() { + // Encrypt something that decrypts fine but isn't a valid payload + // shape at all — this must not happen in practice (only this app + // ever writes these files), but the error path must still never + // echo back plaintext content, generic malformed-shape or not. + let dir = temp_dir("malformed"); + let plaintext = b"{\"not\": \"a real export\"}".to_vec(); + let encrypted = settings_crypto::encrypt(&plaintext, "correct password").unwrap(); + let path = dir.join("export.triplec"); + std::fs::write(&path, &encrypted).unwrap(); + + let err = read_and_decrypt(&path, "correct password").unwrap_err(); + assert!(!err.contains("not a real export"), "leaked plaintext into the error: {}", err); + assert!(err.contains("doesn't look like a valid settings export")); + + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn the_wrong_password_is_reported_without_a_version_check_ever_running() { + let dir = temp_dir("wrong-password"); + let path = write_export( + &dir, + "export.triplec", + &sample_payload(SETTINGS_EXPORT_FORMAT_VERSION), + "correct password", + ); + + let err = read_and_decrypt(&path, "wrong password").unwrap_err(); + assert!(err.contains("Wrong password"), "unexpected message: {}", err); + + std::fs::remove_dir_all(&dir).ok(); + } } diff --git a/app/src-tauri/src/models/settings_export.rs b/app/src-tauri/src/models/settings_export.rs index 0e71df8..acf33bc 100644 --- a/app/src-tauri/src/models/settings_export.rs +++ b/app/src-tauri/src/models/settings_export.rs @@ -2,12 +2,28 @@ //! //! `SettingsExportPayload` is the whole plaintext export before encryption //! and after decryption (see `storage::settings_crypto`). It bundles -//! `AppSettings` (already the non-secret shape persisted to `settings.json`) -//! with the global secrets that live in the OS keychain instead — the shared -//! Claude Code OAuth login and the model gateway's two keys. Per-project -//! settings, per-project secrets, and anything living in a project's Docker -//! volumes are deliberately out of scope: this exports the *host* -//! environment, not any one project's. +//! `AppSettings` — with one field carved out, see below — with the global +//! secrets that live in the OS keychain instead: the shared Claude Code +//! OAuth login and the model gateway's two keys. Per-project settings, +//! per-project secrets, and anything living in a project's Docker volumes +//! are deliberately out of scope: this exports the *host* environment, not +//! any one project's. +//! +//! **`AppSettings` is not entirely the non-secret shape it looks like.** +//! `WebTerminalSettings::access_token` is a live bearer credential for a +//! server that binds every interface, stored as a plain field on the +//! struct that is otherwise safe to treat as config. A review of this +//! feature caught it: exporting `AppSettings` wholesale would have carried +//! that token along as if it were as inert as a port number, and — worse — +//! importing it would apply `web_terminal.enabled` and the token together +//! with no more warning than any other setting, letting a crafted export +//! silently stand up a LAN-listening terminal server with an +//! attacker-known token on the next launch. `export_settings` / +//! `apply_settings_import` blank this field out of the `settings` they +//! read from and write to, and it travels only through +//! [`ExportedSecrets::web_terminal_access_token`] instead, with the same +//! "only overwrite what the import actually has" treatment as the other +//! three secrets. use serde::{Deserialize, Serialize}; @@ -34,6 +50,12 @@ pub struct ExportedSecrets { pub gateway_api_key: Option, #[serde(default)] pub gateway_master_key: Option, + /// See the module doc comment — this is `AppSettings::web_terminal + /// .access_token`, carved out because it is a live bearer credential, + /// not config, despite living on a struct that is otherwise safe to + /// export wholesale. + #[serde(default)] + pub web_terminal_access_token: Option, } impl ExportedSecrets { @@ -41,6 +63,7 @@ impl ExportedSecrets { self.claude_oauth_token.is_none() && self.gateway_api_key.is_none() && self.gateway_master_key.is_none() + && self.web_terminal_access_token.is_none() } } @@ -78,31 +101,31 @@ pub struct SettingsImportPreview { pub has_claude_oauth_token: bool, pub has_gateway_api_key: bool, pub has_gateway_master_key: bool, + pub has_web_terminal_access_token: bool, + /// Whether the imported settings turn the web terminal on. Named + /// separately from the token above: `enabled` and the token are two + /// different fields, either can be true without the other, and + /// "this import turns on a service that listens on your network" is + /// exactly the kind of change a wholesale settings replace must not + /// bury in a generic "settings replaced" line — see the module doc + /// comment on why this field exists at all. + pub enables_web_terminal: bool, } impl SettingsImportPreview { pub fn from_payload(payload: &SettingsExportPayload) -> Self { + let non_blank = |s: &Option| s.as_deref().is_some_and(|v| !v.trim().is_empty()); Self { exported_at: payload.exported_at.clone(), app_version: payload.app_version.clone(), custom_env_var_count: payload.settings.global_custom_env_vars.len(), gateway_model_count: payload.settings.gateway.models.len(), has_claude_code_settings: payload.settings.global_claude_code_settings.is_some(), - has_claude_oauth_token: payload - .secrets - .claude_oauth_token - .as_deref() - .is_some_and(|t| !t.trim().is_empty()), - has_gateway_api_key: payload - .secrets - .gateway_api_key - .as_deref() - .is_some_and(|k| !k.trim().is_empty()), - has_gateway_master_key: payload - .secrets - .gateway_master_key - .as_deref() - .is_some_and(|k| !k.trim().is_empty()), + has_claude_oauth_token: non_blank(&payload.secrets.claude_oauth_token), + has_gateway_api_key: non_blank(&payload.secrets.gateway_api_key), + has_gateway_master_key: non_blank(&payload.secrets.gateway_master_key), + has_web_terminal_access_token: non_blank(&payload.secrets.web_terminal_access_token), + enables_web_terminal: payload.settings.web_terminal.enabled, } } } @@ -133,6 +156,7 @@ mod tests { claude_oauth_token: Some("sk-super-secret-token".to_string()), gateway_api_key: Some("sk-another-secret".to_string()), gateway_master_key: Some("sk-triple-c-yet-another".to_string()), + web_terminal_access_token: Some("wt-super-secret-token".to_string()), }); let preview = SettingsImportPreview::from_payload(&payload); let serialized = serde_json::to_string(&preview).unwrap(); @@ -140,9 +164,11 @@ mod tests { assert!(!serialized.contains("sk-super-secret-token")); assert!(!serialized.contains("sk-another-secret")); assert!(!serialized.contains("sk-triple-c-yet-another")); + assert!(!serialized.contains("wt-super-secret-token")); assert!(preview.has_claude_oauth_token); assert!(preview.has_gateway_api_key); assert!(preview.has_gateway_master_key); + assert!(preview.has_web_terminal_access_token); } #[test] @@ -154,11 +180,27 @@ mod tests { claude_oauth_token: Some(" ".to_string()), gateway_api_key: None, gateway_master_key: None, + web_terminal_access_token: Some(" ".to_string()), }); let preview = SettingsImportPreview::from_payload(&payload); assert!(!preview.has_claude_oauth_token); assert!(!preview.has_gateway_api_key); assert!(!preview.has_gateway_master_key); + assert!(!preview.has_web_terminal_access_token); + } + + #[test] + fn enabling_the_web_terminal_is_surfaced_regardless_of_whether_a_token_came_with_it() { + // `enabled` and the token are independent fields — a crafted export + // could set one without the other, and both are worth a user's + // attention: this is the field that exists specifically so "this + // import turns on a service that listens on your network" cannot + // hide inside a generic "settings replaced" summary. + let mut payload = payload_with(ExportedSecrets::default()); + payload.settings.web_terminal.enabled = true; + let preview = SettingsImportPreview::from_payload(&payload); + assert!(preview.enables_web_terminal); + assert!(!preview.has_web_terminal_access_token); } #[test] diff --git a/app/src-tauri/src/storage/settings_crypto.rs b/app/src-tauri/src/storage/settings_crypto.rs index 499e7d4..37d40df 100644 --- a/app/src-tauri/src/storage/settings_crypto.rs +++ b/app/src-tauri/src/storage/settings_crypto.rs @@ -24,6 +24,7 @@ use aes_gcm::aead::{Aead, KeyInit}; use aes_gcm::{Aes256Gcm, Nonce}; use argon2::{Algorithm, Argon2, Params, Version}; use rand::RngCore; +use zeroize::Zeroizing; /// Identifies the file as a Triple-C settings export and pins the format — /// a change to the salt/nonce lengths or the KDF/cipher choice below needs a @@ -44,11 +45,16 @@ fn argon2_params() -> Params { Params::new(19 * 1024, 2, 1, Some(KEY_LEN)).expect("hardcoded Argon2 params are valid") } -fn derive_key(password: &str, salt: &[u8]) -> Result<[u8; KEY_LEN], String> { +/// The derived key is wrapped in `Zeroizing` so it is overwritten with zeros +/// when it drops rather than left in freed memory for whatever reuses that +/// stack slot next — cheap insurance (`zeroize` is already in the dependency +/// tree via `aes-gcm`) for material that exists only to decrypt live +/// credentials. +fn derive_key(password: &str, salt: &[u8]) -> Result, String> { let argon2 = Argon2::new(Algorithm::Argon2id, Version::V0x13, argon2_params()); - let mut key = [0u8; KEY_LEN]; + let mut key = Zeroizing::new([0u8; KEY_LEN]); argon2 - .hash_password_into(password.as_bytes(), salt, &mut key) + .hash_password_into(password.as_bytes(), salt, &mut *key) .map_err(|e| format!("Failed to derive encryption key: {}", e))?; Ok(key) } @@ -64,7 +70,7 @@ pub fn encrypt(plaintext: &[u8], password: &str) -> Result, String> { rand::rng().fill_bytes(&mut nonce_bytes); let nonce = Nonce::from_slice(&nonce_bytes); - let cipher = Aes256Gcm::new_from_slice(&key) + let cipher = Aes256Gcm::new_from_slice(&*key) .map_err(|e| format!("Failed to initialize cipher: {}", e))?; let ciphertext = cipher .encrypt(nonce, plaintext) @@ -84,7 +90,12 @@ pub fn encrypt(plaintext: &[u8], password: &str) -> Result, String> { /// fails to verify for the wrong key on essentially any ciphertext, so there /// is no reliable way to tell "wrong password" from "corrupted file" apart, /// and guessing would be worse than saying so. -pub fn decrypt(data: &[u8], password: &str) -> Result, String> { +/// +/// Returns `Zeroizing>` rather than a plain `Vec` — the plaintext +/// this recovers is the whole settings-plus-secrets payload, so it gets the +/// same "wipe it when it drops" treatment as the derived key in +/// [`derive_key`]. +pub fn decrypt(data: &[u8], password: &str) -> Result>, String> { if data.len() < HEADER_LEN { return Err("This does not look like a Triple-C settings export (file too short).".to_string()); } @@ -96,11 +107,12 @@ pub fn decrypt(data: &[u8], password: &str) -> Result, String> { let ciphertext = &data[HEADER_LEN..]; let key = derive_key(password, salt)?; - let cipher = Aes256Gcm::new_from_slice(&key) + let cipher = Aes256Gcm::new_from_slice(&*key) .map_err(|e| format!("Failed to initialize cipher: {}", e))?; let nonce = Nonce::from_slice(nonce_bytes); cipher .decrypt(nonce, ciphertext) + .map(Zeroizing::new) .map_err(|_| "Wrong password, or the file is corrupted.".to_string()) } @@ -113,7 +125,7 @@ mod tests { let plaintext = b"{\"settings\": \"whatever\"}"; let encrypted = encrypt(plaintext, "correct horse battery staple").unwrap(); let decrypted = decrypt(&encrypted, "correct horse battery staple").unwrap(); - assert_eq!(decrypted, plaintext); + assert_eq!(&*decrypted, plaintext); } #[test] diff --git a/app/src/components/settings/ImportSettingsModal.test.tsx b/app/src/components/settings/ImportSettingsModal.test.tsx index 345d916..be8bc0e 100644 --- a/app/src/components/settings/ImportSettingsModal.test.tsx +++ b/app/src/components/settings/ImportSettingsModal.test.tsx @@ -24,6 +24,8 @@ const samplePreview: SettingsImportPreview = { has_claude_oauth_token: true, has_gateway_api_key: false, has_gateway_master_key: false, + has_web_terminal_access_token: false, + enables_web_terminal: false, }; describe("ImportSettingsModal", () => { @@ -54,6 +56,16 @@ describe("ImportSettingsModal", () => { expect(await screen.findByText(/settings imported/i)).toBeInTheDocument(); }); + it("shows a distinct warning when the import would enable the web terminal", async () => { + previewSettingsImport.mockResolvedValue({ ...samplePreview, enables_web_terminal: true }); + render(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "hunter2" } }); + fireEvent.click(screen.getByRole("button", { name: /choose file/i })); + + expect(await screen.findByText(/enables the remote web terminal/i)).toBeInTheDocument(); + }); + it("closes quietly when the file picker is dismissed", async () => { previewSettingsImport.mockResolvedValue(null); const onClose = vi.fn(); diff --git a/app/src/components/settings/ImportSettingsModal.tsx b/app/src/components/settings/ImportSettingsModal.tsx index c8abb8d..c0b79f4 100644 --- a/app/src/components/settings/ImportSettingsModal.tsx +++ b/app/src/components/settings/ImportSettingsModal.tsx @@ -3,7 +3,7 @@ import Modal from "../ui/Modal"; import Button from "../ui/Button"; import Field, { inputClass } from "../ui/Field"; import { applySettingsImport, previewSettingsImport } from "../../lib/tauri-commands"; -import { describeImport } from "../../lib/settingsImportPreview"; +import { describeImport, describeImportWarnings } from "../../lib/settingsImportPreview"; import type { AppSettings, SettingsImportPreview } from "../../lib/types"; interface Props { @@ -114,6 +114,14 @@ export default function ImportSettingsModal({ onClose, onImported }: Props) { ))} + {describeImportWarnings(preview).map((warning) => ( +

+ {warning} +

+ ))} {error &&

{error}

} ) : ( diff --git a/app/src/lib/settingsImportPreview.test.ts b/app/src/lib/settingsImportPreview.test.ts index 4173f42..cad7b81 100644 --- a/app/src/lib/settingsImportPreview.test.ts +++ b/app/src/lib/settingsImportPreview.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { describeImport } from "./settingsImportPreview"; +import { describeImport, describeImportWarnings } from "./settingsImportPreview"; import type { SettingsImportPreview } from "./types"; function preview(overrides: Partial = {}): SettingsImportPreview { @@ -12,6 +12,8 @@ function preview(overrides: Partial = {}): SettingsImport has_claude_oauth_token: false, has_gateway_api_key: false, has_gateway_master_key: false, + has_web_terminal_access_token: false, + enables_web_terminal: false, ...overrides, }; } @@ -52,4 +54,32 @@ describe("describeImport", () => { expect(items.some((i) => i.includes("env var"))).toBe(false); expect(items.some((i) => i.includes("gateway model"))).toBe(false); }); + + it("names the web terminal access token like any other present secret", () => { + const items = describeImport(preview({ has_web_terminal_access_token: true })); + expect(items).toContain("The web terminal access token"); + }); +}); + +describe("describeImportWarnings", () => { + it("is empty when nothing about the import needs extra attention", () => { + expect(describeImportWarnings(preview())).toEqual([]); + }); + + it("warns when the import enables the web terminal, regardless of the token", () => { + // `enabled` and the token are independent — the warning is about the + // service turning on, whether or not a token came with it. + expect(describeImportWarnings(preview({ enables_web_terminal: true }))).toEqual([ + "Enables the remote web terminal, which listens on your network.", + ]); + expect( + describeImportWarnings( + preview({ enables_web_terminal: true, has_web_terminal_access_token: true }), + ), + ).toHaveLength(1); + }); + + it("does not warn just because a web terminal token is present but the terminal is off", () => { + expect(describeImportWarnings(preview({ has_web_terminal_access_token: true }))).toEqual([]); + }); }); diff --git a/app/src/lib/settingsImportPreview.ts b/app/src/lib/settingsImportPreview.ts index dde8d46..0b34db1 100644 --- a/app/src/lib/settingsImportPreview.ts +++ b/app/src/lib/settingsImportPreview.ts @@ -1,7 +1,9 @@ import type { SettingsImportPreview } from "./types"; /** Named things a `SettingsImportPreview` says an import will change, for - * `ImportSettingsModal`'s confirmation list. */ + * `ImportSettingsModal`'s confirmation list. Does not include anything + * `describeImportWarnings` covers — those get their own, more visible + * treatment rather than blending into this list. */ export function describeImport(preview: SettingsImportPreview): string[] { const items: string[] = ["Your global settings (all of them — this replaces what's here now)"]; if (preview.custom_env_var_count > 0) { @@ -16,5 +18,22 @@ export function describeImport(preview: SettingsImportPreview): string[] { if (preview.has_claude_oauth_token) items.push("Your shared Claude login"); if (preview.has_gateway_api_key) items.push("The gateway provider API key"); if (preview.has_gateway_master_key) items.push("The gateway master key"); + if (preview.has_web_terminal_access_token) items.push("The web terminal access token"); return items; } + +/** + * Things about an import that deserve more attention than a bullet in a + * long list — currently just the one, but deliberately its own function + * rather than a flag inside `describeImport`: a setting that turns on a + * network-listening service is exactly the kind of change a "your settings + * were replaced" summary is bad at surfacing, on purpose or (if the file + * came from someone else) not. + */ +export function describeImportWarnings(preview: SettingsImportPreview): string[] { + const warnings: string[] = []; + if (preview.enables_web_terminal) { + warnings.push("Enables the remote web terminal, which listens on your network."); + } + return warnings; +} diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index baadc0b..bca46dc 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -304,6 +304,12 @@ export interface SettingsImportPreview { has_claude_oauth_token: boolean; has_gateway_api_key: boolean; has_gateway_master_key: boolean; + has_web_terminal_access_token: boolean; + /** Whether the import turns the web terminal on — surfaced separately + * from the token above since either can be true without the other, and + * "this enables a service that listens on your network" must not hide + * inside a generic "settings replaced" summary. */ + enables_web_terminal: boolean; } /** What `inspect_ca_cert_path` reports about a corporate CA path. Errors ride From a606e3ab2024967ecfe9475a622a7ce62a5c759c Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 27 Aug 2026 13:13:48 -0700 Subject: [PATCH 3/4] Validate settings imports before writing secrets; disclose base URLs A rejected import (bad env var name, disallowed host path) used to leave keychain secrets already overwritten while the settings themselves stayed unchanged. apply_settings_import now runs update_settings's validation (extracted into validate_settings_update) before any secret write. Also from this review round: sharpened two format-version tests that previously passed against the pre-fix code too, added a direct test for split_settings_and_secrets, warned on a dormant web terminal token even when the terminal import leaves it off, matched the password-length check to the frontend's unit of measure, zeroized the export plaintext buffer, and surfaced non-blank Ollama/llama.cpp/OpenAI-compatible/gateway base URLs in the import preview so a traffic redirect isn't silent. --- CLAUDE.md | 28 ++- .../src/commands/settings_commands.rs | 52 +++-- .../src/commands/settings_export_commands.rs | 208 ++++++++++++++---- app/src-tauri/src/models/settings_export.rs | 49 ++++- .../settings/ImportSettingsModal.test.tsx | 4 + app/src/lib/settingsImportPreview.test.ts | 23 +- app/src/lib/settingsImportPreview.ts | 26 ++- app/src/lib/types.ts | 6 + 8 files changed, 319 insertions(+), 77 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7267b16..55eafd2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -601,6 +601,15 @@ deliberately out of scope — this is not a project backup. gateway key). Secrets are restored *before* the settings replace runs, not after — replacing settings is what triggers `reconcile_gateway`, and restoring the other way round leaves a real window where a gateway recreation happens against the destination's old keys. +- **The imported settings are validated *before* any secret is written, not just before the + settings replace.** `apply_settings_import` calls + `settings_commands::validate_settings_update(¤t, &settings)` — the same checks + `update_settings` runs internally, pulled out into its own function specifically so this caller + can run them first — and only proceeds to the three keychain writes if that passes. A review + caught the earlier ordering: writing secrets first meant a rejected import (a bad env var name, a + disallowed host path) still left the keychain overwritten with the file's secrets while the + settings themselves stayed unchanged, a silently half-applied state the error message gave no + hint of. - **`read_and_decrypt` checks `format_version` before attempting to parse the full payload, not after.** A version bump that isn't deserialize-compatible is exactly the case that check exists for, and parsing the full struct first would fail on the shape mismatch before the version check @@ -610,10 +619,21 @@ deliberately out of scope — this is not a project backup. - **The 8-character password minimum is enforced in `export_settings` itself, not only in the export modal.** The frontend minimum is a UX nudge; the Rust command is the actual boundary a weak password has to cross, and Argon2id's memory-hardness buys little against an attacker who - can just try a short password directly. The derived key and the decrypted plaintext are both - wrapped in `zeroize::Zeroizing` for the same reason every other secret in this codebase gets - handled carefully — cheap insurance (`zeroize` is already pulled in transitively via `aes-gcm`) - for material that exists only to hold or produce live credentials. + can just try a short password directly. Measured with `.chars().count()` (Unicode scalar values) + rather than `.len()` (bytes), to stay as close as this pair of languages allows to the frontend's + `.length` check (UTF-16 code units) — the two only diverge on astral-plane characters. The + derived key and both plaintext buffers — the payload built for export, and whatever `decrypt` + recovers on import — are wrapped in `zeroize::Zeroizing` for the same reason every other secret + in this codebase gets handled carefully — cheap insurance (`zeroize` is already pulled in + transitively via `aes-gcm`) for material that exists only to hold or produce live credentials. +- **The preview also discloses non-blank custom base URLs** (`global_ollama`, `global_llamacpp`, + `global_openai_compatible`, `gateway.api_base`) so an import that would redirect model traffic to + a different server is visible in the confirmation dialog rather than discovered later — these are + endpoints, not secrets, so `SettingsImportPreview` carries and `describeImport` renders the actual + URL rather than just a presence flag. `describeImportWarnings` additionally calls out a web + terminal token that arrives with the terminal left *off*: `start_web_terminal` only mints a fresh + token when none is already set, so a planted token would otherwise activate silently the next + time someone turns the terminal on, with no import-time signal that it wasn't freshly generated. ## Testing diff --git a/app/src-tauri/src/commands/settings_commands.rs b/app/src-tauri/src/commands/settings_commands.rs index 5a8b0c6..060469d 100644 --- a/app/src-tauri/src/commands/settings_commands.rs +++ b/app/src-tauri/src/commands/settings_commands.rs @@ -10,19 +10,24 @@ pub async fn get_settings(state: State<'_, AppState>) -> Result, -) -> Result { - let before = state.settings_store.get(); - +/// Everything `update_settings` refuses a save over, run against the store's +/// *current* value and the incoming one. +/// +/// Pulled out so a caller that does other, harder-to-undo work alongside a +/// settings save — `settings_export_commands::apply_settings_import` +/// restores three keychain secrets in the same command — can run this +/// *first* and bail before touching anything, rather than discovering the +/// rejection only when `update_settings` itself runs partway through. +pub fn validate_settings_update( + before: &AppSettings, + incoming: &AppSettings, +) -> Result<(), String> { // The global half of the same rule the project half gets in // `update_project`: a global custom env var is merged into every project's // container environment, so an unchecked name here reaches all of them. crate::models::validate_env_vars_update( &before.global_custom_env_vars, - &settings.global_custom_env_vars, + &incoming.global_custom_env_vars, )?; // The same for the two host paths this struct owns. `update_project` @@ -40,14 +45,26 @@ pub async fn update_settings( crate::commands::project_commands::validate_mounted_host_path( "SSH key path", before.default_ssh_key_path.as_deref(), - settings.default_ssh_key_path.as_deref(), + incoming.default_ssh_key_path.as_deref(), )?; crate::commands::project_commands::validate_mounted_host_path( "CA certificate path", before.ca_cert_path.as_deref(), - settings.ca_cert_path.as_deref(), + incoming.ca_cert_path.as_deref(), )?; + Ok(()) +} + +#[tauri::command] +pub async fn update_settings( + settings: AppSettings, + state: State<'_, AppState>, +) -> Result { + let before = state.settings_store.get(); + + validate_settings_update(&before, &settings)?; + let saved = state.settings_store.update(settings)?; // Persisting a setting is not the same as applying it. The gateway is the @@ -122,7 +139,10 @@ async fn reconcile_gateway(before: &GatewaySettings, after: &GatewaySettings) { GatewayAction::StopIfRunning => { log::info!("Model gateway disabled in settings — stopping the container"); if let Err(e) = docker::gateway::stop_gateway_container().await { - log::error!("Failed to stop the model gateway after it was disabled: {}", e); + log::error!( + "Failed to stop the model gateway after it was disabled: {}", + e + ); } } GatewayAction::RestartIfRunning => { @@ -138,10 +158,7 @@ async fn reconcile_gateway(before: &GatewaySettings, after: &GatewaySettings) { } #[tauri::command] -pub async fn pull_image( - image_name: String, - app_handle: tauri::AppHandle, -) -> Result<(), String> { +pub async fn pull_image(image_name: String, app_handle: tauri::AppHandle) -> Result<(), String> { use tauri::Emitter; docker::pull_image(&image_name, move |msg| { let _ = app_handle.emit("image-pull-progress", msg); @@ -334,7 +351,10 @@ mod tests { let before = enabled_gateway(); let mut after = before.clone(); after.enabled = false; - assert_eq!(gateway_action(&before, &after), GatewayAction::StopIfRunning); + assert_eq!( + gateway_action(&before, &after), + GatewayAction::StopIfRunning + ); // Still true when it was already off — a stray running container is // still a container that shouldn't be up. assert_eq!(gateway_action(&after, &after), GatewayAction::StopIfRunning); diff --git a/app/src-tauri/src/commands/settings_export_commands.rs b/app/src-tauri/src/commands/settings_export_commands.rs index dc5c3e3..cf0860a 100644 --- a/app/src-tauri/src/commands/settings_export_commands.rs +++ b/app/src-tauri/src/commands/settings_export_commands.rs @@ -33,6 +33,7 @@ use std::path::{Path, PathBuf}; use tauri::State; use tauri_plugin_dialog::DialogExt; +use zeroize::Zeroizing; use crate::models::{ AppSettings, ExportedSecrets, SettingsExportPayload, SettingsImportPreview, @@ -122,7 +123,11 @@ pub async fn export_settings( window: tauri::Window, state: State<'_, AppState>, ) -> Result { - if password.len() < MIN_PASSWORD_LEN { + // `.chars().count()` — Unicode scalar values, not bytes — to stay as + // close as this pair of languages allows to the frontend's `.length` + // check (UTF-16 code units); the two only diverge on astral-plane + // characters, which no reasonable password touches. + if password.chars().count() < MIN_PASSWORD_LEN { return Err(format!( "Use a password of at least {} characters.", MIN_PASSWORD_LEN @@ -146,8 +151,10 @@ pub async fn export_settings( secrets, }; - let plaintext = serde_json::to_vec(&payload) - .map_err(|e| format!("Failed to prepare settings for export: {}", e))?; + let plaintext = Zeroizing::new( + serde_json::to_vec(&payload) + .map_err(|e| format!("Failed to prepare settings for export: {}", e))?, + ); let encrypted = settings_crypto::encrypt(&plaintext, &password)?; std::fs::write(&dest, &encrypted).map_err(|e| format!("Failed to write export file: {}", e))?; @@ -200,18 +207,33 @@ pub async fn preview_settings_import( /// on import." A user who wants to clear a secret already has dedicated UI /// for that (signing out of shared auth, clearing the gateway key). /// -/// Order matters here: secrets are restored **before** the settings replace -/// runs (which is what triggers `reconcile_gateway`), so a gateway -/// recreation that replace provokes sees the final key material rather than -/// racing it — restoring the other way round left a real window where the -/// running gateway and the keychain briefly disagreed. +/// Order matters here, twice over. /// -/// The pending path is only cleared on success. A failure here (a rejected -/// host path, a keychain write failure surfaced some other way) leaves the -/// import pending so the frontend can let the user retry `apply` without -/// making them pick the file and re-enter the password again — the -/// preview's job was confirming *what* to import, not spending the one -/// attempt at applying it. +/// First: the imported settings are **validated before any secret is +/// written**, using the same checks `update_settings` itself runs +/// (`settings_commands::validate_settings_update`). Restoring a secret is +/// hard to undo unnoticed — a stale env-var-name rejection or a disallowed +/// host path used to be caught only when `update_settings` ran, by which +/// point the three keychain secrets below were already overwritten with the +/// file's, each with a fresh rotation id, silently flagging every project +/// container for recreation — while the error the user saw talked only +/// about the rejected setting and said nothing about the credentials that +/// had already moved. Failing this check first makes a rejected import +/// leave nothing touched, matching what "the import failed" is supposed to +/// mean. +/// +/// Second, among the things that *do* get written: secrets are restored +/// **before** the settings replace runs (which is what triggers +/// `reconcile_gateway`), so a gateway recreation that replace provokes sees +/// the final key material rather than racing it — restoring the other way +/// round left a real window where the running gateway and the keychain +/// briefly disagreed. +/// +/// The pending path is only cleared on success. A failure here (rejected by +/// the validation above, or some other error) leaves the import pending so +/// the frontend can let the user retry `apply` without making them pick the +/// file and re-enter the password again — the preview's job was confirming +/// *what* to import, not spending the one attempt at applying it. #[tauri::command] pub async fn apply_settings_import( password: String, @@ -230,21 +252,7 @@ pub async fn apply_settings_import( let payload = read_and_decrypt(&path, &password)?; - if let Some(token) = non_blank(payload.secrets.claude_oauth_token) { - if let Err(e) = secure::store_claude_oauth_token(&token) { - log::warn!("Settings import: could not restore the shared Claude login: {}", e); - } - } - if let Some(key) = non_blank(payload.secrets.gateway_api_key) { - if let Err(e) = secure::store_gateway_api_key(&key) { - log::warn!("Settings import: could not restore the gateway provider API key: {}", e); - } - } - if let Some(key) = non_blank(payload.secrets.gateway_master_key) { - if let Err(e) = secure::store_gateway_master_key(&key) { - log::warn!("Settings import: could not restore the gateway master key: {}", e); - } - } + let current = state.settings_store.get(); // The web-terminal token lives inside `AppSettings` itself rather than // the keychain, so "leave an absent secret alone" has to be done by @@ -254,9 +262,37 @@ pub async fn apply_settings_import( // `split_settings_and_secrets`). let mut settings = payload.settings; settings.web_terminal.access_token = non_blank(payload.secrets.web_terminal_access_token) - .or_else(|| state.settings_store.get().web_terminal.access_token); + .or_else(|| current.web_terminal.access_token.clone()); - let saved = crate::commands::settings_commands::update_settings(settings, state.clone()).await?; + crate::commands::settings_commands::validate_settings_update(¤t, &settings)?; + + if let Some(token) = non_blank(payload.secrets.claude_oauth_token) { + if let Err(e) = secure::store_claude_oauth_token(&token) { + log::warn!( + "Settings import: could not restore the shared Claude login: {}", + e + ); + } + } + if let Some(key) = non_blank(payload.secrets.gateway_api_key) { + if let Err(e) = secure::store_gateway_api_key(&key) { + log::warn!( + "Settings import: could not restore the gateway provider API key: {}", + e + ); + } + } + if let Some(key) = non_blank(payload.secrets.gateway_master_key) { + if let Err(e) = secure::store_gateway_master_key(&key) { + log::warn!( + "Settings import: could not restore the gateway master key: {}", + e + ); + } + } + + let saved = + crate::commands::settings_commands::update_settings(settings, state.clone()).await?; state.pending_settings_import.lock().await.take(); @@ -289,7 +325,8 @@ struct FormatVersionProbe { /// so neither error path below ever interpolates what `serde_json` /// actually says — only a fixed, generic message. fn read_and_decrypt(path: &Path, password: &str) -> Result { - let encrypted = std::fs::read(path).map_err(|e| format!("Failed to read export file: {}", e))?; + let encrypted = + std::fs::read(path).map_err(|e| format!("Failed to read export file: {}", e))?; let plaintext = settings_crypto::decrypt(&encrypted, password)?; let probe: FormatVersionProbe = serde_json::from_slice(&plaintext) @@ -302,8 +339,9 @@ fn read_and_decrypt(path: &Path, password: &str) -> Result PathBuf { - let plaintext = serde_json::to_vec(payload).unwrap(); + fn write_export( + dir: &std::path::Path, + name: &str, + payload: &SettingsExportPayload, + password: &str, + ) -> PathBuf { + write_raw_export(dir, name, &serde_json::to_value(payload).unwrap(), password) + } + + /// Like `write_export`, but takes an arbitrary `serde_json::Value` rather + /// than a real `SettingsExportPayload` — for fixtures that are + /// deliberately not shape-compatible, which the typed helper above can't + /// produce at all. + fn write_raw_export( + dir: &std::path::Path, + name: &str, + value: &serde_json::Value, + password: &str, + ) -> PathBuf { + let plaintext = serde_json::to_vec(value).unwrap(); let encrypted = settings_crypto::encrypt(&plaintext, password).unwrap(); let path = dir.join(name); std::fs::write(&path, &encrypted).unwrap(); path } + #[test] + fn splitting_settings_moves_the_web_terminal_token_out_rather_than_copying_it() { + let mut settings = AppSettings::default(); + settings.web_terminal.access_token = Some("super-secret-token".to_string()); + + let (settings, secrets) = split_settings_and_secrets(settings); + + assert_eq!(settings.web_terminal.access_token, None); + assert_eq!( + secrets.web_terminal_access_token, + Some("super-secret-token".to_string()) + ); + } + + #[test] + fn splitting_settings_with_no_token_leaves_it_absent_on_both_sides() { + let (settings, secrets) = split_settings_and_secrets(AppSettings::default()); + + assert_eq!(settings.web_terminal.access_token, None); + assert_eq!(secrets.web_terminal_access_token, None); + } + fn sample_payload(format_version: u32) -> SettingsExportPayload { SettingsExportPayload { format_version, @@ -348,11 +426,24 @@ mod tests { #[test] fn a_file_from_a_newer_format_is_refused_before_the_full_shape_is_parsed() { + // Shape-incompatible with the *current* `SettingsExportPayload` (a + // future version could easily have changed `settings` from an object + // to something else) as well as newer — so this only passes under + // the probe-first ordering. Parsing the full struct first (the old + // behavior) would fail on the shape mismatch and never reach the + // version check, producing the "unexpected shape" message instead of + // "newer version" / "Update Triple-C". let dir = temp_dir("newer-format"); - let path = write_export( + let path = write_raw_export( &dir, "export.triplec", - &sample_payload(SETTINGS_EXPORT_FORMAT_VERSION + 1), + &serde_json::json!({ + "format_version": SETTINGS_EXPORT_FORMAT_VERSION + 1, + "exported_at": "2026-08-27T00:00:00Z", + "app_version": "9.9.9", + "settings": "this-app-version-stores-settings-differently", + "secrets": {}, + }), "correct password", ); @@ -381,18 +472,35 @@ mod tests { #[test] fn a_malformed_payload_produces_a_generic_error_not_a_raw_serde_message() { - // Encrypt something that decrypts fine but isn't a valid payload - // shape at all — this must not happen in practice (only this app - // ever writes these files), but the error path must still never - // echo back plaintext content, generic malformed-shape or not. + // A `format_version` the probe accepts, but a `settings` field of + // the wrong *type* rather than just a missing field — this is what + // makes `serde_json` produce an "invalid type: string `...`, expected + // struct AppSettings" error that quotes the offending value + // verbatim. That value here stands in for plaintext that, in a real + // export, could be a live credential — the assertion below is only + // meaningful against a fixture that actually exercises serde's + // value-quoting behavior, which a merely-missing-field fixture does + // not. let dir = temp_dir("malformed"); - let plaintext = b"{\"not\": \"a real export\"}".to_vec(); - let encrypted = settings_crypto::encrypt(&plaintext, "correct password").unwrap(); - let path = dir.join("export.triplec"); - std::fs::write(&path, &encrypted).unwrap(); + let path = write_raw_export( + &dir, + "export.triplec", + &serde_json::json!({ + "format_version": SETTINGS_EXPORT_FORMAT_VERSION, + "exported_at": "2026-08-27T00:00:00Z", + "app_version": "0.4.14", + "settings": "NOT-A-REAL-CREDENTIAL-abc123", + "secrets": {}, + }), + "correct password", + ); let err = read_and_decrypt(&path, "correct password").unwrap_err(); - assert!(!err.contains("not a real export"), "leaked plaintext into the error: {}", err); + assert!( + !err.contains("NOT-A-REAL-CREDENTIAL-abc123"), + "leaked plaintext into the error: {}", + err + ); assert!(err.contains("doesn't look like a valid settings export")); std::fs::remove_dir_all(&dir).ok(); @@ -409,7 +517,11 @@ mod tests { ); let err = read_and_decrypt(&path, "wrong password").unwrap_err(); - assert!(err.contains("Wrong password"), "unexpected message: {}", err); + assert!( + err.contains("Wrong password"), + "unexpected message: {}", + err + ); std::fs::remove_dir_all(&dir).ok(); } diff --git a/app/src-tauri/src/models/settings_export.rs b/app/src-tauri/src/models/settings_export.rs index acf33bc..550d163 100644 --- a/app/src-tauri/src/models/settings_export.rs +++ b/app/src-tauri/src/models/settings_export.rs @@ -110,11 +110,24 @@ pub struct SettingsImportPreview { /// bury in a generic "settings replaced" line — see the module doc /// comment on why this field exists at all. pub enables_web_terminal: bool, + /// Non-blank custom base URLs the import would set, so a redirect of + /// model traffic to somewhere other than the usual provider is visible + /// at import time rather than discovered later. These are endpoints, not + /// secrets — safe to show verbatim, unlike everything above. + #[serde(default)] + pub ollama_base_url: Option, + #[serde(default)] + pub llamacpp_base_url: Option, + #[serde(default)] + pub openai_compatible_base_url: Option, + #[serde(default)] + pub gateway_api_base: Option, } impl SettingsImportPreview { pub fn from_payload(payload: &SettingsExportPayload) -> Self { let non_blank = |s: &Option| s.as_deref().is_some_and(|v| !v.trim().is_empty()); + let non_blank_value = |s: &Option| s.clone().filter(|v| !v.trim().is_empty()); Self { exported_at: payload.exported_at.clone(), app_version: payload.app_version.clone(), @@ -126,6 +139,12 @@ impl SettingsImportPreview { has_gateway_master_key: non_blank(&payload.secrets.gateway_master_key), has_web_terminal_access_token: non_blank(&payload.secrets.web_terminal_access_token), enables_web_terminal: payload.settings.web_terminal.enabled, + ollama_base_url: non_blank_value(&payload.settings.global_ollama.base_url), + llamacpp_base_url: non_blank_value(&payload.settings.global_llamacpp.base_url), + openai_compatible_base_url: non_blank_value( + &payload.settings.global_openai_compatible.base_url, + ), + gateway_api_base: non_blank_value(&payload.settings.gateway.api_base), } } } @@ -138,8 +157,14 @@ mod tests { fn payload_with(secrets: ExportedSecrets) -> SettingsExportPayload { let mut settings = AppSettings::default(); settings.global_custom_env_vars = vec![ - crate::models::EnvVar { key: "A".to_string(), value: "1".to_string() }, - crate::models::EnvVar { key: "B".to_string(), value: "2".to_string() }, + crate::models::EnvVar { + key: "A".to_string(), + value: "1".to_string(), + }, + crate::models::EnvVar { + key: "B".to_string(), + value: "2".to_string(), + }, ]; SettingsExportPayload { format_version: SETTINGS_EXPORT_FORMAT_VERSION, @@ -203,6 +228,26 @@ mod tests { assert!(!preview.has_web_terminal_access_token); } + #[test] + fn custom_base_urls_are_surfaced_but_blank_ones_read_as_absent() { + let mut payload = payload_with(ExportedSecrets::default()); + payload.settings.global_ollama.base_url = Some("http://attacker.example:11434".to_string()); + payload.settings.global_llamacpp.base_url = Some(" ".to_string()); + payload.settings.gateway.api_base = Some("https://gateway.example/v1".to_string()); + + let preview = SettingsImportPreview::from_payload(&payload); + assert_eq!( + preview.ollama_base_url.as_deref(), + Some("http://attacker.example:11434") + ); + assert_eq!(preview.llamacpp_base_url, None); + assert_eq!(preview.openai_compatible_base_url, None); + assert_eq!( + preview.gateway_api_base.as_deref(), + Some("https://gateway.example/v1") + ); + } + #[test] fn counts_reflect_the_real_settings() { let payload = payload_with(ExportedSecrets::default()); diff --git a/app/src/components/settings/ImportSettingsModal.test.tsx b/app/src/components/settings/ImportSettingsModal.test.tsx index be8bc0e..2d9807e 100644 --- a/app/src/components/settings/ImportSettingsModal.test.tsx +++ b/app/src/components/settings/ImportSettingsModal.test.tsx @@ -26,6 +26,10 @@ const samplePreview: SettingsImportPreview = { has_gateway_master_key: false, has_web_terminal_access_token: false, enables_web_terminal: false, + ollama_base_url: null, + llamacpp_base_url: null, + openai_compatible_base_url: null, + gateway_api_base: null, }; describe("ImportSettingsModal", () => { diff --git a/app/src/lib/settingsImportPreview.test.ts b/app/src/lib/settingsImportPreview.test.ts index cad7b81..8124888 100644 --- a/app/src/lib/settingsImportPreview.test.ts +++ b/app/src/lib/settingsImportPreview.test.ts @@ -14,6 +14,10 @@ function preview(overrides: Partial = {}): SettingsImport has_gateway_master_key: false, has_web_terminal_access_token: false, enables_web_terminal: false, + ollama_base_url: null, + llamacpp_base_url: null, + openai_compatible_base_url: null, + gateway_api_base: null, ...overrides, }; } @@ -59,6 +63,19 @@ describe("describeImport", () => { const items = describeImport(preview({ has_web_terminal_access_token: true })); expect(items).toContain("The web terminal access token"); }); + + it("names custom base URLs verbatim, since they're endpoints rather than secrets", () => { + const items = describeImport( + preview({ + ollama_base_url: "http://10.0.0.5:11434", + gateway_api_base: "https://gateway.example/v1", + }), + ); + expect(items).toContain("Ollama server: http://10.0.0.5:11434"); + expect(items).toContain("Gateway upstream: https://gateway.example/v1"); + expect(items.some((i) => i.includes("llama.cpp"))).toBe(false); + expect(items.some((i) => i.includes("OpenAI-compatible"))).toBe(false); + }); }); describe("describeImportWarnings", () => { @@ -79,7 +96,9 @@ describe("describeImportWarnings", () => { ).toHaveLength(1); }); - it("does not warn just because a web terminal token is present but the terminal is off", () => { - expect(describeImportWarnings(preview({ has_web_terminal_access_token: true }))).toEqual([]); + it("warns about a dormant web terminal token even while the terminal stays off", () => { + expect(describeImportWarnings(preview({ has_web_terminal_access_token: true }))).toEqual([ + "Includes a web terminal access token that will activate the next time the web terminal is turned on.", + ]); }); }); diff --git a/app/src/lib/settingsImportPreview.ts b/app/src/lib/settingsImportPreview.ts index 0b34db1..6e9c099 100644 --- a/app/src/lib/settingsImportPreview.ts +++ b/app/src/lib/settingsImportPreview.ts @@ -19,21 +19,37 @@ export function describeImport(preview: SettingsImportPreview): string[] { if (preview.has_gateway_api_key) items.push("The gateway provider API key"); if (preview.has_gateway_master_key) items.push("The gateway master key"); if (preview.has_web_terminal_access_token) items.push("The web terminal access token"); + if (preview.ollama_base_url) items.push(`Ollama server: ${preview.ollama_base_url}`); + if (preview.llamacpp_base_url) items.push(`llama.cpp server: ${preview.llamacpp_base_url}`); + if (preview.openai_compatible_base_url) { + items.push(`OpenAI-compatible server: ${preview.openai_compatible_base_url}`); + } + if (preview.gateway_api_base) items.push(`Gateway upstream: ${preview.gateway_api_base}`); return items; } /** * Things about an import that deserve more attention than a bullet in a - * long list — currently just the one, but deliberately its own function - * rather than a flag inside `describeImport`: a setting that turns on a - * network-listening service is exactly the kind of change a "your settings - * were replaced" summary is bad at surfacing, on purpose or (if the file - * came from someone else) not. + * long list — deliberately its own function rather than a flag inside + * `describeImport`: a setting that turns on a network-listening service is + * exactly the kind of change a "your settings were replaced" summary is bad + * at surfacing, on purpose or (if the file came from someone else) not. + * + * A token that arrives with the terminal left *off* gets its own warning + * too, distinct from the "enables it now" one: `start_web_terminal` only + * mints a fresh token when none is already set, so a planted token here + * would silently become live the next time someone flips the terminal on + * through the UI, with no import-time signal that it wasn't freshly + * generated. */ export function describeImportWarnings(preview: SettingsImportPreview): string[] { const warnings: string[] = []; if (preview.enables_web_terminal) { warnings.push("Enables the remote web terminal, which listens on your network."); + } else if (preview.has_web_terminal_access_token) { + warnings.push( + "Includes a web terminal access token that will activate the next time the web terminal is turned on.", + ); } return warnings; } diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index bca46dc..3ddb549 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -310,6 +310,12 @@ export interface SettingsImportPreview { * "this enables a service that listens on your network" must not hide * inside a generic "settings replaced" summary. */ enables_web_terminal: boolean; + /** Non-blank custom base URLs the import would set — endpoints, not + * secrets, so shown verbatim to disclose a redirect of model traffic. */ + ollama_base_url: string | null; + llamacpp_base_url: string | null; + openai_compatible_base_url: string | null; + gateway_api_base: string | null; } /** What `inspect_ca_cert_path` reports about a corporate CA path. Errors ride From 97e58db3c1afe3b211f0182fd0d646e308c56115 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 27 Aug 2026 14:24:06 -0700 Subject: [PATCH 4/4] Close gateway-secret desync, TOCTOU, and undisclosed custom-image gaps Round 4 review findings: - Disclose and warn on a custom Docker image the import would set (HIGH): it's the image every project container is created from, so an undisclosed change here was a sharper version of the redirected-base-URL problem round 3 already flagged for the model backends. - Recreate a running gateway container when an import restores a new secret with the shape unchanged (MEDIUM): reconcile_gateway's shape comparison can't see a secret-only change, so the container would otherwise keep serving old key material indefinitely. - Report keychain write failures back to the caller instead of only logging them (MEDIUM): apply_settings_import now returns SettingsImportOutcome with secret_restore_warnings so a partial restore can't read as unqualified success. - Pin a hash of the previewed file's ciphertext and refuse to apply if it changed on disk (MEDIUM): closes a TOCTOU between preview and apply. - Sanitize and cap every free-form string a preview surfaces, and move the warning boxes above the replace list in the UI (MEDIUM): an unbounded base URL or image name could otherwise push the security warnings below the scroll fold. - Validate the Docker socket path on import the same as the SSH key and CA cert paths (LOW): it was the one mounted host path validate_settings_update didn't cover. - Fix ExportedSecrets::is_empty() to treat whitespace-only as blank, like every other secret-presence check in this feature (LOW). - Authenticate the file header as AEAD associated data (LOW, defense in depth) and correct two doc comments that overstated the password not being cached. --- CLAUDE.md | 52 ++++- .../src/commands/settings_commands.rs | 11 + .../src/commands/settings_export_commands.rs | 204 ++++++++++++++---- app/src-tauri/src/lib.rs | 8 +- app/src-tauri/src/models/settings_export.rs | 141 ++++++++++-- app/src-tauri/src/storage/settings_crypto.rs | 28 ++- .../settings/ImportSettingsModal.test.tsx | 42 +++- .../settings/ImportSettingsModal.tsx | 43 ++-- app/src/lib/settingsImportPreview.test.ts | 21 ++ app/src/lib/settingsImportPreview.ts | 12 ++ app/src/lib/tauri-commands.ts | 4 +- app/src/lib/types.ts | 14 ++ 12 files changed, 491 insertions(+), 89 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 55eafd2..c03fb36 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -588,11 +588,18 @@ deliberately out of scope — this is not a project backup. handing Rust a host path string is the exact shape of bug that produced this app's past criticals. `preview_settings_import` resolves the chosen path itself and remembers it (`AppState::pending_settings_import`) so `apply_settings_import` re-reads the same file without - a path ever crossing back over IPC. -- **The password is re-entered, not cached, between preview and apply.** Nothing here holds - decrypted plaintext — secrets included — in memory for longer than one command's execution. - `preview_settings_import` returns counts and presence flags only (`SettingsImportPreview`), - never a secret value, so it's safe to hand to the frontend and render directly. + a path ever crossing back over IPC. It also pins a hash of the file's ciphertext next to that + path, and `apply_settings_import` refuses to proceed if the file on disk no longer matches it — + otherwise confirming a preview would not actually be binding on what gets applied, which matters + given this feature's own threat model: a file shared between people may sit in a synced or + otherwise shared directory that changes between the two calls. +- **The decrypted payload is not cached between preview and apply — only the password is reused.** + The frontend holds the password in React state and passes it to both calls; nothing in Rust + holds decrypted plaintext — secrets included — in memory for longer than one command's + execution, so `apply_settings_import` always re-decrypts rather than reusing anything + `preview_settings_import` computed. `preview_settings_import` returns counts and presence flags + only (`SettingsImportPreview`), never a secret value, so it's safe to hand to the frontend and + render directly. - **Import replaces settings wholesale, but only writes secrets actually present in the file.** An import is "restore this environment," so the settings half is a full replace, not a field-by-field merge. Secrets are different on purpose: an absent secret in the export means @@ -601,6 +608,23 @@ deliberately out of scope — this is not a project backup. gateway key). Secrets are restored *before* the settings replace runs, not after — replacing settings is what triggers `reconcile_gateway`, and restoring the other way round leaves a real window where a gateway recreation happens against the destination's old keys. +- **A restored gateway secret nudges a running gateway container to recreate itself, even when + nothing about the gateway's *shape* changed.** `reconcile_gateway`'s `gateway_shape_changed` only + compares port/provider/base URL/models — deliberately, since that's what's rendered into the + container's config — so a secret-only change (same shape, new key) is invisible to it. Left + alone, a running container would keep serving the old key material indefinitely after an import + that restored a new one. `apply_settings_import` tracks whether either gateway secret was + actually written and, if the gateway is enabled and its container both exists and is running, + calls `docker::gateway::ensure_gateway_running` directly afterward — its own fingerprint already + includes the secret rotation id (`storage::secure::get_gateway_secret_version`), so it recreates + exactly when it should and no more. +- **A keychain write failing during import is reported back, not only logged.** Each of the three + `secure::store_*` calls collects its error into `SettingsImportOutcome::secret_restore_warnings` + in addition to logging it — an import that silently restores two of three secrets but not the + third must not read as unqualified success just because the settings half of the import (which + runs after, and is validated before any of this) went through. `apply_settings_import` returns + `SettingsImportOutcome { settings, secret_restore_warnings }` rather than bare `AppSettings` for + this reason; `ImportSettingsModal` shows any warnings alongside the "Settings imported" message. - **The imported settings are validated *before* any secret is written, not just before the settings replace.** `apply_settings_import` calls `settings_commands::validate_settings_update(¤t, &settings)` — the same checks @@ -634,6 +658,24 @@ deliberately out of scope — this is not a project backup. terminal token that arrives with the terminal left *off*: `start_web_terminal` only mints a fresh token when none is already set, so a planted token would otherwise activate silently the next time someone turns the terminal on, with no import-time signal that it wasn't freshly generated. +- **The preview also discloses a custom Docker image, and warns on one every time — not just on + change.** `custom_image_name`/`image_source` weren't in scope for the base-URL disclosure above, + but a review pointed out they're a sharper version of the same problem: this is the image *every* + project container is created from (`models::container_config::resolve_image_name`), so a crafted + export pointing it at an attacker-controlled image is a path to running arbitrary code with + whatever a project's containers are allowed to reach, not merely a redirected API endpoint. + `describeImportWarnings` fires on `image_source == Custom` unconditionally rather than only when + it differs from the destination's current value, since re-importing the same risky configuration + is still worth surfacing every time a user confirms an import. +- **Every free-form string a preview surfaces is sanitized and length-capped before it's built.** + `SettingsImportPreview::from_payload`'s `sanitize_for_preview` strips control characters and caps + at 100 characters (`MAX_PREVIEW_STRING_LEN`) for every base URL and the custom image name — a + review noted that, unlike the count- and boolean-derived fields the preview started with, these + are verbatim strings from a not-yet-trusted decrypted payload rendered directly into the + confirmation dialog. Unbounded, a single pathological value (very long, or holding embedded + newlines) could push the security warnings above the scroll fold in the dialog that exists + specifically to make them unmissable — the frontend's `
  • `/warning boxes also get `break-all` + as a second layer against the same failure mode. ## Testing diff --git a/app/src-tauri/src/commands/settings_commands.rs b/app/src-tauri/src/commands/settings_commands.rs index 060469d..13309bb 100644 --- a/app/src-tauri/src/commands/settings_commands.rs +++ b/app/src-tauri/src/commands/settings_commands.rs @@ -53,6 +53,17 @@ pub fn validate_settings_update( incoming.ca_cert_path.as_deref(), )?; + // Third host path this struct owns, same reasoning: any project with + // `allow_docker_access` bind-mounts this path in as the Docker socket + // (`project_commands.rs`'s container creation), so an unchecked value + // here is a read-write bind mount of whatever it names into every such + // project's container. + crate::commands::project_commands::validate_mounted_host_path( + "Docker socket path", + before.docker_socket_path.as_deref(), + incoming.docker_socket_path.as_deref(), + )?; + Ok(()) } diff --git a/app/src-tauri/src/commands/settings_export_commands.rs b/app/src-tauri/src/commands/settings_export_commands.rs index cf0860a..1bdd35b 100644 --- a/app/src-tauri/src/commands/settings_export_commands.rs +++ b/app/src-tauri/src/commands/settings_export_commands.rs @@ -17,9 +17,12 @@ //! (`AppState::pending_settings_import`) so `apply_settings_import` re-reads //! the same file without the path ever crossing back over IPC. //! -//! The password is re-entered (not cached) between preview and apply, so -//! that nothing here holds decrypted plaintext — export/import secrets -//! included — in memory for longer than one command's execution. +//! The *decrypted payload* is not cached between preview and apply — the +//! password the frontend passes to each call is what it already held for +//! the first, not a fresh secret extracted from the user, but nothing here +//! keeps the plaintext itself — export/import secrets included — around for +//! longer than one command's execution; `apply_settings_import` re-decrypts +//! the file rather than reusing anything `preview_settings_import` computed. //! //! **This is new attack surface**: a settings export is a file one person //! can hand another and ask them to import, together with a password, and @@ -29,19 +32,39 @@ //! and treat that as the standing example of the class of thing to keep //! checking for here, not a one-off fixed bug. -use std::path::{Path, PathBuf}; +#[cfg(test)] +use std::path::Path; +use std::path::PathBuf; +use sha2::{Digest, Sha256}; use tauri::State; use tauri_plugin_dialog::DialogExt; use zeroize::Zeroizing; use crate::models::{ - AppSettings, ExportedSecrets, SettingsExportPayload, SettingsImportPreview, - SETTINGS_EXPORT_FORMAT_VERSION, + AppSettings, ExportedSecrets, SettingsExportPayload, SettingsImportOutcome, + SettingsImportPreview, SETTINGS_EXPORT_FORMAT_VERSION, }; use crate::storage::{secure, settings_crypto}; use crate::AppState; +/// What `preview_settings_import` pins so `apply_settings_import` can tell +/// whether the file it's about to re-read is the same one the user actually +/// saw a preview of. Confirming a preview is only meaningful if it's binding +/// on what gets applied — without this, a file replaced on disk between the +/// two calls (this app's own stated threat model is a file shared between +/// people, which may sit in a synced or shared directory) would decrypt and +/// apply silently different content than what the confirmation dialog showed. +#[derive(Debug, Clone)] +pub struct PendingSettingsImport { + path: PathBuf, + ciphertext_hash: [u8; 32], +} + +fn hash_ciphertext(data: &[u8]) -> [u8; 32] { + Sha256::digest(data).into() +} + const FILE_EXTENSION: &str = "triplec"; /// Enforced here, not only in the export modal: the frontend's minimum is a @@ -166,10 +189,13 @@ pub async fn export_settings( /// preview (counts and presence flags only — never a secret value) for a /// confirmation UI. `Ok(None)` means the picker was dismissed. /// -/// Remembers the resolved path in `AppState::pending_settings_import` for -/// `apply_settings_import` to re-read; does **not** remember the decrypted -/// payload itself, so the password must be supplied again to actually apply -/// it — seeing the preview is not the same as committing to it. +/// Remembers the resolved path *and a hash of the file's ciphertext* in +/// `AppState::pending_settings_import` for `apply_settings_import` to check +/// against — does **not** remember the decrypted payload itself, so the +/// password must be supplied again to actually apply it — seeing the preview +/// is not the same as committing to it. The hash exists so it also can't be +/// swapped out from under that commitment: `apply_settings_import` refuses to +/// proceed if the file on disk no longer matches what was just previewed. #[tauri::command] pub async fn preview_settings_import( password: String, @@ -184,10 +210,14 @@ pub async fn preview_settings_import( return Ok(None); }; - let payload = read_and_decrypt(&path, &password)?; + 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 preview = SettingsImportPreview::from_payload(&payload); - *state.pending_settings_import.lock().await = Some(path); + *state.pending_settings_import.lock().await = Some(PendingSettingsImport { + path, + ciphertext_hash: hash_ciphertext(&encrypted), + }); Ok(Some(preview)) } @@ -196,7 +226,14 @@ pub async fn preview_settings_import( /// for. Fails if no preview is pending — this is not a general "decrypt and /// apply this file" entry point, deliberately: seeing the preview first is /// required, not just encouraged, since it is the only place a user is told -/// what an import is about to touch before it touches it. +/// what an import is about to touch before it touches it. That requirement +/// is only real if the file can't change out from under it, so this also +/// refuses to proceed if the file's ciphertext no longer matches the hash +/// `preview_settings_import` pinned — a file replaced on disk between the +/// two calls (this feature's own threat model is a file shared between +/// people, which may sit in a synced or shared directory) must not be able +/// to apply silently different content than what the confirmation dialog +/// showed. /// /// Global settings are replaced wholesale — an import is "restore this /// environment," not a field-by-field merge. Global secrets are handled @@ -227,30 +264,48 @@ pub async fn preview_settings_import( /// `reconcile_gateway`), so a gateway recreation that replace provokes sees /// the final key material rather than racing it — restoring the other way /// round left a real window where the running gateway and the keychain -/// briefly disagreed. +/// briefly disagreed. A gateway *secret* alone (same shape, new key) is +/// invisible to `reconcile_gateway`'s shape comparison, so this additionally +/// nudges a running gateway container to recreate itself whenever a secret +/// this import carried was actually written — otherwise the running +/// container keeps serving the old key material indefinitely while every +/// project container is handed the new one. /// -/// The pending path is only cleared on success. A failure here (rejected by -/// the validation above, or some other error) leaves the import pending so -/// the frontend can let the user retry `apply` without making them pick the -/// file and re-enter the password again — the preview's job was confirming -/// *what* to import, not spending the one attempt at applying it. +/// A keychain write failing is reported back rather than only logged: an +/// import that silently restores two of three secrets but not the third +/// must not read as unqualified success. +/// +/// The pending import is only cleared on success. A failure here (rejected +/// by the validation above, a stale-file mismatch, or some other error) +/// leaves it pending so the frontend can let the user retry `apply` without +/// making them pick the file and re-enter the password again — the +/// preview's job was confirming *what* to import, not spending the one +/// attempt at applying it. #[tauri::command] pub async fn apply_settings_import( password: String, state: State<'_, AppState>, -) -> Result { +) -> Result { if password.is_empty() { return Err("A password is required to import settings.".to_string()); } - let path = state + let pending = state .pending_settings_import .lock() .await .clone() .ok_or_else(|| "No import is pending — choose a file first.".to_string())?; - let payload = read_and_decrypt(&path, &password)?; + let encrypted = std::fs::read(&pending.path) + .map_err(|e| format!("Failed to read export file: {}", e))?; + if hash_ciphertext(&encrypted) != pending.ciphertext_hash { + return Err( + "This file changed since you reviewed it — choose it again to see an up-to-date preview." + .to_string(), + ); + } + let payload = read_and_decrypt_bytes(&encrypted, &password)?; let current = state.settings_store.get(); @@ -266,37 +321,81 @@ pub async fn apply_settings_import( crate::commands::settings_commands::validate_settings_update(¤t, &settings)?; + let mut secret_restore_warnings = Vec::new(); + let mut gateway_secret_changed = false; + if let Some(token) = non_blank(payload.secrets.claude_oauth_token) { if let Err(e) = secure::store_claude_oauth_token(&token) { log::warn!( "Settings import: could not restore the shared Claude login: {}", e ); + secret_restore_warnings + .push(format!("Could not restore your shared Claude login: {}", e)); } } if let Some(key) = non_blank(payload.secrets.gateway_api_key) { - if let Err(e) = secure::store_gateway_api_key(&key) { - log::warn!( - "Settings import: could not restore the gateway provider API key: {}", - e - ); + match secure::store_gateway_api_key(&key) { + Ok(()) => gateway_secret_changed = true, + Err(e) => { + log::warn!( + "Settings import: could not restore the gateway provider API key: {}", + e + ); + secret_restore_warnings.push(format!( + "Could not restore the gateway provider API key: {}", + e + )); + } } } if let Some(key) = non_blank(payload.secrets.gateway_master_key) { - if let Err(e) = secure::store_gateway_master_key(&key) { - log::warn!( - "Settings import: could not restore the gateway master key: {}", - e - ); + match secure::store_gateway_master_key(&key) { + Ok(()) => gateway_secret_changed = true, + Err(e) => { + log::warn!( + "Settings import: could not restore the gateway master key: {}", + e + ); + secret_restore_warnings + .push(format!("Could not restore the gateway master key: {}", e)); + } } } let saved = crate::commands::settings_commands::update_settings(settings, state.clone()).await?; + // `reconcile_gateway` (inside `update_settings`) only reacts to a changed + // *shape* — port, provider, base URL, models — because that's what's + // rendered into the container's config. A secret changing with the shape + // held constant is invisible to it, so a running gateway container would + // otherwise keep serving the old key material forever after an import + // that restored a new one, while `docker::gateway`'s own fingerprint + // (which does include the secret rotation id) means the *next* unrelated + // settings save would suddenly and confusingly recreate it instead. + if gateway_secret_changed && saved.gateway.enabled { + match crate::docker::gateway::gateway_container_presence().await { + Ok((true, true)) => { + if let Err(e) = crate::docker::gateway::ensure_gateway_running(&saved.gateway).await + { + log::error!( + "Settings import: could not apply the restored gateway credentials to the running gateway container: {}", + e + ); + } + } + Ok(_) => {} + Err(e) => log::debug!("Settings import: gateway reconcile skipped ({})", e), + } + } + state.pending_settings_import.lock().await.take(); - Ok(saved) + Ok(SettingsImportOutcome { + settings: saved, + secret_restore_warnings, + }) } fn non_blank(value: Option) -> Option { @@ -310,8 +409,22 @@ struct FormatVersionProbe { format_version: u32, } -/// Decrypt and parse an export file, checking the format version **before** -/// attempting to deserialize the full payload. +/// Read and decrypt an export file at `path`, then parse it — see +/// `read_and_decrypt_bytes` for why the format-version check runs before the +/// full parse. Every real caller already has the file's bytes in hand by the +/// time it needs this (`preview_settings_import`/`apply_settings_import` +/// both hash the ciphertext first) and calls `read_and_decrypt_bytes` +/// directly to avoid reading the file twice; this path-based wrapper only +/// exists now for tests that don't need that. +#[cfg(test)] +fn read_and_decrypt(path: &Path, password: &str) -> Result { + let encrypted = + std::fs::read(path).map_err(|e| format!("Failed to read export file: {}", e))?; + read_and_decrypt_bytes(&encrypted, password) +} + +/// Decrypt and parse an already-read export file's bytes, checking the +/// format version **before** attempting to deserialize the full payload. /// /// That ordering is not just tidiness: a version bump that isn't /// deserialize-compatible (a field's type changes, not just a new @@ -324,10 +437,8 @@ struct FormatVersionProbe { /// password), but the plaintext it decrypts to can hold a live credential, /// so neither error path below ever interpolates what `serde_json` /// actually says — only a fixed, generic message. -fn read_and_decrypt(path: &Path, password: &str) -> Result { - let encrypted = - std::fs::read(path).map_err(|e| format!("Failed to read export file: {}", e))?; - let plaintext = settings_crypto::decrypt(&encrypted, password)?; +fn read_and_decrypt_bytes(encrypted: &[u8], password: &str) -> Result { + let plaintext = settings_crypto::decrypt(encrypted, password)?; let probe: FormatVersionProbe = serde_json::from_slice(&plaintext) .map_err(|_| "This file doesn't look like a valid settings export.".to_string())?; @@ -356,6 +467,21 @@ mod tests { assert_eq!(non_blank(Some(" a ".to_string())), Some(" a ".to_string())); } + #[test] + fn ciphertext_hashing_is_deterministic_and_tamper_sensitive() { + // What `apply_settings_import` compares against the pinned hash from + // `preview_settings_import` to detect a file swapped out from under a + // pending import — this only defends anything if identical bytes + // always hash identically and any change to those bytes changes the + // hash. + let bytes = b"pretend this is an encrypted export file"; + assert_eq!(hash_ciphertext(bytes), hash_ciphertext(bytes)); + + let mut tampered = bytes.to_vec(); + tampered[0] ^= 0xFF; + assert_ne!(hash_ciphertext(bytes), hash_ciphertext(&tampered)); + } + fn write_export( dir: &std::path::Path, name: &str, diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 679c3df..b789ad8 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -37,7 +37,13 @@ pub struct AppState { /// dangerous. Deliberately re-decrypted rather than cached in plaintext: /// nothing here holds a decrypted secret in memory for longer than one /// command's execution. - pub pending_settings_import: Arc>>, + /// + /// Also pins a hash of the file's ciphertext at preview time, so + /// `apply_settings_import` can refuse to proceed if the file on disk + /// changed underneath the pending import — otherwise confirming a + /// preview is not actually binding on what gets applied. + pub pending_settings_import: + Arc>>, } // ───────────────────────────────────────────────────────────────────────────── diff --git a/app/src-tauri/src/models/settings_export.rs b/app/src-tauri/src/models/settings_export.rs index 550d163..cf6c5d4 100644 --- a/app/src-tauri/src/models/settings_export.rs +++ b/app/src-tauri/src/models/settings_export.rs @@ -27,7 +27,7 @@ use serde::{Deserialize, Serialize}; -use super::AppSettings; +use super::{AppSettings, ImageSource}; /// Bumped when the shape of [`SettingsExportPayload`] changes in a way that /// isn't just an additive, `#[serde(default)]`-covered field — e.g. if a @@ -60,13 +60,26 @@ pub struct ExportedSecrets { impl ExportedSecrets { pub fn is_empty(&self) -> bool { - self.claude_oauth_token.is_none() - && self.gateway_api_key.is_none() - && self.gateway_master_key.is_none() - && self.web_terminal_access_token.is_none() + let blank = |s: &Option| s.as_deref().is_none_or(|v| v.trim().is_empty()); + blank(&self.claude_oauth_token) + && blank(&self.gateway_api_key) + && blank(&self.gateway_master_key) + && blank(&self.web_terminal_access_token) } } +/// What `apply_settings_import` hands back: the settings that were actually +/// saved, plus a human-readable note for each keychain secret this import +/// carried but could not be restored. A keychain write failing partway +/// through must not read as unqualified success just because the settings +/// half of the import went through. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SettingsImportOutcome { + pub settings: AppSettings, + #[serde(default)] + pub secret_restore_warnings: Vec, +} + /// The full plaintext payload — this is what gets encrypted on export and /// what decryption recovers on import. Never written to disk unencrypted; /// see `storage::settings_crypto`. @@ -122,12 +135,49 @@ pub struct SettingsImportPreview { pub openai_compatible_base_url: Option, #[serde(default)] pub gateway_api_base: Option, + /// Whether the import sets a custom Docker image, and its name if so — + /// disclosed for the same reason as the base URLs above, and arguably + /// more sharply: this is the image *every* project container is created + /// from (`models::container_config::resolve_image_name`), so a crafted + /// export pointing it at an attacker-controlled image is a path to + /// running arbitrary code with whatever a project's containers are + /// allowed to reach (the Docker socket, an SSH key, project files) — + /// not merely a redirected API endpoint. + #[serde(default)] + pub image_source: ImageSource, + #[serde(default)] + pub custom_image_name: Option, +} + +/// A cap on how much of a decrypted, not-yet-trusted string gets echoed back +/// into a preview a user reads and a UI renders without truncation of its +/// own. Applied to every field above that carries free-form text straight +/// from the import file rather than a count or a boolean — a base URL or an +/// image name a hostile export author controls has had no validation done +/// on it yet at preview time, and nothing stops it from being pathological +/// (embedded control characters, or long enough to blow out the confirmation +/// dialog and push the security warnings below it off screen). +const MAX_PREVIEW_STRING_LEN: usize = 100; + +fn sanitize_for_preview(value: &str) -> String { + let cleaned: String = value.chars().filter(|c| !c.is_control()).collect(); + let trimmed = cleaned.trim(); + if trimmed.chars().count() > MAX_PREVIEW_STRING_LEN { + let truncated: String = trimmed.chars().take(MAX_PREVIEW_STRING_LEN).collect(); + format!("{}…", truncated) + } else { + trimmed.to_string() + } } impl SettingsImportPreview { pub fn from_payload(payload: &SettingsExportPayload) -> Self { let non_blank = |s: &Option| s.as_deref().is_some_and(|v| !v.trim().is_empty()); - let non_blank_value = |s: &Option| s.clone().filter(|v| !v.trim().is_empty()); + let sanitized_non_blank = |s: &Option| { + s.as_deref() + .map(sanitize_for_preview) + .filter(|v| !v.is_empty()) + }; Self { exported_at: payload.exported_at.clone(), app_version: payload.app_version.clone(), @@ -139,12 +189,14 @@ impl SettingsImportPreview { has_gateway_master_key: non_blank(&payload.secrets.gateway_master_key), has_web_terminal_access_token: non_blank(&payload.secrets.web_terminal_access_token), enables_web_terminal: payload.settings.web_terminal.enabled, - ollama_base_url: non_blank_value(&payload.settings.global_ollama.base_url), - llamacpp_base_url: non_blank_value(&payload.settings.global_llamacpp.base_url), - openai_compatible_base_url: non_blank_value( + ollama_base_url: sanitized_non_blank(&payload.settings.global_ollama.base_url), + llamacpp_base_url: sanitized_non_blank(&payload.settings.global_llamacpp.base_url), + openai_compatible_base_url: sanitized_non_blank( &payload.settings.global_openai_compatible.base_url, ), - gateway_api_base: non_blank_value(&payload.settings.gateway.api_base), + gateway_api_base: sanitized_non_blank(&payload.settings.gateway.api_base), + image_source: payload.settings.image_source.clone(), + custom_image_name: sanitized_non_blank(&payload.settings.custom_image_name), } } } @@ -155,17 +207,19 @@ mod tests { use crate::models::AppSettings; fn payload_with(secrets: ExportedSecrets) -> SettingsExportPayload { - let mut settings = AppSettings::default(); - settings.global_custom_env_vars = vec![ - crate::models::EnvVar { - key: "A".to_string(), - value: "1".to_string(), - }, - crate::models::EnvVar { - key: "B".to_string(), - value: "2".to_string(), - }, - ]; + let settings = AppSettings { + global_custom_env_vars: vec![ + crate::models::EnvVar { + key: "A".to_string(), + value: "1".to_string(), + }, + crate::models::EnvVar { + key: "B".to_string(), + value: "2".to_string(), + }, + ], + ..AppSettings::default() + }; SettingsExportPayload { format_version: SETTINGS_EXPORT_FORMAT_VERSION, exported_at: "2026-08-27T00:00:00Z".to_string(), @@ -264,4 +318,49 @@ mod tests { } .is_empty()); } + + #[test] + fn a_secrets_bundle_holding_only_whitespace_still_reports_itself_as_empty() { + // Matches the "blank counts as absent" rule every other consumer of + // these fields applies (`has_claude_oauth_token` and friends above) — + // a keychain entry that exists but holds only whitespace carries + // nothing usable, so the export-time "nothing to export" log line + // must still fire for it. + assert!(ExportedSecrets { + claude_oauth_token: Some(" ".to_string()), + ..Default::default() + } + .is_empty()); + } + + #[test] + fn a_custom_docker_image_is_surfaced() { + let mut payload = payload_with(ExportedSecrets::default()); + payload.settings.image_source = crate::models::ImageSource::Custom; + payload.settings.custom_image_name = Some("ghcr.io/attacker/triple-c:latest".to_string()); + + let preview = SettingsImportPreview::from_payload(&payload); + assert_eq!(preview.image_source, crate::models::ImageSource::Custom); + assert_eq!( + preview.custom_image_name.as_deref(), + Some("ghcr.io/attacker/triple-c:latest") + ); + } + + #[test] + fn preview_strings_are_stripped_of_control_characters_and_capped_in_length() { + let mut payload = payload_with(ExportedSecrets::default()); + payload.settings.global_ollama.base_url = + Some(format!("http://example.test/{}\u{0007}bell", "x".repeat(200))); + + let preview = SettingsImportPreview::from_payload(&payload); + let shown = preview.ollama_base_url.expect("non-blank base url"); + assert!(!shown.contains('\u{0007}'), "control character leaked into the preview"); + // +1 for the trailing ellipsis appended when truncated. + assert!( + shown.chars().count() <= MAX_PREVIEW_STRING_LEN + 1, + "preview string was not capped: {} chars", + shown.chars().count() + ); + } } diff --git a/app/src-tauri/src/storage/settings_crypto.rs b/app/src-tauri/src/storage/settings_crypto.rs index 37d40df..385edf1 100644 --- a/app/src-tauri/src/storage/settings_crypto.rs +++ b/app/src-tauri/src/storage/settings_crypto.rs @@ -19,8 +19,14 @@ //! GCM's requirement that a (key, nonce) pair never repeat. Both hold //! because a fresh random value is drawn for each, on every call to //! [`encrypt`]. +//! +//! The whole header (magic + salt + nonce) is passed to AES-GCM as +//! associated data, not just placed alongside the ciphertext — free to do, +//! and it makes tampering with any header byte fail the same authentication +//! check the ciphertext gets, by construction rather than as a side effect +//! of the salt/nonce also feeding key derivation and the cipher. -use aes_gcm::aead::{Aead, KeyInit}; +use aes_gcm::aead::{Aead, KeyInit, Payload}; use aes_gcm::{Aes256Gcm, Nonce}; use argon2::{Algorithm, Argon2, Params, Version}; use rand::RngCore; @@ -70,16 +76,23 @@ pub fn encrypt(plaintext: &[u8], password: &str) -> Result, String> { rand::rng().fill_bytes(&mut nonce_bytes); let nonce = Nonce::from_slice(&nonce_bytes); + let mut header = Vec::with_capacity(HEADER_LEN); + header.extend_from_slice(MAGIC); + header.extend_from_slice(&salt); + header.extend_from_slice(&nonce_bytes); + let cipher = Aes256Gcm::new_from_slice(&*key) .map_err(|e| format!("Failed to initialize cipher: {}", e))?; + // The header (magic + salt + nonce) is authenticated as associated data + // even though none of it is secret: it costs nothing extra here, and it + // means tampering with any header byte is caught by the same tag check + // that already covers the ciphertext, by construction rather than as a + // side effect of the header also feeding key/nonce derivation. let ciphertext = cipher - .encrypt(nonce, plaintext) + .encrypt(nonce, Payload { msg: plaintext, aad: &header }) .map_err(|e| format!("Encryption failed: {}", e))?; - let mut out = Vec::with_capacity(HEADER_LEN + ciphertext.len()); - out.extend_from_slice(MAGIC); - out.extend_from_slice(&salt); - out.extend_from_slice(&nonce_bytes); + let mut out = header; out.extend_from_slice(&ciphertext); Ok(out) } @@ -102,6 +115,7 @@ pub fn decrypt(data: &[u8], password: &str) -> Result>, String if &data[..MAGIC.len()] != MAGIC { return Err("This does not look like a Triple-C settings export (unrecognized file).".to_string()); } + let header = &data[..HEADER_LEN]; let salt = &data[MAGIC.len()..MAGIC.len() + SALT_LEN]; let nonce_bytes = &data[MAGIC.len() + SALT_LEN..HEADER_LEN]; let ciphertext = &data[HEADER_LEN..]; @@ -111,7 +125,7 @@ pub fn decrypt(data: &[u8], password: &str) -> Result>, String .map_err(|e| format!("Failed to initialize cipher: {}", e))?; let nonce = Nonce::from_slice(nonce_bytes); cipher - .decrypt(nonce, ciphertext) + .decrypt(nonce, Payload { msg: ciphertext, aad: header }) .map(Zeroizing::new) .map_err(|_| "Wrong password, or the file is corrupted.".to_string()) } diff --git a/app/src/components/settings/ImportSettingsModal.test.tsx b/app/src/components/settings/ImportSettingsModal.test.tsx index 2d9807e..9a56f02 100644 --- a/app/src/components/settings/ImportSettingsModal.test.tsx +++ b/app/src/components/settings/ImportSettingsModal.test.tsx @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import ImportSettingsModal from "./ImportSettingsModal"; -import type { AppSettings, SettingsImportPreview } from "../../lib/types"; +import type { AppSettings, SettingsImportOutcome, SettingsImportPreview } from "../../lib/types"; const previewSettingsImport = vi.fn(); const applySettingsImport = vi.fn(); @@ -30,8 +30,14 @@ const samplePreview: SettingsImportPreview = { llamacpp_base_url: null, openai_compatible_base_url: null, gateway_api_base: null, + image_source: "registry", + custom_image_name: null, }; +function outcome(settings: AppSettings, secretRestoreWarnings: string[] = []): SettingsImportOutcome { + return { settings, secret_restore_warnings: secretRestoreWarnings }; +} + describe("ImportSettingsModal", () => { it("keeps 'Choose file' disabled until a password is entered", () => { render(); @@ -43,7 +49,7 @@ describe("ImportSettingsModal", () => { it("shows the preview and confirms with the same password used to open it", async () => { previewSettingsImport.mockResolvedValue(samplePreview); - applySettingsImport.mockResolvedValue({} as AppSettings); + applySettingsImport.mockResolvedValue(outcome({} as AppSettings)); const onImported = vi.fn(); render(); @@ -70,6 +76,38 @@ describe("ImportSettingsModal", () => { expect(await screen.findByText(/enables the remote web terminal/i)).toBeInTheDocument(); }); + it("warns about a custom Docker image every time, not just on change", async () => { + previewSettingsImport.mockResolvedValue({ + ...samplePreview, + image_source: "custom", + custom_image_name: "ghcr.io/attacker/triple-c:latest", + }); + render(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "hunter2" } }); + fireEvent.click(screen.getByRole("button", { name: /choose file/i })); + + expect( + await screen.findByText(/custom docker image: ghcr\.io\/attacker\/triple-c:latest/i), + ).toBeInTheDocument(); + }); + + it("shows a secret-restore warning alongside success rather than hiding it", async () => { + previewSettingsImport.mockResolvedValue(samplePreview); + applySettingsImport.mockResolvedValue( + outcome({} as AppSettings, ["Could not restore the gateway master key: keychain locked"]), + ); + render(); + + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "hunter2" } }); + fireEvent.click(screen.getByRole("button", { name: /choose file/i })); + await screen.findByText(/2 global custom env vars/i); + + fireEvent.click(screen.getByRole("button", { name: /^import$/i })); + expect(await screen.findByText(/settings imported/i)).toBeInTheDocument(); + expect(await screen.findByText(/could not restore the gateway master key/i)).toBeInTheDocument(); + }); + it("closes quietly when the file picker is dismissed", async () => { previewSettingsImport.mockResolvedValue(null); const onClose = vi.fn(); diff --git a/app/src/components/settings/ImportSettingsModal.tsx b/app/src/components/settings/ImportSettingsModal.tsx index c0b79f4..c914c3b 100644 --- a/app/src/components/settings/ImportSettingsModal.tsx +++ b/app/src/components/settings/ImportSettingsModal.tsx @@ -27,6 +27,7 @@ export default function ImportSettingsModal({ onClose, onImported }: Props) { const [error, setError] = useState(null); const [preview, setPreview] = useState(null); const [applied, setApplied] = useState(false); + const [secretWarnings, setSecretWarnings] = useState([]); const handleChooseFile = async () => { setError(null); @@ -46,9 +47,10 @@ export default function ImportSettingsModal({ onClose, onImported }: Props) { setError(null); setBusy(true); try { - const settings = await applySettingsImport(password); + const outcome = await applySettingsImport(password); setApplied(true); - onImported(settings); + setSecretWarnings(outcome.secret_restore_warnings); + onImported(outcome.settings); } catch (e) { setError(String(e)); } finally { @@ -99,29 +101,46 @@ export default function ImportSettingsModal({ onClose, onImported }: Props) { } > {applied ? ( -

    Settings imported.

    +
    +

    Settings imported.

    + {secretWarnings.map((warning) => ( +

    + {warning} +

    + ))} +
    ) : preview ? (

    Exported {new Date(preview.exported_at).toLocaleString()} from Triple-C{" "} {preview.app_version}.

    -
    -

    This will replace:

    -
      - {describeImport(preview).map((item) => ( -
    • {item}
      • - ))} -
    -
    + {/* Warnings render before the replace list, deliberately: the list + * below can run long, and the one thing here that most needs to + * stay above the fold while scrolling is "this turns on a + * network-listening service" or "this runs a different image" — + * not a bullet buried among ordinary settings. */} {describeImportWarnings(preview).map((warning) => (

    {warning}

    ))} +
    +

    This will replace:

    +
      + {describeImport(preview).map((item) => ( +
    • + {item} +
      • + ))} +
    +
    {error &&

    {error}

    }
    ) : ( diff --git a/app/src/lib/settingsImportPreview.test.ts b/app/src/lib/settingsImportPreview.test.ts index 8124888..aa94e9d 100644 --- a/app/src/lib/settingsImportPreview.test.ts +++ b/app/src/lib/settingsImportPreview.test.ts @@ -18,6 +18,8 @@ function preview(overrides: Partial = {}): SettingsImport llamacpp_base_url: null, openai_compatible_base_url: null, gateway_api_base: null, + image_source: "registry", + custom_image_name: null, ...overrides, }; } @@ -76,6 +78,18 @@ describe("describeImport", () => { expect(items.some((i) => i.includes("llama.cpp"))).toBe(false); expect(items.some((i) => i.includes("OpenAI-compatible"))).toBe(false); }); + + it("names a custom Docker image when set, falling back to a placeholder if unnamed", () => { + expect( + describeImport(preview({ image_source: "custom", custom_image_name: "ghcr.io/me/triple-c" })), + ).toContain("Docker image: ghcr.io/me/triple-c"); + expect(describeImport(preview({ image_source: "custom", custom_image_name: null }))).toContain( + "Docker image: (no image name set)", + ); + expect(describeImport(preview({ image_source: "registry" })).some((i) => i.includes("Docker image"))).toBe( + false, + ); + }); }); describe("describeImportWarnings", () => { @@ -101,4 +115,11 @@ describe("describeImportWarnings", () => { "Includes a web terminal access token that will activate the next time the web terminal is turned on.", ]); }); + + it("warns about a custom Docker image every time, not only when it changes", () => { + expect( + describeImportWarnings(preview({ image_source: "custom", custom_image_name: "evil:latest" })), + ).toEqual(["Runs every project container from a custom Docker image: evil:latest."]); + expect(describeImportWarnings(preview({ image_source: "registry" }))).toEqual([]); + }); }); diff --git a/app/src/lib/settingsImportPreview.ts b/app/src/lib/settingsImportPreview.ts index 6e9c099..c8e05e6 100644 --- a/app/src/lib/settingsImportPreview.ts +++ b/app/src/lib/settingsImportPreview.ts @@ -25,6 +25,9 @@ export function describeImport(preview: SettingsImportPreview): string[] { items.push(`OpenAI-compatible server: ${preview.openai_compatible_base_url}`); } if (preview.gateway_api_base) items.push(`Gateway upstream: ${preview.gateway_api_base}`); + if (preview.image_source === "custom") { + items.push(`Docker image: ${preview.custom_image_name ?? "(no image name set)"}`); + } return items; } @@ -41,6 +44,10 @@ export function describeImport(preview: SettingsImportPreview): string[] { * would silently become live the next time someone flips the terminal on * through the UI, with no import-time signal that it wasn't freshly * generated. + * + * A custom Docker image gets a warning every time, not just on change: it's + * the image every project container is created from, so it's worth calling + * out regardless of what was configured before the import. */ export function describeImportWarnings(preview: SettingsImportPreview): string[] { const warnings: string[] = []; @@ -51,5 +58,10 @@ export function describeImportWarnings(preview: SettingsImportPreview): string[] "Includes a web terminal access token that will activate the next time the web terminal is turned on.", ); } + if (preview.image_source === "custom") { + warnings.push( + `Runs every project container from a custom Docker image: ${preview.custom_image_name ?? "(no image name set)"}.`, + ); + } return warnings; } diff --git a/app/src/lib/tauri-commands.ts b/app/src/lib/tauri-commands.ts index dbceeec..c9a2b1f 100644 --- a/app/src/lib/tauri-commands.ts +++ b/app/src/lib/tauri-commands.ts @@ -1,5 +1,5 @@ import { invoke } from "@tauri-apps/api/core"; -import type { Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ContainerInfo, AppSettings, SettingsImportPreview, UpdateInfo, ImageUpdateInfo, FileEntry, FileContents, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, BrowserViewPopoutState, BrowserPageState, PlaywrightDetection, BrowserSetupOutcome, BrowserInstallTarget, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo, UploadOutcome } from "./types"; +import type { Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ContainerInfo, AppSettings, SettingsImportPreview, SettingsImportOutcome, UpdateInfo, ImageUpdateInfo, FileEntry, FileContents, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, BrowserViewPopoutState, BrowserPageState, PlaywrightDetection, BrowserSetupOutcome, BrowserInstallTarget, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo, UploadOutcome } from "./types"; // Docker export const checkDocker = () => invoke("check_docker"); @@ -49,7 +49,7 @@ export const exportSettings = (password: string) => export const previewSettingsImport = (password: string) => invoke("preview_settings_import", { password }); export const applySettingsImport = (password: string) => - invoke("apply_settings_import", { password }); + invoke("apply_settings_import", { password }); // AWS export const awsSsoRefresh = (projectId: string) => diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index 3ddb549..ae2250c 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -316,6 +316,20 @@ export interface SettingsImportPreview { llamacpp_base_url: string | null; openai_compatible_base_url: string | null; gateway_api_base: string | null; + /** Whether the import sets a custom Docker image, and its name if so — + * this is the image every project container is created from, so worth + * more attention than an ordinary setting. */ + image_source: ImageSource; + custom_image_name: string | null; +} + +/** What `apply_settings_import` returns: the settings that were actually + * saved, plus a note for each keychain secret the import carried but could + * not be restored (a partial keychain failure must not read as unqualified + * success just because the settings half went through). */ +export interface SettingsImportOutcome { + settings: AppSettings; + secret_restore_warnings: string[]; } /** What `inspect_ca_cert_path` reports about a corporate CA path. Errors ride