From fe95038b7df4ca6abc2cadabbc30332da67e6d3d Mon Sep 17 00:00:00 2001 From: "fctl-sh[bot]" <343971779+fctl-sh[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:21:02 +0000 Subject: [PATCH 1/7] fix(control): end workspaces' connections when their app is uninstalled A workspace's GitHub connection worked through whatever installation of the GitHub App its account had, and a Linear one through fctl's app wherever it was installed again: once uninstalled and installed again, every connection made before came back. An account's or a Linear workspace's admins had no way to end what they let a workspace reach, such as a former admin's own workspace, which kept minting tokens there. A GitHub connection now holds only through the installation it was made through: sessions fail with a message that says so, and pull request news stops, until the workspace's admins connect the account again with a GitHub sign-in. When Linear says fctl's app was uninstalled, or refuses to renew its token, the fctl workspaces' connections to that Linear workspace end, and installing the app again connects only the fctl workspace it is installed from. --- crates/fctl-control/src/linear.rs | 30 +++++++--- crates/fctl-control/src/pull_requests.rs | 8 ++- crates/fctl-control/src/workspaces.rs | 71 +++++++++++++++--------- docs/architecture.md | 23 ++++++-- docs/security.md | 14 ++++- 5 files changed, 100 insertions(+), 46 deletions(-) diff --git a/crates/fctl-control/src/linear.rs b/crates/fctl-control/src/linear.rs index eb5097e3..f0c540da 100644 --- a/crates/fctl-control/src/linear.rs +++ b/crates/fctl-control/src/linear.rs @@ -285,17 +285,29 @@ pub async fn store( /// Marks fctl's app in a Linear workspace revoked, and forgets its tokens, /// which no longer work: uninstalled there, or its token no longer renews. -/// Its fctl workspaces' admins connect it again. +/// The fctl workspaces connected to it stop working there, and their admins +/// connect it again: installing the app again, which takes an admin of the +/// Linear workspace, connects only the fctl workspace it is installed from, +/// so that the Linear workspace's admins can end what they let fctl +/// workspaces reach, a former admin's own one too. pub async fn mark_revoked(db: &D1Database, organization: &str, now: i64) -> AppResult<()> { - q( + crate::db::batch( db, - "UPDATE linear_installations SET status = 'revoked', secret_enc = '', expires_at = NULL, updated_at = ? \ - WHERE organization_id = ?", - &[n(now), s(organization)], - )? - .run() - .await?; - Ok(()) + vec![ + q( + db, + "UPDATE linear_installations SET status = 'revoked', secret_enc = '', expires_at = NULL, updated_at = ? \ + WHERE organization_id = ?", + &[n(now), s(organization)], + )?, + q( + db, + "DELETE FROM workspace_linear WHERE organization_id = ?", + &[s(organization)], + )?, + ], + ) + .await } /// Removes fctl's app from a Linear workspace no fctl workspace works in diff --git a/crates/fctl-control/src/pull_requests.rs b/crates/fctl-control/src/pull_requests.rs index aa9f79c4..b3dfb341 100644 --- a/crates/fctl-control/src/pull_requests.rs +++ b/crates/fctl-control/src/pull_requests.rs @@ -84,13 +84,15 @@ pub(crate) fn key(repo: &RepoRef) -> String { /// /// With `answered` (parameters, `?N`): GitHub's id of the account that /// answered and of the installation it answered through, only where the -/// connection is to that account, or, from before ids were kept, recorded -/// that installation, since a login can change hands. +/// connection was made through that installation, or, from before +/// installations were recorded, is to that account, since a login can +/// change hands, and a connection made through an installation since +/// uninstalled has ended (`crate::workspaces::installation`). pub(crate) fn in_reach(answered: Option<(&str, &str)>) -> String { let account = answered .map(|(account, installation)| { format!( - " AND (rg.account_id = {account} OR (rg.account_id IS NULL AND rg.installation_id = {installation}))" + " AND (rg.installation_id = {installation} OR (rg.installation_id IS NULL AND rg.account_id = {account}))" ) }) .unwrap_or_default(); diff --git a/crates/fctl-control/src/workspaces.rs b/crates/fctl-control/src/workspaces.rs index 1afa3ea7..a6a62a6c 100644 --- a/crates/fctl-control/src/workspaces.rs +++ b/crates/fctl-control/src/workspaces.rs @@ -667,16 +667,18 @@ pub fn scope(connections: &[ConnectionRow], owner: &str) -> Option { } /// The installation of the GitHub App that serves a workspace's connection -/// to `account`: the one on that login as GitHub says now (the app may have -/// been installed again since), while the login still belongs to the -/// account the workspace connected, by id (`fctl_domain::github::binding`). -/// A connection from before ids were kept takes the account of the -/// installation it recorded, when GitHub still has it, else the one that -/// has the login now. The installation is remembered with the workspace's -/// connection and the others bound to that account; never with another -/// workspace's connection from before ids were kept, whose recorded -/// installation is what tells it which account it connected. A workspace -/// without an active connection to `account` gets none. +/// to `account`: the one on that login as GitHub says now, while the login +/// still belongs to the account the workspace connected, by id +/// (`fctl_domain::github::binding`), and that installation is the one the +/// workspace connected through. Uninstalling the app from an account ends +/// the connections made through it: installing it again brings none back +/// (`installed_again`), so that an account's admins can end what they let +/// workspaces reach, for a former admin's own workspace too, and the +/// workspace's admins connect it again, with a GitHub sign-in that checks +/// what they may connect now. A connection from before installations were +/// recorded keeps the one it finds the first time, and one from before ids +/// were kept the account it finds then. A workspace without an active +/// connection to `account` gets none. pub async fn installation( db: &D1Database, gh: &crate::github::Github<'_>, @@ -694,17 +696,26 @@ pub async fn installation( .installation_id .and_then(|id| u64::try_from(id).ok()); let stored = connection.account_id.and_then(|id| u64::try_from(id).ok()); - let mut connected = stored; - if stored.is_none() - && let Some(recorded) = known.filter(|k| *k != found.id) - && let Some(info) = gh.installation_by_id(recorded).await? - { - connected = info.account_id; + if let Some(recorded) = known.filter(|k| *k != found.id) { + // Where the installation it recorded is still there, the login is + // another account's now; else the app was installed again. + let connected = match stored { + Some(id) => Some(id), + None => gh + .installation_by_id(recorded) + .await? + .and_then(|i| i.account_id), + }; + if fctl_domain::github::binding(connected, found.account_id) == Binding::Other { + return Err(not_connected(account)); + } + return Err(installed_again(account)); } - if fctl_domain::github::binding(connected, found.account_id) == Binding::Other { + if fctl_domain::github::binding(stored, found.account_id) == Binding::Other { return Err(not_connected(account)); } - // A connection from before ids were kept keeps the account from now on. + // A connection from before ids were kept keeps the account from now on, + // and one from before installations were recorded the installation. if stored.is_none() && let Some(id) = found.account_id { @@ -716,17 +727,11 @@ pub async fn installation( .run() .await?; } - if known != Some(found.id) { + if known.is_none() { q( db, - "UPDATE workspace_github SET installation_id = ?1 \ - WHERE (workspace_id = ?2 AND account = ?3) OR (?4 IS NOT NULL AND account_id = ?4)", - &[ - n(found.id as i64), - s(&connection.workspace_id), - s(&connection.account), - on(found.account_id.map(|id| id as i64)), - ], + "UPDATE workspace_github SET installation_id = ? WHERE workspace_id = ? AND account = ? AND installation_id IS NULL", + &[n(found.id as i64), s(&connection.workspace_id), s(&connection.account)], )? .run() .await?; @@ -744,6 +749,18 @@ fn not_used(account: &str) -> AppError { )) } +/// The error for a connection made through an installation of the GitHub +/// App that the account no longer has: it was uninstalled there, and maybe +/// installed again since ([`installation`]). +pub fn installed_again(account: &str) -> AppError { + AppError(ApiError::new( + ErrorCode::Forbidden, + format!( + "fctl's GitHub App was uninstalled from {account} after this workspace connected it, which ended the connection. The workspace's admins connect it again under its GitHub tab." + ), + )) +} + /// The error for a connection whose login belongs to another GitHub /// account now. pub fn not_connected(account: &str) -> AppError { diff --git a/docs/architecture.md b/docs/architecture.md index 363f5e9e..32c1c558 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -634,10 +634,18 @@ sessions what pull request lookups and webhooks say only through the installation on a login that still belongs to the account the workspace connected (`workspaces::installation`, `fctl_domain::github::binding`). Otherwise sessions fail, and links refuse, saying that the workspace's -admins connect the account again, by its name now. A connection from -before ids were kept takes the account of the installation it recorded, -when GitHub still has it, else the one that has the login, the first time -it is used. +admins connect the account again, by its name now. A connection is also +to the installation it was made through (`workspace_github.installation_id`): +uninstalling the app from an account ends every workspace's connection +there, and installing it again brings none back, so that an account's +admins can end what they let workspaces reach, a former admin's own +workspace too. Sessions then fail, and pull request news stops +(`pull_requests::in_reach`), until the workspace's admins connect the +account again, with a GitHub sign-in that checks what they may connect +now. A connection from before ids were kept takes the account of the +installation it recorded, when GitHub still has it, else the one that has +the login, the first time it is used, and one from before installations +were recorded the installation it finds then. Installing the app from a workspace, or giving it more repositories, goes through `/api/v1/github/install`, which keeps the workspace in a signed @@ -1823,7 +1831,12 @@ issue's delegate (the person stays its assignee), and mention. and two renewals that race end alike, as Linear answers a refresh it already answered the same for half an hour. One whose renewal Linear refuses, or that Linear says was uninstalled (an `OAuthApp` - revocation), is revoked here until an admin connects it again. A + revocation), is revoked here, and every fctl workspace's connection to + it ends (`crate::linear::mark_revoked`): installing the app again, which + takes an admin of the Linear workspace, connects only the fctl + workspace it is installed from, so that the Linear workspace's admins + can end what they let fctl workspaces reach, a former admin's own one + too. A workspace that disconnects a Linear workspace hears nothing more from it and reaches none of its tickets, whatever other workspaces do; once no fctl workspace works in it, fctl revokes the app's token there (the diff --git a/docs/security.md b/docs/security.md index 55eae46f..148920e8 100644 --- a/docs/security.md +++ b/docs/security.md @@ -330,7 +330,15 @@ its VM, and nothing outside it except through these channels: A connection is to the account, by GitHub's id, not its name: when a connected account is renamed or deleted and someone else takes its name, the workspace's sessions get no tokens there, and their pull requests no - news from it, until its admins connect the account again. + news from it, until its admins connect the account again. It is also to + the installation of the app it was made through: uninstalling the app + from an account ends every workspace's connection there, and installing + it again brings none back, so that the account's admins can end what + they let workspaces reach, a former admin's own workspace too. Each + workspace's admins connect it again, with a GitHub sign-in that checks + what they may connect then. Short of that, a connection keeps what its + admin's sign-in gave it: fctl doesn't check their standing on GitHub + again ([ADR 0029](adr/0029-workspaces-connect-github-accounts-with-a-github-sign-in.md)). - **Claude.** A short-lived access token for the session's owner. Refresh tokens never enter a sandbox: when the access token expires, Claude asks the control plane for a new one through its host. A session gets the @@ -664,7 +672,9 @@ Cloudflare does around the Worker, its server (`deploy/standalone`) does: to it; once none is, the control plane revokes them at Linear and deletes them (within the hour if Linear doesn't answer), and it forgets them as soon as Linear says the app was uninstalled or refuses to renew - them. A delegation waiting for someone to link their account keeps what + them, which ends every fctl workspace's connection to that Linear + workspace: installing the app again, which takes one of its admins, + connects only the fctl workspace it is installed from. A delegation waiting for someone to link their account keeps what Linear sent for an hour, and which Linear agent session reached which fctl session goes with the session. - Hosts keep a stopped session's sandbox so it can resume, and remove it From f1144838acdfa5f59f332bd845272f488ca9b54e Mon Sep 17 00:00:00 2001 From: "fctl-sh[bot]" <343971779+fctl-sh[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:30:19 +0000 Subject: [PATCH 2/7] fix(control): keep revoked Linear connections in sight until reconnected Ending every fctl workspace's connection when Linear revokes fctl's app deleted their rows, so a workspace's Linear tab showed nothing about why its projects stopped working, and the hourly job then forgot the installation. The connections stay now, revoked since `workspace_linear.revoked_at` (a new migration), and the tab shows them revoked with its prompt to connect again; installing the app again clears only the connection of the workspace it is installed from, and nothing takes tickets, delegations or replies through a revoked one. --- .../0042_linear_connections_revoked.sql | 9 +++++++++ crates/fctl-control/src/api/linear.rs | 4 ++-- crates/fctl-control/src/delegations.rs | 8 +++++--- crates/fctl-control/src/linear.rs | 15 ++++++++------- crates/fctl-control/src/tickets.rs | 12 ++++++++---- docs/architecture.md | 11 ++++++----- 6 files changed, 38 insertions(+), 21 deletions(-) create mode 100644 crates/fctl-control/migrations/0042_linear_connections_revoked.sql diff --git a/crates/fctl-control/migrations/0042_linear_connections_revoked.sql b/crates/fctl-control/migrations/0042_linear_connections_revoked.sql new file mode 100644 index 00000000..05557bb3 --- /dev/null +++ b/crates/fctl-control/migrations/0042_linear_connections_revoked.sql @@ -0,0 +1,9 @@ +-- When Linear revokes fctl's app in a Linear workspace (it was uninstalled +-- there, or its token no longer renews), every fctl workspace's connection +-- to it ends: installing the app again, which takes one of the Linear +-- workspace's admins, connects only the fctl workspace it is installed +-- from, so that the Linear workspace's admins can end what they let fctl +-- workspaces reach. The others keep their row, revoked since `revoked_at`, +-- which their Linear tab shows until their admins connect it again or +-- disconnect it. +ALTER TABLE workspace_linear ADD COLUMN revoked_at INTEGER; diff --git a/crates/fctl-control/src/api/linear.rs b/crates/fctl-control/src/api/linear.rs index 9e09a922..ed502515 100644 --- a/crates/fctl-control/src/api/linear.rs +++ b/crates/fctl-control/src/api/linear.rs @@ -193,7 +193,7 @@ pub async fn teams(ctx: &Ctx, id: &str, organization: &str) -> AppResult = q( &ctx.db, - "SELECT COUNT(*) AS n FROM workspace_linear WHERE workspace_id = ? AND organization_id = ?", + "SELECT COUNT(*) AS n FROM workspace_linear WHERE workspace_id = ? AND organization_id = ? AND revoked_at IS NULL", &[s(a.id()), s(organization)], )? .first() @@ -529,7 +529,7 @@ async fn install( q( &ctx.db, "INSERT INTO workspace_linear (workspace_id, organization_id, connected_by, linear_user, created_at) VALUES (?, ?, ?, ?, ?) \ - ON CONFLICT (workspace_id, organization_id) DO UPDATE SET connected_by = excluded.connected_by", + ON CONFLICT (workspace_id, organization_id) DO UPDATE SET connected_by = excluded.connected_by, revoked_at = NULL", &[ s(a.id()), s(&viewer.organization), diff --git a/crates/fctl-control/src/delegations.rs b/crates/fctl-control/src/delegations.rs index 1196260d..b5c36266 100644 --- a/crates/fctl-control/src/delegations.rs +++ b/crates/fctl-control/src/delegations.rs @@ -92,7 +92,8 @@ async fn thread_of(db: &D1Database, session_id: &str) -> AppResult App let c: Option = q( db, "SELECT COUNT(*) AS n FROM sessions s JOIN projects p ON p.id = s.project_id \ - JOIN workspace_linear wl ON wl.workspace_id = p.workspace_id WHERE s.id = ? AND wl.organization_id = ?", + JOIN workspace_linear wl ON wl.workspace_id = p.workspace_id WHERE s.id = ? AND wl.organization_id = ? \ + AND wl.revoked_at IS NULL", &[s(session_id), s(organization)], )? .first() @@ -387,7 +389,7 @@ async fn takers(db: &D1Database, organization: &str) -> AppResult> { let rows: Vec = q( db, &format!( - "{PROJECT_SELECT} WHERE p.archived_at IS NULL AND p.workspace_id IN (SELECT workspace_id FROM workspace_linear WHERE organization_id = ?2) \ + "{PROJECT_SELECT} WHERE p.archived_at IS NULL AND p.workspace_id IN (SELECT workspace_id FROM workspace_linear WHERE organization_id = ?2 AND revoked_at IS NULL) \ AND json_extract(p.settings, '$.tickets.tracker') = 'linear' \ AND json_extract(p.settings, '$.tickets.linear.organization') = ?2 ORDER BY p.created_at" ), diff --git a/crates/fctl-control/src/linear.rs b/crates/fctl-control/src/linear.rs index f0c540da..b5168a01 100644 --- a/crates/fctl-control/src/linear.rs +++ b/crates/fctl-control/src/linear.rs @@ -285,11 +285,12 @@ pub async fn store( /// Marks fctl's app in a Linear workspace revoked, and forgets its tokens, /// which no longer work: uninstalled there, or its token no longer renews. -/// The fctl workspaces connected to it stop working there, and their admins -/// connect it again: installing the app again, which takes an admin of the -/// Linear workspace, connects only the fctl workspace it is installed from, -/// so that the Linear workspace's admins can end what they let fctl -/// workspaces reach, a former admin's own one too. +/// The fctl workspaces connected to it stop working there, revoked, until +/// their admins connect it again: installing the app again, which takes an +/// admin of the Linear workspace, connects only the fctl workspace it is +/// installed from (`workspace_linear.revoked_at`), so that the Linear +/// workspace's admins can end what they let fctl workspaces reach, a +/// former admin's own one too. pub async fn mark_revoked(db: &D1Database, organization: &str, now: i64) -> AppResult<()> { crate::db::batch( db, @@ -302,8 +303,8 @@ pub async fn mark_revoked(db: &D1Database, organization: &str, now: i64) -> AppR )?, q( db, - "DELETE FROM workspace_linear WHERE organization_id = ?", - &[s(organization)], + "UPDATE workspace_linear SET revoked_at = ? WHERE organization_id = ? AND revoked_at IS NULL", + &[n(now), s(organization)], )?, ], ) diff --git a/crates/fctl-control/src/tickets.rs b/crates/fctl-control/src/tickets.rs index 1e7ca200..d9fb1119 100644 --- a/crates/fctl-control/src/tickets.rs +++ b/crates/fctl-control/src/tickets.rs @@ -110,6 +110,10 @@ pub async fn connections(ctx: &Ctx, workspace_id: &str) -> AppResult, app_user_handle: Option, status: String, + /// Linear revoked fctl's app while this workspace was connected, + /// and its admins haven't connected it again. + #[serde(deserialize_with = "crate::db::opt_int", default)] + revoked_at: Option, linear_user: Option, connected_by: Option, by_name: Option, @@ -120,7 +124,7 @@ pub async fn connections(ctx: &Ctx, workspace_id: &str) -> AppResult = q( db, "SELECT i.organization_id, i.name, i.url_key, i.logo_url, i.app_user_id, i.app_user_name, i.app_user_handle, \ - i.status, wl.linear_user, \ + i.status, wl.revoked_at, wl.linear_user, \ wl.connected_by, u.name AS by_name, u.avatar_url AS by_avatar, wl.created_at \ FROM workspace_linear wl JOIN linear_installations i ON i.organization_id = wl.organization_id \ LEFT JOIN users u ON u.id = wl.connected_by WHERE wl.workspace_id = ? ORDER BY wl.created_at", @@ -182,7 +186,7 @@ pub async fn connections(ctx: &Ctx, workspace_id: &str) -> AppResult AppResult { let connected: Option = q( db, - "SELECT COUNT(*) AS n FROM workspace_linear WHERE workspace_id = ? AND organization_id = ?", + "SELECT COUNT(*) AS n FROM workspace_linear WHERE workspace_id = ? AND organization_id = ? AND revoked_at IS NULL", &[s(workspace_id), s(&l.organization)], )? .first() @@ -621,7 +625,7 @@ pub async fn call(ctx: &Ctx, session: &RunnerSession, request: Value) -> AppResu pub async fn works_in(db: &D1Database, workspace_id: &str, organization: &str) -> AppResult { let c: Option = q( db, - "SELECT COUNT(*) AS n FROM workspace_linear WHERE workspace_id = ? AND organization_id = ?", + "SELECT COUNT(*) AS n FROM workspace_linear WHERE workspace_id = ? AND organization_id = ? AND revoked_at IS NULL", &[s(workspace_id), s(organization)], )? .first() diff --git a/docs/architecture.md b/docs/architecture.md index 32c1c558..11b50cf2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1832,11 +1832,12 @@ issue's delegate (the person stays its assignee), and mention. already answered the same for half an hour. One whose renewal Linear refuses, or that Linear says was uninstalled (an `OAuthApp` revocation), is revoked here, and every fctl workspace's connection to - it ends (`crate::linear::mark_revoked`): installing the app again, which - takes an admin of the Linear workspace, connects only the fctl - workspace it is installed from, so that the Linear workspace's admins - can end what they let fctl workspaces reach, a former admin's own one - too. A + it ends (`crate::linear::mark_revoked`, `workspace_linear.revoked_at`): + installing the app again, which takes an admin of the Linear workspace, + connects only the fctl workspace it is installed from, and the others' + Linear tabs show theirs revoked until their admins connect it again, so + that the Linear workspace's admins can end what they let fctl + workspaces reach, a former admin's own one too. A workspace that disconnects a Linear workspace hears nothing more from it and reaches none of its tickets, whatever other workspaces do; once no fctl workspace works in it, fctl revokes the app's token there (the From ab26f1ba4866ac1e89b982f79eb8c883c247b7ba Mon Sep 17 00:00:00 2001 From: "fctl-sh[bot]" <343971779+fctl-sh[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:35:51 +0000 Subject: [PATCH 3/7] fix(control): mark connections already revoked, and only revoke current tokens Connections to Linear workspaces whose installation was revoked before this change had no `revoked_at`, so installing the app again would have brought them all back: the migration marks them as of the revocation. A renewal that Linear refused revoked the installation even when another renewal had kept new tokens meanwhile, which now left every connection revoked for good. Revoking after a refused renewal now takes that the installation still has the tokens that were refused. --- .../0042_linear_connections_revoked.sql | 6 ++++++ crates/fctl-control/src/delegations.rs | 2 +- crates/fctl-control/src/linear.rs | 21 +++++++++++++------ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/crates/fctl-control/migrations/0042_linear_connections_revoked.sql b/crates/fctl-control/migrations/0042_linear_connections_revoked.sql index 05557bb3..70797e6c 100644 --- a/crates/fctl-control/migrations/0042_linear_connections_revoked.sql +++ b/crates/fctl-control/migrations/0042_linear_connections_revoked.sql @@ -7,3 +7,9 @@ -- which their Linear tab shows until their admins connect it again or -- disconnect it. ALTER TABLE workspace_linear ADD COLUMN revoked_at INTEGER; + +-- Those of installations Linear revoked before this: their connections end +-- too, rather than come back when someone installs the app again. +UPDATE workspace_linear SET revoked_at = (SELECT i.updated_at FROM linear_installations i + WHERE i.organization_id = workspace_linear.organization_id) +WHERE organization_id IN (SELECT organization_id FROM linear_installations WHERE status = 'revoked'); diff --git a/crates/fctl-control/src/delegations.rs b/crates/fctl-control/src/delegations.rs index b5c36266..4213c48d 100644 --- a/crates/fctl-control/src/delegations.rs +++ b/crates/fctl-control/src/delegations.rs @@ -1314,7 +1314,7 @@ pub async fn linked( /// fctl's app was uninstalled from a Linear workspace, or its access /// revoked there. pub async fn revoked(db: &D1Database, organization: &str, now: i64) -> AppResult<()> { - linear::mark_revoked(db, organization, now).await + linear::mark_revoked(db, organization, now, None).await } async fn audit( diff --git a/crates/fctl-control/src/linear.rs b/crates/fctl-control/src/linear.rs index b5168a01..70fec5ed 100644 --- a/crates/fctl-control/src/linear.rs +++ b/crates/fctl-control/src/linear.rs @@ -291,19 +291,28 @@ pub async fn store( /// installed from (`workspace_linear.revoked_at`), so that the Linear /// workspace's admins can end what they let fctl workspaces reach, a /// former admin's own one too. -pub async fn mark_revoked(db: &D1Database, organization: &str, now: i64) -> AppResult<()> { +/// `refused`: the sealed tokens whose renewal Linear refused, which revoke +/// it only while they are still the installation's: another renewal may +/// have kept new ones meanwhile, or an admin installed the app again. +pub async fn mark_revoked( + db: &D1Database, + organization: &str, + now: i64, + refused: Option<&str>, +) -> AppResult<()> { crate::db::batch( db, vec![ q( db, - "UPDATE linear_installations SET status = 'revoked', secret_enc = '', expires_at = NULL, updated_at = ? \ - WHERE organization_id = ?", - &[n(now), s(organization)], + "UPDATE linear_installations SET status = 'revoked', secret_enc = '', expires_at = NULL, updated_at = ?1 \ + WHERE organization_id = ?2 AND (?3 IS NULL OR secret_enc = ?3)", + &[n(now), s(organization), os(refused)], )?, q( db, - "UPDATE workspace_linear SET revoked_at = ? WHERE organization_id = ? AND revoked_at IS NULL", + "UPDATE workspace_linear SET revoked_at = ?1 WHERE organization_id = ?2 AND revoked_at IS NULL \ + AND EXISTS (SELECT 1 FROM linear_installations i WHERE i.organization_id = ?2 AND i.status = 'revoked')", &[n(now), s(organization)], )?, ], @@ -423,7 +432,7 @@ async fn renew( "Linear refused to renew the token of {}: {v}", row.organization_id ); - mark_revoked(db, &row.organization_id, now).await?; + mark_revoked(db, &row.organization_id, now, Some(&row.secret_enc)).await?; return Err(not_configured(&format!( "Linear no longer takes fctl's access to {}: an admin of the fctl workspace connects it again", row.name From f79e81b65257fe73ffcc6ebce028f4c809111e44 Mon Sep 17 00:00:00 2001 From: "fctl-sh[bot]" <343971779+fctl-sh[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:39:00 +0000 Subject: [PATCH 4/7] fix(control): let a renewal that won a race bring its connections back A renewal that kept new tokens after another renewal of the same token had given up made the installation active again, but the connections that giving up had revoked stayed revoked. The winner now clears the revocation of the connections revoked with the installation, as it takes the installation back. --- crates/fctl-control/src/linear.rs | 37 +++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/crates/fctl-control/src/linear.rs b/crates/fctl-control/src/linear.rs index 70fec5ed..66d36bbf 100644 --- a/crates/fctl-control/src/linear.rs +++ b/crates/fctl-control/src/linear.rs @@ -457,20 +457,33 @@ async fn renew( access_token: tokens.access_token.clone(), refresh_token: tokens.refresh_token.clone().or(secret.refresh_token), }; - q( + crate::db::batch( db, - // Active again, in case another renewal of the same token gave up - // meanwhile: this one won. - "UPDATE linear_installations SET secret_enc = ?, expires_at = ?, status = 'active', updated_at = ? \ - WHERE organization_id = ?", - &[ - s(&keys.seal_json(&aad(&row.organization_id), &new)?), - on(tokens.expires_at(now)), - n(now), - s(&row.organization_id), + vec![ + // The connections such a renewal revoked come back with it: those + // revoked when the installation was (`mark_revoked`). + q( + db, + "UPDATE workspace_linear SET revoked_at = NULL WHERE organization_id = ?1 \ + AND revoked_at = (SELECT updated_at FROM linear_installations \ + WHERE organization_id = ?1 AND status = 'revoked')", + &[s(&row.organization_id)], + )?, + q( + db, + // Active again, in case another renewal of the same token gave + // up meanwhile: this one won. + "UPDATE linear_installations SET secret_enc = ?, expires_at = ?, status = 'active', updated_at = ? \ + WHERE organization_id = ?", + &[ + s(&keys.seal_json(&aad(&row.organization_id), &new)?), + on(tokens.expires_at(now)), + n(now), + s(&row.organization_id), + ], + )?, ], - )? - .run() + ) .await?; Ok(new) } From dae1dae19d80e8350691c4d734c2ad8e6ff01849 Mon Sep 17 00:00:00 2001 From: "fctl-sh[bot]" <343971779+fctl-sh[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:34:22 +0000 Subject: [PATCH 5/7] fix: confirm it's you before linking a Linear account What a linked Linear account delegates to fctl, or mentions it in, starts sessions as its fctl person, on their Claude subscription, and its replies reach their sessions' agents. With a stolen browser session or token, someone could link their own Linear account to the victim's, with nothing to confirm and nothing said, and go on starting the victim's sessions from Linear long after the stolen token was revoked. Linking a Linear account is now a sensitive action (`Sensitive::LinkLinear`), which takes a passkey or a code where the instance asks for that: the app confirms before it goes on to Linear, and a link opened elsewhere (Linear's own button, the CLI) goes back to Settings to confirm and link again. A new link also emails the person when email is set up, saying which Linear account it is and what it does, as connecting a Claude subscription does. --- crates/fctl-control/src/api/linear.rs | 43 +++++++++++++++++++++++++- crates/fctl-domain/src/verify.rs | 7 +++++ docs/architecture.md | 8 ++++- docs/security.md | 9 +++--- web/src/components/project/tickets.tsx | 11 +++++-- web/src/lib/api.ts | 9 ++++++ web/src/lib/tickets.ts | 6 ++++ web/src/routes/settings.tsx | 11 +++++-- 8 files changed, 94 insertions(+), 10 deletions(-) diff --git a/crates/fctl-control/src/api/linear.rs b/crates/fctl-control/src/api/linear.rs index ed502515..5cb3e643 100644 --- a/crates/fctl-control/src/api/linear.rs +++ b/crates/fctl-control/src/api/linear.rs @@ -22,6 +22,7 @@ use crate::linear; use crate::workspaces as ws; use fctl_domain::cookies::Cookie; use fctl_domain::linear as lin; +use fctl_domain::verify::Sensitive; use fctl_proto::tickets::{ ConnectLinearRequest, LinearAccount, LinearLink, LinearTeamList, MyLinear, WorkspaceLinear, }; @@ -370,6 +371,14 @@ pub async fn link(ctx: &mut Ctx, req: &Request) -> AppResult { let thread = http::query(req, "thread").filter(|t| { !t.is_empty() && t.len() <= 64 && t.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-') }); + let project = clean_slug(http::query(req, "project").as_deref()); + // What the account delegates then starts sessions as them, so a stolen + // session can't link one alone, where the instance asks for that. The + // app confirms before it comes here; Linear's own button comes straight + // here, and goes back to where they confirm and link it again. + if ctx.verified(Sensitive::LinkLinear).is_err() { + return http::redirect(&Back::Person { project }.url("confirm")); + } begin( ctx, &keys, @@ -377,7 +386,7 @@ pub async fn link(ctx: &mut Ctx, req: &Request) -> AppResult { purpose: String::new(), user: user.id, workspace: None, - project: clean_slug(http::query(req, "project").as_deref()), + project, thread, state: String::new(), verifier: String::new(), @@ -607,6 +616,15 @@ async fn linked( Some(&viewer.organization), Some(json!({ "name": viewer.organization_name })), ); + // Best effort, as for a Claude connection: the account is linked, + // whatever the email does. + if ctx.cfg.email.is_some() { + let instance = crate::instance::name(&ctx.db, &ctx.cfg) + .await + .unwrap_or_else(|_| ctx.cfg.instance_name.clone()); + let (subject, text) = linked_notice(&instance, &user.email, &ctx.cfg, &viewer); + crate::email::notice(ctx, &user, &subject, text); + } let reached = crate::delegations::linked( &ctx.env, &ctx.cfg, @@ -624,6 +642,29 @@ async fn linked( Ok(thread.and(reached.first().cloned())) } +/// The email that tells someone a Linear account was linked to theirs: +/// what it delegates to fctl starts sessions as them from then on, so one +/// they didn't link is someone else starting their sessions. +fn linked_notice( + instance: &str, + email: &str, + cfg: &crate::config::Config, + viewer: &linear::Viewer, +) -> (String, String) { + let who = match viewer.email.as_deref() { + Some(e) if !e.is_empty() => format!("{} ({e})", viewer.name), + _ => viewer.name.clone(), + }; + ( + "A Linear account was linked to your account".to_owned(), + format!( + "{who}, in the Linear workspace {}, is now linked to your {instance} account ({email}): what it delegates to fctl there, or mentions fctl in, starts sessions as you, on your Claude subscription, and its replies reach your sessions' agents.\n\nIf you didn't link it, unlink it in Settings now, sign out your other devices there and change your password: {}", + viewer.organization_name, + cfg.url("/settings#linear") + ), + ) +} + // ───────────────────────────── the webhook ───────────────────────────── /// `POST /api/v1/linear/webhook`: Linear says that people delegated an diff --git a/crates/fctl-domain/src/verify.rs b/crates/fctl-domain/src/verify.rs index 82a87436..76613421 100644 --- a/crates/fctl-domain/src/verify.rs +++ b/crates/fctl-domain/src/verify.rs @@ -65,6 +65,12 @@ pub enum Sensitive { /// so a stolen session or token could otherwise have their sessions run /// where it watches, for good. ConnectClaude, + /// Linking a Linear account: what it delegates to fctl or mentions it + /// in then starts sessions as the person, on their subscription, and + /// its replies reach their sessions' agents, so a stolen session or + /// token could otherwise go on starting their sessions from Linear long + /// after it was revoked. + LinkLinear, /// Approving a CLI's sign-in, or confirming it's you for a CLI that is /// signed in already. ApproveCli, @@ -98,6 +104,7 @@ impl Sensitive { DeleteProject => "delete a project", NewToken => "make an access token", ConnectClaude => "connect a Claude subscription", + LinkLinear => "link a Linear account", ApproveCli => "approve the CLI", RemovePasskey => "remove a passkey", Export => "export your data", diff --git a/docs/architecture.md b/docs/architecture.md index 11b50cf2..78dbdc7a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1876,7 +1876,13 @@ issue's delegate (the person stays its assignee), and mention. Linear's own button in an agent session that waits for it): a Linear sign-in of theirs, with `read` alone, shows fctl who they are there (`viewer`), which D1 keeps (`linear_accounts`: their Linear id, name, - email and avatar), and fctl revokes the sign-in at once. A Linear + email and avatar), and fctl revokes the sign-in at once. Linking takes + confirming it's you where the instance asks for that + (`Sensitive::LinkLinear`), since what the account delegates then runs + as you: the app confirms before it goes on to Linear, and a link opened + from elsewhere (Linear's button, the CLI) goes back to Settings + (`?linear=confirm`) to confirm and link again. fctl emails the person + once it's linked, when email is set up. A Linear account is one person's: whoever links it last has it. Matching addresses would let anyone whose Linear address matches someone's run their work on that person's subscription, and an address in Linear says diff --git a/docs/security.md b/docs/security.md index 148920e8..17e4243b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -117,11 +117,12 @@ owner and purpose), so a value copied between rows does not decrypt. two-factor sign-in, removing a passkey, adding or registering a runner for a workspace and replacing its token, new access tokens and CLI sign-ins, connecting a Claude subscription (whose account a full login's - sessions show in), exporting your data, deleting accounts, projects and + sessions show in), linking a Linear account (whose delegations start + sessions as you), exporting your data, deleting accounts, projects and workspaces, and, without email, making someone a password reset link. - So a stolen browser session, CLI or - access token can't do them alone. Connecting a Claude subscription also - emails the person, when email is set up. A passkey (which must verify the + So a stolen browser session, CLI or access token can't do them alone. + Connecting a Claude subscription and linking a Linear account also + email the person, when email is set up. A passkey (which must verify the person), a code from the authenticator app or a recovery code confirms it, for 15 minutes, for the browser, CLI or token that confirmed; a sign-in with a passkey or a second factor counts. People with neither a diff --git a/web/src/components/project/tickets.tsx b/web/src/components/project/tickets.tsx index d1959b71..557bcbaa 100644 --- a/web/src/components/project/tickets.tsx +++ b/web/src/components/project/tickets.tsx @@ -38,7 +38,7 @@ import { toast, } from "@/components/ui"; import type * as G from "@/gen"; -import { ApiFailure, api, linearLinkUrl, workspaces } from "@/lib/api"; +import { ApiFailure, api, goConfirmed, linearLinkUrl, workspaces } from "@/lib/api"; import { cn } from "@/lib/cn"; import { agoShort, stamp } from "@/lib/format"; import { keys, queryClient, useInstance } from "@/lib/queries"; @@ -1236,7 +1236,14 @@ function YourAccount({ project, status }: { project: G.Project; status?: G.Proje Link your Linear account. fctl starts sessions only for people it knows in {name}, as them. It takes a Linear sign-in, which fctl forgets at once.

- + { + e.preventDefault(); + goConfirmed(linearLinkUrl(project.slug)).catch((err: Error) => toast(err.message, "error")); + }} + > diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 5779748e..95688eb5 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -42,6 +42,15 @@ export async function confirmFirst(): Promise { return (await confirmer?.()) ?? false; } +/** + * Follows a link that takes confirming it's you first, where the instance + * asks for it, as linking a Linear account does: the browser's own + * navigation can't go again by itself once the server refuses it. + */ +export async function goConfirmed(href: string): Promise { + if (await confirmFirst()) window.location.assign(href); +} + async function request(method: string, path: string, body?: unknown, again = true): Promise { let res: Response; try { diff --git a/web/src/lib/tickets.ts b/web/src/lib/tickets.ts index 7558c2eb..0713150e 100644 --- a/web/src/lib/tickets.ts +++ b/web/src/lib/tickets.ts @@ -47,6 +47,7 @@ export const LINEAR_OUTCOMES = [ "failed", "other_user", "not_app", + "confirm", ] as const; export type LinearOutcome = (typeof LINEAR_OUTCOMES)[number]; @@ -79,6 +80,11 @@ export function linearOutcome(o: LinearOutcome): { tone: "ok" | "warn" | "error" }; case "failed": return { tone: "error", text: "Linear didn't finish the sign-in. Try again in a minute." }; + case "confirm": + return { + tone: "warn", + text: "Linking a Linear account takes confirming it's you, so nothing changed. Link it again from here.", + }; } } diff --git a/web/src/routes/settings.tsx b/web/src/routes/settings.tsx index c7ecbb30..86700dc4 100644 --- a/web/src/routes/settings.tsx +++ b/web/src/routes/settings.tsx @@ -54,7 +54,7 @@ import { } from "@/components/ui"; import { WorkspaceAvatar } from "@/components/workspace/avatar"; import type * as G from "@/gen"; -import { api, confirmFirst, emails, linearLinkUrl, workspaces as wsApi } from "@/lib/api"; +import { api, confirmFirst, emails, goConfirmed, linearLinkUrl, workspaces as wsApi } from "@/lib/api"; import { avatarImage } from "@/lib/avatar-image"; import { tokenOnRunnersIn } from "@/lib/claude"; import { cn } from "@/lib/cn"; @@ -1067,7 +1067,14 @@ function LinearAccounts({ returned }: { returned?: LinearOutcome }) { )}
- + { + e.preventDefault(); + goConfirmed(linearLinkUrl()).catch((err: Error) => toast(err.message, "error")); + }} + > From d6aabe185312d724051e70bedfa5ae85d4d3d48c Mon Sep 17 00:00:00 2001 From: "fctl-sh[bot]" <343971779+fctl-sh[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:42:52 +0000 Subject: [PATCH 6/7] fix(control): keep a revoked Linear installation revoked Bringing connections back when a renewal won a race also brought them back when the uninstall webhook landed during a renewal, and a repeated revocation moved the time they were matched by. A renewal now keeps its new tokens only over the ones it renewed, and a revocation applies only to an active installation: once revoked (uninstalled, or a renewal of its current tokens refused), it stays so, with its connections, until an admin connects it again. Linear answers a renewal it already answered the same for half an hour, so two renewals of the same tokens don't refuse each other. --- crates/fctl-control/src/linear.rs | 48 +++++++++++++------------------ 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/crates/fctl-control/src/linear.rs b/crates/fctl-control/src/linear.rs index 66d36bbf..8447195f 100644 --- a/crates/fctl-control/src/linear.rs +++ b/crates/fctl-control/src/linear.rs @@ -292,8 +292,11 @@ pub async fn store( /// workspace's admins can end what they let fctl workspaces reach, a /// former admin's own one too. /// `refused`: the sealed tokens whose renewal Linear refused, which revoke -/// it only while they are still the installation's: another renewal may -/// have kept new ones meanwhile, or an admin installed the app again. +/// it only while they are still the installation's: an admin may have +/// installed the app again meanwhile. Linear answers a renewal it already +/// answered the same for half an hour, so two renewals of the same tokens +/// don't refuse each other. A revocation stays until an admin connects +/// the app again: a renewal keeps new tokens only over the ones it renewed. pub async fn mark_revoked( db: &D1Database, organization: &str, @@ -306,7 +309,7 @@ pub async fn mark_revoked( q( db, "UPDATE linear_installations SET status = 'revoked', secret_enc = '', expires_at = NULL, updated_at = ?1 \ - WHERE organization_id = ?2 AND (?3 IS NULL OR secret_enc = ?3)", + WHERE organization_id = ?2 AND status = 'active' AND (?3 IS NULL OR secret_enc = ?3)", &[n(now), s(organization), os(refused)], )?, q( @@ -457,33 +460,22 @@ async fn renew( access_token: tokens.access_token.clone(), refresh_token: tokens.refresh_token.clone().or(secret.refresh_token), }; - crate::db::batch( + q( db, - vec![ - // The connections such a renewal revoked come back with it: those - // revoked when the installation was (`mark_revoked`). - q( - db, - "UPDATE workspace_linear SET revoked_at = NULL WHERE organization_id = ?1 \ - AND revoked_at = (SELECT updated_at FROM linear_installations \ - WHERE organization_id = ?1 AND status = 'revoked')", - &[s(&row.organization_id)], - )?, - q( - db, - // Active again, in case another renewal of the same token gave - // up meanwhile: this one won. - "UPDATE linear_installations SET secret_enc = ?, expires_at = ?, status = 'active', updated_at = ? \ - WHERE organization_id = ?", - &[ - s(&keys.seal_json(&aad(&row.organization_id), &new)?), - on(tokens.expires_at(now)), - n(now), - s(&row.organization_id), - ], - )?, + // Only over the tokens it renewed: once the installation was revoked + // meanwhile (uninstalled, or its renewal refused), it stays so, with + // its connections, until an admin connects it again (`mark_revoked`). + "UPDATE linear_installations SET secret_enc = ?, expires_at = ?, updated_at = ? \ + WHERE organization_id = ? AND secret_enc = ?", + &[ + s(&keys.seal_json(&aad(&row.organization_id), &new)?), + on(tokens.expires_at(now)), + n(now), + s(&row.organization_id), + s(&row.secret_enc), ], - ) + )? + .run() .await?; Ok(new) } From 393e78c4f290de03bc2c654bcb5291566da4dc9d Mon Sep 17 00:00:00 2001 From: "fctl-sh[bot]" <343971779+fctl-sh[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:57:39 +0000 Subject: [PATCH 7/7] fix(web): let new-tab clicks on the Linear link through, and test it The links that link a Linear account confirmed it's you on every click, so one meant to open a new tab (with Cmd, Ctrl or Shift) went in the current one. `clickConfirmed` takes plain clicks alone and leaves the others to the browser; the server sends such a tab back to confirm where it must. Tests cover `goConfirmed`, `clickConfirmed`, the Tickets tab's link and the action's wording. --- crates/fctl-domain/src/verify.rs | 4 ++ web/src/components/project/tickets.test.tsx | 8 +++ web/src/components/project/tickets.tsx | 9 ++-- web/src/lib/api.test.ts | 55 ++++++++++++++++++++- web/src/lib/api.ts | 17 ++++++- web/src/routes/settings.tsx | 9 ++-- 6 files changed, 90 insertions(+), 12 deletions(-) diff --git a/crates/fctl-domain/src/verify.rs b/crates/fctl-domain/src/verify.rs index 76613421..ab114ba0 100644 --- a/crates/fctl-domain/src/verify.rs +++ b/crates/fctl-domain/src/verify.rs @@ -325,6 +325,10 @@ mod tests { message(Sensitive::ConnectClaude, APP), "Confirm it's you to connect a Claude subscription, with a code from your authenticator app." ); + assert_eq!( + message(Sensitive::LinkLinear, APP), + "Confirm it's you to link a Linear account, with a code from your authenticator app." + ); } #[test] diff --git a/web/src/components/project/tickets.test.tsx b/web/src/components/project/tickets.test.tsx index a661a4f7..623358c7 100644 --- a/web/src/components/project/tickets.test.tsx +++ b/web/src/components/project/tickets.test.tsx @@ -25,6 +25,11 @@ vi.mock("@/lib/api", () => { api: never, workspaces: never, linearLinkUrl: (project?: string) => `/api/v1/linear/link${project ? `?project=${project}` : ""}`, + clickConfirmed: (e: { preventDefault(): void }, href: string) => { + e.preventDefault(); + calls.push({ name: "clickConfirmed", args: [href] }); + return undefined; + }, ApiFailure: class extends Error {}, }; }); @@ -347,6 +352,9 @@ describe("the Tickets tab", () => { expect(c.textContent).toContain("assign one of ENG's issues to fctl"); const link = c.querySelector('a[href="/api/v1/linear/link?project=web"]'); expect(link?.textContent).toContain("Link Linear account"); + // Linking confirms it's you first, where the instance asks for that. + act(() => link?.click()); + expect(calls).toContainEqual({ name: "clickConfirmed", args: ["/api/v1/linear/link?project=web"] }); }); it("moves tickets through the team's states, each with a switch", () => { diff --git a/web/src/components/project/tickets.tsx b/web/src/components/project/tickets.tsx index 557bcbaa..28508593 100644 --- a/web/src/components/project/tickets.tsx +++ b/web/src/components/project/tickets.tsx @@ -38,7 +38,7 @@ import { toast, } from "@/components/ui"; import type * as G from "@/gen"; -import { ApiFailure, api, goConfirmed, linearLinkUrl, workspaces } from "@/lib/api"; +import { ApiFailure, api, clickConfirmed, linearLinkUrl, workspaces } from "@/lib/api"; import { cn } from "@/lib/cn"; import { agoShort, stamp } from "@/lib/format"; import { keys, queryClient, useInstance } from "@/lib/queries"; @@ -1239,10 +1239,9 @@ function YourAccount({ project, status }: { project: G.Project; status?: G.Proje { - e.preventDefault(); - goConfirmed(linearLinkUrl(project.slug)).catch((err: Error) => toast(err.message, "error")); - }} + onClick={(e) => + clickConfirmed(e, linearLinkUrl(project.slug))?.catch((err: Error) => toast(err.message, "error")) + } > diff --git a/web/src/lib/api.test.ts b/web/src/lib/api.test.ts index baaf487b..52b60acc 100644 --- a/web/src/lib/api.test.ts +++ b/web/src/lib/api.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { ApiFailure, api, confirmFirst, setConfirmer } from "./api"; +import { ApiFailure, api, clickConfirmed, confirmFirst, goConfirmed, setConfirmer } from "./api"; const asked = { code: "verification_required", @@ -113,3 +113,56 @@ describe("before a download", () => { await expect(confirmFirst()).resolves.toBe(false); }); }); + +describe("links that take confirming it's you", () => { + const status = (required: boolean, verified: boolean) => ({ + status: 200, + body: { required, verified, methods: ["passkey"] }, + }); + const LINK = "/api/v1/linear/link"; + /** The browser's own navigation, as a spy. */ + function navigation() { + const assign = vi.fn(); + vi.stubGlobal("location", { assign }); + return assign; + } + const click = (keys: Partial> = {}) => ({ + button: 0, + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + ...keys, + preventDefault: vi.fn(), + }); + + it("goes once confirmed, and stays when the person cancels", async () => { + const assign = navigation(); + answer(status(true, false)); + confirmer(true); + await goConfirmed(LINK); + expect(assign).toHaveBeenCalledWith(LINK); + for (const u of unregister.splice(0)) u(); + assign.mockClear(); + answer(status(true, false)); + confirmer(false); + await goConfirmed(LINK); + expect(assign).not.toHaveBeenCalled(); + }); + + it("takes a plain click, and leaves one that opens a new tab to the browser", async () => { + const assign = navigation(); + answer(status(false, false)); + const plain = click(); + await clickConfirmed(plain, LINK); + expect(plain.preventDefault).toHaveBeenCalled(); + expect(assign).toHaveBeenCalledWith(LINK); + assign.mockClear(); + for (const keys of [{ metaKey: true }, { ctrlKey: true }, { shiftKey: true }]) { + const e = click(keys); + expect(clickConfirmed(e, LINK)).toBeUndefined(); + expect(e.preventDefault).not.toHaveBeenCalled(); + } + expect(assign).not.toHaveBeenCalled(); + }); +}); diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 95688eb5..387112fb 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -48,7 +48,22 @@ export async function confirmFirst(): Promise { * navigation can't go again by itself once the server refuses it. */ export async function goConfirmed(href: string): Promise { - if (await confirmFirst()) window.location.assign(href); + if (await confirmFirst()) location.assign(href); +} + +/** + * A click on a link to `href` that takes confirming it's you first + * ([`goConfirmed`]): a plain click confirms, then goes. One that opens a + * new tab or window (with a modifier key) goes on as the browser does, and + * the server sends that tab back to confirm where it must. + */ +export function clickConfirmed( + e: Pick, + href: string, +): Promise | undefined { + if (e.button !== 0 || e.metaKey || e.ctrlKey || e.shiftKey || e.altKey) return undefined; + e.preventDefault(); + return goConfirmed(href); } async function request(method: string, path: string, body?: unknown, again = true): Promise { diff --git a/web/src/routes/settings.tsx b/web/src/routes/settings.tsx index 86700dc4..8614da0c 100644 --- a/web/src/routes/settings.tsx +++ b/web/src/routes/settings.tsx @@ -54,7 +54,7 @@ import { } from "@/components/ui"; import { WorkspaceAvatar } from "@/components/workspace/avatar"; import type * as G from "@/gen"; -import { api, confirmFirst, emails, goConfirmed, linearLinkUrl, workspaces as wsApi } from "@/lib/api"; +import { api, clickConfirmed, confirmFirst, emails, linearLinkUrl, workspaces as wsApi } from "@/lib/api"; import { avatarImage } from "@/lib/avatar-image"; import { tokenOnRunnersIn } from "@/lib/claude"; import { cn } from "@/lib/cn"; @@ -1070,10 +1070,9 @@ function LinearAccounts({ returned }: { returned?: LinearOutcome }) { { - e.preventDefault(); - goConfirmed(linearLinkUrl()).catch((err: Error) => toast(err.message, "error")); - }} + onClick={(e) => + clickConfirmed(e, linearLinkUrl())?.catch((err: Error) => toast(err.message, "error")) + } >