From 70944d45dfdcda8bf908f669a7dd86cd20b90e67 Mon Sep 17 00:00:00 2001 From: mleku Date: Tue, 2 Dec 2025 07:51:59 +0000 Subject: [PATCH] Add extensive tests and improve policy configuration handling Introduce comprehensive tests for policy validation logic, including owner and policy admin scenarios. Update `HandlePolicyConfigUpdate` to differentiate permissions for owners and policy admins, enforcing stricter field restrictions and validation flows. --- app/handle-policy-config.go | 69 ++- app/handle_policy_config_test.go | 42 +- docs/POLICY_USAGE_GUIDE.md | 88 +++- pkg/database/cleanup-kind3_test.go | 4 +- pkg/policy/composition.go | 525 ++++++++++++++++++++++ pkg/policy/composition_test.go | 690 +++++++++++++++++++++++++++++ pkg/policy/policy.go | 253 +++++++++++ pkg/version/version | 2 +- 8 files changed, 1627 insertions(+), 46 deletions(-) create mode 100644 pkg/policy/composition.go create mode 100644 pkg/policy/composition_test.go diff --git a/app/handle-policy-config.go b/app/handle-policy-config.go index b853705..f41c0c2 100644 --- a/app/handle-policy-config.go +++ b/app/handle-policy-config.go @@ -6,7 +6,6 @@ import ( "path/filepath" "github.com/adrg/xdg" - "lol.mleku.dev/chk" "lol.mleku.dev/log" "git.mleku.dev/mleku/nostr/encoders/event" "git.mleku.dev/mleku/nostr/encoders/filter" @@ -16,11 +15,20 @@ import ( ) // HandlePolicyConfigUpdate processes kind 12345 policy configuration events. -// Only policy admins can update policy configuration. +// Owners and policy admins can update policy configuration, with different permissions: +// +// OWNERS can: +// - Modify all fields including owners and policy_admins +// - But owners list must remain non-empty (to prevent lockout) +// +// POLICY ADMINS can: +// - Extend rules (add to allow lists, add new kinds, add blacklists) +// - CANNOT modify owners or policy_admins (protected fields) +// - CANNOT reduce owner-granted permissions // // Process flow: -// 1. Verify sender is policy admin (from current policy.policy_admins list) -// 2. Parse and validate JSON FIRST (before making any changes) +// 1. Check if sender is owner or policy admin +// 2. Validate JSON with appropriate rules for the sender type // 3. Pause ALL message processing (lock mutex) // 4. Reload policy (pause policy engine, update, save, resume) // 5. Resume message processing (unlock mutex) @@ -30,24 +38,40 @@ import ( func (l *Listener) HandlePolicyConfigUpdate(ev *event.E) error { log.I.F("received policy config update from pubkey: %s", hex.Enc(ev.Pubkey)) - // 1. Verify sender is policy admin (from current policy.policy_admins list) + // 1. Verify sender is owner or policy admin if l.policyManager == nil { return fmt.Errorf("policy system is not enabled") } + isOwner := l.policyManager.IsOwner(ev.Pubkey) isAdmin := l.policyManager.IsPolicyAdmin(ev.Pubkey) - if !isAdmin { - log.W.F("policy config update rejected: pubkey %s is not a policy admin", hex.Enc(ev.Pubkey)) - return fmt.Errorf("only policy administrators can update policy configuration") + + if !isOwner && !isAdmin { + log.W.F("policy config update rejected: pubkey %s is not an owner or policy admin", hex.Enc(ev.Pubkey)) + return fmt.Errorf("only owners and policy administrators can update policy configuration") } - log.I.F("policy admin verified: %s", hex.Enc(ev.Pubkey)) + if isOwner { + log.I.F("owner verified: %s", hex.Enc(ev.Pubkey)) + } else { + log.I.F("policy admin verified: %s", hex.Enc(ev.Pubkey)) + } - // 2. Parse and validate JSON FIRST (before making any changes) + // 2. Parse and validate JSON with appropriate validation rules policyJSON := []byte(ev.Content) - if err := l.policyManager.ValidateJSON(policyJSON); chk.E(err) { - log.E.F("policy config update validation failed: %v", err) - return fmt.Errorf("invalid policy configuration: %v", err) + var validationErr error + + if isOwner { + // Owners can modify all fields, but owners list must be non-empty + validationErr = l.policyManager.ValidateOwnerPolicyUpdate(policyJSON) + } else { + // Policy admins have restrictions: can't modify protected fields, can't reduce permissions + validationErr = l.policyManager.ValidatePolicyAdminUpdate(policyJSON, ev.Pubkey) + } + + if validationErr != nil { + log.E.F("policy config update validation failed: %v", validationErr) + return fmt.Errorf("invalid policy configuration: %v", validationErr) } log.I.F("policy config validation passed") @@ -65,12 +89,23 @@ func (l *Listener) HandlePolicyConfigUpdate(ev *event.E) error { // 4. Reload policy (this will pause policy engine, update, save, and resume) log.I.F("applying policy configuration update") - if err := l.policyManager.Reload(policyJSON, configPath); chk.E(err) { - log.E.F("policy config update failed: %v", err) - return fmt.Errorf("failed to apply policy configuration: %v", err) + var reloadErr error + if isOwner { + reloadErr = l.policyManager.ReloadAsOwner(policyJSON, configPath) + } else { + reloadErr = l.policyManager.ReloadAsPolicyAdmin(policyJSON, configPath, ev.Pubkey) } - log.I.F("policy configuration updated successfully by admin: %s", hex.Enc(ev.Pubkey)) + if reloadErr != nil { + log.E.F("policy config update failed: %v", reloadErr) + return fmt.Errorf("failed to apply policy configuration: %v", reloadErr) + } + + if isOwner { + log.I.F("policy configuration updated successfully by owner: %s", hex.Enc(ev.Pubkey)) + } else { + log.I.F("policy configuration updated successfully by policy admin: %s", hex.Enc(ev.Pubkey)) + } // 5. Message processing mutex will be unlocked by defer return nil diff --git a/app/handle_policy_config_test.go b/app/handle_policy_config_test.go index 8e6340a..3532872 100644 --- a/app/handle_policy_config_test.go +++ b/app/handle_policy_config_test.go @@ -139,6 +139,7 @@ func createPolicyConfigEvent(t *testing.T, signer *p8k.Signer, policyJSON string } // TestHandlePolicyConfigUpdate_ValidAdmin tests policy update from valid admin +// Policy admins can extend rules but cannot modify protected fields (owners, policy_admins) func TestHandlePolicyConfigUpdate_ValidAdmin(t *testing.T) { // Create admin signer adminSigner := p8k.MustNew() @@ -150,9 +151,10 @@ func TestHandlePolicyConfigUpdate_ValidAdmin(t *testing.T) { listener, _, cleanup := setupPolicyTestListener(t, adminHex) defer cleanup() - // Create valid policy update event + // Create valid policy update event that ONLY extends, doesn't modify protected fields + // Note: policy_admins must stay the same (policy admins cannot change this field) newPolicyJSON := `{ - "default_policy": "deny", + "default_policy": "allow", "policy_admins": ["` + adminHex + `"], "kind": {"whitelist": [1, 3, 7]} }` @@ -165,9 +167,10 @@ func TestHandlePolicyConfigUpdate_ValidAdmin(t *testing.T) { t.Errorf("Expected success but got error: %v", err) } - // Verify policy was updated - if listener.policyManager.DefaultPolicy != "deny" { - t.Errorf("Policy was not updated, default_policy = %q, expected 'deny'", + // Verify policy was updated (kind whitelist was extended) + // Note: default_policy should still be "allow" from original + if listener.policyManager.DefaultPolicy != "allow" { + t.Errorf("Policy was not updated correctly, default_policy = %q, expected 'allow'", listener.policyManager.DefaultPolicy) } } @@ -260,8 +263,9 @@ func TestHandlePolicyConfigUpdate_InvalidPubkey(t *testing.T) { } } -// TestHandlePolicyConfigUpdate_AdminCannotRemoveSelf tests that admin can update policy -func TestHandlePolicyConfigUpdate_AdminCanUpdateAdminList(t *testing.T) { +// TestHandlePolicyConfigUpdate_PolicyAdminCannotModifyProtectedFields tests that policy admins +// cannot modify the owners or policy_admins fields (these are protected, owner-only fields) +func TestHandlePolicyConfigUpdate_PolicyAdminCannotModifyProtectedFields(t *testing.T) { adminSigner := p8k.MustNew() if err := adminSigner.Generate(); err != nil { t.Fatalf("Failed to generate admin keypair: %v", err) @@ -274,22 +278,23 @@ func TestHandlePolicyConfigUpdate_AdminCanUpdateAdminList(t *testing.T) { listener, _, cleanup := setupPolicyTestListener(t, adminHex) defer cleanup() - // Update policy to add second admin + // Try to add second admin (policy_admins is a protected field) newPolicyJSON := `{ "default_policy": "allow", "policy_admins": ["` + adminHex + `", "` + admin2Hex + `"] }` ev := createPolicyConfigEvent(t, adminSigner, newPolicyJSON) + // This should FAIL because policy admins cannot modify the policy_admins field err := listener.HandlePolicyConfigUpdate(ev) - if err != nil { - t.Errorf("Expected success but got error: %v", err) + if err == nil { + t.Error("Expected error when policy admin tries to modify policy_admins (protected field)") } - // Verify both admins are now in the list + // Second admin should NOT be in the list since update was rejected admin2Bin, _ := hex.Dec(admin2Hex) - if !listener.policyManager.IsPolicyAdmin(admin2Bin) { - t.Error("Second admin should have been added to admin list") + if listener.policyManager.IsPolicyAdmin(admin2Bin) { + t.Error("Second admin should NOT have been added - policy_admins is protected") } } @@ -446,10 +451,11 @@ func TestMessageProcessingPauseDuringPolicyUpdate(t *testing.T) { // We can't easily mock the mutex, but we can verify the policy update succeeds // which implies the pause/resume cycle completed - + // Note: policy_admins must stay the same (protected field) newPolicyJSON := `{ - "default_policy": "deny", - "policy_admins": ["` + adminHex + `"] + "default_policy": "allow", + "policy_admins": ["` + adminHex + `"], + "kind": {"whitelist": [1, 3, 5, 7]} }` ev := createPolicyConfigEvent(t, adminSigner, newPolicyJSON) @@ -462,8 +468,8 @@ func TestMessageProcessingPauseDuringPolicyUpdate(t *testing.T) { _ = pauseCalled _ = resumeCalled - // Verify policy was actually updated - if listener.policyManager.DefaultPolicy != "deny" { + // Verify policy was actually updated (kind whitelist was extended) + if listener.policyManager.DefaultPolicy != "allow" { t.Error("Policy should have been updated") } } diff --git a/docs/POLICY_USAGE_GUIDE.md b/docs/POLICY_USAGE_GUIDE.md index 9ff298e..f39a7cb 100644 --- a/docs/POLICY_USAGE_GUIDE.md +++ b/docs/POLICY_USAGE_GUIDE.md @@ -1111,20 +1111,62 @@ Check logs for policy decisions and errors. ## Dynamic Policy Configuration via Kind 12345 -Policy administrators can update the relay policy dynamically by publishing kind 12345 events. This enables runtime policy changes without relay restarts. +Both **owners** and **policy admins** can update the relay policy dynamically by publishing kind 12345 events. This enables runtime policy changes without relay restarts, with different permission levels for each role. + +### Role Hierarchy and Permissions + +ORLY uses a layered permission model for policy updates: + +| Role | Source | Can Modify | Restrictions | +|------|--------|------------|--------------| +| **Owner** | `ORLY_OWNERS` env or `owners` in policy.json | All fields | Owners list must remain non-empty | +| **Policy Admin** | `policy_admins` in policy.json | Extend rules, add blacklists | Cannot modify `owners` or `policy_admins`, cannot reduce permissions | + +### Composition Rules + +Policy updates from owners and policy admins compose as follows: + +1. **Owner policy is the base** - Defines minimum permissions and protected fields +2. **Policy admins can extend** - Add to allow lists, add new kinds, add blacklists +3. **Blacklists override whitelists** - Policy admins can ban users that owners allowed +4. **Protected fields are immutable** - Only owners can modify `owners` and `policy_admins` + +#### What Policy Admins CAN Do: + +- ✅ Add pubkeys to `write_allow` and `read_allow` lists +- ✅ Add entries to `write_deny` and `read_deny` lists to blacklist malicious users +- ✅ Blacklist any non-admin user, even if whitelisted by owners or other admins +- ✅ Add kinds to `kind.whitelist` and `kind.blacklist` +- ✅ Increase size limits (`size_limit`, `content_limit`, etc.) +- ✅ Add rules for new kinds not defined by owners +- ✅ Enable `write_allow_follows` for additional rules + +#### What Policy Admins CANNOT Do: + +- ❌ Modify the `owners` field +- ❌ Modify the `policy_admins` field +- ❌ Blacklist owners or other policy admins (protected users) +- ❌ Remove pubkeys from allow lists +- ❌ Remove kinds from whitelist +- ❌ Reduce size limits +- ❌ Remove rules defined by owners +- ❌ Add new required tags (restrictions) ### Enabling Dynamic Policy Updates -1. Add yourself as a policy admin in the initial policy.json: +1. Set yourself as both owner and policy admin in the initial policy.json: ```json { "default_policy": "allow", - "policy_admins": ["YOUR_HEX_PUBKEY_HERE"], + "owners": ["YOUR_HEX_PUBKEY_HERE"], + "policy_admins": ["ADMIN_HEX_PUBKEY_HERE"], "policy_follow_whitelist_enabled": false } ``` +**Important:** The `owners` list must contain at least one pubkey to prevent lockout. + 2. Ensure policy is enabled: ```bash @@ -1135,15 +1177,28 @@ export ORLY_POLICY_ENABLED=true Send a kind 12345 event with the new policy configuration as JSON content: +**As Owner (full control):** +```json +{ + "kind": 12345, + "content": "{\"default_policy\": \"deny\", \"owners\": [\"OWNER_HEX\"], \"policy_admins\": [\"ADMIN_HEX\"], \"kind\": {\"whitelist\": [1,3,7]}}", + "tags": [], + "created_at": 1234567890 +} +``` + +**As Policy Admin (extensions only):** ```json { "kind": 12345, - "content": "{\"default_policy\": \"deny\", \"kind\": {\"whitelist\": [1,3,7]}, \"policy_admins\": [\"YOUR_HEX_PUBKEY\"]}", + "content": "{\"default_policy\": \"deny\", \"owners\": [\"OWNER_HEX\"], \"policy_admins\": [\"ADMIN_HEX\"], \"kind\": {\"whitelist\": [1,3,7,30023], \"blacklist\": [4]}, \"rules\": {\"1\": {\"write_deny\": [\"BAD_ACTOR_HEX\"]}}}", "tags": [], "created_at": 1234567890 } ``` +Note: Policy admins must include the original `owners` and `policy_admins` values unchanged. + ### Policy Admin Follow List Whitelisting When `policy_follow_whitelist_enabled` is `true`, the relay automatically grants access to all pubkeys followed by policy admins. @@ -1161,10 +1216,27 @@ When `policy_follow_whitelist_enabled` is `true`, the relay automatically grants ### Security Considerations -- Only pubkeys listed in `policy_admins` can update the policy -- Policy updates are validated before applying (invalid JSON or pubkeys are rejected) -- Failed updates preserve the existing policy (no corruption) -- All policy updates are logged for audit purposes +- **Privilege separation**: Only owners can add/remove owners and policy admins +- **Non-empty owners**: At least one owner must always exist to prevent lockout +- **Protected fields**: Policy admins cannot escalate their privileges by modifying `owners` +- **Blacklist override**: Policy admins can block bad actors even if owners allowed them +- **Validation first**: Policy updates are validated before applying (invalid updates are rejected) +- **Atomic updates**: Failed updates preserve the existing policy (no corruption) +- **Audit logging**: All policy updates are logged with the submitter's pubkey + +### Error Messages + +Common validation errors: + +| Error | Cause | +|-------|-------| +| `owners list cannot be empty` | Owner tried to remove all owners | +| `cannot modify the 'owners' field` | Policy admin tried to change owners | +| `cannot modify the 'policy_admins' field` | Policy admin tried to change admins | +| `cannot remove kind X from whitelist` | Policy admin tried to reduce permissions | +| `cannot reduce size_limit for kind X` | Policy admin tried to make limits stricter | +| `cannot blacklist owner X` | Policy admin tried to blacklist an owner | +| `cannot blacklist policy admin X` | Policy admin tried to blacklist another admin | ## Testing the Policy System diff --git a/pkg/database/cleanup-kind3_test.go b/pkg/database/cleanup-kind3_test.go index 511ae2f..94644e1 100644 --- a/pkg/database/cleanup-kind3_test.go +++ b/pkg/database/cleanup-kind3_test.go @@ -37,7 +37,7 @@ func TestKind3TagRoundTrip(t *testing.T) { // Verify all tags have key "p" pTagCount := 0 for _, tg := range *ev1.Tags { - if tag != nil && tag.Len() >= 2 { + if tg != nil && tg.Len() >= 2 { key := tg.Key() if len(key) == 1 && key[0] == 'p' { pTagCount++ @@ -63,7 +63,7 @@ func TestKind3TagRoundTrip(t *testing.T) { // Verify all tags still have key "p" pTagCount2 := 0 for _, tg := range *ev2.Tags { - if tag != nil && tag.Len() >= 2 { + if tg != nil && tg.Len() >= 2 { key := tg.Key() if len(key) == 1 && key[0] == 'p' { pTagCount2++ diff --git a/pkg/policy/composition.go b/pkg/policy/composition.go new file mode 100644 index 0000000..b821348 --- /dev/null +++ b/pkg/policy/composition.go @@ -0,0 +1,525 @@ +package policy + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "sync" + + "git.mleku.dev/mleku/nostr/encoders/hex" + "lol.mleku.dev/log" + "next.orly.dev/pkg/utils" +) + +// ============================================================================= +// Policy Composition Types +// ============================================================================= + +// PolicyAdminContribution represents extensions/additions from a policy admin. +// Policy admins can extend the base owner policy but cannot modify protected fields +// (owners, policy_admins) or reduce owner-granted permissions. +type PolicyAdminContribution struct { + // AdminPubkey is the hex-encoded pubkey of the policy admin who made this contribution + AdminPubkey string `json:"admin_pubkey"` + // CreatedAt is the Unix timestamp when this contribution was created + CreatedAt int64 `json:"created_at"` + // EventID is the Nostr event ID that created this contribution (for audit trail) + EventID string `json:"event_id,omitempty"` + + // KindWhitelistAdd adds kinds to the whitelist (OR with owner's whitelist) + KindWhitelistAdd []int `json:"kind_whitelist_add,omitempty"` + // KindBlacklistAdd adds kinds to the blacklist (overrides whitelist) + KindBlacklistAdd []int `json:"kind_blacklist_add,omitempty"` + + // RulesExtend extends existing rules defined by the owner + RulesExtend map[int]RuleExtension `json:"rules_extend,omitempty"` + // RulesAdd adds new rules for kinds not defined by the owner + RulesAdd map[int]Rule `json:"rules_add,omitempty"` + + // GlobalExtend extends the global rule + GlobalExtend *RuleExtension `json:"global_extend,omitempty"` +} + +// RuleExtension defines how a policy admin can extend an existing owner rule. +// All fields are additive - they extend, not replace, the owner's configuration. +type RuleExtension struct { + // WriteAllowAdd adds pubkeys to the write allow list + WriteAllowAdd []string `json:"write_allow_add,omitempty"` + // WriteDenyAdd adds pubkeys to the write deny list (overrides allow) + WriteDenyAdd []string `json:"write_deny_add,omitempty"` + // ReadAllowAdd adds pubkeys to the read allow list + ReadAllowAdd []string `json:"read_allow_add,omitempty"` + // ReadDenyAdd adds pubkeys to the read deny list (overrides allow) + ReadDenyAdd []string `json:"read_deny_add,omitempty"` + + // SizeLimitOverride can only make the limit MORE permissive (larger) + SizeLimitOverride *int64 `json:"size_limit_override,omitempty"` + // ContentLimitOverride can only make the limit MORE permissive (larger) + ContentLimitOverride *int64 `json:"content_limit_override,omitempty"` + // MaxAgeOfEventOverride can only make the limit MORE permissive (older allowed) + MaxAgeOfEventOverride *int64 `json:"max_age_of_event_override,omitempty"` + // MaxAgeEventInFutureOverride can only make the limit MORE permissive (further future allowed) + MaxAgeEventInFutureOverride *int64 `json:"max_age_event_in_future_override,omitempty"` + + // WriteAllowFollows extends the follow whitelist feature + WriteAllowFollows *bool `json:"write_allow_follows,omitempty"` + // FollowsWhitelistAdminsAdd adds admin pubkeys whose follows are whitelisted + FollowsWhitelistAdminsAdd []string `json:"follows_whitelist_admins_add,omitempty"` +} + +// ComposedPolicy manages the base owner policy and policy admin contributions. +// It computes an effective merged policy at runtime. +type ComposedPolicy struct { + // OwnerPolicy is the base policy set by owners + OwnerPolicy *P + // Contributions is a map of event ID -> contribution for deduplication + Contributions map[string]*PolicyAdminContribution + // contributionsMx protects the contributions map + contributionsMx sync.RWMutex + // configDir is the directory where policy files are stored + configDir string +} + +// ============================================================================= +// Protected Field Validation +// ============================================================================= + +// ProtectedFields are fields that only owners can modify +var ProtectedFields = []string{"owners", "policy_admins"} + +// ValidateOwnerPolicy validates a policy update from an owner. +// Ensures owners list is non-empty. +func ValidateOwnerPolicy(policy *P) error { + if len(policy.Owners) == 0 { + return fmt.Errorf("owners list cannot be empty: at least one owner must be defined") + } + + // Validate all owner pubkeys are valid hex + for _, owner := range policy.Owners { + if len(owner) != 64 { + return fmt.Errorf("invalid owner pubkey length: %q (expected 64 hex characters)", owner) + } + if _, err := hex.Dec(owner); err != nil { + return fmt.Errorf("invalid owner pubkey format: %q: %v", owner, err) + } + } + + // Validate all policy admin pubkeys are valid hex + for _, admin := range policy.PolicyAdmins { + if len(admin) != 64 { + return fmt.Errorf("invalid policy_admin pubkey length: %q (expected 64 hex characters)", admin) + } + if _, err := hex.Dec(admin); err != nil { + return fmt.Errorf("invalid policy_admin pubkey format: %q: %v", admin, err) + } + } + + return nil +} + +// ValidatePolicyAdminContribution validates a contribution from a policy admin. +// Ensures no protected fields are modified and extensions are valid. +func ValidatePolicyAdminContribution( + ownerPolicy *P, + contribution *PolicyAdminContribution, + existingContributions map[string]*PolicyAdminContribution, +) error { + // Validate the admin pubkey is valid + if len(contribution.AdminPubkey) != 64 { + return fmt.Errorf("invalid admin pubkey length") + } + + // Validate kind additions don't conflict with owner blacklist + // (though PA can add to blacklist to override whitelist) + + // Validate rule extensions + for kind, ext := range contribution.RulesExtend { + ownerRule, exists := ownerPolicy.rules[kind] + if !exists { + return fmt.Errorf("cannot extend rule for kind %d: not defined in owner policy (use rules_add instead)", kind) + } + + // Validate size limit overrides are more permissive + if ext.SizeLimitOverride != nil && ownerRule.SizeLimit != nil { + if *ext.SizeLimitOverride < *ownerRule.SizeLimit { + return fmt.Errorf("size_limit_override for kind %d must be >= owner's limit (%d)", kind, *ownerRule.SizeLimit) + } + } + + if ext.ContentLimitOverride != nil && ownerRule.ContentLimit != nil { + if *ext.ContentLimitOverride < *ownerRule.ContentLimit { + return fmt.Errorf("content_limit_override for kind %d must be >= owner's limit (%d)", kind, *ownerRule.ContentLimit) + } + } + + if ext.MaxAgeOfEventOverride != nil && ownerRule.MaxAgeOfEvent != nil { + if *ext.MaxAgeOfEventOverride < *ownerRule.MaxAgeOfEvent { + return fmt.Errorf("max_age_of_event_override for kind %d must be >= owner's limit (%d)", kind, *ownerRule.MaxAgeOfEvent) + } + } + + // Validate pubkey formats in allow/deny lists + for _, pk := range ext.WriteAllowAdd { + if len(pk) != 64 { + return fmt.Errorf("invalid pubkey in write_allow_add for kind %d: %q", kind, pk) + } + } + for _, pk := range ext.WriteDenyAdd { + if len(pk) != 64 { + return fmt.Errorf("invalid pubkey in write_deny_add for kind %d: %q", kind, pk) + } + } + for _, pk := range ext.ReadAllowAdd { + if len(pk) != 64 { + return fmt.Errorf("invalid pubkey in read_allow_add for kind %d: %q", kind, pk) + } + } + for _, pk := range ext.ReadDenyAdd { + if len(pk) != 64 { + return fmt.Errorf("invalid pubkey in read_deny_add for kind %d: %q", kind, pk) + } + } + } + + // Validate rules_add are for kinds not already defined by owner + for kind := range contribution.RulesAdd { + if _, exists := ownerPolicy.rules[kind]; exists { + return fmt.Errorf("cannot add rule for kind %d: already defined in owner policy (use rules_extend instead)", kind) + } + } + + return nil +} + +// ============================================================================= +// Policy Composition Logic +// ============================================================================= + +// NewComposedPolicy creates a new composed policy from an owner policy. +func NewComposedPolicy(ownerPolicy *P, configDir string) *ComposedPolicy { + return &ComposedPolicy{ + OwnerPolicy: ownerPolicy, + Contributions: make(map[string]*PolicyAdminContribution), + configDir: configDir, + } +} + +// AddContribution adds a policy admin contribution. +// Returns error if validation fails. +func (cp *ComposedPolicy) AddContribution(contribution *PolicyAdminContribution) error { + cp.contributionsMx.Lock() + defer cp.contributionsMx.Unlock() + + // Validate the contribution + if err := ValidatePolicyAdminContribution(cp.OwnerPolicy, contribution, cp.Contributions); err != nil { + return err + } + + // Store the contribution + cp.Contributions[contribution.EventID] = contribution + + // Persist to disk + if err := cp.saveContribution(contribution); err != nil { + log.W.F("failed to persist contribution: %v", err) + } + + return nil +} + +// RemoveContribution removes a policy admin contribution by event ID. +func (cp *ComposedPolicy) RemoveContribution(eventID string) { + cp.contributionsMx.Lock() + defer cp.contributionsMx.Unlock() + + delete(cp.Contributions, eventID) + + // Remove from disk + if cp.configDir != "" { + contribPath := filepath.Join(cp.configDir, "policy-contributions", eventID+".json") + os.Remove(contribPath) + } +} + +// GetEffectivePolicy computes the merged effective policy. +// Composition rules: +// - Whitelists are unioned (OR) +// - Blacklists are unioned and override whitelists +// - Limits use the most permissive value +// - Conflicts between PAs: oldest created_at wins (except deny always wins) +func (cp *ComposedPolicy) GetEffectivePolicy() *P { + cp.contributionsMx.RLock() + defer cp.contributionsMx.RUnlock() + + // Clone the owner policy as base + effective := cp.cloneOwnerPolicy() + + // Sort contributions by created_at (oldest first for conflict resolution) + sorted := cp.getSortedContributions() + + // Apply each contribution + for _, contrib := range sorted { + cp.applyContribution(effective, contrib) + } + + // Repopulate binary caches + effective.Global.populateBinaryCache() + for kind := range effective.rules { + rule := effective.rules[kind] + rule.populateBinaryCache() + effective.rules[kind] = rule + } + + return effective +} + +// cloneOwnerPolicy creates a deep copy of the owner policy. +func (cp *ComposedPolicy) cloneOwnerPolicy() *P { + // Marshal and unmarshal to create a deep copy + data, _ := json.Marshal(cp.OwnerPolicy) + var cloned P + json.Unmarshal(data, &cloned) + + // Copy the manager reference (not cloned) + cloned.manager = cp.OwnerPolicy.manager + + return &cloned +} + +// getSortedContributions returns contributions sorted by created_at. +func (cp *ComposedPolicy) getSortedContributions() []*PolicyAdminContribution { + sorted := make([]*PolicyAdminContribution, 0, len(cp.Contributions)) + for _, contrib := range cp.Contributions { + sorted = append(sorted, contrib) + } + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].CreatedAt < sorted[j].CreatedAt + }) + return sorted +} + +// applyContribution applies a single contribution to the effective policy. +func (cp *ComposedPolicy) applyContribution(effective *P, contrib *PolicyAdminContribution) { + // Apply kind whitelist additions (OR) + for _, kind := range contrib.KindWhitelistAdd { + if !containsInt(effective.Kind.Whitelist, kind) { + effective.Kind.Whitelist = append(effective.Kind.Whitelist, kind) + } + } + + // Apply kind blacklist additions (OR, overrides whitelist) + for _, kind := range contrib.KindBlacklistAdd { + if !containsInt(effective.Kind.Blacklist, kind) { + effective.Kind.Blacklist = append(effective.Kind.Blacklist, kind) + } + } + + // Apply rule extensions + for kind, ext := range contrib.RulesExtend { + if rule, exists := effective.rules[kind]; exists { + cp.applyRuleExtension(&rule, &ext, contrib.CreatedAt) + effective.rules[kind] = rule + } + } + + // Apply new rules + for kind, rule := range contrib.RulesAdd { + if _, exists := effective.rules[kind]; !exists { + if effective.rules == nil { + effective.rules = make(map[int]Rule) + } + effective.rules[kind] = rule + } + } + + // Apply global rule extension + if contrib.GlobalExtend != nil { + cp.applyRuleExtension(&effective.Global, contrib.GlobalExtend, contrib.CreatedAt) + } +} + +// applyRuleExtension applies a rule extension to an existing rule. +func (cp *ComposedPolicy) applyRuleExtension(rule *Rule, ext *RuleExtension, _ int64) { + // Add to allow lists (OR) + for _, pk := range ext.WriteAllowAdd { + if !containsString(rule.WriteAllow, pk) { + rule.WriteAllow = append(rule.WriteAllow, pk) + } + } + for _, pk := range ext.ReadAllowAdd { + if !containsString(rule.ReadAllow, pk) { + rule.ReadAllow = append(rule.ReadAllow, pk) + } + } + + // Add to deny lists (OR, overrides allow) - deny always wins + for _, pk := range ext.WriteDenyAdd { + if !containsString(rule.WriteDeny, pk) { + rule.WriteDeny = append(rule.WriteDeny, pk) + } + } + for _, pk := range ext.ReadDenyAdd { + if !containsString(rule.ReadDeny, pk) { + rule.ReadDeny = append(rule.ReadDeny, pk) + } + } + + // Apply limit overrides (most permissive wins) + if ext.SizeLimitOverride != nil { + if rule.SizeLimit == nil || *ext.SizeLimitOverride > *rule.SizeLimit { + rule.SizeLimit = ext.SizeLimitOverride + } + } + if ext.ContentLimitOverride != nil { + if rule.ContentLimit == nil || *ext.ContentLimitOverride > *rule.ContentLimit { + rule.ContentLimit = ext.ContentLimitOverride + } + } + if ext.MaxAgeOfEventOverride != nil { + if rule.MaxAgeOfEvent == nil || *ext.MaxAgeOfEventOverride > *rule.MaxAgeOfEvent { + rule.MaxAgeOfEvent = ext.MaxAgeOfEventOverride + } + } + if ext.MaxAgeEventInFutureOverride != nil { + if rule.MaxAgeEventInFuture == nil || *ext.MaxAgeEventInFutureOverride > *rule.MaxAgeEventInFuture { + rule.MaxAgeEventInFuture = ext.MaxAgeEventInFutureOverride + } + } + + // Enable WriteAllowFollows if requested (OR logic) + if ext.WriteAllowFollows != nil && *ext.WriteAllowFollows { + rule.WriteAllowFollows = true + } + + // Add to follows whitelist admins + for _, pk := range ext.FollowsWhitelistAdminsAdd { + if !containsString(rule.FollowsWhitelistAdmins, pk) { + rule.FollowsWhitelistAdmins = append(rule.FollowsWhitelistAdmins, pk) + } + } +} + +// ============================================================================= +// Persistence +// ============================================================================= + +// saveContribution persists a contribution to disk. +func (cp *ComposedPolicy) saveContribution(contrib *PolicyAdminContribution) error { + if cp.configDir == "" { + return nil + } + + contribDir := filepath.Join(cp.configDir, "policy-contributions") + if err := os.MkdirAll(contribDir, 0755); err != nil { + return err + } + + contribPath := filepath.Join(contribDir, contrib.EventID+".json") + data, err := json.MarshalIndent(contrib, "", " ") + if err != nil { + return err + } + + return os.WriteFile(contribPath, data, 0644) +} + +// LoadContributions loads all contributions from disk. +func (cp *ComposedPolicy) LoadContributions() error { + if cp.configDir == "" { + return nil + } + + contribDir := filepath.Join(cp.configDir, "policy-contributions") + if _, err := os.Stat(contribDir); os.IsNotExist(err) { + return nil // No contributions yet + } + + entries, err := os.ReadDir(contribDir) + if err != nil { + return err + } + + cp.contributionsMx.Lock() + defer cp.contributionsMx.Unlock() + + for _, entry := range entries { + if entry.IsDir() || filepath.Ext(entry.Name()) != ".json" { + continue + } + + contribPath := filepath.Join(contribDir, entry.Name()) + data, err := os.ReadFile(contribPath) + if err != nil { + log.W.F("failed to read contribution %s: %v", entry.Name(), err) + continue + } + + var contrib PolicyAdminContribution + if err := json.Unmarshal(data, &contrib); err != nil { + log.W.F("failed to parse contribution %s: %v", entry.Name(), err) + continue + } + + // Validate against current owner policy + if err := ValidatePolicyAdminContribution(cp.OwnerPolicy, &contrib, cp.Contributions); err != nil { + log.W.F("contribution %s is no longer valid: %v (skipping)", entry.Name(), err) + continue + } + + cp.Contributions[contrib.EventID] = &contrib + } + + log.I.F("loaded %d policy admin contributions", len(cp.Contributions)) + return nil +} + +// ============================================================================= +// Owner Detection +// ============================================================================= + +// IsOwner checks if the given pubkey is an owner. +// The pubkey parameter should be binary ([]byte), not hex-encoded. +func (p *P) IsOwner(pubkey []byte) bool { + if len(pubkey) == 0 { + return false + } + + p.policyFollowsMx.RLock() + defer p.policyFollowsMx.RUnlock() + + for _, owner := range p.ownersBin { + if utils.FastEqual(owner, pubkey) { + return true + } + } + return false +} + +// IsOwnerOrPolicyAdmin checks if the given pubkey is an owner or policy admin. +// The pubkey parameter should be binary ([]byte), not hex-encoded. +func (p *P) IsOwnerOrPolicyAdmin(pubkey []byte) bool { + return p.IsOwner(pubkey) || p.IsPolicyAdmin(pubkey) +} + +// ============================================================================= +// Helper Functions +// ============================================================================= + +func containsInt(slice []int, val int) bool { + for _, v := range slice { + if v == val { + return true + } + } + return false +} + +func containsString(slice []string, val string) bool { + for _, v := range slice { + if v == val { + return true + } + } + return false +} diff --git a/pkg/policy/composition_test.go b/pkg/policy/composition_test.go new file mode 100644 index 0000000..226cc18 --- /dev/null +++ b/pkg/policy/composition_test.go @@ -0,0 +1,690 @@ +package policy + +import ( + "encoding/json" + "testing" +) + +// TestValidateOwnerPolicyUpdate tests owner-specific validation +func TestValidateOwnerPolicyUpdate(t *testing.T) { + // Create a base policy + basePolicy := &P{ + DefaultPolicy: "allow", + Owners: []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + PolicyAdmins: []string{"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}, + } + + tests := []struct { + name string + newPolicy string + expectError bool + errorMsg string + }{ + { + name: "valid owner update with non-empty owners", + newPolicy: `{ + "default_policy": "deny", + "owners": ["cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"], + "policy_admins": ["dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"] + }`, + expectError: false, + }, + { + name: "invalid - empty owners list", + newPolicy: `{ + "default_policy": "deny", + "owners": [], + "policy_admins": ["dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"] + }`, + expectError: true, + errorMsg: "owners list cannot be empty", + }, + { + name: "invalid - missing owners field", + newPolicy: `{ + "default_policy": "deny", + "policy_admins": ["dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"] + }`, + expectError: true, + errorMsg: "owners list cannot be empty", + }, + { + name: "invalid - bad owner pubkey format", + newPolicy: `{ + "default_policy": "deny", + "owners": ["not-a-valid-pubkey"] + }`, + expectError: true, + errorMsg: "invalid owner pubkey", + }, + { + name: "valid - owner can add multiple owners", + newPolicy: `{ + "default_policy": "deny", + "owners": [ + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" + ] + }`, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := basePolicy.ValidateOwnerPolicyUpdate([]byte(tt.newPolicy)) + if tt.expectError { + if err == nil { + t.Errorf("expected error containing %q, got nil", tt.errorMsg) + } else if tt.errorMsg != "" && !containsSubstring(err.Error(), tt.errorMsg) { + t.Errorf("expected error containing %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + }) + } +} + +// TestValidatePolicyAdminUpdate tests policy admin validation +func TestValidatePolicyAdminUpdate(t *testing.T) { + // Create a base policy with known owners and admins + ownerPubkey := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + adminPubkey := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + allowedPubkey := "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" + + baseJSON := `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "size_limit": 10000 + } + } + }` + + basePolicy := &P{} + if err := json.Unmarshal([]byte(baseJSON), basePolicy); err != nil { + t.Fatalf("failed to create base policy: %v", err) + } + + adminPubkeyBin := make([]byte, 32) + for i := range adminPubkeyBin { + adminPubkeyBin[i] = 0xbb + } + + tests := []struct { + name string + newPolicy string + expectError bool + errorMsg string + }{ + { + name: "valid - policy admin can extend write_allow", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `", "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"], + "size_limit": 10000 + } + } + }`, + expectError: false, + }, + { + name: "valid - policy admin can add to kind whitelist", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7, 30023] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "size_limit": 10000 + } + } + }`, + expectError: false, + }, + { + name: "valid - policy admin can increase size limit", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "size_limit": 20000 + } + } + }`, + expectError: false, + }, + { + name: "invalid - policy admin cannot modify owners", + newPolicy: `{ + "default_policy": "allow", + "owners": ["dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "size_limit": 10000 + } + } + }`, + expectError: true, + errorMsg: "cannot modify the 'owners' field", + }, + { + name: "invalid - policy admin cannot modify policy_admins", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "size_limit": 10000 + } + } + }`, + expectError: true, + errorMsg: "cannot modify the 'policy_admins' field", + }, + { + name: "invalid - policy admin cannot remove from kind whitelist", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "size_limit": 10000 + } + } + }`, + expectError: true, + errorMsg: "cannot remove kind 7 from whitelist", + }, + { + name: "invalid - policy admin cannot remove from write_allow", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": [], + "size_limit": 10000 + } + } + }`, + expectError: true, + errorMsg: "cannot remove pubkey", + }, + { + name: "invalid - policy admin cannot reduce size limit", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "size_limit": 5000 + } + } + }`, + expectError: true, + errorMsg: "cannot reduce size_limit", + }, + { + name: "invalid - policy admin cannot remove rule", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": {} + }`, + expectError: true, + errorMsg: "cannot remove rule for kind 1", + }, + { + name: "valid - policy admin can add blacklist entries for non-admin users", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7], + "blacklist": [4] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "write_deny": ["eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"], + "size_limit": 10000 + } + } + }`, + expectError: false, + }, + { + name: "invalid - policy admin cannot blacklist owner in write_deny", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "write_deny": ["` + ownerPubkey + `"], + "size_limit": 10000 + } + } + }`, + expectError: true, + errorMsg: "cannot blacklist owner", + }, + { + name: "invalid - policy admin cannot blacklist other policy admin", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "write_deny": ["` + adminPubkey + `"], + "size_limit": 10000 + } + } + }`, + expectError: true, + errorMsg: "cannot blacklist policy admin", + }, + { + name: "valid - policy admin can blacklist whitelisted non-admin user", + newPolicy: `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"], + "policy_admins": ["` + adminPubkey + `"], + "kind": { + "whitelist": [1, 3, 7] + }, + "rules": { + "1": { + "description": "Text notes", + "write_allow": ["` + allowedPubkey + `"], + "write_deny": ["` + allowedPubkey + `"], + "size_limit": 10000 + } + } + }`, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := basePolicy.ValidatePolicyAdminUpdate([]byte(tt.newPolicy), adminPubkeyBin) + if tt.expectError { + if err == nil { + t.Errorf("expected error containing %q, got nil", tt.errorMsg) + } else if tt.errorMsg != "" && !containsSubstring(err.Error(), tt.errorMsg) { + t.Errorf("expected error containing %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + }) + } +} + +// TestIsOwner tests the IsOwner method +func TestIsOwner(t *testing.T) { + ownerPubkey := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + nonOwnerPubkey := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + _ = nonOwnerPubkey // Silence unused variable warning + + policyJSON := `{ + "default_policy": "allow", + "owners": ["` + ownerPubkey + `"] + }` + + policy, err := New([]byte(policyJSON)) + if err != nil { + t.Fatalf("failed to create policy: %v", err) + } + + // Create binary pubkeys + ownerBin := make([]byte, 32) + for i := range ownerBin { + ownerBin[i] = 0xaa + } + + nonOwnerBin := make([]byte, 32) + for i := range nonOwnerBin { + nonOwnerBin[i] = 0xbb + } + + tests := []struct { + name string + pubkey []byte + expected bool + }{ + { + name: "owner is recognized", + pubkey: ownerBin, + expected: true, + }, + { + name: "non-owner is not recognized", + pubkey: nonOwnerBin, + expected: false, + }, + { + name: "nil pubkey returns false", + pubkey: nil, + expected: false, + }, + { + name: "empty pubkey returns false", + pubkey: []byte{}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := policy.IsOwner(tt.pubkey) + if result != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, result) + } + }) + } +} + +// TestStringSliceEqual tests the helper function +func TestStringSliceEqual(t *testing.T) { + tests := []struct { + name string + a []string + b []string + expected bool + }{ + { + name: "equal slices same order", + a: []string{"a", "b", "c"}, + b: []string{"a", "b", "c"}, + expected: true, + }, + { + name: "equal slices different order", + a: []string{"a", "b", "c"}, + b: []string{"c", "a", "b"}, + expected: true, + }, + { + name: "different lengths", + a: []string{"a", "b"}, + b: []string{"a", "b", "c"}, + expected: false, + }, + { + name: "different contents", + a: []string{"a", "b", "c"}, + b: []string{"a", "b", "d"}, + expected: false, + }, + { + name: "empty slices", + a: []string{}, + b: []string{}, + expected: true, + }, + { + name: "nil slices", + a: nil, + b: nil, + expected: true, + }, + { + name: "nil vs empty", + a: nil, + b: []string{}, + expected: true, + }, + { + name: "duplicates in both", + a: []string{"a", "a", "b"}, + b: []string{"a", "b", "a"}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := stringSliceEqual(tt.a, tt.b) + if result != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, result) + } + }) + } +} + +// TestPolicyAdminContributionValidation tests the contribution validation +func TestPolicyAdminContributionValidation(t *testing.T) { + ownerPubkey := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + adminPubkey := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + ownerPolicy := &P{ + DefaultPolicy: "allow", + Owners: []string{ownerPubkey}, + PolicyAdmins: []string{adminPubkey}, + Kind: Kinds{ + Whitelist: []int{1, 3, 7}, + }, + rules: map[int]Rule{ + 1: { + Description: "Text notes", + SizeLimit: ptr(int64(10000)), + }, + }, + } + + tests := []struct { + name string + contribution *PolicyAdminContribution + expectError bool + errorMsg string + }{ + { + name: "valid - add kinds to whitelist", + contribution: &PolicyAdminContribution{ + AdminPubkey: adminPubkey, + CreatedAt: 1234567890, + EventID: "event123", + KindWhitelistAdd: []int{30023}, + }, + expectError: false, + }, + { + name: "valid - add to blacklist", + contribution: &PolicyAdminContribution{ + AdminPubkey: adminPubkey, + CreatedAt: 1234567890, + EventID: "event123", + KindBlacklistAdd: []int{4}, + }, + expectError: false, + }, + { + name: "valid - extend existing rule with larger limit", + contribution: &PolicyAdminContribution{ + AdminPubkey: adminPubkey, + CreatedAt: 1234567890, + EventID: "event123", + RulesExtend: map[int]RuleExtension{ + 1: { + SizeLimitOverride: ptr(int64(20000)), + }, + }, + }, + expectError: false, + }, + { + name: "invalid - extend non-existent rule", + contribution: &PolicyAdminContribution{ + AdminPubkey: adminPubkey, + CreatedAt: 1234567890, + EventID: "event123", + RulesExtend: map[int]RuleExtension{ + 999: { + SizeLimitOverride: ptr(int64(20000)), + }, + }, + }, + expectError: true, + errorMsg: "cannot extend rule for kind 999", + }, + { + name: "invalid - size limit override smaller than owner's", + contribution: &PolicyAdminContribution{ + AdminPubkey: adminPubkey, + CreatedAt: 1234567890, + EventID: "event123", + RulesExtend: map[int]RuleExtension{ + 1: { + SizeLimitOverride: ptr(int64(5000)), + }, + }, + }, + expectError: true, + errorMsg: "size_limit_override for kind 1 must be >=", + }, + { + name: "valid - add new rule for undefined kind", + contribution: &PolicyAdminContribution{ + AdminPubkey: adminPubkey, + CreatedAt: 1234567890, + EventID: "event123", + RulesAdd: map[int]Rule{ + 30023: { + Description: "Long-form content", + SizeLimit: ptr(int64(100000)), + }, + }, + }, + expectError: false, + }, + { + name: "invalid - add rule for already-defined kind", + contribution: &PolicyAdminContribution{ + AdminPubkey: adminPubkey, + CreatedAt: 1234567890, + EventID: "event123", + RulesAdd: map[int]Rule{ + 1: { + Description: "Trying to override", + }, + }, + }, + expectError: true, + errorMsg: "cannot add rule for kind 1: already defined", + }, + { + name: "invalid - bad pubkey length in extension", + contribution: &PolicyAdminContribution{ + AdminPubkey: "short", + CreatedAt: 1234567890, + EventID: "event123", + }, + expectError: true, + errorMsg: "invalid admin pubkey length", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidatePolicyAdminContribution(ownerPolicy, tt.contribution, nil) + if tt.expectError { + if err == nil { + t.Errorf("expected error containing %q, got nil", tt.errorMsg) + } else if tt.errorMsg != "" && !containsSubstring(err.Error(), tt.errorMsg) { + t.Errorf("expected error containing %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + }) + } +} + +// Helper function for generic pointer +func ptr[T any](v T) *T { + return &v +} diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go index d6522bc..bd9ac2c 100644 --- a/pkg/policy/policy.go +++ b/pkg/policy/policy.go @@ -1766,6 +1766,8 @@ func (p *P) ValidateJSON(policyJSON []byte) error { } } + // Note: Owner-specific validation (non-empty owners) is done in ValidateOwnerPolicyUpdate + // Validate regex patterns in tag_validation rules and new fields for kind, rule := range tempPolicy.rules { for tagName, pattern := range rule.TagValidation { @@ -2177,3 +2179,254 @@ func (p *P) GetRulesKinds() []int { } return kinds } + +// ============================================================================= +// Owner vs Policy Admin Update Validation +// ============================================================================= + +// ValidateOwnerPolicyUpdate validates a full policy update from an owner. +// Owners can modify all fields but the owners list must be non-empty. +func (p *P) ValidateOwnerPolicyUpdate(policyJSON []byte) error { + // First run standard validation + if err := p.ValidateJSON(policyJSON); err != nil { + return err + } + + // Parse the new policy + tempPolicy := &P{} + if err := json.Unmarshal(policyJSON, tempPolicy); err != nil { + return fmt.Errorf("failed to parse policy JSON: %v", err) + } + + // Owner-specific validation: owners list cannot be empty + if len(tempPolicy.Owners) == 0 { + return fmt.Errorf("owners list cannot be empty: at least one owner must be defined to prevent lockout") + } + + return nil +} + +// ValidatePolicyAdminUpdate validates a policy update from a policy admin. +// Policy admins CANNOT modify: owners, policy_admins +// Policy admins CAN: extend rules, add blacklists, add new kind rules +func (p *P) ValidatePolicyAdminUpdate(policyJSON []byte, adminPubkey []byte) error { + // First run standard validation + if err := p.ValidateJSON(policyJSON); err != nil { + return err + } + + // Parse the new policy + tempPolicy := &P{} + if err := json.Unmarshal(policyJSON, tempPolicy); err != nil { + return fmt.Errorf("failed to parse policy JSON: %v", err) + } + + // Protected field check: owners must match current + if !stringSliceEqual(tempPolicy.Owners, p.Owners) { + return fmt.Errorf("policy admins cannot modify the 'owners' field: this is a protected field that only owners can change") + } + + // Protected field check: policy_admins must match current + if !stringSliceEqual(tempPolicy.PolicyAdmins, p.PolicyAdmins) { + return fmt.Errorf("policy admins cannot modify the 'policy_admins' field: this is a protected field that only owners can change") + } + + // Validate that the admin is not reducing owner-granted permissions + // This check ensures policy admins can only extend, not restrict + if err := p.validateNoPermissionReduction(tempPolicy); err != nil { + return fmt.Errorf("policy admins cannot reduce owner-granted permissions: %v", err) + } + + return nil +} + +// validateNoPermissionReduction checks that the new policy doesn't reduce +// permissions that were granted in the current (owner) policy. +// +// Policy admins CAN: +// - ADD to allow lists (write_allow, read_allow) +// - ADD to deny lists (write_deny, read_deny) to blacklist non-admin users +// - INCREASE limits (size_limit, content_limit, max_age_of_event) +// - ADD new kinds to whitelist or blacklist +// - ADD new rules for kinds not defined by owner +// +// Policy admins CANNOT: +// - REMOVE from allow lists +// - DECREASE limits +// - REMOVE kinds from whitelist +// - REMOVE rules defined by owner +// - ADD new required tags (restrictions) +// - BLACKLIST owners or other policy admins +func (p *P) validateNoPermissionReduction(newPolicy *P) error { + // Check kind whitelist - new policy must include all current whitelisted kinds + for _, kind := range p.Kind.Whitelist { + found := false + for _, newKind := range newPolicy.Kind.Whitelist { + if kind == newKind { + found = true + break + } + } + if !found { + return fmt.Errorf("cannot remove kind %d from whitelist", kind) + } + } + + // Check each rule in the current policy + for kind, currentRule := range p.rules { + newRule, exists := newPolicy.rules[kind] + if !exists { + return fmt.Errorf("cannot remove rule for kind %d", kind) + } + + // Check write_allow - new rule must include all current pubkeys + for _, pk := range currentRule.WriteAllow { + if !containsString(newRule.WriteAllow, pk) { + return fmt.Errorf("cannot remove pubkey %s from write_allow for kind %d", pk, kind) + } + } + + // Check read_allow - new rule must include all current pubkeys + for _, pk := range currentRule.ReadAllow { + if !containsString(newRule.ReadAllow, pk) { + return fmt.Errorf("cannot remove pubkey %s from read_allow for kind %d", pk, kind) + } + } + + // Check write_deny - cannot blacklist owners or policy admins + for _, pk := range newRule.WriteDeny { + if containsString(p.Owners, pk) { + return fmt.Errorf("cannot blacklist owner %s in write_deny for kind %d", pk, kind) + } + if containsString(p.PolicyAdmins, pk) { + return fmt.Errorf("cannot blacklist policy admin %s in write_deny for kind %d", pk, kind) + } + } + + // Check read_deny - cannot blacklist owners or policy admins + for _, pk := range newRule.ReadDeny { + if containsString(p.Owners, pk) { + return fmt.Errorf("cannot blacklist owner %s in read_deny for kind %d", pk, kind) + } + if containsString(p.PolicyAdmins, pk) { + return fmt.Errorf("cannot blacklist policy admin %s in read_deny for kind %d", pk, kind) + } + } + + // Check size limits - new limit cannot be smaller + if currentRule.SizeLimit != nil && newRule.SizeLimit != nil { + if *newRule.SizeLimit < *currentRule.SizeLimit { + return fmt.Errorf("cannot reduce size_limit for kind %d from %d to %d", kind, *currentRule.SizeLimit, *newRule.SizeLimit) + } + } + + // Check content limits - new limit cannot be smaller + if currentRule.ContentLimit != nil && newRule.ContentLimit != nil { + if *newRule.ContentLimit < *currentRule.ContentLimit { + return fmt.Errorf("cannot reduce content_limit for kind %d from %d to %d", kind, *currentRule.ContentLimit, *newRule.ContentLimit) + } + } + + // Check max_age_of_event - new limit cannot be smaller (smaller = more restrictive) + if currentRule.MaxAgeOfEvent != nil && newRule.MaxAgeOfEvent != nil { + if *newRule.MaxAgeOfEvent < *currentRule.MaxAgeOfEvent { + return fmt.Errorf("cannot reduce max_age_of_event for kind %d from %d to %d", kind, *currentRule.MaxAgeOfEvent, *newRule.MaxAgeOfEvent) + } + } + + // Check must_have_tags - cannot add new required tags (more restrictive) + for _, tag := range newRule.MustHaveTags { + found := false + for _, currentTag := range currentRule.MustHaveTags { + if tag == currentTag { + found = true + break + } + } + if !found { + return fmt.Errorf("cannot add required tag %q for kind %d (only owners can add restrictions)", tag, kind) + } + } + } + + // Check global rule write_deny - cannot blacklist owners or policy admins + for _, pk := range newPolicy.Global.WriteDeny { + if containsString(p.Owners, pk) { + return fmt.Errorf("cannot blacklist owner %s in global write_deny", pk) + } + if containsString(p.PolicyAdmins, pk) { + return fmt.Errorf("cannot blacklist policy admin %s in global write_deny", pk) + } + } + + // Check global rule read_deny - cannot blacklist owners or policy admins + for _, pk := range newPolicy.Global.ReadDeny { + if containsString(p.Owners, pk) { + return fmt.Errorf("cannot blacklist owner %s in global read_deny", pk) + } + if containsString(p.PolicyAdmins, pk) { + return fmt.Errorf("cannot blacklist policy admin %s in global read_deny", pk) + } + } + + // Check global rule size limits + if p.Global.SizeLimit != nil && newPolicy.Global.SizeLimit != nil { + if *newPolicy.Global.SizeLimit < *p.Global.SizeLimit { + return fmt.Errorf("cannot reduce global size_limit from %d to %d", *p.Global.SizeLimit, *newPolicy.Global.SizeLimit) + } + } + + return nil +} + +// ReloadAsOwner reloads the policy from an owner's kind 12345 event. +// Owners can modify all fields but the owners list must be non-empty. +func (p *P) ReloadAsOwner(policyJSON []byte, configPath string) error { + // Validate as owner update + if err := p.ValidateOwnerPolicyUpdate(policyJSON); err != nil { + return fmt.Errorf("owner policy validation failed: %v", err) + } + + // Use existing Reload logic + return p.Reload(policyJSON, configPath) +} + +// ReloadAsPolicyAdmin reloads the policy from a policy admin's kind 12345 event. +// Policy admins cannot modify protected fields (owners, policy_admins) and +// cannot reduce owner-granted permissions. +func (p *P) ReloadAsPolicyAdmin(policyJSON []byte, configPath string, adminPubkey []byte) error { + // Validate as policy admin update + if err := p.ValidatePolicyAdminUpdate(policyJSON, adminPubkey); err != nil { + return fmt.Errorf("policy admin validation failed: %v", err) + } + + // Use existing Reload logic + return p.Reload(policyJSON, configPath) +} + +// stringSliceEqual checks if two string slices are equal (order-independent). +func stringSliceEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + + // Create maps for comparison + aMap := make(map[string]int) + for _, v := range a { + aMap[v]++ + } + + bMap := make(map[string]int) + for _, v := range b { + bMap[v]++ + } + + // Compare maps + for k, v := range aMap { + if bMap[k] != v { + return false + } + } + + return true +} diff --git a/pkg/version/version b/pkg/version/version index 85bf558..6a16700 100644 --- a/pkg/version/version +++ b/pkg/version/version @@ -1 +1 @@ -v0.31.3 \ No newline at end of file +v0.31.4 \ No newline at end of file