diff --git a/packages/api/src/lib/audit.ts b/packages/api/src/lib/audit.ts new file mode 100644 index 0000000..8d68e95 --- /dev/null +++ b/packages/api/src/lib/audit.ts @@ -0,0 +1,27 @@ +import type { Context } from "hono" + +/** + * Structured audit logging for security-relevant events (ISM-0580, ISM-0585). + * + * Never pass credentials, tokens, or unnecessary personal information in `detail` — + * records are retained and must not become a secondary disclosure path (APP 11). + */ +export type AuditEvent = + | "auth.failure" + | "auth.misconfigured" + | "user.created" + | "user.deleted" + | "post.created" + | "post.deleted" + +export function auditLog(event: AuditEvent, c: Context, detail: Record = {}): void { + const entry = { + timestamp: new Date().toISOString(), + event, + method: c.req.method, + path: new URL(c.req.url).pathname, + sourceIp: c.req.header("x-forwarded-for") ?? c.req.header("x-real-ip") ?? "unknown", + ...detail, + } + console.log(JSON.stringify(entry)) +} diff --git a/packages/api/src/middleware/auth.ts b/packages/api/src/middleware/auth.ts index dde32d9..ba701f4 100644 --- a/packages/api/src/middleware/auth.ts +++ b/packages/api/src/middleware/auth.ts @@ -1,4 +1,6 @@ import type { MiddlewareHandler } from "hono" +import { createHash, timingSafeEqual } from "node:crypto" +import { auditLog } from "../lib/audit" /** * Simple token-based auth middleware. @@ -7,24 +9,47 @@ import type { MiddlewareHandler } from "hono" * GET, POST → public (no token required) * PUT, DELETE, PATCH → require Bearer token * - * BUG: The allow-list check uses `'post'` (lowercase) instead of `'POST'`. - * HTTP methods are always uppercase per RFC 7231, so POST is never matched - * as a public method — POST requests incorrectly require a token. + * The shared static token authenticates a secret, not an identity, so it provides no + * per-actor attribution and no authorization model. See + * docs/plans/2026-08-18-api-auth-security-assessment.md (W4) — this should be replaced + * with per-identity credentials before any production use. + */ +const PUBLIC_METHODS = ["GET", "POST"] + +const BEARER = /^Bearer (\S+)$/ + +/** + * Compare two secrets in constant time (ISM-1546, CWE-208). * - * Fix: change `'post'` to `'POST'` in the public methods array. + * Both sides are SHA-256 digested first so that `timingSafeEqual` always receives + * equal-length buffers — it throws on a length mismatch, and comparing raw lengths + * would itself leak the secret's length. */ -export const authMiddleware: MiddlewareHandler = async (c, next) => { - // BUG: 'post' should be 'POST' — POST is never treated as public - const publicMethods = ["GET", "post"] +function secretsMatch(a: string, b: string): boolean { + const ha = createHash("sha256").update(a, "utf8").digest() + const hb = createHash("sha256").update(b, "utf8").digest() + return timingSafeEqual(ha, hb) +} - if (publicMethods.includes(c.req.method)) { +export const authMiddleware: MiddlewareHandler = async (c, next) => { + if (PUBLIC_METHODS.includes(c.req.method.toUpperCase())) { return next() } - const token = c.req.header("Authorization")?.replace("Bearer ", "") - if (!token || token !== (process.env.API_TOKEN ?? "test-token")) { + // Read per-request rather than at module load: there is no fail-open default, so a + // missing API_TOKEN must deny rather than fall back to a well-known value + // (ISM-1546, ISM-0421, CWE-798). + const expected = process.env.API_TOKEN + if (!expected) { + auditLog("auth.misconfigured", c, { reason: "API_TOKEN is not set" }) + return c.json({ error: "Unauthorized", status: 401 }, 401) + } + + const token = BEARER.exec(c.req.header("Authorization") ?? "")?.[1] + if (!token || !secretsMatch(token, expected)) { + auditLog("auth.failure", c, { reason: token ? "invalid token" : "missing or malformed Authorization header" }) return c.json({ error: "Unauthorized", status: 401 }, 401) } return next() -} +} \ No newline at end of file diff --git a/packages/api/src/middleware/validate.ts b/packages/api/src/middleware/validate.ts index 0a2997b..38064aa 100644 --- a/packages/api/src/middleware/validate.ts +++ b/packages/api/src/middleware/validate.ts @@ -19,3 +19,49 @@ export function requireFields(fields: string[]): MiddlewareHandler { return next() } } + +/** + * Per-field constraints for validated string input (ISM-1238, CWE-20). + */ +export type FieldRule = { + maxLength: number + pattern?: RegExp +} + +/** + * Validate that each named field is a string within bounds and matching its format. + * + * Rejects non-string types (objects, arrays, numbers, booleans) which would otherwise be + * persisted verbatim, and enforces an upper length bound so unbounded input cannot be used + * to exhaust memory. + * + * Returns an error message, or `null` when the input is acceptable. + */ +export function validateFields( + body: Record, + rules: Record, +): string | null { + for (const [field, rule] of Object.entries(rules)) { + const value = body[field] + + if (typeof value !== "string") { + return `${field} must be a string` + } + if (value.length === 0) { + return `${field} is required` + } + if (value.length > rule.maxLength) { + return `${field} must be at most ${rule.maxLength} characters` + } + if (rule.pattern && !rule.pattern.test(value)) { + return `${field} format is invalid` + } + } + return null +} + +/** Conservative single-line email shape check — deliberately not RFC 5322 exhaustive. */ +export const EMAIL_PATTERN = /^[^\s@]+@[^\s@.]+(\.[^\s@.]+)+$/ + +/** Usernames are restricted to an unambiguous, non-injectable character set. */ +export const USERNAME_PATTERN = /^[A-Za-z0-9._-]+$/ diff --git a/packages/api/src/routes/posts.ts b/packages/api/src/routes/posts.ts index 44c8c54..4d4e533 100644 --- a/packages/api/src/routes/posts.ts +++ b/packages/api/src/routes/posts.ts @@ -1,9 +1,17 @@ import { Hono } from "hono" import { db } from "../lib/db" import { notFound, badRequest } from "../lib/errors" +import { auditLog } from "../lib/audit" +import { validateFields } from "../middleware/validate" const router = new Hono() +const POST_RULES = { + authorId: { maxLength: 64 }, + title: { maxLength: 200 }, + body: { maxLength: 10_000 }, +} + router.get("/", (c) => { const authorId = c.req.query("authorId") const posts = authorId ? db.posts.findByAuthor(authorId) : db.posts.findAll() @@ -21,13 +29,19 @@ router.post("/", async (c) => { if (!body || !body.authorId || !body.title || !body.body) { return badRequest(c, "authorId, title, and body are required") } + const invalid = validateFields(body, POST_RULES) + if (invalid) return badRequest(c, invalid) + const post = db.posts.create({ authorId: body.authorId, title: body.title, body: body.body }) + auditLog("post.created", c, { postId: post.id }) return c.json(post, 201) }) router.delete("/:id", (c) => { - const ok = db.posts.delete(c.req.param("id")) + const id = c.req.param("id") + const ok = db.posts.delete(id) if (!ok) return notFound(c) + auditLog("post.deleted", c, { postId: id }) return c.json({ deleted: true }) }) diff --git a/packages/api/src/routes/users.ts b/packages/api/src/routes/users.ts index 53e605a..43efdb1 100644 --- a/packages/api/src/routes/users.ts +++ b/packages/api/src/routes/users.ts @@ -1,12 +1,16 @@ import { Hono } from "hono" import { db } from "../lib/db" -import { notFound } from "../lib/errors" -// BUG: missing import — `badRequest` is used below but not imported here. -// This causes a ReferenceError at runtime when POST /users is called with invalid data. -// Fix: add `badRequest` to the import from "../lib/errors" +import { notFound, badRequest } from "../lib/errors" +import { auditLog } from "../lib/audit" +import { validateFields, EMAIL_PATTERN, USERNAME_PATTERN } from "../middleware/validate" const router = new Hono() +const USER_RULES = { + username: { maxLength: 64, pattern: USERNAME_PATTERN }, + email: { maxLength: 254, pattern: EMAIL_PATTERN }, +} + router.get("/", (c) => { return c.json(db.users.findAll()) }) @@ -20,16 +24,23 @@ router.get("/:id", (c) => { router.post("/", async (c) => { const body = await c.req.json().catch(() => null) if (!body || !body.username || !body.email) { - // BUG: badRequest is not imported — this will throw ReferenceError return badRequest(c, "username and email are required") } + const invalid = validateFields(body, USER_RULES) + if (invalid) return badRequest(c, invalid) + + // Allow-list the persisted fields so caller-supplied `id`/`createdAt` cannot be injected; + // db.users.create overrides them a second time as defence in depth. const user = db.users.create({ username: body.username, email: body.email }) + auditLog("user.created", c, { userId: user.id }) return c.json(user, 201) }) router.delete("/:id", (c) => { - const ok = db.users.delete(c.req.param("id")) + const id = c.req.param("id") + const ok = db.users.delete(id) if (!ok) return notFound(c) + auditLog("user.deleted", c, { userId: id }) return c.json({ deleted: true }) }) diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index a2a1377..b6f7974 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -1,14 +1,10 @@ /** * Shared types used by both the API and any consumers. - * - * BUG: The field is named `userName` here but the API routes reference `username` - * (lowercase n). This causes a type error in routes/users.ts and a runtime - * mismatch when serialising responses. */ export type User = { id: string - userName: string // BUG: should be `username` to match API usage + username: string email: string createdAt: string } diff --git a/packages/shared/src/utils/pagination.ts b/packages/shared/src/utils/pagination.ts index 12f8062..eee20c2 100644 --- a/packages/shared/src/utils/pagination.ts +++ b/packages/shared/src/utils/pagination.ts @@ -6,10 +6,19 @@ import type { PaginatedResponse } from "../types" * @param items Full array of items * @param page 1-indexed page number * @param size Number of items per page - * - * TODO: implement this function — it is currently a stub. - * The test in packages/shared/test/pagination.test.ts exercises the full contract. */ export function paginate(items: T[], page: number, size: number): PaginatedResponse { - throw new Error("not implemented") + const pageSize = Math.max(1, Math.floor(size)) + const currentPage = Math.max(1, Math.floor(page)) + const total = items.length + const totalPages = Math.ceil(total / pageSize) + const start = (currentPage - 1) * pageSize + + return { + data: items.slice(start, start + pageSize), + page: currentPage, + pageSize, + total, + totalPages, + } } diff --git a/tsconfig.json b/tsconfig.json index 53de6fd..b4bf326 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -5,6 +5,7 @@ "moduleResolution": "bundler", "strict": true, "skipLibCheck": true, + "types": ["bun-types"], "paths": { "@e2e/shared": ["./packages/shared/src/index.ts"] }