Platform updates + security hardening (supersedes #7) - #8
Merged
Conversation
CodeQL alerts (both High): - js/incomplete-multi-character-sanitization at scripts/check-markdown-links.mjs The heading slugifier used a single-pass regex to strip HTML tags. Nested patterns like <<script>> survived one iteration. Replace with a fixpoint loop and a stricter <[^<>]*> character class. - js/clear-text-logging at scripts/identity/bootstrap.mjs Break the taint path from Microsoft Graph credential material to the top-level catch: wrap setAzdValue so the underlying value never appears in a thrown error, drop credential.keyId interpolation from the rollback error, and redact base64/base64url-looking blobs from the final stderr emission. Defence in depth also applied: - Add Content-Security response headers (X-Content-Type-Options, X-Frame-Options, Referrer-Policy, HSTS, Permissions-Policy, COOP) to every route via next.config.js, and disable x-powered-by. - Introduce src/lib/http/content-disposition.ts (RFC 6266 filename + filename*) and use it in every response that emits a Content-Disposition header so a user-controlled filename cannot inject additional headers. - Reduce PII in worker.ts stub log lines (drop recipient identifiers, subjects and free-text message bodies). Vulnerable transitive dependency: - npm audit reported nanoid@<3.3.17 (GHSA-2v37-7h3g-55p8, High) pulled in via postcss. Bump postcss to 8.5.26 and add a scoped override forcing nanoid ^3.3.17 in the postcss subtree; docx nanoid@5 is unaffected. Verification: - npm run lint, npm run typecheck, npm run docs:check, npm run validate:release, npm run test (34 tests / 10 suites, all pass). - npm audit: 0 vulnerabilities. - NODE_ENV=production npm run build succeeds; standalone server serves the six new security headers on GET /.
Independent-tester deep sweep of the whole API surface, addressing findings
from a broader review than the two CodeQL alerts alone.
## Input validation
New src/lib/http/validation.ts. Every [id] route parameter and every id
field taken from a request body now goes through assertUuid before it
reaches Prisma, so IDOR probes and injection payloads cannot touch the
database. Applied across:
documents download, documents upload, applications/{id}/answers,
documents, licence, submit; ai/licence-analysis/{id}, application-
insight (both GET and POST); admin/modules/{id}/toggle and version;
admin/policies/{id}, activate, source; ai/chat conversationId and
analysisId; register email/name/password patterns.
assertSlug guards requirementKey and sectionKey against path traversal
by rejecting anything outside a bounded slug pattern.
assertContentLength returns 413 before req.formData() / req.json() so a
malicious client cannot force the server to buffer arbitrary amounts of
memory before validation.
fileSignatureMatchesMime is a narrow magic-byte allow-list (PDF, PNG,
JPEG, GIF, WEBP, SVG, MS Word CFB, OOXML). Applied to document uploads
and PDF licence-analysis uploads so a spoofed Content-Type cannot bypass
the MIME allow-list.
Filename normalisation on the upload path strips CR/LF/NUL and any
path separators before persistence.
## CSRF (same-origin) coverage
isTrustedMutationOrigin was only wired to some admin policy routes.
Extended to every state-changing endpoint: register, upload, ai/chat,
ai/licence-analysis POST, ai/application-insight POST, applications/
{id}/answers PUT, applications/{id}/submit POST, applications/{id}/
licence POST, admin/modules/{id}/toggle POST, admin/modules/{id}/version
POST. Any request whose Origin (or Referer, as fallback) does not match
NEXTAUTH_URL is rejected with 403.
## Auth hardening
- NextAuth cookies now use httpOnly, sameSite=lax, secure in production,
and the __Host- / __Secure- prefixes; useSecureCookies is toggled by
NODE_ENV.
- Session maxAge reduced from 8h to 4h.
- Credentials.authorize always runs bcrypt.compare, even when the user
does not exist, so response timing cannot be used to enumerate accounts.
- Register endpoint: dedicated per-IP rate limit (5/min), same-origin
check, RFC-5322-ish email pattern and length cap, name allow-list,
password policy raised to 12+ chars with 3-of-4 character-class rule,
bcryptjs rounds bumped 12 -> 14, generic 202 response used regardless
of whether the address already exists (so 409 vs 201 no longer discloses
enrolment status), userId dropped from the response body, IP + user
agent captured in the audit log, and the catch block no longer echoes
raw error.message.
## Race condition on submit
Application submission now uses an optimistic updateMany filtered on
status = DRAFT and applicantId = current user, so two concurrent
submissions cannot both flip the row (409 on the second).
## Rate limiting
New src/lib/http/rate-limit.ts — in-memory fixed window with a note that
production ingress should apply a Redis-backed limit for cross-replica
enforcement. Applied to register (5/min per IP), officer chat (30/min
per user id) and applicant chat (10/min per IP). All rate-limit denials
carry Retry-After and X-RateLimit-Reset.
## AI hardening
- sanitiseUserContent strips prompt-injection markers (system/policy/
licence tags, "ignore previous instructions", CR chars) and clips oversized
input before it reaches the model, wrapped around chatOfficer,
chatApplicant, analyseLicence and assessCompliance.
- JSON.parse of AI responses now uses a reviver that drops __proto__,
constructor, and prototype keys as a prototype-pollution defence.
- ai/chat propagates only sanitised error text on failure.
- language codes must match ^[a-z]{2}(-[A-Za-z0-9]{2,8})?$.
## Response headers
next.config.js now emits a strict Content-Security-Policy in addition
to the six headers added in the previous commit (default-src self,
frame-ancestors self, form-action self, object-src none, connect-src
self, image-src limited to blob storage). Health check no longer
reveals database-connected/-disconnected state.
## Path traversal
safeRelativeCallbackUrl now double-decodes and rejects .., \, NUL and
control characters, plus explicitly rejects protocol-relative URLs.
safeAuthRedirect gained the same traversal filter.
## .env.example
Removed dangerous defaults: NEXT_PUBLIC_DEMO_MODE, NEXT_PUBLIC_SHOW_SAMPLE_
BANNER, AUTH_ENABLE_DEMO_CREDENTIALS and DEMO_PASSWORD now default to a
production-safe posture; NEXTAUTH_SECRET is blank with a generate-me
comment; the previous "change-me-in-production-use-openssl-rand-base64-32"
placeholder value would have been a valid string that started a
deployment.
## Auditable dependency posture
npm audit currently flags GHSA-2v37-7h3g-55p8 through the postcss ->
nanoid chain (postcss 8.x requires nanoid ^3.3.17, the advisory fix
range is <3.3.18 which has not been published, and postcss itself does
not exercise the vulnerable custom-generator code path). Rather than
downgrade postcss / next / next-auth, a new scripts/audit-with-allowlist
.mjs enforces npm audit --audit-level=low but records this single
advisory in a documented allow-list with an explicit removal condition.
CI now calls the wrapper (npm run audit:allowlist) instead of raw
npm audit.
## Tests
Added tests/security-helpers.test.ts covering assertUuid, assertSlug,
assertContentLength, fileSignatureMatchesMime, checkRateLimit,
requestClientAddress, safeRelativeCallbackUrl (including encoded / double-
encoded traversal), and contentDispositionHeader. Total suite now 57
tests / 18 suites, all pass.
## Live verification
Standalone production server confirmed to emit CSP + the other six
headers, register rejects mismatched Origin with 403, register rejects a
non-JSON body with 403 at the origin check, /api/health/ready returns
{"status":"ready"} only. Lint, typecheck, docs check, release check and
audit:allowlist all clean.
E2E testing of the hardening pass surfaced a read-modify-write race in
submitApplication that pre-existed on main: five concurrent POSTs to
/api/applications/{id}/submit produced five workflow_events, five audit
rows, and five workflow-engine transitions even though the row already
carried the routes optimistic updateMany.
Fix: promote the status update inside submitApplication to an
optimistic updateMany filtered on status = DRAFT. Only the first
concurrent caller flips the row; later callers now see count = 0 and
return "Application has already been submitted" without emitting a
workflow event or audit log.
Verified locally against Postgres 16:
- 5 parallel curl POSTs to submit the same application
- Before: 5 x 200 status, 5 workflow_events, 5 audit rows
- After: 1 x 200 (winner), 1 x 400 (workflow-engine loser), 3 x 409
(route-level updateMany loser); 1 workflow_event, 1 audit row, status
= UNDER_REVIEW
npm run lint, typecheck, test (57 tests / 18 suites), production build,
audit:allowlist all clean.
…nd platform updates Public-safe consolidation of the solution-accelerator work onto main. Excludes internal-only material (the pricing explorer under pitch/* and customer-specific configuration packs). Highlights: - Taxi and private hire: a seedable, activatable policy for the taxi_private_hire regime with a per-regime single-active rule, clearing the taxi policy readiness warning when taxi modules are enabled. - Setup logo: preserve transparent PNG logos (no forced white panel) and add a configurable logo scale (50-200%) with an optional white backdrop and live preview. - Secure external read-only API (/api/v1): SHA-256 hashed Bearer API keys with scopes, expiry and revocation; per-scope authorisation, per-client rate limiting and no-store responses; ADMIN-only /admin/api-access management UI with same-origin, audited create/revoke. - Supporting platform work: Statement of Licensing Policy library, licence template library, setup wizard, Microsoft Entra identity and installer refinements. - Prisma migration add_api_clients_and_logo_prefs; tests and docs updated.
Folds azure-samples/security/codeql-fixes into this branch so a single PR carries both the platform updates and the security work, superseding PR #7. Conflict resolutions (feature changes kept alongside the security fixes): - src/lib/http/rate-limit.ts: adopt the hardened limiter ({ max }, resetEpoch, requestClientAddress); update the /api/v1 caller accordingly. - ai/chat: keep multi-regime policy context; drop the old inline limiter in favour of the unified officer/applicant limiter; keep the CSRF-origin and UUID validation. - ai/application-insight, applications/licence, documents/download: keep feature behaviour and add assertUuid validation + origin checks. - next.config.js: keep config and add the Content-Security-Policy header.
Thom McKiernan (thommck)
approved these changes
Aug 24, 2026
Thom McKiernan (thommck)
left a comment
Thom McKiernan (thommck)
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A single, public-safe consolidation of the accelerator's outstanding work plus the security hardening from #7 (which this PR supersedes). Internal-only material (the pricing explorer under
pitch/*and customer-specific configuration packs) has been excluded.What's included
Features
taxi_private_hireregime with a per-regime single-active rule; clears the taxi policy readiness warning when taxi modules are enabled./api/v1): SHA-256 hashed Bearer keys with scopes, expiry and revocation; per-scope authorisation, per-client rate limiting andno-storeresponses; ADMIN-only/admin/api-accessmanagement UI with same-origin, audited create/revoke.Security (incorporates #7)
assertUuid), CSRF/same-origin checks, Content-Security-Policy, hardened rate limiting, race-safe submit, and CodeQL fixes — carried over unchanged where possible; merged with feature code where files overlapped.Supersedes
Closes #7 — its commits are included here so only one approval is needed.
Validation
npm run validate:release(public-safe) ✅security-helpers.test.ts) ✅ · production build ✅add_api_clients_and_logo_prefs; docs link check ✅