feat(credentials): enforce and surface lock-advertised PIN length bounds - #1279
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1279 +/- ##
=======================================
Coverage 99.07% 99.07%
=======================================
Files 62 62
Lines 7680 7714 +34
Branches 520 520
=======================================
+ Hits 7609 7643 +34
Misses 71 71
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
5 tasks
raman325
force-pushed
the
feat/credential-length-capabilities
branch
from
June 22, 2026 02:10
a2ab72f to
ac2213c
Compare
raman325
force-pushed
the
feat/credential-length-capabilities
branch
from
July 14, 2026 04:21
1581ffd to
634bb5c
Compare
raman325
force-pushed
the
feat/credential-length-capabilities
branch
from
August 11, 2026 03:38
634bb5c to
aea0b45
Compare
Derive the tightest-common (min, max) PIN length across all bound locks and use it in two places: - The slot coordinator validates a non-empty PIN against every bound lock's advertised range before writing. This is the authoritative gate and raises ServiceValidationError naming each offending lock with its required range. Empty PINs (slot clears) are exempt, and locks whose capabilities are not cached (disconnected or unprobed) fail open so the sync layer surfaces any later device rejection. - The PIN text entity surfaces the live bounds as native_min/native_max hints, falling back to the default range for non-credential keys or an unsatisfiable intersection, and always widening to admit the current stored value so HA state rendering never raises. Adds LockCapabilities.length_bounds, the aggregate_length_bounds helper, and BaseLock.cached_capabilities (a synchronous, I/O-free read of the warmed capability cache). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 180b413b60c6
Address code-review findings on the length-bounds feature: - Stop surfacing the advertised minimum as native_min. Home Assistant's text.set_value validates len(value) < min before the coordinator, which blocked the empty string that clears a slot and pre-empted the per-lock error. native_min stays permissive; the coordinator is the authoritative minimum gate. The maximum is still surfaced as a hard ceiling. - Probe newly added locks in the background and re-push state so native_max reflects them instead of waiting for an unrelated write. - Restore @Final on _get_cached_capabilities (lost when cached_capabilities was inserted above it). - Compute the bound once per render now that native_min is constant. - Document the shared entry-wide lock set used by the gate and the entity. - Make the "0 means unbounded/unknown" length convention explicit on CredentialTypeCapability. Tests: boundary lengths, the unbounded-max message branch, removal reverting bounds, and a service-level regression proving a PIN clears when a lock advertises a positive minimum. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: de2e84619c12
`main` reached 100% line coverage in #1403, and rebasing this branch onto it left one new line uncovered: the skip in `_validate_credential_length` for a lock whose capabilities are cached but advertise no PIN type. That branch is distinct from the uncached one above it -- the limits are known here, they just say nothing about PIN length -- and treating that silence as a rejection would let a single non-PIN lock in the entry block every write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 4b3b4b9257ea
raman325
force-pushed
the
feat/credential-length-capabilities
branch
from
August 21, 2026 03:18
1fd3022 to
73ecca3
Compare
… lock `aggregate_length_bounds` returns a minimum above its maximum to signal that the locks cannot agree, and says the caller decides how to present it. The caller discarded the minimum and surfaced the maximum anyway, so a lock demanding at least 8 characters alongside one accepting at most 6 capped the field at 6 -- while the coordinator rejected anything under 8. The field refused the only value it would accept, and the error asked for something the widget would not let you type. `_DEFAULT_MAX` was already documented as the fallback "when bound locks advertise nothing or an unsatisfiable range"; only the first half was implemented. The coordinator still refuses the write, naming both locks and what each one wants, which is the part a person can act on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XDxiHpQJkRKWctS9BfQmbY Entire-Checkpoint: e0f125ab7a79
`native_max` surfaced the tightest advertised ceiling, which Home Assistant turns into the field's `maxlength`. A lock claiming a limit lower than it really accepts therefore stopped the keystrokes with no message at all -- the field simply refused to grow and nothing said why. That is the failure the minimum was already kept permissive to avoid, reintroduced at the other end and made worse: a generic error became no error. Both ends now stay at the permissive default and the coordinator is the only gate. It names the lock and the range it claims, which is what somebody needs to recognise an advertisement their lock does not actually honour. Everything that existed to keep that ceiling fresh goes with it: the per-key credential-type map, the add- and remove-lock overrides, the background capability probe, the state re-push, and the widening that stopped a newly-tightened ceiling raising on an already-stored PIN. `aggregate_length_bounds` had no caller left either. Net 250 lines removed for 36. A zero or absent advertisement was already read as "unknown" rather than a literal limit, and still is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XDxiHpQJkRKWctS9BfQmbY Entire-Checkpoint: 923b0f206016
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Derive the tightest-common
(min, max)PIN length across all of a slot's bound locks, then enforce it on write and surface it in the UI.ServiceValidationErrornaming each offending lock and its required range. Empty PINs (slot clears) are exempt. Locks whose capabilities are not cached (disconnected or not yet probed) and locks that do not advertise the credential type are skipped, so the write proceeds rather than blocking on unknown limits and the sync layer surfaces any later device rejection.native_min/native_maxnow reflect the live tightest-common range across the bound locks. Non-credential keys (the slot name) and an unsatisfiable intersection fall back to the default range so the control is never rendered inverted, and the range always widens to admit the current stored value (the empty string after a clear, or a PIN written before the lock advertised its limits) so HA state rendering never raises. The hints do not block input; the coordinator gate is what rejects out-of-range PINs.New building blocks:
LockCapabilities.length_bounds(credential_type)— effective(min, max)for one lock, normalizing non-positive advertised bounds to "unbounded"/"no minimum".aggregate_length_bounds(...)— folds many locks into one tightest-common range (largest min, smallest max); UI defaults deliberately live in the caller.BaseLock.cached_capabilities— synchronous, I/O-free read of the warmed capability cache for synchronous callers.This is groundwork that generalizes cleanly to other credential types:
CREDENTIAL_TYPE_BY_CONF_KEYintext.pyis the single per-type knob, and the helpers are credential-type-parametric.Type of change
Additional information