Consume dunite-geoip and dunite-image-upload (DEV-531) #458

Merged

longjacksonle merged 2 commits from refactor/DEV-531-consume-dunite-geoip-image into main

2026-08-04 18:30:47 +02:00

longjacksonle commented

2026-08-04 18:16:44 +02:00

Owner

Closes DEV-531. DEV-525 extracted this code out of bunyip so a8n-tools could use it; this is the other half, where bunyip stops carrying the original.

Net -329 lines across the two commits. No behaviour change anywhere.

What moved

was	is now
`crates/bunyip-domain/src/services/geoip.rs` (the whole file)	`dunite_geoip::{GeoIpService, GeoIpError}`
`is_non_public_ip`, `login_location_decision`, `LoginLocationDecision` in `services/auth.rs`	`dunite_geoip::*`
`sniff_image_mime`, `enforce_dimensions`, `MAX_AVATAR_SIZE`, `ALLOWED_MIME_TYPES`, `MAX_IMAGE_DIMENSION` in `handlers/avatar.rs`	`dunite_image_upload::{validate_image, ImagePolicy}`

services/geoip.rs survives as a re-export so every crate::services::GeoIpService path in bunyip is untouched. Storage and serving stay in handlers/avatar.rs; only the validation left.

The shared implementations are byte-identical to what they replace - which is exactly why now was the moment to converge them, before either copy drifted.

One visible difference

GeoIpService::new now returns dunite_geoip::GeoIpError rather than AppError. The only caller logs it via Display, so nothing needed converting. The shared crates own their errors deliberately: returning dunite_core::AppError would force every consumer onto one dunite-core revision, the version cascade DEV-515 backed out of.

Test count: 604 -> 596, deliberately

Eight tests removed here, all of which re-tested logic that a dependency now owns:

4 normalizer cases from geoip.rs
non_public_ip_detection and login_location_decision_branches from auth.rs
4 sniff/dimension cases from avatar.rs, replaced by 2 that cover what is still bunyip's to get right (the error conversion, and that the shared policy's cap still matches the user_avatars.size_bytes CHECK - if those diverged, a valid upload would pass validation and then fail on insert with a database error instead of a useful message)

Upstream, dunite-geoip and dunite-image-upload carry 21 tests covering all of the above and more: a declared dimension bomb, unrecognised content failing closed, the absence of SVG from the allowlist, and that the error messages describe the rule rather than the bytes. None of those four were tested in bunyip.

Two things deliberately left alone

infer and imagesize stay direct dependencies of bunyip-api, for the feedback-attachment path (BUNYIP-90). Its allowlist admits non-images (text/plain), so it does not fit ImagePolicy. Folding that surface in is a separate decision, not a side effect of this one.
bunyip warns and disables when the IP2Location .BIN fails to load; a8n exits. Both are defensible - bunyip treats alerts as best-effort, a8n treats a set-but-broken path as a misconfiguration an operator must see - and they were already divergent before this PR. Worth a decision at some point, but changing it here would be a behaviour change smuggled into a de-duplication.

Also

Bumps every dunite pin in the workspace from cc10e83 to the merged 302af65, so bunyip stays on one git checkout rather than two. Verified additive: git diff cc10e83..302af65 in dunite touches only the two new crates and the manifests.

just check-build, just check-clippy and just check-fmt are clean.

