Skip to content

Commit 4fc3b2a

Browse files
committed
fix: security hardening phase 1
- Enforce ENCRYPTION_KEY on mainnet startup (hard-fail if missing) - Extend invoice expiry by 30 min on payment detection (prevent stuck detected invoices) - Redact memo and amount from info-level logs (moved to debug) - Webhook retry: fail closed on secret decrypt error instead of fallback to raw ciphertext - Remove premature try_detect_fee from mempool path (fix fee ledger consistency) - Normalize confirmed webhook payload across all code paths (always include price_zec, received_zec, overpaid) - Admin key comparison: use subtle::ConstantTimeEq for HMAC tag comparison - Session validation: remove expected memo from error response - Update roadmap: add payment link server-side resolution, refine rate limiting scope
1 parent c623094 commit 4fc3b2a

10 files changed

Lines changed: 46 additions & 13 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ hmac = "0.12"
3737
hex = "0.4"
3838
rand = "0.8"
3939
base64 = "0.22"
40+
subtle = "2"
4041

4142
# QR code generation
4243
qrcode = "0.14"

ROADMAP.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ Privacy-preserving Zcash payment gateway. Non-custodial, shielded-only.
3232
- [x] `ALLOWED_ORIGINS` config for production deployment
3333
- [x] Concurrent batch raw tx fetching (futures::join_all, batches of 20)
3434
- [x] CipherScan raw tx endpoint (`GET /api/tx/{txid}/raw`)
35-
- [ ] Rate limiting on public endpoints (actix-web-middleware or tower)
36-
- [ ] Invoice lookup auth (merchant can only see own invoices)
37-
- [ ] Merchant registration guard (admin key or invite-only in production)
35+
- [ ] **Payment link server-side resolution** — move invoice creation from public API endpoint (`POST /api/payment-links/{slug}/checkout`) into the Next.js server component. Invoice creation happens server-to-server (authenticated with internal key), removing the unauthenticated public endpoint entirely. Follows Stripe's model: buyers load a web page, not an API. Page-level protection via Vercel/Cloudflare.
36+
- [ ] Rate limiting on public endpoints — per-API-key for authenticated routes, per-IP for remaining unauthenticated routes (registration). `actix-governor` keyed limiters, `429` with `Retry-After`.
37+
- [ ] Invoice lookup auth (merchant can only see own invoices via API; checkout page unaffected)
3838
- [ ] Input validation hardening (UFVK format check, address validation)
3939
- [x] **Switch from UFVK to UIVK storage** — accept UFVK or UIVK at registration, derive and store only the UIVK (discard FVK). Existing merchants migrated on startup. Reduces data exposure per principle of least privilege.
4040
- [ ] **Account deletion cooldown** — schedule deletion for 48h instead of immediate hard-delete. Protects against compromised sessions. Merchant can cancel within the window. After 48h, purge all data (viewing keys, invoices, products, sessions).

src/api/admin.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use actix_web::web;
22
use hmac::{Hmac, Mac};
33
use sha2::Sha256;
44
use sqlx::SqlitePool;
5+
use subtle::ConstantTimeEq;
56

67
type HmacSha256 = Hmac<Sha256>;
78

@@ -33,7 +34,7 @@ pub fn authenticate_admin(req: &actix_web::HttpRequest) -> bool {
3334
mac2.update(provided.as_bytes());
3435
let provided_tag = mac2.finalize().into_bytes();
3536

36-
expected_tag == provided_tag
37+
expected_tag.ct_eq(&provided_tag).into()
3738
}
3839

