Skip to content

fix: always try retained SPKs when opening sealed mail (#50) - #59

Open
chtnnh wants to merge 5 commits into
ni5arga:mainfrom
chtnnh:fix/50-spk-open-cap
Open

fix: always try retained SPKs when opening sealed mail (#50)#59
chtnnh wants to merge 5 commits into
ni5arga:mainfrom
chtnnh:fix/50-spk-open-cap

Conversation

@chtnnh

@chtnnh chtnnh commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • npm test / npm run typecheck
  • Aged SPK opens behind 24 OTKs

@commitchan

Copy link
Copy Markdown
Collaborator

Reviewed the substance and I agree with it. Holding merge only because the branch is conflictingmain has moved (#48, and now #63/#64/#65/#66). Please rebase onto latest main and force-push; I would rather you resolve the prekey-path conflicts than me.

This has also become the gating PR of the three prekey fixes, which is not obvious from the issue, so writing down why.

The fix is right

secretsForOpen() was slicing a concatenated [...otks, ...spks] list at OPEN_SECRET_CAP, which means the cap was never really a trial-decrypt budget — it was a budget that OTKs got to spend first and SPKs got whatever was left. The SPK ring is the fallback tier, the one that exists precisely so mail in flight across a rotation still opens; letting a full OTK pool evict it inverts the layering. Capping the OTK walk and always appending the full ring is the correct reading of what the constant was ever for, and the renamed doc comment now says the true thing.

The cost is bounded and small: the SPK ring is at most RECEIVE_KEY_RETENTION_MS / RECEIVE_KEY_ROTATION_MS ≈ 6–7 keys, so worst case moves from 28 to ~35 X25519 trials. That is a real but modest battery delta on a tier that cannot be allowed to silently drop mail.

Why this now gates #58

On today's main this bug is latent, which is presumably why it has not bitten anyone. The OTK pool does not actually reach OTK_POOL_CEILING in steady state, because issueOtks re-shares already-issued keys instead of minting new ones — so the pool settles around 16, 16 + 6 = 22 < 28, and every SPK survives the slice.

#58 removes that re-share loop (correctly — it is the #49 bug). The side effect is that the pool now pins at the ceiling. Measured on a local merge of #58 onto a0e6dc9:

# bundle OTK counts across six unanswered sends, and the resulting pool
main:      8,8,8,8,8,8   pool=16
with #58:  8,8,8,0,0,0   pool=24

And with the pool at 24, a probe on that same tree:

pool=24  spkRing=6  secretsTried=28  agedSpkOpens=false

24 OTKs + cap 28 leaves room for 4 SPKs out of a 6-key ring, so anything sealed to an SPK older than roughly 4 hours stops opening — inside the 6h window we advertise as retained. So #58 alone converts one silent-loss bug (#49) into a different one (#50). Landed together they fix both, and #58's own regression test still passes with this change in.

So the order I want is: #59 rebased and merged → #58#60. Nothing in #58 needs to change; it just should not go first. I have said the same on that PR.

Test note

The split into two tests is right — the old one was asserting the cap and the pool ceiling in the same case and would have gone green for the wrong reason after this change. One nit for when you rebase: assert.ok(secrets.length <= OPEN_SECRET_CAP + spks.length) and >= spks.length are both satisfied by a fair range of wrong implementations. The open() assertion underneath is what actually proves it, so this is cosmetic — but an exact assert.equal(secrets.length, OTK_POOL_CEILING + spks.length) after loading a known pool would pin the behaviour rather than bracket it.

OPEN_SECRET_CAP now limits only the OTK walk. Slicing SPKs off behind a
full OTK pool made in-flight SPK mail silently undeliverable.

Co-authored-by: Cursor <cursoragent@cursor.com>
@chtnnh

chtnnh commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (resolved FORWARD-SECRECY.md against the #48 forged-prekeys row) and force-pushed.

Also tightened the #50 length assert to an exact equality after loading a known pool:

assert.equal(secrets.length, OTK_POOL_CEILING + spks.length)

Tracking issue for the OTK-pool starvation / eviction #68

chtnnh and others added 4 commits July 23, 2026 15:56
CI's `npx expo install --check` started failing after upstream SDK 57
patch bumps (expo ~57.0.8 and related packages).

Co-authored-by: Cursor <cursoragent@cursor.com>
Lock @napi-rs/wasm-runtime's @emnapi/core and @emnapi/runtime peers at
the top level so CI's npm ci does not demand an unlocked 1.11.2.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
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.

[High][FS] OPEN_SECRET_CAP can drop aged SPK secrets behind a full OTK pool

2 participants