Make policy validation write-only and add corresponding tests

Updated policy validation logic to apply only to write operations, ensuring constraints like max_expiry_duration and required tags do not affect read operations. Added corresponding test cases to verify behavior for both valid and invalid inputs. This change improves clarity between write and read validation rules.

bump tag to update binary

pkg/policy/new_fields_test.go

@@ -113,11 +113,11 @@ func TestMaxExpiryDuration(t *testing.T) {
 			expectAllow:       true,
 		},
 		{
-			name:              "valid expiry at exact limit",
+			name:              "expiry at exact limit rejected",
 			maxExpiryDuration: "PT1H",
-			eventExpiry:       3600, // exactly 1 hour
+			eventExpiry:       3600, // exactly 1 hour - >= means this is rejected
 			hasExpiryTag:      true,
-			expectAllow:       true,
+			expectAllow:       false,
 		},
 		{
 			name:              "expiry exceeds limit",
@@ -235,6 +235,79 @@ func TestMaxExpiryDurationPrecedence(t *testing.T) {
 	}
 }
 
+// Test that max_expiry_duration only applies to writes, not reads
+func TestMaxExpiryDurationWriteOnly(t *testing.T) {
+	signer, pubkey := generateTestKeypair(t)
+
+	// Policy with strict max_expiry_duration
+	policyJSON := []byte(`{
+		"default_policy": "allow",
+		"rules": {
+			"4": {
+				"description": "DM events with expiry",
+				"max_expiry_duration": "PT10M",
+				"privileged": true
+			}
+		}
+	}`)
+
+	policy, err := New(policyJSON)
+	if err != nil {
+		t.Fatalf("Failed to create policy: %v", err)
+	}
+
+	// Create event WITHOUT an expiry tag - this would fail write validation
+	// but should still be readable
+	ev := createTestEventForNewFields(t, signer, "test DM", 4)
+	if err := ev.Sign(signer); chk.E(err) {
+		t.Fatalf("Failed to sign: %v", err)
+	}
+
+	// Write should fail (no expiry tag when max_expiry_duration is set)
+	allowed, err := policy.CheckPolicy("write", ev, pubkey, "127.0.0.1")
+	if err != nil {
+		t.Fatalf("CheckPolicy write error: %v", err)
+	}
+	if allowed {
+		t.Error("Write should be denied for event without expiry tag when max_expiry_duration is set")
+	}
+
+	// Read should succeed (validation constraints don't apply to reads)
+	allowed, err = policy.CheckPolicy("read", ev, pubkey, "127.0.0.1")
+	if err != nil {
+		t.Fatalf("CheckPolicy read error: %v", err)
+	}
+	if !allowed {
+		t.Error("Read should be allowed - max_expiry_duration is write-only validation")
+	}
+
+	// Also test with an event that has expiry exceeding the limit
+	ev2 := createTestEventForNewFields(t, signer, "test DM 2", 4)
+	expiryTs := ev2.CreatedAt + 7200 // 2 hours - exceeds 10 minute limit
+	addTagString(ev2, "expiration", int64ToString(expiryTs))
+	if err := ev2.Sign(signer); chk.E(err) {
+		t.Fatalf("Failed to sign: %v", err)
+	}
+
+	// Write should fail (expiry exceeds limit)
+	allowed, err = policy.CheckPolicy("write", ev2, pubkey, "127.0.0.1")
+	if err != nil {
+		t.Fatalf("CheckPolicy write error: %v", err)
+	}
+	if allowed {
+		t.Error("Write should be denied for event with expiry exceeding max_expiry_duration")
+	}
+
+	// Read should still succeed
+	allowed, err = policy.CheckPolicy("read", ev2, pubkey, "127.0.0.1")
+	if err != nil {
+		t.Fatalf("CheckPolicy read error: %v", err)
+	}
+	if !allowed {
+		t.Error("Read should be allowed - max_expiry_duration is write-only validation")
+	}
+}
+
 // =============================================================================
 // ProtectedRequired Tests
 // =============================================================================
@@ -1071,6 +1144,94 @@ func TestNewFieldsInGlobalRule(t *testing.T) {
 	}
 }
 
