Skip to content

feat: add avatar upload functionality for users and projects - #376

Merged
pikann merged 3 commits into
masterfrom
feature/add-avatar-upload-functionality
Aug 8, 2026
Merged

feat: add avatar upload functionality for users and projects#376
pikann merged 3 commits into
masterfrom
feature/add-avatar-upload-functionality

Conversation

@pikann

@pikann pikann commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds avatar upload for users, agents, and projects, reusing the existing S3/MinIO attachment pipeline. Uploads are center-cropped and resized server-side into two PNG variants (256px "full", 64px "thumb") and stored as object-storage keys rather than URLs, so every list/detail endpoint can presign a display URL with zero extra joins.

Agents with no custom avatar fall back to a provider-logo default (Anthropic, OpenAI, Mistral, Claude Code, Codex, ...) instead of bare initials; everything else falls back to initials as before. That "real avatar → default avatar → initials" priority is defined once (resolveAgentAvatarUrl/resolveMemberAvatarUrl in lib/provider-logos.ts) and reused across every display site instead of being re-derived inline at each one.

Changes

Backend

  • services/api/migrations/000033, 000034 — add avatar_key/avatar_thumb_key to users, agents, and projects (drops the unused, never-populated agents.avatar_url column).
  • domain/attachment/avatar_service.go + service/attachment/avatar_service.go — new AvatarService: validates content-type/size, presigns the upload, then on completion downloads, crops, and resizes into the two derived variants and deletes the raw upload.
  • platform/storage — new GetObject/PutObject on storage.Client, backing the crop/resize step.
  • New endpoints per owner type: POST/POST/DELETE .../avatar[/initiate-upload|/complete-upload] for /users/me, /projects/{id}/agents/{id}, /admin/agents/{id}, and /projects/{id}.
  • ProjectMember now also carries the backing agent's agent_type/llm_provider/acp_provider, so member-based UI (team page, assignee/reporter pickers) can resolve a default avatar without an extra request.
  • Task/doc activity authors now propagate avatar keys too, so comment/activity avatars resolve correctly.
  • Integration test (test/integration/attachment_test.go) plus unit tests for the avatar service, handlers, and repositories.

Frontend

  • components/shared/avatar-upload.tsx (new) — upload/remove control with a hover-to-change overlay; the image, initials fallback, and hover overlay all share one shape via rounded-[inherit] so they can't drift out of sync; remove button hidden when the current avatar is just the provider-logo default (nothing to remove).
  • components/shared/entity-avatar.tsx (new) — display-only helper for the hand-rolled avatar chips that don't use the Avatar primitive.
  • lib/provider-logos.ts (new) — provider → logo SVG mapping plus resolveAgentAvatarUrl/resolveMemberAvatarUrl, the single definition of the avatar-priority chain, reused everywhere instead of each site re-deriving it.
  • lib/avatar-api.ts (new) — upload/remove orchestration; normalizes the server response so removal always explicitly clears the cached avatar instead of the field being silently omitted.
  • Wired into: profile page, agent detail/cards/picker, project settings, sidebar, team page, task assignee/reporter chips (card/row/subtask/properties panel), mention dropdown, and activity/comment authors.
  • lib/project-api.ts — shared getProjectInitials() so the sidebar and every other project surface render the same initials for the same name.
  • 18 static provider-logo SVGs (public/provider-logos/), sourced from @lobehub/icons-static-svg (MIT), used only as static assets — not a runtime dependency.
  • i18n strings for the new avatar upload/remove/error copy across all 9 locales.

Test plan

  • go build ./..., go vet ./..., go test ./..., golangci-lint run all pass
  • tsc -b --noEmit, biome check, npm run build all pass
  • npm run test — 398 tests passing
  • Manually verified via the dev stack: upload/remove avatar for a user, agent, and project; every surface (sidebar, team page, assignee/reporter pickers, task chips) updates without a reload; agents with no upload show their provider logo instead of initials

- Implemented avatar upload initiation and completion endpoints for user profiles.
- Added avatar key fields to users and projects in the database schema.
- Enhanced user and project handlers to support avatar URL resolution.
- Updated tests to cover avatar upload scenarios, including validation for content type and file size.
- Introduced a fake storage client for testing avatar uploads.
- Modified response presenters to include avatar URLs in user and project responses.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The avatar upload pipeline takes untrusted image bytes, re-downloads and re-decodes them on the server with no size or pixel-dimension bound. An authenticated user can PUT a multi-GB object to their own presigned URL and complete it, or upload a tiny compressed "decompression bomb", forcing the API process to read/allocate unbounded memory in a single request. Because this is self-service (/me/avatar/...) it is reachable by any authenticated user and degrades availability for everyone.

Reviewed changes

  • Avatar upload core — new attachmentdom.AvatarService + attachmentsvc implementation: validate-then-presign initiate, complete that re-reads the raw object, center-crops and resizes to full.png/thumb.png, deletes the raw object, and returns two storage keys persisted directly on the owner row.
  • Storage — new unbounded S3Client.GetObject/PutObject; ResolveAvatarURL presigns per-item at read time.
  • Endpoints & authz — self-service user, project (projects.write), and agent (agents.write, project-scoped + global) initiate/complete/delete routes; all handlers wired with the avatar service in bootstrap.
  • Schema — migrations 000033/000034 add avatar_key/avatar_thumb_key to users/agents/projects; 000033 drops agents.avatar_url.
  • Read propagation — avatar URLs (and agent provider metadata) surfaced on user/project/agent/member responses and task/doc activity actor avatars.
  • Frontend — shared AvatarUpload + EntityAvatarContent components, avatar-api.ts (client mirror of the 5 MiB/content-type whitelist), provider-logo default avatars + SVG assets, i18n, and cache-merge wiring across profile, project settings, agent detail, and team pages.

