From 54ad58c87043eb427eb0af8e7410b85c34654952 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sun, 5 Apr 2026 11:33:58 -0400 Subject: [PATCH] refactor(events): drop verify_pin alias and clarify audit-log comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review feedback on the Task 2 commit (7fda351): - The 'verify_pin as _verify_pin_hash' alias was unnecessary — the method self.verify_pin and the module-level verify_pin do not collide (one is accessed via self, the other via the bare name). Removing the alias matches how web/blueprints/system.py already imports verify_pin and makes the call site read cleanly. - The comment on the insert_arm_state None argument now explains WHY (PBKDF2 salt is fresh per call, so re-hashing is worthless for audit correlation) instead of only referencing the issue. No behavior change. Part of issue #2. --- vigilar/events/state.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/vigilar/events/state.py b/vigilar/events/state.py index 12f66a7..5ec28ce 100644 --- a/vigilar/events/state.py +++ b/vigilar/events/state.py @@ -5,7 +5,7 @@ import time from sqlalchemy.engine import Engine -from vigilar.alerts.pin import verify_pin as _verify_pin_hash +from vigilar.alerts.pin import verify_pin from vigilar.config import VigilarConfig from vigilar.constants import ArmState, EventType, Severity, Topics from vigilar.storage.queries import get_current_arm_state, insert_arm_state, insert_event @@ -46,7 +46,7 @@ class ArmStateFSM: if not self._pin_hash: # No PIN configured — allow all transitions return True - return _verify_pin_hash(pin, self._pin_hash) + return verify_pin(pin, self._pin_hash) def transition( self, @@ -66,7 +66,9 @@ class ArmStateFSM: old_state = self._state self._state = new_state - # Log to database (pin_hash column is no longer populated — see #2) + # pin_hash is always None here: PBKDF2 uses a random salt per call, so + # re-hashing the pin now would produce a value unrelated to the stored + # hash, making the column useless for audit correlation. See issue #2. insert_arm_state(self._engine, new_state.value, triggered_by, None) # Log event