feat(auth): observe credential authorization differences - #6841
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — server-only observe-only telemetry for the credential-authorization rollout (#6827).
What I checked:
- No protocol surface touched (
static/schemas/source/**,docs/reference/**,dist/**all untouched) — so no changeset is required and none expected. Changeset-scope gate not triggered. - No
oneOf, released-artifact, or 3.1.x-patch concerns (all schema/dist paths untouched). - Privacy claim holds: new
org_authorization_shadowevents emit only route/decision/role metadata, no user/identity/email/org IDs (distinctId is staticserver-metrics). - No breaking-class protocol change; hard gate does not apply.
gated_paths: false; no author team gate;high_risk: false.
Medium findings (1 — non-blocking):
- server/src/middleware/organization-authorization-observer.ts:149 — WorkOS shadow-comparison calls have no timeout, sampling, or concurrency bound. Operability gap worth addressing, but the observer is fire-and-forget and off the user-visible path.
Decision table: no critical/high findings, gated_paths false, no deletions, the single medium is not data-loss/schema/infra category, high_risk false, no prior escalation, no team gate, and fewer than three medium findings. Falls through to row 9 → approve.
Medium findings
- server/src/middleware/organization-authorization-observer.ts:149 — WorkOS shadow-comparison calls have no timeout, sampling, or concurrency bound
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean cosmetic/lint delta with no new findings.
This is a subsequent review of a PR previously approved. The delta since the prior head is a single-hunk lint fix in server/src/middleware/organization-authorization-observer.ts: let selector tightened to const, plus a documenting comment and a // lgtm[js/user-controlled-bypass] suppression. The suppression is accurate — the observer is void-called in res.on('finish') and only emits captureEvent telemetry; the !selector.organizationId branch only selects which observe-only event fires and never changes the response.
The prior medium finding (WorkOS shadow-comparison calls lack a timeout/sampling/concurrency bound) sits on unchanged lines outside this delta hunk. No new findings surfaced in this run.
Decision path: no critical/high findings (row 1 n/a); gated_paths false (row 2 n/a); high_risk false (rows 3/5 n/a); no medium-category escalation triggers (row 4 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a); fewer than three medium findings (row 8 n/a). Falls through to row 9 → approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — no blocking or medium findings.
This is a subsequent review on top of a prior approve. The delta refines the org-authorization observer's operability: a new fail-fast WorkOS client (3s timeout, maxRetries 0) matching the existing getPipesWorkos shape, plus a module-level concurrency cap (MAX_CONCURRENT_COMPARISONS = 5) with the counter released in finally. Fire-and-forget observer calls are now bounded on timeout, retries, and fan-out.
Changes are confined to server middleware/auth plumbing (server/src/auth/workos-client.ts, server/src/middleware/*) with an accompanying unit test — no protocol surface (static/schemas/source/**, docs/reference/**, dist/**), no changeset-requiring wire changes, and no migrations. No high-risk paths, no gated paths, no no-auto-approve team match.
Reviewer surfaced no critical/high/medium findings. Rows 1–8 do not fire; falls through to row 9.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — server operational hardening, no blocking findings.
This PR hardens the organization authorization observer: a dedicated fail-fast WorkOS client (3s timeout, maxRetries 0) and a process-wide bounded comparison pool (MAX_CONCURRENT_COMPARISONS = 5), with concurrency accounting balanced across the normal, saturated, and throw paths. Includes accompanying unit tests.
Checks:
- No critical/high/medium findings from the reviewer.
- Changes are server operational-only (auth client, middleware, tests) — no protocol surface (
static/schemas/source/**,docs/reference/**,dist/**) touched, so no changeset required and no schema↔docs coherence concern. - No high-risk paths (high_risk: false), no gated paths (gated_paths: false), no no-auto-approve team match.
- No breaking-class protocol change; nothing implicating 3.1.x patch eligibility, oneOf discriminators, or released-artifact immutability.
- Prior decision was approve; this is a clean continuation.
None of decision-table rows 1–8 fire → row 9 approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking findings.
Delta since the prior approve is a single line: removal of a cosmetic // lgtm[js/user-controlled-bypass] CodeQL suppression on the if (!selector.organizationId) branch in organization-authorization-observer.ts. That branch only selects which observe-only telemetry event fires (invoked via void post-finish in request-metrics.ts) and never gates the response, so removing the suppression is behavior-neutral.
Checked against the decision table:
- No critical/high/medium findings (row 1, 4–6, 8 do not fire).
- gated_paths: false (row 2 n/a).
- high_risk: false, no deleted/modified sensitive-path findings (rows 3, 5 n/a).
- Prior decision was
approve, notescalate(row 6 n/a). - No no-auto-approve team match (row 7 n/a).
- Falls through to row 9 → approve.
Summary
ORG_AUTHORIZATION_OBSERVER_ENABLED=falseas an emergency kill switchThis is the observe-only first stage for #6827. It is intentionally separated from enforcement in #6839 so production traffic can establish the compatibility baseline before exact-credential authorization is enabled.
Verification
Refs #6827