feat: avatars for humans #273

2026-06-11T22:04:17Z

talksik commented

2026-06-11 22:04:17 +00:00

(Migrated from github.com)

Closes #70

Summary by CodeRabbit

New Features
- Profile picture management in Settings: upload, replace, or remove your avatar (includes camera capture and re-cropping).
- Avatars everywhere: user avatars now appear across the app (network, particles, reactions, members, top bars).
- Faster, secure avatar loading: avatar previews use signed download URLs with caching/refresh for smoother display.
- Edit dialog UI and preview: clickable avatar opens editor with preview, validation, and size limits.

Closes #70  ## Summary by CodeRabbit * **New Features** * Profile picture management in Settings: upload, replace, or remove your avatar (includes camera capture and re-cropping). * Avatars everywhere: user avatars now appear across the app (network, particles, reactions, members, top bars). * Faster, secure avatar loading: avatar previews use signed download URLs with caching/refresh for smoother display. * Edit dialog UI and preview: clickable avatar opens editor with preview, validation, and size limits.

coderabbitai[bot] commented

2026-06-11 22:04:31 +00:00

(Migrated from github.com)

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 73480fa0-39ce-4438-aeef-71baef5ae49d

📥 Commits

Reviewing files that changed from the base of the PR and between 6f8349c62f and 6ce7cc8651.

📒 Files selected for processing (3)

go/internal/handler/handler.go
go/migrations/000017_human_avatar.down.sql
js/desktop/src/features/particles/stream-card.tsx

💤 Files with no reviewable changes (1)

js/desktop/src/features/particles/stream-card.tsx

🚧 Files skipped from review as they are similar to previous changes (2)

go/internal/handler/handler.go
go/migrations/000017_human_avatar.down.sql

📝 Walkthrough

Walkthrough

Implements user-controlled profile avatars: DB migration and model field, repository/service update/delete support, HTTP handlers and routes, desktop API client methods, avatar utilities/hooks/components, settings edit dialog with upload/camera, and avatar rendering across particle and network UIs.

Changes

Avatar Management Feature

Layer / File(s)	Summary
Database schema and model `go/migrations/000017_human_avatar.up.sql`, `go/migrations/000017_human_avatar.down.sql`, `go/internal/human/models.go`	Adds nullable `avatar_object_id` column and maps it into the Go `Human` model as `AvatarObjectID`.
Repository and service layer `go/internal/human/repository.go`, `go/internal/human/service.go`, `go/internal/human/service_test.go`	Repository selects/scans `avatar_object_id`, adds `updateAvatarObjectID`; service exposes `UpdateAvatar`/`DeleteAvatar`, validates parameters and maps repo errors; tests cover avatar lifecycle.
HTTP handlers and routing `go/internal/handler/handler.go`, `go/cmd/orion/main.go`	Adds `UpdateAvatar` and `DeleteAvatar` handlers (5MB request limit, depot uploads, best-effort depot cleanup), maps avatar field into DTO, renames `DownloadParticleMedia`→`GetObjectDownloadUrl`, and wires settings/particles routes.
API client refactor and avatar methods `js/desktop/src/api/client.ts`, `js/desktop/src/api/types.ts`	Introduces centralized `send` helper, JSON fetch delegation, and client methods `updateAvatar`, `deleteAvatar`, `getAvatarDownloadUrl`; `HumanSchema` adds optional `avatar_object_id`.
Avatar data resolution and utilities `js/desktop/src/hooks/use-avatar-url.ts`, `js/desktop/src/lib/avatar-image.ts`, `js/desktop/src/lib/humans.ts`, `js/desktop/src/hooks/use-presence-positions.ts`, `js/desktop/src/stores/auth-store.ts`	Adds `useAvatarUrl` (React Query) to fetch signed URLs, `toAvatarBlob` to center-crop and encode images, surfaces `avatarObjectId` in human display/presence shapes, and adds `refreshUser` to auth store.
HumanAvatar component `js/desktop/src/components/human-avatar.tsx`	New component resolves signed avatar URLs via `useAvatarUrl` and renders `AvatarImage` or `AvatarFallback` (initials) as fallback.
Avatar edit dialog `js/desktop/src/features/settings/avatar-edit-dialog.tsx`	Adds AvatarEditDialog with upload + camera capture UI, file validation (MIME and 30MB source cap), image conversion via `toAvatarBlob`, API calls for save/remove, preview lifecycle, and CameraCapture subcomponent.
Settings page avatar UI `js/desktop/src/features/settings-page.tsx`	Makes profile avatar clickable `HumanAvatar` with camera overlay and opens AvatarEditDialog for editing.
Particle and network views avatar integration `js/desktop/src/features/particles/*`, `js/desktop/src/features/network-settings.tsx`	Replaces direct initials avatars with `HumanAvatar` across particle list, cards, reaction bar, presence indicators, members overlay, top bar, and network settings; memoized avatar resolution now includes `avatarObjectId`.

Sequence Diagram(s)

sequenceDiagram
  participant Desktop as Desktop Client
  participant Api as ApiServer (handler)
  participant Depot as DepotService
  participant DB as Database

  Desktop->>Api: PUT /humans/me/avatar (blob)
  Api->>Depot: Upload avatar blob -> returns objectId
  Depot-->>Api: objectId
  Api->>Api: humanSvc.UpdateAvatar(userId, objectId)
  Api->>DB: repository.updateAvatarObjectID(userId, objectId)
  DB-->>Api: OK
  Api-->>Desktop: 204 No Content

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped to town with camera bright,

