From 16eed14501dc7b4c26c12fb9dc6c96483cd84841 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Thu, 3 Sep 2026 01:04:27 -0700 Subject: [PATCH 1/3] feat: keep every frontend node_modules and build output out of the root Docker build context so a host forms/node_modules from a native make forms or make dev no longer overwrites the forms image's own pnpm install and aborts the build with ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY, set CI=true in the forms, web and admin app build stages so pnpm reinstalls instead of prompting in a build with no TTY, and document the symptom on the troubleshooting page (#292) --- .dockerignore | 13 ++++++++----- admin/Dockerfile | 2 ++ deploy/docker/forms.Dockerfile | 2 ++ docs/content/docs/development/troubleshooting.mdx | 1 + web/Dockerfile | 2 ++ 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.dockerignore b/.dockerignore index 2989c715..9d14a99d 100644 --- a/.dockerignore +++ b/.dockerignore @@ -11,14 +11,17 @@ .vscode .agentd -# Frontend bits — the web service bind-mounts ./web directly, so we -# never want the heavyweight frontend tree to land in the Go build -# context. -web/node_modules -web/.vite +# Frontend trees: the Go images never need them, and a host node_modules +# copied over the forms image's own install makes pnpm abort the build (#292). +**/node_modules +**/.vite web/dist web/build web/coverage +forms/dist +admin/dist +site/dist +site/.astro # Compiled host binaries that would shadow / inflate the context. bin diff --git a/admin/Dockerfile b/admin/Dockerfile index 7a3f3642..318d61ea 100644 --- a/admin/Dockerfile +++ b/admin/Dockerfile @@ -5,6 +5,8 @@ # the heavy pnpm build runs only once. FROM --platform=$BUILDPLATFORM node:22-alpine AS build WORKDIR /app +# No TTY in a build: CI=true makes pnpm reinstall instead of prompting. +ENV CI=true RUN corepack enable && corepack prepare pnpm@11.9.0 --activate COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./ RUN pnpm install --frozen-lockfile diff --git a/deploy/docker/forms.Dockerfile b/deploy/docker/forms.Dockerfile index 2e8d0fc9..34c9b2aa 100644 --- a/deploy/docker/forms.Dockerfile +++ b/deploy/docker/forms.Dockerfile @@ -9,6 +9,8 @@ # $TARGETARCH (no QEMU). FROM --platform=$BUILDPLATFORM node:22-alpine AS appbuilder WORKDIR /app +# No TTY in a build: CI=true makes pnpm reinstall instead of prompting. +ENV CI=true RUN corepack enable && corepack prepare pnpm@11.9.0 --activate COPY forms/package.json forms/pnpm-lock.yaml forms/pnpm-workspace.yaml ./ RUN pnpm install --frozen-lockfile diff --git a/docs/content/docs/development/troubleshooting.mdx b/docs/content/docs/development/troubleshooting.mdx index 3e146b05..46582860 100644 --- a/docs/content/docs/development/troubleshooting.mdx +++ b/docs/content/docs/development/troubleshooting.mdx @@ -64,6 +64,7 @@ Newer builds return the invite-only refusal with its own machine code, `registra |---|---| | `no space left on device`, often from a random service mid-compile | Docker is out of disk. Free space with `docker builder prune -af` and `docker image prune -af`, check the host has about 10 GB free, then re-run `make up` | | `failed to authorize: ... EOF` while pulling a base image | A transient registry blip. Re-run `make up`; completed layers are cached | +| `ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY` while building the `forms` image | A `forms/node_modules` from a native `make forms` or `make dev` was shipped into the Docker build context and overwrote the image's own install, and pnpm will not recreate it without a terminal. Current checkouts keep every `node_modules` out of the context and run pnpm in CI mode; on an older checkout, `rm -rf forms/node_modules` and re-run `make up` | | `failed to xattr /path/._something: operation not permitted` on macOS | The checkout is on a filesystem without native extended attributes (exFAT, NTFS or a network share), so macOS writes `._*` sidecar files that BuildKit cannot read. Run `dot_clean -m .` then `find . -name '._*' -delete` and re-run. Cloning to an APFS volume avoids it | ## The stack is up but something is wrong diff --git a/web/Dockerfile b/web/Dockerfile index b646aadb..741632c8 100644 --- a/web/Dockerfile +++ b/web/Dockerfile @@ -5,6 +5,8 @@ # heavy pnpm build runs only once. FROM --platform=$BUILDPLATFORM node:22-alpine AS build WORKDIR /app +# No TTY in a build: CI=true makes pnpm reinstall instead of prompting. +ENV CI=true RUN corepack enable && corepack prepare pnpm@11.9.0 --activate COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./ RUN pnpm install --frozen-lockfile From f579497c81fc95df836055ee4be3408d85e6b5f9 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Thu, 3 Sep 2026 01:06:36 -0700 Subject: [PATCH 2/3] feat: make profile and workspace avatar changes survive a refresh: user avatar upload and remove go through the user service so the cached /auth/me copy is dropped instead of serving the old avatar for 30 minutes, the org list mapper carries avatar_url into the persisted workspace pointer it previously discarded, the avatar mutation hooks patch the cached user, org pointer and org queries with the server's answer before invalidating, all four handlers delete the replaced or removed object under our own avatars/ prefix with millisecond keys so quick replacements cannot collide on an immutable URL, the org avatar removal is audited, the uploader copy stops promising WebP and GIF, and the API docs note that remove deletes the stored file (fixes #295) --- .../docs/api/reference/account-org.mdx | 4 +- internal/api/handler/avatar.go | 106 +++++++++++++++--- internal/app/user/onboarding.go | 13 +++ internal/app/user/service.go | 1 + .../components/app/avatar/AvatarUploader.tsx | 2 +- .../app/organizations/getOrganizations.ts | 2 + .../lib/api/hooks/app/avatar/useOrgAvatar.ts | 41 +++++-- .../lib/api/hooks/app/avatar/useUserAvatar.ts | 18 +-- .../models/app/organizations/Organization.ts | 2 + 9 files changed, 157 insertions(+), 32 deletions(-) diff --git a/docs/content/docs/api/reference/account-org.mdx b/docs/content/docs/api/reference/account-org.mdx index 6f6003d9..53475676 100644 --- a/docs/content/docs/api/reference/account-org.mdx +++ b/docs/content/docs/api/reference/account-org.mdx @@ -470,7 +470,7 @@ Auth: Session only (not available to API keys). `DELETE /auth/me/avatar` -Clears the profile image. Returns `204 No Content`. +Clears the profile image and deletes the stored file. Returns `204 No Content`. Auth: Session only (not available to API keys). @@ -1456,7 +1456,7 @@ Auth: Session only (not available to API keys). Requires a selected organization `DELETE /organization/avatar` -Clears the workspace image. Owner only. Returns `204 No Content`. +Clears the workspace image and deletes the stored file. Owner only. Returns `204 No Content`. Auth: Session only (not available to API keys). Requires a selected organization (owner only). diff --git a/internal/api/handler/avatar.go b/internal/api/handler/avatar.go index a2231afd..d4b22fb9 100644 --- a/internal/api/handler/avatar.go +++ b/internal/api/handler/avatar.go @@ -8,11 +8,14 @@ // Constants: // // - max size: 2 MiB. Anything larger gets a 400. -// - accepted MIME: image/png, image/jpeg, image/webp, image/gif. -// - object key: avatars/{kind}/{id}-{epoch}.{ext} +// - accepted MIME: image/png, image/jpeg (see allowedAvatarMIME). +// - object key: avatars/{kind}/{id}-{epoch_ms}.{ext} // // The epoch suffix forces cache busting on replacement so the -// browser doesn't keep showing the old avatar at the same URL. +// browser doesn't keep showing the old avatar at the same URL (the +// objects are served immutable, so a reused key would never refresh). +// The previous object is deleted best-effort once the row points at +// the new one, so replacing or removing an avatar doesn't leak blobs. package handler @@ -42,6 +45,9 @@ import ( const ( avatarMaxBytes int64 = 2 * 1024 * 1024 avatarMaxDimension = 1024 // px — reject anything bigger so a phone-camera dump doesn't sneak through + + userAvatarKeyPrefix = "avatars/users/" + orgAvatarKeyPrefix = "avatars/organizations/" ) // Intentionally narrow allowlist: only PNG and JPEG. WebP, GIF and @@ -77,17 +83,23 @@ func (h *Handler) UploadUserAvatar(c *gin.Context) { return } - key := fmt.Sprintf("avatars/users/%s-%d%s", userID.String(), time.Now().Unix(), ext) - url, xerr := putPublicObject(c.Request.Context(), h.Storage, key, bytesRead, mime) + ctx := c.Request.Context() + previous := h.currentUserAvatarURL(ctx, userID) + + key := fmt.Sprintf("%s%s-%d%s", userAvatarKeyPrefix, userID.String(), time.Now().UnixMilli(), ext) + url, xerr := putPublicObject(ctx, h.Storage, key, bytesRead, mime) if xerr != nil { errx.Handle(c, xerr) return } - if err := h.UserRepo.UpdateAvatar(c.Request.Context(), userID, &url); err != nil { - errx.Handle(c, errx.InternalError()) + // Through the service, not the repo: /auth/me is served from a cached + // copy, and only the service drops it. + if xerr := h.UserService.UpdateAvatar(ctx, userID, &url); xerr != nil { + errx.Handle(c, xerr) return } + h.deleteAvatarObject(ctx, previous, userAvatarKeyPrefix, key) h.auditOrg(c, models.AuditActionUpdate, models.AuditEntityUser, &userID, nil, map[string]string{"field": "avatar_url"}) @@ -103,10 +115,14 @@ func (h *Handler) DeleteUserAvatar(c *gin.Context) { return } - if err := h.UserRepo.UpdateAvatar(c.Request.Context(), userID, nil); err != nil { - errx.Handle(c, errx.InternalError()) + ctx := c.Request.Context() + previous := h.currentUserAvatarURL(ctx, userID) + + if xerr := h.UserService.UpdateAvatar(ctx, userID, nil); xerr != nil { + errx.Handle(c, xerr) return } + h.deleteAvatarObject(ctx, previous, userAvatarKeyPrefix, "") h.auditOrg(c, models.AuditActionUpdate, models.AuditEntityUser, &userID, nil, map[string]string{"field": "avatar_url", "value": "cleared"}) @@ -140,17 +156,21 @@ func (h *Handler) UploadOrganizationAvatar(c *gin.Context) { return } - key := fmt.Sprintf("avatars/organizations/%s-%d%s", orgID.String(), time.Now().Unix(), ext) - url, xerr := putPublicObject(c.Request.Context(), h.Storage, key, bytesRead, mime) + ctx := c.Request.Context() + previous := h.currentOrgAvatarURL(ctx, *orgID) + + key := fmt.Sprintf("%s%s-%d%s", orgAvatarKeyPrefix, orgID.String(), time.Now().UnixMilli(), ext) + url, xerr := putPublicObject(ctx, h.Storage, key, bytesRead, mime) if xerr != nil { errx.Handle(c, xerr) return } - if err := h.OrgRepo.UpdateAvatar(c.Request.Context(), *orgID, &url); err != nil { + if err := h.OrgRepo.UpdateAvatar(ctx, *orgID, &url); err != nil { errx.Handle(c, errx.InternalError()) return } + h.deleteAvatarObject(ctx, previous, orgAvatarKeyPrefix, key) h.auditOrg(c, models.AuditActionUpdate, models.AuditEntityOrganization, orgID, nil, map[string]string{"field": "avatar_url"}) @@ -175,14 +195,74 @@ func (h *Handler) DeleteOrganizationAvatar(c *gin.Context) { return } - if err := h.OrgRepo.UpdateAvatar(c.Request.Context(), *orgID, nil); err != nil { + ctx := c.Request.Context() + previous := h.currentOrgAvatarURL(ctx, *orgID) + + if err := h.OrgRepo.UpdateAvatar(ctx, *orgID, nil); err != nil { errx.Handle(c, errx.InternalError()) return } + h.deleteAvatarObject(ctx, previous, orgAvatarKeyPrefix, "") + + // Audited like the upload so teammates' org switcher refreshes live. + h.auditOrg(c, models.AuditActionUpdate, models.AuditEntityOrganization, orgID, nil, map[string]string{"field": "avatar_url", "value": "cleared"}) c.Status(http.StatusNoContent) } +// currentUserAvatarURL returns the avatar URL stored on the user row, or "" +// when there is none or the lookup fails (cleanup is best-effort). +func (h *Handler) currentUserAvatarURL(ctx context.Context, userID uuid.UUID) string { + u, xerr := h.UserService.GetUser(ctx, userID) + if xerr != nil || u == nil || u.AvatarURL == nil { + return "" + } + return *u.AvatarURL +} + +// currentOrgAvatarURL is the organization counterpart of currentUserAvatarURL. +func (h *Handler) currentOrgAvatarURL(ctx context.Context, orgID uuid.UUID) string { + org, xerr := h.OrganizationService.Get(ctx, orgID) + if xerr != nil || org == nil || org.AvatarURL == nil { + return "" + } + return *org.AvatarURL +} + +// deleteAvatarObject removes the object behind a previous avatar URL once the +// row no longer points at it. Only keys under our own prefix are touched, so +// an external URL (an OAuth profile picture, say) is left alone, and keepKey +// guards the freshly written object. Failures are ignored: an orphaned blob +// is harmless, a failed request after a successful update is not. +func (h *Handler) deleteAvatarObject(ctx context.Context, previousURL, prefix, keepKey string) { + if h.Storage == nil || previousURL == "" { + return + } + key := avatarKeyFromURL(previousURL, prefix) + if key == "" || key == keepKey { + return + } + _ = h.Storage.Delete(ctx, key) +} + +// avatarKeyFromURL recovers the object key from a public avatar URL. Both +// storage backends build the URL differently, but the key always starts with +// the known prefix, so that is what is looked for. +func avatarKeyFromURL(url, prefix string) string { + idx := strings.Index(url, prefix) + if idx < 0 { + return "" + } + key := url[idx:] + if q := strings.IndexAny(key, "?#"); q >= 0 { + key = key[:q] + } + if key == prefix || strings.Contains(key, "..") { + return "" + } + return key +} + func (h *Handler) requireOrgOwner(c *gin.Context, orgID, userID uuid.UUID) *errx.Error { m, err := h.OrgRepo.GetMember(c.Request.Context(), orgID, userID) if err != nil || m == nil { diff --git a/internal/app/user/onboarding.go b/internal/app/user/onboarding.go index 70d43bb8..18a1a1ea 100644 --- a/internal/app/user/onboarding.go +++ b/internal/app/user/onboarding.go @@ -41,3 +41,16 @@ func (s *userService) UpdateUndoSendSeconds(ctx context.Context, userID uuid.UUI return nil } + +// UpdateAvatar sets (or clears, with nil) the user's avatar URL. It must go +// through the service so the cached /auth/me copy is dropped; writing the +// repository directly leaves the old avatar served until UserTTL expires. +func (s *userService) UpdateAvatar(ctx context.Context, userID uuid.UUID, avatarURL *string) *errx.Error { + if err := s.userRepository.UpdateAvatar(ctx, userID, avatarURL); err != nil { + return errx.InternalError() + } + + s.cache.Del(ctx, getUserKey(userID)) + + return nil +} diff --git a/internal/app/user/service.go b/internal/app/user/service.go index f88cda4a..e6644dcc 100644 --- a/internal/app/user/service.go +++ b/internal/app/user/service.go @@ -16,6 +16,7 @@ type UserService interface { CompleteOnboarding(ctx context.Context, userID uuid.UUID, firstName, lastName, referralSource, role, teamSize string) *errx.Error UpdateProfile(ctx context.Context, userID uuid.UUID, firstName, lastName string) *errx.Error UpdateUndoSendSeconds(ctx context.Context, userID uuid.UUID, seconds int) *errx.Error + UpdateAvatar(ctx context.Context, userID uuid.UUID, avatarURL *string) *errx.Error } type userService struct { diff --git a/web/src/components/app/avatar/AvatarUploader.tsx b/web/src/components/app/avatar/AvatarUploader.tsx index aa66ae28..0c5c8837 100644 --- a/web/src/components/app/avatar/AvatarUploader.tsx +++ b/web/src/components/app/avatar/AvatarUploader.tsx @@ -139,7 +139,7 @@ export function AvatarUploader({
{uploading ? "Uploading…" - : `PNG, JPG, WebP or GIF. We resize to ${AVATAR_OUTPUT_DIMENSION}px before upload.`} + : `PNG or JPG. We resize to ${AVATAR_OUTPUT_DIMENSION}px before upload.`}