Merge pull request #398 from bugdrophq/codex/fix-admin-request-logging

fix: keep encoded admin requests out of URL logs

Author: neonwatty

docs/protocol/cross-plane-safety.md

@@ -2,11 +2,13 @@
 
 The current public system remains supported. This harness compares its tracked
 runtime, assets, widget build inputs and Wrangler configuration to the reviewed
-`test/cross-plane/fixtures/current-public-baseline.v2.json`. V1 remains the
+`test/cross-plane/fixtures/current-public-baseline.v3.json`. V1 remains the
 pre-admin-read historical record. V2 is an intentional public-plane change:
 the protected inventory route, read-only Durable Object deletion check, and
-admin URL-log exclusion. The change was independently reviewed, and the new
-inventory and legacy count paths have targeted regression tests. This baseline
+admin URL-log exclusion. V3 corrects that exclusion to use Hono's decoded routing
+path, so percent-encoded aliases of the admin route do not leak query strings to
+the global request logger. The change was independently reviewed, and a regression
+test fails against V2 and passes against V3. This baseline
 update does not itself authorize deployment or a change to public delivery.
 Public fingerprint mutations and changed/skipped/retried browser evidence must fail.
 An intentional future public change requires explicit baseline review; do not

src/index.ts

@@ -51,7 +51,8 @@ app.use('*', async (c, next) => {
 // including malformed query strings, out of URL logs.
 const requestLogger = logger();
 app.use('*', (c, next) => {
-  const path = new URL(c.req.url).pathname;
+  // Use Hono's decoded path so aliases that reach this route cannot bypass the log exclusion.
+  const path = c.req.path;
   if (path === '/internal/admin' || path.startsWith('/internal/admin/')) return next();
   return requestLogger(c, next);
 });

test/adminInstallations.test.ts

@@ -147,7 +147,21 @@ describe('server-only administrator installation inventory', () => {
         env,
         {} as ExecutionContext
       );
+      await mounted.fetch(
+        new Request(
+          'https://worker.example/intern%61l/admin/installations?cursor=accidental-secret',
+          { headers: { Authorization: `Bearer ${SECRET}` } }
+        ),
+        env,
+        {} as ExecutionContext
+      );
       expect(logs).not.toHaveBeenCalled();
+      await mounted.fetch(
+        new Request('https://worker.example/api/health'),
+        env,
+        {} as ExecutionContext
+      );
+      expect(logs).toHaveBeenCalled();
     } finally {
       logs.mockRestore();
     }

test/cross-plane/current-public.test.ts

@@ -2,7 +2,7 @@ import { createHash } from 'node:crypto';
 import { execFileSync } from 'node:child_process';
 import { readFileSync } from 'node:fs';
 import { describe, expect, it } from 'vitest';
-import baseline from './fixtures/current-public-baseline.v2.json';
+import baseline from './fixtures/current-public-baseline.v3.json';
 
 function publicFiles(): string[] {
   return execFileSync('git', ['ls-files', '-z'], { encoding: 'utf8' })
@@ -22,7 +22,7 @@ function fingerprint(paths: string[], read: (path: string) => Uint8Array): strin
   return hash.digest('hex');
 }
 
-describe('current public plane matches the reviewed admin-read baseline', () => {
+describe('current public plane matches the reviewed admin URL-log baseline', () => {
   it('matches the reviewed tracked runtime, assets and configuration byte for byte', () => {
     const paths = publicFiles();
     expect(paths.length).toBe(baseline.fileCount);

test/cross-plane/fixtures/current-public-baseline.v3.json

@@ -0,0 +1,7 @@
+{
+  "schemaVersion": 3,
+  "baseCommit": "8c0d2b34d78a2a260513268112c5413e2d4d9d43",
+  "description": "Reviewed public-plane change aligning the admin URL-log exclusion with Hono's decoded routing path. SHA-256 of sorted tracked path + NUL + exact bytes + NUL; V2 remains the pre-fix record.",
+  "fileCount": 146,
+  "sha256": "aea765d5ecf2b93e83cae1733a11f60271c045016ef44e9dcbe7d646ebe212bb"
+}