A tiny blob compressed just right,
Snap or drop — a portrait new,
Replaced the initials with a view,
Now profiles glow with me and you.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: avatars for humans' clearly summarizes the main objective of adding avatar functionality for users.
Linked Issues check	✅ Passed	The pull request implements the core requirements from issue `#70`: user-controlled avatar creation via photo snapping or uploading, persistent user-controlled avatars to ensure recognition and consent, and avoids arbitrary video frame usage.
Out of Scope Changes check	✅ Passed	All changes are within scope of the linked issue. Backend adds avatar storage/retrieval infrastructure, frontend implements UI for avatar upload/capture, and media handling ensures avatars are square-cropped and properly formatted.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch worktree-refactored-strolling-treasure

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/flowy-live/llink/pull/273?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)  No actionable comments were generated in the recent review. 🎉 <details> <summary>ℹ️ Recent review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `73480fa0-39ce-4438-aeef-71baef5ae49d` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6f8349c62f5c33eea90050e4cf45d8740322f316 and 6ce7cc865174662c305e04498f5d412b1a13223c. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `go/internal/handler/handler.go` * `go/migrations/000017_human_avatar.down.sql` * `js/desktop/src/features/particles/stream-card.tsx` </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * js/desktop/src/features/particles/stream-card.tsx </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * go/internal/handler/handler.go * go/migrations/000017_human_avatar.down.sql </details> </details> ---  <details> <summary>📝 Walkthrough</summary> ## Walkthrough Implements user-controlled profile avatars: DB migration and model field, repository/service update/delete support, HTTP handlers and routes, desktop API client methods, avatar utilities/hooks/components, settings edit dialog with upload/camera, and avatar rendering across particle and network UIs. ## Changes **Avatar Management Feature** |Layer / File(s)|Summary| |---|---| |**Database schema and model** `go/migrations/000017_human_avatar.up.sql`, `go/migrations/000017_human_avatar.down.sql`, `go/internal/human/models.go`|Adds nullable `avatar_object_id` column and maps it into the Go `Human` model as `AvatarObjectID`.| |**Repository and service layer** `go/internal/human/repository.go`, `go/internal/human/service.go`, `go/internal/human/service_test.go`|Repository selects/scans `avatar_object_id`, adds `updateAvatarObjectID`; service exposes `UpdateAvatar`/`DeleteAvatar`, validates parameters and maps repo errors; tests cover avatar lifecycle.| |**HTTP handlers and routing** `go/internal/handler/handler.go`, `go/cmd/orion/main.go`|Adds `UpdateAvatar` and `DeleteAvatar` handlers (5MB request limit, depot uploads, best-effort depot cleanup), maps avatar field into DTO, renames `DownloadParticleMedia`→`GetObjectDownloadUrl`, and wires settings/particles routes.| |**API client refactor and avatar methods** `js/desktop/src/api/client.ts`, `js/desktop/src/api/types.ts`|Introduces centralized `send` helper, JSON fetch delegation, and client methods `updateAvatar`, `deleteAvatar`, `getAvatarDownloadUrl`; `HumanSchema` adds optional `avatar_object_id`.| |**Avatar data resolution and utilities** `js/desktop/src/hooks/use-avatar-url.ts`, `js/desktop/src/lib/avatar-image.ts`, `js/desktop/src/lib/humans.ts`, `js/desktop/src/hooks/use-presence-positions.ts`, `js/desktop/src/stores/auth-store.ts`|Adds `useAvatarUrl` (React Query) to fetch signed URLs, `toAvatarBlob` to center-crop and encode images, surfaces `avatarObjectId` in human display/presence shapes, and adds `refreshUser` to auth store.| |**HumanAvatar component** `js/desktop/src/components/human-avatar.tsx`|New component resolves signed avatar URLs via `useAvatarUrl` and renders `AvatarImage` or `AvatarFallback` (initials) as fallback.| |**Avatar edit dialog** `js/desktop/src/features/settings/avatar-edit-dialog.tsx`|Adds AvatarEditDialog with upload + camera capture UI, file validation (MIME and 30MB source cap), image conversion via `toAvatarBlob`, API calls for save/remove, preview lifecycle, and CameraCapture subcomponent.| |**Settings page avatar UI** `js/desktop/src/features/settings-page.tsx`|Makes profile avatar clickable `HumanAvatar` with camera overlay and opens AvatarEditDialog for editing.| |**Particle and network views avatar integration** `js/desktop/src/features/particles/*`, `js/desktop/src/features/network-settings.tsx`|Replaces direct initials avatars with `HumanAvatar` across particle list, cards, reaction bar, presence indicators, members overlay, top bar, and network settings; memoized avatar resolution now includes `avatarObjectId`.| ## Sequence Diagram(s) ```mermaid sequenceDiagram participant Desktop as Desktop Client participant Api as ApiServer (handler) participant Depot as DepotService participant DB as Database Desktop->>Api: PUT /humans/me/avatar (blob) Api->>Depot: Upload avatar blob -> returns objectId Depot-->>Api: objectId Api->>Api: humanSvc.UpdateAvatar(userId, objectId) Api->>DB: repository.updateAvatarObjectID(userId, objectId) DB-->>Api: OK Api-->>Desktop: 204 No Content ``` ## Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes ## Poem > 🐰 I hopped to town with camera bright, > > A tiny blob compressed just right, > > Snap or drop — a portrait new, > > Replaced the initials with a view, > > Now profiles glow with me and you. </details>   <details> <summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary> ### ❌ Failed checks (1 warning) | Check name | Status | Explanation | Resolution | | :----------------: | :--------- | :------------------------------------------------------------------------------------ | :--------------------------------------------------------------------------------- | | Docstring Coverage | ⚠️ Warning | Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | <details> <summary>✅ Passed checks (4 passed)</summary> | Check name | Status | Explanation | | :------------------------: | :------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Title check | ✅ Passed | The title 'feat: avatars for humans' clearly summarizes the main objective of adding avatar functionality for users. | | Linked Issues check | ✅ Passed | The pull request implements the core requirements from issue `#70`: user-controlled avatar creation via photo snapping or uploading, persistent user-controlled avatars to ensure recognition and consent, and avoids arbitrary video frame usage. | | Out of Scope Changes check | ✅ Passed | All changes are within scope of the linked issue. Backend adds avatar storage/retrieval infrastructure, frontend implements UI for avatar upload/capture, and media handling ensures avatars are square-cropped and properly formatted. | </details> ✏️ Tip: You can configure your own custom pre-merge checks in the settings. </details>   <details> <summary>✨ Finishing Touches</summary> <details> <summary>📝 Generate docstrings</summary> - [ ]  Create stacked PR - [ ]  Commit on current branch </details> <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ]  Create PR with unit tests - [ ]  Commit unit tests in branch `worktree-refactored-strolling-treasure` </details> </details>   > [!WARNING] > There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. > > <details> > <summary>🔧 golangci-lint (2.12.2)</summary> > > level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" > > > > > </details>   --- Comment `@coderabbitai help` to get the list of available commands and usage tips.

coderabbitai[bot] (Migrated from github.com) reviewed 2026-06-11 22:11:54 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/internal/handler/handler.go (1)
787-800: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Missing authorization check allows arbitrary object download URL issuance.