3940
fn unauthorized() -> actix_web::HttpResponse {

src/api/sessions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ pub async fn open(
125125

126126
if session_outputs.is_empty() {
127127
return HttpResponse::BadRequest().json(serde_json::json!({
128-
"error": format!("No output with session memo found. Expected memo: {}", expected_memo),
128+
"error": "No output with matching session memo found in transaction",
129129
}));
130130
}
131131
session_outputs.iter().map(|o| o.amount_zatoshis as i64).sum()

src/config.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,13 @@ impl Config {
9999
pub fn fee_enabled(&self) -> bool {
100100
self.fee_address.is_some() && self.fee_ufvk.is_some() && self.fee_rate > 0.0
101101
}
102+
103+
pub fn validate(&self) -> anyhow::Result<()> {
104+
if !self.is_testnet() && self.encryption_key.is_empty() {
105+
anyhow::bail!(
106+
"ENCRYPTION_KEY is required in production (mainnet). Set a 64-char hex key."
107+
);
108+
}
109+
Ok(())
110+
}
102111
}

src/invoices/mod.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,16 @@ pub async fn create_invoice(
249249

250250
tracing::info!(
251251
invoice_id = %id,
252-
memo = %memo_code,
253252
currency = %currency,
254-
amount = %amount,
255253
diversifier_index = div_index,
256254
"Invoice created with unique address"
257255
);
256+
tracing::debug!(
257+
invoice_id = %id,
258+
memo = %memo_code,
259+
amount = %amount,
260+
"Invoice details"
261+
);
258262

259263
Ok(CreateInvoiceResponse {
260264
invoice_id: id,
@@ -356,13 +360,17 @@ pub async fn get_pending_invoices(pool: &SqlitePool) -> anyhow::Result<Vec<Invoi
356360
/// Returns true if the status actually changed (used to gate webhook dispatch).
357361
pub async fn mark_detected(pool: &SqlitePool, invoice_id: &str, txid: &str, received_zatoshis: i64) -> anyhow::Result<bool> {
358362
let now = Utc::now().format("%Y-%m-%dT%H:%M:%SZ").to_string();
363+
let new_expires = (Utc::now() + Duration::minutes(30))
364+
.format("%Y-%m-%dT%H:%M:%SZ")
365+
.to_string();
359366
let result = sqlx::query(
360-
"UPDATE invoices SET status = 'detected', detected_txid = ?, detected_at = ?, received_zatoshis = ?
367+
"UPDATE invoices SET status = 'detected', detected_txid = ?, detected_at = ?, received_zatoshis = ?, expires_at = ?
361368
WHERE id = ? AND status IN ('pending', 'underpaid')"
362369
)
363370
.bind(txid)
364371
.bind(&now)
365372
.bind(received_zatoshis)
373+
.bind(&new_expires)
366374
.bind(invoice_id)
367375
.execute(pool)
368376
.await?;

src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ async fn main() -> anyhow::Result<()> {
3535
.init();
3636

3737
let config = config::Config::from_env()?;
38+
config.validate()?;
3839
let pool = db::create_pool(&config.database_url).await?;
3940
db::migrate_encrypt_ufvks(&pool, &config.encryption_key).await?;
4041
db::migrate_ufvk_to_uivk(&pool, &config.encryption_key).await?;

src/scanner/mod.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,8 @@ async fn scan_mempool(
238238
Ok(outputs) => {
239239
for output in &outputs {
240240
let recipient_hex = hex::encode(output.recipient_raw);
241-
tracing::info!(txid, memo = %output.memo, amount = output.amount_zec, "Decrypted mempool tx");
241+
tracing::info!(txid, "Decrypted mempool output");
242+
tracing::debug!(txid, memo = %output.memo, amount = output.amount_zec, "Decrypted output details");
242243

243244
if let Some(invoice) = matching::find_matching_invoice(&pending, &recipient_hex, &output.memo) {
244245
let entry = invoice_totals.entry(invoice.id.clone())
@@ -275,7 +276,6 @@ async fn scan_mempool(
275276
let overpaid = new_received > invoice.price_zatoshis + 1000;
276277
spawn_payment_webhook(pool, http, invoice_id, "detected", txid,
277278
invoice.price_zatoshis, new_received, overpaid, &config.encryption_key);
278-
try_detect_fee(pool, config, raw_hex, invoice_id).await;
279279
}
280280
} else if invoice.status == "pending" {
281281
invoices::mark_underpaid(pool, invoice_id, new_received, txid).await?;
@@ -311,7 +311,10 @@ async fn scan_blocks(
311311
Ok(true) => {
312312
let changed = invoices::mark_confirmed(pool, &invoice.id).await?;
313313
if changed {
314-
spawn_webhook(pool, http, &invoice.id, "confirmed", txid, &config.encryption_key);
314+
let overpaid = invoice.received_zatoshis > invoice.price_zatoshis + 1000;
315+
spawn_payment_webhook(pool, http, &invoice.id, "confirmed", txid,
316+
invoice.price_zatoshis, invoice.received_zatoshis, overpaid,
317+
&config.encryption_key);
315318
on_invoice_confirmed(pool, http, config, invoice).await;
316319
}
317320
}

src/webhooks/mod.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,17 @@ pub async fn retry_failed(pool: &SqlitePool, http: &reqwest::Client, encryption_
365365
.await?;
366366

367367
for (id, url, payload, raw_secret, attempts) in rows {
368-
let secret = crate::crypto::decrypt_webhook_secret(&raw_secret, encryption_key)
369-
.unwrap_or(raw_secret);
368+
let secret = match crate::crypto::decrypt_webhook_secret(&raw_secret, encryption_key) {
369+
Ok(s) => s,
370+
Err(e) => {
371+
tracing::error!(delivery_id = %id, error = %e, "Failed to decrypt webhook secret, marking delivery failed");
372+
sqlx::query("UPDATE webhook_deliveries SET status = 'failed', response_error = 'Webhook secret decryption failed' WHERE id = ?")
373+
.bind(&id)
374+
.execute(pool)
375+
.await?;
376+
continue;
377+
}
378+
};
370379
if let Err(reason) = crate::validation::resolve_and_check_host(&url) {
371380
tracing::warn!(delivery_id = %id, %url, %reason, "Webhook retry blocked: SSRF protection");
372381
sqlx::query("UPDATE webhook_deliveries SET status = 'failed' WHERE id = ?")

0 commit comments

Comments
 (0)