+// =============================================================================
+// New() Validation Tests - Ensures invalid configs fail at load time
+// =============================================================================
+
+// TestNewRejectsInvalidMaxExpiryDuration verifies that New() fails fast when
+// given an invalid max_expiry_duration format like "T10M" instead of "PT10M".
+// This prevents silent failures where constraints are ignored.
+func TestNewRejectsInvalidMaxExpiryDuration(t *testing.T) {
+	tests := []struct {
+		name        string
+		json        string
+		expectError bool
+		errorMatch  string
+	}{
+		{
+			name: "valid PT10M format accepted",
+			json: `{
+				"rules": {
+					"4": {"max_expiry_duration": "PT10M"}
+				}
+			}`,
+			expectError: false,
+		},
+		{
+			name: "invalid T10M format (missing P prefix) rejected",
+			json: `{
+				"rules": {
+					"4": {"max_expiry_duration": "T10M"}
+				}
+			}`,
+			expectError: true,
+			errorMatch:  "max_expiry_duration",
+		},
+		{
+			name: "invalid 10M format (missing PT prefix) rejected",
+			json: `{
+				"rules": {
+					"4": {"max_expiry_duration": "10M"}
+				}
+			}`,
+			expectError: true,
+			errorMatch:  "max_expiry_duration",
+		},
+		{
+			name: "valid P7D format accepted",
+			json: `{
+				"rules": {
+					"1": {"max_expiry_duration": "P7D"}
+				}
+			}`,
+			expectError: false,
+		},
+		{
+			name: "invalid 7D format (missing P prefix) rejected",
+			json: `{
+				"rules": {
+					"1": {"max_expiry_duration": "7D"}
+				}
+			}`,
+			expectError: true,
+			errorMatch:  "max_expiry_duration",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			policy, err := New([]byte(tt.json))
+
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("New() should have rejected invalid config, but returned policy: %+v", policy)
+					return
+				}
+				if tt.errorMatch != "" && !contains(err.Error(), tt.errorMatch) {
+					t.Errorf("Error %q should contain %q", err.Error(), tt.errorMatch)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("New() unexpected error for valid config: %v", err)
+				}
+				if policy == nil {
+					t.Error("New() returned nil policy for valid config")
+				}
+			}
+		})
+	}
+}
+
 // =============================================================================
 // ValidateJSON Tests for New Fields
 // =============================================================================

pkg/policy/policy.go

