fix(startup): enforce keychain consent before credential access

Keep credential-store operations behind a per-process consent gate, prevent duplicate native grant requests, and defer legacy token migration until explicit consent. Harden the RTL/sidebar, custom window controls, bidi copy, and queue editor fixes.

Refs #17
This commit is contained in:
NimBold
2026-07-19 07:07:29 +03:30
parent f47eb7507f
commit 6e85c0842f
17 changed files with 221 additions and 79 deletions
+10 -38
View File
@@ -45,18 +45,17 @@ struct LegacyData {
}
pub fn init(storage_layout: &crate::storage::StorageLayout) -> Result<DbState, String> {
init_at_path_internal(storage_layout.data_dir(), storage_layout.is_portable(), false)
init_at_path_internal(storage_layout.data_dir(), storage_layout.is_portable())
}
#[cfg(test)]
fn init_at_path(app_data_dir: &Path) -> Result<DbState, String> {
init_at_path_internal(app_data_dir, false, false)
init_at_path_internal(app_data_dir, false)
}
fn init_at_path_internal(
app_data_dir: &Path,
portable: bool,
migrate_legacy_keychain: bool,
) -> Result<DbState, String> {
fs::create_dir_all(app_data_dir)
.map_err(|error| format!("failed to create app data directory: {error}"))?;
@@ -78,12 +77,7 @@ fn init_at_path_internal(
}
migrate_schema(&mut connection, version)?;
import_legacy_data(
&mut connection,
app_data_dir,
portable,
migrate_legacy_keychain,
)?;
import_legacy_data(&mut connection, app_data_dir, portable)?;
if portable {
sanitize_persisted_downloads(&mut connection)?;
}
@@ -188,7 +182,6 @@ fn import_legacy_data(
connection: &mut Connection,
app_data_dir: &Path,
portable: bool,
migrate_keychain: bool,
) -> Result<(), String> {
let legacy_app_dir = app_data_dir
.parent()
@@ -226,31 +219,10 @@ fn import_legacy_data(
let mut pending_pairing_token = None;
if !portable {
if let Some(token) = legacy.pairing_token.take() {
let migrated = if migrate_keychain {
let keychain_has_token = get_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID)
.ok()
.is_some_and(|value| !value.trim().is_empty());
if !keychain_has_token {
if let Err(error) =
set_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID, &token)
{
log::warn!(
"Legacy pairing token could not be migrated to the credential store; it will remain pending in the current database: {}",
error
);
false
} else {
true
}
} else {
true
}
} else {
false
};
if !migrated {
pending_pairing_token = Some(token);
}
// Legacy migration is deliberately deferred until the
// explicit frontend consent action. Database initialization
// must never touch the OS credential store.
pending_pairing_token = Some(token);
}
}
if portable {
@@ -1738,7 +1710,7 @@ mod tests {
.unwrap();
drop(connection);
let state = init_at_path_internal(temp.path(), true, true).unwrap();
let state = init_at_path_internal(temp.path(), true).unwrap();
let connection = state.lock().unwrap();
let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap();
assert!(saved.get("password").is_none());
@@ -1828,7 +1800,7 @@ mod tests {
});
fs::write(&store_path, serde_json::to_vec(&store).unwrap()).unwrap();
let state = init_at_path_internal(&current, true, true).unwrap();
let state = init_at_path_internal(&current, true).unwrap();
let connection = state.lock().unwrap();
let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap();
assert!(saved.get("password").is_none());
@@ -2104,7 +2076,7 @@ mod tests {
drop(connection);
drop(state);
let state = init_at_path_internal(temp.path(), true, true).unwrap();
let state = init_at_path_internal(temp.path(), true).unwrap();
let connection = state.lock().unwrap();
let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap();
assert!(saved.get("password").is_none());
+71
View File
@@ -2859,6 +2859,12 @@ pub enum TaskHandle {
pub struct AppState {
pub download_coordinator: download::DownloadCoordinator,
pub storage_layout: crate::storage::StorageLayout,
/// Credential-store access is denied until the frontend has completed the
/// startup consent decision for this process. This is intentionally
/// process-local: a new binary must pass through the consent barrier
/// again before any stale or early IPC caller can reach the OS store.
pub keychain_access_authorized: Arc<AtomicBool>,
pub keychain_grant_in_progress: Arc<AtomicBool>,
pub extension_pairing_token: extension_server::SharedExtensionToken,
pub extension_frontend_ready: extension_server::SharedFrontendReady,
pub extension_acks: extension_server::SharedExtensionAcks,
@@ -2872,6 +2878,38 @@ pub struct AppState {
pub queue_manager: Arc<queue::QueueManager>,
}
const KEYCHAIN_CONSENT_REQUIRED_ERROR: &str =
"Credential-store access requires explicit user consent";
const KEYCHAIN_GRANT_IN_PROGRESS_ERROR: &str =
"Credential-store access is already being requested";
fn require_keychain_access(state: &AppState) -> Result<(), String> {
if state
.keychain_access_authorized
.load(Ordering::Acquire)
{
Ok(())
} else {
Err(KEYCHAIN_CONSENT_REQUIRED_ERROR.to_string())
}
}
struct KeychainGrantGuard(Arc<AtomicBool>);
impl Drop for KeychainGrantGuard {
fn drop(&mut self) {
self.0.store(false, Ordering::Release);
}
}
fn begin_keychain_grant(state: &AppState) -> Result<KeychainGrantGuard, String> {
state
.keychain_grant_in_progress
.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
.map(|_| KeychainGrantGuard(Arc::clone(&state.keychain_grant_in_progress)))
.map_err(|_| KEYCHAIN_GRANT_IN_PROGRESS_ERROR.to_string())
}
#[derive(Clone, Serialize, TS)]
#[ts(export, export_to = "../../src/bindings/")]
pub struct DownloadProgressEvent {
@@ -5676,6 +5714,7 @@ fn set_keychain_password(
crate::db::save_pairing_token_to_settings(&connection, &password, true)?;
return Ok(());
}
require_keychain_access(&state)?;
crate::db::set_keychain_password(&id, &password)
}
@@ -5689,6 +5728,7 @@ fn get_keychain_password(
{
return Err("portable pairing token is stored in portable settings".to_string());
}
require_keychain_access(&state)?;
crate::db::get_keychain_password(&id)
}
@@ -5702,17 +5742,20 @@ fn delete_keychain_password(
{
return Ok(());
}
require_keychain_access(&state)?;
crate::db::delete_keychain_password(&id)
}
#[tauri::command]
fn save_site_login(
database: tauri::State<'_, crate::db::DbState>,
state: tauri::State<'_, AppState>,
id: String,
url_pattern: String,
username: String,
password: String,
) -> Result<(), String> {
require_keychain_access(&state)?;
let connection = database.lock()?;
crate::db::save_site_login(&connection, &id, &url_pattern, &username, &password)
}
@@ -5720,12 +5763,28 @@ fn save_site_login(
#[tauri::command]
fn delete_site_login(
database: tauri::State<'_, crate::db::DbState>,
state: tauri::State<'_, AppState>,
id: String,
) -> Result<(), String> {
require_keychain_access(&state)?;
let connection = database.lock()?;
crate::db::delete_site_login(&connection, &id)
}
/// Arm credential-store access after the frontend has completed its startup
/// decision. The process-local gate prevents any stale or early IPC caller
/// from producing a native OS prompt before the explanation is visible.
#[tauri::command]
fn authorize_keychain_access(state: tauri::State<'_, AppState>) -> Result<(), String> {
if state.storage_layout.is_portable() {
return Ok(());
}
state
.keychain_access_authorized
.store(true, Ordering::Release);
Ok(())
}
#[derive(Serialize, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export, export_to = "../../src/bindings/")]
@@ -5761,6 +5820,7 @@ fn hydrate_extension_pairing_token(
});
}
require_keychain_access(&app_state)?;
let migration_error = crate::db::migrate_legacy_pairing_token(&connection).err();
let keychain_token = crate::db::get_keychain_password(crate::db::PAIRING_TOKEN_KEYCHAIN_ID)
.ok()
@@ -5821,6 +5881,7 @@ fn regenerate_pairing_token(
if app_state.storage_layout.is_portable() {
crate::db::save_pairing_token_to_settings(&connection, &generated, true)?;
} else {
require_keychain_access(&app_state)?;
if let Err(error) = crate::db::migrate_legacy_pairing_token(&connection) {
let token = app_state
.extension_pairing_token
@@ -5868,6 +5929,7 @@ fn grant_keychain_access(
database: tauri::State<'_, crate::db::DbState>,
app_state: tauri::State<'_, AppState>,
) -> Result<PairingTokenHydration, String> {
let _grant_guard = begin_keychain_grant(&app_state)?;
let connection = database.lock()?;
if app_state.storage_layout.is_portable() {
@@ -5882,6 +5944,9 @@ fn grant_keychain_access(
if let Ok(mut pairing_token) = app_state.extension_pairing_token.write() {
*pairing_token = token.clone();
}
app_state
.keychain_access_authorized
.store(true, Ordering::Release);
return Ok(PairingTokenHydration {
token,
token_changed: false,
@@ -5933,6 +5998,9 @@ fn grant_keychain_access(
*pairing_token = token.clone();
}
}
app_state
.keychain_access_authorized
.store(true, Ordering::Release);
Ok(PairingTokenHydration {
token,
token_changed: false,
@@ -8442,6 +8510,8 @@ pub fn run() {
app.manage(AppState {
download_coordinator: download::DownloadCoordinator::spawn(app.handle().clone()),
storage_layout,
keychain_access_authorized: Arc::new(AtomicBool::new(false)),
keychain_grant_in_progress: Arc::new(AtomicBool::new(false)),
extension_pairing_token,
extension_frontend_ready,
extension_acks,
@@ -8971,6 +9041,7 @@ pub fn run() {
set_keychain_password, get_keychain_password, delete_keychain_password,
save_site_login, delete_site_login,
hydrate_extension_pairing_token, get_session_pairing_token, regenerate_pairing_token, grant_keychain_access,
authorize_keychain_access,
acknowledge_pairing_token_change,
check_file_exists, toggle_tray_icon, set_extension_pairing_token,
get_extension_server_port, set_extension_frontend_ready, ack_extension_download, set_concurrent_limit, set_global_speed_limit, remove_download,
+2 -10
View File
@@ -8,17 +8,9 @@
"height": 760,
"minWidth": 960,
"minHeight": 640,
"titleBarStyle": "Overlay",
"trafficLightPosition": {
"x": 17,
"y": 28
},
"hiddenTitle": true,
"transparent": true,
"windowEffects": {
"effects": ["sidebar"],
"state": "active"
}
"decorations": false,
"shadow": false
}
]
},
+1 -4
View File
@@ -10,10 +10,7 @@
"minHeight": 640,
"transparent": true,
"decorations": false,
"windowEffects": {
"effects": ["mica"],
"state": "active"
}
"shadow": false
}
]
},