From a3e3358315aced1a486f58d1bc2a785365758876 Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Tue, 28 Apr 2026 00:37:34 +0300 Subject: [PATCH 1/6] Add `GetEntryClaims` admin endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `GET /v1/entries/{type}/{name}/claims` so the admin UI (and other admin tooling) can read the claims attached to a published entry name without having to walk the full entries list. The endpoint sits in the existing `manageEntries` role group alongside the matching `PUT`. The service-layer call returns a non-nil claims map even when the entry has no claims set, so the JSON response shape (`{"claims": {...}}`) stays stable for callers. Errors are mapped uniformly: invalid entry type → 400, missing entry → 404, no managed source configured → 503. A new `entry.claims.read` audit event covers the read path. Tests added at three layers — handler-level table tests, db-layer tests against a real Postgres (including empty-claims, wrong-type, and no-managed-source cases), and an authz integration test that exercises the role gate end-to-end (writer + super-admin allowed; admin-without- manageEntries, no-role, and unauthenticated rejected). Swagger regenerated via `task docs`. --- docs/thv-registry-api/docs.go | 100 ++++++++++ docs/thv-registry-api/swagger.json | 100 ++++++++++ docs/thv-registry-api/swagger.yaml | 65 +++++++ internal/api/v1/entries.go | 58 ++++++ internal/api/v1/entries_test.go | 118 ++++++++++++ internal/api/v1/routes.go | 2 + internal/audit/audit.go | 1 + internal/authz/authz_get_entry_claims_test.go | 108 +++++++++++ internal/service/db/get_entry_claims_test.go | 180 ++++++++++++++++++ internal/service/db/impl_entries.go | 55 ++++++ internal/service/mocks/mock_service.go | 20 ++ internal/service/options_entries.go | 22 +++ internal/service/service.go | 6 + 13 files changed, 835 insertions(+) create mode 100644 internal/authz/authz_get_entry_claims_test.go create mode 100644 internal/service/db/get_entry_claims_test.go diff --git a/docs/thv-registry-api/docs.go b/docs/thv-registry-api/docs.go index 69badc08..a72db8db 100644 --- a/docs/thv-registry-api/docs.go +++ b/docs/thv-registry-api/docs.go @@ -431,6 +431,15 @@ const docTemplate = `{ }, "type": "object" }, + "internal_api_v1.entryClaimsResponse": { + "properties": { + "claims": { + "additionalProperties": {}, + "type": "object" + } + }, + "type": "object" + }, "internal_api_v1.meResponse": { "properties": { "roles": { @@ -1988,6 +1997,97 @@ const docTemplate = `{ } }, "/v1/entries/{type}/{name}/claims": { + "get": { + "description": "Get the claims for a published entry name. Claims are stored at the\nentry-name level and are shared by every version of that name.", + "parameters": [ + { + "description": "Entry Type (server or skill)", + "in": "path", + "name": "type", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Entry Name", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/internal_api_v1.entryClaimsResponse" + } + } + }, + "description": "Entry claims" + }, + "400": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Bad request" + }, + "404": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Not found" + }, + "500": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Internal server error" + }, + "503": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "No managed source available" + } + }, + "summary": "Get entry claims", + "tags": [ + "v1" + ] + }, "put": { "description": "Update claims for a published entry name", "parameters": [ diff --git a/docs/thv-registry-api/swagger.json b/docs/thv-registry-api/swagger.json index e2eac076..1cf9d833 100644 --- a/docs/thv-registry-api/swagger.json +++ b/docs/thv-registry-api/swagger.json @@ -424,6 +424,15 @@ }, "type": "object" }, + "internal_api_v1.entryClaimsResponse": { + "properties": { + "claims": { + "additionalProperties": {}, + "type": "object" + } + }, + "type": "object" + }, "internal_api_v1.meResponse": { "properties": { "roles": { @@ -1981,6 +1990,97 @@ } }, "/v1/entries/{type}/{name}/claims": { + "get": { + "description": "Get the claims for a published entry name. Claims are stored at the\nentry-name level and are shared by every version of that name.", + "parameters": [ + { + "description": "Entry Type (server or skill)", + "in": "path", + "name": "type", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Entry Name", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/internal_api_v1.entryClaimsResponse" + } + } + }, + "description": "Entry claims" + }, + "400": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Bad request" + }, + "404": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Not found" + }, + "500": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Internal server error" + }, + "503": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "No managed source available" + } + }, + "summary": "Get entry claims", + "tags": [ + "v1" + ] + }, "put": { "description": "Update claims for a published entry name", "parameters": [ diff --git a/docs/thv-registry-api/swagger.yaml b/docs/thv-registry-api/swagger.yaml index ab449349..b0cd824f 100644 --- a/docs/thv-registry-api/swagger.yaml +++ b/docs/thv-registry-api/swagger.yaml @@ -290,6 +290,12 @@ components: description: Number of skills in registry type: integer type: object + internal_api_v1.entryClaimsResponse: + properties: + claims: + additionalProperties: {} + type: object + type: object internal_api_v1.meResponse: properties: roles: @@ -1353,6 +1359,65 @@ paths: tags: - v1 /v1/entries/{type}/{name}/claims: + get: + description: |- + Get the claims for a published entry name. Claims are stored at the + entry-name level and are shared by every version of that name. + parameters: + - description: Entry Type (server or skill) + in: path + name: type + required: true + schema: + type: string + - description: Entry Name + in: path + name: name + required: true + schema: + type: string + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/internal_api_v1.entryClaimsResponse' + description: Entry claims + "400": + content: + application/json: + schema: + additionalProperties: + type: string + type: object + description: Bad request + "404": + content: + application/json: + schema: + additionalProperties: + type: string + type: object + description: Not found + "500": + content: + application/json: + schema: + additionalProperties: + type: string + type: object + description: Internal server error + "503": + content: + application/json: + schema: + additionalProperties: + type: string + type: object + description: No managed source available + summary: Get entry claims + tags: + - v1 put: description: Update claims for a published entry name parameters: diff --git a/internal/api/v1/entries.go b/internal/api/v1/entries.go index 42559351..027ee44b 100644 --- a/internal/api/v1/entries.go +++ b/internal/api/v1/entries.go @@ -185,6 +185,64 @@ func (routes *Routes) deletePublishedEntry(w http.ResponseWriter, r *http.Reques w.WriteHeader(http.StatusNoContent) } +// entryClaimsResponse is the response body for fetching or returning entry claims. +type entryClaimsResponse struct { + Claims map[string]any `json:"claims"` +} + +// getEntryClaims handles GET /v1/entries/{type}/{name}/claims +// +// @Summary Get entry claims +// @Description Get the claims for a published entry name. Claims are stored at the +// @Description entry-name level and are shared by every version of that name. +// @Tags v1 +// @Produce json +// @Param type path string true "Entry Type (server or skill)" +// @Param name path string true "Entry Name" +// @Success 200 {object} entryClaimsResponse "Entry claims" +// @Failure 400 {object} map[string]string "Bad request" +// @Failure 404 {object} map[string]string "Not found" +// @Failure 500 {object} map[string]string "Internal server error" +// @Failure 503 {object} map[string]string "No managed source available" +// @Router /v1/entries/{type}/{name}/claims [get] +func (routes *Routes) getEntryClaims(w http.ResponseWriter, r *http.Request) { + entryType, err := common.GetAndValidateURLParam(r, "type") + if err != nil { + common.WriteErrorResponse(w, err.Error(), http.StatusBadRequest) + return + } + + name, err := common.GetAndValidateURLParam(r, "name") + if err != nil { + common.WriteErrorResponse(w, err.Error(), http.StatusBadRequest) + return + } + + claims, err := routes.service.GetEntryClaims(r.Context(), + service.WithEntryType(entryType), + service.WithName(name), + ) + if err != nil { + if errors.Is(err, service.ErrInvalidEntryType) { + common.WriteErrorResponse(w, err.Error(), http.StatusBadRequest) + return + } + if errors.Is(err, service.ErrNotFound) { + common.WriteErrorResponse(w, err.Error(), http.StatusNotFound) + return + } + if errors.Is(err, service.ErrNoManagedSource) { + common.WriteErrorResponse(w, "no managed source available", http.StatusServiceUnavailable) + return + } + slog.ErrorContext(r.Context(), "failed to get entry claims", "error", err, "type", entryType) + common.WriteErrorResponse(w, "failed to get entry claims", http.StatusInternalServerError) + return + } + + common.WriteJSONResponse(w, entryClaimsResponse{Claims: claims}, http.StatusOK) +} + // updateEntryClaimsRequest is the request body for updating entry claims. type updateEntryClaimsRequest struct { Claims map[string]any `json:"claims"` diff --git a/internal/api/v1/entries_test.go b/internal/api/v1/entries_test.go index e1d1c4c3..7ced2fbd 100644 --- a/internal/api/v1/entries_test.go +++ b/internal/api/v1/entries_test.go @@ -501,6 +501,124 @@ func TestUpdateEntryClaims(t *testing.T) { } } +func TestGetEntryClaims(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + setupMock func(*mocks.MockRegistryService) + wantStatus int + wantClaims map[string]any + wantError string + }{ + { + name: "success - server type", + path: "/entries/server/test%2Fserver/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(map[string]any{"org": "acme", "team": "platform"}, nil) + }, + wantStatus: http.StatusOK, + wantClaims: map[string]any{"org": "acme", "team": "platform"}, + }, + { + name: "success - skill type", + path: "/entries/skill/test%2Fskill/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(map[string]any{"org": "acme"}, nil) + }, + wantStatus: http.StatusOK, + wantClaims: map[string]any{"org": "acme"}, + }, + { + name: "success - empty claims returns empty object", + path: "/entries/server/test%2Fserver/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(map[string]any{}, nil) + }, + wantStatus: http.StatusOK, + wantClaims: map[string]any{}, + }, + { + name: "unsupported entry type from service", + path: "/entries/server/test%2Fserver/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, fmt.Errorf("invalid option: %w", service.ErrInvalidEntryType)) + }, + wantStatus: http.StatusBadRequest, + wantError: "invalid entry type", + }, + { + name: "entry not found", + path: "/entries/server/test%2Fmissing/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, service.ErrNotFound) + }, + wantStatus: http.StatusNotFound, + }, + { + name: "no managed source", + path: "/entries/server/test%2Fserver/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, service.ErrNoManagedSource) + }, + wantStatus: http.StatusServiceUnavailable, + wantError: "no managed source available", + }, + { + name: "generic service error", + path: "/entries/server/test%2Fserver/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, fmt.Errorf("unexpected error")) + }, + wantStatus: http.StatusInternalServerError, + wantError: "failed to get entry claims", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockSvc := mocks.NewMockRegistryService(ctrl) + tt.setupMock(mockSvc) + + router := Router(mockSvc, nil) + req, err := http.NewRequest(http.MethodGet, tt.path, nil) + require.NoError(t, err) + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, tt.wantStatus, rr.Code) + + if tt.wantError != "" { + var response map[string]string + err = json.Unmarshal(rr.Body.Bytes(), &response) + require.NoError(t, err) + assert.Contains(t, response["error"], tt.wantError) + } + + if tt.wantStatus == http.StatusOK { + var resp entryClaimsResponse + err = json.Unmarshal(rr.Body.Bytes(), &resp) + require.NoError(t, err) + assert.NotNil(t, resp.Claims, "claims must be a non-nil JSON object") + assert.Equal(t, tt.wantClaims, resp.Claims) + } + }) + } +} + // mustMarshal is a test helper that marshals v to JSON or panics. func mustMarshal(v any) []byte { b, err := json.Marshal(v) diff --git a/internal/api/v1/routes.go b/internal/api/v1/routes.go index 7f600383..63efc3ae 100644 --- a/internal/api/v1/routes.go +++ b/internal/api/v1/routes.go @@ -92,6 +92,8 @@ func Router(svc service.RegistryService, authCfg *config.AuthConfig) http.Handle auditmw.Audited(auditmw.EventEntryPublish, auditmw.ResourceTypeEntry, "", routes.publishEntry)) r.Delete("/entries/{type}/{name}/versions/{version}", auditmw.AuditedEntry(auditmw.EventEntryDelete, routes.deletePublishedEntry)) + r.Get("/entries/{type}/{name}/claims", + auditmw.AuditedEntry(auditmw.EventEntryClaimsRead, routes.getEntryClaims)) r.Put("/entries/{type}/{name}/claims", auditmw.AuditedEntry(auditmw.EventEntryClaims, routes.updateEntryClaims)) }) diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 77e6a766..c8d81c9e 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -59,6 +59,7 @@ const ( EventRegistryList = "registry.list" EventRegistryRead = "registry.read" EventRegistryEntriesList = "registry.entries.list" + EventEntryClaimsRead = "entry.claims.read" EventUserInfo = "user.info" ) diff --git a/internal/authz/authz_get_entry_claims_test.go b/internal/authz/authz_get_entry_claims_test.go new file mode 100644 index 00000000..28af64fa --- /dev/null +++ b/internal/authz/authz_get_entry_claims_test.go @@ -0,0 +1,108 @@ +//go:build integration + +package authz_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive-registry-server/internal/config" +) + +// TestAuthzIntegration_GetEntryClaims exercises the GET /v1/entries/{type}/{name}/claims +// endpoint across roles: +// - manageEntries (writer) and superAdmin can read claims (200) +// - admin (manageSources + manageRegistries only) is denied (403) +// - tokens with no matching role are denied (403) +// - unauthenticated requests are rejected (401) +// +//nolint:paralleltest,tparallel // subtests share state — publish first, then read +func TestAuthzIntegration_GetEntryClaims(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + env := setupEnv(t, &config.AuthConfig{ + Mode: config.AuthModeOAuth, + Authz: authzRolesConfig(), + }) + + platformWriter := env.oidc.token(t, map[string]any{"org": "acme", "team": "platform", "role": "writer"}) + platformAdmin := env.oidc.token(t, map[string]any{"org": "acme", "team": "platform", "role": "admin"}) + superAdmin := env.oidc.token(t, map[string]any{"org": "acme", "role": "super-admin"}) + noRole := env.oidc.token(t, map[string]any{"org": "acme"}) + + waitForSync(t, env, superAdmin) + + const claimsPath = "/v1/entries/server/io.test%2Fget-claims-entry/claims" + publishedClaims := map[string]any{"org": "acme", "team": "platform"} + + t.Run("writer publishes entry", func(t *testing.T) { + resp := doRequest(t, "POST", env.baseURL+"/v1/entries", platformWriter, publishReq{ + Claims: publishedClaims, + Server: serverJSON("get-claims-entry"), + }) + assertStatus(t, resp, 201) + }) + + assertClaims := func(t *testing.T, body string, want map[string]any) { + t.Helper() + var resp struct { + Claims map[string]any `json:"claims"` + } + require.NoError(t, json.Unmarshal([]byte(body), &resp)) + assert.Equal(t, want, resp.Claims) + } + + t.Run("manageEntries reads claims", func(t *testing.T) { + resp := doRequest(t, "GET", env.baseURL+claimsPath, platformWriter, nil) + body := assertStatus(t, resp, 200) + assertClaims(t, body, publishedClaims) + }) + + t.Run("superAdmin reads claims", func(t *testing.T) { + resp := doRequest(t, "GET", env.baseURL+claimsPath, superAdmin, nil) + body := assertStatus(t, resp, 200) + assertClaims(t, body, publishedClaims) + }) + + t.Run("manageSources/manageRegistries denied", func(t *testing.T) { + resp := doRequest(t, "GET", env.baseURL+claimsPath, platformAdmin, nil) + assertStatus(t, resp, 403) + }) + + t.Run("token with no role denied", func(t *testing.T) { + resp := doRequest(t, "GET", env.baseURL+claimsPath, noRole, nil) + assertStatus(t, resp, 403) + }) + + t.Run("unauthenticated rejected", func(t *testing.T) { + resp := doRequest(t, "GET", env.baseURL+claimsPath, "", nil) + assertStatus(t, resp, 401) + }) + + t.Run("not found returns 404", func(t *testing.T) { + resp := doRequest(t, "GET", + env.baseURL+"/v1/entries/server/io.test%2Fnon-existent/claims", + platformWriter, nil) + assertStatus(t, resp, 404) + }) + + t.Run("invalid entry type returns 400", func(t *testing.T) { + resp := doRequest(t, "GET", + env.baseURL+"/v1/entries/widget/io.test%2Fget-claims-entry/claims", + platformWriter, nil) + assertStatus(t, resp, 400) + }) + + t.Run("cleanup", func(t *testing.T) { + resp := doRequest(t, "DELETE", + env.baseURL+"/v1/entries/server/io.test%2Fget-claims-entry/versions/1.0.0", + platformWriter, nil) + assertStatus(t, resp, 204) + }) +} diff --git a/internal/service/db/get_entry_claims_test.go b/internal/service/db/get_entry_claims_test.go new file mode 100644 index 00000000..8ab7920a --- /dev/null +++ b/internal/service/db/get_entry_claims_test.go @@ -0,0 +1,180 @@ +package database + +import ( + "context" + "testing" + + upstreamv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive-registry-server/internal/service" +) + +func TestGetEntryClaims_ReturnsClaims(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSource(t, svc, "gec-claims") + + ctx := context.Background() + + _, err := svc.PublishServerVersion(ctx, + service.WithServerData(&upstreamv0.ServerJSON{ + Name: "com.test/gec-claims", + Version: "1.0.0", + }), + service.WithClaims(map[string]any{"org": "acme", "team": "platform"}), + service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"platform", "ops"}}), + ) + require.NoError(t, err) + + claims, err := svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeServer), + service.WithName("com.test/gec-claims"), + ) + require.NoError(t, err) + assert.Equal(t, map[string]any{"org": "acme", "team": "platform"}, claims) +} + +func TestGetEntryClaims_EmptyClaimsReturnsNonNilMap(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSource(t, svc, "gec-empty") + + ctx := context.Background() + + // Publish a server with no claims (anonymous mode). + _, err := svc.PublishServerVersion(ctx, + service.WithServerData(&upstreamv0.ServerJSON{ + Name: "com.test/gec-empty", + Version: "1.0.0", + }), + ) + require.NoError(t, err) + + claims, err := svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeServer), + service.WithName("com.test/gec-empty"), + ) + require.NoError(t, err) + assert.NotNil(t, claims, "claims map must be non-nil for stable JSON shape") + assert.Empty(t, claims) +} + +func TestGetEntryClaims_EntryNotFound(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSource(t, svc, "gec-not-found") + + ctx := context.Background() + + _, err := svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeServer), + service.WithName("com.test/nonexistent"), + ) + assert.ErrorIs(t, err, service.ErrNotFound) +} + +func TestGetEntryClaims_WrongTypeReturnsNotFound(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSource(t, svc, "gec-wrong-type") + + ctx := context.Background() + + // Publish a server, then try to fetch it as a skill. + _, err := svc.PublishServerVersion(ctx, + service.WithServerData(&upstreamv0.ServerJSON{ + Name: "com.test/gec-wrong-type", + Version: "1.0.0", + }), + service.WithClaims(map[string]any{"org": "acme"}), + ) + require.NoError(t, err) + + _, err = svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeSkill), + service.WithName("com.test/gec-wrong-type"), + ) + assert.ErrorIs(t, err, service.ErrNotFound) +} + +func TestGetEntryClaims_NoManagedSource(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + ctx := context.Background() + + _, err := svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeServer), + service.WithName("com.test/no-managed-source"), + ) + assert.ErrorIs(t, err, service.ErrNoManagedSource) +} + +func TestGetEntryClaims_SkillType(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSourceWithRegistry(t, svc, "gec-skill") + + ctx := context.Background() + + skill := &service.Skill{ + Namespace: "com.test", + Name: "gec-skill", + Version: "1.0.0", + Title: "Test Skill", + } + _, err := svc.PublishSkill(ctx, skill, + service.WithClaims(map[string]any{"org": "acme"}), + ) + require.NoError(t, err) + + claims, err := svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeSkill), + service.WithName("gec-skill"), + ) + require.NoError(t, err) + assert.Equal(t, map[string]any{"org": "acme"}, claims) +} + +func TestGetEntryClaims_InvalidEntryType(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + ctx := context.Background() + + // Bypass WithEntryType (which validates) and write directly so we exercise + // the impl-side mapEntryType branch. + _, err := svc.GetEntryClaims(ctx, + func(o any) error { + opts, ok := o.(*service.GetEntryClaimsOptions) + if !ok { + return nil + } + opts.EntryType = "widget" + return nil + }, + service.WithName("anything"), + ) + assert.ErrorIs(t, err, service.ErrInvalidEntryType) +} diff --git a/internal/service/db/impl_entries.go b/internal/service/db/impl_entries.go index d43a100f..966037bd 100644 --- a/internal/service/db/impl_entries.go +++ b/internal/service/db/impl_entries.go @@ -154,3 +154,58 @@ func mapEntryType(entryType string) (sqlc.EntryType, error) { return "", fmt.Errorf("%w: %s", service.ErrInvalidEntryType, entryType) } } + +// GetEntryClaims returns the claims map for a published entry within the managed source. +// The returned map is non-nil even when the entry has no claims set, so callers can +// rely on a stable JSON shape. The endpoint is admin-side and gated by role; it does +// not run a claim-subset check against the caller's JWT. +func (s *dbService) GetEntryClaims(ctx context.Context, opts ...service.Option) (map[string]any, error) { + ctx, span := s.startSpan(ctx, "dbService.GetEntryClaims") + defer span.End() + + options := &service.GetEntryClaimsOptions{} + for _, opt := range opts { + if err := opt(options); err != nil { + otel.RecordError(span, err) + return nil, fmt.Errorf("invalid option: %w", err) + } + } + + span.SetAttributes( + attribute.String("entry.type", options.EntryType), + attribute.String("entry.name", options.Name), + ) + + entryType, err := mapEntryType(options.EntryType) + if err != nil { + otel.RecordError(span, err) + return nil, err + } + + querier := sqlc.New(s.pool) + + source, err := getManagedSource(ctx, querier) + if err != nil { + otel.RecordError(span, err) + return nil, err + } + + row, err := querier.GetRegistryEntryByName(ctx, sqlc.GetRegistryEntryByNameParams{ + SourceID: source.ID, + EntryType: entryType, + Name: options.Name, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, fmt.Errorf("%w: %s", service.ErrNotFound, options.Name) + } + otel.RecordError(span, err) + return nil, fmt.Errorf("failed to look up registry entry: %w", err) + } + + claims := db.DeserializeClaims(row.Claims) + if claims == nil { + claims = map[string]any{} + } + return claims, nil +} diff --git a/internal/service/mocks/mock_service.go b/internal/service/mocks/mock_service.go index 60933fd6..16f8ab04 100644 --- a/internal/service/mocks/mock_service.go +++ b/internal/service/mocks/mock_service.go @@ -152,6 +152,26 @@ func (mr *MockRegistryServiceMockRecorder) DeleteSource(ctx, name any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteSource", reflect.TypeOf((*MockRegistryService)(nil).DeleteSource), ctx, name) } +// GetEntryClaims mocks base method. +func (m *MockRegistryService) GetEntryClaims(ctx context.Context, opts ...service.Option) (map[string]any, error) { + m.ctrl.T.Helper() + varargs := []any{ctx} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetEntryClaims", varargs...) + ret0, _ := ret[0].(map[string]any) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetEntryClaims indicates an expected call of GetEntryClaims. +func (mr *MockRegistryServiceMockRecorder) GetEntryClaims(ctx any, opts ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEntryClaims", reflect.TypeOf((*MockRegistryService)(nil).GetEntryClaims), varargs...) +} + // GetRegistryByName mocks base method. func (m *MockRegistryService) GetRegistryByName(ctx context.Context, name string) (*service.RegistryInfo, error) { m.ctrl.T.Helper() diff --git a/internal/service/options_entries.go b/internal/service/options_entries.go index 330317a4..177c00ef 100644 --- a/internal/service/options_entries.go +++ b/internal/service/options_entries.go @@ -37,3 +37,25 @@ func (o *UpdateEntryClaimsOptions) setJWTClaims(claims map[string]any) error { o.JWTClaims = claims return nil } + +// GetEntryClaimsOptions is the options for the GetEntryClaims operation. +type GetEntryClaimsOptions struct { + EntryType string // EntryTypeServer or EntryTypeSkill + Name string +} + +func (o *GetEntryClaimsOptions) setEntryType(entryType string) error { + switch entryType { + case EntryTypeServer, EntryTypeSkill: + o.EntryType = entryType + return nil + default: + return fmt.Errorf("%w: must be %q or %q", ErrInvalidEntryType, EntryTypeServer, EntryTypeSkill) + } +} + +//nolint:unparam +func (o *GetEntryClaimsOptions) setName(name string) error { + o.Name = name + return nil +} diff --git a/internal/service/service.go b/internal/service/service.go index cccee491..f6363ffb 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -154,6 +154,12 @@ type RegistryService interface { // UpdateEntryClaims updates the claims on a published entry within the managed source. UpdateEntryClaims(ctx context.Context, opts ...Option) error + + // GetEntryClaims returns the claims map for a published entry within the managed source. + // The returned map is non-nil even when the entry has no claims set. + // Returns ErrInvalidEntryType for unknown entry types, ErrNotFound when the entry + // does not exist, and ErrNoManagedSource when no managed source is configured. + GetEntryClaims(ctx context.Context, opts ...Option) (map[string]any, error) } // SourceInfo represents detailed information about a source From 1f7430213372cb9ceb65f478ee9b21aa8823a215 Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Tue, 28 Apr 2026 16:03:19 +0300 Subject: [PATCH 2/6] Gate `GetEntryClaims` by JWT claim subset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run `validateClaimsSubsetBytes` against the entry's stored claims inside `GetEntryClaims` and surface `ErrClaimsInsufficient` as 403 in the handler. Without this, a `manageEntries` caller scoped to one team could read the claim metadata of an entry scoped to a different team even though the matching `PUT` would deny them — that asymmetry contradicts the default-deny visibility rule in `auth.md` §4. `GetEntryClaimsOptions` gains a `JWTClaims` field and `setJWTClaims` setter so the handler can plumb the caller's JWT through `WithJWTClaims`, mirroring how `UpdateEntryClaims` already works. Super-admin still bypasses uniformly via the existing helper. Tests: handler-level case for `ErrClaimsInsufficient` → 403; db-layer cases for cross-team denied / covering caller succeeds; integration sub-test exercising a cross-team writer against a platform-scoped entry. Swagger regenerated for the new 403 response. --- docs/thv-registry-api/docs.go | 13 ++++ docs/thv-registry-api/swagger.json | 13 ++++ docs/thv-registry-api/swagger.yaml | 8 +++ internal/api/v1/entries.go | 14 ++++- internal/api/v1/entries_test.go | 10 ++++ internal/authz/authz_get_entry_claims_test.go | 14 ++++- internal/service/db/get_entry_claims_test.go | 60 +++++++++++++++++++ internal/service/db/impl_entries.go | 14 ++++- internal/service/options_entries.go | 7 +++ 9 files changed, 146 insertions(+), 7 deletions(-) diff --git a/docs/thv-registry-api/docs.go b/docs/thv-registry-api/docs.go index a72db8db..7bf03317 100644 --- a/docs/thv-registry-api/docs.go +++ b/docs/thv-registry-api/docs.go @@ -2043,6 +2043,19 @@ const docTemplate = `{ }, "description": "Bad request" }, + "403": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Forbidden" + }, "404": { "content": { "application/json": { diff --git a/docs/thv-registry-api/swagger.json b/docs/thv-registry-api/swagger.json index 1cf9d833..c3b8cd09 100644 --- a/docs/thv-registry-api/swagger.json +++ b/docs/thv-registry-api/swagger.json @@ -2036,6 +2036,19 @@ }, "description": "Bad request" }, + "403": { + "content": { + "application/json": { + "schema": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } + } + }, + "description": "Forbidden" + }, "404": { "content": { "application/json": { diff --git a/docs/thv-registry-api/swagger.yaml b/docs/thv-registry-api/swagger.yaml index b0cd824f..99dfd09e 100644 --- a/docs/thv-registry-api/swagger.yaml +++ b/docs/thv-registry-api/swagger.yaml @@ -1391,6 +1391,14 @@ paths: type: string type: object description: Bad request + "403": + content: + application/json: + schema: + additionalProperties: + type: string + type: object + description: Forbidden "404": content: application/json: diff --git a/internal/api/v1/entries.go b/internal/api/v1/entries.go index 027ee44b..5a96d196 100644 --- a/internal/api/v1/entries.go +++ b/internal/api/v1/entries.go @@ -201,6 +201,7 @@ type entryClaimsResponse struct { // @Param name path string true "Entry Name" // @Success 200 {object} entryClaimsResponse "Entry claims" // @Failure 400 {object} map[string]string "Bad request" +// @Failure 403 {object} map[string]string "Forbidden" // @Failure 404 {object} map[string]string "Not found" // @Failure 500 {object} map[string]string "Internal server error" // @Failure 503 {object} map[string]string "No managed source available" @@ -218,15 +219,24 @@ func (routes *Routes) getEntryClaims(w http.ResponseWriter, r *http.Request) { return } - claims, err := routes.service.GetEntryClaims(r.Context(), + opts := []service.Option{ service.WithEntryType(entryType), service.WithName(name), - ) + } + if jwtClaims := auth.ClaimsFromContext(r.Context()); jwtClaims != nil { + opts = append(opts, service.WithJWTClaims(map[string]any(jwtClaims))) + } + + claims, err := routes.service.GetEntryClaims(r.Context(), opts...) if err != nil { if errors.Is(err, service.ErrInvalidEntryType) { common.WriteErrorResponse(w, err.Error(), http.StatusBadRequest) return } + if errors.Is(err, service.ErrClaimsInsufficient) { + common.WriteErrorResponse(w, err.Error(), http.StatusForbidden) + return + } if errors.Is(err, service.ErrNotFound) { common.WriteErrorResponse(w, err.Error(), http.StatusNotFound) return diff --git a/internal/api/v1/entries_test.go b/internal/api/v1/entries_test.go index 7ced2fbd..18c4cc34 100644 --- a/internal/api/v1/entries_test.go +++ b/internal/api/v1/entries_test.go @@ -571,6 +571,16 @@ func TestGetEntryClaims(t *testing.T) { wantStatus: http.StatusServiceUnavailable, wantError: "no managed source available", }, + { + name: "claims insufficient", + path: "/entries/server/test%2Fserver/claims", + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, service.ErrClaimsInsufficient) + }, + wantStatus: http.StatusForbidden, + wantError: "insufficient claims", + }, { name: "generic service error", path: "/entries/server/test%2Fserver/claims", diff --git a/internal/authz/authz_get_entry_claims_test.go b/internal/authz/authz_get_entry_claims_test.go index 28af64fa..3be05de7 100644 --- a/internal/authz/authz_get_entry_claims_test.go +++ b/internal/authz/authz_get_entry_claims_test.go @@ -13,9 +13,10 @@ import ( ) // TestAuthzIntegration_GetEntryClaims exercises the GET /v1/entries/{type}/{name}/claims -// endpoint across roles: -// - manageEntries (writer) and superAdmin can read claims (200) -// - admin (manageSources + manageRegistries only) is denied (403) +// endpoint across roles and claim coverage: +// - manageEntries (writer) covering the entry's claims and superAdmin can read (200) +// - manageEntries (writer) NOT covering the entry's claims is denied (403) +// - admin (manageSources + manageRegistries only) is denied by role gate (403) // - tokens with no matching role are denied (403) // - unauthenticated requests are rejected (401) // @@ -32,6 +33,7 @@ func TestAuthzIntegration_GetEntryClaims(t *testing.T) { }) platformWriter := env.oidc.token(t, map[string]any{"org": "acme", "team": "platform", "role": "writer"}) + dataWriter := env.oidc.token(t, map[string]any{"org": "acme", "team": "data", "role": "writer"}) platformAdmin := env.oidc.token(t, map[string]any{"org": "acme", "team": "platform", "role": "admin"}) superAdmin := env.oidc.token(t, map[string]any{"org": "acme", "role": "super-admin"}) noRole := env.oidc.token(t, map[string]any{"org": "acme"}) @@ -70,6 +72,12 @@ func TestAuthzIntegration_GetEntryClaims(t *testing.T) { assertClaims(t, body, publishedClaims) }) + t.Run("cross-team writer denied", func(t *testing.T) { + // dataWriter has manageEntries role but no claim coverage for team=platform. + resp := doRequest(t, "GET", env.baseURL+claimsPath, dataWriter, nil) + assertStatus(t, resp, 403) + }) + t.Run("manageSources/manageRegistries denied", func(t *testing.T) { resp := doRequest(t, "GET", env.baseURL+claimsPath, platformAdmin, nil) assertStatus(t, resp, 403) diff --git a/internal/service/db/get_entry_claims_test.go b/internal/service/db/get_entry_claims_test.go index 8ab7920a..12850e3a 100644 --- a/internal/service/db/get_entry_claims_test.go +++ b/internal/service/db/get_entry_claims_test.go @@ -67,6 +67,66 @@ func TestGetEntryClaims_EmptyClaimsReturnsNonNilMap(t *testing.T) { assert.Empty(t, claims) } +func TestGetEntryClaims_ClaimsInsufficient(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSource(t, svc, "gec-insufficient") + + ctx := context.Background() + + // Publish an entry scoped to team=data. + _, err := svc.PublishServerVersion(ctx, + service.WithServerData(&upstreamv0.ServerJSON{ + Name: "com.test/gec-insufficient", + Version: "1.0.0", + }), + service.WithClaims(map[string]any{"org": "acme", "team": "data"}), + service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"data"}}), + ) + require.NoError(t, err) + + // A caller in team=platform must not be able to read its claims. + _, err = svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeServer), + service.WithName("com.test/gec-insufficient"), + service.WithJWTClaims(map[string]any{"org": "acme", "team": "platform"}), + ) + assert.ErrorIs(t, err, service.ErrClaimsInsufficient) +} + +func TestGetEntryClaims_ClaimsSufficient(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSource(t, svc, "gec-sufficient") + + ctx := context.Background() + + _, err := svc.PublishServerVersion(ctx, + service.WithServerData(&upstreamv0.ServerJSON{ + Name: "com.test/gec-sufficient", + Version: "1.0.0", + }), + service.WithClaims(map[string]any{"org": "acme", "team": "platform"}), + service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"platform"}}), + ) + require.NoError(t, err) + + // A caller whose JWT covers the entry's claims must succeed. + claims, err := svc.GetEntryClaims(ctx, + service.WithEntryType(service.EntryTypeServer), + service.WithName("com.test/gec-sufficient"), + service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"platform", "ops"}}), + ) + require.NoError(t, err) + assert.Equal(t, map[string]any{"org": "acme", "team": "platform"}, claims) +} + func TestGetEntryClaims_EntryNotFound(t *testing.T) { t.Parallel() diff --git a/internal/service/db/impl_entries.go b/internal/service/db/impl_entries.go index 966037bd..5262f039 100644 --- a/internal/service/db/impl_entries.go +++ b/internal/service/db/impl_entries.go @@ -157,8 +157,9 @@ func mapEntryType(entryType string) (sqlc.EntryType, error) { // GetEntryClaims returns the claims map for a published entry within the managed source. // The returned map is non-nil even when the entry has no claims set, so callers can -// rely on a stable JSON shape. The endpoint is admin-side and gated by role; it does -// not run a claim-subset check against the caller's JWT. +// rely on a stable JSON shape. Access is gated by the manageEntries role plus a +// JWT-subset check against the entry's claims, mirroring the matching PUT and the +// default-deny visibility rule (auth.md §4). func (s *dbService) GetEntryClaims(ctx context.Context, opts ...service.Option) (map[string]any, error) { ctx, span := s.startSpan(ctx, "dbService.GetEntryClaims") defer span.End() @@ -203,6 +204,15 @@ func (s *dbService) GetEntryClaims(ctx context.Context, opts ...service.Option) return nil, fmt.Errorf("failed to look up registry entry: %w", err) } + gateClaims := options.JWTClaims + if s.skipAuthz { + gateClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, gateClaims, row.Claims); err != nil { + otel.RecordError(span, err) + return nil, err + } + claims := db.DeserializeClaims(row.Claims) if claims == nil { claims = map[string]any{} diff --git a/internal/service/options_entries.go b/internal/service/options_entries.go index 177c00ef..903d99c1 100644 --- a/internal/service/options_entries.go +++ b/internal/service/options_entries.go @@ -42,6 +42,7 @@ func (o *UpdateEntryClaimsOptions) setJWTClaims(claims map[string]any) error { type GetEntryClaimsOptions struct { EntryType string // EntryTypeServer or EntryTypeSkill Name string + JWTClaims map[string]any } func (o *GetEntryClaimsOptions) setEntryType(entryType string) error { @@ -59,3 +60,9 @@ func (o *GetEntryClaimsOptions) setName(name string) error { o.Name = name return nil } + +//nolint:unparam +func (o *GetEntryClaimsOptions) setJWTClaims(claims map[string]any) error { + o.JWTClaims = claims + return nil +} From 3bb2ceefb73bef05a8cd1f676d9e1c18faaf82bf Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Tue, 28 Apr 2026 21:59:42 +0300 Subject: [PATCH 3/6] Lock JWT-claims call shape and document `GetEntryClaims` posture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `TestGetEntryClaims_PassesJWTClaimsToService` so the JWT-bearing call shape (four mock matchers: ctx + entryType + name + jwtClaims) is covered. The existing table test uses three matchers and would silently break if the handler started passing a third option; the new case locks the contract in place. Also expand the `getEntryClaims` godoc to call out the authorization model explicitly: role gate in middleware, JWT-subset check in the service layer (mirrors the matching PUT), and the anonymous-mode short-circuit. Note that the nil-claims-to-{} normalisation is dead code in authz mode (publish forbids empty claims per auth.md §6 and the gate denies them per §4) — so future readers don't assume it's load-bearing for the authz path. --- internal/api/v1/entries.go | 16 +++++++++++++++- internal/api/v1/entries_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/api/v1/entries.go b/internal/api/v1/entries.go index 5a96d196..08391967 100644 --- a/internal/api/v1/entries.go +++ b/internal/api/v1/entries.go @@ -190,7 +190,21 @@ type entryClaimsResponse struct { Claims map[string]any `json:"claims"` } -// getEntryClaims handles GET /v1/entries/{type}/{name}/claims +// getEntryClaims handles GET /v1/entries/{type}/{name}/claims. +// +// Authorization model: +// - manageEntries role gate runs in middleware (see routes.go). +// - Service-layer JWT subset check denies cross-team reads (403) when authz +// is enabled, mirroring the matching PUT. +// - Anonymous mode (no JWT in context): the handler skips WithJWTClaims, the +// gate short-circuits, and any caller reads any entry's claims. Intended, +// but worth knowing if you ever run partial-anonymous deployments. +// +// The response envelope `{"claims": {...}}` is always a non-nil JSON object — +// the impl normalises a missing/nil claims blob to `map[string]any{}`. Under +// authz, that branch is unreachable in practice (publish forbids empty claims +// per auth.md §6, and the gate denies empty-claim rows per §4); the +// normalisation is for the auth-off / synced-source case. // // @Summary Get entry claims // @Description Get the claims for a published entry name. Claims are stored at the diff --git a/internal/api/v1/entries_test.go b/internal/api/v1/entries_test.go index 18c4cc34..d0efb012 100644 --- a/internal/api/v1/entries_test.go +++ b/internal/api/v1/entries_test.go @@ -629,6 +629,39 @@ func TestGetEntryClaims(t *testing.T) { } } +// TestGetEntryClaims_PassesJWTClaimsToService verifies the handler plumbs JWT +// claims through to the service when they're present in the request context. +// Without this case, the table test above would let a regression slip — its +// mock expectations use exactly three matchers (ctx + 2 options) and would +// fail at runtime if the handler started passing a third option silently. +func TestGetEntryClaims_PassesJWTClaimsToService(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockSvc := mocks.NewMockRegistryService(ctrl) + // Four matchers: ctx + WithEntryType + WithName + WithJWTClaims. + mockSvc.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(map[string]any{"org": "acme", "team": "platform"}, nil) + + router := Router(mockSvc, nil) + req, err := http.NewRequest(http.MethodGet, "/entries/server/test%2Fserver/claims", nil) + require.NoError(t, err) + req = req.WithContext(auth.ContextWithClaims( + req.Context(), jwt.MapClaims{"org": "acme", "team": "platform"}, + )) + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + + var resp entryClaimsResponse + require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp)) + assert.Equal(t, map[string]any{"org": "acme", "team": "platform"}, resp.Claims) +} + // mustMarshal is a test helper that marshals v to JSON or panics. func mustMarshal(v any) []byte { b, err := json.Marshal(v) From 6c9fabd95db9cb9bc078222b9944bd1df283ce6d Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Tue, 28 Apr 2026 22:23:18 +0300 Subject: [PATCH 4/6] Tighten `getEntryClaims` godoc and cover super-admin bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The godoc previously said the nil-claims-to-{} normalisation was "unreachable in practice" under authz — that's wrong for super-admin, who bypasses the subset check uniformly and would hit that branch when reading legacy or synced rows that lack claims. Reword to call out the super-admin path explicitly so the next reader doesn't trust a stale invariant. Add `TestGetEntryClaims_SuperAdminBypassesSubsetCheck` at the db layer: super-admin caller with non-covering JWT claims still reads the entry. Pairs with the existing cross-team-denied case to lock in both halves of the bypass contract. --- internal/api/v1/entries.go | 7 ++-- internal/service/db/get_entry_claims_test.go | 34 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/internal/api/v1/entries.go b/internal/api/v1/entries.go index 08391967..d1c0ab76 100644 --- a/internal/api/v1/entries.go +++ b/internal/api/v1/entries.go @@ -202,9 +202,10 @@ type entryClaimsResponse struct { // // The response envelope `{"claims": {...}}` is always a non-nil JSON object — // the impl normalises a missing/nil claims blob to `map[string]any{}`. Under -// authz, that branch is unreachable in practice (publish forbids empty claims -// per auth.md §6, and the gate denies empty-claim rows per §4); the -// normalisation is for the auth-off / synced-source case. +// authz, that branch is reachable only by super-admin (the publish path +// forbids empty claims per auth.md §6, and the gate denies empty-claim rows +// for everyone else per §4) or by callers in `skipAuthz=true` deployments. +// Either way the response shape stays stable. // // @Summary Get entry claims // @Description Get the claims for a published entry name. Claims are stored at the diff --git a/internal/service/db/get_entry_claims_test.go b/internal/service/db/get_entry_claims_test.go index 12850e3a..521098fc 100644 --- a/internal/service/db/get_entry_claims_test.go +++ b/internal/service/db/get_entry_claims_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/stacklok/toolhive-registry-server/internal/auth" "github.com/stacklok/toolhive-registry-server/internal/service" ) @@ -97,6 +98,39 @@ func TestGetEntryClaims_ClaimsInsufficient(t *testing.T) { assert.ErrorIs(t, err, service.ErrClaimsInsufficient) } +func TestGetEntryClaims_SuperAdminBypassesSubsetCheck(t *testing.T) { + t.Parallel() + + svc, cleanup := setupTestService(t) + defer cleanup() + + createManagedSource(t, svc, "gec-superadmin") + + ctx := context.Background() + + // Publish an entry scoped to team=data with JWT claims that cover it. + _, err := svc.PublishServerVersion(ctx, + service.WithServerData(&upstreamv0.ServerJSON{ + Name: "com.test/gec-superadmin", + Version: "1.0.0", + }), + service.WithClaims(map[string]any{"org": "acme", "team": "data"}), + service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"data"}}), + ) + require.NoError(t, err) + + // A super-admin caller in a completely different org must still read the + // entry's claims — the bypass is uniform across every claim check. + superAdminCtx := auth.ContextWithRoles(ctx, []auth.Role{auth.RoleSuperAdmin}) + claims, err := svc.GetEntryClaims(superAdminCtx, + service.WithEntryType(service.EntryTypeServer), + service.WithName("com.test/gec-superadmin"), + service.WithJWTClaims(map[string]any{"org": "contoso"}), + ) + require.NoError(t, err) + assert.Equal(t, map[string]any{"org": "acme", "team": "data"}, claims) +} + func TestGetEntryClaims_ClaimsSufficient(t *testing.T) { t.Parallel() From a0b65fa8da94d3a3a07dbb0961aed0f6063a8340 Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Wed, 29 Apr 2026 12:03:25 +0300 Subject: [PATCH 5/6] Document why `GetEntryClaims` is managed-source-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reviewer asked why this endpoint hides claims for entries that aren't backed by the managed source — the existing godoc said "within the managed source" without explaining the reasoning. Spell it out so the next reader doesn't need to read the matching PUT and the sync writer to figure it out. The constraint follows from the data model. Synced-source entries derive their claims from upstream: git/api/file entries inherit the source row's claims (they're copied per entry on every sync), and kubernetes entries take their claims from the `toolhive.stacklok.dev/authz-claims` annotation per CRD (per sync.md §3, K8s sources deliberately don't inherit). On top of that, the temp-table upsert overwrites entry claims on every sync via `ON CONFLICT DO UPDATE SET claims = EXCLUDED.claims` — even if the API let you write claims on a synced entry, the next sync would clobber the write. The matching PUT handles this by being managed-source-only too. GET keeps that symmetry. The OpenAPI description gets the same clarification so it shows up in the public docs, and points readers at the source/registry list endpoints for synced-entry claims. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/thv-registry-api/docs.go | 2 +- docs/thv-registry-api/swagger.json | 2 +- docs/thv-registry-api/swagger.yaml | 7 +++++-- internal/api/v1/entries.go | 7 +++++-- internal/service/db/impl_entries.go | 19 ++++++++++++++----- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/docs/thv-registry-api/docs.go b/docs/thv-registry-api/docs.go index 7bf03317..e9966326 100644 --- a/docs/thv-registry-api/docs.go +++ b/docs/thv-registry-api/docs.go @@ -1998,7 +1998,7 @@ const docTemplate = `{ }, "/v1/entries/{type}/{name}/claims": { "get": { - "description": "Get the claims for a published entry name. Claims are stored at the\nentry-name level and are shared by every version of that name.", + "description": "Get the claims for an API-published entry name within the managed source.\nClaims are stored at the entry-name level and are shared by every version of that name.\nSynced-source entries (git/api/file/kubernetes) are out of scope: their claims come from\nupstream (the source manifest or the ` + "`" + `toolhive.stacklok.dev/authz-claims` + "`" + ` annotation) and\nare surfaced through the ` + "`" + `/v1/sources/{name}/entries` + "`" + ` and ` + "`" + `/v1/registries/{name}/entries` + "`" + ` lists.", "parameters": [ { "description": "Entry Type (server or skill)", diff --git a/docs/thv-registry-api/swagger.json b/docs/thv-registry-api/swagger.json index c3b8cd09..ac5c2472 100644 --- a/docs/thv-registry-api/swagger.json +++ b/docs/thv-registry-api/swagger.json @@ -1991,7 +1991,7 @@ }, "/v1/entries/{type}/{name}/claims": { "get": { - "description": "Get the claims for a published entry name. Claims are stored at the\nentry-name level and are shared by every version of that name.", + "description": "Get the claims for an API-published entry name within the managed source.\nClaims are stored at the entry-name level and are shared by every version of that name.\nSynced-source entries (git/api/file/kubernetes) are out of scope: their claims come from\nupstream (the source manifest or the `toolhive.stacklok.dev/authz-claims` annotation) and\nare surfaced through the `/v1/sources/{name}/entries` and `/v1/registries/{name}/entries` lists.", "parameters": [ { "description": "Entry Type (server or skill)", diff --git a/docs/thv-registry-api/swagger.yaml b/docs/thv-registry-api/swagger.yaml index 99dfd09e..35aa7c07 100644 --- a/docs/thv-registry-api/swagger.yaml +++ b/docs/thv-registry-api/swagger.yaml @@ -1361,8 +1361,11 @@ paths: /v1/entries/{type}/{name}/claims: get: description: |- - Get the claims for a published entry name. Claims are stored at the - entry-name level and are shared by every version of that name. + Get the claims for an API-published entry name within the managed source. + Claims are stored at the entry-name level and are shared by every version of that name. + Synced-source entries (git/api/file/kubernetes) are out of scope: their claims come from + upstream (the source manifest or the `toolhive.stacklok.dev/authz-claims` annotation) and + are surfaced through the `/v1/sources/{name}/entries` and `/v1/registries/{name}/entries` lists. parameters: - description: Entry Type (server or skill) in: path diff --git a/internal/api/v1/entries.go b/internal/api/v1/entries.go index d1c0ab76..47c51d80 100644 --- a/internal/api/v1/entries.go +++ b/internal/api/v1/entries.go @@ -208,8 +208,11 @@ type entryClaimsResponse struct { // Either way the response shape stays stable. // // @Summary Get entry claims -// @Description Get the claims for a published entry name. Claims are stored at the -// @Description entry-name level and are shared by every version of that name. +// @Description Get the claims for an API-published entry name within the managed source. +// @Description Claims are stored at the entry-name level and are shared by every version of that name. +// @Description Synced-source entries (git/api/file/kubernetes) are out of scope: their claims come from +// @Description upstream (the source manifest or the `toolhive.stacklok.dev/authz-claims` annotation) and +// @Description are surfaced through the `/v1/sources/{name}/entries` and `/v1/registries/{name}/entries` lists. // @Tags v1 // @Produce json // @Param type path string true "Entry Type (server or skill)" diff --git a/internal/service/db/impl_entries.go b/internal/service/db/impl_entries.go index 5262f039..ae7d9f0f 100644 --- a/internal/service/db/impl_entries.go +++ b/internal/service/db/impl_entries.go @@ -155,11 +155,20 @@ func mapEntryType(entryType string) (sqlc.EntryType, error) { } } -// GetEntryClaims returns the claims map for a published entry within the managed source. -// The returned map is non-nil even when the entry has no claims set, so callers can -// rely on a stable JSON shape. Access is gated by the manageEntries role plus a -// JWT-subset check against the entry's claims, mirroring the matching PUT and the -// default-deny visibility rule (auth.md §4). +// GetEntryClaims returns the claims map for an API-published entry within the +// managed source. Synced-source entries (git/api/file/kubernetes) are out of +// scope here for two reasons: their claim source-of-truth is upstream — the +// source row's claims for git/api/file (inherited per entry) or the +// `toolhive.stacklok.dev/authz-claims` annotation for kubernetes — and every +// sync overwrites entry claims via `ON CONFLICT DO UPDATE SET claims = EXCLUDED.claims`, +// so any API write would be ephemeral. The matching PUT is managed-source-only +// for the same reason. To inspect synced-entry claims, list endpoints under +// `/v1/sources/{name}/entries` and `/v1/registries/{name}/entries` surface them. +// +// The returned map is non-nil even when the entry has no claims set, so callers +// can rely on a stable JSON shape. Access is gated by the manageEntries role +// plus a JWT-subset check against the entry's claims, mirroring the matching +// PUT and the default-deny visibility rule (auth.md §4). func (s *dbService) GetEntryClaims(ctx context.Context, opts ...service.Option) (map[string]any, error) { ctx, span := s.startSpan(ctx, "dbService.GetEntryClaims") defer span.End() From 64b5135ef6b55268ead743da7779dbb8bc9d419d Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Wed, 6 May 2026 19:32:17 +0300 Subject: [PATCH 6/6] Quiet `goconst` on `team=data` test fixture Extract the repeated `"data"` team-name fixture in `get_entry_claims_test.go` to a local `teamDataClaim` constant. The literal appears in five test cases plus the `impl_source.go:973` data URL subtype string and one fixture in `shadowing_get_skill_version_test.go`, which puts it over `goconst`'s default `min-occurrences: 3` threshold. With the constant declared, `goconst`'s `match-constant` heuristic silences the warning without changing what the tests assert. --- internal/service/db/get_entry_claims_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/service/db/get_entry_claims_test.go b/internal/service/db/get_entry_claims_test.go index 521098fc..d55b9dde 100644 --- a/internal/service/db/get_entry_claims_test.go +++ b/internal/service/db/get_entry_claims_test.go @@ -12,6 +12,11 @@ import ( "github.com/stacklok/toolhive-registry-server/internal/service" ) +// teamDataClaim is the team-name fixture used across the +// `GetEntryClaims` tests. Extracted to keep `goconst` happy when the same +// literal would otherwise repeat across files and test cases. +const teamDataClaim = "data" + func TestGetEntryClaims_ReturnsClaims(t *testing.T) { t.Parallel() @@ -84,8 +89,8 @@ func TestGetEntryClaims_ClaimsInsufficient(t *testing.T) { Name: "com.test/gec-insufficient", Version: "1.0.0", }), - service.WithClaims(map[string]any{"org": "acme", "team": "data"}), - service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"data"}}), + service.WithClaims(map[string]any{"org": "acme", "team": teamDataClaim}), + service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{teamDataClaim}}), ) require.NoError(t, err) @@ -114,8 +119,8 @@ func TestGetEntryClaims_SuperAdminBypassesSubsetCheck(t *testing.T) { Name: "com.test/gec-superadmin", Version: "1.0.0", }), - service.WithClaims(map[string]any{"org": "acme", "team": "data"}), - service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"data"}}), + service.WithClaims(map[string]any{"org": "acme", "team": teamDataClaim}), + service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{teamDataClaim}}), ) require.NoError(t, err) @@ -128,7 +133,7 @@ func TestGetEntryClaims_SuperAdminBypassesSubsetCheck(t *testing.T) { service.WithJWTClaims(map[string]any{"org": "contoso"}), ) require.NoError(t, err) - assert.Equal(t, map[string]any{"org": "acme", "team": "data"}, claims) + assert.Equal(t, map[string]any{"org": "acme", "team": teamDataClaim}, claims) } func TestGetEntryClaims_ClaimsSufficient(t *testing.T) {