feat(sticker): return one client-renderable path for all stickers (#535)

GET /v1/sticker/user mixed two path shapes in the same `path` field:
self-uploaded stickers stored an absolute anonymous-GET download URL,
while collected stickers stored the auth-gated `file/preview/...` key,
which is not renderable in an <img> (AuthMiddleware only reads the token
from a header). Clients had to branch on the path shape.

Normalize server-side so `path` is always directly renderable:

- Add a 1-method `stickerURLResolver` (satisfied by file.IService) and
  resolve every stored path through a single `toResp` mapper used by
  list/add/collect/update. Absolute URLs pass through; object keys (and
  any leading `file/preview/`) resolve via DownloadURL to the permanent
  public/CDN URL. Resolve failure falls back to the stored value (warned).
- collect stores the bare object key instead of the preview path, keeping
  storage backend-agnostic. Idempotency stays keyed on SourcePathHash.
- No new response field, no client change, no migration: legacy rows
  normalize on read.

Security: reject `.`/`..` path segments at collect ingress and again at
the renderablePath URL-generation boundary. Without this, a stored
`sticker/../a.png` would resolve through url.JoinPath (which cleans `..`)
to an object outside the sticker keyspace. Verified the escape only
occurs for a literal `..` middle segment.

Tests: renderablePath unit tests (passthrough, key resolution, prefix
strip, traversal fallback, error/empty/nil fallback); add/collect
integration assertions updated to the normalized contract plus
traversal-rejection cases.

Author: an9xyz

modules/sticker/api.go

@@ -25,11 +25,14 @@
 package sticker
 
 import (
+	"strings"
+
 	"github.com/Mininglamp-OSS/octo-lib/config"
 	"github.com/Mininglamp-OSS/octo-lib/pkg/log"
 	"github.com/Mininglamp-OSS/octo-lib/pkg/util"
 	"github.com/Mininglamp-OSS/octo-lib/pkg/wkhttp"
 	commonmod "github.com/Mininglamp-OSS/octo-server/modules/common"
+	filemod "github.com/Mininglamp-OSS/octo-server/modules/file"
 	"github.com/Mininglamp-OSS/octo-server/pkg/errcode"
 	"github.com/Mininglamp-OSS/octo-server/pkg/httperr"
 	"github.com/Mininglamp-OSS/octo-server/pkg/metrics"
@@ -50,12 +53,25 @@ const (
 	defaultStickerPlaceholder = "[表情]"
 )
 
+// stickerURLResolver resolves a stored sticker object key into a directly
+// renderable, anonymous-GET download URL. It is the single capability the
+// sticker module needs from modules/file; kept as a 1-method interface so tests
+// inject a deterministic fake instead of a real storage backend. Satisfied by
+// file.IService (modules/file).
+type stickerURLResolver interface {
+	DownloadURL(path string, filename string) (string, error)
+}
+
 // Sticker 用户自定义贴纸 API。
 type Sticker struct {
 	ctx *config.Context
 	log.Log
 	db       *stickerDB
 	settings *commonmod.SystemSettings
+	// fileURL resolves a stored path into a client-renderable URL. See
+	// renderablePath — this is what lets GET /v1/sticker/user return one uniform
+	// renderable `path` for both self-uploaded and collected stickers.
+	fileURL stickerURLResolver
 }
 
 // New 创建 Sticker 实例。settings 走进程内共享单例，配额变更（管理端写
@@ -66,6 +82,7 @@ func New(ctx *config.Context) *Sticker {
 		Log:      log.NewTLog("Sticker"),
 		db:       newStickerDB(ctx),
 		settings: commonmod.EnsureSystemSettings(ctx),
+		fileURL:  filemod.NewService(ctx),
 	}
 	// 运营可见性：签发/校验 handle 的「能力」由 OCTO_MASTER_KEY 决定（stickersig.Enabled，
 	// 部署级 env），「是否强制客户端必须带 handle」的「策略」由 system_setting
@@ -105,6 +122,62 @@ func (s *Sticker) Route(r *wkhttp.WKHttp) {
 	}
 }
 
+// renderablePath normalizes a stored sticker path into a value the client can
+// drop straight into an <img src> — no per-client "absolute vs relative" branch.
+//
+// Two shapes reach the DB:
+//   - self-uploaded stickers store the /v1/file/upload response `path`, which is
+//     already an absolute anonymous-GET download/CDN URL → passed through as-is.
+//   - collected stickers (and any relative object key) store the object key. The
+//     authenticated /v1/file/preview/<key> endpoint cannot be used directly in an
+//     <img> because AuthMiddleware only reads the token from a header, so we
+//     resolve the key through the file service's DownloadURL — the same permanent
+//     public/CDN URL /v1/file/preview redirects to.
+//
+// Any leading "file/preview/" is stripped before resolving so legacy collected
+// rows (which stored the preview path) normalize identically to new rows (which
+// store the bare key). On resolve failure the stored value is returned unchanged
+// — no worse than the pre-normalization behavior.
+func (s *Sticker) renderablePath(stored string) string {
+	if stored == "" {
+		return stored
+	}
+	if strings.HasPrefix(stored, "http://") || strings.HasPrefix(stored, "https://") {
+		return stored
+	}
+	if s.fileURL == nil {
+		return stored
+	}
+	key := strings.TrimPrefix(stored, "file/preview/")
+	// Defense-in-depth against keyspace escape: never hand a "."/".." segment to
+	// DownloadURL (url.JoinPath resolves "..", escaping the sticker/ prefix). The
+	// collect ingress already rejects such keys, but this also confines any
+	// legacy row or the self-upload path. A traversal key falls back to the
+	// stored value (a broken but non-escaping render), matching pre-change
+	// behavior where /v1/file/preview rejected "..".
+	if hasUnsafeSegment(key) {
+		return stored
+	}
+	url, err := s.fileURL.DownloadURL(key, "")
+	if err != nil || url == "" {
+		// Fail open, but make silent degradation observable: on a misconfigured
+		// backend every list item would quietly return its raw stored value.
+		// Low-cardinality — the error, never the per-user path, is logged.
+		s.Warn("解析贴纸渲染 URL 失败，回退到存储原值", zap.Error(err))
+		return stored
+	}
+	return url
+}
+
+// toResp maps a model to the wire shape with a client-renderable path. Every
+// endpoint that returns a sticker funnels through here so the `path` contract is
+// uniform across list/add/collect/update.
+func (s *Sticker) toResp(m *StickerModel) stickerResp {
+	r := toStickerResp(m)
+	r.Path = s.renderablePath(r.Path)
+	return r
+}
+
 // list 返回当前用户的自定义贴纸（扁平列表，最新在前）。空集合返回
 // {"list":[]} 而非 404 —— 正是 issue #26 要消灭的噪音。
 func (s *Sticker) list(ctx *wkhttp.Context) {
@@ -119,7 +192,7 @@ func (s *Sticker) list(ctx *wkhttp.Context) {
 
 	list := make([]stickerResp, 0, len(models))
 	for _, m := range models {
-		list = append(list, toStickerResp(m))
+		list = append(list, s.toResp(m))
 	}
 	ctx.Response(listStickerResp{List: list})
 }
@@ -331,13 +404,14 @@ func (s *Sticker) add(ctx *wkhttp.Context) {
 		zap.Bool("handle_required", stickersig.Enabled()),
 		zap.Bool("shortcode_set", shortcode != ""),
 		zap.Int("keyword_count", len(decodeStickerKeywords(keywordsStore))))
-	ctx.Response(toStickerResp(m))
+	ctx.Response(s.toResp(m))
 }
 
 // collect adds a sticker sent by another user into the caller's personal
 // sticker list. Unlike add(), the source path is not required to belong to the
 // caller; it must still point at the reserved sticker object keyspace. The
-// stored path is a stable authenticated preview URL for that source object, and
+// stored path is the bare source object key; the response (and later list)
+// resolve it to a client-renderable URL via toResp → renderablePath.
 // SourcePathHash makes repeat taps idempotent without charging quota again.
 //
 // collect intentionally does not require the upload handle even when
@@ -437,7 +511,7 @@ func (s *Sticker) collect(ctx *wkhttp.Context) {
 			return
 		}
 		observeStickerCollect("idempotent_hit")
-		ctx.Response(toStickerResp(existing))
+		ctx.Response(s.toResp(existing))
 		return
 	}
 
