Merged
Conversation
…ivity, Hackers' Choice (WS-A) Backend slice of the DevPost-replacement program: data contracts + API table, ordered tasks, and the bugs-found table (incl. the missing membership check on /devpost and /demo-video and the demo video never reaching judges). Full plan lives in frontend-ohack.dev/docs/plans/team-dashboard-devpost-replacement.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- common/utils/validators.py: normalize_deadline_iso, validate_deadlines (naive datetime -> event timezone, unknown-key rejection, ordering checks), sanitize_markdown (denylist-based, preserves generic "<"), validate_https_url, and the constraints.peer_vote_* range checks, all wired into validate_hackathon_data_partial. - services/hackathons_service.py::save_hackathon persists `deadlines`, turning an explicit null into a Firestore DELETE_FIELD on update (vs. simply omitted on create). get_single_hackathon_event strips project_story off every team in the event payload (size guard — the dashboard fetches it per-team instead). - Tests: test/common/utils/test_validators.py (+), api/messages/tests/test_hackathon_deadlines.py (new). Part of the team-dashboard-devpost-replacement plan, WS-A task 1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mentor-availability, deadline reminders; fix devpost/demo-video auth gap New self-serve, deadline-aware team writes, split from the admin-only api.teams.teams_service.edit_team path: - POST /api/team/<id>/project — partial update of project_tagline/ project_story/project_built_with/project_links/project_thumbnail_url/ project_images; sets project_submission_status=draft on first save; 400 invalid_project, 403 not_team_member, 409 submissions_closed. - POST /api/team/<id>/project/submit — requires tagline+story; idempotent; submitted|late depending on the event's submission window. - POST /api/team/<id>/mentor-availability — signal-only "open to mentors / heads-down" toggle, no deadline gate. - GET /api/hackathons/<event_id>/submissions/window — public server-clock window state for the dashboard's deadline strip. - Deadline reminders: build_reminder_message (never nags a done team), send_deadline_reminders (idempotent per event+kind+hours, only_if_due for the hourly cron), send_due_reminders_for_current_events. Security fix (Part 9 bug #1): POST /api/team/<id>/devpost and /demo-video used to call edit_team directly with NO membership check — any logged-in user could overwrite any team's Devpost link or demo video. Both now route through self_serve_team_edit, which enforces team membership (or admin) and the submission deadline. Tests: api/submissions/tests (service + view-signature + route dispatch), api/teams/tests (devpost/demo-video regression coverage). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
services/volunteers_service.py::get_volunteer_self_status mirrors
api.mentors.mentors_service.get_mentor_self_status's shape/leanness
({f"is_{type}": bool, "volunteer": {"name","isSelected"}|None}) for any
volunteer_type via the existing find_volunteer_by_caller_identity
resolver. type=mentor still delegates to the original mentor-specific
service unchanged; type=hacker is new. Backs the team dashboard's "am I
an approved hacker for this event" gate and Hackers' Choice eligibility.
Part of the team-dashboard-devpost-replacement plan, WS-A task 5.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
common/utils/github.py::get_repo_activity(org, repo) makes exactly 3 GitHub API calls (get_repo, get_commits().get_page(0), get_pulls( state="open").totalCount) and derives last-commit info, last-24h commit count, and top-8 contributors from that single commit page. A 409 "empty repository" from get_commits is a valid all-zeros result (a fresh team repo), not an error. api/github/github_service.py::get_github_activity validates org/repo names, caches successes only (5 min TTL, separate from the existing issues cache), and translates UnknownObjectException/ RateLimitExceededException into 404/503. Part 9 bug #7 fixed while touching this file: GET /api/github/issues let a request through with no `org` (200 instead of 400), and its log line printed len(issues) where `issues` is the response dict — that logged the dict's key count, not the issue count. Tests: api/github/tests (exactly-3-calls assertion via a fake Github client, 24h math, contributor cap, empty-repo, service-layer caching and error translation, the /issues and /activity route fixes). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
An assigned-slate approval vote, deliberately not a popularity contest:
each eligible voter (an isSelected hacker; optionally gated on their own
team having submitted, via constraints.peer_vote_requires_submission)
gets a deterministic, exposure-balanced slate of submitted projects
(never their own team) seeded on sha256(event_id:propel_id), and
approves up to peer_vote_max_picks of them. Scoring is the Wilson score
interval lower bound of approvals/shown (not a raw rate), so a team
shown to a handful of people isn't buried by exposure alone. No tallies
are ever shown to a voter, only to admins.
- GET .../peer-vote/slate, POST .../ballot (voter-facing, require_user)
- GET .../results, POST .../ballots/<propel_id>/void,
POST .../publish (admin) — publish appends "Hackers' Choice" to the
winning team's awards[] exactly once and writes a public summary doc
- GET .../summary (public)
constraints.peer_vote_enabled defaults to False and is checked on every
voter-facing route; a disabled/unconfigured event returns
{"status":"disabled"} from the slate route regardless of anything else.
The slate's first-ever build for a voter runs inside a Firestore
transaction that re-checks ballot existence, so a double-request can't
double-persist a slate or double-increment exposure counts.
Tests: own-team exclusion, unsubmitted/inactive teams excluded, slate
capped and exposure-balanced, repeat-GET idempotence, the full ballot
validation matrix, voided-ballot exclusion from results, Wilson lower
bound spec values, idempotent publish, summary gating.
Part of the team-dashboard-devpost-replacement plan, WS-A task 7.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No rubric/scoring/round/results logic changes. get_team_details and format_team_for_judge now return demo_video_url (the field a team's dashboard actually writes) and fill the legacy video_url key from it, so judge pages can finally show a team's demo video next to the existing GitHub/Devpost links. Bug fixes found and fixed along the way (Part 9 #3, #4): - get_bulk_judge_details always returned an empty judges list — it called an undefined name, fetch_judge_scores_by_event (the correctly imported fetch_judge_scores_by_event_id was never used), which NameError'd straight into the function's own blanket except. - update_judge_assignment_details (PUT /api/judge/assignments/<id>) always 400'd "Assignment not found" — it looked assignments up via fetch_judge_assignments_by_judge_id("") (an empty judge_id) instead of by the assignment's own id. Added fetch_judge_assignment_by_id (direct doc-get) to db/firestore.py + db/db.py and used that instead. Tests: api/judging/tests/test_format_team.py. Part of the team-dashboard-devpost-replacement plan, WS-A task 9. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- api/__init__.py: register api.submissions.submissions_views and api.peer_votes.peer_votes_views after broadcasts_views. Verified the full app (all blueprints) registers with no route conflicts (306 total routes). - .github/workflows/deadline-reminders.yml: hourly cron hitting POST /api/hackathons/deadlines/remind-due with X-Api-Key. Requires the BACKEND_CRON_TOKEN secret to be set on Fly (backend env) and in GitHub Actions — NOT done by this change; see open questions. - CLAUDE.md: new "Project submissions + Hackers' Choice (Sep 2026)" section (data contracts, endpoints, deploy-order note, documented-only Part 9 findings #5/#6/#14/#15), plus a new gotcha entry for the common.utils.slack-before-common.utils.firebase import-order requirement (load_dotenv() timing) that bit this work directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…bugs Review findings on api/peer_votes/peer_votes_service.py: - HIGH: publish_results crowned an arbitrary "winner" when 0 ballots were cast (compute_results emits a row per submitted team regardless of vote count, so the sort fell through to team name). Now 409s "no_ballots" when results["ballots"] == 0 OR the rank-1 team has 0 approvals. - HIGH: compute_results scored teams against the raw exposure-doc count (how many slates a team was ever persisted into) instead of ballots actually cast. `shown` now counts non-voided ballots that have picks and include the team in their slate; the raw count is kept separately as exposure_shown. Voiding a ballot now removes it from both shown and approvals. - LOW: compute_voting_window's end_date fallback double-appended "T23:59:59" onto an end_date that already carried a time, producing an unparseable string and silently falling back to permanently closed. Only bare dates get the suffix now. - LOW: naive or "Z"-suffixed stored deadline strings crashed compute_voting_window (TypeError/ValueError) instead of degrading to "closed"; now parsed via normalize_deadline_iso with a warning log on failure. - LOW: ballot writes (submit_ballot, void_ballot) used set(merge=True); switched to a full set() of the rebuilt doc per spec. - LOW: a voided ballot rendered status "voted" with stale picks on the voter's own slate page; now renders "voided" with picks:null. get_results now also returns ballots_detail (voter_propel_id, voted_at, voided, picks_count only — no names/emails/picks) so the admin UI can drive void. - LOW: peer_vote_max_picks could be >= slate_size if the two fields were set inconsistently across separate saves (the validator only checks the relationship within one payload); _settings() now clamps at read time. Regression tests added for every case above. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r bypasses
common/utils/validators.py::sanitize_markdown (LOW):
- Loop the tag-strip pass to a fixpoint so a nested bypass like
"<scr<script>ipt>" can't reassemble into a live tag on a single pass.
- Match on*= attributes preceded by "/" as well as whitespace, catching
"<img/onerror=...>".
- Neutralize javascript:/vbscript:/data: targets in unquoted HTML
attributes, not just quoted ones.
- Also neutralize those targets in markdown link/image syntax
("[text](javascript:...)" -> "[text](#)").
- List<String>/Map<K,V> still survive untouched; regexes stay linear.
api/submissions/submissions_service.py (HIGH/MEDIUM/LOW):
- compute_submission_window now parses stored submission/late_submission_until
strings through normalize_deadline_iso before comparing against `now` — a
naive or "Z"-suffixed stored value used to crash (TypeError/ValueError)
instead of degrading to no_deadline; unparseable values are logged and
treated as absent.
- send_deadline_reminders' only_if_due window narrowed from
[deadline-h, deadline) to a one-hour-wide [deadline-h, deadline-h+1h) —
the old window let a deadline set with only a few hours' notice fall due
for both the 24h and 6h tiers on the same cron tick and fire both at once.
- submit_project now checks the already-submitted idempotent path BEFORE
the deadline gate, so a team that submitted on time never gets a
spurious 409 revisiting the endpoint after close.
- self_serve_team_edit (the /devpost and /demo-video bridge) now also
busts the hackathons_service event cache after delegating to edit_team,
which alone only clears the generic per-function caches — fixes a stale
DevPost link/demo video on the event page for up to 10 minutes.
api/teams/teams_service.py::edit_team (MEDIUM):
- field_mappings now carries project_tagline, project_story,
project_built_with, project_links, project_thumbnail_url, project_images,
and project_submission_status, making the documented admin override path
(PATCH /api/team/edit) actually work. project_submission_status is
validated against draft/submitted/late (400 + no write otherwise);
tagline/story get the same sanitize_markdown treatment as the self-serve
save_project path; a status change stamps project_updated_at.
Regression tests added for every case above.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…minders api/messages/tests/test_cache_invalidation.py (LOW, Part 9 bug #16): test_save_hackathon_clears_cache patched 'validate_hackathon_data', which was renamed to 'validate_hackathon_data_partial' — the patch context manager raised AttributeError since hackathons_service no longer imports the old name at all. Retargeted the patch and its return value (now a (cleaned_data, skipped_fields) tuple, matching the real signature). api/messages/tests/test_hackathon_deadlines.py (LOW): added regression tests confirming save_hackathon's actual behavior for `deadlines: {}` and top-level `deadlines: null` on an update — both are a no-op (an empty map merges zero sub-fields under set(merge=True), leaving the stored deadlines map untouched), not a clear-all. CLAUDE.md / api/submissions/README.md: documented all of this session's behavior changes — admin project_* override now real and validated, the narrower reminder due-window, safe naive/"Z"-suffixed deadline parsing, the deadlines {}/null no-op semantics, ballots_detail + voided-ballot rendering, the shown vs exposure_shown split, and the sanitize_markdown hardening. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Project submissions, deadlines, mentor availability, GitHub activity, Hackers' Choice (plan + implementation)
…c without a teams[] key Hackathon docs created via the admin UI can lack the `teams` key entirely; queue_team inserted the team doc and then crashed on event_collection_dict["teams"], leaving the team orphaned from its event (seen on test.ohack.dev, event fall-2026). Linking now goes through _append_team_to_hackathon (tolerates missing key/doc, idempotent); remove_team uses the same tolerant read. Regression tests added. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 420393e)
…teams-key Fix KeyError: 'teams' when creating/removing a team on a hackathon without a teams[] key
…itHub org lookup fails POST /api/team/approve 500'd on fall-2026 (test): the event's github_org was stored as "https://github.com/Opportunity-Hack-2026" and approve_team passed it verbatim to PyGithub's get_organization -> UnknownObjectException 404, which create_github_repo did not catch (only create_repo was wrapped) and approve_team only catches ValueError. - validators.normalize_github_org(): strip https://github.com/, www., @, trailing path and whitespace (mirrors the frontend's githubOrgSlug). validate_hackathon_data_partial now stores the normalized slug, which covers both create and update since save_hackathon uses its output. - approve_team: normalize the stored value too (events saved before this fix) and return an actionable message when it is empty. - common/utils/github.py: get_github_organization() wraps the lookup and raises ValueError distinguishing 404 (bad slug / no such org) from 401/403 (token). create_github_repo resolves the org once and passes it into does_repo_exist instead of looking it up twice. - Tests for the normalizer and the partial validator. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…N prefix Three fixes found testing the team dashboard on test.ohack.dev: - get_repo_activity drops commits by GITHUB_ACTIVITY_EXCLUDED_LOGINS (default gregv). create_github_repo seeds LICENSE + README as the token owner, so every new team repo showed "2 commits, last commit by gregv" and the dashboard's "Push code to your repo" row was done before the team pushed anything. Matched on GitHub login or, for unlinked commits, the git author name; counts, last commit and contributors all exclude. - get_team is no longer @cached whole. Gunicorn runs --workers 2 and the TTL caches are per process, so a project save only busted the cache in the worker that handled it and the public team page kept showing the old story for up to 10 min about half the time. The team doc is one Firestore get and is now read fresh; only the member-profile fan-out (db.get_all over users) is cached, keyed by the tuple of member ids. - common/utils/cdn.py::cdn_server() is the single, lazily-read, normalized CDN origin (rstrip("/"), default https://cdn.ohack.dev). upload_to_cdn / generate_signed_upload_url build URLs with it and the submissions validator delegates to it, so "Thumbnail: must be an ohack CDN URL under teams/<id>/" can't come from the two sides reading CDN_SERVER differently. Rejections now log the URL + expected prefix at WARNING. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lusions-team-cache-cdn GitHub activity: exclude bootstrap account; fresh get_team; shared CDN prefix
…malization fix(teams): normalize github_org to a slug; readable error when the GitHub org lookup fails
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if not (auth_user and auth_user.user_id): | ||
| return _unauthorized() | ||
| body = request.get_json() or {} | ||
| return submit_ballot(auth_user.user_id, event_id, body.get("picks")) |
| if not is_admin(auth_user): | ||
| return _forbidden() | ||
| body = request.get_json() or {} | ||
| return publish_results(event_id, auth_user.user_id, team_id=body.get("team_id")) |
| if not (auth_user and auth_user.user_id): | ||
| return _unauthorized() | ||
| payload = request.get_json() or {} | ||
| return save_project(auth_user.user_id, teamid, payload, admin=is_admin(auth_user)) |
| only_if_due = bool(body.get("only_if_due", False)) | ||
| force = bool(body.get("force", False)) | ||
| actor = auth_user.user_id if is_admin_caller else "cron" | ||
| return send_deadline_reminders(event_id, kind, hours_before, only_if_due=only_if_due, force=force, actor=actor) |
…r sanitizer, upload gate - save_project (autosave hot path): drop the per-save Slack audit and only flush hackathon caches on the first (legacy -> draft) save. - Reminders: never write the admin's PropelAuth id onto the hackathon doc; strip reminders_sent from the public event + list payloads; normalize the stored deadline (naive/"Z" no longer 500s the cron); don't record the idempotency key when nothing was delivered; isolate per-event failures but keep the hourly job red (HTTP 500) when one fails. - sanitize_markdown: scope on*=/href=/src= scrubbing to HTML-tag spans so prose and code (`const onSubmit = ...`) is no longer silently corrupted. - GET /api/github/activity: refuse private repos (404, same as not-found). - POST /api/messages/upload-image: teams/<id>/ directories are writable only by that team's members (or admins); reject `..` and bare `teams`. Regression tests added for each. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
gregv
changed the title
Stacked on #288: review hardening (quiet autosave, no PII/private-repo leaks, safer sanitizer, upload gate)
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
2 participants
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.
Backend half of the frontend team-dashboard PR (opportunity-hack/frontend-ohack.dev#367): a team's single home that replaces DevPost, plus the Hackers' Choice peer vote. Judging itself (rubric, rounds, scoring, results) is unchanged.
What's in it
Project submissions + deadlines (
api/submissions/, new blueprint)POST /api/team/<id>/project(autosave),/project/submit,/mentor-availability; publicGET /api/hackathons/<event>/submissions/window.project_*fields, membership-gated and deadline-aware (409submissions_closed; admins bypass; already-submitted is idempotent even after close).deadlines {submission, late_submission_until, voting_opens, voting_closes}: validated, timezone-normalized, allowlisted insave_hackathon..github/workflows/deadline-reminders.yml).Hackers' Choice (
api/peer_votes/, new blueprint)slate,ballot), admin routes (results,void,publish), publicsummary.GitHub
GET /api/github/activity?org&repofor the dashboard "Code activity" card (3 GitHub calls, 5 min success-only cache, repo-bootstrap account excluded).github_orgis normalized to a bare slug on save and at approval; readable error when the org can't be found.Teams / cache / CDN
KeyError: 'teams'when creating/removing a team on a hackathon doc with noteams[].get_teamis no longer cached whole (per-worker cache made the public team page stale); only the member-profile fan-out is cached.cdn_server()is the single source for the CDN origin (fixes the "must be an ohack CDN URL" thumbnail bug).GET /api/volunteer/<event>/me?type=hackerfor the dashboard gate and vote eligibility.Security fix:
POST /api/team/<id>/devpostand/demo-videohad no membership check (any logged-in user could overwrite any team's link/video). Both now require team membership (or admin) and respect the deadline.Judging (video only + 3 pre-existing bugs): judges now get the team's
demo_video_url. Fixedget_bulk_judge_details(NameError -> always empty),update_judge_assignment_details(always "Assignment not found"), and the/issuesview (missing-org check, wrong log count). Note the first two turn on previously dead code paths.Review hardening
Found while reviewing this PR. These land via the stacked PR #289 (base
develop) - merge it to include them here; each has a regression test.save_projectno longer posts a (blocking, no-timeout) Slack audit per save or flushes the hackathon caches on every save; only the first save flushes.reminders_sent.byis now"admin"/"cron"(the real actor stays in the private Slack audit), andreminders_sentis stripped from the public event and list payloads.sanitize_markdownno longer corrupts prose/code:on*=/href=scrubbing is scoped to HTML-tag spans (const onSubmit = () => save()used to becomeconst => save())./api/github/activityrefuses private repos (same 404 as not-found)./api/messages/upload-image:teams/<id>/...directories are writable only by that team's members/admins (..and bareteamsrejected). The thumbnail validator trusts that prefix.Zstored deadlines are normalized (were a 500); nothing-delivered runs don't record the idempotency key; one bad event no longer stops the rest, and the cron returns 500 so the job goes red.Deploy notes
deadlinessilently.BACKEND_CRON_TOKEN(must match the GitHub repo secret of the same name); optionalGITHUB_ACTIVITY_EXCLUDED_LOGINS(defaultgregv),CDN_SERVER. Optional repo variableBACKEND_URLfor the cron.main.teams.project_*,hackathons.deadlines,peer_votes,hackathons/<id>/peer_vote/*); no migration or new index.Test plan (page-level, needs frontend #367 deployed)
/hack/<event>/manageteamas a team member: edit tagline/story (autosaves, "Saved"), upload a thumbnail, Submit. Reload after the deadline: still "submitted", edits blocked with the closed message.POST /api/team/<other>/devpostreturns 403./hack/<event>/team/<id>: Project section, submission tag, deep link#project./admin/hackathons/<event>?section=deadlines: set deadlines, "Send reminder now" (24h): team Slack channels get a tailored nudge; a second press 409salready_sent./hack/<event>/voteas an approved hacker: a 5-team slate without your own team, pick up to 2, re-open (same slate, picks kept), change picks.?section=judging&subtab=peer-vote: results table, void a ballot, Publish (winner gets the "Hackers' Choice" award).const onSubmit = () => save()saves and renders intact; a non-memberPOST /api/messages/upload-imagewithdirectory=teams/<other>/projectreturns 403./judge/<event>/team/<id>: demo video embeds.Not in this PR (known follow-ups)
submit_ballot/void_ballotwrite the full doc from a read (a concurrent re-vote and void can race)./api/github/activityhas no per-caller rate limit (each new org/repo pair costs 3 GitHub calls from the shared token)._append_team_to_hackathonis still read-modify-write (ArrayUnionwould be atomic).Notes for reviewers
main.yml). Run locally, per directory:ENVIRONMENT=test pytest api/submissions api/peer_votes api/github api/teams/tests api/judging/tests api/volunteers/tests test/common/utils/test_validators.py, thenapi/messages/tests/<file>individually.github_views.pyis the existingstr(e)inget_issues_api, not new.🤖 Generated with Claude Code