fix(media): validate Content-Type and magic bytes before sending to model by howie · Pull Request #793 · openabdev/openab

howie · 2026-05-11T11:54:05Z

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491969620754567270/1503586535088590868

Fixes #776.

Root cause

When a Slack bot token lacks the files:read OAuth scope, Slack serves the workspace login HTML page (~55 KB) at HTTP 200 with Content-Type: text/html instead of the requested file binary. download_and_encode_image accepted this response because:

It never inspected the HTTP response Content-Type header.
On resize_and_compress failure for a body <= 1 MB it fell back to forwarding raw bytes under the Slack-reported MIME (image/png), bypassing any format check.

The result: a ContentBlock::Image { media_type: "image/png", data: <base64 of HTML> } flowed to Anthropic, which 400'd with Could not process image. Because claude-agent-acp persists the user message into the session JSONL before the API reply, the bad block replayed on every subsequent turn until an operator manually deleted the JSONL inside the pod.

Changes

src/media.rs (primary change)

Add MediaFetchError enum: NotAnImage (silent skip), UnsupportedResponseType, InvalidImageBody, SizeExceeded, Network, HttpStatus.
Add validate_image_response(content_type, body) pure helper that:
- Rejects any response Content-Type not in {image/png, image/jpeg, image/gif, image/webp} (strips params, case-insensitive).
- Sniffs magic bytes via image::ImageReader::with_guessed_format() (zero new dependencies) and rejects anything that doesn't decode as one of the four supported formats.
Change download_and_encode_image signature from -> Option<ContentBlock> to -> Result<ContentBlock, MediaFetchError>, capturing the Content-Type header before consuming the response with .bytes().
Remove the <= 1 MB resize-error fallback (the direct bug path).

src/slack.rs (call site)

On validation failure, collect filenames and post one aggregated user-facing warning after the file loop:

":warning: I couldn't access the file(s) you shared (photo.png). This often means the bot is missing the files:read OAuth scope. Please ask an admin to reinstall the app with that scope."

Transient errors (Network, HttpStatus) log at warn! and skip silently.

src/discord.rs (call site)

Same Result pattern but log-only on failure (Discord URLs are signed-public; the Slack scope hint is not applicable). Preserves the existing is_video_file fallback for Err(NotAnImage).

Tests

13 new unit tests in src/media.rs::tests for validate_image_response, including the exact bug reproduction and a corrupt-GIF regression:

validate_rejects_html_body_labeled_as_image_png
  body: b"<!DOCTYPE html>..."
  content_type: Some("image/png")
  expected: Err(InvalidImageBody { magic_prefix_hex: "3c21444f43545950" })

Smoke validation:

RUSTFLAGS='-C linker=/tmp/zigcc-wrapper' CC=/tmp/zigcc-wrapper AR=/tmp/zigar-wrapper cargo test --locked media::tests

Result: 24 media tests passed.

Manual test plan (post-deploy)

Install with a bot token missing files:read. Confirm via x-oauth-scopes from auth.test.
Upload an image to the bot in a Slack thread.
Expected: bot replies with the scope warning. Anthropic is never called with the bad block. No JSONL poisoning.
Grant files:read, rotate token, redeploy. Upload an image in the same thread.
Expected: succeeds on first try -- no manual JSONL deletion needed.

Out of scope / follow-ups

Session JSONL persistence: deferring claude-agent-acp write-to-JSONL until after model 200 requires changes in the claude-agent-acp Node project (separate repo). This PR prevents bad bytes from reaching the child process.
Startup preflight: auth.test + apps.permissions.info at boot to warn on missing files:read (useful early-warning, separate concern).
download_and_transcribe / download_and_read_text_file: analogous hardening for audio/text-file paths (lower-priority, separate PR).

At a Glance

Slack/Discord attachment
  -> download_and_encode_image()
  -> reject text/error responses early
  -> sniff and validate image bytes
  -> resize/compress or pass through validated GIF
  -> model-bound ContentBlock::Image

Prior Art & Industry Research

Not applicable: this is a narrow bug fix in existing attachment validation, not a new runtime, persistence, delivery, scheduling, or architectural subsystem.

