Skip to content

Commit aa938a3

Browse files
ryanbas21claude
andcommitted
refactor(core): deepen architecture — rules registry, enricher pipeline, export composition
Five architectural improvements to devtools-core: 1. Extract diagnosis rules into registry (diagnosis/rules/) - 846-line monolith → 160-line engine + 7 focused rule modules - FlowRule type defines internal seam; flowRules array is the registry - Adding a diagnostic category = one file + one array entry 2. Simplify EventStore interface - Remove setLastOidcEventId from service shape - Track lastOidcEventId in updateSummary alongside lastSdkEventId - Every adapter gets OIDC tracking automatically via append 3. Move export composition into core - exportAsJson/exportAsMarkdown now in devtools-core/src/export/ - Delete standalone export-helpers.ts; all consumers import from core - Eliminates duplicated redact→diagnose→render recipe 4. Refactor detector pipeline with enricher pattern - OidcEnricher type + enrichers array replaces hardcoded merge - annotateOidc stays as base gate; DPoP/PAR are enrichers - Adding a detector = one function + one array entry 5. Narrow barrel exports - Remove 14 internal-only symbols from public barrel - ~20 organized exports vs 30+ flat exports Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1c8688f commit aa938a3

19 files changed

Lines changed: 802 additions & 775 deletions

File tree

packages/devtools-core/src/diagnosis/diagnosis-engine.ts

