Skip to content

fix(transport): pin Nostr verification-cache security fixes - #2912

Merged
erskingardner merged 1 commit into
masterfrom
codex/nostr-verification-cache-security
Nov 3, 2026
Merged

erskingardner merged 1 commit into
masterfrom
codex/nostr-verification-cache-security

Conversation

@erskingardner

@erskingardner erskingardner commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Problem

MDK's pinned SDK accepts an event on a verification-cache hit without authenticating its current bytes or signature. A relay can reuse a verified ID with altered directory metadata or an invalid signature, affecting the live-directory boundary tracked in #2521.

Fix

Pin a fork revision that backports both upstream verification-cache fixes onto MDK's exact previous revision. Every event's content must match its ID, and skipping signature verification requires the exact previously verified signature; different valid signatures remain accepted. All dependency and workspace versions stay unchanged.

Verification

  • Fork: shared-cache regressions (5), relay verification regressions (4), acquisition tests (29), and just precommit passed.
  • Local: cargo fmt --all.
  • Local: just fast-ci ran once; its Apple mock-build test fails with an inherited CARGO_TARGET_DIR, reproduced on merge-base c4b60b2de. The other static gates passed.
  • Local: env -u CARGO_TARGET_DIR python3 crates/marmot-uniffi/test-release-profile.py ReleaseProfileTests.test_apple_builders_keep_symbols_and_builder_owned_profile passed (mock Cargo only).
  • Local: just clippy test-convergence-policy-pin completed the remaining fast-ci gates, including both workspace lint passes.
  • Local: cargo test -p transport-nostr-adapter --locked — 254 passed.
  • Local: cargo test -p transport-nostr-adapter --features sdk --locked — 467 passed.
  • Local: cargo nextest run -p transport-nostr --all-features --locked — 143 passed.
  • Local: 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.
  • Local: 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 46e214d6b, covering the MDK diff and the complete fork backport.

Fork pin: a8efab6d23846d044746def3dc2766fd1a7e227c, on codex/mdk-verification-cache-security, based directly on 48801bf6. Ports upstream 0e08c306 and merged a991ff00 (upstream #1508), retaining the fork's SharedState structure.

Related to #2521. This PR leaves that issue open: MDK's independent live-directory identity check is still wanted.

Summary by CodeRabbit

  • Bug Fixes
    • Events with altered content or invalid signatures are now rejected, even if their IDs were previously verified and cached. This ensures cached verification results do not allow invalid event data to pass validation. The update strengthens consistency in how event authenticity is checked across repeated encounters with the same event ID.

Reject tampered content and invalid signatures on SDK verification-cache hits by adopting the narrowly backported upstream fixes. Preserve all dependency and workspace versions. Related to mdk#2521; independent live-directory identity validation remains pending.
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The workspace and adapter now pin a newer Nostr fork revision. The changelog describes its cached-event verification update and notes that the live-directory identity check remains pending.

Changes

Nostr SDK verification-cache update

Layer / File(s) Summary
Update Nostr dependency pins
Cargo.toml, crates/transport-nostr-adapter/Cargo.toml, CHANGELOG.md
The workspace and adapter dependency pins use the same new fork revision. The changelog describes the cached-event verification update and notes that the live-directory identity check remains pending.

Estimated code review effort: 2 (Simple) | ~8 minutes

Possibly related issues

Possibly related PRs

  • marmot-protocol/mdk#2547: Its inbound processing depends on authenticated SDK event IDs, which relates to the verification behavior in this PR.
Merge Risk: Minimal · up to 46e21

The pinned verification-cache fixes are ready to merge; the separate live-directory check remains explicitly tracked.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the transport fix and the pinned Nostr verification-cache security fixes.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.)

Finishing Touches
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • 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.
Recent review info
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: 14935eba-7755-4308-9b4a-9506bdd0748c
Commits

Reviewing files that changed from the base of the PR and between 2b5f4e5 and 46e214d.

Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing: 3
  • CHANGELOG.md
  • Cargo.toml
  • crates/transport-nostr-adapter/Cargo.toml

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

@erskingardner
erskingardner added this pull request to the merge queue Nov 3, 2026
Merged via the queue into master with commit d62840e Nov 3, 2026
46 checks passed
@erskingardner
erskingardner deleted the codex/nostr-verification-cache-security branch November 3, 2026 10:05
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.

1 participant