Why This Approach

The model contract requires real PNG/JPEG/GIF/WebP bytes. Validating after download but before building ContentBlock::Image blocks Slack HTML/login pages and corrupted image bodies at the boundary closest to the failure.

Generic binary responses such as application/octet-stream still pass through to magic-byte validation, so CDN behavior is preserved.

Alternatives Considered

Strict Content-Type allow-list: rejected valid CDN images served as application/octet-stream.
Keep the old raw-byte fallback after resize failure: preserved the original poisoning path.
Fix only Slack call sites: left Discord and future callers exposed to the same invalid-byte contract drift.

…odel Fixes openabdev#776. When a Slack bot token lacks the `files:read` OAuth scope, Slack serves the workspace login HTML page (~55 KB) at HTTP 200 with a `text/html` Content-Type instead of the requested file binary. `download_and_encode_image` previously accepted this response because: 1. It never inspected the HTTP response `Content-Type` header. 2. On `resize_and_compress` failure for a body ≤ 1 MB it fell back to forwarding the raw bytes under the Slack-reported MIME (`image/png`), bypassing any format check. The result: a `ContentBlock::Image { media_type: "image/png", data: <base64 HTML> }` flowed through to Anthropic, which 400'd with "Could not process image". Because claude-agent-acp persists the user message into the session JSONL before the API reply, the bad block replayed on every subsequent turn in that Slack thread until an operator manually deleted the JSONL inside the pod. Changes: - Add `MediaFetchError` enum to `src/media.rs` so callers can distinguish "not an image, skip silently" (`NotAnImage`) from "claimed image, got unexpected bytes" (`UnsupportedResponseType`, `InvalidImageBody`). - Add `validate_image_response(content_type, body)` pure helper that: - Rejects any HTTP response whose Content-Type (stripped of params, lowercased) is not in `{image/png, image/jpeg, image/gif, image/webp}`. - Sniffs magic bytes via `image::ImageReader::with_guessed_format()` (no new dependencies) and rejects anything that doesn't decode as one of the four supported formats. - Change `download_and_encode_image` signature from `-> Option<ContentBlock>` to `-> Result<ContentBlock, MediaFetchError>`, capturing the Content-Type header before consuming the response with `.bytes()`. - Remove the ≤ 1 MB resize-error fallback that was the direct bug path. - Update `src/slack.rs` call site: on validation failure, collect filenames and post one aggregated user-visible warning to the Slack thread: ":warning: I couldn't access the file(s) you shared (`<name>`). This often means the bot is missing the `files:read` OAuth scope. Please ask an admin to reinstall the app with that scope." - Update `src/discord.rs` call site: `warn!` log on failure (Discord URLs are signed-public so the Slack scope hint is not applicable there). Preserve the existing `is_video_file` fallback for `Err(NotAnImage)`. - Add 12 unit tests for `validate_image_response` including the exact bug repro case (HTML body labeled `image/png`, first 8 bytes `3c21444f43545950`). Out of scope / follow-up issues: - Secondary defense: deferring claude-agent-acp JSONL persistence until after model returns 200 (requires changes in the claude-agent-acp Node project). - Startup preflight calling Slack `auth.test` to warn loudly on missing scopes. - Same Content-Type/magic-byte hardening for `download_and_transcribe` and `download_and_read_text_file`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