Line 789 only verifies authentication, then Line 799 returns a signed URL for any provided object ID. This is an IDOR risk and can expose private media/avatars across users or networks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go/internal/handler/handler.go` around lines 787 - 800, GetObjectDownloadUrl
currently only checks authentication via middleware.EmailFromContext but then
issues a signed URL from h.depotSvc.GetDownloadURL for any object id; to fix,
inside Handler.GetObjectDownloadUrl fetch the particle/media metadata and verify
the caller has access (ownership or visibility) before calling
h.depotSvc.GetDownloadURL—use the authenticated identity returned by
middleware.EmailFromContext to authorize (or call your particle service /
Firebase as noted in the TODO) and return http.StatusForbidden when the caller
is not permitted; ensure this authorization check happens prior to requesting
the signed URL.

🧹 Nitpick comments (1)

js/desktop/src/features/particles/particle-list-view.tsx (1)
153-193: ⚡ Quick win

Extract shared avatar-derivation logic to avoid drift with stream-card.tsx.

This memoized DM/creator/fallback avatar resolution is duplicated with near-identical logic in js/desktop/src/features/particles/stream-card.tsx (Line 44-84). A shared helper (e.g., resolveStreamAvatarDisplay) would keep behavior consistent and reduce future regressions.

As per coding guidelines, "Write simpler, maintainable code in the Electron app instead of clever behavior."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/desktop/src/features/particles/particle-list-view.tsx` around lines 153 -
193, The avatar derivation logic inside the avatar useMemo should be extracted
into a shared helper (e.g., export function resolveStreamAvatarDisplay) and both
particle-list-view.tsx and stream-card.tsx should call that helper instead of
duplicating logic; implement resolveStreamAvatarDisplay to accept the same
inputs used in the memo (particle, userId, network, latestChild, isDM), return
the same shape ({ initials, avatarObjectId }), export it from a common utilities
module, replace the useMemo body in particle-list-view (the avatar variable) and
the duplicated block in stream-card.tsx to call the helper (keeping memoization
callers as needed), and update imports accordingly so behavior and return shape
remain identical.
Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@go/internal/handler/handler.go`:
- Around line 345-365: The handler currently clears avatar_object_id before
capturing the prior object and thus leaks the old object; also on upload the
code uploads to h.depotSvc but if the subsequent DB update
(h.humanSvc.*Update/SetAvatar) fails the newly uploaded blob is orphaned. Fix by
first calling h.humanSvc.GetByID(r.Context(), humanId) to capture
oldAvatarObjectID, then call h.humanSvc.DeleteAvatar(...) (or the DB change);
after the DB operation succeeds, call h.depotSvc.Delete(r.Context(),
oldAvatarObjectID) to remove the previous blob; for uploads, perform
h.depotSvc.Upload(...) then attempt the DB update (e.g.,
h.humanSvc.SetAvatar/UpdateAvatar); if the DB update fails, call
h.depotSvc.Delete(r.Context(), newObjectID) to roll back the uploaded blob so it
isn’t orphaned. Ensure you reference and use h.humanSvc.GetByID,
h.humanSvc.DeleteAvatar (or Update/SetAvatar), h.depotSvc.Upload, and
h.depotSvc.Delete in this new order and add error logging around the rollback
delete.
- Around line 381-391: The handler wraps the request body with
http.MaxBytesReader but treats any upload-size error from
h.depotSvc.CreateFromReader as a 500; detect the MaxBytesReader overflow and
return http.StatusRequestEntityTooLarge instead. Update the error handling after
calling h.depotSvc.CreateFromReader: if errors.Is(err, http.ErrBodyTooLarge) (or
if that sentinel isn't available, match the "request body too large" error
string) respond with http.Error(w, "request entity too large",
http.StatusRequestEntityTooLarge); otherwise keep the existing flog.Error and
500 behavior. Ensure you import the errors package if needed and reference the
r.Body wrapping and h.depotSvc.CreateFromReader call in your change.

In `@go/migrations/000017_human_avatar.down.sql`:
- Line 4: The rollback migration SQL in 000017_human_avatar.down.sql contains a
trailing comma after the DROP COLUMN list ("DROP COLUMN IF EXISTS
avatar_object_id,") which makes the statement invalid; remove the trailing comma
so the DROP COLUMN clause is a properly terminated SQL statement (ensure the
DROP COLUMN list ends without a comma and the statement ends with the
appropriate semicolon or end-of-statement token).

In `@js/desktop/src/api/client.ts`:
- Around line 160-163: The getAvatarDownloadUrl method builds the path using
objectId directly, which can break for reserved characters; update
getAvatarDownloadUrl to encode the id before interpolation (e.g., use
encodeURIComponent on the objectId when constructing `/humans/avatar/${...}`) so
the call to this.fetch('GET', `/humans/avatar/${objectId}`) always sends a valid
path; ensure any callers that pass objectId still provide the raw id and let
this method perform encoding.

In `@js/desktop/src/lib/avatar-image.ts`:
- Around line 11-12: Validate and normalize the incoming size parameter at the
start of the avatar generation function (the function with signature "{ size =
512, mirror = false }: { size?: number; mirror?: boolean } = {}, ):
Promise<Blob>"). Ensure size is a positive integer before using it to create the
canvas: check Number.isInteger(size) && size > 0 (or coerce via Math.floor and
then verify >0), and if invalid either throw a clear error or fall back to a
safe default (e.g., 512); then use that validated/normalized value for the
canvas creation to prevent runtime errors or broken avatars.

---

Outside diff comments:
In `@go/internal/handler/handler.go`:
- Around line 787-800: GetObjectDownloadUrl currently only checks authentication
via middleware.EmailFromContext but then issues a signed URL from
h.depotSvc.GetDownloadURL for any object id; to fix, inside
Handler.GetObjectDownloadUrl fetch the particle/media metadata and verify the
caller has access (ownership or visibility) before calling
h.depotSvc.GetDownloadURL—use the authenticated identity returned by
middleware.EmailFromContext to authorize (or call your particle service /
Firebase as noted in the TODO) and return http.StatusForbidden when the caller
is not permitted; ensure this authorization check happens prior to requesting
the signed URL.

---

Nitpick comments:
In `@js/desktop/src/features/particles/particle-list-view.tsx`:
- Around line 153-193: The avatar derivation logic inside the avatar useMemo
should be extracted into a shared helper (e.g., export function
resolveStreamAvatarDisplay) and both particle-list-view.tsx and stream-card.tsx
should call that helper instead of duplicating logic; implement
resolveStreamAvatarDisplay to accept the same inputs used in the memo (particle,
userId, network, latestChild, isDM), return the same shape ({ initials,
avatarObjectId }), export it from a common utilities module, replace the useMemo
body in particle-list-view (the avatar variable) and the duplicated block in
stream-card.tsx to call the helper (keeping memoization callers as needed), and
update imports accordingly so behavior and return shape remain identical.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f1b2fa73-7efe-4b67-abc5-45c5f2ee1d57

📥 Commits

Reviewing files that changed from the base of the PR and between ca3bbf204e and fa9d54aab5.

📒 Files selected for processing (25)

go/cmd/orion/main.go
go/internal/handler/handler.go
go/internal/human/models.go
go/internal/human/repository.go
go/internal/human/service.go
go/internal/human/service_test.go
go/migrations/000017_human_avatar.down.sql
go/migrations/000017_human_avatar.up.sql
js/desktop/src/api/client.ts
js/desktop/src/api/types.ts
js/desktop/src/components/human-avatar.tsx
js/desktop/src/features/network-settings.tsx
js/desktop/src/features/particles/particle-list-view.tsx
js/desktop/src/features/particles/playback-page-indicator.tsx
js/desktop/src/features/particles/reaction-bar.tsx
js/desktop/src/features/particles/stream-card.tsx
js/desktop/src/features/particles/stream-members-overlay.tsx
js/desktop/src/features/particles/stream-top-bar.tsx
js/desktop/src/features/settings-page.tsx
js/desktop/src/features/settings/avatar-edit-dialog.tsx
js/desktop/src/hooks/use-avatar-url.ts
js/desktop/src/hooks/use-presence-positions.ts
js/desktop/src/lib/avatar-image.ts
js/desktop/src/lib/humans.ts
js/desktop/src/stores/auth-store.ts

**Actionable comments posted: 5** > [!CAUTION] > Some comments are outside the diff and can’t be posted inline due to platform limitations. > > > > <details> > <summary>⚠️ Outside diff range comments (1)</summary><blockquote> > > <details> > <summary>go/internal/handler/handler.go (1)</summary><blockquote> > > `787-800`: _⚠️ Potential issue_ | _🔴 Critical_ | _🏗️ Heavy lift_ > > **Missing authorization check allows arbitrary object download URL issuance.** > > Line 789 only verifies authentication, then Line 799 returns a signed URL for any provided object ID. This is an IDOR risk and can expose private media/avatars across users or networks. > > <details> > <summary>🤖 Prompt for AI Agents</summary> > > ``` > Verify each finding against current code. Fix only still-valid issues, skip the > rest with a brief reason, keep changes minimal, and validate. > > In `@go/internal/handler/handler.go` around lines 787 - 800, GetObjectDownloadUrl > currently only checks authentication via middleware.EmailFromContext but then > issues a signed URL from h.depotSvc.GetDownloadURL for any object id; to fix, > inside Handler.GetObjectDownloadUrl fetch the particle/media metadata and verify > the caller has access (ownership or visibility) before calling > h.depotSvc.GetDownloadURL—use the authenticated identity returned by > middleware.EmailFromContext to authorize (or call your particle service / > Firebase as noted in the TODO) and return http.StatusForbidden when the caller > is not permitted; ensure this authorization check happens prior to requesting > the signed URL. > ``` > > </details> > >  > > </blockquote></details> > > </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>js/desktop/src/features/particles/particle-list-view.tsx (1)</summary><blockquote> `153-193`: _⚡ Quick win_ **Extract shared avatar-derivation logic to avoid drift with `stream-card.tsx`.** This memoized DM/creator/fallback avatar resolution is duplicated with near-identical logic in `js/desktop/src/features/particles/stream-card.tsx` (Line 44-84). A shared helper (e.g., `resolveStreamAvatarDisplay`) would keep behavior consistent and reduce future regressions. As per coding guidelines, "Write simpler, maintainable code in the Electron app instead of clever behavior." <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/src/features/particles/particle-list-view.tsx` around lines 153 - 193, The avatar derivation logic inside the avatar useMemo should be extracted into a shared helper (e.g., export function resolveStreamAvatarDisplay) and both particle-list-view.tsx and stream-card.tsx should call that helper instead of duplicating logic; implement resolveStreamAvatarDisplay to accept the same inputs used in the memo (particle, userId, network, latestChild, isDM), return the same shape ({ initials, avatarObjectId }), export it from a common utilities module, replace the useMemo body in particle-list-view (the avatar variable) and the duplicated block in stream-card.tsx to call the helper (keeping memoization callers as needed), and update imports accordingly so behavior and return shape remain identical. ``` </details>  _Source: Coding guidelines_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@go/internal/handler/handler.go`: - Around line 345-365: The handler currently clears avatar_object_id before capturing the prior object and thus leaks the old object; also on upload the code uploads to h.depotSvc but if the subsequent DB update (h.humanSvc.*Update/SetAvatar) fails the newly uploaded blob is orphaned. Fix by first calling h.humanSvc.GetByID(r.Context(), humanId) to capture oldAvatarObjectID, then call h.humanSvc.DeleteAvatar(...) (or the DB change); after the DB operation succeeds, call h.depotSvc.Delete(r.Context(), oldAvatarObjectID) to remove the previous blob; for uploads, perform h.depotSvc.Upload(...) then attempt the DB update (e.g., h.humanSvc.SetAvatar/UpdateAvatar); if the DB update fails, call h.depotSvc.Delete(r.Context(), newObjectID) to roll back the uploaded blob so it isn’t orphaned. Ensure you reference and use h.humanSvc.GetByID, h.humanSvc.DeleteAvatar (or Update/SetAvatar), h.depotSvc.Upload, and h.depotSvc.Delete in this new order and add error logging around the rollback delete. - Around line 381-391: The handler wraps the request body with http.MaxBytesReader but treats any upload-size error from h.depotSvc.CreateFromReader as a 500; detect the MaxBytesReader overflow and return http.StatusRequestEntityTooLarge instead. Update the error handling after calling h.depotSvc.CreateFromReader: if errors.Is(err, http.ErrBodyTooLarge) (or if that sentinel isn't available, match the "request body too large" error string) respond with http.Error(w, "request entity too large", http.StatusRequestEntityTooLarge); otherwise keep the existing flog.Error and 500 behavior. Ensure you import the errors package if needed and reference the r.Body wrapping and h.depotSvc.CreateFromReader call in your change. In `@go/migrations/000017_human_avatar.down.sql`: - Line 4: The rollback migration SQL in 000017_human_avatar.down.sql contains a trailing comma after the DROP COLUMN list ("DROP COLUMN IF EXISTS avatar_object_id,") which makes the statement invalid; remove the trailing comma so the DROP COLUMN clause is a properly terminated SQL statement (ensure the DROP COLUMN list ends without a comma and the statement ends with the appropriate semicolon or end-of-statement token). In `@js/desktop/src/api/client.ts`: - Around line 160-163: The getAvatarDownloadUrl method builds the path using objectId directly, which can break for reserved characters; update getAvatarDownloadUrl to encode the id before interpolation (e.g., use encodeURIComponent on the objectId when constructing `/humans/avatar/${...}`) so the call to this.fetch('GET', `/humans/avatar/${objectId}`) always sends a valid path; ensure any callers that pass objectId still provide the raw id and let this method perform encoding. In `@js/desktop/src/lib/avatar-image.ts`: - Around line 11-12: Validate and normalize the incoming size parameter at the start of the avatar generation function (the function with signature "{ size = 512, mirror = false }: { size?: number; mirror?: boolean } = {}, ): Promise<Blob>"). Ensure size is a positive integer before using it to create the canvas: check Number.isInteger(size) && size > 0 (or coerce via Math.floor and then verify >0), and if invalid either throw a clear error or fall back to a safe default (e.g., 512); then use that validated/normalized value for the canvas creation to prevent runtime errors or broken avatars. --- Outside diff comments: In `@go/internal/handler/handler.go`: - Around line 787-800: GetObjectDownloadUrl currently only checks authentication via middleware.EmailFromContext but then issues a signed URL from h.depotSvc.GetDownloadURL for any object id; to fix, inside Handler.GetObjectDownloadUrl fetch the particle/media metadata and verify the caller has access (ownership or visibility) before calling h.depotSvc.GetDownloadURL—use the authenticated identity returned by middleware.EmailFromContext to authorize (or call your particle service / Firebase as noted in the TODO) and return http.StatusForbidden when the caller is not permitted; ensure this authorization check happens prior to requesting the signed URL. --- Nitpick comments: In `@js/desktop/src/features/particles/particle-list-view.tsx`: - Around line 153-193: The avatar derivation logic inside the avatar useMemo should be extracted into a shared helper (e.g., export function resolveStreamAvatarDisplay) and both particle-list-view.tsx and stream-card.tsx should call that helper instead of duplicating logic; implement resolveStreamAvatarDisplay to accept the same inputs used in the memo (particle, userId, network, latestChild, isDM), return the same shape ({ initials, avatarObjectId }), export it from a common utilities module, replace the useMemo body in particle-list-view (the avatar variable) and the duplicated block in stream-card.tsx to call the helper (keeping memoization callers as needed), and update imports accordingly so behavior and return shape remain identical. ``` </details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ]  Push a commit to this branch (recommended) - [ ]  Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `f1b2fa73-7efe-4b67-abc5-45c5f2ee1d57` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between ca3bbf204eb997627d682e97a2dfd3e03235bdfb and fa9d54aab5ba3fc28c8c8930154394e653515576. </details> <details> <summary>📒 Files selected for processing (25)</summary> * `go/cmd/orion/main.go` * `go/internal/handler/handler.go` * `go/internal/human/models.go` * `go/internal/human/repository.go` * `go/internal/human/service.go` * `go/internal/human/service_test.go` * `go/migrations/000017_human_avatar.down.sql` * `go/migrations/000017_human_avatar.up.sql` * `js/desktop/src/api/client.ts` * `js/desktop/src/api/types.ts` * `js/desktop/src/components/human-avatar.tsx` * `js/desktop/src/features/network-settings.tsx` * `js/desktop/src/features/particles/particle-list-view.tsx` * `js/desktop/src/features/particles/playback-page-indicator.tsx` * `js/desktop/src/features/particles/reaction-bar.tsx` * `js/desktop/src/features/particles/stream-card.tsx` * `js/desktop/src/features/particles/stream-members-overlay.tsx` * `js/desktop/src/features/particles/stream-top-bar.tsx` * `js/desktop/src/features/settings-page.tsx` * `js/desktop/src/features/settings/avatar-edit-dialog.tsx` * `js/desktop/src/hooks/use-avatar-url.ts` * `js/desktop/src/hooks/use-presence-positions.ts` * `js/desktop/src/lib/avatar-image.ts` * `js/desktop/src/lib/humans.ts` * `js/desktop/src/stores/auth-store.ts` </details> </details>

go/internal/handler/handler.go

						
				@@ -334,6 +335,83 @@ func (h *Handler) UpdateSettings(w http.ResponseWriter, r *http.Request) {

					w.WriteHeader(http.StatusNoContent)

coderabbitai[bot] (Migrated from github.com) commented

2026-06-11 22:11:52 +00:00

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Avatar blob lifecycle is inconsistent and leaks objects.

Line 345 clears avatar_object_id before reading prior state, so the old avatar object ID is lost and never deleted.
Also, if Line 383 upload succeeds but Line 394 DB update fails, the newly uploaded object is orphaned.

Suggested lifecycle shape

+// DeleteAvatar:
+// 1) read current human/avatar_object_id
+// 2) clear avatar_object_id in DB
+// 3) best-effort delete old object using captured ID

+// UpdateAvatar:
+// 1) read current avatar_object_id (oldID)
+// 2) upload new object (newID)
+// 3) update DB to newID
+// 4) if DB update fails, best-effort delete newID
+// 5) after successful DB update, best-effort delete oldID

Also applies to: 383-399

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go/internal/handler/handler.go` around lines 345 - 365, The handler currently
clears avatar_object_id before capturing the prior object and thus leaks the old
object; also on upload the code uploads to h.depotSvc but if the subsequent DB
update (h.humanSvc.*Update/SetAvatar) fails the newly uploaded blob is orphaned.
Fix by first calling h.humanSvc.GetByID(r.Context(), humanId) to capture
oldAvatarObjectID, then call h.humanSvc.DeleteAvatar(...) (or the DB change);
after the DB operation succeeds, call h.depotSvc.Delete(r.Context(),
oldAvatarObjectID) to remove the previous blob; for uploads, perform
h.depotSvc.Upload(...) then attempt the DB update (e.g.,
h.humanSvc.SetAvatar/UpdateAvatar); if the DB update fails, call
h.depotSvc.Delete(r.Context(), newObjectID) to roll back the uploaded blob so it
isn’t orphaned. Ensure you reference and use h.humanSvc.GetByID,
h.humanSvc.DeleteAvatar (or Update/SetAvatar), h.depotSvc.Upload, and
h.depotSvc.Delete in this new order and add error logging around the rollback
delete.

_⚠️ Potential issue_ | _🟠 Major_ | _🏗️ Heavy lift_ **Avatar blob lifecycle is inconsistent and leaks objects.** Line 345 clears `avatar_object_id` before reading prior state, so the old avatar object ID is lost and never deleted. Also, if Line 383 upload succeeds but Line 394 DB update fails, the newly uploaded object is orphaned. <details> <summary>Suggested lifecycle shape</summary> ```diff +// DeleteAvatar: +// 1) read current human/avatar_object_id +// 2) clear avatar_object_id in DB +// 3) best-effort delete old object using captured ID +// UpdateAvatar: +// 1) read current avatar_object_id (oldID) +// 2) upload new object (newID) +// 3) update DB to newID +// 4) if DB update fails, best-effort delete newID +// 5) after successful DB update, best-effort delete oldID ``` </details> Also applies to: 383-399 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go/internal/handler/handler.go` around lines 345 - 365, The handler currently clears avatar_object_id before capturing the prior object and thus leaks the old object; also on upload the code uploads to h.depotSvc but if the subsequent DB update (h.humanSvc.*Update/SetAvatar) fails the newly uploaded blob is orphaned. Fix by first calling h.humanSvc.GetByID(r.Context(), humanId) to capture oldAvatarObjectID, then call h.humanSvc.DeleteAvatar(...) (or the DB change); after the DB operation succeeds, call h.depotSvc.Delete(r.Context(), oldAvatarObjectID) to remove the previous blob; for uploads, perform h.depotSvc.Upload(...) then attempt the DB update (e.g., h.humanSvc.SetAvatar/UpdateAvatar); if the DB update fails, call h.depotSvc.Delete(r.Context(), newObjectID) to roll back the uploaded blob so it isn’t orphaned. Ensure you reference and use h.humanSvc.GetByID, h.humanSvc.DeleteAvatar (or Update/SetAvatar), h.depotSvc.Upload, and h.depotSvc.Delete in this new order and add error logging around the rollback delete. ``` </details>

go/internal/handler/handler.go

						
				@@ -337,0 +400,4 @@

						flog.Error("failed to update human avatar", "error", err, "humanId", humanId)

						http.Error(w, "internal server error", http.StatusInternalServerError)

						// best effort

coderabbitai[bot] (Migrated from github.com) commented

2026-06-11 22:11:53 +00:00

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Return 413 for oversized avatar uploads instead of 500.

When http.MaxBytesReader limit is exceeded, the handler currently falls into a generic 500. This should map to http.StatusRequestEntityTooLarge.

Suggested fix

  object, err := h.depotSvc.CreateFromReader(r.Context(), depot.CreateFromReaderInput{
      Prefix:      "avatars",
      Name:        fmt.Sprintf("%s-avatar", humanId),
      ContentType: r.Header.Get("Content-Type"),
  }, r.Body)
  if err != nil {
+     var maxErr *http.MaxBytesError
+     if errors.As(err, &maxErr) {
+         http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge)
+         return
+     }
      flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId)
      http.Error(w, "internal server error", http.StatusInternalServerError)
      return
  }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	r.Body = http.MaxBytesReader(w, r.Body, maxBodySize)

	object, err := h.depotSvc.CreateFromReader(r.Context(), depot.CreateFromReaderInput{
		Prefix:      "avatars",
		Name:        fmt.Sprintf("%s-avatar", humanId),
		ContentType: r.Header.Get("Content-Type"),
	}, r.Body)
	if err != nil {
		var maxErr *http.MaxBytesError
		if errors.As(err, &maxErr) {
			http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge)
			return
		}
		flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId)
		http.Error(w, "internal server error", http.StatusInternalServerError)
		return
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go/internal/handler/handler.go` around lines 381 - 391, The handler wraps the
request body with http.MaxBytesReader but treats any upload-size error from
h.depotSvc.CreateFromReader as a 500; detect the MaxBytesReader overflow and
return http.StatusRequestEntityTooLarge instead. Update the error handling after
calling h.depotSvc.CreateFromReader: if errors.Is(err, http.ErrBodyTooLarge) (or
if that sentinel isn't available, match the "request body too large" error
string) respond with http.Error(w, "request entity too large",
http.StatusRequestEntityTooLarge); otherwise keep the existing flog.Error and
500 behavior. Ensure you import the errors package if needed and reference the
r.Body wrapping and h.depotSvc.CreateFromReader call in your change.

✅ Addressed in commits ffac812 to 6bf2c99

_⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Return 413 for oversized avatar uploads instead of 500.** When `http.MaxBytesReader` limit is exceeded, the handler currently falls into a generic 500. This should map to `http.StatusRequestEntityTooLarge`. <details> <summary>Suggested fix</summary> ```diff object, err := h.depotSvc.CreateFromReader(r.Context(), depot.CreateFromReaderInput{ Prefix: "avatars", Name: fmt.Sprintf("%s-avatar", humanId), ContentType: r.Header.Get("Content-Type"), }, r.Body) if err != nil { + var maxErr *http.MaxBytesError + if errors.As(err, &maxErr) { + http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge) + return + } flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId) http.Error(w, "internal server error", http.StatusInternalServerError) return } ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion r.Body = http.MaxBytesReader(w, r.Body, maxBodySize) object, err := h.depotSvc.CreateFromReader(r.Context(), depot.CreateFromReaderInput{ Prefix: "avatars", Name: fmt.Sprintf("%s-avatar", humanId), ContentType: r.Header.Get("Content-Type"), }, r.Body) if err != nil { var maxErr *http.MaxBytesError if errors.As(err, &maxErr) { http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge) return } flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId) http.Error(w, "internal server error", http.StatusInternalServerError) return } ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go/internal/handler/handler.go` around lines 381 - 391, The handler wraps the request body with http.MaxBytesReader but treats any upload-size error from h.depotSvc.CreateFromReader as a 500; detect the MaxBytesReader overflow and return http.StatusRequestEntityTooLarge instead. Update the error handling after calling h.depotSvc.CreateFromReader: if errors.Is(err, http.ErrBodyTooLarge) (or if that sentinel isn't available, match the "request body too large" error string) respond with http.Error(w, "request entity too large", http.StatusRequestEntityTooLarge); otherwise keep the existing flog.Error and 500 behavior. Ensure you import the errors package if needed and reference the r.Body wrapping and h.depotSvc.CreateFromReader call in your change. ``` </details>    ✅ Addressed in commits ffac812 to 6bf2c99

go/migrations/000017_human_avatar.down.sql

						
				@@ -0,0 +1,6 @@

				BEGIN;

coderabbitai[bot] (Migrated from github.com) commented

2026-06-11 22:11:53 +00:00

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix invalid SQL in rollback migration.

Line 4 has a trailing comma, which makes the down migration fail to execute.

Suggested fix

 ALTER TABLE humans
-  DROP COLUMN IF EXISTS avatar_object_id,
+  DROP COLUMN IF EXISTS avatar_object_id;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

ALTER TABLE humans
  DROP COLUMN IF EXISTS avatar_object_id;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go/migrations/000017_human_avatar.down.sql` at line 4, The rollback migration
SQL in 000017_human_avatar.down.sql contains a trailing comma after the DROP
COLUMN list ("DROP COLUMN IF EXISTS avatar_object_id,") which makes the
statement invalid; remove the trailing comma so the DROP COLUMN clause is a
properly terminated SQL statement (ensure the DROP COLUMN list ends without a
comma and the statement ends with the appropriate semicolon or end-of-statement
token).

✅ Addressed in commits a45beb9 to a25fb83

_⚠️ Potential issue_ | _🔴 Critical_ | _⚡ Quick win_ **Fix invalid SQL in rollback migration.** Line 4 has a trailing comma, which makes the down migration fail to execute. <details> <summary>Suggested fix</summary> ```diff ALTER TABLE humans - DROP COLUMN IF EXISTS avatar_object_id, + DROP COLUMN IF EXISTS avatar_object_id; ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion ALTER TABLE humans DROP COLUMN IF EXISTS avatar_object_id; ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go/migrations/000017_human_avatar.down.sql` at line 4, The rollback migration SQL in 000017_human_avatar.down.sql contains a trailing comma after the DROP COLUMN list ("DROP COLUMN IF EXISTS avatar_object_id,") which makes the statement invalid; remove the trailing comma so the DROP COLUMN clause is a properly terminated SQL statement (ensure the DROP COLUMN list ends without a comma and the statement ends with the appropriate semicolon or end-of-statement token). ``` </details>    ✅ Addressed in commits a45beb9 to a25fb83