Closes DEV-531. DEV-525 extracted this code *out of* bunyip so a8n-tools could use it; this is the other half, where bunyip stops carrying the original. Net **-329 lines** across the two commits. No behaviour change anywhere. ### What moved | was | is now | | --- | --- | | `crates/bunyip-domain/src/services/geoip.rs` (the whole file) | `dunite_geoip::{GeoIpService, GeoIpError}` | | `is_non_public_ip`, `login_location_decision`, `LoginLocationDecision` in `services/auth.rs` | `dunite_geoip::*` | | `sniff_image_mime`, `enforce_dimensions`, `MAX_AVATAR_SIZE`, `ALLOWED_MIME_TYPES`, `MAX_IMAGE_DIMENSION` in `handlers/avatar.rs` | `dunite_image_upload::{validate_image, ImagePolicy}` | `services/geoip.rs` survives as a re-export so every `crate::services::GeoIpService` path in bunyip is untouched. Storage and serving stay in `handlers/avatar.rs`; only the validation left. The shared implementations are byte-identical to what they replace - which is exactly why now was the moment to converge them, before either copy drifted. ### One visible difference `GeoIpService::new` now returns `dunite_geoip::GeoIpError` rather than `AppError`. The only caller logs it via `Display`, so nothing needed converting. The shared crates own their errors deliberately: returning `dunite_core::AppError` would force every consumer onto one `dunite-core` revision, the version cascade DEV-515 backed out of. ### Test count: 604 -> 596, deliberately Eight tests removed here, all of which re-tested logic that a dependency now owns: - 4 normalizer cases from `geoip.rs` - `non_public_ip_detection` and `login_location_decision_branches` from `auth.rs` - 4 sniff/dimension cases from `avatar.rs`, replaced by 2 that cover what is still bunyip's to get right (the error conversion, and that the shared policy's cap still matches the `user_avatars.size_bytes` CHECK - if those diverged, a valid upload would pass validation and then fail on insert with a database error instead of a useful message) Upstream, `dunite-geoip` and `dunite-image-upload` carry **21 tests** covering all of the above and more: a declared dimension bomb, unrecognised content failing closed, the absence of SVG from the allowlist, and that the error messages describe the rule rather than the bytes. None of those four were tested in bunyip. ### Two things deliberately left alone - **`infer` and `imagesize` stay direct dependencies** of `bunyip-api`, for the feedback-attachment path (BUNYIP-90). Its allowlist admits non-images (`text/plain`), so it does not fit `ImagePolicy`. Folding that surface in is a separate decision, not a side effect of this one. - **bunyip warns and disables when the IP2Location `.BIN` fails to load; a8n exits.** Both are defensible - bunyip treats alerts as best-effort, a8n treats a set-but-broken path as a misconfiguration an operator must see - and they were already divergent before this PR. Worth a decision at some point, but changing it here would be a behaviour change smuggled into a de-duplication. ### Also Bumps every dunite pin in the workspace from `cc10e83` to the merged `302af65`, so bunyip stays on one git checkout rather than two. Verified additive: `git diff cc10e83..302af65` in dunite touches only the two new crates and the manifests. `just check-build`, `just check-clippy` and `just check-fmt` are clean.

longjacksonle added 2 commits

2026-08-04 18:16:44 +02:00

refactor(geoip): consume dunite-geoip instead of bunyip's copy (DEV-531) cb25933063

DEV-525 extracted this code out of bunyip into `dunite-geoip` so a8n-tools could use it too. This is the other half: bunyip stops carrying the original.

`services/geoip.rs` becomes a re-export, keeping the module as the seam so every `crate::services::GeoIpService` path is unchanged. `is_non_public_ip` and `login_location_decision` come from the crate rather than from `services/auth.rs`.

No behaviour change. The shared implementations are byte-identical to what they replace, which is what made now the moment to converge them. The one visible difference is that `GeoIpService::new` returns `dunite_geoip::GeoIpError` rather than `AppError`; the only caller logs it via `Display`, so nothing needed converting. The shared crate owns its error deliberately - returning `dunite_core::AppError` would force every consumer onto one dunite-core revision, the version cascade DEV-515 backed out of.

Six tests move out with the code: four normalizer cases from `geoip.rs`, and `non_public_ip_detection` plus `login_location_decision_branches` from `auth.rs`. `dunite-geoip` covers all six and adds four more, including that an unrecognised country code fails closed. Re-testing a dependency from the consumer is noise, not coverage.

Also bumps every dunite pin in the workspace from cc10e83 to the merged 302af65, so bunyip stays on one git checkout rather than two. Verified additive: `git diff cc10e83..302af65` in dunite touches only the two new crates and the manifests.

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

refactor(avatar): consume dunite-image-upload for the validation (DEV-531)

E2E PR gate / Install + reachability (no deployment secrets) (pull_request) Successful in 31s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 14m0s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

4af42c9798

The other half of the DEV-525 extraction. `handlers/avatar.rs` keeps storage and serving and drops its copies of the checks: `sniff_image_mime`, `enforce_dimensions`, `MAX_AVATAR_SIZE`, `ALLOWED_MIME_TYPES` and `MAX_IMAGE_DIMENSION` all come from the shared crate now, via `ImagePolicy::avatar()` which carries bunyip's exact numbers.

No behaviour change: every check still runs against file content, never the declared MIME or the filename, and the limits are the same 2 MiB / 4096px / PNG-JPEG-WebP-GIF. The error wording is now the crate's, which describes the rule and never the bytes, converted to `AppError::validation("avatar", ...)` at one call site so the client still attaches it to the right input.

`infer` and `imagesize` stay direct dependencies for the feedback-attachment path (BUNYIP-90), whose allowlist admits non-images and so does not fit `ImagePolicy`. Folding that surface in is a separate decision, not a side effect of this one.

Four unit tests move upstream, replaced by two that cover what is still bunyip's to get right: the error conversion, and that the shared policy's cap still matches the `user_avatars.size_bytes` CHECK - if the two ever diverged, a valid upload would pass validation and then fail on insert with a database error instead of a useful message. `dunite-image-upload` has eleven tests covering the rest, including a declared dimension bomb and the absence of SVG from the allowlist, neither of which was tested here.

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

longjacksonle scheduled this pull request to auto merge when all checks succeed

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