feat(branding): white-label the app — anonymous assets, upload hardening, banner, presets, configurable footer - #237
Merged
Conversation
The published logo/favicon URLs pointed at file_storage's download route,
which is gated by `file-storage.download`. No logged-out request carries
that permission, so a deployment that uploaded a logo got a broken image
on the sign-in page, the public landing page and every `<link rel=icon>` —
precisely the screens a white-label deployment is judged on.
Modelled on the IIASA.GeoWiki Branding module, which solves the same
problem with an `[AllowAnonymous]` branding-image endpoint:
- Branding now serves its own `GET /api/branding/{logo,favicon}`, exempted
through the `register_public_routes` hook. The rules are exact + GET-only,
so upload/clear on the same paths stay behind `branding.manage`. Only the
two ids held in branding settings are served — not arbitrary file_storage
content.
- Published URLs carry `?v=<file id>`. A replaced image is a new id, so the
URL is content-addressed: versioned requests get
`max-age=31536000, immutable`, an unversioned one gets `max-age=3600` so it
self-corrects, and a 404 is never cached (mirrors BrandingImageCache).
- Uploads are magic-number checked. The declared content-type is
caller-controlled, so a payload renamed `logo.png` was previously stored
and served back under an image type.
- SVG is dropped from the allow-list: it is an XML document that can carry
`<script>`, so serving one from our own origin is stored XSS. Responses
also carry `Content-Disposition: attachment` + `nosniff` as a second layer
(both ignored by `<img>`/`<link rel=icon>`, so images still render).
Locale copy, the file picker's `accept`, and the README follow the narrowed
allow-list. 68 branding tests pass (33 new); full suite 1539 passed, JS 44
passed, `make lint` clean.
Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
Every branding upload mints a new file_storage id — that is what makes the published URL content-addressed — so the previous file was simply dropped on the floor. Nothing else references it and nothing reaps it, meaning each logo tweak left another orphan in the store. IIASA.GeoWiki has no equivalent problem: it overwrites one blob at a fixed key per image type, so there is nothing to clean up. Carrying a fresh id per upload is what buys the free cache-busting, and the cost is owning cleanup. BrandingService now captures the outgoing id and reaps it after the settings write succeeds. Cleanup is best effort and logged, never fatal: the rebrand has already been persisted and is what the admin asked for, so a storage fault must not surface as a 500 on a successful save. The service takes the storage handle as an optional keyword so the published constructor stays backwards compatible; branding's own dependency always supplies one, sharing the request session so the delete commits with the settings write. 5 new tests: replace reaps the old file and keeps the new one, clear reaps, a second clear is harmless, logo and favicon are reaped independently, and a failing delete still returns 200. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
The sidebar and mobile bar sit on --color-app-sidebar (near-black) in every theme, while the sign-in card and public page are light. A single uploaded logo therefore cannot read on both surfaces: dark ink disappears in the sidebar, white ink disappears on the sign-in card. IIASA.GeoWiki solves this with BrandingImageType.LogoLight/LogoDark. Adds a second slot on the same footing as the existing images — settings field, anonymous GET /api/branding/logo-dark, gated upload/clear, its own ?v= cache-busting and the same reap-on-replace cleanup. Named for the *surface* rather than the theme, because here the sidebar is dark whatever the theme is. Purely additive: with nothing uploaded the payload reports logoDarkUrl null and callers fall back to logoUrl, so existing deployments are unchanged. `darkSurfaceLogo()` in packages/ui/lib/brand states that fallback once; only the two always-dark call sites in SidebarLayout use it, and the footer keeps the primary logo since it follows the theme on bg-background. ImageField moves out of Manage.tsx into its own component — a third slot would have pushed the page past the 300-line cap, and the split is by responsibility rather than to squeeze under it. Note: regenerating packages/i18n also picked up keycloak.* keys that were missing from the checked-in file (it was last generated without keycloak installed). That is the generator's correct output for the current module set, kept rather than hand-editing a file marked do-not-edit. 11 new branding tests + 4 for the fallback helper. Full suite 1553 passed, JS 48 passed, make lint clean. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
The root template branded <title> and theme-color server-side but left the favicon to BrandingHead on the client, with a stated reason: keeping file_storage's download-route shape out of framework code. That reason no longer holds. Since branding serves its own favicon the URL is its own stable path, not file_storage's. Meanwhile the cost was real — the browser painted the default favicon on every full page load and only swapped once React hydrated. branding_head now forwards a favicon_url that the *module* supplies, via a new BrandingServices.favicon_url property read duck-typed like app_name and primary_color already are. The framework never learns the route shape, so the SM009 framework→plugin boundary stays intact, and a host running an older branding release simply gets None and omits the link. BrandingHead still applies it client-side, so a favicon changed at runtime updates without a reload. Full suite 1556 passed, JS 48 passed, make lint clean. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
_reap swallows cleanup faults, but that cannot rescue a failure during flush — the shared request session is left dirty and the commit at request end surfaces it regardless. Reaping in the request session is the deliberate choice (the delete stays atomic with the settings write rather than orphaning on a late rollback), so say so instead of implying the guard is total. The test comment now names what it actually covers. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
Ported from IIASA.GeoWiki's top banner (MaxTopBannerMessageLength, AllowedTopBannerSeverities, NormalizeTopBannerSeverity). Nothing in this repo's layouts offered any equivalent. An admin sets a message and a severity (info/warning/danger); an empty message hides the bar. It renders above all three shells — app, public and auth — because an outage notice is most useful to people who cannot sign in, and the shared prop already reaches guest pages. Validation splits the way design_pack already does here: the settings validator normalises an unknown severity to "info" (settings hydrate from the DB and must never stop a boot), while the update DTO rejects it with a 422 so a typo is reported rather than silently downgraded. Severity colours are semantic rather than brand-tinted — a warning wearing the deployment's primary colour stops reading as a warning. The bar is role="status", not "alert": ambient page context should not interrupt a screen-reader user mid-task. Not dismissible, matching the reference — per-user dismissal would need storage branding does not own. 16 new tests. Full suite 1573 passed, JS 48 passed, make lint clean. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
Ported from IIASA.GeoWiki's BrandingPresets + ApplyPresetAsync: a named bundle of values applied through the ordinary update path, so every existing validator still runs (including the design-pack registry check — a preset is not trusted just because it ships with the module). One deliberate narrowing. GeoWiki presets set brand name and tagline because each preset *is* a specific tenant (Global Canopy Atlas, Forest Observation System). This module ships a generic app, so a preset that overwrote app_name — or a logo an admin had just uploaded, or a live outage banner — would destroy exactly the work the branding page exists to do. PRESET_FIELDS restricts presets to appearance (primary_color, design_pack) and BrandingPreset.__post_init__ rejects anything else at construction, so a future preset cannot quietly smuggle in an identity field. Kept in the module rather than made a registry, matching the reference: a module that wants to contribute a look ships a design pack, which is already the registry-backed mechanism for exactly that. Applying is a direct server action, not staged local state — a preset is a jump to a known-good look, and a pending-edit affordance would misrepresent the button. 12 new tests. Full suite 1585 passed, JS 48 passed, make lint clean. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
Ported from IIASA.GeoWiki's FooterAppService — brand text, up to 6 columns of 8 links, and a social row, stored as two JSON blobs in the settings store since there is no branding table. Whole-object replace on PUT, as there: a partial merge into nested link lists has no obvious semantics. Previously the footer was rendered by the layout but its content was frozen framework constants, so a white-labelled deployment still advertised "SimpleModule / MIT / github.com/antosubash". BrandingFooter now renders the configured shape when one exists and keeps its original single row otherwise, so untouched deployments look identical. validate_href is the security-relevant part and is ported faithfully. These URLs are admin-authored and rendered into an anchor on every page, guest ones included, so only http(s) and single-leading-slash app paths are allowed — javascript: and data: are refused, and "//host" is correctly treated as the off-site absolute URL it is rather than an in-app path. Reads are lenient (a mangled settings row degrades to "no footer" rather than breaking every render); writes are bounded in count and serialised size. Editor rows carry a client-only id rather than being keyed by array index. Biome flagged the index keys and was right to: removing a middle row makes React reuse the wrong DOM node, so the inputs below the gap keep the deleted row's text. stripIds drops them before the payload is sent. Two files crossed the 300-line cap as a result, so both were split by responsibility — FooterCard (its own draft state and save) out of Manage.tsx, and SidebarUserMenu out of SidebarLayout.tsx. 22 new tests. Full suite 1618 passed, JS 48 passed, make lint clean. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
Found by browser QA. Clicking a preset persisted the new colour correctly but the page kept showing the old one until a manual refresh — the colour field stayed on the placeholder and no preset showed as active, so the one-click feature looked broken. useState only seeds on mount. Every other control on the page edits local state first, so local and server agree; a preset is the one change that *originates on the server*, and router.reload() refreshes Inertia props without re-running the state initialisers. Sync the fields from the branding shared prop, keyed on the primitive values rather than the object: its identity changes on every reload, which would otherwise wipe out whatever the admin was mid-way through typing. Verified both directions in the browser — applying a preset now updates the field and the active-preset highlight immediately, and in-progress typing survives. Full suite 1618 passed, JS 48 passed, make lint clean. Claude-Session: https://claude.ai/code/session_01XHXqKzFARAVjxuw5yFUNCa
Deploying simple-module-python with
|
| Latest commit: |
41dddde
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3afc1a35.simple-module-python.pages.dev |
| Branch Preview URL: | https://worktree-branding-improve.simple-module-python.pages.dev |
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.
Summary
Makes the branding module actually usable for a white-label deployment, using the IIASA.GeoWiki Branding module as the reference implementation.
Bug fixes
file_storage's download route, which requiresfile-storage.download— a permission no anonymous request carries. Any deployment that uploaded a logo got a broken image on the sign-in page, the public landing page, and every favicon. Branding now serves its own anonymousGET /api/branding/{logo,logo-dark,favicon}via theregister_public_routeshook (exact + GET-only, so upload/clear stay behindbranding.manage, and only the ids held in branding settings are ever served).file_storageforever — each upload mints a new id, so the previous one was dropped on the floor. Now reaped, best-effort so a storage fault can't fail an otherwise-successful rebrand.useStateonly seeds on mount, so a server-originated change never reached the fields.Hardening
logo.pngwas previously stored and served back under an image type.<script>, so serving one from our own origin is stored XSS. Responses also carryContent-Disposition: attachment+nosniffas a second layer.javascript:anddata:are refused, and//hostis treated as the off-site absolute URL it is rather than a path.Features (ported from the GeoWiki reference)
PRESET_FIELDS; a preset can never overwrite the app name, an uploaded logo, or a live banner.?v=<file id>makes URLs content-addressed: versioned requests getmax-age=31536000, immutable, unversioned getmax-age=3600, and a 404 is never cached.Verification
make lint(ruff, ty, biome, tsc, 300-line cap, metadata) · 1618 Python tests · 48 JS tests · production build · 12 e2e tests/branding/, 11 scenarios. Verified end-to-end that an uploaded logo now loads on the logged-out login page, that ajavascript:footer URL is rejected with 422 and never stored, and that cache headers are correct in both versioned and unversioned form. One P1 bug was found (stale form after applying a preset) and fixed in41dddde.Notes for the reviewer
packages/i18nalso picked upkeycloak.*keys missing from the checked-in file (it was last generated without keycloak installed). That is the generator's correct output for the current module set, kept rather than hand-editing a file marked do-not-edit.make doctorreports SM020 (Keycloak + Users both installed as auth providers) and two SM003 warnings onkeycloak/audit_log. All pre-existing onmainand untouched here — but note it currently blocks a dev boot, so the QA run usedSM_MODULES_ENABLEDto exclude Keycloak.Test plan
javascript:alert(1)as a footer link URL and confirm it is rejected