Deep comprehensive audit: dependency + OAuth callback hardening

docs/development/DEEP_AUDIT_OVERLAP_2026-02-28.md

@@ -0,0 +1,42 @@
+# Deep Audit Overlap Ledger (2026-02-28)
+
+## Purpose
+Track overlap against currently open audit PRs so this branch remains incremental and avoids duplicate fixes where possible.
+
+## Open Audit PRs Reviewed
+- #44 `audit/architect-deep-audit-2026-02-28` -> `main`
+- #45 `audit/phase-1-deps-security-20260228` -> `main`
+- #46 `audit/phase-2-oauth-hardening-20260228` -> `audit/phase-1-deps-security-20260228`
+- #47 `audit/phase-3-rate-limit-units-20260228` -> `audit/phase-2-oauth-hardening-20260228`
+- #48 `audit/full-code-quality-main-20260228` -> `main`
+
+## Overlap Assessment
+
+### Dependency hardening overlap
+- Potential overlap area: #45 and #48 both touch dependency remediation.
+- This branch kept dependency work scoped to currently reproducible high vulnerabilities from `npm audit` on `main`.
+- Effective changes here:
+  - `hono` floor raised to `^4.12.3`
+  - `rollup` floor raised to `^4.59.0`
+  - `minimatch` floors raised to `^10.2.4` and `^9.0.9` for `@typescript-eslint/typescript-estree`
+- Result: high vulnerabilities cleared in this branch; follow-up dev-tooling update also cleared the remaining moderate `ajv` advisory.
+
+### Auth/server overlap
+- PR #44/#46 touch auth-related files including `index.ts` and `lib/auth/server.ts`.
+- This branch intentionally targets distinct controls not represented in those PR descriptions:
+  - Manual OAuth callback URL trust boundary validation (protocol/host/port/path enforcement).
+  - Removal of sensitive OAuth URL query logging (state/challenge leak reduction).
+  - Local callback server hardening: method allowlist (`GET` only), no-store headers, one-time code consumption semantics.
+
+### Rate-limit overlap
+- PR #47 focuses retry-after unit parsing in `lib/request/fetch-helpers.ts`.
+- This branch does not modify retry-after parsing logic and therefore does not duplicate that unit-conversion patchline.
+
+## Exclusions in This Branch
+- No medium/low-only cleanup work.
+- No refactor-only churn.
+- No duplication of chained phase-branch mechanics used by PR #45 -> #46 -> #47.
+
+## Verification Snapshot
+- Baseline before fixes: `npm audit --audit-level=high` reported 3 high + 1 moderate.
+- Final state after dependency and tooling updates: `npm audit` reports 0 vulnerabilities.

docs/development/DEEP_AUDIT_REPORT_2026-02-28.md

