From 3fb92dba3acb6698ff74157de14979be247994b9 Mon Sep 17 00:00:00 2001 From: Hannah Casey <61227037+hanaCasey@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:34:11 +0000 Subject: [PATCH 1/2] fix(frontend): never read an unknown membership as an organiser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The voting page decided `isOrganizer` as `!myMembership || myMembership.role === OWNER`, on the reasoning that `HackathonService.Get` admits only confirmed participants, owners and global admins — so a viewer with no membership row must be an admin looking in. That premise is the BACKEND's, and this is not the backend's view. `myMembership` is matched against `locals.platformUser`, which `hooks.server.ts` deliberately leaves unset when `WhoAmI` answers `UNAVAILABLE` (it logs "proceeding without platform user" and carries on). The gRPC channel's reconnect backoff is capped at 2s, so a backend that has come back by the time the layout issues its own `Get` serves a plain member a page with no membership row on it — and the default then handed them the entire organiser panel: open/close voting, the ballot rules, category create/edit/delete, the tally, placements, and the four ballot and result exports. Every other gate in the app fails closed by comparing to HACKATHON_ROLE_OWNER, and so does the backend's own `VoteService.isOrganizer`, whose comment says a role lookup that errors is read as "not an organizer". This one was the outlier. `mayManageVoting` joins the other gates in capabilities.ts, citing the `hackathon:write` enforcement behind each RPC in that panel, and the admin escape hatch is now stated rather than inferred from an absence. The test asserts the PROPERTY over every exported gate rather than over the one that was wrong, so a helper added later cannot reintroduce the same default silently; `mayPreferProjects` is excluded by name and is the positive control that keeps the sweep from being a list that happens to hold. Claude-Session: https://claude.ai/code/session_01HM1BPLRwr3yVMoPuqcXHJN --- .../lib/server/hackathon/capabilities.test.ts | 116 ++++++++++++++++++ .../src/lib/server/hackathon/capabilities.ts | 36 ++++++ .../my/hackathon/[id]/voting/+page.server.ts | 18 +-- 3 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 components/frontend/src/lib/server/hackathon/capabilities.test.ts diff --git a/components/frontend/src/lib/server/hackathon/capabilities.test.ts b/components/frontend/src/lib/server/hackathon/capabilities.test.ts new file mode 100644 index 00000000..2a748411 --- /dev/null +++ b/components/frontend/src/lib/server/hackathon/capabilities.test.ts @@ -0,0 +1,116 @@ +/** + * The property behind issue #182: a participant must never be shown the + * organiser's controls. + * + * Every gate in this module answers one question — "may this viewer be OFFERED + * an organiser action?" — and the answer has to be no whenever the viewer's + * membership is not known to be an owner. The `mayManageVoting` case is why + * this file exists: the voting route used to decide that a viewer with NO + * membership row was an admin looking in, reasoning that + * `HackathonService.Get` admits nobody else without one. That is true of the + * backend's view and not of the frontend's, because `myMembership` is matched + * against `locals.platformUser`, which `hooks.server.ts` deliberately leaves + * unset when `WhoAmI` answers `UNAVAILABLE` — so an absent row means "I could + * not ask", not "an admin". + * + * The sweep below is the part worth keeping. It asserts the property over + * EVERY exported gate rather than over the one that was wrong, so a helper + * added later cannot reintroduce the same default without turning this red. + * `mayPreferProjects` is excluded by name and with its reason: it is the one + * gate here that is not an organiser gate at all. + */ +import { describe, expect, it } from "vitest" +import type { HackathonMember } from "$lib/server/grpc/generated/hackathon/entities/hackathon_member" +import { HackathonRole } from "$lib/server/grpc/generated/hackathon/entities/hackathon_role" +import * as capabilities from "./capabilities" +import { + mayManagePages, + mayManageParticipants, + mayManagePhases, + mayManageTracks, + mayManageVoting, + mayPreferProjects, +} from "./capabilities" + +/** Only the two fields every gate here reads. */ +function member(role: HackathonRole, isWaiting = false): HackathonMember { + return { role, isWaiting } as HackathonMember +} + +const OWNER = member(HackathonRole.HACKATHON_ROLE_OWNER) +const MEMBER = member(HackathonRole.HACKATHON_ROLE_MEMBER) + +/** + * Every organiser gate this module exports, discovered from the module rather + * than listed by hand: a new helper joins the sweep by existing. + */ +const organiserGates = Object.entries(capabilities).filter( + ([name, fn]) => typeof fn === "function" && name !== "mayPreferProjects", +) as [string, (m: HackathonMember | undefined, isAdmin?: boolean) => boolean][] + +describe("organiser gates", () => { + it("covers every exported gate but the one that is not one", () => { + // A positive control on the sweep itself: an empty or accidentally + // filtered list would make all four assertions below vacuous. + expect(organiserGates.length).toBeGreaterThanOrEqual(5) + expect(organiserGates.map(([name]) => name)).toContain("mayManageVoting") + expect(organiserGates.map(([name]) => name)).not.toContain( + "mayPreferProjects", + ) + }) + + it.each(organiserGates)("%s refuses an unknown membership", (_name, gate) => { + expect(gate(undefined, false)).toBe(false) + }) + + it.each(organiserGates)("%s refuses a plain member", (_name, gate) => { + expect(gate(MEMBER, false)).toBe(false) + }) + + it.each(organiserGates)("%s admits the owner", (_name, gate) => { + expect(gate(OWNER, false)).toBe(true) + }) + + it.each(organiserGates)( + "%s admits a global admin who never joined", + (_name, gate) => { + // The escape hatch casbin gives an admin, and the reason "no membership + // row" was ever read as "organiser". It is stated now, not inferred. + expect(gate(undefined, true)).toBe(true) + }, + ) +}) + +describe("mayManageVoting", () => { + it("does not read an absent membership as an admin", () => { + expect(mayManageVoting(undefined, false)).toBe(false) + }) + + it("agrees with the other hackathon:write gates", () => { + // All of these mirror the same casbin rule (`hackathon:write`, granted to + // Owner and to an admin through the global escape hatch), so a viewer who + // is offered one must be offered all of them — otherwise the sidebar and + // the page it leads to can disagree about who the organiser is. + for (const m of [undefined, MEMBER, OWNER]) { + expect(mayManageVoting(m)).toBe(mayManageParticipants(m)) + expect(mayManageVoting(m)).toBe(mayManagePhases(m)) + expect(mayManageVoting(m)).toBe(mayManagePages(m)) + expect(mayManageVoting(m)).toBe(mayManageTracks(m)) + } + }) +}) + +describe("mayPreferProjects", () => { + it("is offered to a plain member, unlike the organiser gates", () => { + // The control that proves the sweep above is testing a real distinction + // rather than a list that happens to hold. + expect(mayPreferProjects(MEMBER)).toBe(true) + }) + + it("is withheld from a waitlisted member and from a non-participant", () => { + expect( + mayPreferProjects(member(HackathonRole.HACKATHON_ROLE_MEMBER, true)), + ).toBe(false) + expect(mayPreferProjects(undefined)).toBe(false) + }) +}) diff --git a/components/frontend/src/lib/server/hackathon/capabilities.ts b/components/frontend/src/lib/server/hackathon/capabilities.ts index a7be927e..21b7831a 100644 --- a/components/frontend/src/lib/server/hackathon/capabilities.ts +++ b/components/frontend/src/lib/server/hackathon/capabilities.ts @@ -126,3 +126,39 @@ export function mayManageParticipants( return membership?.role === HackathonRole.HACKATHON_ROLE_OWNER } + +/** + * Whether to show the organiser's voting panel — open and close voting, set the + * ballot rules, create and edit categories, record placements, export ballots. + * + * Mirrors the backend exactly, same as `mayManageParticipants`: every mutation + * behind that panel enforces hackathon-scoped `hackathon:write` — + * `CreateVoteCategory` (`vote_service.go:257`), `EditVoteCategory` (`:315`), + * `DeleteVoteCategory` (`:418`), `ExportVotes` (`:1071`), `CreateVoteResult` + * (`:1286`), `EditVoteResult` (`:1329`), `DeleteVoteResult` (`:1378`), + * `SuggestResults` (`:1419`), `ExportResults` (`:1593`) — which casbin grants to + * `Owner` outright and to an admin through the global escape hatch + * (`rbac.go:176`). It is the same rule `VoteService.isOrganizer` + * (`vote_service.go:791`) applies when it decides who may not cast a ballot. + * + * **Fails closed, and that is the point.** The voting route used to read + * "no membership row" as "an admin looking in", on the reasoning that + * `HackathonService.Get` admits nobody else without one. The premise holds for + * the BACKEND's view; it does not hold for the frontend's, because + * `myMembership` is matched against `locals.platformUser`, and + * `hooks.server.ts` deliberately proceeds with that unset when `WhoAmI` answers + * `UNAVAILABLE`. A backend that has come back by the time the layout issues its + * `Get` — the reconnect backoff is capped at 2s — therefore serves a plain + * member a page with no membership row on it, and the old default handed them + * the whole organiser panel. Unknown membership is now not-an-organiser, which + * is also what the backend's own `isOrganizer` does with a role lookup that + * errors. + */ +export function mayManageVoting( + membership: HackathonMember | undefined, + isAdmin = false, +): boolean { + if (isAdmin) return true + + return membership?.role === HackathonRole.HACKATHON_ROLE_OWNER +} diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/voting/+page.server.ts b/components/frontend/src/routes/(app)/my/hackathon/[id]/voting/+page.server.ts index 30072b8c..d3cc8187 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/voting/+page.server.ts +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/voting/+page.server.ts @@ -1,6 +1,7 @@ import type { Actions, PageServerLoad } from "./$types" import type { ActionFailure, Cookies } from "@sveltejs/kit" import { requireGrpc } from "$lib/server/grpc/client" +import { mayManageVoting } from "$lib/server/hackathon/capabilities" import { fail } from "@sveltejs/kit" import { ClientError, Status } from "nice-grpc-common" @@ -38,9 +39,6 @@ const SUBMISSION_STATUS_LABEL: Partial> = { const EXPORT_CSV = 1 const EXPORT_JSON = 2 -/** HackathonRole: UNSPECIFIED=0, OWNER=1, MEMBER=2 */ -const HACKATHON_ROLE_OWNER = 1 - /** Every action answers with this one shape, so `form?.x` stays typed. */ type VotingForm = { message?: string @@ -171,15 +169,17 @@ function safeName(raw: string): string { export const load: PageServerLoad = async (event) => { const { vote, team } = requireGrpc(event.locals.grpc) - const { hackathon, myMembership } = await event.parent() + const { hackathon, myMembership, isGlobalAdmin } = await event.parent() const hackathonId = event.params.id const myUserId = event.locals.platformUser?.id ?? "" - // The parent layout's Get only admits confirmed participants, hackathon - // owners and global admins — so a viewer who reached this page with no - // membership row at all is an admin looking in. - const isOrganizer = - !myMembership || myMembership.role === HACKATHON_ROLE_OWNER + // Owner-or-admin, and nothing else — see `mayManageVoting`. This used to read + // "no membership row" as "an admin looking in", which is the one gate in the + // app that failed OPEN: `myMembership` is matched against + // `locals.platformUser`, which `hooks.server.ts` leaves unset when `WhoAmI` + // answers `UNAVAILABLE`, so a plain member could be handed the whole + // organiser panel. The admin escape hatch is now stated rather than inferred. + const isOrganizer = mayManageVoting(myMembership ?? undefined, isGlobalAdmin) let categories: Category[] = [] let serviceAvailable = true From a8a5cdcd35c7d230c43f970631187a35f1bf3db9 Mon Sep 17 00:00:00 2001 From: Hannah Casey <61227037+hanaCasey@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:34:18 +0000 Subject: [PATCH 2/2] fix(frontend): drop the dead "Edit team" control from the Teams list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TeamCard drew an "Edit team" pencil on every row the viewer was a member of, and the participant-facing Teams list is the only place that card is mounted. The button carried no handler of any kind — no `onclick`, no `href`, no form — because there is no participant-side team editor: teams are edited from the organiser's Manage Teams board, which sits behind a route guard. So the one control that list offered a participant was an organiser affordance that could not have worked, which is exactly what #182 describes. `isOwn` now draws the fact it actually carries — which row is yours — worded the way the submissions page already words it, rather than being deleted along with the button and leaving a prop nothing reads. The two absence assertions in the test are paired with a positive control on the badge, so they cannot go green by the whole prop stopping at the DOM. Claude-Session: https://claude.ai/code/session_01HM1BPLRwr3yVMoPuqcXHJN --- .../lib/components/hackathon/TeamCard.svelte | 21 +++---- .../lib/components/hackathon/TeamCard.test.ts | 63 +++++++++++++++++++ 2 files changed, 73 insertions(+), 11 deletions(-) create mode 100644 components/frontend/src/lib/components/hackathon/TeamCard.test.ts diff --git a/components/frontend/src/lib/components/hackathon/TeamCard.svelte b/components/frontend/src/lib/components/hackathon/TeamCard.svelte index 14ac4a37..413b4ab0 100644 --- a/components/frontend/src/lib/components/hackathon/TeamCard.svelte +++ b/components/frontend/src/lib/components/hackathon/TeamCard.svelte @@ -1,6 +1,5 @@