Skip to content

Commit 05ccf3d

Browse files
author
Clawdio
committed
fix(security): Address review feedback
- Add test for PolicyDecision PartialEq (Issue #3) - Clarify matches_action location in mod.rs comment (Issue #2) - Enhance TimeOfDay UTC docs with timezone conversion examples (Issue #4) - Note: Issue #1 (PolicyDecision PartialEq) was already present in codebase All 105 tests pass, clippy clean, formatting clean.
1 parent 716e309 commit 05ccf3d

3 files changed

Lines changed: 39 additions & 4 deletions

File tree

crates/nv-security/src/policy/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,6 @@ pub use rate_limit::RateLimiter;
3636
pub use signing::{sign_policy, verify_policy};
3737
pub use types::PolicyDecision;
3838

39-
// Note: Internal types (Condition, DefaultPolicy, PolicyConfig, PolicyRule, matches_action)
40-
// are NOT re-exported, keeping the public API surface minimal.
41-
// They remain accessible within this module via their submodules (types::*, util::*).
39+
// Note: Internal types (Condition, DefaultPolicy, PolicyConfig, PolicyRule from types::*)
40+
// and utility functions (matches_action from util::*) are NOT re-exported, keeping the
41+
// public API surface minimal. They remain accessible within this module via their submodules.

crates/nv-security/src/policy/tests.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,39 @@ mod integration_tests {
3333
assert!(matches!(cloned, PolicyDecision::Deny { .. }));
3434
}
3535

36+
#[test]
37+
fn test_policy_decision_partial_eq() {
38+
let allow1 = PolicyDecision::Allow;
39+
let allow2 = PolicyDecision::Allow;
40+
assert_eq!(allow1, allow2);
41+
42+
let deny1 = PolicyDecision::Deny {
43+
reason: "test".to_string(),
44+
};
45+
let deny2 = PolicyDecision::Deny {
46+
reason: "test".to_string(),
47+
};
48+
let deny3 = PolicyDecision::Deny {
49+
reason: "other".to_string(),
50+
};
51+
assert_eq!(deny1, deny2);
52+
assert_ne!(deny1, deny3);
53+
54+
let confirm1 = PolicyDecision::Confirm {
55+
prompt: "test?".to_string(),
56+
};
57+
let confirm2 = PolicyDecision::Confirm {
58+
prompt: "test?".to_string(),
59+
};
60+
assert_eq!(confirm1, confirm2);
61+
62+
let rate1 = PolicyDecision::RateLimit { wait_ms: 100 };
63+
let rate2 = PolicyDecision::RateLimit { wait_ms: 100 };
64+
let rate3 = PolicyDecision::RateLimit { wait_ms: 200 };
65+
assert_eq!(rate1, rate2);
66+
assert_ne!(rate1, rate3);
67+
}
68+
3669
#[test]
3770
fn test_conditions_time_of_day() {
3871
let toml = r#"

crates/nv-security/src/policy/types.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ pub enum Condition {
6363
/// Time of day constraint (24-hour format, evaluated in UTC)
6464
///
6565
/// **Important:** Times are evaluated in UTC, not local time.
66-
/// For example, `after: "09:00"` means 09:00 UTC.
66+
/// For example, `after: "09:00"` means 09:00 UTC (which is 1:00 AM PST
67+
/// or 5:00 AM EDT). If you want to restrict actions to local business
68+
/// hours, you must convert your local time to UTC.
6769
///
6870
/// Supports midnight crossing (e.g., `after: "22:00", before: "06:00"`).
6971
#[serde(rename = "time_of_day")]

0 commit comments

Comments
 (0)