Skip to content

fix: restrict CORS to trusted origins via ALLOWED_ORIGINS (#184) - #220

Open
cass-clearly wants to merge 1 commit into
mainfrom
fix/restrict-cors-origins-184
Open

fix: restrict CORS to trusted origins via ALLOWED_ORIGINS (#184)#220
cass-clearly wants to merge 1 commit into
mainfrom
fix/restrict-cors-origins-184

Conversation

@cass-clearly

Copy link
Copy Markdown
Owner

What changed

  • server/index.js: Replaced open cors() with origin-restricted config using ALLOWED_ORIGINS env var
  • docker-compose.remarq.yml: Passes ALLOWED_ORIGINS to the server container (defaults to http://localhost:3333)
  • README.md: Added Environment Variables table documenting DATABASE_URL, PORT, and ALLOWED_ORIGINS

Why

Closes #184. The server used app.use(cors()) with no configuration, allowing requests from any origin. This is a security risk in production — CORS should restrict allowed origins to trusted domains.

How to verify

# Default (no env var) — only localhost:3333 allowed
node server/index.js

# Custom origins
ALLOWED_ORIGINS=https://example.com,https://app.example.com node server/index.js

# Verify CORS headers
curl -i -H "Origin: https://evil.com" http://localhost:3333/health
# → No Access-Control-Allow-Origin header

curl -i -H "Origin: http://localhost:3333" http://localhost:3333/health
# → Access-Control-Allow-Origin: http://localhost:3333
# → Access-Control-Allow-Credentials: true

Manual testing checklist

  • Existing tests pass (npm run test:server — 135 pass, 98.63% coverage)
  • Lint passes (npm run lint)
  • Format passes (npm run format:check)

🤖 Generated with Claude Code

Closes #184. The server previously used `cors()` with no config,
allowing requests from any origin. Now reads `ALLOWED_ORIGINS`
(comma-separated) from the environment and defaults to
`http://localhost:3333`. Credentials are enabled.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cass-clearly

Copy link
Copy Markdown
Owner Author

Review: Approved

CORS restriction correctly implemented. Changes:

  • Reads ALLOWED_ORIGINS env var (comma-separated)
  • Defaults to http://localhost:3333
  • credentials: true for cookies/auth
  • Documented in README + docker-compose

All tests pass. No issues found.

@cass-clearly cass-clearly left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix: restrict CORS to trusted origins

Verdict: Approve (posted as comment because GitHub blocks self-approval)

What's Good

  • Clean, minimal change — single line in server/index.js that does exactly what the issue asks
  • ALLOWED_ORIGINS env var with sensible default (http://localhost:3333) means zero friction for local dev
  • Docker Compose pass-through is correct and won't break existing deployments (default matches what they're already running)
  • README env variable table is a nice improvement — fills a gap in the docs
  • credentials: true is the right pairing with restricted origins for future cookie-based auth (#41)

Observations (non-blocking)

  1. Wildcard behavior: If someone sets ALLOWED_ORIGINS=*, the current code creates origin: ['*'] (an array), which cors doesn't treat as a wildcard — it would block all origins. Could add a note in the README or a guard, but edge case behavior is fine to leave for a follow-up.
  2. Origin validation: No input validation on the env var value, but that's consistent with how the rest of the server config works.

This is exactly the right scope for a security fix. Approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configure CORS to restrict allowed origins

1 participant