From 72536648f12b2977fcf2bc1a15196e061adbaa2b 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:20:00 -0300 Subject: [PATCH 1/5] feat(api): admin agent config read and save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thirteenth layer of the agent remote-configuration stack (split from #465): GET /api/admin/agents/{id}/config returns the current overlay (verbatim only to agent:configure holders, redacted otherwise, fail-closed) with an admin ETag, and GET …/config/revisions/{rev} returns one revision the same way; PUT saves a new revision with If-Match (428 without, 409 on conflict, 200 for a no-op), validating the overlay on its own and against the instances a save validates against (only errors the overlay introduces block; file-origin errors are warnings). Needs agent:configure to write. Co-Authored-By: Claude Opus 5.5 --- docs/docs.go | 288 +++++++++ docs/swagger.json | 288 +++++++++ docs/swagger.yaml | 202 +++++++ internal/api/handler/agent_config.go | 534 +++++++++++++++++ .../agent_config_admin_integration_test.go | 565 ++++++++++++++++++ internal/api/handler/agent_config_test.go | 74 +++ internal/api/handler/api.go | 7 + 7 files changed, 1958 insertions(+) create mode 100644 internal/api/handler/agent_config.go create mode 100644 internal/api/handler/agent_config_admin_integration_test.go create mode 100644 internal/api/handler/agent_config_test.go diff --git a/docs/docs.go b/docs/docs.go index 18000aeb..00e45741 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -21,6 +21,240 @@ const docTemplate = `{ "host": "{{.Host}}", "basePath": "{{.BasePath}}", "paths": { + "/admin/agents/{id}/config": { + "get": { + "description": "Returns the current configuration revision (overlay as an RFC 7396 merge patch, snake_case). Revision 0 means no overlay. The ETag is the plain revision number; send it as If-Match when saving. The overlay is verbatim for callers that also hold agent:configure; for every other caller it is redacted like an instance report (secret-like keys and values become ••••), and it is redacted whenever that check cannot be evaluated. Prefer ${env:NAME} placeholders to literal secrets.", + "produces": [ + "application/json" + ], + "tags": [ + "Agent Configuration" + ], + "summary": "Get an agent's configuration overlay", + "parameters": [ + { + "type": "string", + "description": "Agent ID", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + }, + "put": { + "description": "Creates the next configuration revision. Requires If-Match with the current revision (\"0\" for the first save): missing is 428, stale is 409 with current-revision. A semantically unchanged overlay returns 200 with the current revision and creates nothing. The overlay is validated on its own, and the merged config is validated against every fresh apply-mode instance's reported base (or the latest reported one); only errors the overlay introduces block (errors already present in the instance's own file are ignored, R59). Errors are a 422 with overlay and instances (errors plus non-blocking warnings) lists. Needs agent:configure.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Agent Configuration" + ], + "summary": "Save an agent's configuration overlay", + "parameters": [ + { + "type": "string", + "description": "Agent ID", + "name": "id", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Current revision, e.g. \\", + "name": "If-Match", + "in": "header", + "required": true + }, + { + "description": "Overlay and optional comment", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/handler.agentConfigPutRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "415": { + "description": "Unsupported Media Type", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "422": { + "description": "Unprocessable Entity", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "428": { + "description": "Precondition Required", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, + "/admin/agents/{id}/config/revisions/{rev}": { + "get": { + "description": "The overlay is verbatim for callers that also hold agent:configure and redacted (secret-like keys and values become ••••) for every other caller, as on GET config.", + "produces": [ + "application/json" + ], + "tags": [ + "Agent Configuration" + ], + "summary": "Get one configuration revision", + "parameters": [ + { + "type": "string", + "description": "Agent ID", + "name": "id", + "in": "path", + "required": true + }, + { + "type": "integer", + "description": "Revision number", + "name": "rev", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, "/admin/ai-diagnostics/runs": { "get": { "description": "Lists dashboard suggestion runs across all SSPs, newest first, with optional status and SSP filters.", @@ -36829,6 +37063,19 @@ const docTemplate = `{ } } }, + "handler.GenericDataResponse-handler_agentConfigRevisionResponse": { + "type": "object", + "properties": { + "data": { + "description": "Wrapped response data", + "allOf": [ + { + "$ref": "#/definitions/handler.agentConfigRevisionResponse" + } + ] + } + } + }, "handler.GenericDataResponse-handler_bulkControlLinkResponse": { "type": "object", "properties": { @@ -38823,6 +39070,47 @@ const docTemplate = `{ } } }, + "handler.agentConfigPutRequest": { + "type": "object", + "properties": { + "comment": { + "type": "string" + }, + "overlay": { + "type": "object" + } + } + }, + "handler.agentConfigRevisionResponse": { + "type": "object", + "properties": { + "agent-id": { + "type": "string" + }, + "comment": { + "type": "string" + }, + "created-at": { + "type": "string" + }, + "created-by": { + "type": "string" + }, + "overlay": { + "description": "omitted in lists", + "type": "object" + }, + "overlay-size": { + "type": "integer" + }, + "revert-of": { + "type": "integer" + }, + "revision": { + "type": "integer" + } + } + }, "handler.attachFilterResponsibilityRequest": { "type": "object", "required": [ diff --git a/docs/swagger.json b/docs/swagger.json index ccf3ea61..d3b3c663 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -15,6 +15,240 @@ "host": "localhost:8080", "basePath": "/api", "paths": { + "/admin/agents/{id}/config": { + "get": { + "description": "Returns the current configuration revision (overlay as an RFC 7396 merge patch, snake_case). Revision 0 means no overlay. The ETag is the plain revision number; send it as If-Match when saving. The overlay is verbatim for callers that also hold agent:configure; for every other caller it is redacted like an instance report (secret-like keys and values become ••••), and it is redacted whenever that check cannot be evaluated. Prefer ${env:NAME} placeholders to literal secrets.", + "produces": [ + "application/json" + ], + "tags": [ + "Agent Configuration" + ], + "summary": "Get an agent's configuration overlay", + "parameters": [ + { + "type": "string", + "description": "Agent ID", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + }, + "put": { + "description": "Creates the next configuration revision. Requires If-Match with the current revision (\"0\" for the first save): missing is 428, stale is 409 with current-revision. A semantically unchanged overlay returns 200 with the current revision and creates nothing. The overlay is validated on its own, and the merged config is validated against every fresh apply-mode instance's reported base (or the latest reported one); only errors the overlay introduces block (errors already present in the instance's own file are ignored, R59). Errors are a 422 with overlay and instances (errors plus non-blocking warnings) lists. Needs agent:configure.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Agent Configuration" + ], + "summary": "Save an agent's configuration overlay", + "parameters": [ + { + "type": "string", + "description": "Agent ID", + "name": "id", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Current revision, e.g. \\", + "name": "If-Match", + "in": "header", + "required": true + }, + { + "description": "Overlay and optional comment", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/handler.agentConfigPutRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "415": { + "description": "Unsupported Media Type", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "422": { + "description": "Unprocessable Entity", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "428": { + "description": "Precondition Required", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, + "/admin/agents/{id}/config/revisions/{rev}": { + "get": { + "description": "The overlay is verbatim for callers that also hold agent:configure and redacted (secret-like keys and values become ••••) for every other caller, as on GET config.", + "produces": [ + "application/json" + ], + "tags": [ + "Agent Configuration" + ], + "summary": "Get one configuration revision", + "parameters": [ + { + "type": "string", + "description": "Agent ID", + "name": "id", + "in": "path", + "required": true + }, + { + "type": "integer", + "description": "Revision number", + "name": "rev", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, "/admin/ai-diagnostics/runs": { "get": { "description": "Lists dashboard suggestion runs across all SSPs, newest first, with optional status and SSP filters.", @@ -36823,6 +37057,19 @@ } } }, + "handler.GenericDataResponse-handler_agentConfigRevisionResponse": { + "type": "object", + "properties": { + "data": { + "description": "Wrapped response data", + "allOf": [ + { + "$ref": "#/definitions/handler.agentConfigRevisionResponse" + } + ] + } + } + }, "handler.GenericDataResponse-handler_bulkControlLinkResponse": { "type": "object", "properties": { @@ -38817,6 +39064,47 @@ } } }, + "handler.agentConfigPutRequest": { + "type": "object", + "properties": { + "comment": { + "type": "string" + }, + "overlay": { + "type": "object" + } + } + }, + "handler.agentConfigRevisionResponse": { + "type": "object", + "properties": { + "agent-id": { + "type": "string" + }, + "comment": { + "type": "string" + }, + "created-at": { + "type": "string" + }, + "created-by": { + "type": "string" + }, + "overlay": { + "description": "omitted in lists", + "type": "object" + }, + "overlay-size": { + "type": "integer" + }, + "revert-of": { + "type": "integer" + }, + "revision": { + "type": "integer" + } + } + }, "handler.attachFilterResponsibilityRequest": { "type": "object", "required": [ diff --git a/docs/swagger.yaml b/docs/swagger.yaml index ab45a23d..4bab4023 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1650,6 +1650,13 @@ definitions: - $ref: '#/definitions/handler.SubscriptionsResponse' description: Wrapped response data type: object + handler.GenericDataResponse-handler_agentConfigRevisionResponse: + properties: + data: + allOf: + - $ref: '#/definitions/handler.agentConfigRevisionResponse' + description: Wrapped response data + type: object handler.GenericDataResponse-handler_bulkControlLinkResponse: properties: data: @@ -2826,6 +2833,33 @@ definitions: subject-id: type: string type: object + handler.agentConfigPutRequest: + properties: + comment: + type: string + overlay: + type: object + type: object + handler.agentConfigRevisionResponse: + properties: + agent-id: + type: string + comment: + type: string + created-at: + type: string + created-by: + type: string + overlay: + description: omitted in lists + type: object + overlay-size: + type: integer + revert-of: + type: integer + revision: + type: integer + type: object handler.attachFilterResponsibilityRequest: properties: controlId: @@ -12582,6 +12616,174 @@ info: title: Continuous Compliance Framework API version: "1" paths: + /admin/agents/{id}/config: + get: + description: Returns the current configuration revision (overlay as an RFC 7396 + merge patch, snake_case). Revision 0 means no overlay. The ETag is the plain + revision number; send it as If-Match when saving. The overlay is verbatim + for callers that also hold agent:configure; for every other caller it is redacted + like an instance report (secret-like keys and values become ••••), and it + is redacted whenever that check cannot be evaluated. Prefer ${env:NAME} placeholders + to literal secrets. + parameters: + - description: Agent ID + in: path + name: id + required: true + type: string + produces: + - application/json + responses: + "200": + description: OK + schema: + $ref: '#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/api.Error' + "403": + description: Forbidden + schema: + $ref: '#/definitions/api.Error' + "404": + description: Not Found + schema: + $ref: '#/definitions/api.Error' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/api.Error' + security: + - OAuth2Password: [] + summary: Get an agent's configuration overlay + tags: + - Agent Configuration + put: + consumes: + - application/json + description: 'Creates the next configuration revision. Requires If-Match with + the current revision ("0" for the first save): missing is 428, stale is 409 + with current-revision. A semantically unchanged overlay returns 200 with the + current revision and creates nothing. The overlay is validated on its own, + and the merged config is validated against every fresh apply-mode instance''s + reported base (or the latest reported one); only errors the overlay introduces + block (errors already present in the instance''s own file are ignored, R59). + Errors are a 422 with overlay and instances (errors plus non-blocking warnings) + lists. Needs agent:configure.' + parameters: + - description: Agent ID + in: path + name: id + required: true + type: string + - description: Current revision, e.g. \ + in: header + name: If-Match + required: true + type: string + - description: Overlay and optional comment + in: body + name: body + required: true + schema: + $ref: '#/definitions/handler.agentConfigPutRequest' + produces: + - application/json + responses: + "200": + description: OK + schema: + $ref: '#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse' + "201": + description: Created + schema: + $ref: '#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/api.Error' + "403": + description: Forbidden + schema: + $ref: '#/definitions/api.Error' + "404": + description: Not Found + schema: + $ref: '#/definitions/api.Error' + "409": + description: Conflict + schema: + $ref: '#/definitions/api.Error' + "413": + description: Request Entity Too Large + schema: + $ref: '#/definitions/api.Error' + "415": + description: Unsupported Media Type + schema: + $ref: '#/definitions/api.Error' + "422": + description: Unprocessable Entity + schema: + $ref: '#/definitions/api.Error' + "428": + description: Precondition Required + schema: + $ref: '#/definitions/api.Error' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/api.Error' + security: + - OAuth2Password: [] + summary: Save an agent's configuration overlay + tags: + - Agent Configuration + /admin/agents/{id}/config/revisions/{rev}: + get: + description: The overlay is verbatim for callers that also hold agent:configure + and redacted (secret-like keys and values become ••••) for every other caller, + as on GET config. + parameters: + - description: Agent ID + in: path + name: id + required: true + type: string + - description: Revision number + in: path + name: rev + required: true + type: integer + produces: + - application/json + responses: + "200": + description: OK + schema: + $ref: '#/definitions/handler.GenericDataResponse-handler_agentConfigRevisionResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/api.Error' + "403": + description: Forbidden + schema: + $ref: '#/definitions/api.Error' + "404": + description: Not Found + schema: + $ref: '#/definitions/api.Error' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/api.Error' + security: + - OAuth2Password: [] + summary: Get one configuration revision + tags: + - Agent Configuration /admin/ai-diagnostics/runs: get: description: Lists dashboard suggestion runs across all SSPs, newest first, diff --git a/internal/api/handler/agent_config.go b/internal/api/handler/agent_config.go new file mode 100644 index 00000000..951fedc4 --- /dev/null +++ b/internal/api/handler/agent_config.go @@ -0,0 +1,534 @@ +package handler + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "net/http" + "strconv" + "strings" + "time" + + "github.com/compliance-framework/api/internal/api" + "github.com/compliance-framework/api/internal/api/middleware" + "github.com/compliance-framework/api/internal/authn" + "github.com/compliance-framework/api/internal/authz" + "github.com/compliance-framework/api/internal/service/relational" + "github.com/compliance-framework/api/internal/service/relational/agentcfg" + "github.com/compliance-framework/api/pkg/agentconfig" + "github.com/google/uuid" + "github.com/labstack/echo/v4" + echomiddleware "github.com/labstack/echo/v4/middleware" + "go.uber.org/zap" + "gorm.io/gorm" +) + +const ( + // agentConfigBodyLimit bounds PUT/preview/revert bodies. agentconfig.MaxOverlayBytes + // (256 KiB) is measured on the compact overlay, but clients may send it pretty-printed, + // which can roughly double or triple it, inside an envelope with a comment of up to + // maxRevisionCommentLen characters (at most ~12 KiB JSON-escaped). Four times the overlay + // limit (1 MiB) leaves room for that, so an overlay is rejected by ValidateOverlay with a + // precise 422 rather than by the transport with a 413. agentConfigBodyLimitStr is the + // same limit for echo's BodyLimit middleware. + agentConfigBodyLimit = 4 * agentconfig.MaxOverlayBytes + agentConfigBodyLimitStr = "1M" + maxRevisionCommentLen = 2000 +) + +// AgentConfigHandler serves the admin routes for agent remote configuration: the current +// overlay, its revisions, preview/validation and the reporting instances. +type AgentConfigHandler struct { + sugar *zap.SugaredLogger + db *gorm.DB + svc *agentcfg.Service + // pdp decides whether a reader also holds agent:configure, which gets overlays + // unredacted. nil means nobody does (fail closed). + pdp authz.PDP +} + +func NewAgentConfigHandler(sugar *zap.SugaredLogger, db *gorm.DB, svc *agentcfg.Service, pdp authz.PDP) *AgentConfigHandler { + return &AgentConfigHandler{sugar: sugar, db: db, svc: svc, pdp: pdp} +} + +// Register mounts the routes on an /admin/agents group of their own (so they inherit no +// group guard). Writes need agent:configure. +func (h *AgentConfigHandler) Register(g *echo.Group, guard middleware.ResourceGuard) { + write := guard.Do(authz.ActionConfigure) + g.GET("/:id/config", h.Get, guard.Read()) + g.PUT("/:id/config", h.Put, write, echomiddleware.BodyLimit(agentConfigBodyLimitStr)) + g.GET("/:id/config/revisions/:rev", h.GetRevision, guard.Read()) +} + +// ---- DTOs (A4.4) ---- + +type agentConfigRevisionResponse struct { + AgentID string `json:"agent-id"` + Revision int64 `json:"revision"` + Overlay json.RawMessage `json:"overlay,omitempty" swaggertype:"object"` // omitted in lists + OverlaySize int `json:"overlay-size"` + Comment *string `json:"comment"` + CreatedBy *string `json:"created-by"` + CreatedAt *time.Time `json:"created-at"` + RevertOf *int64 `json:"revert-of"` +} + +// instanceValidationErrors groups the errors of one validated instance in a 422 body. +// Errors are the ones the overlay introduces (they block); Warnings are already present in +// Merge(base, {}), i.e. they come from the host file, and do not block (R59). +type instanceValidationErrors struct { + InstanceID string `json:"instance-id"` + Hostname *string `json:"hostname"` + Errors []agentconfig.FieldError `json:"errors"` + Warnings []agentconfig.FieldError `json:"warnings"` +} + +// agentConfigPutRequest is the PUT body. +type agentConfigPutRequest struct { + Overlay json.RawMessage `json:"overlay" swaggertype:"object"` + Comment *string `json:"comment"` +} + +// ---- Handlers ---- + +// Get godoc +// +// @Summary Get an agent's configuration overlay +// @Description Returns the current configuration revision (overlay as an RFC 7396 merge patch, snake_case). Revision 0 means no overlay. The ETag is the plain revision number; send it as If-Match when saving. The overlay is verbatim for callers that also hold agent:configure; for every other caller it is redacted like an instance report (secret-like keys and values become ••••), and it is redacted whenever that check cannot be evaluated. Prefer ${env:NAME} placeholders to literal secrets. +// @Tags Agent Configuration +// @Produce json +// @Param id path string true "Agent ID" +// @Success 200 {object} handler.GenericDataResponse[handler.agentConfigRevisionResponse] +// @Failure 400 {object} api.Error +// @Failure 403 {object} api.Error +// @Failure 404 {object} api.Error +// @Failure 500 {object} api.Error +// @Security OAuth2Password +// @Router /admin/agents/{id}/config [get] +func (h *AgentConfigHandler) Get(ctx echo.Context) error { + agent, errResp := h.resolveAgent(ctx) + if agent == nil { + return errResp + } + cur, err := h.svc.Current(ctx.Request().Context(), *agent.ID) + if err != nil { + return h.internalError(ctx, "load agent configuration", err) + } + resp := revisionResponse(*agent.ID, cur, true) + if err := h.redactForReader(ctx, &resp); err != nil { + return h.internalError(ctx, "redact overlay", err) + } + ctx.Response().Header().Set(headerETag, agentconfig.AdminETag(resp.Revision)) + return ctx.JSON(http.StatusOK, GenericDataResponse[agentConfigRevisionResponse]{Data: resp}) +} + +// Put godoc +// +// @Summary Save an agent's configuration overlay +// @Description Creates the next configuration revision. Requires If-Match with the current revision ("0" for the first save): missing is 428, stale is 409 with current-revision. A semantically unchanged overlay returns 200 with the current revision and creates nothing. The overlay is validated on its own, and the merged config is validated against every fresh apply-mode instance's reported base (or the latest reported one); only errors the overlay introduces block (errors already present in the instance's own file are ignored, R59). Errors are a 422 with overlay and instances (errors plus non-blocking warnings) lists. Needs agent:configure. +// @Tags Agent Configuration +// @Accept json +// @Produce json +// @Param id path string true "Agent ID" +// @Param If-Match header string true "Current revision, e.g. \"7\"" +// @Param body body handler.agentConfigPutRequest true "Overlay and optional comment" +// @Success 200 {object} handler.GenericDataResponse[handler.agentConfigRevisionResponse] +// @Success 201 {object} handler.GenericDataResponse[handler.agentConfigRevisionResponse] +// @Failure 400 {object} api.Error +// @Failure 403 {object} api.Error +// @Failure 404 {object} api.Error +// @Failure 409 {object} api.Error +// @Failure 413 {object} api.Error +// @Failure 415 {object} api.Error +// @Failure 422 {object} api.Error +// @Failure 428 {object} api.Error +// @Failure 500 {object} api.Error +// @Security OAuth2Password +// @Router /admin/agents/{id}/config [put] +func (h *AgentConfigHandler) Put(ctx echo.Context) error { + agent, errResp := h.resolveAgent(ctx) + if agent == nil { + return errResp + } + expected, ok := agentconfig.ParseRevisionIfMatch(ctx.Request().Header.Get(headerIfMatch)) + if !ok { + return preconditionRequired(ctx) + } + body, bodyErr := readJSONBody(ctx, agentConfigBodyLimit) + if bodyErr != nil { + return bodyErr.respond(ctx) + } + var req agentConfigPutRequest + if err := decodeStrict(body, &req); err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + if isNullOrEmpty(req.Overlay) { + return ctx.JSON(http.StatusBadRequest, api.NewError(errors.New("overlay is required"))) + } + comment, err := normalizeComment(req.Comment) + if err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + return h.save(ctx, agent, expected, req.Overlay, comment, nil) +} + +// save implements PUT/revert steps 3-7 (A4.3). +func (h *AgentConfigHandler) save(ctx echo.Context, agent *relational.Agent, expected int64, overlay json.RawMessage, comment *string, revertOf *int64) error { + reqCtx := ctx.Request().Context() + agentID := *agent.ID + + cur, err := h.svc.Current(reqCtx, agentID) + if err != nil { + return h.internalError(ctx, "load agent configuration", err) + } + curRev, curOverlay := int64(0), json.RawMessage(`{}`) + if cur != nil { + curRev, curOverlay = cur.Revision, json.RawMessage(cur.Overlay) + } + if expected != curRev { + return revisionConflict(ctx, curRev) + } + + // R14: a semantically unchanged overlay creates no revision. Only a valid JSON object + // can be a no-op; anything else falls through to validation. + if diff, err := agentconfig.DiffJSON(curOverlay, overlay); err == nil && len(diff) == 0 { + ctx.Response().Header().Set(headerETag, agentconfig.AdminETag(curRev)) + return ctx.JSON(http.StatusOK, GenericDataResponse[agentConfigRevisionResponse]{Data: revisionResponse(agentID, cur, true)}) + } + + bases, _, err := h.svc.ValidationBases(reqCtx, agentID) + if err != nil { + return h.internalError(ctx, "load validation bases", err) + } + + result := validateCandidate(overlay, bases) + if result.blocking() { + return ctx.JSON(http.StatusUnprocessableEntity, result.errorBody()) + } + + compact, err := compactJSON(overlay) + if err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + createdBy, createdByID := revisionAuthor(ctx) + rev, err := h.svc.CreateRevision(reqCtx, agentcfg.CreateRevisionParams{ + AgentID: agentID, + ExpectedRevision: expected, + Overlay: compact, + Comment: comment, + CreatedBy: createdBy, + CreatedByID: createdByID, + RevertOf: revertOf, + }) + if err != nil { + var conflict *agentcfg.RevisionConflictError + switch { + case errors.As(err, &conflict): + return revisionConflict(ctx, conflict.Current) + case errors.Is(err, agentcfg.ErrNotFound): + return ctx.JSON(http.StatusNotFound, api.NotFound()) + default: + return h.internalError(ctx, "create revision", err) + } + } + ctx.Response().Header().Set(headerETag, agentconfig.AdminETag(rev.Revision)) + return ctx.JSON(http.StatusCreated, GenericDataResponse[agentConfigRevisionResponse]{Data: revisionResponse(agentID, rev, true)}) +} + +// candidateResult is the outcome of the candidate validation pipeline (A4.2). +type candidateResult struct { + overlay []agentconfig.FieldError + instances []instanceValidationErrors +} + +// blocking reports whether a save must be refused (422): overlay errors, or errors the +// overlay introduces on a validated instance (R6, R48, R59). File-origin errors never block. +func (r candidateResult) blocking() bool { + return len(r.overlay) > 0 || len(r.instances) > 0 +} + +func (r candidateResult) errorBody() api.Error { + return api.Error{Errors: map[string]any{ + "body": "configuration overlay is invalid", + "overlay": nonNil(r.overlay), + "instances": nonNil(r.instances), + }} +} + +// validateCandidate runs the pipeline shared by PUT, revert and preview: +// 1. ValidateOverlay (the overlay on its own); +// 2. when (1) passed: Merge(base, overlay).ValidateEditable() for every validation base, +// grouped by instance. Only errors the overlay introduces are kept (R59, see +// splitIntroduced); an instance is listed only when it has at least one. With no bases +// (standalone) only (1) runs. +func validateCandidate(overlay json.RawMessage, bases []agentcfg.InstanceBase) candidateResult { + var r candidateResult + if err := agentconfig.ValidateOverlay(overlay); err != nil { + var verrs agentconfig.ValidationErrors + if errors.As(err, &verrs) { + r.overlay = verrs + } else { + r.overlay = []agentconfig.FieldError{{Path: "", Code: agentconfig.FieldCodeParse, Message: err.Error()}} + } + } + if len(r.overlay) > 0 { + return r + } + for _, b := range bases { + if _, introduced, fileOrigin := splitIntroduced(b.Base, overlay); len(introduced) > 0 { + r.instances = append(r.instances, instanceValidationErrors{ + InstanceID: b.Instance.InstanceID.String(), + Hostname: b.Instance.Hostname, + Errors: introduced, + Warnings: nonNil(fileOrigin), + }) + } + } + return r +} + +// splitIntroduced validates Merge(base, overlay) and splits its errors into the ones the +// overlay introduces and the ones already present in Merge(base, {}) (file-origin, R59). +// Errors are matched on (Path, Code, Message), so an overlay that replaces a bad value +// with a different bad value still introduces an error. +func splitIntroduced(base agentconfig.Config, overlay json.RawMessage) (eff *agentconfig.Config, introduced, fileOrigin []agentconfig.FieldError) { + eff, all := mergeAndValidate(base, overlay) + if len(all) == 0 { + return eff, nil, nil + } + _, baseline := mergeAndValidate(base, json.RawMessage(`{}`)) + type key struct{ path, code, message string } + seen := make(map[key]bool, len(baseline)) + for _, e := range baseline { + seen[key{e.Path, e.Code, e.Message}] = true + } + for _, e := range all { + if seen[key{e.Path, e.Code, e.Message}] { + fileOrigin = append(fileOrigin, e) + } else { + introduced = append(introduced, e) + } + } + return eff, introduced, fileOrigin +} + +// mergeAndValidate merges the overlay onto a base and validates the editable part. +func mergeAndValidate(base agentconfig.Config, overlay json.RawMessage) (*agentconfig.Config, []agentconfig.FieldError) { + eff, err := agentconfig.Merge(base, overlay) + if err != nil { + return nil, []agentconfig.FieldError{{Path: "", Code: agentconfig.FieldCodeParse, Message: err.Error()}} + } + if err := eff.ValidateEditable(); err != nil { + var verrs agentconfig.ValidationErrors + if errors.As(err, &verrs) { + return &eff, verrs + } + return &eff, []agentconfig.FieldError{{Path: "", Code: agentconfig.FieldCodeInvalidValue, Message: err.Error()}} + } + return &eff, nil +} + +// GetRevision godoc +// +// @Summary Get one configuration revision +// @Description The overlay is verbatim for callers that also hold agent:configure and redacted (secret-like keys and values become ••••) for every other caller, as on GET config. +// @Tags Agent Configuration +// @Produce json +// @Param id path string true "Agent ID" +// @Param rev path integer true "Revision number" +// @Success 200 {object} handler.GenericDataResponse[handler.agentConfigRevisionResponse] +// @Failure 400 {object} api.Error +// @Failure 403 {object} api.Error +// @Failure 404 {object} api.Error +// @Failure 500 {object} api.Error +// @Security OAuth2Password +// @Router /admin/agents/{id}/config/revisions/{rev} [get] +func (h *AgentConfigHandler) GetRevision(ctx echo.Context) error { + agent, errResp := h.resolveAgent(ctx) + if agent == nil { + return errResp + } + revNumber, err := parseRevisionParam(ctx.Param("rev")) + if err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + rev, err := h.svc.GetRevision(ctx.Request().Context(), *agent.ID, revNumber) + if errors.Is(err, agentcfg.ErrNotFound) { + return ctx.JSON(http.StatusNotFound, api.NotFoundCustomMsg("revision not found")) + } + if err != nil { + return h.internalError(ctx, "load revision", err) + } + resp := revisionResponse(*agent.ID, rev, true) + if err := h.redactForReader(ctx, &resp); err != nil { + return h.internalError(ctx, "redact overlay", err) + } + return ctx.JSON(http.StatusOK, GenericDataResponse[agentConfigRevisionResponse]{Data: resp}) +} + +// ---- helpers ---- + +// resolveAgent loads :id. On failure it returns nil and the already-written error response. +func (h *AgentConfigHandler) resolveAgent(ctx echo.Context) (*relational.Agent, error) { + id, err := uuid.Parse(ctx.Param("id")) + if err != nil { + return nil, ctx.JSON(http.StatusBadRequest, api.InvalidUUID()) + } + var agent relational.Agent + if err := h.db.WithContext(ctx.Request().Context()).First(&agent, "id = ?", id).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, ctx.JSON(http.StatusNotFound, api.NotFoundCustomMsg("agent not found")) + } + return nil, h.internalError(ctx, "load agent", err) + } + return &agent, nil +} + +// canConfigure reports whether the caller holds agent:configure on the agent in :id, +// evaluated against the PDP like the route guard does. It fails closed: no PDP, an +// unavailable PDP or an evaluation error all count as no. +func (h *AgentConfigHandler) canConfigure(ctx echo.Context) bool { + if h.pdp == nil { + return false + } + subject := middleware.SubjectFromContext(ctx) + resource := authz.Resource{Type: authz.ResourceAgent, ID: ctx.Param("id")} + reqCtx := map[string]any{"method": ctx.Request().Method, "path": ctx.Path()} + decision, err := h.pdp.Evaluate(ctx.Request().Context(), subject, authz.ActionConfigure, resource, reqCtx) + if err != nil { + h.sugar.Warnw("agent:configure check failed; returning the overlay redacted", "path", ctx.Path(), "error", err) + return false + } + return decision.Allow +} + +// redactForReader redacts resp.Overlay (agentconfig.RedactDocument) unless the caller holds +// agent:configure, so plain readers never see literal secrets typed into an overlay. +// OverlaySize stays the stored size. +func (h *AgentConfigHandler) redactForReader(ctx echo.Context, resp *agentConfigRevisionResponse) error { + if len(resp.Overlay) == 0 || h.canConfigure(ctx) { + return nil + } + redacted, _, err := agentconfig.RedactDocument(resp.Overlay) + if err != nil { + return err + } + resp.Overlay = redacted + return nil +} + +func (h *AgentConfigHandler) internalError(ctx echo.Context, what string, err error) error { + h.sugar.Errorw("Agent configuration request failed", "operation", what, "path", ctx.Path(), "error", err) + return ctx.JSON(http.StatusInternalServerError, api.InternalServerError()) +} + +func revisionResponse(agentID uuid.UUID, rev *relational.AgentConfigRevision, withOverlay bool) agentConfigRevisionResponse { + if rev == nil { + resp := agentConfigRevisionResponse{AgentID: agentID.String(), Revision: 0, OverlaySize: 2} + if withOverlay { + resp.Overlay = json.RawMessage(`{}`) + } + return resp + } + createdAt := rev.CreatedAt.UTC() + createdBy := rev.CreatedBy + resp := agentConfigRevisionResponse{ + AgentID: agentID.String(), + Revision: rev.Revision, + OverlaySize: len(rev.Overlay), + Comment: rev.Comment, + CreatedBy: &createdBy, + CreatedAt: &createdAt, + RevertOf: rev.RevertOf, + } + if withOverlay { + resp.Overlay = json.RawMessage(rev.Overlay) + } + return resp +} + +func preconditionRequired(ctx echo.Context) error { + return ctx.JSON(http.StatusPreconditionRequired, api.NewError(errors.New("If-Match header with the current revision is required"))) +} + +func revisionConflict(ctx echo.Context, current int64) error { + return ctx.JSON(http.StatusConflict, api.Error{Errors: map[string]any{ + "body": agentcfg.ErrRevisionConflict.Error(), + "current-revision": current, + }}) +} + +// revisionAuthor returns the user subject (email) and user_uuid claim of the caller. +func revisionAuthor(ctx echo.Context) (string, *uuid.UUID) { + claims, ok := ctx.Get("user").(*authn.UserClaims) + if !ok || claims == nil { + return "", nil + } + var id *uuid.UUID + if claims.UserUUID != "" { + if parsed, err := uuid.Parse(claims.UserUUID); err == nil { + id = &parsed + } + } + return claims.Subject, id +} + +// decodeStrict decodes a request envelope, rejecting unknown keys and trailing data. +func decodeStrict(body []byte, dst any) error { + if len(bytes.TrimSpace(body)) == 0 { + return errors.New("request body is required") + } + dec := json.NewDecoder(bytes.NewReader(body)) + dec.DisallowUnknownFields() + if err := dec.Decode(dst); err != nil { + return fmt.Errorf("invalid request body: %w", err) + } + if dec.More() { + return errors.New("invalid request body: unexpected data after the JSON object") + } + return nil +} + +func normalizeComment(c *string) (*string, error) { + if c == nil { + return nil, nil + } + trimmed := strings.TrimSpace(*c) + if trimmed == "" { + return nil, nil + } + if len([]rune(trimmed)) > maxRevisionCommentLen { + return nil, fmt.Errorf("comment must be at most %d characters", maxRevisionCommentLen) + } + return &trimmed, nil +} + +func parseRevisionParam(s string) (int64, error) { + rev, err := strconv.ParseInt(s, 10, 64) + if err != nil || rev < 1 { + return 0, errors.New("revision must be a positive integer") + } + return rev, nil +} + +func isNullOrEmpty(raw json.RawMessage) bool { + t := bytes.TrimSpace(raw) + return len(t) == 0 || bytes.Equal(t, []byte("null")) +} + +func compactJSON(raw json.RawMessage) (json.RawMessage, error) { + var buf bytes.Buffer + if err := json.Compact(&buf, raw); err != nil { + return nil, fmt.Errorf("overlay is not valid JSON: %w", err) + } + return buf.Bytes(), nil +} + +// nonNil returns an empty (non-nil) slice for nil, so JSON renders [] rather than null. +func nonNil[T any](s []T) []T { + if s == nil { + return []T{} + } + return s +} diff --git a/internal/api/handler/agent_config_admin_integration_test.go b/internal/api/handler/agent_config_admin_integration_test.go new file mode 100644 index 00000000..be512d5a --- /dev/null +++ b/internal/api/handler/agent_config_admin_integration_test.go @@ -0,0 +1,565 @@ +//go:build integration + +package handler + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/compliance-framework/api/internal/api" + "github.com/compliance-framework/api/internal/api/middleware" + "github.com/compliance-framework/api/internal/authn" + "github.com/compliance-framework/api/internal/authz" + "github.com/compliance-framework/api/internal/service/relational" + "github.com/compliance-framework/api/internal/service/relational/agentcfg" + "github.com/compliance-framework/api/internal/tests" + "github.com/compliance-framework/api/pkg/agentconfig" + "github.com/google/uuid" + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/suite" + "go.uber.org/zap" +) + +// Integration tests for the admin agent-configuration routes (LLD A4) and the re-guarded +// /admin/agents routes (A2.7, R40), against Postgres. + +const ( + acaVendorPlugin = "ghcr.io/vendor/ssh:v1" + acaVendorPolicy = "ghcr.io/vendor/ssh-policies:v1" + acaDigest = "sha256:0000000000000000000000000000000000000000000000000000000000000000" + acaOtherDigest = "sha256:1111111111111111111111111111111111111111111111111111111111111111" +) + +func TestAgentConfigAdminAPI(t *testing.T) { + suite.Run(t, new(AgentConfigAdminIntegrationSuite)) +} + +type AgentConfigAdminIntegrationSuite struct { + tests.IntegrationTestSuite + logger *zap.SugaredLogger + server *api.Server // builtin PDP + svc *agentcfg.Service + agent *relational.Agent + token string // the suite's dummy (password) user: admin under builtin +} + +func (s *AgentConfigAdminIntegrationSuite) SetupSuite() { + s.IntegrationTestSuite.SetupSuite() + s.logger = zap.NewNop().Sugar() +} + +func (s *AgentConfigAdminIntegrationSuite) SetupTest() { + s.Require().NoError(s.Migrator.Refresh()) + s.server = s.newServer(nil) + s.svc = agentcfg.NewService(s.DB, agentcfg.Settings{}, nil) + agent, err := s.CreateAgent("config-agent") + s.Require().NoError(err) + s.agent = agent + token, err := s.GetAuthToken() + s.Require().NoError(err) + s.token = *token +} + +// newServer builds an API server; a nil PEP means the builtin PDP (RegisterHandlers' default). +func (s *AgentConfigAdminIntegrationSuite) newServer(pep *middleware.PEP) *api.Server { + metrics := api.NewMetricsHandler(context.Background(), s.logger) + srv := api.NewServer(context.Background(), s.logger, s.Config, metrics) + RegisterHandlers(srv, s.logger, s.DB, s.Config, &APIServices{PEP: pep}) + return srv +} + +// cedarServer builds a server enforcing through the embedded Cedar PDP. Roles are read from +// ccf_role_assignments behind a per-PDP cache, so create the role rows first. +func (s *AgentConfigAdminIntegrationSuite) cedarServer() *api.Server { + pdp, err := authz.Open(authz.Options{Driver: authz.DriverCedar}, authz.Deps{DB: s.DB, Config: s.Config, Logger: s.logger}) + s.Require().NoError(err) + return s.newServer(middleware.NewPEP(pdp, authz.FailClosed, s.logger)) +} + +// userToken creates a user (optionally with a manual role row) and returns a signed JWT. +func (s *AgentConfigAdminIntegrationSuite) userToken(email, authMethod, role string) (relational.User, string) { + user := relational.User{Email: email, FirstName: "Test", LastName: "User", AuthMethod: authMethod} + s.Require().NoError(s.DB.Create(&user).Error) + if role != "" { + s.Require().NoError(s.DB.Create(&relational.CCFRoleAssignment{ + RoleName: role, + AssigneeType: relational.RoleAssigneeTypeUser, + AssigneeID: relational.NormalizeAssigneeID(email), + Source: relational.RoleAssignmentSourceManual, + }).Error) + } + token, err := authn.GenerateJWTToken(&user, s.Config.JWTPrivateKey) + s.Require().NoError(err) + return user, *token +} + +// ---- request helpers ---- + +// send serves one request. body is sent raw (nil = no body); headers are key/value pairs and +// override the default Content-Type (application/json whenever a body is sent). +func (s *AgentConfigAdminIntegrationSuite) send(srv *api.Server, token, method, path string, body []byte, headers ...string) *httptest.ResponseRecorder { + s.Require().Zero(len(headers)%2, "headers must be key/value pairs") + var req *http.Request + if body != nil { + req = httptest.NewRequest(method, path, bytes.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + } else { + req = httptest.NewRequest(method, path, nil) + } + if token != "" { + req.Header.Set(echo.HeaderAuthorization, "Bearer "+token) + } + for i := 0; i < len(headers); i += 2 { + req.Header.Set(headers[i], headers[i+1]) + } + rec := httptest.NewRecorder() + srv.E().ServeHTTP(rec, req) + return rec +} + +// call sends as the dummy user through the builtin server. +func (s *AgentConfigAdminIntegrationSuite) call(method, path string, body []byte, headers ...string) *httptest.ResponseRecorder { + return s.send(s.server, s.token, method, path, body, headers...) +} + +func (s *AgentConfigAdminIntegrationSuite) agentPath(agentID uuid.UUID, suffix string) string { + return "/api/admin/agents/" + agentID.String() + suffix +} + +func (s *AgentConfigAdminIntegrationSuite) path(suffix string) string { + return s.agentPath(*s.agent.ID, suffix) +} + +// putBody wraps a raw overlay in the PUT envelope. +func acaPutBody(overlay string) []byte { + return []byte(`{"overlay":` + overlay + `}`) +} + +// put saves overlay through srv as token; ifMatch "" omits the header. +func (s *AgentConfigAdminIntegrationSuite) put(srv *api.Server, token, ifMatch, overlay string) *httptest.ResponseRecorder { + var headers []string + if ifMatch != "" { + headers = []string{"If-Match", ifMatch} + } + return s.send(srv, token, http.MethodPut, s.path("/config"), acaPutBody(overlay), headers...) +} + +// save PUTs overlay as the dummy user and requires a 201 with the expected revision. +func (s *AgentConfigAdminIntegrationSuite) save(ifMatch, overlay string, wantRev int64) { + rec := s.put(s.server, s.token, ifMatch, overlay) + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) + s.Require().Equal(agentconfig.AdminETag(wantRev), rec.Header().Get("ETag")) +} + +type acaErrors struct { + Errors map[string]json.RawMessage `json:"errors"` +} + +func (s *AgentConfigAdminIntegrationSuite) errorsOf(rec *httptest.ResponseRecorder) map[string]json.RawMessage { + var body acaErrors + s.Require().NoError(json.Unmarshal(rec.Body.Bytes(), &body), rec.Body.String()) + s.Require().NotNil(body.Errors, rec.Body.String()) + return body.Errors +} + +func (s *AgentConfigAdminIntegrationSuite) errorBody(rec *httptest.ResponseRecorder) string { + var msg string + s.Require().NoError(json.Unmarshal(s.errorsOf(rec)["body"], &msg), rec.Body.String()) + return msg +} + +// validationErrors decodes a 422 body. +type acaValidationErrors struct { + Body string `json:"body"` + Overlay []agentconfig.FieldError `json:"overlay"` + Instances []instanceValidationErrors `json:"instances"` +} + +func (s *AgentConfigAdminIntegrationSuite) unprocessable(rec *httptest.ResponseRecorder) acaValidationErrors { + s.Require().Equal(http.StatusUnprocessableEntity, rec.Code, rec.Body.String()) + var body struct { + Errors acaValidationErrors `json:"errors"` + } + s.Require().NoError(json.Unmarshal(rec.Body.Bytes(), &body), rec.Body.String()) + s.Equal("configuration overlay is invalid", body.Errors.Body) + // Slices are always present ([]), never null. + raw := s.errorsOf(rec) + for _, k := range []string{"overlay", "instances"} { + s.True(strings.HasPrefix(string(raw[k]), "["), "%s must be a JSON array: %s", k, raw[k]) + } + return body.Errors +} + +func (s *AgentConfigAdminIntegrationSuite) revisionCount(agentID uuid.UUID) int64 { + var n int64 + s.Require().NoError(s.DB.Model(&relational.AgentConfigRevision{}).Where("agent_id = ?", agentID).Count(&n).Error) + return n +} + +func acaData[T any](s *AgentConfigAdminIntegrationSuite, rec *httptest.ResponseRecorder) T { + var out GenericDataResponse[T] + s.Require().NoError(json.Unmarshal(rec.Body.Bytes(), &out), rec.Body.String()) + return out.Data +} + +// ---- instance helpers ---- + +// acaBase returns a reported base (redacted as the agent does: Redact clears the client secret) in the given mode with the vendor ssh plugin +// plus any extra plugins. +func acaBase(mode string, extra map[string]*agentconfig.Plugin) json.RawMessage { + schedule := "* * * * *" + plugins := map[string]*agentconfig.Plugin{ + "ssh": { + Source: acaVendorPlugin, + Schedule: &schedule, + Policies: []string{acaVendorPolicy}, + Config: map[string]string{"host": "localhost"}, + }, + } + for k, v := range extra { + plugins[k] = v + } + cfg := agentconfig.Config{ + Daemon: true, + API: &agentconfig.APIConfig{ + URL: "http://api:8080", + Auth: &agentconfig.APIAuth{ClientID: uuid.NewString()}, // redacted: no client secret + }, + RemoteConfig: &agentconfig.RemoteConfig{Mode: mode}, + Plugins: plugins, + } + raw, err := json.Marshal(cfg) + if err != nil { + panic(err) + } + return raw +} + +// report stores a config report for a new instance of agentID and returns its instance id. +func (s *AgentConfigAdminIntegrationSuite) report(agentID uuid.UUID, mode string, mutate func(*agentconfig.Report)) uuid.UUID { + instanceID := uuid.New() + base := acaBase(mode, nil) + r := agentconfig.Report{ + Hostname: "host-" + instanceID.String()[:8], + AgentVersion: "v1.0.0", + Mode: mode, + Daemon: true, + Status: agentconfig.StatusApplied, + Base: base, + Effective: base, + EffectiveDigest: acaDigest, + RemoteConfig: &agentconfig.RemoteConfig{Mode: mode}, + } + if mode == agentconfig.ModeReport { + r.Status = agentconfig.StatusNotApplicable + } + if mutate != nil { + mutate(&r) + } + s.Require().NoError(s.svc.UpsertReport(context.Background(), agentID, nil, instanceID, r)) + return instanceID +} + +// ---- GET /config ---- + +func (s *AgentConfigAdminIntegrationSuite) TestGetConfigRevisionZero() { + rec := s.call(http.MethodGet, s.path("/config"), nil) + s.Require().Equal(http.StatusOK, rec.Code, rec.Body.String()) + s.Equal(`"0"`, rec.Header().Get("ETag")) + + var body struct { + Data map[string]json.RawMessage `json:"data"` + } + s.Require().NoError(json.Unmarshal(rec.Body.Bytes(), &body)) + s.JSONEq(`"`+s.agent.ID.String()+`"`, string(body.Data["agent-id"])) + s.JSONEq(`0`, string(body.Data["revision"])) + s.JSONEq(`{}`, string(body.Data["overlay"])) + s.JSONEq(`null`, string(body.Data["comment"])) + s.JSONEq(`null`, string(body.Data["created-at"])) + s.JSONEq(`null`, string(body.Data["revert-of"])) +} + +func (s *AgentConfigAdminIntegrationSuite) TestGetConfigAfterSave() { + overlay := `{"verbosity":2,"plugins":{"ssh":{"labels":{"env":"prod"}}}}` + s.save(`"0"`, overlay, 1) + + rec := s.call(http.MethodGet, s.path("/config"), nil) + s.Require().Equal(http.StatusOK, rec.Code, rec.Body.String()) + s.Equal(`"1"`, rec.Header().Get("ETag")) + got := acaData[agentConfigRevisionResponse](s, rec) + s.Equal(int64(1), got.Revision) + s.JSONEq(overlay, string(got.Overlay)) + s.Require().NotNil(got.CreatedBy) + s.Equal("dummy@example.com", *got.CreatedBy) +} + +func (s *AgentConfigAdminIntegrationSuite) TestGetConfigBadAndUnknownAgent() { + rec := s.call(http.MethodGet, "/api/admin/agents/not-a-uuid/config", nil) + s.Equal(http.StatusBadRequest, rec.Code, rec.Body.String()) + + rec = s.call(http.MethodGet, s.agentPath(uuid.New(), "/config"), nil) + s.Equal(http.StatusNotFound, rec.Code, rec.Body.String()) +} + +// ---- PUT /config: revisions and concurrency ---- + +func (s *AgentConfigAdminIntegrationSuite) TestPutRevisionFlow() { + // No If-Match => 428. + rec := s.put(s.server, s.token, "", `{"verbosity":1}`) + s.Require().Equal(http.StatusPreconditionRequired, rec.Code, rec.Body.String()) + s.Equal("If-Match header with the current revision is required", s.errorBody(rec)) + + // First save with "0" => 201 + ETag "1". + rec = s.put(s.server, s.token, `"0"`, `{"verbosity":1}`) + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) + s.Equal(`"1"`, rec.Header().Get("ETag")) + created := acaData[agentConfigRevisionResponse](s, rec) + s.Equal(int64(1), created.Revision) + s.JSONEq(`{"verbosity":1}`, string(created.Overlay)) + + // Same (stale) If-Match with a different overlay => 409 with the current revision. + rec = s.put(s.server, s.token, `"0"`, `{"verbosity":2}`) + s.Require().Equal(http.StatusConflict, rec.Code, rec.Body.String()) + s.JSONEq(`{"errors":{"body":"configuration revision conflict","current-revision":1}}`, rec.Body.String()) + + // Semantically identical overlay (different whitespace) with the current If-Match => 200, no row. + rec = s.put(s.server, s.token, `"1"`, `{ "verbosity" : 1 }`) + s.Require().Equal(http.StatusOK, rec.Code, rec.Body.String()) + s.Equal(`"1"`, rec.Header().Get("ETag")) + s.Equal(int64(1), acaData[agentConfigRevisionResponse](s, rec).Revision) + s.Equal(int64(1), s.revisionCount(*s.agent.ID)) + + // Weak and bare If-Match forms are accepted. + rec = s.put(s.server, s.token, `W/"1"`, `{"verbosity":2}`) + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) + rec = s.put(s.server, s.token, `2`, `{"verbosity":0}`) + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) + s.Equal(int64(3), s.revisionCount(*s.agent.ID)) + + // If-Match "*" and lists are not a revision => 428. + rec = s.put(s.server, s.token, `*`, `{"verbosity":1}`) + s.Equal(http.StatusPreconditionRequired, rec.Code, rec.Body.String()) +} + +func (s *AgentConfigAdminIntegrationSuite) TestPutWithComment() { + body := []byte(`{"overlay":{"verbosity":1},"comment":" raise verbosity "}`) + rec := s.call(http.MethodPut, s.path("/config"), body, "If-Match", `"0"`) + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) + got := acaData[agentConfigRevisionResponse](s, rec) + s.Require().NotNil(got.Comment) + s.Equal("raise verbosity", *got.Comment) +} + +// ---- PUT /config: overlay-level validation (422) ---- + +func (s *AgentConfigAdminIntegrationSuite) TestPutOverlayValidationErrors() { + cases := []struct { + name, overlay, path, code string + }{ + {"locked key", `{"api":{}}`, "/api", agentconfig.FieldCodeLockedKey}, + {"non-string config value", `{"plugins":{"x":{"source":"ghcr.io/x/x:v1","config":{"port":2222}}}}`, "/plugins/x/config/port", agentconfig.FieldCodeInvalidType}, + {"unknown field", `{"foo":true}`, "/foo", agentconfig.FieldCodeUnknownField}, + {"policy_bundles is not a field", `{"policy_bundles":{"banner":{"modules":{}}}}`, "/policy_bundles", agentconfig.FieldCodeUnknownField}, + {"masked value", `{"plugins":{"ssh":{"config":{"password":"••••"}}}}`, "/plugins/ssh/config/password", agentconfig.FieldCodeMaskedValue}, + } + for _, tc := range cases { + s.Run(tc.name, func() { + errs := s.unprocessable(s.put(s.server, s.token, `"0"`, tc.overlay)) + s.Require().NotEmpty(errs.Overlay) + s.Equal(tc.path, errs.Overlay[0].Path) + s.Equal(tc.code, errs.Overlay[0].Code) + s.Empty(errs.Instances) + }) + } + s.Equal(int64(0), s.revisionCount(*s.agent.ID)) +} + +// ---- PUT /config: validation against instance bases (R14, R48) ---- + +func (s *AgentConfigAdminIntegrationSuite) TestPutValidatesAgainstFreshInstances() { + fresh := s.report(*s.agent.ID, agentconfig.ModeApplySafe, nil) + // A report-mode instance is never validated against. + s.report(*s.agent.ID, agentconfig.ModeReport, nil) + + // A schedule-only patch of a plugin present in the fresh base => 201. + s.save(`"0"`, `{"plugins":{"ssh":{"schedule":"*/5 * * * *"}}}`, 1) + + // A new plugin without a source makes the merged config invalid for the fresh instance. + errs := s.unprocessable(s.put(s.server, s.token, `"1"`, `{"plugins":{"newp":{"schedule":"* * * * *"}}}`)) + s.Empty(errs.Overlay) + s.Require().Len(errs.Instances, 1) + s.Equal(fresh.String(), errs.Instances[0].InstanceID) + s.Require().NotNil(errs.Instances[0].Hostname) + s.Require().NotEmpty(errs.Instances[0].Errors) + s.Equal("/plugins/newp/source", errs.Instances[0].Errors[0].Path) + s.Equal(agentconfig.FieldCodeRequired, errs.Instances[0].Errors[0].Code) +} + +func (s *AgentConfigAdminIntegrationSuite) TestPutStandaloneWithoutInstances() { + // With no reporting instance only overlay-level checks run, so a plugin without a source + // saves (it cannot be merged against anything). + s.save(`"0"`, `{"plugins":{"newp":{"schedule":"* * * * *"}}}`, 1) +} + +// ---- PUT /config: request body handling ---- + +func (s *AgentConfigAdminIntegrationSuite) TestPutBodyHandling() { + // Over the body limit => 413. + big := []byte(`{"overlay":{"verbosity":1},"comment":"` + strings.Repeat("a", agentConfigBodyLimit) + `"}`) + rec := s.call(http.MethodPut, s.path("/config"), big, "If-Match", `"0"`) + s.Equal(http.StatusRequestEntityTooLarge, rec.Code) + + // An overlay over MaxOverlayBytes but within the body limit reaches validation => 422. + oversized := fmt.Sprintf(`{"overlay":{"plugins":{"ssh":{"labels":{"a":%q}}}}}`, strings.Repeat("v", agentconfig.MaxOverlayBytes)) + errs := s.unprocessable(s.call(http.MethodPut, s.path("/config"), []byte(oversized), "If-Match", `"0"`)) + s.Require().NotEmpty(errs.Overlay) + s.Equal(agentconfig.FieldCodeSize, errs.Overlay[0].Code) + + // A non-JSON media type => 415. + rec = s.call(http.MethodPut, s.path("/config"), acaPutBody(`{"verbosity":1}`), "If-Match", `"0"`, echo.HeaderContentType, "text/plain") + s.Equal(http.StatusUnsupportedMediaType, rec.Code, rec.Body.String()) + + // Unknown envelope key => 400. + rec = s.call(http.MethodPut, s.path("/config"), []byte(`{"overlay":{"verbosity":1},"note":"x"}`), "If-Match", `"0"`) + s.Equal(http.StatusBadRequest, rec.Code, rec.Body.String()) + + // Missing overlay => 400. + rec = s.call(http.MethodPut, s.path("/config"), []byte(`{"comment":"x"}`), "If-Match", `"0"`) + s.Equal(http.StatusBadRequest, rec.Code, rec.Body.String()) + + // Comment over 2000 characters => 400. + long := fmt.Sprintf(`{"overlay":{"verbosity":1},"comment":%q}`, strings.Repeat("é", maxRevisionCommentLen+1)) + rec = s.call(http.MethodPut, s.path("/config"), []byte(long), "If-Match", `"0"`) + s.Equal(http.StatusBadRequest, rec.Code, rec.Body.String()) + + s.Equal(int64(0), s.revisionCount(*s.agent.ID)) + + // application/json with parameters is accepted (R13). + rec = s.call(http.MethodPut, s.path("/config"), acaPutBody(`{"verbosity":1}`), "If-Match", `"0"`, echo.HeaderContentType, "application/json; charset=utf-8") + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) +} + +// ---- Revert ---- + +// ---- Revisions ---- + +// ---- Preview ---- + +// ---- Instances ---- + +// ---- Agent deletion ---- + +// Deleting an agent removes its instances and its revisions (the purge path for an overlay +// that held a secret); other agents' revisions are kept. +func (s *AgentConfigAdminIntegrationSuite) TestDeleteAgentRemovesInstancesAndRevisions() { + other, err := s.CreateAgent("other-agent") + s.Require().NoError(err) + s.report(*s.agent.ID, agentconfig.ModeApplySafe, nil) + s.report(*s.agent.ID, agentconfig.ModeReport, nil) + s.save(`"0"`, `{"verbosity":1}`, 1) + s.save(`"1"`, `{"plugins":{"ssh":{"config":{"password":"hunter2"}}}}`, 2) + rec := s.send(s.server, s.token, http.MethodPut, s.agentPath(*other.ID, "/config"), acaPutBody(`{"verbosity":1}`), "If-Match", `"0"`) + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) + + rec = s.call(http.MethodDelete, s.path(""), nil) + s.Require().Equal(http.StatusNoContent, rec.Code, rec.Body.String()) + + var instances int64 + s.Require().NoError(s.DB.Model(&relational.AgentInstance{}).Where("agent_id = ?", *s.agent.ID).Count(&instances).Error) + s.Zero(instances) + s.Zero(s.revisionCount(*s.agent.ID)) + s.Equal(int64(1), s.revisionCount(*other.ID)) + + // The (soft-deleted) agent is gone for the config routes. + rec = s.call(http.MethodGet, s.path("/config"), nil) + s.Equal(http.StatusNotFound, rec.Code, rec.Body.String()) +} + +// ---- Builtin authz (R39) ---- + +// ---- Cedar authz (R40) ---- + +// Overlays are verbatim for agent:configure holders and redacted for read-only callers. +func (s *AgentConfigAdminIntegrationSuite) TestCedarOverlayRedactedForReaders() { + overlay := `{"plugins":{"ssh":{"config":{"password":"hunter2","host":"db","pass_ref":"${env:SSH_PASS}"},"policy_data":{"api_token":"t0k3n","threshold":3}}}}` + s.save(`"0"`, overlay, 1) + _, viewer := s.userToken("viewer@example.com", "", "viewer") + _, contributor := s.userToken("contributor@example.com", "", "contributor") + _, admin := s.userToken("cedar-admin@example.com", "", "admin") + srv := s.cedarServer() + + redacted := `{"plugins":{"ssh":{"config":{"password":"••••","host":"db","pass_ref":"${env:SSH_PASS}"},"policy_data":{"api_token":"••••","threshold":3}}}}` + for _, path := range []string{s.path("/config"), s.path("/config/revisions/1")} { + for _, token := range []string{viewer, contributor} { + rec := s.send(srv, token, http.MethodGet, path, nil) + s.Require().Equal(http.StatusOK, rec.Code, rec.Body.String()) + got := acaData[agentConfigRevisionResponse](s, rec) + s.JSONEq(redacted, string(got.Overlay), path) + s.NotContains(rec.Body.String(), "hunter2", path) + s.NotContains(rec.Body.String(), "t0k3n", path) + } + rec := s.send(srv, admin, http.MethodGet, path, nil) + s.Require().Equal(http.StatusOK, rec.Code, rec.Body.String()) + got := acaData[agentConfigRevisionResponse](s, rec) + s.JSONEq(overlay, string(got.Overlay), "a configure holder gets the literal overlay: %s", path) + } +} + +// configureFailsPDP allows everything except agent:configure, which is unavailable. +type configureFailsPDP struct{} + +func (configureFailsPDP) Evaluate(_ context.Context, _ authz.Subject, action string, _ authz.Resource, _ map[string]any) (authz.Decision, error) { + if action == authz.ActionConfigure { + return authz.Decision{}, authz.ErrUnavailable + } + return authz.Decision{Allow: true}, nil +} + +func (p configureFailsPDP) Evaluations(ctx context.Context, reqs []authz.EvalRequest) ([]authz.Decision, error) { + out := make([]authz.Decision, len(reqs)) + for i, r := range reqs { + d, err := p.Evaluate(ctx, r.Subject, r.Action, r.Resource, r.Context) + if err != nil { + return nil, err + } + out[i] = d + } + return out, nil +} + +// When agent:configure cannot be evaluated, the overlay is redacted (fail closed), whatever +// the PEP's fail mode. +func (s *AgentConfigAdminIntegrationSuite) TestOverlayRedactedWhenConfigureCheckFails() { + s.save(`"0"`, `{"plugins":{"ssh":{"config":{"password":"hunter2"}}}}`, 1) + srv := s.newServer(middleware.NewPEP(configureFailsPDP{}, authz.FailOpen, s.logger)) + rec := s.send(srv, s.token, http.MethodGet, s.path("/config"), nil) + s.Require().Equal(http.StatusOK, rec.Code, rec.Body.String()) + got := acaData[agentConfigRevisionResponse](s, rec) + s.JSONEq(`{"plugins":{"ssh":{"config":{"password":"••••"}}}}`, string(got.Overlay)) +} + +func (s *AgentConfigAdminIntegrationSuite) TestCedarAdminCanEditSchedule() { + s.report(*s.agent.ID, agentconfig.ModeApplySafe, nil) + _, admin := s.userToken("cedar-admin@example.com", "", "admin") + srv := s.cedarServer() + + rec := s.put(srv, admin, `"0"`, `{"plugins":{"ssh":{"schedule":"*/5 * * * *"}}}`) + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) +} + +func (s *AgentConfigAdminIntegrationSuite) TestCedarUserWithoutRoleDenied() { + _, token := s.userToken("nobody@example.com", "", "") + srv := s.cedarServer() + rec := s.send(srv, token, http.MethodGet, s.path("/config"), nil) + s.Equal(http.StatusForbidden, rec.Code, rec.Body.String()) + rec = s.send(srv, token, http.MethodGet, "/api/admin/agents", nil) + s.Equal(http.StatusForbidden, rec.Code, rec.Body.String()) +} + +// ---- CORS (R13) ---- diff --git a/internal/api/handler/agent_config_test.go b/internal/api/handler/agent_config_test.go new file mode 100644 index 00000000..bc2a4ddf --- /dev/null +++ b/internal/api/handler/agent_config_test.go @@ -0,0 +1,74 @@ +package handler + +import ( + "encoding/json" + "testing" + + "github.com/compliance-framework/api/pkg/agentconfig" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testPluginSource = "ghcr.io/compliance-framework/plugin-local-ssh:v1.0.0" + +func strPtrT(s string) *string { return &s } + +// badCronBase is a reported base whose file already has an invalid schedule on plugin x. +func badCronBase() agentconfig.Config { + return agentconfig.Config{Plugins: map[string]*agentconfig.Plugin{ + "x": {Source: testPluginSource, Schedule: strPtrT("not a cron")}, + "y": {Source: testPluginSource, Schedule: strPtrT("@hourly")}, + }} +} + +func TestSplitIntroduced(t *testing.T) { + tests := []struct { + name string + overlay string + wantIntroduced []string // paths + wantFileOrigin []string // paths + }{ + { + name: "unrelated overlay keeps the file error as a warning", + overlay: `{"verbosity":1}`, + wantFileOrigin: []string{"/plugins/x/schedule"}, + }, + { + name: "replacing the bad cron with another bad cron is introduced", + overlay: `{"plugins":{"x":{"schedule":"also bad"}}}`, + wantIntroduced: []string{"/plugins/x/schedule"}, + }, + { + name: "a new bad cron on another plugin is introduced", + overlay: `{"plugins":{"y":{"schedule":"nope"}}}`, + wantIntroduced: []string{"/plugins/y/schedule"}, + wantFileOrigin: []string{"/plugins/x/schedule"}, + }, + { + name: "fixing the file error leaves nothing", + overlay: `{"plugins":{"x":{"schedule":"@daily"}}}`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + eff, introduced, fileOrigin := splitIntroduced(badCronBase(), json.RawMessage(tt.overlay)) + require.NotNil(t, eff) + assert.Equal(t, tt.wantIntroduced, fieldPaths(introduced)) + assert.Equal(t, tt.wantFileOrigin, fieldPaths(fileOrigin)) + for _, e := range append(introduced, fileOrigin...) { + assert.Equal(t, agentconfig.FieldCodeCron, e.Code) + } + }) + } +} + +func fieldPaths(errs []agentconfig.FieldError) []string { + if len(errs) == 0 { + return nil + } + out := make([]string, 0, len(errs)) + for _, e := range errs { + out = append(out, e.Path) + } + return out +} diff --git a/internal/api/handler/api.go b/internal/api/handler/api.go index f43a16c8..48466c73 100644 --- a/internal/api/handler/api.go +++ b/internal/api/handler/api.go @@ -225,6 +225,13 @@ func RegisterHandlers(server *api.Server, logger *zap.SugaredLogger, db *gorm.DB agentsGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) agentHandler.Register(agentsGroup, agentGuard.Read(), pep.Authorize(authz.ResourceAdmin, authz.ActionManage)) + // Admin agent-configuration routes, on their own group object so they inherit no group + // guard (same prefix; precedent /admin/users). + agentConfigHandler := NewAgentConfigHandler(logger, db, agentCfgSvc, pdp) + agentConfigGroup := server.API().Group("/admin/agents") + agentConfigGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) + agentConfigHandler.Register(agentConfigGroup, agentGuard) + // Agent-facing configuration sync: agent JWT only (strict — it ignores // StrictDisablePublicAgentEndpoints) and agent:sync. agentConfigSyncHandler := NewAgentConfigSyncHandler(logger, agentCfgSvc) From f405320d499b5fa975620cbfa1b0b3417f630ce9 Mon Sep 17 00:00:00 2001 From: "ccf-lisa[bot]" <286799724+ccf-lisa[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 07:27:52 -0300 Subject: [PATCH 2/5] fix(api): drop the hand-synced BodyLimit on agent config PUT; readJSONBody enforces the limit --- internal/api/handler/agent_config.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/api/handler/agent_config.go b/internal/api/handler/agent_config.go index 951fedc4..2425f48d 100644 --- a/internal/api/handler/agent_config.go +++ b/internal/api/handler/agent_config.go @@ -19,7 +19,6 @@ import ( "github.com/compliance-framework/api/pkg/agentconfig" "github.com/google/uuid" "github.com/labstack/echo/v4" - echomiddleware "github.com/labstack/echo/v4/middleware" "go.uber.org/zap" "gorm.io/gorm" ) @@ -30,11 +29,9 @@ const ( // which can roughly double or triple it, inside an envelope with a comment of up to // maxRevisionCommentLen characters (at most ~12 KiB JSON-escaped). Four times the overlay // limit (1 MiB) leaves room for that, so an overlay is rejected by ValidateOverlay with a - // precise 422 rather than by the transport with a 413. agentConfigBodyLimitStr is the - // same limit for echo's BodyLimit middleware. - agentConfigBodyLimit = 4 * agentconfig.MaxOverlayBytes - agentConfigBodyLimitStr = "1M" - maxRevisionCommentLen = 2000 + // precise 422 rather than by readJSONBody with a 413. + agentConfigBodyLimit = 4 * agentconfig.MaxOverlayBytes + maxRevisionCommentLen = 2000 ) // AgentConfigHandler serves the admin routes for agent remote configuration: the current @@ -57,7 +54,7 @@ func NewAgentConfigHandler(sugar *zap.SugaredLogger, db *gorm.DB, svc *agentcfg. func (h *AgentConfigHandler) Register(g *echo.Group, guard middleware.ResourceGuard) { write := guard.Do(authz.ActionConfigure) g.GET("/:id/config", h.Get, guard.Read()) - g.PUT("/:id/config", h.Put, write, echomiddleware.BodyLimit(agentConfigBodyLimitStr)) + g.PUT("/:id/config", h.Put, write) g.GET("/:id/config/revisions/:rev", h.GetRevision, guard.Read()) } From 281c434f5fd2ead31fc3160fbc64d2dff8d51449 Mon Sep 17 00:00:00 2001 From: "ccf-lisa[bot]" <286799724+ccf-lisa[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 08:38:47 -0300 Subject: [PATCH 3/5] fix(api): validate each distinct reported base once on save validateCandidate merged and validated the overlay against every instance of the validation set. ValidationBases now groups instances by base content (BaseKey): validate each distinct base once and attribute its errors to every instance in the group, so the 422 body still lists each instance and the cost of a save follows the distinct bases. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler/agent_config.go | 19 ++++-- ...onfig_admin_regression_integration_test.go | 59 +++++++++++++++++++ internal/api/handler/agent_config_test.go | 33 +++++++++++ 3 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 internal/api/handler/agent_config_admin_regression_integration_test.go diff --git a/internal/api/handler/agent_config.go b/internal/api/handler/agent_config.go index 2425f48d..733944fd 100644 --- a/internal/api/handler/agent_config.go +++ b/internal/api/handler/agent_config.go @@ -258,7 +258,9 @@ func (r candidateResult) errorBody() api.Error { // 2. when (1) passed: Merge(base, overlay).ValidateEditable() for every validation base, // grouped by instance. Only errors the overlay introduces are kept (R59, see // splitIntroduced); an instance is listed only when it has at least one. With no bases -// (standalone) only (1) runs. +// (standalone) only (1) runs. Instances that share a BaseKey (the same reported base) +// are validated once and the result is attributed to each of them, so the cost follows +// the distinct bases, not the instance count. func validateCandidate(overlay json.RawMessage, bases []agentcfg.InstanceBase) candidateResult { var r candidateResult if err := agentconfig.ValidateOverlay(overlay); err != nil { @@ -272,13 +274,22 @@ func validateCandidate(overlay json.RawMessage, bases []agentcfg.InstanceBase) c if len(r.overlay) > 0 { return r } + type outcome struct{ introduced, fileOrigin []agentconfig.FieldError } + byBase := map[string]outcome{} for _, b := range bases { - if _, introduced, fileOrigin := splitIntroduced(b.Base, overlay); len(introduced) > 0 { + o, done := byBase[b.BaseKey] + if !done || b.BaseKey == "" { + _, o.introduced, o.fileOrigin = splitIntroduced(b.Base, overlay) + if b.BaseKey != "" { + byBase[b.BaseKey] = o + } + } + if len(o.introduced) > 0 { r.instances = append(r.instances, instanceValidationErrors{ InstanceID: b.Instance.InstanceID.String(), Hostname: b.Instance.Hostname, - Errors: introduced, - Warnings: nonNil(fileOrigin), + Errors: o.introduced, + Warnings: nonNil(o.fileOrigin), }) } } diff --git a/internal/api/handler/agent_config_admin_regression_integration_test.go b/internal/api/handler/agent_config_admin_regression_integration_test.go new file mode 100644 index 00000000..81fb4f94 --- /dev/null +++ b/internal/api/handler/agent_config_admin_regression_integration_test.go @@ -0,0 +1,59 @@ +//go:build integration + +package handler + +import ( + "encoding/json" + "fmt" + "net/http" + "runtime" + "strings" + + "github.com/compliance-framework/api/pkg/agentconfig" + "github.com/google/uuid" +) + +// Regression (review #477/#481, fp f2b689648c35): a save validates against every fresh +// apply-mode instance, but instances that report the same base must not each be loaded and +// validated: the cost of a save follows the distinct bases, not the instance count. +func (s *AgentConfigAdminIntegrationSuite) TestRegressionSaveCostFollowsDistinctBases() { + const ( + instances = 40 + baseBytes = 1 << 20 // 1 MiB reported base, shared by every instance + // Budget for the whole PUT: a few copies of one base plus request overhead. Loading + // and validating every instance's copy allocates well over instances*baseBytes. + allocBudget = 32 << 20 + ) + schedule := "* * * * *" + base, err := json.Marshal(agentconfig.Config{ + Daemon: true, + API: &agentconfig.APIConfig{URL: "http://api:8080", Auth: &agentconfig.APIAuth{ClientID: uuid.NewString()}}, + RemoteConfig: &agentconfig.RemoteConfig{Mode: agentconfig.ModeApplySafe}, + Plugins: map[string]*agentconfig.Plugin{ + "ssh": { + Source: acaVendorPlugin, + Schedule: &schedule, + Policies: []string{acaVendorPolicy}, + Config: map[string]string{"host": "localhost", "banner": strings.Repeat("b", baseBytes)}, + }, + }, + }) + s.Require().NoError(err) + for i := 0; i < instances; i++ { + s.report(*s.agent.ID, agentconfig.ModeApplySafe, func(r *agentconfig.Report) { + r.Base = base + r.Effective = base + }) + } + + runtime.GC() + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + rec := s.put(s.server, s.token, `"0"`, `{"verbosity":1}`) + runtime.ReadMemStats(&after) + + s.Require().Equal(http.StatusCreated, rec.Code, rec.Body.String()) + allocated := after.TotalAlloc - before.TotalAlloc + s.LessOrEqual(allocated, uint64(allocBudget), + fmt.Sprintf("PUT allocated %d MiB for %d instances sharing one %d MiB base", allocated>>20, instances, baseBytes>>20)) +} diff --git a/internal/api/handler/agent_config_test.go b/internal/api/handler/agent_config_test.go index bc2a4ddf..4655281c 100644 --- a/internal/api/handler/agent_config_test.go +++ b/internal/api/handler/agent_config_test.go @@ -4,7 +4,10 @@ import ( "encoding/json" "testing" + "github.com/compliance-framework/api/internal/service/relational" + "github.com/compliance-framework/api/internal/service/relational/agentcfg" "github.com/compliance-framework/api/pkg/agentconfig" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -72,3 +75,33 @@ func fieldPaths(errs []agentconfig.FieldError) []string { } return out } + +// Instances that share a BaseKey are validated once, and every one of them is still listed +// with the errors the overlay introduces on that base. +func TestValidateCandidateAttributesSharedBaseErrors(t *testing.T) { + shared := badCronBase() + good := agentconfig.Config{Plugins: map[string]*agentconfig.Plugin{ + "z": {Source: testPluginSource, Schedule: strPtrT("@hourly")}, + }} + ids := []uuid.UUID{uuid.New(), uuid.New(), uuid.New(), uuid.New()} + bases := []agentcfg.InstanceBase{ + {Instance: relational.AgentInstance{InstanceID: ids[0], Hostname: strPtrT("a")}, Base: shared, BaseKey: "k1"}, + {Instance: relational.AgentInstance{InstanceID: ids[1], Hostname: strPtrT("b")}, Base: shared, BaseKey: "k1"}, + {Instance: relational.AgentInstance{InstanceID: ids[2], Hostname: strPtrT("c")}, Base: good, BaseKey: "k2"}, + {Instance: relational.AgentInstance{InstanceID: ids[3], Hostname: strPtrT("d")}, Base: shared}, // no key: validated on its own + } + // Plugin z has a source only in the k2 base: elsewhere the overlay adds it without one. + r := validateCandidate(json.RawMessage(`{"plugins":{"z":{"schedule":"* * * * *"}}}`), bases) + require.Empty(t, r.overlay) + require.Len(t, r.instances, 3, "the k2 base already has plugin z") + for i, want := range []struct { + id uuid.UUID + host string + }{{ids[0], "a"}, {ids[1], "b"}, {ids[3], "d"}} { + got := r.instances[i] + assert.Equal(t, want.id.String(), got.InstanceID) + assert.Equal(t, want.host, *got.Hostname) + assert.Equal(t, []string{"/plugins/z/source"}, fieldPaths(got.Errors)) + assert.Equal(t, []string{"/plugins/x/schedule"}, fieldPaths(got.Warnings), "file-origin") + } +} From 515dfa24dfc45431e566ab4f34d07f54ce053511 Mon Sep 17 00:00:00 2001 From: "ccf-lisa[bot]" <286799724+ccf-lisa[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 08:39:19 -0300 Subject: [PATCH 4/5] fix(api): reject a stray closing brace after a strict JSON body decodeStrict checked for trailing data with dec.More(), which reports false for a trailing '}' or ']', so {"overlay":{}}} was accepted. Require the next token to be io.EOF instead. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler/agent_config.go | 4 +++- .../agent_config_body_regression_test.go | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 internal/api/handler/agent_config_body_regression_test.go diff --git a/internal/api/handler/agent_config.go b/internal/api/handler/agent_config.go index 733944fd..d34352cc 100644 --- a/internal/api/handler/agent_config.go +++ b/internal/api/handler/agent_config.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "strconv" "strings" @@ -492,7 +493,8 @@ func decodeStrict(body []byte, dst any) error { if err := dec.Decode(dst); err != nil { return fmt.Errorf("invalid request body: %w", err) } - if dec.More() { + // Token, not More: More reports false for a stray '}' or ']', so it accepted them. + if _, err := dec.Token(); !errors.Is(err, io.EOF) { return errors.New("invalid request body: unexpected data after the JSON object") } return nil diff --git a/internal/api/handler/agent_config_body_regression_test.go b/internal/api/handler/agent_config_body_regression_test.go new file mode 100644 index 00000000..4e465803 --- /dev/null +++ b/internal/api/handler/agent_config_body_regression_test.go @@ -0,0 +1,24 @@ +package handler + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Regression (review #481, fp 8ef552cfc250): decodeStrict must reject any data after the +// JSON object, including a stray closing brace or bracket. +func TestDecodeStrictRejectsTrailingData(t *testing.T) { + for _, body := range []string{ + `{"overlay":{}}}`, + `{"overlay":{}}]`, + `{"overlay":{}} x`, + `{"overlay":{}} {}`, + } { + var req agentConfigPutRequest + assert.Error(t, decodeStrict([]byte(body), &req), body) + } + var req agentConfigPutRequest + require.NoError(t, decodeStrict([]byte(" {\"overlay\":{}} \n"), &req), "trailing whitespace is fine") +} From 8f8c3700c598b52f4e72847c2a991930dc820843 Mon Sep 17 00:00:00 2001 From: "ccf-lisa[bot]" <286799724+ccf-lisa[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 08:40:07 -0300 Subject: [PATCH 5/5] fix(api): reject a NUL in a revision comment with a 400 normalizeComment let a NUL character through, and Postgres cannot store one in a text column, so PUT (and revert) failed with a 500 from the insert. Reject it as a 400, like the overlay (O11) and report NUL checks. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler/agent_config.go | 4 ++++ ...fig_comment_regression_integration_test.go | 14 ++++++++++++ .../agent_config_comment_regression_test.go | 22 +++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 internal/api/handler/agent_config_comment_regression_integration_test.go create mode 100644 internal/api/handler/agent_config_comment_regression_test.go diff --git a/internal/api/handler/agent_config.go b/internal/api/handler/agent_config.go index d34352cc..0dc78ff0 100644 --- a/internal/api/handler/agent_config.go +++ b/internal/api/handler/agent_config.go @@ -511,6 +511,10 @@ func normalizeComment(c *string) (*string, error) { if len([]rune(trimmed)) > maxRevisionCommentLen { return nil, fmt.Errorf("comment must be at most %d characters", maxRevisionCommentLen) } + // Postgres cannot store a NUL in a text column: the insert would fail with a 500. + if strings.ContainsRune(trimmed, 0) { + return nil, errors.New("comment must not contain a NUL character") + } return &trimmed, nil } diff --git a/internal/api/handler/agent_config_comment_regression_integration_test.go b/internal/api/handler/agent_config_comment_regression_integration_test.go new file mode 100644 index 00000000..38810d11 --- /dev/null +++ b/internal/api/handler/agent_config_comment_regression_integration_test.go @@ -0,0 +1,14 @@ +//go:build integration + +package handler + +import "net/http" + +// Regression (review #481, fp a042387242aa): a NUL in the revision comment is a 400 on +// PUT (not a 500 from the insert), and nothing is stored. +func (s *AgentConfigAdminIntegrationSuite) TestRegressionCommentNULIsBadRequest() { + rec := s.send(s.server, s.token, http.MethodPut, s.path("/config"), + []byte(`{"overlay":{"verbosity":1},"comment":"a\u0000b"}`), "If-Match", `"0"`) + s.Equal(http.StatusBadRequest, rec.Code, rec.Body.String()) + s.Equal(int64(0), s.revisionCount(*s.agent.ID)) +} diff --git a/internal/api/handler/agent_config_comment_regression_test.go b/internal/api/handler/agent_config_comment_regression_test.go new file mode 100644 index 00000000..575c1bdb --- /dev/null +++ b/internal/api/handler/agent_config_comment_regression_test.go @@ -0,0 +1,22 @@ +package handler + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Regression (review #481, fp a042387242aa): a NUL character cannot be stored in the +// comment column, so it is a 400, not a 500 from the insert. +func TestNormalizeCommentRejectsNUL(t *testing.T) { + c := "a\x00b" + _, err := normalizeComment(&c) + assert.Error(t, err) + + ok := " fine " + got, err := normalizeComment(&ok) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "fine", *got) +}