feat(credentials): enforce and surface lock-advertised PIN length bounds#1279
Open
raman325 wants to merge 2 commits into
Open
feat(credentials): enforce and surface lock-advertised PIN length bounds#1279raman325 wants to merge 2 commits into
raman325 wants to merge 2 commits into
Conversation
github-actions
Bot
added
python
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1279 +/- ##
==========================================
+ Coverage 97.10% 97.13% +0.03%
==========================================
Files 54 54
Lines 6553 6638 +85
Branches 460 460
==========================================
+ Hits 6363 6448 +85
Misses 190 190
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
5 tasks
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
raman325
force-pushed
the
feat/credential-length-capabilities
branch
from
a2ab72f to
ac2213c
Compare
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