Extract dunite-image-upload for avatar validation (DEV-525) #33

Merged

longjacksonle merged 1 commit from feat/DEV-525-dunite-image-upload into main

2026-08-04 18:04:45 +02:00

longjacksonle commented

2026-08-04 17:57:09 +02:00

Owner

Stacked on #32 (feat/DEV-525-dunite-geoip). Merge that first.

The three rules it owns

Type comes from the bytes, never the request. A declared Content-Type and a filename are both attacker-controlled. sniff_mime reads the magic bytes and matches an allowlist, so a shell script announced as image/png with a filename of avatar.png is rejected on content. Unrecognised content fails closed rather than falling through to an allowlist check that a missing type would trivially pass.

Dimensions are parsed from the header, not decoded. A few hundred bytes of PNG can declare a billion pixels. check_dimensions reads the header and stops, so no decoder ever runs against adversarial input and the bomb is refused before anything allocates. There is a test that builds exactly that file - a 68-byte PNG declaring 65535x65535 - and asserts it is refused.

Size is the caller's job to enforce while streaming, and the column's job to enforce again. ImagePolicy::max_bytes is the app-layer cap; a CHECK on the stored bytes is what makes a bypassed app-layer check harmless.

Policy as a parameter

ImagePolicy rather than constants, because the limits are policy and two surfaces in one app can reasonably differ - an avatar is small and must be an image, a bug-report attachment may be larger with a wider allowlist. What must not differ between apps is how the limits are enforced, which is what this crate owns.

ImagePolicy::avatar() carries bunyip's existing numbers (2 MiB, 4096px) so the two stay comparable. There is a test asserting SVG is not on the list: it is a document format that can carry script, and serving one back to a browser is stored XSS that no amount of sniffing makes safe.

Error type

ImageValidationError is owned here, not dunite_core::AppError - the version cascade DEV-515 backed out of. Its Display messages describe the rule and never the bytes, since they reach the uploader; there is a test pinning that.

Not here

Storage, HTTP, multipart parsing. The consumer streams the upload, enforcing the byte cap as it goes so an oversized body is refused mid-stream rather than after buffering, and hands the finished bytes here.

Tests

11 unit tests. The ones carrying weight: a script rejected regardless of what it claimed, a real-but-disallowed image (BMP) caught on content, unrecognised bytes failing closed, and the declared dimension bomb.

Adoption

a8n-tools/saas consumes this in the follow-up PR. bunyip still has the original in bunyip-api/src/handlers/avatar.rs; swapping it over is tracked on DEV-531 alongside the geoip one.

Second new crate for DEV-525. a8n-tools is gaining profile avatars, which bunyip already has (BUNYIP-408). The checks that make accepting an image upload safe are identical in both apps and expensive to get wrong once, so they land here rather than as a second copy. > **Stacked on #32** (`feat/DEV-525-dunite-geoip`). Merge that first. ### The three rules it owns **Type comes from the bytes, never the request.** A declared `Content-Type` and a filename are both attacker-controlled. `sniff_mime` reads the magic bytes and matches an allowlist, so a shell script announced as `image/png` with a filename of `avatar.png` is rejected on content. Unrecognised content fails closed rather than falling through to an allowlist check that a missing type would trivially pass. **Dimensions are parsed from the header, not decoded.** A few hundred bytes of PNG can declare a billion pixels. `check_dimensions` reads the header and stops, so no decoder ever runs against adversarial input and the bomb is refused before anything allocates. There is a test that builds exactly that file - a 68-byte PNG declaring 65535x65535 - and asserts it is refused. **Size is the caller's job to enforce while streaming, and the column's job to enforce again.** `ImagePolicy::max_bytes` is the app-layer cap; a `CHECK` on the stored bytes is what makes a bypassed app-layer check harmless. ### Policy as a parameter `ImagePolicy` rather than constants, because the limits are policy and two surfaces in one app can reasonably differ - an avatar is small and must be an image, a bug-report attachment may be larger with a wider allowlist. What must *not* differ between apps is how the limits are enforced, which is what this crate owns. `ImagePolicy::avatar()` carries bunyip's existing numbers (2 MiB, 4096px) so the two stay comparable. There is a test asserting SVG is not on the list: it is a document format that can carry script, and serving one back to a browser is stored XSS that no amount of sniffing makes safe. ### Error type `ImageValidationError` is owned here, not `dunite_core::AppError` - the version cascade DEV-515 backed out of. Its `Display` messages describe the rule and never the bytes, since they reach the uploader; there is a test pinning that. ### Not here Storage, HTTP, multipart parsing. The consumer streams the upload, enforcing the byte cap as it goes so an oversized body is refused mid-stream rather than after buffering, and hands the finished bytes here. ### Tests 11 unit tests. The ones carrying weight: a script rejected regardless of what it claimed, a real-but-disallowed image (BMP) caught on content, unrecognised bytes failing closed, and the declared dimension bomb. ### Adoption a8n-tools/saas consumes this in the follow-up PR. bunyip still has the original in `bunyip-api/src/handlers/avatar.rs`; swapping it over is tracked on DEV-531 alongside the geoip one.

longjacksonle added 1 commit

2026-08-04 17:57:10 +02:00

feat(image-upload): extract dunite-image-upload for avatar validation (DEV-525)

Check / fmt + clippy + test (pull_request) Successful in 25s

Details

create-release / create-release (pull_request) Has been skipped

Details

6c535f447e

DEV-525 is adding profile avatars to a8n-tools, which bunyip already has (BUNYIP-408). The checks that make accepting an image upload safe are identical in both apps, easy to get subtly wrong, and expensive to get wrong once, so they land here rather than as a second copy.

Type comes from the bytes, never the request. A declared `Content-Type` and a filename are both attacker-controlled, so `sniff_mime` reads the magic bytes and matches an allowlist: a shell script announced as `image/png` with a filename of `avatar.png` is rejected on content. Unrecognised content fails closed rather than falling through to an allowlist check that a missing type would trivially pass.

Dimensions are parsed from the header, not decoded. A few hundred bytes of PNG can declare a billion pixels; `check_dimensions` reads the header and stops, so no decoder ever runs against adversarial input and the bomb is refused before anything allocates. There is a test that builds exactly that file and asserts it is refused.

`ImagePolicy` makes the limits a parameter rather than constants, because they are policy and two surfaces in one app can reasonably differ - an avatar is small and must be an image, a bug-report attachment may be larger. What must not differ between apps is how the limits are enforced, which is what this crate owns. `ImagePolicy::avatar()` carries bunyip's existing numbers so the two stay comparable, and there is a test asserting SVG is not on the list: it is a document format that can carry script, and serving one back is stored XSS that no amount of sniffing makes safe.

`ImageValidationError` is owned here rather than borrowed from dunite-core, for the version-cascade reason DEV-515 backed out of. Its messages describe the rule and never the bytes, since they reach the uploader.

Storage, HTTP and multipart parsing stay with the consumer: it streams the upload, enforcing the byte cap as it goes so an oversized body is refused mid-stream rather than after buffering, and hands the finished bytes here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FcwfAHYwMmiAYbMrCc7cJG