fix(rbac): close four anonymous surfaces from the GA review (G1, G2, G3)

internal/server/api_g_cluster_test.go

@@ -0,0 +1,70 @@
+// @spec system-rbac
+//
+//	AC-21  TestGCluster_AnonymousIsRefusedOnPrivilegedSurfaces
+package server
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/Hanalyx/openwatch/internal/auth"
+	"github.com/google/uuid"
+)
+
+// @ac AC-21
+// AC-21 (NEGATIVE PATH): three surfaces that were reachable anonymously are
+// now refused, and one disclosure is withheld.
+//
+// Each was a distinct class of problem, so each is asserted separately rather
+// than as one loop:
+//
+//   - enqueue-test-job wrote to the REAL job queue with no authentication. An
+//     unauthenticated write to a work queue is a denial-of-service primitive
+//     and a source of unbounded table growth.
+//   - /admin/license:verify was an oracle: submit any JWT and learn whether it
+//     verifies, its tier, its features, its expiry, and whether it was signed
+//     with the PREVIOUS key, which leaks key-rotation state.
+//   - connectivity:check gated on host:read, but it is not a read: it makes
+//     the server open an SSH or ICMP connection to a managed host on demand.
+//   - GET /license disclosed customer_id, the paying organisation's identity,
+//     to anyone who could reach the port.
+func TestGCluster_AnonymousIsRefusedOnPrivilegedSurfaces(t *testing.T) {
+	t.Run("system-rbac/AC-21", func(t *testing.T) {
+		url, _ := freshAPIServer(t)
+
+		// The job-queue DoS primitive.
+		req, _ := http.NewRequest("POST", url+"/api/v1/diagnostics:enqueue-test-job", nil)
+		if r := doReq(t, req); r.StatusCode == http.StatusAccepted {
+			r.Body.Close()
+			t.Error("anonymous enqueue-test-job must not reach the job queue")
+		} else {
+			r.Body.Close()
+		}
+
+		// The license oracle.
+		req2, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", nil)
+		if r := doReq(t, req2); r.StatusCode == http.StatusOK {
+			r.Body.Close()
+			t.Error("anonymous license:verify must not answer; it is a signature and entitlement oracle")
+		} else {
+			r.Body.Close()
+		}
+
+		// A viewer holds host:read but must not be able to make the server
+		// open a connection to a managed host.
+		// A real UUID: the all-zeros one is rejected as invalid input before
+		// the permission check runs, which would assert nothing about RBAC.
+		hid := uuid.New()
+		req3 := asRole(t, "POST",
+			url+"/api/v1/hosts/"+hid.String()+"/connectivity:check",
+			auth.RoleViewer, nil)
+		// Required header, or the request 400s on validation before RBAC runs
+		// and the assertion proves nothing.
+		req3.Header.Set("Idempotency-Key", "ac21-"+hid.String())
+		r3 := doReq(t, req3)
+		defer r3.Body.Close()
+		if r3.StatusCode != http.StatusForbidden {
+			t.Errorf("viewer connectivity:check = %d, want 403: it is a side effect, not a read", r3.StatusCode)
+		}
+	})
+}

internal/server/api_license_test.go

@@ -16,6 +16,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/Hanalyx/openwatch/internal/auth"
 	"github.com/Hanalyx/openwatch/internal/license"
 	"github.com/golang-jwt/jwt/v5"
 )
