fix: store avatars in R2 (not base64) to stop context-window blow-up#32
Conversation
Base64 avatars written to user.image flowed through the memory auth-overlay into every <memory> system block uncapped, blowing the context window on small models (372K tokens on a one-line query). - new POST /api/avatar/presign: presigns a PUT to u/<userId>/avatar/*, returns the public URL, no DB row; 503 when R2 unset - ChangeAvatar uploads via presign and stores only the URL; upload is disabled unless ATTACHMENTS_ENABLED (reuses the existing R2 gate) - rename the memory auth-overlay key image -> avatar - socials now carry the provider email decoded from the OIDC idToken (Google); GitHub has no idToken so it stays email-less (#10) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Another prompt-bloat vector: the model could persist a base64 image / data URL into the profile doc, which then rides in every <memory> block. Guard rejects any string value (walked through arrays/objects) that is a data URL or a long unbroken base64 run; the tool description forbids it too. Threshold 512 avoids flagging wallet addresses / hashes / IDs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review — #32 (avatar R2 migration)Overall the migration looks right: the A few things I'd like to see addressed before merge, ordered by impact. 1. Avatar files leak in R2 — DELETE path is a no-op
The same leak applies to replacing an avatar (the upload path): the old I'd add a
If you prefer not to expand the API surface, documenting the leak in the PR description so a future R2 sweep is on someone's radar would be the minimum. Today the description only mentions the on-DB cleanup that's needed; the on-R2 cleanup isn't called out. 2. SVG slips past the allow list
Cheapest fix: (SVG can be allowed if you pair it with 3. Uploads are silently dropped when R2 isn't configured
4. Stale fallback shape for
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
…, EMPTY_AUTH_INFO (#28, #10) - getAuthInfo now fetches the GitHub per-provider email via the stored OAuth access token (default scope already grants user:email); Google keeps using the OIDC idToken. Best-effort, behind the recall LRU. - new DELETE /api/avatar deletes the R2 object on avatar delete AND on replace — avatars have no DB row, so nothing else swept the old object. - avatar presign rejects image/svg+xml (inline SVG XSS on a public bucket). - toast instead of silently dropping an upload when R2 is unconfigured. - extract shared EMPTY_AUTH_INFO; drop the stale per-site fallback casts. - docs: APIS.md (svg + DELETE route), MEMORY.md (image -> avatar overlay). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…erified email, base64url guard - getAuthInfo drops a legacy `data:` avatar (treats as no avatar) so existing base64 rows stop bloating <memory> immediately — no migration. - emailFromIdToken only surfaces a VERIFIED email (email_verified claim). - save_memory base64 guard now also covers the url-safe (-_) alphabet. - docs/env: document Content-Type value; fix "backed by R2" wording. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ReviewSolid PR — the rename is complete across every call site, the new endpoints are properly withAuth-wrapped, SVG XSS is gated, and the base64 guard in save_memory closes the second bloat vector. A few real concerns before merge: P1 — Correctness
P2 — Behavior / leaks
Docs — rule #1 violation
P3 — Defense in depth / style
Notes
|
| function generateId(): string { | ||
| const bytes = randomBytes(ID_LEN); | ||
| let out = ""; | ||
| for (let i = 0; i < ID_LEN; i++) out += ID_ALPHABET[bytes[i] % ID_ALPHABET.length]; |
There was a problem hiding this comment.
P3 — modulo bias, cosmetic but cheap to fix. 256 % 36 === 4, so values 0..3 are ~0.3% more likely than the rest. Standard fix:
| for (let i = 0; i < ID_LEN; i++) out += ID_ALPHABET[bytes[i] % ID_ALPHABET.length]; | |
| function generateId(): string { | |
| const limit = Math.floor(256 / ID_ALPHABET.length) * ID_ALPHABET.length; // 252 | |
| const bytes = randomBytes(ID_LEN + 4); | |
| let out = ""; | |
| let i = 0; | |
| while (out.length < ID_LEN) { | |
| if (bytes[i] < limit) out += ID_ALPHABET[bytes[i] % ID_ALPHABET.length]; | |
| i++; | |
| } | |
| return out; | |
| } |
Non-security-critical (collisions are still ~0 in practice), but worth doing if you have other crypto-grade ID generators elsewhere in the codebase.
There was a problem hiding this comment.
Declining — the ID is only used to namespace R2 keys under u/<userId>/avatar/<id>-<safe-name> for the presign URL, not as a security boundary (ownership is enforced server-side in DELETE /api/avatar via the key prefix check). With ID_LEN of 16 the collision space is still effectively zero at human upload rates, and the rejection-sampling loop adds a branch + 4 extra bytes on a hot path that has to run before every avatar change. Filing a separate ticket if we ever need a uniformly-distributed ID for a security-sensitive use.
🤖 Generated with Claude Code
…APIS.md - onError handler cleans up the just-uploaded R2 object when updateUser rejects (no DB row tracks it, retention sweep excludes avatars). - pendingUploadRef tracks in-flight uploads so rapid re-uploads delete the prior in-flight upload on its onSuccess instead of the original avatar (previousImage). - setIsUploading(false) moves into finally. - docs/APIS.md: avatar (not image) + socials[].email? shape, including the verified-email contract and issue #28 data-URL filter note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough fix — the avatar R2 path + base64 guard are well-scoped. A few findings to address before merge. Blocking / security
Robustness
Nice-to-haves (not blocking)
Verified / no concern
Overall: solid PR, just tighten the SVG gate + the URL slicing before shipping. |
| // origin. (attachments enforces R2_ALLOWED_CONTENT_TYPES, which omits | ||
| // svg by default; we mirror that intent here.) | ||
| const contentType = parsed.data.contentType.toLowerCase(); | ||
| if (!contentType.startsWith("image/") || contentType === "image/svg+xml") { |
There was a problem hiding this comment.
The SVG rejection uses an exact-string match (contentType === "image/svg+xml"), but the comment justifies the block by saying SVG can carry <script>/<foreignObject> in an inline-disposition bucket. A client that sends image/svg+xml; charset=utf-8 (or any other parameter, or trailing whitespace after lowercase) bypasses the check entirely — R2 stores the object with that content-type, and the comment's XSS concern still applies.
Either tighten to contentType.startsWith("image/svg") or, better, mirror the attachment route and validate against R2_ALLOWED_CONTENT_TYPES so the SVG exclusion is centrally configured.
| if (!contentType.startsWith("image/") || contentType === "image/svg+xml") { | |
| if (!contentType.startsWith("image/") || contentType.startsWith("image/svg")) { |
| // Not one of our R2 objects (e.g. an OAuth-hosted avatar) → nothing to do. | ||
| if (!url.startsWith(`${base}/`)) return new NextResponse(null, { status: 204 }); | ||
|
|
||
| const key = url.slice(base.length + 1); |
There was a problem hiding this comment.
url.slice(base.length + 1) doesn't strip query strings or fragments. A URL like https://file.example/u/<id>/avatar/x.png?v=1 produces key u/<id>/avatar/x.png?v=1, and deleteObject fails silently via the .catch(() => undefined) swallow — the user thinks their old avatar was swept but the object is still there. Same for #frag.
Parse the URL first so only the pathname reaches R2:
| const key = url.slice(base.length + 1); | |
| const parsedUrl = new URL(url); | |
| if (parsedUrl.origin !== new URL(base).origin) return new NextResponse(null, { status: 204 }); | |
| const key = parsedUrl.pathname.slice(1); |
| // ponytail: clean up whatever was the avatar BEFORE this upload | ||
| // succeeded. If a second upload raced us, that's the one | ||
| // pendingUploadRef now points to — we delete it instead of | ||
| // the user's actual current avatar. |
There was a problem hiding this comment.
setIsUploading(false) is outside the try/catch and not in a finally. Today the catch only calls toast.error(error.message) (which doesn't throw), but the moment someone replaces that with an async helper (or re-throws to surface a stricter error), the upload spinner sticks indefinitely — the button stays disabled via isPending = updatePending || isUploading || isDeleting, and the dropdown's <Spinner /> keeps spinning. Move it into a finally for unconditional cleanup:
| // the user's actual current avatar. | |
| } catch (error) { | |
| if (error instanceof Error) { | |
| toast.error(error.message); | |
| } | |
| } finally { | |
| setIsUploading(false); | |
| } |
There was a problem hiding this comment.
7 issues found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/api/avatar/presign.test.ts">
<violation number="1" location="tests/api/avatar/presign.test.ts:139">
P3: Missing error-code assertion in the 400-for-missing-url test. Every other error case in this file checks both the status and the `code` field — skipping it here means a false-negative refactor (route returning 400 for the wrong reason) slips through. Add `expect((await res.json()).code).toBe("BAD_REQUEST")`.</violation>
</file>
<file name="tests/lib/memory/queries.test.ts">
<violation number="1" location="tests/lib/memory/queries.test.ts:227">
P2: Test pollution risk: `vi.stubGlobal` cleaned up inside the test body (after assertions) means a failed assertion leaks the stubbed `fetch` to later tests. Safer to move `vi.unstubAllGlobals()` into the shared `afterEach` (or add it to `beforeEach`) so cleanup always runs regardless of test outcome.</violation>
</file>
<file name="components/auth/settings/account/change-avatar.tsx">
<violation number="1" location="components/auth/settings/account/change-avatar.tsx:116">
P1: Rapid avatar re-uploads can leave the saved avatar URL pointing at a deleted R2 object. The success handler deletes `pendingUploadRef.current` from another in-flight upload; tracking each upload with a sequence/token or only deleting objects known to be superseded would avoid deleting a later successful upload.</violation>
</file>
<file name="app/api/avatar/route.ts">
<violation number="1" location="app/api/avatar/route.ts:53">
P2: Avatar cleanup can silently fail and leave public R2 objects orphaned, because every `deleteObject` error is converted to a 204. Consider only treating missing-object responses as success and surfacing config/R2 failures so failed cleanup is visible.</violation>
</file>
<file name="docs/APIS.md">
<violation number="1" location="docs/APIS.md:116">
P3: The DELETE avatar response docs overstate the external-URL no-op behavior. With `R2_PUBLIC_BASE_URL` unset, the handler returns `AVATAR_UPLOADS_NOT_CONFIGURED` before it can classify an external URL, so the line should note that the 204 no-op only applies when R2 public base config is available.</violation>
</file>
<file name="lib/memory/queries.ts">
<violation number="1" location="lib/memory/queries.ts:197">
P2: A slow GitHub emails request can now delay memory loading on cache-miss prompts and the Memory profile endpoint, because this `fetch()` has no timeout or abort signal. Consider bounding it so GitHub lookup remains best-effort instead of holding the whole auth overlay.</violation>
<violation number="2" location="lib/memory/queries.ts:211">
P2: Memory can show an unverified GitHub address when none of the returned emails are verified, because `fetchGithubEmail()` falls back to `emails[0]`. It would be safer to return no email unless a verified GitHub email was found, matching the OIDC path above.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // succeeded. If a second upload raced us, that's the one | ||
| // pendingUploadRef now points to — we delete it instead of | ||
| // the user's actual current avatar. | ||
| const toDelete = |
There was a problem hiding this comment.
P1: Rapid avatar re-uploads can leave the saved avatar URL pointing at a deleted R2 object. The success handler deletes pendingUploadRef.current from another in-flight upload; tracking each upload with a sequence/token or only deleting objects known to be superseded would avoid deleting a later successful upload.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/auth/settings/account/change-avatar.tsx, line 116:
<comment>Rapid avatar re-uploads can leave the saved avatar URL pointing at a deleted R2 object. The success handler deletes `pendingUploadRef.current` from another in-flight upload; tracking each upload with a sequence/token or only deleting objects known to be superseded would avoid deleting a later successful upload.</comment>
<file context>
@@ -64,34 +76,66 @@ export function ChangeAvatar({ className }: ChangeAvatarProps) {
+ // succeeded. If a second upload raced us, that's the one
+ // pendingUploadRef now points to — we delete it instead of
+ // the user's actual current avatar.
+ const toDelete =
+ pendingUploadRef.current === image ? previousImage : pendingUploadRef.current;
+ pendingUploadRef.current = null;
</file context>
| @@ -150,14 +150,14 @@ describe("lib/memory/queries", () => { | |||
| }); | |||
There was a problem hiding this comment.
P2: Test pollution risk: vi.stubGlobal cleaned up inside the test body (after assertions) means a failed assertion leaks the stubbed fetch to later tests. Safer to move vi.unstubAllGlobals() into the shared afterEach (or add it to beforeEach) so cleanup always runs regardless of test outcome.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/lib/memory/queries.test.ts, line 227:
<comment>Test pollution risk: `vi.stubGlobal` cleaned up inside the test body (after assertions) means a failed assertion leaks the stubbed `fetch` to later tests. Safer to move `vi.unstubAllGlobals()` into the shared `afterEach` (or add it to `beforeEach`) so cleanup always runs regardless of test outcome.</comment>
<file context>
@@ -191,12 +207,57 @@ describe("lib/memory/queries", () => {
expect(info.socials).toEqual([{ provider: "google" }]);
});
+
+ it("fetches the GitHub email via the stored access token (issue #10)", async () => {
+ const fetchMock = vi.fn().mockResolvedValue({
+ ok: true,
</file context>
| return NextResponse.json({ code: "FORBIDDEN" }, { status: 403 }); | ||
| } | ||
|
|
||
| await deleteObject(key).catch(() => undefined); |
There was a problem hiding this comment.
P2: Avatar cleanup can silently fail and leave public R2 objects orphaned, because every deleteObject error is converted to a 204. Consider only treating missing-object responses as success and surfacing config/R2 failures so failed cleanup is visible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/avatar/route.ts, line 53:
<comment>Avatar cleanup can silently fail and leave public R2 objects orphaned, because every `deleteObject` error is converted to a 204. Consider only treating missing-object responses as success and surfacing config/R2 failures so failed cleanup is visible.</comment>
<file context>
@@ -0,0 +1,55 @@
+ return NextResponse.json({ code: "FORBIDDEN" }, { status: 403 });
+ }
+
+ await deleteObject(key).catch(() => undefined);
+ return new NextResponse(null, { status: 204 });
+});
</file context>
| verified: boolean; | ||
| }>; | ||
| const pick = | ||
| emails.find((e) => e.primary && e.verified) ?? emails.find((e) => e.verified) ?? emails[0]; |
There was a problem hiding this comment.
P2: Memory can show an unverified GitHub address when none of the returned emails are verified, because fetchGithubEmail() falls back to emails[0]. It would be safer to return no email unless a verified GitHub email was found, matching the OIDC path above.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/memory/queries.ts, line 211:
<comment>Memory can show an unverified GitHub address when none of the returned emails are verified, because `fetchGithubEmail()` falls back to `emails[0]`. It would be safer to return no email unless a verified GitHub email was found, matching the OIDC path above.</comment>
<file context>
@@ -148,30 +148,68 @@ export async function writeSummary(
+ verified: boolean;
+ }>;
+ const pick =
+ emails.find((e) => e.primary && e.verified) ?? emails.find((e) => e.verified) ?? emails[0];
+ return typeof pick?.email === "string" ? pick.email : undefined;
} catch {
</file context>
| emails.find((e) => e.primary && e.verified) ?? emails.find((e) => e.verified) ?? emails[0]; | |
| emails.find((e) => e.primary && e.verified) ?? emails.find((e) => e.verified); |
| const res = await fetch("https://api.github.com/user/emails", { | ||
| headers: { | ||
| Authorization: `Bearer ${accessToken}`, | ||
| Accept: "application/vnd.github+json", | ||
| "User-Agent": "langgraph-app", | ||
| }, | ||
| }); |
There was a problem hiding this comment.
P2: A slow GitHub emails request can now delay memory loading on cache-miss prompts and the Memory profile endpoint, because this fetch() has no timeout or abort signal. Consider bounding it so GitHub lookup remains best-effort instead of holding the whole auth overlay.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/memory/queries.ts, line 197:
<comment>A slow GitHub emails request can now delay memory loading on cache-miss prompts and the Memory profile endpoint, because this `fetch()` has no timeout or abort signal. Consider bounding it so GitHub lookup remains best-effort instead of holding the whole auth overlay.</comment>
<file context>
@@ -148,30 +148,68 @@ export async function writeSummary(
+async function fetchGithubEmail(accessToken: string | null): Promise<string | undefined> {
+ if (!accessToken) return undefined;
+ try {
+ const res = await fetch("https://api.github.com/user/emails", {
+ headers: {
+ Authorization: `Bearer ${accessToken}`,
</file context>
| const res = await fetch("https://api.github.com/user/emails", { | |
| headers: { | |
| Authorization: `Bearer ${accessToken}`, | |
| Accept: "application/vnd.github+json", | |
| "User-Agent": "langgraph-app", | |
| }, | |
| }); | |
| const res = await fetch("https://api.github.com/user/emails", { | |
| headers: { | |
| Authorization: `Bearer ${accessToken}`, | |
| Accept: "application/vnd.github+json", | |
| "User-Agent": "langgraph-app", | |
| }, | |
| signal: AbortSignal.timeout(3_000), | |
| }); |
| expect((await del({ url: "https://file.example/x" })).status).toBe(401); | ||
| }); | ||
|
|
||
| it("returns 400 when url is missing", async () => { |
There was a problem hiding this comment.
P3: Missing error-code assertion in the 400-for-missing-url test. Every other error case in this file checks both the status and the code field — skipping it here means a false-negative refactor (route returning 400 for the wrong reason) slips through. Add expect((await res.json()).code).toBe("BAD_REQUEST").
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/api/avatar/presign.test.ts, line 139:
<comment>Missing error-code assertion in the 400-for-missing-url test. Every other error case in this file checks both the status and the `code` field — skipping it here means a false-negative refactor (route returning 400 for the wrong reason) slips through. Add `expect((await res.json()).code).toBe("BAD_REQUEST")`.</comment>
<file context>
@@ -104,3 +119,51 @@ describe("POST /api/avatar/presign", () => {
+ expect((await del({ url: "https://file.example/x" })).status).toBe(401);
+ });
+
+ it("returns 400 when url is missing", async () => {
+ expect((await del({})).status).toBe(400);
+ });
</file context>
| | | | | ||
| | ------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Request body | `{ url: string }` — the avatar's public URL. | | ||
| | Response | `204 No Content`. Idempotent (R2 404 swallowed). An external / non-R2 URL is a no-op 204. | |
There was a problem hiding this comment.
P3: The DELETE avatar response docs overstate the external-URL no-op behavior. With R2_PUBLIC_BASE_URL unset, the handler returns AVATAR_UPLOADS_NOT_CONFIGURED before it can classify an external URL, so the line should note that the 204 no-op only applies when R2 public base config is available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/APIS.md, line 116:
<comment>The DELETE avatar response docs overstate the external-URL no-op behavior. With `R2_PUBLIC_BASE_URL` unset, the handler returns `AVATAR_UPLOADS_NOT_CONFIGURED` before it can classify an external URL, so the line should note that the 204 no-op only applies when R2 public base config is available.</comment>
<file context>
@@ -98,11 +98,23 @@ Best-effort cleanup of a composer chip. Removes the row + the R2 object. Idempot
+| | |
+| ------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| Request body | `{ url: string }` — the avatar's public URL. |
+| Response | `204 No Content`. Idempotent (R2 404 swallowed). An external / non-R2 URL is a no-op 204. |
+| Failure codes | 400 `BAD_REQUEST` (invalid JSON / missing url). 401 `UNAUTHORIZED`. 403 `FORBIDDEN` (key outside the caller's avatar path). 503 `AVATAR_UPLOADS_NOT_CONFIGURED`. |
</file context>
| | Response | `204 No Content`. Idempotent (R2 404 swallowed). An external / non-R2 URL is a no-op 204. | | |
| | Response | `204 No Content`. Idempotent (R2 404 swallowed). An external / non-R2 URL is a no-op 204 when R2 public-base config is available. | |
1 participant
Summary
Fixes the
gpt-5-nano"context window exceeded" blow-up (#28): a base64 avatar written intouser.imageflowed through the memory auth-overlay into every<memory>system block, uncapped (MEMORY_PROFILE_MAX_BYTESonly caps the stored doc). One short query ballooned to 372K tokens.Also fixes #10 (per-provider email in the Memory tab's Socials row).
Changes
POST /api/avatar/presign— presigns a PUT tou/<userId>/avatar/<id>-<name>, returns the public URL only, no DB row, 503 when R2 unset. Kept separate from/api/attachments/presignon purpose: attachment rows are subject to a retention sweep that would delete the object and break the stored avatar URL.ChangeAvatarnow uploads via presign and stores only the URL — base64 fallback removed. Upload is disabled unless R2 is configured (reuses the existingATTACHMENTS_ENABLEDgate, no new env key).image→avatar.save_memoryrejects base64 / data-URL values (walked through nested arrays/objects) + description forbids it — closes the second bloat vector where the model could persist an image blob into the profile.getAuthInfodecodes each account's OIDCidTokenfor theemailclaim. Google works; GitHub issues no idToken (OAuth2, not OIDC), so github rows stay email-less — noted on the issue.Not in this PR
user.imagestill bloat those users' prompts until they re-upload/delete. A one-off cleanup (null outdata:-prefixeduser.image) would fully clear the S1 — call it out if wanted.Test plan
pnpm test— 743 passed (new: avatar presign route 401/400×3/201/503; idToken decode; base64 rejection)pnpm typecheckcleanpnpm lintcleanuser.imageis an R2 URL, Memory tab showsavatar+ Socials emailCloses #28
Closes #10
🤖 Generated with Claude Code
Summary by cubic
Stores user avatars in R2 and saves only the URL to stop base64 images from exploding the memory context. Also adds verified per‑provider emails in Memory → Socials and blocks any base64/blob data from being saved. Closes #28 and #10.
Bug Fixes
POST /api/avatar/presign; we store only the public URL; rejectsimage/svg+xml; gated byATTACHMENTS_ENABLED, returns 503 when R2 isn’t configured.DELETE /api/avatarremoves the previous R2 avatar on delete and on replace (owner-scoped, idempotent).updateUserfailure and deletes the prior in-flight upload on rapid re-uploads.image→avatar.save_memoryrejects base64/data-URL strings anywhere in the value (covers base64url-_); tool instructions forbid storing blobs.idTokenonly ifemail_verified, GitHub via the API using the stored access token.user.imagevalues are ignored in the overlay, stopping prompt bloat immediately.Migration
ATTACHMENTS_ENABLED="true"and R2 is configured.Written for commit 9a48a53. Summary will update on new commits.
Greptile Summary
This PR fixes the 372K-token context-window blow-up (#28) by routing avatar uploads through a new presigned R2 path (
POST /api/avatar/presign+DELETE /api/avatar) and storing only the public URL — never base64. It also renames the memory auth-overlay keyimage→avatar, filters legacydata:rows from the overlay on read, adds acontainsBase64guard insave_memoryto block blobs at write time, and adds per-provider emails to the Memory Socials row (Google via OIDC idToken decode, GitHub via API).ChangeAvataruploads via presign + PUT,updateUser({ image: publicUrl })stores only the URL; delete flow clears the DB first then removes the R2 object;ATTACHMENTS_ENABLEDgates both paths.getAuthInfonow filtersdata:-prefixed rows so existing legacy avatars stop flowing into<memory>immediately;save_memoryrejects any patch value containing a data-URL or 512+ consecutive base64/base64url characters.getAuthInfodecodes each account's OIDCidTokenfor verified email (Google); falls back toGET /user/emailswith the stored access token for GitHub; failures are best-effort and leave the row email-less.Confidence Score: 5/5
Safe to merge; all new routes are auth-gated, the base64 bloat vectors are closed at two independent layers, and the existing-user migration takes effect immediately without a schema change.
The core fix is correct end-to-end. Both findings are speculative: the inverted pendingUploadRef delete target is guarded in practice by React batched mutation state keeping the button disabled, and the generic presign error message is a UX nit. No data loss, no security regression, no broken migration path.
components/auth/settings/account/change-avatar.tsx — the pendingUploadRef onSuccess cleanup logic and the presign error messaging.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant UI as ChangeAvatar (UI) participant Presign as POST /api/avatar/presign participant R2 as Cloudflare R2 participant BA as Better Auth (updateUser) participant Del as DELETE /api/avatar UI->>Presign: "{name, contentType, sizeBytes}" Presign-->>UI: "{uploadUrl, publicUrl, uploadHeaders}" UI->>R2: PUT file (with uploadHeaders) R2-->>UI: 200 OK UI->>BA: "updateUser({image: publicUrl})" BA-->>UI: onSuccess UI->>Del: "{url: previousAvatarUrl}" Del->>R2: deleteObject(key) R2-->>Del: 204 Del-->>UI: 204%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant UI as ChangeAvatar (UI) participant Presign as POST /api/avatar/presign participant R2 as Cloudflare R2 participant BA as Better Auth (updateUser) participant Del as DELETE /api/avatar UI->>Presign: "{name, contentType, sizeBytes}" Presign-->>UI: "{uploadUrl, publicUrl, uploadHeaders}" UI->>R2: PUT file (with uploadHeaders) R2-->>UI: 200 OK UI->>BA: "updateUser({image: publicUrl})" BA-->>UI: onSuccess UI->>Del: "{url: previousAvatarUrl}" Del->>R2: deleteObject(key) R2-->>Del: 204 Del-->>UI: 204Comments Outside Diff (1)
components/auth/settings/account/change-avatar.tsx, line 75-94 (link)setIsUploading(false)is placed after the try/catch block rather than in afinally. If the catch handler is ever extended to do async work that throws, the upload spinner gets stuck indefinitely. Usingfinallymakes the cleanup unconditional.Prompt To Fix With AI
Reviews (2): Last reviewed commit: "fix(avatar): orphan-proof upload on Bett..." | Re-trigger Greptile
Context used: