fix(backend): close D3 (registration-form self-path) and D6 (attachment window bypass) - #194
Merged
Merged
Conversation
resolveRegistrationTarget checked Hackathon:Write for the on_behalf_of path but returned the caller unconditionally on the self path. So any authenticated user could POST SubmitRegistrationForm for ANY hackathon — a private one they were never invited to included — which both wrote a FormResponse row into that event and turned the validation errors into a form-schema oracle (they name missing and unknown fields). Require a Participant row on the self path. Join is what writes it, and a private event's Join requires an invite, so a participant row is the proof the caller belongs in this form. Waitlisted participants pass — they are exactly who still needs to submit or correct their answers. The check runs before the form is loaded, so a non-participant is refused before any schema detail leaks. Pins it: registration_gate_test.go asserts a non-participant gets PermissionDenied and a waitlisted joiner passes the gate (reaching the form-not-defined FailedPrecondition — a different code is the proof).
…indow (D6) The SUBMISSION_ATTACHMENT presign path checked casbin Submission:Write but not the submissions window or the create_project_submissions capability — the two gates every other submission write goes through (CreateSubmission, EditSubmission, FinalizeSubmission in team_service.go). So a team could presign and upload an attachment after the deadline closed, or while the submit capability was off, slipping work in past the window the submission handlers enforce. Add the same requireWindowOpen(windowSubmissions) + requireCapability( CreateProjectSubmissions) calls the submission handlers use, on the same hackathon id already resolved from the submission's team.
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.
Two backend authorization holes from the audit, same shape as the D1/D2 fix already merged.
D3 — registration-form self-path had no gate
resolveRegistrationTargetrequiredHackathon:Writefor theon_behalf_ofpath but returned the caller unconditionally on the self path. Any authenticated user couldSubmitRegistrationFormfor any hackathon — a private one they were never invited to — which (a) wrote aFormResponserow into that event and (b) turned the validation errors into a form-schema oracle (they name missing/unknown fields).Fix: require a
Participantrow on the self path (Join writes it; a private event's Join needs an invite). Waitlisted participants pass — they're exactly who still needs their form. The check runs before the form is loaded, so a non-participant is refused before any schema detail leaks.Pinned:
registration_gate_test.go— non-participant →PermissionDenied; waitlisted joiner → passes the gate (reaching the form-not-definedFailedPrecondition, a different code = proof). This handler had zero prior tests.D6 — attachment upload escaped the submissions window + capability
The
SUBMISSION_ATTACHMENTpresign checked casbinSubmission:Writebut notrequireWindowOpen(windowSubmissions)orrequireCapability(CreateProjectSubmissions)— the two gatesCreateSubmission/EditSubmission/FinalizeSubmissionall enforce. So a team could upload attachments after the deadline, or while the submit capability was off.Fix: add the same two calls the submission handlers use, on the hackathon id already resolved from the submission's team.
Verification (devcontainer, fast tier)
go build ./...cleango test -tags "test unittest" ./internal/service/... ./internal/middleware/...— OK (D3's 2 new specs pass)gofmt -lemptyFollow-up (noted, not in this PR)
A dedicated D6 attachment-window integration test — the fix reuses the exact gate functions CreateSubmission's tests already exercise, but a call-site test would pin the wiring. The submission chain has no reusable setup helper (~70 lines inline per test), so it's deferred rather than bloating this PR.