Conversation
…dcard subdomain origins Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add configurable allowed headers and origins to CORS middleware
feat(cors): configurable allowed headers, credentials, maxAge, and wildcard subdomain origins
Mar 11, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the HTTP CORS middleware to support additional configuration (allowed headers, credentials, max-age, and wildcard subdomain origins) and updates unit/e2e tests accordingly, along with some minor formatting-only cleanup in unrelated module tests/types.
Changes:
- Extend
CORSMiddlewareto accept a full config object (allowedHeaders, allowCredentials, maxAge) and add wildcard subdomain matching for origins. - Update the HTTP plugin CORS middleware factory to build and pass the new CORS config.
- Add unit + e2e coverage for the new CORS behaviors.
Reviewed changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/http/modules.go | Updates CORS middleware factory to build CORSMiddlewareConfig and pass headers/credentials/maxAge. |
| module/http_middleware.go | Adds CORSMiddlewareConfig, wildcard origin matching, configurable allowed headers, credentials, and max-age response header support. |
| module/http_middleware_test.go | Adds unit tests covering allowedHeaders defaults, allowCredentials, maxAge, and wildcard subdomains. |
| e2e_middleware_test.go | Adds an end-to-end test verifying factory-driven full CORS config behavior. |
| module/scan_provider_test.go | Formatting-only changes (gofmt alignment). |
| module/platform_do_database.go | Formatting-only changes (comment alignment). |
| module/platform_do_database_test.go | Formatting-only changes (map literal alignment). |
Comment on lines
+389
to
+400
| func corsOriginAllowed(origin string, allowedOrigins []string) bool { | ||
| for _, allowed := range allowedOrigins { | ||
| if allowed == "*" || allowed == origin { | ||
| return true | ||
| } | ||
| // Wildcard subdomain matching: "*.example.com" matches "sub.example.com" | ||
| if strings.HasPrefix(allowed, "*.") { | ||
| suffix := allowed[1:] // ".example.com" | ||
| if strings.HasSuffix(origin, suffix) { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
corsOriginAllowed does suffix matching against the full Origin header value (e.g. http://app.example.com:3000). This means patterns like *.example.com will not match origins that include a port, and the logic isn't robust to URL parsing/normalization. Consider parsing Origin with url.Parse and doing wildcard checks against u.Hostname() (and optionally validating scheme/port), while still using the full origin string for the exact-match case.
Comment on lines
408
to
+416
| origin := r.Header.Get("Origin") | ||
|
|
||
| // Check if origin is allowed | ||
| allowed := false | ||
| for _, allowedOrigin := range m.allowedOrigins { | ||
| if allowedOrigin == "*" || allowedOrigin == origin { | ||
| allowed = true | ||
| break | ||
| } | ||
| } | ||
| allowed := corsOriginAllowed(origin, m.allowedOrigins) | ||
|
|
||
| if allowed { | ||
| w.Header().Set("Access-Control-Allow-Origin", origin) | ||
| w.Header().Set("Access-Control-Allow-Methods", strings.Join(m.allowedMethods, ", ")) | ||
| w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization") | ||
| w.Header().Set("Access-Control-Allow-Headers", strings.Join(m.allowedHeaders, ", ")) |
There was a problem hiding this comment.
corsOriginAllowed returns true for allowedOrigins containing "*" even when the request has no Origin header (i.e., origin == ""). In that case the middleware sets Access-Control-Allow-Origin to an empty value, which is an invalid CORS response. Consider short-circuiting CORS handling when Origin is empty (only apply CORS headers when the request includes an Origin header).
| corsCfg.AllowCredentials = allowCreds | ||
| } | ||
| if maxAge, ok := cfg["maxAge"].(int); ok { | ||
| corsCfg.MaxAge = maxAge |
There was a problem hiding this comment.
maxAge is only read when the config value is an int. Elsewhere in this file you accept both int and float64 for numeric config values (e.g. cacheMaxAge, burstSize, requestsPerMinute), and other parts of the repo parse maxAge as float64 (e.g. plugins/api/plugin.go). Consider accepting float64 here too (and casting to int) so JSON-decoded / UI-provided configs work consistently.
Suggested change
| corsCfg.MaxAge = maxAge | |
| corsCfg.MaxAge = maxAge | |
| } else if maxAgeFloat, ok := cfg["maxAge"].(float64); ok { | |
| corsCfg.MaxAge = int(maxAgeFloat) |
Comment on lines
175
to
180
| func corsMiddlewareFactory(name string, cfg map[string]any) modular.Module { | ||
| allowedOrigins := []string{"*"} | ||
| allowedMethods := []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"} | ||
| corsCfg := module.CORSMiddlewareConfig{ | ||
| AllowedOrigins: []string{"*"}, | ||
| AllowedMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"}, | ||
| } | ||
| if origins, ok := cfg["allowedOrigins"].([]any); ok { |
There was a problem hiding this comment.
This factory now supports allowedHeaders, allowCredentials, and maxAge, but the HTTP plugin’s module schema for http.middleware.cors still only lists allowedOrigins/allowedMethods (see plugins/http/schemas.go). Because the generated workflow JSON Schema sets config.additionalProperties=false per module type, these new keys won’t be represented/allowed in generated schemas and editor tooling. Please update the CORS module schema fields/defaults to include the new config options (and ensure any related docs/UI schema stay in sync).
Contributor
|
@copilot apply changes based on the comments in this thread Build failing, lint failing, test failing, validate configs failing |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…gin guard, port-aware wildcard, float64 maxAge, schema update Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Contributor
Author
All review comments addressed in commit
|
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
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.
http.middleware.corshad hardcodedAccess-Control-Allow-Headers: Content-Type, Authorizationwith no support forallowCredentials,maxAge, or wildcard subdomain origin matching — blocking real-world browser apps that send CSRF tokens, cookies, or custom headers.Changes
module/http_middleware.goCORSMiddlewareConfigstruct (AllowedOrigins,AllowedMethods,AllowedHeaders,AllowCredentials,MaxAge)NewCORSMiddlewareWithConfig(name, CORSMiddlewareConfig)constructor;NewCORSMiddlewaredelegates to it (backward compat: defaults headers to[Content-Type, Authorization])corsOriginAllowedhelper: supports"*", exact match, and"*.example.com"wildcard subdomain patternsProcess: uses configurable headers, emitsAccess-Control-Allow-Credentials: trueandAccess-Control-Max-Agewhen configuredplugins/http/modules.gocorsMiddlewareFactoryparses new config keys:allowedHeaders,allowCredentials,maxAgeUsage
Wildcard subdomain matching (
*.example.com) matches any direct subdomain. WhenallowCredentials: true, the actual requestOriginis always reflected in the response (never*).Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.