- Remove dead hinted field from UnsupportedResponseType (always None) - Eliminate double reader.format() call with fmt@ binding - Deduplicate hex_prefix() in resize error path (compute once, reuse) - Promote strip_mime_params to media::strip_mime_params (pub crate), slack.rs delegates to it -- single source of truth for MIME stripping Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Critical: change Content-Type check from allow-list to block-list (Codex finding). The allow-list rejected application/octet-stream before magic-byte check ran, silently dropping valid images from CDNs. Only text/* is now rejected early; everything else falls through to magic-byte verification. Also: - Soften Slack warning message: no longer attributes all failures to files:read scope; now mentions format support as a second cause - Add SizeExceeded to Slack user notification (was silent) - Log failures from send_message() instead of using let _ = - Log discarded io::Error from with_guessed_format - Fix doc comments: download_and_encode_image (SizeExceeded fires pre-HTTP), validate_image_response (Content-Type check short-circuits, not sequential) - Replace inline "Validate Content-Type..." comment with WHY explanation - Restore doc comment on strip_mime_params wrapper in slack.rs - Add tests: octet-stream acceptance (Codex regression fix), JSON body rejection by magic bytes, missing Content-Type + invalid body Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Codex adversarial review found that user-controlled filenames embedded in the mrkdwn warning message could inject Slack markup (backtick break-out, <!here> mentions, <@uid> pings). Replace backticks and angle brackets with safe ASCII equivalents before embedding in the message. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

howie · 2026-05-11T12:58:06Z

Codex Challenge Report — Adversarial Review

Finding 1: Slack filename mrkdwn injection [FIXED]

Filenames embedded in the Slack warning message were user-controlled. A filename containing backticks, <@uid>, or <!here> could break out of the inline-code wrapper and inject Slack markup (mentions, @here pings, formatting). Fixed in commit 4e1a682: backticks and angle brackets are now sanitized before embedding.

Finding 2: Corrupt GIF bodies pass magic-byte check [Known Issue — not in scope]

GIF format is detected by magic bytes (GIF89a/GIF87a) but is passed through without decoding in resize_and_compress to preserve animation. A body with valid GIF magic bytes but corrupt/truncated payload will pass validate_image_response and be forwarded to Anthropic. PNG/JPEG/WebP are caught by the full decode step.

This is pre-existing behavior from before this PR. Fixing it would require decoding GIF frames for validation, which risks breaking animated GIF support. Filed as a known limitation; a follow-up PR should add frame-count validation for GIFs.

Finding 3: failed_image_files Vec is unbounded per event [Acceptable]

The Vec is bounded by Slack's own message attachment limit (~20 files). Not a persistent leak. Acceptable for now.

No TOCTOU between Content-Type capture and body read

Headers and body come from the same immutable reqwest::Response. Server can lie in headers but body validation catches that.

hex_prefix cannot panic

Uses .take(8) with no indexing; handles empty and short slices correctly.

Mixed success: one valid PNG + one HTML file in same message

Valid PNG → pushed to extra_blocks. HTML file → pushed to failed_image_files. Agent receives the valid PNG. User receives one warning message for the failed file. Behavior is correct.

Generated by /pr-review-cycle-codex Step 8 — Codex adversarial challenge

howie · 2026-05-12T02:37:12Z

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491969620754567270/1503586535088590868

shaun-agent · 2026-05-13T09:56:24Z

OpenAB PR-Screening Report

This was generated by an agent-run OpenAB PR-screening workflow. Feedback is welcome; a 👍 reaction is useful if this format helped.

Intent

PR #793 fixes a Slack media-ingestion failure where OpenAB could fetch non-image bytes from Slack, label them as an image, and forward them to the model. The user-visible problem is a confusing Anthropic image-processing error; the operator-visible problem is worse: a bad image block can be persisted into the agent session history and poison later turns in the same Slack thread.

Feat

This is a bug fix and reliability hardening PR. It changes image downloading from a loose Option<ContentBlock> path into a typed Result<ContentBlock, MediaFetchError> path, validates HTTP response content and image magic bytes before forwarding to the model, removes the unsafe raw-byte fallback after resize failure, and adds Slack user-facing warnings for invalid/inaccessible image files.

Who It Serves

Primary beneficiaries are Slack users and OpenAB operators. Users get actionable feedback instead of a provider-level 400. Operators avoid manual PVC/session cleanup when Slack app scopes are wrong or file downloads return HTML/error pages. Discord users also benefit from the shared validation path, though Discord failures remain log-only because Discord attachment URLs have different auth semantics.

Rewritten Prompt

Fix Slack/Discord image ingestion so OpenAB never forwards unverified fetched bytes as model image input.

Requirements:

Capture the HTTP response Content-Type before consuming the body.
Reject explicitly textual responses such as text/html.
Validate downloaded image bodies by format detection/magic bytes before base64 encoding.
Support only model-compatible image formats: PNG, JPEG, GIF, WebP.
Remove any fallback that forwards raw bytes after decode/resize failure.
Return structured media-fetch errors so Slack can distinguish non-images, validation failures, oversized files, network failures, and HTTP failures.
In Slack, warn the user when an image cannot be processed because it is invalid, inaccessible, or likely blocked by missing files:read.
In Discord, preserve video-link fallback for non-image attachments.
Add focused tests for HTML mislabeled as image, missing/generic content type, truncated bodies, MIME parameters, and supported image formats.

Merge Pitch

This PR is worth moving forward because it closes a high-impact ingestion bug with a narrow, well-contained validation layer. The fix prevents a bad Slack fetch from reaching the model, improves user feedback, and reduces the chance of poisoned long-lived sessions. The main reviewer concern should be whether validation is strict enough for corrupted files without breaking legitimate Slack/CDN behavior, especially application/octet-stream downloads and GIF pass-through.

Best-Practice Comparison

OpenClaw principles relevant here:

explicit media loading/normalization pipeline: relevant; this PR moves OpenAB toward a more explicit media validation boundary.
delivery routing and run logs: partly relevant; warnings/logs improve diagnosis when a platform returns an auth/error page instead of media.
retry/backoff and durable job history: not directly relevant to media ingestion.

Hermes Agent principles relevant here:

self-contained prompt/session safety: relevant; bad media should not enter the session transcript as a durable poison pill.
atomic writes for persisted state: conceptually relevant, but the deeper JSONL persistence behavior lives in the agent runtime, not OpenAB core.
fresh session per scheduled run / daemon tick model: not relevant.

The practical lesson from both systems is to treat platform media fetches as untrusted input. Validate at the boundary, fail before model submission, and make recovery possible through normal user/operator action.

Implementation Options

Option A: Conservative
Keep this PR focused on response validation and user warnings. Accept that malformed-but-magic-valid GIFs may still pass through, and track deeper GIF validation/session persistence as follow-ups.

Option B: Balanced
Merge this validation layer, then immediately follow with two small hardening PRs: GIF frame validation and Slack startup preflight for files:read scope. This keeps the fix shippable while addressing the remaining known weak spots.

Option C: Ambitious
Build a full media-ingestion subsystem with adapter-specific fetch policies, strict decode validation for every supported format, provider capability checks at startup, and a session-write policy that only persists media blocks after a successful model call.

Comparison Table

Option	Speed to ship	Complexity	Reliability	Maintainability	User impact	Fit for OpenAB right now
A. Current focused validation	High	Low	High for #776 path	High	High	Strong
B. Validation + near-term hardening	Medium	Medium	Very high	High	Very high	Best next sequence
C. Full media subsystem	Low	High	Highest long-term	Medium	High	Too large for this PR

Recommendation

Recommend moving forward with Option A for this PR, with Option B follow-ups tracked explicitly.

The PR directly addresses #776 and includes the right core changes: validate before encoding, remove raw-byte fallback, and show Slack users an actionable warning. The remaining risk is bounded: GIF validation is still weaker than PNG/JPEG/WebP because the current path can accept GIF magic bytes without fully proving the payload is model-usable. That is a good follow-up, not a reason to hold the whole fix if CI is green.

Validation attempted:

cargo check was run in a clean detached worktree at /tmp/openab-pr793-clean.
The check could not complete because this container lacks linker cc: error: linker 'cc' not found.
PR metadata says the author ran the full test suite successfully, and GitHub discussion-check CI is green.

Project status:

PR fix(media): validate Content-Type and magic bytes before sending to model #793 is already in Project perf: cache dependency build layer in Dockerfile #1 status PR-Screening; no board move was needed.

F1: validate_gif_body now decodes only the first frame instead of collect_frames() — avoids full in-memory decode of large animated GIFs. F2: remove duplicate validate_gif_body call from resize_and_compress; download_and_encode_image already runs validate_image_response before calling resize, so the second call was redundant. F3: add MediaFetchError::ProcessingFailed(image::ImageError) for the case where body passed validation but resize/compress failed — previously returned the misleading InvalidImageBody variant for a validated image. F4: extend Slack warning message to mention "file is too large" so the message is accurate when SizeExceeded failures are included. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Behavior: - slack: add explicit ProcessingFailed arm -> push to failed_image_files and log "post-processing failed" (not "download failed") - slack: extract sanitize_slack_filename() pub(crate) fn; add 4 unit tests for backtick/angle-bracket injection prevention API: - validate_image_response: change return type Result<ImageFormat> -> Result<()> (sole caller only checked Ok/Err; format detection ran twice) Docs: - validate_image_response: add block-list vs allow-list design rationale - validate_gif_body: add doc comment explaining first-frame-only and cursor independence; log original error via debug! before mapping to InvalidImageBody - ProcessingFailed variant: expand doc to clarify semantic difference from InvalidImageBody and expected caller behavior - download_and_encode_image: add ProcessingFailed to error listing Tests: - validate_rejects_mixed_case_text_content_type: pin .to_lowercase() normalization Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

chaodu-agent

LGTM ✅ — prior image-validation findings are resolved. Latest head keeps the #776 fix intact, avoids full GIF frame decode, removes duplicate GIF validation, separates post-validation processing failures with ProcessingFailed, and updates the Slack warning to cover size-limit failures. CI is green.

howie requested a review from thepagent as a code owner May 11, 2026 11:54

github-actions Bot added pending-screening PR awaiting automated screening closing-soon PR missing Discord Discussion URL — will auto-close in 3 days labels May 11, 2026

howie and others added 3 commits May 11, 2026 20:05

github-actions Bot added pending-maintainer and removed closing-soon PR missing Discord Discussion URL — will auto-close in 3 days labels May 13, 2026

shaun-agent added 2 commits May 13, 2026 10:00

fix(media): validate GIF bodies before pass-through

225ea2f

style(discord): apply rustfmt

63c4b8d

This comment has been minimized.

Sign in to view

chaodu-agent added pending-contributor and removed pending-maintainer labels May 13, 2026

howie and others added 2 commits May 13, 2026 20:56

antigenius0910 mentioned this pull request May 14, 2026

bug(slack): unauthenticated file fetch returns HTML, gets forwarded to model as image, poisons session #776

Open

github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label May 15, 2026

chaodu-agent approved these changes May 15, 2026

View reviewed changes

chaodu-agent added pending-maintainer and removed pending-contributor closing-soon PR missing Discord Discussion URL — will auto-close in 3 days pending-screening PR awaiting automated screening labels May 15, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(media): validate Content-Type and magic bytes before sending to model#793

fix(media): validate Content-Type and magic bytes before sending to model#793
howie wants to merge 8 commits into
openabdev:mainfrom
howie:fix/776-validate-fetched-image-bytes

howie commented May 11, 2026 •

edited by shaun-agent

Loading

Uh oh!

howie commented May 11, 2026

Uh oh!

howie commented May 12, 2026

Uh oh!

shaun-agent commented May 13, 2026

Uh oh!

This comment has been minimized.

chaodu-agent left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

howie commented May 11, 2026 • edited by shaun-agent Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause

Changes

Tests

Manual test plan (post-deploy)

Out of scope / follow-ups

At a Glance

Prior Art & Industry Research

Why This Approach

Alternatives Considered

Uh oh!

howie commented May 11, 2026

Codex Challenge Report — Adversarial Review

Finding 1: Slack filename mrkdwn injection [FIXED]

Finding 2: Corrupt GIF bodies pass magic-byte check [Known Issue — not in scope]

Finding 3: failed_image_files Vec is unbounded per event [Acceptable]

No TOCTOU between Content-Type capture and body read

hex_prefix cannot panic

Mixed success: one valid PNG + one HTML file in same message

Uh oh!

howie commented May 12, 2026

Uh oh!

shaun-agent commented May 13, 2026

OpenAB PR-Screening Report

Intent

Feat

Who It Serves

Rewritten Prompt

Merge Pitch

Best-Practice Comparison

Implementation Options

Comparison Table

Recommendation

Uh oh!

This comment has been minimized.

chaodu-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

howie commented May 11, 2026 •

edited by shaun-agent

Loading