From b79f78d1b01615628f21a23ef45dcfbce69ed487 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Nov 2025 17:47:11 +0000 Subject: [PATCH 01/15] Add form-csrf task command for handleForm and withCSRF middleware Creates task documentation for implementing: - handleForm: Factory for secure form handling with JSON Schema validation - withCSRF: Stateless CSRF protection using double-submit cookie pattern --- ai/commands/form-csrf.md | 118 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 ai/commands/form-csrf.md diff --git a/ai/commands/form-csrf.md b/ai/commands/form-csrf.md new file mode 100644 index 00000000..3b53396b --- /dev/null +++ b/ai/commands/form-csrf.md @@ -0,0 +1,118 @@ +## ✅ Form Handling & CSRF Middleware + +Implement secure form submission handling with JSON Schema validation and CSRF protection middleware. + +Constraints { +Before beginning, read and respect the constraints in please.mdc. +Remember to use the TDD process when implementing code. +Follow the existing middleware patterns in src/server/middleware/. +Use @paralleldrive/cuid2 for token generation. +} + +--- + +# Epic: handleForm & withCSRF Middleware + +## Files to Create + +- `src/server/middleware/handle-form.js` - Form handling factory +- `src/server/middleware/handle-form.test.js` - Tests for handleForm +- `src/server/middleware/with-csrf.js` - CSRF protection middleware +- `src/server/middleware/with-csrf.test.js` - Tests for withCSRF +- Update `src/server/middleware/index.js` - Export new middleware + +--- + +## Task 1: handleForm + +### Overview + +Factory function that creates middleware for secure form submission handling with JSON Schema validation. + +### Signature + +```javascript +handleForm({ name, schema, processSubmission, pii, honeypotField }) +``` + +### Parameters + +- `name` (string) - Identifier for the form, used in logging +- `schema` (object) - JSON Schema for validating request body +- `processSubmission` (function) - Async function receiving validated form data as `{ [fieldName]: value }` +- `pii` (string[]) - Field names to register with logger scrubber +- `honeypotField` (string, optional) - Field name that must be empty, rejects submission if filled + +### Functional Requirements + +1. Given a request with a body matching the JSON Schema, should pass validated fields to `processSubmission` +2. Given a request with a body failing JSON Schema validation, should return 400 status with an array of validation failure descriptions +3. Given a request where the honeypot field contains a value, should reject with 400 status and generic validation error (no indication of honeypot detection) +4. Given a request missing required fields per schema, should return 400 status with validation failures indicating missing fields +5. Given `processSubmission` throws an error, should surface error through standard `createRoute` error handling +6. Given a successful submission, should return `{ request, response }` without setting status or body (caller handles success response) +7. Given PII fields, should pass them to `request.locals.logger.scrub(pii)` +8. Given a request with fields not defined in schema, should return 400 status with validation failures indicating undeclared fields +9. Given the `honeypotField` parameter is omitted, should skip honeypot validation + +### Implementation Notes + +- Use a JSON Schema validation library (suggest ajv) +- Configure ajv with `additionalProperties: false` behavior for requirement 8 +- Honeypot rejection should appear identical to validation errors (security by obscurity) +- Ensure validation errors are descriptive but don't leak sensitive schema details + +--- + +## Task 2: withCSRF + +### Overview + +Middleware providing stateless CSRF protection using the double-submit cookie pattern. + +### Signature + +```javascript +withCSRF // Direct export, no parameters +``` + +### Functional Requirements + +1. Given a GET/HEAD/OPTIONS request, should set a CSRF token cookie and attach token to `response.locals.csrfToken` for inclusion in forms +2. Given a POST/PUT/PATCH/DELETE request with matching token in cookie and header (`X-CSRF-Token`), should allow request to proceed +3. Given a POST/PUT/PATCH/DELETE request with matching token in cookie and body field (`_csrf`), should allow request to proceed +4. Given a POST/PUT/PATCH/DELETE request where token is missing from cookie, should return 403 status with error message +5. Given a POST/PUT/PATCH/DELETE request where submitted token does not match cookie token, should return 403 status with error message +6. Given a request, should generate tokens using CUID2 +7. Given setting the CSRF cookie, should set `SameSite=Strict` and `Secure=true` (in production) +8. Given setting the CSRF cookie, should not set `HttpOnly` (client must read token to submit it) +9. Given any CSRF rejection, should log the failure with request ID but without exposing token values + +### Implementation Notes + +- Use `@paralleldrive/cuid2` for token generation (already in dependencies) +- Cookie name suggestion: `csrf_token` +- Detect production via `process.env.NODE_ENV === 'production'` +- Safe methods (GET/HEAD/OPTIONS) should still set the cookie for subsequent unsafe requests +- Middleware signature: `async ({ request, response }) => { ... }` + +--- + +## Testing Approach + +Follow the existing test patterns in `src/server/middleware/*.test.js`: +- Use vitest with riteway assertions +- Use `createServer` from `../test-utils.js` for mock objects +- Extend `createServer` as needed for cookie/body mocking +- Test each functional requirement explicitly +- Include edge cases (empty strings, missing fields, malformed data) + +--- + +## Export Updates + +Add to `src/server/middleware/index.js`: +```javascript +export { handleForm } from "./handle-form.js"; +export { withCSRF } from "./with-csrf.js"; +``` From c2feb3a77549e419a7863ba3d893b5ac18ca9e70 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Nov 2025 18:09:06 +0000 Subject: [PATCH 02/15] Move form-csrf task to correct tasks/ folder Task files belong in tasks/, not ai/commands/ --- {ai/commands => tasks}/form-csrf.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {ai/commands => tasks}/form-csrf.md (100%) diff --git a/ai/commands/form-csrf.md b/tasks/form-csrf.md similarity index 100% rename from ai/commands/form-csrf.md rename to tasks/form-csrf.md From fe29a7838326cd8eb06bd2f02ba1dca0dc6cd4ba Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Nov 2025 19:04:43 +0000 Subject: [PATCH 03/15] Add SHA3 hash comparison requirement for CSRF token validation Prevents timing attacks by comparing hashes instead of raw tokens --- tasks/form-csrf.md | 1 + 1 file changed, 1 insertion(+) diff --git a/tasks/form-csrf.md b/tasks/form-csrf.md index 3b53396b..5af777be 100644 --- a/tasks/form-csrf.md +++ b/tasks/form-csrf.md @@ -87,6 +87,7 @@ withCSRF // Direct export, no parameters 7. Given setting the CSRF cookie, should set `SameSite=Strict` and `Secure=true` (in production) 8. Given setting the CSRF cookie, should not set `HttpOnly` (client must read token to submit it) 9. Given any CSRF rejection, should log the failure with request ID but without exposing token values +10. Given token comparison, should compare SHA3 hashes of tokens to prevent timing attacks ### Implementation Notes From 2edb0fce6b77f2eadf70f5bb9ffcc5872642aa1c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Nov 2025 19:46:43 +0000 Subject: [PATCH 04/15] Implement handleForm and withCSRF middleware handleForm: Factory for secure form handling with JSON Schema validation - Validates request body against JSON Schema (ajv) - Honeypot field detection for bot protection - PII field scrubbing via logger - Rejects undeclared fields (additionalProperties: false) - Surfaces processSubmission errors through createRoute withCSRF: Stateless CSRF protection using double-submit cookie pattern - Sets token cookie with SameSite=Strict, Secure (in production) - Validates token from X-CSRF-Token header or _csrf body field - Uses SHA3 hash comparison to prevent timing attacks - Logs rejections without exposing token values Dependencies: ajv, js-sha3 --- package-lock.json | 344 +++++------------ package.json | 4 +- src/server/middleware/handle-form.js | 62 ++++ src/server/middleware/handle-form.test.js | 426 ++++++++++++++++++++++ src/server/middleware/index.js | 2 + src/server/middleware/with-csrf.js | 75 ++++ src/server/middleware/with-csrf.test.js | 315 ++++++++++++++++ 7 files changed, 978 insertions(+), 250 deletions(-) create mode 100644 src/server/middleware/handle-form.js create mode 100644 src/server/middleware/handle-form.test.js create mode 100644 src/server/middleware/with-csrf.js create mode 100644 src/server/middleware/with-csrf.test.js diff --git a/package-lock.json b/package-lock.json index d07bc3bf..d085524c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,10 +10,12 @@ "license": "MIT", "dependencies": { "@paralleldrive/cuid2": "^3.1.0", + "ajv": "^8.17.1", "chalk": "^4.1.2", "commander": "^11.1.0", "error-causes": "^3.0.2", - "fs-extra": "^11.1.1" + "fs-extra": "^11.1.1", + "js-sha3": "^0.9.3" }, "bin": { "aidd": "bin/aidd.js" @@ -115,54 +117,6 @@ "node": ">=18" } }, - "node_modules/@better-auth/core": { - "version": "1.4.5", - "resolved": "https://registry.npmjs.org/@better-auth/core/-/core-1.4.5.tgz", - "integrity": "sha512-dQ3hZOkUJzeBXfVEPTm2LVbzmWwka1nqd9KyWmB2OMlMfjr7IdUeBX4T7qJctF67d7QDhlX95jMoxu6JG0Eucw==", - "optional": true, - "peer": true, - "dependencies": { - "@standard-schema/spec": "^1.0.0", - "zod": "^4.1.12" - }, - "peerDependencies": { - "@better-auth/utils": "0.3.0", - "@better-fetch/fetch": "1.1.18", - "better-call": "1.1.4", - "jose": "^6.1.0", - "kysely": "^0.28.5", - "nanostores": "^1.0.1" - } - }, - "node_modules/@better-auth/telemetry": { - "version": "1.4.5", - "resolved": "https://registry.npmjs.org/@better-auth/telemetry/-/telemetry-1.4.5.tgz", - "integrity": "sha512-r3NyksbaBYA10SC86JA6QwmZfHwFutkUGcphgWGfu6MVx1zutYmZehIeC8LxTjOWZqqF9FI8vLjglWBHvPQeTg==", - "optional": true, - "peer": true, - "dependencies": { - "@better-auth/utils": "0.3.0", - "@better-fetch/fetch": "1.1.18" - }, - "peerDependencies": { - "@better-auth/core": "1.4.5" - } - }, - "node_modules/@better-auth/utils": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/@better-auth/utils/-/utils-0.3.0.tgz", - "integrity": "sha512-W+Adw6ZA6mgvnSnhOki270rwJ42t4XzSK6YWGF//BbVXL6SwCLWfyzBc1lN2m/4RM28KubdBKQ4X5VMoLRNPQw==", - "license": "MIT", - "optional": true, - "peer": true - }, - "node_modules/@better-fetch/fetch": { - "version": "1.1.18", - "resolved": "https://registry.npmjs.org/@better-fetch/fetch/-/fetch-1.1.18.tgz", - "integrity": "sha512-rEFOE1MYIsBmoMJtQbl32PGHHXuG2hDxvEd7rUHE0vCBoFQVSDqaVs9hkZEtHCxRoY+CljXKFCOuJ8uxqw1LcA==", - "optional": true, - "peer": true - }, "node_modules/@esbuild/aix-ppc64": { "version": "0.25.10", "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.25.10.tgz", @@ -709,6 +663,30 @@ "url": "https://opencollective.com/eslint" } }, + "node_modules/@eslint/eslintrc/node_modules/ajv": { + "version": "6.12.6", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", + "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", + "dev": true, + "license": "MIT", + "dependencies": { + "fast-deep-equal": "^3.1.1", + "fast-json-stable-stringify": "^2.0.0", + "json-schema-traverse": "^0.4.1", + "uri-js": "^4.2.2" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/epoberezkin" + } + }, + "node_modules/@eslint/eslintrc/node_modules/json-schema-traverse": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz", + "integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==", + "dev": true, + "license": "MIT" + }, "node_modules/@eslint/js": { "version": "9.36.0", "resolved": "https://registry.npmjs.org/@eslint/js/-/js-9.36.0.tgz", @@ -1319,20 +1297,6 @@ "node": ">= 0.4" } }, - "node_modules/@noble/ciphers": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/@noble/ciphers/-/ciphers-2.0.1.tgz", - "integrity": "sha512-xHK3XHPUW8DTAobU+G0XT+/w+JLM7/8k1UFdB5xg/zTFPnFCobhftzw8wl4Lw2aq/Rvir5pxfZV5fEazmeCJ2g==", - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": ">= 20.19.0" - }, - "funding": { - "url": "https://paulmillr.com/funding/" - } - }, "node_modules/@noble/hashes": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/@noble/hashes/-/hashes-2.0.1.tgz", @@ -1966,14 +1930,6 @@ "win32" ] }, - "node_modules/@standard-schema/spec": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/@standard-schema/spec/-/spec-1.0.0.tgz", - "integrity": "sha512-m2bOd0f2RT9k8QJx1JN85cZYyH1RqFBdlwtkSlf4tBDYLCiiZnv1fIIwacK6cqwXavOydf0NPToMQgpKq+dVlA==", - "license": "MIT", - "optional": true, - "peer": true - }, "node_modules/@textlint/ast-node-types": { "version": "12.6.1", "resolved": "https://registry.npmjs.org/@textlint/ast-node-types/-/ast-node-types-12.6.1.tgz", @@ -2244,16 +2200,15 @@ } }, "node_modules/ajv": { - "version": "6.12.6", - "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", - "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", - "dev": true, + "version": "8.17.1", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.17.1.tgz", + "integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==", "license": "MIT", "dependencies": { - "fast-deep-equal": "^3.1.1", - "fast-json-stable-stringify": "^2.0.0", - "json-schema-traverse": "^0.4.1", - "uri-js": "^4.2.2" + "fast-deep-equal": "^3.1.3", + "fast-uri": "^3.0.1", + "json-schema-traverse": "^1.0.0", + "require-from-string": "^2.0.2" }, "funding": { "type": "github", @@ -2477,102 +2432,6 @@ "dev": true, "license": "Apache-2.0" }, - "node_modules/better-auth": { - "version": "1.4.5", - "resolved": "https://registry.npmjs.org/better-auth/-/better-auth-1.4.5.tgz", - "integrity": "sha512-pHV2YE0OogRHvoA6pndHXCei4pcep/mjY7psSaHVrRgjBtumVI68SV1g9U9XPRZ4KkoGca9jfwuv+bB2UILiFw==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@better-auth/core": "1.4.5", - "@better-auth/telemetry": "1.4.5", - "@better-auth/utils": "0.3.0", - "@better-fetch/fetch": "1.1.18", - "@noble/ciphers": "^2.0.0", - "@noble/hashes": "^2.0.0", - "better-call": "1.1.4", - "defu": "^6.1.4", - "jose": "^6.1.0", - "kysely": "^0.28.5", - "ms": "4.0.0-nightly.202508271359", - "nanostores": "^1.0.1", - "zod": "^4.1.12" - }, - "peerDependencies": { - "@lynx-js/react": "*", - "@sveltejs/kit": "^2.0.0", - "@tanstack/react-start": "^1.0.0", - "next": "^14.0.0 || ^15.0.0 || ^16.0.0", - "react": "^18.0.0 || ^19.0.0", - "react-dom": "^18.0.0 || ^19.0.0", - "solid-js": "^1.0.0", - "svelte": "^4.0.0 || ^5.0.0", - "vue": "^3.0.0" - }, - "peerDependenciesMeta": { - "@lynx-js/react": { - "optional": true - }, - "@sveltejs/kit": { - "optional": true - }, - "@tanstack/react-start": { - "optional": true - }, - "next": { - "optional": true - }, - "react": { - "optional": true - }, - "react-dom": { - "optional": true - }, - "solid-js": { - "optional": true - }, - "svelte": { - "optional": true - }, - "vue": { - "optional": true - } - } - }, - "node_modules/better-auth/node_modules/ms": { - "version": "4.0.0-nightly.202508271359", - "resolved": "https://registry.npmjs.org/ms/-/ms-4.0.0-nightly.202508271359.tgz", - "integrity": "sha512-WC/Eo7NzFrOV/RRrTaI0fxKVbNCzEy76j2VqNV8SxDf9D69gSE2Lh0QwYvDlhiYmheBYExAvEAxVf5NoN0cj2A==", - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": ">=20" - } - }, - "node_modules/better-call": { - "version": "1.1.4", - "resolved": "https://registry.npmjs.org/better-call/-/better-call-1.1.4.tgz", - "integrity": "sha512-NJouLY6IVKv0nDuFoc6FcbKDFzEnmgMNofC9F60Mwx1Ecm7X6/Ecyoe5b+JSVZ42F/0n46/M89gbYP1ZCVv8xQ==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@better-auth/utils": "^0.3.0", - "@better-fetch/fetch": "^1.1.4", - "rou3": "^0.7.10", - "set-cookie-parser": "^2.7.1" - }, - "peerDependencies": { - "zod": "^4.0.0" - }, - "peerDependenciesMeta": { - "zod": { - "optional": true - } - } - }, "node_modules/bignumber.js": { "version": "9.3.1", "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.3.1.tgz", @@ -3255,7 +3114,7 @@ "version": "6.1.4", "resolved": "https://registry.npmjs.org/defu/-/defu-6.1.4.tgz", "integrity": "sha512-mEQCMmwJu317oSz8CwdIOdwf3xMif1ttiM8LTufzc3g6kR+9Pe236twL8j3IYT1F7GfRgGcW6MWxzZjLIkuHIg==", - "devOptional": true, + "dev": true, "license": "MIT" }, "node_modules/degenerator": { @@ -3913,6 +3772,30 @@ "url": "https://opencollective.com/eslint" } }, + "node_modules/eslint/node_modules/ajv": { + "version": "6.12.6", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", + "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", + "dev": true, + "license": "MIT", + "dependencies": { + "fast-deep-equal": "^3.1.1", + "fast-json-stable-stringify": "^2.0.0", + "json-schema-traverse": "^0.4.1", + "uri-js": "^4.2.2" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/epoberezkin" + } + }, + "node_modules/eslint/node_modules/json-schema-traverse": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz", + "integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==", + "dev": true, + "license": "MIT" + }, "node_modules/esm": { "version": "3.2.25", "resolved": "https://registry.npmjs.org/esm/-/esm-3.2.25.tgz", @@ -4109,7 +3992,6 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", - "dev": true, "license": "MIT" }, "node_modules/fast-diff": { @@ -4133,6 +4015,22 @@ "dev": true, "license": "MIT" }, + "node_modules/fast-uri": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.0.tgz", + "integrity": "sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/fastify" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fastify" + } + ], + "license": "BSD-3-Clause" + }, "node_modules/fault": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/fault/-/fault-1.0.4.tgz", @@ -5593,16 +5491,11 @@ "jiti": "lib/jiti-cli.mjs" } }, - "node_modules/jose": { - "version": "6.1.3", - "resolved": "https://registry.npmjs.org/jose/-/jose-6.1.3.tgz", - "integrity": "sha512-0TpaTfihd4QMNwrz/ob2Bp7X04yuxJkjRGi4aKmOqwhov54i6u79oCv7T+C7lo70MKH6BesI3vscD1yb/yzKXQ==", - "license": "MIT", - "optional": true, - "peer": true, - "funding": { - "url": "https://github.com/sponsors/panva" - } + "node_modules/js-sha3": { + "version": "0.9.3", + "resolved": "https://registry.npmjs.org/js-sha3/-/js-sha3-0.9.3.tgz", + "integrity": "sha512-BcJPCQeLg6WjEx3FE591wVAevlli8lxsxm9/FzV4HXkV49TmBH38Yvrpce6fjbADGMKFrBMGTqrVz3qPIZ88Gg==", + "license": "MIT" }, "node_modules/js-tokens": { "version": "9.0.1", @@ -5632,10 +5525,9 @@ "license": "MIT" }, "node_modules/json-schema-traverse": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz", - "integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==", - "dev": true, + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", + "integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==", "license": "MIT" }, "node_modules/json-stable-stringify-without-jsonify": { @@ -5667,17 +5559,6 @@ "json-buffer": "3.0.1" } }, - "node_modules/kysely": { - "version": "0.28.8", - "resolved": "https://registry.npmjs.org/kysely/-/kysely-0.28.8.tgz", - "integrity": "sha512-QUOgl5ZrS9IRuhq5FvOKFSsD/3+IA6MLE81/bOOTRA/YQpKDza2sFdN5g6JCB9BOpqMJDGefLCQ9F12hRS13TA==", - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": ">=20.0.0" - } - }, "node_modules/levn": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/levn/-/levn-0.4.1.tgz", @@ -6326,23 +6207,6 @@ "node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1" } }, - "node_modules/nanostores": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/nanostores/-/nanostores-1.1.0.tgz", - "integrity": "sha512-yJBmDJr18xy47dbNVlHcgdPrulSn1nhSE6Ns9vTG+Nx9VPT6iV1MD6aQFp/t52zpf82FhLLTXAXr30NuCnxvwA==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/ai" - } - ], - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": "^20.0.0 || >=22.0.0" - } - }, "node_modules/natural-compare": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/natural-compare/-/natural-compare-1.4.0.tgz", @@ -7111,7 +6975,7 @@ "version": "19.1.1", "resolved": "https://registry.npmjs.org/react/-/react-19.1.1.tgz", "integrity": "sha512-w8nqGImo45dmMIfljjMwOGtbmC/mk4CMYhWIicdSflH91J9TyCyczcPFXJzrZ/ZXcgGRFeP6BU0BEJTw6tZdfQ==", - "devOptional": true, + "dev": true, "license": "MIT", "peer": true, "engines": { @@ -7122,7 +6986,7 @@ "version": "19.1.1", "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.1.1.tgz", "integrity": "sha512-Dlq/5LAZgF0Gaz6yiqZCf6VCcZs1ghAJyrsu84Q/GT0gV+mCxbfmKNoGRKBYMJ8IEdGPqu49YWXD02GCknEDkw==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "scheduler": "^0.26.0" @@ -7316,6 +7180,15 @@ "node": ">=0.10" } }, + "node_modules/require-from-string": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz", + "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/resolve": { "version": "2.0.0-next.5", "resolved": "https://registry.npmjs.org/resolve/-/resolve-2.0.0-next.5.tgz", @@ -7430,14 +7303,6 @@ "fsevents": "~2.3.2" } }, - "node_modules/rou3": { - "version": "0.7.10", - "resolved": "https://registry.npmjs.org/rou3/-/rou3-0.7.10.tgz", - "integrity": "sha512-aoFj6f7MJZ5muJ+Of79nrhs9N3oLGqi2VEMe94Zbkjb6Wupha46EuoYgpWSOZlXww3bbd8ojgXTAA2mzimX5Ww==", - "license": "MIT", - "optional": true, - "peer": true - }, "node_modules/run-applescript": { "version": "7.1.0", "resolved": "https://registry.npmjs.org/run-applescript/-/run-applescript-7.1.0.tgz", @@ -7537,7 +7402,7 @@ "version": "0.26.0", "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.26.0.tgz", "integrity": "sha512-NlHwttCI/l5gCPR3D1nNXtWABUmBwvZpEQiD4IXSbIDq8BzLIK/7Ir5gTFSGZDUu37K5cMNp0hFtzO38sC7gWA==", - "devOptional": true, + "dev": true, "license": "MIT" }, "node_modules/semver": { @@ -7553,14 +7418,6 @@ "node": ">=10" } }, - "node_modules/set-cookie-parser": { - "version": "2.7.2", - "resolved": "https://registry.npmjs.org/set-cookie-parser/-/set-cookie-parser-2.7.2.tgz", - "integrity": "sha512-oeM1lpU/UvhTxw+g3cIfxXHyJRc/uidd3yK1P242gzHds0udQBYzs3y8j4gCCW+ZJ7ad0yctld8RYO+bdurlvw==", - "license": "MIT", - "optional": true, - "peer": true - }, "node_modules/set-function-length": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/set-function-length/-/set-function-length-1.2.2.tgz", @@ -9098,17 +8955,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/zod": { - "version": "4.1.13", - "resolved": "https://registry.npmjs.org/zod/-/zod-4.1.13.tgz", - "integrity": "sha512-AvvthqfqrAhNH9dnfmrfKzX5upOdjUVJYFqNSlkmGf64gRaTzlPwz99IHYnVs28qYAybvAlBV+H7pn0saFY4Ig==", - "license": "MIT", - "optional": true, - "peer": true, - "funding": { - "url": "https://github.com/sponsors/colinhacks" - } - }, "node_modules/zwitch": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/zwitch/-/zwitch-1.0.5.tgz", diff --git a/package.json b/package.json index 6681240c..27a05447 100644 --- a/package.json +++ b/package.json @@ -66,10 +66,12 @@ "homepage": "https://github.com/paralleldrive/aidd#readme", "dependencies": { "@paralleldrive/cuid2": "^3.1.0", + "ajv": "^8.17.1", "chalk": "^4.1.2", "commander": "^11.1.0", "error-causes": "^3.0.2", - "fs-extra": "^11.1.1" + "fs-extra": "^11.1.1", + "js-sha3": "^0.9.3" }, "peerDependencies": { "better-auth": "^1.4.5" diff --git a/src/server/middleware/handle-form.js b/src/server/middleware/handle-form.js new file mode 100644 index 00000000..8712a3d2 --- /dev/null +++ b/src/server/middleware/handle-form.js @@ -0,0 +1,62 @@ +/** + * Form handling middleware factory with JSON Schema validation + */ + +import Ajv from "ajv"; + +const ajv = new Ajv({ allErrors: true }); + +const formatErrors = (errors) => { + return errors.map((err) => { + if (err.keyword === "required") { + return `Missing required field: ${err.params.missingProperty}`; + } + if (err.keyword === "additionalProperties") { + return `Undeclared field not allowed: ${err.params.additionalProperty}`; + } + if (err.keyword === "type") { + const field = err.instancePath.slice(1) || "root"; + return `Field '${field}' ${err.message}`; + } + return err.message; + }); +}; + +const handleForm = + ({ name, schema, processSubmission, pii, honeypotField }) => + async ({ request, response }) => { + // Register PII fields with logger scrubber + if (pii?.length && response.locals?.logger?.scrub) { + response.locals.logger.scrub(pii); + } + + const body = request.body || {}; + + // Check honeypot field if configured + if (honeypotField && body[honeypotField]) { + response.status(400); + response.json({ + errors: ["Validation failed"], + }); + return { request, response }; + } + + // Validate against schema + const validate = ajv.compile(schema); + const valid = validate(body); + + if (!valid) { + response.status(400); + response.json({ + errors: formatErrors(validate.errors), + }); + return { request, response }; + } + + // Process the validated submission + await processSubmission(body); + + return { request, response }; + }; + +export { handleForm }; diff --git a/src/server/middleware/handle-form.test.js b/src/server/middleware/handle-form.test.js new file mode 100644 index 00000000..17a5eab3 --- /dev/null +++ b/src/server/middleware/handle-form.test.js @@ -0,0 +1,426 @@ +import { describe, test, vi } from "vitest"; +import { assert } from "riteway/vitest"; +import { handleForm } from "./handle-form.js"; + +describe("handleForm", () => { + // Req 1: Valid body passes to processSubmission + test("passes validated fields to processSubmission when body matches schema", async () => { + const processSubmission = vi.fn().mockResolvedValue({}); + const schema = { + type: "object", + properties: { + name: { type: "string" }, + email: { type: "string" }, + }, + required: ["name", "email"], + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "contact", + schema, + processSubmission, + pii: [], + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "John", email: "john@example.com" }, + }, + response: mockResponse, + }); + + assert({ + given: "a request body matching the schema", + should: "call processSubmission with validated fields", + actual: processSubmission.mock.calls[0]?.[0], + expected: { name: "John", email: "john@example.com" }, + }); + }); + + // Req 2: Invalid body returns 400 with validation errors + test("returns 400 with validation errors when body fails schema", async () => { + const processSubmission = vi.fn(); + const schema = { + type: "object", + properties: { + name: { type: "string" }, + age: { type: "number" }, + }, + required: ["name", "age"], + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "profile", + schema, + processSubmission, + pii: [], + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "John", age: "not a number" }, + }, + response: mockResponse, + }); + + assert({ + given: "a request body failing schema validation", + should: "return 400 status", + actual: mockResponse.status.mock.calls[0]?.[0], + expected: 400, + }); + + assert({ + given: "a request body failing schema validation", + should: "return array of validation errors", + actual: Array.isArray(mockResponse.json.mock.calls[0]?.[0]?.errors), + expected: true, + }); + + assert({ + given: "a request body failing schema validation", + should: "not call processSubmission", + actual: processSubmission.mock.calls.length, + expected: 0, + }); + }); + + // Req 3: Honeypot field filled rejects with generic error + test("rejects with 400 and generic error when honeypot field is filled", async () => { + const processSubmission = vi.fn(); + const schema = { + type: "object", + properties: { + name: { type: "string" }, + website: { type: "string" }, + }, + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "signup", + schema, + processSubmission, + pii: [], + honeypotField: "website", + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "Bot", website: "http://spam.com" }, + }, + response: mockResponse, + }); + + assert({ + given: "a request with filled honeypot field", + should: "return 400 status", + actual: mockResponse.status.mock.calls[0]?.[0], + expected: 400, + }); + + assert({ + given: "a request with filled honeypot field", + should: "return generic validation error (no honeypot indication)", + actual: mockResponse.json.mock.calls[0]?.[0]?.errors?.some((e) => + e.toLowerCase().includes("honeypot"), + ), + expected: false, + }); + + assert({ + given: "a request with filled honeypot field", + should: "not call processSubmission", + actual: processSubmission.mock.calls.length, + expected: 0, + }); + }); + + // Req 4: Missing required fields returns 400 with specific errors + test("returns 400 with missing field errors when required fields absent", async () => { + const processSubmission = vi.fn(); + const schema = { + type: "object", + properties: { + name: { type: "string" }, + email: { type: "string" }, + }, + required: ["name", "email"], + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "contact", + schema, + processSubmission, + pii: [], + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "John" }, + }, + response: mockResponse, + }); + + assert({ + given: "a request missing required fields", + should: "return 400 status", + actual: mockResponse.status.mock.calls[0]?.[0], + expected: 400, + }); + + const errors = mockResponse.json.mock.calls[0]?.[0]?.errors || []; + assert({ + given: "a request missing required fields", + should: "indicate missing field in error", + actual: errors.some((e) => e.includes("email")), + expected: true, + }); + }); + + // Req 5: processSubmission error surfaces through createRoute + test("throws error when processSubmission throws", async () => { + const processSubmission = vi.fn().mockRejectedValue(new Error("DB error")); + const schema = { + type: "object", + properties: { name: { type: "string" } }, + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "test", + schema, + processSubmission, + pii: [], + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + let error; + try { + await middleware({ + request: { body: { name: "John" } }, + response: mockResponse, + }); + } catch (e) { + error = e; + } + + assert({ + given: "processSubmission throws an error", + should: "surface error for createRoute error handling", + actual: error?.message, + expected: "DB error", + }); + }); + + // Req 6: Successful submission returns { request, response } without setting status/body + test("returns request/response without setting status on success", async () => { + const processSubmission = vi.fn().mockResolvedValue({}); + const schema = { + type: "object", + properties: { name: { type: "string" } }, + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "test", + schema, + processSubmission, + pii: [], + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + const result = await middleware({ + request: { body: { name: "John" } }, + response: mockResponse, + }); + + assert({ + given: "a successful form submission", + should: "return request object", + actual: typeof result.request, + expected: "object", + }); + + assert({ + given: "a successful form submission", + should: "return response object", + actual: typeof result.response, + expected: "object", + }); + + assert({ + given: "a successful form submission", + should: "not set status (caller handles response)", + actual: mockResponse.status.mock.calls.length, + expected: 0, + }); + }); + + // Req 7: PII fields passed to logger.scrub + test("passes PII fields to logger.scrub", async () => { + const processSubmission = vi.fn().mockResolvedValue({}); + const scrubFn = vi.fn(); + const schema = { + type: "object", + properties: { + name: { type: "string" }, + ssn: { type: "string" }, + }, + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "sensitive", + schema, + processSubmission, + pii: ["ssn"], + }); + + const mockResponse = { + locals: { logger: { scrub: scrubFn } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { body: { name: "John", ssn: "123-45-6789" }, locals: {} }, + response: mockResponse, + }); + + assert({ + given: "PII fields configured", + should: "call logger.scrub with PII field names", + actual: scrubFn.mock.calls[0]?.[0], + expected: ["ssn"], + }); + }); + + // Req 8: Undeclared fields return 400 + test("returns 400 when request contains undeclared fields", async () => { + const processSubmission = vi.fn(); + const schema = { + type: "object", + properties: { + name: { type: "string" }, + }, + required: ["name"], + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "strict", + schema, + processSubmission, + pii: [], + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "John", extraField: "not allowed" }, + }, + response: mockResponse, + }); + + assert({ + given: "a request with undeclared fields", + should: "return 400 status", + actual: mockResponse.status.mock.calls[0]?.[0], + expected: 400, + }); + + const errors = mockResponse.json.mock.calls[0]?.[0]?.errors || []; + assert({ + given: "a request with undeclared fields", + should: "indicate undeclared field in error", + actual: errors.some( + (e) => e.includes("extraField") || e.includes("additional"), + ), + expected: true, + }); + }); + + // Req 9: Honeypot omitted skips validation + test("skips honeypot validation when honeypotField not provided", async () => { + const processSubmission = vi.fn().mockResolvedValue({}); + const schema = { + type: "object", + properties: { + name: { type: "string" }, + website: { type: "string" }, + }, + additionalProperties: false, + }; + + const middleware = handleForm({ + name: "no-honeypot", + schema, + processSubmission, + pii: [], + // honeypotField intentionally omitted + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "Human", website: "http://real-site.com" }, + }, + response: mockResponse, + }); + + assert({ + given: "no honeypotField configured and website field filled", + should: "allow submission (call processSubmission)", + actual: processSubmission.mock.calls.length, + expected: 1, + }); + }); +}); diff --git a/src/server/middleware/index.js b/src/server/middleware/index.js index 6ba35eb7..521fb57a 100644 --- a/src/server/middleware/index.js +++ b/src/server/middleware/index.js @@ -11,3 +11,5 @@ export { } from "./with-config.js"; export { withServerError } from "./with-server-error.js"; export { createWithAuth, createWithOptionalAuth } from "./with-auth.js"; +export { handleForm } from "./handle-form.js"; +export { withCSRF } from "./with-csrf.js"; diff --git a/src/server/middleware/with-csrf.js b/src/server/middleware/with-csrf.js new file mode 100644 index 00000000..43f028cf --- /dev/null +++ b/src/server/middleware/with-csrf.js @@ -0,0 +1,75 @@ +/** + * CSRF protection middleware using double-submit cookie pattern + */ + +import { createId } from "@paralleldrive/cuid2"; +import { sha3_256 } from "js-sha3"; + +const SAFE_METHODS = ["GET", "HEAD", "OPTIONS"]; +const COOKIE_NAME = "csrf_token"; + +const parseCookies = (cookieHeader) => { + if (!cookieHeader) return {}; + return cookieHeader.split(";").reduce((cookies, cookie) => { + const [name, value] = cookie.trim().split("="); + cookies[name] = value; + return cookies; + }, {}); +}; + +const hashToken = (token) => sha3_256(token || ""); + +const tokensMatch = (token1, token2) => { + return hashToken(token1) === hashToken(token2); +}; + +const buildCookieString = (token) => { + const parts = [`${COOKIE_NAME}=${token}`, "SameSite=Strict"]; + if (process.env.NODE_ENV === "production") { + parts.push("Secure"); + } + return parts.join("; "); +}; + +const rejectRequest = (response, requestId) => { + console.log({ + message: "CSRF validation failed", + requestId, + }); + response.status(403); + response.json({ + error: "CSRF validation failed", + requestId, + }); +}; + +const withCSRF = async ({ request, response }) => { + if (!response.locals) response.locals = {}; + + if (SAFE_METHODS.includes(request.method)) { + const token = createId(); + response.locals.csrfToken = token; + response.setHeader("Set-Cookie", buildCookieString(token)); + return { request, response }; + } + + // Unsafe method - validate CSRF token + const cookies = parseCookies(request.headers?.cookie); + const cookieToken = cookies[COOKIE_NAME]; + const headerToken = request.headers?.["x-csrf-token"]; + const bodyToken = request.body?._csrf; + const submittedToken = headerToken || bodyToken; + + if ( + !cookieToken || + !submittedToken || + !tokensMatch(cookieToken, submittedToken) + ) { + rejectRequest(response, response.locals?.requestId); + return { request, response }; + } + + return { request, response }; +}; + +export { withCSRF }; diff --git a/src/server/middleware/with-csrf.test.js b/src/server/middleware/with-csrf.test.js new file mode 100644 index 00000000..11c3d36b --- /dev/null +++ b/src/server/middleware/with-csrf.test.js @@ -0,0 +1,315 @@ +import { describe, test, vi } from "vitest"; +import { assert } from "riteway/vitest"; +import { withCSRF } from "./with-csrf.js"; + +describe("withCSRF", () => { + // Req 1: GET/HEAD/OPTIONS sets token cookie and response.locals.csrfToken + test("sets CSRF token cookie and response.locals.csrfToken for GET request", async () => { + const cookies = {}; + const mockResponse = { + locals: {}, + setHeader: vi.fn((name, value) => { + if (name === "Set-Cookie") cookies.raw = value; + }), + }; + + await withCSRF({ + request: { method: "GET", headers: {} }, + response: mockResponse, + }); + + assert({ + given: "a GET request", + should: "attach csrfToken to response.locals", + actual: typeof mockResponse.locals.csrfToken, + expected: "string", + }); + + assert({ + given: "a GET request", + should: "set CSRF token cookie", + actual: cookies.raw?.includes("csrf_token="), + expected: true, + }); + }); + + // Req 2: POST with matching token in header allowed + test("allows POST request with matching token in header", async () => { + const token = "test-token-123"; + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + status: vi.fn(), + json: vi.fn(), + }; + + const result = await withCSRF({ + request: { + method: "POST", + headers: { + cookie: `csrf_token=${token}`, + "x-csrf-token": token, + }, + }, + response: mockResponse, + }); + + assert({ + given: "a POST request with matching token in header", + should: "allow request to proceed and return request/response", + actual: result.request.method, + expected: "POST", + }); + + assert({ + given: "a POST request with matching token in header", + should: "not set error status", + actual: mockResponse.status.mock.calls.length, + expected: 0, + }); + }); + + // Req 3: POST with matching token in body allowed + test("allows POST request with matching token in body field", async () => { + const token = "test-token-456"; + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + status: vi.fn(), + json: vi.fn(), + }; + + const result = await withCSRF({ + request: { + method: "POST", + headers: { + cookie: `csrf_token=${token}`, + }, + body: { _csrf: token }, + }, + response: mockResponse, + }); + + assert({ + given: "a POST request with matching token in body", + should: "allow request to proceed", + actual: result.request.method, + expected: "POST", + }); + + assert({ + given: "a POST request with matching token in body", + should: "not set error status", + actual: mockResponse.status.mock.calls.length, + expected: 0, + }); + }); + + // Req 4: Missing cookie token returns 403 + test("rejects POST request when token is missing from cookie", async () => { + const mockResponse = { + locals: { requestId: "req-123" }, + setHeader: vi.fn(), + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await withCSRF({ + request: { + method: "POST", + headers: { + "x-csrf-token": "some-token", + }, + }, + response: mockResponse, + }); + + assert({ + given: "a POST request with no CSRF cookie", + should: "return 403 status", + actual: mockResponse.status.mock.calls[0]?.[0], + expected: 403, + }); + + assert({ + given: "a POST request with no CSRF cookie", + should: "return error message", + actual: mockResponse.json.mock.calls[0]?.[0]?.error, + expected: "CSRF validation failed", + }); + }); + + // Req 5: Mismatched token returns 403 + test("rejects POST request when tokens do not match", async () => { + const mockResponse = { + locals: { requestId: "req-456" }, + setHeader: vi.fn(), + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await withCSRF({ + request: { + method: "POST", + headers: { + cookie: "csrf_token=cookie-token", + "x-csrf-token": "different-token", + }, + }, + response: mockResponse, + }); + + assert({ + given: "a POST request with mismatched tokens", + should: "return 403 status", + actual: mockResponse.status.mock.calls[0]?.[0], + expected: 403, + }); + }); + + // Req 6: Tokens generated with CUID2 (verified by format) + test("generates tokens using CUID2 format", async () => { + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + }; + + await withCSRF({ + request: { method: "GET", headers: {} }, + response: mockResponse, + }); + + // CUID2 tokens are 24 characters by default and lowercase alphanumeric + const token = mockResponse.locals.csrfToken; + + assert({ + given: "a GET request", + should: + "generate token with CUID2 format (24 chars, lowercase alphanumeric)", + actual: /^[a-z0-9]{24,}$/.test(token), + expected: true, + }); + }); + + // Req 7: Cookie has SameSite=Strict, Secure in production + test("sets SameSite=Strict on CSRF cookie", async () => { + let cookieValue = ""; + const mockResponse = { + locals: {}, + setHeader: vi.fn((name, value) => { + if (name === "Set-Cookie") cookieValue = value; + }), + }; + + await withCSRF({ + request: { method: "GET", headers: {} }, + response: mockResponse, + }); + + assert({ + given: "setting CSRF cookie", + should: "include SameSite=Strict", + actual: cookieValue.includes("SameSite=Strict"), + expected: true, + }); + }); + + test("sets Secure flag in production", async () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = "production"; + + let cookieValue = ""; + const mockResponse = { + locals: {}, + setHeader: vi.fn((name, value) => { + if (name === "Set-Cookie") cookieValue = value; + }), + }; + + await withCSRF({ + request: { method: "GET", headers: {} }, + response: mockResponse, + }); + + process.env.NODE_ENV = originalEnv; + + assert({ + given: "production environment", + should: "include Secure flag on cookie", + actual: cookieValue.includes("Secure"), + expected: true, + }); + }); + + // Req 8: No HttpOnly on cookie + test("does not set HttpOnly on CSRF cookie", async () => { + let cookieValue = ""; + const mockResponse = { + locals: {}, + setHeader: vi.fn((name, value) => { + if (name === "Set-Cookie") cookieValue = value; + }), + }; + + await withCSRF({ + request: { method: "GET", headers: {} }, + response: mockResponse, + }); + + assert({ + given: "setting CSRF cookie", + should: "not include HttpOnly (client must read token)", + actual: cookieValue.includes("HttpOnly"), + expected: false, + }); + }); + + // Req 9: Log rejections without exposing tokens + test("logs CSRF rejection with request ID but without token values", async () => { + const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + const mockResponse = { + locals: { requestId: "req-789" }, + setHeader: vi.fn(), + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await withCSRF({ + request: { + method: "POST", + headers: { + cookie: "csrf_token=secret-cookie-token", + "x-csrf-token": "secret-header-token", + }, + }, + response: mockResponse, + }); + + const logCall = consoleSpy.mock.calls[0]?.[0]; + consoleSpy.mockRestore(); + + assert({ + given: "a CSRF rejection", + should: "log the failure", + actual: logCall?.message?.includes("CSRF") || logCall?.includes?.("CSRF"), + expected: true, + }); + + assert({ + given: "a CSRF rejection", + should: "include request ID in log", + actual: JSON.stringify(logCall)?.includes("req-789"), + expected: true, + }); + + assert({ + given: "a CSRF rejection", + should: "not expose token values in log", + actual: + !JSON.stringify(logCall)?.includes("secret-cookie-token") && + !JSON.stringify(logCall)?.includes("secret-header-token"), + expected: true, + }); + }); +}); From ba5de09764daeacc1dd70bd49c4d9244d994e30f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 7 Dec 2025 02:05:55 +0000 Subject: [PATCH 05/15] Replace ajv with TypeBox for form validation - TypeBox provides TypeScript-native schemas with JSON Schema output - Better DX: schema IS the type (no duplication) - Smaller footprint: removed 4 packages, added 1 - MCP/OpenAPI compatible out of the box --- package-lock.json | 56 ++--------- package.json | 2 +- src/server/middleware/handle-form.js | 31 +++--- src/server/middleware/handle-form.test.js | 112 ++++++++++------------ 4 files changed, 74 insertions(+), 127 deletions(-) diff --git a/package-lock.json b/package-lock.json index d085524c..f2654b53 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "MIT", "dependencies": { "@paralleldrive/cuid2": "^3.1.0", - "ajv": "^8.17.1", + "@sinclair/typebox": "^0.34.41", "chalk": "^4.1.2", "commander": "^11.1.0", "error-causes": "^3.0.2", @@ -1930,6 +1930,12 @@ "win32" ] }, + "node_modules/@sinclair/typebox": { + "version": "0.34.41", + "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.34.41.tgz", + "integrity": "sha512-6gS8pZzSXdyRHTIqoqSVknxolr1kzfy4/CeDnrzsVz8TTIWUbOBr6gnzOmTYJ3eXQNh4IYHIGi5aIL7sOZ2G/g==", + "license": "MIT" + }, "node_modules/@textlint/ast-node-types": { "version": "12.6.1", "resolved": "https://registry.npmjs.org/@textlint/ast-node-types/-/ast-node-types-12.6.1.tgz", @@ -2199,22 +2205,6 @@ "node": ">= 14" } }, - "node_modules/ajv": { - "version": "8.17.1", - "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.17.1.tgz", - "integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==", - "license": "MIT", - "dependencies": { - "fast-deep-equal": "^3.1.3", - "fast-uri": "^3.0.1", - "json-schema-traverse": "^1.0.0", - "require-from-string": "^2.0.2" - }, - "funding": { - "type": "github", - "url": "https://github.com/sponsors/epoberezkin" - } - }, "node_modules/anchor-markdown-header": { "version": "0.6.0", "resolved": "https://registry.npmjs.org/anchor-markdown-header/-/anchor-markdown-header-0.6.0.tgz", @@ -3992,6 +3982,7 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", + "dev": true, "license": "MIT" }, "node_modules/fast-diff": { @@ -4015,22 +4006,6 @@ "dev": true, "license": "MIT" }, - "node_modules/fast-uri": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.0.tgz", - "integrity": "sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/fastify" - }, - { - "type": "opencollective", - "url": "https://opencollective.com/fastify" - } - ], - "license": "BSD-3-Clause" - }, "node_modules/fault": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/fault/-/fault-1.0.4.tgz", @@ -5524,12 +5499,6 @@ "dev": true, "license": "MIT" }, - "node_modules/json-schema-traverse": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", - "integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==", - "license": "MIT" - }, "node_modules/json-stable-stringify-without-jsonify": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/json-stable-stringify-without-jsonify/-/json-stable-stringify-without-jsonify-1.0.1.tgz", @@ -7180,15 +7149,6 @@ "node": ">=0.10" } }, - "node_modules/require-from-string": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz", - "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==", - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/resolve": { "version": "2.0.0-next.5", "resolved": "https://registry.npmjs.org/resolve/-/resolve-2.0.0-next.5.tgz", diff --git a/package.json b/package.json index 27a05447..05414f66 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "homepage": "https://github.com/paralleldrive/aidd#readme", "dependencies": { "@paralleldrive/cuid2": "^3.1.0", - "ajv": "^8.17.1", + "@sinclair/typebox": "^0.34.41", "chalk": "^4.1.2", "commander": "^11.1.0", "error-causes": "^3.0.2", diff --git a/src/server/middleware/handle-form.js b/src/server/middleware/handle-form.js index 8712a3d2..6e451295 100644 --- a/src/server/middleware/handle-form.js +++ b/src/server/middleware/handle-form.js @@ -1,22 +1,21 @@ /** - * Form handling middleware factory with JSON Schema validation + * Form handling middleware factory with TypeBox validation */ -import Ajv from "ajv"; - -const ajv = new Ajv({ allErrors: true }); +import { TypeCompiler } from "@sinclair/typebox/compiler"; const formatErrors = (errors) => { - return errors.map((err) => { - if (err.keyword === "required") { - return `Missing required field: ${err.params.missingProperty}`; + return [...errors].map((err) => { + const path = err.path.slice(1) || "root"; + + if (err.message.includes("Required")) { + return `Missing required field: ${path}`; } - if (err.keyword === "additionalProperties") { - return `Undeclared field not allowed: ${err.params.additionalProperty}`; + if (err.message.includes("Unexpected property")) { + return `Undeclared field not allowed: ${path}`; } - if (err.keyword === "type") { - const field = err.instancePath.slice(1) || "root"; - return `Field '${field}' ${err.message}`; + if (err.message.includes("Expected")) { + return `Field '${path}' ${err.message.toLowerCase()}`; } return err.message; }); @@ -41,14 +40,14 @@ const handleForm = return { request, response }; } - // Validate against schema - const validate = ajv.compile(schema); - const valid = validate(body); + // Validate against schema using TypeBox compiler + const validator = TypeCompiler.Compile(schema); + const valid = validator.Check(body); if (!valid) { response.status(400); response.json({ - errors: formatErrors(validate.errors), + errors: formatErrors(validator.Errors(body)), }); return { request, response }; } diff --git a/src/server/middleware/handle-form.test.js b/src/server/middleware/handle-form.test.js index 17a5eab3..80a3c9d5 100644 --- a/src/server/middleware/handle-form.test.js +++ b/src/server/middleware/handle-form.test.js @@ -1,20 +1,19 @@ import { describe, test, vi } from "vitest"; import { assert } from "riteway/vitest"; +import { Type } from "@sinclair/typebox"; import { handleForm } from "./handle-form.js"; describe("handleForm", () => { // Req 1: Valid body passes to processSubmission test("passes validated fields to processSubmission when body matches schema", async () => { const processSubmission = vi.fn().mockResolvedValue({}); - const schema = { - type: "object", - properties: { - name: { type: "string" }, - email: { type: "string" }, + const schema = Type.Object( + { + name: Type.String(), + email: Type.String(), }, - required: ["name", "email"], - additionalProperties: false, - }; + { additionalProperties: false }, + ); const middleware = handleForm({ name: "contact", @@ -47,15 +46,13 @@ describe("handleForm", () => { // Req 2: Invalid body returns 400 with validation errors test("returns 400 with validation errors when body fails schema", async () => { const processSubmission = vi.fn(); - const schema = { - type: "object", - properties: { - name: { type: "string" }, - age: { type: "number" }, + const schema = Type.Object( + { + name: Type.String(), + age: Type.Number(), }, - required: ["name", "age"], - additionalProperties: false, - }; + { additionalProperties: false }, + ); const middleware = handleForm({ name: "profile", @@ -102,14 +99,13 @@ describe("handleForm", () => { // Req 3: Honeypot field filled rejects with generic error test("rejects with 400 and generic error when honeypot field is filled", async () => { const processSubmission = vi.fn(); - const schema = { - type: "object", - properties: { - name: { type: "string" }, - website: { type: "string" }, + const schema = Type.Object( + { + name: Type.String(), + website: Type.String(), }, - additionalProperties: false, - }; + { additionalProperties: false }, + ); const middleware = handleForm({ name: "signup", @@ -159,15 +155,13 @@ describe("handleForm", () => { // Req 4: Missing required fields returns 400 with specific errors test("returns 400 with missing field errors when required fields absent", async () => { const processSubmission = vi.fn(); - const schema = { - type: "object", - properties: { - name: { type: "string" }, - email: { type: "string" }, + const schema = Type.Object( + { + name: Type.String(), + email: Type.String(), }, - required: ["name", "email"], - additionalProperties: false, - }; + { additionalProperties: false }, + ); const middleware = handleForm({ name: "contact", @@ -208,11 +202,10 @@ describe("handleForm", () => { // Req 5: processSubmission error surfaces through createRoute test("throws error when processSubmission throws", async () => { const processSubmission = vi.fn().mockRejectedValue(new Error("DB error")); - const schema = { - type: "object", - properties: { name: { type: "string" } }, - additionalProperties: false, - }; + const schema = Type.Object( + { name: Type.String() }, + { additionalProperties: false }, + ); const middleware = handleForm({ name: "test", @@ -248,11 +241,10 @@ describe("handleForm", () => { // Req 6: Successful submission returns { request, response } without setting status/body test("returns request/response without setting status on success", async () => { const processSubmission = vi.fn().mockResolvedValue({}); - const schema = { - type: "object", - properties: { name: { type: "string" } }, - additionalProperties: false, - }; + const schema = Type.Object( + { name: Type.String() }, + { additionalProperties: false }, + ); const middleware = handleForm({ name: "test", @@ -298,14 +290,13 @@ describe("handleForm", () => { test("passes PII fields to logger.scrub", async () => { const processSubmission = vi.fn().mockResolvedValue({}); const scrubFn = vi.fn(); - const schema = { - type: "object", - properties: { - name: { type: "string" }, - ssn: { type: "string" }, + const schema = Type.Object( + { + name: Type.String(), + ssn: Type.String(), }, - additionalProperties: false, - }; + { additionalProperties: false }, + ); const middleware = handleForm({ name: "sensitive", @@ -336,14 +327,12 @@ describe("handleForm", () => { // Req 8: Undeclared fields return 400 test("returns 400 when request contains undeclared fields", async () => { const processSubmission = vi.fn(); - const schema = { - type: "object", - properties: { - name: { type: "string" }, + const schema = Type.Object( + { + name: Type.String(), }, - required: ["name"], - additionalProperties: false, - }; + { additionalProperties: false }, + ); const middleware = handleForm({ name: "strict", @@ -386,14 +375,13 @@ describe("handleForm", () => { // Req 9: Honeypot omitted skips validation test("skips honeypot validation when honeypotField not provided", async () => { const processSubmission = vi.fn().mockResolvedValue({}); - const schema = { - type: "object", - properties: { - name: { type: "string" }, - website: { type: "string" }, + const schema = Type.Object( + { + name: Type.String(), + website: Type.String(), }, - additionalProperties: false, - }; + { additionalProperties: false }, + ); const middleware = handleForm({ name: "no-honeypot", From 0e4d62d82c0eeade76d9d1d0df5447dc041199f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 00:49:10 +0000 Subject: [PATCH 06/15] Enhance CSRF and form middleware with TypeScript, logging, and configuration CSRF middleware: - Add createWithCSRF factory with configurable maxAge (default: 3 hours) - Add Path=/ cookie attribute for cross-route support - Use response.locals.log with console.log fallback - Log attack investigation details (method, url, hasCookie, hasHeader, hasBody) - Add TypeScript definitions for createWithCSRF and withCSRF Form handling: - Log honeypot triggers and validation failures - Use response.locals.log with console.log fallback - Add TypeScript definitions for handleForm and HandleFormOptions Also adds csrfToken, log, and logger to response.locals TypeScript interface --- src/server/index.d.ts | 82 +++++++++++++++++ src/server/middleware/handle-form.js | 21 ++++- src/server/middleware/index.js | 2 +- src/server/middleware/with-csrf.js | 111 ++++++++++++++++-------- src/server/middleware/with-csrf.test.js | 67 ++++++++++++++ 5 files changed, 244 insertions(+), 39 deletions(-) diff --git a/src/server/index.d.ts b/src/server/index.d.ts index a636565d..baf068f0 100644 --- a/src/server/index.d.ts +++ b/src/server/index.d.ts @@ -36,6 +36,9 @@ export interface Response { config?: ConfigObject; serverError?: (options?: ErrorOptions) => ErrorResponse; auth?: { user: User; session: Session } | null; + csrfToken?: string; + log?: (data: Record) => void; + logger?: { scrub: (fields: string[]) => void }; [key: string]: any; }; [key: string]: any; @@ -244,3 +247,82 @@ export function createWithAuth(options: WithAuthOptions): Middleware; * }); */ export function createWithOptionalAuth(options: WithOptionalAuthOptions): Middleware; + +// Form handling middleware +import type { TObject } from "@sinclair/typebox"; + +export interface HandleFormOptions { + /** Identifier for the form, used in logging */ + name: string; + /** TypeBox schema for validating request body */ + schema: T; + /** Async function receiving validated form data */ + processSubmission: (data: Record) => Promise; + /** Field names to register with logger scrubber for PII protection */ + pii?: string[]; + /** Field name that must be empty - rejects submission if filled (bot protection) */ + honeypotField?: string; +} + +/** + * Creates middleware for secure form submission handling with TypeBox validation + * + * @example + * import { Type } from '@sinclair/typebox'; + * import { handleForm } from 'aidd/server'; + * + * const ContactSchema = Type.Object({ + * name: Type.String(), + * email: Type.String({ format: 'email' }), + * message: Type.String(), + * }, { additionalProperties: false }); + * + * const withContactForm = handleForm({ + * name: 'contact', + * schema: ContactSchema, + * processSubmission: async (data) => { + * await sendEmail(data.email, data.message); + * }, + * pii: ['email'], + * honeypotField: 'website', + * }); + */ +export function handleForm(options: HandleFormOptions): Middleware; + +// CSRF middleware +export interface CSRFOptions { + /** Cookie max age in seconds (default: 3 hours = 10800) */ + maxAge?: number; +} + +/** + * Creates CSRF protection middleware with configurable options + * + * Uses double-submit cookie pattern: + * - GET/HEAD/OPTIONS: Sets cookie, exposes token via response.locals.csrfToken + * - POST/PUT/PATCH/DELETE: Validates token from header or body against cookie + * - Compares using SHA3 hash (timing-attack safe) + * + * @example + * // Custom 1-hour cookie lifetime + * const withCSRF = createWithCSRF({ maxAge: 60 * 60 }); + */ +export function createWithCSRF(options?: CSRFOptions): Middleware; + +/** + * Default CSRF middleware with 3-hour cookie lifetime + * + * @example + * import { createRoute, withCSRF } from 'aidd/server'; + * + * // Form page - sets cookie and provides token + * export const getForm = createRoute(withCSRF, async ({ response }) => { + * response.json({ csrfToken: response.locals.csrfToken }); + * }); + * + * // Form submission - validates token + * export const submitForm = createRoute(withCSRF, handleContactForm, async ({ response }) => { + * response.json({ success: true }); + * }); + */ +export const withCSRF: Middleware; diff --git a/src/server/middleware/handle-form.js b/src/server/middleware/handle-form.js index 6e451295..926c8f59 100644 --- a/src/server/middleware/handle-form.js +++ b/src/server/middleware/handle-form.js @@ -4,6 +4,11 @@ import { TypeCompiler } from "@sinclair/typebox/compiler"; +const log = (response, data) => { + const logger = response.locals?.log || console.log; + logger(data); +}; + const formatErrors = (errors) => { return [...errors].map((err) => { const path = err.path.slice(1) || "root"; @@ -33,6 +38,11 @@ const handleForm = // Check honeypot field if configured if (honeypotField && body[honeypotField]) { + log(response, { + message: "Form honeypot triggered", + form: name, + requestId: response.locals?.requestId, + }); response.status(400); response.json({ errors: ["Validation failed"], @@ -45,10 +55,15 @@ const handleForm = const valid = validator.Check(body); if (!valid) { - response.status(400); - response.json({ - errors: formatErrors(validator.Errors(body)), + const errors = formatErrors(validator.Errors(body)); + log(response, { + message: "Form validation failed", + form: name, + requestId: response.locals?.requestId, + errorCount: errors.length, }); + response.status(400); + response.json({ errors }); return { request, response }; } diff --git a/src/server/middleware/index.js b/src/server/middleware/index.js index 521fb57a..b92a18fb 100644 --- a/src/server/middleware/index.js +++ b/src/server/middleware/index.js @@ -12,4 +12,4 @@ export { export { withServerError } from "./with-server-error.js"; export { createWithAuth, createWithOptionalAuth } from "./with-auth.js"; export { handleForm } from "./handle-form.js"; -export { withCSRF } from "./with-csrf.js"; +export { createWithCSRF, withCSRF } from "./with-csrf.js"; diff --git a/src/server/middleware/with-csrf.js b/src/server/middleware/with-csrf.js index 43f028cf..1ed15512 100644 --- a/src/server/middleware/with-csrf.js +++ b/src/server/middleware/with-csrf.js @@ -1,5 +1,17 @@ /** * CSRF protection middleware using double-submit cookie pattern + * + * @param {Object} options + * @param {number} [options.maxAge=10800] - Cookie max age in seconds (default: 3 hours) + * @returns {Function} CSRF middleware + * + * @example + * // Use default 3-hour cookie lifetime + * const withCSRF = createWithCSRF(); + * + * @example + * // Custom cookie lifetime (1 hour) + * const withCSRF = createWithCSRF({ maxAge: 60 * 60 }); */ import { createId } from "@paralleldrive/cuid2"; @@ -7,6 +19,7 @@ import { sha3_256 } from "js-sha3"; const SAFE_METHODS = ["GET", "HEAD", "OPTIONS"]; const COOKIE_NAME = "csrf_token"; +const DEFAULT_MAX_AGE = 3 * 60 * 60; // 3 hours in seconds const parseCookies = (cookieHeader) => { if (!cookieHeader) return {}; @@ -19,22 +32,25 @@ const parseCookies = (cookieHeader) => { const hashToken = (token) => sha3_256(token || ""); -const tokensMatch = (token1, token2) => { - return hashToken(token1) === hashToken(token2); -}; +const tokensMatch = (token1, token2) => hashToken(token1) === hashToken(token2); -const buildCookieString = (token) => { - const parts = [`${COOKIE_NAME}=${token}`, "SameSite=Strict"]; - if (process.env.NODE_ENV === "production") { - parts.push("Secure"); - } - return parts.join("; "); +const log = (response, data) => { + const logger = response.locals?.log || console.log; + logger(data); }; -const rejectRequest = (response, requestId) => { - console.log({ +const rejectRequest = ( + response, + { requestId, method, url, hasCookie, hasHeader, hasBody }, +) => { + log(response, { message: "CSRF validation failed", requestId, + method, + url, + hasCookie, + hasHeader, + hasBody, }); response.status(403); response.json({ @@ -43,33 +59,58 @@ const rejectRequest = (response, requestId) => { }); }; -const withCSRF = async ({ request, response }) => { - if (!response.locals) response.locals = {}; +const createWithCSRF = ({ maxAge = DEFAULT_MAX_AGE } = {}) => { + const buildCookieString = (token) => { + const parts = [ + `${COOKIE_NAME}=${token}`, + "SameSite=Strict", + "Path=/", + `Max-Age=${maxAge}`, + ]; + if (process.env.NODE_ENV === "production") { + parts.push("Secure"); + } + return parts.join("; "); + }; - if (SAFE_METHODS.includes(request.method)) { - const token = createId(); - response.locals.csrfToken = token; - response.setHeader("Set-Cookie", buildCookieString(token)); - return { request, response }; - } + return async ({ request, response }) => { + if (!response.locals) response.locals = {}; - // Unsafe method - validate CSRF token - const cookies = parseCookies(request.headers?.cookie); - const cookieToken = cookies[COOKIE_NAME]; - const headerToken = request.headers?.["x-csrf-token"]; - const bodyToken = request.body?._csrf; - const submittedToken = headerToken || bodyToken; + if (SAFE_METHODS.includes(request.method)) { + const token = createId(); + response.locals.csrfToken = token; + response.setHeader("Set-Cookie", buildCookieString(token)); + return { request, response }; + } - if ( - !cookieToken || - !submittedToken || - !tokensMatch(cookieToken, submittedToken) - ) { - rejectRequest(response, response.locals?.requestId); - return { request, response }; - } + // Unsafe method - validate CSRF token + const cookies = parseCookies(request.headers?.cookie); + const cookieToken = cookies[COOKIE_NAME]; + const headerToken = request.headers?.["x-csrf-token"]; + const bodyToken = request.body?._csrf; + const submittedToken = headerToken || bodyToken; - return { request, response }; + if ( + !cookieToken || + !submittedToken || + !tokensMatch(cookieToken, submittedToken) + ) { + rejectRequest(response, { + requestId: response.locals?.requestId, + method: request.method, + url: request.url, + hasCookie: Boolean(cookieToken), + hasHeader: Boolean(headerToken), + hasBody: Boolean(bodyToken), + }); + return { request, response }; + } + + return { request, response }; + }; }; -export { withCSRF }; +// Default export with 3-hour cookie lifetime +const withCSRF = createWithCSRF(); + +export { createWithCSRF, withCSRF }; diff --git a/src/server/middleware/with-csrf.test.js b/src/server/middleware/with-csrf.test.js index 11c3d36b..82eae94e 100644 --- a/src/server/middleware/with-csrf.test.js +++ b/src/server/middleware/with-csrf.test.js @@ -278,6 +278,7 @@ describe("withCSRF", () => { await withCSRF({ request: { method: "POST", + url: "/api/submit", headers: { cookie: "csrf_token=secret-cookie-token", "x-csrf-token": "secret-header-token", @@ -311,5 +312,71 @@ describe("withCSRF", () => { !JSON.stringify(logCall)?.includes("secret-header-token"), expected: true, }); + + assert({ + given: "a CSRF rejection", + should: "include attack investigation details", + actual: + logCall?.method === "POST" && + logCall?.url === "/api/submit" && + logCall?.hasCookie === true && + logCall?.hasHeader === true, + expected: true, + }); + }); + + test("uses response.locals.log when available", async () => { + const customLog = vi.fn(); + + const mockResponse = { + locals: { requestId: "req-custom", log: customLog }, + setHeader: vi.fn(), + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await withCSRF({ + request: { + method: "POST", + headers: {}, + }, + response: mockResponse, + }); + + assert({ + given: "response.locals.log is available", + should: "use custom logger instead of console.log", + actual: customLog.mock.calls.length, + expected: 1, + }); + + assert({ + given: "response.locals.log is available", + should: "pass log data to custom logger", + actual: customLog.mock.calls[0]?.[0]?.message, + expected: "CSRF validation failed", + }); + }); + + test("sets Path=/ on CSRF cookie", async () => { + let cookieValue = ""; + const mockResponse = { + locals: {}, + setHeader: vi.fn((name, value) => { + if (name === "Set-Cookie") cookieValue = value; + }), + }; + + await withCSRF({ + request: { method: "GET", headers: {} }, + response: mockResponse, + }); + + assert({ + given: "setting CSRF cookie", + should: "include Path=/ for all routes", + actual: cookieValue.includes("Path=/"), + expected: true, + }); }); }); From 0e5bbc3e92e581e8f0dae9af9c64e525e6d8216b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 00:57:17 +0000 Subject: [PATCH 07/15] Add documentation for form, CSRF, and auth middleware - Add createWithAuth and createWithOptionalAuth docs - Add handleForm with TypeBox validation docs - Add createWithCSRF/withCSRF docs with security features - Add recommended default middleware stack (with CSRF warning) - Add frontend CSRF integration examples (React + vanilla JS) - Add complete backend-for-frontend example --- docs/server/README.md | 417 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 417 insertions(+) diff --git a/docs/server/README.md b/docs/server/README.md index 1730fb16..e6bc0b48 100644 --- a/docs/server/README.md +++ b/docs/server/README.md @@ -37,6 +37,10 @@ export default createRoute( - [createWithConfig](#createwithconfig) - [loadConfigFromEnv](#loadconfigfromenv) - [withServerError](#withservererror) + - [createWithAuth](#createwithauth) + - [createWithOptionalAuth](#createwithoptionalauth) + - [handleForm](#handleform) + - [createWithCSRF / withCSRF](#createwithcsrf--withcsrf) - [convertMiddleware](#convertmiddleware) - [createServer](#createserver) - [Examples](#examples) @@ -330,6 +334,233 @@ response.locals.serverError({ --- +### createWithAuth + +Factory that creates authentication middleware requiring a valid session. + +```javascript +createWithAuth(options: WithAuthOptions): Middleware + +interface WithAuthOptions { + auth: BetterAuthInstance; // better-auth instance (required) + onUnauthenticated?: (context: ServerContext) => void; // Custom 401 handler +} +``` + +**Example:** +```javascript +import { createRoute, createWithAuth } from 'aidd/server'; +import { auth } from '~/lib/auth.server'; + +const withAuth = createWithAuth({ auth }); + +export default createRoute( + withAuth, + async ({ response }) => { + const { user } = response.locals.auth; + response.json({ email: user.email }); + } +); +``` + +**Features:** +- Returns 401 if no valid session +- Attaches user and session to `response.locals.auth` +- Integrates with [better-auth](https://github.com/better-auth/better-auth) + +--- + +### createWithOptionalAuth + +Factory that creates auth middleware allowing anonymous requests. + +```javascript +createWithOptionalAuth(options: WithOptionalAuthOptions): Middleware + +interface WithOptionalAuthOptions { + auth: BetterAuthInstance; // better-auth instance (required) +} +``` + +**Example:** +```javascript +import { createRoute, createWithOptionalAuth } from 'aidd/server'; +import { auth } from '~/lib/auth.server'; + +const withOptionalAuth = createWithOptionalAuth({ auth }); + +export default createRoute( + withOptionalAuth, + async ({ response }) => { + const user = response.locals.auth?.user; + response.json({ + greeting: user ? `Hello, ${user.name}` : 'Hello, guest' + }); + } +); +``` + +**Features:** +- Attaches user if session exists +- Sets `response.locals.auth` to `null` if no session +- Never returns 401 + +--- + +### handleForm + +Factory that creates form handling middleware with TypeBox validation. + +```javascript +handleForm(options: HandleFormOptions): Middleware + +interface HandleFormOptions { + name: string; // Form identifier for logging + schema: TObject; // TypeBox schema + processSubmission: (data: object) => Promise; + pii?: string[]; // Fields to scrub from logs + honeypotField?: string; // Bot trap field (must be empty) +} +``` + +**Example:** +```javascript +import { createRoute, handleForm, withCSRF } from 'aidd/server'; +import { Type } from '@sinclair/typebox'; + +const ContactSchema = Type.Object({ + name: Type.String(), + email: Type.String({ format: 'email' }), + message: Type.String(), +}, { additionalProperties: false }); + +const withContactForm = handleForm({ + name: 'contact', + schema: ContactSchema, + processSubmission: async (data) => { + await sendEmail(data.email, data.message); + }, + pii: ['email'], + honeypotField: 'website', // Hidden field - bots fill it +}); + +export default createRoute( + withCSRF, + withContactForm, + async ({ response }) => { + response.json({ success: true }); + } +); +``` + +**Features:** +- **TypeBox Validation** - Type-safe schema with JSON Schema output +- **Honeypot Protection** - Rejects bots that fill hidden fields +- **PII Scrubbing** - Registers sensitive fields with logger +- **Detailed Errors** - Returns 400 with validation failure descriptions +- **Undeclared Field Rejection** - Rejects fields not in schema + +**Error Response (400):** +```json +{ + "errors": [ + "Missing required field: email", + "Field 'age' expected number" + ] +} +``` + +--- + +### createWithCSRF / withCSRF + +CSRF protection middleware using the double-submit cookie pattern. + +```javascript +createWithCSRF(options?: CSRFOptions): Middleware + +interface CSRFOptions { + maxAge?: number; // Cookie lifetime in seconds (default: 3 hours) +} + +// Pre-configured with 3-hour cookie +const withCSRF: Middleware; +``` + +**Example - Default Configuration:** +```javascript +import { createRoute, withCSRF } from 'aidd/server'; + +// GET - Sets cookie, provides token +export const getForm = createRoute( + withCSRF, + async ({ response }) => { + response.json({ + csrfToken: response.locals.csrfToken + }); + } +); + +// POST - Validates token +export const submitForm = createRoute( + withCSRF, + async ({ response }) => { + response.json({ success: true }); + } +); +``` + +**Example - Custom Cookie Lifetime:** +```javascript +import { createWithCSRF } from 'aidd/server'; + +const withCSRF = createWithCSRF({ maxAge: 60 * 60 }); // 1 hour +``` + +**Client-Side Usage:** +```javascript +// Read token from response or cookie +const csrfToken = response.csrfToken; + +// Submit with header +fetch('/api/submit', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': csrfToken + }, + body: JSON.stringify({ name: 'John' }) +}); + +// Or include in body +fetch('/api/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ name: 'John', _csrf: csrfToken }) +}); +``` + +**Security Features:** +- **Double-Submit Cookie** - Token in cookie + header/body must match +- **SHA3 Hash Comparison** - Prevents timing attacks +- **SameSite=Strict** - Prevents CSRF from other sites +- **Secure Flag** - HTTPS-only in production +- **Path=/** - Works across all routes +- **No HttpOnly** - Client must read cookie (by design) +- **3-Hour Default Lifetime** - Configurable via `maxAge` + +**Safe Methods (GET, HEAD, OPTIONS):** +- Sets CSRF cookie +- Attaches token to `response.locals.csrfToken` +- No validation required + +**Unsafe Methods (POST, PUT, PATCH, DELETE):** +- Validates token from `X-CSRF-Token` header OR `_csrf` body field +- Returns 403 if validation fails +- Logs failure with request details (no token values) + +--- + ### convertMiddleware Converts traditional Express-style middleware to AIDD functional middleware. @@ -396,6 +627,66 @@ test('withRequestId adds request ID', async () => { --- +## Recommended Default Middleware Stack + +Every route should include these essential middleware. **All form submissions require CSRF protection.** + +```javascript +import { + createRoute, + withRequestId, + withServerError, + withCSRF, + createWithCors, + createWithConfig, + loadConfigFromEnv +} from 'aidd/server'; +import { asyncPipe } from 'aidd/utils'; + +// Configure once +const withCors = createWithCors({ + allowedOrigins: process.env.ALLOWED_ORIGINS?.split(',') || ['http://localhost:3000'] +}); + +const withConfig = createWithConfig(() => + loadConfigFromEnv(['DATABASE_URL', 'API_SECRET']) +); + +// Default middleware for all routes +const defaultMiddleware = asyncPipe( + withRequestId, // Always first - enables request tracking + withServerError, // Standardized error responses + withCors, // CORS headers + withConfig, // Environment configuration +); + +// Default middleware for form routes (includes CSRF) +const formMiddleware = asyncPipe( + defaultMiddleware, + withCSRF, // REQUIRED for all form submissions +); + +// API route (no forms) +export const apiRoute = createRoute( + defaultMiddleware, + async ({ response }) => { + response.json({ data: 'ok' }); + } +); + +// Form route (CSRF required) +export const formRoute = createRoute( + formMiddleware, + async ({ response }) => { + response.json({ csrfToken: response.locals.csrfToken }); + } +); +``` + +> ⚠️ **Security**: All form submissions MUST include `withCSRF` middleware. Omitting CSRF protection exposes your application to cross-site request forgery attacks. + +--- + ## Examples ### Complete API with Auth @@ -478,6 +769,132 @@ export default createRoute( ); ``` +### Frontend CSRF Integration + +**React Example:** +```jsx +import { useState, useEffect } from 'react'; + +function ContactForm() { + const [csrfToken, setCsrfToken] = useState(''); + + // Fetch CSRF token on mount + useEffect(() => { + fetch('/api/csrf') + .then(res => res.json()) + .then(data => setCsrfToken(data.csrfToken)); + }, []); + + const handleSubmit = async (e) => { + e.preventDefault(); + const formData = new FormData(e.target); + + const response = await fetch('/api/contact', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': csrfToken, // Include token in header + }, + body: JSON.stringify(Object.fromEntries(formData)), + }); + + if (response.ok) { + alert('Message sent!'); + } + }; + + return ( +
+ + + + + + +
+ + +``` + +**Backend for Frontend Examples:** +```javascript +import { createRoute, handleForm, withCSRF } from 'aidd/server'; +import { Type } from '@sinclair/typebox'; + +// GET /api/csrf - Provides token to frontend +export const getCsrf = createRoute( + withCSRF, + async ({ response }) => { + response.json({ csrfToken: response.locals.csrfToken }); + } +); + +// POST /api/contact - Validates and processes form +const ContactSchema = Type.Object({ + name: Type.String(), + email: Type.String(), + message: Type.String(), + website: Type.Optional(Type.String()), // Honeypot +}, { additionalProperties: false }); + +const withContactForm = handleForm({ + name: 'contact', + schema: ContactSchema, + processSubmission: async (data) => { + await sendEmail(data); + }, + pii: ['email'], + honeypotField: 'website', +}); + +export const postContact = createRoute( + withCSRF, + withContactForm, + async ({ response }) => { + response.json({ success: true }); + } +); +``` + +--- + ### Rate Limiting Middleware ```javascript From cac5ee928eaa1c9e53babb9a2195b1bfb0f7fcfd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 01:05:43 +0000 Subject: [PATCH 08/15] Add typecheck to test scripts Include npm run typecheck in test and test:unit scripts to catch type errors during CI/test runs, matching how lint is already included. --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 05414f66..6ca2f526 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,8 @@ "lint": "eslint --fix && echo 'Lint fix complete.'", "format": "prettier --write . && echo 'Format complete.'", "format:check": "prettier --check . && echo 'Format check complete.'", - "test": "vitest run && echo 'Test complete.' && npm run -s lint", - "test:unit": "vitest run --exclude '**/*-e2e.test.js' && echo 'Unit tests complete.' && npm run -s lint", + "test": "vitest run && echo 'Test complete.' && npm run -s lint && npm run -s typecheck", + "test:unit": "vitest run --exclude '**/*-e2e.test.js' && echo 'Unit tests complete.' && npm run -s lint && npm run -s typecheck", "test:e2e": "vitest run **/*-e2e.test.js && echo 'E2E tests complete.'", "typecheck": "tsc --noEmit && echo 'Type check complete.'", "check-status": "[ -n \"$(git status --porcelain)\" ] && { echo '❌ Uncommitted changes'; exit 1; } || echo '✅ Git status is clean.'", From 81e780023a2b9c50657525f0b6f59cbecd6e80e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 01:14:24 +0000 Subject: [PATCH 09/15] Fix CSRF token regeneration on every GET request The middleware was generating a new token on every safe method request, which would invalidate tokens stored by clients. Now it reuses existing cookie tokens and only generates new ones when no cookie exists. This prevents the scenario where: 1. Client fetches token A from /api/csrf 2. Client navigates, triggering GET to another withCSRF route 3. Cookie gets overwritten with token B 4. POST with stored token A fails because cookie has token B --- src/server/middleware/with-csrf.js | 7 ++- src/server/middleware/with-csrf.test.js | 65 +++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/server/middleware/with-csrf.js b/src/server/middleware/with-csrf.js index 1ed15512..84116e2a 100644 --- a/src/server/middleware/with-csrf.js +++ b/src/server/middleware/with-csrf.js @@ -77,8 +77,13 @@ const createWithCSRF = ({ maxAge = DEFAULT_MAX_AGE } = {}) => { if (!response.locals) response.locals = {}; if (SAFE_METHODS.includes(request.method)) { - const token = createId(); + // Reuse existing token if present, otherwise generate new one + const cookies = parseCookies(request.headers?.cookie); + const existingToken = cookies[COOKIE_NAME]; + const token = existingToken || createId(); + response.locals.csrfToken = token; + // Always set cookie to refresh expiry response.setHeader("Set-Cookie", buildCookieString(token)); return { request, response }; } diff --git a/src/server/middleware/with-csrf.test.js b/src/server/middleware/with-csrf.test.js index 82eae94e..7b5a22e2 100644 --- a/src/server/middleware/with-csrf.test.js +++ b/src/server/middleware/with-csrf.test.js @@ -358,6 +358,71 @@ describe("withCSRF", () => { }); }); + // Req 10: Reuse existing token on subsequent GET requests + test("reuses existing token from cookie on GET request", async () => { + const existingToken = "existing-token-abc123"; + let cookieValue = ""; + const mockResponse = { + locals: {}, + setHeader: vi.fn((name, value) => { + if (name === "Set-Cookie") cookieValue = value; + }), + }; + + await withCSRF({ + request: { + method: "GET", + headers: { + cookie: `csrf_token=${existingToken}`, + }, + }, + response: mockResponse, + }); + + assert({ + given: "a GET request with existing CSRF cookie", + should: "reuse the existing token in response.locals", + actual: mockResponse.locals.csrfToken, + expected: existingToken, + }); + + assert({ + given: "a GET request with existing CSRF cookie", + should: "set cookie with same token (to refresh expiry)", + actual: cookieValue.includes(`csrf_token=${existingToken}`), + expected: true, + }); + }); + + test("generates new token on GET when no cookie exists", async () => { + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + }; + + await withCSRF({ + request: { + method: "GET", + headers: {}, + }, + response: mockResponse, + }); + + assert({ + given: "a GET request with no existing CSRF cookie", + should: "generate a new token", + actual: typeof mockResponse.locals.csrfToken, + expected: "string", + }); + + assert({ + given: "a GET request with no existing CSRF cookie", + should: "generate token with CUID2 format", + actual: /^[a-z0-9]{24,}$/.test(mockResponse.locals.csrfToken), + expected: true, + }); + }); + test("sets Path=/ on CSRF cookie", async () => { let cookieValue = ""; const mockResponse = { From c486a5df189c69c78816351426d806c9c1e93cc8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 01:46:38 +0000 Subject: [PATCH 10/15] Address code review feedback for form/CSRF middleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Performance: - Move TypeCompiler.Compile outside request handler (compile once) Correctness: - Fix cookie parser to handle values containing '=' - Make honeypot field optional in test schemas (matches real usage) Documentation: - Add JSDoc to handleForm with full parameter docs and example - Fix task doc: request.locals → response.locals - Add detailed timing attack rationale comments for SHA3 hashing Test coverage: - Add tests for HEAD and OPTIONS methods (safe methods) - Add test for empty string honeypot (should allow submission) --- src/server/middleware/handle-form.js | 40 +++++++++++++--- src/server/middleware/handle-form.test.js | 49 ++++++++++++++++++- src/server/middleware/with-csrf.js | 13 ++++- src/server/middleware/with-csrf.test.js | 58 +++++++++++++++++++++++ tasks/form-csrf.md | 4 +- 5 files changed, 154 insertions(+), 10 deletions(-) diff --git a/src/server/middleware/handle-form.js b/src/server/middleware/handle-form.js index 926c8f59..b39ada93 100644 --- a/src/server/middleware/handle-form.js +++ b/src/server/middleware/handle-form.js @@ -1,5 +1,25 @@ /** - * Form handling middleware factory with TypeBox validation + * Create a form handler with validation, honeypot support, and PII scrubbing. + * + * @param {Object} opts + * @param {string} opts.name - Name of the form for logging, metrics, and errors + * @param {import('@sinclair/typebox').TObject} opts.schema - TypeBox schema for the request body + * @param {(body: Record) => Promise} opts.processSubmission - + * Async function that processes a valid submission + * @param {string[]} [opts.pii] - + * Field names that may contain PII, used to configure logger scrubbing + * @param {string} [opts.honeypotField] - + * Optional honeypot field. Any non-empty value causes the submission to be rejected + * @returns {Function} Middleware function that validates and processes the form + * + * @example + * const withContactForm = handleForm({ + * name: 'contact', + * schema: Type.Object({ email: Type.String() }), + * processSubmission: async (body) => { await sendEmail(body.email); }, + * pii: ['email'], + * honeypotField: 'website', + * }); */ import { TypeCompiler } from "@sinclair/typebox/compiler"; @@ -26,9 +46,17 @@ const formatErrors = (errors) => { }); }; -const handleForm = - ({ name, schema, processSubmission, pii, honeypotField }) => - async ({ request, response }) => { +const handleForm = ({ + name, + schema, + processSubmission, + pii, + honeypotField, +}) => { + // Compile schema once when middleware is created, not on every request + const validator = TypeCompiler.Compile(schema); + + return async ({ request, response }) => { // Register PII fields with logger scrubber if (pii?.length && response.locals?.logger?.scrub) { response.locals.logger.scrub(pii); @@ -50,8 +78,7 @@ const handleForm = return { request, response }; } - // Validate against schema using TypeBox compiler - const validator = TypeCompiler.Compile(schema); + // Validate against pre-compiled schema const valid = validator.Check(body); if (!valid) { @@ -72,5 +99,6 @@ const handleForm = return { request, response }; }; +}; export { handleForm }; diff --git a/src/server/middleware/handle-form.test.js b/src/server/middleware/handle-form.test.js index 80a3c9d5..33ba35da 100644 --- a/src/server/middleware/handle-form.test.js +++ b/src/server/middleware/handle-form.test.js @@ -102,7 +102,7 @@ describe("handleForm", () => { const schema = Type.Object( { name: Type.String(), - website: Type.String(), + website: Type.Optional(Type.String()), }, { additionalProperties: false }, ); @@ -152,6 +152,53 @@ describe("handleForm", () => { }); }); + // Req 3b: Empty string honeypot allows submission + test("allows submission when honeypot field is empty string", async () => { + const processSubmission = vi.fn().mockResolvedValue({}); + const schema = Type.Object( + { + name: Type.String(), + website: Type.Optional(Type.String()), + }, + { additionalProperties: false }, + ); + + const middleware = handleForm({ + name: "contact", + schema, + processSubmission, + pii: [], + honeypotField: "website", + }); + + const mockResponse = { + locals: { logger: { scrub: vi.fn() } }, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "Human", website: "" }, + }, + response: mockResponse, + }); + + assert({ + given: "a request with empty string honeypot field", + should: "allow submission (call processSubmission)", + actual: processSubmission.mock.calls.length, + expected: 1, + }); + + assert({ + given: "a request with empty string honeypot field", + should: "not return error status", + actual: mockResponse.status.mock.calls.length, + expected: 0, + }); + }); + // Req 4: Missing required fields returns 400 with specific errors test("returns 400 with missing field errors when required fields absent", async () => { const processSubmission = vi.fn(); diff --git a/src/server/middleware/with-csrf.js b/src/server/middleware/with-csrf.js index 84116e2a..d4fa0efe 100644 --- a/src/server/middleware/with-csrf.js +++ b/src/server/middleware/with-csrf.js @@ -24,7 +24,10 @@ const DEFAULT_MAX_AGE = 3 * 60 * 60; // 3 hours in seconds const parseCookies = (cookieHeader) => { if (!cookieHeader) return {}; return cookieHeader.split(";").reduce((cookies, cookie) => { - const [name, value] = cookie.trim().split("="); + const parts = cookie.trim().split("="); + const name = parts[0]; + // Rejoin remaining parts to handle values containing '=' + const value = parts.slice(1).join("="); cookies[name] = value; return cookies; }, {}); @@ -32,6 +35,14 @@ const parseCookies = (cookieHeader) => { const hashToken = (token) => sha3_256(token || ""); +// Hash both tokens before comparison. +// Reasons: +// 1. Comparing raw tokens is vulnerable to subtle timing leaks, +// especially if timing-safe compare helpers get broken by +// compiler or engine optimizations. +// 2. A cryptographic hash makes any change in the input completely +// change the output, so there is no prefix-based timing signal. +// 3. Hashing also keeps raw CSRF token values out of logs and errors. const tokensMatch = (token1, token2) => hashToken(token1) === hashToken(token2); const log = (response, data) => { diff --git a/src/server/middleware/with-csrf.test.js b/src/server/middleware/with-csrf.test.js index 7b5a22e2..ff351dad 100644 --- a/src/server/middleware/with-csrf.test.js +++ b/src/server/middleware/with-csrf.test.js @@ -33,6 +33,64 @@ describe("withCSRF", () => { }); }); + // Req 1b: HEAD requests are treated as safe methods + test("sets CSRF token cookie for HEAD request", async () => { + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + status: vi.fn(), + json: vi.fn(), + }; + + await withCSRF({ + request: { method: "HEAD", headers: {} }, + response: mockResponse, + }); + + assert({ + given: "a HEAD request", + should: "attach csrfToken to response.locals", + actual: typeof mockResponse.locals.csrfToken, + expected: "string", + }); + + assert({ + given: "a HEAD request", + should: "not return 403 status", + actual: mockResponse.status.mock.calls.length, + expected: 0, + }); + }); + + // Req 1c: OPTIONS requests are treated as safe methods + test("sets CSRF token cookie for OPTIONS request", async () => { + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + status: vi.fn(), + json: vi.fn(), + }; + + await withCSRF({ + request: { method: "OPTIONS", headers: {} }, + response: mockResponse, + }); + + assert({ + given: "an OPTIONS request", + should: "attach csrfToken to response.locals", + actual: typeof mockResponse.locals.csrfToken, + expected: "string", + }); + + assert({ + given: "an OPTIONS request", + should: "not return 403 status", + actual: mockResponse.status.mock.calls.length, + expected: 0, + }); + }); + // Req 2: POST with matching token in header allowed test("allows POST request with matching token in header", async () => { const token = "test-token-123"; diff --git a/tasks/form-csrf.md b/tasks/form-csrf.md index 5af777be..47375ce9 100644 --- a/tasks/form-csrf.md +++ b/tasks/form-csrf.md @@ -51,7 +51,7 @@ handleForm({ name, schema, processSubmission, pii, honeypotField }) 4. Given a request missing required fields per schema, should return 400 status with validation failures indicating missing fields 5. Given `processSubmission` throws an error, should surface error through standard `createRoute` error handling 6. Given a successful submission, should return `{ request, response }` without setting status or body (caller handles success response) -7. Given PII fields, should pass them to `request.locals.logger.scrub(pii)` +7. Given PII fields, should pass them to `response.locals.logger.scrub(pii)` 8. Given a request with fields not defined in schema, should return 400 status with validation failures indicating undeclared fields 9. Given the `honeypotField` parameter is omitted, should skip honeypot validation @@ -87,7 +87,7 @@ withCSRF // Direct export, no parameters 7. Given setting the CSRF cookie, should set `SameSite=Strict` and `Secure=true` (in production) 8. Given setting the CSRF cookie, should not set `HttpOnly` (client must read token to submit it) 9. Given any CSRF rejection, should log the failure with request ID but without exposing token values -10. Given token comparison, should compare SHA3 hashes of tokens to prevent timing attacks +10. Given token comparison, should hash both the cookie token and the request token with SHA3 before comparing. This serves two purposes: (1) It makes the comparison robust against timing attacks even if low-level timing-safe comparison helpers or JIT optimizations are imperfect, because any change in the input completely changes the hash, preventing prefix-based guessing. (2) It keeps raw CSRF token values out of logs and error messages. ### Implementation Notes From 21ea9c26c7579932e9964dbd3a34ce9dc16c0c47 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 02:06:57 +0000 Subject: [PATCH 11/15] Add timing-safe compare security rule Add universal security rule that prohibits timing-safe compare functions on raw secret values. Instead, always hash both tokens with SHA3 before comparison. Covers all languages: Node.js, Ruby, Python, Go, Java, etc. Added to /review checklist for code review enforcement. --- ai/rules/review.mdc | 1 + ai/rules/security/timing-safe-compare.mdc | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 ai/rules/security/timing-safe-compare.mdc diff --git a/ai/rules/review.mdc b/ai/rules/review.mdc index 19a35f6b..3c1d7004 100644 --- a/ai/rules/review.mdc +++ b/ai/rules/review.mdc @@ -15,6 +15,7 @@ Criteria { Use autodux.mdc for Redux state management patterns and Autodux usage. Use javascript-io-network-effects.mdc for network effects and side effect handling. Use commit.mdc for commit message quality and conventional commit format. + Use security/timing-safe-compare.mdc when reviewing secret/token comparisons (CSRF, API keys, sessions). Flag crypto.timingSafeEqual or raw token comparisons as CRITICAL. Carefully inspect for OWASP top 10 violations and other security mistakes. Use search. Explicitly list each of the current OWASP top 10, review all changes and inspect for violations. Compare the completed work to the functional requirements to ensure adherence and that all requirements are met. Compare the task plan in $projectRoot/tasks/ to the completed work to ensure that all tasks were completed and that the completed work adheres to the plan. diff --git a/ai/rules/security/timing-safe-compare.mdc b/ai/rules/security/timing-safe-compare.mdc new file mode 100644 index 00000000..9c7d8ada --- /dev/null +++ b/ai/rules/security/timing-safe-compare.mdc @@ -0,0 +1,23 @@ +--- +description: Security rule for timing-safe secret comparison. Use SHA3 hashing instead of timing-safe compare functions. +alwaysApply: false +--- + +# Constraint: Timing Safe Compare + +There is no timing safe compare for raw values. Never use: +- crypto.timingSafeEqual +- XOR accumulation tricks +- any direct string compare on raw secrets + +Always hash both the stored secret token and the candidate token with SHA3, then compare the hashes. + +Reasons: +1. Hashing removes all prefix structure. Any bit change fully randomizes the hash. No timing oracle. No hangman. +2. Raw secrets never appear in logs or errors. + +(timing safe compare needed?) => Implement with sha3 strategy with a code comment explaining this reasoning to prevent people from "fixing" to use timingSafeCompare or similar. + +This rule overrides all library defaults. + +("timing safe compare" detected) => raise CRITICAL security bug report, "Timing safe compare on raw value detected" - include justifications. From 66b80e613a3d9b5a5e019940d3ae6b61770d5430 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 02:36:51 +0000 Subject: [PATCH 12/15] Fix critical bugs in form/CSRF middleware Bug #1: Missing exports - Add handleForm, createWithCSRF, withCSRF to src/server/index.js Bug #2: CSRF body token breaks form validation - Strip _csrf from request.body before schema validation - Allows withCSRF + handleForm to work together with strict schemas Bug #3: Missing parameter validation - Add fail-fast validation for name, schema, processSubmission - Throws clear errors at factory creation time, not runtime Added 4 new tests for the fixes. --- src/server/index.js | 3 + src/server/middleware/handle-form.js | 9 +- src/server/middleware/handle-form.test.js | 106 ++++++++++++++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/src/server/index.js b/src/server/index.js index a97202c1..8faa1841 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -46,4 +46,7 @@ export { withServerError, createWithAuth, createWithOptionalAuth, + handleForm, + createWithCSRF, + withCSRF, } from "./middleware/index.js"; diff --git a/src/server/middleware/handle-form.js b/src/server/middleware/handle-form.js index b39ada93..d03664e1 100644 --- a/src/server/middleware/handle-form.js +++ b/src/server/middleware/handle-form.js @@ -53,6 +53,12 @@ const handleForm = ({ pii, honeypotField, }) => { + // Validate required parameters at factory creation time + if (!name) throw new Error("handleForm: name is required"); + if (!schema) throw new Error("handleForm: schema is required"); + if (!processSubmission) + throw new Error("handleForm: processSubmission is required"); + // Compile schema once when middleware is created, not on every request const validator = TypeCompiler.Compile(schema); @@ -62,7 +68,8 @@ const handleForm = ({ response.locals.logger.scrub(pii); } - const body = request.body || {}; + // Strip _csrf token from body before validation (used by withCSRF middleware) + const { _csrf, ...body } = request.body || {}; // Check honeypot field if configured if (honeypotField && body[honeypotField]) { diff --git a/src/server/middleware/handle-form.test.js b/src/server/middleware/handle-form.test.js index 33ba35da..e09b044d 100644 --- a/src/server/middleware/handle-form.test.js +++ b/src/server/middleware/handle-form.test.js @@ -458,4 +458,110 @@ describe("handleForm", () => { expected: 1, }); }); + + // Req 10: Parameter validation at factory time + test("throws error when name is missing", () => { + const schema = Type.Object({ name: Type.String() }); + + let error; + try { + handleForm({ + schema, + processSubmission: async () => {}, + }); + } catch (e) { + error = e; + } + + assert({ + given: "handleForm called without name", + should: "throw error with clear message", + actual: error?.message, + expected: "handleForm: name is required", + }); + }); + + test("throws error when schema is missing", () => { + let error; + try { + handleForm({ + name: "test", + processSubmission: async () => {}, + }); + } catch (e) { + error = e; + } + + assert({ + given: "handleForm called without schema", + should: "throw error with clear message", + actual: error?.message, + expected: "handleForm: schema is required", + }); + }); + + test("throws error when processSubmission is missing", () => { + const schema = Type.Object({ name: Type.String() }); + + let error; + try { + handleForm({ + name: "test", + schema, + }); + } catch (e) { + error = e; + } + + assert({ + given: "handleForm called without processSubmission", + should: "throw error with clear message", + actual: error?.message, + expected: "handleForm: processSubmission is required", + }); + }); + + // Req 11: Strip _csrf from body before validation (for withCSRF compatibility) + test("strips _csrf field from body before validation", async () => { + const processSubmission = vi.fn().mockResolvedValue({}); + const schema = Type.Object( + { + name: Type.String(), + }, + { additionalProperties: false }, + ); + + const middleware = handleForm({ + name: "csrf-compat", + schema, + processSubmission, + }); + + const mockResponse = { + locals: {}, + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "John", _csrf: "csrf-token-value" }, + }, + response: mockResponse, + }); + + assert({ + given: "a request with _csrf field and strict schema", + should: "allow submission (strip _csrf before validation)", + actual: processSubmission.mock.calls.length, + expected: 1, + }); + + assert({ + given: "a request with _csrf field", + should: "pass body without _csrf to processSubmission", + actual: processSubmission.mock.calls[0]?.[0], + expected: { name: "John" }, + }); + }); }); From 076399a326aa502c524b6e601397d6f93bf4ae51 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 02:41:57 +0000 Subject: [PATCH 13/15] Fix file paths in review.mdc --- ai/rules/review.mdc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ai/rules/review.mdc b/ai/rules/review.mdc index 0cc74e85..8cb970ca 100644 --- a/ai/rules/review.mdc +++ b/ai/rules/review.mdc @@ -8,12 +8,12 @@ Act as a top-tier principal software engineer to conduct a thorough code review Criteria { Before beginning, read and respect the constraints in please.mdc. - Use javascript.mdc for JavaScript/TypeScript code quality and best practices. + Use javascript/javascript.mdc for JavaScript/TypeScript code quality and best practices. Use tdd.mdc for test coverage and test quality assessment. Use stack.mdc for NextJS + React/Redux + Shadcn UI architecture and patterns. Use ui.mdc for UI/UX design and component quality. - Use autodux.mdc for Redux state management patterns and Autodux usage. - Use javascript-io-network-effects.mdc for network effects and side effect handling. + Use frameworks/redux/autodux.mdc for Redux state management patterns and Autodux usage. + Use javascript/javascript-io-network-effects.mdc for network effects and side effect handling. Use commit.md for commit message quality and conventional commit format. Use security/timing-safe-compare.mdc when reviewing secret/token comparisons (CSRF, API keys, sessions). Flag crypto.timingSafeEqual or raw token comparisons as CRITICAL. Carefully inspect for OWASP top 10 violations and other security mistakes. Use search. Explicitly list each of the current OWASP top 10, review all changes and inspect for violations. From 33080d5898300c802b9440f27425a9d04abbe374 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 02:48:16 +0000 Subject: [PATCH 14/15] Fix js-sha3 import for Node.js ESM compatibility --- src/server/middleware/with-csrf.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/server/middleware/with-csrf.js b/src/server/middleware/with-csrf.js index d4fa0efe..9ec6ec5a 100644 --- a/src/server/middleware/with-csrf.js +++ b/src/server/middleware/with-csrf.js @@ -15,7 +15,9 @@ */ import { createId } from "@paralleldrive/cuid2"; -import { sha3_256 } from "js-sha3"; +import sha3 from "js-sha3"; + +const { sha3_256 } = sha3; const SAFE_METHODS = ["GET", "HEAD", "OPTIONS"]; const COOKIE_NAME = "csrf_token"; From cb12d1ce081c5ccf223ef58c4c5bb56a0ef02f4d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Dec 2025 03:03:18 +0000 Subject: [PATCH 15/15] Fix CSRF bypass allowing form processing after rejection Bug: When withCSRF rejected a request with 403, the asyncPipe pipeline continued executing subsequent middleware. handleForm would still call processSubmission() if the body passed schema validation, allowing attackers to trigger side effects (database writes, emails, etc.) despite CSRF rejection. Fix: handleForm now checks response.statusCode before processing. If a prior middleware set an error status (>= 400), it skips calling processSubmission and returns early. Added integration tests verifying: - CSRF rejection returns 403 with proper error message - processSubmission is NOT called after CSRF rejection - Valid CSRF tokens allow normal form processing --- src/server/middleware/handle-form.js | 5 + src/server/middleware/handle-form.test.js | 38 ++++- src/server/middleware/with-csrf.test.js | 175 ++++++++++++++++++++++ 3 files changed, 217 insertions(+), 1 deletion(-) diff --git a/src/server/middleware/handle-form.js b/src/server/middleware/handle-form.js index d03664e1..d05ec4ac 100644 --- a/src/server/middleware/handle-form.js +++ b/src/server/middleware/handle-form.js @@ -63,6 +63,11 @@ const handleForm = ({ const validator = TypeCompiler.Compile(schema); return async ({ request, response }) => { + // Don't process if a prior middleware already rejected the request + if (response.statusCode && response.statusCode >= 400) { + return { request, response }; + } + // Register PII fields with logger scrubber if (pii?.length && response.locals?.logger?.scrub) { response.locals.logger.scrub(pii); diff --git a/src/server/middleware/handle-form.test.js b/src/server/middleware/handle-form.test.js index e09b044d..a94d92a4 100644 --- a/src/server/middleware/handle-form.test.js +++ b/src/server/middleware/handle-form.test.js @@ -521,7 +521,43 @@ describe("handleForm", () => { }); }); - // Req 11: Strip _csrf from body before validation (for withCSRF compatibility) + // Req 11: Skip processing if prior middleware rejected request + test("skips processing if response already has error status", async () => { + const processSubmission = vi.fn(); + const schema = Type.Object( + { name: Type.String() }, + { additionalProperties: false }, + ); + + const middleware = handleForm({ + name: "after-csrf", + schema, + processSubmission, + }); + + const mockResponse = { + locals: {}, + statusCode: 403, // Prior middleware (e.g., withCSRF) rejected + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await middleware({ + request: { + body: { name: "Attacker" }, + }, + response: mockResponse, + }); + + assert({ + given: "a response with 403 status from prior middleware", + should: "not call processSubmission", + actual: processSubmission.mock.calls.length, + expected: 0, + }); + }); + + // Req 12: Strip _csrf from body before validation (for withCSRF compatibility) test("strips _csrf field from body before validation", async () => { const processSubmission = vi.fn().mockResolvedValue({}); const schema = Type.Object( diff --git a/src/server/middleware/with-csrf.test.js b/src/server/middleware/with-csrf.test.js index ff351dad..084c7eb9 100644 --- a/src/server/middleware/with-csrf.test.js +++ b/src/server/middleware/with-csrf.test.js @@ -1,6 +1,9 @@ import { describe, test, vi } from "vitest"; import { assert } from "riteway/vitest"; +import { Type } from "@sinclair/typebox"; import { withCSRF } from "./with-csrf.js"; +import { handleForm } from "./handle-form.js"; +import { asyncPipe } from "../../../utils/async-pipe.js"; describe("withCSRF", () => { // Req 1: GET/HEAD/OPTIONS sets token cookie and response.locals.csrfToken @@ -503,3 +506,175 @@ describe("withCSRF", () => { }); }); }); + +describe("withCSRF + handleForm integration", () => { + test("CSRF rejection prevents handleForm from calling processSubmission", async () => { + const processSubmission = vi.fn(); + const schema = Type.Object( + { name: Type.String() }, + { additionalProperties: false }, + ); + + const pipeline = asyncPipe( + withCSRF, + handleForm({ + name: "protected-form", + schema, + processSubmission, + }), + ); + + let statusCode = null; + let jsonBody = null; + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + status: vi.fn((code) => { + statusCode = code; + mockResponse.statusCode = code; + return mockResponse; + }), + json: vi.fn((body) => { + jsonBody = body; + }), + }; + + // POST with no CSRF token - should be rejected + await pipeline({ + request: { + method: "POST", + headers: {}, + body: { name: "Attacker" }, + }, + response: mockResponse, + }); + + assert({ + given: "a POST request without CSRF token through pipeline", + should: "return 403 status", + actual: statusCode, + expected: 403, + }); + + assert({ + given: "a POST request without CSRF token through pipeline", + should: "return CSRF error message", + actual: jsonBody?.error, + expected: "CSRF validation failed", + }); + + assert({ + given: "a POST request rejected by withCSRF", + should: "NOT call processSubmission (prevent side effects)", + actual: processSubmission.mock.calls.length, + expected: 0, + }); + }); + + test("valid CSRF token allows handleForm to call processSubmission", async () => { + const processSubmission = vi.fn().mockResolvedValue({}); + const schema = Type.Object( + { name: Type.String() }, + { additionalProperties: false }, + ); + const token = "valid-csrf-token"; + + const pipeline = asyncPipe( + withCSRF, + handleForm({ + name: "protected-form", + schema, + processSubmission, + }), + ); + + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + status: vi.fn().mockReturnThis(), + json: vi.fn(), + }; + + await pipeline({ + request: { + method: "POST", + headers: { + cookie: `csrf_token=${token}`, + "x-csrf-token": token, + }, + body: { name: "ValidUser" }, + }, + response: mockResponse, + }); + + assert({ + given: "a POST request with valid CSRF token", + should: "call processSubmission with form data", + actual: processSubmission.mock.calls[0]?.[0], + expected: { name: "ValidUser" }, + }); + + assert({ + given: "a POST request with valid CSRF token", + should: "not return error status", + actual: mockResponse.status.mock.calls.length, + expected: 0, + }); + }); + + test("mismatched CSRF token prevents side effects", async () => { + const processSubmission = vi.fn(); + const schema = Type.Object( + { name: Type.String() }, + { additionalProperties: false }, + ); + + const pipeline = asyncPipe( + withCSRF, + handleForm({ + name: "protected-form", + schema, + processSubmission, + }), + ); + + let statusCode = null; + const mockResponse = { + locals: {}, + setHeader: vi.fn(), + status: vi.fn((code) => { + statusCode = code; + mockResponse.statusCode = code; + return mockResponse; + }), + json: vi.fn(), + }; + + // POST with mismatched CSRF tokens + await pipeline({ + request: { + method: "POST", + headers: { + cookie: "csrf_token=cookie-token", + "x-csrf-token": "different-header-token", + }, + body: { name: "Attacker" }, + }, + response: mockResponse, + }); + + assert({ + given: "a POST request with mismatched CSRF tokens", + should: "return 403 status", + actual: statusCode, + expected: 403, + }); + + assert({ + given: "a POST request with mismatched CSRF tokens", + should: "NOT call processSubmission", + actual: processSubmission.mock.calls.length, + expected: 0, + }); + }); +});