Tutor questions require owned selected attempts; similarity context filters eligibility before ranking. Uploads move to a permission-aware boundary with reference ACLs, canonical legacy aliases, pre-mutation attachment checks and card-aware moderator rules. Nginx stops caching media and supplies native byte ranges. Verified 37 deployed-image backend tests, 69 frontend tests/build, real pgvector/Nginx/browser checks, and two independent reviews.
3.7 KiB
Review — follow-up validation (static, read-only)
-
Finding 1 — RESOLVED.
owns_source(backend/app/utils/upload_access.py:115–127) no longer treats every file as moderator-owned. The moderator grant is nowuser.is_moderator and not cards and path.startswith('questions/')(line 121), so a live card reference (cardsnon-empty fromcard_deck_ids) forces the card ACL path:card_access(lines 130–134) requiresuser_id == user.id/is_shared == 1/ admin — a non-owner moderator fails. Reads (can_read_upload, lines 137–152) and attachments (validate_image_attachments, lines 183–188) both fall through toreferences()only when thequestions/andimages/doc_grants do not apply, so moderator attachment of a card-only file returns 403. Admin still succeeds viaowns_source:116. An independent question reference grants moderator access becauserequire_question_access(backend/app/utils/quiz_access.py:69–90) returns for moderators, andvalidate_image_attachments:186–187usesbool(ids)for moderators. Covered bytest_card_only_moderator_denial_admin_success_and_question_grant(backend/tests/test_related_privacy.py). -
Finding 2 — RESOLVED.
stored_upload_path(upload_access.py:60–85) canonicalizes case (_authoritylowercases host), default port 80/443, query/fragment (dropped viaurlsplit), percent (unquoteon path/host), backslash (replace('\\','/')), and..(posixpath.normpath), and rejects anything that does not normalize back under/uploads/— so hostile out-of-root paths returnNone. Both classification (references/card_deck_ids/document_for_filecompare canonicalized stored values to the route path) and permission checks use this. Anonymous access hitsget_current_user("", db)→ 401 (uploads.py:37–41); owner success and peer 404 covered bytest_legacy_aliases_are_canonical_for_classification_and_permissions. New-write strictness is unchanged:validate_image_attachments:168–175rejects query/fragment,%in netloc, backslash, and bad ports on absolute URLs, andlocal_upload_path:24–31rejects\ % ? # :, control chars, and../empty parts. The image chooser dedupes viastored_upload_path(row.path) or row.pathin a set (questions.py:445–447). -
Orphan/LMS handling — RESOLVED.
protected(uploads.py:35–36) defaults non-LMS-prefix paths to private, anddocument_for_file/questions/cardsforce protection even inside a legacy prefix. Unreferencedcourse_files|course_thumbnails|scorm|certificatesfiles stay anonymous (testtest_orphaned_files...). Hostile paths can't escape:upload_file:34–47rejects symlinks and resolvesis_relative_to(root);stored_upload_pathnormalizes..before the/uploads/prefix check. -
Note (not a blocker):
owns_source:123–126grants moderators theimages/doc_N/branch without checkingcards, unlike the card-awarequestions/clause. Currently unreachable as a leak because no code path writesFlashcard.image_path(generation attasks/quiz_tasks.py:655–660sets none;CardEdithas only front/back), and moderators already hold document access. One-line hardening when cards get images:if match and not cards:. -
Residual risks (unverified by me): I ran no tests/commands/live DB — static only. Parent re-verified 37 backend tests, 69 frontend tests/build, Nginx 206/HEAD/416 with no-store/Vary/SVG sandbox, and pgvector eligibility-before-LIMIT; those remain parent-attested, not re-run here. Full-table scans in
references/card_deck_ids/document_for_fileper request are an acknowledged perf ceiling (upload_access.py:88). Thepedshub_mediacookie is not HttpOnly (deliberate,frontend/src/utils/token.js). No blockers remain for the two P1 findings.