Merge branch 'fix/narrow-admission-check'
This commit is contained in:
@@ -239,12 +239,10 @@ pub async fn upload(
|
||||
// vanish with, at best, a vague "could not be processed". Rejecting here gives them a
|
||||
// reason at the door that they can act on, and it uses the SAME budget the worker
|
||||
// enforces, so admission and processing cannot disagree.
|
||||
if mime.starts_with("image/")
|
||||
&& let Err(e) = crate::services::imaging::probe_decodable(&temp_abs)
|
||||
{
|
||||
if mime.starts_with("image/") && crate::services::imaging::exceeds_decode_budget(&temp_abs) {
|
||||
let mp = crate::services::imaging::megapixels(&temp_abs);
|
||||
tracing::info!(
|
||||
error = ?e, %mime, megapixels = ?mp,
|
||||
%mime, megapixels = ?mp,
|
||||
"rejecting an image that exceeds the decode budget at admission"
|
||||
);
|
||||
let _ = tokio::fs::remove_file(&temp_abs).await;
|
||||
|
||||
@@ -99,13 +99,29 @@ pub fn megapixels(path: &Path) -> Option<f64> {
|
||||
Some(f64::from(w) * f64::from(h) / 1_000_000.0)
|
||||
}
|
||||
|
||||
/// Reject an image the compression worker could never process, reading only its header.
|
||||
/// True when an image cannot be decoded specifically because it would exceed the memory
|
||||
/// budget — read from the header, no pixels touched.
|
||||
///
|
||||
/// Called at upload admission so the guest is told at the door, with a reason they can act
|
||||
/// on, instead of the upload being accepted with a 201 and then silently soft-deleted
|
||||
/// minutes later when the worker gives up on it.
|
||||
pub fn probe_decodable(path: &Path) -> Result<()> {
|
||||
decoder_within_budget(path).map(|_| ())
|
||||
/// Called at upload admission so a guest who sends a 100 MP photo is told at the door, with
|
||||
/// a reason they can act on, instead of the upload being accepted with a 201 and then
|
||||
/// silently soft-deleted minutes later when the worker gives up on it.
|
||||
///
|
||||
/// Deliberately narrow: ONLY the budget. A corrupt, truncated or unsupported file also
|
||||
/// fails to build a decoder, but rejecting those here would change a contract the
|
||||
/// adversarial suite pins on purpose — acceptance follows the magic bytes, and a payload
|
||||
/// with a valid JPEG header is accepted regardless of what follows it. Those go to the
|
||||
/// compression worker as before, which handles them gracefully and (since the retry
|
||||
/// classifier) no longer burns backoff on them.
|
||||
pub fn exceeds_decode_budget(path: &Path) -> bool {
|
||||
match decoder_within_budget(path) {
|
||||
Ok(_) => false,
|
||||
Err(e) => e.chain().any(|cause| {
|
||||
matches!(
|
||||
cause.downcast_ref::<image::ImageError>(),
|
||||
Some(image::ImageError::Limits(_))
|
||||
)
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// Decode an image from disk with decompression-bomb limits applied and its EXIF
|
||||
@@ -186,23 +202,49 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn probe_agrees_with_the_decoder_on_both_sides() {
|
||||
// Admission and processing must never disagree — a photo accepted at the door and
|
||||
// then rejected by the worker is the exact failure this pair exists to prevent.
|
||||
fn admission_rejects_only_the_over_budget_case() {
|
||||
// Admission and processing must agree about SIZE — a photo accepted at the door and
|
||||
// then rejected by the worker for being too big is the failure this pair prevents.
|
||||
assert!(
|
||||
probe_decodable(Path::new(HUGE)).is_err(),
|
||||
"probe must reject what the decoder rejects"
|
||||
exceeds_decode_budget(Path::new(HUGE)),
|
||||
"admission must reject what the decoder rejects for size"
|
||||
);
|
||||
let ordinary = concat!(
|
||||
env!("CARGO_MANIFEST_DIR"),
|
||||
"/../e2e/fixtures/media/portrait-exif6.jpg"
|
||||
);
|
||||
assert!(
|
||||
probe_decodable(Path::new(ordinary)).is_ok(),
|
||||
"probe must accept what the decoder accepts"
|
||||
!exceeds_decode_budget(Path::new(ordinary)),
|
||||
"admission must accept an ordinary photo"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn admission_does_not_reject_a_merely_undecodable_file() {
|
||||
// The narrowing that keeps the adversarial contract intact: a payload with valid
|
||||
// JPEG magic bytes and nothing behind them cannot be decoded, but acceptance follows
|
||||
// the magic bytes by design (07-adversarial/file-upload-attacks). It is the worker's
|
||||
// job to fail it, not admission's — admission is only the resource guard.
|
||||
let dir = std::env::temp_dir().join("eventsnap-imaging-test");
|
||||
std::fs::create_dir_all(&dir).expect("tmp dir");
|
||||
let stub = dir.join("magic-only.jpg");
|
||||
let mut bytes = vec![0u8; 1024];
|
||||
bytes[..3].copy_from_slice(&[0xFF, 0xD8, 0xFF]);
|
||||
std::fs::write(&stub, &bytes).expect("write stub");
|
||||
|
||||
assert!(
|
||||
!exceeds_decode_budget(&stub),
|
||||
"a corrupt file is not an over-budget file"
|
||||
);
|
||||
assert!(
|
||||
decode_oriented(&stub)
|
||||
.map(|i| (i.width(), i.height()))
|
||||
.is_err(),
|
||||
"...but it must still fail in the worker"
|
||||
);
|
||||
let _ = std::fs::remove_file(&stub);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn still_decodes_an_ordinary_photo_and_applies_orientation() {
|
||||
// The guard must not have become a blanket refusal. This fixture is 40x20 stored with
|
||||
|
||||
Reference in New Issue
Block a user