fix(middleware): add missing return after AbortWithStatusJSON in CanvasOrBearer - #2032
Closed
molecule-ai[bot] wants to merge 1 commit into
Closed
fix(middleware): add missing return after AbortWithStatusJSON in CanvasOrBearer#2032molecule-ai[bot] wants to merge 1 commit into
molecule-ai[bot] wants to merge 1 commit into
Conversation
…asOrBearer CRITICAL — Core-OffSec Audit #32 (re-confirmed by App-QA at 15:30Z). CanvasOrBearer: c.AbortWithStatusJSON(401, ...) had no trailing return, so execution fell through into the downstream handler (vh.Save for /canvas/viewport), which overwrote the 401 with its own 200 response. Impact: cosmetic-only route today (viewport drag state), but the same pattern would be a real bypass if CanvasOrBearer is applied more widely. All other AbortWithStatusJSON calls in wsauth_middleware.go correctly return — this was the only missing one. Fix: one-line return after the abort. Regression tests added to both NoCreds and WrongOrigin test cases, explicitly asserting the handler body does NOT appear in the final response (body == `{"ok":true}` fails the test). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
[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. |
Contributor
Author
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…t only (#2032 CR2) Agent-reviewer flagged that the previous commit treated EVERY status==="failed" as transient (log+poll until timeout). This masks real boot regressions (image pull errors, panics, PYTHONPATH issues, quota failures) by blurring them into a generic polling-timeout error. Fix: only tolerate the pre-start credential-abort shape (uptime_seconds===0 AND no last_sample_error). All other failed states immediately hard-throw with boot_stage / last_error / image detail, as the original code did. Also updates comments to match the narrowed behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
Hermes cold-boot can exceed the bootstrap-watcher deadline, setting status=failed prematurely; heartbeat later recovers to online. Instead of hard-throwing on the first 'failed' sighting, log a warning and retry. Genuine terminal failures still surface via the waitFor timeout. Fixes #2032
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
Hermes cold-boot can exceed the bootstrap-watcher deadline, setting status=failed prematurely; heartbeat later recovers to online. Instead of hard-throwing on the first 'failed' sighting, log a warning and retry. Genuine terminal failures still surface via the waitFor timeout. Fixes #2032
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
The status-pagination bug (RCA, #2440-family): merge/verify status readers fetched only the FIRST page of a commit's statuses. On high-churn PRs Gitea caps the combined GET /commits/{sha}/status `statuses` array at the default page size (~30) and pushes older-but-still-current required-context rows past it. A reader of that truncated view records the required context as ABSENT (missing) even though its current SUCCESS row exists — wrongly blocking, or mis-reading the gate. Confirmed on #2448/#2426/#2438/#2331/#2259/#2055/#2032 (reviewers had to manually paginate to verify gates this whole session). Live proof on PR #2331 head: combined /status returns 30 rows; exhaustive /statuses returns 50 rows across 20 distinct contexts. Two verify-by-state readers consumed that capped combined view for required-context decisions and are fixed here to page the dedicated /commits/{sha}/statuses list to EXHAUSTION (until a short/empty page), then collapse to newest-row-per-context: - prod-auto-deploy.py (wait-ci gate): replaced the single combined /status fetch with fetch_all_statuses() (paginated). A required context past page 1 no longer reads "missing" forever and times out a legitimate prod deploy. latest_status_for_context now selects newest-by-id so the oldest-first /statuses ordering can't let a stale run shadow the current one. - audit-force-merge.sh: replaced the single combined /status fetch with a page loop over /commits/{sha}/statuses, accumulating all rows before the newest-wins CHECK_STATE collapse. A required SUCCESS past the cap no longer reads "missing" and emits a false-positive incident.force_merge. gitea-merge-queue.py already paginates /statuses to exhaustion (get_combined_status + api_paginated) — left unchanged; it is the reference behavior this change brings the other two readers in line with. STRENGTHENING ONLY — fail-closed preserved, NO fail-open path introduced: - prod-auto-deploy: a genuinely-absent required context appears on NO page, so ci_context_state() still returns "missing", context_is_satisfied() rejects it, and the gate never greens (times out). Any page that errors or is not a list raises (fetch_all_statuses/_api_json_list) — a partial list never passes as complete. - audit-force-merge: any non-200 page or non-array body aborts with exit 1; an absent required context has no CHECK_STATE entry so `${...:-missing}` keeps it not-green and the audit still fires. Tests (mutation-resistant): added regressions that (a) place a required SUCCESS on page 2+ behind a full page of churn and assert the reader FINDS it, and (b) make a required context genuinely absent on all pages and assert the reader STILL fail-closes (missing/never-satisfied → blocks/times out). Mocks the paginated HTTP responses. Also locks newest-wins collapse, short-page stop, full-page continue, and page-error propagation. Refs: status-pagination RCA, #2440-family. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
CRITICAL (Audit #32): missing return after c.AbortWithStatusJSON(401, ...) in CanvasOrBearer — requests fall through to downstream handler which overwrites 401 with 200. CVSS Low (cosmetic route only). Fix: +1 line return. Regression tests added.