Pro: withhold message features from a revoked sender - #2177
Merged
mpretty-cyro merged 3 commits intoAug 23, 2026
Merged
Conversation
The backend serves a 24h retry_in, so a client's second poll is a day after its first and no revocation behaviour is reachable within a test. sessionForceProRevocationRefresh discards the scheduled poll instead of shortening the interval or adding a fetch path, so the ordinary scheduler still decides when to poll and a test exercises that path rather than a parallel one. Cancelling is the local equivalent of moving a stored next-poll instant into the past: the pending poll here is a WorkManager job carrying a delay, and schedule() enqueues with none, so its KEEP policy would otherwise preserve a job up to 48h from running. The cancel sits immediately before the scheduling it affects rather than where the flag is parsed. The two orderings are not equivalent — reversed, it would cancel the poll it was meant to force. true or 1 enables and any other value disables, rather than presence enabling, since a key present with a disabling value has to disable. An absent extra leaves the stored value alone, as the others do: apply runs on each HomeActivity creation rather than once per test, and a fresh install creates it again after onboarding with no extras attached. Reachable only where ALLOW_QA_LAUNCH_CONFIG is set, checked both where the flag is written and where it is read.
The revocation list overrides the validity of proofs already in circulation, but libsession's decode cannot see it — the list is cached locally rather than travelling with the message — so a cryptographically valid proof carried its features regardless of whether the sender's generation had been revoked. A revoked sender kept the higher character limit. Cleared where the bitset is populated rather than at each consumer, so the bitset is truthful and everything reading it conforms without knowing about revocation: the incoming character limit, the features listed in message info, and what is persisted with the message. Honours the entry's effective timestamp, as every other read of the list does, so a revocation dated in the future does not withdraw features early. The badge and avatar are unaffected. They resolve from the contact's stored proof via a separate path that already filters revocations.
The comment said "only once per install", which predates clearProExpiryView() resetting the latch when the account reads Active again. The real model is once per Pro cycle, which is the intended behaviour: Pro can only expire once per cycle, so one warning per cycle is the point. Records why, and that firing per-launch or per-status-change was considered and rejected -- the stale comment invited exactly that "fix", and the other clients are being aligned to this model rather than away from it. Also notes the latch is written on dismissal rather than on display, so a CTA shown to a process that dies before the user dismisses it is shown again. Comment only, no behaviour change.
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.
One behaviour change, one test hook and one comment correction, all in the Pro revocation path. Each commit carries its own reasoning; the summary below is the shape of the set.
The bug
Message features survived a sender's revocation. The revocation list overrides the validity of proofs already in circulation, but libsession's decode cannot see it — the list is cached locally rather than travelling with the message — so a cryptographically valid proof carried its features regardless of whether the sender's generation had been revoked, and a revoked sender kept the higher character limit.
Cleared where the bitset is populated rather than at each consumer, so the bitset is truthful and everything reading it conforms without knowing about revocation: the incoming character limit, the features listed in message info, and what is persisted with the message. The entry's effective timestamp is honoured, as every other read of the list does, so a revocation dated in the future does not withdraw features early.
The badge and avatar are unaffected — they resolve from the contact's stored proof via a separate path that already filters revocations.
Test hook
sessionForceProRevocationRefreshdiscards the scheduled poll rather than shortening the interval or adding a fetch path, so the ordinary scheduler still decides when to poll and a test exercises that path rather than a parallel one. The backend serves a 24hretry_in, so a client's second poll is a day after its first and no revocation behaviour is otherwise reachable within a test.Cancelling is the local equivalent of moving a stored next-poll instant into the past: the pending poll here is a WorkManager job carrying a delay, and
schedule()enqueues with none, so itsKEEPpolicy would otherwise preserve a job up to 48h from running. The cancel sits immediately before the scheduling it affects rather than where the flag is parsed — reversed, it would cancel the poll it was meant to force.trueor1enables and any other value disables, rather than presence enabling, since a key present with a disabling value has to disable. Reachable only whereALLOW_QA_LAUNCH_CONFIGis set, checked both where the flag is written and where it is read.Comment correction
The expiry-CTA comment claimed "only once per install", which predates
clearProExpiryView()resetting the latch when the account reads Active again. The real model is once per Pro cycle, which is the intended behaviour — Pro can only expire once per cycle, so one warning per cycle is the point — and the other clients are being aligned to this model rather than away from it. The stale comment invited a per-launch "fix"; the note records why that was rejected, and that the latch is written on dismissal rather than on display. Comment only, no behaviour change.