fix(auth): treat user_id as advisory on /playlists/trending

api/auth_middleware.go

@@ -349,12 +349,28 @@ func (app *ApiServer) authMiddleware(c *fiber.Ctx) error {
 
 	// Not authorized to act on behalf of myId.
 	//
-	// Exception: /users/:userId/feed/for-you accepts user_id as a viewer hint
-	// used only for response decoration (has_current_user_reposted etc.); the
-	// path :userId — not user_id — controls what gets personalized. Treat the
-	// query user_id as advisory rather than authoritative on this route so
-	// the endpoint can be called like the other public read endpoints.
-	allowUnauthenticatedViewerId := strings.HasSuffix(c.Path(), "/feed/for-you")
+	// Exceptions: public discovery reads where user_id is purely a
+	// viewer hint used for response decoration
+	// (has_current_user_reposted, has_current_user_saved, etc.) with no
+	// permission semantics tied to it. Treat the query user_id as
+	// advisory rather than authoritative on these routes so logged-in
+	// SDK clients can pass it without forging signature headers.
+	//
+	// Anything in this list MUST satisfy two conditions:
+	//   1. Method is GET (no writes — writes must remain authoritative).
+	//   2. user_id is used only to populate has_current_user_* / similar
+	//      decoration flags. It MUST NOT control content selection,
+	//      permission, or row visibility — that responsibility lives on
+	//      a path :userId param or an explicit auth middleware.
+	//
+	// Follow-up: the list below is the tactical patch for the most
+	// affected discovery surfaces. The longer-term fix is a per-route
+	// opt-in marker (e.g. an `advisoryUserId` middleware attached at
+	// route registration) so this allowlist doesn't have to grow with
+	// every new public read. Tracked in api#TBD.
+	path := c.Path()
+	allowUnauthenticatedViewerId := c.Method() == fiber.MethodGet &&
+		isAdvisoryUserIdPath(path)
 
 	if myId != 0 && !pkceAuthed && !allowUnauthenticatedViewerId && !app.isAuthorizedRequest(c.Context(), myId, wallet) {
 		return fiber.NewError(
@@ -382,6 +398,68 @@ func (app *ApiServer) authMiddleware(c *fiber.Ctx) error {
 	return c.Next()
 }
 
+// isAdvisoryUserIdPath matches request paths where ?user_id is treated as a
+// viewer hint for response decoration only — no permission semantics. See the
+// big comment in authMiddleware for the contract; this matcher must stay in
+// sync with it.
+//
+// All entries are conceptual route patterns rewritten as a literal path-match
+// or a small predicate. Anything load-bearing on user_id (e.g. /me, /account,
+// any write) is intentionally absent.
+func isAdvisoryUserIdPath(path string) bool {
+	// Normalize: drop the /v1 prefix so the matcher is version-agnostic.
+	stripped := strings.TrimPrefix(path, "/v1")
+
+	switch stripped {
+	case "/playlists",
+		"/playlists/trending",
+		"/playlists/top",
+		"/playlists/new-releases",
+		"/playlists/by_permalink",
+		"/playlists/search",
+		"/tracks",
+		"/tracks/trending",
+		"/tracks/recommended",
+		"/tracks/trending/ids",
+		"/users",
+		"/users/search",
+		"/users/top",
+		"/users/genre/top":
+		return true
+	}
+
+	// /users/:userId/feed/for-you and other /feed/for-you variants.
+	if strings.HasSuffix(stripped, "/feed/for-you") {
+		return true
+	}
+
+	// Dynamic single-resource reads: /playlists/<id>, /tracks/<id>,
+	// /users/<id>, /users/handle/<handle>. These are decorative — the path
+	// param controls the resource; user_id only personalizes flags.
+	segs := strings.Split(strings.TrimPrefix(stripped, "/"), "/")
+	if len(segs) >= 2 {
+		switch segs[0] {
+		case "playlists", "tracks":
+			// /playlists/<id>, /tracks/<id> — but NOT /playlists/<id>/stream
+			// etc. (those endpoints may have their own semantics). Match
+			// only the two-segment case here; sub-resources stay strict.
+			if len(segs) == 2 {
+				return true
+			}
+		case "users":
+			// /users/<id>, /users/handle/<handle> — single user fetch.
+			// Their sub-resource reads (/users/<id>/tracks, /followers,
+			// etc.) are NOT in this tactical exemption; if needed, add
+			// them explicitly.
+			if len(segs) == 2 || (len(segs) == 3 && segs[1] == "handle") {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // Middleware to require auth for the userId in the route params
 // Returns a 403 if the authedWallet is not authorized to act on behalf of the userId
 // Should be placed after authMiddleware

api/auth_middleware_test.go

@@ -435,3 +435,58 @@ func base64Encode(s string) string {
 func base64EncodeBytes(b []byte) string {
 	return base64.StdEncoding.EncodeToString(b)
 }
+
+// TestIsAdvisoryUserIdPath documents the contract for which public read
+// surfaces treat ?user_id as advisory (decoration only) vs authoritative
+// (permission/selection). The list MUST stay narrow — anything that
+// materially uses user_id beyond decoration should NOT be marked advisory.
+func TestIsAdvisoryUserIdPath(t *testing.T) {
+	cases := []struct {
+		path string
+		want bool
+	}{
+		// Documented exempt — viewer hint only.
+		{"/v1/playlists/trending", true},
+		{"/v1/playlists/top", true},
+		{"/v1/playlists/new-releases", true},
+		{"/v1/playlists/by_permalink", true},
+		{"/v1/playlists/search", true},
+		{"/v1/tracks/trending", true},
+		{"/v1/tracks/recommended", true},
+
+		// Single-resource reads — :id is the resource selector; user_id
+		// only personalizes has_current_user_*.
+		{"/v1/playlists/abc123", true},
+		{"/v1/tracks/def456", true},
+		{"/v1/users/oaM5J", true},
+		{"/v1/users/handle/somebody", true},
+
+		// For You — the canonical example.
+		{"/v1/users/oaM5J/feed/for-you", true},
+
+		// NOT exempt — authoritative on myId. /me derives "who is the
+		// caller" from user_id, so impersonation here would be a
+		// security hole. (See big comment in authMiddleware.)
+		{"/v1/me", false},
+		{"/v1/oauth/me", false},
+		{"/v1/users/account/0xabc", false},
+
+		// NOT exempt — sub-resource reads that this tactical patch
+		// doesn't claim coverage for. They may need to be added in a
+		// follow-up; for now they stay strict (which is the existing
+		// pre-patch behavior).
+		{"/v1/playlists/abc/tracks", false},
+		{"/v1/users/oaM5J/tracks", false},
+		{"/v1/users/oaM5J/followers", false},
+
+		// Random paths should never match.
+		{"/v1/notifications", false},
+		{"/v1/", false},
+		{"/", false},
+	}
+
+	for _, tc := range cases {
+		got := isAdvisoryUserIdPath(tc.path)
+		assert.Equalf(t, tc.want, got, "isAdvisoryUserIdPath(%q)", tc.path)
+	}
+}

api/v1_playlists_trending_test.go

@@ -289,6 +289,23 @@ func TestGetTrendingPlaylists_Albums(t *testing.T) {
 		})
 	}
 
+	// Regression: a signed-out SDK caller passes user_id as a viewer hint
+	// (purely for has_current_user_* decoration). The middleware must
+	// treat it as advisory and not 403 the request — otherwise every
+	// logged-in client gets an empty trending list. Same path serves both
+	// type=playlist and type=album, so one exemption covers both.
+	{
+		viewer := trashid.MustEncodeHashID(1)
+		status, body := testGet(t,
+			app,
+			"/v1/playlists/trending?limit=5&type=album&user_id="+viewer,
+			nil)
+		assert.Equal(t, 200, status)
+		jsonAssert(t, body, map[string]any{
+			"data.#": 5,
+		})
+	}
+
 	// Cache safety net: if an album becomes private after the qualified-ids
 	// cache was populated (a stale entry), the response handler should still
 	// drop it before returning. Flip album 1 to private and re-call — the