notify/email.go accumulated multi-recipient errors with
multierror.Append(fmt.Errorf(...)) instead of
multierror.Append(result, ...), so the accumulator was overwritten each
iteration and only the last failing recipient's error survived; earlier
failures were silently dropped. The telegram notifier did it correctly.
Replace hashicorp/go-multierror with the stdlib errors.Join everywhere
it was used (notify/email.go, notify/telegram.go, rest/api/rest_private.go,
store/service/service.go, store/image/image.go and store/engine/bolt.go),
which fixes the bug and drops the direct dependency. It stays indirect
because go-pkgz/lcw/v2 still imports it. A regression test in
email_test.go now sends two failing recipients and asserts both errors
are reported.
The delete_me token built by deleteMeCtrl omitted the user's Picture, so the
avatar-removal branch in deleteMeRequestCtrl never ran for real requests and
avatars survived account deletion. Carry Picture in the token so the stored
avatar is removed when the request is processed.
Make the removal best-effort: the avatar stores report an already-missing
avatar as an error with no distinguishable sentinel, and the user's data is
already deleted at that point, so a missing avatar (e.g. a repeated request)
no longer fails the whole deletion with a 400.
Only remove a well-formed avatar id ("<hash>.image") so a malformed picture
can't make a filesystem-backed store target an unexpected path.
Migrate the REST router off go-chi/chi onto go-pkgz/routegroup (backed by the
stdlib http.ServeMux), removing the last use of go-chi from the backend:
- rest.go routes() builds the tree with routegroup (Mount/Group/Route/With) and
net/http method+path patterns instead of chi's Get/Post/Route/Mount helpers
- chi.URLParam(...) -> r.PathValue(...) in the admin, public and private handlers
- rest_public_test.go loadPictureCtrl test uses routegroup + http.ServeMux
- rest_test.go: add TestRest_FileServerStaticAssets (bare /web -> /web/ redirect,
cache headers, 404, directory-listing block) and update the path-traversal test
for ServeMux normalising a literal ".." (encoded traversal is still rejected
by the handler)
- drop go-chi/chi from go.mod, go.sum and vendor; update the CLAUDE.md reference
readAndValidateImage caps the byte size of incoming images but the resize()
helper that follows still called image.Decode unconditionally, allocating
pixel memory proportional to the *declared* image dimensions. A ~100 KB
compressed PNG or GIF that declares 65535x65535 px forces image.Decode to
allocate ~17 GB of raster, OOMing the service on a single comment upload
(or on the proxy's CacheExternal path when caching a malicious upstream).
Hardening:
- maxImagePixels = 16 MP constant. Covers any realistic image (~4096x4096)
while bounding peak allocation.
- resize() now runs image.DecodeConfig first (cheap, no pixel allocation)
to read declared width/height before any full decode.
- Multiplication of width × height uses int64 to defeat 32-bit overflow
(GOARCH=386, 32-bit arm): on those targets, int(cfg.Width)*int(cfg.Height)
could wrap below maxImagePixels and bypass the cap. GIF's 16-bit logical
screen and JPEG's 16-bit SOF dimensions both reach this if int-multiplied.
- Bytes exceeding the cap, or non-image input that fails DecodeConfig,
return nil. prepareImage propagates the rejection as a clear error
instead of storing the malformed/oversized data verbatim.
- The no-resize-needed path returns the validated original bytes verbatim
so animated GIFs round-trip without being flattened to a single frame.
The DecodeConfig precheck applies even when MaxWidth/MaxHeight are 0
(resize disabled) — the dimension cap is unconditional defense-in-depth.
Two adjacent fixes surfaced by the new resize contract:
1. readAndValidateImage previously did `data[:512]` without a bounds check,
panicking on any body shorter than 512 bytes. Now bounded with min().
2. image/webp was listed as an allowed format but no WebP decoder was
registered, so DecodeConfig would refuse legitimate WebP uploads. Added
`_ "golang.org/x/image/webp"` (already in go.mod via x/image/draw) so
the registered decoders match the allowlist.
Tests:
- TestService_resizeRejectsDecompressionBomb builds a 14-byte GIF87a header
declaring 65535x65535 and asserts resize() refuses it both at the unit
level and through SaveWithID end-to-end (no store write).
- TestService_SaveWithIDShortPayload regression-tests the short-body panic.
- TestService_SaveWithIDWebP regression-tests WebP round-trip through
prepareImage with the new DecodeConfig requirement.
- TestService_resize subtests updated to assert non-image bytes are now
refused (previously the helper fell back to returning the raw bytes
verbatim, letting malformed content reach the store).
Address all golangci-lint v2.10.1 (CI's version) findings:
* Add http.MaxBytesReader hard cap to ParseMultipartForm sites in
rest_private.savePictureCtrl (32MB) and api/migrator (256MB) — fixes
G120 by bounding total request body before form parsing.
* Suppress G70x in CLI subcommands cmd/{backup,cleanup,import,remap}.go:
all four issue HTTP requests against operator-supplied RemarkURL/CLI
flags, never user input. Each suppression carries a one-line reason.
* Suppress G122 in image fs_store cleanup walk: staging directory tree
is server-only, no untrusted symlinks land there.
Apply go fix ./... analysers (Go 1.26) across backend and examples:
- interface{} → any (type alias, no behaviour change)
- for i := 0; i < N; i++ → for range N / for i := range N
- slices.Contains / slices.ContainsFunc replacing manual loops
- strings.SplitSeq replacing strings.Split in range (avoids allocation)
- strings.CutPrefix replacing HasPrefix+TrimPrefix
- min() replacing manual if/else
- fmt.Appendf replacing []byte(fmt.Sprintf(...))
- strings.Builder replacing string += concatenation
- wg.Go(func(){}) replacing wg.Add(1)/go/wg.Done() pattern
- removed redundant ii := i loop variable copies (unnecessary since Go 1.22)
omitempty on struct-typed JSON fields: go fix removed omitempty from
struct-typed fields (time.Time, PostInfo, UserDetailEntry) because
encoding/json's omitempty never applied to struct types — it was always
a no-op. Kept as bare tags (no omitzero replacement) to preserve the
existing serialisation behaviour.
Replace strings.Split(RemoteAddr, ":") with net.SplitHostPort for correct
IPv6 address extraction in vote deduplication and comment IP tracking.
Harden image proxy: add SSRF-safe transport blocking private/reserved IPs
at connection time with DNS rebinding protection, sanitize error messages
to prevent information leakage, add response size limit via io.LimitReader.
Fix shadowed error variables in BlockedUsers, SetTitle, and Delete methods.
Exclude gosec taint analysis false positives at linter config level.
Replace WriteHeader() + RenderJSON() pattern with EncodeJSON() which
properly sets Content-Type header before writing status code. The
previous pattern caused Content-Type to default to text/plain instead
of application/json, breaking frontend JSON parsing.
Fixes#1979
Replace go-chi/render with go-pkgz/rest for JSON responses and custom
helpers for HTML/plain text responses.
Key changes:
- Replace render.JSON/render.Status with rest.RenderJSON and explicit
w.WriteHeader() calls
- Replace render.DecodeJSON with json.NewDecoder().Decode()
- Add SendErrorJSON helper that sets Content-Type header before
WriteHeader (required since rest.RenderJSON can't set headers after
WriteHeader is called)
- Add HTMLResponse and PlainTextResponse helpers
Fix export double-execution in migrator.go:
The original code called Export twice - once to io.Discard to check for
errors, then again to actually write. This was wasteful and had a race
condition risk. Now file mode buffers to memory first for atomic
success/failure, while stream mode writes directly with proper error
handling.
Previously, proxied and local images were checked for presence in the
storage before previewing or posting the comment. That logic resulted in
an inability to post with an image when a proxy for images is enabled,
as proxied images are not downloaded to disk before the first time
someone loads them, which could only happen after the user either
previews or posts the message.
After this change, preview and post only checks the local images'
presence and ignore the proxied ones.
Previous behaviour is preserved for query parameters way of requesting
the subscription. The new behaviour with the possibility to confirm
the email right away without a separate /email/confirm call is enabled
only with request params sent in the request body, which was not a thing
before 27fc339e, which was merged just now and is not part
of any tagged version yet.
I haven't found a linter for these, so I had to catch these manually.
I found #757 to fix one of these, and I thought it would be good
to fix everything at once.
Previously it was done through writing bot first,
clicking a button, copying the token, and pasting
it into the web interface.
The new flow is way simpler: click the link
to write bot a message, then click the "Check"
button in the web UI and you got notifications
enabled.
That function returns an error in a never
expected condition, and that error would be
logged message on the caller side:
none of the callers handles it.
That change hides that error from the caller
so that function would have a signature that
better fit what it does and how it behaves.
* replace store.Locator with SiteID where only it is used
* add EmailSubscription flag to User information
* add /user endpoint test for email subscription
* move email unsubscription page outside of API and make it HTML
* make separate HTML template for SendErrorHTML
* fix error template name
* add test for SendErrorHTML, introduce MustExecute function
* fix content check in test of TestSendErrorHTML
* fix logging test to be more generic and not depend on line numbers
* add API methods for setting and deleting email
* fix service.SetStringUserDetail signature to return string
* switch table test with description to t.Run()
* remove debug logging
* clarify error handling, functions names
* add email integration test
* add information about email subscription to readme
* change email API calls method from PUT to POST
* typo fix, remove unneeded capturing of range variable
* email test draft
* fix notify mock, email notification test draft
* add MockDestination to startupT return
* fix tests
* add email retrieval for notifications sending
* fix mock for notify
* rearrange mock notify declaration
* add GET /email API handler, fix typos
* revert startupT signature change
* get rid of startupTWithDest workaround
* add rest examples for rest notification
* improve email messages formatting
* fix email send repeater location
* remove unneeded context from sendMessage
* change signatures of buildMessage functions to have same field name
* add missing authenticate call on TLS connection
* add dev user auth token to email requests
* change email verification template
* email code and tests cleanup
* replace fixed spaces with normal ones
* human-readable variables names for new comment reply notification
* rename Comment to CommentText
* add html for comment email notification
* fix comment notification html style
* fix email test
* fix notify email messages rendering
* fix comments on rest examples for email
* explicitly state email notify email template fields
* clarify email API documentation
* change email test not to check quoted-printable part of message
* Fix link color, add unsubscribe link
* fix rest examples tokens
* add UnsubscribeLink support to Email
* add unsubscribe email handler
* fix new reply notification email style
