Skip to content

fix(directory): verify events before forwarding and persistence - #2918

Merged
erskingardner merged 2 commits into
masterfrom
codex/2521-directory-verification
Nov 3, 2026
Merged

erskingardner merged 2 commits into
masterfrom
codex/2521-directory-verification

Conversation

@erskingardner

Copy link
Copy Markdown
Member

Problem

MDK's live directory forwarder relied on SDK authentication, and raw directory ingestion accepted unsigned records. The SDK cache fix in #2912 repaired that dependency, but MDK still needed its own protection against forged profiles, follow lists, and relay lists arriving through another producer or a future regression.

Fix

Authenticate each event's content, ID, and signature before live forwarding, and independently authenticate raw records at directory ingestion before either store changes. Invalid records are ignored; valid updates retain existing projection and freshness behavior. Existing ingestion fixtures now carry real signatures while retaining their assertions.

Verification

  • Owner-level regressions failed before the guards and passed afterward: the forwarder previously emitted forged copies, and unsigned ingestion previously created a stored profile.
  • cargo fmt --all
  • cargo nextest run -p transport-nostr --all-features --locked — 145 passed.
  • cargo nextest run -p nostr-directory --locked — 80 passed.
  • cargo nextest run -p marmot-app --lib --features test-policy-overrides,otlp-export --locked -E 'test(/directory/) | test(/^(tests::relay_plane_runtime|relay_telemetry_export)::/) | test(/ingesting_|stale_live_relay_list_|zero_timestamp_relay_list_|same_second_multiline_|onboarding/)' — 195 passed.
  • cargo clippy -p transport-nostr -p nostr-directory -p marmot-app --all-targets --all-features --locked -- -D warnings
  • After tightening two tampering fixtures to retain the original valid signatures, both affected cases and scoped lint passed again on the final head.
  • just binding-docs-gate
  • cargo nextest run -p cgka-conformance-simulator --locked --test app_runtime_journeys --test process_orchestrator -E 'test(=public_app_03_late_join_and_messaging) | test(=engine_app_runtime_and_process_adapters_reach_equivalent_public_state)' — 2 passed.
  • cargo nextest run -p wn-cli --locked --test cli --features test-policy-overrides -E 'test(=local_group_message_workflow_runs_through_the_dm_contract) | test(=daemon_start_status_execute_and_stop_are_user_facing_commands)' — 2 passed.
  • Review: Claude Opus 5.5 and GLM 5.3 via Numes independently recommend merge at 8772b465c; read-only checkouts stayed clean.

The wire tests cover retained verification state after the default SDK seen tracker is cleared, and actual relay → SDK → directory forwarder → sync worker → app persistence, including exact unchanged cache/shared rows and reopening. The owner regressions isolate MDK's guards; the wire tests also traverse the already-fixed SDK. Four tests are added to existing binaries, with no new production seam. No API signatures, dependency versions, workspace versions, or schemas change.

Fixes #2521.

Test audit: wall times for new tests and signed-fixture updates
Test Seconds
directory_forwarder_verifies_each_notification_independently 0.011
directory_wire_rejects_forgery_when_verified_ids_outlive_seen_state 0.022
directory_ingress_authenticates_before_writing_either_store 0.067
live_directory_forgery_cannot_change_persisted_metadata 0.165
ingesting_all_read_nip65_list_replaces_cached_write_targets 0.323
stale_live_relay_list_events_do_not_replace_newer_cached_lists 0.270
zero_timestamp_relay_list_event_populates_legacy_empty_state 0.172
same_second_multiline_bio_does_not_replace_cached_single_line_bio 0.113
directory_entry_reconciles_nostr_components_independently 0.270
ingesting_remote_contact_list_does_not_promote_follows_and_caps_stored_follows 0.886
ingesting_kind0_profile_persists_only_bounded_unknown_fields 0.279
ingesting_kind0_profile_preserves_multiline_about_and_single_line_fields 0.285

The eight fixture updates preserve their existing assertions. The two direct guard tests provide red/green evidence; the two wire tests cover transport and persistence risks.

Independently verify live directory notifications and raw directory ingestion before either storage tier changes. Cover forged producer notifications, unsigned and tampered DTOs, retained SDK verification state after seen-state loss, and real relay-to-app persistence with reopening. Preserve existing projection and recency assertions using signed fixtures. Fixes #2521.
…ases

Keep the original ID and its valid signature while altering signed fields, and assert signature-only verification succeeds while ID verification fails. This isolates content binding from bad-signature rejection in the raw ingress and app persistence tests.
@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Next included review available in 45 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 96 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 481170ed-1133-428a-bd46-b407b139e7dc
Commits

Reviewing files that changed from the base of the PR and between 085adde and 8772b46.

Files selected for processing: 7
  • CHANGELOG.md
  • crates/marmot-app/src/directory_tests/sync.rs
  • crates/marmot-app/src/tests.rs
  • crates/nostr-directory/src/store.rs
  • crates/nostr-directory/src/store/tests.rs
  • crates/transport-nostr/src/lib.rs
  • crates/transport-nostr/src/tests.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@erskingardner
erskingardner added this pull request to the merge queue Nov 3, 2026
Merged via the queue into master with commit a3850da Nov 3, 2026
47 checks passed
@erskingardner
erskingardner deleted the codex/2521-directory-verification branch November 3, 2026 12:27
@erskingardner erskingardner mentioned this pull request Nov 3, 2026
28 tasks done
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.

Verify live directory event identity independently of SDK ID-cache hits

1 participant