Skip to main content

FEN — Design & Security Review Findings

Status: recorded · review pass over the design docs (doc/*.md, ARCHITECTURE.md) and the implementation (packages/*, apps/fen, services/fen-provisioner, infra/*) as of 5db2fed. Every finding is tracked as a GitHub issue (#15, #17–#35); resolutions land in their own PRs and are annotated per finding below. Scope: correctness, security, and topics left out. This document records problems; it does not change the design. Each finding has a proposed direction, not a final decision. Legend — severity: 🔴 high (blocks a core guarantee or ships a real vuln) · 🟠 medium (wrong or missing, exploitable/observable under realistic conditions) · 🟡 low (correctness/clarity, low blast radius).

Summary​

The cryptographic core is sound (libsodium-only, XChaCha20-Poly1305, Ed25519, encrypt-then-sign, canonical serialization with golden vectors) and the provisioner shows real security care (replay cache on decoded bytes, constant error responses, no secret logging, X-Forwarded-For not trusted). The serious problems are in the authorization layer the crypto cannot provide — membership control, credential rotation, and the availability/griefing consequences of the completeness gate — plus a set of spec-vs-implementation drifts where the code contradicts the normative docs.

#SeverityAreaOne-lineTracked
1🔴Backend/rotationS3-key rotation on removal locks out remaining members (no new-credential channel)#15 · addressed by PR #40
2🔴Invite/threat modelLeaked invite = permanent irrevocable compromise in v1 (rotation out of v1 scope)#17 · addressed below
3🔴MembershipAcceptance underdesigned: no ack event, self-certifying joins, attacker-winnable tiebreak, invite_id missing from payload#18 · addressed below
4🔴Event logEvent-suppression via author-chosen event_id collision#19
5🔴AvailabilityCompleteness gate is a one-actor denial-of-settlement weapon#20 · addressed below
6🟠CryptoEd25519→X25519 key reuse for wrapping used without justification#21
7🟠CryptoEpoch-key wrap construction unspecified (static vs ephemeral ECDH → forward secrecy claim unbacked)#22
8🟠SyncconditionalWrite:false makes probe().usable always false; multi-device read-modify-write PUT loses events#23
9🟠Impl vs specExpenseUpdated spec'd as full snapshot, implemented as a patch#24
10🟠Impl vs specSplit-sum invariant validated nowhere in the reducer#25
11🟠Impl vs specSignature verification absent from the ingest/reduce path (layer unimplemented)#26 · addressed below
12🟠Infra/DRGarage replication_factor=1 single node contradicts the "≥2 backends" durability mitigation#27
13🟠PrivacyPubkey filenames leak the cross-group social graph (only an open question today)#28
14🟠AbuseProvisioner group-creation abuse cheap to mount (free self-minted keys, rotatable IP)#29
15🟡Crypto hygieneCanonical parser doesn't enforce id/hex/UUID shape or NFC (partial §9 hardening)#30
16🟡Impl vs specpermission_rules.dart fully stubbed (UnimplementedError)#31
17🟡Impl vs specParticipant.public_key still documented as X25519 (signatures are Ed25519)#32
18🟡Ops/legalNo at-rest encryption for the local SQLite event log; no GDPR/compliance surface#33
19🟡Deep linksAASA / assetlinks are TODO placeholders → https trust anchor degrades to hijackable custom scheme#34 · addressed by documenting the launch gate requirement
20🟡VariousTopics left out (see §"Absent topics")#35

1. 🔴 S3-key rotation on removal locks out remaining members​

Tracked: #15 · Addressed: PR #40 implements the direction below as two-phase rotation, with one deviation: the new descriptor travels in a StorageKeyRotated event encrypted under the post-removal epoch key (which KeyEpochUpdate already wraps per member) rather than being per-member-wrapped itself — same recipient set, no new wrapping machinery, and straggler recovery unifies with KeyRewrapRequest. See data-model.md §3.10, backend-architecture.md §6.1, security.md §4.8 (C-5, E-5).

Where: doc/security.md §4.8 (invariant E-1, E-2), doc/backend-architecture.md §6, services/fen-provisioner/internal/api/server.go (handleRotateKey).

Problem. Invariant E-1 requires member removal to rotate the per-group S3 key. handleRotateKey mints a new key and immediately DenyBucketKeys the old one. The per-group S3 key is shared by every member, so rotation invalidates the credential for all remaining members at once. The only documented distribution channel for a StorageDescriptor is the invite package (one-time, at join) and the bucket itself — which remaining members can no longer read once the old key is denied. There is no specified mechanism to deliver the new credential to the members who should keep access. As written, "rotate on removal" locks out the whole group.

Direction. Define a credential-hand-off protocol, e.g. a signed StorageKeyRotated event carrying the new descriptor per-member-wrapped (X25519 to each remaining member), published under the old key, with the provisioner deferring DenyBucketKey(old) until either all remaining members ack or a bounded window elapses. Document the exact security window this opens (removed member can still write during the overlap) and how the key-epoch rotation interleaves.

2. 🔴 A leaked invite is a permanent, irrevocable compromise in v1​

Tracked: #17 · Resolved 2026-07-05: direction (b) — the provisioner mints a short-TTL (default 30 min), single-use invite credential distinct from the group's shared key (POST /v1/groups/:id/invites), the joiner exchanges it for the shared credential once its own MemberJoined write succeeds (POST /v1/groups/:id/invites/:invite_id/complete), and the reaper denies+deletes invite credentials once used or expired. Key-epoch rotation and member removal remain explicitly out of v1 scope, unchanged. See fen.md §2.4, security.md §4.3, and implementation.md §5.5.

Where: doc/fen.md §2.4, doc/security.md §4.3, doc/implementation.md §5.5.

Problem. Three separately-acknowledged decisions compose badly:

  • The invite package carries both group_key and the S3 bearer credential.
  • expiry/revocation are client-enforced only; a malicious holder ignores them and uses the raw S3 creds + group_key directly. expiry is honest-client UX, not a security control.
  • Key-epoch rotation and member removal are explicitly out of v1 scope.

Net: shipping v1 has no recovery path from a leaked invite short of abandoning the group. Invites travel via WhatsApp/iMessage/SMS — i.e. third-party servers, cloud message backups, clipboard sync — so leakage is a realistic, not exotic, event. Also: s (secret) and p (ciphertext) travel in the same URL, so the encryption adds nothing against anyone who sees the whole link; it only defends against partial exposure of p without s.

Direction. Either (a) pull a minimal rotation path into v1, or (b) state the limitation prominently in security.md §4.3 and the onboarding copy, and reduce blast radius (short default expiry actually enforced server-side by the provisioner minting single-use, short-TTL credentials rather than a long-lived bearer key in the link).

3. 🔴 Membership acceptance is underdesigned and partly wrong​

Tracked: #18 · Resolved 2026-07-05: MemberJoinedPayload now carries invite_id; MemberAdmitted is an explicit organiser-authored admission event binding public_key -> invite_id; the reducer keeps self-signed joins pending and promotes only matching organiser admissions; the organiser asks the provisioner which pubkey won an invite via GET /v1/groups/:id/invites/:invite_id, replacing the attacker-winnable (lamport, event_id) duplicate-join tiebreak. The organiser app invokes admission reconciliation when loading a group with pending joins. Real cryptographic enforcement that the MemberAdmitted signature actually belongs to the organiser — deferred at the time to issue #26 — is now in place; see finding 11 below.

Where: doc/security.md §4.3–4.4, doc/fen.md Flow 3, doc/data-model.md §3.3/§3.5, packages/fen_events/lib/src/payloads.dart.

Problems.

  1. §4.3 says "a member must acknowledge [a join] before Bob appears in the roster" — there is no acknowledgement event type in the schema. The mechanism doesn't exist.
  2. MemberJoined is self-certifying: peers verify its signature against the pubkey inside its own payload, so §4.4's "author_id is a known participant" is bootstrapped by the event being validated. Effective membership = possession of the invite link.
  3. fen.md Flow 3 claims a duplicate joiner "cannot read any group content — E2EE." False — they hold group_key + S3 key from the same package and read/write everything in the epoch.
  4. The duplicate-join tiebreak ("first by (lamport, event_id) wins") is attacker-winnable: both sort keys are author-chosen, so a malicious joiner stamps lamport=1 + minimal event_id and deterministically beats the honest joiner.
  5. The rule keys on invite_id, but MemberJoinedPayload has no invite_id field (neither in data-model.md §3.3 nor payloads.dart). As specified it is unimplementable.

Direction. Introduce an explicit organiser-signed admission event (MemberAdmitted) that binds invited_pubkey → invite_id, make roster membership depend on it rather than on the self-signed join, add invite_id to the join payload, and make single-use enforcement organiser-authoritative (the organiser, not a client tiebreak, resolves duplicate joins).

4. 🔴 Event suppression via author-chosen event_id collision​

Tracked: #19

Where: doc/data-model.md §3.9 (reducer invariant 5), doc/canonical-serialization.md §12 (open decision #3), packages/fen_ledger/lib/src/event_reducer.dart.

Problem. Dedup is "first (group_id, event_id) wins; later ones silently ignored." But event_id is an author-assigned UUIDv4 inside the signed envelope, not a content hash. A member can pre-publish an event reusing an event_id an honest event will later use (or flood ids), and peers that saw the malicious one first silently drop the honest event as a "duplicate." author_seq contiguity doesn't catch it — the slot is filled. Uniqueness isn't enforced by construction because it's an author choice.

Direction. Resolve canonical-serialization open decision #3 in favour of a content-hash event_id (SHA-256 of canonical bytes), or bind dedup to (author_id, author_seq) rather than event_id so an author cannot suppress another author's event. Note the AAD dependency on event_id existing pre-encryption (§7) when choosing.

5. 🔴 The completeness gate is a one-actor denial-of-settlement weapon​

Tracked: #20 · Resolved 2026-07-08 (design): the gate was spec-only (no author_frontier gate exists in code yet), and it was the design that was the weapon — so this fix is design-primary, adopting both halves of the Direction because each closes a different half of the attack. (1) Provable claimed_seq (data-model.md §3.9 and new §3.12; fen.md §2.5; error-handling.md Error 7): claimed_seq now advances only on receipt of a validly-signed event authored by that member, and the per-author high-water-mark advertisements peers exchange are demoted to routing hints that request ranges but never raise the frontier. So no member can inflate another member's frontier, and none can raise their own with a fabricated future seq — the zero-cost, no-key "advertise 999 and never upload" freeze is gone. (2) A bounded escape (new §3.12; Error 7 recovery option 4): a frontier stalled past a timeout is overridden by an organiser-signed SettlementSnapshot over the provable subset ("settle on what's provable") — the same organiser-signed settle-on-a-cut event already used before member removal (§3.10). It is organiser-only (enforced at ingest by VerifyingEventSink, issue #26) and checkable: its covered_frontier names the exact cut, so every member recomputes and rejects a snapshot whose balances don't match the provable log — the organiser picks the cut, never the numbers. SettlementSnapshotPayload is brought in line with that spec (epoch, covered_frontier, stalled_members, as_of) and codec-wired with a round-trip test.

Deliberately NOT covered: building the gate itself (author_frontier table, gate evaluation, timeout T, snapshot-authoring UI) — a separate implementation milestone; and a malicious/absent organiser (a quorum/rotation override that removes the organiser as a single point of failure is out of v1 scope, riding with the never-returning-organiser open question in §20 Absent topics). v1's threat model — a single ordinary member freezing the whole group — is fully closed.

Where: doc/error-handling.md Error 7, doc/data-model.md §3.9, §3.12.

Problem. Settlement is hard-blocked until contiguous_seq == claimed_seq for every member. claimed_seq rises when any peer advertises a high-water mark. So one member can advertise author_seq: 999, never upload 1..999, and permanently block settlement for the whole group — a single-actor denial of the app's core purpose. No timeout, quorum override, or "settle on what's provable" escape hatch exists. Combined with the bearer key (any member can also delete others' logs, §4.3), one bad actor can both corrupt and freeze the group.

Direction. Add a bounded escape: organiser-signed SettlementSnapshot can override the gate for members whose frontier is stalled beyond a timeout; or require the high-water-mark claim to be signed by that member over an event they actually published so a member can only block on their own real events, not fabricated future ones.

6. 🟠 Ed25519→X25519 key reuse for wrapping used without justification​

Tracked: #21

Where: packages/fen_crypto/lib/src/key_exchange.dart (x25519PublicFromEd25519, x25519SecretFromEd25519), doc/security.md §4.1.

Problem. One identity keypair is reused for both signing (Ed25519) and ECDH key-wrapping (X25519 via crypto_sign_ed25519_*_to_curve25519). Sharing a key across signing and KEM is a known cross-protocol footgun (the two schemes' proofs don't compose for free; Ed25519→X25519 has cofactor/malleability caveats). It's usually fine in practice, but here it reads as a convenience ("one key both signs and wraps") with no security argument.

Direction. Either derive a separate wrapping key, or keep the reuse and add an explicit justification citing the specific safety conditions (domain separation, no shared nonces, the libsodium conversion's guarantees).

7. 🟠 Epoch-key wrap construction is unspecified​

Tracked: #22

Where: doc/data-model.md §3.3 (KeyEpochUpdatePayload.member_keys[].wrapped_key), §3.10.

Problem. wrapped_key is "group_key encrypted to this member's pubkey," but the construction is undefined: static-static ECDH (no forward secrecy; identical wraps to the same member each epoch) or ephemeral-static/ECIES (needs a per-recipient ephemeral pubkey field the payload lacks). This is load-bearing for the "forward secrecy on removal" claim (§4.8) and is missing.

Direction. Specify an ECIES-style wrap (ephemeral X25519 pubkey per recipient + HKDF + AEAD), add the ephemeral-pubkey field to the payload, and state the forward-secrecy properties precisely.

8. 🟠 conditionalWrite:false breaks probe().usable; multi-device PUT loses events​

Tracked: #23

Where: packages/fen_sync/lib/src/storage_backend.dart (BackendCaps.usable), packages/fen_sync/lib/src/s3_backend.dart (probe), packages/fen_sync/lib/src/sync_manager.dart (flush), doc/security.md §4.8 (invariant E-3), doc/key-management.md §4.7 multi-device.

Problem. BackendCaps.usable requires conditionalWrite == true; S3Backend.probe() hardcodes false ("Garage If-Match support unconfirmed"). So probe().usable is always false and invariant E-3 ("a backend must pass probe() before binding") can never be satisfied as coded. flush() does a read-modify-write PUT of the whole log file with ifMatchEtag; if Garage ignores If-Match, two devices of the same member (permitted by §4.7) silently lose-update each other's events. The "single writer per object" guarantee holds only for one device, which §4.7's key-copy multi-device path explicitly breaks.

Direction. Confirm Garage's conditional-write behaviour and either (a) rely on it and fix probe(), or (b) redesign append to not require whole-file RMW (e.g. per-event object keys logs/<pubkey>/<author_seq>.json, which also removes the collision surface). Reconcile the multi-device section with the single-writer assumption now, since key-copy is possible in v1.

9. 🟠 ExpenseUpdated: full-snapshot spec vs patch implementation​

Tracked: #24

Where: doc/data-model.md §3.8 (Projection Mechanics, "full snapshot… not a diff"), packages/fen_events/lib/src/payloads.dart (ExpenseUpdatedPayload.patch), packages/fen_ledger/lib/src/event_reducer.dart (_expenseUpdated).

Problem. The normative doc says ExpenseUpdated is a complete snapshot with partial-patch semantics "explicitly excluded." The code models it as Map<String,dynamic> patch and applies patch[x] ?? old.x — a diff. It's idempotent, but contradicts the spec, and a patch changing settlementAmount while omitting splits silently violates the split-sum invariant.

Direction. Pick one and align docs+code. If snapshot: type the payload with all required fields and validate the split-sum on apply. If patch: update the normative doc and add cross-field validation.

10. 🟠 Split-sum invariant validated nowhere in the reducer​

Tracked: #25

Where: doc/security.md §4.4, doc/canonical-serialization.md §8, packages/fen_ledger/lib/src/event_reducer.dart, packages/fen_events/lib/src/payloads.dart.

Problem. sum(splits) == settlementAmount is mandated "on write and on receipt," but neither reduce() nor the payload constructors check it. A crafted event violating it corrupts balances and breaks the sum(net) == 0 cross-check that debt simplification relies on.

Direction. Enforce the invariant at ingest (before reduce) and reject violating events per the §9 "reject, don't sanitize" rule; add a golden test with a deliberately unbalanced event.

11. 🟠 Signature verification is absent from the ingest/reduce path​

Tracked: #26 · Resolved: VerifyingEventSink (apps/fen/lib/core/data/verifying_event_sink.dart) implements the EventSink interface and is now the only path a raw synced line can take before it reaches reduce(). For each line it: decodes the canonical SignedEnvelope; verifies author_signature against the envelope's own claimed author_id (Ed25519, over the still-encrypted envelope, BEFORE decrypting); decrypts and decodes the payload; dedupes by event_id against the existing log and the rest of the batch; and resolves roster authority by folding the same reduce() state the projector uses (organiser from GroupCreated, active members via MemberAdmitted) to check the claimed author is currently allowed to publish that event type. Only events passing every check are appended to the local log; everything else is dropped and reported via a typed EventRejection — part of the EventSink/SyncManager contract itself (packages/fen_sync/lib/src/sync_manager.dart: both ingest() and pull() return List<EventRejection> rather than discarding it), not an app-only bonus type reachable only from a test. Tests cover a tampered envelope, a signature valid under the wrong key, a non-roster author, a not-yet-admitted member attempting a privileged event, a forged MemberAdmitted (right shape, wrong signer), and genuine cross-device round trips through the real JoinInviteService/AddExpenseService encoders.

Production wiring (cross-review follow-up): an earlier draft of this fix left VerifyingEventSink registered but never actually invoked by any production code path — the same class of defect PR #56 originally shipped. GroupSyncService (apps/fen/lib/core/data/group_sync_service.dart) is now the real, reachable call site: it builds a real S3Backend from the group's group_storage binding and push+pulls through StorageBackendSyncManager, wired opportunistically into groupBalanceProvider (the same group-load point MemberAdmissionService.reconcilePendingAdmissions uses). This is deliberately not a continuous background pull loop — that remains a separate, larger milestone — but it is a genuine, tested, non-test production call site, not just DI registration.

Deliberately NOT covered by this fix (see doc/security.md §4.4's "Honestly remaining" note for the full list): the EXPENSE EVENTS content-validation rules (split-sum, currency, participant-is-member — issue #25); permission_rules.dart's creator_only edit-permission check (issue #31); organiser-only authorship of GroupSettingsUpdated/GroupClosed/MemberRemoved (member-removal-style enforcement, issue #15-style); a continuous background pull loop (separate milestone, above); and the inherent bootstrap-trust question of a brand-new group's very first GroupCreated. Implementing this issue also surfaced (and fixed, as a necessary prerequisite — confirmed via a repo-wide grep for every SignedEnvelope(/hkdf(/encrypt( construction site) a separate, pre-existing bug where BOTH join_invite_service.dart (MemberJoined) and add_expense_service.dart (ExpenseLogged) sealed events with a different content-key derivation and AAD usage than the shared sealEvent() helper the other two producers used — making those event types undecryptable by any single consistent receiver. Both now use sealEvent(); documented (but did not fix, as it is not independently exploitable given Ed25519 already covers the same fields) that sealEvent() itself computes aadBytes() but never actually binds it into the AEAD call.

Where: packages/fen_ledger/lib/src/event_reducer.dart, packages/fen_sync/lib/src/sync_manager.dart (EventSink is an interface only), doc/data-model.md §3.9 (reducer invariant 3).

Problem. reduce() trusts a GroupEvent and never verifies its Ed25519 signature; the EventSink that must decrypt+verify+dedup before reducing is declared but not implemented in the repo. So today nothing enforces the security-critical checks (invariants 3, 4, roster membership). Correct layering, but the layer that carries all the security is the missing one.

Direction. Implement the ingest layer (verify signature against roster pubkey → check AAD/epoch → dedup → split-sum → reduce) and make it the only path into the reducer; add tests that a tampered or non-roster-signed event never reaches state.

12. 🟠 replication_factor = 1 contradicts the durability mitigation​

Tracked: #27

Where: infra/managed-backend/garage.toml, doc/data-model.md §3.9 ("publish to ≥2 backends"), doc/security.md §4.8 (invariant E-4: one backend per group).

Problem. §3.9 admits the residual data-loss risk is an event that exists on only one device before a peer copies it, and offers "publish to ≥2 backends immediately" as the mitigation. But a group is bound to exactly one backend (E-4), and that backend is a single-node, replication_factor=1 Garage on a NAS. The named mitigation is unavailable, so the SPOF is real until peers happen to sync.

Direction. Either raise replication / add a second replica target, or drop the "≥2 backends" claim and rely explicitly on multi-device replicas + document the RPO. Add a tested restore procedure (referenced in operations.md but not proven).

13. 🟠 Pubkey filenames leak the cross-group social graph​

Tracked: #28

Where: doc/security.md §4.5, §4.8 (open question #4), doc/backend-architecture.md §1, §5.

Problem. logs/<member_pubkey>.jsonl exposes group membership, group sizes, and activity timing to the operator; pubkeys are stable across groups, so the operator can correlate the same person across groups. Client IP (logged/visible per §4.8 B) further de-pseudonymizes. Currently only an open question.

Direction. Make the decision: derive an opaque per-group filename, e.g. HKDF(group_key, "filename", member_pubkey) → hex, so filenames don't correlate across groups and don't reveal raw identity pubkeys. Cheap; closes a standing leak.

14. 🟠 Provisioner group-creation abuse is cheap​

Tracked: #29

Where: services/fen-provisioner/internal/api/server.go (handleCreateGroup, clientIP), internal/ratelimit, doc/security.md §4.8 (open question), infra/managed-backend/docker-compose.yml.

Problem. POST /v1/groups is Ed25519-signed, but keys are self-minted and free, so per-pubkey rate limiting is defeated by a fresh keypair per request. Per-IP limiting trusts only CF-Connecting-IP; if the origin is reachable directly (compose doesn't firewall Garage/provisioner to the tunnel), the header is absent and RemoteAddr is used — rotatable via proxies. A NAS-hosted managed tier is exposed to storage-exhaustion abuse (each group = a bucket + quota). Attestation is correctly deferred, but there's no interim cost.

Direction. Bind creation to something scarce (proof-of-work, a small deposit, or invite-tree provenance), firewall the origin so only the tunnel can reach it (making CF-Connecting-IP trustworthy), and cap total buckets/global create rate independent of identity.

15. 🟡 Canonical parser doesn't enforce id/hex/UUID shape or NFC​

Tracked: #30

Where: packages/fen_events/lib/src/serialization.dart, doc/canonical-serialization.md §3 (C4, C6), §9.

Problem. The "re-encode and compare" gate catches key-order/whitespace/dup-key/escaping issues (nice), but: (a) _writeString re-emits surrogate code units without validation and NFC (C4) is delegated elsewhere and not enforced here; (b) nothing checks event_id/group_id are lowercase UUIDv4 or author_id is 64-char lowercase hex (C6). Malformed-but-round-trippable ids pass. The §9 hardening list is only partially implemented.

Direction. Add explicit shape checks for ids/hex/UUID and reject unpaired surrogates / non-NFC in the envelope; keep the round-trip gate as a backstop.

16. 🟡 permission_rules.dart is fully stubbed​

Tracked: #31

Where: packages/fen_ledger/lib/src/permission_rules.dart.

Problem. canEdit/canDelete/canRemove all throw UnimplementedError. The edit/delete/remove authorization surface that §4.4 depends on has no implementation yet. Acceptable for a POC, but means no permission enforcement exists in code.

Direction. Implement against §4.4 rules; add unit tests for creator_only vs any_member and organiser-only removal/close.

17. 🟡 Participant.public_key documented as X25519 (it's Ed25519)​

Tracked: #32

Where: doc/data-model.md §3.2, §3.4 (participants.public_key "base64 X25519").

Problem. Comment says "X25519 public key; used to verify event signatures" — signatures are Ed25519. Leftover from the pre-Ed25519 rewrite; misleads implementers (and the wrap needs the Ed25519→X25519 conversion, not a stored X25519 key).

Direction. Correct to Ed25519 (hex, per the rest of the codebase) throughout the data model.

18. 🟡 No at-rest encryption for local data; no compliance surface​

Tracked: #33

Where: doc/security.md §4.1 (key storage), doc/key-management.md §4.7, doc/operations.md.

Problem. Keys live in Keychain/Keystore (good), but the local SQLite event log — all plaintext expense content, names, amounts — is on-device unencrypted. Device loss before remote-wipe exposes everything; §4.7 covers key recovery, not local-data confidentiality. Separately, a managed service holding EU users' IP-linked (pseudonymous) data has GDPR duties (processor role, breach notification) that operations.md doesn't address — "no analytics" ≠ no obligation.

Direction. Consider SQLCipher / biometric app-lock for the local DB; add a short compliance section (data-processor stance, retention, breach process, IP-as-personal-data).

19. 🟡 AASA / assetlinks are placeholders → https trust anchor degrades​

Tracked: #34 · Addressed: The isInviteLink parser now explicitly documents the custom-scheme fallback as an insecure downgrade and establishes real AASA/assetlinks as a launch gate.

Where: infra/well-known/apple-app-site-association, infra/well-known/assetlinks.json, apps/fen/lib/features/onboarding/invite_link_parser.dart (isInviteLink).

Problem. Both files are TODO-REPLACE placeholders, so Universal/App Links won't verify. The parser deliberately excludes plain http (good) and prefers the TLS-verified https form — but until AASA/assetlinks are real, invites fall back to the fen:// custom scheme, which any app can register and hijack (invite-secret interception).

Direction. Treat real AASA/assetlinks as a launch gate; document that the custom-scheme fallback is a downgrade and consider warning on it.

20. Absent topics (assorted)​

Tracked: #35

  • Epoch-wrap forward secrecy (see #7) and cross-protocol key reuse (#6) — both unbacked.
  • FX-rate integrity: the pre-filled settlement suggestion comes from a plain frankfurter.app fetch; a MITM could bias the number. Low severity (user confirms), unmentioned.
  • Backup KEK is underspecified: Method A KEK = HKDF(device_secret, …) but Secure Enclave keys are non-extractable — what is the IKM concretely? Method C says Argon2id with no parameters. Not implementable as written.
  • Payload schema versioning: only the envelope fmt is versioned. An older client silently ignoring a future membership-affecting event could keep trusting a removed member — forward-compat vs security tension, unaddressed.
  • Orphan/close deletion is loud-by-omission: the 6-month orphan sweep and 45-day close purge can destroy a group's canonical copy (financial record) with only client-side warnings; the never-returning organiser is only an open question.
  • KeyRewrapRequest has no rate-limit/dedup rule — a griefer can force repeated organiser re-wraps.

What's genuinely good (keep)​

  • Encrypt-then-sign with canonical AAD binding {group_id, epoch, event_id, author_id, event_type} — correct; prevents cross-epoch/cross-group replay.
  • Provisioner replay cache keyed on decoded pubkey/sig bytes (defeats hex-case malleability) and refusal to trust X-Forwarded-For.
  • Lamport + per-author author_seq as a minimal sequencer substitute — sound and well-argued.
  • Money layer: integer minor units, decimal strings on the wire, deterministic remainder distribution — avoids the usual float disasters.
  • security.md §4.8 already names several trade-offs honestly; the gap is turning them into mechanisms in the spec and code.

Suggested triage order​

  1. Unblock the protocol: #1 (rotation hand-off) and #3 (membership acceptance) — they gate the most.
  2. Close the log-integrity holes: #4 (event_id suppression), #10/#11 (invariant + verify at ingest), #8 (append model).
  3. Availability & abuse: #5 (settlement gate escape), #14 (create abuse), #12 (durability).
  4. Privacy & clarity: #13 (filenames), #9/#15/#16/#17 (spec/impl drift), #2 (threat-model honesty).
  5. Left-out topics: #6/#7 (wrap crypto), #18/#19 and §20.