Security hardening from adversarial audit

Wallet:
- identity.json (NIP-49 ncryptsec) now written 0600 in a 0700 dir so a local
  user can't grind the wallet password offline (+ regression test).
- Wallet password held as a ZeroingString through init_nostr so it's scrubbed
  on drop instead of lingering in a plain String for the session.
- Replaced 4 .unwrap() on re-read tx_meta with graceful guards (archive wipe
  mid-send could otherwise panic the nostr/task thread).
- Tor::http_request/post: bind the client once via let-else and propagate TLS
  builder errors, fixing a TOCTOU unwrap panic on concurrent Tor restart.

goblin-nip05d server (redeployed to goblin.st, verified live):
- One-name-per-pubkey now enforced by a partial UNIQUE index (closes the
  check-then-insert race); INSERT rows-affected==0 returns 409 not a false 201.
- NIP-98 replay protection: one-time auth event-id enforcement within the
  freshness window; tightened forward skew to +5s.
- Rate-limited the unauthenticated GET endpoints; SQLite in WAL mode.
- Verified live: replay rejected, second name for a pubkey blocked.

Audit verdict: fund-safety invariants (never auto-pay Invoice1; S2/I2
finalization bound to counterparty npub) and Tor-from-day-one all hold.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/nostr/client.rs

@@ -591,11 +591,12 @@ async fn handle_wrap(svc: &Arc<NostrService>, wallet: &Wallet, client: &Client,
 					});
 					match svc.send_payment_dm(&sender_hex, &reply_text, None).await {
 						Ok(event_id) => {
-							let mut meta = svc.store.tx_meta(&slate.id.to_string()).unwrap();
-							meta.status = NostrSendStatus::RepliedS2;
-							meta.sent_event_id = Some(event_id);
-							meta.updated_at = unix_time();
-							svc.store.save_tx_meta(&meta);
+							if let Some(mut meta) = svc.store.tx_meta(&slate.id.to_string()) {
+								meta.status = NostrSendStatus::RepliedS2;
+								meta.sent_event_id = Some(event_id);
+								meta.updated_at = unix_time();
+								svc.store.save_tx_meta(&meta);
+							}
 						}
 						Err(e) => warn!("nostr: S2 reply dispatch failed: {e}"),
 					}

src/nostr/identity.rs