Lines changed: 32 additions & 710 deletions
Large diffs are not rendered by default.
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import type { AuthEvent } from '@wolfcola/devtools-types';
2+
import type { IssueCandidate } from './types.js';
3+
4+
export function collectCorsIssues(events: readonly AuthEvent[]): IssueCandidate[] {
5+
const candidates: IssueCandidate[] = [];
6+
7+
for (const event of events) {
8+
if (event.data._tag !== 'network') continue;
9+
const { data } = event;
10+
const origin = data.requestHeaders['origin'] ?? '';
11+
const allowOrigin = data.responseHeaders['access-control-allow-origin'] ?? '';
12+
const allowCredentials = data.responseHeaders['access-control-allow-credentials'] ?? '';
13+
const hasOriginHeader = 'origin' in data.requestHeaders;
14+
15+
if (data.status === 0 && event.flags.isCors) {
16+
candidates.push({
17+
dedupKey: `cors:status-zero:${origin}`,
18+
eventId: event.id,
19+
issue: {
20+
id: 'cors:status-zero',
21+
severity: 'error',
22+
category: 'cors',
23+
title: 'Network failure (status 0)',
24+
description:
25+
'The request never reached the server. This is almost always a CORS preflight rejection.',
26+
steps: [
27+
`Your auth server must include this origin in allowed origins: ${origin || '(unknown)'}`,
28+
'Check the OPTIONS preflight request in the Network tab.',
29+
'If using credentials, wildcard (*) is not allowed as the allowed origin.',
30+
],
31+
relevantData: origin ? { origin } : undefined,
32+
},
33+
});
34+
}
35+
36+
if (hasOriginHeader && !allowOrigin && data.status !== 0 && event.flags.isCors) {
37+
candidates.push({
38+
dedupKey: `cors:missing-allow-origin:${origin}`,
39+
eventId: event.id,
40+
issue: {
41+
id: 'cors:missing-allow-origin',
42+
severity: 'error',
43+
category: 'cors',
44+
title: 'Missing CORS header',
45+
description: 'The server response is missing Access-Control-Allow-Origin.',
46+
steps: [
47+
`Add ${origin} to allowed origins on your auth server.`,
48+
'Verify the request origin matches what is configured in your AS CORS settings.',
49+
],
50+
relevantData: { 'missing-header': 'access-control-allow-origin', origin },
51+
},
52+
});
53+
}
54+
55+
if (allowOrigin === '*' && allowCredentials === 'true') {
56+
candidates.push({
57+
dedupKey: `cors:wildcard-with-credentials:${data.url}`,
58+
eventId: event.id,
59+
issue: {
60+
id: 'cors:wildcard-with-credentials',
61+
severity: 'error',
62+
category: 'cors',
63+
title: 'Wildcard CORS with credentials',
64+
description:
65+
'access-control-allow-origin: * cannot be used together with access-control-allow-credentials: true.',
66+
steps: [
67+
`Replace wildcard with an explicit origin: ${origin || '(your app origin)'}`,
68+
'Configure your auth server to reflect the specific requesting origin.',
69+
],
70+
relevantData: {
71+
'access-control-allow-origin': '*',
72+
'access-control-allow-credentials': 'true',
73+
},
74+
},
75+
});
76+
}
77+
78+
if (
79+
hasOriginHeader &&
80+
allowCredentials === 'false' &&
81+
data.requestHeaders['cookie'] !== undefined
82+
) {
83+
candidates.push({
84+
dedupKey: `cors:credentials-not-allowed:${origin}`,
85+
eventId: event.id,
86+
issue: {
87+
id: 'cors:credentials-not-allowed',
88+
severity: 'warning',
89+
category: 'cors',
90+
title: 'Credentials not allowed by server',
91+
description:
92+
'The server set access-control-allow-credentials: false but cookies were sent.',
93+
steps: [
94+
'Enable credentials on the auth server CORS config.',
95+
'Or remove the cookie from the request.',
96+
],
97+
},
98+
});
99+
}
100+
}
101+
102+
return candidates;
103+
}
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
import type { AuthEvent } from '@wolfcola/devtools-types';
2+
import { decodeJwtPayload } from '../../annotators/jwt-utils.js';
3+
import type { IssueCandidate } from './types.js';
4+
5+
export function collectDpopIssues(events: readonly AuthEvent[]): IssueCandidate[] {
6+
const candidates: IssueCandidate[] = [];
7+
8+
for (const event of events) {
9+
const sem = event.oidcSemantics;
10+
if (!sem?.dpop) continue;
11+
12+
if (event.data._tag !== 'network') continue;
13+
const { data } = event;
14+
15+
// Check DPoP proof structure
16+
if (sem.dpop.proofJwt) {
17+
const payload = decodeJwtPayload(sem.dpop.proofJwt);
18+
if (payload) {
19+
const requiredClaims = ['htm', 'htu', 'iat', 'jti'];
20+
const missing = requiredClaims.filter((c) => !(c in payload));
21+
if (missing.length > 0) {
22+
candidates.push({
23+
dedupKey: `dpop:invalid-structure:${event.id}`,
24+
eventId: event.id,
25+
issue: {
26+
id: 'dpop:invalid-structure',
27+
severity: 'error',
28+
category: 'dpop',
29+
title: 'DPoP proof missing required claims',
30+
description: `The DPoP proof JWT is missing: ${missing.join(', ')}.`,
31+
steps: [
32+
'Include all required claims: htm, htu, iat, jti.',
33+
'Add ath when using DPoP with resource requests.',
34+
],
35+
relevantData: { 'missing-claims': missing.join(', ') },
36+
},
37+
});
38+
}
39+
40+
// htm mismatch
41+
if (typeof payload['htm'] === 'string' && payload['htm'] !== data.method) {
42+
candidates.push({
43+
dedupKey: `dpop:method-mismatch:${event.id}`,
44+
eventId: event.id,
45+
issue: {
46+
id: 'dpop:method-mismatch',
47+
severity: 'error',
48+
category: 'dpop',
49+
title: 'DPoP method mismatch',
50+
description: `DPoP proof htm="${payload['htm']}" does not match actual method "${data.method}".`,
51+
steps: ['The htm claim must match the HTTP method of the request.'],
52+
relevantData: { htm: payload['htm'] as string, method: data.method },
53+
},
54+
});
55+
}
56+
57+
// htu mismatch
58+
if (typeof payload['htu'] === 'string') {
59+
const htu = payload['htu'] as string;
60+
const urlNoQuery = data.url.split('?')[0];
61+
if (htu !== urlNoQuery && htu !== data.url) {
62+
candidates.push({
63+
dedupKey: `dpop:uri-mismatch:${event.id}`,
64+
eventId: event.id,
65+
issue: {
66+
id: 'dpop:uri-mismatch',
67+
severity: 'error',
68+
category: 'dpop',
69+
title: 'DPoP URI mismatch',
70+
description: 'The DPoP proof htu does not match the request URL.',
71+
steps: [
72+
'The htu claim must match the URL of the request (without query/fragment).',
73+
],
74+
relevantData: { htu, url: urlNoQuery },
75+
},
76+
});
77+
}
78+
}
79+
}
80+
}
81+
82+
// DPoP nonce required error
83+
if (sem.dpop.nonce && data.status === 400) {
84+
const body = data.responseBody as Record<string, unknown> | null;
85+
if (body && body['error'] === 'use_dpop_nonce') {
86+
candidates.push({
87+
dedupKey: `dpop:nonce-required:${event.id}`,
88+
eventId: event.id,
89+
issue: {
90+
id: 'dpop:nonce-required',
91+
severity: 'info',
92+
category: 'dpop',
93+
title: 'DPoP nonce required',
94+
description:
95+
'The server requires a DPoP nonce. The client should retry with the provided nonce.',
96+
steps: [
97+
'Include the DPoP-Nonce header value in the next DPoP proof.',
98+
'This is expected behavior for server nonce enforcement.',
99+
],
100+
relevantData: { nonce: sem.dpop.nonce },
101+
},
102+
});
103+
}
104+
}
105+
}
106+
107+
// Check for token requests to DPoP servers missing DPoP header
108+
const dpopServers = new Set<string>();
109+
for (const event of events) {
110+
if (event.oidcSemantics?.dpop?.tokenType?.toLowerCase() === 'dpop') {
111+
if (event.data._tag === 'network') {
112+
try {
113+
dpopServers.add(new URL(event.data.url).origin);
114+
} catch {
115+
// ignore invalid URLs
116+
}
117+
}
118+
}
119+
}
120+
for (const event of events) {
121+
if (event.data._tag !== 'network') continue;
122+
if (event.oidcSemantics?.oidcPhase !== 'token') continue;
123+
if (event.data.requestHeaders['dpop']) continue;
124+
try {
125+
const origin = new URL(event.data.url).origin;
126+
if (dpopServers.has(origin)) {
127+
candidates.push({
128+
dedupKey: `dpop:missing-proof:${event.id}`,
129+
eventId: event.id,
130+
issue: {
131+
id: 'dpop:missing-proof',
132+
severity: 'warning',
133+
category: 'dpop',
134+
title: 'Missing DPoP proof',
135+
description:
136+
'This token endpoint previously issued DPoP tokens but this request lacks a DPoP header.',
137+
steps: ['Include a DPoP proof JWT in the DPoP header.'],
138+
},
139+
});
140+
}
141+
} catch {
142+
// ignore
143+
}
144+
}
145+
146+
return candidates;
147+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import type { AuthEvent } from '@wolfcola/devtools-types';
2+
import type { IssueCandidate } from './types.js';
3+
4+
export function collectFlowConfigIssues(events: readonly AuthEvent[]): IssueCandidate[] {
5+
const candidates: IssueCandidate[] = [];
6+
7+
for (const event of events) {
8+
if (event.data._tag !== 'sdk') continue;
9+
const { data } = event;
10+
const { nodeStatus } = data;
11+
const errorCode = data.error?.code ?? '';
12+
13+
if (nodeStatus === 'error' || nodeStatus === 'failure') {
14+
const nodeName = data.nodeName ?? '';
15+
candidates.push({
16+
dedupKey: `flow:node-error:${event.id}`,
17+
eventId: event.id,
18+
issue: {
19+
id: 'flow:node-error',
20+
severity: 'error',
21+
category: 'flow-config',
22+
title: nodeName ? `Node error: ${nodeName}` : 'Node error',
23+
description: `A DaVinci node returned status "${nodeStatus}".`,
24+
steps: [
25+
'Check connector configuration in DaVinci admin.',
26+
'Review the error code in the SDK State tab.',
27+
],
28+
relevantData: nodeName ? { node: nodeName, status: nodeStatus } : { status: nodeStatus },
29+
},
30+
});
31+
}
32+
33+
if (errorCode === 'CONNECTOR_ERROR') {
34+
const httpStatus = data.error?.internalHttpStatus;
35+
candidates.push({
36+
dedupKey: `flow:connector-error:${event.id}`,
37+
eventId: event.id,
38+
issue: {
39+
id: 'flow:connector-error',
40+
severity: 'error',
41+
category: 'flow-config',
42+
title: httpStatus ? `Connector error (HTTP ${httpStatus})` : 'Connector error',
43+
description: 'A DaVinci connector returned an HTTP error from its upstream endpoint.',
44+
steps: [
45+
'Verify connector credentials and endpoint URL in DaVinci admin.',
46+
'Check the upstream service is reachable from your DaVinci environment.',
47+
],
48+
relevantData: httpStatus ? { 'internal-http-status': String(httpStatus) } : undefined,
49+
},
50+
});
51+
}
52+
53+
if (errorCode === 'NOT_FOUND') {
54+
candidates.push({
55+
dedupKey: `flow:policy-not-found`,
56+
eventId: event.id,
57+
issue: {
58+
id: 'flow:policy-not-found',
59+
severity: 'error',
60+
category: 'flow-config',
61+
title: 'Flow policy not found',
62+
description: 'The policy ID used to start this flow does not exist in the environment.',
63+
steps: [
64+
'Verify the policy ID (acr_values or flowId) matches your DaVinci environment.',
65+
'Check that the policy is published and assigned to the correct application.',
66+
],
67+
},
68+
});
69+
}
70+
}
71+
72+
return candidates;
73+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export type {
2+
FlowRule,
3+
IssueCandidate,
4+
Severity,
5+
DiagnosisCategory,
6+
FlowIssue,
7+
EventIssue,
8+
} from './types.js';
9+
export { collectCorsIssues } from './cors.js';
10+
export { collectTokenIssues } from './token.js';
11+
export { collectFlowConfigIssues } from './flow-config.js';
12+
export { collectOidcIssues } from './oidc.js';
13+
export { collectOidcFlowIssues } from './oidc-flow.js';
14+
export { collectDpopIssues } from './dpop.js';
15+
export { collectParIssues } from './par.js';

0 commit comments

Comments
 (0)