@@ -0,0 +1,86 @@
+# Deep Comprehensive Audit Report (2026-02-28)
+
+## Scope
+Full repository deep audit focused on high-impact risk classes:
+- Dependency and supply-chain vulnerabilities.
+- OAuth callback security boundaries.
+- Local OAuth callback server hardening and reliability behavior.
+
+## Branch and Baseline
+- Branch: `audit/deep-comprehensive-20260228-111117`
+- Base: `origin/main` (`ab970af` at branch creation)
+
+## Findings and Actions
+
+### Phase 1: Dependency vulnerability remediation
+**Risk class:** High severity supply-chain vulnerabilities reported by `npm audit`.
+
+**Baseline findings:**
+- High: `hono` (GHSA-xh87-mx6m-69f3)
+- High: `minimatch` (GHSA-3ppc-4f35-3m26, GHSA-7r86-cg39-jmmj, GHSA-23c5-xmqv-rm74)
+- High: `rollup` (GHSA-mw96-cpmx-2vgc)
+- Moderate: `ajv` (GHSA-2g4f-4pwh-qvx6)
+
+**Remediation:**
+- Updated override and dependency floors:
+  - `hono`: `^4.12.3`
+  - `rollup`: `^4.59.0`
+  - `minimatch`: `^10.2.4`
+  - `@typescript-eslint/typescript-estree` nested `minimatch`: `^9.0.9`
+
+**Outcome:**
+- Initial pass cleared all high/critical findings.
+- Follow-up tooling update (`npm update eslint`) removed the remaining moderate `ajv` advisory.
+- Final audit status: `npm audit` reports 0 vulnerabilities.
+
+### Phase 2: Manual OAuth callback trust hardening
+**Risk class:** Callback URL trust boundary and OAuth state handling hardening.
+
+**Remediation:**
+- Added manual callback URL validation in `index.ts` for manual paste flow:
+  - Protocol must be `http`.
+  - Host must be `localhost` or `127.0.0.1`.
+  - Port must be `1455`.
+  - Path must be `/auth/callback`.
+- Validation is applied in both `validate` and `callback` paths.
+- Removed sensitive full OAuth URL logging with query parameters; replaced with non-sensitive auth endpoint logging.
+
+**Tests added/updated:**
+- `test/index.test.ts`:
+  - Reject non-localhost host in manual callback URL.
+  - Reject unexpected protocol in manual callback URL.
+
+### Phase 3: Local OAuth server behavior hardening
+**Risk class:** Local callback endpoint attack surface and callback handling reliability.
+
+**Remediation:**
+- `lib/auth/server.ts`:
+  - Enforced `GET`-only callback handling (returns `405` + `Allow: GET` for others).
+  - Added no-cache controls (`Cache-Control: no-store`, `Pragma: no-cache`).
+  - Implemented one-time captured-code consumption semantics in `waitForCode`.
+
+**Tests added/updated:**
+- `test/server.unit.test.ts`:
+  - Reject non-GET methods.
+  - Assert cache-control headers on success.
+  - Assert captured authorization code is consumed once.
+
+## Deferred/Residual Items
+- No remaining vulnerabilities from `npm audit` at time of verification.
+- Medium/low style and refactor-only opportunities remain out of scope for this security-focused pass.
+
+## Verification Evidence
+Commands executed after remediation:
+- `npm run lint` -> pass
+- `npm run typecheck` -> pass
+- `npm test` -> pass
+- `npx vitest run test/server.unit.test.ts test/index.test.ts` -> pass
+- `npm run audit:all` -> pass
+- `npm audit` -> pass (0 vulnerabilities)
+
+## Atomic Commit Map
+1. `fix(audit phase 1): remediate high dependency vulnerabilities`
+2. `fix(audit phase 2): harden manual OAuth callback validation`
+3. `fix(audit phase 3): tighten local OAuth callback server behavior`
+4. `docs(audit): publish overlap ledger and deep audit report`
+5. `chore(audit): refresh eslint toolchain to clear residual moderate advisory`

index.ts

@@ -30,6 +30,7 @@ import {
         createAuthorizationFlow,
         exchangeAuthorizationCode,
         parseAuthorizationInput,
+        AUTHORIZE_URL,
         REDIRECT_URI,
 } from "./lib/auth/auth.js";
 import { queuedRefresh, getRefreshQueueMetrics } from "./lib/refresh-queue.js";
@@ -377,6 +378,36 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
 				};
 		};
 
