Skip to content

Commit 236d1ec

Browse files
peonkalvinnchau
andcommitted
fix(doctor): authorize agent sign-in by backend-owned capability
The offered-state auth gate required the current doctor check to offer FixType::Auth, which is impossible for an installed Copilot: the pinned doctor crate defines auth_command but no auth_status_command, so check_single_ai_agent reports AuthStatus::NotApplicable, status Pass, and fix_type None. The gate therefore blocked Copilot's only supported sign-in path (the product always presents Sign in for it via supportsAuth && !supportsAuthStatus). Authorize Auth against a backend-owned AuthCapability read from the pinned AI_AGENT_CHECKS table instead of trusting offered-state alone: - None (no auth_command, e.g. goose/pi): sign-in never authorized. - Probeable (has an auth-status probe, e.g. claude/codex/amp/cursor): authorized only when the check currently offers Auth, unchanged. - Unprobeable (login command but no status probe, i.e. Copilot): Doctor can't report Auth, so authorize the registered login when the agent is installed (path/bridge_path resolved) and offers no install fix. A not-installed provider still offers Command, so a forged sign-in rejects. The capability oracle is the static crate table, not renderer input; an unknown check id has no capability and rejects. Tests: replace the offered-state auth regressions with capability-based ones — probeable forged/offered cases, installed-Copilot allowed despite no offered fix, not-installed Copilot rejected, no-login-flow agent rejected, and an auth_capability table-mapping regression. Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
1 parent da6333a commit 236d1ec

1 file changed

Lines changed: 151 additions & 33 deletions

File tree

src-tauri/src/commands/agent_setup.rs

Lines changed: 151 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -597,22 +597,86 @@ fn resolve_update_command(
597597
})
598598
}
599599

