From 6cd5d3becf023d592198d5410f4f7ee21099dece Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Mon, 19 Oct 2026 09:59:19 +0200 Subject: [PATCH 1/6] fix(file_storage): show owner-scoped callers their own usage, not the tenant's Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../file_storage/endpoints/views.py | 8 +++- modules/file_storage/file_storage/queries.py | 8 ++++ modules/file_storage/file_storage/reads.py | 4 ++ .../tests/test_file_storage_p3_scope_usage.py | 46 +++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 modules/file_storage/tests/test_file_storage_p3_scope_usage.py diff --git a/modules/file_storage/file_storage/endpoints/views.py b/modules/file_storage/file_storage/endpoints/views.py index ca218c1d8..1b63a9ed9 100644 --- a/modules/file_storage/file_storage/endpoints/views.py +++ b/modules/file_storage/file_storage/endpoints/views.py @@ -91,8 +91,12 @@ async def browse( # stays None until an operator says what the ceiling is — the # screen reports usage rather than inventing a total. "backend": settings.backend, - "used_bytes": aggregates.used_bytes, - "quota_bytes": settings.quota_bytes, + # The quota measures the tenant, so only callers who see the whole + # tenant get it; everyone else sees their own usage, no ceiling. + "used_bytes": ( + aggregates.used_bytes if owner is None else await service.used_bytes_by(owner) + ), + "quota_bytes": settings.quota_bytes if owner is None else None, "max_file_size_bytes": settings.max_file_size_bytes, # Above this the page uploads in resumable parts; null when the # backend cannot (a third-party one without multipart support). diff --git a/modules/file_storage/file_storage/queries.py b/modules/file_storage/file_storage/queries.py index 9ee747497..68566ac99 100644 --- a/modules/file_storage/file_storage/queries.py +++ b/modules/file_storage/file_storage/queries.py @@ -161,6 +161,14 @@ async def content_type_facets(db: AsyncSession, *, created_by: str | None = None return [{"value": str(row[0]), "count": int(row[1])} for row in rows] +async def used_bytes_by(db: AsyncSession, owner: str) -> int: + """Total size of one uploader's files, summed in the database.""" + query = select(func.coalesce(func.sum(StoredFile.size_bytes), 0)).where( + StoredFile.created_by == owner + ) + return int((await db.execute(query)).scalar_one()) + + def public_url_for(file_id: object, filename: str) -> str: """The anonymous URL of a public file, with its name as a trailing segment.""" name = quote(filename, safe="") diff --git a/modules/file_storage/file_storage/reads.py b/modules/file_storage/file_storage/reads.py index c83b8b2a5..8e78a641a 100644 --- a/modules/file_storage/file_storage/reads.py +++ b/modules/file_storage/file_storage/reads.py @@ -99,5 +99,9 @@ async def content_type_facets(self, *, created_by: str | None = None) -> list[di async def uploader_facets(self) -> list[dict]: return [facet.as_dict() for facet in (await self.storage_aggregates()).uploaders] + async def used_bytes_by(self, owner: str) -> int: + """Bytes held by one uploader's files (not cached, not tenant-wide).""" + return await queries.used_bytes_by(self.db, owner) + async def used_bytes(self) -> int: return (await self.storage_aggregates()).used_bytes diff --git a/modules/file_storage/tests/test_file_storage_p3_scope_usage.py b/modules/file_storage/tests/test_file_storage_p3_scope_usage.py new file mode 100644 index 000000000..9509f8334 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_p3_scope_usage.py @@ -0,0 +1,46 @@ +"""Owner-scoped callers see their own usage and no tenant quota; downloads are sandboxed.""" + +from __future__ import annotations + +from file_storage import constants +from file_storage_uploads_helpers import API, INERTIA, upload_text + +VIEW = f"{constants.ROUTE_PREFIX_VIEW}/" + + +async def _props(client) -> dict: + resp = await client.get(VIEW, headers=INERTIA) + assert resp.status_code == 200, resp.text + return resp.json()["props"] + + +class TestUsageScope: + async def test_member_sees_own_usage_and_no_quota(self, tenant_client): + async with ( + tenant_client("member") as a, + tenant_client("member", tenant_id=a.tenant_id) as b, + tenant_client("member", tenant_id=a.tenant_id) as empty, + ): + a.client._transport.app.state.file_storage.settings.quota_bytes = 1024**3 + await upload_text(a.client, "a.txt") # 2 bytes + await b.client.post( + f"{API}/upload", files={"file": ("b.txt", b"x" * 100, "text/plain")} + ) + + assert (await _props(a.client))["used_bytes"] == 2 + assert (await _props(b.client))["used_bytes"] == 100 + assert (await _props(empty.client))["used_bytes"] == 0 + for who in (a, b, empty): + assert (await _props(who.client))["quota_bytes"] is None + + async def test_manager_keeps_tenant_wide_usage_and_quota(self, tenant_client): + async with ( + tenant_client("member") as a, + tenant_client("admin", tenant_id=a.tenant_id) as boss, + ): + boss.client._transport.app.state.file_storage.settings.quota_bytes = 1024**3 + await upload_text(a.client, "a.txt") + + props = await _props(boss.client) + assert props["used_bytes"] == 2 + assert props["quota_bytes"] == 1024**3 From 0f87a2e162bd1f9b882a4c2d23348d4da80f09e8 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Mon, 19 Oct 2026 09:59:19 +0200 Subject: [PATCH 2/6] fix(file_storage): sandbox CSP on streamed authenticated downloads Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../file_storage/endpoints/api.py | 3 +- modules/file_storage/file_storage/serving.py | 9 +++-- .../test_file_storage_p3_download_csp.py | 34 +++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 modules/file_storage/tests/test_file_storage_p3_download_csp.py diff --git a/modules/file_storage/file_storage/endpoints/api.py b/modules/file_storage/file_storage/endpoints/api.py index 1432dcb69..5fbba2b13 100644 --- a/modules/file_storage/file_storage/endpoints/api.py +++ b/modules/file_storage/file_storage/endpoints/api.py @@ -44,7 +44,7 @@ StoredFileNotFoundError, StreamDownload, ) -from file_storage.serving import etag_for, not_found, thumbnail_response +from file_storage.serving import etag_for, not_found, sandbox_headers, thumbnail_response router = APIRouter() @@ -221,6 +221,7 @@ async def download_file( headers={ "Content-Disposition": content_disposition(row.filename, attachment=True), "ETag": etag, + **sandbox_headers(), }, etag=etag, ) diff --git a/modules/file_storage/file_storage/serving.py b/modules/file_storage/file_storage/serving.py index 08fba06c3..2b020698f 100644 --- a/modules/file_storage/file_storage/serving.py +++ b/modules/file_storage/file_storage/serving.py @@ -33,15 +33,18 @@ def not_found(t) -> HTTPException: ) -def _headers(etag: str, cache_control: str) -> dict[str, str]: +def sandbox_headers() -> dict[str, str]: + """Headers that stop served bytes from running as the app's origin.""" return { - "ETag": etag, - "Cache-Control": cache_control, "X-Content-Type-Options": "nosniff", "Content-Security-Policy": constants.PUBLIC_CSP, } +def _headers(etag: str, cache_control: str) -> dict[str, str]: + return {"ETag": etag, "Cache-Control": cache_control, **sandbox_headers()} + + def _not_modified(request: Request, etag: str) -> bool: sent = request.headers.get("if-none-match", "") return etag in {part.strip().removeprefix("W/") for part in sent.split(",")} diff --git a/modules/file_storage/tests/test_file_storage_p3_download_csp.py b/modules/file_storage/tests/test_file_storage_p3_download_csp.py new file mode 100644 index 000000000..e5cdb29c3 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_p3_download_csp.py @@ -0,0 +1,34 @@ +"""Authenticated downloads carry the sandbox CSP.""" + +from __future__ import annotations + +from file_storage import constants +from file_storage_uploads_helpers import API, upload_text + + +class TestDownloadSandbox: + async def test_streamed_download_is_sandboxed(self, tenant_client): + async with tenant_client("member") as a: + file_id = await upload_text(a.client, "a.txt") + resp = await a.client.get(f"{API}/files/{file_id}/download") + + assert resp.status_code == 200 + assert resp.headers["content-security-policy"] == constants.PUBLIC_CSP + assert resp.headers["x-content-type-options"] == "nosniff" + assert "attachment" in resp.headers["content-disposition"] + + async def test_thumbnail_keeps_the_sandbox_csp(self, tenant_client): + async with tenant_client("member") as a: + import io + + from PIL import Image + + buf = io.BytesIO() + Image.new("RGB", (4, 4)).save(buf, "PNG") + resp = await a.client.post( + f"{API}/upload", files={"file": ("p.png", buf.getvalue(), "image/png")} + ) + file_id = resp.json()["id"] + thumb = await a.client.get(f"{API}/files/{file_id}/thumbnail") + assert thumb.status_code == 200 + assert thumb.headers["content-security-policy"] == constants.PUBLIC_CSP From 35fbafbae060f7de0ab69834e6845dd8a602020f Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Mon, 19 Oct 2026 10:00:36 +0200 Subject: [PATCH 3/6] fix(settings): show a field's validation error on its own row below the control Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../settings/pages/components/ModuleFieldRow.tsx | 13 ++++++++++++- modules/settings/tests-js/ModuleForm.test.tsx | 5 +++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/modules/settings/settings/pages/components/ModuleFieldRow.tsx b/modules/settings/settings/pages/components/ModuleFieldRow.tsx index 3adea9c12..02bdb9c86 100644 --- a/modules/settings/settings/pages/components/ModuleFieldRow.tsx +++ b/modules/settings/settings/pages/components/ModuleFieldRow.tsx @@ -63,12 +63,23 @@ export function ModuleFieldRow({
- {error &&

{error}

}
+ + {/* Its own grid row, spanning the control and trailing columns: inside + the control's cell it shared that cell with the input and wrapped a + word per line whenever the cell was narrow. */} + {error && ( +

+ {error} +

+ )} ); } diff --git a/modules/settings/tests-js/ModuleForm.test.tsx b/modules/settings/tests-js/ModuleForm.test.tsx index 2dee26c2a..7ecaadce9 100644 --- a/modules/settings/tests-js/ModuleForm.test.tsx +++ b/modules/settings/tests-js/ModuleForm.test.tsx @@ -102,6 +102,11 @@ describe('ModuleForm', () => { fireEvent.click(screen.getByRole('button', { name: 'Save' })); await waitFor(() => expect(screen.getByText(/item rejected/)).toBeVisible()); expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + // The error is a row of its own beneath the control, not a child of the + // control's cell, where it wrapped a word per line. + const error = screen.getByText(/item rejected/); + expect(error.contains(screen.getByLabelText('retention_days'))).toBe(false); + expect(error.className).toContain('lg:col-span-2'); }); test('shows an error whose loc names no field at form level', async () => { From 84cd566a6d4f342eaf1eb9101124f7fb572b4bec Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Mon, 19 Oct 2026 10:00:36 +0200 Subject: [PATCH 4/6] fix(settings): saving a field's default removes its override row Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/settings/settings/reload.py | 18 ++++++ .../test_settings_default_clears_override.py | 61 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 modules/settings/tests/test_settings_default_clears_override.py diff --git a/modules/settings/settings/reload.py b/modules/settings/settings/reload.py index 92514c884..c115c8192 100644 --- a/modules/settings/settings/reload.py +++ b/modules/settings/settings/reload.py @@ -68,6 +68,17 @@ def _relocate(exc: ValidationError, field_name: str) -> ValidationError: return ValidationError.from_exception_data(exc.title, lines) +def _is_default(cls: type[BaseSettings], field_name: str, value: Any) -> bool: + """Whether the validated ``value`` is what the field defaults to.""" + info = cls.model_fields[field_name] + if info.is_required(): + return False + default = info.get_default(call_default_factory=True) + return to_jsonable_python(_unwrap_secrets(default)) == to_jsonable_python( + _unwrap_secrets(value) + ) + + async def apply_changes_and_reload( app: FastAPI, bus: EventBus, @@ -106,6 +117,7 @@ async def apply_changes_and_reload( merged.update(diff) validated = cls(**merged) + dumped_all = validated.model_dump() # A json field stores what validated, not what was sent: a field validator # may have turned the editor's text into the declared type (``json.loads`` @@ -116,6 +128,12 @@ async def apply_changes_and_reload( # are unwrapped before the validated value is made JSON-able. for field_name, raw_value in diff.items(): vtype = value_type_for_field(cls, field_name) + if _is_default(cls, field_name, dumped_all.get(field_name, raw_value)): + # Saving the default is clearing the override: a stored row would + # keep the field labelled "overridden in DB" while holding exactly + # what the code would have supplied anyway. + await store.clear_override(package, field_name) + continue if vtype == "json": dumped = validated.model_dump(include={field_name}).get(field_name, raw_value) raw_value = to_jsonable_python(_unwrap_secrets(dumped)) diff --git a/modules/settings/tests/test_settings_default_clears_override.py b/modules/settings/tests/test_settings_default_clears_override.py new file mode 100644 index 000000000..dedb48b22 --- /dev/null +++ b/modules/settings/tests/test_settings_default_clears_override.py @@ -0,0 +1,61 @@ +"""Saving a field's default removes its override row instead of storing it.""" + +from __future__ import annotations + +import pytest +from background_tasks.settings import DEFAULT_RETENTION_DAYS +from settings.service import SettingService +from settings.store import SettingsStore + +URL = "/api/settings/modules" + + +async def _overrides(app, package: str) -> dict: + async with app.state.sm.db.session_factory() as db: + return await SettingsStore(SettingService(db)).get_overrides(package) + + +async def _field(client, package: str, name: str) -> dict: + resp = await client.get(URL) + module = next(m for m in resp.json()["modules"] if m["package"] == package) + return next(f for f in module["fields"] if f["name"] == name) + + +@pytest.mark.asyncio +async def test_clearing_optional_field_removes_row_and_reports_default(authenticated_client, app): + put = authenticated_client.put + assert (await put(f"{URL}/file_storage", json={"quota_bytes": 1000})).status_code == 200 + assert "quota_bytes" in await _overrides(app, "file_storage") + + resp = await put(f"{URL}/file_storage", json={"quota_bytes": None}) + + assert resp.status_code == 200, resp.text + assert "quota_bytes" not in await _overrides(app, "file_storage") + field = await _field(authenticated_client, "file_storage", "quota_bytes") + assert field["source"] == "default" + assert not field["db_override"] + assert app.state.file_storage.settings.quota_bytes is None + + +@pytest.mark.asyncio +async def test_non_default_value_is_still_stored(authenticated_client, app): + resp = await authenticated_client.put(f"{URL}/file_storage", json={"quota_bytes": 1000}) + + assert resp.status_code == 200, resp.text + assert "quota_bytes" in await _overrides(app, "file_storage") + field = await _field(authenticated_client, "file_storage", "quota_bytes") + assert field["source"] == "db" + + +@pytest.mark.asyncio +async def test_value_equal_to_non_none_default_removes_row(authenticated_client, app): + put = authenticated_client.put + assert (await put(f"{URL}/background_tasks", json={"retention_days": 7})).status_code == 200 + assert "retention_days" in await _overrides(app, "background_tasks") + + resp = await put(f"{URL}/background_tasks", json={"retention_days": DEFAULT_RETENTION_DAYS}) + + assert resp.status_code == 200, resp.text + assert "retention_days" not in await _overrides(app, "background_tasks") + field = await _field(authenticated_client, "background_tasks", "retention_days") + assert field["source"] == "default" From b203ec02d4aa4d9819316d9f4c7a40bbcc178e9e Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Mon, 19 Oct 2026 10:02:49 +0200 Subject: [PATCH 5/6] fix(settings): a no-op save of a default removes its stale override row Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/settings/settings/reload.py | 11 +++++++++++ .../test_settings_default_clears_override.py | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/modules/settings/settings/reload.py b/modules/settings/settings/reload.py index c115c8192..d361dba41 100644 --- a/modules/settings/settings/reload.py +++ b/modules/settings/settings/reload.py @@ -106,6 +106,17 @@ async def apply_changes_and_reload( # rewritten to the store on every save. merged = current.model_dump() diff = {k: v for k, v in changes.items() if merged.get(k) != v} + + # A row that already holds the default (stored before saving a default + # cleared the row) is not a change, so the diff never reaches it; without + # this the field stays "overridden in DB" however often it is saved. + unchanged = set(changes) - set(diff) - env_supplied_fields(cls) + if unchanged: + stored = await store.get_overrides(package) + for name in sorted(unchanged & set(stored)): + if _is_default(cls, name, merged.get(name)): + await store.clear_override(package, name) + if not diff: return current diff --git a/modules/settings/tests/test_settings_default_clears_override.py b/modules/settings/tests/test_settings_default_clears_override.py index dedb48b22..f28dc823a 100644 --- a/modules/settings/tests/test_settings_default_clears_override.py +++ b/modules/settings/tests/test_settings_default_clears_override.py @@ -59,3 +59,22 @@ async def test_value_equal_to_non_none_default_removes_row(authenticated_client, assert "retention_days" not in await _overrides(app, "background_tasks") field = await _field(authenticated_client, "background_tasks", "retention_days") assert field["source"] == "default" + + +@pytest.mark.asyncio +async def test_noop_save_removes_a_legacy_null_row(authenticated_client, app): + async with app.state.sm.db.session_factory() as db: + await SettingsStore(SettingService(db)).set_override( + "file_storage", "quota_bytes", "null", "json" + ) + await db.commit() + field = await _field(authenticated_client, "file_storage", "quota_bytes") + assert field["db_override"] + + resp = await authenticated_client.put(f"{URL}/file_storage", json={"quota_bytes": None}) + + assert resp.status_code == 200, resp.text + assert "quota_bytes" not in await _overrides(app, "file_storage") + field = await _field(authenticated_client, "file_storage", "quota_bytes") + assert field["source"] == "default" + assert not field["db_override"] From 7d639f012a660044324c8f5e9833f421ab896e44 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Mon, 19 Oct 2026 10:03:06 +0200 Subject: [PATCH 6/6] docs: changelog for the #551 QA follow-ups Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88ff6a74d..6e9b553fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -336,6 +336,13 @@ All notable changes to this project are documented in this file. The format is b `tenants` resolver it selects among the user's own memberships. ### Fixed +- `file_storage` browse page: a caller without `file_storage.manage` sees their + own usage and no tenant quota, instead of the tenant-wide total; the + authenticated download now carries the same sandbox CSP as public serving. +- Settings editor: saving a field's default (clearing an optional field + included) removes its override row instead of storing it, so the field + reads "default" again; a field's validation error sits on its own row below + the control instead of wrapping beside it. - A runtime permission source that raises is no longer cached as empty until invalidated: it contributes nothing for 30 seconds (one logged traceback), then is retried; `invalidate_source` retries it at once (#517).