diff --git a/apps/fabro-web/app/components/run-summary-panel.test.tsx b/apps/fabro-web/app/components/run-summary-panel.test.tsx index 52e79cf0f..14fb04510 100644 --- a/apps/fabro-web/app/components/run-summary-panel.test.tsx +++ b/apps/fabro-web/app/components/run-summary-panel.test.tsx @@ -252,7 +252,7 @@ describe("RunSummaryPanelView", () => { }); test("renders non-user actor with kind label", () => { - for (const kind of ["agent", "system", "slack", "webhook", "worker", "anonymous"]) { + for (const kind of ["agent", "system", "slack", "webhook", "worker"]) { const tree = render({ run: makeRun({ created_by: { kind } as any }) }); expect(instanceText(cellAfterLabel(tree, "Created by"))).toContain(kind); } diff --git a/apps/fabro-web/app/lib/principal-display.tsx b/apps/fabro-web/app/lib/principal-display.tsx index fa9d4ec16..666f0f64c 100644 --- a/apps/fabro-web/app/lib/principal-display.tsx +++ b/apps/fabro-web/app/lib/principal-display.tsx @@ -4,7 +4,6 @@ import { ChatBubbleLeftEllipsisIcon, Cog6ToothIcon, CpuChipIcon, - QuestionMarkCircleIcon, ServerIcon, } from "@heroicons/react/20/solid"; import type { Principal } from "@qltysh/fabro-api-client"; @@ -57,10 +56,5 @@ export function principalDisplay(actor: Principal): PrincipalDisplay { return { glyph: principalIconGlyph(), label: "webhook" }; case "worker": return { glyph: principalIconGlyph(), label: "worker" }; - case "anonymous": - return { - glyph: principalIconGlyph(), - label: "anonymous", - }; } } diff --git a/docs/internal/logging-strategy.md b/docs/internal/logging-strategy.md index 63f6f8f54..013904b50 100644 --- a/docs/internal/logging-strategy.md +++ b/docs/internal/logging-strategy.md @@ -118,7 +118,7 @@ Fields are key-value pairs that make events queryable. Include enough context th | `error` | Error value on failure | | `path` | File system path | | `duration_ms` | Elapsed time in milliseconds | -| `principal_kind` | HTTP caller category (`user`, `worker`, `webhook`, `anonymous`, etc.) | +| `principal_kind` | HTTP caller category (`user`, `worker`, `webhook`, `none`, etc.); `none` means no request principal was established | | `auth_status` | HTTP authentication result (`missing`, `invalid`, `expired`, `authenticated`) | | `idp_issuer`, `idp_subject` | Canonical user identity for authenticated user requests | diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 35d7511e5..d075ce98b 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -9014,7 +9014,6 @@ components: - $ref: "#/components/schemas/PrincipalSlack" - $ref: "#/components/schemas/PrincipalAgent" - $ref: "#/components/schemas/PrincipalSystem" - - $ref: "#/components/schemas/PrincipalAnonymous" discriminator: propertyName: kind mapping: @@ -9024,7 +9023,6 @@ components: slack: "#/components/schemas/PrincipalSlack" agent: "#/components/schemas/PrincipalAgent" system: "#/components/schemas/PrincipalSystem" - anonymous: "#/components/schemas/PrincipalAnonymous" PrincipalUser: type: object @@ -9114,15 +9112,6 @@ components: system_kind: $ref: "#/components/schemas/SystemActorKind" - PrincipalAnonymous: - type: object - required: - - kind - properties: - kind: - type: string - enum: [anonymous] - RunEvent: description: > Internal RunEvent-compatible JSON payload. The server validates this diff --git a/docs/public/changelog/2026-05-02.mdx b/docs/public/changelog/2026-05-02.mdx index 48a501a59..2d55f8d05 100644 --- a/docs/public/changelog/2026-05-02.mdx +++ b/docs/public/changelog/2026-05-02.mdx @@ -11,7 +11,7 @@ The dock listens to interview events and refreshes as questions arrive, so a par ## Principal attribution and auth routing -Run events and run creation now carry clearer principal information for users, workers, systems, Slack interactions, webhooks, agents, and anonymous actors. API clients get explicit provenance objects instead of older actor-shaped fields that could lose where a run came from. +Run events and run creation now carry clearer principal information for users, workers, systems, Slack interactions, webhooks, and agents. API clients get explicit provenance objects instead of older actor-shaped fields that could lose where a run came from. This also closes attribution gaps across web, CLI, worker-token, Slack, and human-interview paths. Runs created or advanced through different surfaces now preserve who or what took the action more consistently. @@ -19,7 +19,7 @@ This also closes attribution gaps across web, CLI, worker-token, Slack, and huma - Run specs now include client and server provenance shapes -- Run events use unified principal shapes for user, worker, system, Slack, webhook, agent, and anonymous subjects +- Run events use unified principal shapes for user, worker, system, Slack, webhook, and agent subjects diff --git a/lib/crates/fabro-api/tests/principal_round_trip.rs b/lib/crates/fabro-api/tests/principal_round_trip.rs index ca1180e60..601e6372c 100644 --- a/lib/crates/fabro-api/tests/principal_round_trip.rs +++ b/lib/crates/fabro-api/tests/principal_round_trip.rs @@ -118,7 +118,6 @@ fn principal_round_trips_every_variant_through_api_type() { Principal::System { system_kind: SystemActorKind::Watchdog, }, - Principal::Anonymous, ]; for principal in variants { diff --git a/lib/crates/fabro-server/src/auth/cli_flow.rs b/lib/crates/fabro-server/src/auth/cli_flow.rs index 987c2631b..3a9d6e055 100644 --- a/lib/crates/fabro-server/src/auth/cli_flow.rs +++ b/lib/crates/fabro-server/src/auth/cli_flow.rs @@ -1671,11 +1671,11 @@ client_id = "github-client-id" 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!(first.principal.as_ref().unwrap().display(), "octocat"); assert_eq!(second.auth_status, AuthStatus::Authenticated); - assert_eq!(second.principal.display(), "octocat"); + assert_eq!(second.principal.as_ref().unwrap().display(), "octocat"); assert_eq!(third.auth_status, AuthStatus::Authenticated); - assert_eq!(third.principal.display(), "octocat"); + assert_eq!(third.principal.as_ref().unwrap().display(), "octocat"); } #[tokio::test] @@ -2080,7 +2080,7 @@ client_id = "github-client-id" let contexts = captured.lock().expect("captured auth contexts").clone(); assert_eq!(contexts[0].auth_status, AuthStatus::Authenticated); - assert_eq!(contexts[0].principal.display(), "octocat"); + assert_eq!(contexts[0].principal.as_ref().unwrap().display(), "octocat"); assert_eq!(contexts[1].auth_status, AuthStatus::Invalid); assert_eq!( contexts[1].auth_error_code, @@ -2273,8 +2273,8 @@ client_id = "github-client-id" let contexts = captured.lock().expect("captured auth contexts").clone(); assert_eq!(contexts[0].auth_status, AuthStatus::Authenticated); - assert_eq!(contexts[0].principal.display(), "octocat"); - let Principal::User(user) = &contexts[0].principal else { + assert_eq!(contexts[0].principal.as_ref().unwrap().display(), "octocat"); + let Some(Principal::User(user)) = &contexts[0].principal else { panic!("expected user principal"); }; assert_eq!( diff --git a/lib/crates/fabro-server/src/principal_middleware.rs b/lib/crates/fabro-server/src/principal_middleware.rs index f9e8c385c..2012b74c7 100644 --- a/lib/crates/fabro-server/src/principal_middleware.rs +++ b/lib/crates/fabro-server/src/principal_middleware.rs @@ -19,7 +19,7 @@ use crate::worker_token::{self, WORKER_TOKEN_KID, WorkerScopeSet}; #[derive(Clone, Debug)] pub(crate) struct RequestAuthContext { - pub principal: Principal, + pub principal: Option, pub auth_status: AuthStatus, pub auth_error_code: Option, pub user_profile: Option, @@ -76,7 +76,7 @@ impl RequestAuthContext { #[must_use] pub(crate) fn initial() -> Self { Self { - principal: Principal::Anonymous, + principal: None, auth_status: AuthStatus::Missing, auth_error_code: None, user_profile: None, @@ -87,7 +87,7 @@ impl RequestAuthContext { #[must_use] pub(crate) fn authenticated(principal: Principal, user_profile: Option) -> Self { Self { - principal, + principal: Some(principal), auth_status: AuthStatus::Authenticated, auth_error_code: None, user_profile, @@ -98,7 +98,7 @@ impl RequestAuthContext { #[must_use] pub(crate) fn authenticated_worker(run_id: RunId, scopes: WorkerScopeSet) -> Self { Self { - principal: Principal::Worker { run_id }, + principal: Some(Principal::Worker { run_id }), auth_status: AuthStatus::Authenticated, auth_error_code: None, user_profile: None, @@ -125,7 +125,7 @@ impl RequestAuthContext { #[must_use] pub(crate) fn rejected(status: AuthStatus, code: Option) -> Self { Self { - principal: Principal::Anonymous, + principal: None, auth_status: status, auth_error_code: code, user_profile: None, @@ -148,7 +148,7 @@ impl AuthStatus { #[derive(Clone, Debug)] pub(crate) struct RequestAuthLogContext { - pub principal: Principal, + pub principal: Option, pub auth_status: AuthStatus, pub auth_error_code: Option, } @@ -172,22 +172,23 @@ impl AuthContextSlot { pub(crate) fn log_snapshot(&self) -> RequestAuthLogContext { let context = self.0.lock().expect("auth context lock poisoned"); RequestAuthLogContext { - principal: principal_without_log_unused_fields(&context.principal), + principal: principal_without_log_unused_fields(context.principal.as_ref()), auth_status: context.auth_status, auth_error_code: context.auth_error_code, } } } -fn principal_without_log_unused_fields(principal: &Principal) -> Principal { +fn principal_without_log_unused_fields(principal: Option<&Principal>) -> Option { match principal { - Principal::User(user) => Principal::User(UserPrincipal { + Some(Principal::User(user)) => Some(Principal::User(UserPrincipal { identity: user.identity.clone(), login: user.login.clone(), auth_method: user.auth_method, avatar_url: None, - }), - principal => principal.clone(), + })), + Some(principal) => Some(principal.clone()), + None => None, } } @@ -402,7 +403,7 @@ fn auth_slot_from_parts(parts: &Parts) -> AuthContextSlot { pub(crate) fn require_user(slot: &AuthContextSlot) -> Result { let context = slot.0.lock().expect("auth context lock poisoned"); match &context.principal { - Principal::User(user) => Ok(user.clone()), + Some(Principal::User(user)) => Ok(user.clone()), _ => Err(auth_rejection(context.auth_status, context.auth_error_code)), } } @@ -412,7 +413,7 @@ pub(crate) fn require_authenticated_user( ) -> Result { let context = slot.snapshot(); match context.principal { - Principal::User(principal) => { + Some(Principal::User(principal)) => { let Some(profile) = context.user_profile else { return Err(ApiError::new( StatusCode::INTERNAL_SERVER_ERROR, @@ -428,11 +429,11 @@ pub(crate) fn require_authenticated_user( pub(crate) fn require_run_management_actor(slot: &AuthContextSlot) -> Result { let context = slot.0.lock().expect("auth context lock poisoned"); match &context.principal { - Principal::User(user) => Ok(Principal::User(user.clone())), - Principal::Worker { run_id } if context.worker_scopes.has_agent_run_tools() => { + Some(Principal::User(user)) => Ok(Principal::User(user.clone())), + Some(Principal::Worker { run_id }) if context.worker_scopes.has_agent_run_tools() => { Ok(Principal::Worker { run_id: *run_id }) } - Principal::Worker { .. } => Err(ApiError::forbidden()), + Some(Principal::Worker { .. }) => Err(ApiError::forbidden()), _ => Err(auth_rejection(context.auth_status, context.auth_error_code)), } } @@ -443,9 +444,9 @@ fn require_worker_or_user_for_run( ) -> Result<(), ApiError> { let context = slot.0.lock().expect("auth context lock poisoned"); match &context.principal { - Principal::User(_) => Ok(()), - Principal::Worker { run_id } if run_id == route_run_id => Ok(()), - Principal::Worker { .. } => Err(ApiError::forbidden()), + Some(Principal::User(_)) => Ok(()), + Some(Principal::Worker { run_id }) if run_id == route_run_id => Ok(()), + Some(Principal::Worker { .. }) => Err(ApiError::forbidden()), _ => Err(auth_rejection(context.auth_status, context.auth_error_code)), } } @@ -453,8 +454,8 @@ fn require_worker_or_user_for_run( fn require_worker_for_run(slot: &AuthContextSlot, route_run_id: &RunId) -> Result<(), ApiError> { let context = slot.0.lock().expect("auth context lock poisoned"); match &context.principal { - Principal::Worker { run_id } if run_id == route_run_id => Ok(()), - Principal::Worker { .. } | Principal::User(_) => Err(ApiError::forbidden()), + Some(Principal::Worker { run_id }) if run_id == route_run_id => Ok(()), + Some(Principal::Worker { .. } | Principal::User(_)) => Err(ApiError::forbidden()), _ => Err(auth_rejection(context.auth_status, context.auth_error_code)), } } @@ -465,13 +466,13 @@ fn require_run_management_target( ) -> Result { let context = slot.0.lock().expect("auth context lock poisoned"); match &context.principal { - Principal::User(user) => Ok(Principal::User(user.clone())), - Principal::Worker { run_id } + Some(Principal::User(user)) => Ok(Principal::User(user.clone())), + Some(Principal::Worker { run_id }) if run_id == route_run_id || context.worker_scopes.has_agent_run_tools() => { Ok(Principal::Worker { run_id: *run_id }) } - Principal::Worker { .. } => Err(ApiError::forbidden()), + Some(Principal::Worker { .. }) => Err(ApiError::forbidden()), _ => Err(auth_rejection(context.auth_status, context.auth_error_code)), } } @@ -687,7 +688,7 @@ mod tests { let context = classify_request(&request, state.as_ref()); assert_eq!(context.auth_status, AuthStatus::Authenticated); - assert!(matches!(context.principal, Principal::User(_))); + assert!(matches!(context.principal, Some(Principal::User(_)))); assert!(context.user_profile.is_some()); } @@ -727,7 +728,7 @@ mod tests { let context = classify_request(&request, state.as_ref()); assert_eq!(context.auth_status, AuthStatus::Authenticated); - assert_eq!(context.principal, Principal::Worker { run_id }); + assert_eq!(context.principal, Some(Principal::Worker { run_id })); assert!(!context.worker_scopes.has_agent_run_tools()); } @@ -746,7 +747,7 @@ mod tests { let context = classify_request(&request, state.as_ref()); assert_eq!(context.auth_status, AuthStatus::Authenticated); - assert_eq!(context.principal, Principal::Worker { run_id }); + assert_eq!(context.principal, Some(Principal::Worker { run_id })); assert!(context.worker_scopes.has_agent_run_tools()); } @@ -825,7 +826,7 @@ mod tests { assert_eq!(context.auth_status, AuthStatus::Missing); assert_eq!(context.auth_error_code, None); - assert_eq!(context.principal, Principal::Anonymous); + assert_eq!(context.principal, None); } #[test] diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 7bff4968f..4710c40ca 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -1876,7 +1876,10 @@ async fn http_log_middleware(mut req: axum_extract::Request, next: Next) -> Resp let status = response.status().as_u16(); let latency_ms = start.elapsed().as_millis(); let auth_context = auth_slot.log_snapshot(); - let principal_kind = auth_context.principal.kind(); + let principal_kind = auth_context + .principal + .as_ref() + .map_or("none", Principal::kind); let auth_status = auth_context.auth_status.as_str(); macro_rules! emit_http_log { @@ -1914,27 +1917,27 @@ async fn http_log_middleware(mut req: axum_extract::Request, next: Next) -> Resp macro_rules! emit_principal_http_log { ($level:ident) => {{ match &auth_context.principal { - Principal::User(user) => emit_http_log!( + Some(Principal::User(user)) => emit_http_log!( $level, user_auth_method = user.auth_method.as_str(), idp_issuer = user.identity.issuer(), idp_subject = user.identity.subject(), login = user.login.as_str(), ), - Principal::Worker { run_id } => { + Some(Principal::Worker { run_id }) => { emit_http_log!($level, run_id = run_id.to_string().as_str(),) } - Principal::Webhook { delivery_id } => { + Some(Principal::Webhook { delivery_id }) => { emit_http_log!($level, delivery_id = delivery_id.as_str(),) } - Principal::Slack { + Some(Principal::Slack { team_id, user_id, .. - } => emit_http_log!( + }) => emit_http_log!( $level, team_id = team_id.as_str(), user_id = user_id.as_str(), ), - Principal::Agent { .. } | Principal::System { .. } | Principal::Anonymous => { + None | Some(Principal::Agent { .. } | Principal::System { .. }) => { emit_http_log!($level) } } diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index 033e46db6..e231599b2 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -1365,7 +1365,7 @@ client_id = "github-client-id" let contexts = captured.lock().expect("captured auth contexts").clone(); assert_eq!(contexts[0].auth_status, AuthStatus::Authenticated); - assert!(matches!(contexts[0].principal, Principal::User(_))); + assert!(matches!(contexts[0].principal, Some(Principal::User(_)))); assert_eq!(contexts[1].auth_status, AuthStatus::Invalid); assert_eq!( contexts[1].auth_error_code, diff --git a/lib/crates/fabro-types/src/principal.rs b/lib/crates/fabro-types/src/principal.rs index 2c0ff3807..4f1962ea2 100644 --- a/lib/crates/fabro-types/src/principal.rs +++ b/lib/crates/fabro-types/src/principal.rs @@ -39,7 +39,6 @@ pub enum Principal { System { system_kind: SystemActorKind, }, - Anonymous, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, IntoStaticStr)] @@ -97,7 +96,6 @@ impl Principal { Self::Slack { .. } => "slack", Self::Agent { .. } => "agent", Self::System { .. } => "system", - Self::Anonymous => "anonymous", } } @@ -123,7 +121,6 @@ impl Principal { } => session_id.clone(), Self::Agent { .. } => "agent".to_string(), Self::System { system_kind } => format!("system:{system_kind}"), - Self::Anonymous => "anonymous".to_string(), } } } @@ -291,11 +288,6 @@ mod tests { }); } - #[test] - fn round_trips_anonymous_variant() { - assert_round_trip(&Principal::Anonymous); - } - #[test] fn auth_method_as_str_matches_serde() { assert_eq!(AuthMethod::Github.as_str(), "github"); diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 5e97116d9..7839b95b6 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -271,7 +271,6 @@ models/preflight-workflow-summary.ts models/preview-url-request.ts models/preview-url-response.ts models/principal-agent.ts -models/principal-anonymous.ts models/principal-slack.ts models/principal-system.ts models/principal-user.ts diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 87b9c189c..af8ae77e0 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -244,7 +244,6 @@ export * from './preview-url-request'; export * from './preview-url-response'; export * from './principal'; export * from './principal-agent'; -export * from './principal-anonymous'; export * from './principal-slack'; export * from './principal-system'; export * from './principal-user'; diff --git a/lib/packages/fabro-api-client/src/models/principal-anonymous.ts b/lib/packages/fabro-api-client/src/models/principal-anonymous.ts deleted file mode 100644 index daac61df0..000000000 --- a/lib/packages/fabro-api-client/src/models/principal-anonymous.ts +++ /dev/null @@ -1,25 +0,0 @@ -/* tslint:disable */ -/* eslint-disable */ -/** - * Fabro Run API - * HTTP API for managing Fabro workflow run executions. - * - * The version of the OpenAPI document: 0.1.0 - * - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - - - -export interface PrincipalAnonymous { - 'kind': PrincipalAnonymousKindEnum; -} - -export const PrincipalAnonymousKindEnum = { - ANONYMOUS: 'anonymous' -} as const; - -export type PrincipalAnonymousKindEnum = typeof PrincipalAnonymousKindEnum[keyof typeof PrincipalAnonymousKindEnum]; diff --git a/lib/packages/fabro-api-client/src/models/principal.ts b/lib/packages/fabro-api-client/src/models/principal.ts index e3597295d..08b5422df 100644 --- a/lib/packages/fabro-api-client/src/models/principal.ts +++ b/lib/packages/fabro-api-client/src/models/principal.ts @@ -24,9 +24,6 @@ import type { IdpIdentity } from './idp-identity'; import type { PrincipalAgent } from './principal-agent'; // May contain unused imports in some cases // @ts-ignore -import type { PrincipalAnonymous } from './principal-anonymous'; -// May contain unused imports in some cases -// @ts-ignore import type { PrincipalSlack } from './principal-slack'; // May contain unused imports in some cases // @ts-ignore @@ -47,4 +44,4 @@ import type { SystemActorKind } from './system-actor-kind'; /** * @type Principal */ -export type Principal = { kind: 'agent' } & PrincipalAgent | { kind: 'anonymous' } & PrincipalAnonymous | { kind: 'slack' } & PrincipalSlack | { kind: 'system' } & PrincipalSystem | { kind: 'user' } & PrincipalUser | { kind: 'webhook' } & PrincipalWebhook | { kind: 'worker' } & PrincipalWorker; +export type Principal = { kind: 'agent' } & PrincipalAgent | { kind: 'slack' } & PrincipalSlack | { kind: 'system' } & PrincipalSystem | { kind: 'user' } & PrincipalUser | { kind: 'webhook' } & PrincipalWebhook | { kind: 'worker' } & PrincipalWorker; diff --git a/lib/packages/fabro-api-client/tests/principal-exhaustive.ts b/lib/packages/fabro-api-client/tests/principal-exhaustive.ts index 114b1e640..5fff8b8f4 100644 --- a/lib/packages/fabro-api-client/tests/principal-exhaustive.ts +++ b/lib/packages/fabro-api-client/tests/principal-exhaustive.ts @@ -12,8 +12,6 @@ export function principalKind(principal: Principal): string { switch (principal.kind) { case "agent": return "agent"; - case "anonymous": - return "anonymous"; case "slack": return "slack"; case "system":