From a40f9596b1f748e5412b559fdf0ca18657d5f39e Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Fri, 11 Sep 2026 19:18:26 -0400 Subject: [PATCH] fix(oauth): name the scope ceiling as what allows the scopes on the consent page The consent page and didbot-oauth said "current policies will allow these scopes" over a list only the scope ceiling produced, so a token policy refusing the exchange or a write policy refusing a write contradicted a promise the page had already made. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: I08711d22e6b64c82ad22bfaf95370ac90b392d9d --- crates/didbot-agentd/src/bin/didbot-oauth.rs | 14 +++++++++----- crates/didbot-serve/src/oauth/par.rs | 5 ++++- crates/didbot-serve/src/routes.rs | 17 ++++++++++------- crates/didbot-serve/tests/oauth_agent_flow.rs | 17 +++++++++++++++-- docs/agentd.md | 2 +- 5 files changed, 39 insertions(+), 16 deletions(-) diff --git a/crates/didbot-agentd/src/bin/didbot-oauth.rs b/crates/didbot-agentd/src/bin/didbot-oauth.rs index 850e3376..d43c8477 100644 --- a/crates/didbot-agentd/src/bin/didbot-oauth.rs +++ b/crates/didbot-agentd/src/bin/didbot-oauth.rs @@ -45,8 +45,12 @@ account. --direct insists on it rather than trying the socket first. "; /// What an approval's answer says of the login it made, whichever mode it -/// came through: the scopes requested, then the scopes current policies -/// allow. +/// came through: the scopes requested, then the scopes the scope ceiling +/// allows. +/// +/// Named for the check that produced the second list. A token policy can +/// still refuse the exchange, and a write policy can still refuse a write a +/// listed scope covers. fn print_scopes(requested: &[String], granted: &[String]) { print!("{}", scope_block(requested, granted)); } @@ -59,9 +63,9 @@ fn scope_block(requested: &[String], granted: &[String]) -> String { block.push_str(&format!("* {}\n", printable(scope))); } if granted == requested { - block.push_str("Current policies allow these scopes:\n"); + block.push_str("The scope ceiling allows these scopes:\n"); } else { - block.push_str("Current policies only allow these scopes:\n"); + block.push_str("The scope ceiling allows only these scopes:\n"); } for scope in granted { block.push_str(&format!("* {}\n", printable(scope))); @@ -348,7 +352,7 @@ mod tests { block.contains(r"* repo:generic\u{1b}[2K\u{1b}[Aadmin"), "{block}" ); - assert!(block.contains("Current policies only allow these scopes:")); + assert!(block.contains("The scope ceiling allows only these scopes:")); } #[test] diff --git a/crates/didbot-serve/src/oauth/par.rs b/crates/didbot-serve/src/oauth/par.rs index 7d5670af..c02afefd 100644 --- a/crates/didbot-serve/src/oauth/par.rs +++ b/crates/didbot-serve/src/oauth/par.rs @@ -1087,7 +1087,10 @@ mod tests { .consume(&reference) .expect("the reference is live"); assert_eq!(pending.granted_scope.to_string(), "atproto"); - assert_eq!(record.requested.to_string(), "atproto repo:app.bsky.feed.post"); + assert_eq!( + record.requested.to_string(), + "atproto repo:app.bsky.feed.post" + ); } /// `plan/scope-policy.md`'s "log every scopedown", and the shape the diff --git a/crates/didbot-serve/src/routes.rs b/crates/didbot-serve/src/routes.rs index f23c92b7..e4da9a7b 100644 --- a/crates/didbot-serve/src/routes.rs +++ b/crates/didbot-serve/src/routes.rs @@ -1370,10 +1370,9 @@ fn authorize_error_response(err: crate::oauth::authorize::AuthorizeError) -> Res /// The heading is a placeholder, per this project's copywriting rule. The /// rest is plain labels over the attributes, which are the load-bearing part. /// -/// The page lists the scopes requested, then the scopes current policies -/// would allow if approved. Approving covers that second list, and the -/// ceiling is asked again at every use (`data-approves`, -/// `data-ceiling-checked`). +/// The page lists the scopes requested, then the scopes the scope ceiling +/// allows. Approving covers that second list, and the ceiling is asked again +/// at every use (`data-approves`, `data-ceiling-checked`). /// /// There is no form. Approving is `bot.did.approveAuthorization`, which /// takes the account's own agent token as `Credential::AgentSelf` — a @@ -1409,16 +1408,20 @@ fn authorize_page(pending: &crate::oauth::authorize::AuthorizePending) -> String .collect(); format!("") }; + // Named for the check that produced it. The scope ceiling is the only + // thing that decides this list; a token policy can still refuse the + // exchange, and a write policy can still refuse a write a listed scope + // covers, so a list labelled "policies" would promise their answers too. let allowed = match &record.verdict { Verdict::Allow => format!( - "If approved, current policies will allow these scopes:{}", + "The scope ceiling allows these scopes:{}", items(&record.requested) ), Verdict::Narrow { granted, .. } => format!( - "If approved, current policies will only allow these scopes:{}", + "The scope ceiling allows only these scopes:{}", items(granted) ), - Verdict::Deny { .. } => "Current policies do not allow this request.".to_owned(), + Verdict::Deny { .. } => "No scopes are granted.".to_owned(), }; let approves = if record.verdict.is_deny() { "" diff --git a/crates/didbot-serve/tests/oauth_agent_flow.rs b/crates/didbot-serve/tests/oauth_agent_flow.rs index e7557398..5c2eca13 100644 --- a/crates/didbot-serve/tests/oauth_agent_flow.rs +++ b/crates/didbot-serve/tests/oauth_agent_flow.rs @@ -879,6 +879,19 @@ async fn a_policy_loaded_after_par_leaves_the_record_and_refuses_the_token() { assert_eq!(status, StatusCode::OK, "{page}"); assert!(page.contains("data-approval-token")); + // The list is the scope ceiling's answer, and says so. The denial the + // operator just loaded is about to refuse the exchange below, so a page + // claiming policies allow what it lists would be claiming an answer no + // policy gave. + assert!( + page.contains("The scope ceiling allows these scopes:"), + "{page}" + ); + assert!( + !page.contains("policies will allow") && !page.contains("policies allow"), + "the page promises an answer the token gate is about to refuse: {page}" + ); + let (status, answer) = post_xrpc( &fixture.app, "bot.did.approveAuthorization", @@ -1412,7 +1425,7 @@ async fn a_narrowed_request_issues_a_token_at_the_granted_scopes_and_names_the_c page.contains("data-approves=\"granted\" data-ceiling-checked=\"each-use\""), "{page}" ); - // The scopes requested, then the scopes current policies allow. + // The scopes requested, then the scopes the scope ceiling allows. assert!( page.contains( "This application has requested these scopes: