Stacked on #288: review hardening (quiet autosave, no PII/private-repo leaks, safer sanitizer, upload gate) - #289
Merged
Merged
Conversation
…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>
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.
Stacked on #288. The base is
develop, which is #288's head branch, so merging this updates #288 automatically. Review it as the hardening pass on #288.Fixes the highest-risk problems found reviewing #288. Each has a regression test written first (and seen failing).
Changes
save_projectno longer audits; it flushes caches only on the first (legacy -> draft) save.submit_projectstill audits.reminders_sent.bystored the admin's ID on the hackathon doc, which the unauthenticated event endpoint serves.byis"admin"/"cron"(the real actor stays in the private Slack audit).reminders_sentis stripped from the public event and list payloads (_PRIVATE_HACKATHON_FIELDS).sanitize_markdowncorrupted prose and code.const onSubmit = () => save()becameconst => save(); "online = true" lost words.on*=/href=/src=scrubbing now runs only inside HTML-tag spans. Attack strings (<img alt="a>b" onerror=...>,<svg/onload=...>, ...) are still stripped.GET /api/github/activity(unauthenticated) could read private repos (commit messages, authors, PR counts) via the shared token.PrivateRepoErrorbefore any commit/PR call and return the same 404 as not-found. Team repos are created public, so nothing legitimate changes.teams/<id>/CDN prefix was not access-controlled./api/messages/upload-imageaccepted anydirectoryfrom any logged-in user, so a stranger could overwrite another team's project thumbnail (the validator trusts that prefix).authorize_team_upload_directory:teams/<id>/...uploads need team membership or admin;..and bareteamsare rejected. Other directories unchanged. Adds ~8 lines tomessages_views.upload_image; only the project editor uploads underteams/.invalid_deadline); no key recorded when nothing was delivered; per-event failures are isolated, but the endpoint returns 500 so the hourly GitHub job still goes red.Also adds a short "don't regress" section to
CLAUDE.md.Tests
CI does not run pytest (the step is commented out in
main.yml), so these were run locally: 224 passed acrossapi/submissions api/peer_votes api/github api/messages/tests api/teams/tests(plusapi/judging/tests,api/volunteers/tests,test/common/utils/test_validators.py). New tests:test_submissions_service.py(autosave quiet, reminders, upload gate),test_validators.py(prose kept / attacks still scrubbed),test_github_activity.py(private repo),test_hackathon_public_payload.py,test_upload_image_gate.py(route-level 403/200/unaffected).Test plan
/hack/<event>/manageteamas a team member: type in the story for ~10s. It autosaves ("Saved") and the audit Slack channel gets noproject_saveposts. Submit still posts oneproject_submitaudit.const onSubmit = () => save()and "our app is online = true": it round-trips intact on/hack/<event>/team/<id>.POST /api/messages/upload-imagewithdirectory=teams/<other-team>/projectreturns 403not_team_member.GET /api/messages/hackathon/<event>after sending a reminder from?section=deadlines: response has noreminders_sent. Firestore doc showsby: "admin".GET /api/github/activity?org=<org>&repo=<a-private-repo>: 404repo_not_found; a public team repo still returns activity.Not included (listed on #288)
Re-publish leaving the old Hackers' Choice award, publish not requiring voting closed, the ballot/void write race, activity-endpoint rate limiting, and the 1-hour reminder window vs hourly cron.
🤖 Generated with Claude Code