js/desktop/src/api/client.ts

coderabbitai[bot] (Migrated from github.com) commented

2026-06-11 22:11:53 +00:00

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Encode objectId before building the avatar URL path.

Line 161 interpolates objectId directly. Reserved characters can produce a malformed route and wrong request target.

Suggested fix

   async getAvatarDownloadUrl(objectId: string): Promise<string> {
-    const response = await this.fetch('GET', `/humans/avatar/${objectId}`);
+    const encodedObjectId = encodeURIComponent(objectId);
+    const response = await this.fetch('GET', `/humans/avatar/${encodedObjectId}`);
     const data = await response.json();
     return data.url;
   }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  async getAvatarDownloadUrl(objectId: string): Promise<string> {
    const encodedObjectId = encodeURIComponent(objectId);
    const response = await this.fetch('GET', `/humans/avatar/${encodedObjectId}`);
    const data = await response.json();
    return data.url;
  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/desktop/src/api/client.ts` around lines 160 - 163, The
getAvatarDownloadUrl method builds the path using objectId directly, which can
break for reserved characters; update getAvatarDownloadUrl to encode the id
before interpolation (e.g., use encodeURIComponent on the objectId when
constructing `/humans/avatar/${...}`) so the call to this.fetch('GET',
`/humans/avatar/${objectId}`) always sends a valid path; ensure any callers that
pass objectId still provide the raw id and let this method perform encoding.

_⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Encode `objectId` before building the avatar URL path.** Line 161 interpolates `objectId` directly. Reserved characters can produce a malformed route and wrong request target. <details> <summary>Suggested fix</summary> ```diff async getAvatarDownloadUrl(objectId: string): Promise<string> { - const response = await this.fetch('GET', `/humans/avatar/${objectId}`); + const encodedObjectId = encodeURIComponent(objectId); + const response = await this.fetch('GET', `/humans/avatar/${encodedObjectId}`); const data = await response.json(); return data.url; } ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion async getAvatarDownloadUrl(objectId: string): Promise<string> { const encodedObjectId = encodeURIComponent(objectId); const response = await this.fetch('GET', `/humans/avatar/${encodedObjectId}`); const data = await response.json(); return data.url; } ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/src/api/client.ts` around lines 160 - 163, The getAvatarDownloadUrl method builds the path using objectId directly, which can break for reserved characters; update getAvatarDownloadUrl to encode the id before interpolation (e.g., use encodeURIComponent on the objectId when constructing `/humans/avatar/${...}`) so the call to this.fetch('GET', `/humans/avatar/${objectId}`) always sends a valid path; ensure any callers that pass objectId still provide the raw id and let this method perform encoding. ``` </details>

js/desktop/src/lib/avatar-image.ts

						
				@@ -0,0 +9,4 @@

				export async function toAvatarBlob(

				  source: ImageBitmap,

				  { size = 512, mirror = false }: { size?: number; mirror?: boolean } = {},

				): Promise<Blob> {

coderabbitai[bot] (Migrated from github.com) commented

2026-06-11 22:11:53 +00:00

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate size as a positive integer before creating the canvas.

Line 22 depends on size; invalid values can yield broken avatar output or throw at runtime.

Suggested fix

 export async function toAvatarBlob(
   source: ImageBitmap,
   { size = 512, mirror = false }: { size?: number; mirror?: boolean } = {},
 ): Promise<Blob> {
+  if (!Number.isInteger(size) || size <= 0) {
+    throw new Error('Avatar size must be a positive integer');
+  }
   const side = Math.min(source.width, source.height);
   if (side === 0) {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  { size = 512, mirror = false }: { size?: number; mirror?: boolean } = {},
): Promise<Blob> {
  if (!Number.isInteger(size) || size <= 0) {
    throw new Error('Avatar size must be a positive integer');
  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/desktop/src/lib/avatar-image.ts` around lines 11 - 12, Validate and
normalize the incoming size parameter at the start of the avatar generation
function (the function with signature "{ size = 512, mirror = false }: { size?:
number; mirror?: boolean } = {}, ): Promise<Blob>"). Ensure size is a positive
integer before using it to create the canvas: check Number.isInteger(size) &&
size > 0 (or coerce via Math.floor and then verify >0), and if invalid either
throw a clear error or fall back to a safe default (e.g., 512); then use that
validated/normalized value for the canvas creation to prevent runtime errors or
broken avatars.

✅ Addressed in commits ffac812 to 6bf2c99

_⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Validate `size` as a positive integer before creating the canvas.** Line 22 depends on `size`; invalid values can yield broken avatar output or throw at runtime. <details> <summary>Suggested fix</summary> ```diff export async function toAvatarBlob( source: ImageBitmap, { size = 512, mirror = false }: { size?: number; mirror?: boolean } = {}, ): Promise<Blob> { + if (!Number.isInteger(size) || size <= 0) { + throw new Error('Avatar size must be a positive integer'); + } const side = Math.min(source.width, source.height); if (side === 0) { ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion { size = 512, mirror = false }: { size?: number; mirror?: boolean } = {}, ): Promise<Blob> { if (!Number.isInteger(size) || size <= 0) { throw new Error('Avatar size must be a positive integer'); } ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/src/lib/avatar-image.ts` around lines 11 - 12, Validate and normalize the incoming size parameter at the start of the avatar generation function (the function with signature "{ size = 512, mirror = false }: { size?: number; mirror?: boolean } = {}, ): Promise<Blob>"). Ensure size is a positive integer before using it to create the canvas: check Number.isInteger(size) && size > 0 (or coerce via Math.floor and then verify >0), and if invalid either throw a clear error or fall back to a safe default (e.g., 512); then use that validated/normalized value for the canvas creation to prevent runtime errors or broken avatars. ``` </details>    ✅ Addressed in commits ffac812 to 6bf2c99

coderabbitai[bot] (Migrated from github.com) reviewed 2026-06-11 22:21:17 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/internal/handler/handler.go (1)
793-805: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Enforce object-level authorization before minting signed URLs.

This handler only checks that the caller is authenticated, then signs whatever object ID is in the path. With this layer now serving avatar/object downloads, any authenticated user who learns another object's ID can mint a valid URL without a membership or ownership check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go/internal/handler/handler.go` around lines 793 - 805, In
GetObjectDownloadUrl, do an object-level authorization check before calling
h.depotSvc.GetDownloadURL: extract the caller identity using
middleware.EmailFromContext(r.Context()), fetch the object's metadata (e.g., via
an existing particle/object lookup like a particle service or depot metadata
method), verify the object's visibility or that the caller is an owner/member
(compare the fetched metadata's owner/members/visibility fields to the
email/identity), and if the check fails return http.Error(...,
http.StatusForbidden); only call h.depotSvc.GetDownloadURL(r.Context(),
objectID) and write the URL on success. Ensure you use the existing symbols
Handler.GetObjectDownloadUrl, middleware.EmailFromContext, and
h.depotSvc.GetDownloadURL to locate the code to change.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@go/internal/handler/handler.go`:
- Around line 388-398: The UpdateAvatar function contains an extra closing brace
'}' after the error handling block (the one closing the function early) which
causes a parse error; remove that unmatched '}' so the function body continues
normally (locate the error handling block in UpdateAvatar around the errors.As
check and the flog.Error/http.Error calls and delete the extraneous closing
brace), then run go build to confirm parsing succeeds.

---

Outside diff comments:
In `@go/internal/handler/handler.go`:
- Around line 793-805: In GetObjectDownloadUrl, do an object-level authorization
check before calling h.depotSvc.GetDownloadURL: extract the caller identity
using middleware.EmailFromContext(r.Context()), fetch the object's metadata
(e.g., via an existing particle/object lookup like a particle service or depot
metadata method), verify the object's visibility or that the caller is an
owner/member (compare the fetched metadata's owner/members/visibility fields to
the email/identity), and if the check fails return http.Error(...,
http.StatusForbidden); only call h.depotSvc.GetDownloadURL(r.Context(),
objectID) and write the URL on success. Ensure you use the existing symbols
Handler.GetObjectDownloadUrl, middleware.EmailFromContext, and
h.depotSvc.GetDownloadURL to locate the code to change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5eee9ab3-504d-4409-8a31-6fdeca6719b6

📥 Commits

Reviewing files that changed from the base of the PR and between fa9d54aab5 and 6bf2c9983a.

📒 Files selected for processing (2)

go/internal/handler/handler.go
js/desktop/src/lib/avatar-image.ts

🚧 Files skipped from review as they are similar to previous changes (1)

js/desktop/src/lib/avatar-image.ts

**Actionable comments posted: 1** > [!CAUTION] > Some comments are outside the diff and can’t be posted inline due to platform limitations. > > > > <details> > <summary>⚠️ Outside diff range comments (1)</summary><blockquote> > > <details> > <summary>go/internal/handler/handler.go (1)</summary><blockquote> > > `793-805`: _⚠️ Potential issue_ | _🟠 Major_ | _🏗️ Heavy lift_ > > **Enforce object-level authorization before minting signed URLs.** > > This handler only checks that the caller is authenticated, then signs whatever object ID is in the path. With this layer now serving avatar/object downloads, any authenticated user who learns another object's ID can mint a valid URL without a membership or ownership check. > > <details> > <summary>🤖 Prompt for AI Agents</summary> > > ``` > Verify each finding against current code. Fix only still-valid issues, skip the > rest with a brief reason, keep changes minimal, and validate. > > In `@go/internal/handler/handler.go` around lines 793 - 805, In > GetObjectDownloadUrl, do an object-level authorization check before calling > h.depotSvc.GetDownloadURL: extract the caller identity using > middleware.EmailFromContext(r.Context()), fetch the object's metadata (e.g., via > an existing particle/object lookup like a particle service or depot metadata > method), verify the object's visibility or that the caller is an owner/member > (compare the fetched metadata's owner/members/visibility fields to the > email/identity), and if the check fails return http.Error(..., > http.StatusForbidden); only call h.depotSvc.GetDownloadURL(r.Context(), > objectID) and write the URL on success. Ensure you use the existing symbols > Handler.GetObjectDownloadUrl, middleware.EmailFromContext, and > h.depotSvc.GetDownloadURL to locate the code to change. > ``` > > </details> > >  > > </blockquote></details> > > </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@go/internal/handler/handler.go`: - Around line 388-398: The UpdateAvatar function contains an extra closing brace '}' after the error handling block (the one closing the function early) which causes a parse error; remove that unmatched '}' so the function body continues normally (locate the error handling block in UpdateAvatar around the errors.As check and the flog.Error/http.Error calls and delete the extraneous closing brace), then run go build to confirm parsing succeeds. --- Outside diff comments: In `@go/internal/handler/handler.go`: - Around line 793-805: In GetObjectDownloadUrl, do an object-level authorization check before calling h.depotSvc.GetDownloadURL: extract the caller identity using middleware.EmailFromContext(r.Context()), fetch the object's metadata (e.g., via an existing particle/object lookup like a particle service or depot metadata method), verify the object's visibility or that the caller is an owner/member (compare the fetched metadata's owner/members/visibility fields to the email/identity), and if the check fails return http.Error(..., http.StatusForbidden); only call h.depotSvc.GetDownloadURL(r.Context(), objectID) and write the URL on success. Ensure you use the existing symbols Handler.GetObjectDownloadUrl, middleware.EmailFromContext, and h.depotSvc.GetDownloadURL to locate the code to change. ``` </details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ]  Push a commit to this branch (recommended) - [ ]  Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `5eee9ab3-504d-4409-8a31-6fdeca6719b6` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between fa9d54aab5ba3fc28c8c8930154394e653515576 and 6bf2c9983a0f6d4a00f24c531ca201850f7a6537. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `go/internal/handler/handler.go` * `js/desktop/src/lib/avatar-image.ts` </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * js/desktop/src/lib/avatar-image.ts </details> </details>

go/internal/handler/handler.go

						
				@@ -334,6 +335,83 @@ func (h *Handler) UpdateSettings(w http.ResponseWriter, r *http.Request) {

					w.WriteHeader(http.StatusNoContent)

coderabbitai[bot] (Migrated from github.com) commented

2026-06-11 22:21:17 +00:00

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove the unmatched } in UpdateAvatar.

Line 398 closes the function early, so Line 400 starts with a bare assignment and the file no longer parses. That matches the CI failure exactly.

Suggested fix

  if err != nil {
  	var maxErr *http.MaxBytesError
  	if errors.As(err, &maxErr) {
  		http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge)
  		return
  	}
  	flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId)
  	http.Error(w, "internal server error", http.StatusInternalServerError)
  	return
  }
-	}
 
 	err = h.humanSvc.UpdateAvatar(r.Context(), humanId, object.ID)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	if err != nil {
		var maxErr *http.MaxBytesError
		if errors.As(err, &maxErr) {
			http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge)
			return
		}
		flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId)
		http.Error(w, "internal server error", http.StatusInternalServerError)
		return
	}

	err = h.humanSvc.UpdateAvatar(r.Context(), humanId, object.ID)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go/internal/handler/handler.go` around lines 388 - 398, The UpdateAvatar
function contains an extra closing brace '}' after the error handling block (the
one closing the function early) which causes a parse error; remove that
unmatched '}' so the function body continues normally (locate the error handling
block in UpdateAvatar around the errors.As check and the flog.Error/http.Error
calls and delete the extraneous closing brace), then run go build to confirm
parsing succeeds.

Source: Pipeline failures

_⚠️ Potential issue_ | _🔴 Critical_ | _⚡ Quick win_ **Remove the unmatched `}` in `UpdateAvatar`.** Line 398 closes the function early, so Line 400 starts with a bare assignment and the file no longer parses. That matches the CI failure exactly. <details> <summary>Suggested fix</summary> ```diff if err != nil { var maxErr *http.MaxBytesError if errors.As(err, &maxErr) { http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge) return } flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId) http.Error(w, "internal server error", http.StatusInternalServerError) return } - } err = h.humanSvc.UpdateAvatar(r.Context(), humanId, object.ID) ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion if err != nil { var maxErr *http.MaxBytesError if errors.As(err, &maxErr) { http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge) return } flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId) http.Error(w, "internal server error", http.StatusInternalServerError) return } err = h.humanSvc.UpdateAvatar(r.Context(), humanId, object.ID) ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go/internal/handler/handler.go` around lines 388 - 398, The UpdateAvatar function contains an extra closing brace '}' after the error handling block (the one closing the function early) which causes a parse error; remove that unmatched '}' so the function body continues normally (locate the error handling block in UpdateAvatar around the errors.As check and the flog.Error/http.Error calls and delete the extraneous closing brace), then run go build to confirm parsing succeeds. ``` </details>   _Source: Pipeline failures_

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: FlowyLabs/llink#273

				`@@ -334,6 +335,83 @@ func (h Handler) UpdateSettings(w http.ResponseWriter, r http.Request) {`
				`w.WriteHeader(http.StatusNoContent)`