🛡️ Sentinel: [HIGH] Fix CORS wildcard credentials vulnerability#92
🛡️ Sentinel: [HIGH] Fix CORS wildcard credentials vulnerability#92mattjoyce wants to merge 1 commit into
Conversation
…ng wildcard - Detected that when `AllowedOrigins` includes the wildcard `*`, the server was incorrectly returning `Access-Control-Allow-Credentials: true` alongside `Access-Control-Allow-Origin: *`. - This is a significant security risk (and violates the CORS specification that modern browsers enforce), as it theoretically allows any domain to perform cross-origin authenticated requests. - Updated `corsMiddleware` in `internal/api/server.go` to explicitly set `Access-Control-Allow-Credentials: false` when `allowAll` (`*`) is enabled. Exact origin matches continue to permit credentials. Co-authored-by: mattjoyce <278869+mattjoyce@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR addresses a CORS security issue by detecting wildcard origins in the allowlist and conditionally disabling credentials. The middleware now sets ChangesCORS Wildcard and Credentials Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Pure resolver (config -> ResolvedWorker, fail-closed on undefined grant) and a pure credential builder kept separate from the Setpgid lifecycle (Hickey A1). Threaded through the executor and spawnPlugin; unconfined is a no-op, a confined worker on a non-Unix platform fails the spawn closed. Unit-tested on darwin; the privileged EACCES wall test is linux+root-gated and validated on the Dell host.
…#92) Validated on a privileged Linux host (Dell, root golang:1.25 container): the dropped worker gets EACCES on the 0600 age key. t.TempDir nests under 0700 root parents the worker can't traverse, so the probe never ran; a single 0755 dir under /tmp fixes it. Wall test now PASS on privilege, skips cleanly elsewhere.
🚨 Severity: HIGH
💡 Vulnerability: The API server's CORS configuration allowed the wildcard
*inAllowedOriginswhile simultaneously settingAccess-Control-Allow-Credentials: true.🎯 Impact: This is a significant security risk because it attempts to allow any website to make authenticated cross-origin requests, potentially exposing sensitive data or actions to unauthorized domains. While modern browsers block this specific combination, misconfigured or non-standard clients might still exploit it, and it represents a fundamentally insecure server posture.
🔧 Fix: Updated
corsMiddlewareininternal/api/server.go. When the wildcard*is present in theallowedOriginslist, the middleware now explicitly setsAccess-Control-Allow-Origin: *and forcesAccess-Control-Allow-Credentials: false. For explicitly listed origins (no wildcard), the behavior remains intact: the specific origin is allowed and credentials are permitted.✅ Verification: Tested the
internal/apipackage viago test -v ./internal/api/...which successfully ran the integration tests.PR created automatically by Jules for task 16077443379988911092 started by @mattjoyce
Summary by CodeRabbit
Bug Fixes
Documentation