fix(web): bound route label cardinality with an allowlist

Truncating path depth bounded depth, not breadth. The first path segment is
client-supplied and /api/[...slug] is a catch-all, so /wp-admin, /.env, and
/api/<anything> each minted a new time series. Scanner traffic could grow the
series count without limit, which is exactly what the normalization was
supposed to prevent.

Match the truncated path against a known-route set and report anything else as
`other`, bounding distinct route labels to that set plus one regardless of what
is requested. A route missing from the set loses granularity rather than
breaking, so it fails closed.

Also strengthens the metrics-port exclusion assertion. It checked for the
absence of a `/metrics` label, which became vacuous once unknown paths collapse
to `other` — it now asserts the total observation count, and fails if the port
filter is removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: brendan-kellam

packages/web/src/httpMetrics.integration.test.ts

@@ -59,7 +59,12 @@ describe('httpMetrics', () => {
         expect(browse).toHaveLength(1);
         expect(browse[0].trim().endsWith('2')).toBe(true);
 
-        // The scrape of the metrics port must not appear at all.
-        expect(counts.some(line => line.includes('route="/metrics"'))).toBe(false);
+        // The scrape of the metrics port must not be recorded. Asserted on the
+        // total observation count rather than on the absence of a `/metrics`
+        // label: `/metrics` is not a known route, so it would land in `other`
+        // and an absent-label check would pass even with the filter removed.
+        const total = counts.reduce((sum, line) => sum + Number(line.trim().split(' ').pop()), 0);
+        expect(total).toBe(3);
+        expect(counts.some(line => line.includes('route="other"'))).toBe(false);
     });
 });

packages/web/src/httpMetrics.test.ts

@@ -1,51 +1,79 @@
 import { describe, expect, it } from 'vitest';
