Keep the SSRF-safe Transport in clientForConfig when a per-target timeout is set #69

New Issue

clawbot · 2026-08-07T13:16:10+02:00

clawbot commented

2026-08-07 13:16:10 +02:00

Part of the road to 1.0 (see #33).

The delivery engine's shared HTTP client uses an SSRF-safe dialer (NewSSRFSafeTransport, internal/delivery/ssrf.go, wired onto e.client). However clientForConfig() (internal/delivery/engine.go) constructs a fresh http.Client that does not carry that Transport when a per-target timeout is configured, which bypasses the request-time private-IP guard for that path.

This is currently latent — the UI does not expose a per-target timeout — but it is a real bypass if a target config ever carries one, so it should be closed before 1.0.

Definition of done:

clientForConfig() builds its client on top of the SSRF-safe Transport, overriding only the timeout
a test asserts that a client returned for a config with a per-target timeout still refuses a private/reserved destination

Part of the road to 1.0 (see #33). The delivery engine's shared HTTP client uses an SSRF-safe dialer (`NewSSRFSafeTransport`, `internal/delivery/ssrf.go`, wired onto `e.client`). However `clientForConfig()` (`internal/delivery/engine.go`) constructs a fresh `http.Client` that does not carry that Transport when a per-target timeout is configured, which bypasses the request-time private-IP guard for that path. This is currently latent — the UI does not expose a per-target timeout — but it is a real bypass if a target config ever carries one, so it should be closed before 1.0. Definition of done: - `clientForConfig()` builds its client on top of the SSRF-safe Transport, overriding only the timeout - a test asserts that a client returned for a config with a per-target timeout still refuses a private/reserved destination

clawbot added this to the 1.0.0 milestone 2026-08-07 13:16:10 +02:00

clawbot referenced this issue

2026-08-07 13:16:34 +02:00

1.0/mvp #33

clawbot commented

2026-08-07 13:45:46 +02:00

Implementation instructions

Confine this change to internal/delivery/ ONLY (engine.go plus a delivery-package test). Do not touch handlers or server code.

The problem: the engine's shared client uses an SSRF-safe transport (NewSSRFSafeTransport in internal/delivery/ssrf.go). clientForConfig() in internal/delivery/engine.go builds a fresh http.Client WITHOUT that transport when a per-target timeout is configured, bypassing the request-time private-IP guard for that path.

Fix:

make clientForConfig() build its client on top of the SSRF-safe transport (reuse the same transport the shared e.client uses), overriding only the Timeout
ensure every client the engine uses for outbound target requests carries the SSRF-safe transport — there must be no code path that makes an outbound target request without it

Definition of done:

a client returned by clientForConfig() for a config that sets a per-target timeout still refuses connections to private/reserved/link-local addresses (covered by a test)
behaviour is unchanged when no per-target timeout is set
the SSRF-safe transport is not duplicated in a way that drops its dialer

Gates and process:

make fmt before committing
validate with docker build . (must exit 0; host Go 1.25 vs go.mod 1.26, so Docker is the gate)
branch from main named issue-69-ssrf-client-timeout; commit subject ends with (closes #69)
open a PR (base main) and comment on it with the diff summary and the docker build . result
no AI-assistant/tooling references anywhere

## Implementation instructions Confine this change to `internal/delivery/` ONLY (`engine.go` plus a delivery-package test). Do not touch handlers or server code. The problem: the engine's shared client uses an SSRF-safe transport (`NewSSRFSafeTransport` in `internal/delivery/ssrf.go`). `clientForConfig()` in `internal/delivery/engine.go` builds a fresh `http.Client` WITHOUT that transport when a per-target timeout is configured, bypassing the request-time private-IP guard for that path. Fix: - make `clientForConfig()` build its client on top of the SSRF-safe transport (reuse the same transport the shared `e.client` uses), overriding only the `Timeout` - ensure every client the engine uses for outbound target requests carries the SSRF-safe transport — there must be no code path that makes an outbound target request without it Definition of done: - a client returned by `clientForConfig()` for a config that sets a per-target timeout still refuses connections to private/reserved/link-local addresses (covered by a test) - behaviour is unchanged when no per-target timeout is set - the SSRF-safe transport is not duplicated in a way that drops its dialer Gates and process: - `make fmt` before committing - validate with `docker build .` (must exit 0; host Go 1.25 vs go.mod 1.26, so Docker is the gate) - branch from `main` named `issue-69-ssrf-client-timeout`; commit subject ends with ` (closes #69)` - open a PR (base `main`) and comment on it with the diff summary and the `docker build .` result - no AI-assistant/tooling references anywhere

sneak referenced this issue from a commit

2026-08-07 14:00:03 +02:00

Keep the SSRF-safe transport in clientForConfig (closes #69)

clawbot referenced a pull request that will close this issue

2026-08-07 14:00:10 +02:00

Keep the SSRF-safe transport in clientForConfig (closes #69) #74

sneak closed this issue

2026-08-07 14:03:38 +02:00

sneak referenced this issue from a commit

2026-08-07 14:03:40 +02:00

Keep the SSRF-safe transport in clientForConfig (closes #69) (#74)

clawbot referenced this issue