🚨 Avatar bytes and decoded dimensions are never bounded server-side

The MaxAvatarUploadSize check at initiate uses the client-declared file_size; nothing ties the actual upload to it. A presigned single-part PUT does not enforce a maximum object size, GetObject reads the object fully into memory via io.ReadAll with no cap, and the decode/crop path has no pixel-count limit.

Technical details
# Bound avatar upload bytes and decoded dimensions on the server

## Affected sites
- services/api/internal/service/attachment/avatar_service.go:47-49 (initiate) — `FileSize` is client-declared, advisory only
- services/api/internal/service/attachment/avatar_service.go:100 — `s.store.GetObject(...)` reads the whole object via `io.ReadAll` (platform/storage/s3.go:248) with no cap
- services/api/internal/service/attachment/avatar_service.go:181-201 — `decodeAvatarImage` + `cropToSquare` allocate RGBA sized by decoded dimensions with no pixel cap

## Required outcome
- Re-validate the downloaded object's real size against `MaxAvatarUploadSize` (and/or `f.FileSize`) before decoding; reject oversized objects.
- Cap decoded pixel dimensions before full decode — e.g. read `image.Config`/`webp.DecodeConfig` first and reject above a sane total-pixel budget, so a small compressed image can't expand to a huge bitmap.

## Suggested approach
- In `CompleteAvatarUpload`, enforce `len(raw) > MaxAvatarUploadSize` and bail before decode; gate decoding with a dimension check on config bounds (product of Dx*Dy) before `image.Decode`.
- Optionally stream the S3 body through `io.LimitReader(out.Body, max+1)` inside `GetObject` for defense-in-depth.

## Open questions for the human
- Confirm the maximum acceptable decoded pixel budget for avatars (e.g. 4096x4096, given the resized variants are only 256/64 px).

ℹ️ Rollout: 000033 drops agents.avatar_url with no down migration

Verified via repo-wide search that no live Go code still reads/writes agents.avatar_url (only migrations/000008 and docs reference it), so the drop is safe if the migration and the new app build deploy atomically. If migrations are ever applied ahead of the app rollout, the old deployed binary briefly queries a column that no longer exists.

ℹ️ Nitpicks

  • Agent avatar routes are gated on PermissionAgentsWrite, which also gates agent creation — a broad surface, worth confirming it's intended.
  • CompleteAvatarUpload has no row lock on the UploadStatus == Pending check; two concurrent completes for the same FileID both run the (idempotent) work. Cosmetic, not a correctness issue.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread services/api/internal/service/attachment/avatar_service.go Outdated
Comment thread services/api/internal/service/attachment/avatar_service.go

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The size guard added in this commit rejects oversized uploads only after the object has already been fully slurped into memory, so the raw-read side of the earlier memory-exhaustion concern is mitigated post-allocation, not eliminated. The dimension/decode side (decompression bombs) is properly fixed.

Reviewed changes

  • Bounded avatar decode dimensionsdecodeAvatarImage now runs a header-only DecodeConfig guard (checkAvatarDimensions, MaxAvatarDecodeDimension = 8192) before the full decoder runs, so a small compressed "decompression bomb" can't force an unbounded pixel buffer; new ErrAvatarDimensionsTooLarge maps to 400.
  • Re-validated actual upload size at completeCompleteAvatarUpload re-checks the downloaded bytes against MaxAvatarUploadSize before decoding, closing the declare-small/PUT-large bypass (the presigned PUT enforces no max); proven by a new integration test.
  • Split projectServiceBase in bootstrap so WithAvatarService propagates through the cached project wrapper, with a comment documenting the pointer-mutation semantics.
  • Added unit + integration coverage — owner-mismatch completion rejection, actual-bytes-exceeded rejection, project full flow, agent/project/user replace/remove key cleanup, and the decode-dimension guard.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread services/api/internal/service/attachment/avatar_service.go Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

The prior CAUTION's remaining memory-exhaustion vector is now fully closed: GetObject takes a maxBytes bound and reads through io.LimitReader, and CompleteAvatarUpload passes attachmentdom.MaxAvatarUploadSize+1, so the server never allocates more than ~5 MiB for an oversized object—the truncation still trips the existing ErrAvatarTooLarge check. All Client.GetObject callers were migrated to the new signature.

Reviewed changes

  • Bounded raw avatar readS3Client.GetObject now accepts maxBytes and wraps the body in io.LimitReader (≤0 = unbounded), with the contract documented on the storage.Client interface.
  • Avatar read capped at size limitCompleteAvatarUpload bounds its download at MaxAvatarUploadSize+1, converting the prior post-allocation size check into a genuine allocation bound.
  • Fake storage parityfakeStorageClient.GetObject mirrors the io.LimitReader truncation so unit/integration tests exercise the same capped-read behavior.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pikann
pikann merged commit 53988b0 into master Aug 8, 2026
6 checks passed
@pikann
pikann deleted the feature/add-avatar-upload-functionality branch August 8, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant