feat(file_storage): adopt MultiTenantMixin on StoredFile (#383) [stack 9/11] - #393
Merged
Merged
Conversation
StoredFile is now tenant-scoped: get/download/delete of another tenant's id
404s, listings, facets and bucket totals count only the active tenant, and the
unique key index widens to (tenant_id, key) (SM024 clean). New storage keys are
prefixed with {tenant_id}/; existing rows keep their stored key. The owning
tenant is settled before any bytes reach the backend, so a strict install with
no tenant fails closed without orphaning an object.
The browse-screen AggregateCache is keyed by tenant; a commit drops only the
slots of the tenants it wrote, plus the unscoped slot.
Permissions: upload/download on tenant member/admin/owner, delete on tenant
admin/owner only. The platform `user` role keeps upload/download but no longer
carries delete (every account holds it, which would hand delete back to every
member). The Files menu entry is visible to tenant roles.
Platform files: branding uploads, serves and reaps its images with
platform=True — owned by PLATFORM_TENANT_ID (= DEFAULT_TENANT_ID), looked up
under all_tenants() but restricted to that owner. Anonymous logo/favicon routes
keep working with no tenant bound, and a branding setting pointed at a
tenant's file id 404s instead of publishing it. The audit-log label resolver
names files across tenants deliberately (platform screen; entries already
record the filename).
Migration c7f2d9a41e83 (extends mainline b5d3f08a6e17): add tenant_id
nullable, backfill DEFAULT_TENANT_ID, NOT NULL, swap unique index, add
tenant_id index; downgrade reverses.
Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
Deploying simple-module-python with
|
| Latest commit: |
3e0e4d1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d2e08c46.simple-module-python.pages.dev |
| Branch Preview URL: | https://tenancy-09-file-storage.simple-module-python.pages.dev |
…m scope narrow (review of #383) (a) PLATFORM_TENANT_ID = "platform" in simple_module_db, distinct from DEFAULT_TENANT_ID. is_valid_tenant_id refuses it, so nothing can bind it; HostSettings.default_tenant and tenants (slug, derived slug, Tenant.id) refuse it too. Migration c7f2d9a41e83 is edited in place - it is unreleased, no deploy has run it - to back-fill only the files the SYSTEM-scope branding settings reference into the platform owner and everything else into DEFAULT_TENANT_ID. (b) platform_scope flushes the session's pending writes before entering all_tenants() and disables autoflush inside, so unrelated writes are still stamped/guarded with the bound tenant. delete() runs read+write in one scope. owning_tenant falls back to the install's fallback tenant like the flush guard. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
…stalls (ship review) With multi_tenant off the install is the only tenant, so on_startup maps file_storage.delete onto the platform user role (as dashboard does). With it on the grant stays on the organisation-admin roles. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
# Conflicts: # modules/tenants/tenants/models.py
antosubash
added this pull request to stack #397
October 1, 2026 16:14
antosubash
marked this pull request as ready for review
October 1, 2026 16:16
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. |
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.
Closes #383 (which replaces #361). Stack 9/11 of the tenancy-adoption series (base: #392). This is the first core module to adopt
MultiTenantMixin.What
StoredFileadoptsMultiTenantMixin. The unique index is now(tenant_id, key)(SM024 is clean).c7f2d9a41e83: adds the column as nullable, backfills it withDEFAULT_TENANT_ID, makes it NOT NULL, swaps the unique index and adds atenant_idindex. The downgrade reverses all of it. Both directions are tested against a real Alembic run on SQLite with existing rows.{tenant_id}/YYYY/MM/DD/<uuid><ext>. Existing rows keep their keys, which hold the full object path. The owning tenant is decided before the bytes are written, so a strict-mode upload with no tenant fails without leaving an orphaned object.getis now a tenant-filteredselectinstead ofdb.get, which avoids the identity-map caveat. Cross-tenant get, download and delete return 404, and bulk delete skips ids the caller can't see.file_storage/scope.py,platform=True): owned by the default tenant. Branding uses them, which keeps anonymous logo, dark-logo and favicon fetches working. Only rows whose owner is the platform are readable this way, and the file id comes from SYSTEM-scope settings, never from the request. This is a stopgap until branding goes per-tenant in layer 11.all_tenants=True), because that screen is a platform screen.Permissions. Tenant
membergets upload and download; tenantadminandowneralso get delete. The platformuserrole losesfile_storage.delete, because every account holdsuserand keeping it would hand delete to every tenant member. This affects single-tenant installs too: there, only admins can delete.Tests
defaultmake test-js: 463 passedmake lintandmake doctor: cleanmake test-py-pg) has not been run yet.Follow-ups
users_access_token.expires_athas SQLite drift. It predates this change and is unrelated.https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV