Skip to main content

max / makenotwork

Clear Run #20 low/minor audit items - profile: revoke other-device sessions on password change even for a legacy (untracked) session — it has no user_sessions row, so deleting all the user's tracked sessions can't log it out, while the tracked branch keeps using delete_other_sessions per the existing note. - storage: make StorageBackend::upload_multipart required so a backend can't silently fall back to reading the whole file into RAM; add the test backend's impl. - helpers: strip control chars (incl. DEL/C1) in hx_toast before encoding so a toast can't be silently dropped by HeaderValue::from_str. - checkout: mirror the amount_subtotal reconciliation + WAM escalation into the guest-checkout handler. - domains: promote escape_html to a shared helper and escape the interpolated domain/token in the raw-format HTML responses. - wordlist: fix the doc drift (6 words / ~66 bits, not 5 / ~55). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-15 22:33 UTC
Commit: 7652c935290c0c89a8b79a15e375644e00dd2af5
Parent: 32f7f8a
8 files changed, +88 insertions, -29 deletions
@@ -23,6 +23,18 @@ pub use crate::rate_limit::{
23 23 CloudflareIpKeyExtractor, SyncAppKeyExtractor,
24 24 };
25 25
26 + /// Escape HTML special characters, including `'` so the output is safe in
27 + /// single-quoted attribute contexts as well as element text. Use whenever a
28 + /// value is interpolated into a raw `format!`-built HTML string instead of an
29 + /// auto-escaped template.
30 + pub fn escape_html(s: &str) -> String {
31 + s.replace('&', "&amp;")
32 + .replace('<', "&lt;")
33 + .replace('>', "&gt;")
34 + .replace('"', "&quot;")
35 + .replace('\'', "&#39;")
36 + }
37 +
26 38 /// Extract the client IP from request headers.
27 39 ///
28 40 /// Honors `CF-Connecting-IP` only — and that header is trustworthy on every
@@ -179,6 +191,13 @@ pub fn htmx_toast_response(
179 191 }
180 192
181 193 pub fn hx_toast(message: &str, toast_type: &str) -> HeaderValue {
194 + // Strip control characters (C0, DEL/0x7F, C1) before encoding. serde_json
195 + // escapes the < 0x20 controls, but DEL and the C1 range pass through raw and
196 + // make `HeaderValue::from_str` reject the whole header — which silently drops
197 + // the toast. Stripping them keeps the JSON header-safe; the fallback below
198 + // stays as defense-in-depth.
199 + let message: String = message.chars().filter(|c| !c.is_control()).collect();
200 + let toast_type: String = toast_type.chars().filter(|c| !c.is_control()).collect();
182 201 let json = serde_json::json!({
183 202 "showToast": {
184 203 "message": message,
@@ -187,7 +206,7 @@ pub fn hx_toast(message: &str, toast_type: &str) -> HeaderValue {
187 206 })
188 207 .to_string();
189 208 HeaderValue::from_str(&json).unwrap_or_else(|e| {
190 - tracing::warn!(message, error = %e, "hx_toast produced invalid header value");
209 + tracing::warn!(error = %e, "hx_toast produced invalid header value");
191 210 HeaderValue::from_static("")
192 211 })
193 212 }
@@ -38,9 +38,15 @@ pub(super) async fn add_domain(
38 38 db::custom_domains::create_custom_domain(&state.db, session_user.id, &domain, &verification_token)
39 39 .await?;
40 40
41 + // Escape interpolated values even though `validate_domain` restricts the
42 + // domain to `[a-z0-9-]` and the token is server-generated: this is the one
43 + // raw-format HTML path, so it shouldn't rely on upstream validation staying
44 + // strict (audit Run #20 UX INFO).
45 + let domain_esc = crate::helpers::escape_html(&domain);
46 + let token_esc = crate::helpers::escape_html(&verification_token);
41 47 let instructions = format!(
42 48 "Add two DNS records, then click Verify: a CNAME <code>{0}</code> &rarr; <code>connect.makenot.work</code> (set DNS-only / unproxied), and a TXT <code>_mnw-verify.{0}</code> with value <code>{1}</code>.",
43 - domain, verification_token
49 + domain_esc, token_esc
44 50 );
45 51
46 52 Ok(axum::response::Html(format!(
@@ -85,7 +91,7 @@ pub(super) async fn verify_domain(
85 91 if !matched {
86 92 return Ok(axum::response::Html(format!(
87 93 "<p class=\"error\">TXT record not found. Add <code>_mnw-verify.{}</code> TXT <code>{}</code> and try again.</p>",
88 - cd.domain, cd.verification_token
94 + crate::helpers::escape_html(&cd.domain), crate::helpers::escape_html(&cd.verification_token)
89 95 )));
90 96 }
91 97
@@ -191,14 +191,24 @@ pub(in crate::routes::api) async fn update_password(
191 191 .await
192 192 .ok()
193 193 .flatten();
194 - if let Some(current_id) = current_tracking_id {
195 - let revoked_ids = db::sessions::delete_other_sessions(&state.db, current_id, user.id).await?;
196 - for id in &revoked_ids {
197 - state.session_cache.remove(id);
198 - }
199 - if !revoked_ids.is_empty() {
200 - tracing::info!(user_id = %user.id, revoked = revoked_ids.len(), event = "password_change_revoke_sessions", "Revoked other sessions on password change");
201 - }
194 + let revoked_ids = if let Some(current_id) = current_tracking_id {
195 + db::sessions::delete_other_sessions(&state.db, current_id, user.id).await?
196 + } else {
197 + // Legacy current session: no SESSION_TRACKING_KEY, so it has no
198 + // `user_sessions` row and the auth extractor never validates it against
199 + // that table (auth.rs). Deleting ALL of the user's tracked sessions
200 + // therefore revokes every OTHER device's cookie without logging out
201 + // this untracked one — so the note above (don't swap delete_all into
202 + // the *tracked* branch, where it WOULD kill the current row) still
203 + // holds. Other untracked legacy sessions can't be targeted by id and
204 + // are left to expire.
205 + db::sessions::delete_all_sessions_for_user(&state.db, user.id).await?
206 + };
207 + for id in &revoked_ids {
208 + state.session_cache.remove(id);
209 + }
210 + if !revoked_ids.is_empty() {
211 + tracing::info!(user_id = %user.id, revoked = revoked_ids.len(), event = "password_change_revoke_sessions", "Revoked other sessions on password change");
202 212 }
203 213
204 214 // Rotate session ID so old session cookie is invalidated
@@ -70,15 +70,7 @@ pub fn format_size(bytes: &u64) -> String {
70 70 }
71 71 }
72 72
73 - /// Escape HTML special characters, including `'` so the output is safe in
74 - /// single-quoted attribute contexts as well as element text.
75 - fn escape_html(s: &str) -> String {
76 - s.replace('&', "&amp;")
77 - .replace('<', "&lt;")
78 - .replace('>', "&gt;")
79 - .replace('"', "&quot;")
80 - .replace('\'', "&#39;")
81 - }
73 + use crate::helpers::escape_html;
82 74
83 75 /// Get the repos root path from config, or return 404 if git is not configured.
84 76 pub(crate) fn repos_root(state: &AppState) -> Result<std::path::PathBuf> {
@@ -648,6 +648,31 @@ pub(super) async fn handle_guest_checkout_completed(
648 648 "guest transaction completed"
649 649 );
650 650
651 + // Defense-in-depth reconciliation, mirroring the single-item and
652 + // cart paths: our line items are server-built, so Stripe's pre-tax
653 + // subtotal should equal what we credit. A mismatch is logged loudly
654 + // and escalated; the server amount stays authoritative.
655 + if let Some(subtotal) = session.amount_subtotal
656 + && subtotal != i64::from(tx.amount_cents)
657 + {
658 + tracing::error!(
659 + session_id = %session_id, credited_cents = %tx.amount_cents, stripe_subtotal_cents = %subtotal,
660 + "guest checkout amount mismatch: credited transaction amount differs from Stripe session subtotal"
661 + );
662 + if let Some(wam) = state.wam.clone() {
663 + let session_id = session_id.to_string();
664 + let credited = tx.amount_cents;
665 + tokio::spawn(async move {
666 + let body = format!(
667 + "Credited amount {credited} cents != Stripe session subtotal {subtotal} cents \
668 + (guest session {session_id}). The server amount is authoritative; investigate a \
669 + price-edit / Stripe Tax / currency edge."
670 + );
671 + wam.create_ticket("Guest checkout amount mismatch", Some(&body), "high", "stripe-amount-mismatch", Some(&session_id)).await;
672 + });
673 + }
674 + }
675 +
651 676 // Increment sales count inside transaction
652 677 db::items::increment_sales_count(&mut *db_tx, meta.item_id)
653 678 .await
@@ -9,7 +9,7 @@ use std::str::FromStr;
9 9
10 10 use crate::config::StorageConfig;
11 11 use crate::db::{ItemId, ProjectId, UserId};
12 - use crate::error::{AppError, Result, ResultExt};
12 + use crate::error::{AppError, Result};
13 13
14 14 /// Allowed audio file extensions and their MIME types
15 15 const ALLOWED_AUDIO_TYPES: &[(&str, &str)] = &[
@@ -276,13 +276,11 @@ pub trait StorageBackend: Send + Sync {
276 276 tracing::warn!("delete_prefix called on a storage backend that does not implement it");
277 277 Ok(())
278 278 }
279 - /// Upload a file via S3 multipart upload. Default falls back to single upload.
280 - async fn upload_multipart(&self, s3_key: &str, content_type: &str, file_path: &std::path::Path) -> Result<()> {
281 - let data = tokio::fs::read(file_path)
282 - .await
283 - .context("read multipart upload source file")?;
284 - self.upload_object(s3_key, content_type, data, None).await
285 - }
279 + /// Upload a file via S3 multipart upload. Required (not defaulted): a
280 + /// default that `tokio::fs::read`s the whole file into RAM + single PUT
281 + /// silently defeats streaming, so a future backend that forgot to override
282 + /// it would quietly lose multipart. Every backend must declare its strategy.
283 + async fn upload_multipart(&self, s3_key: &str, content_type: &str, file_path: &std::path::Path) -> Result<()>;
286 284 async fn check_connectivity(&self) -> std::result::Result<(), String>;
287 285 fn bucket(&self) -> &str;
288 286 }
@@ -1,7 +1,8 @@
1 1 //! Curated wordlist for license key generation.
2 2 //!
3 3 //! 2048 short common English words (3-6 letters), no offensive or confusable terms.
4 - //! Each key picks 5 random words from this list (~55 bits of entropy).
4 + //! Each key picks 6 random words from this list (~66 bits of entropy) — see
5 + //! the birthday-collision rationale in `crypto.rs`.
5 6
6 7 pub static WORDLIST: [&str; 2048] = [
7 8 // 2048 words total
@@ -80,6 +80,14 @@ impl StorageBackend for InMemoryStorage {
80 80 Ok(())
81 81 }
82 82
83 + async fn upload_multipart(&self, s3_key: &str, _content_type: &str, file_path: &std::path::Path) -> Result<()> {
84 + let data = tokio::fs::read(file_path)
85 + .await
86 + .map_err(|e| AppError::Storage(format!("read multipart source: {e}")))?;
87 + self.objects.lock().unwrap().insert(s3_key.to_string(), data);
88 + Ok(())
89 + }
90 +
83 91 async fn delete_object(&self, _auth: &S3DeleteAuthority, s3_key: &str) -> Result<()> {
84 92 self.objects.lock().unwrap().remove(s3_key);
85 93 Ok(())