@@ -52,6 +52,42 @@ pub struct NostrIdentity {
 /// NIP-49 scrypt work factor (~64 MiB, interactive-grade).
 const NCRYPTSEC_LOG_N: u8 = 16;
 
+/// Write a file with owner-only (0600) permissions on Unix.
+fn write_private(path: &PathBuf, data: &[u8]) -> std::io::Result<()> {
+	#[cfg(unix)]
+	{
+		use std::io::Write;
+		use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
+		let mut f = std::fs::OpenOptions::new()
+			.write(true)
+			.create(true)
+			.truncate(true)
+			.mode(0o600)
+			.open(path)?;
+		f.write_all(data)?;
+		// Also fix the mode if the file already existed with looser perms.
+		let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600));
+		Ok(())
+	}
+	#[cfg(not(unix))]
+	{
+		std::fs::write(path, data)
+	}
+}
+
+/// Restrict a directory to owner-only access on Unix.
+fn restrict_dir(dir: &PathBuf) {
+	#[cfg(unix)]
+	{
+		use std::os::unix::fs::PermissionsExt;
+		let _ = std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700));
+	}
+	#[cfg(not(unix))]
+	{
+		let _ = dir;
+	}
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum IdentityError {
 	#[error("identity io error: {0}")]
@@ -81,11 +117,15 @@ impl NostrIdentity {
 		serde_json::from_str(&raw).ok()
 	}
 
-	/// Persist the identity file.
+	/// Persist the identity file with owner-only permissions (the ncryptsec
+	/// blob must not be world-readable: a local attacker could grind the
+	/// wallet password offline otherwise).
 	pub fn save(&self, nostr_dir: &PathBuf) -> Result<(), IdentityError> {
 		fs::create_dir_all(nostr_dir)?;
+		restrict_dir(nostr_dir);
 		let raw = serde_json::to_string_pretty(self)?;
-		fs::write(Self::path(nostr_dir), raw)?;
+		let path = Self::path(nostr_dir);
+		write_private(&path, raw.as_bytes())?;
 		Ok(())
 	}
 
@@ -214,6 +254,23 @@ mod tests {
 		assert_eq!(unlocked.public_key(), keys.public_key());
 	}
 
+	#[cfg(unix)]
+	#[test]
+	fn identity_file_is_owner_only() {
+		use std::os::unix::fs::PermissionsExt;
+		let dir = std::env::temp_dir().join(format!("goblin-id-test-{}", std::process::id()));
+		let (identity, _) = NostrIdentity::create_derived(NIP06_MNEMONIC, "pw", 0).unwrap();
+		identity.save(&dir).unwrap();
+		let meta = std::fs::metadata(NostrIdentity::path(&dir)).unwrap();
+		// The ncryptsec blob must never be group/world readable.
+		assert_eq!(
+			meta.permissions().mode() & 0o077,
+			0,
+			"identity.json must be 0600"
+		);
+		let _ = std::fs::remove_dir_all(&dir);
+	}
+
 	#[test]
 	fn reencrypt_changes_password() {
 		let (mut identity, keys) = NostrIdentity::create_derived(NIP06_MNEMONIC, "old", 0).unwrap();

src/tor/tor.rs

@@ -311,13 +311,21 @@ impl Tor {
 					return None;
 				}
 			}
-			if Self::client_config().is_none() {
+			// Bind the client once: a concurrent Tor restart/close could clear
+			// the config between an is_none() check and an unwrap (TOCTOU panic).
+			let Some((client_cfg, _)) = Self::client_config() else {
 				error!("Tor: client not launched");
 				return None;
-			}
+			};
 			// Create http tor-powered client to post data.
-			let client = Self::client_config().unwrap().0.isolated_client();
-			let tls_conn = TlsConnector::builder().unwrap().build().unwrap();
+			let client = client_cfg.isolated_client();
+			let tls_conn = match TlsConnector::builder().and_then(|b| b.build()) {
+				Ok(c) => c,
+				Err(e) => {
+					error!("Tor: TLS connector build failed: {:?}", e);
+					return None;
+				}
+			};
 			let conn = ArtiHttpConnector::new(client, tls_conn);
 			let http = hyper_tor::Client::builder().build::<_, hyper_tor::Body>(conn);
 			// Create request.
@@ -366,12 +374,19 @@ impl Tor {
 		body: Option<String>,
 		headers: Vec<(String, String)>,
 	) -> Option<String> {
-		if Self::client_config().is_none() {
+		// Bind once to avoid a TOCTOU panic if Tor restarts mid-request.
+		let Some((client_cfg, _)) = Self::client_config() else {
 			error!("Tor: client not launched");
 			return None;
-		}
-		let client = Self::client_config().unwrap().0.isolated_client();
-		let tls_conn = TlsConnector::builder().unwrap().build().unwrap();
+		};
+		let client = client_cfg.isolated_client();
+		let tls_conn = match TlsConnector::builder().and_then(|b| b.build()) {
+			Ok(c) => c,
+			Err(e) => {
+				error!("Tor: TLS connector build failed: {:?}", e);
+				return None;
+			}
+		};
 		let conn = ArtiHttpConnector::new(client, tls_conn);
 		let http = hyper_tor::Client::builder().build::<_, hyper_tor::Body>(conn);
 		let mut builder = hyper_tor::Request::builder()

src/wallet/wallet.rs

@@ -334,7 +334,9 @@ impl Wallet {
 			return Err(Error::GenericError("Already opened".to_string()));
 		}
 		// Keep the password for nostr identity setup after opening.
-		let nostr_password = password.to_string();
+		// Keep a zeroized copy of the password for nostr identity setup; the
+		// original is moved into open_wallet below.
+		let nostr_password = password.clone();
 
 		// Create new wallet instance if sync thread was stopped or instance was not created.
 		let has_instance = {
@@ -409,7 +411,8 @@ impl Wallet {
 
 	/// Initialize the nostr identity and service for this wallet.
 	/// Failures are logged and disable nostr for the session only.
-	fn init_nostr(&self, password: &str) {
+	/// The password is held as a [`ZeroingString`] so it is scrubbed on drop.
+	fn init_nostr(&self, password: &ZeroingString) {
 		{
 			let r_nostr = self.nostr.read();
 			if r_nostr.is_some() {
@@ -2146,11 +2149,12 @@ async fn handle_task(w: &Wallet, t: WalletTask) {
 							.await
 						{
 							Ok(event_id) => {
-								let mut meta = service.store.tx_meta(&s.id.to_string()).unwrap();
-								meta.status = crate::nostr::NostrSendStatus::AwaitingS2;
-								meta.sent_event_id = Some(event_id);
-								meta.updated_at = crate::nostr::unix_time();
-								service.store.save_tx_meta(&meta);
+								if let Some(mut meta) = service.store.tx_meta(&s.id.to_string()) {
+									meta.status = crate::nostr::NostrSendStatus::AwaitingS2;
+									meta.sent_event_id = Some(event_id);
+									meta.updated_at = crate::nostr::unix_time();
+									service.store.save_tx_meta(&meta);
+								}
 								// Update contact last paid time.
 								if let Some(mut contact) = service.store.contact(receiver) {
 									contact.last_paid_at = Some(crate::nostr::unix_time());
@@ -2249,12 +2253,14 @@ async fn handle_task(w: &Wallet, t: WalletTask) {
 						});
 						match service.send_payment_dm(&request.npub, &text, None).await {
 							Ok(event_id) => {
-								let mut meta =
-									service.store.tx_meta(&reply.id.to_string()).unwrap();
-								meta.status = crate::nostr::NostrSendStatus::PaidAwaitingFinalize;
-								meta.sent_event_id = Some(event_id);
-								meta.updated_at = crate::nostr::unix_time();
-								service.store.save_tx_meta(&meta);
+								if let Some(mut meta) = service.store.tx_meta(&reply.id.to_string())
+								{
+									meta.status =
+										crate::nostr::NostrSendStatus::PaidAwaitingFinalize;
+									meta.sent_event_id = Some(event_id);
+									meta.updated_at = crate::nostr::unix_time();
+									service.store.save_tx_meta(&meta);
+								}
 							}
 							Err(e) => error!("nostr pay reply dispatch failed: {e}"),
 						}
@@ -2286,12 +2292,14 @@ async fn handle_task(w: &Wallet, t: WalletTask) {
 							});
 							match service.send_payment_dm(&request.npub, &text, None).await {
 								Ok(event_id) => {
-									let mut meta =
-										service.store.tx_meta(&reply.id.to_string()).unwrap();
-									meta.status = crate::nostr::NostrSendStatus::RepliedS2;
-									meta.sent_event_id = Some(event_id);
-									meta.updated_at = crate::nostr::unix_time();
-									service.store.save_tx_meta(&meta);
+									if let Some(mut meta) =
+										service.store.tx_meta(&reply.id.to_string())
+									{
+										meta.status = crate::nostr::NostrSendStatus::RepliedS2;
+										meta.sent_event_id = Some(event_id);
+										meta.updated_at = crate::nostr::unix_time();
+										service.store.save_tx_meta(&meta);
+									}
 								}
 								Err(e) => error!("nostr accept reply dispatch failed: {e}"),
 							}

tests/replay_check.rs

@@ -0,0 +1,105 @@
+use base64::Engine;
+use nostr_sdk::prelude::*;
+use sha2::{Digest, Sha256};
+use std::process::Command;
+
+#[tokio::test]
+#[ignore]
+async fn replay_and_double_name_rejected() {
+	let keys = Keys::generate();
+	let pk = keys.public_key().to_hex();
+	let server = "https://goblin.st";
+
+	// Build a register POST for name A.
+	let name_a = format!("r{}", &pk[..7]);
+	let url = format!("{server}/api/v1/register");
+	let body = serde_json::json!({"name": name_a, "pubkey": pk}).to_string();
+	let ph = hex::encode(Sha256::digest(body.as_bytes()));
+	let ev = EventBuilder::new(Kind::HttpAuth, "")
+		.tag(Tag::custom(TagKind::custom("u"), [url.clone()]))
+		.tag(Tag::custom(TagKind::custom("method"), ["POST".to_string()]))
+		.tag(Tag::custom(TagKind::custom("payload"), [ph]))
+		.sign_with_keys(&keys)
+		.unwrap();
+	let auth = format!(
+		"Nostr {}",
+		base64::engine::general_purpose::STANDARD.encode(ev.as_json())
+	);
+	let post = |a: &str, b: &str| {
+		String::from_utf8_lossy(
+			&Command::new("curl")
+				.args([
+					"-s",
+					"-X",
+					"POST",
+					&url,
+					"-H",
+					&format!("Authorization: {a}"),
+					"-H",
+					"Content-Type: application/json",
+					"-d",
+					b,
+				])
+				.output()
+				.unwrap()
+				.stdout,
+		)
+		.to_string()
+	};
+	let r1 = post(&auth, &body);
+	let r2 = post(&auth, &body); // exact replay (same auth event id)
+	println!("first:  {r1}");
+	println!("replay: {r2}");
+	assert!(r1.contains("nip05"), "first register should succeed");
+	assert!(
+		r2.contains("replayed"),
+		"replay should be rejected, got: {r2}"
+	);
+
+	// Second DISTINCT name with a FRESH signature but same pubkey -> one-name rule.
+	let name_b = format!("s{}", &pk[..7]);
+	let body_b = serde_json::json!({"name": name_b, "pubkey": pk}).to_string();
+	let ph_b = hex::encode(Sha256::digest(body_b.as_bytes()));
+	let ev_b = EventBuilder::new(Kind::HttpAuth, "")
+		.tag(Tag::custom(TagKind::custom("u"), [url.clone()]))
+		.tag(Tag::custom(TagKind::custom("method"), ["POST".to_string()]))
+		.tag(Tag::custom(TagKind::custom("payload"), [ph_b]))
+		.sign_with_keys(&keys)
+		.unwrap();
+	let auth_b = format!(
+		"Nostr {}",
+		base64::engine::general_purpose::STANDARD.encode(ev_b.as_json())
+	);
+	let r3 = post(&auth_b, &body_b);
+	println!("2nd name: {r3}");
+	assert!(
+		r3.contains("already has a name") || r3.contains("pubkey already"),
+		"one-name rule should block, got: {r3}"
+	);
+
+	// Cleanup name A.
+	let del_url = format!("{server}/api/v1/register/{name_a}");
+	let ev_d = EventBuilder::new(Kind::HttpAuth, "")
+		.tag(Tag::custom(TagKind::custom("u"), [del_url.clone()]))
+		.tag(Tag::custom(
+			TagKind::custom("method"),
+			["DELETE".to_string()],
+		))
+		.sign_with_keys(&keys)
+		.unwrap();
+	let auth_d = format!(
+		"Nostr {}",
+		base64::engine::general_purpose::STANDARD.encode(ev_d.as_json())
+	);
+	let _ = Command::new("curl")
+		.args([
+			"-s",
+			"-X",
+			"DELETE",
+			&del_url,
+			"-H",
+			&format!("Authorization: {auth_d}"),
+		])
+		.output();
+	println!("✓ replay + one-name-per-pubkey enforced");
+}