Accept only ASCII digits in PINs and compare them as bytes

[Sloth-on-meth]

Problem

str.isdigit() is True for Unicode digits such as "٣٣٣٣" or "²²²²". Those passed validate_pin_input, then hmac.compare_digest(str, str) raised TypeError ("comparing strings with non-ASCII characters is not supported"). Result: a 500 logged as EXCEPTION, and the attempt skipped the failure counters. A non-ASCII PIN in config.ini [pins] would make the same call raise for every login attempt, since the loop compares against every user.

Changes

• New is_valid_pin() (4–8 ASCII digits) and pins_equal() (bytes compare_digest) in users_store.py, used by the keypad, the store, and the migrate endpoints in place of five inline isdigit() checks.
• Non-ASCII digits now get the normal 400 Invalid PIN format and are counted.

(The matching non-ASCII admin-password fix is part of the secrets PR #45.)

Tests

tests/test_ascii_pins.py: validator accept/reject matrix, pins_equal with non-ASCII, store rejects Unicode PINs, Unicode PIN → counted 400, and a non-ASCII config PIN doesn't break other users' logins. Full suite passes (121), ruff clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PemRAtiBKjoZV6HkBCGDBC

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes
  • PINs must contain 4–8 ASCII digits; malformed PINs and PINs using non-ASCII digits are rejected.
  • Non-ASCII PINs no longer cause authentication errors. Invalid PIN attempts are counted as failed attempts, and valid logins continue to work even when another configured PIN contains Unicode digits.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c5be9231-5f90-4d7e-999a-9a7062cc24c7

📥 Commits

Reviewing files that changed from the base of the PR and between 8665ab2 and a5f9301.

📒 Files selected for processing (3)

• app.py
• tests/test_ascii_pins.py
• users_store.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

PIN handling now uses shared helpers to accept only 4–8 ASCII digits and to compare encoded PIN values. Storage, authentication, and migration paths use these helpers. Tests cover Unicode PIN rejection and valid ASCII PIN handling.

Changes

ASCII PIN handling

Layer / File(s)	Summary
Shared PIN rules users_store.py, tests/test_ascii_pins.py	is_valid_pin accepts strings containing 4–8 ASCII digits. pins_equal compares UTF-8-encoded PINs. User storage and disabled-user lookup use the shared helpers. Tests cover validation, comparison, and user creation.
Authentication and migration integration app.py, tests/test_ascii_pins.py	Input validation, door authentication, and single-user and bulk migration use the shared helpers. Tests check that a Unicode-digit PIN request returns HTTP 400 and increments the failed-attempt count, and that an ASCII PIN login returns HTTP 200.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a5f93

No actionable merge-blocking risk remains in the reviewed changes; the test fixture isolates the mutated application state.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to a5f93

ASCII-only validation improves authentication failure handling. However, an existing Unicode PIN override can now be ignored in favor of an older config PIN, allowing someone who knows that older credential to open the configured door. This depends on legacy overlapping records; their presence in deployed installations is unknown.

Retained concerns

• Medium · security · inferred: An active legacy JSON user with a 4–8-character Unicode-digit PIN no longer overrides a same-name config PIN. Because effective_pins starts with config credentials and only replaces them when the JSON PIN passes validation, upgrading can silently restore a superseded ASCII credential. Someone who knows that credential can authenticate as the affected user and invoke the configured door command. This is conditional on overlapping legacy records; disabled users are not re-enabled.

Security review details

Security Blast Radius

• inferred — The credential fallback concern affects active identities with overlapping config and legacy Unicode JSON PINs in an installation. Exploitation requires endpoint reachability and knowledge or guessing of the restored ASCII PIN, not an admin session. Its sensitive outcome is the configured Home Assistant entity command; it does not establish admin access or disclosure of the service token.

Security Findings and Attack Paths

• inferred — For an active JSON override containing Unicode digits and a same-name ASCII config credential, base behavior selected the JSON value and rejected the old credential through a comparison exception. Head behavior skips the override, accepts the old credential, and can send the door command. This is a source-supported conditional regression, not evidence that such records exist in production.

Trust Boundaries and Controls

• observed — Admin migration retains authenticated-session and CSRF requirements. Ordinary admin creation and update reject usernames still defined in config, limiting routine creation of overlapping records. New store writes reject Unicode PINs, and successful PIN authentication still checks active blocks. None of these controls removes an already-persisted active Unicode override or prevents its config fallback.

Resilience and Maintainability Implications

• observed — The added tests assert Unicode rejection, encoded comparison behavior, counted HTTP 400 responses, and successful ASCII authentication despite a Unicode config PIN. They support the intended failure-containment improvement. The tests were inspected, not executed during this review.

Hardening Proposals

• proposed — Preserve JSON ownership when an override PIN is invalid: suppress the same-name config credential rather than restoring it implicitly. Require an explicit valid replacement to recover access, and add a regression case for an active Unicode JSON override over an ASCII config PIN.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: PIN validation now accepts only ASCII digits, and PIN comparisons use bytes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}