diff --git a/src/cli.ts b/src/cli.ts index e1cb8dbe2..14221abe4 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -2325,7 +2325,8 @@ ADMIN serve MCP server (stdio) serve --http [--port N] HTTP MCP server with OAuth 2.1 --token-ttl N Access token TTL in seconds (default: 3600) - --enable-dcr Enable Dynamic Client Registration + --enable-dcr Enable Dynamic Client Registration (DCR clients default to authorization_code) + --enable-dcr-insecure Also allow the consent-bypassing client_credentials grant on DCR (implies --enable-dcr) --public-url URL Public issuer URL (required behind proxy/tunnel) connect --token Wire Claude Code to a remote gbrain (bearer token) [--install] [--json] Print the paste-ready command, or --install to run it diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index e7003973c..5a0539466 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -259,6 +259,11 @@ interface ServeHttpOptions { port: number; tokenTtl: number; enableDcr: boolean; + /** + * #1353: allow the consent-bypassing client_credentials grant on the DCR path. + * Off by default; DCR clients default to authorization_code. Implies enableDcr. + */ + enableDcrInsecure?: boolean; /** * Public URL the server is reachable at (e.g., https://brain.example.com). * Used as the OAuth issuer in discovery metadata. Defaults to @@ -396,7 +401,7 @@ export function skillPublishStatus(publishSkills: boolean): { bannerValue: strin } export async function runServeHttp(engine: BrainEngine, options: ServeHttpOptions) { - const { port, tokenTtl, enableDcr, publicUrl, logFullParams } = options; + const { port, tokenTtl, enableDcr, enableDcrInsecure, publicUrl, logFullParams } = options; // v0.34.1 (#864, D11): default bind flipped from 0.0.0.0 to 127.0.0.1. // gbrain's primary use case is a personal-knowledge brain on a laptop; // the pre-v0.34 default exposed brains on every interface. Server @@ -479,8 +484,28 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption sql, tokenTtl, dcrDisabled: !enableDcr, + allowClientCredentialsDcr: enableDcrInsecure === true, }); + // #1353: loud stderr security WARN when DCR is enabled. DCR is an + // unauthenticated network registration endpoint; surface the posture change + // (and the extra blast radius of --enable-dcr-insecure) so it's visible in + // logs, not buried in the neutral "DCR: enabled" banner line. + if (enableDcr) { + console.error( + 'SECURITY WARNING: Dynamic Client Registration (--enable-dcr) is ON. ' + + 'Any network caller can self-register an OAuth client. DCR clients default ' + + 'to the authorization_code (consent-bearing) grant. See SECURITY.md.', + ); + if (enableDcrInsecure) { + console.error( + 'SECURITY WARNING: --enable-dcr-insecure is ON — self-registered DCR ' + + 'clients may request the client_credentials grant, which BYPASSES the ' + + '/authorize consent screen. Only use this on a trusted network.', + ); + } + } + // Sweep expired tokens on startup (non-blocking) try { const swept = await oauthProvider.sweepExpiredTokens(); @@ -2133,7 +2158,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption ║ Engine: ${(config.engine || 'pglite').padEnd(40)}║ ║ Issuer: ${issuerUrl.origin.padEnd(40)}║ ║ Clients: ${String((clientCount[0] as any).count).padEnd(40)}║ -║ DCR: ${(enableDcr ? 'enabled' : 'disabled').padEnd(40)}║ +║ DCR: ${(enableDcr ? (enableDcrInsecure ? 'enabled (INSECURE: client_credentials)' : 'enabled') : 'disabled').padEnd(40)}║ ║ Skills: ${skillStatus.bannerValue.padEnd(40)}║ ║ Token TTL: ${(tokenTtl + 's').padEnd(40)}║ ╠══════════════════════════════════════════════════════╣ diff --git a/src/commands/serve.ts b/src/commands/serve.ts index 058c859e7..de7352dc4 100644 --- a/src/commands/serve.ts +++ b/src/commands/serve.ts @@ -85,7 +85,12 @@ export async function runServe( const ttlIdx = args.indexOf('--token-ttl'); const tokenTtl = ttlIdx >= 0 ? parseInt(args[ttlIdx + 1]) || 3600 : 3600; - const enableDcr = args.includes('--enable-dcr'); + // #1353: --enable-dcr-insecure opts into the consent-bypassing + // client_credentials grant on the DCR path. It implies --enable-dcr (you + // can't allow insecure DCR clients without DCR). Plain --enable-dcr keeps + // the secure default: DCR clients are authorization_code (consent-bearing). + const enableDcrInsecure = args.includes('--enable-dcr-insecure'); + const enableDcr = args.includes('--enable-dcr') || enableDcrInsecure; const publicUrlIdx = args.indexOf('--public-url'); const publicUrl = publicUrlIdx >= 0 ? args[publicUrlIdx + 1] : undefined; @@ -114,7 +119,7 @@ export async function runServe( const suppressBootstrapToken = args.includes('--suppress-bootstrap-token'); const { runServeHttp } = await import('./serve-http.ts'); - await runServeHttp(engine, { port, tokenTtl, enableDcr, publicUrl, logFullParams, bind, suppressBootstrapToken }); + await runServeHttp(engine, { port, tokenTtl, enableDcr, enableDcrInsecure, publicUrl, logFullParams, bind, suppressBootstrapToken }); return; } diff --git a/src/core/oauth-provider.ts b/src/core/oauth-provider.ts index 83358536b..52845ea18 100644 --- a/src/core/oauth-provider.ts +++ b/src/core/oauth-provider.ts @@ -22,7 +22,7 @@ import type { import type { OAuthServerProvider, AuthorizationParams } from '@modelcontextprotocol/sdk/server/auth/provider.js'; import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/server/auth/clients.js'; import type { AuthInfo as SdkAuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; -import { InvalidTokenError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; +import { InvalidTokenError, InvalidClientMetadataError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; import { hashToken, generateToken, isUndefinedColumnError } from './utils.ts'; import { hasScope, assertAllowedScopes, parseScopeString, InvalidScopeError } from './scope.ts'; import type { AuthInfo as CoreAuthInfo } from './operations.ts'; @@ -183,6 +183,16 @@ interface GBrainOAuthProviderOptions { * before mcpAuthRouter ran). */ dcrDisabled?: boolean; + /** + * Allow the consent-bypassing `client_credentials` grant on the unauthenticated + * Dynamic Client Registration path. Default false (#1353): a self-registered + * DCR client defaults to `authorization_code` (which goes through /authorize + * consent), and an explicit `client_credentials` request is rejected. Operators + * who genuinely need machine-to-machine DCR clients opt in via + * `--enable-dcr-insecure`. Manual CLI / admin registration is unaffected + * (operator-trusted, registers grants directly). + */ + allowClientCredentialsDcr?: boolean; } // --------------------------------------------------------------------------- @@ -190,7 +200,7 @@ interface GBrainOAuthProviderOptions { // --------------------------------------------------------------------------- class GBrainClientsStore implements OAuthRegisteredClientsStore { - constructor(private sql: SqlQuery) {} + constructor(private sql: SqlQuery, private allowClientCredentialsDcr = false) {} async getClient(clientId: string): Promise { const rows = await this.sql` @@ -243,6 +253,24 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore { // registration entry points share one allow-list. const authMethod = validateTokenEndpointAuthMethod(client.token_endpoint_auth_method); + // v0.42 (#1353): the DCR path is the unauthenticated network entry point. + // `client_credentials` skips /authorize consent entirely, so a self- + // registered DCR client must NOT get it by default. Default the grant to + // `authorization_code` (the consent-bearing flow) when unspecified, and + // reject an explicit `client_credentials` request unless the operator opted + // in via `--enable-dcr-insecure`. Manual CLI/admin registration bypasses + // this store method, so operators can still mint machine clients directly. + const grantTypes = (client.grant_types && client.grant_types.length > 0) + ? client.grant_types + : ['authorization_code']; + if (!this.allowClientCredentialsDcr && grantTypes.includes('client_credentials')) { + throw new InvalidClientMetadataError( + 'client_credentials grant is not permitted via dynamic client registration; ' + + 'restart the server with --enable-dcr-insecure to allow it, or register the ' + + 'client via the gbrain CLI / admin API.', + ); + } + const clientId = generateToken('gbrain_cl_'); // v0.34.1 (#909): RFC 7591 §2 — clients that authenticate at the token // endpoint via PKCE alone declare `token_endpoint_auth_method: "none"`. @@ -273,7 +301,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore { client_id_issued_at, source_id, federated_read) VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'}, ${pgArray((client.redirect_uris || []).map(String))}, - ${pgArray(client.grant_types || ['client_credentials'])}, + ${pgArray(grantTypes)}, ${client.scope || ''}, ${authMethod}, ${now}, ${'default'}, ${pgArray(['default'])}) `; @@ -286,7 +314,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore { client_id_issued_at, source_id) VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'}, ${pgArray((client.redirect_uris || []).map(String))}, - ${pgArray(client.grant_types || ['client_credentials'])}, + ${pgArray(grantTypes)}, ${client.scope || ''}, ${authMethod}, ${now}, ${'default'}) `; @@ -298,7 +326,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore { client_id_issued_at) VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'}, ${pgArray((client.redirect_uris || []).map(String))}, - ${pgArray(client.grant_types || ['client_credentials'])}, + ${pgArray(grantTypes)}, ${client.scope || ''}, ${authMethod}, ${now}) `; @@ -313,7 +341,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore { client_id_issued_at) VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'}, ${pgArray((client.redirect_uris || []).map(String))}, - ${pgArray(client.grant_types || ['client_credentials'])}, + ${pgArray(grantTypes)}, ${client.scope || ''}, ${authMethod}, ${now}) `; @@ -350,7 +378,7 @@ export class GBrainOAuthProvider implements OAuthServerProvider { constructor(options: GBrainOAuthProviderOptions) { this.sql = options.sql; - this._clientsStore = new GBrainClientsStore(this.sql); + this._clientsStore = new GBrainClientsStore(this.sql, options.allowClientCredentialsDcr === true); this.dcrDisabled = options.dcrDisabled === true; this.tokenTtl = options.tokenTtl || 3600; this.refreshTtl = options.refreshTtl || 30 * 24 * 3600; diff --git a/test/oauth.test.ts b/test/oauth.test.ts index 3c466ec5b..ee528edd3 100644 --- a/test/oauth.test.ts +++ b/test/oauth.test.ts @@ -11,7 +11,7 @@ import { } from '../src/core/oauth-provider.ts'; import { hashToken, generateToken } from '../src/core/utils.ts'; import { PGLITE_SCHEMA_SQL } from '../src/core/pglite-schema.ts'; -import { InvalidTokenError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; +import { InvalidTokenError, InvalidClientMetadataError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; import type { AuthInfo as CoreAuthInfo } from '../src/core/operations.ts'; // --------------------------------------------------------------------------- @@ -1489,12 +1489,50 @@ describe('v0.41.3 DCR validator (T5)', () => { test('DCR accepts "client_secret_basic" — codex F3 regression', async () => { const reg = await provider.clientsStore.registerClient!({ client_name: 'dcr-basic-test', - grant_types: ['client_credentials'], + grant_types: ['authorization_code'], scope: 'read', - redirect_uris: [], + redirect_uris: ['https://example.test/cb'], token_endpoint_auth_method: 'client_secret_basic', } as any); expect(reg.client_id).toStartWith('gbrain_cl_'); expect(reg.client_secret).toStartWith('gbrain_cs_'); }); }); + +describe('#1353 DCR default-grant hardening', () => { + test('DCR rejects explicit client_credentials by default', async () => { + await expect( + provider.clientsStore.registerClient!({ + client_name: 'cc-default-test', + grant_types: ['client_credentials'], + scope: 'read', + redirect_uris: [], + token_endpoint_auth_method: 'client_secret_post', + } as any), + ).rejects.toThrow(InvalidClientMetadataError); + }); + + test('DCR defaults to authorization_code when grant_types unspecified', async () => { + const reg = await provider.clientsStore.registerClient!({ + client_name: 'no-grant-test', + scope: 'read', + redirect_uris: ['https://example.test/cb'], + token_endpoint_auth_method: 'none', + } as any); + const stored = await provider.clientsStore.getClient(reg.client_id); + expect(stored?.grant_types).toEqual(['authorization_code']); + }); + + test('--enable-dcr-insecure (allowClientCredentialsDcr) permits client_credentials', async () => { + const insecure = new GBrainOAuthProvider({ sql, allowClientCredentialsDcr: true }); + const reg = await insecure.clientsStore.registerClient!({ + client_name: 'cc-allowed-test', + grant_types: ['client_credentials'], + scope: 'read', + redirect_uris: [], + token_endpoint_auth_method: 'client_secret_post', + } as any); + const stored = await insecure.clientsStore.getClient(reg.client_id); + expect(stored?.grant_types).toEqual(['client_credentials']); + }); +});