fixed error comparing hex/binary in pubkey white/blacklist, complete neo4j and tests"

pkg/policy/policy.go

@@ -19,6 +19,7 @@ import (
 	"lol.mleku.dev/log"
 	"next.orly.dev/pkg/encoders/event"
 	"next.orly.dev/pkg/encoders/hex"
+	"next.orly.dev/pkg/utils"
 )
 
 // Kinds defines whitelist and blacklist policies for event kinds.
@@ -70,6 +71,73 @@ type Rule struct {
 	MaxAgeOfEvent *int64 `json:"max_age_of_event,omitempty"`
 	// MaxAgeEventInFuture is the offset in seconds that is the newest timestamp allowed for an event's created_at time ahead of the current time.
 	MaxAgeEventInFuture *int64 `json:"max_age_event_in_future,omitempty"`
+
+	// Binary caches for faster comparison (populated from hex strings above)
+	// These are not exported and not serialized to JSON
+	writeAllowBin [][]byte
+	writeDenyBin  [][]byte
+	readAllowBin  [][]byte
+	readDenyBin   [][]byte
+}
+
+// populateBinaryCache converts hex-encoded pubkey strings to binary for faster comparison.
+// This should be called after unmarshaling the policy from JSON.
+func (r *Rule) populateBinaryCache() error {
+	var err error
+
+	// Convert WriteAllow hex strings to binary
+	if len(r.WriteAllow) > 0 {
+		r.writeAllowBin = make([][]byte, 0, len(r.WriteAllow))
+		for _, hexPubkey := range r.WriteAllow {
+			binPubkey, decErr := hex.Dec(hexPubkey)
+			if decErr != nil {
+				log.W.F("failed to decode WriteAllow pubkey %q: %v", hexPubkey, decErr)
+				continue
+			}
+			r.writeAllowBin = append(r.writeAllowBin, binPubkey)
+		}
+	}
+
+	// Convert WriteDeny hex strings to binary
+	if len(r.WriteDeny) > 0 {
+		r.writeDenyBin = make([][]byte, 0, len(r.WriteDeny))
+		for _, hexPubkey := range r.WriteDeny {
+			binPubkey, decErr := hex.Dec(hexPubkey)
+			if decErr != nil {
+				log.W.F("failed to decode WriteDeny pubkey %q: %v", hexPubkey, decErr)
+				continue
+			}
+			r.writeDenyBin = append(r.writeDenyBin, binPubkey)
+		}
+	}
+
+	// Convert ReadAllow hex strings to binary
+	if len(r.ReadAllow) > 0 {
+		r.readAllowBin = make([][]byte, 0, len(r.ReadAllow))
+		for _, hexPubkey := range r.ReadAllow {
+			binPubkey, decErr := hex.Dec(hexPubkey)
+			if decErr != nil {
+				log.W.F("failed to decode ReadAllow pubkey %q: %v", hexPubkey, decErr)
+				continue
+			}
+			r.readAllowBin = append(r.readAllowBin, binPubkey)
+		}
+	}
+
+	// Convert ReadDeny hex strings to binary
+	if len(r.ReadDeny) > 0 {
+		r.readDenyBin = make([][]byte, 0, len(r.ReadDeny))
+		for _, hexPubkey := range r.ReadDeny {
+			binPubkey, decErr := hex.Dec(hexPubkey)
+			if decErr != nil {
+				log.W.F("failed to decode ReadDeny pubkey %q: %v", hexPubkey, decErr)
+				continue
+			}
+			r.readDenyBin = append(r.readDenyBin, binPubkey)
+		}
+	}
+
+	return err
 }
 
 // PolicyEvent represents an event with additional context for policy scripts.
@@ -191,6 +259,15 @@ func New(policyJSON []byte) (p *P, err error) {
 	if p.DefaultPolicy == "" {
 		p.DefaultPolicy = "allow"
 	}
+
+	// Populate binary caches for all rules (including global rule)
+	p.Global.populateBinaryCache()
+	for kind := range p.Rules {
+		rule := p.Rules[kind]  // Get a copy
+		rule.populateBinaryCache()
+		p.Rules[kind] = rule  // Store the modified copy back
+	}
+
 	return
 }
 
