From 4426d3736914e02c3220180dbe3b9f51d39e343d Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 11:06:40 -0400 Subject: [PATCH 01/10] feat: adds Worker logging, Sentry tunnel, and error tracking --- .github/workflows/ci.yml | 2 + package.json | 4 +- pnpm-lock.yaml | 76 +++++ src/app/index.tsx | 5 + src/app/lib/sentry.ts | 99 +++++++ src/app/pages/PrivacyPage.tsx | 54 +++- src/worker/index.ts | 196 ++++++++++++- tests/worker/oauth.test.ts | 505 ++++++++++++++++++++++++++++++++++ wrangler.toml | 8 + 9 files changed, 940 insertions(+), 9 deletions(-) create mode 100644 src/app/lib/sentry.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab287a4..b9ea4bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,8 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm run typecheck - run: pnpm test + - name: WAF smoke tests + run: pnpm test:waf - name: Verify CSP hash run: node scripts/verify-csp-hash.mjs - name: Install Playwright browsers diff --git a/package.json b/package.json index ec70e3e..a49f07c 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,8 @@ "test:watch": "vitest --config vitest.workspace.ts", "deploy": "wrangler deploy", "typecheck": "tsc --noEmit", - "test:e2e": "E2E_PORT=$(node -e \"const s=require('net').createServer();s.listen(0,()=>{console.log(s.address().port);s.close()})\") playwright test" + "test:e2e": "E2E_PORT=$(node -e \"const s=require('net').createServer();s.listen(0,()=>{console.log(s.address().port);s.close()})\") playwright test", + "test:waf": "bash hack/scripts/waf-smoke-test.sh" }, "dependencies": { "@kobalte/core": "^0.13.11", @@ -20,6 +21,7 @@ "@octokit/plugin-paginate-rest": "^14.0.0", "@octokit/plugin-retry": "^8.1.0", "@octokit/plugin-throttling": "^11.0.3", + "@sentry/solid": "^10.46.0", "@solidjs/router": "^0.16.1", "corvu": "^0.7.2", "idb": "^8.0.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1711d83..3e96fc9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -23,6 +23,9 @@ importers: '@octokit/plugin-throttling': specifier: ^11.0.3 version: 11.0.3(@octokit/core@7.0.6) + '@sentry/solid': + specifier: ^10.46.0 + version: 10.46.0(@solidjs/router@0.16.1(solid-js@1.9.11))(solid-js@1.9.11) '@solidjs/router': specifier: ^0.16.1 version: 0.16.1(solid-js@1.9.11) @@ -815,6 +818,43 @@ packages: '@rolldown/pluginutils@1.0.0-rc.10': resolution: {integrity: sha512-UkVDEFk1w3mveXeKgaTuYfKWtPbvgck1dT8TUG3bnccrH0XtLTuAyfCoks4Q/M5ZGToSVJTIQYCzy2g/atAOeg==} + '@sentry-internal/browser-utils@10.46.0': + resolution: {integrity: sha512-WB1gBT9G13V02ekZ6NpUhoI1aGHV2eNfjEPthkU2bGBvFpQKnstwzjg7waIRGR7cu+YSW2Q6UI6aQLgBeOPD1g==} + engines: {node: '>=18'} + + '@sentry-internal/feedback@10.46.0': + resolution: {integrity: sha512-c4pI/z9nZCQXe9GYEw/hE/YTY9AxGBp8/wgKI+T8zylrN35SGHaXv63szzE1WbI8lacBY8lBF7rstq9bQVCaHw==} + engines: {node: '>=18'} + + '@sentry-internal/replay-canvas@10.46.0': + resolution: {integrity: sha512-ub314MWUsekVCuoH0/HJbbimlI24SkV745UW2pj9xRbxOAEf1wjkmIzxKrMDbTgJGuEunug02XZVdJFJUzOcDw==} + engines: {node: '>=18'} + + '@sentry-internal/replay@10.46.0': + resolution: {integrity: sha512-JBsWeXG6bRbxBFK8GzWymWGOB9QE7Kl57BeF3jzgdHTuHSWZ2mRnAmb1K05T4LU+gVygk6yW0KmdC8Py9Qzg9A==} + engines: {node: '>=18'} + + '@sentry/browser@10.46.0': + resolution: {integrity: sha512-80DmGlTk5Z2/OxVOzLNxwolMyouuAYKqG8KUcoyintZqHbF6kO1RulI610HmyUt3OagKeBCqt9S7w0VIfCRL+Q==} + engines: {node: '>=18'} + + '@sentry/core@10.46.0': + resolution: {integrity: sha512-N3fj4zqBQOhXliS1Ne9euqIKuciHCGOJfPGQLwBoW9DNz03jF+NB8+dUKtrJ79YLoftjVgf8nbgwtADK7NR+2Q==} + engines: {node: '>=18'} + + '@sentry/solid@10.46.0': + resolution: {integrity: sha512-OR/yj/jxs+wCsaJ6Nh7PtS9+FHbezs2oH/ZBfs9438Nqn99iLPYxVpH9CSN+AGtyrPWxSI5MCh5VP8LZTlUDTw==} + engines: {node: '>=18'} + peerDependencies: + '@solidjs/router': ^0.13.4 || ^0.14.0 || ^0.15.0 + '@tanstack/solid-router': ^1.132.27 + solid-js: ^1.8.4 + peerDependenciesMeta: + '@solidjs/router': + optional: true + '@tanstack/solid-router': + optional: true + '@sindresorhus/is@7.2.0': resolution: {integrity: sha512-P1Cz1dWaFfR4IR+U13mqqiGsLFf1KbayybWwdd2vfctdV6hDpUkgCY0nKOLLTMSoRd/jJNjtbqzf13K8DCCXQw==} engines: {node: '>=18'} @@ -2351,6 +2391,42 @@ snapshots: '@rolldown/pluginutils@1.0.0-rc.10': {} + '@sentry-internal/browser-utils@10.46.0': + dependencies: + '@sentry/core': 10.46.0 + + '@sentry-internal/feedback@10.46.0': + dependencies: + '@sentry/core': 10.46.0 + + '@sentry-internal/replay-canvas@10.46.0': + dependencies: + '@sentry-internal/replay': 10.46.0 + '@sentry/core': 10.46.0 + + '@sentry-internal/replay@10.46.0': + dependencies: + '@sentry-internal/browser-utils': 10.46.0 + '@sentry/core': 10.46.0 + + '@sentry/browser@10.46.0': + dependencies: + '@sentry-internal/browser-utils': 10.46.0 + '@sentry-internal/feedback': 10.46.0 + '@sentry-internal/replay': 10.46.0 + '@sentry-internal/replay-canvas': 10.46.0 + '@sentry/core': 10.46.0 + + '@sentry/core@10.46.0': {} + + '@sentry/solid@10.46.0(@solidjs/router@0.16.1(solid-js@1.9.11))(solid-js@1.9.11)': + dependencies: + '@sentry/browser': 10.46.0 + '@sentry/core': 10.46.0 + solid-js: 1.9.11 + optionalDependencies: + '@solidjs/router': 0.16.1(solid-js@1.9.11) + '@sindresorhus/is@7.2.0': {} '@solid-primitives/event-listener@2.4.5(solid-js@1.9.11)': diff --git a/src/app/index.tsx b/src/app/index.tsx index 51793ba..da9f83b 100644 --- a/src/app/index.tsx +++ b/src/app/index.tsx @@ -1,5 +1,10 @@ import "./index.css"; import { render } from "solid-js/web"; +import { initSentry } from "./lib/sentry"; import App from "./App"; +// Initialize Sentry before rendering — captures errors from first paint. +// No-ops in dev/test (guarded by import.meta.env.DEV check). +initSentry(); + render(() => , document.getElementById("app")!); diff --git a/src/app/lib/sentry.ts b/src/app/lib/sentry.ts new file mode 100644 index 0000000..ab680fb --- /dev/null +++ b/src/app/lib/sentry.ts @@ -0,0 +1,99 @@ +import * as Sentry from "@sentry/solid"; + +const SENTRY_DSN = + "https://placeholder@o0.ingest.sentry.io/0"; // TODO: replace with real DSN + +/** Strip OAuth credentials from any captured URL. */ +function scrubUrl(url: string): string { + return url + .replace(/code=[^&\s]+/g, "code=[REDACTED]") + .replace(/state=[^&\s]+/g, "state=[REDACTED]") + .replace(/access_token=[^&\s]+/g, "access_token=[REDACTED]"); +} + +/** Allowed console breadcrumb prefixes — drop everything else. */ +const ALLOWED_CONSOLE_PREFIXES = [ + "[auth]", + "[api]", + "[poll]", + "[dashboard]", + "[settings]", +]; + +export function initSentry(): void { + // Only initialize in production — never in dev/test + if (import.meta.env.DEV) return; + + Sentry.init({ + dsn: SENTRY_DSN, + tunnel: "/api/error-reporting", + environment: import.meta.env.MODE, + + // ── Privacy: absolute minimum data ────────────────────────── + sendDefaultPii: false, + + // ── Disable everything except error tracking ──────────────── + tracesSampleRate: 0, + profilesSampleRate: 0, + // No replay integration — DOM capture is unnecessary for auth + // debugging and carries high privacy cost. + + // ── Only capture errors from our own code ─────────────────── + allowUrls: [/^https:\/\/gh\.gordoncode\.dev/], + + // ── Scrub sensitive data before it leaves the browser ──────── + beforeSend(event) { + // Strip OAuth params from captured URLs + if (event.request?.url) { + event.request.url = scrubUrl(event.request.url); + } + if (event.request?.query_string) { + event.request.query_string = "[REDACTED]"; + } + // Remove headers and cookies entirely + delete event.request?.headers; + delete event.request?.cookies; + // Remove user identity — we never want to track users + delete event.user; + // Scrub URLs in stack trace frames + if (event.exception?.values) { + for (const ex of event.exception.values) { + if (ex.stacktrace?.frames) { + for (const frame of ex.stacktrace.frames) { + if (frame.abs_path) { + frame.abs_path = scrubUrl(frame.abs_path); + } + } + } + } + } + return event; + }, + + beforeBreadcrumb(breadcrumb) { + // Scrub URLs in navigation breadcrumbs + if (breadcrumb.category === "navigation") { + if (breadcrumb.data?.from) + breadcrumb.data.from = scrubUrl(breadcrumb.data.from as string); + if (breadcrumb.data?.to) + breadcrumb.data.to = scrubUrl(breadcrumb.data.to as string); + } + // Scrub URLs in fetch/xhr breadcrumbs + if ( + breadcrumb.category === "fetch" || + breadcrumb.category === "xhr" + ) { + if (breadcrumb.data?.url) + breadcrumb.data.url = scrubUrl(breadcrumb.data.url as string); + } + // Only keep our own tagged console logs — drop third-party noise + if (breadcrumb.category === "console") { + const msg = breadcrumb.message ?? ""; + if (!ALLOWED_CONSOLE_PREFIXES.some((p) => msg.startsWith(p))) { + return null; + } + } + return breadcrumb; + }, + }); +} diff --git a/src/app/pages/PrivacyPage.tsx b/src/app/pages/PrivacyPage.tsx index 84a3011..410fffc 100644 --- a/src/app/pages/PrivacyPage.tsx +++ b/src/app/pages/PrivacyPage.tsx @@ -15,12 +15,13 @@ export default function PrivacyPage() {

- GitHub Tracker does not collect, store, or transmit any personal - data. All data stays in your browser. + GitHub Tracker stores your data in your browser. We use a + privacy-hardened error monitoring service to diagnose issues, as + described below.

- What we store + What we store in your browser

  • @@ -34,14 +35,55 @@ export default function PrivacyPage() {
+

+ Error monitoring +

+

+ We use{" "} + + Sentry + {" "} + (Functional Software, Inc.) to capture JavaScript errors so we can + fix bugs. When an error occurs, the following is sent: +

+
    +
  • Error type, message, and stack trace (file and line number)
    • +
  • Browser and operating system version
    • +
  • Page URL with authentication parameters redacted
    • +
  • Application log messages related to the error
    • +
+

+ What is never sent: IP addresses, cookies, request + headers, user identity, access tokens, OAuth codes, DOM content, + screen recordings, keystrokes, or performance traces. All sensitive + URL parameters are stripped before data leaves your browser. +

+

+ Error data is stored on Sentry's US-based infrastructure and + retained per Sentry's{" "} + + privacy policy + . Error monitoring is disabled during local development. +

+

What we don't do

    -
  • No analytics or tracking scripts
    • -
  • No server-side data storage
    • -
  • No third-party data sharing
    • +
  • No analytics or behavioral tracking
  • No cookies
    • +
  • No session recordings or screen capture
    • +
  • No user identification or profiling

diff --git a/src/worker/index.ts b/src/worker/index.ts index 04969d5..2d0da7d 100644 --- a/src/worker/index.ts +++ b/src/worker/index.ts @@ -3,6 +3,8 @@ export interface Env { GITHUB_CLIENT_ID: string; GITHUB_CLIENT_SECRET: string; ALLOWED_ORIGIN: string; + SENTRY_HOST: string; // e.g. "o123456.ingest.sentry.io" + SENTRY_PROJECT_ID: string; // e.g. "7890123" } // Predefined error strings only (SDR-006) @@ -12,6 +14,30 @@ type ErrorCode = | "method_not_allowed" | "not_found"; +// Structured logging — Cloudflare auto-indexes JSON fields for querying. +// NEVER log secrets: codes, tokens, client_secret, cookie values. +function log( + level: "info" | "warn" | "error", + event: string, + data: Record, + request?: Request +): void { + const entry: Record = { + worker: "github-tracker", + event, + ...data, + }; + if (request) { + const cf = (request as unknown as { cf?: Record }).cf; + entry.origin = request.headers.get("Origin"); + entry.user_agent = request.headers.get("User-Agent"); + entry.cf_country = cf?.country; + entry.cf_colo = cf?.colo; + entry.cf_city = cf?.city; + } + console[level](JSON.stringify(entry)); +} + function errorResponse( code: ErrorCode, status: number, @@ -51,6 +77,103 @@ function getCorsHeaders( return {}; } +// ── Sentry tunnel ───────────────────────────────────────────────────────── +// Proxies Sentry event envelopes through our own domain so the browser +// treats them as same-origin (no CSP change, no ad-blocker interference). +// The DSN is validated against env.SENTRY_HOST + env.SENTRY_PROJECT_ID to +// prevent abuse as an open proxy. +const SENTRY_ENVELOPE_MAX_BYTES = 256 * 1024; // 256 KB — Sentry rejects >200KB compressed + +async function handleSentryTunnel( + request: Request, + env: Env, +): Promise { + if (request.method !== "POST") { + return new Response(null, { status: 405, headers: securityHeaders() }); + } + + if (!env.SENTRY_HOST || !env.SENTRY_PROJECT_ID) { + log("warn", "sentry_tunnel_not_configured", {}, request); + return new Response(null, { status: 404, headers: securityHeaders() }); + } + + const contentLength = parseInt(request.headers.get("Content-Length") ?? "0", 10); + if (contentLength > SENTRY_ENVELOPE_MAX_BYTES) { + log("warn", "sentry_tunnel_payload_too_large", { content_length: contentLength }, request); + return new Response(null, { status: 413, headers: securityHeaders() }); + } + + let body: string; + try { + body = await request.text(); + } catch { + return new Response(null, { status: 400, headers: securityHeaders() }); + } + + // Sentry envelope format: first line is JSON header with dsn field + const firstNewline = body.indexOf("\n"); + if (firstNewline === -1) { + log("warn", "sentry_tunnel_invalid_envelope", {}, request); + return new Response(null, { status: 400, headers: securityHeaders() }); + } + + let envelopeHeader: { dsn?: string }; + try { + envelopeHeader = JSON.parse(body.substring(0, firstNewline)); + } catch { + log("warn", "sentry_tunnel_header_parse_failed", {}, request); + return new Response(null, { status: 400, headers: securityHeaders() }); + } + + if (typeof envelopeHeader.dsn !== "string") { + // client_report envelopes may omit dsn — forward if host is configured + log("info", "sentry_tunnel_no_dsn", {}, request); + return new Response(null, { status: 200, headers: securityHeaders() }); + } + + // Validate DSN matches our project — prevents open proxy abuse + let dsnUrl: URL; + try { + dsnUrl = new URL(envelopeHeader.dsn); + } catch { + log("warn", "sentry_tunnel_invalid_dsn", {}, request); + return new Response(null, { status: 400, headers: securityHeaders() }); + } + + const dsnProjectId = dsnUrl.pathname.replace(/\//g, ""); + if (dsnUrl.hostname !== env.SENTRY_HOST || dsnProjectId !== env.SENTRY_PROJECT_ID) { + log("warn", "sentry_tunnel_dsn_mismatch", { + dsn_host: dsnUrl.hostname, + dsn_project: dsnProjectId, + }, request); + return new Response(null, { status: 403, headers: securityHeaders() }); + } + + // Forward to Sentry ingest endpoint + const sentryUrl = `https://${env.SENTRY_HOST}/api/${env.SENTRY_PROJECT_ID}/envelope/`; + try { + const sentryResp = await fetch(sentryUrl, { + method: "POST", + headers: { "Content-Type": "application/x-sentry-envelope" }, + body, + }); + + log("info", "sentry_tunnel_forwarded", { + sentry_status: sentryResp.status, + }, request); + + return new Response(null, { + status: sentryResp.status, + headers: securityHeaders(), + }); + } catch (err) { + log("error", "sentry_tunnel_fetch_failed", { + error: err instanceof Error ? err.message : "unknown", + }, request); + return new Response(null, { status: 502, headers: securityHeaders() }); + } +} + // GitHub OAuth code format validation (SDR-005): alphanumeric, 1-40 chars. // GitHub's code format is undocumented and has changed historically — validate // loosely here; GitHub's server validates the actual code. @@ -61,19 +184,26 @@ async function handleTokenExchange( env: Env, cors: Record ): Promise { + log("info", "token_exchange_started", { method: request.method }, request); + if (request.method !== "POST") { + log("warn", "token_exchange_wrong_method", { method: request.method }, request); return errorResponse("method_not_allowed", 405, cors); } const contentType = request.headers.get("Content-Type") ?? ""; if (!contentType.includes("application/json")) { + log("warn", "token_exchange_bad_content_type", { content_type: contentType }, request); return errorResponse("invalid_request", 400, cors); } let body: unknown; try { body = await request.json(); - } catch { + } catch (err) { + log("warn", "token_exchange_json_parse_failed", { + error: err instanceof Error ? err.message : "unknown", + }, request); return errorResponse("invalid_request", 400, cors); } @@ -82,6 +212,14 @@ async function handleTokenExchange( body === null || typeof (body as Record)["code"] !== "string" ) { + log("warn", "token_exchange_missing_code", { + body_type: typeof body, + body_is_null: body === null, + has_code: body !== null && typeof body === "object" && "code" in body, + code_type: body !== null && typeof body === "object" + ? typeof (body as Record)["code"] + : "n/a", + }, request); return errorResponse("invalid_request", 400, cors); } @@ -89,10 +227,18 @@ async function handleTokenExchange( // Strict code format validation before touching GitHub (SDR-005) if (!VALID_CODE_RE.test(code)) { + log("warn", "token_exchange_invalid_code_format", { + code_length: code.length, + code_has_spaces: code.includes(" "), + code_has_newlines: code.includes("\n"), + }, request); return errorResponse("invalid_request", 400, cors); } + log("info", "github_oauth_request_sent", {}, request); + let githubData: Record; + let githubStatus: number; try { const githubResp = await fetch( "https://github.com/login/oauth/access_token", @@ -109,8 +255,13 @@ async function handleTokenExchange( }), } ); + githubStatus = githubResp.status; githubData = (await githubResp.json()) as Record; - } catch { + } catch (err) { + log("error", "github_oauth_fetch_failed", { + error: err instanceof Error ? err.message : "unknown", + error_name: err instanceof Error ? err.name : "unknown", + }, request); return errorResponse("token_exchange_failed", 400, cors); } @@ -119,9 +270,22 @@ async function handleTokenExchange( typeof githubData["error"] === "string" || typeof githubData["access_token"] !== "string" ) { + log("error", "github_oauth_error_response", { + github_status: githubStatus, + github_error: githubData["error"], + github_error_description: githubData["error_description"], + github_error_uri: githubData["error_uri"], + has_access_token: "access_token" in githubData, + }, request); return errorResponse("token_exchange_failed", 400, cors); } + log("info", "token_exchange_succeeded", { + github_status: githubStatus, + scope: githubData["scope"], + token_type: githubData["token_type"], + }, request); + // Return only allowed fields — never forward full GitHub response. const allowed = { access_token: githubData["access_token"], @@ -144,15 +308,39 @@ export default { const url = new URL(request.url); const origin = request.headers.get("Origin"); const cors = getCorsHeaders(origin, env.ALLOWED_ORIGIN); + const corsMatched = Object.keys(cors).length > 0; + + // Log all API requests (skip static asset requests to reduce noise) + if (url.pathname.startsWith("/api/")) { + log("info", "api_request", { + method: request.method, + pathname: url.pathname, + cors_matched: corsMatched, + allowed_origin: env.ALLOWED_ORIGIN, + }, request); + + if (!corsMatched && origin !== null) { + log("warn", "cors_origin_mismatch", { + request_origin: origin, + allowed_origin: env.ALLOWED_ORIGIN, + }, request); + } + } // CORS preflight for the token exchange endpoint only if (request.method === "OPTIONS" && url.pathname === "/api/oauth/token") { + log("info", "cors_preflight", { cors_matched: corsMatched }, request); return new Response(null, { status: 204, headers: { ...cors, "Access-Control-Max-Age": "86400", ...securityHeaders() }, }); } + // Sentry tunnel — same-origin proxy, no CORS needed (browser sends as first-party) + if (url.pathname === "/api/error-reporting") { + return handleSentryTunnel(request, env); + } + if (url.pathname === "/api/oauth/token") { return handleTokenExchange(request, env, cors); } @@ -164,6 +352,10 @@ export default { } if (url.pathname.startsWith("/api/")) { + log("warn", "api_not_found", { + method: request.method, + pathname: url.pathname, + }, request); return errorResponse("not_found", 404, cors); } diff --git a/tests/worker/oauth.test.ts b/tests/worker/oauth.test.ts index d647f67..aa6cbaf 100644 --- a/tests/worker/oauth.test.ts +++ b/tests/worker/oauth.test.ts @@ -9,6 +9,8 @@ function makeEnv(overrides: Partial = {}): Env { GITHUB_CLIENT_ID: "test_client_id", GITHUB_CLIENT_SECRET: "test_client_secret", ALLOWED_ORIGIN, + SENTRY_HOST: "o123456.ingest.sentry.io", + SENTRY_PROJECT_ID: "7890123", ...overrides, }; } @@ -41,11 +43,48 @@ function makeRequest( // Valid 20-char hex code const VALID_CODE = "a1b2c3d4e5f6a1b2c3d4"; +/** Parse all structured log calls from a console spy, returning {level, entry} tuples. */ +function collectLogs(spies: { + info: ReturnType; + warn: ReturnType; + error: ReturnType; +}): Array<{ level: string; entry: Record }> { + const logs: Array<{ level: string; entry: Record }> = []; + for (const [level, spy] of Object.entries(spies)) { + for (const call of spy.mock.calls) { + try { + logs.push({ level, entry: JSON.parse(call[0] as string) }); + } catch { + // non-JSON console output — ignore + } + } + } + return logs; +} + +/** Find the first log entry matching a given event name. */ +function findLog( + logs: Array<{ level: string; entry: Record }>, + event: string +): { level: string; entry: Record } | undefined { + return logs.find((l) => l.entry.event === event); +} + describe("Worker OAuth endpoint", () => { let originalFetch: typeof globalThis.fetch; + let consoleSpy: { + info: ReturnType; + warn: ReturnType; + error: ReturnType; + }; beforeEach(() => { originalFetch = globalThis.fetch; + consoleSpy = { + info: vi.spyOn(console, "info").mockImplementation(() => {}), + warn: vi.spyOn(console, "warn").mockImplementation(() => {}), + error: vi.spyOn(console, "error").mockImplementation(() => {}), + }; }); afterEach(() => { @@ -397,4 +436,470 @@ describe("Worker OAuth endpoint", () => { expect(assetFetch).toHaveBeenCalledOnce(); expect(res.status).toBe(200); }); + + // ── Structured logging ──────────────────────────────────────────────────── + + describe("Structured logging", () => { + // ── Log format & metadata ───────────────────────────────────────────── + + it("logs are valid JSON with worker identifier and event name", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ access_token: "ghu_tok", token_type: "bearer" }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + expect(logs.length).toBeGreaterThan(0); + for (const { entry } of logs) { + expect(entry.worker).toBe("github-tracker"); + expect(typeof entry.event).toBe("string"); + expect((entry.event as string).length).toBeGreaterThan(0); + } + }); + + it("logs include request metadata (origin, user_agent)", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ access_token: "ghu_tok", token_type: "bearer" }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const apiLog = findLog(logs, "api_request"); + expect(apiLog).toBeDefined(); + expect(apiLog!.entry.origin).toBe(ALLOWED_ORIGIN); + }); + + // ── API request & CORS logging ──────────────────────────────────────── + + it("logs api_request for every /api/ request", async () => { + const req = makeRequest("POST", "/api/oauth/token", { body: {} }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const apiLog = findLog(logs, "api_request"); + expect(apiLog).toBeDefined(); + expect(apiLog!.level).toBe("info"); + expect(apiLog!.entry.method).toBe("POST"); + expect(apiLog!.entry.pathname).toBe("/api/oauth/token"); + expect(apiLog!.entry.cors_matched).toBe(true); + }); + + it("logs cors_origin_mismatch when origin does not match", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ access_token: "ghu_tok", token_type: "bearer" }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { + body: { code: VALID_CODE }, + origin: "https://evil.example.com", + }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const corsLog = findLog(logs, "cors_origin_mismatch"); + expect(corsLog).toBeDefined(); + expect(corsLog!.level).toBe("warn"); + expect(corsLog!.entry.request_origin).toBe("https://evil.example.com"); + expect(corsLog!.entry.allowed_origin).toBe(ALLOWED_ORIGIN); + }); + + it("does not log cors_origin_mismatch when origin matches", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ access_token: "ghu_tok", token_type: "bearer" }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { + body: { code: VALID_CODE }, + origin: ALLOWED_ORIGIN, + }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + expect(findLog(logs, "cors_origin_mismatch")).toBeUndefined(); + }); + + it("does not log cors_origin_mismatch when origin header is absent", async () => { + const req = new Request("https://gh.gordoncode.dev/api/health", { method: "GET" }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + expect(findLog(logs, "cors_origin_mismatch")).toBeUndefined(); + }); + + it("logs cors_preflight for OPTIONS /api/oauth/token", async () => { + const req = makeRequest("OPTIONS", "/api/oauth/token", { origin: ALLOWED_ORIGIN }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const preflightLog = findLog(logs, "cors_preflight"); + expect(preflightLog).toBeDefined(); + expect(preflightLog!.level).toBe("info"); + expect(preflightLog!.entry.cors_matched).toBe(true); + }); + + it("does not log api_request for non-API routes (static assets)", async () => { + const req = new Request("https://gh.gordoncode.dev/index.html"); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + expect(findLog(logs, "api_request")).toBeUndefined(); + }); + + it("logs api_not_found for unknown API routes", async () => { + const req = makeRequest("GET", "/api/nonexistent"); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const notFoundLog = findLog(logs, "api_not_found"); + expect(notFoundLog).toBeDefined(); + expect(notFoundLog!.level).toBe("warn"); + expect(notFoundLog!.entry.pathname).toBe("/api/nonexistent"); + }); + + // ── Token exchange lifecycle logging ────────────────────────────────── + + it("logs full success lifecycle: api_request → token_exchange_started → github_oauth_request_sent → token_exchange_succeeded", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ + access_token: "ghu_tok", + token_type: "bearer", + scope: "repo read:org", + }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const events = logs.map((l) => l.entry.event); + expect(events).toContain("api_request"); + expect(events).toContain("token_exchange_started"); + expect(events).toContain("github_oauth_request_sent"); + expect(events).toContain("token_exchange_succeeded"); + + const successLog = findLog(logs, "token_exchange_succeeded")!; + expect(successLog.level).toBe("info"); + expect(successLog.entry.scope).toBe("repo read:org"); + expect(successLog.entry.token_type).toBe("bearer"); + expect(successLog.entry.github_status).toBe(200); + }); + + it("logs token_exchange_wrong_method for non-POST requests", async () => { + const req = makeRequest("GET", "/api/oauth/token"); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const methodLog = findLog(logs, "token_exchange_wrong_method"); + expect(methodLog).toBeDefined(); + expect(methodLog!.level).toBe("warn"); + expect(methodLog!.entry.method).toBe("GET"); + }); + + it("logs token_exchange_bad_content_type for wrong Content-Type", async () => { + const req = makeRequest("POST", "/api/oauth/token", { + body: { code: VALID_CODE }, + contentType: "text/plain", + }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const ctLog = findLog(logs, "token_exchange_bad_content_type"); + expect(ctLog).toBeDefined(); + expect(ctLog!.level).toBe("warn"); + expect(ctLog!.entry.content_type).toBe("text/plain"); + }); + + it("logs token_exchange_json_parse_failed for malformed JSON body", async () => { + const req = new Request("https://gh.gordoncode.dev/api/oauth/token", { + method: "POST", + headers: { + "Origin": ALLOWED_ORIGIN, + "Content-Type": "application/json", + }, + body: "not-valid-json{{{", + }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const parseLog = findLog(logs, "token_exchange_json_parse_failed"); + expect(parseLog).toBeDefined(); + expect(parseLog!.level).toBe("warn"); + expect(typeof parseLog!.entry.error).toBe("string"); + }); + + it("logs token_exchange_missing_code when code field is absent", async () => { + const req = makeRequest("POST", "/api/oauth/token", { body: { not_code: "abc" } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const codeLog = findLog(logs, "token_exchange_missing_code"); + expect(codeLog).toBeDefined(); + expect(codeLog!.level).toBe("warn"); + expect(codeLog!.entry.body_type).toBe("object"); + expect(codeLog!.entry.has_code).toBe(false); + }); + + it("logs token_exchange_missing_code when code is not a string", async () => { + const req = makeRequest("POST", "/api/oauth/token", { body: { code: 12345 } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const codeLog = findLog(logs, "token_exchange_missing_code"); + expect(codeLog).toBeDefined(); + expect(codeLog!.entry.has_code).toBe(true); + expect(codeLog!.entry.code_type).toBe("number"); + }); + + it("logs token_exchange_invalid_code_format for regex-failing codes", async () => { + const req = makeRequest("POST", "/api/oauth/token", { body: { code: "abc def!!" } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const fmtLog = findLog(logs, "token_exchange_invalid_code_format"); + expect(fmtLog).toBeDefined(); + expect(fmtLog!.level).toBe("warn"); + expect(fmtLog!.entry.code_length).toBe(9); + expect(fmtLog!.entry.code_has_spaces).toBe(true); + }); + + it("logs github_oauth_fetch_failed when GitHub is unreachable", async () => { + globalThis.fetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const fetchLog = findLog(logs, "github_oauth_fetch_failed"); + expect(fetchLog).toBeDefined(); + expect(fetchLog!.level).toBe("error"); + expect(fetchLog!.entry.error).toBe("fetch failed"); + expect(fetchLog!.entry.error_name).toBe("TypeError"); + }); + + it("logs github_oauth_error_response with GitHub error details", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ + error: "bad_verification_code", + error_description: "The code passed is incorrect or expired.", + error_uri: "https://docs.github.com/apps/managing-oauth-apps/troubleshooting-oauth-app-access-token-request-errors/#bad-verification-code", + }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const errLog = findLog(logs, "github_oauth_error_response"); + expect(errLog).toBeDefined(); + expect(errLog!.level).toBe("error"); + expect(errLog!.entry.github_error).toBe("bad_verification_code"); + expect(errLog!.entry.github_error_description).toBe("The code passed is incorrect or expired."); + expect(errLog!.entry.github_error_uri).toContain("docs.github.com"); + expect(errLog!.entry.has_access_token).toBe(false); + }); + + it("logs github_oauth_error_response when access_token is missing from GitHub response", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ token_type: "bearer" }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const errLog = findLog(logs, "github_oauth_error_response"); + expect(errLog).toBeDefined(); + expect(errLog!.entry.has_access_token).toBe(false); + }); + + // ── Security: logs must NEVER contain secrets ───────────────────────── + + it("logs never contain access tokens, codes, or client secrets", async () => { + const sensitiveToken = "ghu_SuperSecretToken123"; + const sensitiveSecret = "test_client_secret"; + + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ + access_token: sensitiveToken, + token_type: "bearer", + scope: "repo", + }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const allLogText = logs.map((l) => JSON.stringify(l.entry)).join("\n"); + + expect(allLogText).not.toContain(sensitiveToken); + expect(allLogText).not.toContain(sensitiveSecret); + expect(allLogText).not.toContain(VALID_CODE); + }); + + it("logs never contain secrets on error paths either", async () => { + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ + error: "bad_verification_code", + error_description: "The code is wrong", + }), { status: 200 }) + ); + + const req = makeRequest("POST", "/api/oauth/token", { body: { code: VALID_CODE } }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const allLogText = logs.map((l) => JSON.stringify(l.entry)).join("\n"); + + expect(allLogText).not.toContain(VALID_CODE); + expect(allLogText).not.toContain("test_client_secret"); + }); + }); + + // ── Sentry tunnel ───────────────────────────────────────────────────────── + + describe("Sentry tunnel (/api/error-reporting)", () => { + const SENTRY_HOST = "o123456.ingest.sentry.io"; + const SENTRY_PROJECT_ID = "7890123"; + const VALID_DSN = `https://abc123@${SENTRY_HOST}/${SENTRY_PROJECT_ID}`; + + function makeEnvelope(dsn: string, eventPayload = "{}"): string { + return `${JSON.stringify({ dsn })}\n${JSON.stringify({ type: "event" })}\n${eventPayload}`; + } + + function makeTunnelRequest(body: string): Request { + return new Request("https://gh.gordoncode.dev/api/error-reporting", { + method: "POST", + headers: { "Content-Type": "application/x-sentry-envelope" }, + body, + }); + } + + it("forwards valid envelope to Sentry and returns Sentry's status code", async () => { + const mockFetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + globalThis.fetch = mockFetch; + + const req = makeTunnelRequest(makeEnvelope(VALID_DSN)); + const res = await worker.fetch(req, makeEnv()); + + expect(res.status).toBe(200); + expect(mockFetch).toHaveBeenCalledOnce(); + + const [url, init] = mockFetch.mock.calls[0] as [string, RequestInit]; + expect(url).toBe(`https://${SENTRY_HOST}/api/${SENTRY_PROJECT_ID}/envelope/`); + expect(init.method).toBe("POST"); + }); + + it("rejects GET requests with 405", async () => { + const req = new Request("https://gh.gordoncode.dev/api/error-reporting", { method: "GET" }); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(405); + }); + + it("rejects envelopes with mismatched DSN host", async () => { + const badDsn = `https://abc@evil.ingest.sentry.io/${SENTRY_PROJECT_ID}`; + const req = makeTunnelRequest(makeEnvelope(badDsn)); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(403); + + const logs = collectLogs(consoleSpy); + const mismatchLog = findLog(logs, "sentry_tunnel_dsn_mismatch"); + expect(mismatchLog).toBeDefined(); + expect(mismatchLog!.entry.dsn_host).toBe("evil.ingest.sentry.io"); + }); + + it("rejects envelopes with mismatched DSN project ID", async () => { + const badDsn = `https://abc@${SENTRY_HOST}/9999999`; + const req = makeTunnelRequest(makeEnvelope(badDsn)); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(403); + + const logs = collectLogs(consoleSpy); + const mismatchLog = findLog(logs, "sentry_tunnel_dsn_mismatch"); + expect(mismatchLog).toBeDefined(); + expect(mismatchLog!.entry.dsn_project).toBe("9999999"); + }); + + it("returns 400 for invalid envelope format (no newline)", async () => { + const req = makeTunnelRequest("not an envelope"); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(400); + }); + + it("returns 400 for invalid JSON in envelope header", async () => { + const req = makeTunnelRequest("{invalid json\n{}"); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(400); + }); + + it("returns 200 for client_report envelopes without DSN", async () => { + const envelope = `${JSON.stringify({ type: "client_report" })}\n{}`; + const req = makeTunnelRequest(envelope); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(200); + }); + + it("returns 400 for invalid DSN URL", async () => { + const envelope = `${JSON.stringify({ dsn: "not-a-url" })}\n{}`; + const req = makeTunnelRequest(envelope); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(400); + }); + + it("returns 404 when SENTRY_HOST is not configured", async () => { + const req = makeTunnelRequest(makeEnvelope(VALID_DSN)); + const res = await worker.fetch(req, makeEnv({ SENTRY_HOST: "", SENTRY_PROJECT_ID: "" })); + expect(res.status).toBe(404); + }); + + it("returns 502 when Sentry is unreachable", async () => { + globalThis.fetch = vi.fn().mockRejectedValue(new Error("connection refused")); + + const req = makeTunnelRequest(makeEnvelope(VALID_DSN)); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(502); + + const logs = collectLogs(consoleSpy); + const fetchLog = findLog(logs, "sentry_tunnel_fetch_failed"); + expect(fetchLog).toBeDefined(); + expect(fetchLog!.level).toBe("error"); + }); + + it("logs sentry_tunnel_forwarded on successful proxy", async () => { + globalThis.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + + const req = makeTunnelRequest(makeEnvelope(VALID_DSN)); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const fwdLog = findLog(logs, "sentry_tunnel_forwarded"); + expect(fwdLog).toBeDefined(); + expect(fwdLog!.level).toBe("info"); + expect(fwdLog!.entry.sentry_status).toBe(200); + }); + + it("includes security headers on all tunnel responses", async () => { + const req = new Request("https://gh.gordoncode.dev/api/error-reporting", { method: "GET" }); + const res = await worker.fetch(req, makeEnv()); + expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff"); + expect(res.headers.get("X-Frame-Options")).toBe("DENY"); + }); + + it("never logs the envelope body contents", async () => { + const sensitivePayload = '{"user":{"email":"user@example.com"}}'; + globalThis.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + + const req = makeTunnelRequest(makeEnvelope(VALID_DSN, sensitivePayload)); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const allLogText = logs.map((l) => JSON.stringify(l.entry)).join("\n"); + expect(allLogText).not.toContain("user@example.com"); + expect(allLogText).not.toContain(sensitivePayload); + }); + }); }); diff --git a/wrangler.toml b/wrangler.toml index e4d95c7..70fe3bb 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -14,6 +14,14 @@ not_found_handling = "single-page-application" pattern = "gh.gordoncode.dev" custom_domain = true +# Sentry tunnel — public identifiers, not secrets. +# Set SENTRY_HOST and SENTRY_PROJECT_ID after creating a Sentry project. +# Example: SENTRY_HOST = "o123456.ingest.sentry.io" +# SENTRY_PROJECT_ID = "7890123" +[vars] +SENTRY_HOST = "" +SENTRY_PROJECT_ID = "" + [observability] enabled = true head_sampling_rate = 1 From 8226c9d413501901bc80c814f8c0996725ae1548 Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 11:11:31 -0400 Subject: [PATCH 02/10] refactor(worker): simplifies Sentry config to single SENTRY_DSN var --- src/worker/index.ts | 41 +++++++++++++++++++++++--------------- tests/worker/oauth.test.ts | 7 +++---- wrangler.toml | 10 ++++------ 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/worker/index.ts b/src/worker/index.ts index 2d0da7d..3ab240a 100644 --- a/src/worker/index.ts +++ b/src/worker/index.ts @@ -3,8 +3,7 @@ export interface Env { GITHUB_CLIENT_ID: string; GITHUB_CLIENT_SECRET: string; ALLOWED_ORIGIN: string; - SENTRY_HOST: string; // e.g. "o123456.ingest.sentry.io" - SENTRY_PROJECT_ID: string; // e.g. "7890123" + SENTRY_DSN: string; // e.g. "https://key@o123456.ingest.sentry.io/7890123" } // Predefined error strings only (SDR-006) @@ -80,10 +79,22 @@ function getCorsHeaders( // ── Sentry tunnel ───────────────────────────────────────────────────────── // Proxies Sentry event envelopes through our own domain so the browser // treats them as same-origin (no CSP change, no ad-blocker interference). -// The DSN is validated against env.SENTRY_HOST + env.SENTRY_PROJECT_ID to -// prevent abuse as an open proxy. +// The envelope DSN is validated against env.SENTRY_DSN to prevent open proxy abuse. const SENTRY_ENVELOPE_MAX_BYTES = 256 * 1024; // 256 KB — Sentry rejects >200KB compressed +/** Parse host and project ID from a Sentry DSN URL. Returns null if invalid. */ +function parseSentryDsn(dsn: string): { host: string; projectId: string } | null { + if (!dsn) return null; + try { + const url = new URL(dsn); + const projectId = url.pathname.replace(/\//g, ""); + if (!url.hostname || !projectId) return null; + return { host: url.hostname, projectId }; + } catch { + return null; + } +} + async function handleSentryTunnel( request: Request, env: Env, @@ -92,7 +103,8 @@ async function handleSentryTunnel( return new Response(null, { status: 405, headers: securityHeaders() }); } - if (!env.SENTRY_HOST || !env.SENTRY_PROJECT_ID) { + const allowedDsn = parseSentryDsn(env.SENTRY_DSN); + if (!allowedDsn) { log("warn", "sentry_tunnel_not_configured", {}, request); return new Response(null, { status: 404, headers: securityHeaders() }); } @@ -126,31 +138,28 @@ async function handleSentryTunnel( } if (typeof envelopeHeader.dsn !== "string") { - // client_report envelopes may omit dsn — forward if host is configured + // client_report envelopes may omit dsn — drop silently log("info", "sentry_tunnel_no_dsn", {}, request); return new Response(null, { status: 200, headers: securityHeaders() }); } - // Validate DSN matches our project — prevents open proxy abuse - let dsnUrl: URL; - try { - dsnUrl = new URL(envelopeHeader.dsn); - } catch { + // Validate envelope DSN matches our project — prevents open proxy abuse + const envelopeDsn = parseSentryDsn(envelopeHeader.dsn); + if (!envelopeDsn) { log("warn", "sentry_tunnel_invalid_dsn", {}, request); return new Response(null, { status: 400, headers: securityHeaders() }); } - const dsnProjectId = dsnUrl.pathname.replace(/\//g, ""); - if (dsnUrl.hostname !== env.SENTRY_HOST || dsnProjectId !== env.SENTRY_PROJECT_ID) { + if (envelopeDsn.host !== allowedDsn.host || envelopeDsn.projectId !== allowedDsn.projectId) { log("warn", "sentry_tunnel_dsn_mismatch", { - dsn_host: dsnUrl.hostname, - dsn_project: dsnProjectId, + dsn_host: envelopeDsn.host, + dsn_project: envelopeDsn.projectId, }, request); return new Response(null, { status: 403, headers: securityHeaders() }); } // Forward to Sentry ingest endpoint - const sentryUrl = `https://${env.SENTRY_HOST}/api/${env.SENTRY_PROJECT_ID}/envelope/`; + const sentryUrl = `https://${allowedDsn.host}/api/${allowedDsn.projectId}/envelope/`; try { const sentryResp = await fetch(sentryUrl, { method: "POST", diff --git a/tests/worker/oauth.test.ts b/tests/worker/oauth.test.ts index aa6cbaf..95a89d2 100644 --- a/tests/worker/oauth.test.ts +++ b/tests/worker/oauth.test.ts @@ -9,8 +9,7 @@ function makeEnv(overrides: Partial = {}): Env { GITHUB_CLIENT_ID: "test_client_id", GITHUB_CLIENT_SECRET: "test_client_secret", ALLOWED_ORIGIN, - SENTRY_HOST: "o123456.ingest.sentry.io", - SENTRY_PROJECT_ID: "7890123", + SENTRY_DSN: "https://abc123@o123456.ingest.sentry.io/7890123", ...overrides, }; } @@ -850,9 +849,9 @@ describe("Worker OAuth endpoint", () => { expect(res.status).toBe(400); }); - it("returns 404 when SENTRY_HOST is not configured", async () => { + it("returns 404 when SENTRY_DSN is not configured", async () => { const req = makeTunnelRequest(makeEnvelope(VALID_DSN)); - const res = await worker.fetch(req, makeEnv({ SENTRY_HOST: "", SENTRY_PROJECT_ID: "" })); + const res = await worker.fetch(req, makeEnv({ SENTRY_DSN: "" })); expect(res.status).toBe(404); }); diff --git a/wrangler.toml b/wrangler.toml index 70fe3bb..7dffaf5 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -14,13 +14,11 @@ not_found_handling = "single-page-application" pattern = "gh.gordoncode.dev" custom_domain = true -# Sentry tunnel — public identifiers, not secrets. -# Set SENTRY_HOST and SENTRY_PROJECT_ID after creating a Sentry project. -# Example: SENTRY_HOST = "o123456.ingest.sentry.io" -# SENTRY_PROJECT_ID = "7890123" +# Sentry tunnel — DSN is a public identifier, not a secret. +# Set after creating a Sentry project. +# Example: SENTRY_DSN = "https://key@o123456.ingest.sentry.io/7890123" [vars] -SENTRY_HOST = "" -SENTRY_PROJECT_ID = "" +SENTRY_DSN = "" [observability] enabled = true From fb08477d8b95a12dcbb340b32b7c00a4346b8d94 Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 11:15:10 -0400 Subject: [PATCH 03/10] refactor(sentry): uses VITE_SENTRY_DSN env var, removes placeholder --- src/app/lib/sentry.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/lib/sentry.ts b/src/app/lib/sentry.ts index ab680fb..22451b7 100644 --- a/src/app/lib/sentry.ts +++ b/src/app/lib/sentry.ts @@ -1,8 +1,5 @@ import * as Sentry from "@sentry/solid"; -const SENTRY_DSN = - "https://placeholder@o0.ingest.sentry.io/0"; // TODO: replace with real DSN - /** Strip OAuth credentials from any captured URL. */ function scrubUrl(url: string): string { return url @@ -24,8 +21,11 @@ export function initSentry(): void { // Only initialize in production — never in dev/test if (import.meta.env.DEV) return; + const dsn = import.meta.env.VITE_SENTRY_DSN; + if (!dsn) return; + Sentry.init({ - dsn: SENTRY_DSN, + dsn, tunnel: "/api/error-reporting", environment: import.meta.env.MODE, From f948c7c1a89422159a1f16f9d8959d1bd6838001 Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 11:19:57 -0400 Subject: [PATCH 04/10] refactor(sentry): hardcodes DSN directly, removes env var indirection --- src/app/lib/sentry.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/lib/sentry.ts b/src/app/lib/sentry.ts index 22451b7..48a90b1 100644 --- a/src/app/lib/sentry.ts +++ b/src/app/lib/sentry.ts @@ -17,15 +17,15 @@ const ALLOWED_CONSOLE_PREFIXES = [ "[settings]", ]; -export function initSentry(): void { - // Only initialize in production — never in dev/test - if (import.meta.env.DEV) return; +// DSN is a public project identifier, not a secret. +// Set this after creating a Sentry project. +const SENTRY_DSN = ""; - const dsn = import.meta.env.VITE_SENTRY_DSN; - if (!dsn) return; +export function initSentry(): void { + if (import.meta.env.DEV || !SENTRY_DSN) return; Sentry.init({ - dsn, + dsn: SENTRY_DSN, tunnel: "/api/error-reporting", environment: import.meta.env.MODE, From 684f76bf7c69b2c9be2f5ec4cef1f64a2f122b9b Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 11:28:25 -0400 Subject: [PATCH 05/10] chore(sentry): sets production DSN --- src/app/lib/sentry.ts | 2 +- wrangler.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/lib/sentry.ts b/src/app/lib/sentry.ts index 48a90b1..dbf915a 100644 --- a/src/app/lib/sentry.ts +++ b/src/app/lib/sentry.ts @@ -19,7 +19,7 @@ const ALLOWED_CONSOLE_PREFIXES = [ // DSN is a public project identifier, not a secret. // Set this after creating a Sentry project. -const SENTRY_DSN = ""; +const SENTRY_DSN = "https://4dc4335a9746201c02ff2107c0d20f73@o284235.ingest.us.sentry.io/4511122822922240"; export function initSentry(): void { if (import.meta.env.DEV || !SENTRY_DSN) return; diff --git a/wrangler.toml b/wrangler.toml index 7dffaf5..c82cf10 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -18,7 +18,7 @@ custom_domain = true # Set after creating a Sentry project. # Example: SENTRY_DSN = "https://key@o123456.ingest.sentry.io/7890123" [vars] -SENTRY_DSN = "" +SENTRY_DSN = "https://4dc4335a9746201c02ff2107c0d20f73@o284235.ingest.us.sentry.io/4511122822922240" [observability] enabled = true From a283084637fc9263ac9088c994959ad6bb91ad1a Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 13:47:54 -0400 Subject: [PATCH 06/10] fix: addresses PR review findings across security, testing, and quality - Fixes CI failure: removes test:waf script referencing gitignored hack/ dir - Fixes Content-Length bypass: enforces body.length after read, not header - Fixes parseSentryDsn path extraction: uses split/pop instead of replace - Fixes query_string scrubbing: uses scrubUrl instead of blanket redaction - Caches parsed DSN per isolate to avoid repeated URL parsing - Replaces securityHeaders() function with SECURITY_HEADERS constant - Moves token_exchange_started log after method check for accuracy - Simplifies token_exchange_missing_code log payload to 2 fields - Removes allowed_origin from api_request log (env var leak pattern) - Exports scrubUrl/beforeSendHandler/beforeBreadcrumbHandler for testing - Adds 26 new tests: sentry.ts unit tests, 413 enforcement, tunnel log assertions, OPTIONS tunnel, CORS mismatch for tunnel path --- .github/workflows/ci.yml | 2 - package.json | 3 +- src/app/lib/sentry.ts | 123 +++++++++++---------- src/worker/index.ts | 70 ++++++------ tests/lib/sentry.test.ts | 214 +++++++++++++++++++++++++++++++++++++ tests/worker/oauth.test.ts | 77 ++++++++++++- wrangler.toml | 3 - 7 files changed, 390 insertions(+), 102 deletions(-) create mode 100644 tests/lib/sentry.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9ea4bf..ab287a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,8 +16,6 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm run typecheck - run: pnpm test - - name: WAF smoke tests - run: pnpm test:waf - name: Verify CSP hash run: node scripts/verify-csp-hash.mjs - name: Install Playwright browsers diff --git a/package.json b/package.json index a49f07c..35e4b75 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,7 @@ "test:watch": "vitest --config vitest.workspace.ts", "deploy": "wrangler deploy", "typecheck": "tsc --noEmit", - "test:e2e": "E2E_PORT=$(node -e \"const s=require('net').createServer();s.listen(0,()=>{console.log(s.address().port);s.close()})\") playwright test", - "test:waf": "bash hack/scripts/waf-smoke-test.sh" + "test:e2e": "E2E_PORT=$(node -e \"const s=require('net').createServer();s.listen(0,()=>{console.log(s.address().port);s.close()})\") playwright test" }, "dependencies": { "@kobalte/core": "^0.13.11", diff --git a/src/app/lib/sentry.ts b/src/app/lib/sentry.ts index dbf915a..181dda6 100644 --- a/src/app/lib/sentry.ts +++ b/src/app/lib/sentry.ts @@ -1,7 +1,8 @@ import * as Sentry from "@sentry/solid"; +import type { ErrorEvent, Breadcrumb } from "@sentry/solid"; -/** Strip OAuth credentials from any captured URL. */ -function scrubUrl(url: string): string { +/** Strip OAuth credentials from any captured URL or query string. */ +export function scrubUrl(url: string): string { return url .replace(/code=[^&\s]+/g, "code=[REDACTED]") .replace(/state=[^&\s]+/g, "state=[REDACTED]") @@ -17,10 +18,67 @@ const ALLOWED_CONSOLE_PREFIXES = [ "[settings]", ]; -// DSN is a public project identifier, not a secret. -// Set this after creating a Sentry project. const SENTRY_DSN = "https://4dc4335a9746201c02ff2107c0d20f73@o284235.ingest.us.sentry.io/4511122822922240"; +export function beforeSendHandler(event: ErrorEvent): ErrorEvent | null { + // Strip OAuth params from captured URLs + if (event.request?.url) { + event.request.url = scrubUrl(event.request.url); + } + if (event.request?.query_string) { + event.request.query_string = + typeof event.request.query_string === "string" + ? scrubUrl(event.request.query_string) + : "[REDACTED]"; + } + // Remove headers and cookies entirely + delete event.request?.headers; + delete event.request?.cookies; + // Remove user identity — we never want to track users + delete event.user; + // Scrub URLs in stack trace frames + if (event.exception?.values) { + for (const ex of event.exception.values) { + if (ex.stacktrace?.frames) { + for (const frame of ex.stacktrace.frames) { + if (frame.abs_path) { + frame.abs_path = scrubUrl(frame.abs_path); + } + } + } + } + } + return event; +} + +export function beforeBreadcrumbHandler( + breadcrumb: Breadcrumb, +): Breadcrumb | null { + // Scrub URLs in navigation breadcrumbs + if (breadcrumb.category === "navigation") { + if (breadcrumb.data?.from) + breadcrumb.data.from = scrubUrl(breadcrumb.data.from as string); + if (breadcrumb.data?.to) + breadcrumb.data.to = scrubUrl(breadcrumb.data.to as string); + } + // Scrub URLs in fetch/xhr breadcrumbs + if ( + breadcrumb.category === "fetch" || + breadcrumb.category === "xhr" + ) { + if (breadcrumb.data?.url) + breadcrumb.data.url = scrubUrl(breadcrumb.data.url as string); + } + // Only keep our own tagged console logs — drop third-party noise + if (breadcrumb.category === "console") { + const msg = breadcrumb.message ?? ""; + if (!ALLOWED_CONSOLE_PREFIXES.some((p) => msg.startsWith(p))) { + return null; + } + } + return breadcrumb; +} + export function initSentry(): void { if (import.meta.env.DEV || !SENTRY_DSN) return; @@ -35,65 +93,12 @@ export function initSentry(): void { // ── Disable everything except error tracking ──────────────── tracesSampleRate: 0, profilesSampleRate: 0, - // No replay integration — DOM capture is unnecessary for auth - // debugging and carries high privacy cost. // ── Only capture errors from our own code ─────────────────── allowUrls: [/^https:\/\/gh\.gordoncode\.dev/], // ── Scrub sensitive data before it leaves the browser ──────── - beforeSend(event) { - // Strip OAuth params from captured URLs - if (event.request?.url) { - event.request.url = scrubUrl(event.request.url); - } - if (event.request?.query_string) { - event.request.query_string = "[REDACTED]"; - } - // Remove headers and cookies entirely - delete event.request?.headers; - delete event.request?.cookies; - // Remove user identity — we never want to track users - delete event.user; - // Scrub URLs in stack trace frames - if (event.exception?.values) { - for (const ex of event.exception.values) { - if (ex.stacktrace?.frames) { - for (const frame of ex.stacktrace.frames) { - if (frame.abs_path) { - frame.abs_path = scrubUrl(frame.abs_path); - } - } - } - } - } - return event; - }, - - beforeBreadcrumb(breadcrumb) { - // Scrub URLs in navigation breadcrumbs - if (breadcrumb.category === "navigation") { - if (breadcrumb.data?.from) - breadcrumb.data.from = scrubUrl(breadcrumb.data.from as string); - if (breadcrumb.data?.to) - breadcrumb.data.to = scrubUrl(breadcrumb.data.to as string); - } - // Scrub URLs in fetch/xhr breadcrumbs - if ( - breadcrumb.category === "fetch" || - breadcrumb.category === "xhr" - ) { - if (breadcrumb.data?.url) - breadcrumb.data.url = scrubUrl(breadcrumb.data.url as string); - } - // Only keep our own tagged console logs — drop third-party noise - if (breadcrumb.category === "console") { - const msg = breadcrumb.message ?? ""; - if (!ALLOWED_CONSOLE_PREFIXES.some((p) => msg.startsWith(p))) { - return null; - } - } - return breadcrumb; - }, + beforeSend: beforeSendHandler, + beforeBreadcrumb: beforeBreadcrumbHandler, }); } diff --git a/src/worker/index.ts b/src/worker/index.ts index 3ab240a..52700d2 100644 --- a/src/worker/index.ts +++ b/src/worker/index.ts @@ -47,18 +47,16 @@ function errorResponse( headers: { "Content-Type": "application/json", ...corsHeaders, - ...securityHeaders(), + ...SECURITY_HEADERS, }, }); } -function securityHeaders(): Record { - return { - "X-Content-Type-Options": "nosniff", - "Referrer-Policy": "strict-origin-when-cross-origin", - "X-Frame-Options": "DENY", - }; -} +const SECURITY_HEADERS: Record = { + "X-Content-Type-Options": "nosniff", + "Referrer-Policy": "strict-origin-when-cross-origin", + "X-Frame-Options": "DENY", +}; // CORS: strict equality only (SDR-004) function getCorsHeaders( @@ -82,12 +80,14 @@ function getCorsHeaders( // The envelope DSN is validated against env.SENTRY_DSN to prevent open proxy abuse. const SENTRY_ENVELOPE_MAX_BYTES = 256 * 1024; // 256 KB — Sentry rejects >200KB compressed +let _dsnCache: { dsn: string; parsed: { host: string; projectId: string } | null } | undefined; + /** Parse host and project ID from a Sentry DSN URL. Returns null if invalid. */ function parseSentryDsn(dsn: string): { host: string; projectId: string } | null { if (!dsn) return null; try { const url = new URL(dsn); - const projectId = url.pathname.replace(/\//g, ""); + const projectId = url.pathname.split("/").filter(Boolean).pop() ?? ""; if (!url.hostname || !projectId) return null; return { host: url.hostname, projectId }; } catch { @@ -100,33 +100,36 @@ async function handleSentryTunnel( env: Env, ): Promise { if (request.method !== "POST") { - return new Response(null, { status: 405, headers: securityHeaders() }); + return new Response(null, { status: 405, headers: SECURITY_HEADERS }); } - const allowedDsn = parseSentryDsn(env.SENTRY_DSN); + if (!_dsnCache || _dsnCache.dsn !== env.SENTRY_DSN) { + _dsnCache = { dsn: env.SENTRY_DSN, parsed: parseSentryDsn(env.SENTRY_DSN) }; + } + const allowedDsn = _dsnCache.parsed; if (!allowedDsn) { log("warn", "sentry_tunnel_not_configured", {}, request); - return new Response(null, { status: 404, headers: securityHeaders() }); - } - - const contentLength = parseInt(request.headers.get("Content-Length") ?? "0", 10); - if (contentLength > SENTRY_ENVELOPE_MAX_BYTES) { - log("warn", "sentry_tunnel_payload_too_large", { content_length: contentLength }, request); - return new Response(null, { status: 413, headers: securityHeaders() }); + return new Response(null, { status: 404, headers: SECURITY_HEADERS }); } let body: string; try { body = await request.text(); } catch { - return new Response(null, { status: 400, headers: securityHeaders() }); + log("warn", "sentry_tunnel_body_read_failed", {}, request); + return new Response(null, { status: 400, headers: SECURITY_HEADERS }); + } + + if (body.length > SENTRY_ENVELOPE_MAX_BYTES) { + log("warn", "sentry_tunnel_payload_too_large", { body_length: body.length }, request); + return new Response(null, { status: 413, headers: SECURITY_HEADERS }); } // Sentry envelope format: first line is JSON header with dsn field const firstNewline = body.indexOf("\n"); if (firstNewline === -1) { log("warn", "sentry_tunnel_invalid_envelope", {}, request); - return new Response(null, { status: 400, headers: securityHeaders() }); + return new Response(null, { status: 400, headers: SECURITY_HEADERS }); } let envelopeHeader: { dsn?: string }; @@ -134,20 +137,20 @@ async function handleSentryTunnel( envelopeHeader = JSON.parse(body.substring(0, firstNewline)); } catch { log("warn", "sentry_tunnel_header_parse_failed", {}, request); - return new Response(null, { status: 400, headers: securityHeaders() }); + return new Response(null, { status: 400, headers: SECURITY_HEADERS }); } if (typeof envelopeHeader.dsn !== "string") { // client_report envelopes may omit dsn — drop silently log("info", "sentry_tunnel_no_dsn", {}, request); - return new Response(null, { status: 200, headers: securityHeaders() }); + return new Response(null, { status: 200, headers: SECURITY_HEADERS }); } // Validate envelope DSN matches our project — prevents open proxy abuse const envelopeDsn = parseSentryDsn(envelopeHeader.dsn); if (!envelopeDsn) { log("warn", "sentry_tunnel_invalid_dsn", {}, request); - return new Response(null, { status: 400, headers: securityHeaders() }); + return new Response(null, { status: 400, headers: SECURITY_HEADERS }); } if (envelopeDsn.host !== allowedDsn.host || envelopeDsn.projectId !== allowedDsn.projectId) { @@ -155,7 +158,7 @@ async function handleSentryTunnel( dsn_host: envelopeDsn.host, dsn_project: envelopeDsn.projectId, }, request); - return new Response(null, { status: 403, headers: securityHeaders() }); + return new Response(null, { status: 403, headers: SECURITY_HEADERS }); } // Forward to Sentry ingest endpoint @@ -173,13 +176,13 @@ async function handleSentryTunnel( return new Response(null, { status: sentryResp.status, - headers: securityHeaders(), + headers: SECURITY_HEADERS, }); } catch (err) { log("error", "sentry_tunnel_fetch_failed", { error: err instanceof Error ? err.message : "unknown", }, request); - return new Response(null, { status: 502, headers: securityHeaders() }); + return new Response(null, { status: 502, headers: SECURITY_HEADERS }); } } @@ -193,13 +196,13 @@ async function handleTokenExchange( env: Env, cors: Record ): Promise { - log("info", "token_exchange_started", { method: request.method }, request); - if (request.method !== "POST") { log("warn", "token_exchange_wrong_method", { method: request.method }, request); return errorResponse("method_not_allowed", 405, cors); } + log("info", "token_exchange_started", {}, request); + const contentType = request.headers.get("Content-Type") ?? ""; if (!contentType.includes("application/json")) { log("warn", "token_exchange_bad_content_type", { content_type: contentType }, request); @@ -222,10 +225,8 @@ async function handleTokenExchange( typeof (body as Record)["code"] !== "string" ) { log("warn", "token_exchange_missing_code", { - body_type: typeof body, - body_is_null: body === null, has_code: body !== null && typeof body === "object" && "code" in body, - code_type: body !== null && typeof body === "object" + code_type: body !== null && typeof body === "object" && "code" in body ? typeof (body as Record)["code"] : "n/a", }, request); @@ -307,7 +308,7 @@ async function handleTokenExchange( headers: { "Content-Type": "application/json", ...cors, - ...securityHeaders(), + ...SECURITY_HEADERS, }, }); } @@ -325,7 +326,6 @@ export default { method: request.method, pathname: url.pathname, cors_matched: corsMatched, - allowed_origin: env.ALLOWED_ORIGIN, }, request); if (!corsMatched && origin !== null) { @@ -341,7 +341,7 @@ export default { log("info", "cors_preflight", { cors_matched: corsMatched }, request); return new Response(null, { status: 204, - headers: { ...cors, "Access-Control-Max-Age": "86400", ...securityHeaders() }, + headers: { ...cors, "Access-Control-Max-Age": "86400", ...SECURITY_HEADERS }, }); } @@ -356,7 +356,7 @@ export default { if (url.pathname === "/api/health" && request.method === "GET") { return new Response("OK", { - headers: securityHeaders(), + headers: SECURITY_HEADERS, }); } diff --git a/tests/lib/sentry.test.ts b/tests/lib/sentry.test.ts new file mode 100644 index 0000000..5e612a9 --- /dev/null +++ b/tests/lib/sentry.test.ts @@ -0,0 +1,214 @@ +import { describe, it, expect } from "vitest"; +import { + scrubUrl, + beforeSendHandler, + beforeBreadcrumbHandler, +} from "../../src/app/lib/sentry"; + +describe("scrubUrl", () => { + it("strips code= parameter", () => { + expect(scrubUrl("https://example.com/cb?code=abc123&state=xyz")).toBe( + "https://example.com/cb?code=[REDACTED]&state=[REDACTED]", + ); + }); + + it("strips access_token= parameter", () => { + expect(scrubUrl("https://example.com?access_token=ghu_secret")).toBe( + "https://example.com?access_token=[REDACTED]", + ); + }); + + it("strips state= parameter", () => { + expect(scrubUrl("https://example.com?state=random_state_value")).toBe( + "https://example.com?state=[REDACTED]", + ); + }); + + it("strips multiple sensitive params at once", () => { + const url = + "https://example.com/cb?code=abc&state=xyz&access_token=ghu_tok&other=safe"; + const result = scrubUrl(url); + expect(result).toContain("code=[REDACTED]"); + expect(result).toContain("state=[REDACTED]"); + expect(result).toContain("access_token=[REDACTED]"); + expect(result).toContain("other=safe"); + }); + + it("returns URL unchanged when no sensitive params present", () => { + const url = "https://example.com/page?tab=issues&sort=updated"; + expect(scrubUrl(url)).toBe(url); + }); + + it("handles empty string", () => { + expect(scrubUrl("")).toBe(""); + }); + + it("handles params at end of string (no trailing &)", () => { + expect(scrubUrl("https://example.com?code=abc")).toBe( + "https://example.com?code=[REDACTED]", + ); + }); +}); + +describe("beforeSendHandler", () => { + it("scrubs OAuth params from request URL", () => { + const event = { + request: { + url: "https://gh.gordoncode.dev/cb?code=abc123&state=xyz", + }, + }; + const result = beforeSendHandler(event as never); + expect(result!.request!.url).toBe( + "https://gh.gordoncode.dev/cb?code=[REDACTED]&state=[REDACTED]", + ); + }); + + it("scrubs query_string selectively when it is a string", () => { + const event = { + request: { + url: "https://gh.gordoncode.dev/cb", + query_string: "code=abc123&tab=issues", + }, + }; + const result = beforeSendHandler(event as never); + expect(result!.request!.query_string).toBe("code=[REDACTED]&tab=issues"); + }); + + it("redacts query_string entirely when it is not a string", () => { + const event = { + request: { + url: "https://gh.gordoncode.dev/cb", + query_string: [["code", "abc123"]], + }, + }; + const result = beforeSendHandler(event as never); + expect(result!.request!.query_string).toBe("[REDACTED]"); + }); + + it("deletes request headers and cookies", () => { + const event = { + request: { + url: "https://gh.gordoncode.dev", + headers: { Authorization: "Bearer ghu_token" }, + cookies: "session=abc", + }, + }; + const result = beforeSendHandler(event as never); + expect(result!.request!.headers).toBeUndefined(); + expect(result!.request!.cookies).toBeUndefined(); + }); + + it("deletes user identity", () => { + const event = { + request: { url: "https://gh.gordoncode.dev" }, + user: { id: "123", email: "test@example.com" }, + }; + const result = beforeSendHandler(event as never); + expect((result as unknown as Record).user).toBeUndefined(); + }); + + it("scrubs URLs in stack trace frames", () => { + const event = { + request: { url: "https://gh.gordoncode.dev" }, + exception: { + values: [ + { + stacktrace: { + frames: [ + { abs_path: "https://gh.gordoncode.dev/app.js?code=secret" }, + { abs_path: "https://gh.gordoncode.dev/lib.js" }, + ], + }, + }, + ], + }, + }; + const result = beforeSendHandler(event as never); + const frames = result!.exception!.values![0].stacktrace!.frames!; + expect(frames[0].abs_path).toBe( + "https://gh.gordoncode.dev/app.js?code=[REDACTED]", + ); + expect(frames[1].abs_path).toBe("https://gh.gordoncode.dev/lib.js"); + }); + + it("handles events with no request", () => { + const event = {}; + const result = beforeSendHandler(event as never); + expect(result).toBeDefined(); + }); +}); + +describe("beforeBreadcrumbHandler", () => { + it("scrubs navigation breadcrumb URLs", () => { + const breadcrumb = { + category: "navigation", + data: { + from: "https://gh.gordoncode.dev/cb?code=abc", + to: "https://gh.gordoncode.dev/dashboard?state=xyz", + }, + }; + const result = beforeBreadcrumbHandler(breadcrumb as never); + expect(result!.data!.from).toBe( + "https://gh.gordoncode.dev/cb?code=[REDACTED]", + ); + expect(result!.data!.to).toBe( + "https://gh.gordoncode.dev/dashboard?state=[REDACTED]", + ); + }); + + it("scrubs fetch breadcrumb URLs", () => { + const breadcrumb = { + category: "fetch", + data: { url: "https://api.github.com?access_token=ghu_tok" }, + }; + const result = beforeBreadcrumbHandler(breadcrumb as never); + expect(result!.data!.url).toBe( + "https://api.github.com?access_token=[REDACTED]", + ); + }); + + it("scrubs xhr breadcrumb URLs", () => { + const breadcrumb = { + category: "xhr", + data: { url: "https://api.github.com?code=abc" }, + }; + const result = beforeBreadcrumbHandler(breadcrumb as never); + expect(result!.data!.url).toBe( + "https://api.github.com?code=[REDACTED]", + ); + }); + + it("keeps allowed console breadcrumbs", () => { + const prefixes = ["[auth]", "[api]", "[poll]", "[dashboard]", "[settings]"]; + for (const prefix of prefixes) { + const breadcrumb = { + category: "console", + message: `${prefix} some message`, + }; + expect(beforeBreadcrumbHandler(breadcrumb as never)).not.toBeNull(); + } + }); + + it("drops untagged console breadcrumbs", () => { + const breadcrumb = { + category: "console", + message: "random third-party log", + }; + expect(beforeBreadcrumbHandler(breadcrumb as never)).toBeNull(); + }); + + it("drops console breadcrumbs with empty message", () => { + const breadcrumb = { category: "console", message: "" }; + expect(beforeBreadcrumbHandler(breadcrumb as never)).toBeNull(); + }); + + it("drops console breadcrumbs with no message", () => { + const breadcrumb = { category: "console" }; + expect(beforeBreadcrumbHandler(breadcrumb as never)).toBeNull(); + }); + + it("passes through non-console, non-navigation breadcrumbs unchanged", () => { + const breadcrumb = { category: "ui.click", message: "button" }; + expect(beforeBreadcrumbHandler(breadcrumb as never)).toBe(breadcrumb); + }); +}); diff --git a/tests/worker/oauth.test.ts b/tests/worker/oauth.test.ts index 95a89d2..dbfe718 100644 --- a/tests/worker/oauth.test.ts +++ b/tests/worker/oauth.test.ts @@ -638,7 +638,6 @@ describe("Worker OAuth endpoint", () => { const codeLog = findLog(logs, "token_exchange_missing_code"); expect(codeLog).toBeDefined(); expect(codeLog!.level).toBe("warn"); - expect(codeLog!.entry.body_type).toBe("object"); expect(codeLog!.entry.has_code).toBe(false); }); @@ -827,12 +826,22 @@ describe("Worker OAuth endpoint", () => { const req = makeTunnelRequest("not an envelope"); const res = await worker.fetch(req, makeEnv()); expect(res.status).toBe(400); + + const logs = collectLogs(consoleSpy); + const log = findLog(logs, "sentry_tunnel_invalid_envelope"); + expect(log).toBeDefined(); + expect(log!.level).toBe("warn"); }); it("returns 400 for invalid JSON in envelope header", async () => { const req = makeTunnelRequest("{invalid json\n{}"); const res = await worker.fetch(req, makeEnv()); expect(res.status).toBe(400); + + const logs = collectLogs(consoleSpy); + const log = findLog(logs, "sentry_tunnel_header_parse_failed"); + expect(log).toBeDefined(); + expect(log!.level).toBe("warn"); }); it("returns 200 for client_report envelopes without DSN", async () => { @@ -840,6 +849,11 @@ describe("Worker OAuth endpoint", () => { const req = makeTunnelRequest(envelope); const res = await worker.fetch(req, makeEnv()); expect(res.status).toBe(200); + + const logs = collectLogs(consoleSpy); + const log = findLog(logs, "sentry_tunnel_no_dsn"); + expect(log).toBeDefined(); + expect(log!.level).toBe("info"); }); it("returns 400 for invalid DSN URL", async () => { @@ -847,6 +861,11 @@ describe("Worker OAuth endpoint", () => { const req = makeTunnelRequest(envelope); const res = await worker.fetch(req, makeEnv()); expect(res.status).toBe(400); + + const logs = collectLogs(consoleSpy); + const log = findLog(logs, "sentry_tunnel_invalid_dsn"); + expect(log).toBeDefined(); + expect(log!.level).toBe("warn"); }); it("returns 404 when SENTRY_DSN is not configured", async () => { @@ -900,5 +919,61 @@ describe("Worker OAuth endpoint", () => { expect(allLogText).not.toContain("user@example.com"); expect(allLogText).not.toContain(sensitivePayload); }); + + it("rejects OPTIONS with 405", async () => { + const req = new Request("https://gh.gordoncode.dev/api/error-reporting", { + method: "OPTIONS", + }); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(405); + expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff"); + }); + + it("returns 413 when body exceeds size limit", async () => { + const oversizedBody = "x".repeat(256 * 1024 + 1); + const req = makeTunnelRequest(oversizedBody); + const res = await worker.fetch(req, makeEnv()); + expect(res.status).toBe(413); + + const logs = collectLogs(consoleSpy); + const sizeLog = findLog(logs, "sentry_tunnel_payload_too_large"); + expect(sizeLog).toBeDefined(); + expect(sizeLog!.level).toBe("warn"); + expect(sizeLog!.entry.body_length).toBe(256 * 1024 + 1); + }); + + it("allows body at exactly the size limit", async () => { + // Build a valid envelope that is exactly at the limit + const header = JSON.stringify({ dsn: VALID_DSN }); + const padding = "x".repeat(256 * 1024 - header.length - 1); // -1 for newline + const body = `${header}\n${padding}`; + expect(body.length).toBe(256 * 1024); + + globalThis.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + const req = makeTunnelRequest(body); + const res = await worker.fetch(req, makeEnv()); + // Should not be 413 — the body is within limits + expect(res.status).not.toBe(413); + }); + + it("logs cors_origin_mismatch for tunnel requests with wrong origin", async () => { + globalThis.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + + const req = new Request("https://gh.gordoncode.dev/api/error-reporting", { + method: "POST", + headers: { + "Content-Type": "application/x-sentry-envelope", + "Origin": "https://evil.example.com", + }, + body: makeEnvelope(VALID_DSN), + }); + await worker.fetch(req, makeEnv()); + + const logs = collectLogs(consoleSpy); + const corsLog = findLog(logs, "cors_origin_mismatch"); + expect(corsLog).toBeDefined(); + expect(corsLog!.level).toBe("warn"); + expect(corsLog!.entry.request_origin).toBe("https://evil.example.com"); + }); }); }); diff --git a/wrangler.toml b/wrangler.toml index c82cf10..e2a74cf 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -14,9 +14,6 @@ not_found_handling = "single-page-application" pattern = "gh.gordoncode.dev" custom_domain = true -# Sentry tunnel — DSN is a public identifier, not a secret. -# Set after creating a Sentry project. -# Example: SENTRY_DSN = "https://key@o123456.ingest.sentry.io/7890123" [vars] SENTRY_DSN = "https://4dc4335a9746201c02ff2107c0d20f73@o284235.ingest.us.sentry.io/4511122822922240" From d6161322b5fd9c5a11fc2f4efeef012013028b7a Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 13:54:37 -0400 Subject: [PATCH 07/10] fix(ci): moves WAF smoke tests from gitignored hack/ to tracked scripts/ --- .github/workflows/ci.yml | 2 + package.json | 3 +- prek.toml | 9 ++++ scripts/waf-smoke-test.sh | 103 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 1 deletion(-) create mode 100755 scripts/waf-smoke-test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab287a4..b9ea4bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,8 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm run typecheck - run: pnpm test + - name: WAF smoke tests + run: pnpm test:waf - name: Verify CSP hash run: node scripts/verify-csp-hash.mjs - name: Install Playwright browsers diff --git a/package.json b/package.json index 35e4b75..585f87f 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,8 @@ "test:watch": "vitest --config vitest.workspace.ts", "deploy": "wrangler deploy", "typecheck": "tsc --noEmit", - "test:e2e": "E2E_PORT=$(node -e \"const s=require('net').createServer();s.listen(0,()=>{console.log(s.address().port);s.close()})\") playwright test" + "test:e2e": "E2E_PORT=$(node -e \"const s=require('net').createServer();s.listen(0,()=>{console.log(s.address().port);s.close()})\") playwright test", + "test:waf": "bash scripts/waf-smoke-test.sh" }, "dependencies": { "@kobalte/core": "^0.13.11", diff --git a/prek.toml b/prek.toml index 3416b9e..d368da6 100644 --- a/prek.toml +++ b/prek.toml @@ -40,6 +40,15 @@ pass_filenames = false always_run = true priority = 0 +[[repos.hooks]] +id = "waf" +name = "WAF smoke tests" +language = "system" +entry = "pnpm test:waf" +pass_filenames = false +always_run = true +priority = 0 + [[repos.hooks]] id = "e2e" name = "Playwright E2E tests" diff --git a/scripts/waf-smoke-test.sh b/scripts/waf-smoke-test.sh new file mode 100755 index 0000000..7017caf --- /dev/null +++ b/scripts/waf-smoke-test.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# WAF Smoke Tests — validates Cloudflare WAF rules for gh.gordoncode.dev +# +# Usage: pnpm test:waf +# +# Rules validated: +# 1. Path Allowlist — blocks all paths except known SPA routes, /assets/*, /api/* +# 2. Scanner User-Agents — challenges empty/malicious User-Agent strings +# Rate limit rule exists but is not tested here (triggers a 10-minute IP block). + +set -euo pipefail + +BASE="https://gh.gordoncode.dev" +PASS=0 +FAIL=0 + +assert_status() { + local expected="$1" actual="$2" label="$3" + if [[ "$actual" == "$expected" ]]; then + echo " PASS [${actual}] ${label}" + PASS=$((PASS + 1)) + else + echo " FAIL [${actual}] ${label} (expected ${expected})" + FAIL=$((FAIL + 1)) + fi +} + +fetch() { + curl -s -o /dev/null -w "%{http_code}" "$@" +} + +# ============================================================ +# Rule 1: Path Allowlist +# ============================================================ +echo "=== Rule 1: Path Allowlist ===" +echo "--- Allowed paths (should pass) ---" + +for path in "/" "/login" "/oauth/callback" "/onboarding" "/dashboard" "/settings" "/privacy"; do + status=$(fetch "${BASE}${path}") + assert_status "200" "$status" "GET ${path}" +done + +status=$(fetch "${BASE}/index.html") +assert_status "307" "$status" "GET /index.html (html_handling redirect)" + +status=$(fetch "${BASE}/assets/nonexistent.js") +assert_status "200" "$status" "GET /assets/nonexistent.js" + +status=$(fetch "${BASE}/api/health") +assert_status "200" "$status" "GET /api/health" + +status=$(fetch -X POST "${BASE}/api/oauth/token") +assert_status "400" "$status" "POST /api/oauth/token (no body)" + +status=$(fetch "${BASE}/api/nonexistent") +assert_status "404" "$status" "GET /api/nonexistent" + +echo "--- Blocked paths (should be 403) ---" + +for path in "/wp-admin" "/wp-login.php" "/.env" "/.env.production" \ + "/.git/config" "/.git/HEAD" "/xmlrpc.php" \ + "/phpmyadmin/" "/phpMyAdmin/" "/.htaccess" "/.htpasswd" \ + "/cgi-bin/" "/admin/" "/wp-content/debug.log" \ + "/config.php" "/backup.zip" "/actuator/health" \ + "/manager/html" "/wp-config.php" "/eval-stdin.php" \ + "/.aws/credentials" "/.ssh/id_rsa" "/robots.txt" \ + "/sitemap.xml" "/favicon.ico" "/random/garbage/path"; do + status=$(fetch "${BASE}${path}") + assert_status "403" "$status" "GET ${path}" +done + +# ============================================================ +# Rule 2: Scanner User-Agents +# ============================================================ +echo "" +echo "=== Rule 2: Scanner User-Agents ===" +echo "--- Normal UAs (should pass) ---" + +status=$(fetch -H "User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36" "${BASE}/") +assert_status "200" "$status" "Normal browser UA" + +status=$(fetch "${BASE}/") +assert_status "200" "$status" "Default curl UA" + +echo "--- Malicious UAs (should be 403 — managed challenge, no JS) ---" + +status=$(fetch -H "User-Agent:" "${BASE}/") +assert_status "403" "$status" "Empty User-Agent" + +for ua in "sqlmap/1.7" "Nikto/2.1.6" "Nmap Scripting Engine" "masscan/1.3" "Mozilla/5.0 zgrab/0.x"; do + status=$(fetch -H "User-Agent: ${ua}" "${BASE}/") + assert_status "403" "$status" "UA: ${ua}" +done + +# ============================================================ +# Summary +# ============================================================ +echo "" +TOTAL=$((PASS + FAIL)) +echo "=== Results: ${PASS}/${TOTAL} passed, ${FAIL} failed ===" +if [[ $FAIL -gt 0 ]]; then + exit 1 +fi From 27cdba40c477f17076ac832df4bf8cd19e5ead82 Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 14:01:47 -0400 Subject: [PATCH 08/10] fix(ci): removes WAF smoke tests from CI (Cloudflare blocks runner IPs) --- .github/workflows/ci.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9ea4bf..ab287a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,8 +16,6 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm run typecheck - run: pnpm test - - name: WAF smoke tests - run: pnpm test:waf - name: Verify CSP hash run: node scripts/verify-csp-hash.mjs - name: Install Playwright browsers From 848325e0046ac8b5d216a0a831d95ea50121c03f Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 14:06:13 -0400 Subject: [PATCH 09/10] feat(ci): adds WAF bypass header for CI smoke tests --- .github/workflows/ci.yml | 4 ++++ scripts/waf-smoke-test.sh | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab287a4..0f40a7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,10 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm run typecheck - run: pnpm test + - name: WAF smoke tests + run: pnpm test:waf + env: + WAF_BYPASS_TOKEN: ${{ secrets.WAF_BYPASS_TOKEN }} - name: Verify CSP hash run: node scripts/verify-csp-hash.mjs - name: Install Playwright browsers diff --git a/scripts/waf-smoke-test.sh b/scripts/waf-smoke-test.sh index 7017caf..a23d05b 100755 --- a/scripts/waf-smoke-test.sh +++ b/scripts/waf-smoke-test.sh @@ -14,6 +14,14 @@ BASE="https://gh.gordoncode.dev" PASS=0 FAIL=0 +# When WAF_BYPASS_TOKEN is set (CI), send a header that a Cloudflare WAF rule +# uses to skip Bot Fight Mode for this request. Without it (local dev), requests +# pass through normally since residential IPs aren't challenged. +BYPASS=() +if [[ -n "${WAF_BYPASS_TOKEN:-}" ]]; then + BYPASS=(-H "X-CI-Bypass: ${WAF_BYPASS_TOKEN}") +fi + assert_status() { local expected="$1" actual="$2" label="$3" if [[ "$actual" == "$expected" ]]; then @@ -26,7 +34,7 @@ assert_status() { } fetch() { - curl -s -o /dev/null -w "%{http_code}" "$@" + curl -s -o /dev/null -w "%{http_code}" "${BYPASS[@]}" "$@" } # ============================================================ From 2c6804242a230f88da1304501fe012a4615bd801 Mon Sep 17 00:00:00 2001 From: testvalue Date: Sat, 28 Mar 2026 14:17:14 -0400 Subject: [PATCH 10/10] fix(ci): moves WAF smoke tests to deploy workflow --- .github/workflows/ci.yml | 4 ---- .github/workflows/deploy.yml | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0f40a7a..ab287a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,10 +16,6 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm run typecheck - run: pnpm test - - name: WAF smoke tests - run: pnpm test:waf - env: - WAF_BYPASS_TOKEN: ${{ secrets.WAF_BYPASS_TOKEN }} - name: Verify CSP hash run: node scripts/verify-csp-hash.mjs - name: Install Playwright browsers diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index e18ee99..d877e5c 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -20,6 +20,10 @@ jobs: - run: pnpm test - name: Verify CSP hash run: node scripts/verify-csp-hash.mjs + - name: WAF smoke tests + run: pnpm test:waf + env: + WAF_BYPASS_TOKEN: ${{ secrets.WAF_BYPASS_TOKEN }} - run: pnpm run build env: VITE_GITHUB_CLIENT_ID: ${{ vars.VITE_GITHUB_CLIENT_ID }}