fix(permissions): clear stale TCC entries on upgrade and grant click (#153)

Signed-off-by: Logan Nguyen <lg.131.dev@gmail.com>

Author: quiet-node

src-tauri/src/lib.rs

@@ -1122,6 +1122,16 @@ pub fn run() {
                 );
                 if did_upgrade {
                     updater::tcc_reset::tccutil_reset(&app.config().identifier);
+                    // Persist that the running version's csreq is what
+                    // owns any TCC entries on disk now (or there are no
+                    // entries, which is also fine). The click-time grant
+                    // flow consults this so the user's first grant click
+                    // after an upgrade does not trigger a second
+                    // reset+relaunch on top of the one we are about to
+                    // schedule below. Held in the sidecar (not memory)
+                    // because the relaunch wipes any in-process state
+                    // before the user could ever click.
+                    sidecar.last_reset_for_version = Some(running_version.clone());
                 }
 
                 // Restore persisted snooze flags into the live state.
@@ -1135,6 +1145,9 @@ pub fn run() {
                 // snooze.
                 updater_state
                     .set_last_seen_update_version(sidecar.last_seen_update_version.clone());
+                // Mirror the on-disk reset marker so click-time decisions
+                // don't have to re-read the sidecar.
+                updater_state.set_last_reset_for_version(sidecar.last_reset_for_version.clone());
 
                 // Record the running version BEFORE any potential restart
                 // so the post-restart launch reads a sidecar where the
@@ -1339,7 +1352,11 @@ pub fn run() {
             #[cfg(not(coverage))]
             updater::commands::snooze_update_chat,
             #[cfg(not(coverage))]
-            updater::commands::snooze_update_settings
+            updater::commands::snooze_update_settings,
+            #[cfg(not(coverage))]
+            updater::commands::reset_and_relaunch_for_grant,
+            #[cfg(not(coverage))]
+            updater::commands::consume_pending_grant_resume
         ])
         .build(tauri::generate_context!())
         .expect("error while building tauri application")

src-tauri/src/updater/commands.rs

@@ -1,6 +1,7 @@
 use crate::config::defaults::{DEFAULT_UPDATER_STATE_FILENAME, MAX_UPDATER_SNOOZE_HOURS};
 use crate::updater::poller;
-use crate::updater::state::{UpdaterSnapshot, UpdaterState};
+use crate::updater::state::{SnoozeSidecar, UpdaterSnapshot, UpdaterState};
+use crate::updater::tcc_reset;
 use std::path::PathBuf;
 use std::time::{SystemTime, UNIX_EPOCH};
 use tauri::{AppHandle, Manager, State};
@@ -107,6 +108,121 @@ fn sidecar_path(app: &AppHandle) -> Result<PathBuf, String> {
     Ok(dir.join(DEFAULT_UPDATER_STATE_FILENAME))
 }
 
+/// Stores `service` on `sidecar` so the post-restart launch can resume the
+/// grant flow. Returns `Err` when the service string is not one of the
+/// values Thuki resets at click time, so callers cannot smuggle arbitrary
+/// strings into a later `tccutil reset` invocation.
+pub fn prepare_pending_reregister(
+    sidecar: &mut SnoozeSidecar,
+    service: &str,
+) -> Result<&'static str, String> {
+    let canonical = tcc_reset::validate_click_time_service(service)
+        .ok_or_else(|| format!("unsupported tcc service: {service}"))?;
+    sidecar.pending_reregister = Some(canonical.to_string());
+    Ok(canonical)
+}
+
+/// Removes `pending_reregister` from `sidecar` and returns its previous
+/// value. The caller is responsible for persisting the cleared sidecar so
+/// the resume flow does not loop on the next restart.
+pub fn take_pending_reregister(sidecar: &mut SnoozeSidecar) -> Option<String> {
+    sidecar.pending_reregister.take()
+}
+
+/// Pure decision helper. Returns `true` when the click-time reset can be
+/// skipped because the startup path already cleared TCC for the running
+/// version. The marker survives A's reset+restart by being persisted to
+/// the sidecar, which is why the comparison is meaningful even though
+/// `was_reset_at_startup` would always be `false` on a freshly relaunched
+/// process.
+pub fn click_time_reset_can_skip(
+    last_reset_for_version: Option<&str>,
+    running_version: &str,
+) -> bool {
+    last_reset_for_version == Some(running_version)
+}
+
+/// Click-time grant flow: persist a "resume after restart" marker, clear
+/// the stale TCC entry for the requested service, and relaunch. The
+/// frontend hands the service string straight in, so the validator inside
+/// `prepare_pending_reregister` is the trust boundary.
+///
+/// Returns `true` when a relaunch has been scheduled and `false` when the
+/// running process already has a clean TCC slate (the startup path's most
+/// recent reset matches the running version). In the `false` case the
+/// frontend should run the in-line open-Settings + polling flow without
+/// expecting a relaunch.
+///
+/// Sequencing matters when a relaunch is scheduled. Sidecar must be saved
+/// BEFORE `tccutil reset` runs so a crash between the two does not leave
+/// the user with a cleared grant and no resume marker. The restart is
+/// deferred so Tauri can finish dispatching the IPC reply (otherwise the
+/// frontend sees a disconnect error rather than a clean relaunch).
+#[cfg_attr(coverage_nightly, coverage(off))]
+#[tauri::command]
+pub fn reset_and_relaunch_for_grant(
+    app: AppHandle,
+    state: State<'_, UpdaterState>,
+    service: String,
+) -> Result<bool, String> {
+    // Validate first so a hostile string never reaches `tccutil` even when
+    // the startup-clean path skips the reset.
+    let canonical = tcc_reset::validate_click_time_service(&service)
+        .ok_or_else(|| format!("unsupported tcc service: {service}"))?;
+
+    let running = app.package_info().version.to_string();
+    let snooze = state.snooze_clone();
+    if click_time_reset_can_skip(snooze.last_reset_for_version.as_deref(), &running) {
+        // Startup path already reset TCC for this exact version, so the
+        // running binary's csreq already owns whatever TCC entries (if
+        // any) System Settings will display. A second reset+relaunch
+        // would only add a jarring quit on every grant click.
+        return Ok(false);
+    }
+
+    let mut snooze = snooze;
+    prepare_pending_reregister(&mut snooze, canonical)?;
+
+    let path = sidecar_path(&app)?;
+    snooze.save(&path).map_err(|e| e.to_string())?;
+    state.set_pending_reregister(Some(canonical.to_string()));
+
+    let bundle_id = app.config().identifier.clone();
+    tcc_reset::tccutil_reset_service(&bundle_id, canonical);
+
+    let app_handle = app.clone();
+    tauri::async_runtime::spawn(async move {
+        tokio::time::sleep(std::time::Duration::from_millis(150)).await;
+        eprintln!(
+            "thuki: [updater] relaunching after click-time TCC reset \
+             to refresh tccd PID tracking"
+        );
+        app_handle.restart();
+    });
+
+    Ok(true)
+}
+
+/// Frontend-facing companion to `reset_and_relaunch_for_grant`. Reads the
+/// `pending_reregister` flag, clears it (in memory and on disk), and
+/// returns the value so PermissionsStep can resume the right step on a
+/// fresh launch without forcing the user to click a second time.
+#[cfg_attr(coverage_nightly, coverage(off))]
+#[tauri::command]
+pub fn consume_pending_grant_resume(
+    app: AppHandle,
+    state: State<'_, UpdaterState>,
+) -> Result<Option<String>, String> {
+    let mut snooze = state.snooze_clone();
+    let value = take_pending_reregister(&mut snooze);
+    if value.is_some() {
+        let path = sidecar_path(&app)?;
+        snooze.save(&path).map_err(|e| e.to_string())?;
+        state.set_pending_reregister(None);
+    }
+    Ok(value)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -144,4 +260,66 @@ mod tests {
     fn snooze_deadline_zero_hours_is_now() {
         assert_eq!(snooze_deadline(1_700_000_000, 0), 1_700_000_000);
     }
+
+    #[test]
+    fn prepare_pending_reregister_accepts_accessibility() {
+        let mut sidecar = SnoozeSidecar::default();
+        let canonical = prepare_pending_reregister(&mut sidecar, "Accessibility").unwrap();
+        assert_eq!(canonical, "Accessibility");
+        assert_eq!(sidecar.pending_reregister.as_deref(), Some("Accessibility"));
+    }
+
+    #[test]
+    fn prepare_pending_reregister_accepts_screen_capture() {
+        let mut sidecar = SnoozeSidecar::default();
+        let canonical = prepare_pending_reregister(&mut sidecar, "ScreenCapture").unwrap();
+        assert_eq!(canonical, "ScreenCapture");
+        assert_eq!(sidecar.pending_reregister.as_deref(), Some("ScreenCapture"));
+    }
+
+    #[test]
+    fn prepare_pending_reregister_rejects_unsupported_service() {
+        let mut sidecar = SnoozeSidecar::default();
+        let err = prepare_pending_reregister(&mut sidecar, "Camera").unwrap_err();
+        assert!(err.contains("Camera"), "error must surface offending value");
+        // Sidecar must remain untouched on rejection so a hostile call
+        // cannot pollute the persisted resume marker.
+        assert!(sidecar.pending_reregister.is_none());
+    }
+
+    #[test]
+    fn take_pending_reregister_returns_and_clears_value() {
+        let mut sidecar = SnoozeSidecar {
+            pending_reregister: Some("Accessibility".to_string()),
+            ..SnoozeSidecar::default()
+        };
+        assert_eq!(
+            take_pending_reregister(&mut sidecar),
+            Some("Accessibility".to_string()),
+        );
+        assert!(sidecar.pending_reregister.is_none());
+    }
+
+    #[test]
+    fn take_pending_reregister_returns_none_when_unset() {
+        let mut sidecar = SnoozeSidecar::default();
+        assert!(take_pending_reregister(&mut sidecar).is_none());
+    }
+
+    #[test]
+    fn click_time_reset_can_skip_when_versions_match() {
+        assert!(click_time_reset_can_skip(Some("0.8.5"), "0.8.5"));
+    }
+
+    #[test]
+    fn click_time_reset_does_not_skip_when_versions_differ() {
+        assert!(!click_time_reset_can_skip(Some("0.8.4"), "0.8.5"));
+    }
+
+    #[test]
+    fn click_time_reset_does_not_skip_when_marker_is_absent() {
+        // No prior startup reset for this binary recorded: a stale csreq
+        // grant could still be on disk, so the click MUST clean it up.
+        assert!(!click_time_reset_can_skip(None, "0.8.5"));
+    }
 }

src-tauri/src/updater/state.rs

@@ -32,6 +32,22 @@ pub struct SnoozeSidecar {
     /// just in memory) so the comparison survives an app restart.
     #[serde(default)]
     pub last_seen_update_version: Option<String>,
+    /// TCC service the user just clicked "Grant" for, persisted across the
+    /// reset+relaunch cycle so the post-restart launch knows to resume the
+    /// grant flow (open System Settings, trigger a new csreq registration)
+    /// instead of waiting for another click. Cleared on consumption.
+    #[serde(default)]
+    pub pending_reregister: Option<String>,
+    /// SemVer string of the binary the startup path most recently ran a
+    /// `tccutil reset` for. The click-time grant flow consults this to
+    /// decide whether ANOTHER reset+relaunch is necessary: if it matches
+    /// the running version, the running binary's csreq already owns the
+    /// only TCC entries on disk (or there are none), so a click does not
+    /// need a redundant restart. Persisted (rather than held in memory)
+    /// because A's reset+restart drops any in-process flag before the
+    /// user can ever click.
+    #[serde(default)]
+    pub last_reset_for_version: Option<String>,
 }
 
 impl SnoozeSidecar {
@@ -135,13 +151,31 @@ impl UpdaterState {
         inner.snooze.last_seen_update_version = version;
     }
 
+    /// Mirror the on-disk `pending_reregister` field into the live state
+    /// so a crash before the next sidecar save still leaves the in-memory
+    /// view consistent with what was persisted.
+    pub fn set_pending_reregister(&self, value: Option<String>) {
+        let mut inner = self.inner.lock().expect("updater state mutex");
+        inner.snooze.pending_reregister = value;
+    }
+
     pub fn snooze_clone(&self) -> SnoozeSidecar {
         self.inner
             .lock()
             .expect("updater state mutex")
             .snooze
             .clone()
     }
+
+    /// Mirrors the persisted `last_reset_for_version` from the sidecar
+    /// into live state. Called both at startup (when seeding from disk)
+    /// and immediately after a successful startup-time `tccutil reset`
+    /// (so the click-time grant flow sees the new value without re-reading
+    /// the sidecar).
+    pub fn set_last_reset_for_version(&self, version: Option<String>) {
+        let mut inner = self.inner.lock().expect("updater state mutex");
+        inner.snooze.last_reset_for_version = version;
+    }
 }
 
 #[derive(Debug, Clone, Serialize)]
@@ -179,6 +213,8 @@ mod tests {
             chat_snoozed_until: Some(1_700_001_000),
             last_launched_version: None,
             last_seen_update_version: None,
+            pending_reregister: None,
+            last_reset_for_version: Some("0.8.5".to_string()),
         };
         original.save(&path).unwrap();
 
@@ -205,13 +241,53 @@ mod tests {
             chat_snoozed_until: None,
             last_launched_version: Some("0.8.1".to_string()),
             last_seen_update_version: None,
+            pending_reregister: None,
+            last_reset_for_version: None,
+        };
+        original.save(&path).unwrap();
+
+        let loaded = SnoozeSidecar::load(&path).unwrap();
+        assert_eq!(loaded, original);
+    }
+
+    #[test]
+    fn snooze_sidecar_round_trips_pending_reregister() {
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("updater_state.json");
+
+        let original = SnoozeSidecar {
+            settings_snoozed_until: None,
+            chat_snoozed_until: None,
+            last_launched_version: Some("0.8.5".to_string()),
+            last_seen_update_version: None,
+            pending_reregister: Some("Accessibility".to_string()),
+            last_reset_for_version: Some("0.8.5".to_string()),
         };
         original.save(&path).unwrap();
 
         let loaded = SnoozeSidecar::load(&path).unwrap();
         assert_eq!(loaded, original);
     }
 
