diff --git a/pkg/policy/new_fields_test.go b/pkg/policy/new_fields_test.go index a048a19..4703347 100644 --- a/pkg/policy/new_fields_test.go +++ b/pkg/policy/new_fields_test.go @@ -1071,6 +1071,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 // ============================================================================= diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go index bd9ac2c..65688f9 100644 --- a/pkg/policy/policy.go +++ b/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) } @@ -1784,7 +1792,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 +1823,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) } } diff --git a/pkg/version/version b/pkg/version/version index 9e5a5c6..477e302 100644 --- a/pkg/version/version +++ b/pkg/version/version @@ -1 +1 @@ -v0.31.5 +v0.31.6