fix(csrf): return 403 on CSRF rejection instead of silent empty 200#2692
fix(csrf): return 403 on CSRF rejection instead of silent empty 200#2692elibosley wants to merge 1 commit into
Conversation
When local_prepend.php rejects a POST for a bad/missing CSRF token it logged the error and `exit`ed with no status code, so the client received an empty HTTP 200. A fetch-based caller can't tell that apart from a genuine empty result — e.g. the Unraid API Status panel renders "Not Running" (service down) when the request was actually just blocked by the CSRF gate. Return an explicit 403 with a small JSON body so the rejection is observable and can't masquerade as a real response. Validation logic is unchanged — the gate still rejects and still logs; only the response shape of a rejected request changes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe CSRF termination helper now responds with HTTP 403 and a JSON body containing the failure reason, while setting the JSON content type when headers are available. ChangesCSRF response handling
Estimated code review effort: 1 (Trivial) | ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
1 participant
Problem
When
local_prepend.phprejects a POST for a missing/invalid CSRF token, it logs the error andexits with no status code — so the client receives an empty HTTP 200. A fetch/XHR caller can't distinguish that from a genuine empty result.Concretely: Settings → Management Access → Unraid API Status issues a fetch POST to
unraid-api.php. When that POST is blocked by the CSRF gate, the empty 200 is read as "the service returned nothing" and the panel renders Not Running — even though the API is up and the request was simply blocked. (Full first-paint reproduction and the client-side fix are in unraid/api#2039.)Change
csrf_terminate()now returns an explicit 403 with a small JSON body ({"error":"<reason> csrf_token"}) instead of a silent empty 200.Validation logic is unchanged — the gate still rejects the same requests and still logs the same
wrong csrf_tokenline. Only the response shape of an already-rejected request changes, so a blocked read is observable and can't masquerade as a real (empty) response. A legitimate request carrying a valid token never reaches this path.headers_sent()is guarded so this is a no-op if output already began.Testing
php -lclean.Note / possible follow-up
The token is read as
$_POST['csrf_token'] ?? $_SERVER['HTTP_X_CSRF_TOKEN'].??only falls through onnull, not on an empty string, so a caller that sends a blankcsrf_token=body field overrides a validx-csrf-tokenheader. If we want the header to win in that case,($_POST['csrf_token'] ?? '') ?: ($_SERVER['HTTP_X_CSRF_TOKEN'] ?? null)would do it without weakening validation. Left out of this PR to keep the change minimal — happy to add if desired.Summary by CodeRabbit