diff --git a/CHANGELOG.md b/CHANGELOG.md index 6413c39..4a8d16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,22 @@ popover, not for the person who wrote the commit. ## [Unreleased] +## [1.8.9] - 2026-09-26 + +### Security +- **Attaching a file to an encrypted stash no longer risks its key.** On an account with + encryption on, syncing a stash right after adding an attachment to it could overwrite that + file's encryption key with a copy of its plain name and size - which destroyed the key. + The file then looked broken on every other device, and only the one that uploaded it still + had a readable copy. Cloud sync no longer sends an attachment's name, size or type on an + ordinary sync; only the upload itself sets them +- **Files attached before encryption was turned on are re-encrypted automatically.** They + used to stay readable on the server indefinitely, because nothing re-uploaded them once + encryption was on. A few are now re-encrypted in the background after each sync until + none are left +- An uploaded file's type is no longer sent to cloud storage in the clear on an encrypted + account + ## [1.8.8] - 2026-09-25 ### Changed diff --git a/package.json b/package.json index e86bb4c..b6b2fdc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "stashpad", - "version": "1.8.8", + "version": "1.8.9", "description": "The staging area for your AI context.", "author": { "name": "Nico Wiedemann", diff --git a/screenshots/mock-backend.ts b/screenshots/mock-backend.ts index 090f6c5..e76fc8b 100644 --- a/screenshots/mock-backend.ts +++ b/screenshots/mock-backend.ts @@ -185,6 +185,8 @@ export function installMockBackend(overrides: Partial = {}): void { }; case 'upload_attachment_to_cloud': return false; + case 'convert_attachments_to_encrypted': + return { converted: 0, remaining: 0, unrecoverable: 0 }; case 'check_screen_recording_permission': return true; case 'check_apple_intelligence_available': diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index d2f771b..9021020 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -5858,7 +5858,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stashpad" -version = "1.8.8" +version = "1.8.9" dependencies = [ "active-win-pos-rs", "aes-gcm", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 4edfa68..2f605a8 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "stashpad" -version = "1.8.8" +version = "1.8.9" description = "The staging area for your AI context." authors = ["Nico Wiedemann "] repository = "https://github.com/EarMaster/stashpad" diff --git a/src-tauri/src/e2ee_session.rs b/src-tauri/src/e2ee_session.rs index b8ef86e..902a006 100644 --- a/src-tauri/src/e2ee_session.rs +++ b/src-tauri/src/e2ee_session.rs @@ -192,9 +192,15 @@ pub fn seal_stash_payload( for stash in stashes.iter_mut() { let id = stash["id"].as_str().unwrap_or_default().to_string(); - // A tombstone carries no text, and the server stores none for one either. Sealing - // an empty string would only add a blob nobody reads. + strip_attachment_details(stash); + + // A tombstone carries no text: the server stores none for one, and sealing an empty + // string would only add a blob nobody reads. So the text is removed here rather + // than skipped - the local row keeps it after a delete, and skipping sealing used + // to send it along in the clear. if stash["deleted"].as_bool().unwrap_or(false) { + stash["content"] = serde_json::json!(""); + stash["enhancedContent"] = serde_json::Value::Null; continue; } @@ -213,6 +219,25 @@ pub fn seal_stash_payload( Ok(()) } +/// Reduce each attachment in a pushed stash to its id and deleted flag. +/// +/// Its name, type and size already travel sealed, in the descriptor `seal_attachment` +/// builds and the upload stores - together with the file's own key. This installation keeps +/// no copy of that descriptor, so it cannot send it again, and sending the local plaintext +/// instead is what went wrong: the server wrote it over the descriptor, the key was lost, +/// and the file name was back on the server in the clear. The id is all a push needs to +/// file an attachment under its stash, and the flag to say it was removed. +fn strip_attachment_details(stash: &mut serde_json::Value) { + let Some(attachments) = stash["attachments"].as_array_mut() else { + return; + }; + for attachment in attachments.iter_mut() { + if let Some(fields) = attachment.as_object_mut() { + fields.retain(|name, _| name == "id" || name == "deleted"); + } + } +} + /// Open a stash-sync response on its way in. /// /// Records that cannot be opened are removed from `synced` and returned, so the caller can @@ -273,7 +298,11 @@ pub fn seal_context_payload( for ctx in contexts.iter_mut() { let id = ctx["id"].as_str().unwrap_or_default().to_string(); + // As for a stash: a deleted context leaves without its name, description or rules. if ctx["deleted"].as_bool().unwrap_or(false) { + ctx["name"] = serde_json::json!(""); + ctx["description"] = serde_json::Value::Null; + ctx["rules"] = serde_json::json!([]); continue; } @@ -733,6 +762,40 @@ mod tests { }); } + /// An attachment's plaintext name, type and size must not leave with the push: they + /// travel sealed in the descriptor, and the server once wrote this copy over it. + #[test] + fn a_push_carries_no_attachment_details() { + with_key(|| { + let mut payload = stash_payload(); + payload["stashes"][0]["attachments"] = serde_json::json!([{ + "id": "44444444-4444-4444-8444-444444444444", + "fileName": "Q3-layoffs.xlsx", + "fileSize": 9001, + "mimeType": "application/vnd.ms-excel", + "syntax": null + }]); + seal_stash_payload(&mut payload, USER).expect("seal"); + + assert_eq!( + payload["stashes"][0]["attachments"], + serde_json::json!([{ "id": "44444444-4444-4444-8444-444444444444" }]) + ); + assert!(!payload.to_string().contains("Q3-layoffs")); + }); + } + + #[test] + fn without_a_key_attachment_details_are_sent_as_before() { + let _guard = lock_or_recover(&TEST_LOCK); + clear_content_key(); + let mut payload = stash_payload(); + payload["stashes"][0]["attachments"] = + serde_json::json!([{ "id": "a", "fileName": "shot.png", "fileSize": 5 }]); + seal_stash_payload(&mut payload, USER).expect("seal"); + assert_eq!(payload["stashes"][0]["attachments"][0]["fileName"], "shot.png"); + } + #[test] fn without_a_key_nothing_is_touched() { let _guard = lock_or_recover(&TEST_LOCK); @@ -743,15 +806,37 @@ mod tests { assert!(payload["cryptoVersion"].is_null()); } - /// A tombstone carries no text and the server stores none for one either. + /// A tombstone carries no text, and the local row still holds it after a delete - so it + /// has to be removed on the way out, not merely left unsealed. #[test] - fn a_delete_is_not_sealed() { + fn a_delete_leaves_without_its_text() { with_key(|| { let mut payload = stash_payload(); payload["stashes"][0]["deleted"] = serde_json::json!(true); - payload["stashes"][0]["content"] = serde_json::json!(""); seal_stash_payload(&mut payload, USER).expect("seal"); assert_eq!(payload["stashes"][0]["content"], ""); + assert!(payload["stashes"][0]["enhancedContent"].is_null()); + assert!(!payload.to_string().contains("the original text")); + }); + } + + #[test] + fn a_deleted_context_leaves_without_its_name_or_rules() { + with_key(|| { + let mut payload = serde_json::json!({ + "contexts": [{ + "id": "55555555-5555-4555-8555-555555555555", + "name": "Steuer 2026", + "description": "Belege", + "rules": [{ "ruleType": "process", "value": "excel.exe" }], + "deleted": true + }] + }); + seal_context_payload(&mut payload, USER).expect("seal"); + let ctx = &payload["contexts"][0]; + assert_eq!(ctx["name"], ""); + assert!(ctx["description"].is_null()); + assert_eq!(ctx["rules"], serde_json::json!([])); }); } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 09c72f0..5040944 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -555,6 +555,7 @@ pub fn run() { sync::sync_stashes_api, sync::sync_contexts_api, sync::upload_attachment_to_cloud, + sync::convert_attachments_to_encrypted, sync::download_attachment_from_cloud, sync::connect_websocket, sync::disconnect_websocket, diff --git a/src-tauri/src/sync.rs b/src-tauri/src/sync.rs index d4e07b2..87900fb 100644 --- a/src-tauri/src/sync.rs +++ b/src-tauri/src/sync.rs @@ -568,6 +568,8 @@ pub async fn upload_attachment_to_cloud( &user_id, )?; + let is_sealed = sealed.is_some(); + // For an encrypted account the name, type and syntax travel inside the sealed blob, so // the columns that used to hold them carry nothing. let (file_content, declared_name, declared_size, declared_mime, declared_syntax) = match sealed @@ -625,7 +627,10 @@ pub async fn upload_attachment_to_cloud( // 3. PUT file to R2 let put_resp = client .put(upload_url) - .header("Content-Type", attachment.mime_type.unwrap_or_else(|| "application/octet-stream".to_string())) + .header( + "Content-Type", + upload_content_type(is_sealed, attachment.mime_type.as_deref()), + ) .body(file_content) .send() .await @@ -1231,3 +1236,300 @@ mod tests { assert!(!snippet.ends_with('…')); } } + +// --------------------------------------------------------------------------------------- +// Re-encrypting attachments written before encryption was switched on +// --------------------------------------------------------------------------------------- + +/// Set once this session has done all the attachment conversion it can: everything is +/// sealed, or what is left is nothing this installation can fix. Saves a listing request - +/// and a storage check per file on the server - on every later sync. +static ATTACHMENTS_SETTLED: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(false); + +/// Most attachments re-encrypted in one sync cycle. +/// +/// Each is a download or a disk read, an encryption and an upload, and a sync cycle should +/// stay short: the rest are picked up by the next one, and the one after. +const MAX_CONVERSIONS_PER_CYCLE: usize = 5; + +/// What one pass achieved, for the log and the sync panel. +#[derive(Debug, Default, Clone, serde::Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AttachmentConversion { + pub converted: usize, + /// Plaintext attachments the account still has, as the server counted before this pass. + pub remaining: i64, + /// Sealed bytes whose key was lost, that this installation holds no copy of. + pub unrecoverable: usize, +} + +/// Where the readable bytes of an attachment come from. +#[derive(Debug, PartialEq)] +enum PlaintextSource { + /// This installation's own copy - always preferred, and the only way to rescue a file + /// whose key a sync push once overwrote. + Local(String), + /// The server's copy, which is plaintext: it was uploaded before encryption. + Server, + /// Sealed bytes with their key gone, and no local copy. Nothing here can open them. + Unrecoverable, +} + +fn plaintext_source(local_path: Option<&str>, damaged: bool) -> PlaintextSource { + match local_path.filter(|path| !path.trim().is_empty() && std::path::Path::new(path).exists()) + { + Some(path) => PlaintextSource::Local(path.to_string()), + None if damaged => PlaintextSource::Unrecoverable, + None => PlaintextSource::Server, + } +} + +/// The `Content-Type` an upload declares to the object store, which keeps it as metadata. +/// +/// Sealed bytes say nothing: the real type travels in the sealed descriptor, and declaring +/// it here would hand it to the bucket in the clear beside the ciphertext. +fn upload_content_type(sealed: bool, mime_type: Option<&str>) -> &str { + match (sealed, mime_type) { + (false, Some(mime)) => mime, + _ => "application/octet-stream", + } +} + +/// Re-encrypt the attachments this account still stores in plaintext. +/// +/// Records are converted by the sync sweep, but an uploaded file is never uploaded again, so +/// files attached before encryption was switched on stayed plaintext on the server - bytes, +/// name and type - indefinitely. This works through the server's list of them a few at a +/// time, on every sync, until none are left, and then records that on the account. +/// +/// Each file is proposed, uploaded to a staging key and confirmed, and the server swaps it +/// in only on confirmation (`cloud/src/routes/attachments.rs`), so an interrupted pass costs +/// nothing and the next one simply tries again. +#[tauri::command] +pub async fn convert_attachments_to_encrypted( + state: State<'_, Arc>, + settings_state: State<'_, Arc>, +) -> Result { + use std::sync::atomic::Ordering; + + let mut report = AttachmentConversion::default(); + if ATTACHMENTS_SETTLED.load(Ordering::Relaxed) || !crate::e2ee_session::is_unlocked() { + return Ok(report); + } + + let settings = settings_state.inner().clone(); + let user_id = { + let settings = settings.lock_settings(); + settings + .cloud_config + .as_ref() + .and_then(|c| c.user_id.clone()) + .unwrap_or_default() + }; + + // Files this pass could not convert stay in the list, so the next page starts past them. + let mut offset: i64 = 0; + let mut first_page = true; + loop { + let page = e2ee_get(&settings, &format!("/attachments/unsealed?offset={}", offset)).await?; + + if !page["encrypted"].as_bool().unwrap_or(false) { + ATTACHMENTS_SETTLED.store(true, Ordering::Relaxed); + return Ok(report); + } + if first_page { + report.remaining = page["remaining"].as_i64().unwrap_or(0); + first_page = false; + } + + if page["remaining"].as_i64().unwrap_or(0) == 0 { + if !page["done"].as_bool().unwrap_or(false) { + // The server verifies this rather than taking it on trust. + e2ee_post(&settings, "/e2ee/attachments-done", serde_json::json!({})).await?; + log::info!("[Attachment] Every attachment on this account is encrypted"); + } + ATTACHMENTS_SETTLED.store(true, Ordering::Relaxed); + return Ok(report); + } + + let items = page["attachments"].as_array().cloned().unwrap_or_default(); + if items.is_empty() { + break; + } + + let mut skipped: i64 = 0; + for item in &items { + if report.converted >= MAX_CONVERSIONS_PER_CYCLE { + return Ok(report); + } + let Some(id) = item["id"].as_str() else { + skipped += 1; + continue; + }; + + let local_path: Option = { + let db = state.lock_db(); + db.conn + .query_row( + "SELECT file_path FROM attachments WHERE id = ?1", + params![id], + |row| row.get(0), + ) + .optional() + .map_err(|e| e.to_string())? + }; + + let source = + plaintext_source(local_path.as_deref(), item["damaged"].as_bool().unwrap_or(false)); + if source == PlaintextSource::Unrecoverable { + report.unrecoverable += 1; + skipped += 1; + continue; + } + + match reencrypt_attachment(&settings, &user_id, item, source).await { + Ok(true) => { + report.converted += 1; + log::info!("[Attachment] Re-encrypted {}", id); + } + Ok(false) => skipped += 1, + Err(e) => { + // One file failing must not hold up the rest; it is retried next sync. + log::warn!("[Attachment] Could not re-encrypt {}: {}", id, e); + skipped += 1; + } + } + } + offset += skipped; + } + + // Every page read, and what is left is nothing this installation can do more about. + if report.converted == 0 { + ATTACHMENTS_SETTLED.store(true, Ordering::Relaxed); + } + if report.unrecoverable > 0 { + log::warn!( + "[Attachment] {} attachment(s) lost their key and are not on this device", + report.unrecoverable + ); + } + Ok(report) +} + +/// Seal one attachment and swap it in. `Ok(false)` when the server says it already is. +async fn reencrypt_attachment( + settings: &Arc, + user_id: &str, + item: &serde_json::Value, + source: PlaintextSource, +) -> Result { + let id = item["id"].as_str().ok_or("attachment without an id")?; + + let plaintext = match source { + PlaintextSource::Local(path) => tauri::async_runtime::spawn_blocking(move || fs::read(path)) + .await + .map_err(|e| format!("Attachment read task failed: {}", e))? + .map_err(|e| format!("Could not read the local copy: {}", e))?, + PlaintextSource::Server => { + let link = e2ee_get(settings, &format!("/attachments/{}", id)).await?; + let url = link["downloadUrl"] + .as_str() + .ok_or_else(|| "No download URL in response".to_string())?; + let response = transfer_client()? + .get(url) + .send() + .await + .map_err(|e| format!("Failed to download attachment: {}", e))?; + if !response.status().is_success() { + return Err(format!("Attachment download failed: {}", response.status()).into()); + } + let bytes = response + .bytes() + .await + .map_err(|e| format!("Failed to read attachment body: {}", e))? + .to_vec(); + // The listing said how long the plaintext is. Anything else is not the file + // this row describes, and sealing it would make that permanent. + if Some(bytes.len() as i64) != item["fileSize"].as_i64() { + return Err("the stored file is not the size its record says".into()); + } + bytes + } + PlaintextSource::Unrecoverable => return Ok(false), + }; + + let sealed = crate::e2ee_session::seal_attachment( + &plaintext, + item["fileName"].as_str().unwrap_or("attachment"), + item["mimeType"].as_str(), + item["syntax"].as_str(), + id, + user_id, + )? + .ok_or("this installation is locked")?; + + let proposal = e2ee_post( + settings, + &format!("/attachments/{}/rekey", id), + serde_json::json!({ "fileName": sealed.metadata, "fileSize": sealed.bytes.len() }), + ) + .await?; + if proposal["alreadySealed"].as_bool() == Some(true) { + return Ok(false); + } + let upload_url = proposal["uploadUrl"] + .as_str() + .ok_or_else(|| "No upload URL in response".to_string())?; + + let put = transfer_client()? + .put(upload_url) + .header("Content-Type", upload_content_type(true, None)) + .body(sealed.bytes) + .send() + .await + .map_err(|e| format!("Failed to upload the encrypted file: {}", e))?; + if !put.status().is_success() { + return Err(format!("Storage rejected the encrypted file: {}", put.status()).into()); + } + + e2ee_post( + settings, + &format!("/attachments/{}/rekey/confirm", id), + serde_json::json!({}), + ) + .await?; + Ok(true) +} + +#[cfg(test)] +mod conversion_tests { + use super::*; + + #[test] + fn a_local_copy_is_always_preferred() { + let file = std::env::temp_dir().join("stashpad-conversion-test.bin"); + fs::write(&file, b"x").unwrap(); + let path = file.to_string_lossy().to_string(); + + assert_eq!(plaintext_source(Some(&path), false), PlaintextSource::Local(path.clone())); + // Even for sealed bytes whose key was lost: the local copy is the rescue. + assert_eq!(plaintext_source(Some(&path), true), PlaintextSource::Local(path.clone())); + fs::remove_file(&file).ok(); + } + + #[test] + fn without_a_local_copy_only_plaintext_can_come_from_the_server() { + assert_eq!(plaintext_source(None, false), PlaintextSource::Server); + assert_eq!(plaintext_source(Some(""), false), PlaintextSource::Server); + assert_eq!(plaintext_source(Some("/no/such/file"), false), PlaintextSource::Server); + assert_eq!(plaintext_source(None, true), PlaintextSource::Unrecoverable); + } + + #[test] + fn a_sealed_upload_tells_storage_nothing_about_its_type() { + assert_eq!(upload_content_type(true, Some("image/png")), "application/octet-stream"); + assert_eq!(upload_content_type(false, Some("image/png")), "image/png"); + assert_eq!(upload_content_type(false, None), "application/octet-stream"); + } +} diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index f51f177..891cede 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "../node_modules/@tauri-apps/cli/config.schema.json", "productName": "stashpad", - "version": "1.8.8", + "version": "1.8.9", "identifier": "org.stashpad", "build": { "frontendDist": "../dist", diff --git a/src/lib/services/__tests__/cloud-sync.test.ts b/src/lib/services/__tests__/cloud-sync.test.ts index 01eb1cb..7290f53 100644 --- a/src/lib/services/__tests__/cloud-sync.test.ts +++ b/src/lib/services/__tests__/cloud-sync.test.ts @@ -39,6 +39,7 @@ function createAdapter(overrides: Partial = {}) { importStashes: vi.fn().mockResolvedValue(undefined), importContexts: vi.fn().mockResolvedValue(undefined), uploadAttachmentToCloud: vi.fn().mockResolvedValue(false), + convertAttachmentsToEncrypted: vi.fn().mockResolvedValue({ converted: 0, remaining: 0, unrecoverable: 0 }), downloadAttachmentFromCloud: vi.fn().mockResolvedValue('/cache/file.png'), connectWebSocket: vi.fn().mockResolvedValue(undefined), disconnectWebSocket: vi.fn().mockResolvedValue(undefined), diff --git a/src/lib/services/cloud-sync.ts b/src/lib/services/cloud-sync.ts index d02f101..c25e34c 100644 --- a/src/lib/services/cloud-sync.ts +++ b/src/lib/services/cloud-sync.ts @@ -735,6 +735,21 @@ export class CloudSyncService { // marked as present locally and are not immediately pushed back up. const uploadedAttachments = await this.uploadPendingAttachments(localStashes); + // Files uploaded before encryption was switched on are still plaintext on the + // server; a few are re-encrypted per sync until none are left. Never allowed to + // fail the sync: the stashes it carried are fine either way, and the pass simply + // resumes next time. + try { + const conversion = await this.adapter.convertAttachmentsToEncrypted(); + if (conversion.converted > 0) { + console.info( + `[CloudSync] Re-encrypted ${conversion.converted} of ${conversion.remaining} older attachment(s)` + ); + } + } catch (e) { + console.warn('[CloudSync] Could not re-encrypt older attachments:', e); + } + // Update last sync timestamp if (this.settings?.cloudConfig && (stashResponse || contextResponse)) { this.settings.cloudConfig.lastSyncAt = diff --git a/src/lib/services/desktop-adapter.ts b/src/lib/services/desktop-adapter.ts index 26d9334..576be9c 100644 --- a/src/lib/services/desktop-adapter.ts +++ b/src/lib/services/desktop-adapter.ts @@ -12,7 +12,7 @@ // See the GNU Affero General Public License for more details. import { invoke } from '@tauri-apps/api/core'; -import type { IStorageService, StashItem, AppContext, Settings, FilePreviewData, Context, Attachment, CloudConfig, ExportSummary, ImportPreview, CloudUsage, StashPosition, LocalKeyStatus, E2eeStatus, E2eeEnableResult, E2eeConversionProgress, CreatedAccessKey } from '../types'; +import type { IStorageService, StashItem, AppContext, Settings, FilePreviewData, Context, Attachment, CloudConfig, ExportSummary, ImportPreview, CloudUsage, StashPosition, LocalKeyStatus, E2eeStatus, E2eeEnableResult, E2eeConversionProgress, CreatedAccessKey, AttachmentConversion } from '../types'; /** Called after any local write so cloud sync can be scheduled. */ type MutationListener = () => void; @@ -344,6 +344,10 @@ export class DesktopStorageAdapter implements IStorageService { return await invoke('upload_attachment_to_cloud', { attachmentId }); } + async convertAttachmentsToEncrypted(): Promise { + return await invoke('convert_attachments_to_encrypted'); + } + async getDeviceName(): Promise { return await invoke('get_device_name'); } diff --git a/src/lib/types.ts b/src/lib/types.ts index ae0d4c3..bd9f24f 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -148,6 +148,14 @@ export interface Settings { autoUpdateChecks?: boolean; } +/** What one pass of re-encrypting older attachments achieved. */ +export interface AttachmentConversion { + converted: number; + remaining: number; + /** Files whose key was lost and that this installation holds no copy of. */ + unrecoverable: number; +} + export interface IStorageService { saveStash(stash: StashItem, options?: { invertPosition?: boolean }): Promise; saveStashes(stashes: StashItem[]): Promise; @@ -262,6 +270,11 @@ export interface IStorageService { openSystemPromptFile(): Promise; /** Uploads the attachment's bytes. Resolves true only if bytes were actually sent. */ uploadAttachmentToCloud(attachmentId: string): Promise; + /** + * Re-encrypts a few of the attachments an encrypted account still stores in plaintext - + * files uploaded before encryption was switched on. A no-op once none are left. + */ + convertAttachmentsToEncrypted(): Promise; // Device passphrase - only ever anything but "notNeeded" on a machine with no OS // credential store, where a typed passphrase is the only real protection available.