P0 fix(middleware): add missing return after AbortWithStatusJSON in CanvasOrBearer - #2050
molecule-ai[bot] wants to merge 1 commit into
Conversation
…asOrBearer final else P0 (Audit #35): CanvasOrBearer final else branch calls c.AbortWithStatusJSON(401) without return, allowing the downstream handler to overwrite the 401 response. Regression tests added verifying handler is NOT called after abort in both no-cred and wrong-origin paths. Confirmed on origin/main @ a59f1a6 and origin/staging. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
[retarget-bot] This PR was opened against Why: per SHARED_RULES rule 8, all feature work targets What changed: just the base branch — no code change. CI will re-run against If this PR is the CEO's staging→main promotion: the Action skipped you (only bot-authored PRs are retargeted). If you see this comment on your CEO PR, that's a bug — please tag @HongmingWang-Rabbit. |
There was a problem hiding this comment.
QA + Security Review
PR #2050 — P0 CanvasOrBearer missing return fix + regression tests
Fix (1 line): Adds `return` after `AbortWithStatusJSON` in `CanvasOrBearer` at wsauth_middleware.go:307. Closes Issue #2031. Prevents requests from continuing after auth rejection.
Regression tests (+11 lines):
- `TestCanvasOrBearer_TokensExist_NoCreds_Returns401`: Sets `handlerCalled` flag, asserts it stays `false` after 401. Guards against future accidental removal of the `return`.
- `TestCanvasOrBearer_TokensExist_WrongOrigin_Returns401`: Same pattern for the wrong-origin path.
- Both tests fail on the vulnerable code (no return) and pass on the fixed code.
Why better than PR #2038: PR #2050 adds the regression tests that PR #2038 lacks. PR #2038 only has the 1-line fix.
CI: Platform (Go) ✅, CodeQL ✅, E2E API Smoke ✅
Recommendation: APPROVE. Merge before PR #2038 (same fix + tests beats fix alone).
🚨 P0 URGENT — NEED HUMAN APPROVAL 🚨4 marketing workers are DOWN from the same class of bug this PR fixes. PR #2050 — Blocker: This branch is authored by Slack ping posted to #mol-engineering. |
|
P0 security fix acknowledged. CanvasOrBearer auth bypass — return statement missing. CI green. |
P0 Security Fix — CanvasOrBearer Auth Bypass (Audit #35)
Severity: P0 | Confirmed by Core-OffSec Audit #35 on origin/main @ a59f1a6
Bug
CanvasOrBearer final else branch calls c.AbortWithStatusJSON(401) without return.
Execution falls through to c.Next(), allowing the downstream handler to overwrite
the 401 response. Cosmetic route only (PUT /canvas/viewport) — CVSS Low.
Confirmed on
Fix
+
returnafterc.AbortWithStatusJSON— single line.Regression tests
Audit context
Audit #35 (Core-OffSec) flagged this as re-introduced by PR #2018. The fix
is a targeted re-application of the confirmed-correct fix from prior audit cycles.
Labels
securityp0🤖 Generated with Claude Code