Skip to content

fix: split CORS policy across public, credentialed, and admin routes - #316

Open
bamiebot-maker wants to merge 3 commits into
Divinity-Labs:mainfrom
bamiebot-maker:fix/cors-policy-split
Open

bamiebot-maker wants to merge 3 commits into
Divinity-Labs:mainfrom
bamiebot-maker:fix/cors-policy-split

Conversation

@bamiebot-maker

Copy link
Copy Markdown

Summary

Splits the blanket \origin: true\ CORS configuration into three explicit security tiers:

  1. Public Reads (\public): Stays open to any origin with explicit \credentials: false, ensuring public market rates, health checks, and discovery are world-accessible without risking ambient credential leakage.
  2. Credentialed Routes (\credentialed): Narrows policy on /usage/me\ and any requests carrying \Authorization\ / \Cookie\ headers to explicitly configured origins (\CORS_ALLOWED_ORIGINS) with explicit \credentials: true.
  3. Admin Routes (\�dmin): Closed to cross-origin browsers by default (\origin: false, \credentials: false) with optional override via \CORS_ADMIN_ALLOWED_ORIGINS\ for internal operator tools.

Also documents the full route classification matrix in \docs/routes.md\ and adds comprehensive test coverage in \src/tests/cors.test.ts\ for allowed and refused origins across all classes.

Related issue

Closes #268

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Docs
  • Tests
  • CI / tooling

Checklist

  • I have read CONTRIBUTING.md

  • px tsc --noEmit\ passes

  • pm run build\ passes
  • I added / updated tests where relevant
  • I updated docs where relevant

Closes Divinity-Labs#268

- Implement dynamic CORS delegator classifying routes into public, credentialed, and admin tiers
- Keep public reads open to any origin with explicit credentials: false
- Restrict credentialed routes (/usage/me and requests carrying auth credentials) to CORS_ALLOWED_ORIGINS with credentials: true
- Close operator admin endpoints (/admin/*) against cross-origin browser requests by default
- Document route security matrix and CORS policies in docs/routes.md and README.md
- Add unit and integration tests covering allowed and refused origins on each class
@drips-wave

drips-wave Bot commented Oct 16, 2026

Copy link
Copy Markdown

@bamiebot-maker Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Miracle656

Copy link
Copy Markdown
Collaborator

One line from green. The drift guard is the only thing failing:

Found 1 environment variable(s) read in source but missing from .env.example and not in allow-list:
  - CORS_PUBLIC_BLOCKED_ORIGINS (read at src/cors.ts:120)

Add it to .env.example with a comment saying what it does and what the format is (comma-separated origins, presumably, and what an empty value means). The guard exists because a variable that only lives in someone's shell is a variable the next operator does not know to set.

Splitting CORS across public, credentialed and admin surfaces is the right shape — a single permissive policy in front of /admin/* is a real problem — so I want this. I will review it properly on the next green run.

@bamiebot-maker

Copy link
Copy Markdown
Author

Hi @Miracle656, thanks for the review! I have added \CORS_PUBLIC_BLOCKED_ORIGINS\ to .env.example\ with documentation describing its format (comma-separated origins) and behavior (empty/unset permits all public cross-origin requests). \envDrift.test.ts\ and all 20 CORS tests are now green.

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.

One CORS policy covers public reads and authenticated routes alike

2 participants