+    #[test]
+    fn snooze_sidecar_back_compat_old_file_without_pending_reregister() {
+        // Sidecars written before the click-time grant flow shipped lack
+        // the field. Loading must default it to None so existing snooze /
+        // version state is preserved across the upgrade.
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("updater_state.json");
+        std::fs::write(
+            &path,
+            r#"{"settings_snoozed_until":null,"chat_snoozed_until":null,
+                "last_launched_version":"0.8.4","last_seen_update_version":null}"#,
+        )
+        .unwrap();
+
+        let loaded = SnoozeSidecar::load(&path).unwrap();
+        assert_eq!(loaded.last_launched_version.as_deref(), Some("0.8.4"));
+        assert!(loaded.pending_reregister.is_none());
+    }
+
     #[test]
     fn snooze_sidecar_back_compat_old_file_without_version_field() {
         // Old (pre-0.8.2) sidecar files were written without the
@@ -406,6 +482,30 @@ mod tests {
         assert_eq!(snap.settings_snoozed_until, Some(2));
     }
 
+    #[test]
+    fn set_last_reset_for_version_round_trips_through_snooze_clone() {
+        let state = UpdaterState::default();
+        state.set_last_reset_for_version(Some("0.8.5".to_string()));
+        assert_eq!(
+            state.snooze_clone().last_reset_for_version.as_deref(),
+            Some("0.8.5"),
+        );
+        state.set_last_reset_for_version(None);
+        assert!(state.snooze_clone().last_reset_for_version.is_none());
+    }
+
+    #[test]
+    fn set_pending_reregister_round_trips_through_snooze_clone() {
+        let state = UpdaterState::default();
+        state.set_pending_reregister(Some("Accessibility".to_string()));
+        assert_eq!(
+            state.snooze_clone().pending_reregister.as_deref(),
+            Some("Accessibility"),
+        );
+        state.set_pending_reregister(None);
+        assert!(state.snooze_clone().pending_reregister.is_none());
+    }
+
     #[test]
     fn system_time_to_unix_returns_some_for_now() {
         let now = SystemTime::now();