fix(webapp): org avatars blocked by img-src CSP and avatar overflow on failed load - #4600
fix(webapp): org avatars blocked by img-src CSP and avatar overflow on failed load#4600kathiekiwi wants to merge 7 commits into
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
WalkthroughThe webapp now handles failed avatar image loads and displays a globe fallback, including failures detected before hydration. Image CSP matching supports trailing-slash path prefixes while preserving exact matching for other paths. The default image sources include Google favicon redirect shards and the Trigger.dev changelog path. Tests cover avatar-related CSP behavior, favicon shard restrictions, changelog paths, and dashboard image directives. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| "https://t0.gstatic.com/faviconV2", | ||
| "https://t1.gstatic.com/faviconV2", | ||
| "https://t2.gstatic.com/faviconV2", | ||
| "https://t3.gstatic.com/faviconV2", | ||
| "https://trigger.dev/changelog/", |
There was a problem hiding this comment.
🟡 Release-notes entry for this server fix is missing
The user-facing release note for this webapp-only fix was deleted in the final commit (the .server-changes/org-avatar-csp.md file removed in commit 0d92205e9), so this change ships without an entry in the release notes.
Impact: Users reading the release notes will not see that broken organization avatars and changelog images were fixed.
Repository rule requiring a .server-changes note for server-only PRs
AGENTS.md and CONTRIBUTING.md both state: when a PR modifies only server components (apps/webapp/, apps/supervisor/, …) with no public package changes, a .server-changes/ markdown file must be added (frontmatter area + type, body = one plain sentence for users). This PR only touches apps/webapp/app/utils/cspImageOrigins.ts, apps/webapp/app/components/primitives/Avatar.tsx and two test files, and the previously-added note was removed by the last commit, leaving no entry.
Prompt for agents
This PR changes only apps/webapp (no packages/*), so per AGENTS.md and CONTRIBUTING.md it needs a .server-changes/ markdown file. The note that existed (.server-changes/org-avatar-csp.md, with frontmatter area: webapp / type: fix and a one-line user-facing description about org avatars and changelog images displaying again) was deleted in the final commit. Restore a .server-changes entry with user-facing wording so the fix appears in release notes.
Was this helpful? React with 👍 or 👎 to provide feedback.
What & why
Org avatars disappeared from the sidebar, replaced by alt text spilling across it. Two bugs stacked: the document img-src CSP pins the Google favicon endpoint org avatars are stored as, but Google 302-redirects it to
tN.gstatic.comand CSP re-checks the redirect target, so the avatar is refused. Changelog images served fromtrigger.devin the agent chat were also missing from the allowlist. AndAvatar.tsxhad no clipping and no error fallback, so a refused image degraded into overflowing alt text.What's inside
CSP allowlist —
app/utils/cspImageOrigins.ts: the base sources gain the four gstatic shardst0–t3.gstatic.com, path-pinned to/faviconV2, plushttps://trigger.dev/changelog/as a path prefix. No wildcards — the no-wildcard beacon policy stands. The shard hosts are Google-operated with no public write path, so the enumeration is as narrow as the existings2/faviconsentry; if Google ever adds at4, the failure mode is one broken avatar, not a broken page.Avatar fallback —
app/components/primitives/Avatar.tsx: the image box clips, and a failed load falls back to the globe icon. That covers failures before hydration too —onErrornever replays for a node that already failed, so a ref checkscomplete && naturalWidth === 0at attach time. The error state resets when the URL changes (key={avatar.url}).Testing
The CSP test helper now implements CSP's real path-matching rule (trailing slash = prefix, otherwise exact, query ignored) and asserts the pins hold: the gstatic redirect target passes,
beacon.pngon gstatic, at9shard, and non-changelogtrigger.devpaths stay blocked. 39 tests green plus webapp typecheck. Verified against a running webapp that the served directive contains the new sources.