refactor: dedupe auth helpers and tidy principal tests

Replace hand-built RequestAuthContext literals in github_webhook with the
existing ::invalid()/::authenticated() constructors, collapse the duplicate
demo/real principal layers into a single cloneable layer, and forward the
_with_anyhow error constructors to their _with_source twins to drop the
duplicated cause-collection bodies. refresh_credential_from_headers now
reuses jwt_auth::bearer_token_from_headers for Authorization parsing.
test_support shares one TEST_DEV_TOKEN-derived bearer header instead of a
hand-pasted literal. Replace for-loops in principal/cli_flow tests with
per-variant cases to honor the no-loops-in-tests rule.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

lib/crates/fabro-server/src/auth/cli_flow.rs

@@ -28,7 +28,7 @@ use url::{Host, Url};
 
 use crate::auth::browser_shell::browser_shell;
 use crate::auth::{self, AuthCode, ConsumeOutcome, JwtSubject, REFRESH_TOKEN_PREFIX, RefreshToken};
-use crate::jwt_auth::{AuthMode, ConfiguredAuth};
+use crate::jwt_auth::{AuthMode, ConfiguredAuth, bearer_token_from_headers};
 use crate::principal_middleware::{AuthContextSlot, AuthStatus, RequestAuth, RequestAuthContext};
 use crate::server::AppState;
 use crate::web_auth::{
@@ -946,18 +946,13 @@ enum RefreshCredential {
 }
 
 fn refresh_credential_from_headers(headers: &HeaderMap) -> RefreshCredential {
-    let Some(value) = headers.get(header::AUTHORIZATION) else {
-        return RefreshCredential::Missing;
-    };
-    let Ok(value) = value.to_str() else {
-        return RefreshCredential::Invalid;
-    };
-    let Some(bearer) = value.strip_prefix("Bearer ") else {
-        return RefreshCredential::Invalid;
-    };
-    match bearer.strip_prefix(REFRESH_TOKEN_PREFIX) {
-        Some(secret) => RefreshCredential::Present(secret.to_string()),
-        None => RefreshCredential::Invalid,
+    match bearer_token_from_headers(headers) {
+        None => RefreshCredential::Missing,
+        Some(Err(_)) => RefreshCredential::Invalid,
+        Some(Ok(bearer)) => match bearer.strip_prefix(REFRESH_TOKEN_PREFIX) {
+            Some(secret) => RefreshCredential::Present(secret.to_string()),
+            None => RefreshCredential::Invalid,
+        },
     }
 }
 
@@ -1688,11 +1683,14 @@ client_id = "github-client-id"
         assert_eq!(confirm.status(), StatusCode::SEE_OTHER);
 
         let contexts = captured.lock().expect("captured auth contexts").clone();
-        assert_eq!(contexts.len(), 3);
-        for context in contexts {
-            assert_eq!(context.auth_status, AuthStatus::Authenticated);
-            assert_eq!(context.principal.display(), "octocat");
-        }
+        let [first, second, third] = <[RequestAuthContext; 3]>::try_from(contexts)
+            .expect("expected three captured auth contexts");
+        assert_eq!(first.auth_status, AuthStatus::Authenticated);
+        assert_eq!(first.principal.display(), "octocat");
+        assert_eq!(second.auth_status, AuthStatus::Authenticated);
+        assert_eq!(second.principal.display(), "octocat");
+        assert_eq!(third.auth_status, AuthStatus::Authenticated);
+        assert_eq!(third.principal.display(), "octocat");
     }
 
     #[tokio::test]

lib/crates/fabro-server/src/server.rs

@@ -122,9 +122,8 @@ use crate::github_webhooks::{
 use crate::ip_allowlist::{IpAllowlistConfig, ip_allowlist_middleware};
 use crate::jwt_auth::{self, AuthMode};
 use crate::principal_middleware::{
-    AuthContextSlot, AuthStatus, RequestAuth, RequestAuthContext, RequireCommandLog,
-    RequireRunBlob, RequireRunScoped, RequireStageArtifact, RequiredUser, principal_middleware,
-    require_user,
+    AuthContextSlot, RequestAuth, RequestAuthContext, RequireCommandLog, RequireRunBlob,
+    RequireRunScoped, RequireStageArtifact, RequiredUser, principal_middleware, require_user,
 };
 use crate::request_id::{self, RequestId};
 use crate::run_files::{FilesInFlight, list_run_files, new_files_in_flight};
@@ -975,10 +974,7 @@ pub fn build_router_with_options(
         .clone()
         .unwrap_or_else(|| Arc::new(GithubEndpoints::production_defaults()));
     let webhook_secret = state.server_secret(WEBHOOK_SECRET_ENV);
-    let demo_principal_layer =
-        middleware::from_fn_with_state(Arc::clone(&state), principal_middleware);
-    let real_principal_layer =
-        middleware::from_fn_with_state(Arc::clone(&state), principal_middleware);
+    let principal_layer = middleware::from_fn_with_state(Arc::clone(&state), principal_middleware);
     let api_common = if web_enabled {
         Router::new()
             .route("/openapi.json", get(openapi_spec))
@@ -993,7 +989,7 @@ pub fn build_router_with_options(
             api_common
                 .clone()
                 .merge(demo_routes())
-                .layer(demo_principal_layer),
+                .layer(principal_layer.clone()),
         )
         .layer(axum::Extension(auth_mode.clone()))
         .layer(axum::Extension(Arc::clone(&github_endpoints)))
@@ -1001,7 +997,7 @@ pub fn build_router_with_options(
 
     let mut real_router = Router::new().nest(
         "/api/v1",
-        api_common.merge(real_routes()).layer(real_principal_layer),
+        api_common.merge(real_routes()).layer(principal_layer),
     );
     if web_enabled {
         real_router = real_router.nest("/auth", web_auth::routes().merge(auth::web_routes()));
@@ -1402,35 +1398,23 @@ async fn github_webhook(
         .get("x-hub-signature-256")
         .and_then(|value| value.to_str().ok())
     else {
-        auth_slot.replace(RequestAuthContext {
-            principal:       Principal::Anonymous,
-            auth_status:     AuthStatus::Invalid,
-            auth_error_code: Some("unauthorized"),
-            user_profile:    None,
-        });
+        auth_slot.replace(RequestAuthContext::invalid());
         warn!(delivery = %delivery_id, "Webhook missing X-Hub-Signature-256 header");
         return StatusCode::UNAUTHORIZED;
     };
 
     if !verify_signature(&secret, &body, signature) {
-        auth_slot.replace(RequestAuthContext {
-            principal:       Principal::Anonymous,
-            auth_status:     AuthStatus::Invalid,
-            auth_error_code: Some("unauthorized"),
-            user_profile:    None,
-        });
+        auth_slot.replace(RequestAuthContext::invalid());
         warn!(delivery = %delivery_id, "Webhook HMAC signature mismatch");
         return StatusCode::UNAUTHORIZED;
     }
 
-    auth_slot.replace(RequestAuthContext {
-        principal:       Principal::Webhook {
+    auth_slot.replace(RequestAuthContext::authenticated(
+        Principal::Webhook {
             delivery_id: delivery_id.to_string(),
         },
-        auth_status:     AuthStatus::Authenticated,
-        auth_error_code: None,
-        user_profile:    None,
-    });
+        None,
+    ));
 
     let event_type = headers
         .get("x-github-event")

lib/crates/fabro-server/src/test_support.rs

@@ -1,4 +1,4 @@
-use std::sync::Arc;
+use std::sync::{Arc, OnceLock};
 
 use axum::extract::Request;
 use axum::http::{HeaderValue, header};
@@ -57,12 +57,13 @@ pub fn with_test_user(router: Router) -> Router {
 
 async fn inject_test_user_bearer(mut req: Request, next: Next) -> Response {
     if req.uri().path().starts_with("/api/") && !req.headers().contains_key(header::AUTHORIZATION) {
-        req.headers_mut().insert(
-            header::AUTHORIZATION,
-            HeaderValue::from_static(
-                "Bearer fabro_dev_abababababababababababababababababababababababababababababababab",
-            ),
-        );
+        static BEARER: OnceLock<HeaderValue> = OnceLock::new();
+        let bearer = BEARER.get_or_init(|| {
+            HeaderValue::from_str(&format!("Bearer {TEST_DEV_TOKEN}"))
+                .expect("dev token bearer header is valid")
+        });
+        req.headers_mut()
+            .insert(header::AUTHORIZATION, bearer.clone());
     }
     next.run(req).await
 }

lib/crates/fabro-types/src/principal.rs

@@ -166,37 +166,64 @@ mod tests {
         );
     }
 
-    #[test]
-    fn round_trips_all_variants() {
-        let variants = [
-            Principal::user(identity(), "octocat".to_string(), AuthMethod::Github),
-            Principal::Worker {
-                run_id: fixtures::RUN_1,
-            },
-            Principal::Webhook {
-                delivery_id: "delivery-1".to_string(),
-            },
-            Principal::Slack {
-                team_id:   "T1".to_string(),
-                user_id:   "U1".to_string(),
-                user_name: Some("ada".to_string()),
-            },
-            Principal::Agent {
-                session_id:        Some("session".to_string()),
-                parent_session_id: Some("parent".to_string()),
-                model:             Some("gpt".to_string()),
-            },
-            Principal::System {
-                system_kind: SystemActorKind::Engine,
-            },
-            Principal::Anonymous,
-        ];
+    #[track_caller]
+    fn assert_round_trip(principal: &Principal) {
+        let value = serde_json::to_value(principal).unwrap();
+        let parsed: Principal = serde_json::from_value(value).unwrap();
+        assert_eq!(&parsed, principal);
+    }
 
-        for principal in variants {
-            let value = serde_json::to_value(&principal).unwrap();
-            let parsed: Principal = serde_json::from_value(value).unwrap();
-            assert_eq!(parsed, principal);
-        }
+    #[test]
+    fn round_trips_user_variant() {
+        assert_round_trip(&Principal::user(
+            identity(),
+            "octocat".to_string(),
+            AuthMethod::Github,
+        ));
+    }
+
+    #[test]
+    fn round_trips_worker_variant() {
+        assert_round_trip(&Principal::Worker {
+            run_id: fixtures::RUN_1,
+        });
+    }
+
+    #[test]
+    fn round_trips_webhook_variant() {
+        assert_round_trip(&Principal::Webhook {
+            delivery_id: "delivery-1".to_string(),
+        });
+    }
+
+    #[test]
+    fn round_trips_slack_variant() {
+        assert_round_trip(&Principal::Slack {
+            team_id:   "T1".to_string(),
+            user_id:   "U1".to_string(),
+            user_name: Some("ada".to_string()),
+        });
+    }
+
+    #[test]
+    fn round_trips_agent_variant() {
+        assert_round_trip(&Principal::Agent {
+            session_id:        Some("session".to_string()),
+            parent_session_id: Some("parent".to_string()),
+            model:             Some("gpt".to_string()),
+        });
+    }
+
+    #[test]
+    fn round_trips_system_variant() {
+        assert_round_trip(&Principal::System {
+            system_kind: SystemActorKind::Engine,
+        });
+    }
+
+    #[test]
+    fn round_trips_anonymous_variant() {
+        assert_round_trip(&Principal::Anonymous);
     }
 
     #[test]

lib/crates/fabro-workflow/src/error.rs

@@ -276,15 +276,7 @@ impl Error {
     }
 
     pub fn handler_with_anyhow(message: impl Into<String>, source: &anyhow::Error) -> Self {
-        let message = message.into();
-        let causes = source.chain().map(ToString::to_string).collect::<Vec<_>>();
-        let rendered = render_with_causes(&message, &causes);
-        let failure_class = classify_failure_reason(&rendered);
-        Self::Handler {
-            message,
-            failure_class,
-            causes,
-        }
+        Self::handler_with_source(message, source.as_ref())
     }
 
     /// Smart constructor for Engine errors. Classifies the failure reason
@@ -315,15 +307,7 @@ impl Error {
     }
 
     pub fn engine_with_anyhow(message: impl Into<String>, source: &anyhow::Error) -> Self {
-        let message = message.into();
-        let causes = source.chain().map(ToString::to_string).collect::<Vec<_>>();
-        let rendered = render_with_causes(&message, &causes);
-        let failure_class = classify_failure_reason(&rendered);
-        Self::Engine {
-            message,
-            failure_class,
-            causes,
-        }
+        Self::engine_with_source(message, source.as_ref())
     }
 
     #[must_use]