Lane 8 PR 5: handouts, private files, rules' Make it ours (#734) #893
Loading…
Reference in a new issue
No description provided.
Delete branch "build/lane8-5-handouts"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Part of #734 (lane 8), §3.8 and §3.10. Stacked on #890 (PR 4). Server and bot; no pages.
Migration
8a7b8c9d0e1f_handouts_reveals(on8f6a)lore_attachments(image, PDF or text; random path; PDF page count).lore_entries.source_reference_entry_id, for "Make it ours".shelf_revealsgains:shelf_reveal_audience_people);session_idbecomes nullable (W8).Every column is
ADD/DROP COLUMN IF EXISTS, so either order with lane 10's10b2on the same table works.CHECK constraints:
ck_notifications_kindgainshandout_shownandck_notifications_subject_typegainslore_entry. Both keep the union of what came before.Up → down → up passes with seeded rows, including a reveal made outside play, which the downgrade deletes before restoring
NOT NULL.One cross-migration test fix: PR 3's parties downgrade drops
shelf_reveals.audience_party_idif it's present.Private files (W7): the security surface
Storage:
media/private/campaigns/{cid}/files/{uuid}.{ext}. It's outside anything the public/media/location will serve:location ^~ /media/private/ { return 404; }.Download:
GET /api/campaigns/{cid}/files/{id}. The backend authorises every request:It then answers
X-Accel-Redirect: /internal-files/.... That nginx location isinternaland aliases onlyprivate/.Cache-Control: private, no-storefor anything narrower than the table.Proven against a real frontend-prod image:
/media/private/...URL/internal-files/...directlyTests:
private/;Uploads: magic-byte validation, PDFs checked with
pypdf(new dependency,pypdf==5.1.0), and images re-encoded.(This was caught in review: the first version stored files under the public
/media/alias.)The rest
handout_shownas a bell kind (categorywiki), whose sentence carries the title only, never the body.GET /api/bot/files/{id}(bot-auth), plus the bot's handler. Additive; the contract stays at v2.POST /reference/{id}/make-ours(GM-only) creates a GM-only page linked to its source.Departures and follow-ups
session_id: nullfor a reveal made outside play./media/lore/{id}/image.*. That predates this PR, and it's filed separately.Checks
webapp/backendalone.changelog.d/fragment.🤖 Generated with Claude Code
Server: attachments (lore_attachments), PDF validation via magic bytes and pypdf page counts, the private files route (X-Accel-Redirect + nginx's internal location, W7), reveals with audience/page/nullable session (W8), handout_shown bell kind, and reference/{id}/make-ours. Bot: GET /api/bot/files/{id} (additive, contract stays v2) and the handout_shown channel/DM handler. Migration 8a7b8c9d0e1f on 8f6a7b8c9d0e; up/down/up proven on real rows. Targeted backend suite green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>files_service stored attachments at media_root/campaigns/{cid}/files/..., a subtree nginx's public `location /media/` (Cache-Control: public, max-age=31536000, immutable, no auth) already serves whole. The uuid name was no protection: a revoked reveal's link kept working forever, and anyone the URL reached could read the file with no backend check at all. Move every attachment one level down, to media_root/private/..., which `location ^~ /media/private/ { return 404; }` refuses outright, independent of and in addition to nginx's `internal` location (now aliased into the same private/ subtree) that already gated the X-Accel-Redirect path. LoreAttachment.path is unaffected (still campaign/kind-shaped, no schema change); files_service.absolute_path and internal_redirect_path are now the only two places that add the private/ prefix back on. Proven against a real frontend-prod image + a stand-in backend: a direct GET on the public /media/private/... path 404s, GET /internal-files/... directly still 404s, the X-Accel-Redirect path still serves the file at 200 with the backend's own Cache-Control intact, and an ordinary public /media/ picture is unaffected. New tests assert save_attachment never writes outside media_root/private, that absolute_path/internal_redirect_path agree with it, and (skipped outside a full checkout) that nginx.conf carries both halves of the fix. Page pictures aren't stored privately by any path today, so item 5 of the review doesn't apply yet — flagged for whichever PR does that. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>GET /api/sessions/{id}/revealed is readable by every member and listed every live shelf_reveals row on the session. Lane 8 PR 5 put handout reveals, each with an audience of its own, on that same table, so a handout shown to one player, or to the GMs only, had its title listed for the whole table. tests/wiki/test_handouts_api.py reproduced it (marcus saw a reveal chosen for priya) before the fix. * shelf_service.list_reveals now takes a required viewer and keeps a handout reveal only for the people in its audience and the GMs, through handout_service.reveal_visible_to: the rule may_download already applied to the file, which now uses the same predicate. A shelf card's reveal is still the whole table's. This follows the survey's "revealed through the existing shelf-reveal mechanism" (WIKI-14/TABLE-1): handouts stay in the session's list for their audience, rather than being taken out of it. * A handout reveal's session_id must be one of the campaign's own sessions (422 otherwise), or a GM could list a title on another table's page. * 8a7b8c9d0e1f's downgrade deletes every handout reveal (lore_entry_id set), not only sessionless ones: the older schema has no reveal audience and would show the title to everyone. * 8a7b8c9d0e1f creates the three indexes the model declares and the migration lacked (ix_shelf_reveals_lore_entry_live, partial on revoked_at IS NULL, and the people table's user_id/character_id), and drops them on the way down. A test takes the migration down and up and holds every index on both tables to the model's definitions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>