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/api/reference/account-org.mdx b/docs/content/docs/api/reference/account-org.mdx index 6f6003d9..4aebc31e 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 on a best-effort basis. 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 on a best-effort basis. Owner only. Returns `204 No Content`. Auth: Session only (not available to API keys). Requires a selected organization (owner only). 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/internal/api/handler/avatar.go b/internal/api/handler/avatar.go index a2231afd..50e59d5e 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}-{nonce}.{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 := avatarObjectKey(userAvatarKeyPrefix, userID, 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 := avatarObjectKey(orgAvatarKeyPrefix, *orgID, 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,82 @@ 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) } +// avatarObjectKey builds a key that is unique per upload: the epoch keeps keys +// sortable, the random nonce keeps two uploads in the same millisecond from +// sharing an immutably cached URL. +func avatarObjectKey(prefix string, id uuid.UUID, ext string) string { + nonce := strings.ReplaceAll(uuid.NewString(), "-", "")[:12] + return fmt.Sprintf("%s%s-%d-%s%s", prefix, id.String(), time.Now().UnixMilli(), nonce, ext) +} + +// 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/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 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.`}