@@ -467,9 +541,14 @@ func (s *Sticker) collect(ctx *wkhttp.Context) {
 	}
 
 	m := &StickerModel{
-		StickerID:      util.GenerUUID(),
-		UID:            loginUID,
-		Path:           source.DisplayPath,
+		StickerID: util.GenerUUID(),
+		UID:       loginUID,
+		// Store the bare object key, not the auth-gated file/preview/ path. The
+		// list/collect responses resolve it to a renderable URL via toResp →
+		// renderablePath; storing the key keeps the DB backend-agnostic (a CDN /
+		// download-host change re-resolves correctly instead of stranding a
+		// baked-in preview path).
+		Path:           source.SourceKey,
 		Placeholder:    placeholder,
 		Format:         source.Format,
 		Sort:           req.Sort,
@@ -499,7 +578,7 @@ func (s *Sticker) collect(ctx *wkhttp.Context) {
 		zap.String("format", source.Format),
 		zap.Bool("shortcode_set", shortcode != ""),
 		zap.Int("keyword_count", len(decodeStickerKeywords(keywordsStore))))
-	ctx.Response(toStickerResp(m))
+	ctx.Response(s.toResp(m))
 }
 
 // update partially updates the current user's sticker metadata. Missing fields
@@ -596,7 +675,7 @@ func (s *Sticker) update(ctx *wkhttp.Context) {
 		return
 	}
 
