Add extensive tests and improve policy configuration handling
Some checks failed
Go / build-and-release (push) Has been cancelled
Some checks failed
Go / build-and-release (push) Has been cancelled
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user