600+
/// The sign-in capability the pinned doctor crate declares for a check, read
601+
/// from the backend-owned `AI_AGENT_CHECKS` table — never renderer input. It is
602+
/// the authorization oracle for [`authorize_auth`]: "currently offers `Auth`"
603+
/// is only valid for agents Doctor can actually probe.
604+
#[derive(Debug, PartialEq, Eq)]
605+
enum AuthCapability {
606+
/// No `auth_command` (goose, pi): the agent has no sign-in flow, so a
607+
/// renderer-requested `Auth` is never authorized.
608+
None,
609+
/// Both a login command and a status probe (claude, codex, amp, cursor):
610+
/// Doctor reports `Auth` when installed-but-signed-out, so authorize a
611+
/// sign-in only when the check currently offers it.
612+
Probeable,
613+
/// A login command but no status probe (copilot): Doctor can't observe the
614+
/// auth state, so it reports `Pass`/no fix even when a sign-in is needed.
615+
/// Authorize the registered login for the installed agent directly.
616+
Unprobeable,
617+
}
618+
619+
/// The sign-in capability the pinned crate declares for a crate check id
620+
/// (`ai-agent-*`). Resolved from the static `AI_AGENT_CHECKS` table so the auth
621+
/// gate authorizes against backend-owned recipe metadata, not a renderer claim.
622+
/// An unknown id has no capability, so auth is never authorized for it.
623+
fn auth_capability(check_id: &str) -> AuthCapability {
624+
doctor::agents::AI_AGENT_CHECKS
625+
.iter()
626+
.find(|info| info.id == check_id)
627+
.map(|info| {
628+
match (
629+
info.auth_command.is_some(),
630+
info.auth_status_command.is_some(),
631+
) {
632+
(false, _) => AuthCapability::None,
633+
(true, true) => AuthCapability::Probeable,
634+
(true, false) => AuthCapability::Unprobeable,
635+
}
636+
})
637+
.unwrap_or(AuthCapability::None)
638+
}
639+
600640
/// Authorize a renderer-requested `Auth` action against the provider's current
601-
/// doctor state (`offered` = the fix the check actually offers now). Sign-in is
602-
/// only authorized when the check currently offers `Auth` (installed but not
603-
/// authenticated); a request against a check that offers a different fix
604-
/// (missing install → `Command`/`Bridge`) or no fix at all (already
605-
/// authenticated, or auth status unknown) fails closed before the auth shell
606-
/// command runs. Pure so the boundary is unit-testable without a Tauri handle.
607-
fn authorize_auth(provider_id: &str, offered: Option<FixType>) -> Result<(), String> {
608-
match offered {
609-
Some(FixType::Auth) => Ok(()),
610-
Some(offered) => Err(format!(
611-
"'{provider_id}' currently offers the '{offered:?}' fix, not sign-in"
612-
)),
613-
None => Err(format!(
614-
"'{provider_id}' offers no sign-in in its current state"
615-
)),
641+
/// doctor `check` and its backend-owned [`AuthCapability`]. Sign-in fails closed
642+
/// unless the pinned crate declares a login flow for the check:
643+
///
644+
/// - `None` (no `auth_command`): never authorized.
645+
/// - `Probeable` (has an auth-status probe): authorized only when the check
646+
/// currently offers `Auth` (installed but not authenticated); a check that
647+
/// offers an install fix or is already authenticated (no fix) rejects.
648+
/// - `Unprobeable` (a login command but no status probe, e.g. Copilot): Doctor
649+
/// can't report `Auth`, so authorize the registered login when the agent is
650+
/// installed (`path`/`bridge_path` resolved) and offers no install fix. A
651+
/// not-installed provider still offers `Command`, so a forged sign-in against
652+
/// it rejects.
653+
///
654+
/// Pure so the boundary is unit-testable without a Tauri handle.
655+
fn authorize_auth(provider_id: &str, check: &doctor::DoctorCheck) -> Result<(), String> {
656+
match auth_capability(&check.id) {
657+
AuthCapability::None => Err(format!("'{provider_id}' has no sign-in flow")),
658+
AuthCapability::Probeable => match &check.fix_type {
659+
Some(FixType::Auth) => Ok(()),
660+
Some(offered) => Err(format!(
661+
"'{provider_id}' currently offers the '{offered:?}' fix, not sign-in"
662+
)),
663+
None => Err(format!(
664+
"'{provider_id}' offers no sign-in in its current state"
665+
)),
666+
},
667+
AuthCapability::Unprobeable => {
668+
if check.path.is_none() && check.bridge_path.is_none() {
669+
return Err(format!(
670+
"'{provider_id}' is not installed, so sign-in is unavailable"
671+
));
672+
}
673+
match &check.fix_type {
674+
None => Ok(()),
675+
Some(offered) => Err(format!(
676+
"'{provider_id}' currently offers the '{offered:?}' fix, not sign-in"
677+
)),
678+
}
679+
}
616680
}
617681
}
618682

@@ -823,13 +887,14 @@ async fn run_auth(
823887
) -> Result<(), String> {
824888
// The renderer only names the *action*; before running the auth shell
825889
// command we re-read the provider's doctor check and authorize `Auth`
826-
// against the fix it currently offers. A forged/stale sign-in for a
827-
// provider that is missing (offers `Command`/`Bridge`), already
828-
// authenticated, or has an unknown auth state (offers no fix) rejects here
829-
// rather than executing the static `<agent> login` command on demand.
830-
// `find_check` also fails closed on an unknown provider.
890+
// against its backend-owned sign-in capability (see [`authorize_auth`]). A
891+
// forged/stale sign-in rejects here — for a probe-capable agent unless it
892+
// currently offers `Auth`, and for an unprobeable one (Copilot) unless the
893+
// agent is installed and offers no install fix — rather than executing the
894+
// static `<agent> login` command on demand. `find_check` also fails closed
895+
// on an unknown provider.
831896
let check = find_check(app, provider_id).await?;
832-
authorize_auth(provider_id, check.fix_type)?;
897+
authorize_auth(provider_id, &check)?;
833898
set_phase(app, registry, provider_id, SetupPhase::Authenticating);
834899
run_fix(app, registry, provider_id, FixType::Auth, None).await?;
835900

@@ -1283,21 +1348,74 @@ mod tests {
12831348
}
12841349

12851350
#[test]
1286-
fn authorize_auth_rejects_a_forged_sign_in_against_a_non_auth_state() {
1287-
// Regression for the auth bypass: `run_auth` re-reads the provider check
1288-
// and authorizes `Auth` against its current offered fix. A provider that
1289-
// is missing (offers `Command`/`Bridge`) or already authenticated
1290-
// (offers no fix) must reject before the `<agent> login` command runs.
1291-
assert!(authorize_auth("copilot-acp", Some(FixType::Command)).is_err());
1292-
assert!(authorize_auth("amp-acp", Some(FixType::Bridge)).is_err());
1293-
assert!(authorize_auth("copilot-acp", None).is_err());
1351+
fn authorize_auth_rejects_forged_sign_in_for_a_probeable_agent() {
1352+
// Probe-capable agents (claude/codex/amp/cursor) report `Auth` only when
1353+
// installed-but-signed-out, so a forged sign-in against a missing
1354+
// (offers `Command`/`Bridge`) or already-authenticated (no fix) codex
1355+
// check must reject before the `<agent> login` command runs.
1356+
let mut check = check_with_fix(Some(FixType::Command)); // id = ai-agent-codex
1357+
assert!(authorize_auth("codex-acp", &check).is_err());
1358+
check.fix_type = Some(FixType::Bridge);
1359+
assert!(authorize_auth("codex-acp", &check).is_err());
1360+
check.fix_type = None;
1361+
assert!(authorize_auth("codex-acp", &check).is_err());
12941362
}
12951363

12961364
#[test]
1297-
fn authorize_auth_allows_a_currently_offered_sign_in() {
1298-
// Installed-but-signed-out is exactly the state that offers `Auth`, so a
1299-
// legitimate sign-in is authorized and reaches the auth command.
1300-
assert!(authorize_auth("copilot-acp", Some(FixType::Auth)).is_ok());
1365+
fn authorize_auth_allows_a_currently_offered_sign_in_for_a_probeable_agent() {
1366+
// Installed-but-signed-out is the state a probe-capable agent reports as
1367+
// offering `Auth`, so a legitimate sign-in is authorized.
1368+
let check = check_with_fix(Some(FixType::Auth)); // id = ai-agent-codex
1369+
assert!(authorize_auth("codex-acp", &check).is_ok());
1370+
}
1371+
1372+
#[test]
1373+
fn authorize_auth_allows_installed_copilot_despite_no_offered_fix() {
1374+
// Regression for the Copilot auth-gate defect: Copilot declares an
1375+
// `auth_command` but no `auth_status_command`, so Doctor reports
1376+
// `Pass`/`fix_type = None` even when a sign-in is needed. The gate must
1377+
// authorize the registered login for the *installed* agent (resolved
1378+
// `path`) rather than blocking its only sign-in path.
1379+
let mut check = check_with_fix(None);
1380+
check.id = "ai-agent-copilot".into();
1381+
check.path = Some("/opt/homebrew/bin/copilot".into());
1382+
assert!(authorize_auth("copilot-acp", &check).is_ok());
1383+
}
1384+
1385+
#[test]
1386+
fn authorize_auth_rejects_copilot_sign_in_when_not_installed() {
1387+
// A not-installed Copilot still offers a `Command` install fix and has no
1388+
// resolved binary, so a forged sign-in against it must reject — the
1389+
// unprobeable capability authorizes login only for an installed agent.
1390+
let mut check = check_with_fix(Some(FixType::Command));
1391+
check.id = "ai-agent-copilot".into();
1392+
check.path = None;
1393+
check.bridge_path = None;
1394+
assert!(authorize_auth("copilot-acp", &check).is_err());
1395+
}
1396+
1397+
#[test]
1398+
fn authorize_auth_rejects_sign_in_for_an_agent_with_no_login_flow() {
1399+
// Goose declares no `auth_command`, so a renderer-requested sign-in is
1400+
// never authorized regardless of the reported check state.
1401+
let mut check = check_with_fix(None);
1402+
check.id = "ai-agent-goose".into();
1403+
check.path = Some("/usr/local/bin/goose".into());
1404+
assert!(authorize_auth("goose", &check).is_err());
1405+
}
1406+
1407+
#[test]
1408+
fn auth_capability_reflects_the_pinned_crate_table() {
1409+
// The gate's oracle is the backend-owned `AI_AGENT_CHECKS` table, not a
1410+
// renderer claim: probe-capable agents, the unprobeable Copilot, the
1411+
// no-login goose, and an unknown id each resolve to their capability.
1412+
assert_eq!(auth_capability("ai-agent-codex"), AuthCapability::Probeable);
1413+
assert_eq!(
1414+
auth_capability("ai-agent-copilot"),
1415+
AuthCapability::Unprobeable
1416+
);
1417+
assert_eq!(auth_capability("ai-agent-goose"), AuthCapability::None);
1418+
assert_eq!(auth_capability("ai-agent-nope"), AuthCapability::None);
13011419
}
13021420

13031421
#[test]

0 commit comments

Comments
 (0)