@@ -468,11 +468,19 @@ func (p *P) UnmarshalJSON(data []byte) error {
 // New creates a new policy from JSON configuration.
 // If policyJSON is empty, returns a policy with default settings.
 // The default_policy field defaults to "allow" if not specified.
+// Returns an error if the policy JSON contains invalid values (e.g., invalid
+// ISO-8601 duration format for max_expiry_duration, invalid regex patterns, etc.).
 func New(policyJSON []byte) (p *P, err error) {
 	p = &P{
 		DefaultPolicy: "allow", // Set default value
 	}
 	if len(policyJSON) > 0 {
+		// Validate JSON before loading to fail fast on invalid configurations.
+		// This prevents silent failures where invalid values (like "T10M" instead
+		// of "PT10M" for max_expiry_duration) are ignored and constraints don't apply.
+		if err = p.ValidateJSON(policyJSON); err != nil {
+			return nil, fmt.Errorf("policy validation failed: %v", err)
+		}
 		if err = json.Unmarshal(policyJSON, p); chk.E(err) {
 			return nil, fmt.Errorf("failed to unmarshal policy JSON: %v", err)
 		}
@@ -1272,7 +1280,8 @@ func (p *P) checkRulePolicy(
 	}
 
 	// Check required tags
-	if len(rule.MustHaveTags) > 0 {
+	// Only apply for write access - we validate what goes in, not what comes out
+	if access == "write" && len(rule.MustHaveTags) > 0 {
 		for _, requiredTag := range rule.MustHaveTags {
 			if ev.Tags.GetFirst([]byte(requiredTag)) == nil {
 				return false, nil
@@ -1281,7 +1290,8 @@ func (p *P) checkRulePolicy(
 	}
 
 	// Check expiry time (uses maxExpirySeconds which is parsed from MaxExpiryDuration or MaxExpiry)
-	if rule.maxExpirySeconds != nil && *rule.maxExpirySeconds > 0 {
+	// Only apply for write access - we validate what goes in, not what comes out
+	if access == "write" && rule.maxExpirySeconds != nil && *rule.maxExpirySeconds > 0 {
 		expiryTag := ev.Tags.GetFirst([]byte("expiration"))
 		if expiryTag == nil {
 			return false, nil // Must have expiry if max_expiry is set
@@ -1294,7 +1304,7 @@ func (p *P) checkRulePolicy(
 			return false, nil // Invalid expiry format
 		}
 		maxAllowedExpiry := ev.CreatedAt + *rule.maxExpirySeconds
-		if expiryTs > maxAllowedExpiry {
+		if expiryTs >= maxAllowedExpiry {
 			log.D.F("expiration %d exceeds max allowed %d (created_at %d + max_expiry %d)",
 				expiryTs, maxAllowedExpiry, ev.CreatedAt, *rule.maxExpirySeconds)
 			return false, nil // Expiry too far in the future
@@ -1302,7 +1312,8 @@ func (p *P) checkRulePolicy(
 	}
 
 	// Check ProtectedRequired (NIP-70: events must have "-" tag)
-	if rule.ProtectedRequired {
+	// Only apply for write access - we validate what goes in, not what comes out
+	if access == "write" && rule.ProtectedRequired {
 		protectedTag := ev.Tags.GetFirst([]byte("-"))
 		if protectedTag == nil {
 			log.D.F("protected_required: event missing '-' tag (NIP-70)")
@@ -1311,7 +1322,8 @@ func (p *P) checkRulePolicy(
 	}
 
 	// Check IdentifierRegex (validates "d" tag values)
-	if rule.identifierRegexCache != nil {
+	// Only apply for write access - we validate what goes in, not what comes out
+	if access == "write" && rule.identifierRegexCache != nil {
 		dTags := ev.Tags.GetAll([]byte("d"))
 		if len(dTags) == 0 {
 			log.D.F("identifier_regex: event missing 'd' tag")
@@ -1328,7 +1340,8 @@ func (p *P) checkRulePolicy(
 	}
 
 	// Check MaxAgeOfEvent (maximum age of event in seconds)
-	if rule.MaxAgeOfEvent != nil && *rule.MaxAgeOfEvent > 0 {
+	// Only apply for write access - we validate what goes in, not what comes out
+	if access == "write" && rule.MaxAgeOfEvent != nil && *rule.MaxAgeOfEvent > 0 {
 		currentTime := time.Now().Unix()
 		maxAllowedTime := currentTime - *rule.MaxAgeOfEvent
 		if ev.CreatedAt < maxAllowedTime {
@@ -1337,7 +1350,8 @@ func (p *P) checkRulePolicy(
 	}
 
 	// Check MaxAgeEventInFuture (maximum time event can be in the future in seconds)
-	if rule.MaxAgeEventInFuture != nil && *rule.MaxAgeEventInFuture > 0 {
+	// Only apply for write access - we validate what goes in, not what comes out
+	if access == "write" && rule.MaxAgeEventInFuture != nil && *rule.MaxAgeEventInFuture > 0 {
 		currentTime := time.Now().Unix()
 		maxFutureTime := currentTime + *rule.MaxAgeEventInFuture
 		if ev.CreatedAt > maxFutureTime {
@@ -1784,7 +1798,7 @@ func (p *P) ValidateJSON(policyJSON []byte) error {
 		// Validate MaxExpiryDuration format
 		if rule.MaxExpiryDuration != "" {
 			if _, err := parseDuration(rule.MaxExpiryDuration); err != nil {
-				return fmt.Errorf("invalid max_expiry_duration %q in kind %d: %v", rule.MaxExpiryDuration, kind, err)
+				return fmt.Errorf("invalid max_expiry_duration %q in kind %d: %v (format must be ISO-8601 duration, e.g. \"PT10M\" for 10 minutes, \"P7D\" for 7 days, \"P1DT12H\" for 1 day 12 hours)", rule.MaxExpiryDuration, kind, err)
 			}
 		}
 		// Validate FollowsWhitelistAdmins pubkeys
@@ -1815,7 +1829,7 @@ func (p *P) ValidateJSON(policyJSON []byte) error {
 	// Validate global rule MaxExpiryDuration format
 	if tempPolicy.Global.MaxExpiryDuration != "" {
 		if _, err := parseDuration(tempPolicy.Global.MaxExpiryDuration); err != nil {
-			return fmt.Errorf("invalid max_expiry_duration %q in global rule: %v", tempPolicy.Global.MaxExpiryDuration, err)
+			return fmt.Errorf("invalid max_expiry_duration %q in global rule: %v (format must be ISO-8601 duration, e.g. \"PT10M\" for 10 minutes, \"P7D\" for 7 days, \"P1DT12H\" for 1 day 12 hours)", tempPolicy.Global.MaxExpiryDuration, err)
 		}
 	}
 

pkg/version/version

@@ -1 +1 @@
-v0.31.5
+v0.31.7