fix: uniform decrypt error and clear stored credentials on logout (#11632)

* fix: uniform decrypt error and clear stored credentials on logout

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kv72vdCDggCZEjJSmNCwnX

* fix: also clear mcp-oauth-callback on logout, cover the new keys in the logout test

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kv72vdCDggCZEjJSmNCwnX

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

backend/windmill-common/src/variables.rs

@@ -14,7 +14,7 @@ use crate::utils::WarnAfterExt;
 use crate::worker::Connection;
 use crate::{worker::WORKER_GROUP, BASE_URL, DB};
 use chrono::{SecondsFormat, Utc};
-use magic_crypt::{MagicCrypt256, MagicCryptError, MagicCryptTrait};
+use magic_crypt::{MagicCrypt256, MagicCryptTrait};
 use quick_cache::sync::Cache;
 use serde::{Deserialize, Serialize};
 
@@ -375,13 +375,15 @@ pub fn encrypt(mc: &MagicCrypt256, value: &str) -> String {
     mc.encrypt_str_to_base64(value)
 }
 
+const DECRYPT_FAILED: &str =
+    "Could not decrypt value. The value may have been encrypted with a different key.";
+
+/// Every failure returns the same message so responses carry no signal about why decryption
+/// failed (padding, base64, UTF-8); the cause is only logged.
 pub fn decrypt(mc: &MagicCrypt256, value: String) -> error::Result<String> {
-    mc.decrypt_base64_to_string(value).map_err(|e| match e {
-        MagicCryptError::DecryptError(_) => error::Error::internal_err(
-            "Could not decrypt value. The value may have been encrypted with a different key."
-                .to_string(),
-        ),
-        _ => error::Error::internal_err(e.to_string()),
+    mc.decrypt_base64_to_string(value).map_err(|e| {
+        tracing::warn!("decryption failed: {e}");
+        error::Error::internal_err(DECRYPT_FAILED.to_string())
     })
 }
 
@@ -746,7 +748,31 @@ pub async fn get_variable_or_self_as<T: Authable + Sync>(
 
 #[cfg(test)]
 mod tests {
-    use super::{can_bind_as_prologue_const, escape_js_single_quoted, is_valid_js_identifier};
+    use super::{
+        can_bind_as_prologue_const, decrypt, escape_js_single_quoted, is_valid_js_identifier,
+        DECRYPT_FAILED,
+    };
+    use crate::error::Error;
+    use magic_crypt::MagicCryptTrait;
+
+    #[test]
+    fn decrypt_failures_all_return_the_same_message() {
+        let mc = magic_crypt::new_magic_crypt!("workspace-key", 256);
+        let other = magic_crypt::new_magic_crypt!("another-key", 256);
+        let failures = [
+            "not base64 at all!".to_string(),
+            mc.encrypt_bytes_to_base64(&[0xff, 0xfe, 0xfd]),
+            other.encrypt_str_to_base64("secret"),
+        ];
+        for value in failures {
+            match decrypt(&mc, value.clone()) {
+                Err(Error::InternalErrLoc { message, .. }) => {
+                    assert_eq!(message, DECRYPT_FAILED, "for {value:?}")
+                }
+                other => panic!("expected the uniform decrypt error for {value:?}, got {other:?}"),
+            }
+        }
+    }
 
     #[test]
     fn is_valid_js_identifier_gates_prologue_injection() {

frontend/src/lib/logout.test.ts

@@ -12,9 +12,25 @@ import { clearUser } from './logout'
 describe('clearUser', () => {
 	beforeEach(() => {
 		sessionStorage.clear()
+		localStorage.clear()
 		vi.clearAllMocks()
 	})
 
+	it('drops credential-bearing localStorage entries and keeps preferences', async () => {
+		const credentialKeys = ['oauth-callback', 'mcp-oauth-callback', 'test_dev_token']
+		for (const key of credentialKeys) {
+			localStorage.setItem(key, 'a-token')
+		}
+		localStorage.setItem('dark-mode', 'true')
+
+		await clearUser()
+
+		for (const key of credentialKeys) {
+			expect(localStorage.getItem(key)).toBeNull()
+		}
+		expect(localStorage.getItem('dark-mode')).toBe('true')
+	})
+
 	// Logging out while impersonating must not leave the impersonator's own token behind.
 	it('drops the impersonation token backup', async () => {
 		sessionStorage.setItem('pre_impersonation_token', 'a-token')

frontend/src/lib/logout.ts

@@ -6,15 +6,19 @@ import { clearStores } from './storeUtils'
 // Note: logout and logoutWithRedirect have been moved to logoutKit.ts
 // as they depend on SvelteKit navigation
 
-// The impersonation backup holds the impersonator's own auth token, so it must go on
-// logout too, not only when impersonation is ended from the banner — otherwise a usable
-// token stays readable for the lifetime of the tab.
-function clearImpersonationFromStorage(): void {
+// Every browser-storage entry that can hold a credential must go on logout. The impersonation
+// backup is the impersonator's own auth token; `oauth-callback` and `mcp-oauth-callback` carry a
+// connected account's token response when the OAuth popup had no opener to post it to;
+// `test_dev_token` is the session token of the /test_dev pages.
+function clearCredentialsFromStorage(): void {
 	try {
 		sessionStorage.removeItem('pre_impersonation_token')
 		sessionStorage.removeItem('pre_impersonation_email')
+		localStorage.removeItem('oauth-callback')
+		localStorage.removeItem('mcp-oauth-callback')
+		localStorage.removeItem('test_dev_token')
 	} catch (e) {
-		console.error('error interacting with session storage', e)
+		console.error('error interacting with browser storage', e)
 	}
 }
 
@@ -23,7 +27,7 @@ export async function clearUser() {
 		noteSessionEmail(undefined)
 		accountSetup.reset()
 		clearStores()
-		clearImpersonationFromStorage()
+		clearCredentialsFromStorage()
 		await UserService.logout()
 	} catch (error) {}
 }