+		const MANUAL_OAUTH_ALLOWED_HOSTS = new Set(["127.0.0.1", "localhost"]);
+
+		const getManualOAuthUrlValidationError = (
+			input: string,
+		): string | undefined => {
+			const raw = input.trim();
+			if (!raw) return undefined;
+
+			let parsedUrl: URL;
+			try {
+				parsedUrl = new URL(raw);
+			} catch {
+				return undefined;
+			}
+
+			if (parsedUrl.protocol !== "http:") {
+				return `Invalid callback URL protocol. Use ${REDIRECT_URI}`;
+			}
+			if (!MANUAL_OAUTH_ALLOWED_HOSTS.has(parsedUrl.hostname.toLowerCase())) {
+				return `Invalid callback URL host. Use ${REDIRECT_URI}`;
+			}
+			if (parsedUrl.port !== "1455") {
+				return `Invalid callback URL port. Use ${REDIRECT_URI}`;
+			}
+			if (parsedUrl.pathname !== "/auth/callback") {
+				return `Invalid callback URL path. Use ${REDIRECT_URI}`;
+			}
+			return undefined;
+		};
+
 		const buildManualOAuthFlow = (
 				pkce: { verifier: string },
 				url: string,
@@ -387,6 +418,10 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
                 method: "code" as const,
                 instructions: AUTH_LABELS.INSTRUCTIONS_MANUAL,
                 validate: (input: string): string | undefined => {
+                        const callbackValidationError = getManualOAuthUrlValidationError(input);
+                        if (callbackValidationError) {
+                                return callbackValidationError;
+                        }
                         const parsed = parseAuthorizationInput(input);
                         if (!parsed.code) {
                                 return "No authorization code found. Paste the full callback URL (e.g., http://localhost:1455/auth/callback?code=...)";
@@ -400,6 +435,14 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
                         return undefined;
                 },
                 callback: async (input: string) => {
+                        const callbackValidationError = getManualOAuthUrlValidationError(input);
+                        if (callbackValidationError) {
+                                return {
+                                        type: "failed" as const,
+                                        reason: "invalid_response" as const,
+                                        message: callbackValidationError,
+                                };
+                        }
                         const parsed = parseAuthorizationInput(input);
                         if (!parsed.code || !parsed.state) {
                                 return {
@@ -437,7 +480,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
 		forceNewLogin: boolean = false,
 	): Promise<TokenResult> => {
 		const { pkce, state, url } = await createAuthorizationFlow({ forceNewLogin });
-		logInfo(`OAuth URL: ${url}`);
+		logInfo(`OAuth authorization flow initialized at ${AUTHORIZE_URL}`);
 
                 let serverInfo: Awaited<ReturnType<typeof startLocalOAuthServer>> | null = null;
                 try {

lib/auth/server.ts

@@ -16,8 +16,15 @@ const successHtml = fs.readFileSync(path.join(__dirname, "..", "oauth-success.ht
  */
 export function startLocalOAuthServer({ state }: { state: string }): Promise<OAuthServerInfo> {
 	let pollAborted = false;
+	let capturedCode: string | undefined;
 	const server = http.createServer((req, res) => {
 		try {
+			if ((req.method ?? "GET").toUpperCase() !== "GET") {
+				res.statusCode = 405;
+				res.setHeader("Allow", "GET");
+				res.end("Method not allowed");
+				return;
+			}
 			const url = new URL(req.url || "", "http://localhost");
 			if (url.pathname !== "/auth/callback") {
 				res.statusCode = 404;
@@ -40,13 +47,17 @@ export function startLocalOAuthServer({ state }: { state: string }): Promise<OAu
 			res.setHeader("X-Frame-Options", "DENY");
 			res.setHeader("X-Content-Type-Options", "nosniff");
 			res.setHeader("Content-Security-Policy", "default-src 'self'; script-src 'none'");
+			res.setHeader("Cache-Control", "no-store");
+			res.setHeader("Pragma", "no-cache");
 			res.end(successHtml);
-			(server as http.Server & { _lastCode?: string })._lastCode = code;
-	} catch (err) {
-		logError(`Request handler error: ${(err as Error)?.message ?? String(err)}`);
-		res.statusCode = 500;
-		res.end("Internal error");
-	}
+			if (!capturedCode) {
+				capturedCode = code;
+			}
+		} catch (err) {
+			logError(`Request handler error: ${(err as Error)?.message ?? String(err)}`);
+			res.statusCode = 500;
+			res.end("Internal error");
+		}
 	});
 
 	server.unref();
@@ -61,20 +72,23 @@ export function startLocalOAuthServer({ state }: { state: string }): Promise<OAu
 						pollAborted = true;
 						server.close();
 					},
-				waitForCode: async () => {
-					const POLL_INTERVAL_MS = 100;
-					const TIMEOUT_MS = 5 * 60 * 1000;
-					const maxIterations = Math.floor(TIMEOUT_MS / POLL_INTERVAL_MS);
-					const poll = () => new Promise<void>((r) => setTimeout(r, POLL_INTERVAL_MS));
-					for (let i = 0; i < maxIterations; i++) {
-						if (pollAborted) return null;
-						const lastCode = (server as http.Server & { _lastCode?: string })._lastCode;
-						if (lastCode) return { code: lastCode };
-						await poll();
-					}
-					logWarn("OAuth poll timeout after 5 minutes");
-					return null;
-				},
+					waitForCode: async () => {
+						const POLL_INTERVAL_MS = 100;
+						const TIMEOUT_MS = 5 * 60 * 1000;
+						const maxIterations = Math.floor(TIMEOUT_MS / POLL_INTERVAL_MS);
+						const poll = () => new Promise<void>((r) => setTimeout(r, POLL_INTERVAL_MS));
+						for (let i = 0; i < maxIterations; i++) {
+							if (pollAborted) return null;
+							if (capturedCode) {
+								const code = capturedCode;
+								capturedCode = undefined;
+								return { code };
+							}
+							await poll();
+						}
+						logWarn("OAuth poll timeout after 5 minutes");
+						return null;
+					},
 				});
 			})
 			.on("error", (err: NodeJS.ErrnoException) => {
@@ -84,14 +98,14 @@ export function startLocalOAuthServer({ state }: { state: string }): Promise<OAu
 				resolve({
 					port: 1455,
 					ready: false,
-				close: () => {
-					pollAborted = true;
-					try {
-						server.close();
-					} catch (err) {
-					logError(`Failed to close OAuth server: ${(err as Error)?.message ?? String(err)}`);
-					}
-				},
+					close: () => {
+						pollAborted = true;
+						try {
+							server.close();
+						} catch (err) {
+							logError(`Failed to close OAuth server: ${(err as Error)?.message ?? String(err)}`);
+						}
+					},
 					waitForCode: () => Promise.resolve(null),
 				});
 			});