diff --git a/CONCEPTS.md b/CONCEPTS.md index 52d260d..5c1c308 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -100,6 +100,12 @@ An Action type that sends an atmo.pub notification to the Automation's Owner, wi The per-user daily cap Airglow imposes on Notification action sends, set below atmo.pub's 100/day per-recipient limit. Airglow notifications bypass the budget, so the reserved headroom keeps system alerts deliverable even when an Automation exhausts it. Budget-exhausted sends log as Delivery failures, not Skips, so the condition stays visible and trips the failing-Automation alert. +## AT Protocol + +### AppView + +The AT Protocol application-layer service that aggregates and indexes social graph data — profile metadata, feeds, and social relationships — and exposes it via the XRPC protocol. A distinct tier from the PDS (the user's personal data store), which Airglow accesses directly via OAuth. Responses from the AppView are treated as untrusted network input because the service is a third-party dependency outside Airglow's trust boundary. + ## UI architecture ### Island diff --git a/app/components/Layout/Footer/index.tsx b/app/components/Layout/Footer/index.tsx index 560cfa0..b2f4f99 100644 --- a/app/components/Layout/Footer/index.tsx +++ b/app/components/Layout/Footer/index.tsx @@ -23,7 +23,10 @@ export function Footer() {
- Airglow.run +

Automations for the AT Protocol.

