From 883dfab8f93bb88bfc5553e4e2fa3b5c702c67c9 Mon Sep 17 00:00:00 2001 From: "ccf-lisa[bot]" <286799724+ccf-lisa[bot]@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:11:48 -0300 Subject: [PATCH] feat(authz): agent configure/sync actions and per-route agent guards Tenth layer of the agent remote-configuration stack (split from #465): the agent resource gains configure (write an agent's overlay) and sync (an agent fetching its overlay and reporting) in the manifest and Cedar roles; the builtin PDP requires the admin check for users on agent:* and lets agent service accounts only register, ingest and sync (R39); /admin/agents list/get need agent:read while writes and key routes stay admin:manage (R40). Co-Authored-By: Claude Opus 5.5 --- authz-roles.yaml | 2 +- internal/api/handler/agents.go | 22 ++-- internal/api/handler/api.go | 6 +- .../agent_config_authz_integration_test.go | 80 ++++++++++++ internal/authz/agent_config_authz_test.go | 118 ++++++++++++++++++ internal/authz/builtin.go | 30 ++++- internal/authz/manifest.yaml | 15 ++- internal/authz/pdp.go | 6 + 8 files changed, 258 insertions(+), 21 deletions(-) create mode 100644 internal/authz/agent_config_authz_integration_test.go create mode 100644 internal/authz/agent_config_authz_test.go diff --git a/authz-roles.yaml b/authz-roles.yaml index 4f9baf84..9dfe6905 100644 --- a/authz-roles.yaml +++ b/authz-roles.yaml @@ -12,7 +12,7 @@ # contributor author OSCAL/registers/workflows/dashboards/evidence; no admin # auditor read everything + record evidence and maintain the risk/poam register # viewer read-only -# agent service accounts: ingest evidence/heartbeats, register +# agent service accounts: ingest evidence/heartbeats/artifacts, register, sync remote config # # Matching is case-insensitive. `users` keys are the login email (the subject id) and # work for every user today. `groups` keys are resolved group names (native CCF groups diff --git a/internal/api/handler/agents.go b/internal/api/handler/agents.go index 9c33899f..4bac302e 100644 --- a/internal/api/handler/agents.go +++ b/internal/api/handler/agents.go @@ -72,16 +72,18 @@ func NewAgentHandler(sugar *zap.SugaredLogger, db *gorm.DB) *AgentHandler { return &AgentHandler{sugar: sugar, db: db} } -func (h *AgentHandler) Register(api *echo.Group) { - api.GET("", h.ListAgents) - api.POST("", h.CreateAgent) - api.GET("/:id", h.GetAgent) - api.PUT("/:id", h.UpdateAgent) - api.DELETE("/:id", h.DeleteAgent) - api.POST("/:id/keys", h.CreateAgentKey) - api.GET("/:id/keys", h.ListAgentKeys) - api.GET("/:id/keys/:keyId", h.GetAgentKey) - api.DELETE("/:id/keys/:keyId", h.DeleteAgentKey) +// Register mounts the agent routes with per-route guards (R40): the list and get reads use +// readGuard (agent:read), while writes and every key route use adminGuard (admin:manage). +func (h *AgentHandler) Register(api *echo.Group, readGuard, adminGuard echo.MiddlewareFunc) { + api.GET("", h.ListAgents, readGuard) + api.GET("/:id", h.GetAgent, readGuard) + api.POST("", h.CreateAgent, adminGuard) + api.PUT("/:id", h.UpdateAgent, adminGuard) + api.DELETE("/:id", h.DeleteAgent, adminGuard) + api.POST("/:id/keys", h.CreateAgentKey, adminGuard) + api.GET("/:id/keys", h.ListAgentKeys, adminGuard) + api.GET("/:id/keys/:keyId", h.GetAgentKey, adminGuard) + api.DELETE("/:id/keys/:keyId", h.DeleteAgentKey, adminGuard) } func (h *AgentHandler) ListAgents(ctx echo.Context) error { diff --git a/internal/api/handler/api.go b/internal/api/handler/api.go index fdbb9463..a8a87381 100644 --- a/internal/api/handler/api.go +++ b/internal/api/handler/api.go @@ -213,11 +213,13 @@ func RegisterHandlers(server *api.Server, logger *zap.SugaredLogger, db *gorm.DB agentSubjectTemplateGroup := server.API().Group("/agent/subject-templates") subjectTemplateHandler.RegisterAgent(agentSubjectTemplateGroup, agentIngestMiddleware, pep.For(authz.ResourceSubjectTemplate).Update()) + // Agent routes are guarded per route (R40): list/get need agent:read, writes and keys stay + // admin:manage. The builtin PDP still requires the admin check for users on agent:*. + agentGuard := pep.For(authz.ResourceAgent) agentHandler := NewAgentHandler(logger, db) agentsGroup := server.API().Group("/admin/agents") agentsGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) - agentsGroup.Use(pep.Authorize(authz.ResourceAdmin, authz.ActionManage)) - agentHandler.Register(agentsGroup) + agentHandler.Register(agentsGroup, agentGuard.Read(), pep.Authorize(authz.ResourceAdmin, authz.ActionManage)) userHandler := NewUserHandler(logger, db) diff --git a/internal/authz/agent_config_authz_integration_test.go b/internal/authz/agent_config_authz_integration_test.go new file mode 100644 index 00000000..5395d5d1 --- /dev/null +++ b/internal/authz/agent_config_authz_integration_test.go @@ -0,0 +1,80 @@ +//go:build integration + +package authz + +import ( + "context" + "testing" + "time" + + "github.com/compliance-framework/api/internal/service/relational" + "github.com/stretchr/testify/require" + "go.uber.org/zap" +) + +// R53: a user's effective Cedar roles are the union of their direct assignment and one +// assignment per native group they belong to, as resolved through the production wiring +// (DB role resolver + DB group PIP). +func TestUserAndGroupGrantsCombine(t *testing.T) { + db := setupAuthzDB(t) + user := createUser(t, db, "u@x.example", "password") + grant(t, db, relational.RoleAssigneeTypeUser, user.Email, "viewer", relational.RoleAssignmentSourceManual) + grant(t, db, relational.RoleAssigneeTypeGroup, "ccf-contributors", "contributor", relational.RoleAssignmentSourceManual) + addNativeGroup(t, db, user, "ccf-contributors") + + m, err := DefaultManifest() + require.NoError(t, err) + policies, err := CompileRolePolicies(m) + require.NoError(t, err) + defaults := &RoleAssignments{Agents: DefaultAgentRole} + defaults.normalize() + const ttl = 50 * time.Millisecond + cedarPDP := NewCedar(policies, NewDBRoleResolver(db, defaults, ttl, zap.NewNop().Sugar()), zap.NewNop().Sugar()) + pdp := newResolvingPDP(cedarPDP, NewDBGroupResolver(db, zap.NewNop().Sugar()), zap.NewNop().Sugar()) + + subject := Subject{Type: "user", ID: user.Email} + allow := func(action, resource string) bool { + t.Helper() + d, err := pdp.Evaluate(context.Background(), subject, action, Resource{Type: resource}, nil) + require.NoError(t, err) + return d.Allow + } + + require.True(t, allow(ActionCreate, ResourceCatalog), "catalog create via the group grant") + require.True(t, allow(ActionRead, ResourceCatalog), "catalog read via the direct viewer grant") + require.False(t, allow(ActionConfigure, ResourceAgent), "configure is in neither grant") + + // Remove the membership; once the role cache expires the group grant no longer applies. + require.NoError(t, db.Where("user_id = ?", user.ID.String()).Delete(&relational.UserGroupMembership{}).Error) + time.Sleep(2 * ttl) + require.False(t, allow(ActionCreate, ResourceCatalog), "catalog create without the membership") + require.True(t, allow(ActionRead, ResourceCatalog), "the direct viewer grant still applies") +} + +// R39: under the builtin driver a user on the agent resource needs the admin check, so an SSO +// user without the required admin group is denied every agent action, while a password user +// (super admin) is allowed. +func TestBuiltinAgentResourceRequiresAdminForUsers(t *testing.T) { + db := setupAuthzDB(t) + cfg := ssoEnabledConfig([]string{"ccf-admins"}) + ssoUser := createUser(t, db, "sso@x.example", "sso") + createSSOLink(t, db, ssoUser, "test", []string{"engineering"}) + createUser(t, db, "pw@x.example", "password") + + b := NewBuiltin(db, cfg, zap.NewNop().Sugar()) + for _, action := range []string{ActionRead, ActionConfigure} { + dec, err := b.Evaluate(context.Background(), Subject{Type: "user", ID: ssoUser.Email}, action, Resource{Type: ResourceAgent}, nil) + require.NoError(t, err) + require.False(t, dec.Allow, "SSO non-admin %s on agent must be denied", action) + + dec, err = b.Evaluate(context.Background(), Subject{Type: "user", ID: "pw@x.example"}, action, Resource{Type: ResourceAgent}, nil) + require.NoError(t, err) + require.True(t, dec.Allow, "password user %s on agent must be allowed", action) + } + + adminUser := createUser(t, db, "admin@x.example", "sso") + createSSOLink(t, db, adminUser, "test", []string{"ccf-admins"}) + dec, err := b.Evaluate(context.Background(), Subject{Type: "user", ID: adminUser.Email}, ActionConfigure, Resource{Type: ResourceAgent}, nil) + require.NoError(t, err) + require.True(t, dec.Allow, "SSO admin-group member must be allowed") +} diff --git a/internal/authz/agent_config_authz_test.go b/internal/authz/agent_config_authz_test.go new file mode 100644 index 00000000..1a051475 --- /dev/null +++ b/internal/authz/agent_config_authz_test.go @@ -0,0 +1,118 @@ +package authz + +import ( + "context" + "slices" + "testing" +) + +// Agent remote configuration vocabulary (A2.4): the agent resource declares configure and +// sync. +func TestManifestAgentConfigVocabulary(t *testing.T) { + m, err := DefaultManifest() + if err != nil { + t.Fatal(err) + } + res, ok := m.Resources[ResourceAgent] + if !ok { + t.Fatal("agent resource missing from manifest") + } + for _, action := range []string{ActionRead, ActionRegister, ActionIngest, ActionConfigure, ActionSync} { + if !slices.Contains(res.Actions, action) { + t.Errorf("agent resource missing action %q (have %v)", action, res.Actions) + } + } + if got := m.Roles["agent"][ResourceAgent]; !slices.Contains(got, ActionSync) { + t.Errorf("agent role agent grants = %v, want sync", got) + } +} + +// The Cedar matrix for the new actions (A2 tests). +func TestCedarAgentConfigMatrix(t *testing.T) { + c := mustCedar(t, &RoleAssignments{ + Users: map[string]string{ + "admin@x": "admin", + "viewer@x": "viewer", + "contributor@x": "contributor", + }, + }) + user := func(id string) Subject { return Subject{Type: "user", ID: id} } + agent := Subject{Type: "agent", ID: "agent-1"} + + cases := []struct { + subj Subject + action string + resource string + want bool + }{ + {agent, ActionSync, ResourceAgent, true}, + {agent, ActionConfigure, ResourceAgent, false}, + {agent, ActionRead, ResourceAgent, false}, + + {user("viewer@x"), ActionRead, ResourceAgent, true}, + {user("viewer@x"), ActionConfigure, ResourceAgent, false}, + {user("viewer@x"), ActionSync, ResourceAgent, false}, + {user("viewer@x"), ActionRead, ResourceArtifact, true}, // "*": [read] covers #464 artifacts + {user("viewer@x"), ActionIngest, ResourceArtifact, false}, + {user("contributor@x"), ActionRead, ResourceAgent, true}, + {user("contributor@x"), ActionConfigure, ResourceAgent, false}, + + {user("admin@x"), ActionConfigure, ResourceAgent, true}, + + {Subject{Type: "anonymous"}, ActionRead, ResourceAgent, false}, + } + for _, tc := range cases { + if got := allows(t, c, tc.subj, tc.action, tc.resource); got != tc.want { + t.Errorf("%s %s on %s: allow = %v, want %v", tc.subj.ID, tc.action, tc.resource, got, tc.want) + } + } +} + +// Builtin (R39): agent service accounts are allowed on the agent resource, anonymous is +// denied, and users go through the admin check (a user subject without an id is denied +// before any DB access, so a nil DB proves the admin path is taken). +func TestBuiltinAgentResource(t *testing.T) { + b := NewBuiltin(nil, nil, nil) + ctx := context.Background() + + for _, action := range []string{ActionRegister, ActionIngest, ActionSync} { + dec, err := b.Evaluate(ctx, Subject{Type: "agent", ID: "agent-1"}, action, Resource{Type: ResourceAgent}, nil) + if err != nil || !dec.Allow { + t.Errorf("agent %s: allow=%v err=%v, want allow", action, dec.Allow, err) + } + } + for _, action := range []string{ActionRead, ActionConfigure, ActionCreate, ActionUpdate, ActionDelete} { + dec, err := b.Evaluate(ctx, Subject{Type: "agent", ID: "agent-1"}, action, Resource{Type: ResourceAgent}, nil) + if err != nil || dec.Allow { + t.Errorf("agent %s: allow=%v err=%v, want deny (Cedar parity)", action, dec.Allow, err) + } + } + for _, action := range []string{ActionRead, ActionConfigure, ActionSync} { + dec, err := b.Evaluate(ctx, Subject{Type: "anonymous"}, action, Resource{Type: ResourceAgent}, nil) + if err != nil || dec.Allow { + t.Errorf("anonymous %s: allow=%v err=%v, want deny", action, dec.Allow, err) + } + } + dec, err := b.Evaluate(ctx, Subject{Type: "user", ID: ""}, ActionRead, Resource{Type: ResourceAgent}, nil) + if err != nil || dec.Allow { + t.Errorf("user without id on agent:read: allow=%v err=%v, want deny via the admin check", dec.Allow, err) + } + + // Batch: the user admin decision is memoized across agent and admin requests. + decs, err := b.Evaluations(ctx, []EvalRequest{ + {Subject: Subject{Type: "user", ID: ""}, Action: ActionConfigure, Resource: Resource{Type: ResourceAgent}}, + {Subject: Subject{Type: "user", ID: ""}, Action: ActionManage, Resource: Resource{Type: ResourceAdmin}}, + {Subject: Subject{Type: "agent", ID: "a"}, Action: ActionSync, Resource: Resource{Type: ResourceAgent}}, + {Subject: Subject{Type: "user", ID: "x@y"}, Action: ActionRead, Resource: Resource{Type: ResourceEvidence}}, + {Subject: Subject{Type: "agent", ID: "a"}, Action: ActionConfigure, Resource: Resource{Type: ResourceAgent}}, + }) + if err != nil { + t.Fatal(err) + } + want := []bool{false, false, true, true, false} + for i, d := range decs { + if d.Allow != want[i] { + t.Errorf("batch[%d] allow = %v, want %v", i, d.Allow, want[i]) + } + } +} diff --git a/internal/authz/builtin.go b/internal/authz/builtin.go index 44c5036b..735fcf5b 100644 --- a/internal/authz/builtin.go +++ b/internal/authz/builtin.go @@ -21,8 +21,9 @@ func init() { // Builtin is the default, in-process PDP. It reproduces CCF's pre-authz access rules with // zero behavior change: admin resources require SSO admin-group membership (password -// users are treated as super admins), and every other resource is allowed once the -// request is authenticated — the authn middleware having already enforced authentication +// users are treated as super admins), the agent resource requires the same admin check for +// users (agent service accounts may register, ingest and sync; anonymous is denied; R39), and every other +// resource is allowed once the request is authenticated — the authn middleware having already enforced authentication // and any public-endpoint policy before the PEP runs. // // In Phase 1 the builtin driver resolves SSO facts itself (it holds db + config), acting @@ -45,10 +46,27 @@ func NewBuiltin(db *gorm.DB, cfg *config.Config, logger *zap.SugaredLogger) *Bui } // Evaluate implements PDP. -func (b *Builtin) Evaluate(ctx context.Context, s Subject, _ string, r Resource, _ map[string]any) (Decision, error) { +func (b *Builtin) Evaluate(ctx context.Context, s Subject, action string, r Resource, _ map[string]any) (Decision, error) { switch r.Type { case ResourceAdmin: return b.evaluateAdmin(ctx, s) + case ResourceAgent: + // Agent configuration can push plugins and policies to hosts, so under builtin a + // user needs the admin check for every agent action (parity with /admin/agents, + // R39). Agent service accounts may only register, ingest and sync (parity with + // Cedar's agent role); anonymous subjects are denied. + switch s.Type { + case "agent": + switch action { + case ActionRegister, ActionIngest, ActionSync: + return Decision{Allow: true, Reason: "builtin: agent service account"}, nil + } + return Decision{Allow: false, Reason: "builtin: agent service accounts may only register, ingest and sync on the agent resource"}, nil + case "user": + return b.evaluateAdmin(ctx, s) + default: + return Decision{Allow: false, Reason: "builtin: anonymous access to agent resource"}, nil + } default: // Phase 1: authenticated = allowed. The authn middleware already enforced // authentication (and any public-endpoint policy) before the PEP runs, so any @@ -60,7 +78,8 @@ func (b *Builtin) Evaluate(ctx context.Context, s Subject, _ string, r Resource, } // Evaluations implements PDP by evaluating each request independently, in order. Admin -// decisions are memoized per subject for the batch: evaluateAdmin ignores the action and +// decisions (resource admin, and resource agent for users) are memoized per subject for the +// batch: evaluateAdmin ignores the action and // keys only on the subject, so a batch enumerating several admin.* actions (e.g. // /me/permissions) would otherwise repeat the same user + SSO-link DB lookups once per // action. The memo keeps the facts inside the builtin driver and preserves both ordering @@ -71,7 +90,8 @@ func (b *Builtin) Evaluations(ctx context.Context, reqs []EvalRequest) ([]Decisi out := make([]Decision, len(reqs)) for i, req := range reqs { - if req.Resource.Type == ResourceAdmin { + // Agent-resource requests from users resolve through the same admin check. + if req.Resource.Type == ResourceAdmin || (req.Resource.Type == ResourceAgent && req.Subject.Type == "user") { key := adminKey{req.Subject.Type, req.Subject.ID} if d, ok := adminMemo[key]; ok { out[i] = d diff --git a/internal/authz/manifest.yaml b/internal/authz/manifest.yaml index 0f4cab0e..57a12195 100644 --- a/internal/authz/manifest.yaml +++ b/internal/authz/manifest.yaml @@ -26,7 +26,9 @@ # or a BYO PDP. This file declares everything and ships the C0 role set, but does not wire # any of it into an engine on its own. The default `builtin` driver does NOT consume these # roles — it allows every authenticated non-admin request and only gates `admin` via SSO -# groups (zero behavior change). The embedded Cedar engine (BCH-1316) DOES honor this C0 role +# groups (zero behavior change), except the `agent` resource: users need the same admin +# check there, agent service accounts may only register, ingest and sync, and anonymous is +# denied (R39). The embedded Cedar engine (BCH-1316) DOES honor this C0 role # set; select it with `authz.driver: cedar` (see docs/authz-oss-cedar.md). C1/C2 stay # Enterprise/BYO. schemaVersion: 1 @@ -258,7 +260,9 @@ resources: is_active: bool is_locked: bool agent: - actions: [read, create, update, delete, register, ingest] + # configure gates writes of the agent's remote-configuration overlay; sync is the agent + # service account fetching its overlay and reporting its effective config. + actions: [read, create, update, delete, register, ingest, configure, sync] attributes: is_active: bool notification: @@ -289,6 +293,11 @@ resources: # does not consume these roles; the cedar driver does. Operators may override or extend them # in their PAP. "*" means all resources / all actions. roles: + # viewer/auditor/contributor read agent configurations through "*": [read]. This is + # intended (R40/RA-21). Operators can narrow it with a Cedar `forbid`. Instance base/effective reports are redacted, and overlays are returned + # verbatim only to callers that also hold agent:configure; other agent:read holders get + # them redacted. Secrets still belong in ${env:NAME} placeholders, not literal overlay values. + # # Full access. admin: "*": ["*"] @@ -356,7 +365,7 @@ roles: agent: evidence: [create] heartbeat: [ingest] - agent: [register, ingest] + agent: [register, ingest, sync] risk-template: [update] subject-template: [update] playback: [execute] diff --git a/internal/authz/pdp.go b/internal/authz/pdp.go index 2ae60f59..47d89fe8 100644 --- a/internal/authz/pdp.go +++ b/internal/authz/pdp.go @@ -136,4 +136,10 @@ const ( ActionTrigger = "trigger" ActionExport = "export" ActionSubscribe = "subscribe" + + // Agent remote configuration (resource agent). ActionConfigure writes an agent's + // configuration overlay; ActionSync is the agent fetching its overlay and reporting. + ActionRegister = "register" + ActionConfigure = "configure" + ActionSync = "sync" )