@@ -94,9 +95,10 @@ func TestAPI_License_FreeTier(t *testing.T) {
 func TestAPI_License_VerifyTamperedJWT(t *testing.T) {
 	t.Run("api-license/AC-05", func(t *testing.T) {
 		url, _ := freshAPIServer(t)
-		body := strings.NewReader(`{"license_jwt":"not.a.valid"}`)
-		req, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", body)
-		req.Header.Set("Content-Type", "application/json")
+		// :verify is admin-gated: anonymously it is a signature and
+		// entitlement oracle on an /admin/ path.
+		req := asRole(t, "POST", url+"/api/v1/admin/license:verify", auth.RoleAdmin,
+			map[string]string{"license_jwt": "not.a.valid"})
 		resp := doReq(t, req)
 		defer resp.Body.Close()
 		if resp.StatusCode != http.StatusOK {
@@ -222,9 +224,8 @@ func TestAPI_License_VerifyValidJWT(t *testing.T) {
 		url, _ := freshAPIServer(t)
 		jwtBlob := mintTestLicenseJWT(t, []string{"premium_diagnostics", "remediation_execution"})
 
-		body := strings.NewReader(`{"license_jwt":"` + jwtBlob + `"}`)
-		req, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", body)
-		req.Header.Set("Content-Type", "application/json")
+		req := asRole(t, "POST", url+"/api/v1/admin/license:verify", auth.RoleAdmin,
+			map[string]string{"license_jwt": jwtBlob})
 		resp := doReq(t, req)
 		defer resp.Body.Close()
 		var got struct {
@@ -253,9 +254,8 @@ func TestAPI_License_VerifyDoesNotInstall(t *testing.T) {
 		url, _ := freshAPIServer(t)
 		jwtBlob := mintTestLicenseJWT(t, []string{"premium_diagnostics"})
 
-		body := strings.NewReader(`{"license_jwt":"` + jwtBlob + `"}`)
-		req, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", body)
-		req.Header.Set("Content-Type", "application/json")
+		req := asRole(t, "POST", url+"/api/v1/admin/license:verify", auth.RoleAdmin,
+			map[string]string{"license_jwt": jwtBlob})
 		resp := doReq(t, req)
 		io.Copy(io.Discard, resp.Body)
 		resp.Body.Close()

internal/server/api_signoff_test.go

@@ -319,7 +319,9 @@ func TestSignoff_DoD15_PolicyReload(t *testing.T) {
 func TestSignoff_DoD16_QueueWorkerCorrelation(t *testing.T) {
 	t.Run("release-stage-0-signoff/AC-10", func(t *testing.T) {
 		url, pool := freshAPIServer(t)
-		req, _ := http.NewRequest("POST", url+"/api/v1/diagnostics:enqueue-test-job", nil)
+		// enqueue-test-job writes to the real job queue and is now admin-gated:
+		// unauthenticated it was a denial-of-service primitive.
+		req := asRole(t, "POST", url+"/api/v1/diagnostics:enqueue-test-job", auth.RoleAdmin, nil)
 		req.Header.Set("X-Correlation-Id", "req-end2end-001")
 		resp := doReq(t, req)
 		defer resp.Body.Close()

internal/server/api_system_connectivity_test.go

@@ -404,7 +404,7 @@ func TestAPI_HostConnectivity_Check_NotFound_Returns404(t *testing.T) {
 	t.Run("api-host-connectivity-check/AC-03", func(t *testing.T) {
 		url, _ := freshAPIServer(t)
 		missing, _ := uuid.NewV7()
-		req := asRole(t, "POST", url+"/api/v1/hosts/"+missing.String()+"/connectivity:check", auth.RoleViewer, nil)
+		req := asRole(t, "POST", url+"/api/v1/hosts/"+missing.String()+"/connectivity:check", auth.RoleOpsLead, nil)
 		req.Header.Set("Idempotency-Key", "test-key-"+missing.String())
 		resp, err := http.DefaultClient.Do(req)
 		if err != nil {
@@ -429,7 +429,7 @@ func TestAPI_HostConnectivity_Check_MissingIdempotencyKey_Returns400(t *testing.
 			 VALUES ($1, $2, $3::inet, $4)`,
 			id, "h-"+id.String(), "192.0.2.10", uid)
 
-		req := asRole(t, "POST", url+"/api/v1/hosts/"+id.String()+"/connectivity:check", auth.RoleViewer, nil)
+		req := asRole(t, "POST", url+"/api/v1/hosts/"+id.String()+"/connectivity:check", auth.RoleOpsLead, nil)
 		// Intentionally no Idempotency-Key.
 		resp, err := http.DefaultClient.Do(req)
 		if err != nil {

internal/server/handlers.go

@@ -276,7 +276,13 @@ func (h *handlers) GetLicense(w http.ResponseWriter, r *http.Request) {
 		resp.Tier = api.LicenseStateResponseTier(lic.Tier)
 		resp.Status = api.LicenseStateResponseStatus(lic.Status)
 		resp.Features = featuresToStrings(lic.Features)
-		resp.CustomerId = &lic.CustomerID
+		// customer_id identifies the paying organisation. The tier, status and
+		// feature list are operational facts a UI needs before login, but the
+		// customer identity is not, and this route is reachable anonymously.
+		// Disclose it only to a caller who can already read system config.
+		if auth.FromContext(r.Context()).HasPermission(auth.SystemRead) {
+			resp.CustomerId = &lic.CustomerID
+		}
 		exp := lic.ExpiresAt
 		resp.ExpiresAt = &exp
 		grace := lic.InGracePeriod
@@ -290,6 +296,13 @@ func (h *handlers) GetLicense(w http.ResponseWriter, r *http.Request) {
 // PostAdminLicenseVerify dry-run validates a JWT without installing.
 // Spec: app/specs/api/license.spec.yaml AC-4, AC-5, AC-6.
 func (h *handlers) PostAdminLicenseVerify(w http.ResponseWriter, r *http.Request) {
+	// Anonymous, this is a signature and entitlement oracle on an /admin/
+	// path: submit any JWT and learn whether it verifies, which tier and
+	// features it carries, when it expires, and whether it was signed with the
+	// previous key. That last one leaks key-rotation state. Admin read.
+	if denied := auth.EnforcePermission(w, r, auth.SystemRead); denied {
+		return
+	}
 	var req api.LicenseVerifyRequest
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
 		writeError(w, http.StatusBadRequest, "validation.field_required", "client",
@@ -807,6 +820,14 @@ func (h *handlers) PostDiagnosticsEvaluateAlert(w http.ResponseWriter, r *http.R
 // carrying the request's correlation_id.
 // Spec release-stage-0-signoff AC-10.
 func (h *handlers) PostDiagnosticsEnqueueTestJob(w http.ResponseWriter, r *http.Request) {
+	// This writes to the REAL job queue. Unauthenticated it is a denial-of-
+	// service primitive and a source of unbounded table growth: anyone who can
+	// reach the port can enqueue without limit. It is a Stage-0 walking-
+	// skeleton demo, so gate it behind the same permission as any other
+	// system-mutating operation.
+	if denied := auth.EnforcePermission(w, r, auth.SystemConfigWrite); denied {
+		return
+	}
 	jobID, err := queue.Enqueue(r.Context(), h.pool, "diagnostics.test_job", nil)
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "queue.enqueue_failed", "server",

internal/server/host_connectivity_check_handler.go

@@ -32,7 +32,13 @@ func (h *handlers) PostHostConnectivityCheck(
 	id openapitypes.UUID,
 	_ api.PostHostConnectivityCheckParams,
 ) {
-	if denied := auth.EnforcePermission(w, r, auth.HostRead); denied {
+	// This is not a read. It makes the server open an SSH or ICMP connection
+	// to a managed host on demand, so it is a side effect a viewer should not
+	// be able to trigger. A dedicated permission already existed in the
+	// registry and was never wired to anything; wire it. Viewer and auditor
+	// lose this (they hold host:read but not host:connectivity_check);
+	// ops_lead, security_admin and admin keep it.
+	if denied := auth.EnforcePermission(w, r, auth.HostConnectivityCheck); denied {
 		return
 	}
 	if h.liveSvc == nil {

specs/system/rbac.spec.yaml

@@ -1,7 +1,7 @@
 spec:
   id: system-rbac
   title: RBAC permission registry and middleware
-  version: "1.2.0"
+  version: "1.3.0"
   status: approved
   tier: 1
 
@@ -175,3 +175,21 @@ spec:
         ErrRoleExceedsGrant and names the offending permissions so the API can
         return them.
       priority: critical
+    - id: AC-21
+      description: >
+        Privileged surfaces are not reachable anonymously, and identity is not
+        disclosed to a caller who has none. POST /diagnostics:enqueue-test-job
+        writes to the real job queue and MUST require an admin-mutating
+        permission: unauthenticated it is a denial-of-service primitive and a
+        source of unbounded table growth. POST /admin/license:verify MUST
+        require an admin read: anonymously it answers whether any submitted JWT
+        verifies and reports its tier, features, expiry, and whether it was
+        signed with the previous key, which leaks key-rotation state. POST
+        /hosts/{id}/connectivity:check MUST require host:connectivity_check
+        rather than host:read, because it makes the server open an SSH or ICMP
+        connection to a managed host on demand; that is a side effect, not a
+        read, and viewer and auditor therefore lose it. GET /license MUST omit
+        customer_id unless the caller holds system:read, since the tier and
+        feature list are operational facts a pre-login UI needs but the paying
+        organisation's identity is not.
+      priority: critical