fix: unify PIN hashing across CLI, FSM, and web (closes #2) #7
Reference in New Issue
Block a user
Delete Branch "fix/issue-2-pin-unification"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #2.
Stacking: This PR is stacked on top of #5 (test isolation fixture,
fix/issue-3-test-isolation). Merge #5 first; this will auto-rebase onto main.Problem (recap from the issue)
Three incompatible PIN hashing schemes across two config keys:
Consequences:
vigilar config set-pinoutput was unusable by any verifier; the web arm endpoint checked the PIN but never transitioned state (stubbed{"ok": true}without calling the FSM); the FSM used trivially brute-forceable unsalted SHA-256.Resolution
alerts/pin(hash_pin/verify_pin).[security] pin_hash.[system] arm_pin_hashis still parsed (old configs don't fail validation) but ignored at runtime. A WARNING logs at startup if it's set and the new key isn't.POST /system/api/armpublishes aSYSTEM_ARM_REQUESTmessage on localhost MQTT with{mode, pin, triggered_by: "web"}. The event processor subscribes,ArmStateFSMverifies the PIN viaalerts.pin.verify_pinand performs the transition. HTTP response is202 Acceptedwith{"ok": true, "mode": ...}(matches the codebase convention used bycameras.py:108and theapiPostJS helper). Clients poll/system/statusfor the new state./system/statusno longer a stub: reads the real arm state fromarm_state_logviaget_current_arm_state. Without this, polling after the new async flow would have been a UX dead-end.vigilar config shownow redactssecurity.recovery_passphrase_hash(found during Task 3 code review — was leaking)._publish_arm_requestguards against silent MQTT connect-timeout failures — paho'sconnect()logs and returns without raising on handshake timeout, which would have produced 202 responses with no MQTT delivery. We checkbus.connectedafter connect and raise explicitly so the web returns 503.Security model notes
Commits (16)
cdd8cf4feat(constants): add Topics.SYSTEM_ARM_REQUEST7fda351fix(events): ArmStateFSM uses PBKDF2 via alerts.pinaf2ac56refactor(events): drop verify_pin alias and clarify audit-log comment8ffe1a3fix(cli): set-pin emits PBKDF2 under [security] pin_hash94cc206fix(cli): redact security.recovery_passphrase_hash in show_cmdc9dd348feat(config): deprecation warning for [system] arm_pin_hash7006078style(config): move log handle below import blockf4d66ddfeat(events): processor handles SYSTEM_ARM_REQUEST over MQTTc275404refactor(events): drop forward-ref quote and test triggered_by defaultefd5c4afix(web): arm/disarm actually transition the FSM via MQTT9f203d8fix(web): align arm/disarm 202 response shape with {"ok": true}08c689efix(web): raise on MQTT connect timeout in _publish_arm_requestac52198test: end-to-end PIN unification regression guardd72c04edocs(operator-guide): PIN hashing is unified038ab0afix: address final-review items (status endpoint, docs, tests)Plus
45afaf8(plan doc) at the base of the branch.Test plan
tests/unit/test_events.py::TestArmStateFSMtests/unit/test_cli_set_pin.pytests/unit/test_config.py::test_deprecation_warning_for_arm_pin_hashtests/unit/test_config.py::test_no_deprecation_warning_when_security_pin_hash_settests/unit/test_events.py::TestArmRequestDispatch(happy path, bad mode, default triggered_by)tests/unit/test_system_pin.py/system/statusreflects real FSM state —tests/unit/test_web.py::test_system_status_reflects_fsm_arm_statetests/unit/test_pin.py::test_verify_pin_rejects_malformed_hashtests/unit/test_pin_unification.pyconfig/vigilar.tomlstays clean after pytest (thanks to the session fixture from #5)Migration note for operators
If you previously ran
vigilar config set-pin, you will see a deprecation warning on the next start. Re-run the command and paste the newpin_hash = "pbkdf2_sha256$..."line into[security](not[system]). The old[system] arm_pin_hashcan then be deleted.Adds _handle_arm_request and a dedicated bus.subscribe on Topics.SYSTEM_ARM_REQUEST. Payload {mode, pin, triggered_by} is dispatched to ArmStateFSM.transition, which verifies the PIN via alerts.pin.verify_pin and performs the state change. This is the missing link for web /system/api/arm to actually move the system into an armed state. Part of issue #2.Was: /system/api/arm verified the PIN against [security] pin_hash and returned {ok: true} without ever calling the FSM. State never changed. Now: the endpoint publishes a SYSTEM_ARM_REQUEST message to the local MQTT broker. The event processor (see previous commit) picks it up, ArmStateFSM verifies the PIN via alerts.pin.verify_pin and performs the transition. Response is 202 Accepted; clients poll /system/status for the new state. Design: PIN travels over localhost-only MQTT, which matches the existing trust boundary for the internal bus.038ab0af12to5745388880Ship it.