feat(render): http(s) URLs for images, configurable bg overlay, doc fixes by ajianaz · Pull Request #78 · codecoradev/cosy

ajianaz · 2026-09-03T07:06:38Z

Closes #74

What

bg_image / images accept http(s):// URLs — image_to_data_uri() detects URL input and fetches with a 10 s timeout and a 10 MB size cap (checked against both Content-Length and actual body). MIME type comes from the Content-Type header (only image/* trusted) with URL-extension fallback. Works for bg_image, logo, and the |b64 filter since it lives at the data-URI layer.
New brand field bg_image_overlay (0.0–1.0, default 0.7 = previous hardcoded value) on blog-hero — controls the gradient overlay opacity on top of the background photo.
Server safety — render_handler now wraps the blocking render in tokio::task::spawn_blocking, so remote fetches (and resvg) never block the async runtime. reqwest moved from dev-dependencies to dependencies (blocking + json + rustls-tls, no openssl).
Docs — corrected --scale default in the CLI guide (actual default is 2.0 retina, not 1.0), documented the 2x output default and URL support in README, added bg_image_overlay to the branding/configuration guides, and added the dark-artwork opacity guidance (~0.55) from the issue.

Why

Real-world blog-hero usage (issue #74) needed remote images without a manual download step, user control over the hardcoded gradient overlay, and the documented defaults match reality.

Testing

cargo test — 136 tests pass, including 6 new: URL detection, fetch success via local TCP server, Content-Type → extension fallback, oversized Content-Length rejection, HTTP error status rejection
E2E CLI: blog-hero with bg_image = http://127.0.0.1:8765/photo.png → photo renders behind gradient; bg_image_overlay: 0.3 shows visibly more photo than default 0.7 (visually verified)
E2E server: cosy serve + POST /api/render with URL background → HTTP 200, valid 1200×675 PNG

…ixes - image_to_data_uri now accepts http(s):// URLs: fetched with a 10s timeout and a 10 MB size cap; mime from Content-Type with URL extension fallback (ref: #74 item 1) - blog-hero: new brand field bg_image_overlay (0.0-1.0, default 0.7) controls the gradient overlay opacity on top of bg_image, replacing the hardcoded 0.7 (ref: #74 item 2) - server render handler wraps blocking render in spawn_blocking so remote fetches never block the async runtime - docs: correct --scale default (2.0, not 1.0), document the 2x output default and URL support, add bg_image_opacity dark-artwork guidance (ref: #74 item 3) Closes #74

github-actions · 2026-09-03T07:11:18Z

🔍 Cora AI Code Review

⚠️ Review could not complete. Cora produced an empty result. Check the workflow logs for errors.

Review powered by cora-code · BYOK · MIT

CodeCora review on this PR flagged an SSRF vector: cosy serve renders attacker-controlled JSON, and image_to_data_uri would GET any http(s):// URL — including loopback, RFC1918, and cloud-metadata link-local targets — while default redirect following could bypass naive host filters, and fetch errors echoed internal details. - Resolve the URL host and require a globally routable address before connecting; re-validate scheme + host on every redirect hop (max 5) - IPv4-mapped IPv6, CGNAT, benchmark, documentation, multicast ranges are rejected too - deny by default: CLI render opts in locally; cosy serve only via the explicit --allow-private-images flag - fetch failures now log the cause and return a generic 'failed to load remote image' instead of echoing URLs and byte counts - 5 new tests: private-target block, redirect-to-metadata block, non-http scheme rejection, IP classification table, generic errors

ajianaz · 2026-09-03T07:27:44Z

SSRF fix pushed (`9c49725`)

Addressing the CodeCora 🔴 blocked finding in full:

Private-address guard (deny by default) — before connecting, the URL host is resolved and must be globally routable: loopback, RFC1918, link-local (incl. cloud metadata 169.254.169.254), CGNAT, benchmark/documentation ranges, multicast, broadcast, IPv6 ULA/link-local/multicast, and IPv4-mapped IPv6 are all rejected.
Redirect hardening — redirects capped at 5, and every hop re-validates scheme (http/https only) + resolved host, so a public first hop can't bounce into internal targets.
Opt-in, not bypass — standalone CLI enables private fetches (local, user-driven); cosy serve requires the explicit --allow-private-images flag. Default posture: blocked.
No info leak — fetch failures log the detailed cause server-side and return a generic failed to load remote image to clients (no URLs, no byte counts, no internal hosts).

New tests: private-target block, redirect-to-metadata block, non-http scheme rejection, and a 21-entry IP classification table. E2E verified: server without the flag blocks 127.0.0.1 bg_image (warn logged, render proceeds without bg); with the flag it renders normally.

Note on remaining behavior: a blocked bg_image degrades gracefully (renders without the background) rather than failing the request — consistent with the pre-existing local-file-missing behavior, and it avoids confirming to the caller whether an internal host exists.

Addresses the second CodeCora block round in full: - DNS rebinding: validate_url_host resolved independently of reqwest's connection resolution, so a hostile domain could rotate to a private IP between the two lookups. The fetch now pins the connection to the validated address via ClientBuilder::resolve — no second resolution. - Unbounded body: response.bytes() buffered everything before the size check; chunked encoding bypassed the Content-Length pre-check. The body is now streamed via take(MAX + 1) so memory is capped during transfer, not after. - Redirects are removed entirely (Policy::none + explicit 3xx error): each hop previously needed its own pin, which is not expressible; a redirect response is now an error in all modes. - https-only by default: plain http:// requires allow_private (local CLI or the server's --allow-private-images), per the 'use HTTPS for external connections' finding. - Tests updated: redirects not followed in either mode, https-only default, loopback hostname blocked, plus the existing SSRF table.

