diff --git a/agent-network/README.md b/agent-network/README.md index 14e725603..014874ee6 100644 --- a/agent-network/README.md +++ b/agent-network/README.md @@ -112,7 +112,10 @@ Two roles delegate Agent Network access without account-admin rights: - **`usage_viewer`** — the regular User baseline plus read on `agent_network.usage` (the aggregated usage and cost overview) and read-only access to the resources the usage filters resolve against: users, groups, - peers, and the provider list. No policies, no request-level access logs. + peers, and the provider list (connection config redacted — no upstream URLs + or operator-supplied header values). No policies, and no account-wide + request-level access logs; like any caller, it still reads its own requests + through the self-scoped endpoints below. Every authenticated user, regardless of role, can read the caller-scoped self-service endpoint `GET /api/agent-network/me/setup` (the endpoint, providers, diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go index 8bbc92653..6adae4814 100644 --- a/management/internals/modules/agentnetwork/manager.go +++ b/management/internals/modules/agentnetwork/manager.go @@ -179,14 +179,52 @@ func (m *managerImpl) GetAllProviders(ctx context.Context, accountID, userID str if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Read); err != nil { return nil, err } - return m.store.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, accountID) + providers, err := m.store.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, accountID) + if err != nil { + return nil, err + } + return m.redactProvidersForViewer(ctx, accountID, userID, providers) } func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, providerID string) (*types.Provider, error) { if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Read); err != nil { return nil, err } - return m.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, accountID, providerID) + provider, err := m.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, accountID, providerID) + if err != nil { + return nil, err + } + redacted, err := m.redactProvidersForViewer(ctx, accountID, userID, []*types.Provider{provider}) + if err != nil { + return nil, err + } + return redacted[0], nil +} + +// redactProvidersForViewer strips the connection configuration from +// providers handed to a caller who holds only the read grant on +// agent_network.providers. Update is the managing signal: a role that can +// edit a provider sees its config in the edit form anyway, while a +// read-only role (usage_viewer) only needs the display surface the usage +// filters resolve against — upstream URLs and operator-supplied header +// values are not part of that. Validation errors fail closed. +func (m *managerImpl) redactProvidersForViewer(ctx context.Context, accountID, userID string, providers []*types.Provider) ([]*types.Provider, error) { + canManage, _, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Update) + if err != nil { + return nil, status.NewPermissionValidationError(err) + } + if canManage { + return providers, nil + } + out := make([]*types.Provider, 0, len(providers)) + for _, p := range providers { + if p == nil { + out = append(out, nil) + continue + } + out = append(out, p.RedactedForViewer()) + } + return out, nil } // DiscoverProviderModels asks the vendor which models a credential can reach. diff --git a/management/internals/modules/agentnetwork/provider_redaction_test.go b/management/internals/modules/agentnetwork/provider_redaction_test.go new file mode 100644 index 000000000..96598996a --- /dev/null +++ b/management/internals/modules/agentnetwork/provider_redaction_test.go @@ -0,0 +1,87 @@ +package agentnetwork + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/permissions/modules" + "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/server/store" +) + +// These tests pin the provider read surface per grant: a caller holding +// providers read together with update (managers) gets the full record, +// while read-only viewers (usage_viewer) get the display surface only — +// connection configuration is redacted before it reaches the wire layer. + +func TestGetAllProviders_RedactsConnectionConfigForReadOnlyViewer(t *testing.T) { + ctx := context.Background() + f := newBootstrapFixture(t) + + saved := newSynthTestProvider() + saved.ExtraValues = map[string]string{"x-portkey-config": "cfg-123"} + saved.IdentityHeaderUserID = "X-User" + saved.IdentityHeaderGroups = "X-Groups" + saved.SkipTLSVerification = true + require.NoError(t, f.store.SaveAgentNetworkProvider(ctx, saved)) + + f.expectPermission(testAccountID, "viewer", modules.AgentNetworkProviders, operations.Read, true) + f.expectPermission(testAccountID, "viewer", modules.AgentNetworkProviders, operations.Update, false) + + providers, err := f.manager.GetAllProviders(ctx, testAccountID, "viewer") + require.NoError(t, err) + require.Len(t, providers, 1) + p := providers[0] + assert.Equal(t, saved.ID, p.ID, "identity survives redaction") + assert.Equal(t, saved.Name, p.Name) + assert.Equal(t, saved.ProviderID, p.ProviderID) + assert.Equal(t, saved.Models, p.Models, "the model list backs the usage filters and stays") + assert.True(t, p.Enabled) + assert.Empty(t, p.UpstreamURL, "upstream URL is connection config") + assert.Empty(t, p.ExtraValues, "operator-typed header values are connection config") + assert.Empty(t, p.IdentityHeaderUserID) + assert.Empty(t, p.IdentityHeaderGroups) + assert.False(t, p.SkipTLSVerification) + assert.Empty(t, p.APIKey) + assert.Empty(t, p.SessionPrivateKey) + + stored, err := f.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, testAccountID, saved.ID) + require.NoError(t, err) + assert.NotEmpty(t, stored.UpstreamURL, "redaction must not write back to the store") +} + +func TestGetProvider_FullConfigForManagingCaller(t *testing.T) { + ctx := context.Background() + f := newBootstrapFixture(t) + + saved := newSynthTestProvider() + saved.ExtraValues = map[string]string{"x-portkey-config": "cfg-123"} + require.NoError(t, f.store.SaveAgentNetworkProvider(ctx, saved)) + + f.expectPermission(testAccountID, "admin", modules.AgentNetworkProviders, operations.Read, true) + f.expectPermission(testAccountID, "admin", modules.AgentNetworkProviders, operations.Update, true) + + p, err := f.manager.GetProvider(ctx, testAccountID, "admin", saved.ID) + require.NoError(t, err) + assert.Equal(t, saved.UpstreamURL, p.UpstreamURL, "a caller who can edit the provider sees its config") + assert.Equal(t, saved.ExtraValues, p.ExtraValues) +} + +func TestGetProvider_RedactsForReadOnlyViewer(t *testing.T) { + ctx := context.Background() + f := newBootstrapFixture(t) + + saved := newSynthTestProvider() + require.NoError(t, f.store.SaveAgentNetworkProvider(ctx, saved)) + + f.expectPermission(testAccountID, "viewer", modules.AgentNetworkProviders, operations.Read, true) + f.expectPermission(testAccountID, "viewer", modules.AgentNetworkProviders, operations.Update, false) + + p, err := f.manager.GetProvider(ctx, testAccountID, "viewer", saved.ID) + require.NoError(t, err) + assert.Equal(t, saved.ID, p.ID) + assert.Empty(t, p.UpstreamURL) +} diff --git a/management/internals/modules/agentnetwork/setup.go b/management/internals/modules/agentnetwork/setup.go index a1e111f50..204dcb382 100644 --- a/management/internals/modules/agentnetwork/setup.go +++ b/management/internals/modules/agentnetwork/setup.go @@ -14,8 +14,11 @@ import ( // groups authorize. It deliberately performs no role permission check: // the result is scoped to the caller's own groups, which is strictly // tighter than any role gate, so every authenticated user (any role) may -// read it. Peers and users carry the same groups, so the answer matches -// what the proxy enforces for the caller's machines at request time. +// read it. The group source matches enforcement: the proxy authorizes +// each Agent Network request against the calling user's groups as well — +// session validation resolves them from the same user record's +// auto-groups — so this answer and the proxy's verdict are computed from +// the same memberships. func (m *managerImpl) GetSetupForUser(ctx context.Context, accountID, userID string) (*types.EffectiveSetup, error) { user, err := m.store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) if err != nil { @@ -204,7 +207,13 @@ func effectiveModelsForProvider(provider *types.Provider, policies []*types.Poli } out := make([]string, 0, len(declared)) for _, id := range declared { - if _, ok := seen[normaliseModelID(id)]; ok { + // Compare through the canonical id the proxy's parser emits — a + // Bedrock declaration may carry the region/version form + // ("eu.anthropic.claude-...-v1:0") while the allowlist holds the + // stripped id the parser matches at request time, and the raw + // forms would never intersect. The declared id itself is what + // gets advertised, matching the router's route claim. + if _, ok := seen[normaliseModelID(normalizePricingModelID(provider.ProviderID, id))]; ok { out = append(out, id) } } diff --git a/management/internals/modules/agentnetwork/setup_realstore_test.go b/management/internals/modules/agentnetwork/setup_realstore_test.go index 0ba8e60ba..03540a444 100644 --- a/management/internals/modules/agentnetwork/setup_realstore_test.go +++ b/management/internals/modules/agentnetwork/setup_realstore_test.go @@ -114,6 +114,36 @@ func TestEffectiveSetup_RealStore_AllowlistIntersectsDeclaredModels(t *testing.T assert.Equal(t, []string{"gpt-5.4"}, p.Models, "allowlist ∩ declared, in declared order and casing") } +func TestEffectiveSetup_RealStore_AllowlistMatchesBedrockDeclaredIDsCanonically(t *testing.T) { + mgr, s := newSetupTestMgr(t) + ctx := context.Background() + + require.NoError(t, s.SaveAgentNetworkSettings(ctx, newSynthTestSettings())) + // A Bedrock operator typically declares the region/version form the + // vendor lists, while the allowlist holds the canonical id the proxy's + // parser emits at request time. The intersection must compare through + // the same normalization the parser applies, and the declared (raw) + // id is what gets advertised — it is what the router claims. + provider := newSynthTestProvider() + provider.ProviderID = "bedrock_api" + provider.Name = "Bedrock" + provider.Models = []types.ProviderModel{ + {ID: "eu.anthropic.claude-sonnet-4-5-20250929-v1:0"}, + {ID: "eu.amazon.nova-pro-v1:0"}, + } + require.NoError(t, s.SaveAgentNetworkProvider(ctx, provider)) + require.NoError(t, s.SaveAgentNetworkGuardrail(ctx, newSetupTestGuardrail("guard-1", "anthropic.claude-sonnet-4-5"))) + require.NoError(t, s.SaveAgentNetworkPolicy(ctx, newSynthTestPolicy(provider.ID, "grp-eng", "guard-1"))) + + setup, err := mgr.effectiveSetupForGroups(ctx, testAccountID, []string{"grp-eng"}) + require.NoError(t, err) + require.Len(t, setup.Providers, 1) + p := setup.Providers[0] + assert.False(t, p.AllModelsAllowed) + assert.Equal(t, []string{"eu.anthropic.claude-sonnet-4-5-20250929-v1:0"}, p.Models, + "the allowlisted canonical id must admit the declared region/version form, and only it") +} + func TestEffectiveSetup_RealStore_UnrestrictedPolicyWinsOverRestricted(t *testing.T) { mgr, s := newSetupTestMgr(t) ctx := context.Background() diff --git a/management/internals/modules/agentnetwork/types/provider.go b/management/internals/modules/agentnetwork/types/provider.go index b9a194bf6..145c63791 100644 --- a/management/internals/modules/agentnetwork/types/provider.go +++ b/management/internals/modules/agentnetwork/types/provider.go @@ -175,6 +175,26 @@ func (p *Provider) FromAPIRequest(req *api.AgentNetworkProviderRequest) { // ToAPIResponse renders the provider as the API representation. The API // key is intentionally never surfaced. +// RedactedForViewer returns a copy with the connection configuration +// blanked: upstream URL, operator-typed extra header values, identity +// header names, the TLS-verification override, and (defence in depth — +// they never reach the wire anyway) the sealed credentials. Read-only +// viewers such as usage_viewer only need the display surface — id, +// catalog id, name, enabled state, and the model list the usage filters +// resolve against — so their responses carry nothing about how the +// operator connects to the vendor. +func (p *Provider) RedactedForViewer() *Provider { + c := *p + c.UpstreamURL = "" + c.APIKey = "" + c.ExtraValues = nil + c.IdentityHeaderUserID = "" + c.IdentityHeaderGroups = "" + c.SkipTLSVerification = false + c.SessionPrivateKey = "" + return &c +} + func (p *Provider) ToAPIResponse() *api.AgentNetworkProvider { models := make([]api.AgentNetworkProviderModel, 0, len(p.Models)) for _, m := range p.Models { diff --git a/management/server/permissions/roles/usage_viewer.go b/management/server/permissions/roles/usage_viewer.go index 83c131f3a..e480ae478 100644 --- a/management/server/permissions/roles/usage_viewer.go +++ b/management/server/permissions/roles/usage_viewer.go @@ -11,8 +11,12 @@ import ( // to the resources the usage filters and display columns resolve against: // users and groups (identity filters and name resolution), peers (agent // principals in the caller column), and the provider list (provider and -// model filter options). It sees no policies and no request-level access -// logs (which can contain captured prompts). +// model filter options — the manager redacts connection config such as +// upstream URLs and operator-supplied header values for callers holding +// read without update). It sees no policies and no account-wide +// request-level access logs (which can contain captured prompts); its own +// requests remain readable through the self-scoped endpoints, like any +// caller's. var UsageViewer = RolePermissions{ Role: types.UserRoleUsageViewer, AutoAllowNew: map[operations.Operation]bool{