Repository navigation
fix: tenancy-adoption review follow-ups (permissions editor, branding reaper) - #404
Merged
Merged
Conversation
- permissions: keep code-registered role grants when the editor saves a role (separate DB overlay on PermissionRegistry, replaced not popped) - branding reaper: commit the soft-delete before dropping the backend object - branding: retrieve failed in-flight read exceptions (in-flight map is already per-app) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying simple-module-python with
|
| Latest commit: |
9b76eff
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://04bbb542.simple-module-python.pages.dev |
| Branch Preview URL: | https://fix-396-tenancy-review-follo.simple-module-python.pages.dev |
Owner
Author
/ship results (review + QA + local CI)Pushed: 9b76eff (test-only tidy: removed a stray Code review (2 passes, sonnet): 1 fixed. Accepted/not changed (design or out of scope):
QA (live app, fresh SQLite DB, API 8100 / Vite 5250):
Local checks: |
antosubash
added a commit
that referenced
this pull request
Oct 7, 2026
antosubash
added a commit
that referenced
this pull request
Oct 7, 2026
…, thumbnails (#410) * feat(file_storage): public files with anonymous serving, list search/sort, thumbnails Fixes #353, fixes #352. - StoredFile.public (default false; migration uses sa.false()), set on upload (public=true) or PATCH /files/{id}; StoredFileOut gains public/public_url. - Anonymous GET /public/{id}[/{filename}] and /public/{id}/thumbnail, exempted via register_public_routes (GET only). Only public, non-deleted rows resolve; other cases are one 404. Cross-tenant lookup by id uses all_tenants(). ETag/304, Cache-Control public, nosniff, sandbox CSP; active content (HTML, SVG, JS) forced to attachment and streamed. - SecurityHeadersMiddleware keeps a CSP the response already set. - GET /files: q, content_type (exact or "image/" prefix), sort. - GET /files/{id}/thumbnail?w=: Pillow WebP, width snapped to a whitelist, pixel-budget guard, variants cached in the storage backend and dropped with the file. - Browse screen: Public badge and make public/private action. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): harden thumbnails (format allowlist, size/pixel caps, concurrency) and add tenant/permission tests Pillow only opens JPEG/PNG/WEBP/GIF, sniffed format must match the declared type, DecompressionBombWarning is an error, source bytes are capped while reading, decodes are bounded by a semaphore, first frame only, metadata not carried over. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): hold the decode slot until the worker thread finishes; stop mutating Pillow's global pixel cap Decode runs as a shielded single-flight task per variant, so a cancelled request neither frees its slot early nor lets retries multiply decodes. The semaphore is per event loop. The explicit header-only pixel budget replaces the racy process-global MAX_IMAGE_PIXELS toggle. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * refactor(optimize): dedupe 404/headers helpers, drop dead code and one-use wrappers in file_storage Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: address code review findings (round 1, pass 1) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: address code review findings (round 1, pass 1b) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): no session cookie on public responses, strict PATCH bool, 16-bit grayscale thumbnails (QA round 1) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): log orphaned thumbnail variant deletes (review round 1, pass 3) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * test: reconcile file_storage/branding tests with #401 and #404 on main Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(migrations): chain file_storage public column after #406's timestamp revision Both branched from c7f2d9a41e83; repoint down_revision so main keeps a single mainline head. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * feat(file_storage): own rate-limit bucket for anonymous public file GETs With #408's shared limiter on main, public file reads shared the 120/minute anonymous default; a page embedding many images would throttle itself. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
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.
Fixes #396
PermissionRegistrynow keeps DB-sourced (admin-editor) grants in a separate overlay (set_role_overlay, a replace) merged with code mappings (map_role) inrole_map.set_role_permissions,load_all_into_registryandsync_admin_all_permissionsuse it, so saving a role no longer pops code grants (users.self_profile, file_storage,dashboard.view) and removed DB keys still drop immediately. Tests:test_permissions_role_overlay.py.FileStorageService.deletegainsdrop_object=True(default behaviour unchanged) and a publicdrop_object(row). The reaper soft-deletes, commits, then drops the object; a failed commit leaves row and bytes intact, a failed drop only orphans an object. Tests:test_branding_reap_ordering.py; two existing fakes updated for the new kwarg.TenantCache.inflightonapp.state.branding; no module-global_INFLIGHTremains), so only the second half needed work: the shared read's done-callback now retrieves the exception, so a failed read whose waiters were all cancelled no longer logs "exception was never retrieved". Test included.Verification:
make lintexit 0;make test-py3576 passed, 9 skipped. Does not touch #398's files beyond permissions/service.py (different hunks).https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4