fix: confirm it's you before linking a Linear account

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,14 +371,22 @@ pub async fn link(ctx: &mut Ctx, req: &Request) -> AppResult<Response> {
     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,
         SignIn {
             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

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",
@@ -318,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]

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

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

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<HTMLAnchorElement>('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", () => {

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, clickConfirmed, 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,13 @@ function YourAccount({ project, status }: { project: G.Project; status?: G.Proje
         <span className="font-medium text-ink">Link your Linear account.</span> fctl starts sessions only for
         people it knows in {name}, as them. It takes a Linear sign-in, which fctl forgets at once.
       </p>
-      <a href={linearLinkUrl(project.slug)} className="shrink-0">
+      <a
+        href={linearLinkUrl(project.slug)}
+        className="shrink-0"
+        onClick={(e) =>
+          clickConfirmed(e, linearLinkUrl(project.slug))?.catch((err: Error) => toast(err.message, "error"))
+        }
+      >
         <Button icon={<LinearIcon className="size-3.5" />}>Link Linear account</Button>
       </a>
     </div>

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<Record<"metaKey" | "ctrlKey" | "shiftKey" | "altKey", boolean>> = {}) => ({
+    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();
+  });
+});

web/src/lib/api.ts

@@ -42,6 +42,30 @@ export async function confirmFirst(): Promise<boolean> {
   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<void> {
+  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<MouseEvent, "button" | "metaKey" | "ctrlKey" | "shiftKey" | "altKey" | "preventDefault">,
+  href: string,
+): Promise<void> | undefined {
+  if (e.button !== 0 || e.metaKey || e.ctrlKey || e.shiftKey || e.altKey) return undefined;
+  e.preventDefault();
+  return goConfirmed(href);
+}
+
 async function request<T>(method: string, path: string, body?: unknown, again = true): Promise<T> {
   let res: Response;
   try {

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.",
+      };
   }
 }
 

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, 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";
@@ -1067,7 +1067,13 @@ function LinearAccounts({ returned }: { returned?: LinearOutcome }) {
             </ul>
           )}
           <div>
-            <a href={linearLinkUrl()} className="inline-block">
+            <a
+              href={linearLinkUrl()}
+              className="inline-block"
+              onClick={(e) =>
+                clickConfirmed(e, linearLinkUrl())?.catch((err: Error) => toast(err.message, "error"))
+              }
+            >
               <Button icon={<LinearIcon className="size-3.5" />}>
                 {accounts.length > 0 ? "Link another Linear account" : "Link your Linear account"}
               </Button>