fix(oauth): accept private-use redirect schemes and loopback ports

POST /oauth/register refused every single-label private-use scheme, so an
MCP client registering workbuddy:// could not connect at all, while the
error it got back advertised "a private custom scheme" as supported.

The cause was two validators that had drifted apart, both of which run on
registration. The provider required a dot in the scheme name, which RFC
8252 section 7.1 only recommends and which protected nothing
(javascript.evil:// passed it). The service behind it had no
custom-scheme branch whatsoever and no [::1], which is what actually
answered the reporter's second symptom, and its localhost exemption was
not gated on http:. So javascript://localhost/%0aalert(1), blob:, about:,
ftp: and wss: were all accepted through the settings UI, and the consent
flow assigns the stored URI to window.location.href. Only the CSP stood
between that and a navigation. Both now decide through one policy in
oauth.helpers.ts.

Widening the scheme set means the deny-list has to carry its own weight:
it grows by the browser, extension and OS-handler schemes that were
previously blocked only as a side effect of the dot rule, including
ms-msdt (Follina) and the ms- namespace by prefix. intent: is denied,
because Chrome follows its embedded fallback URL, which turns it into an
open redirect the authorization server cannot inspect.

Fixing registration alone would have moved the reporter's [::1]:{port}
failure one step later: TREK matched redirect URIs byte for byte while
RFC 8252 section 7.3 frees the port for loopback, so a native client on
an OS-assigned port got a 302 to consent and then invalid_redirect_uri.
The matcher now relaxes the port, and only the port, for http loopback.
Stored URIs are never canonicalised, so the %3A round-trips untouched.

Note this tightens POST /api/oauth/clients for inputs it accepts today.
That is the security half of the change and applies at create time only,
so no already-registered client changes behaviour.

Closes #2227

server/tests/unit/nest/oauth-redirect-uri.test.ts

@@ -0,0 +1,134 @@
+/**
+ * The shared redirect_uri policy (#2227). Both registration paths (DCR through
+ * the SDK clients store and the settings UI through createOAuthClient) used to
+ * carry their own copy of it and disagreed in both directions: DCR refused every
+ * single-label private-use scheme, the settings route accepted
+ * javascript://localhost/…. These cases pin the one policy they now share.
+ */
+import { describe, it, expect } from 'vitest';
+import { classifyRedirectUri, redirectUriMatches, type RedirectUriVerdict } from '../../../src/nest/oauth/oauth.helpers';
+
+describe('classifyRedirectUri', () => {
+  const cases: Array<[string, RedirectUriVerdict]> = [
+    // https and loopback http
+    ['https://a.example.com/cb', 'ok'],
+    ['https://a.example.com:8443/cb?x=1#f', 'ok'],
+    ['http://localhost:8080/cb', 'ok'],
+    ['http://127.0.0.1/cb', 'ok'],
+    ['http://[::1]/cb', 'ok'],
+    ['http://[::1]:8080/oauth/callback', 'ok'],
+    ['http://evil.example.com/cb', 'not_allowed'],
+    ['http://127.0.0.1.evil.com/cb', 'not_allowed'],
+
+    // private-use schemes, with and without the reverse-domain form
+    ['com.example.app:/oauth2redirect', 'ok'],
+    ['workbuddy://workbuddy/mcp/connector%3A/oauth/callback', 'ok'],
+    ['myapp://cb', 'ok'],
+    ['msauth://com.x.y/abc', 'ok'],
+
+    // dangerous schemes, including the ones a localhost host used to smuggle past
+    // the settings route's hostname exemption
+    ['javascript:alert(1)', 'dangerous'],
+    ['javascript://localhost/%0aalert(1)', 'dangerous'],
+    ['blob://localhost/x', 'dangerous'],
+    ['about://localhost/x', 'dangerous'],
+    ['data:text/html,<script>alert(1)</script>', 'dangerous'],
+    ['vbscript:msgbox(1)', 'dangerous'],
+    ['file:///etc/passwd', 'dangerous'],
+    ['view-source:https://x', 'dangerous'],
+    ['filesystem:https://x/temporary/y', 'dangerous'],
+    ['jar:https://x!/y', 'dangerous'],
+    ['resource://x/y', 'dangerous'],
+    ['chrome://settings', 'dangerous'],
+    ['chrome-extension://abc/page.html', 'dangerous'],
+    ['chrome-untrusted://x/y', 'dangerous'],
+    ['moz-extension://a/b', 'dangerous'],
+    ['safari-extension://a/b', 'dangerous'],
+    ['android-app://com.x.y/https/host/path', 'dangerous'],
+    ['intent://x#Intent;S.browser_fallback_url=https%3A%2F%2Fevil.test;end', 'dangerous'],
+    // ms-msdt is Follina; the prefix rule covers the rest of the Windows handlers
+    ['ms-msdt:/id PCWDiagnostic', 'dangerous'],
+    ['ms-browser-extension://a/b', 'dangerous'],
+    ['ms-settings:windowsupdate', 'dangerous'],
+    ['ms-officecmd://x', 'dangerous'],
+
+    // transports that are not browser navigations
+    ['ws://localhost/cb', 'not_allowed'],
+    ['wss://localhost/cb', 'not_allowed'],
+    ['ftp://localhost/x', 'not_allowed'],
+    ['ftps://localhost/x', 'not_allowed'],
+    ['mailto:me@example.com', 'not_allowed'],
+    ['tel:+15550000', 'not_allowed'],
+    ['sms:+15550000', 'not_allowed'],
+    ['urn:ietf:wg:oauth:2.0:oob', 'not_allowed'],
+
+    // malformed
+    ['not a url', 'malformed'],
+    ['', 'malformed'],
+    ['/relative/cb', 'malformed'],
+  ];
+
+  it.each(cases)('RURI-001: %s → %s', (uri, expected) => {
+    expect(classifyRedirectUri(uri)).toBe(expected);
+  });
+
+  it('RURI-002: decides on the parsed protocol, so case and whitespace tricks normalise into the deny-list', () => {
+    for (const uri of ['JavaScript:alert(1)', 'java\tscript:alert(1)', ' javascript:alert(1)', 'JAVASCRIPT:alert(1)']) {
+      expect(classifyRedirectUri(uri)).toBe('dangerous');
+    }
+  });
+
+  it('RURI-004: OS protocol handlers are denied whether or not they carry the ms- prefix', () => {
+    // The prefix rule catches the ms-* namespace; these four are the well-known
+    // siblings that sit outside it and would otherwise read as private-use.
+    for (const uri of ['search-ms:query=x', 'shell:startup', 'microsoft-edge:https://evil.test', 'itms-services://?url=x']) {
+      expect(classifyRedirectUri(uri)).toBe('dangerous');
+    }
+    // Microsoft's own identity redirects have no hyphen and stay allowed.
+    expect(classifyRedirectUri('msauth://com.x.y/abc')).toBe('ok');
+  });
+
+  it('RURI-003: a single-letter scheme is not a private-use scheme', () => {
+    // 'c:\\tmp\\cb' parses with protocol 'c:', which is a Windows path, not a redirect.
+    expect(classifyRedirectUri('c:\\tmp\\cb')).toBe('not_allowed');
+  });
+});
+
+describe('redirectUriMatches', () => {
+  it('RURI-010: byte-exact matches win, including percent-encoding', () => {
+    const uri = 'workbuddy://workbuddy/mcp/connector%3A/oauth/callback';
+    expect(redirectUriMatches(uri, uri)).toBe(true);
+    expect(redirectUriMatches(uri, 'workbuddy://workbuddy/mcp/connector:/oauth/callback')).toBe(false);
+    expect(redirectUriMatches('https://a.example.com/cb', 'https://a.example.com/cb/')).toBe(false);
+  });
+
+  it('RURI-011: a loopback redirect may land on any port (RFC 8252 §7.3)', () => {
+    expect(redirectUriMatches('http://127.0.0.1:8080/callback', 'http://127.0.0.1:54321/callback')).toBe(true);
+    expect(redirectUriMatches('http://[::1]:8080/oauth/callback', 'http://[::1]:54321/oauth/callback')).toBe(true);
+    expect(redirectUriMatches('http://localhost/cb?x=1', 'http://localhost:9999/cb?x=1')).toBe(true);
+  });
+
+  it('RURI-012: only the port is free, so path, query and host still match exactly', () => {
+    expect(redirectUriMatches('http://127.0.0.1:8080/callback', 'http://127.0.0.1:54321/other')).toBe(false);
+    expect(redirectUriMatches('http://127.0.0.1:8080/cb?x=1', 'http://127.0.0.1:54321/cb?x=2')).toBe(false);
+    // The RFC does not allow localhost↔127.0.0.1 cross-matching.
+    expect(redirectUriMatches('http://localhost:8080/cb', 'http://127.0.0.1:54321/cb')).toBe(false);
+  });
+
+  it('RURI-013: non-loopback and non-http URIs stay byte-exact', () => {
+    expect(redirectUriMatches('https://a.example.com:8443/cb', 'https://a.example.com:9443/cb')).toBe(false);
+    expect(redirectUriMatches('http://evil.example.com:80/cb', 'http://evil.example.com:81/cb')).toBe(false);
+    expect(redirectUriMatches('https://localhost:8443/cb', 'https://localhost:9443/cb')).toBe(false);
+    expect(redirectUriMatches('myapp://localhost:1/cb', 'myapp://localhost:2/cb')).toBe(false);
+  });
+
+  it('RURI-015: userinfo and fragment are compared, not silently dropped', () => {
+    expect(redirectUriMatches('http://127.0.0.1:8080/cb', 'http://attacker@127.0.0.1:54321/cb')).toBe(false);
+    expect(redirectUriMatches('http://127.0.0.1:8080/cb', 'http://127.0.0.1:54321/cb#evil')).toBe(false);
+  });
+
+  it('RURI-014: an unparseable side never matches', () => {
+    expect(redirectUriMatches('not a url', 'http://127.0.0.1:1/cb')).toBe(false);
+    expect(redirectUriMatches('http://127.0.0.1:1/cb', 'not a url')).toBe(false);
+  });
+});

server/tests/unit/nest/oauth-sdk.provider.test.ts

@@ -141,23 +141,38 @@ describe('TrekClientsStore.registerClient', () => {
   it('SDKP-011: rejects dangerous schemes', async () => {
     await expect(store().registerClient({ redirect_uris: ['javascript:alert(1)'] } as never))
       .rejects.toThrowError(new InvalidClientMetadataError('Dangerous redirect URI scheme: javascript:alert(1)'));
+    // A host named localhost does not make the scheme safe (#2227).
+    for (const uri of ['javascript://localhost/%0aalert(1)', 'blob://localhost/x', 'about://localhost/x']) {
+      await expect(store().registerClient({ redirect_uris: [uri] } as never))
+        .rejects.toThrowError(new InvalidClientMetadataError(`Dangerous redirect URI scheme: ${uri}`));
+    }
   });
 
-  it('SDKP-012: rejects plain-http non-loopback and dot-less custom schemes', async () => {
+  it('SDKP-012: rejects plain-http non-loopback and non-navigable schemes', async () => {
     const msg = 'redirect_uris must be HTTPS, loopback HTTP, or a private custom scheme';
-    await expect(store().registerClient({ redirect_uris: ['http://evil.example.com/cb'] } as never))
-      .rejects.toThrowError(new InvalidClientMetadataError(msg));
-    await expect(store().registerClient({ redirect_uris: ['myapp://cb'] } as never))
-      .rejects.toThrowError(new InvalidClientMetadataError(msg));
+    for (const uri of ['http://evil.example.com/cb', 'ws://localhost/cb']) {
+      await expect(store().registerClient({ redirect_uris: [uri] } as never))
+        .rejects.toThrowError(new InvalidClientMetadataError(msg));
+    }
+    // These were only ever refused as a side effect of the old dot rule.
+    for (const uri of ['view-source:https://x', 'moz-extension://a/b']) {
+      await expect(store().registerClient({ redirect_uris: [uri] } as never))
+        .rejects.toThrowError(new InvalidClientMetadataError(`Dangerous redirect URI scheme: ${uri}`));
+    }
   });
 
-  it('SDKP-013: accepts https, loopback http, and reverse-DNS custom schemes', async () => {
+  it('SDKP-013: accepts https, loopback http, and private-use custom schemes', async () => {
     for (const uri of [
       'https://a.example.com/cb',
       'http://localhost:8080/cb',
       'http://127.0.0.1/cb',
       'http://[::1]/cb',
       'com.example.app:/oauth',
+      // #2227: single-label private-use schemes are legal (RFC 8252 §7.1 only
+      // SHOULDs the reverse-domain form) and were rejected outright.
+      'workbuddy://workbuddy/mcp/connector%3A/oauth/callback',
+      'myapp://cb',
+      'msauth://com.x.y/abc',
     ]) {
       const oauth = makeOauth();
       await new TrekClientsStore(oauth).registerClient({

server/tests/unit/nest/oauth.service.test.ts

@@ -207,6 +207,41 @@ describe('createOAuthClient', () => {
     expect(result.client).toBeDefined();
   });
 
+  it('allows the http://[::1] loopback redirect URI (#2227)', () => {
+    const { user } = createUser(testDb);
+    const result = createOAuthClient(user.id, 'Test', ['http://[::1]:8080/callback'], ['trips:read']);
+    expect(result.error).toBeUndefined();
+    expect(result.client).toBeDefined();
+  });
+
+  it('allows a private-use custom scheme (#2227)', () => {
+    const { user } = createUser(testDb);
+    const uri = 'workbuddy://workbuddy/mcp/connector%3A/oauth/callback';
+    const result = createOAuthClient(user.id, 'Test', [uri], ['trips:read']);
+    expect(result.error).toBeUndefined();
+    // The stored value must round-trip untouched: the whole authorize/token
+    // chain compares it byte for byte.
+    expect(result.client!.redirect_uris).toEqual([uri]);
+  });
+
+  it('rejects dangerous schemes that a localhost host used to smuggle through (#2227)', () => {
+    const { user } = createUser(testDb);
+    for (const uri of ['javascript://localhost/%0aalert(1)', 'blob://localhost/x', 'about://localhost/x']) {
+      const result = createOAuthClient(user.id, 'Test', [uri], ['trips:read']);
+      expect(result.status).toBe(400);
+      expect(result.error).toContain('Dangerous');
+    }
+  });
+
+  it('rejects non-navigable schemes on a localhost host (#2227)', () => {
+    const { user } = createUser(testDb);
+    for (const uri of ['ftp://localhost/x', 'wss://localhost/x']) {
+      const result = createOAuthClient(user.id, 'Test', [uri], ['trips:read']);
+      expect(result.status).toBe(400);
+      expect(result.error).toContain('HTTPS');
+    }
+  });
+
   it('returns 400 error if no scopes provided', () => {
     const { user } = createUser(testDb);
     const result = createOAuthClient(user.id, 'Test', ['https://example.com/cb'], []);
@@ -623,6 +658,31 @@ describe('validateAuthorizeRequest', () => {
     expect(result.error).toBe('invalid_redirect_uri');
   });
 
+  it('accepts a loopback redirect on the port the OS handed the client (#2227)', () => {
+    const { user } = createUser(testDb);
+    const created = makeClient(user.id, { redirectUris: ['http://[::1]:8080/oauth/callback'] });
+    const clientId = created.client!.client_id as string;
+
+    const result = validateAuthorizeRequest(
+      makeParams({ client_id: clientId, redirect_uri: 'http://[::1]:54321/oauth/callback' }),
+      user.id
+    );
+    expect(result.valid).toBe(true);
+  });
+
+  it('relaxes only the port of a loopback redirect, never the path (#2227)', () => {
+    const { user } = createUser(testDb);
+    const created = makeClient(user.id, { redirectUris: ['http://127.0.0.1:8080/oauth/callback'] });
+    const clientId = created.client!.client_id as string;
+
+    const result = validateAuthorizeRequest(
+      makeParams({ client_id: clientId, redirect_uri: 'http://127.0.0.1:54321/stolen' }),
+      user.id
+    );
+    expect(result.valid).toBe(false);
+    expect(result.error).toBe('invalid_redirect_uri');
+  });
+
   it('validates scope against client allowed_scopes', () => {
     const { user } = createUser(testDb);
     const created = makeClient(user.id, { scopes: ['trips:read'] });