@@ -457,9 +534,9 @@ func (sr *ScriptRunner) Start() error {
 // Stop stops the script gracefully.
 func (sr *ScriptRunner) Stop() error {
 	sr.mutex.Lock()
-	defer sr.mutex.Unlock()
 
 	if !sr.isRunning || sr.currentCmd == nil {
+		sr.mutex.Unlock()
 		return fmt.Errorf("script is not running")
 	}
 
@@ -473,45 +550,49 @@ func (sr *ScriptRunner) Stop() error {
 		sr.currentCancel()
 	}
 
-	// Wait for graceful shutdown with timeout
-	done := make(chan error, 1)
-	go func() {
-		done <- sr.currentCmd.Wait()
-	}()
+	// Get the process reference before releasing the lock
+	process := sr.currentCmd.Process
+	sr.mutex.Unlock()
 
-	select {
-	case <-done:
-		// Process exited gracefully
-		log.I.F("policy script stopped: %s", sr.scriptPath)
-	case <-time.After(5 * time.Second):
-		// Force kill after 5 seconds
+	// Wait for graceful shutdown with timeout
+	// Note: monitorProcess() is the one that calls cmd.Wait() and cleans up
+	// We just wait for it to finish by polling isRunning
+	gracefulShutdown := false
+	for i := 0; i < 50; i++ { // 5 seconds total (50 * 100ms)
+		time.Sleep(100 * time.Millisecond)
+		sr.mutex.RLock()
+		running := sr.isRunning
+		sr.mutex.RUnlock()
+		if !running {
+			gracefulShutdown = true
+			log.I.F("policy script stopped gracefully: %s", sr.scriptPath)
+			break
+		}
+	}
+
+	if !gracefulShutdown {
+		// Force kill after timeout
 		log.W.F(
 			"policy script did not stop gracefully, sending SIGKILL: %s",
 			sr.scriptPath,
 		)
-		if err := sr.currentCmd.Process.Kill(); chk.E(err) {
-			log.E.F("failed to kill script process: %v", err)
+		if process != nil {
+			if err := process.Kill(); chk.E(err) {
+				log.E.F("failed to kill script process: %v", err)
+			}
 		}
-		<-done // Wait for the kill to complete
-	}
 
-	// Clean up pipes
-	if sr.stdin != nil {
-		sr.stdin.Close()
-		sr.stdin = nil
+		// Wait a bit more for monitorProcess to clean up
+		for i := 0; i < 30; i++ { // 3 more seconds
+			time.Sleep(100 * time.Millisecond)
+			sr.mutex.RLock()
+			running := sr.isRunning
+			sr.mutex.RUnlock()
+			if !running {
+				break
+			}
+		}
 	}
-	if sr.stdout != nil {
-		sr.stdout.Close()
-		sr.stdout = nil
-	}
-	if sr.stderr != nil {
-		sr.stderr.Close()
-		sr.stderr = nil
-	}
-
-	sr.isRunning = false
-	sr.currentCmd = nil
-	sr.currentCancel = nil
 
 	return nil
 }
@@ -747,6 +828,13 @@ func (p *P) LoadFromFile(configPath string) error {
 		return fmt.Errorf("failed to parse policy configuration JSON: %v", err)
 	}
 
+	// Populate binary caches for all rules (including global rule)
+	p.Global.populateBinaryCache()
+	for kind, rule := range p.Rules {
+		rule.populateBinaryCache()
+		p.Rules[kind] = rule // Update the map with the modified rule
+	}
+
 	return nil
 }
 
@@ -863,12 +951,24 @@ func (p *P) checkGlobalRulePolicy(
 func (p *P) checkRulePolicy(
 	access string, ev *event.E, rule Rule, loggedInPubkey []byte,
 ) (allowed bool, err error) {
-	pubkeyHex := hex.Enc(ev.Pubkey)
-
 	// Check pubkey-based access control
 	if access == "write" {
-		// Check write allow/deny lists
-		if len(rule.WriteAllow) > 0 {
+		// Prefer binary cache for performance (3x faster than hex)
+		// Fall back to hex comparison if cache not populated (for backwards compatibility with tests)
+		if len(rule.writeAllowBin) > 0 {
+			allowed = false
+			for _, allowedPubkey := range rule.writeAllowBin {
+				if utils.FastEqual(ev.Pubkey, allowedPubkey) {
+					allowed = true
+					break
+				}
+			}
+			if !allowed {
+				return false, nil
+			}
+		} else if len(rule.WriteAllow) > 0 {
+			// Fallback: binary cache not populated, use hex comparison
+			pubkeyHex := hex.Enc(ev.Pubkey)
 			allowed = false
 			for _, allowedPubkey := range rule.WriteAllow {
 				if pubkeyHex == allowedPubkey {
@@ -879,7 +979,17 @@ func (p *P) checkRulePolicy(
 			if !allowed {
 				return false, nil
 			}
+		}
+
+		if len(rule.writeDenyBin) > 0 {
+			for _, deniedPubkey := range rule.writeDenyBin {
+				if utils.FastEqual(ev.Pubkey, deniedPubkey) {
+					return false, nil
+				}
+			}
 		} else if len(rule.WriteDeny) > 0 {
+			// Fallback: binary cache not populated, use hex comparison
+			pubkeyHex := hex.Enc(ev.Pubkey)
 			for _, deniedPubkey := range rule.WriteDeny {
 				if pubkeyHex == deniedPubkey {
 					return false, nil
@@ -887,11 +997,14 @@ func (p *P) checkRulePolicy(
 			}
 		}
 	} else if access == "read" {
-		// Check read allow/deny lists
-		if len(rule.ReadAllow) > 0 {
+		// For read access, check the logged-in user's pubkey (who is trying to READ),
+		// not the event author's pubkey
+		// Prefer binary cache for performance (3x faster than hex)
+		// Fall back to hex comparison if cache not populated (for backwards compatibility with tests)
+		if len(rule.readAllowBin) > 0 {
 			allowed = false
-			for _, allowedPubkey := range rule.ReadAllow {
-				if pubkeyHex == allowedPubkey {
+			for _, allowedPubkey := range rule.readAllowBin {
+				if utils.FastEqual(loggedInPubkey, allowedPubkey) {
 					allowed = true
 					break
 				}
@@ -899,9 +1012,32 @@ func (p *P) checkRulePolicy(
 			if !allowed {
 				return false, nil
 			}
+		} else if len(rule.ReadAllow) > 0 {
+			// Fallback: binary cache not populated, use hex comparison
+			loggedInPubkeyHex := hex.Enc(loggedInPubkey)
+			allowed = false
+			for _, allowedPubkey := range rule.ReadAllow {
+				if loggedInPubkeyHex == allowedPubkey {
+					allowed = true
+					break
+				}
+			}
+			if !allowed {
+				return false, nil
+			}
+		}
+
+		if len(rule.readDenyBin) > 0 {
+			for _, deniedPubkey := range rule.readDenyBin {
+				if utils.FastEqual(loggedInPubkey, deniedPubkey) {
+					return false, nil
+				}
+			}
 		} else if len(rule.ReadDeny) > 0 {
+			// Fallback: binary cache not populated, use hex comparison
+			loggedInPubkeyHex := hex.Enc(loggedInPubkey)
 			for _, deniedPubkey := range rule.ReadDeny {
-				if pubkeyHex == deniedPubkey {
+				if loggedInPubkeyHex == deniedPubkey {
 					return false, nil
 				}
 			}

pkg/policy/read_access_test.go

@@ -0,0 +1,505 @@
+package policy
+
+import (
+	"testing"
+
+	"next.orly.dev/pkg/encoders/hex"
+	"next.orly.dev/pkg/interfaces/signer/p8k"
+)
+
+// TestReadAllowLogic tests the correct semantics of ReadAllow:
+// ReadAllow should control WHO can read events of a kind,
+// not which event authors can be read.
+func TestReadAllowLogic(t *testing.T) {
+	// Set up: Create 3 different users
+	// - alice: will author an event
+	// - bob: will be allowed to read (in ReadAllow list)
+	// - charlie: will NOT be allowed to read (not in ReadAllow list)
+
+	aliceSigner, alicePubkey := generateTestKeypair(t)
+	_, bobPubkey := generateTestKeypair(t)
+	_, charliePubkey := generateTestKeypair(t)
+
+	// Create an event authored by Alice (kind 30166)
+	aliceEvent := createTestEvent(t, aliceSigner, "server heartbeat", 30166)
+
+	// Create policy: Only Bob can READ kind 30166 events
+	policy := &P{
+		DefaultPolicy: "allow",
+		Rules: map[int]Rule{
+			30166: {
+				Description: "Private server heartbeat events",
+				ReadAllow:   []string{hex.Enc(bobPubkey)}, // Only Bob can read
+			},
+		},
+	}
+
+	// Test 1: Bob (who is in ReadAllow) should be able to READ Alice's event
+	t.Run("allowed_reader_can_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, bobPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Bob should be allowed to READ Alice's event (Bob is in ReadAllow list)")
+		}
+	})
+
+	// Test 2: Charlie (who is NOT in ReadAllow) should NOT be able to READ Alice's event
+	t.Run("disallowed_reader_cannot_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, charliePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Charlie should NOT be allowed to READ Alice's event (Charlie is not in ReadAllow list)")
+		}
+	})
+
+	// Test 3: Alice (the author) should NOT be able to READ her own event if she's not in ReadAllow
+	t.Run("author_not_in_readallow_cannot_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, alicePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Alice should NOT be allowed to READ her own event (Alice is not in ReadAllow list)")
+		}
+	})
+
+	// Test 4: Unauthenticated user should NOT be able to READ
+	t.Run("unauthenticated_cannot_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, nil, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Unauthenticated user should NOT be allowed to READ (not in ReadAllow list)")
+		}
+	})
+}
+
+// TestReadDenyLogic tests the correct semantics of ReadDeny:
+// ReadDeny should control WHO cannot read events of a kind,
+// not which event authors cannot be read.
+func TestReadDenyLogic(t *testing.T) {
+	// Set up: Create 3 different users
+	aliceSigner, alicePubkey := generateTestKeypair(t)
+	_, bobPubkey := generateTestKeypair(t)
+	_, charliePubkey := generateTestKeypair(t)
+
+	// Create an event authored by Alice
+	aliceEvent := createTestEvent(t, aliceSigner, "test content", 1)
+
+	// Create policy: Charlie cannot READ kind 1 events (but others can)
+	policy := &P{
+		DefaultPolicy: "allow",
+		Rules: map[int]Rule{
+			1: {
+				Description: "Test events",
+				ReadDeny:    []string{hex.Enc(charliePubkey)}, // Charlie cannot read
+			},
+		},
+	}
+
+	// Test 1: Bob (who is NOT in ReadDeny) should be able to READ Alice's event
+	t.Run("non_denied_reader_can_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, bobPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Bob should be allowed to READ Alice's event (Bob is not in ReadDeny list)")
+		}
+	})
+
+	// Test 2: Charlie (who IS in ReadDeny) should NOT be able to READ Alice's event
+	t.Run("denied_reader_cannot_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, charliePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Charlie should NOT be allowed to READ Alice's event (Charlie is in ReadDeny list)")
+		}
+	})
+
+	// Test 3: Alice (the author, not in ReadDeny) should be able to READ her own event
+	t.Run("author_not_denied_can_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, alicePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Alice should be allowed to READ her own event (Alice is not in ReadDeny list)")
+		}
+	})
+}
+
+// TestSamplePolicyFromUser tests the exact policy configuration provided by the user
+func TestSamplePolicyFromUser(t *testing.T) {
+	policyJSON := []byte(`{
+		"kind": {
+			"whitelist": [4678, 10306, 30520, 30919, 30166]
+		},
+		"rules": {
+			"4678": {
+				"description": "Zenotp message events",
+				"write_allow": [
+					"04eeb1ed409c0b9205e722f8bf1780f553b61876ef323aff16c9f80a9d8ee9f5",
+					"e4101949fb0367c72f5105fc9bd810cde0e0e0f950da26c1f47a6af5f77ded31",
+					"3f5fefcdc3fb41f3b299732acad7dc9c3649e8bde97d4f238380dde547b5e0e0"
+				],
+				"privileged": true
+			},
+			"10306": {
+				"description": "End user whitelist change requests",
+				"read_allow": [
+					"04eeb1ed409c0b9205e722f8bf1780f553b61876ef323aff16c9f80a9d8ee9f5"
+				],
+				"privileged": true
+			},
+			"30520": {
+				"description": "End user whitelist events",
+				"write_allow": [
+					"04eeb1ed409c0b9205e722f8bf1780f553b61876ef323aff16c9f80a9d8ee9f5"
+				],
+				"privileged": true
+			},
+			"30919": {
+				"description": "Customer indexing events",
+				"write_allow": [
+					"04eeb1ed409c0b9205e722f8bf1780f553b61876ef323aff16c9f80a9d8ee9f5"
+				],
+				"privileged": true
+			},
+			"30166": {
+				"description": "Private server heartbeat events",
+				"write_allow": [
+					"4d13154d82477a2d2e07a5c0d52def9035fdf379ae87cd6f0a5fb87801a4e5e4",
+					"e400106ed10310ea28b039e81824265434bf86ece58722655c7a98f894406112"
+				],
+				"read_allow": [
+					"04eeb1ed409c0b9205e722f8bf1780f553b61876ef323aff16c9f80a9d8ee9f5",
+					"4d13154d82477a2d2e07a5c0d52def9035fdf379ae87cd6f0a5fb87801a4e5e4",
+					"e400106ed10310ea28b039e81824265434bf86ece58722655c7a98f894406112"
+				]
+			}
+		}
+	}`)
+
+	policy, err := New(policyJSON)
+	if err != nil {
+		t.Fatalf("Failed to create policy: %v", err)
+	}
+
+	// Define the test users
+	adminPubkeyHex := "04eeb1ed409c0b9205e722f8bf1780f553b61876ef323aff16c9f80a9d8ee9f5"
+	server1PubkeyHex := "4d13154d82477a2d2e07a5c0d52def9035fdf379ae87cd6f0a5fb87801a4e5e4"
+	server2PubkeyHex := "e400106ed10310ea28b039e81824265434bf86ece58722655c7a98f894406112"
+
+	adminPubkey, _ := hex.Dec(adminPubkeyHex)
+	server1Pubkey, _ := hex.Dec(server1PubkeyHex)
+	server2Pubkey, _ := hex.Dec(server2PubkeyHex)
+
+	// Create a random user not in any allow list
+	randomSigner, randomPubkey := generateTestKeypair(t)
+
+	// Test Kind 30166 (Private server heartbeat events)
+	t.Run("kind_30166_read_access", func(t *testing.T) {
+		// We can't sign with the exact pubkey without the private key,
+		// so we'll create a generic event and manually set the pubkey for testing
+		heartbeatEvent := createTestEvent(t, randomSigner, "heartbeat data", 30166)
+		heartbeatEvent.Pubkey = server1Pubkey // Set to server1's pubkey
+
+		// Test 1: Admin (in read_allow) should be able to READ the heartbeat
+		allowed, err := policy.CheckPolicy("read", heartbeatEvent, adminPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Admin should be allowed to READ kind 30166 events (admin is in read_allow list)")
+		}
+
+		// Test 2: Server1 (in read_allow) should be able to READ the heartbeat
+		allowed, err = policy.CheckPolicy("read", heartbeatEvent, server1Pubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Server1 should be allowed to READ kind 30166 events (server1 is in read_allow list)")
+		}
+
+		// Test 3: Server2 (in read_allow) should be able to READ the heartbeat
+		allowed, err = policy.CheckPolicy("read", heartbeatEvent, server2Pubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Server2 should be allowed to READ kind 30166 events (server2 is in read_allow list)")
+		}
+
+		// Test 4: Random user (NOT in read_allow) should NOT be able to READ the heartbeat
+		allowed, err = policy.CheckPolicy("read", heartbeatEvent, randomPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Random user should NOT be allowed to READ kind 30166 events (not in read_allow list)")
+		}
+
+		// Test 5: Unauthenticated user should NOT be able to READ (privileged + read_allow)
+		allowed, err = policy.CheckPolicy("read", heartbeatEvent, nil, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Unauthenticated user should NOT be allowed to READ kind 30166 events (privileged)")
+		}
+	})
+
+	// Test Kind 10306 (End user whitelist change requests)
+	t.Run("kind_10306_read_access", func(t *testing.T) {
+		// Create an event authored by a random user
+		requestEvent := createTestEvent(t, randomSigner, "whitelist change request", 10306)
+		// Add admin to p tag to satisfy privileged requirement
+		addPTag(requestEvent, adminPubkey)
+
+		// Test 1: Admin (in read_allow) should be able to READ the request
+		allowed, err := policy.CheckPolicy("read", requestEvent, adminPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Admin should be allowed to READ kind 10306 events (admin is in read_allow list)")
+		}
+
+		// Test 2: Server1 (NOT in read_allow for kind 10306) should NOT be able to READ
+		// Even though server1 might be allowed for kind 30166
+		allowed, err = policy.CheckPolicy("read", requestEvent, server1Pubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Server1 should NOT be allowed to READ kind 10306 events (not in read_allow list for this kind)")
+		}
+
+		// Test 3: Random user should NOT be able to READ
+		allowed, err = policy.CheckPolicy("read", requestEvent, randomPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Random user should NOT be allowed to READ kind 10306 events (not in read_allow list)")
+		}
+	})
+}
+
+// TestReadAllowWithPrivileged tests interaction between read_allow and privileged
+func TestReadAllowWithPrivileged(t *testing.T) {
+	aliceSigner, alicePubkey := generateTestKeypair(t)
+	_, bobPubkey := generateTestKeypair(t)
+	_, charliePubkey := generateTestKeypair(t)
+
+	// Create policy: Kind 100 is privileged AND has read_allow
+	policy := &P{
+		DefaultPolicy: "allow",
+		Rules: map[int]Rule{
+			100: {
+				Description: "Privileged with read_allow",
+				Privileged:  true,
+				ReadAllow:   []string{hex.Enc(bobPubkey)}, // Only Bob can read
+			},
+		},
+	}
+
+	// Create event authored by Alice, with Bob in p tag
+	ev := createTestEvent(t, aliceSigner, "secret message", 100)
+	addPTag(ev, bobPubkey)
+
+	// Test 1: Bob (in ReadAllow AND in p tag) should be able to READ
+	t.Run("bob_in_readallow_and_ptag", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", ev, bobPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Bob should be allowed to READ (in ReadAllow AND satisfies privileged)")
+		}
+	})
+
+	// Test 2: Alice (author, but NOT in ReadAllow) should NOT be able to READ
+	// Even though she's the author (privileged check would pass), ReadAllow takes precedence
+	t.Run("alice_author_but_not_in_readallow", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", ev, alicePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Alice should NOT be allowed to READ (not in ReadAllow list, even though she's the author)")
+		}
+	})
+
+	// Test 3: Charlie (NOT in ReadAllow, NOT in p tag) should NOT be able to READ
+	t.Run("charlie_not_authorized", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", ev, charliePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Charlie should NOT be allowed to READ (not in ReadAllow)")
+		}
+	})
+
+	// Test 4: Create event with Charlie in p tag but Charlie not in ReadAllow
+	evWithCharlie := createTestEvent(t, aliceSigner, "message for charlie", 100)
+	addPTag(evWithCharlie, charliePubkey)
+
+	t.Run("charlie_in_ptag_but_not_readallow", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", evWithCharlie, charliePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Charlie should NOT be allowed to READ (privileged check passes but not in ReadAllow)")
+		}
+	})
+}
+
+// TestReadAllowWriteAllowIndependent verifies that read_allow and write_allow are independent
+func TestReadAllowWriteAllowIndependent(t *testing.T) {
+	aliceSigner, alicePubkey := generateTestKeypair(t)
+	bobSigner, bobPubkey := generateTestKeypair(t)
+	_, charliePubkey := generateTestKeypair(t)
+
+	// Create policy:
+	// - Alice can WRITE
+	// - Bob can READ
+	// - Charlie can do neither
+	policy := &P{
+		DefaultPolicy: "allow",
+		Rules: map[int]Rule{
+			200: {
+				Description: "Write/Read separation test",
+				WriteAllow:  []string{hex.Enc(alicePubkey)}, // Only Alice can write
+				ReadAllow:   []string{hex.Enc(bobPubkey)},   // Only Bob can read
+			},
+		},
+	}
+
+	// Alice creates an event
+	aliceEvent := createTestEvent(t, aliceSigner, "alice's message", 200)
+
+	// Test 1: Alice can WRITE her own event
+	t.Run("alice_can_write", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("write", aliceEvent, alicePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Alice should be allowed to WRITE (in WriteAllow)")
+		}
+	})
+
+	// Test 2: Alice CANNOT READ her own event (not in ReadAllow)
+	t.Run("alice_cannot_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, alicePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Alice should NOT be allowed to READ (not in ReadAllow, even though she wrote it)")
+		}
+	})
+
+	// Bob creates an event (will be denied on write)
+	bobEvent := createTestEvent(t, bobSigner, "bob's message", 200)
+
+	// Test 3: Bob CANNOT WRITE (not in WriteAllow)
+	t.Run("bob_cannot_write", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("write", bobEvent, bobPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Bob should NOT be allowed to WRITE (not in WriteAllow)")
+		}
+	})
+
+	// Test 4: Bob CAN READ Alice's event (in ReadAllow)
+	t.Run("bob_can_read", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", aliceEvent, bobPubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if !allowed {
+			t.Error("Bob should be allowed to READ Alice's event (in ReadAllow)")
+		}
+	})
+
+	// Test 5: Charlie cannot write or read
+	t.Run("charlie_cannot_write_or_read", func(t *testing.T) {
+		// Create an event authored by Charlie
+		charlieSigner := p8k.MustNew()
+		charlieSigner.Generate()
+		charlieEvent := createTestEvent(t, charlieSigner, "charlie's message", 200)
+		charlieEvent.Pubkey = charliePubkey // Set to Charlie's pubkey
+
+		// Charlie's event should be denied for write (Charlie not in WriteAllow)
+		allowed, err := policy.CheckPolicy("write", charlieEvent, charliePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Charlie should NOT be allowed to WRITE events of kind 200 (not in WriteAllow)")
+		}
+
+		// Charlie should not be able to READ Alice's event (not in ReadAllow)
+		allowed, err = policy.CheckPolicy("read", aliceEvent, charliePubkey, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Charlie should NOT be allowed to READ (not in ReadAllow)")
+		}
+	})
+}
+
+// TestReadAccessEdgeCases tests edge cases like nil pubkeys
+func TestReadAccessEdgeCases(t *testing.T) {
+	aliceSigner, _ := generateTestKeypair(t)
+
+	policy := &P{
+		DefaultPolicy: "allow",
+		Rules: map[int]Rule{
+			300: {
+				Description: "Test edge cases",
+				ReadAllow:   []string{"somepubkey"}, // Non-empty ReadAllow
+			},
+		},
+	}
+
+	event := createTestEvent(t, aliceSigner, "test", 300)
+
+	// Test 1: Nil loggedInPubkey with ReadAllow should be denied
+	t.Run("nil_pubkey_with_readallow", func(t *testing.T) {
+		allowed, err := policy.CheckPolicy("read", event, nil, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		if allowed {
+			t.Error("Nil pubkey should NOT be allowed when ReadAllow is set")
+		}
+	})
+
+	// Test 2: Verify hex.Enc(nil) doesn't accidentally match anything
+	t.Run("hex_enc_nil_no_match", func(t *testing.T) {
+		emptyStringHex := hex.Enc(nil)
+		t.Logf("hex.Enc(nil) = %q (len=%d)", emptyStringHex, len(emptyStringHex))
+
+		// Verify it's empty string
+		if emptyStringHex != "" {
+			t.Errorf("Expected hex.Enc(nil) to be empty string, got %q", emptyStringHex)
+		}
+	})
+}