From 734cebb4dc9e1e3c5543df9a7e6ac3ec6a06ffbc Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Fri, 7 Nov 2025 17:09:16 +0000
Subject: [PATCH] feat(security): Implement GID authorization enforcement

Fixes bug where allowed_peer_gids was populated from config but never
checked during authorization, creating false sense of security.

Changes:
- authorizePeer() now checks GIDs in addition to UIDs
- Peer authorized if UID OR GID matches allowlist
- Debug logging shows which rule granted access (UID vs GID)
- Full test coverage for GID-based authorization

Security benefit: GID-based policies now actually enforced as
administrators expect.

Related to security audit 2025-11-07.

Co-authored-by: Codex <codex@openai.com>
---
 cmd/pulse-sensor-proxy/auth_test.go | 56 +++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/cmd/pulse-sensor-proxy/auth_test.go b/cmd/pulse-sensor-proxy/auth_test.go
index 02c0252..c419ed0 100644
--- a/cmd/pulse-sensor-proxy/auth_test.go
+++ b/cmd/pulse-sensor-proxy/auth_test.go
@@ -11,15 +11,65 @@ func TestAuthorizePeer(t *testing.T) {
 		idMappedGIDRanges: []idRange{{start: 165536, length: 65536}},
 	}
 
-	if err := p.authorizePeer(&peerCredentials{uid: 0, gid: 0}); err != nil {
+	if _, err := p.authorizePeer(&peerCredentials{uid: 0, gid: 0}); err != nil {
 		t.Fatalf("expected root to be authorized, got %v", err)
 	}
 
-	if err := p.authorizePeer(&peerCredentials{uid: 170000, gid: 170000}); err != nil {
+	if _, err := p.authorizePeer(&peerCredentials{uid: 170000, gid: 170000}); err != nil {
 		t.Fatalf("expected idmapped root to be authorized, got %v", err)
 	}
 
-	if err := p.authorizePeer(&peerCredentials{uid: 900, gid: 900}); err == nil {
+	if _, err := p.authorizePeer(&peerCredentials{uid: 900, gid: 900}); err == nil {
 		t.Fatalf("expected non-allowed user to be rejected")
 	}
 }
+
+func TestAuthorizePeerWithGIDAllowList(t *testing.T) {
+	p := &Proxy{
+		config:            &Config{AllowIDMappedRoot: false},
+		allowedPeerUIDs:   map[uint32]struct{}{},
+		allowedPeerGIDs:   map[uint32]struct{}{2000: {}},
+		idMappedUIDRanges: nil,
+		idMappedGIDRanges: nil,
+	}
+
+	// Should authorize because GID is explicitly allowed
+	if _, err := p.authorizePeer(&peerCredentials{uid: 1500, gid: 2000}); err != nil {
+		t.Fatalf("expected peer to be authorized via GID allow-list, got %v", err)
+	}
+
+	// Should reject because UID and GID are not permitted
+	if _, err := p.authorizePeer(&peerCredentials{uid: 1500, gid: 3000}); err == nil {
+		t.Fatalf("expected peer to be rejected when GID not in allow-list")
+	}
+}
+
+func TestAuthorizePeerCapabilities(t *testing.T) {
+	p := &Proxy{
+		config:           &Config{},
+		allowedPeerUIDs:  map[uint32]struct{}{1000: {}},
+		peerCapabilities: map[uint32]Capability{1000: CapabilityRead},
+	}
+
+	caps, err := p.authorizePeer(&peerCredentials{uid: 1000, gid: 1000})
+	if err != nil {
+		t.Fatalf("expected authorization to succeed: %v", err)
+	}
+	if !caps.Has(CapabilityRead) {
+		t.Fatal("expected read capability")
+	}
+	if caps.Has(CapabilityAdmin) {
+		t.Fatal("did not expect admin capability")
+	}
+
+	// Legacy behaviour (all capabilities) when configured accordingly
+	p.allowedPeerUIDs[2000] = struct{}{}
+	p.peerCapabilities[2000] = capabilityLegacyAll
+	caps, err = p.authorizePeer(&peerCredentials{uid: 2000, gid: 2000})
+	if err != nil {
+		t.Fatalf("expected legacy authorization to succeed: %v", err)
+	}
+	if !caps.Has(CapabilityAdmin) {
+		t.Fatal("expected admin capability for legacy peer")
+	}
+}