Skip to content

Commit 460ccf6

Browse files
aidandaly24aidandaly24
committed
fix(gateway): harden inbound auth schema and rename credential flags
- Enforce HTTPS on OIDC discovery URL in schema and CLI validation - Make allowedAudience/allowedClients optional with at-least-one superRefine constraint (audience, clients, or scopes) - Add .strict() to CustomJwtAuthorizerConfigSchema - Rename --agent-client-id/--agent-client-secret to --client-id/--client-secret across CLI, TUI, and primitives - Add HTTPS validation to TUI discovery URL input - Update deployed-state schema to match (optional audience/clients, add allowedScopes) - Update unit tests for new validation rules and field names Constraint: OIDC spec requires HTTPS for discovery endpoints Rejected: Keep --agent-client-id naming | confusing since these are gateway-level OAuth credentials, not agent credentials Confidence: high Scope-risk: moderate
1 parent aec6102 commit 460ccf6

12 files changed

Lines changed: 233 additions & 146 deletions

File tree

‎src/cli/commands/add/__tests__/add-gateway.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ describe('add gateway command', () => {
167167
'client1',
168168
'--allowed-scopes',
169169
'scope1,scope2',
170-
'--agent-client-id',
170+
'--client-id',
171171
'agent-cid',
172-
'--agent-client-secret',
172+
'--client-secret',
173173
'agent-secret',
174174
'--json',
175175
],

‎src/cli/commands/add/__tests__/validate.test.ts‎

Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -220,24 +220,40 @@ describe('validate', () => {
220220
expect(result.error?.includes('Invalid authorizer type')).toBeTruthy();
221221
});
222222

223-
// AC11: CUSTOM_JWT requires discoveryUrl and allowedClients (allowedAudience is optional)
224-
it('returns error for CUSTOM_JWT missing required fields', () => {
225-
const jwtFields: { field: keyof AddGatewayOptions; error: string }[] = [
226-
{ field: 'discoveryUrl', error: '--discovery-url is required for CUSTOM_JWT authorizer' },
227-
{ field: 'allowedClients', error: '--allowed-clients is required for CUSTOM_JWT authorizer' },
228-
];
223+
// AC11: CUSTOM_JWT requires discoveryUrl
224+
it('returns error for CUSTOM_JWT missing discoveryUrl', () => {
225+
const opts = { ...validGatewayOptionsJwt, discoveryUrl: undefined };
226+
const result = validateAddGatewayOptions(opts);
227+
expect(result.valid).toBe(false);
228+
expect(result.error).toBe('--discovery-url is required for CUSTOM_JWT authorizer');
229+
});
229230

230-
for (const { field, error } of jwtFields) {
231-
const opts = { ...validGatewayOptionsJwt, [field]: undefined };
232-
const result = validateAddGatewayOptions(opts);
233-
expect(result.valid, `Should fail for missing ${String(field)}`).toBe(false);
234-
expect(result.error).toBe(error);
235-
}
231+
// AC11b: at least one of audience/clients/scopes required
232+
it('returns error when all of audience, clients, and scopes are missing', () => {
233+
const opts = {
234+
...validGatewayOptionsJwt,
235+
allowedAudience: undefined,
236+
allowedClients: undefined,
237+
allowedScopes: undefined,
238+
};
239+
const result = validateAddGatewayOptions(opts);
240+
expect(result.valid).toBe(false);
241+
expect(result.error).toContain('At least one of');
242+
});
243+
244+
it('allows CUSTOM_JWT with only allowedScopes', () => {
245+
const opts = {
246+
...validGatewayOptionsJwt,
247+
allowedAudience: undefined,
248+
allowedClients: undefined,
249+
allowedScopes: 'scope1',
250+
};
251+
const result = validateAddGatewayOptions(opts);
252+
expect(result.valid).toBe(true);
236253
});
237254

238-
// AC11b: allowedAudience is optional
239-
it('allows CUSTOM_JWT without allowedAudience', () => {
240-
const opts = { ...validGatewayOptionsJwt, allowedAudience: undefined };
255+
it('allows CUSTOM_JWT with only allowedAudience', () => {
256+
const opts = { ...validGatewayOptionsJwt, allowedClients: undefined, allowedScopes: undefined };
241257
const result = validateAddGatewayOptions(opts);
242258
expect(result.valid).toBe(true);
243259
});
@@ -255,11 +271,19 @@ describe('validate', () => {
255271
expect(result.error?.includes('.well-known/openid-configuration')).toBeTruthy();
256272
});
257273

258-
// AC13: Empty comma-separated clients rejected (audience can be empty)
259-
it('returns error for empty clients', () => {
260-
const result = validateAddGatewayOptions({ ...validGatewayOptionsJwt, allowedClients: ' , ' });
274+
it('returns error for HTTP discoveryUrl (HTTPS required)', () => {
275+
const result = validateAddGatewayOptions({
276+
...validGatewayOptionsJwt,
277+
discoveryUrl: 'http://example.com/.well-known/openid-configuration',
278+
});
261279
expect(result.valid).toBe(false);
262-
expect(result.error).toBe('At least one client value is required');
280+
expect(result.error).toBe('Discovery URL must use HTTPS');
281+
});
282+
283+
it('allows CUSTOM_JWT with only allowedClients', () => {
284+
const opts = { ...validGatewayOptionsJwt, allowedAudience: undefined, allowedScopes: undefined };
285+
const result = validateAddGatewayOptions(opts);
286+
expect(result.valid).toBe(true);
263287
});
264288

265289
// AC14: Valid options pass
@@ -268,42 +292,42 @@ describe('validate', () => {
268292
expect(validateAddGatewayOptions(validGatewayOptionsJwt)).toEqual({ valid: true });
269293
});
270294

271-
// AC15: agentClientId and agentClientSecret must be provided together
272-
it('returns error when agentClientId provided without agentClientSecret', () => {
295+
// AC15: clientId and clientSecret must be provided together
296+
it('returns error when clientId provided without clientSecret', () => {
273297
const result = validateAddGatewayOptions({
274298
...validGatewayOptionsJwt,
275-
agentClientId: 'my-client-id',
299+
clientId: 'my-client-id',
276300
});
277301
expect(result.valid).toBe(false);
278-
expect(result.error).toBe('Both --agent-client-id and --agent-client-secret must be provided together');
302+
expect(result.error).toBe('Both --client-id and --client-secret must be provided together');
279303
});
280304

281-
it('returns error when agentClientSecret provided without agentClientId', () => {
305+
it('returns error when clientSecret provided without clientId', () => {
282306
const result = validateAddGatewayOptions({
283307
...validGatewayOptionsJwt,
284-
agentClientSecret: 'my-secret',
308+
clientSecret: 'my-secret',
285309
});
286310
expect(result.valid).toBe(false);
287-
expect(result.error).toBe('Both --agent-client-id and --agent-client-secret must be provided together');
311+
expect(result.error).toBe('Both --client-id and --client-secret must be provided together');
288312
});
289313

290-
// AC16: agent credentials only valid with CUSTOM_JWT
291-
it('returns error when agent credentials used with non-CUSTOM_JWT authorizer', () => {
314+
// AC16: OAuth credentials only valid with CUSTOM_JWT
315+
it('returns error when OAuth credentials used with non-CUSTOM_JWT authorizer', () => {
292316
const result = validateAddGatewayOptions({
293317
...validGatewayOptionsNone,
294-
agentClientId: 'my-client-id',
295-
agentClientSecret: 'my-secret',
318+
clientId: 'my-client-id',
319+
clientSecret: 'my-secret',
296320
});
297321
expect(result.valid).toBe(false);
298-
expect(result.error).toBe('Agent OAuth credentials are only valid with CUSTOM_JWT authorizer');
322+
expect(result.error).toBe('OAuth client credentials are only valid with CUSTOM_JWT authorizer');
299323
});
300324

301-
// AC17: valid CUSTOM_JWT with agent credentials passes
302-
it('passes for CUSTOM_JWT with agent credentials', () => {
325+
// AC17: valid CUSTOM_JWT with OAuth credentials passes
326+
it('passes for CUSTOM_JWT with OAuth credentials', () => {
303327
const result = validateAddGatewayOptions({
304328
...validGatewayOptionsJwt,
305-
agentClientId: 'my-client-id',
306-
agentClientSecret: 'my-secret',
329+
clientId: 'my-client-id',
330+
clientSecret: 'my-secret',
307331
allowedScopes: 'scope1,scope2',
308332
});
309333
expect(result.valid).toBe(true);

‎src/cli/commands/add/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ export interface AddGatewayOptions {
3434
allowedAudience?: string;
3535
allowedClients?: string;
3636
allowedScopes?: string;
37-
agentClientId?: string;
38-
agentClientSecret?: string;
37+
clientId?: string;
38+
clientSecret?: string;
3939
agents?: string;
4040
semanticSearch?: boolean;
4141
exceptionLevel?: string;

‎src/cli/commands/add/validate.ts‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,10 @@ export function validateAddGatewayOptions(options: AddGatewayOptions): Validatio
230230
}
231231

232232
try {
233-
new URL(options.discoveryUrl);
233+
const url = new URL(options.discoveryUrl);
234+
if (url.protocol !== 'https:') {
235+
return { valid: false, error: 'Discovery URL must use HTTPS' };
236+
}
234237
} catch {
235238
return { valid: false, error: 'Discovery URL must be a valid URL' };
236239
}
@@ -239,30 +242,29 @@ export function validateAddGatewayOptions(options: AddGatewayOptions): Validatio
239242
return { valid: false, error: `Discovery URL must end with ${OIDC_WELL_KNOWN_SUFFIX}` };
240243
}
241244

242-
// allowedAudience is optional - empty means no audience validation
243-
244-
if (!options.allowedClients) {
245-
return { valid: false, error: '--allowed-clients is required for CUSTOM_JWT authorizer' };
246-
}
247-
248-
const clients = options.allowedClients
249-
.split(',')
250-
.map(s => s.trim())
251-
.filter(Boolean);
252-
if (clients.length === 0) {
253-
return { valid: false, error: 'At least one client value is required' };
245+
// allowedAudience, allowedClients, allowedScopes are all optional individually,
246+
// but at least one must be provided
247+
const hasAudience = !!options.allowedAudience?.trim();
248+
const hasClients = !!options.allowedClients?.trim();
249+
const hasScopes = !!options.allowedScopes?.trim();
250+
if (!hasAudience && !hasClients && !hasScopes) {
251+
return {
252+
valid: false,
253+
error:
254+
'At least one of --allowed-audience, --allowed-clients, or --allowed-scopes must be provided for CUSTOM_JWT authorizer',
255+
};
254256
}
255257
}
256258

257-
// Validate agent OAuth credentials
258-
if (options.agentClientId && !options.agentClientSecret) {
259-
return { valid: false, error: 'Both --agent-client-id and --agent-client-secret must be provided together' };
259+
// Validate OAuth client credentials
260+
if (options.clientId && !options.clientSecret) {
261+
return { valid: false, error: 'Both --client-id and --client-secret must be provided together' };
260262
}
261-
if (options.agentClientSecret && !options.agentClientId) {
262-
return { valid: false, error: 'Both --agent-client-id and --agent-client-secret must be provided together' };
263+
if (options.clientSecret && !options.clientId) {
264+
return { valid: false, error: 'Both --client-id and --client-secret must be provided together' };
263265
}
264-
if (options.agentClientId && options.authorizerType !== 'CUSTOM_JWT') {
265-
return { valid: false, error: 'Agent OAuth credentials are only valid with CUSTOM_JWT authorizer' };
266+
if (options.clientId && options.authorizerType !== 'CUSTOM_JWT') {
267+
return { valid: false, error: 'OAuth client credentials are only valid with CUSTOM_JWT authorizer' };
266268
}
267269

268270
// Validate exception level if provided

‎src/cli/primitives/GatewayPrimitive.ts‎

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ export interface AddGatewayOptions {
2323
allowedAudience?: string;
2424
allowedClients?: string;
2525
allowedScopes?: string;
26-
agentClientId?: string;
27-
agentClientSecret?: string;
26+
clientId?: string;
27+
clientSecret?: string;
2828
agents?: string;
2929
enableSemanticSearch?: boolean;
3030
exceptionLevel?: string;
@@ -155,8 +155,8 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
155155
.option('--allowed-audience <audience>', 'Comma-separated allowed audiences (for CUSTOM_JWT)')
156156
.option('--allowed-clients <clients>', 'Comma-separated allowed client IDs (for CUSTOM_JWT)')
157157
.option('--allowed-scopes <scopes>', 'Comma-separated allowed scopes (for CUSTOM_JWT)')
158-
.option('--agent-client-id <id>', 'Agent OAuth client ID')
159-
.option('--agent-client-secret <secret>', 'Agent OAuth client secret')
158+
.option('--client-id <id>', 'OAuth client ID for gateway bearer token')
159+
.option('--client-secret <secret>', 'OAuth client secret')
160160
.option('--agents <agents>', 'Comma-separated agent names')
161161
.option('--no-semantic-search', 'Disable semantic search for tool discovery')
162162
.option('--exception-level <level>', 'Exception verbosity level', 'NONE')
@@ -187,8 +187,8 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
187187
allowedAudience: cliOptions.allowedAudience,
188188
allowedClients: cliOptions.allowedClients,
189189
allowedScopes: cliOptions.allowedScopes,
190-
agentClientId: cliOptions.agentClientId,
191-
agentClientSecret: cliOptions.agentClientSecret,
190+
clientId: cliOptions.clientId,
191+
clientSecret: cliOptions.clientSecret,
192192
agents: cliOptions.agents,
193193
enableSemanticSearch: cliOptions.semanticSearch !== false,
194194
exceptionLevel: cliOptions.exceptionLevel,
@@ -293,30 +293,32 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
293293
};
294294

295295
if (options.authorizerType === 'CUSTOM_JWT' && options.discoveryUrl) {
296+
const allowedAudience = options.allowedAudience
297+
? options.allowedAudience
298+
.split(',')
299+
.map(s => s.trim())
300+
.filter(Boolean)
301+
: undefined;
302+
const allowedClients = options.allowedClients
303+
? options.allowedClients
304+
.split(',')
305+
.map(s => s.trim())
306+
.filter(Boolean)
307+
: undefined;
308+
const allowedScopes = options.allowedScopes
309+
? options.allowedScopes
310+
.split(',')
311+
.map(s => s.trim())
312+
.filter(Boolean)
313+
: undefined;
314+
296315
config.jwtConfig = {
297316
discoveryUrl: options.discoveryUrl,
298-
allowedAudience: options.allowedAudience
299-
? options.allowedAudience
300-
.split(',')
301-
.map(s => s.trim())
302-
.filter(Boolean)
303-
: [],
304-
allowedClients: options.allowedClients
305-
? options.allowedClients
306-
.split(',')
307-
.map(s => s.trim())
308-
.filter(Boolean)
309-
: [],
310-
...(options.allowedScopes
311-
? {
312-
allowedScopes: options.allowedScopes
313-
.split(',')
314-
.map(s => s.trim())
315-
.filter(Boolean),
316-
}
317-
: {}),
318-
...(options.agentClientId ? { agentClientId: options.agentClientId } : {}),
319-
...(options.agentClientSecret ? { agentClientSecret: options.agentClientSecret } : {}),
317+
...(allowedAudience?.length ? { allowedAudience } : {}),
318+
...(allowedClients?.length ? { allowedClients } : {}),
319+
...(allowedScopes?.length ? { allowedScopes } : {}),
320+
...(options.clientId ? { clientId: options.clientId } : {}),
321+
...(options.clientSecret ? { clientSecret: options.clientSecret } : {}),
320322
};
321323
}
322324

@@ -363,8 +365,8 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
363365
mcpSpec.agentCoreGateways.push(gateway);
364366
await this.configIO.writeMcpSpec(mcpSpec);
365367

366-
// Auto-create OAuth credential if agent client credentials are provided
367-
if (config.jwtConfig?.agentClientId && config.jwtConfig?.agentClientSecret) {
368+
// Auto-create OAuth credential if client credentials are provided
369+
if (config.jwtConfig?.clientId && config.jwtConfig?.clientSecret) {
368370
await this.createManagedOAuthCredential(config.name, config.jwtConfig);
369371
}
370372

@@ -399,7 +401,7 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
399401

400402
// Write client secret to .env
401403
const envVarName = computeDefaultCredentialEnvVarName(credentialName);
402-
await setEnvVar(envVarName, jwtConfig.agentClientSecret!);
404+
await setEnvVar(envVarName, jwtConfig.clientSecret!);
403405
}
404406

405407
/**
@@ -413,11 +415,9 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
413415
return {
414416
customJwtAuthorizer: {
415417
discoveryUrl: config.jwtConfig.discoveryUrl,
416-
allowedAudience: config.jwtConfig.allowedAudience,
417-
allowedClients: config.jwtConfig.allowedClients,
418-
...(config.jwtConfig.allowedScopes && config.jwtConfig.allowedScopes.length > 0
419-
? { allowedScopes: config.jwtConfig.allowedScopes }
420-
: {}),
418+
...(config.jwtConfig.allowedAudience?.length ? { allowedAudience: config.jwtConfig.allowedAudience } : {}),
419+
...(config.jwtConfig.allowedClients?.length ? { allowedClients: config.jwtConfig.allowedClients } : {}),
420+
...(config.jwtConfig.allowedScopes?.length ? { allowedScopes: config.jwtConfig.allowedScopes } : {}),
421421
},
422422
};
423423
}

‎src/cli/tui/hooks/useCreateMcp.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ export function useCreateGateway() {
2626
allowedAudience: config.jwtConfig?.allowedAudience?.join(','),
2727
allowedClients: config.jwtConfig?.allowedClients?.join(','),
2828
allowedScopes: config.jwtConfig?.allowedScopes?.join(','),
29-
agentClientId: config.jwtConfig?.agentClientId,
30-
agentClientSecret: config.jwtConfig?.agentClientSecret,
29+
clientId: config.jwtConfig?.clientId,
30+
clientSecret: config.jwtConfig?.clientSecret,
3131
enableSemanticSearch: config.enableSemanticSearch,
3232
exceptionLevel: config.exceptionLevel,
3333
});

0 commit comments

Comments
 (0)