From e6e49222ea16c2c60ae746d6795a58e4e2d7b65a Mon Sep 17 00:00:00 2001 From: Avi Date: Thu, 3 Sep 2026 15:52:25 -0500 Subject: [PATCH] feat(signer): add SigningBackend + SigningError + VaultRef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the Step-3 signing-abstraction types: - SigningBackend { Internal, Remote { vault_ref } } — the per-profile choice of where user content is signed. Remote holds ONLY an opaque VaultRef (profile_npub + signer_pubkey); the NIP-46 connection secret is never inlined, so a serialized or leaked backend can never hand a raw secret to a renderer, the audit log, or a crash dump. - VaultRef — opaque, secret-free pointer into the vault's encrypted connection-secret store (resolves to the decrypted secret only at the vault boundary, while unlocked). - SigningError — closed set of signing failures with a stable ErrorKind, user-facing message, and optional technical detail; converts to AppError at the IPC boundary via From, plus a best-effort from_app lift. 10 unit tests, incl. the constraint that serializing a Remote backend never emits a secret. This is the foundation commit; vault integration and IPC rerouting land in follow-ups. --- src/signer/backend.rs | 490 ++++++++++++++++++++++++++++++++++++++++++ src/signer/mod.rs | 3 + 2 files changed, 493 insertions(+) create mode 100644 src/signer/backend.rs diff --git a/src/signer/backend.rs b/src/signer/backend.rs new file mode 100644 index 0000000..86b8b14 --- /dev/null +++ b/src/signer/backend.rs @@ -0,0 +1,490 @@ +//! The per-profile [`SigningBackend`] selection and the [`SigningError`] type. +//! +//! `SigningBackend` is the *choice* of where a profile's user content gets +//! signed: +//! +//! - [`SigningBackend::Internal`] — the key is held locally in the encrypted +//! vault (the embedded signer). +//! - [`SigningBackend::Remote`] — the key is held by a remote NIP-46 signer. +//! This variant holds **only** a [`VaultRef`]: an opaque pointer into the +//! vault's encrypted connection-secret store. The NIP-46 connection secret +//! itself is *never* stored inline here, so a serialized `SigningBackend` +//! (or a leaked one) can never hand a raw connection secret to a renderer, +//! the audit log, or a crash dump. +//! +//! `SigningBackend` is plain data: `Clone`, `PartialEq`, and +//! `Serialize`/`Deserialize` (so it can be persisted per-profile). The heavy +//! lifting — actually signing — is done by the [`crate::signer::Signer`] trait +//! implementations (`EmbeddedSigner`, `Nip46ClientSigner`), selected by the +//! backend. +//! +//! [`SigningError`] is the closed set of ways a signing operation can go +//! wrong. It is the single error type the `Signer` trait, `SigningBackend` +//! helpers, and the IPC reroute layer speak, and it converts to the app-wide +//! [`crate::errors::AppError`] at the IPC boundary via [`From`]. + +use std::fmt; + +use serde::{Deserialize, Serialize}; + +use crate::errors::{AppError, ErrorKind}; + +/// Opaque reference into the vault's encrypted connection-secret store. +/// +/// The reference *names* a stored secret (which profile owns it, which remote +/// signer it points at) but never carries the secret bytes. Resolution — +/// turning a `VaultRef` into the decrypted secret the NIP-46 handshake needs — +/// happens at the vault boundary, only while the vault is unlocked, and the +/// result is `Zeroizing`. +/// +/// Keeping this a distinct newtype means every `VaultRef` in the codebase is +/// unambiguously a pointer, not a secret. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub struct VaultRef { + /// The profile `npub` that owns the connection. + pub profile_npub: String, + /// The remote signer's public key (hex) this reference points at. + pub signer_pubkey: String, +} + +impl VaultRef { + /// Build a reference from a profile `npub` and a remote signer pubkey. + pub fn new(profile_npub: impl Into, signer_pubkey: impl Into) -> Self { + Self { + profile_npub: profile_npub.into(), + signer_pubkey: signer_pubkey.into(), + } + } +} + +impl fmt::Display for VaultRef { + /// A stable, secret-free string form, safe to log or show in the UI. + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "vault://{}#{}", self.profile_npub, self.signer_pubkey) + } +} + +/// Where a profile's user content is signed. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum SigningBackend { + /// The key is held locally in the encrypted vault. + Internal, + /// The key is held by a remote NIP-46 signer. + /// + /// Carries **only** an opaque [`VaultRef`]. The NIP-46 connection secret is + /// stored separately, encrypted, and is resolved on demand — it is never + /// inlined in this variant. + Remote { + /// Opaque pointer into the vault's encrypted connection-secret store. + vault_ref: VaultRef, + }, +} + +impl SigningBackend { + /// `true` when the key is held locally in the vault. + pub fn is_internal(&self) -> bool { + matches!(self, Self::Internal) + } + + /// `true` when the key is held by a remote NIP-46 signer. + pub fn is_remote(&self) -> bool { + matches!(self, Self::Remote { .. }) + } + + /// The opaque vault pointer for a remote backend, if this is one. + /// + /// `None` for [`SigningBackend::Internal`]. This is the *only* place a + /// remote backend exposes its secret location — the secret itself is never + /// a field of this type. + pub fn vault_ref(&self) -> Option<&VaultRef> { + match self { + Self::Remote { vault_ref } => Some(vault_ref), + Self::Internal => None, + } + } +} + +impl fmt::Display for SigningBackend { + /// A stable, secret-free string form. + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Internal => write!(f, "internal (local vault)"), + Self::Remote { vault_ref } => write!(f, "remote ({vault_ref})"), + } + } +} + +/// Canonical error for the signing subsystem. +/// +/// Every signing operation resolves a profile's [`SigningBackend`], enforces +/// identity and permissions, and produces a signed event. `SigningError` is +/// the closed set of ways that can go wrong, each with a stable +/// [`ErrorKind`] for programmatic handling (including IPC) and a user-facing +/// message. +/// +/// It converts to the app-wide [`AppError`] at the IPC boundary via [`From`], +/// so a handler can `?` a signing result and the IPC layer turns it into the +/// JSON error envelope without any string matching. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SigningError { + /// No profile is selected. + NoActiveProfile, + /// The active profile is not stored on this machine. + ProfileNotFound { + /// The `npub` that was looked up. + npub: String, + }, + /// The local (internal) key is not available: the vault is locked, or the + /// stored key is unreadable/invalid. + InternalKeyUnavailable { + /// Technical detail (e.g. "vault locked", "invalid hex"). + detail: String, + }, + /// An external (remote) signer is selected but no matching connection is + /// stored in the vault. + RemoteConnectionMissing { + /// The vault pointer that could not be resolved to a connection. + ref_: VaultRef, + }, + /// The stored connection secret could not be resolved (vault locked or the + /// secret is absent). + SecretResolution { + /// Technical detail. + detail: String, + }, + /// The remote signer's identity does not match the active profile. + IdentityMismatch, + /// The remote signer is not connected (no live relay session). + NotConnected, + /// A NIP-46 permission check denied the requested operation. + PermissionDenied { + /// The NIP-46 method that was denied. + method: String, + }, + /// A NIP-46 connection has expired. + ConnectionExpired, + /// A NIP-46 connection has been revoked. + ConnectionRevoked, + /// The user rejected the signing request. + Rejected, + /// The signing request timed out. + Timeout, + /// The signed event failed to verify or was malformed. + InvalidSignature, + /// A network operation failed. + Network { + /// Technical detail. + detail: String, + }, + /// A filesystem or storage problem. + Storage { + /// Technical detail. + detail: String, + }, + /// An unexpected internal failure. + Internal { + /// Technical detail. + detail: String, + }, +} + +impl SigningError { + /// The stable machine-readable category of this error. + /// + /// Maps onto the app-wide [`ErrorKind`] so the IPC layer and the GUI can + /// make machine-readable decisions without parsing the message. + pub fn kind(&self) -> ErrorKind { + match self { + Self::NoActiveProfile => ErrorKind::NoActiveProfile, + Self::ProfileNotFound { .. } => ErrorKind::ProfileNotFound, + Self::InternalKeyUnavailable { .. } => ErrorKind::VaultLocked, + Self::RemoteConnectionMissing { .. } => ErrorKind::ExternalSignerNotConnected, + Self::SecretResolution { .. } => ErrorKind::VaultLocked, + Self::IdentityMismatch => ErrorKind::ExternalSignerIdentityMismatch, + Self::NotConnected => ErrorKind::ExternalSignerNotConnected, + Self::PermissionDenied { .. } => ErrorKind::Nip46PermissionDenied, + Self::ConnectionExpired => ErrorKind::Nip46ConnectionExpired, + Self::ConnectionRevoked => ErrorKind::Nip46ConnectionRevoked, + Self::Rejected => ErrorKind::SignerRejected, + Self::Timeout => ErrorKind::SignerTimeout, + Self::InvalidSignature => ErrorKind::SignFailed, + Self::Network { .. } => ErrorKind::Network, + Self::Storage { .. } => ErrorKind::VaultMalformed, + Self::Internal { .. } => ErrorKind::Internal, + } + } + + /// The user-facing message, safe to show directly in the GUI. + /// + /// These mirror the existing [`AppError`] copy so the UX is unchanged + /// while the codebase migrates to `SigningError`. + pub fn message(&self) -> &'static str { + match self { + Self::NoActiveProfile => { + "No profile is selected. Choose a profile before publishing." + } + Self::ProfileNotFound { .. } => "That profile is not stored on this computer.", + Self::InternalKeyUnavailable { .. } => { + "Your vault is locked. Enter your password to unlock it." + } + Self::RemoteConnectionMissing { .. } => { + "An external signer is selected but not connected. Connect it, or switch to the local signer." + } + Self::SecretResolution { .. } => { + "The connection secret could not be read. Unlock the vault and try again." + } + Self::IdentityMismatch => { + "The external signer's key does not match this profile. Reconnect with the correct signer." + } + Self::NotConnected => { + "An external signer is selected but not connected. Connect it, or switch to the local signer." + } + Self::PermissionDenied { .. } => { + "This operation is not permitted by the connected signer." + } + Self::ConnectionExpired => "The NIP-46 connection has expired. Reconnect to the signer.", + Self::ConnectionRevoked => { + "The NIP-46 connection has been revoked. Reconnect to the signer." + } + Self::Rejected => "The signing request was rejected by the signer.", + Self::Timeout => { + "The signing request timed out. Check that your signer is running and try again." + } + Self::InvalidSignature => "The note could not be signed.", + Self::Network { .. } => { + "Could not connect to the relay. Check your internet connection and try again." + } + Self::Storage { .. } => { + "Your profile data could not be read. It may have been modified or damaged." + } + Self::Internal { .. } => "Something unexpected went wrong.", + } + } + + /// An optional technical detail, shown only in an expandable area. + /// + /// Never contains secret keys or connection secrets. + pub fn detail(&self) -> Option { + match self { + Self::ProfileNotFound { npub } => Some(format!("No stored profile found for {npub}")), + Self::InternalKeyUnavailable { detail } => Some(detail.clone()), + Self::RemoteConnectionMissing { ref_ } => { + Some(format!("No stored NIP-46 connection for {ref_}")) + } + Self::SecretResolution { detail } => Some(detail.clone()), + Self::PermissionDenied { method } => { + Some(format!("Permission denied for NIP-46 method: {method}")) + } + Self::Network { detail } | Self::Storage { detail } | Self::Internal { detail } => { + Some(detail.clone()) + } + _ => None, + } + } +} + +impl fmt::Display for SigningError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.message()) + } +} + +impl std::error::Error for SigningError {} + +impl From for AppError { + /// Convert a signing error into the app-wide error at the IPC boundary. + /// + /// Uses the simple constructor when there is no technical detail and the + /// details constructor when there is, matching how `AppError` is built + /// elsewhere. + fn from(err: SigningError) -> Self { + match err.detail() { + Some(detail) => AppError::with_details(err.kind(), err.message(), detail), + None => AppError::simple(err.kind(), err.message()), + } + } +} + +impl SigningError { + /// Best-effort lift of an app error into the signing error space. + /// + /// Used where an upstream step (profile lookup, vault I/O) already returns + /// an [`AppError`] and the caller wants to keep speaking `SigningError`. + /// The technical detail is carried through; the category is preserved when + /// it maps cleanly and falls back to [`ErrorKind::Internal`] otherwise. + pub fn from_app(app: &AppError) -> Self { + let detail = app + .details() + .map(str::to_string) + .unwrap_or_else(|| app.message().to_string()); + match app.kind() { + ErrorKind::NoActiveProfile => Self::NoActiveProfile, + ErrorKind::ProfileNotFound => { + // The npub is only present in the detail text; keep it there. + Self::ProfileNotFound { npub: detail } + } + ErrorKind::VaultLocked => Self::InternalKeyUnavailable { + detail: app.message().to_string(), + }, + ErrorKind::ExternalSignerNotConnected => Self::NotConnected, + ErrorKind::ExternalSignerIdentityMismatch => Self::IdentityMismatch, + ErrorKind::Nip46PermissionDenied => Self::PermissionDenied { method: detail }, + ErrorKind::Nip46ConnectionExpired => Self::ConnectionExpired, + ErrorKind::Nip46ConnectionRevoked => Self::ConnectionRevoked, + ErrorKind::SignerRejected => Self::Rejected, + ErrorKind::SignerTimeout => Self::Timeout, + ErrorKind::SignFailed => Self::InvalidSignature, + ErrorKind::Network => Self::Network { detail }, + ErrorKind::VaultMalformed | ErrorKind::Storage => Self::Storage { detail }, + _ => Self::Internal { detail }, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::errors::ErrorKind; + + #[test] + fn internal_backend_has_no_vault_ref() { + let b = SigningBackend::Internal; + assert!(b.is_internal()); + assert!(!b.is_remote()); + assert!(b.vault_ref().is_none()); + assert_eq!(b.to_string(), "internal (local vault)"); + } + + #[test] + fn remote_backend_exposes_only_the_vault_ref() { + let ref_ = VaultRef::new("npub1profile", "deadbeef"); + let b = SigningBackend::Remote { + vault_ref: ref_.clone(), + }; + assert!(b.is_remote()); + assert!(!b.is_internal()); + assert_eq!(b.vault_ref(), Some(&ref_)); + assert_eq!(b.to_string(), "remote (vault://npub1profile#deadbeef)"); + } + + /// The constraint that drives the whole design: serializing a remote + /// backend must never emit a connection secret. Only the ref fields are + /// allowed to appear. + #[test] + fn serialized_remote_backend_never_contains_a_secret() { + let b = SigningBackend::Remote { + vault_ref: VaultRef::new("npub1profile", "deadbeef"), + }; + let json = serde_json::to_string(&b).unwrap(); + assert!(json.contains("npub1profile")); + assert!(json.contains("deadbeef")); + // There is no `secret` field anywhere in the type, so none can appear. + assert!(!json.to_lowercase().contains("secret")); + assert!(!json.contains("nsec")); + } + + #[test] + fn backend_round_trips_through_serde() { + for b in [ + SigningBackend::Internal, + SigningBackend::Remote { + vault_ref: VaultRef::new("npub1profile", "deadbeef"), + }, + ] { + let json = serde_json::to_string(&b).unwrap(); + let back: SigningBackend = serde_json::from_str(&json).unwrap(); + assert_eq!(b, back); + } + } + + #[test] + fn vault_ref_display_is_secret_free() { + let ref_ = VaultRef::new("npub1profile", "deadbeef"); + assert_eq!(ref_.to_string(), "vault://npub1profile#deadbeef"); + assert!(!ref_.to_string().contains("secret")); + } + + #[test] + fn error_kind_mapping() { + assert_eq!( + SigningError::NoActiveProfile.kind(), + ErrorKind::NoActiveProfile + ); + assert_eq!( + SigningError::IdentityMismatch.kind(), + ErrorKind::ExternalSignerIdentityMismatch + ); + assert_eq!( + SigningError::PermissionDenied { + method: "sign_event".into() + } + .kind(), + ErrorKind::Nip46PermissionDenied + ); + assert_eq!(SigningError::Timeout.kind(), ErrorKind::SignerTimeout); + assert_eq!(SigningError::InvalidSignature.kind(), ErrorKind::SignFailed); + assert_eq!( + SigningError::Internal { detail: "x".into() }.kind(), + ErrorKind::Internal + ); + } + + #[test] + fn error_message_is_stable_and_secret_free() { + let err = SigningError::PermissionDenied { + method: "sign_event".into(), + }; + assert!(err.message().contains("not permitted")); + // The dynamic method name lives in the detail, not the user message. + assert_eq!( + err.detail().as_deref(), + Some("Permission denied for NIP-46 method: sign_event") + ); + } + + #[test] + fn signing_error_converts_to_app_error() { + let app: AppError = SigningError::NotConnected.into(); + assert_eq!(app.kind(), ErrorKind::ExternalSignerNotConnected); + assert!(app.message().contains("not connected")); + assert!(app.details().is_none()); + + let with_detail: AppError = SigningError::Internal { + detail: "boom".into(), + } + .into(); + assert_eq!(with_detail.kind(), ErrorKind::Internal); + assert_eq!(with_detail.details(), Some("boom")); + } + + #[test] + fn from_app_preserves_known_kinds() { + let app = AppError::simple(ErrorKind::NoActiveProfile, "no profile"); + assert!(matches!( + SigningError::from_app(&app), + SigningError::NoActiveProfile + )); + + let app = AppError::with_details(ErrorKind::Nip46PermissionDenied, "denied", "sign_event"); + assert!(matches!( + SigningError::from_app(&app), + SigningError::PermissionDenied { method } if method == "sign_event" + )); + + let app = AppError::simple(ErrorKind::Internal, "mystery"); + assert!(matches!( + SigningError::from_app(&app), + SigningError::Internal { .. } + )); + } + + #[test] + fn signing_error_implements_error_trait() { + use std::error::Error; + let err = SigningError::Timeout; + // Display + Error are usable (compile-time + runtime checks). + assert_eq!(err.to_string(), err.message()); + assert!(err.source().is_none()); + } +} diff --git a/src/signer/mod.rs b/src/signer/mod.rs index 397e213..f44df5d 100644 --- a/src/signer/mod.rs +++ b/src/signer/mod.rs @@ -1,10 +1,13 @@ //! The Signer trait - common interface for all signing modes. +pub mod backend; pub mod embedded; pub mod nip46_client; pub mod permissions; pub mod types; +pub use backend::{SigningBackend, SigningError, VaultRef}; + use async_trait::async_trait; use nostr_sdk::prelude::*; use std::sync::Arc;