Skip to content

feat(credentials): enforce and surface lock-advertised PIN length bounds - #1279

Merged
raman325 merged 5 commits into
mainfrom
feat/credential-length-capabilities
Aug 21, 2026
Merged

feat(credentials): enforce and surface lock-advertised PIN length bounds#1279
raman325 merged 5 commits into
mainfrom
feat/credential-length-capabilities

Conversation

@raman325

Copy link
Copy Markdown
Owner

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.

  • Authoritative gate (slot coordinator): a non-empty PIN is validated against every bound lock's advertised length range before it is written. A violation raises ServiceValidationError naming 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.
  • Best-effort hints (PIN text entity): native_min/native_max now 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_KEY in text.py is the single per-type knob, and the helpers are credential-type-parametric.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:

Copilot AI review requested due to automatic review settings June 20, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added python Pull requests that update Python code enhancement New feature or request labels Jun 20, 2026
@codecov

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.07%. Comparing base (7cadf02) to head (fd8babc).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           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           
Flag Coverage Δ
python 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...components/lock_code_manager/domain/credentials.py 100.00% <100.00%> (ø)
...nents/lock_code_manager/domain/slot_coordinator.py 100.00% <100.00%> (ø)
...om_components/lock_code_manager/providers/_base.py 100.00% <100.00%> (ø)
custom_components/lock_code_manager/text.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@raman325 raman325 added the lcm-minor Minor version bump label Jun 22, 2026
@raman325
raman325 force-pushed the feat/credential-length-capabilities branch from a2ab72f to ac2213c Compare June 22, 2026 02:10
@raman325
raman325 force-pushed the feat/credential-length-capabilities branch from 1581ffd to 634bb5c Compare July 14, 2026 04:21
@raman325
raman325 force-pushed the feat/credential-length-capabilities branch from 634bb5c to aea0b45 Compare August 11, 2026 03:38
raman325 and others added 3 commits August 20, 2026 23:16
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
raman325 force-pushed the feat/credential-length-capabilities branch from 1fd3022 to 73ecca3 Compare August 21, 2026 03:18
raman325 and others added 2 commits August 20, 2026 23:38
… 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
@raman325
raman325 merged commit 1ae0353 into main Aug 21, 2026
13 checks passed
@raman325
raman325 deleted the feat/credential-length-capabilities branch August 21, 2026 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lcm-minor Minor version bump python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants