max / makenotwork

server: close license-activation revoke TOCTOU; stop pending-refund dup re-queue - try_create_activation re-checks revoked_at inside the FOR UPDATE lock, so a charge.refunded that revokes a key between the caller's pre-check and the locked activation can no longer let one activation slip through. Returns Ok(None) when no non-revoked row is locked. - handle_charge_refunded gains requeue_if_unmatched: the pending-refund claim path (row already claimed) no longer inserts a second pending_refunds row (the partial-unique index only covers matched_at IS NULL) — it reports unmatched so the caller releases the claim and the stale sweep escalates it. - Cents Add/Sub saturate (matching Sum) instead of debug-panic / release-wrap. Ultra-fuzz Run #23, Payments axis. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-06-18 22:43 UTC

Commit: f42d2d0b8515cfe353fb442bbf07769bec81bc60

Parent: 4674725

6 files changed, +59 insertions, -8 deletions

M server/src/db/license_keys.rs +10 -3

			@@ -185,11 +185,18 @@ pub async fn try_create_activation(
185	185		) -> Result<Option<DbLicenseActivation>> {
186	186		let mut tx = pool.begin().await?;
187	187
188		-	// Lock the license key row to serialize concurrent activations
189		-	sqlx::query("SELECT 1 FROM license_keys WHERE id = $1 FOR UPDATE")
	188	+	// Lock the license key row to serialize concurrent activations, and re-check
	189	+	// revocation INSIDE the lock (MINOR TOCTOU, Run #23): a charge.refunded that
	190	+	// revokes the key between the caller's pre-check and here must block the
	191	+	// activation. No eligible (non-revoked) row => no activation.
	192	+	let locked = sqlx::query("SELECT 1 FROM license_keys WHERE id = $1 AND revoked_at IS NULL FOR UPDATE")
190	193		.bind(license_key_id)
191		-	.fetch_one(&mut *tx)
	194	+	.fetch_optional(&mut *tx)
192	195		.await?;
	196	+	if locked.is_none() {
	197	+	tx.rollback().await?;
	198	+	return Ok(None);
	199	+	}
193	200
194	201		// Check if this machine already has an activation (re-activation is always OK)
195	202		let existing: Option<DbLicenseActivation> = sqlx::query_as(

M server/src/db/validated_types.rs +16 -2

			@@ -361,12 +361,16 @@ impl sqlx::Decode<'_, sqlx::Postgres> for Cents {
361	361
362	362		impl std::ops::Add for Cents {
363	363		type Output = Self;
364		-	fn add(self, rhs: Self) -> Self { Self(self.0 + rhs.0) }
	364	+	// Saturating, matching the `Sum` impl: money values are bounded well below
	365	+	// i64::MAX (PriceCents caps at $10k), so saturation is unreachable in
	366	+	// practice, but it removes the debug-panic / release-wrap footgun next to
	367	+	// the saturating helpers (Run #23 NOTE).
	368	+	fn add(self, rhs: Self) -> Self { Self(self.0.saturating_add(rhs.0)) }
365	369		}
366	370
367	371		impl std::ops::Sub for Cents {
368	372		type Output = Self;
369		-	fn sub(self, rhs: Self) -> Self { Self(self.0 - rhs.0) }
	373	+	fn sub(self, rhs: Self) -> Self { Self(self.0.saturating_sub(rhs.0)) }
370	374		}
371	375
372	376		impl std::ops::Rem<i64> for Cents {
			@@ -537,6 +541,16 @@ mod tests {
537	541		assert!(result.is_err());
538	542		}
539	543
	544	+	#[test]
	545	+	fn cents_add_sub_saturate_instead_of_panicking() {
	546	+	// Run #23: the operators saturate (like Sum) rather than debug-panic /
	547	+	// release-wrap at the i64 boundary.
	548	+	assert_eq!((Cents::new(5) + Cents::new(3)).as_i64(), 8);
	549	+	assert_eq!((Cents::new(5) - Cents::new(3)).as_i64(), 2);
	550	+	assert_eq!((Cents::new(i64::MAX) + Cents::new(1)).as_i64(), i64::MAX);
	551	+	assert_eq!((Cents::new(i64::MIN) - Cents::new(1)).as_i64(), i64::MIN);
	552	+	}
	553	+
540	554		// ── PriceCents ──
541	555
542	556		#[test]

M server/src/routes/stripe/webhook/billing.rs +16 -1

			@@ -293,9 +293,17 @@ pub(super) async fn handle_invoice_payment_failed(
293	293
294	294		/// Handle charge.refunded webhook; revoke license keys on full refund,
295	295		/// log partial refunds without revoking access.
	296	+	/// Process a full refund: revoke transactions/keys/access, or (for the direct
	297	+	/// `charge.refunded` webhook) queue it as pending when no matching transaction
	298	+	/// or tip exists yet. `requeue_if_unmatched` is false when called from the
	299	+	/// pending-refund claim path — that row is already claimed, so re-queuing would
	300	+	/// insert a duplicate (the partial-unique index only covers `matched_at IS
	301	+	/// NULL`); instead we signal "still unmatched" so the caller releases the claim
	302	+	/// and the stale sweep escalates it (MINOR, Run #23).
296	303		pub(super) async fn handle_charge_refunded(
297	304		state: &AppState,
298	305		refund_data: &crate::payments::ChargeRefundData,
	306	+	requeue_if_unmatched: bool,
299	307		) -> Result<()> {
300	308		let payment_intent_id = &refund_data.payment_intent_id;
301	309		tracing::info!(
			@@ -369,7 +377,7 @@ pub(super) async fn handle_charge_refunded(
369	377		.context("refund tip")?;
370	378		if tip_refunded {
371	379		tracing::info!(payment_intent_id = %payment_intent_id, "tip refund processed");
372		-	} else {
	380	+	} else if requeue_if_unmatched {
373	381		// No matching transaction or tip — the payment webhook likely hasn't
374	382		// arrived yet. Queue the refund for later matching rather than
375	383		// silently dropping it.
			@@ -385,6 +393,13 @@ pub(super) async fn handle_charge_refunded(
385	393		)
386	394		.await
387	395		.context("insert pending refund")?;
	396	+	} else {
	397	+	// Claim path: the pending row is already claimed. Don't insert a
	398	+	// duplicate — report it still-unmatched so the caller releases the
	399	+	// claim and the stale-refund sweep escalates it for manual handling.
	400	+	return Err(crate::error::AppError::Internal(anyhow::anyhow!(
	401	+	"pending refund {payment_intent_id} still has no matching transaction or tip"
	402	+	)));
388	403		}
389	404		}
390	405

M server/src/routes/stripe/webhook/checkout_helpers.rs +3 -1

			@@ -170,7 +170,9 @@ pub(super) async fn check_pending_refund(state: &AppState, payment_intent_id: &s
170	170		amount_refunded: pending.amount_refunded,
171	171		};
172	172
173		-	if let Err(e) = super::billing::handle_charge_refunded(state, &refund_data).await {
	173	+	// requeue_if_unmatched = false: this row is already claimed, so an unmatched
	174	+	// result must release the claim (below), not insert a duplicate pending row.
	175	+	if let Err(e) = super::billing::handle_charge_refunded(state, &refund_data, false).await {
174	176		tracing::error!(
175	177		error = ?e, pending_refund_id = %pending.id,
176	178		"failed to process pending refund after payment completion — releasing claim"

M server/src/routes/stripe/webhook/mod.rs +3 -1

			@@ -146,7 +146,9 @@ pub(crate) async fn process_webhook_event(
146	146		let charge: ChargeView = serde_json::from_value(data_object)
147	147		.map_err(\|e\| AppError::BadRequest(format!("Failed to parse Charge: {e}")))?;
148	148		if let Some(refund_data) = ChargeRefundData::from_view(charge) {
149		-	billing::handle_charge_refunded(state, &refund_data).await?;
	149	+	// Direct webhook: queue as pending if the matching payment hasn't
	150	+	// landed yet.
	151	+	billing::handle_charge_refunded(state, &refund_data, true).await?;
150	152		}
151	153		}
152	154		"customer.subscription.updated" => {

M server/tests/workflows/fingerprinting.rs +11

			@@ -287,6 +287,17 @@ async fn license_verify_revoked_key() {
287	287		let body: Value = resp.json();
288	288		assert_eq!(body["valid"], false);
289	289		assert_eq!(body["error"], "key_revoked");
	290	+
	291	+	// A revoked key must never bind a machine activation (Run #23: the in-lock
	292	+	// revoked_at re-check in try_create_activation backstops the verify fast-path).
	293	+	let activations: i64 = sqlx::query_scalar(
	294	+	"SELECT COUNT(*) FROM license_activations WHERE license_key_id = $1::uuid",
	295	+	)
	296	+	.bind(key_id)
	297	+	.fetch_one(&h.db)
	298	+	.await
	299	+	.unwrap();
	300	+	assert_eq!(activations, 0, "a revoked key must not create an activation");
290	301		}
291	302
292	303		// =============================================================================