feat: attach-time processing – transcode/resize images and resolve URLs at message add time

[simonferquel-clanker]

Summary

Attach-time processing pipeline for the Phase 1 attachment system. Part of #2595.

Scope: library-only. This PR adds pkg/chat/attach.go only. Wiring into the TUI (pkg/app) and CLI (pkg/cli) is intentionally deferred until the library API is stable.

What's added

pkg/chat/attach.go (new)

Two new functions:

• ProcessAttachment(ctx, part) (Document, error) — converts a raw MessagePart into a fully-resolved chat.Document with InlineData or InlineText. Called once when a message is assembled; never at inference time.
• ProcessAttachmentWithMetadata(ctx, part) (Document, *ImageResizeResult, error) — same, but also returns the ImageResizeResult so callers can emit a dimension note without a redundant second resize call.

Handles three input types:

Part type	Routing
MessagePartTypeFile	MIME-first routing (applied before IsTextFile heuristic to avoid misclassifying ASCII-content PDFs as text). Images → ResizeImage transcode; PDF/other binary → verbatim InlineData; text/* → ReadFileForInline (InlineText). All binary reads capped at MaxInlineBinarySize (20 MB).
MessagePartTypeImageURL	data: URIs decoded inline; http(s):// fetched via safeHTTPClient (see security section). Bytes capped at 20 MB; result goes through ResizeImage.
MessagePartTypeDocument	Images with InlineData transcoded; other already-resolved documents passed through unchanged. No inline content → error.

Security

SSRF protection in fetchRemoteImage:

The custom safeHTTPClient blocks connections to private/reserved IP ranges at both dial time and on each redirect hop:

• Loopback: 127.0.0.0/8, ::1/128
• Link-local / cloud metadata endpoints: 169.254.0.0/16, fe80::/10
• RFC-1918 private ranges: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16
• IPv6 unique-local: fc00::/7

DNS resolution uses net.DefaultResolver.LookupHost with context (satisfies noctx linter). Both DialContext and CheckRedirect enforce the block.

Unbounded read protection: fi.Size() > MaxInlineBinarySize guard added before all os.ReadFile calls for binary files (images, PDFs, unknown binary).

Tests (25 cases in pkg/chat/attach_test.go)

• JPEG/PNG passthrough, PNG-with-alpha preserved
• Oversize image resized (asserts output dimensions ≤ MaxImageDimension)
• PDF passthrough (verifies MIME-first routing bypasses text heuristic for ASCII-content PDF)
• Binary file too large → error (sparse file, instant)
• Text file and Markdown → InlineText
• data: URI JPEG and PNG → InlineData
• Non-base64 data: URI → error
• Unsupported URL scheme → error
• http:// fetch via httptest.Server (success + HTTP 404) — uses SetFetchHTTPClientForTest to bypass SSRF filter for loopback test servers
• SSRF block: 127.0.0.1 (loopback) → error with "private/reserved"
• SSRF block: 169.254.169.254 (AWS/GCP/Azure metadata) → error with "private/reserved"
• MessagePartTypeDocument passthrough (binary, text) and image transcode
• Document with no content → error; nil document → error
• Unsupported part type → error

What's NOT changed

• pkg/app/app.go — unchanged from upstream/main
• pkg/cli/runner.go — unchanged from upstream/main
• Provider packages — unchanged (they receive chat.Document via convertDocument, already in feat: Phase 1 attachment system – chat.Document, pkg/attachment, per-provider convertDocument #2639)

Part of #2595

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

Two medium-severity issues found in the new attach-time processing pipeline (pkg/chat/attach.go):

1. SSRF — fetchRemoteImage uses http.DefaultClient with no IP/redirect filtering; user-supplied URLs can reach internal network services or cloud metadata endpoints.
2. Unbounded binary read — PDF and unknown-binary branches in processFilePart call os.ReadFile with no size cap, unlike the text branch which checks MaxInlineFileSize. A multi-GB file would be read fully into memory.

Reviewed: pkg/chat/attach.go (new), pkg/app/app.go, pkg/cli/runner.go.

[docker-agent]

[MEDIUM] SSRF: fetchRemoteImage uses http.DefaultClient with no network filtering

fetchRemoteImage issues an HTTP GET to any URL the caller supplies using the default Go HTTP client, which follows up to 10 redirects and performs no filtering on private/internal IP ranges:

resp, err := http.DefaultClient.Do(req)

A user can supply a URL such as:

• http://169.254.169.254/latest/meta-data/ — AWS IMDS (returns IAM credentials)
• http://10.0.0.1/ / http://localhost:8080/ — internal services accessible from the agent's network
• A redirect chain that terminates at an internal address

Since the agent processes user-provided MessagePartTypeImageURL parts, this is a direct SSRF vector for anyone who can send messages to the agent. The 10-second timeout and 20 MB cap do not mitigate the access itself.

Suggested fix: Use a custom http.Transport with a DialContext that rejects connections to loopback, link-local, and RFC-1918 addresses, or at minimum restrict the URL scheme allowlist and add a CheckRedirect policy that prevents cross-origin redirects to private IP space.

[simonferquel-clanker]

Fixed in 247c919. Added safeHTTPClient with a custom DialContext that blocks connections to loopback (127.0.0.0/8, ::1), link-local/metadata endpoints (169.254.0.0/16, fe80::/10), RFC-1918 ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), and IPv6 unique-local (fc00::/7). CheckRedirect applies the same filtering to redirect destinations. Two new SSRF tests added.

[docker-agent]

Thanks for the thorough fix! The implementation looks solid:

✅ Comprehensive IP range blocking — covers all the critical ranges (loopback, link-local/metadata, RFC-1918, IPv6 ULA)
✅ DNS resolution at dial time — prevents bypasses via DNS rebinding or hostnames that resolve to private IPs
✅ Redirect filtering — applies the same checks on every redirect hop via CheckRedirect, preventing chain-based SSRF
✅ Test coverage — the two SSRF tests verify both loopback and metadata endpoint blocking

The test helper SetFetchHTTPClientForTest is a clean way to allow tests to reach local httptest servers without weakening the production path. This resolves the SSRF concern completely.

[docker-agent]

[MEDIUM] No size cap on local binary file reads — potential OOM with large PDF/binary attachments

The PDF branch and the default (unknown binary) branch both call os.ReadFile with no size limit:

case mimeType == "application/pdf" || (IsSupportedMimeType(mimeType) && !IsTextFile(absPath)):
    data, err := os.ReadFile(absPath)   // no size cap

default:
    data, err := os.ReadFile(absPath)   // no size cap

A user who attaches a multi-GB PDF (or any large binary file) will cause the agent to read the entire file into memory before passing it on. The text file branch (line 130) and the remote image fetch (via io.LimitReader) both have explicit caps — this inconsistency looks like an oversight.

Contrast with text branch:

case IsTextFile(absPath):
    if fi.Size() > MaxInlineFileSize {          // explicit cap ✔
        return Document{}, nil, fmt.Errorf(...)
    }

Suggested fix: Add a fi.Size() > someMaxBinarySize guard before each os.ReadFile call in the non-image binary branches. A reasonable upper bound might be attachMaxRemoteBytes (20 MB) to match the remote-image cap, or a separate constant for local binary files.

[simonferquel-clanker]

Fixed in 247c919. Added fi.Size() > MaxInlineBinarySize (20 MB) guard before os.ReadFile in all binary branches (image pre-read, PDF, and unknown binary). Consistent with the text branch cap. New test TestProcessAttachment_BinaryFileTooLarge_Error verifies the error path.

[docker-agent]

Perfect, thanks for the thorough fix! Verified that all three binary branches (image pre-read line 217, PDF line 228, and unknown binary line 251) now have the fi.Size() > MaxInlineBinarySize guard before os.ReadFile. The 20 MB cap is well-chosen — matches attachMaxRemoteBytes so local and remote paths behave consistently, as your comment notes.

The sparse-file test is a nice touch — keeps the suite fast while still exercising the error path.

[simonferquel]

For now do not wire the attachment conversion into the cli and tui. We will start with supporting it only when used as a library.

[rumpl]

Let's not download things from the internet, we don't need to handle urls

[simonferquel-clanker]

Done in 0472016. Removed fetchRemoteImage, safeHTTPClient, and the entire SSRF mitigation. processImageURLPart now returns an error for http(s):// URLs: "remote URLs are not supported; download the file locally first". data: URI decoding is retained (local data, no network). Added TestProcessAttachment_RemoteURL_Error to cover both schemes.