Skip to content

Commit b6be194

Browse files
howieclaude
andcommitted
fix(media): remove <=1MB fallback and fix sanitize_slack_filename & escape
Address two blocking review findings from PR #793: 1. Remove the <=1MB raw-byte fallback in download_and_encode_image. validate_image_response only sniffs magic bytes; resize_and_compress does the full decode. The fallback forwarded raw bytes under Slack's claimed MIME when resize failed on a corrupt/truncated body, reopening the same JSONL poisoning class as #776. Now always returns ProcessingFailed on resize failure. 2. Add & -> &amp; escape to sanitize_slack_filename before < and > replacements. Slack mrkdwn decodes HTML entities before markup parsing, so &lt;@here&gt; would bypass the angle-bracket replacement and render as a mention ping. Add regression tests for both fixes. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent 6d7fd5f commit b6be194

2 files changed

Lines changed: 38 additions & 13 deletions

File tree

src/media.rs

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -272,18 +272,13 @@ pub async fn download_and_encode_image(
272272
let (output_bytes, output_mime) = match resize_and_compress(&bytes) {
273273
Ok(result) => result,
274274
Err(e) => {
275-
if bytes.len() <= 1024 * 1024 {
276-
debug!(filename, error = %e, "resize failed, using validated original");
277-
(bytes.to_vec(), mime.to_string())
278-
} else {
279-
error!(
280-
filename,
281-
error = %e,
282-
size = bytes.len(),
283-
"resize failed after successful validation"
284-
);
285-
return Err(MediaFetchError::ProcessingFailed(e));
286-
}
275+
error!(
276+
filename,
277+
error = %e,
278+
size = bytes.len(),
279+
"resize failed after successful validation"
280+
);
281+
return Err(MediaFetchError::ProcessingFailed(e));
287282
}
288283
};
289284

@@ -756,6 +751,26 @@ mod tests {
756751
));
757752
}
758753

754+
#[test]
755+
fn truncated_png_body_must_not_produce_content_block() {
756+
// Valid PNG magic bytes (8 bytes) + partial IHDR -- body is too short to decode.
757+
// Previously: the <=1MB fallback in download_and_encode_image forwarded raw bytes
758+
// after resize_and_compress failed, reproducing the #776 poisoning class.
759+
// After removing the fallback, resize_and_compress failure must propagate as Err.
760+
let mut truncated = vec![0x89u8, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a];
761+
truncated.extend_from_slice(&[0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52]);
762+
// validate_image_response passes (magic bytes match PNG) -- the fallback was the bug.
763+
assert!(
764+
validate_image_response(Some("image/png"), &truncated).is_ok(),
765+
"magic-byte check still passes for truncated body"
766+
);
767+
// resize_and_compress catches the truncated body at full-decode time.
768+
assert!(
769+
resize_and_compress(&truncated).is_err(),
770+
"truncated PNG must fail at decode -- no raw-byte fallback allowed"
771+
);
772+
}
773+
759774
#[test]
760775
fn media_fetch_error_display_renders() {
761776
let _ = MediaFetchError::NotAnImage.to_string();

src/slack.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,7 +1236,7 @@ fn strip_mime_params(mimetype: &str) -> &str {
12361236
/// Without sanitization, a user-controlled filename such as `<!here>` or
12371237
/// `` `<@U123>` `` would be rendered as a Slack mention or @-here ping.
12381238
pub(crate) fn sanitize_slack_filename(s: &str) -> String {
1239-
s.replace('`', "'").replace('<', "(").replace('>', ")")
1239+
s.replace('&', "&amp;").replace('`', "'").replace('<', "(").replace('>', ")")
12401240
}
12411241

12421242
/// True only when a Slack non-bot event represents a real user message
@@ -1435,6 +1435,16 @@ mod tests {
14351435
);
14361436
}
14371437

1438+
#[test]
1439+
fn sanitize_escapes_ampersand_before_angle_brackets() {
1440+
// Slack mrkdwn decodes HTML entities before markup parsing.
1441+
// "&lt;@here&gt;" would round-trip back to "<@here>" and trigger a mention
1442+
// ping if & is not escaped. The & must be escaped first so downstream
1443+
// Slack entity decoding cannot reconstruct a mrkdwn delimiter.
1444+
assert_eq!(sanitize_slack_filename("&lt;@here&gt;"), "&amp;lt;@here&amp;gt;");
1445+
assert_eq!(sanitize_slack_filename("file&name.png"), "file&amp;name.png");
1446+
}
1447+
14381448
// --- strip_mime_params tests ---
14391449

14401450
/// MIME with charset parameter strips to bare media type.

0 commit comments

Comments
 (0)