-	ctx.Response(toStickerResp(m))
+	ctx.Response(s.toResp(m))
 }
 
 // stickerPathClass is the outcome of authorizing a client-supplied sticker

modules/sticker/api_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strconv"
+	"strings"
 	"sync"
 	"testing"
 
@@ -156,7 +157,7 @@ func TestSticker_ListEmpty(t *testing.T) {
 }
 
 func TestSticker_AddAndList(t *testing.T) {
-	route, _, _ := setupSticker(t)
+	route, _, f := setupSticker(t)
 
 	add := doRequest(t, route, "POST", "/v1/sticker/user", map[string]string{
 		"path":        validStickerPath("abc.png"),
@@ -169,14 +170,24 @@ func TestSticker_AddAndList(t *testing.T) {
 	assert.NotEmpty(t, ab["sticker_id"])
 	assert.Equal(t, "user", ab["category"])
 	assert.Equal(t, "png", ab["format"])
+	// The immediate add response is normalized too (all four endpoints funnel
+	// through toResp), not just the later list read.
+	wantPath := f.renderablePath(validStickerPath("abc.png"))
+	assert.Equal(t, wantPath, ab["path"])
+	assert.False(t, strings.HasPrefix(ab["path"].(string), "file/preview/"),
+		"add response path must be a renderable URL, not the auth-gated preview key")
 
 	w := doRequest(t, route, "GET", "/v1/sticker/user", nil)
 	body := parseJSON(t, w)
 	list, ok := body["list"].([]interface{})
 	require.True(t, ok)
 	require.Equal(t, 1, len(list))
 	item := list[0].(map[string]interface{})
-	assert.Equal(t, validStickerPath("abc.png"), item["path"])
+	// path is normalized to a client-renderable URL (same transform the handler
+	// applies), never the raw auth-gated file/preview/ key.
+	assert.Equal(t, wantPath, item["path"])
+	assert.False(t, strings.HasPrefix(item["path"].(string), "file/preview/"),
+		"list path must be a renderable URL, not the auth-gated preview key")
 	assert.Equal(t, "user", item["category"])
 	assert.Equal(t, "[笑]", item["placeholder"])
 }
@@ -196,7 +207,12 @@ func TestSticker_CollectForeignPathIdempotent(t *testing.T) {
 	require.Equal(t, http.StatusOK, first.Code, "body: %s", first.Body.String())
 	firstBody := parseJSON(t, first)
 	firstID := firstBody["sticker_id"].(string)
-	assert.Equal(t, sourcePath, firstBody["path"])
+	// Collect stores the bare object key; the response path is the normalized
+	// renderable URL (never the auth-gated file/preview/ key the client sent).
+	sourceKey := "sticker/source-uid/foreign.png"
+	assert.Equal(t, f.renderablePath(sourceKey), firstBody["path"])
+	assert.False(t, strings.HasPrefix(firstBody["path"].(string), "file/preview/"),
+		"collect path must be a renderable URL, not the auth-gated preview key")
 	assert.Equal(t, "png", firstBody["format"])
 	assert.Equal(t, "[收藏]", firstBody["placeholder"])
 	assert.Equal(t, "fav_one", firstBody["shortcode"])
@@ -216,7 +232,8 @@ func TestSticker_CollectForeignPathIdempotent(t *testing.T) {
 	stickers, err := f.db.listByUID(testutil.UID)
 	require.NoError(t, err)
 	require.Len(t, stickers, 1)
-	assert.Equal(t, sourcePath, stickers[0].Path)
+	assert.Equal(t, sourceKey, stickers[0].Path, "collect stores the bare object key, not the preview path")
+	assert.Equal(t, sourceKey, stickers[0].SourcePath)
 	assert.NotEmpty(t, stickers[0].SourcePathHash)
 }
 
@@ -281,6 +298,12 @@ func TestSticker_CollectRejectsInvalidSourcePath(t *testing.T) {
 		{"unsupported extension", "file/preview/sticker/source-uid/x.tiff"},
 		{"missing extension", "file/preview/sticker/source-uid/x"},
 		{"nested object", "file/preview/sticker/source-uid/nested/x.png"},
+		// Path-traversal keys must be rejected at ingress so they never reach
+		// storage or renderablePath → DownloadURL (which would resolve ".." and
+		// escape the sticker/ keyspace to the bucket root).
+		{"parent traversal segment", "sticker/../x.png"},
+		{"parent traversal via preview prefix", "file/preview/sticker/../x.png"},
+		{"parent traversal via absolute url", "https://cdn.example.com/bucket/sticker/../x.png"},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {

modules/sticker/model.go

@@ -212,13 +212,38 @@ func parseCollectStickerObjectKey(candidate string, re *regexp.Regexp) (collectS
 		return collectStickerSource{}, false
 	}
 	sourceKey := "sticker/" + m[1] + "/" + m[2] + "." + m[3]
+	// Reject path-traversal / relative segments before the key is ever stored.
+	// The regex's `[^/]+` matches "." and "..", so `sticker/../a.png` parses to
+	// SourceKey "sticker/../a.png". That key is later resolved by renderablePath
+	// via DownloadURL → url.JoinPath, which RESOLVES "..", collapsing the
+	// "sticker/" prefix and escaping the sticker keyspace to the bucket root
+	// (e.g. "<base>/bucket/a.png"). Confining at ingress keeps traversal keys out
+	// of the DB and out of every downstream consumer (this one and file/preview).
+	if hasUnsafeSegment(sourceKey) {
+		return collectStickerSource{}, false
+	}
 	return collectStickerSource{
 		SourceKey:   sourceKey,
 		DisplayPath: "file/preview/" + sourceKey,
 		Format:      format,
 	}, true
 }
 
+// hasUnsafeSegment reports whether key contains a "." or ".." path segment. A
+// ".." segment lets url.JoinPath (used by every storage backend's DownloadURL)
+// escape the object key's intended prefix; "." is normalized away and never
+// legitimate in a generated sticker key. Percent-encoded forms ("%2e%2e") are
+// intentionally NOT decoded here: url.JoinPath leaves them literal (verified),
+// so they cannot traverse — only literal segments can.
+func hasUnsafeSegment(key string) bool {
+	for _, seg := range strings.Split(key, "/") {
+		if seg == "." || seg == ".." {
+			return true
+		}
+	}
+	return false
+}
+
 func stickerSourcePathHash(sourceKey string) string {
 	sum := sha256.Sum256([]byte(sourceKey))
 	return hex.EncodeToString(sum[:])