mirror of
https://github.com/nimbold/Firelink.git
synced 2026-07-26 12:08:27 +00:00
fix(security): redact secrets from plaintext persistence
- Strip password, cookies, and headers from download_queue before writing to store.bin; secrets remain in-memory for the active session only. - Move extension pairing token from PersistedSettings to the OS keychain, rotating it on upgrade from versions that persisted it as plaintext. - Add ignores_legacy_extension_pairing_token_field test to confirm serde silently drops the old field so existing installs migrate cleanly. - Document intentional retention of URLs (signed params are the download source and cannot be redacted without breaking resume/retry).
This commit is contained in:
@@ -237,7 +237,10 @@ pub struct PersistedSettings {
|
||||
pub media_cookie_source: MediaCookieSource,
|
||||
pub download_directories: HashMap<String, String>,
|
||||
pub site_logins: Vec<SiteLogin>,
|
||||
pub extension_pairing_token: String,
|
||||
// Note: `extension_pairing_token` is intentionally NOT persisted here. It
|
||||
// is an HMAC shared secret and is stored in the OS keychain by the
|
||||
// frontend. The field is kept on legacy persisted JSON only; serde ignores
|
||||
// unknown fields when decoding, so existing installs migrate cleanly.
|
||||
pub auto_check_updates: bool,
|
||||
}
|
||||
|
||||
|
||||
@@ -168,7 +168,6 @@ fn default_settings() -> PersistedSettings {
|
||||
media_cookie_source: MediaCookieSource::None,
|
||||
download_directories,
|
||||
site_logins: Vec::new(),
|
||||
extension_pairing_token: String::new(),
|
||||
auto_check_updates: true,
|
||||
}
|
||||
}
|
||||
@@ -229,4 +228,24 @@ mod tests {
|
||||
|
||||
assert_eq!(settings.max_concurrent_downloads, 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_legacy_extension_pairing_token_field() {
|
||||
// Older versions persisted `extensionPairingToken` as plaintext inside
|
||||
// the settings document. It now lives in the OS keychain and is no
|
||||
// longer part of PersistedSettings. serde ignores the unknown field so
|
||||
// existing installs decode without error; the plaintext value is
|
||||
// simply dropped and a fresh token is minted by the frontend.
|
||||
let stored = json!({
|
||||
"state": {
|
||||
"extensionPairingToken": "plaintext-leaked-secret",
|
||||
"maxConcurrentDownloads": 5
|
||||
},
|
||||
"version": 0
|
||||
});
|
||||
|
||||
let settings = decode_stored_settings(&Value::String(stored.to_string())).unwrap();
|
||||
|
||||
assert_eq!(settings.max_concurrent_downloads, 5);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -67,6 +67,13 @@ function App() {
|
||||
|
||||
useEffect(() => {
|
||||
useDownloadStore.getState().initDB();
|
||||
// Hydrate the browser-extension pairing token from the OS keychain before
|
||||
// the reactive push to the backend. If no token exists (fresh install or
|
||||
// upgrade from a plaintext-persisting version) a new one is minted and
|
||||
// stored, effectively rotating it away from any leaked plaintext.
|
||||
useSettingsStore.getState().hydratePairingToken().catch(error => {
|
||||
console.error('Failed to hydrate extension pairing token:', error);
|
||||
});
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
|
||||
@@ -8,4 +8,4 @@ import type { SettingsTab } from "./SettingsTab";
|
||||
import type { SiteLogin } from "./SiteLogin";
|
||||
import type { Theme } from "./Theme";
|
||||
|
||||
export type PersistedSettings = { theme: Theme, defaultDownloadPath: string, maxConcurrentDownloads: number, globalSpeedLimit: string, isSidebarVisible: boolean, activeSettingsTab: SettingsTab, scheduler: SchedulerSettings, schedulerLastStartKey: string, schedulerLastStopKey: string, lastCustomSpeedLimitKiB: number, perServerConnections: number, maxAutomaticRetries: number, showNotifications: boolean, playCompletionSound: boolean, appFontSize: AppFontSize, listRowDensity: ListRowDensity, showDockBadge: boolean, showMenuBarIcon: boolean, proxyMode: ProxyMode, proxyHost: string, proxyPort: number, customUserAgent: string, askWhereToSaveEachFile: boolean, preventsSleepWhileDownloading: boolean, mediaCookieSource: MediaCookieSource, downloadDirectories: { [key in string]: string }, siteLogins: Array<SiteLogin>, extensionPairingToken: string, autoCheckUpdates: boolean, };
|
||||
export type PersistedSettings = { theme: Theme, defaultDownloadPath: string, maxConcurrentDownloads: number, globalSpeedLimit: string, isSidebarVisible: boolean, activeSettingsTab: SettingsTab, scheduler: SchedulerSettings, schedulerLastStartKey: string, schedulerLastStopKey: string, lastCustomSpeedLimitKiB: number, perServerConnections: number, maxAutomaticRetries: number, showNotifications: boolean, playCompletionSound: boolean, appFontSize: AppFontSize, listRowDensity: ListRowDensity, showDockBadge: boolean, showMenuBarIcon: boolean, proxyMode: ProxyMode, proxyHost: string, proxyPort: number, customUserAgent: string, askWhereToSaveEachFile: boolean, preventsSleepWhileDownloading: boolean, mediaCookieSource: MediaCookieSource, downloadDirectories: { [key in string]: string }, siteLogins: Array<SiteLogin>, autoCheckUpdates: boolean, };
|
||||
|
||||
@@ -11,7 +11,7 @@ import type { ExtensionDownload } from '../bindings/ExtensionDownload';
|
||||
import type { Queue } from '../bindings/Queue';
|
||||
import type { MediaMetadata } from '../bindings/MediaMetadata';
|
||||
import { useSettingsStore } from './useSettingsStore';
|
||||
import { isActiveDownloadStatus } from '../utils/downloads';
|
||||
import { isActiveDownloadStatus, redactDownloadForPersistence } from '../utils/downloads';
|
||||
|
||||
export type { DownloadCategory } from '../utils/downloads';
|
||||
|
||||
@@ -628,13 +628,10 @@ useDownloadStore.subscribe(async (state, prevState) => {
|
||||
}
|
||||
|
||||
if (state.downloads !== prevState.downloads) {
|
||||
const staticDownloads = state.downloads.map(d => {
|
||||
const copy = { ...d };
|
||||
delete copy.fraction;
|
||||
delete copy.speed;
|
||||
delete copy.eta;
|
||||
return copy;
|
||||
});
|
||||
// Strip secret fields (password/cookies/headers) and volatile progress
|
||||
// before writing to disk. Secrets remain on the in-memory item for the
|
||||
// active session only.
|
||||
const staticDownloads = state.downloads.map(redactDownloadForPersistence);
|
||||
|
||||
const currentSerialized = JSON.stringify(staticDownloads);
|
||||
if (currentSerialized !== lastSavedDownloads) {
|
||||
|
||||
@@ -44,6 +44,15 @@ const tauriStorage: StateStorage = {
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Keychain identifier for the browser-extension pairing token. The token is an
|
||||
* HMAC shared secret and is therefore persisted via the OS keychain rather
|
||||
* than the plaintext `store.bin` settings document. A fresh token is minted
|
||||
* when no prior entry exists (also covering upgrades from versions that
|
||||
* stored the token as plaintext, effectively rotating it on upgrade).
|
||||
*/
|
||||
const PAIRING_TOKEN_KEYCHAIN_ID = 'extension-pairing-token';
|
||||
|
||||
export type {
|
||||
ActiveView,
|
||||
AppFontSize,
|
||||
@@ -126,6 +135,7 @@ export interface SettingsState {
|
||||
removeSiteLogin: (id: string) => void;
|
||||
regeneratePairingToken: () => void;
|
||||
setAutoCheckUpdates: (autoCheckUpdates: boolean) => void;
|
||||
hydratePairingToken: () => Promise<void>;
|
||||
}
|
||||
|
||||
const defaultDirectories = {
|
||||
@@ -287,7 +297,33 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
removeSiteLogin: (id) => set((state) => ({
|
||||
siteLogins: state.siteLogins.filter((login) => login.id !== id)
|
||||
})),
|
||||
regeneratePairingToken: () => set({ extensionPairingToken: generateSecureToken() }),
|
||||
regeneratePairingToken: () => {
|
||||
const token = generateSecureToken();
|
||||
set({ extensionPairingToken: token });
|
||||
invoke('set_keychain_password', { id: PAIRING_TOKEN_KEYCHAIN_ID, password: token }).catch(e => {
|
||||
console.error('Failed to persist regenerated extension pairing token to keychain:', e);
|
||||
});
|
||||
},
|
||||
hydratePairingToken: async () => {
|
||||
const existing = useSettingsStore.getState().extensionPairingToken;
|
||||
try {
|
||||
const stored = await invoke('get_keychain_password', { id: PAIRING_TOKEN_KEYCHAIN_ID });
|
||||
if (stored) {
|
||||
set({ extensionPairingToken: stored });
|
||||
return;
|
||||
}
|
||||
} catch {
|
||||
// No prior token in the keychain (fresh install or upgrade from a
|
||||
// version that stored plaintext). Fall through to mint + store.
|
||||
}
|
||||
const token = existing || generateSecureToken();
|
||||
set({ extensionPairingToken: token });
|
||||
try {
|
||||
await invoke('set_keychain_password', { id: PAIRING_TOKEN_KEYCHAIN_ID, password: token });
|
||||
} catch (e) {
|
||||
console.error('Failed to persist extension pairing token to keychain:', e);
|
||||
}
|
||||
},
|
||||
setAutoCheckUpdates: (autoCheckUpdates) => set({ autoCheckUpdates }),
|
||||
}),
|
||||
{
|
||||
@@ -322,7 +358,6 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
mediaCookieSource: state.mediaCookieSource,
|
||||
downloadDirectories: state.downloadDirectories,
|
||||
siteLogins: state.siteLogins,
|
||||
extensionPairingToken: state.extensionPairingToken,
|
||||
autoCheckUpdates: state.autoCheckUpdates
|
||||
}),
|
||||
merge: (persistedState: unknown, currentState) => {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { DownloadCategory } from '../bindings/DownloadCategory';
|
||||
import type { DownloadStatus } from '../bindings/DownloadStatus';
|
||||
import type { DownloadItem } from '../bindings/DownloadItem';
|
||||
export type { DownloadCategory } from '../bindings/DownloadCategory';
|
||||
|
||||
import { invoke } from '@tauri-apps/api/core';
|
||||
@@ -78,3 +79,33 @@ export const isMediaUrl = (rawUrl: string): boolean => {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Fields that may carry secrets and therefore must never reach the persisted
|
||||
* `download_queue` document. These are supplied in-memory for the active
|
||||
* session (see `enqueue_download` payloads) but are stripped at the
|
||||
* persistence boundary so `store.bin` contains no plaintext credentials.
|
||||
*/
|
||||
const DOWNLOAD_SECRET_FIELDS = ['password', 'cookies', 'headers'] as const;
|
||||
|
||||
/**
|
||||
* Returns a shallow copy of `item` with secret fields removed. Volatile
|
||||
* progress fields (`fraction`, `speed`, `eta`) are also dropped as in the
|
||||
* existing persistence path.
|
||||
*
|
||||
* Note: `url` is intentionally retained even though it may contain signed
|
||||
* query parameters — redacting it would break resume/retry since the URL is
|
||||
* the download source. Ad-hoc credentials entered in the Add Downloads modal
|
||||
* are therefore session-scoped; site-login passwords (Keychain-backed) are
|
||||
* unaffected by this redaction.
|
||||
*/
|
||||
export const redactDownloadForPersistence = (item: DownloadItem): DownloadItem => {
|
||||
const copy: DownloadItem = { ...item };
|
||||
delete copy.fraction;
|
||||
delete copy.speed;
|
||||
delete copy.eta;
|
||||
for (const field of DOWNLOAD_SECRET_FIELDS) {
|
||||
delete copy[field];
|
||||
}
|
||||
return copy;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user