max / makenotwork

storage+sanitizer: dedup confirm HEAD calls; seal custom-pages URL-attribute allowlist (ultra-fuzz NOTE tail) - Confirm dual-HEAD (Run 2 Storage NOTE): the version/image confirm paths called object_exists() then object_size() -- two S3 HEADs. object_size returns None when the object is absent, so it already is the existence check; dropped the redundant object_exists round-trip at all three sites. - Custom-pages sanitizer (Run 2 UX NOTE): the sanitizer's safety rests on the hand-maintained url_attribute() covering every URL-bearing attribute the ammonia allowlist permits (we let candidate schemes past so our filter is the sole authority). Added a build-time seal that fails if ALLOWED_TAGS x (GENERIC_ATTRS u per-tag) ever permits a known URL-bearing attribute url_attribute doesn't recognise, plus a behavioural test that javascript: URLs are stripped from the covered attributes. storage (31) + versions (10) + gallery (9) + html_sanitizer (20) green; clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-06-23 21:03 UTC

Commit: 83a4e6810f207878286d7a8385d20e5a226b76a1

Parent: 6ef2042

3 files changed, +67 insertions, -27 deletions

M server/src/custom_pages/html_sanitizer.rs +57

			@@ -191,6 +191,63 @@ mod tests {
191	191		assert!(!out.contains("steal"));
192	192		}
193	193
	194	+	/// Seal for the hand-maintained `url_attribute` allowlist. The sanitizer's
	195	+	/// safety rests on the documented invariant that the attribute filter covers
	196	+	/// every URL-bearing attribute on every allowed tag — an uncovered one
	197	+	/// would let a `javascript:` URL through ammonia (which we deliberately let
	198	+	/// candidate schemes past, so our filter is the sole authority). This fails
	199	+	/// the build if the allowlist (`ALLOWED_TAGS` × `GENERIC_ATTRS` ∪ per-tag)
	200	+	/// ever permits a known URL-bearing attribute that `url_attribute` doesn't
	201	+	/// recognise — keeping the two in sync by construction.
	202	+	#[test]
	203	+	fn url_attribute_covers_every_allowed_url_bearing_attribute() {
	204	+	// The canonical set of HTML attributes that carry a URL. If any of these
	205	+	// becomes allowed on a tag without `url_attribute` covering it, a
	206	+	// javascript: payload would survive sanitization.
	207	+	const URL_BEARING: &[&str] = &[
	208	+	"href", "src", "srcset", "poster", "action", "formaction", "cite",
	209	+	"data", "background", "longdesc", "usemap", "ping", "manifest",
	210	+	"codebase", "archive", "xlink:href",
	211	+	];
	212	+	let generic: HashSet<&str> = GENERIC_ATTRS.iter().copied().collect();
	213	+	let per_tag = tag_attributes();
	214	+	let mut gaps = Vec::new();
	215	+	for &tag in ALLOWED_TAGS {
	216	+	let mut allowed: HashSet<&str> = generic.clone();
	217	+	if let Some(attrs) = per_tag.get(tag) {
	218	+	allowed.extend(attrs.iter().copied());
	219	+	}
	220	+	for &attr in &allowed {
	221	+	if URL_BEARING.contains(&attr) && url_attribute(tag, attr).is_none() {
	222	+	gaps.push(format!("<{tag} {attr}>"));
	223	+	}
	224	+	}
	225	+	}
	226	+	assert!(
	227	+	gaps.is_empty(),
	228	+	"url_attribute() does not cover these allowed URL-bearing attributes \
	229	+	(javascript: URLs would leak through them): {gaps:?}"
	230	+	);
	231	+	}
	232	+
	233	+	/// Behavioural counterpart: a javascript: URL in each covered URL attribute
	234	+	/// must be stripped (never echoed into the output).
	235	+	#[test]
	236	+	fn javascript_urls_are_stripped_from_url_attributes() {
	237	+	for html in [
	238	+	"<a href=\"javascript:alert(1)\">x</a>",
	239	+	"<img src=\"javascript:alert(1)\" alt=\"a\">",
	240	+	"<video poster=\"javascript:alert(1)\"></video>",
	241	+	"<img srcset=\"javascript:alert(1) 1x\" alt=\"a\">",
	242	+	] {
	243	+	let out = clean(html);
	244	+	assert!(
	245	+	!out.to_lowercase().contains("javascript:"),
	246	+	"javascript: URL survived sanitization: {html} -> {out}"
	247	+	);
	248	+	}
	249	+	}
	250	+
