Merge pull request 'Scope CORS to browser routes only, fail closed on CORS_ORIGIN' (#4) from mailship-32x-cors-is-not-production-safe-wildcard-ori-52b into main

Reviewed-on: #4
Reviewed-by: matt <matt@lorentz.is>

.env.template

@@ -12,6 +12,10 @@ BRAND_LOGO=
 INDEXER_RELAYS=purplepag.es,relay.damus.io,relay.nostr.band
 DEFAULT_RELAYS=relay.damus.io,nos.lol
 SEARCH_RELAYS=relay.nostr.band
+# CORS_ORIGIN must be set to the exact origin your browser client runs on
+# (e.g. https://app.example.com). There is no wildcard fallback — the server
+# will refuse to start without it.
+CORS_ORIGIN=
 POSTMARK_API_KEY=
 POSTMARK_SENDER_ADDRESS=
 PORT=4738

src/env.ts

@@ -16,6 +16,7 @@ if (!process.env.DEFAULT_RELAYS) throw new Error('DEFAULT_RELAYS is not defined.
 if (!process.env.INDEXER_RELAYS) throw new Error('INDEXER_RELAYS is not defined.')
 if (!process.env.SEARCH_RELAYS) throw new Error('SEARCH_RELAYS is not defined.')
 if (!process.env.PORT) throw new Error('PORT is not defined.')
+if (!process.env.CORS_ORIGIN) throw new Error('CORS_ORIGIN is not defined.')
 if (!process.env.BASE_URL) throw new Error('BASE_URL is not defined.')
 
 export const MAILSHIP_URL = process.env.MAILSHIP_URL
@@ -30,6 +31,7 @@ export const appSigner = Nip01Signer.fromSecret(process.env.MAILSHIP_SECRET)
 export const DEFAULT_RELAYS = process.env.DEFAULT_RELAYS.split(',').map(normalizeRelayUrl)
 export const INDEXER_RELAYS = process.env.INDEXER_RELAYS.split(',').map(normalizeRelayUrl)
 export const SEARCH_RELAYS = process.env.SEARCH_RELAYS.split(',').map(normalizeRelayUrl)
+export const CORS_ORIGIN = process.env.CORS_ORIGIN
 export const PORT = process.env.PORT
 export const SMTP_HOST = process.env.SMTP_HOST
 export const SMTP_PORT = process.env.SMTP_PORT

src/server.ts

@@ -1,7 +1,7 @@
 import { instrument } from 'succinct-async'
 import express, { Request, Response, NextFunction } from 'express'
 import rateLimit from 'express-rate-limit'
-import { appSigner, BRAND_NAME, BRAND_ACCENT, BRAND_LOGO, EVENT_VIEWER_URL } from './env.js'
+import { appSigner, BRAND_NAME, BRAND_ACCENT, BRAND_LOGO, EVENT_VIEWER_URL, CORS_ORIGIN } from './env.js'
 import { render } from './templates.js'
 import { confirmSubscriptionAction, unsubscribeAction, registerSubscription, ActionError } from './actions.js'
 import { getSubscriptionById, insertEvent, getSubscriptionByKey, getSubscriptionByPubkey } from './database.js'
@@ -13,23 +13,31 @@ import { verifyEvent } from 'nostr-tools/pure'
 
 export const server: express.Application = express()
 
+// CORS middleware for browser-facing routes only.
 // The browser hits /subscription with an Authorization header and Content-Type:
 // application/json, which triggers a CORS preflight. Answer it and allow the
 // configured origin so the client can register.
-const corsOrigin = process.env.CORS_ORIGIN ?? '*'
+// Server-to-server routes (/notify) intentionally do NOT get CORS headers.
+const corsMiddleware = (req: Request, res: Response, next: NextFunction) => {
+  // Skip server-to-server routes
+  if (req.path.startsWith('/notify')) {
+    return next()
+  }
 
-server.use((req: Request, res: Response, next: NextFunction) => {
-  res.setHeader('Access-Control-Allow-Origin', corsOrigin)
+  res.setHeader('Access-Control-Allow-Origin', CORS_ORIGIN)
   res.setHeader('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE,OPTIONS')
   res.setHeader('Access-Control-Allow-Headers', 'Content-Type,Authorization')
   res.setHeader('Access-Control-Max-Age', '86400')
+  res.setHeader('Vary', 'Origin')
 
   if (req.method === 'OPTIONS') {
     return res.sendStatus(204)
   }
 
   next()
-})
+}
+
+server.use('/', corsMiddleware)
 
 server.use(express.json())
 

test/cors.test.sh

@@ -0,0 +1,156 @@
+#!/usr/bin/env bash
+# Passing test: CORS is scoped to browser routes only, no wildcard default.
+#
+# The fix: src/env.ts now requires CORS_ORIGIN (fail closed, no wildcard).
+# src/server.ts applies CORS middleware only to browser-facing routes
+# (/, /subscription/*, /confirm, /unsubscribe) and adds Vary: Origin.
+# Server-to-server routes (/notify) get no CORS headers.
+
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+PROJECT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
+PORT="${PORT:-4742}"
+BASE_URL="http://localhost:$PORT"
+PASS=0
+FAIL=0
+
+GREEN='\033[0;32m'
+RED='\033[0;31m'
+NC='\033[0m'
+
+cleanup() {
+  kill "$SERVER_PID" 2>/dev/null || true
+  wait "$SERVER_PID" 2>/dev/null || true
+  rm -rf "$PROJECT_DIR/test-data-cors"
+}
+trap cleanup EXIT
+
+# Build if needed
+cd "$PROJECT_DIR"
+if [ ! -d "dist" ]; then
+  pnpm exec tsc
+fi
+
+SECRET="$(openssl rand -hex 32)"
+
+# Set CORS_ORIGIN to a specific allowed origin (fail closed — must be set)
+export CORS_ORIGIN="https://app.example.com"
+export MAILSHIP_SECRET="$SECRET"
+export MAILSHIP_NAME="Mailship Test"
+export MAILSHIP_URL="$BASE_URL"
+export BASE_URL="$BASE_URL"
+export POSTMARK_API_KEY="test"
+export POSTMARK_SENDER_ADDRESS="test@test.com"
+export DEFAULT_RELAYS="wss://relay.damus.io"
+export INDEXER_RELAYS="wss://purplepag.es"
+export SEARCH_RELAYS="wss://relay.nostr.band"
+export PORT="$PORT"
+export DATA_DIR="$PROJECT_DIR/test-data-cors"
+# SMTP env vars (required by src/env.ts)
+export SMTP_HOST="localhost"
+export SMTP_PORT="1025"
+export SMTP_USER="test"
+export SMTP_PASSWORD="test"
+export SMTP_FROM="test@test.com"
+
+mkdir -p "$DATA_DIR"
+
+echo "=== Starting server on port $PORT (CORS_ORIGIN=$CORS_ORIGIN) ==="
+node dist/index.js &
+SERVER_PID=$!
+
+# Poll until server responds
+for i in 1 2 3 4 5 6 7 8 9 10; do
+  if curl -sf "http://localhost:$PORT/" > /dev/null 2>&1; then
+    echo "Server ready after ${i}s"
+    break
+  fi
+  sleep 1
+done
+
+if ! kill -0 "$SERVER_PID" 2>/dev/null; then
+  echo -e "${RED}Server failed to start${NC}"
+  exit 1
+fi
+
+echo ""
+echo "========================================="
+echo "  CORS TESTS"
+echo "========================================="
+echo ""
+
+pass() {
+  PASS=$((PASS + 1))
+  echo -e "  ${GREEN}✓${NC} $1"
+}
+
+fail() {
+  FAIL=$((FAIL + 1))
+  echo -e "  ${RED}✗${NC} $1"
+}
+
+# Test 1: Browser-facing GET /subscription/email returns the configured origin
+echo "1. CORS on GET /subscription/email"
+CORS_HEADER=$(curl -s -o /dev/null -D - \
+  "http://localhost:$PORT/subscription/email?pubkey=test_pubkey_123" \
+  -H "Origin: https://app.example.com" 2>/dev/null | grep -ia 'access-control-allow-origin' || true | head -1 | tr -d '\r')
+
+if echo "$CORS_HEADER" | grep -q 'https://app.example.com'; then
+  pass "subscription/email returns configured origin (not wildcard)"
+elif echo "$CORS_HEADER" | grep -q '\*'; then
+  fail "BUG: subscription/email still returns wildcard '${CORS_HEADER}'"
+else
+  fail "subscription/email missing Access-Control-Allow-Origin (got: ${CORS_HEADER:-<none>})"
+fi
+
+# Test 2: Vary: Origin is present on browser routes
+echo ""
+echo "2. Vary: Origin header on browser route"
+VARY=$(curl -s -o /dev/null -D - \
+  "http://localhost:$PORT/" \
+  -H "Origin: https://app.example.com" 2>/dev/null | grep -ia 'vary' || true | head -1 | tr -d '\r')
+
+if echo "$VARY" | grep -qi 'origin'; then
+  pass "Vary: Origin is present on GET /"
+else
+  fail "Missing Vary: Origin on GET / (got: ${VARY:-<none>})"
+fi
+
+# Test 3: Server-to-server /notify has NO CORS headers (relay callback)
+echo ""
+echo "3. No CORS on server-to-server POST /notify/:id"
+CORS_NOTIFY=$(curl -s -o /dev/null -D - \
+  "http://localhost:$PORT/notify/test-id" \
+  -X POST -H "Content-Type: application/json" \
+  -H "Origin: https://evil.com" \
+  -d '{"id":"abc","relay":"wss://relay.primal.net"}' 2>/dev/null | grep -ia 'access-control-allow-origin' || true | head -1 | tr -d '\r')
+
+if [ -z "$CORS_NOTIFY" ]; then
+  pass "/notify has no Access-Control-Allow-Origin (server-to-server route)"
+else
+  fail "BUG: /notify returns '${CORS_NOTIFY}' — server-to-server route should have no CORS"
+fi
+
+# Test 4: CORS methods on preflight for subscription route
+echo ""
+echo "4. CORS preflight on PUT /subscription/email"
+METHODS=$(curl -s -o /dev/null -D - \
+  "http://localhost:$PORT/subscription/email" \
+  -X OPTIONS -H "Origin: https://app.example.com" -H "Access-Control-Request-Method: PUT" 2>/dev/null | grep -ia 'access-control-allow-methods' || true | head -1 | tr -d '\r')
+
+if echo "$METHODS" | grep -qi 'PUT'; then
+  pass "OPTIONS preflight returns allowed methods"
+else
+  fail "Preflight missing allowed methods (got: ${METHODS:-<none>})"
+fi
+
+echo ""
+echo "========================================="
+echo "  RESULTS: $PASS passed, $FAIL failed"
+echo "========================================="
+
+if [ "$FAIL" -gt 0 ]; then
+  exit 1
+fi
+exit 0