fix: 53 verified bugs (auth, security/IDOR, scoring, brackets, websocket, nav, DB)#21
Open
itkujo wants to merge 26 commits into
Open
fix: 53 verified bugs (auth, security/IDOR, scoring, brackets, websocket, nav, DB)#21itkujo wants to merge 26 commits into
itkujo wants to merge 26 commits into
Conversation
…er refill, trim CORS origins - ws/handler.go: subscribe with context.WithoutCancel so the global chimw.Timeout(60s) no longer forcibly closes every WebSocket after 60s; disconnect teardown still handled by cancelSub + read-pump done channel - middleware/rate_limit.go: credit only whole-interval refills and advance lastSeen by the consumed time, preserving sub-interval remainder so a steadily-polling client reaches the sustained rate instead of hard-locking - config/config.go: TrimSpace each CORS_ALLOWED_ORIGINS entry and drop empties so the WS origin allowlist matches CORS even with spaces after commas Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lug/email/name Soft-deleted rows kept occupying table-level UNIQUE constraints (teams.slug, venues.slug, leagues.slug, tournaments.slug, seasons(league_id,slug), divisions(tournament_id,slug), pods(division_id,name), users.email) even though the app's existence checks all filter deleted_at IS NULL, causing 23505 unique_violation on recreate/re-register. New migration 00044 drops each blanket UNIQUE and recreates it as a partial unique index scoped to live rows (matching the courts 00006 pattern), with a full goose Down to restore the originals. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…irmation windows - RefMatchConsole: gate keyboard shortcut hook on !gameOverPrompt && !matchOverPrompt so keys don't mutate match state behind the Game/Match Over modal - ScorekeeperMatchConsole: gate keyboard hook on scoring/confirm in-flight pending state and mirror confirm-pending in the scoreboard pending prop to close the auto-confirm race Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- useOverlayWebSocket: replace shared closedByUnmountRef with a per-run cancelled flag captured in the effect closure, and guard each onclose against the run-local sockets map (ws identity check). Reset state/sockets/attempts/timers maps per run. Stops torn-down runs from rescheduling connect() to a stale court/match, leaking zombie sockets, and flapping the live aggregate state on courtID/matchPublicID changes. - applyMessage overlay_data: write by prefix predicate (setQueriesData) instead of a hardcoded (token=null, demo=false) key so token-gated and demo overlays get the in-place merge with no HTTP round-trip; drop the broad invalidate that defeated the zero-fetch design. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ason check - standings: include forfeited matches in recompute (carry win/loss/points) - standings: honor admin override_points and is_withdrawn when ranking - standings: validate division belongs to the given season before recompute - registration: count approved+checked_in for division capacity, not just approved - court queue: order ListMatchesByCourt by court_queue_position so reorder takes effect Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…active users - MeJWT (/auth/me): inject OrgRoleResolver and add the Management-API fallback JWTSession uses, so genuine platform_admins no longer report role='player' and get locked out of the admin/elevated SPA. - OptionalJWT: add the same Path-2 resolver elevation so mixed-auth admin-only writes (standings recompute/override, tournament staff) authorize real platform_admins instead of 403ing them. - ValidateApiKey: reject keys owned by non-active or soft-deleted users so a suspended/banned account can't keep authenticating via a pre-existing API key. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ulk no-show, render-phase setState - useUpdateAnnouncement/useDeleteAnnouncement now invalidate ['announcements'] so league announcement feeds refresh after edit/delete - Bulk No-Show now marks every non-checked-in registration in the division (fetchAllRegistrations walks all pages) instead of only the current page - RegistrationsForDivision pushes data via useEffect, not a render-phase useMemo Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, date-only TZ
- ImageUpload: route /api/v1/uploads through apiPostForm (Bearer + X-Sport) instead of cookie auth
- VenuePicker: fetch selected venue by id via GET /api/v1/venues/{id}
- MapView: build InfoWindow content with DOM/textContent to prevent stored DOM XSS
- formatDate: parse date-only strings as local to fix US off-by-one display
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…scoped routes with correct ids - UserDetail: impersonation onSuccess navigates to /$sportSlug/dashboard (or / fallback) instead of non-existent /dashboard - RecentResults/UpcomingMatches: use typed /$sport/matches/$publicId with sport param from useSport() - MyTeams: link to /$sport/teams/$teamId using numeric team.id (not slug) - PublicDivisionDetail: bracket + matches-tab links use /$sport/matches/$publicId with 'pickleball' fallback - SearchModal: team/org results link by numeric id (backend resolves by id, not slug) - TournamentDirectory: reset pagination to page 1 on status filter change Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ic pagination - upload: derive stored file extension from validated content-type, never the client-supplied filename (prevents stored XSS via .html/.svg) - scoring presets: gate Create/Update/Deactivate/Activate behind platform_admin or tournament_director - registrations: stop leaking admin_notes on public read paths; scope all single-registration writes to the URL division and gate them by role - overlay: add CanManageCourt court-ownership check on all config writes and GetConfig; verify source-profile ownership in SetSourceProfile - public tournaments: filter to public statuses in SQL with a matching count so offset/limit and total are computed over the same set Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lost-update race, series + event validation - bracket: rebuild losers-bracket internal wiring round-by-round (no more flat-index collisions/orphaned matches); wire WB-final loser to grand finals slot 2 for the 2-team (single-WB-round) case. - engine: count SET wins (segmented by GamesPerSet/GamesToWin) for match-over so SetsToWin>1 no longer ends after the first set; fire IsEndChange exactly once per deciding-game midpoint (no double switch at 6-6). - match service: run the engine against the row-locked match inside applyEngineResult (closes the lost-update race) and snapshot the PRE-action state in the event row so Undo restores the prior score correctly. - RecordEvent: validate event_type against AllEventTypes and reject score-mutating types (routed through the engine) instead of re-implementing divergent inline scoring. - match series: RecordMatchResult now validates the child match belongs to the series and is completed, derives the winner from the child match, and recomputes wins idempotently from completed child matches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
Follow-up: 3 flagged residuals folded inAdded on top of the 53 fixes, verified green (go build/vet + logic tests + tsc):
Reviewer note: court-ownership logic is intentionally duplicated onto |
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
1 participant
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.
Automated multi-agent bug review + fixes
This PR fixes 53 confirmed bugs found by an exhaustive automated review of
feature/logto-integration(Go API + React/TS web app). Every finding was independently verified and every fix built/tested green before integration.How it was produced
Verification (integration tree)
go build ./...go vet ./...vite build(full bundle)tsc -b --noEmitHighlights
🔴 Critical
chimw.Timeout(60s)was applied to the/wsmount → every WebSocket (live scoreboards, feeds, OBS overlays) force-disconnected after 60s. Decoupled the WS subscription from the request deadline.🟠 High (26) — grouped:
/auth/meandOptionalJWTnever resolvedplatform_adminelevation (admins sawrole:"player", admin UI hidden; writes denied). Suspended/banned users kept working API keys.admin_notes; overlay-config write IDOR; registration mutations not division-scoped; upload trusted client file extension (stored XSS);MapViewInfoWindow DOM XSS;ImageUploadstill cookie-auth (401 under Logto).RecordMatchResultignoredmatchID/idempotency → inflatable series wins.$sportroute prefix or using slug where the API expects a numeric id → 404s.VenuePickerrendered "Unknown Venue".🟡 21 medium incl. 8 DB soft-delete unique-constraint bugs — fixed with one new migration
00044_partial_unique_soft_delete.sql(partial unique indexesWHERE deleted_at IS NULL; no existing migration modified). 🟢 5 low.Deploy notes
00044migration: builds partial unique indexes; will only fail if prod somehow has duplicate live rows on a constrained key (the old blanket constraint already forbade that). Dedupe live rows first if that ever happened.v1.31.0vs repov1.31.1); regenerate withv1.31.1to confirm byte-for-byte parity.Follow-ups (tracked separately, some being added to this PR)
useAuth(org isolation middleware never runs) — the cross-division IDOR is fixed at the service layer; router-level split still needed.court.goUpdateCourt/DeleteCourt miss the same ownership check now added to overlay handlers.useMatchWebSocket.tshas the same reconnect-race fixed in the overlay hook./pointduring the unconfirmed-game window from non-UI clients).🤖 Generated with Claude Code