ajianaz · 2026-09-03T07:43:52Z

Round 2 addressed (pushed)

All three 🔴 findings fixed:

DNS rebinding / TOCTOU — the connection is now pinned to the validated IP via ClientBuilder::resolve(host, addr): validation resolves once, the fetched SocketAddr is handed to reqwest, and no second DNS lookup happens before connect. Redirects are gone entirely (Policy::none + explicit 3xx rejection), so there is no per-hop resolution gap either.
Unbounded body read — the body is streamed with Read::take(MAX + 1) and checked against the 10 MB cap during transfer; a lying/chunked Content-Length can no longer balloon memory. The Content-Length pre-check remains as a fast-fail.
HTTP for external connections — remote image URLs are now https-only by default. Plain http:// is accepted only with allow_private, i.e. the local CLI (user-driven) or the explicit cosy serve --allow-private-images opt-in. The remaining http:// literals in tests are loopback test fixtures only (127.0.0.1), where TLS is unavailable.

Tests updated/added: redirect 302 rejected in both modes, https-only default enforcement, loopback hostname (not just IP literal) blocked, plus the existing 21-entry IP classification table and private-target block. Full suite: 142 tests green.

::a.b.c.d (IPv4-compatible, first 96 bits zero) now falls through to the embedded IPv4 check alongside ::ffff:a.b.c.d — ::127.0.0.1 and ::169.254.169.254 were previously treated as public IPv6.

reqwest honors HTTP(S)_PROXY/ALL_PROXY env vars by default; a proxy would perform its own DNS and connect to an unvalidated target, defeating the validated-address pin. Force direct connections.

+            .unwrap_or("png"),
+    );
+
+    let bytes = std::fs::read(path)?;


CodeCora round-3 finding: in server mode an attacker-supplied bg_image like '/etc/passwd' was read from disk and base64-embedded into the rendered PNG — a pre-existing arbitrary-file-read vector the URL fetch work made visible. Introduce ImagePolicy { allow_private, allow_local } threaded explicitly through render_template(_data) -> render_slide_to_png -> process_template -> image_to_data_uri (and the |b64 filter): - server default = ImagePolicy::SECURE: https-only public URLs, and local filesystem paths are rejected outright - new --allow-local-image-paths opt-in for the server (e.g. trusted internal pipeline); --allow-private-images unchanged - standalone CLI uses ImagePolicy::UNRESTRICTED (local, user-driven) Replaces the process-global atomics with an explicit parameter so the policy is visible in signatures and tests need no global mutation. New test: secure policy rejects local paths; e2e verified server default rejects '/etc/hostname' (warn logged, render proceeds without the image) while the opt-in embeds it.

ajianaz · 2026-09-03T08:52:52Z

Round 3 addressed — local file read vector (pushed)

The 🔴 finding was real (and pre-existing): cosy serve would read any attacker-supplied local path (bg_image: "/etc/passwd") and embed it into the returned PNG. Fixed structurally:

New ImagePolicy { allow_private, allow_local } threaded explicitly through render_template(_data) → render_slide_to_png → process_template → image_to_data_uri + the |b64 filter. No process globals — the policy is visible in every signature.
Server default = ImagePolicy::SECURE: https-only public URLs (previous rounds) and local filesystem paths rejected outright.
Two orthogonal opt-ins: --allow-private-images (network) and new --allow-local-image-paths (filesystem) — the standalone CLI remains unrestricted since a local user is already trusted.
Verified end-to-end: server default rejects /etc/hostname (warn logged, render proceeds without the image → no read, no exfiltration, no existence oracle); the opt-in embeds it; CLI unchanged.
Bonus: replaces the round-1 global atomic with an explicit parameter, which also removes test-flakiness risk from global state.

Full suite: 143 tests green.

ajianaz · 2026-09-03T09:22:45Z

Closing in favor of a fresh PR: this thread accumulated 6 review rounds + long fix comments, which pushes the Cora review past its 10-minute job timeout (3 consecutive empty-result reviews). All completed findings (SSRF private-IP guard, DNS-rebinding pin, unbounded body streaming, proxy bypass, local file read) are fixed and verified in the branch. The full evidence trail stays here for reference.

ajianaz added 2 commits September 3, 2026 14:06

style: cargo fmt

ea78495