-import { normalizeRoute } from './httpMetrics';
+import { MAX_ROUTE_LABELS, normalizeRoute } from './httpMetrics';
 
 describe('normalizeRoute', () => {
     it('maps the root path', () => {
         expect(normalizeRoute('/')).toBe('/');
         expect(normalizeRoute('')).toBe('/');
     });
 
-    it('keeps two segments for API routes', () => {
+    it('keeps two segments for known API routes', () => {
         expect(normalizeRoute('/api/health')).toBe('/api/health');
         expect(normalizeRoute('/api/commits')).toBe('/api/commits');
         expect(normalizeRoute('/api/auth/callback/github')).toBe('/api/auth');
     });
 
-    it('keeps one segment for page routes', () => {
+    it('keeps one segment for known page routes', () => {
         expect(normalizeRoute('/search')).toBe('/search');
         expect(normalizeRoute('/settings/connections/42')).toBe('/settings');
     });
 
-    it('bounds unbounded repository and file paths', () => {
+    it('collapses unbounded repository and file paths', () => {
         const a = normalizeRoute('/browse/github.com/org/repo/-/blob/src/index.ts');
         const b = normalizeRoute('/browse/github.com/other/repo/-/blob/lib/other.ts');
 
         expect(a).toBe('/browse');
-        expect(b).toBe('/browse');
-        // The point of normalizing: distinct files must not mint distinct labels.
-        expect(a).toBe(b);
+        expect(b).toBe(a);
     });
 
     it('is unaffected by trailing or duplicate slashes', () => {
         expect(normalizeRoute('/search/')).toBe('/search');
         expect(normalizeRoute('//search//')).toBe('/search');
     });
 
-    it('produces a bounded label set for a realistic path mix', () => {
-        const paths = [
-            '/', '/search', '/search?q=foo'.split('?')[0], '/repos', '/settings/general',
-            '/browse/github.com/a/b/-/blob/x.ts', '/browse/github.com/c/d/-/blob/y.ts',
-            '/api/health', '/api/health', '/api/commits', '/api/auth/session',
-            '/_next/static/chunks/main.js', '/_next/static/css/app.css',
-        ];
+    describe('cardinality bounding', () => {
+        it('reports unknown top-level paths as other', () => {
+            expect(normalizeRoute('/wp-admin')).toBe('other');
+            expect(normalizeRoute('/.env')).toBe('other');
+            expect(normalizeRoute('/phpmyadmin/index.php')).toBe('other');
+        });
 
-        const labels = new Set(paths.map(normalizeRoute));
+        it('reports unknown API paths as other, despite the [...slug] catch-all', () => {
+            expect(normalizeRoute('/api/not-a-real-route')).toBe('other');
+            expect(normalizeRoute('/api/12345')).toBe('other');
+            expect(normalizeRoute('/api/health-check')).toBe('other');
+        });
 
-        expect(labels).toEqual(new Set([
-            '/', '/search', '/repos', '/settings', '/browse',
-            '/api/health', '/api/commits', '/api/auth', '/_next',
-        ]));
+        it('stays bounded under scanner traffic', () => {
+            const hostile: string[] = [];
+            for (let i = 0; i < 1000; i++) {
+                hostile.push(`/scan-${i}`);
+                hostile.push(`/api/scan-${i}`);
+                hostile.push(`/${i}/${i}/${i}`);
+            }
+
+            const labels = new Set(hostile.map(normalizeRoute));
+
+            // 3000 distinct hostile paths must produce exactly one label.
+            expect(labels).toEqual(new Set(['other']));
+        });
+
+        it('never exceeds the documented label bound for any input', () => {
+            const paths = [
+                '/', '/search', '/repos', '/settings/general', '/browse/a/b/c',
+                '/api/health', '/api/commits', '/api/auth/session', '/_next/static/x.js',
+                '/wp-admin', '/api/bogus', '/random', '/api/9', '/..%2f', '/a/b/c/d/e',
+            ];
+            for (let i = 0; i < 500; i++) {
+                paths.push(`/junk${i}`, `/api/junk${i}`);
+            }
+
+            const labels = new Set(paths.map(normalizeRoute));
+
+            expect(labels.size).toBeLessThanOrEqual(MAX_ROUTE_LABELS);
+            // Known routes still resolve; only the unknown ones collapse.
+            expect(labels).toContain('/api/health');
+            expect(labels).toContain('other');
+        });
     });
 });

packages/web/src/httpMetrics.ts

@@ -5,15 +5,74 @@ import { httpRequestDuration } from './promClient';
 
 const logger = createLogger('web-http-metrics');
 
+/**
+ * Every path that isn't in this set is reported as `other`.
+ *
+ * Truncating path depth alone does not bound cardinality: the first segment is
+ * client-supplied, and `/api/[...slug]` is a catch-all, so `/wp-admin`,
+ * `/.env`, and `/api/<anything>` would each mint a new time series. Scanner or
+ * bot traffic would then grow the series count without limit. Matching against
+ * a known set instead bounds it to this size plus one, whatever gets requested.
+ *
+ * Adding a route here is deliberate. A missing one is reported as `other`, so
+ * new routes lose granularity rather than breaking, and cardinality holds.
+ */
+const KNOWN_ROUTES = new Set([
+    '/',
+    '/_next',
+    '/askgh',
+    '/browse',
+    '/chat',
+    '/chats',
+    '/invite',
+    '/login',
+    '/oauth',
+    '/onboard',
+    '/redeem',
+    '/repos',
+    '/search',
+    '/settings',
+    '/signup',
+    '/slow',
+    '/api/auth',
+    '/api/avatar',
+    '/api/blame',
+    '/api/changelog',
+    '/api/chat',
+    '/api/commit',
+    '/api/commits',
+    '/api/diff',
+    '/api/ee',
+    '/api/files',
+    '/api/find_definitions',
+    '/api/find_references',
+    '/api/folder_contents',
+    '/api/health',
+    '/api/minidenticon',
+    '/api/models',
+    '/api/offers',
+    '/api/openapi.json',
+    '/api/repo-status',
+    '/api/repos',
+    '/api/search',
+    '/api/source',
+    '/api/stream_search',
+    '/api/symbols',
+    '/api/tree',
+    '/api/version',
+    '/api/webhook',
+]);
+
+const OTHER_ROUTE = 'other';
+
 /**
  * Collapses a request path into a bounded label.
  *
- * The full path can't be used: repository and file paths are unbounded, so
- * `/browse/github.com/org/repo/-/blob/src/index.ts` would mint a new time
- * series for every file anyone views. API paths keep two segments so
- * `/api/health` stays distinct from `/api/commits`; everything else keeps one.
- * That bounds the label to roughly the number of API routes plus top-level
- * pages.
+ * Depth is truncated first, because repository and file paths are unbounded and
+ * `/browse/github.com/org/repo/-/blob/src/index.ts` must not mint a series per
+ * file viewed. API paths keep two segments so `/api/health` stays distinct from
+ * `/api/commits`; everything else keeps one. The result is then matched against
+ * `KNOWN_ROUTES`, which is what actually bounds the label set.
  */
 export const normalizeRoute = (pathname: string): string => {
     const segments = pathname.split('/').filter(segment => segment.length > 0);
@@ -22,9 +81,14 @@ export const normalizeRoute = (pathname: string): string => {
     }
 
     const depth = segments[0] === 'api' ? 2 : 1;
-    return `/${segments.slice(0, depth).join('/')}`;
+    const candidate = `/${segments.slice(0, depth).join('/')}`;
+
+    return KNOWN_ROUTES.has(candidate) ? candidate : OTHER_ROUTE;
 };
 
+/** Upper bound on distinct `route` label values, for tests and review. */
+export const MAX_ROUTE_LABELS = KNOWN_ROUTES.size + 1;
+
 interface RequestStartMessage {
     response?: ServerResponse;
     socket?: { localPort?: number };