feat(remediation): conditional approval (A-keep) — free-core auto-approves

CHANGELOG.md

@@ -12,6 +12,14 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Changed
 
+- Remediation: free-core single-rule remediation is now **auto-approved** on
+  request, so an operator can apply a fix without a separate approver. This
+  removes the self-review deadlock for single-operator workspaces (you could
+  request a fix but never approve your own request). The request lifecycle and
+  the approve/reject flow with separation of duties are retained for the
+  licensed bulk/automated remediation track (which requests with approval
+  required). See `docs/engineering/remediation_governance_adr.md` ("A-keep").
+
 - CI release safety: the release workflow now fails closed on a `v*` tag push
   when no GPG signing key is configured, rather than publishing unsigned
   packages. Manual `workflow_dispatch` trial builds stay permissive (warn +

internal/remediation/service.go

@@ -75,8 +75,21 @@ func scanListRequest(row pgx.Row) (Request, error) {
 // recording a best-effort projected per-framework lift. Returns
 // ErrDuplicateOpen when an open request already exists for the same host+rule.
 // NEVER contacts the host. Emits remediation.requested.
+// Request opens a remediation request for one rule on one host.
+//
+// Approval is conditional on the remediation track (ADR
+// docs/engineering/remediation_governance_adr.md, "A-keep"):
+//
+//   - requiresApproval=false (FREE-CORE single-rule manual remediation): the
+//     request is AUTO-APPROVED on creation — inserted directly in 'approved'
+//     with reviewed_at set and an explanatory review_note, so the requester can
+//     Fix it without a separate approver. This makes single-operator workspaces
+//     workable (no self-review deadlock).
+//   - requiresApproval=true (LICENSED bulk/auto track): the request is inserted
+//     'pending_approval' and must go through Approve/Reject with separation of
+//     duties (the requester cannot review their own request) before it can run.
 func (s *Service) Request(ctx context.Context, hostID uuid.UUID, ruleID string,
-	scanRunID *uuid.UUID, requestedBy uuid.UUID) (Request, error) {
+	scanRunID *uuid.UUID, requestedBy uuid.UUID, requiresApproval bool) (Request, error) {
 	ruleID = strings.TrimSpace(ruleID)
 	if ruleID == "" {
 		return Request{}, ErrInvalidInput
@@ -87,13 +100,27 @@ func (s *Service) Request(ctx context.Context, hostID uuid.UUID, ruleID string,
 	proj, _ := s.ProjectLift(ctx, hostID, ruleID)
 
 	id := uuid.Must(uuid.NewV7())
-	row := s.pool.QueryRow(ctx, `
-		INSERT INTO remediation_requests
-			(id, host_id, rule_id, scan_run_id, status, requested_by,
-			 projected_cis, projected_stig, projected_nist)
-		VALUES ($1, $2, $3, $4, 'pending_approval', $5, $6, $7, $8)
-		RETURNING `+selectCols,
-		id, hostID, ruleID, scanRunID, requestedBy, proj.CIS, proj.STIG, proj.NIST)
+	var row pgx.Row
+	if requiresApproval {
+		row = s.pool.QueryRow(ctx, `
+			INSERT INTO remediation_requests
+				(id, host_id, rule_id, scan_run_id, status, requested_by,
+				 projected_cis, projected_stig, projected_nist)
+			VALUES ($1, $2, $3, $4, 'pending_approval', $5, $6, $7, $8)
+			RETURNING `+selectCols,
+			id, hostID, ruleID, scanRunID, requestedBy, proj.CIS, proj.STIG, proj.NIST)
+	} else {
+		row = s.pool.QueryRow(ctx, `
+			INSERT INTO remediation_requests
+				(id, host_id, rule_id, scan_run_id, status, requested_by,
+				 reviewed_at, review_note,
+				 projected_cis, projected_stig, projected_nist)
+			VALUES ($1, $2, $3, $4, 'approved', $5, now(),
+				'auto-approved: free-core single-rule remediation requires no separate approval',
+				$6, $7, $8)
+			RETURNING `+selectCols,
+			id, hostID, ruleID, scanRunID, requestedBy, proj.CIS, proj.STIG, proj.NIST)
+	}
 	rq, err := scanRequest(row)
 	if err != nil {
 		if isUniqueViolation(err) {
@@ -103,6 +130,11 @@ func (s *Service) Request(ctx context.Context, hostID uuid.UUID, ruleID string,
 	}
 
 	s.emitEvent(ctx, audit.RemediationRequested, rq, requestedBy, "requested")
+	if !requiresApproval {
+		// Record the auto-approval honestly in the audit trail; there is no
+		// human reviewer (reviewed_by stays NULL).
+		s.emitEvent(ctx, audit.RemediationApproved, rq, requestedBy, "auto_approved")
+	}
 	return rq, nil
 }
 

internal/remediation/service_test.go

@@ -102,33 +102,50 @@ func TestRequest_InsertDuplicateInvalidReopen(t *testing.T) {
 		var calls []emitCall
 		svc := NewService(pool, fakeEmitter(&calls))
 
-		rq, err := svc.Request(ctx, hostID, "sshd-permit-root-no", nil, user)
+		// Free-core single-rule remediation (requiresApproval=false) is
+		// AUTO-APPROVED on creation.
+		rq, err := svc.Request(ctx, hostID, "sshd-permit-root-no", nil, user, false)
 		if err != nil {
 			t.Fatalf("Request: %v", err)
 		}
-		if rq.Status != StatusPendingApproval || rq.RuleID != "sshd-permit-root-no" {
-			t.Errorf("requested remediation = %+v", rq)
+		if rq.Status != StatusApproved || rq.RuleID != "sshd-permit-root-no" {
+			t.Errorf("requested remediation = %+v, want status=approved", rq)
 		}
-		if len(calls) != 1 || calls[0].Code != audit.RemediationRequested {
-			t.Errorf("audit calls = %+v, want one requested", calls)
+		if rq.ReviewedAt == nil || rq.ReviewedBy != nil {
+			t.Errorf("auto-approved: want reviewed_at set + reviewed_by nil, got reviewed_at=%v reviewed_by=%v", rq.ReviewedAt, rq.ReviewedBy)
+		}
+		if !strings.Contains(rq.ReviewNote, "auto-approved") {
+			t.Errorf("auto-approved review_note = %q, want an auto-approved note", rq.ReviewNote)
+		}
+		// Auto-approve emits remediation.requested then remediation.approved.
+		if len(calls) != 2 || calls[0].Code != audit.RemediationRequested || calls[1].Code != audit.RemediationApproved {
+			t.Errorf("audit calls = %+v, want requested + approved", calls)
 		}
 
-		// Duplicate open: rejected.
-		if _, err := svc.Request(ctx, hostID, "sshd-permit-root-no", nil, user); !errors.Is(err, ErrDuplicateOpen) {
+		// Duplicate open (the auto-approved request is still open): rejected.
+		if _, err := svc.Request(ctx, hostID, "sshd-permit-root-no", nil, user, false); !errors.Is(err, ErrDuplicateOpen) {
 			t.Errorf("duplicate Request err = %v, want ErrDuplicateOpen", err)
 		}
 
 		// Invalid input (empty rule).
-		if _, err := svc.Request(ctx, hostID, "  ", nil, user); !errors.Is(err, ErrInvalidInput) {
+		if _, err := svc.Request(ctx, hostID, "  ", nil, user, false); !errors.Is(err, ErrInvalidInput) {
 			t.Errorf("empty rule err = %v, want ErrInvalidInput", err)
 		}
 
-		// Reopen after the prior is rejected: a fresh request succeeds.
+		// Approval-required (requiresApproval=true) inserts pending_approval and
+		// reopens after a terminal state: reject -> a fresh request succeeds.
 		reviewer := seedUser(t, pool, "reviewer")
-		if _, err := svc.Reject(ctx, rq.ID, reviewer, "not now"); err != nil {
+		pend, err := svc.Request(ctx, hostID, "needs-approval", nil, user, true)
+		if err != nil {
+			t.Fatalf("approval-required Request: %v", err)
+		}
+		if pend.Status != StatusPendingApproval {
+			t.Errorf("approval-required status = %v, want pending_approval", pend.Status)
+		}
+		if _, err := svc.Reject(ctx, pend.ID, reviewer, "not now"); err != nil {
 			t.Fatalf("Reject: %v", err)
 		}
-		if _, err := svc.Request(ctx, hostID, "sshd-permit-root-no", nil, user); err != nil {
+		if _, err := svc.Request(ctx, hostID, "needs-approval", nil, user, true); err != nil {
 			t.Errorf("reopen after reject failed: %v", err)
 		}
 	})
@@ -146,7 +163,7 @@ func TestLifecycle_Transitions(t *testing.T) {
 		svc := NewService(pool, fakeEmitter(&calls))
 
 		// approve path
-		a, _ := svc.Request(ctx, hostID, "rule-a", nil, requester)
+		a, _ := svc.Request(ctx, hostID, "rule-a", nil, requester, true)
 		got, err := svc.Approve(ctx, a.ID, reviewer, "ok")
 		if err != nil || got.Status != StatusApproved || got.ReviewedBy == nil || *got.ReviewedBy != reviewer {
 			t.Fatalf("Approve = %+v, err %v", got, err)
@@ -157,7 +174,7 @@ func TestLifecycle_Transitions(t *testing.T) {
 		}
 
 		// reject path
-		b, _ := svc.Request(ctx, hostID, "rule-b", nil, requester)
+		b, _ := svc.Request(ctx, hostID, "rule-b", nil, requester, true)
 		if got, err := svc.Reject(ctx, b.ID, reviewer, "no"); err != nil || got.Status != StatusRejected {
 			t.Fatalf("Reject = %+v, err %v", got, err)
 		}
@@ -198,7 +215,7 @@ func TestSeparationOfDuties(t *testing.T) {
 		hostID := seedHost(t, pool, user)
 		svc := NewService(pool, fakeEmitter(&[]emitCall{}))
 
-		rq, _ := svc.Request(ctx, hostID, "rule-s", nil, user)
+		rq, _ := svc.Request(ctx, hostID, "rule-s", nil, user, true)
 		// self-approve blocked
 		if _, err := svc.Approve(ctx, rq.ID, user, "me"); !errors.Is(err, ErrSelfReview) {
 			t.Errorf("self-approve err = %v, want ErrSelfReview", err)
@@ -260,7 +277,7 @@ func TestProjectLift_AndOverlayNeverMutatesRuleState(t *testing.T) {
 		}
 
 		// Request persists the projection snapshot.
-		rq, err := svc.Request(ctx, hostID, "rule-active", nil, requester)
+		rq, err := svc.Request(ctx, hostID, "rule-active", nil, requester, false)
 		if err != nil {
 			t.Fatalf("Request: %v", err)
 		}

internal/server/api_remediation_test.go

@@ -62,8 +62,10 @@ func TestAPI_Remediation_LifecycleAndRBAC(t *testing.T) {
 		if err := json.NewDecoder(or.Body).Decode(&created); err != nil {
 			t.Fatalf("decode created: %v", err)
 		}
-		if created.Status != "pending_approval" || created.RuleID != "sshd-permit-root-no" {
-			t.Errorf("created = %+v", created)
+		// Free core: a single-rule request is auto-approved on creation (no
+		// separate approver). See remediation_governance_adr.md.
+		if created.Status != "approved" || created.RuleID != "sshd-permit-root-no" {
+			t.Errorf("created = %+v, want status=approved (auto-approved free core)", created)
 		}
 
 		// --- duplicate open -> 409 ---
@@ -81,16 +83,22 @@ func TestAPI_Remediation_LifecycleAndRBAC(t *testing.T) {
 			t.Errorf("anonymous list status = %d, want 401/403", ar.StatusCode)
 		}
 
-		// --- approve: ops_lead is 403 (no remediation:approve) ---
-		oa := doReq(t, asRole(t, "POST", base+"/"+created.ID+":approve", auth.RoleOpsLead, map[string]any{}))
+		// --- approve endpoint RBAC + separation of duties. The free-core POST
+		// above auto-approves, so it never yields a pending row; seed an
+		// approval-required (pending_approval) request directly to exercise the
+		// approve endpoint (this is the licensed bulk/auto track's shape). ---
+		pendID := seedPendingRemediation(t, pool, hostID, roleUserIDs[auth.RoleOpsLead], "needs-approval")
+		pendPath := base + "/" + pendID.String()
+
+		// ops_lead (the requester; no remediation:approve) -> 403
+		oa := doReq(t, asRole(t, "POST", pendPath+":approve", auth.RoleOpsLead, map[string]any{}))
 		oa.Body.Close()
 		if oa.StatusCode != http.StatusForbidden {
 			t.Fatalf("ops_lead approve status = %d, want 403", oa.StatusCode)
 		}
 
-		// --- approve: security_admin (different user from the ops_lead
-		// requester) succeeds ---
-		sa := doReq(t, asRole(t, "POST", base+"/"+created.ID+":approve",
+		// security_admin (different user from the requester) approves -> 200
+		sa := doReq(t, asRole(t, "POST", pendPath+":approve",
 			auth.RoleSecurityAdmin, map[string]any{"note": "reviewed"}))
 		defer sa.Body.Close()
 		if sa.StatusCode != http.StatusOK {
@@ -102,21 +110,17 @@ func TestAPI_Remediation_LifecycleAndRBAC(t *testing.T) {
 			t.Errorf("approved status = %q, want approved", approved.Status)
 		}
 
-		// --- re-approve -> 409 wrong state ---
-		ra := doReq(t, asRole(t, "POST", base+"/"+created.ID+":approve", auth.RoleSecurityAdmin, map[string]any{}))
+		// re-approve -> 409 wrong state
+		ra := doReq(t, asRole(t, "POST", pendPath+":approve", auth.RoleSecurityAdmin, map[string]any{}))
 		ra.Body.Close()
 		if ra.StatusCode != http.StatusConflict {
 			t.Errorf("re-approve status = %d, want 409", ra.StatusCode)
 		}
 
-		// --- separation of duties at the HTTP layer: a security_admin
-		// requests, then tries to approve their own -> 409 self_review ---
-		selfReq := doReq(t, asRole(t, "POST", base, auth.RoleSecurityAdmin,
-			map[string]any{"host_id": hostID.String(), "rule_id": "self-rule"}))
-		defer selfReq.Body.Close()
-		var selfRR apiRem
-		_ = json.NewDecoder(selfReq.Body).Decode(&selfRR)
-		selfAp := doReq(t, asRole(t, "POST", base+"/"+selfRR.ID+":approve", auth.RoleSecurityAdmin, map[string]any{}))
+		// separation of duties at the HTTP layer: the security_admin requester
+		// cannot approve their own pending request -> 409 self_review.
+		selfID := seedPendingRemediation(t, pool, hostID, roleUserIDs[auth.RoleSecurityAdmin], "self-rule")
+		selfAp := doReq(t, asRole(t, "POST", base+"/"+selfID.String()+":approve", auth.RoleSecurityAdmin, map[string]any{}))
 		selfAp.Body.Close()
 		if selfAp.StatusCode != http.StatusConflict {
 			t.Errorf("self-approve status = %d, want 409 (separation of duties)", selfAp.StatusCode)
@@ -181,23 +185,18 @@ func TestAPI_Remediation_ExecuteFreeCore(t *testing.T) {
 			t.Fatalf("viewer execute status = %d, want 403", o.StatusCode)
 		}
 
-		// security_admin has the perm, but the request is still pending
-		// (not approved) -> 409 wrong_state. NOT 402.
-		pre := doReq(t, asRole(t, "POST", execURL, auth.RoleSecurityAdmin, map[string]any{}))
+		// Executing a NON-approved (pending) request -> 409 wrong_state, NOT 402
+		// (free core, no license gate). The free-core request above auto-approves,
+		// so seed a pending row to exercise this path.
+		pendID := seedPendingRemediation(t, pool, hostID, roleUserIDs[auth.RoleOpsLead], "rule-pending")
+		pre := doReq(t, asRole(t, "POST", base+"/"+pendID.String()+":execute", auth.RoleSecurityAdmin, map[string]any{}))
 		pre.Body.Close()
 		if pre.StatusCode != http.StatusConflict {
 			t.Fatalf("execute-before-approve status = %d, want 409", pre.StatusCode)
 		}
 
-		// Approve it (security_admin != ops_lead requester).
-		ap := doReq(t, asRole(t, "POST", base+"/"+created.ID+":approve",
-			auth.RoleSecurityAdmin, map[string]any{"note": "ok"}))
-		ap.Body.Close()
-		if ap.StatusCode != http.StatusOK {
-			t.Fatalf("approve status = %d, want 200", ap.StatusCode)
-		}
-
-		// Now execute -> 202 Accepted, a remediation job enqueued.
+		// The free-core request is already approved (auto). Execute it directly
+		// -> 202 Accepted, a remediation job enqueued.
 		ex := doReq(t, asRole(t, "POST", execURL, auth.RoleSecurityAdmin, map[string]any{}))
 		var acc struct {
 			RequestID string `json:"request_id"`
@@ -226,6 +225,22 @@ func TestAPI_Remediation_ExecuteFreeCore(t *testing.T) {
 	})
 }
 
+// seedPendingRemediation inserts a pending_approval remediation request
+// directly (the approval-required / licensed track's shape). The free-core HTTP
+// POST auto-approves, so this is how the approve-endpoint and pending-execute
+// paths are exercised at the HTTP layer.
+func seedPendingRemediation(t *testing.T, pool *pgxpool.Pool, hostID, requestedBy uuid.UUID, ruleID string) uuid.UUID {
+	t.Helper()
+	id := uuid.Must(uuid.NewV7())
+	if _, err := pool.Exec(context.Background(),
+		`INSERT INTO remediation_requests (id, host_id, rule_id, status, requested_by)
+		 VALUES ($1, $2, $3, 'pending_approval', $4)`,
+		id, hostID, ruleID, requestedBy); err != nil {
+		t.Fatalf("seed pending remediation: %v", err)
+	}
+	return id
+}
+
 // countRemediationJobs counts pending remediation jobs on the queue.
 func countRemediationJobs(t *testing.T, pool *pgxpool.Pool) int {
 	t.Helper()

internal/server/remediation_handlers.go

@@ -207,7 +207,10 @@ func (h *handlers) RequestRemediation(w http.ResponseWriter, r *http.Request) {
 		s := uuid.UUID(*req.ScanRunId)
 		scanRunID = &s
 	}
-	rq, err := h.remediationSvc.Request(ctx, hostID, req.RuleId, scanRunID, requestedBy)
+	// Free-core: single-rule manual remediation is auto-approved (no separate
+	// approver). Bulk/auto remediation (OpenWatch+) will request with approval
+	// required via its own gated handler.
+	rq, err := h.remediationSvc.Request(ctx, hostID, req.RuleId, scanRunID, requestedBy, false)
 	if mapRemediationErr(w, err) {
 		return
 	}

internal/worker/remediation_worker_test.go

@@ -36,7 +36,7 @@ func seedApprovedRequest(t *testing.T, pool *pgxpool.Pool, svc *remediation.Serv
 	requester := seedUniqueUser(t, pool)
 	reviewer := seedUniqueUser(t, pool)
 	ctx := context.Background()
-	rq, err := svc.Request(ctx, hostID, ruleID, nil, requester)
+	rq, err := svc.Request(ctx, hostID, ruleID, nil, requester, true)
 	if err != nil {
 		t.Fatalf("seed request: %v", err)
 	}

specs/api/remediation.spec.yaml

@@ -121,7 +121,7 @@ spec:
 
   acceptance_criteria:
     - id: AC-01
-      description: 'Request inserts a pending_approval row (host_id, rule_id, requested_by, optional scan_run_id and projected-lift snapshot) and emits remediation.requested; a second Request for the same host+rule while one is open (pending_approval/approved/dry_run_complete/executing) returns ErrDuplicateOpen; an empty rule_id returns ErrInvalidInput; a fresh Request after the prior one reached a terminal state succeeds'
+      description: 'Approval is conditional on the remediation track. A free-core Request (requiresApproval=false) is AUTO-APPROVED: it inserts an approved row (host_id, rule_id, requested_by, optional scan_run_id and projected-lift snapshot, reviewed_at set, reviewed_by NULL, an auto-approved review_note) and emits remediation.requested then remediation.approved. A requiresApproval=true Request (the licensed bulk/auto track) inserts pending_approval instead. A second Request for the same host+rule while one is open (pending_approval/approved/dry_run_complete/executing) returns ErrDuplicateOpen; an empty rule_id returns ErrInvalidInput; a fresh Request after the prior one reached a terminal state succeeds'
       priority: critical
       references_constraints: [C-02]
     - id: AC-02
@@ -137,11 +137,11 @@ spec:
       priority: critical
       references_constraints: [C-01, C-07]
     - id: AC-05
-      description: 'Endpoint RBAC + lifecycle: POST /api/v1/remediation/requests requires remediation:request and 404s an unknown host_id; GET /requests, GET /requests/{id}, GET /requests/{id}/steps require remediation:read; POST :approve and :reject require remediation:approve; anonymous callers are rejected on all. A full request->approve happy path round-trips through the HTTP layer'
+      description: 'Endpoint RBAC + lifecycle: POST /api/v1/remediation/requests requires remediation:request, 404s an unknown host_id, and (free core) returns the request AUTO-APPROVED; GET /requests, GET /requests/{id}, GET /requests/{id}/steps require remediation:read; POST :approve and :reject require remediation:approve and enforce separation of duties (the requester cannot approve their own request, 409); anonymous callers are rejected on all. The approve endpoint is exercised on an approval-required (pending_approval) request since the free-core request auto-approves'
       priority: critical
       references_constraints: [C-05, C-03]
     - id: AC-06
-      description: 'Execute/rollback are FREE core: a caller WITH remediation:execute executing an approved request gets 202 and a remediation job is enqueued; a caller LACKING remediation:execute is 403; executing a non-approved request is 409; rolling back (remediation:rollback) a non-executed request is 409. No remediation act endpoint returns 402. :dry-run returns 501. request/approve/reject and the GET reads return their normal status codes.'
+      description: 'Execute/rollback are FREE core: a caller WITH remediation:execute executing an approved request gets 202 and a remediation job is enqueued. A free-core request is auto-approved, so it executes directly with no separate approval step. A caller LACKING remediation:execute is 403; executing a non-approved (pending_approval) request is 409; rolling back (remediation:rollback) a non-executed request is 409. No remediation act endpoint returns 402. :dry-run returns 501. request/approve/reject and the GET reads return their normal status codes.'
       priority: critical
       references_constraints: [C-06]
     - id: AC-07