194	251		#[test]
195	252		fn strips_iframe_object_embed_form() {
196	253		for tag in ["iframe", "object", "embed", "form"] {

M server/src/routes/storage/images.rs +6 -18

			@@ -134,16 +134,10 @@ pub(super) async fn project_image_confirm(
134	134		));
135	135		}
136	136
137		-	// Verify the object exists in S3
138		-	if !s3.object_exists(&req.s3_key).await? {
139		-	return Err(AppError::BadRequest(
140		-	"Upload not found. Please try uploading again.".to_string(),
141		-	));
142		-	}
143		-
144		-	// Enforce file size limit
	137	+	// A single HEAD: `object_size` returns None when the object isn't there, so
	138	+	// it doubles as the existence check (no separate object_exists round-trip).
145	139		let file_size_bytes = s3.object_size(&req.s3_key).await?.ok_or_else(\|\| {
146		-	AppError::BadRequest("Could not determine file size. Please try uploading again.".to_string())
	140	+	AppError::BadRequest("Upload not found. Please try uploading again.".to_string())
147	141		})?;
148	142		if file_size_bytes as u64 > FileType::Cover.max_size() {
149	143		super::enqueue_s3_orphan(&state.db, &req.s3_key, "image_upload_rejected").await;
			@@ -392,16 +386,10 @@ pub(super) async fn item_image_confirm(
392	386		));
393	387		}
394	388
395		-	// Verify the object exists in S3
396		-	if !s3.object_exists(&req.s3_key).await? {
397		-	return Err(AppError::BadRequest(
398		-	"Upload not found. Please try uploading again.".to_string(),
399		-	));
400		-	}
401		-
402		-	// Enforce file size limit
	389	+	// A single HEAD: `object_size` returns None when the object isn't there, so
	390	+	// it doubles as the existence check (no separate object_exists round-trip).
403	391		let file_size_bytes = s3.object_size(&req.s3_key).await?.ok_or_else(\|\| {
404		-	AppError::BadRequest("Could not determine file size. Please try uploading again.".to_string())
	392	+	AppError::BadRequest("Upload not found. Please try uploading again.".to_string())
405	393		})?;
406	394		if file_size_bytes as u64 > FileType::Cover.max_size() {
407	395		super::enqueue_s3_orphan(&state.db, &req.s3_key, "image_upload_rejected").await;

M server/src/routes/storage/versions.rs +4 -9

			@@ -135,16 +135,11 @@ pub(super) async fn version_confirm_upload(
135	135		));
136	136		}
137	137
138		-	// Verify the object exists in S3
139		-	if !s3.object_exists(&req.s3_key).await? {
140		-	return Err(AppError::BadRequest(
141		-	"Upload not found. Please try uploading again.".to_string(),
142		-	));
143		-	}
144		-
145		-	// Enforce file size limit (versions are always downloads)
	138	+	// A single HEAD: `object_size` returns None when the object isn't there, so
	139	+	// it doubles as the existence check (no separate object_exists round-trip).
	140	+	// Versions are always downloads, so enforce that size limit.
146	141		let file_size_bytes = s3.object_size(&req.s3_key).await?.ok_or_else(\|\| {
147		-	AppError::BadRequest("Could not determine file size. Please try uploading again.".to_string())
	142	+	AppError::BadRequest("Upload not found. Please try uploading again.".to_string())
148	143		})?;
149	144		if file_size_bytes as u64 > FileType::Download.max_size() {
150	145		super::enqueue_s3_orphan(&state.db, &req.s3_key, "version_upload_rejected").await;