Skip to content

fix: detect shared-key notifications (call to_string())#524

Merged
cpswan merged 1 commit into
trunkfrom
fix/shared-key-notification-detection
Jul 3, 2026
Merged

fix: detect shared-key notifications (call to_string())#524
cpswan merged 1 commit into
trunkfrom
fix/shared-key-notification-detection

Conversation

@cconstab

@cconstab cconstab commented Jul 2, 2026

Copy link
Copy Markdown
Member

In at_client/connections/atmonitorconnection.py the monitor classified incoming
notifications with:

if key.startswith(str(self.atsign.to_string) + ":shared_key@"):

self.atsign.to_string is a bound method, so str(...) of it is a method repr
(e.g. <bound method AtSign.to_string of ...>) that never prefixes a real key. As a
result SHARED_KEY_NOTIFICATION was never emitted and shared-key notifications were
mis-typed as ordinary UPDATE_NOTIFICATIONs. (Line 124 in the same file correctly
uses to_string(), so this is an isolated missing-parentheses bug.)

Consequence: the client never caches incoming shared keys via the notification path
and always falls back to lookup-on-demand — which contributes to first-contact decrypt
failures.

Fix: extract the check into a testable static helper
_is_shared_key_notification(atsign, key) that calls to_string().

Tests: test/monitor_shared_key_test.py — network-free, both branches
(shared-key key vs. regular update key).

The monitor classified notifications with str(self.atsign.to_string) — the bound
method, not its result — so the prefix never matched and SHARED_KEY_NOTIFICATION
was never emitted; shared-key notifications were mis-typed as UPDATE. Extract the
check into a testable _is_shared_key_notification() helper that calls to_string(),
matching the correct usage elsewhere in the file.

Adds network-free unit tests for both branches.
@cconstab cconstab requested review from Xlin123 and cpswan July 3, 2026 01:37
@cpswan cpswan merged commit fc99bc5 into trunk Jul 3, 2026
9 checks passed
@cpswan cpswan deleted the fix/shared-key-notification-detection branch July 3, 2026 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants