Skip to content

Commit d1ac3d5

Browse files
authored
fix(webapp): org avatars blocked by img-src CSP and avatar overflow on failed load (#4600)
## 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.com` and CSP re-checks the redirect target, so the avatar is refused. Changelog images served from `trigger.dev` in the agent chat were also missing from the allowlist. And `Avatar.tsx` had 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 shards `t0`–`t3.gstatic.com`, path-pinned to `/faviconV2`, plus `https://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 existing `s2/favicons` entry; if Google ever adds a `t4`, 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 — `onError` never replays for a node that already failed, so a ref checks `complete && naturalWidth === 0` at attach time. The error state resets when the URL changes (`key={avatar.url}`). **Radio card theming** — `app/components/primitives/RadioButton.tsx`: in the dark themes the checked radio card rendered darker than the unchecked ones. Unchecked cards now sit on `background-bright` (near-black in dark, unchanged white in light) and the checked card uses the `surface-control` tokens, so selection reads black → grey in dark themes; light theme keeps its current look. The API keys route keeps its indigo checked-hover via an explicit override. ## 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.png` on gstatic, a `t9` shard, and non-changelog `trigger.dev` paths stay blocked. 39 tests green plus webapp typecheck. Verified against a running webapp that the served directive contains the new sources.
1 parent 0b52af9 commit d1ac3d5

7 files changed

Lines changed: 90 additions & 15 deletions

File tree

apps/webapp/app/components/primitives/Avatar.tsx

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
StarIcon,
1010
} from "@heroicons/react/20/solid";
1111
import type { Prisma } from "@trigger.dev/database";
12+
import { useCallback, useState } from "react";
1213
import { z } from "zod";
1314
import { cn } from "~/utils/cn";
1415

@@ -76,7 +77,7 @@ export function Avatar({
7677
/>
7778
);
7879
case "image":
79-
return <AvatarImage avatar={avatar} size={size} />;
80+
return <AvatarImage key={avatar.url} avatar={avatar} size={size} />;
8081
}
8182
}
8283

