fix(api): reject control characters in /picture URL segments

Address PR #2045 review feedback (Copilot #2045-1). The previous
safePictureSegment allowed CR/LF/TAB through, so a request such as
GET /api/v1/picture/dev%0Auser/abc.png would inject literal newlines
into the access log line ("GET - /api/v1/picture/dev\nuser/abc.png ...")
— a log-forgery primitive against any operator parsing those logs.

Reject any unicode.IsControl rune in either segment (NUL was already
caught via strings.ContainsAny). New TestRest_LoadPictureRejectsControlCharsInSegment
covers LF, CR, TAB, NUL across both segments.

backend/app/rest/api/rest_public.go

@@ -11,6 +11,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/go-chi/chi/v5"
 	cache "github.com/go-pkgz/lcw/v2"
@@ -365,19 +366,27 @@ func (s *public) listCtrl(w http.ResponseWriter, r *http.Request) {
 }
 
 // safePictureSegment reports whether seg is acceptable as a path segment in
-// the picture URL (no traversal markers, no path separators, no NULs). Picture
-// IDs are server-generated hashes plus a known extension, so any value carrying
-// these characters is hostile and must be rejected before reaching the store.
+// the picture URL (no traversal markers, no path separators, no control
+// characters). Picture IDs are server-generated hashes plus a known
+// extension, so any value carrying these characters is hostile and must be
+// rejected before reaching the store. Rejecting controls (CR, LF, TAB, NUL,
+// etc.) also closes a log-injection vector since the rejected segment is
+// echoed into the access log.
 func safePictureSegment(seg string) bool {
 	if seg == "" || seg == "." || seg == ".." {
 		return false
 	}
-	if strings.ContainsAny(seg, "/\\\x00") {
+	if strings.ContainsAny(seg, "/\\") {
 		return false
 	}
 	if strings.Contains(seg, "..") {
 		return false
 	}
+	for _, r := range seg {
+		if unicode.IsControl(r) {
+			return false
+		}
+	}
 	return true
 }
 

backend/app/rest/api/rest_public_test.go

@@ -1066,3 +1066,41 @@ func TestRest_LoadPictureRejectsPathTraversal(t *testing.T) {
 		})
 	}
 }
+
+// TestRest_LoadPictureRejectsControlCharsInSegment makes sure a CRLF / tab / NUL
+// in the URL segment is rejected by safePictureSegment. Without the rejection
+// the [WARN] log line constructed from %q-formatted segments would still be
+// safe (Go's %q escapes control chars), but a future log change to %s would
+// turn this into log forgery — and no legitimate picture id ever needs control
+// characters, so the right place to slam the door is in the validator.
+func TestRest_LoadPictureRejectsControlCharsInSegment(t *testing.T) {
+	ts, _, teardown := startupT(t)
+	defer teardown()
+
+	cases := []struct {
+		name string
+		path string
+	}{
+		{name: "lf in user segment", path: "/api/v1/picture/dev%0Auser/abc.png"},
+		{name: "cr in user segment", path: "/api/v1/picture/dev%0Duser/abc.png"},
+		{name: "tab in user segment", path: "/api/v1/picture/dev%09user/abc.png"},
+		{name: "lf in id segment", path: "/api/v1/picture/dev_user/abc%0A.png"},
+		{name: "nul in id segment", path: "/api/v1/picture/dev_user/abc%00.png"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			req, err := http.NewRequest(http.MethodGet, ts.URL+c.path, http.NoBody)
+			require.NoError(t, err)
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer func() { _ = resp.Body.Close() }()
+
+			assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
+			body, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			s := string(body)
+			assert.Contains(t, s, "invalid picture id", "must reject as invalid input, not fall through to storage")
+			assert.NotContains(t, s, "no such file", "must not reach the filesystem")
+		})
+	}
+}