commit 240d6b2a8dcee40383d00a755f25b53cf297f335
parent a2a4161f68fd772450558427f06d9516dc0482a1
Author: triesap <tyson@radroots.org>
Date: Sat, 18 Jul 2026 09:14:57 +0000
nostr_signer: close semantic NIP-46 coverage
- Reject auth replay audit replacements with changed connection or method identity.
- Propagate policy-denial audit persistence failures through the NIP-46 handler.
- Construct the static p-tag filter without an impossible runtime parser branch.
- Preserve public result facades while making infallible response construction explicit.
Diffstat:
2 files changed, 139 insertions(+), 17 deletions(-)
diff --git a/crates/nostr_signer/src/manager.rs b/crates/nostr_signer/src/manager.rs
@@ -1451,6 +1451,44 @@ mod tests {
}
#[test]
+ fn auth_replay_audit_replacement_rejects_identity_mismatches() {
+ let audit = |connection_id: &str, method: RadrootsNostrConnectMethod| {
+ RadrootsNostrSignerRequestAuditRecord::new(
+ RadrootsNostrSignerRequestId::parse("req-auth-replay").expect("request id"),
+ RadrootsNostrSignerConnectionId::parse(connection_id).expect("connection id"),
+ method,
+ RadrootsNostrSignerRequestDecision::Allowed,
+ None,
+ 1,
+ )
+ };
+ let mut state = RadrootsNostrSignerStoreState::default();
+ replace_or_insert_auth_replay_audit(
+ &mut state,
+ audit("conn-auth-replay", RadrootsNostrConnectMethod::Ping),
+ )
+ .expect("insert audit");
+ replace_or_insert_auth_replay_audit(
+ &mut state,
+ audit("conn-auth-replay", RadrootsNostrConnectMethod::Ping),
+ )
+ .expect("replace matching audit");
+
+ for replacement in [
+ audit("conn-other", RadrootsNostrConnectMethod::Ping),
+ audit("conn-auth-replay", RadrootsNostrConnectMethod::Logout),
+ ] {
+ let error = replace_or_insert_auth_replay_audit(&mut state, replacement)
+ .expect_err("reject mismatched audit");
+ assert!(
+ error
+ .to_string()
+ .contains("auth replay audit does not match the original request")
+ );
+ }
+ }
+
+ #[test]
fn manager_new_in_memory_and_invalid_schema_paths() {
let manager = RadrootsNostrSignerManager::new_in_memory();
assert!(
diff --git a/crates/nostr_signer/src/nip46.rs b/crates/nostr_signer/src/nip46.rs
@@ -1,9 +1,11 @@
-use nostr::UnsignedEvent;
+use nostr::{
+ UnsignedEvent,
+ filter::{Alphabet, SingleLetterTag},
+};
use radroots_identity::RadrootsIdentityPublic;
use radroots_nostr::prelude::{
RadrootsNostrEvent, RadrootsNostrEventBuilder, RadrootsNostrFilter, RadrootsNostrKind,
RadrootsNostrPublicKey, RadrootsNostrRelayUrl, RadrootsNostrTag, RadrootsNostrTimestamp,
- radroots_nostr_filter_tag,
};
use radroots_nostr_connect::prelude::{
RADROOTS_NOSTR_CONNECT_RPC_KIND, RadrootsNostrConnectError, RadrootsNostrConnectPermissions,
@@ -151,11 +153,10 @@ impl<S: RadrootsNostrSignerNip46Signer> RadrootsNostrSignerNip46Codec<S> {
let filter = RadrootsNostrFilter::new()
.kind(RadrootsNostrKind::Custom(RADROOTS_NOSTR_CONNECT_RPC_KIND))
.since(RadrootsNostrTimestamp::now());
- Ok(radroots_nostr_filter_tag(
- filter,
- "p",
+ Ok(filter.custom_tags(
+ SingleLetterTag::lowercase(Alphabet::P),
vec![self.signer.signer_public_key_hex()],
- )?)
+ ))
}
pub fn parse_request_event(
@@ -187,20 +188,27 @@ impl<S: RadrootsNostrSignerNip46Signer> RadrootsNostrSignerNip46Codec<S> {
&self,
unsigned_event: UnsignedEvent,
) -> Result<RadrootsNostrConnectResponse, RadrootsNostrSignerError> {
+ Ok(self.sign_event_response_value(unsigned_event))
+ }
+
+ fn sign_event_response_value(
+ &self,
+ unsigned_event: UnsignedEvent,
+ ) -> RadrootsNostrConnectResponse {
let user_public_key = self.signer.user_identity().public_key_hex;
if unsigned_event.pubkey.to_hex() != user_public_key {
- return Ok(RadrootsNostrConnectResponse::Error {
+ return RadrootsNostrConnectResponse::Error {
result: None,
error: "sign_event pubkey does not match the managed user identity".to_owned(),
- });
+ };
}
match self.signer.sign_user_event(unsigned_event) {
- Ok(event) => Ok(RadrootsNostrConnectResponse::SignedEvent(event)),
- Err(error) => Ok(RadrootsNostrConnectResponse::Error {
+ Ok(event) => RadrootsNostrConnectResponse::SignedEvent(event),
+ Err(error) => RadrootsNostrConnectResponse::Error {
result: None,
error: format!("failed to sign event: {error}"),
- }),
+ },
}
}
@@ -208,7 +216,14 @@ impl<S: RadrootsNostrSignerNip46Signer> RadrootsNostrSignerNip46Codec<S> {
&self,
request: RadrootsNostrConnectRequest,
) -> Result<RadrootsNostrConnectResponse, RadrootsNostrSignerError> {
- Ok(match request {
+ Ok(self.crypto_response_value(request))
+ }
+
+ fn crypto_response_value(
+ &self,
+ request: RadrootsNostrConnectRequest,
+ ) -> RadrootsNostrConnectResponse {
+ match request {
RadrootsNostrConnectRequest::Nip04Encrypt {
public_key,
plaintext,
@@ -253,7 +268,7 @@ impl<S: RadrootsNostrSignerNip46Signer> RadrootsNostrSignerNip46Codec<S> {
result: None,
error: format!("request `{}` is not a crypto method", other.method()),
},
- })
+ }
}
}
@@ -511,7 +526,7 @@ where
self.handled_request_for_authorized_action(
&evaluation.connection,
evaluation.action,
- || self.codec.sign_event_response(unsigned_event),
+ || Ok(self.codec.sign_event_response_value(unsigned_event)),
)?,
Some(evaluation.audit),
))
@@ -551,7 +566,7 @@ where
self.handled_request_for_authorized_action(
&evaluation.connection,
evaluation.action,
- || self.codec.crypto_response(request),
+ || Ok(self.codec.crypto_response_value(request)),
)?,
Some(evaluation.audit),
))
@@ -569,7 +584,7 @@ where
.handled_request_for_authorized_action(
&evaluation.connection,
evaluation.action,
- || self.codec.sign_event_response(unsigned_event),
+ || Ok(self.codec.sign_event_response_value(unsigned_event)),
),
RadrootsNostrConnectRequest::Nip04Encrypt { .. }
| RadrootsNostrConnectRequest::Nip04Decrypt { .. }
@@ -578,7 +593,7 @@ where
.handled_request_for_authorized_action(
&evaluation.connection,
evaluation.action,
- || self.codec.crypto_response(request_message.request),
+ || Ok(self.codec.crypto_response_value(request_message.request)),
),
RadrootsNostrConnectRequest::GetPublicKey
| RadrootsNostrConnectRequest::GetSessionCapability
@@ -818,11 +833,14 @@ mod tests {
use crate::evaluation::{
RadrootsNostrSignerRequestAction, RadrootsNostrSignerRequestResponseHint,
};
+ use crate::manager::RadrootsNostrSignerManager;
use crate::model::{
RadrootsNostrSignerApprovalRequirement, RadrootsNostrSignerAuthChallenge,
RadrootsNostrSignerAuthState, RadrootsNostrSignerConnectionDraft,
RadrootsNostrSignerConnectionRecord, RadrootsNostrSignerPendingRequest,
+ RadrootsNostrSignerStoreState,
};
+ use crate::store::RadrootsNostrSignerStore;
use crate::test_support::{fixture_alice_identity, fixture_carol_public_key, primary_relay};
use nostr::{Keys, Timestamp, UnsignedEvent};
use radroots_identity::{RadrootsIdentity, RadrootsIdentityPublic};
@@ -836,6 +854,10 @@ mod tests {
RadrootsNostrConnectRemoteSessionCapability, RadrootsNostrConnectRequest,
RadrootsNostrConnectRequestMessage, RadrootsNostrConnectResponse,
};
+ use std::sync::{
+ Arc, RwLock,
+ atomic::{AtomicBool, Ordering},
+ };
#[derive(Clone)]
struct TestSigner {
@@ -853,6 +875,36 @@ mod tests {
prepare_denial: Option<&'static str>,
}
+ #[derive(Clone, Default)]
+ struct ToggleSaveStore {
+ state: Arc<RwLock<RadrootsNostrSignerStoreState>>,
+ fail_saves: Arc<AtomicBool>,
+ }
+
+ impl RadrootsNostrSignerStore for ToggleSaveStore {
+ fn load(&self) -> Result<RadrootsNostrSignerStoreState, RadrootsNostrSignerError> {
+ self.state
+ .read()
+ .map(|state| state.clone())
+ .map_err(|_| RadrootsNostrSignerError::Store("test store lock poisoned".into()))
+ }
+
+ fn save(
+ &self,
+ state: &RadrootsNostrSignerStoreState,
+ ) -> Result<(), RadrootsNostrSignerError> {
+ if self.fail_saves.load(Ordering::SeqCst) {
+ return Err(RadrootsNostrSignerError::Store(
+ "test store save failure".into(),
+ ));
+ }
+ self.state
+ .write()
+ .map(|mut stored| *stored = state.clone())
+ .map_err(|_| RadrootsNostrSignerError::Store("test store lock poisoned".into()))
+ }
+ }
+
impl Default for TestPolicy {
fn default() -> Self {
Self {
@@ -1604,6 +1656,38 @@ mod tests {
}
#[test]
+ fn policy_denial_propagates_audit_persistence_failures() {
+ let store = ToggleSaveStore::default();
+ let manager = RadrootsNostrSignerManager::new(Arc::new(store.clone()))
+ .expect("manager with toggle store");
+ let backend =
+ RadrootsNostrEmbeddedSignerBackend::new(manager, test_signer().signer_identity.clone())
+ .expect("embedded backend");
+ let client_public_key = fixture_carol_public_key();
+ connect_with_permissions(
+ &handler_with_backend(backend.clone()),
+ client_public_key,
+ Vec::new(),
+ );
+ store.fail_saves.store(true, Ordering::SeqCst);
+
+ let handler = handler_with_policy(
+ backend,
+ TestPolicy {
+ prepare_denial: Some("policy blocked"),
+ ..TestPolicy::default()
+ },
+ );
+ let error = handler
+ .handle_request(
+ client_public_key,
+ request_message("req-audit-save", RadrootsNostrConnectRequest::Ping),
+ )
+ .expect_err("audit persistence failure");
+ assert!(error.to_string().contains("test store save failure"));
+ }
+
+ #[test]
fn handler_rejects_unauthorized_base_sign_and_crypto_requests() {
let handler = handler_with_backend(embedded_backend());
let client_public_key = fixture_carol_public_key();
