diff --git a/.claude/rules/auth.md b/.claude/rules/auth.md index dea10288..3392065f 100644 --- a/.claude/rules/auth.md +++ b/.claude/rules/auth.md @@ -63,20 +63,55 @@ bypass bugs. A user should never see an entry via one endpoint but not another. ## 4. Default-Deny When Authz Is Enabled -If an entry has claims the caller's JWT doesn't cover, the entry is invisible. Missing data -means "not permitted," not "open." +When authz is enabled, **unlabeled is not the same as public**. A resource with no claims +is treated as "no rule matches" → deny, not "open to all." This is the Saltzer–Schroeder +*fail-safe defaults* principle and matches how every Zero Trust framework (NIST SP +800-162, OWASP Access Control, AWS IAM "implicit deny") handles the absence of an +authorization rule. **What must hold:** -- Per-user filtering on a row with non-empty claims fails closed. +- A row with claims the caller's JWT doesn't cover is invisible to that caller. +- A row with **empty/missing claims** is also invisible to claim-bearing callers — only + super-admin (or anonymous mode / authz-off, where the gate is bypassed entirely) can + reach it. "Public" must be expressed with an explicit positive claim that the role + config maps to all authenticated users. - Registry access gate returns **403** (not 404) when the caller fails it. 404 would leak registry existence. -- Empty-claims rows are visible to all authenticated users (deliberate — operators can - create "open" entries). Publishing with empty claims is **forbidden** when authz is - active (see §6). - -**Detect**: new code paths that return an entry when the claims check is ambiguous; 404 -responses on registry access denial; publish handlers that accept empty claims when -`authzEnabled`. +- The list filter and the single-resource gate must agree on this rule. Inconsistency + between list and get/put paths produces "visible via one endpoint, invisible via + another" bugs. +- Publishing with empty claims is **forbidden** when authz is active (see §6) — this is + the write-time enforcement that complements the read-time default-deny. + +**Detect**: a gate that returns nil when the resource's claims are empty and the caller's +claims are non-empty; new "empty = open" carve-outs; 404 responses on registry access +denial; publish handlers that accept empty claims when `authzEnabled`. + +**Implementation note**: `validateClaimsSubset` and `validateClaimsSubsetBytes` in +`internal/service/db/claims_filter.go` are package-level functions and intentionally +have no knowledge of `s.skipAuthz`. They short-circuit when `callerClaims == nil`, so +every callsite is responsible for passing `nil` when authz is off — the standard pattern +is `gateClaims := …; if s.skipAuthz { gateClaims = nil }`. Keeping the gate functions +pure preserves the layering: internal-state inspection lives at the callsite, not inside +the matching algorithm. Turning authz off disables the entire gate, including its +default-deny posture on empty claims. + +**Upgrading from "empty = open" to default-deny**: deployments that ran with authz off, +or that ingest synced sources whose entries have no claims, will have rows in +`registry_entry`, `sources`, and `registries` with `claims IS NULL`. After turning authz +on (or upgrading to a release that includes default-deny), those rows are invisible to +every claim-bearing caller. Two recovery paths: + +- **Per-entry**: a super-admin can read and re-tag each affected row via + `PUT /v1/entries/{type}/{name}/claims` (or the equivalent source/registry endpoints). +- **Operator-managed sources**: tag the managed source itself with a tenant-wide claim + (e.g. `{org: "acme"}`) in config so writers can reference it; otherwise no + non-super-admin caller can publish to it. This is the most common stumbling block when + enabling authz on an existing deployment — forgetting to tag the managed source looks + like a permissions bug at publish time. + +There is intentionally no automatic "backfill claims from JWT" path — that would +re-introduce the "empty = the caller's identity" behavior that this rule rejects. ## 5. Subset Validation on Every Write diff --git a/internal/authz/authz_full_test.go b/internal/authz/authz_full_test.go index da854300..2376742c 100644 --- a/internal/authz/authz_full_test.go +++ b/internal/authz/authz_full_test.go @@ -204,7 +204,10 @@ func TestAuthzIntegration_FullAuthzMode(t *testing.T) { assertStatus(t, resp, 403) }) - t.Run("outsider sees only internal source", func(t *testing.T) { + t.Run("outsider sees no sources", func(t *testing.T) { + // Default-deny on empty claims (auth.md §4) plus the tenant-wide + // {org: acme} claim on every fixture source means a contoso outsider + // covers nothing. There are no truly-public sources in this fixture. resp := doRequest(t, "GET", env.baseURL+"/v1/sources", outsider, nil) body := assertStatus(t, resp, 200) var result struct { @@ -213,7 +216,7 @@ func TestAuthzIntegration_FullAuthzMode(t *testing.T) { } `json:"sources"` } require.NoError(t, json.Unmarshal([]byte(body), &result)) - assert.Len(t, result.Sources, 1) + assert.Empty(t, result.Sources) }) t.Run("super-admin sees all sources", func(t *testing.T) { diff --git a/internal/authz/authz_helpers_test.go b/internal/authz/authz_helpers_test.go index bc437329..17c53e7d 100644 --- a/internal/authz/authz_helpers_test.go +++ b/internal/authz/authz_helpers_test.go @@ -151,7 +151,11 @@ func baseSourceConfigs(t *testing.T) []config.SourceConfig { SyncPolicy: &config.SyncPolicyConfig{Interval: "10s"}, Claims: map[string]any{"org": "acme", "team": "data"}, }, - {Name: "internal", Managed: &config.ManagedConfig{}}, + // Tag the managed "internal" source with a tenant-wide claim so any acme + // caller can reference it. Default-deny on empty claims (auth.md §4) + // would otherwise make this source unreferenceable for everyone but + // super-admin. + {Name: "internal", Claims: map[string]any{"org": "acme"}, Managed: &config.ManagedConfig{}}, } } diff --git a/internal/service/db/admin_claims_test.go b/internal/service/db/admin_claims_test.go index e1453810..aca80a97 100644 --- a/internal/service/db/admin_claims_test.go +++ b/internal/service/db/admin_claims_test.go @@ -583,7 +583,9 @@ func TestListSources_ReturnsAllMatchingWithMixedClaims(t *testing.T) { _, err = svc.CreateSource(createCtx, "lsm-open", fileDataSourceReq(nil)) require.NoError(t, err) - // List with acme JWT — should see both acme sources and the open (no-claims) source + // List with acme JWT — should see both acme sources only. + // The open (no-claims) source is invisible by default-deny (auth.md §4): + // unlabeled resources are not visible to claim-bearing callers. listCtx := auth.ContextWithClaims(t.Context(), jwt.MapClaims{"org": "acme"}) sources, err := svc.ListSources(listCtx) require.NoError(t, err) @@ -592,7 +594,7 @@ func TestListSources_ReturnsAllMatchingWithMixedClaims(t *testing.T) { assert.Contains(t, names, "lsm-acme-1") assert.Contains(t, names, "lsm-acme-2") assert.NotContains(t, names, "lsm-contoso") - assert.Contains(t, names, "lsm-open") + assert.NotContains(t, names, "lsm-open") } func TestListRegistries_ReturnsAllMatchingWithMixedClaims(t *testing.T) { @@ -632,7 +634,8 @@ func TestListRegistries_ReturnsAllMatchingWithMixedClaims(t *testing.T) { }) require.NoError(t, err) - // List with acme JWT — should see both acme registries and the open one + // List with acme JWT — should see both acme registries only. + // The open (no-claims) registry is invisible by default-deny (auth.md §4). listCtx := auth.ContextWithClaims(t.Context(), jwt.MapClaims{"org": "acme"}) registries, err := svc.ListRegistries(listCtx) require.NoError(t, err) @@ -641,7 +644,7 @@ func TestListRegistries_ReturnsAllMatchingWithMixedClaims(t *testing.T) { assert.Contains(t, names, "lrm-acme-reg-1") assert.Contains(t, names, "lrm-acme-reg-2") assert.NotContains(t, names, "lrm-contoso-reg") - assert.Contains(t, names, "lrm-open-reg") + assert.NotContains(t, names, "lrm-open-reg") } func TestListSources_AnonymousReturnsAll(t *testing.T) { diff --git a/internal/service/db/claims_filter.go b/internal/service/db/claims_filter.go index 425ed631..b70d0050 100644 --- a/internal/service/db/claims_filter.go +++ b/internal/service/db/claims_filter.go @@ -43,19 +43,23 @@ func checkClaimConsistency(incomingJSON, existingJSON []byte) error { // validateClaimsSubset checks that callerClaims covers resourceClaims. // Returns nil if: -// - callerClaims is nil (anonymous mode — no auth enforcement) -// - resourceClaims is nil/empty (open resource — no restriction) +// - callerClaims is nil (caller has opted out of the gate — callers pass +// nil when skipAuthz is enabled or in anonymous mode) // - the caller is a super-admin (bypasses all claim checks) // - callerClaims is a superset of resourceClaims // -// Returns ErrClaimsInsufficient otherwise. +// Returns ErrClaimsInsufficient otherwise, including when resourceClaims +// is nil/empty (default-deny on unlabeled resources — see auth.md §4). func validateClaimsSubset(ctx context.Context, callerClaims, resourceClaims map[string]any) error { - if callerClaims == nil || len(resourceClaims) == 0 { + if callerClaims == nil { return nil } if auth.IsSuperAdmin(ctx) { return nil } + if len(resourceClaims) == 0 { + return fmt.Errorf("%w: resource has no claims", service.ErrClaimsInsufficient) + } if !claimsContain(callerClaims, resourceClaims) { return fmt.Errorf("%w: caller claims do not cover resource claims", service.ErrClaimsInsufficient) } @@ -63,9 +67,10 @@ func validateClaimsSubset(ctx context.Context, callerClaims, resourceClaims map[ } // validateClaimsSubsetBytes is like validateClaimsSubset but accepts raw JSON -// for resourceClaims. Nil or empty JSON is treated as an open resource. +// for resourceClaims. An empty resource JSON is default-deny when callerClaims +// is non-nil (unlabeled resources are invisible to claim-bearing callers). func validateClaimsSubsetBytes(ctx context.Context, callerClaims map[string]any, resourceClaimsJSON []byte) error { - if callerClaims == nil || len(resourceClaimsJSON) == 0 { + if callerClaims == nil { return nil } resourceClaims := db.DeserializeClaims(resourceClaimsJSON) @@ -90,9 +95,10 @@ func claimsFromCtx(ctx context.Context) map[string]any { // caller's claims are non-empty, the record has stored claims, and they match. // extract retrieves the raw claims JSON from a record; returning ok=false // causes the filter to reject the record with a type error. -// Returns nil when callerClaims is nil or empty so the caller can skip -// filtering entirely. Also returns nil for super-admin users (they see all -// entries regardless of claims). +// Returns nil (no filter applied — every record visible) when: +// - callerClaims is nil/empty (callers pass nil when skipAuthz is enabled +// or in anonymous mode) +// - the caller is a super-admin (uniform bypass) func newClaimsFilterWith( ctx context.Context, callerClaims map[string]any, @@ -155,8 +161,17 @@ func checkClaims(callerJSON, recordJSON []byte) bool { // An empty-array value (e.g. "teams": []) is vacuously satisfied by any caller // value for that key — this is intentional since ValidateClaimValues accepts // empty arrays, and presence of the key is the meaningful signal. +// +// Fails closed (returns false) when a record value has an unsupported type +// (nil, number, nested object, etc.). ValidateClaimValues blocks such values +// at the API edge, but rows persisted via direct DB writes or future sync +// paths could carry them — the gate must not silently treat them as +// vacuously-satisfied requirements. func claimsContain(caller, record map[string]any) bool { for k, rv := range record { + if !isValidClaimValue(rv) { + return false + } cv, ok := caller[k] if !ok { return false @@ -172,6 +187,27 @@ func claimsContain(caller, record map[string]any) bool { return true } +// isValidClaimValue reports whether v is a supported claim value (string, +// []string, or []any of strings). Used by claimsContain to fail closed on +// rows whose values bypassed ValidateClaimValues. +func isValidClaimValue(v any) bool { + switch val := v.(type) { + case string: + return true + case []string: + return true + case []any: + for _, elem := range val { + if _, ok := elem.(string); !ok { + return false + } + } + return true + default: + return false + } +} + // claimsEqual returns true when a and b have exactly the same keys and values. // Used to enforce strict claim consistency on subsequent publishes of the same entry name. func claimsEqual(a, b map[string]any) bool { diff --git a/internal/service/db/claims_filter_test.go b/internal/service/db/claims_filter_test.go index b5c0c026..9435743d 100644 --- a/internal/service/db/claims_filter_test.go +++ b/internal/service/db/claims_filter_test.go @@ -191,6 +191,46 @@ func TestCheckClaims(t *testing.T) { recordJSON: []byte(`{"sub":"u1"}`), want: false, }, + // Default-deny on unsupported record value types (auth.md §4). + // ValidateClaimValues blocks these at the API edge, but synced + // sources or direct DB writes could persist them — claimsContain + // must fail closed rather than treat them as vacuously satisfied. + { + name: "record null value drops record", + callerJSON: []byte(`{"team":"platform"}`), + recordJSON: []byte(`{"team":null}`), + want: false, + }, + { + name: "record number value drops record", + callerJSON: []byte(`{"team":"platform"}`), + recordJSON: []byte(`{"team":42}`), + want: false, + }, + { + name: "record nested object value drops record", + callerJSON: []byte(`{"team":"platform"}`), + recordJSON: []byte(`{"team":{"nested":"x"}}`), + want: false, + }, + { + name: "record mixed-type array drops record", + callerJSON: []byte(`{"team":["platform","ops"]}`), + recordJSON: []byte(`{"team":["platform",42]}`), + want: false, + }, + { + name: "record boolean value drops record", + callerJSON: []byte(`{"team":"platform"}`), + recordJSON: []byte(`{"team":true}`), + want: false, + }, + { + name: "record empty array vacuously satisfied (intentional)", + callerJSON: []byte(`{"team":"platform"}`), + recordJSON: []byte(`{"team":[]}`), + want: true, + }, } for _, tt := range tests { @@ -203,6 +243,38 @@ func TestCheckClaims(t *testing.T) { } } +func TestIsValidClaimValue(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + v any + want bool + }{ + {name: "string", v: "platform", want: true}, + {name: "empty string", v: "", want: true}, + {name: "[]string", v: []string{"a", "b"}, want: true}, + {name: "empty []string", v: []string{}, want: true}, + {name: "[]any of strings", v: []any{"a", "b"}, want: true}, + {name: "empty []any", v: []any{}, want: true}, + {name: "nil", v: nil, want: false}, + {name: "int", v: 42, want: false}, + {name: "float", v: 3.14, want: false}, + {name: "bool", v: true, want: false}, + {name: "nested map", v: map[string]any{"k": "v"}, want: false}, + {name: "[]any with int", v: []any{"a", 42}, want: false}, + {name: "[]any with nil", v: []any{"a", nil}, want: false}, + {name: "[]any with nested map", v: []any{"a", map[string]any{}}, want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, isValidClaimValue(tt.v)) + }) + } +} + func TestMarshalClaims(t *testing.T) { t.Parallel() diff --git a/internal/service/db/claims_filter_validate_test.go b/internal/service/db/claims_filter_validate_test.go index d438f002..39228c98 100644 --- a/internal/service/db/claims_filter_validate_test.go +++ b/internal/service/db/claims_filter_validate_test.go @@ -24,21 +24,28 @@ func TestValidateClaimsSubset(t *testing.T) { wantErr error }{ { - name: "nil caller claims (anonymous) returns nil", + name: "nil caller claims (anonymous or skipAuthz) returns nil", callerClaims: nil, resourceClaims: map[string]any{"sub": "user1"}, wantErr: nil, }, { - name: "nil resource claims returns nil", + name: "nil resource claims returns ErrClaimsInsufficient (default-deny)", callerClaims: map[string]any{"sub": "user1"}, resourceClaims: nil, - wantErr: nil, + wantErr: service.ErrClaimsInsufficient, }, { - name: "empty resource claims returns nil", + name: "empty resource claims returns ErrClaimsInsufficient (default-deny)", callerClaims: map[string]any{"sub": "user1"}, resourceClaims: map[string]any{}, + wantErr: service.ErrClaimsInsufficient, + }, + { + name: "super-admin bypasses default-deny on empty resource claims", + callerClaims: map[string]any{"sub": "admin"}, + resourceClaims: map[string]any{}, + superAdmin: true, wantErr: nil, }, { @@ -117,22 +124,22 @@ func TestValidateClaimsSubsetBytes(t *testing.T) { wantErr error }{ { - name: "nil caller claims returns nil", + name: "nil caller claims (anonymous or skipAuthz) returns nil", callerClaims: nil, resourceJSON: []byte(`{"sub":"user1"}`), wantErr: nil, }, { - name: "nil resource JSON returns nil", + name: "nil resource JSON returns ErrClaimsInsufficient (default-deny)", callerClaims: map[string]any{"sub": "user1"}, resourceJSON: nil, - wantErr: nil, + wantErr: service.ErrClaimsInsufficient, }, { - name: "empty resource JSON returns nil", + name: "empty resource JSON returns ErrClaimsInsufficient (default-deny)", callerClaims: map[string]any{"sub": "user1"}, resourceJSON: []byte{}, - wantErr: nil, + wantErr: service.ErrClaimsInsufficient, }, { name: "matching JSON returns nil", diff --git a/internal/service/db/impl_common.go b/internal/service/db/impl_common.go index 36eca27d..33e77e53 100644 --- a/internal/service/db/impl_common.go +++ b/internal/service/db/impl_common.go @@ -26,6 +26,8 @@ func (s *dbService) CheckReadiness(ctx context.Context) error { // after verifying the caller's claims satisfy the registry's access gate. // Returns ErrClaimsInsufficient if the caller's JWT claims do not cover the // registry's claims. Returns ErrRegistryNotFound if the registry does not exist. +// Callers must pass nil for callerClaims when the gate should be bypassed +// (skipAuthz mode or anonymous mode). func lookupRegistryIDWithGate( ctx context.Context, pool sqlc.DBTX, registryName string, callerClaims map[string]any, ) (uuid.UUID, error) { diff --git a/internal/service/db/impl_entries.go b/internal/service/db/impl_entries.go index 456dedd0..d43a100f 100644 --- a/internal/service/db/impl_entries.go +++ b/internal/service/db/impl_entries.go @@ -49,7 +49,11 @@ func (s *dbService) UpdateEntryClaims(ctx context.Context, opts ...service.Optio } } - if err := validateClaimsSubset(ctx, options.JWTClaims, options.Claims); err != nil { + gateClaims := options.JWTClaims + if s.skipAuthz { + gateClaims = nil + } + if err := validateClaimsSubset(ctx, gateClaims, options.Claims); err != nil { otel.RecordError(span, err) return err } @@ -107,7 +111,11 @@ func (s *dbService) executeUpdateClaimsTransaction( return fmt.Errorf("failed to look up registry entry: %w", err) } - if err := validateClaimsSubsetBytes(ctx, options.JWTClaims, existing.Claims); err != nil { + gateClaims := options.JWTClaims + if s.skipAuthz { + gateClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, gateClaims, existing.Claims); err != nil { return err } diff --git a/internal/service/db/impl_mcp.go b/internal/service/db/impl_mcp.go index 7966fa1d..d371d3c4 100644 --- a/internal/service/db/impl_mcp.go +++ b/internal/service/db/impl_mcp.go @@ -626,7 +626,11 @@ func (s *dbService) PublishServerVersion( } // Validate published claims are a subset of the publisher's JWT claims - if err := validateClaimsSubset(ctx, options.JWTClaims, options.Claims); err != nil { + gateClaims := options.JWTClaims + if s.skipAuthz { + gateClaims = nil + } + if err := validateClaimsSubset(ctx, gateClaims, options.Claims); err != nil { otel.RecordError(span, err) return nil, err } @@ -876,7 +880,11 @@ func (s *dbService) executeDeleteTransaction( } return fmt.Errorf("failed to look up registry entry: %w", err) } - if err := validateClaimsSubsetBytes(ctx, options.JWTClaims, existing.Claims); err != nil { + gateClaims := options.JWTClaims + if s.skipAuthz { + gateClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, gateClaims, existing.Claims); err != nil { return err } } diff --git a/internal/service/db/impl_registry.go b/internal/service/db/impl_registry.go index 48a08185..e01f7d88 100644 --- a/internal/service/db/impl_registry.go +++ b/internal/service/db/impl_registry.go @@ -41,6 +41,9 @@ func (s *dbService) ListRegistries(ctx context.Context) ([]service.RegistryInfo, querier := sqlc.New(tx) callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } registries, err := streamRegistryRows(ctx, querier, callerClaims) if err != nil { @@ -105,7 +108,11 @@ func (s *dbService) GetRegistryByName(ctx context.Context, name string) (*servic } // Validate caller's JWT covers the registry's claims (hide existence on failure) - if err := validateClaimsSubset(ctx, claimsFromCtx(ctx), db.DeserializeClaims(reg.Claims)); err != nil { + callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } + if err := validateClaimsSubset(ctx, callerClaims, db.DeserializeClaims(reg.Claims)); err != nil { otel.RecordError(span, err) return nil, fmt.Errorf("%w: %s", service.ErrRegistryNotFound, name) } @@ -172,6 +179,9 @@ func (s *dbService) CreateRegistry( // Validate registry claims are a subset of the caller's JWT claims callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } if err := validateClaimsSubset(ctx, callerClaims, req.Claims); err != nil { otel.RecordError(span, err) return nil, err @@ -263,6 +273,9 @@ func (s *dbService) UpdateRegistry( // Validate caller's JWT covers both the existing and new registry claims callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } if err := validateClaimsSubsetBytes(ctx, callerClaims, existing.Claims); err != nil { otel.RecordError(span, err) return nil, err @@ -344,7 +357,11 @@ func (s *dbService) DeleteRegistry(ctx context.Context, name string) error { } // Validate caller's JWT covers the registry's claims - if err := validateClaimsSubsetBytes(ctx, claimsFromCtx(ctx), existing.Claims); err != nil { + callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, callerClaims, existing.Claims); err != nil { otel.RecordError(span, err) return err } @@ -410,7 +427,11 @@ func (s *dbService) ListRegistryEntries(ctx context.Context, registryName string } // Validate caller's JWT covers the registry's claims (hide existence on failure) - if err := validateClaimsSubsetBytes(ctx, claimsFromCtx(ctx), registry.Claims); err != nil { + callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, callerClaims, registry.Claims); err != nil { err = fmt.Errorf("%w: %s", service.ErrRegistryNotFound, registryName) otel.RecordError(span, err) return nil, err diff --git a/internal/service/db/impl_skills.go b/internal/service/db/impl_skills.go index 6970a241..bca454d7 100644 --- a/internal/service/db/impl_skills.go +++ b/internal/service/db/impl_skills.go @@ -215,12 +215,33 @@ func (s *dbService) GetSkillVersion( // Iterate rows in priority order (position ascending) and pick the first // one that passes the claims check, promoting lower-priority sources when - // higher-priority ones fail. - callerJSON := marshalClaims(options.Claims) + // higher-priority ones fail. The filter is nil for skipAuthz, anonymous, + // and super-admin callers (uniform bypass — see newClaimsFilterWith), in + // which case the highest-priority row wins outright. + claimsFilter := newClaimsFilterWith( + ctx, options.Claims, + func(record any) ([]byte, bool) { + r, ok := record.(sqlc.GetSkillVersionRow) + return r.Claims, ok + }, + ) + if s.skipAuthz { + claimsFilter = nil + } var row sqlc.GetSkillVersionRow found := false for _, r := range rows { - if s.skipAuthz || callerJSON == nil || checkClaims(callerJSON, r.Claims) { + if claimsFilter == nil { + row = r + found = true + break + } + ok, err := claimsFilter(ctx, r) + if err != nil { + otel.RecordError(span, err) + return nil, err + } + if ok { row = r found = true break @@ -313,7 +334,11 @@ func (s *dbService) PublishSkill( } // Validate published claims are a subset of the publisher's JWT claims - if err := validateClaimsSubset(ctx, options.JWTClaims, options.Claims); err != nil { + gateClaims := options.JWTClaims + if s.skipAuthz { + gateClaims = nil + } + if err := validateClaimsSubset(ctx, gateClaims, options.Claims); err != nil { otel.RecordError(span, err) return nil, err } @@ -667,7 +692,11 @@ func (s *dbService) executeDeleteSkillTransaction( } return fmt.Errorf("failed to look up registry entry: %w", err) } - if err := validateClaimsSubsetBytes(ctx, options.JWTClaims, existing.Claims); err != nil { + gateClaims := options.JWTClaims + if s.skipAuthz { + gateClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, gateClaims, existing.Claims); err != nil { return err } } diff --git a/internal/service/db/impl_source.go b/internal/service/db/impl_source.go index f82cde6c..050c4254 100644 --- a/internal/service/db/impl_source.go +++ b/internal/service/db/impl_source.go @@ -46,7 +46,11 @@ func (s *dbService) CreateSource( } // Validate source claims are a subset of the caller's JWT claims - if err := validateClaimsSubset(ctx, claimsFromCtx(ctx), req.Claims); err != nil { + callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } + if err := validateClaimsSubset(ctx, callerClaims, req.Claims); err != nil { otel.RecordError(span, err) return nil, err } @@ -243,6 +247,9 @@ func (s *dbService) UpdateSource( // Validate caller's JWT covers both the existing and new claims callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } if err := validateClaimsSubsetBytes(ctx, callerClaims, existing.Claims); err != nil { otel.RecordError(span, err) return nil, err @@ -366,7 +373,11 @@ func (s *dbService) DeleteSource(ctx context.Context, name string) error { } // Validate caller's JWT covers the source's claims - if err := validateClaimsSubsetBytes(ctx, claimsFromCtx(ctx), existing.Claims); err != nil { + callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, callerClaims, existing.Claims); err != nil { otel.RecordError(span, err) return err } @@ -429,6 +440,9 @@ func (s *dbService) ListSources(ctx context.Context) ([]service.SourceInfo, erro querier := sqlc.New(tx) callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } dbSources, err := streamSourceRows(ctx, querier, callerClaims) if err != nil { @@ -494,7 +508,11 @@ func (s *dbService) GetSourceByName(ctx context.Context, name string) (*service. info := buildSourceInfoFromGetByNameRow(&source) // Validate caller's JWT covers the source's claims (hide existence on failure) - if err := validateClaimsSubset(ctx, claimsFromCtx(ctx), info.Claims); err != nil { + callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } + if err := validateClaimsSubset(ctx, callerClaims, info.Claims); err != nil { otel.RecordError(span, err) return nil, fmt.Errorf("%w: %s", service.ErrSourceNotFound, name) } @@ -610,7 +628,11 @@ func (s *dbService) ListSourceEntries(ctx context.Context, sourceName string) ([ } // Validate caller's JWT covers the source's claims (hide existence on failure) - if err := validateClaimsSubsetBytes(ctx, claimsFromCtx(ctx), source.Claims); err != nil { + callerClaims := claimsFromCtx(ctx) + if s.skipAuthz { + callerClaims = nil + } + if err := validateClaimsSubsetBytes(ctx, callerClaims, source.Claims); err != nil { err = fmt.Errorf("%w: %s", service.ErrSourceNotFound, sourceName) otel.RecordError(span, err) return nil, err diff --git a/internal/service/db/publish_claims_test.go b/internal/service/db/publish_claims_test.go index f05a84a2..0c59c03b 100644 --- a/internal/service/db/publish_claims_test.go +++ b/internal/service/db/publish_claims_test.go @@ -93,11 +93,11 @@ func TestPublishServerVersion_ClaimsSubset(t *testing.T) { wantErr: nil, }, { - name: "nil request claims with any JWT succeeds", + name: "nil request claims with any JWT returns ErrClaimsInsufficient (default-deny)", serverSlug: "nil-req-claims", claims: nil, jwtClaims: map[string]any{"org": "acme", "team": "eng"}, - wantErr: nil, + wantErr: service.ErrClaimsInsufficient, }, } @@ -168,10 +168,10 @@ func TestPublishSkill_ClaimsSubset(t *testing.T) { wantErr: nil, }, { - name: "nil request claims with any JWT succeeds", + name: "nil request claims with any JWT returns ErrClaimsInsufficient (default-deny)", claims: nil, jwtClaims: map[string]any{"org": "acme", "team": "eng"}, - wantErr: nil, + wantErr: service.ErrClaimsInsufficient, }, } diff --git a/internal/service/db/registry_gate_test.go b/internal/service/db/registry_gate_test.go index 8744f916..6dd65c64 100644 --- a/internal/service/db/registry_gate_test.go +++ b/internal/service/db/registry_gate_test.go @@ -73,10 +73,10 @@ func TestLookupRegistryIDWithGate(t *testing.T) { wantErr error }{ { - name: "nil registry claims with caller claims passes", + name: "nil registry claims with caller claims returns ErrClaimsInsufficient (default-deny)", claims: nil, callerClaims: map[string]any{"org": "acme"}, - wantErr: nil, + wantErr: service.ErrClaimsInsufficient, }, { name: "nil registry claims with nil caller claims passes", @@ -161,10 +161,10 @@ func TestCheckRegistryExistsWithGate(t *testing.T) { wantErr error }{ { - name: "open registry with caller claims passes", + name: "open registry with caller claims returns ErrClaimsInsufficient (default-deny)", claims: nil, callerClaims: map[string]any{"org": "acme"}, - wantErr: nil, + wantErr: service.ErrClaimsInsufficient, }, { name: "matching claims passes", diff --git a/internal/service/db/shadowing_get_skill_version_test.go b/internal/service/db/shadowing_get_skill_version_test.go index e175b644..38afa1d6 100644 --- a/internal/service/db/shadowing_get_skill_version_test.go +++ b/internal/service/db/shadowing_get_skill_version_test.go @@ -10,6 +10,7 @@ import ( "github.com/aws/smithy-go/ptr" "github.com/stretchr/testify/require" + "github.com/stacklok/toolhive-registry-server/internal/auth" "github.com/stacklok/toolhive-registry-server/internal/db/sqlc" "github.com/stacklok/toolhive-registry-server/internal/service" ) @@ -196,3 +197,103 @@ func TestGetSkillVersionClaimsVisibility(t *testing.T) { }) } } + +// TestGetSkillVersion_SuperAdminBypassesPromotion verifies that the version +// promotion loop honours the uniform super-admin bypass — i.e. a super-admin +// caller whose JWT does not cover any source's claims still sees the +// highest-priority row, matching the behaviour of every other claim check +// (auth.md §3, claims_filter.go newClaimsFilterWith godoc). +func TestGetSkillVersion_SuperAdminBypassesPromotion(t *testing.T) { + t.Parallel() + + const ( + entryName = "com.claims/super-admin-skill" + descFromSrcA = "from source-A" + descFromSrcB = "from source-B" + descFromSrcC = "from source-C" + ) + + svc, cleanup := setupTestService(t) + defer cleanup() + + ctx := context.Background() + queries := sqlc.New(svc.pool) + now := time.Now().UTC() + + srcA, srcB, srcC := setupShadowingRegistry(t, svc) + + // Tag every entry with claims the super-admin's JWT does NOT cover, so the + // only path to a non-empty result is the super-admin bypass in + // newClaimsFilterWith. + entryClaimsJSON, err := json.Marshal(map[string]any{"org": "acme", "team": "data"}) + require.NoError(t, err) + + insertEntry := func(src sqlc.Source, desc string) { + entryID, err := queries.InsertRegistryEntry(ctx, sqlc.InsertRegistryEntryParams{ + SourceID: src.ID, + EntryType: sqlc.EntryTypeSKILL, + Name: entryName, + Claims: entryClaimsJSON, + CreatedAt: &now, + UpdatedAt: &now, + }) + require.NoError(t, err) + + versionID, err := queries.InsertEntryVersion(ctx, sqlc.InsertEntryVersionParams{ + EntryID: entryID, + Name: entryName, + Version: "1.0.0", + Title: ptr.String(entryName), + Description: ptr.String(desc), + CreatedAt: &now, + UpdatedAt: &now, + }) + require.NoError(t, err) + + _, err = queries.InsertSkillVersion(ctx, sqlc.InsertSkillVersionParams{ + VersionID: versionID, + Namespace: "com.example", + Status: sqlc.NullSkillStatus{}, + AllowedTools: nil, + Repository: []byte("null"), + Icons: []byte("null"), + Metadata: []byte("null"), + ExtensionMeta: []byte("null"), + }) + require.NoError(t, err) + } + + insertEntry(srcA, descFromSrcA) + insertEntry(srcB, descFromSrcB) + insertEntry(srcC, descFromSrcC) + + // JWT claims that don't cover the entries — the bypass must come from the + // super-admin role, not from claim coverage. The registry-access gate + // (lookupRegistryIDWithGate) is bypassed for the same reason. + saCtx := auth.ContextWithRoles(ctx, []auth.Role{auth.RoleSuperAdmin}) + + result, err := svc.GetSkillVersion( + saCtx, + service.WithName(entryName), + service.WithVersion("1.0.0"), + service.WithNamespace("com.example"), + service.WithRegistryName("claims-registry"), + service.WithClaims(map[string]any{"role": "super-admin"}), + ) + require.NoError(t, err) + require.Equal(t, descFromSrcA, result.Description, + "super-admin must see the highest-priority source's row regardless of claim coverage") + + // Sanity check: the same caller without the super-admin role gets ErrNotFound + // (every entry's claims are uncovered). + _, err = svc.GetSkillVersion( + ctx, + service.WithName(entryName), + service.WithVersion("1.0.0"), + service.WithNamespace("com.example"), + service.WithRegistryName("claims-registry"), + service.WithClaims(map[string]any{"sub": "claims-test-user"}), + ) + require.Error(t, err) + require.True(t, errors.Is(err, service.ErrNotFound)) +} diff --git a/internal/service/db/shadowing_helpers_test.go b/internal/service/db/shadowing_helpers_test.go index 2ec6f4f8..cf5e11fa 100644 --- a/internal/service/db/shadowing_helpers_test.go +++ b/internal/service/db/shadowing_helpers_test.go @@ -2,6 +2,7 @@ package database import ( "context" + "encoding/json" "testing" "time" @@ -15,6 +16,12 @@ import ( // "claims-src-b", and "claims-src-c", a CONFIG registry named // "claims-registry", and links the three sources at positions 0, 1, 2. It // returns the three sources in priority order (A, B, C). +// +// The registry and all three sources are tagged with claims {sub: "claims-test-user"} +// so the access gates pass for the standard shadowing-test caller. Default-deny on +// empty claims (auth.md §4) means resources without claims would be invisible to +// claim-bearing callers, and the shadowing tests are about entry-level dedup, not +// the access gate. func setupShadowingRegistry(t *testing.T, svc *dbService) (sqlc.Source, sqlc.Source, sqlc.Source) { t.Helper() @@ -22,11 +29,15 @@ func setupShadowingRegistry(t *testing.T, svc *dbService) (sqlc.Source, sqlc.Sou queries := sqlc.New(svc.pool) now := time.Now().UTC() + gateClaims, err := json.Marshal(map[string]any{"sub": "claims-test-user"}) + require.NoError(t, err) + srcA, err := queries.InsertSource(ctx, sqlc.InsertSourceParams{ Name: "claims-src-a", CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, }) @@ -37,6 +48,7 @@ func setupShadowingRegistry(t *testing.T, svc *dbService) (sqlc.Source, sqlc.Sou CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, }) @@ -47,6 +59,7 @@ func setupShadowingRegistry(t *testing.T, svc *dbService) (sqlc.Source, sqlc.Sou CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, }) @@ -55,6 +68,7 @@ func setupShadowingRegistry(t *testing.T, svc *dbService) (sqlc.Source, sqlc.Sou reg, err := queries.UpsertRegistry(ctx, sqlc.UpsertRegistryParams{ Name: "claims-registry", CreationType: sqlc.CreationTypeCONFIG, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, }) diff --git a/internal/service/db/shadowing_list_servers_test.go b/internal/service/db/shadowing_list_servers_test.go index f89e0be6..24025200 100644 --- a/internal/service/db/shadowing_list_servers_test.go +++ b/internal/service/db/shadowing_list_servers_test.go @@ -130,12 +130,19 @@ func TestListServersClaimsVisibility(t *testing.T) { queries := sqlc.New(svc.pool) now := time.Now().UTC() + // Tag the registry and sources with claims that the standard test caller + // covers — default-deny on empty claims (auth.md §4) means untagged + // resources are invisible to claim-bearing callers. + gateClaimsJSON, err := json.Marshal(map[string]any{"sub": "claims-test-user"}) + require.NoError(t, err) + // Create three sources. srcA, err := queries.InsertSource(ctx, sqlc.InsertSourceParams{ Name: "claims-src-a", CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaimsJSON, CreatedAt: &now, UpdatedAt: &now, }) @@ -146,6 +153,7 @@ func TestListServersClaimsVisibility(t *testing.T) { CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaimsJSON, CreatedAt: &now, UpdatedAt: &now, }) @@ -156,6 +164,7 @@ func TestListServersClaimsVisibility(t *testing.T) { CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaimsJSON, CreatedAt: &now, UpdatedAt: &now, }) @@ -165,6 +174,7 @@ func TestListServersClaimsVisibility(t *testing.T) { reg, err := queries.UpsertRegistry(ctx, sqlc.UpsertRegistryParams{ Name: "claims-registry", CreationType: sqlc.CreationTypeCONFIG, + Claims: gateClaimsJSON, CreatedAt: &now, UpdatedAt: &now, }) diff --git a/internal/service/db/skip_authz_test.go b/internal/service/db/skip_authz_test.go index dd1890b4..ba4b5a1a 100644 --- a/internal/service/db/skip_authz_test.go +++ b/internal/service/db/skip_authz_test.go @@ -2,6 +2,7 @@ package database import ( "context" + "encoding/json" "testing" "time" @@ -93,11 +94,18 @@ func TestListServers_SkipAuthz(t *testing.T) { srcName := "sa-srv-src-" + tt.name entryName := "com.skipauth/" + tt.name + // Tag registry and source with claims that the caller covers so the + // registry-access gate passes; the entry's own claims drive what we're + // testing (visibility under skipAuthz vs. default-deny on empty). + gateClaims, err := json.Marshal(map[string]any{"org": "caller-org"}) + require.NoError(t, err) + src, err := queries.InsertSource(ctx, sqlc.InsertSourceParams{ Name: srcName, CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, }) @@ -106,6 +114,7 @@ func TestListServers_SkipAuthz(t *testing.T) { reg, err := queries.UpsertRegistry(ctx, sqlc.UpsertRegistryParams{ Name: regName, CreationType: sqlc.CreationTypeCONFIG, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, }) @@ -215,11 +224,15 @@ func TestListSkills_SkipAuthz(t *testing.T) { srcName := "sa-skl-src-" + tt.name entryName := "com.skipauth/" + tt.name + gateClaims, err := json.Marshal(map[string]any{"org": "caller-org"}) + require.NoError(t, err) + src, err := queries.InsertSource(ctx, sqlc.InsertSourceParams{ Name: srcName, CreationType: sqlc.CreationTypeCONFIG, SourceType: "git", Syncable: true, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, }) @@ -228,6 +241,7 @@ func TestListSkills_SkipAuthz(t *testing.T) { reg, err := queries.UpsertRegistry(ctx, sqlc.UpsertRegistryParams{ Name: regName, CreationType: sqlc.CreationTypeCONFIG, + Claims: gateClaims, CreatedAt: &now, UpdatedAt: &now, })