feat(portal): separate favicon upload + brand-color theming#41
Merged
Conversation
Favicon (distinct from logo — square mark vs lockup):
- Tenant.faviconUrl migration + /uploads/favicon (PNG/JPEG/WebP only; no
SVG/ICO — SVG is a script vector and PNG favicons render everywhere).
- Settings → Brand info gains a second uploader (LogoUploader generalized
to BrandImageUploader); /branding + /config/program carry faviconUrl.
- BrandDocument favicon preference: faviconUrl → logoUrl → monogram.
Brand-color theming (spec §5.4, finally applied):
- Every accent token in theme.ts is now a CSS custom property with the
platform teal as fallback; BrandDocument derives and injects
--op-accent* from Tenant.brandColor (hover = shaded, ink = black/white
by WCAG luminance, soft + alpha tints precomputed — inline styles can't
color-math a var()). Buttons, links, and active nav re-tint app-wide;
no brandColor → properties removed → teal default.
- Swept all ${theme.accent}NN hex-suffix concatenations (which would
break against var() values) to explicit accentA* tokens; theme.ts
comment forbids reintroducing the pattern.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL (js/type-confusion-through-parameter-tampering) flags reading
req.body?.length before the type is proven — with express.raw a
mismatched Content-Type leaves req.body as {}. Behavior-identical
reorder: the Buffer guard now precedes validateImageUpload at all three
upload sites (logo, favicon, brand assets), which resolves the recurring
alert structurally instead of dismissing each new copy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n't narrow req.body The reorder in the previous commit wasn't enough: CodeQL treats every fresh req.body access as a new tainted read and doesn't narrow it through Buffer.isBuffer(req.body). Its own suggested changeset shows the requirement — bind to a local after the guard and use only that. Done at all three upload sites (logo, favicon, brand assets); behavior identical, and TypeScript narrows the unknown-typed local through the same guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ImageUpload
Third and final round with js/type-confusion: after the guarded-local
body fix, the query's remaining tainted argument was
req.header('content-type'). validateImageUpload now accepts the raw
string | string[] | undefined union and normalizes with an explicit
Array.isArray branch — the sanitizer shape the query recognizes, and
genuinely more robust against duplicate Content-Type headers. One place,
covers all three upload sites. If the rule fires again after this, the
remaining step is dismissal with justification, not more contortion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Follow-up to #39 — this commit just missed its merge window and has been re-homed onto current main (incl. the #40 conflict resolution; verified: typecheck clean, api 192/192, portal 12/12).
Separate favicon (logos are lockups; favicons are square marks):
Tenant.faviconUrlmigration +POST /uploads/favicon(PNG/JPEG/WebP only — no SVG/ICO; SVG is a script vector and PNG favicons render everywhere)LogoUploadergeneralized toBrandImageUploader);/branding+/config/programcarryfaviconUrlBrand-color theming (spec §5.4 —
brandColorexisted but was never applied):theme.tsis now a CSS custom property with the platform teal as fallback;BrandDocumentderives + injects--op-accent*from the tenant's color (hover shade, WCAG-luminance ink so button text stays readable, precomputed alpha tints — inline styles can't color-math avar())${theme.accent}NNhex-suffix concats (which break againstvar()values) to explicitaccentA*tokens, with a theme.ts comment forbidding the pattern🤖 Generated with Claude Code