diff --git a/app/components/Layout/Footer/styles.css.ts b/app/components/Layout/Footer/styles.css.ts index a1afe97..f5888dc 100644 --- a/app/components/Layout/Footer/styles.css.ts +++ b/app/components/Layout/Footer/styles.css.ts @@ -1,7 +1,7 @@ import { style } from "@vanilla-extract/css"; import { vars } from "../../../styles/theme.css.ts"; import { space } from "../../../styles/tokens/spacing.ts"; -import { fontSize, fontWeight } from "../../../styles/tokens/typography.ts"; +import { fontSize } from "../../../styles/tokens/typography.ts"; export const footer = style({ borderBlockStart: `1px solid ${vars.color.border}`, @@ -36,9 +36,13 @@ export const brandBlock = style({ export const brand = style({ color: vars.color.text, textDecoration: "none", - fontSize: fontSize.md, - fontWeight: fontWeight.semibold, - letterSpacing: "0.02em", + display: "inline-block", +}); + +export const logo = style({ + blockSize: "20px", + inlineSize: "auto", + display: "block", }); export const tagline = style({ @@ -47,10 +51,6 @@ export const tagline = style({ fontSize: fontSize.xs, }); -export const brandAccent = style({ - color: vars.color.accent, -}); - export const col = style({ display: "flex", flexDirection: "column", diff --git a/docs/solutions/security-issues/avatar-url-validation-and-ssrf-on-oauth-callback.md b/docs/solutions/security-issues/avatar-url-validation-and-ssrf-on-oauth-callback.md new file mode 100644 index 0000000..3f5a5a5 --- /dev/null +++ b/docs/solutions/security-issues/avatar-url-validation-and-ssrf-on-oauth-callback.md @@ -0,0 +1,112 @@ +--- +title: Avatar URL validation and SSRF on OAuth callback +date: 2026-06-24 +category: docs/solutions/security-issues +module: auth/callback +problem_type: security_issue +component: authentication +symptoms: + - External avatar URL stored in DB with only a truthiness check — no type, protocol, or hostname validation + - fetch() follows 3xx redirects by default, enabling SSRF to internal services via a spoofed AppView + - Stale CDN avatar URL persists in DB after user removes their Bluesky avatar +root_cause: missing_validation +resolution_type: code_fix +severity: high +related_components: + - database +tags: [security, avatar, ssrf, url-validation, oauth-callback, bluesky, external-fetch] +--- + +# Avatar URL validation and SSRF on OAuth callback + +## Problem + +When fetching a Bluesky profile avatar from the AppView API on OAuth login and storing it in the `users` table, the original code stored the URL with only a truthiness check — no type, protocol, or hostname validation. This allowed injection of arbitrary or malicious URLs, follow-through on redirect chains (SSRF), and stale data persistence when users removed their avatars. + +## Symptoms + +- A compromised or spoofed AppView could inject a `javascript:` URI, an arbitrary HTTPS URL, or a non-string value into the `avatarUrl` column, which is rendered as `` in the Header +- `fetch()` follows 3xx redirects by default; a redirect to a Railway metadata endpoint, `localhost`, or any RFC-1918 host leaks the response or probes internal services (SSRF) +- After a user removes their Bluesky avatar, the old CDN URL remains in the DB indefinitely; the Header renders a broken image on every subsequent login + +## What Didn't Work + +- **Truthiness guard only** (`if (profile.avatar) { ... set({ avatarUrl: profile.avatar }) }`): passes non-string values through (e.g., `true`, a number, an object); does not validate protocol or origin; also prevents the column from ever being cleared when the avatar is absent +- **Adding a `typeof` check** (`typeof profile.avatar === "string"`): rules out non-string types but still allows any arbitrary HTTPS URL from any host — a spoofed AppView or MITM can store tracking pixels, off-domain URLs, or phishing pages +- **String prefix matching** (`profile.avatar.startsWith("https://cdn.bsky.app")`): fragile — allows `https://cdn.bsky.app.evil.com`, does not validate the full URL structure; the `URL` constructor is the correct tool for parsing + +## Solution + +Replace the guarded conditional update with an always-write pattern that validates the URL through the `URL` constructor before storing. + +**Before:** + +```ts +// app/routes/auth/callback.tsx +const profileRes = await fetch( + `https://api.bsky.app/xrpc/app.bsky.actor.getProfile?actor=${encodeURIComponent(did)}`, + { signal: AbortSignal.timeout(5000) }, +); +if (profileRes.ok) { + const profile = (await profileRes.json()) as { avatar?: string }; + if (profile.avatar) { + await db.update(users).set({ avatarUrl: profile.avatar }).where(eq(users.did, did)); + } +} +``` + +**After:** + +```ts +// app/routes/auth/callback.tsx +const profileRes = await fetch( + `https://api.bsky.app/xrpc/app.bsky.actor.getProfile?actor=${encodeURIComponent(did)}`, + { signal: AbortSignal.timeout(5000), redirect: "error" }, +); +if (profileRes.ok) { + const profile = (await profileRes.json()) as { avatar?: string | null }; + const avatarUrl = (() => { + if (typeof profile.avatar !== "string") return null; + try { + const u = new URL(profile.avatar); + return u.protocol === "https:" && u.hostname === "cdn.bsky.app" + ? profile.avatar + : null; + } catch { + return null; + } + })(); + await db.update(users).set({ avatarUrl }).where(eq(users.did, did)); +} +``` + +Key changes: +- `redirect: "error"` — any 3xx response from the AppView throws instead of being silently followed +- `typeof profile.avatar !== "string"` — rejects non-string JSON values before URL parsing +- `new URL(profile.avatar)` in a try/catch — uses the parser to normalise the URL rather than fragile string matching +- `.protocol === "https:" && .hostname === "cdn.bsky.app"` — strict allowlist; anything else becomes `null` +- Unconditional `db.update(...).set({ avatarUrl })` — writes `null` when absent or invalid, clearing any stale value + +## Why This Works + +The root cause is trusting externally-sourced data at face value. The AppView response is network input from a third-party service; even though `api.bsky.app` is the canonical Bluesky endpoint, the code must assume worst case (MITM, compromised upstream). Storing an unvalidated string directly in the DB and rendering it as an HTML attribute is a stored XSS vector if the protocol is unchecked, and an SSRF/phishing vector if the hostname is unpinned. + +`redirect: "error"` closes the SSRF window at the fetch boundary — the application never follows a redirect chain regardless of what the AppView responds with. + +The `URL` constructor normalises the input before the `.hostname` comparison, so attacks like `https://cdn.bsky.app.evil.com` or `https://cdn.bsky.app%2f@evil.com` are correctly rejected. + +Writing `avatarUrl` unconditionally (including `null`) makes the column self-correcting: each login reflects the current profile state rather than accumulating stale values. + +## Prevention + +- **Use the `URL` constructor for URL validation, never string prefix matching.** `startsWith("https://cdn.bsky.app")` is bypassed by `https://cdn.bsky.app.evil.com`; the `URL` constructor normalises the value and exposes `.protocol` and `.hostname` safely. +- **Pin stored URLs to the exact expected hostname.** If a URL must come from `cdn.bsky.app`, assert that. Do not allow any `https:` URL through on the assumption that HTTPS implies safety. +- **Always set `redirect: "error"` when fetching from an external service whose redirect destination you do not control.** Default redirect-following is an SSRF risk when the redirected target is unvalidated. +- **Prefer always-write over conditional-write for nullable columns.** Writing `null` explicitly when a value is absent or invalid keeps the column in sync with the source of truth and avoids stale-data bugs after user profile changes. +- **Apply this pattern to any externally-sourced URL stored in the DB:** type check → `URL` constructor → protocol allowlist → hostname allowlist → store. The pattern is the same whether the URL is an avatar, a webhook endpoint, a link preview, or a redirect URI. + +## Related Issues + +- `app/routes/auth/callback.tsx` — file containing the fix +- `app/components/Layout/Header/index.tsx` — where `avatarUrl` is rendered as `` +- `docs/solutions/security-issues/oauth-grant-revocation-error-contract.md` — related auth module doc (different problem: revocation error-path consistency)