Follow-up to the holistic review of the PIN-unification branch:
- /system/status now reads the real arm state from the arm_state_log
table via get_current_arm_state, instead of returning a hardcoded
'DISARMED' stub. Without this, polling after the new async 202
arm/disarm flow was a UX dead-end — clients never saw the state
change they just requested. DB read failures degrade gracefully.
- Operator guide: correct the claim that 'vigilar config set-pin'
populates recovery_passphrase_hash. It doesn't. recovery_passphrase
_hash has no CLI helper today; it must be set manually.
- Tests: add a fail-closed regression for verify_pin on malformed
stored hashes, and a companion test confirming the deprecation
warning stays silent on a fully migrated config.
All address specific review comments on the branch; no scope creep.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Describes the canonical [security] pin_hash key, the PBKDF2 format
emitted by 'vigilar config set-pin', and the deprecation warning for
the legacy [system] arm_pin_hash. Drops the three-way mismatch
known-limitation.
Code review on 9f203d8 caught a silent-failure mode: MessageBus.connect
logs and returns without raising when the MQTT handshake times out, so
an overloaded broker would let bus.publish() enqueue into paho's outbox
only to be discarded by the immediate disconnect(). The web endpoint
would return 202 even though the FSM never received the request.
Guard with 'if not bus.connected: raise RuntimeError'. The existing
try/except in arm_system/disarm_system catches the exception and turns
it into a 503 with the same log message as other bus failures.
Follow-up to efd5c4a. The plan invented {"accepted": True, ...} for
the new 202 responses, but every other 2xx endpoint in the Flask app
returns {"ok": True, ...} — including cameras.py:108 which is direct
prior art for a 202 with the same convention. The shared JS helper
at static/js/settings.js:54 does 'if (resp.ok && result.ok)' and was
falling into the error branch on our success responses, showing a
bogus "Save failed" toast after every arm/disarm click.
Keep the 202 status. Swap the body key from 'accepted' to 'ok'.
No JS change needed.
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.
Code review follow-up on f4d66dd:
- _handle_arm_request signature used "ArmStateFSM" as a string forward
reference even though the type is imported at module top.
_handle_event uses the bare form; match it for consistency.
- Add a test asserting that omitting triggered_by in an arm-request
payload defaults to "unknown". That value feeds the audit log, so
it deserves explicit regression coverage.
No behavior change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Code review follow-up on c9dd348 — the log = logging.getLogger(__name__)
assignment was interleaved between 'import X' and 'from X import Y'
statements. Move it below all imports per standard ordering.
No behavior change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If a config still has the legacy [system] arm_pin_hash set but no
[security] pin_hash, load_config logs a WARNING telling the operator
to re-run 'vigilar config set-pin'. The legacy field is still parsed
(so old configs don't fail validation) but ignored at runtime.
Part of issue #2 PIN hashing unification.
Adjacent secret leak in show_cmd noticed during Task 3 code review.
SecurityConfig has two sensitive fields and the redaction block only
covered pin_hash. vigilar config show would print the recovery
passphrase hash verbatim whenever one was configured.
One-line fix; same redaction pattern as the surrounding secrets.
Part of issue #2.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Was: HMAC-SHA256(random, pin) written to [system] arm_pin_hash —
no verifier in the codebase accepted this output.
Now: PBKDF2-SHA256 via alerts.pin.hash_pin written to [security]
pin_hash, matching what the web and FSM paths verify against.
Also fixes show_cmd to redact the new location.
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.
Was: unsalted SHA-256 read from [system] arm_pin_hash.
Now: PBKDF2-SHA256 600k iterations read from [security] pin_hash,
matching the web arm/disarm path and the alerts/pin module.
Also drops the redundant pin re-hash on the arm_state_log audit row
(a fresh PBKDF2 salt made the column valueless for traceability).
Part of issue #2 PIN hashing unification.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Topic for web-originated arm/disarm requests that the event processor
will subscribe to and dispatch to ArmStateFSM.transition. Part of the
PIN unification work (issue #2).
Plan document for issue #2 — the three-way PIN hash mismatch across
CLI, events FSM, and web arm/disarm. Proposes canonicalizing on
PBKDF2-SHA256 via alerts/pin and [security] pin_hash, deprecating
[system] arm_pin_hash, and wiring web arm/disarm through MQTT to the
FSM so the web buttons actually transition state.
Nine tasks, TDD throughout. No code changes in this commit.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
