scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-08 03:10:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Remove anonymous principal variant

Browse source
This commit is contained in:
Fabro 2026-06-04 22:13:21 +00:00
parent 7782f9b3cd
commit 04d6587276
16 changed files with 76 additions and 107 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
apps/fabro-web/app/components/run-summary-panel.test.tsx
View file

@ -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);
}

6
apps/fabro-web/app/lib/principal-display.tsx
View file

@ -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(<BoltIcon className="size-3" />), label: "webhook" };
case "worker":
return { glyph: principalIconGlyph(<ServerIcon className="size-3" />), label: "worker" };
case "anonymous":
return {
glyph: principalIconGlyph(<QuestionMarkCircleIcon className="size-3" />),
label: "anonymous",
};
}
}

4
docs/internal/logging-strategy.md
View file

@ -118,11 +118,11 @@ 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.) |
| `auth_status` | HTTP authentication result (`missing`, `invalid`, `expired`, `authenticated`) |
| `idp_issuer`, `idp_subject` | Canonical user identity for authenticated user requests |
For HTTP request logs, use the request `Principal` projection rather than hand-assembled auth strings. User identity fields are present only for `Principal::User`; worker and webhook requests use their variant-specific fields (`run_id`, `delivery_id`).
For HTTP request logs, use the request `Principal` projection rather than hand-assembled auth strings. User identity fields are present only for `Principal::User`; worker and webhook requests use their variant-specific fields (`run_id`, `delivery_id`). Requests without a principal use `principal_kind="none"`; `auth_status` distinguishes missing, invalid, expired, and authenticated auth state.
Server auth intentionally exposes a mutable `RequestAuth` context slot for public auth routes and guard extractors such as `RequiredUser` / `RequireRunScoped` for protected routes. There is no loose `RequestPrincipal` extractor; route-facing extractors should enforce the route's auth contract while the slot supplies the final HTTP log fields.
| `input_tokens` | Token count for LLM input |

11
docs/public/api-reference/fabro-api.yaml
View file

@ -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

4
docs/public/changelog/2026-05-02.mdx
View file

@ -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
<Accordion title="API">
- 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
</Accordion>
<Accordion title="CLI">

1
lib/crates/fabro-api/tests/principal_round_trip.rs
View file

@ -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 {

35
lib/crates/fabro-server/src/auth/cli_flow.rs
View file

@ -1671,11 +1671,20 @@ 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.expect("expected principal").display(),
"octocat"
);
assert_eq!(second.auth_status, AuthStatus::Authenticated);
assert_eq!(second.principal.display(), "octocat");
assert_eq!(
second.principal.expect("expected principal").display(),
"octocat"
);
assert_eq!(third.auth_status, AuthStatus::Authenticated);
assert_eq!(third.principal.display(), "octocat");
assert_eq!(
third.principal.expect("expected principal").display(),
"octocat"
);
}
#[tokio::test]
@ -2080,7 +2089,14 @@ 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()
.expect("expected principal")
.display(),
"octocat"
);
assert_eq!(contexts[1].auth_status, AuthStatus::Invalid);
assert_eq!(
contexts[1].auth_error_code,
@ -2273,8 +2289,15 @@ 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()
.expect("expected principal")
.display(),
"octocat"
);
let Some(Principal::User(user)) = &contexts[0].principal else {
panic!("expected user principal");
};
assert_eq!(

57
lib/crates/fabro-server/src/principal_middleware.rs
View file

@ -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<Principal>,
pub auth_status: AuthStatus,
pub auth_error_code: Option<AuthErrorCode>,
pub user_profile: Option<UserProfile>,
@ -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<UserProfile>) -> 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<AuthErrorCode>) -> 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<Principal>,
pub auth_status: AuthStatus,
pub auth_error_code: Option<AuthErrorCode>,
}
@ -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<Principal> {
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<UserPrincipal, ApiError> {
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<AuthenticatedUser, ApiError> {
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<Principal, ApiError> {
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<Principal, ApiError> {
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]

19
lib/crates/fabro-server/src/server.rs
View file

@ -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 {
@ -1913,28 +1916,28 @@ 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!(
match auth_context.principal.as_ref() {
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)
}
}

2
lib/crates/fabro-server/src/web_auth.rs
View file

@ -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,

8
lib/crates/fabro-types/src/principal.rs
View file

@ -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");

1
lib/packages/fabro-api-client/src/.openapi-generator/FILES generated
View file

@ -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

1
lib/packages/fabro-api-client/src/models/index.ts generated
View file

@ -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';

25
lib/packages/fabro-api-client/src/models/principal-anonymous.ts generated
View file

@ -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];

5
lib/packages/fabro-api-client/src/models/principal.ts generated
View file

@ -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;

2
lib/packages/fabro-api-client/tests/principal-exhaustive.ts
View file

@ -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":