@@ -190,20 +191,36 @@ function AvatarIcon({
190191
}
191192

192193
function AvatarImage({ avatar, size }: { avatar: ImageAvatar; size: number }) {
193-
if (!avatar.url) {
194+
const [failed, setFailed] = useState(false);
195+
196+
// A server-rendered image can finish failing before hydration, so onError never fires.
197+
const detectFailedLoad = useCallback((node: HTMLImageElement | null) => {
198+
if (node && node.complete && node.naturalWidth === 0) {
199+
setFailed(true);
200+
}
201+
}, []);
202+
203+
if (!avatar.url || failed) {
194204
return (
195-
<span className="grid shrink-0 place-items-center" style={styleFromSize(size)}>
205+
<span
206+
role="img"
207+
aria-label="Organization avatar"
208+
className="grid shrink-0 place-items-center overflow-hidden"
209+
style={styleFromSize(size)}
210+
>
196211
<GlobeLinesIcon className="size-[90%] text-text-dimmed" />
197212
</span>
198213
);
199214
}
200215

201216
return (
202-
<span className="grid shrink-0 place-items-center" style={styleFromSize(size)}>
217+
<span className="grid shrink-0 place-items-center overflow-hidden" style={styleFromSize(size)}>
203218
<img
219+
ref={detectFailedLoad}
204220
src={avatar.url}
205221
alt="Organization avatar"
206222
className="size-full rounded-[10%] object-contain"
223+
onError={() => setFailed(true)}
207224
/>
208225
</span>
209226
);

apps/webapp/app/components/primitives/RadioButton.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,31 +22,31 @@ const variants = {
2222
},
2323
"button/small": {
2424
button:
25-
"flex items-center w-fit h-8 pl-2 pr-3 rounded-md border border-border-bright/50 shadow-xs bg-secondary transition hover:bg-background-raised data-disabled:opacity-70 data-disabled:hover:bg-secondary hover:data-[state=checked]:bg-text-bright/4 data-[state=checked]:bg-text-bright/4",
25+
"flex items-center w-fit h-8 pl-2 pr-3 rounded-md border border-border-bright/50 shadow-xs bg-background-bright transition hover:bg-surface-hover-subtle data-disabled:opacity-70 data-disabled:hover:bg-background-bright hover:data-[state=checked]:bg-surface-selected-hover data-[state=checked]:border-border-selected data-[state=checked]:bg-surface-selected",
2626
label: "text-sm text-text-bright select-none",
2727
description: "text-text-dimmed",
2828
inputPosition: "mt-0",
2929
icon: "w-8 h-8 mb-2",
3030
},
3131
button: {
3232
button:
33-
"w-fit py-2 pl-3 pr-4 rounded border border-border-bright/50 shadow-xs bg-secondary hover:bg-background-raised transition data-[state=checked]:bg-background-dimmed data-disabled:opacity-70",
33+
"w-fit py-2 pl-3 pr-4 rounded border border-border-bright/50 shadow-xs bg-background-bright hover:bg-surface-hover-subtle transition hover:data-[state=checked]:bg-surface-selected-hover data-[state=checked]:border-border-selected data-[state=checked]:bg-surface-selected data-disabled:opacity-70",
3434
label: "text-text-bright select-none",
3535
description: "text-text-dimmed",
3636
inputPosition: "mt-1",
3737
icon: "w-8 h-8 mb-2",
3838
},
3939
description: {
4040
button:
41-
"w-full p-2.5 rounded-md border border-border-bright/50 shadow-xs bg-secondary transition hover:bg-background-raised data-disabled:opacity-70 hover:data-[state=checked]:bg-text-bright/4 data-[state=checked]:bg-text-bright/4",
41+
"w-full p-2.5 rounded-md border border-border-bright/50 shadow-xs bg-background-bright transition hover:bg-surface-hover-subtle data-disabled:opacity-70 hover:data-[state=checked]:bg-surface-selected-hover data-[state=checked]:border-border-selected data-[state=checked]:bg-surface-selected",
4242
label: "text-text-bright font-semibold -mt-0.5 text-left text-sm",
4343
description: "text-text-dimmed mt-0 text-left",
4444
inputPosition: "mt-0",
4545
icon: "w-8 h-8 mb-2",
4646
},
4747
icon: {
4848
button:
49-
"w-full p-2.5 pb-4 rounded-sm border border-border-bright/50 shadow-xs bg-secondary hover:bg-background-raised transition data-disabled:opacity-70 data-[state=checked]:bg-background-dimmed",
49+
"w-full p-2.5 pb-4 rounded-sm border border-border-bright/50 shadow-xs bg-background-bright hover:bg-surface-hover-subtle transition data-disabled:opacity-70 hover:data-[state=checked]:bg-surface-selected-hover data-[state=checked]:border-border-selected data-[state=checked]:bg-surface-selected",
5050
label: "text-text-bright font-semibold -mt-1 text-left",
5151
description: "text-text-dimmed mt-0 text-left",
5252
inputPosition: "mt-0",

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.apikeys/route.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,7 @@ function PresetOptions({
10281028
id={`api-key-access-${preset.id}`}
10291029
value={preset.id}
10301030
variant="description"
1031-
className="h-full min-h-[3.5rem] items-start border-grid-bright bg-background-bright p-3 shadow-none [&_p]:mt-0.5 [&_p]:text-xs [&_p]:leading-snug hover:border-border-bright hover:bg-background-hover data-[state=checked]:border-indigo-500/70 data-[state=checked]:bg-indigo-500/10"
1031+
className="h-full min-h-[3.5rem] items-start border-grid-bright bg-background-bright p-3 shadow-none [&_p]:mt-0.5 [&_p]:text-xs [&_p]:leading-snug hover:border-border-bright hover:bg-background-hover data-[state=checked]:border-indigo-500/70 data-[state=checked]:bg-indigo-500/10 hover:data-[state=checked]:bg-indigo-500/15"
10321032
label={
10331033
preset.id === "FULL_ACCESS" ? (
10341034
<span className="flex items-center gap-2">

apps/webapp/app/tailwind.css

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,10 @@
154154
--color-background-dimmed: var(--color-charcoal-850);
155155
--color-background-bright: var(--color-charcoal-800);
156156
--color-background-hover: var(--color-charcoal-750);
157+
--color-surface-hover-subtle: var(--color-charcoal-750);
157158
--color-background-raised: var(--color-charcoal-700);
159+
--color-surface-selected: var(--color-charcoal-650);
160+
--color-surface-selected-hover: var(--color-charcoal-600);
158161
--color-surface-control: var(--color-charcoal-600);
159162
--color-surface-control-hover: var(--color-charcoal-550);
160163
--color-surface-control-active: var(--color-charcoal-500);
@@ -163,6 +166,9 @@
163166
/* Borders, from subtlest to most visible */
164167
--color-grid-dimmed: var(--color-charcoal-750);
165168
--color-grid-bright: var(--color-charcoal-700);
169+
/* Blend of border-bright over the selected card, so it tracks whatever ramp
170+
border-bright is on - the dark themes need no contrast entry of their own */
171+
--color-border-selected: color-mix(in srgb, var(--color-border-bright) 50%, var(--color-surface-selected));
166172
--color-border-bright: var(--color-charcoal-600);
167173
--color-border-brighter: var(--color-charcoal-550);
168174
--color-border-brightest: var(--color-charcoal-500);
@@ -766,14 +772,21 @@
766772
--color-background-dimmed: #fbfbfc;
767773
--color-background-bright: #ffffff;
768774
--color-background-hover: #f2f3f5;
775+
/* Cards on white need a gentler hover than the app-wide surfaces */
776+
--color-surface-hover-subtle: #f7f8f9;
769777
--color-background-raised: #e9eaee;
770778
--color-surface-control: #dcdee3;
771779
--color-surface-control-hover: #cfd2d9;
772780
--color-surface-control-active: #b8bcc6;
781+
/* Selection sits lighter than the controls here - on white a soft grey is
782+
already enough to read as selected */
783+
--color-surface-selected: #eff0f2;
784+
--color-surface-selected-hover: #e7e9ec;
773785

774786
/* Borders */
775787
--color-grid-dimmed: #eceef1;
776788
--color-grid-bright: #e2e4e9;
789+
--color-border-selected: #e0e2e6;
777790
--color-border-bright: #d2d5db;
778791
--color-border-brighter: #b9bdc7;
779792
--color-border-brightest: #9ba1ad;
@@ -931,6 +944,7 @@
931944
--color-background-dimmed: color-mix(in srgb, var(--color-charcoal-850), #000 calc(var(--theme-contrast, 0) * 55%));
932945
--color-background-bright: color-mix(in srgb, var(--color-charcoal-800), #000 calc(var(--theme-contrast, 0) * 45%));
933946
--color-background-hover: color-mix(in srgb, var(--color-charcoal-750), #000 calc(var(--theme-contrast, 0) * 35%));
947+
--color-surface-hover-subtle: color-mix(in srgb, var(--color-charcoal-750), #000 calc(var(--theme-contrast, 0) * 35%));
934948
--color-background-raised: color-mix(in srgb, var(--color-charcoal-700), #000 calc(var(--theme-contrast, 0) * 25%));
935949
--color-input-bg: color-mix(in srgb, var(--color-charcoal-750), #000 calc(var(--theme-contrast, 0) * 35%));
936950

@@ -955,6 +969,9 @@
955969
page-behind surfaces darken a touch so panels separate */
956970
--color-background-deep: color-mix(in srgb, #f1f2f4, #000 calc(var(--theme-contrast, 0) * 10%));
957971
--color-background-hover: color-mix(in srgb, #f2f3f5, #000 calc(var(--theme-contrast, 0) * 8%));
972+
/* Radio card surfaces take no ramp at all on white - their contrast is
973+
carried by border-selected below. Deliberately absent: surface-hover-subtle,
974+
surface-selected, surface-selected-hover. */
958975
--color-background-raised: color-mix(in srgb, #e9eaee, #000 calc(var(--theme-contrast, 0) * 10%));
959976

960977
/* Controls and borders push hard toward black - this is where light-mode
@@ -964,6 +981,8 @@
964981
--color-surface-control-active: color-mix(in srgb, #b8bcc6, #000 calc(var(--theme-contrast, 0) * 32%));
965982
--color-grid-dimmed: color-mix(in srgb, #eceef1, #000 calc(var(--theme-contrast, 0) * 28%));
966983
--color-grid-bright: color-mix(in srgb, #e2e4e9, #000 calc(var(--theme-contrast, 0) * 32%));
984+
/* Steeper than border-bright so the selected card stays the loudest edge */
985+
--color-border-selected: color-mix(in srgb, #e0e2e6, #000 calc(var(--theme-contrast, 0) * 45%));
967986
--color-border-bright: color-mix(in srgb, #d2d5db, #000 calc(var(--theme-contrast, 0) * 38%));
968987
--color-border-brighter: color-mix(in srgb, #b9bdc7, #000 calc(var(--theme-contrast, 0) * 42%));
969988
--color-border-brightest: color-mix(in srgb, #9ba1ad, #000 calc(var(--theme-contrast, 0) * 46%));

apps/webapp/app/utils/cspImageOrigins.test.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ function directivePermits(directive: string, imageUrl: string): boolean {
1717
if (!source.startsWith("http")) return false;
1818
const parsed = new URL(source);
1919
if (parsed.protocol !== url.protocol || parsed.host !== url.host) return false;
20-
return parsed.pathname === "/" || parsed.pathname === url.pathname;
20+
// CSP path matching: a source path ending in "/" matches by prefix, otherwise it
21+
// must match exactly. The query string is never part of the match.
22+
return parsed.pathname.endsWith("/")
23+
? url.pathname.startsWith(parsed.pathname)
24+
: parsed.pathname === url.pathname;
2125
});
2226
}
2327

@@ -104,9 +108,9 @@ describe("parseCspImageOrigins", () => {
104108
});
105109

106110
describe("buildImgSrcDirective", () => {
107-
it("is self, data, blob, the SSO avatar hosts and the favicon endpoint by default", () => {
111+
it("is self, data, blob, the SSO avatar hosts, the favicon endpoints and the changelog by default", () => {
108112
expect(buildImgSrcDirective()).toBe(
109-
"img-src 'self' data: blob: https://avatars.githubusercontent.com https://lh3.googleusercontent.com https://www.google.com/s2/favicons"
113+
"img-src 'self' data: blob: https://avatars.githubusercontent.com https://lh3.googleusercontent.com https://www.google.com/s2/favicons https://t0.gstatic.com/faviconV2 https://t1.gstatic.com/faviconV2 https://t2.gstatic.com/faviconV2 https://t3.gstatic.com/faviconV2 https://trigger.dev/changelog/"
110114
);
111115
});
112116

@@ -120,6 +124,29 @@ describe("buildImgSrcDirective", () => {
120124
);
121125
});
122126

127+
it("permits the gstatic shard the favicon endpoint redirects to", () => {
128+
expect(
129+
directivePermits(
130+
buildImgSrcDirective(),
131+
"https://t2.gstatic.com/faviconV2?url=https://example.com&size=128"
132+
)
133+
).toBe(true);
134+
});
135+
136+
it("permits nothing else on a gstatic shard, and no shard we did not list", () => {
137+
const directive = buildImgSrcDirective();
138+
expect(directivePermits(directive, "https://t2.gstatic.com/beacon.png")).toBe(false);
139+
expect(directivePermits(directive, "https://t9.gstatic.com/faviconV2")).toBe(false);
140+
});
141+
142+
it("permits changelog images by path prefix, and nothing else on our domain", () => {
143+
const directive = buildImgSrcDirective();
144+
expect(directivePermits(directive, "https://trigger.dev/changelog/some-post/image.png")).toBe(
145+
true
146+
);
147+
expect(directivePermits(directive, "https://trigger.dev/anything.png")).toBe(false);
148+
});
149+
123150
it("permits both OAuth avatar hosts", () => {
124151
const directive = buildImgSrcDirective();
125152
expect(directivePermits(directive, "https://avatars.githubusercontent.com/u/1?v=4")).toBe(true);

apps/webapp/app/utils/cspImageOrigins.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
*/
77

88
/**
9-
* Always allowed: own origin, inline data, object URLs, the SSO avatar hosts, and the
10-
* favicon endpoint org avatars are stored as (see `utils/favicon.ts`). The path pins
11-
* that one endpoint — CSP matches the path and ignores the query string.
9+
* Always allowed: own origin, inline data, object URLs, the SSO avatar hosts, the
10+
* favicon endpoint org avatars are stored as (see `utils/favicon.ts`), and our own
11+
* changelog images. The path pins each endpoint — CSP matches the path and ignores the
12+
* query string. The favicon endpoint 302s to a `tN.gstatic.com` shard and CSP re-checks
13+
* only the host on a redirect, so the shards are listed too; their path pin limits
14+
* direct loads only. A trailing "/" matches by prefix.
1215
*/
1316
export const BASE_IMG_SRC_SOURCES = [
1417
"'self'",
@@ -17,6 +20,11 @@ export const BASE_IMG_SRC_SOURCES = [
1720
"https://avatars.githubusercontent.com",
1821
"https://lh3.googleusercontent.com",
1922
"https://www.google.com/s2/favicons",
23+
"https://t0.gstatic.com/faviconV2",
24+
"https://t1.gstatic.com/faviconV2",
25+
"https://t2.gstatic.com/faviconV2",
26+
"https://t3.gstatic.com/faviconV2",
27+
"https://trigger.dev/changelog/",
2028
] as const;
2129

2230
export type RejectedOrigin = { value: string; reason: string };

apps/webapp/test/dashboardAgentImageCsp.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ describe("document image CSP", () => {
2424
expect(directive).not.toMatch(/(^|\s)https?:(\s|$)/);
2525
});
2626

27+
it("allows changelog images", () => {
28+
expect(buildImgSrcDirective().split(" ")).toContain("https://trigger.dev/changelog/");
29+
});
30+
2731
it("sets the header on every document response, not only on /login", () => {
2832
// The set() call must sit outside the /login branch.
2933
const loginBranch = source.slice(

0 commit comments

Comments
 (0)