security: restrict CORS to explicit allowed origins#661
Open
mmcintosh wants to merge 2 commits intoSonicJs-Org:mainfrom
Open
security: restrict CORS to explicit allowed origins#661mmcintosh wants to merge 2 commits intoSonicJs-Org:mainfrom
mmcintosh wants to merge 2 commits intoSonicJs-Org:mainfrom
Conversation
mmcintosh
force-pushed
the
hotfix/cors-configuration
branch
from
661a4d3 to
5e92ace
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| it('valid key with matching scope — calls next()', async () => { | ||
| const hash = await hashApiKey(TOKEN) |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
| }) | ||
|
|
||
| const mw = requireApiKey('search:read') | ||
| const result = await mw(ctx, mockNext) |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
- Replace wildcard origin:'*' with dynamic CORS_ORIGINS check - No CORS_ORIGINS env var = reject all cross-origin requests (secure default) - Add CORS_ORIGINS to Bindings interface - Add X-API-Key to allowed headers - Add CORS_ORIGINS=http://localhost:8787 to dev wrangler.toml configs - Same-origin requests (admin UI) are unaffected Breaking: cross-origin API consumers must be listed in CORS_ORIGINS Fixes VULN-003
Tests were sending Origin headers (localhost:3000, example.com) that don't match the CORS_ORIGINS allowlist. Updated to use http://localhost:8787 and assert the echoed origin instead of wildcard '*'.
mmcintosh
force-pushed
the
hotfix/cors-configuration
branch
from
5e92ace to
cd6828d
Compare
mmcintosh
changed the title
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
1 participant
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.
Security: Restrict CORS to Explicit Allowed Origins
Summary
Replaces the permissive
Access-Control-Allow-Origin: *CORS policy with an explicit allowlist via theCORS_ORIGINSenvironment variable. Cross-origin requests from unlisted origins are now rejected.Changes
1. Origin Allowlist
origincallback parsesCORS_ORIGINSenv var (comma-separated)CORS_ORIGINSis not set, all cross-origin requests are rejected2. Additional Allowed Header
X-API-KeytoallowHeadersso API key authentication works cross-origin3. Default Configuration
my-sonicjs-app/wrangler.tomlsetsCORS_ORIGINS = "http://localhost:8787"for local devpackages/create-app/templates/starter/wrangler.toml) includes the same default4. E2E Tests Updated
Origin: http://localhost:8787(the configured origin)*to the echoed origin value07-api.spec.ts,08-collections-api.spec.ts, andsmoke.spec.tsTechnical Details
Core Changes:
packages/core/src/routes/api.ts-- CORSorigincallback with allowlist parsingpackages/core/src/app.ts-- AddedCORS_ORIGINS?: stringtoBindingsinterfaceConfiguration:
my-sonicjs-app/wrangler.toml-- AddedCORS_ORIGINS = "http://localhost:8787"packages/create-app/templates/starter/wrangler.toml-- Same default for new projectsUpdated Tests:
tests/e2e/07-api.spec.ts-- CORS test uses configured origintests/e2e/08-collections-api.spec.ts-- Updated assertion from*to echoed origintests/e2e/smoke.spec.ts-- Updated CORS smoke testTesting
Performance Impact
No measurable impact -- origin check is a simple string split + includes.
Breaking Changes
CORS_ORIGINS. This is intentional -- the previous*policy was a security risk.Migration Notes
CORS_ORIGINSto your frontend domain(s), e.g.,CORS_ORIGINS = "https://mysite.com,https://admin.mysite.com"http://localhost:8787Known Issues
None.
Demo / Screenshots
N/A -- no UI changes.
Related Issues
(Security hardening -- no linked issue)
Checklist