Make redirect handling configurable in all five languages

.changeset/redirect-policy.md

@@ -0,0 +1,30 @@
+---
+'@smooai/fetch': minor
+---
+
+Make redirect handling configurable in all five languages
+
+Redirects were followed unconditionally everywhere, and TypeScript went further:
+`merge({}, init, { redirect: 'follow' })` put the literal last, so a caller
+passing `redirect: 'manual'` had it **silently overwritten**. Python hardcoded
+`follow_redirects=True` into the httpx kwargs; Rust, Go and .NET set nothing and
+inherited platform defaults that follow up to 10 hops.
+
+That is a security gap, not just an ergonomic one. A caller who resolves a
+hostname and checks it against an SSRF allowlist has that guard defeated by a 302
+to an internal address, because the check was performed on the original host. And
+RFC 8461 forbids following redirects when fetching an MTA-STS policy.
+
+- **TypeScript** — `redirect` is honoured (defaults first, caller last)
+- **Python** — `FetchOptions.follow_redirects`
+- **Rust** — `RequestInit.follow_redirects: Option<bool>` (`None` inherits, so a
+  client-level default survives a per-request `..Default::default()`)
+- **Go** — `ClientBuilder.WithFollowRedirects`, applied to a caller-supplied
+  `*http.Client` too
+- **.NET** — `SmooFetchOptions.FollowRedirects` / `WithFollowRedirects`
+
+Honouring the option was not sufficient on its own: in TS, Rust, Go and .NET a
+3xx is neither "ok" nor "redirected", so it was raised as an error and the option
+was undone a line later. Each now returns a deliberately-unfollowed 3xx as an
+ordinary response. Defaults are unchanged — everything still follows unless a
+caller says otherwise.

dotnet/SmooAI.Fetch.Tests/RedirectPolicyTests.cs

@@ -0,0 +1,159 @@
+using System.Net;
+using SmooAI.Fetch;
+
+namespace SmooAI.Fetch.Tests;
+
+/// <summary>
+/// th-86dc77 — redirects were followed unconditionally (SocketsHttpHandler defaults
+/// AllowAutoRedirect to true) with no way for a caller to decline.
+///
+/// Following one defeats an SSRF check performed on the original host — a 302 to an
+/// internal address bypasses a guard applied to the original hostname — and RFC 8461
+/// forbids it outright when fetching an MTA-STS policy.
+/// </summary>
+public class RedirectPolicyTests
+{
+    private sealed record Reply(bool Arrived);
+
+    /// <summary>Serves a 302 at /start pointing at /landing, and 200 at /landing.</summary>
+    private static (HttpListener listener, string url, Func<bool> landed) StartRedirectingServer()
+    {
+        var landedFlag = false;
+        var port = GetFreePort();
+        var prefix = $"http://127.0.0.1:{port}/";
+        var listener = new HttpListener();
+        listener.Prefixes.Add(prefix);
+        listener.Start();
+
+        _ = Task.Run(async () =>
+        {
+            while (listener.IsListening)
+            {
+                HttpListenerContext ctx;
+                try
+                {
+                    ctx = await listener.GetContextAsync();
+                }
+                catch
+                {
+                    return;
+                }
+
+                if (ctx.Request.Url!.AbsolutePath == "/landing")
+                {
+                    landedFlag = true;
+                    ctx.Response.StatusCode = 200;
+                    ctx.Response.ContentType = "application/json";
+                    var body = System.Text.Encoding.UTF8.GetBytes("{\"arrived\":true}");
+                    await ctx.Response.OutputStream.WriteAsync(body);
+                }
+                else
+                {
+                    // The 302 carries a JSON body on purpose. Without one, a
+                    // deserialization failure would surface as the SAME
+                    // HttpResponseError this library raises for a bad status,
+                    // and the throwing-path test below could not tell the two
+                    // apart.
+                    ctx.Response.StatusCode = 302;
+                    ctx.Response.RedirectLocation = $"{prefix}landing";
+                    ctx.Response.ContentType = "application/json";
+                    var redirectBody = System.Text.Encoding.UTF8.GetBytes("{\"arrived\":false}");
+                    await ctx.Response.OutputStream.WriteAsync(redirectBody);
+                }
+
+                ctx.Response.Close();
+            }
+        });
+
+        return (listener, prefix, () => landedFlag);
+    }
+
+    private static int GetFreePort()
+    {
+        var l = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0);
+        l.Start();
+        var port = ((System.Net.IPEndPoint)l.LocalEndpoint).Port;
+        l.Stop();
+        return port;
+    }
+
+    [Fact]
+    public void FollowRedirects_defaults_to_true()
+    {
+        Assert.True(new SmooFetchOptions().FollowRedirects);
+    }
+
+    [Fact]
+    public async Task A_redirect_is_not_followed_when_the_caller_opts_out()
+    {
+        var (listener, url, landed) = StartRedirectingServer();
+        try
+        {
+            var fetch = SmooFetchBuilder.Create()
+                .WithNoRetry()
+                .WithFollowRedirects(false)
+                .Build();
+
+            using var request = new HttpRequestMessage(HttpMethod.Get, $"{url}start");
+            var response = await fetch.SendAsync(request);
+
+            Assert.Equal(HttpStatusCode.Found, response.StatusCode);
+            // The landing route is mounted so following would visibly succeed —
+            // this asserts the hop did not happen, not that it 404'd.
+            Assert.False(landed(), "the redirect was followed despite WithFollowRedirects(false)");
+        }
+        finally
+        {
+            listener.Stop();
+        }
+    }
+
+    [Fact]
+    public async Task Redirects_are_followed_by_default()
+    {
+        var (listener, url, landed) = StartRedirectingServer();
+        try
+        {
+            var fetch = SmooFetchBuilder.Create().WithNoRetry().Build();
+
+            using var request = new HttpRequestMessage(HttpMethod.Get, $"{url}start");
+            var response = await fetch.SendAsync(request);
+
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.True(landed(), "default behaviour must be unchanged");
+        }
+        finally
+        {
+            listener.Stop();
+        }
+    }
+
+    /// <summary>
+    /// The throwing path. <c>SendAsync</c> never raises, so it cannot show that a
+    /// deliberately-unfollowed 3xx is returned rather than thrown — without the
+    /// <c>IsAcceptable</c> change the option would be honoured and then immediately
+    /// undone by an <see cref="HttpResponseError"/>.
+    /// </summary>
+    [Fact]
+    public async Task An_unfollowed_redirect_is_not_thrown_on_the_json_path()
+    {
+        var (listener, url, _) = StartRedirectingServer();
+        try
+        {
+            var fetch = SmooFetchBuilder.Create()
+                .WithNoRetry()
+                .WithFollowRedirects(false)
+                .Build();
+
+            var reply = await fetch.GetAsync<Reply>($"{url}start");
+
+            // Reached deserialization at all, which means the 3xx passed the
+            // status gate rather than being thrown as an HttpResponseError.
+            Assert.False(reply.Arrived);
+        }
+        finally
+        {
+            listener.Stop();
+        }
+    }
+}

dotnet/SmooAI.Fetch/SmooFetch.cs

@@ -108,6 +108,12 @@ public static SmooFetch Create(Action<SmooFetchOptions>? configure = null)
             handler.ConnectTimeout = connectTimeout;
         }
 
+        // Defaults to true on the handler, so only the opt-out is worth setting.
+        if (!options.FollowRedirects)
+        {
+            handler.AllowAutoRedirect = false;
+        }
+
         var client = new HttpClient(handler) { Timeout = System.Threading.Timeout.InfiniteTimeSpan };
         return new SmooFetch(client, ownsHttpClient: true, options, logger: null);
     }
@@ -245,12 +251,32 @@ private async Task SendJsonNoResponseAsync(
     {
         using var request = BuildRequest(method, path, body, hasBody);
         using var response = await SendAsync(request, cancellationToken).ConfigureAwait(false);
-        if (!response.IsSuccessStatusCode)
+        if (!IsAcceptable(response))
         {
             await ThrowHttpResponseErrorAsync(response, cancellationToken).ConfigureAwait(false);
         }
     }
 
+    /// <summary>
+    /// Whether a response should be returned rather than thrown.
+    /// </summary>
+    /// <remarks>
+    /// A 3xx when <see cref="SmooFetchOptions.FollowRedirects"/> is false is the ANSWER,
+    /// not a failure — the caller asked to see the redirect rather than follow it. Without
+    /// this, honouring the option leaves it useless: the 302 comes back and is immediately
+    /// thrown as an <see cref="HttpResponseError"/> because it is not 2xx.
+    /// </remarks>
+    private bool IsAcceptable(HttpResponseMessage response)
+    {
+        if (response.IsSuccessStatusCode)
+        {
+            return true;
+        }
+
+        var status = (int)response.StatusCode;
+        return !_options.FollowRedirects && status >= 300 && status < 400;
+    }
+
     private HttpRequestMessage BuildRequest(HttpMethod method, string path, object? body, bool hasBody)
     {
         var uri = ResolveUri(path);
@@ -346,7 +372,7 @@ private static async Task<HttpRequestMessage> CloneRequestAsync(HttpRequestMessa
 
     private async Task<TResponse> ReadJsonResponseAsync<TResponse>(HttpResponseMessage response, CancellationToken cancellationToken)
     {
-        if (!response.IsSuccessStatusCode)
+        if (!IsAcceptable(response))
         {
             await ThrowHttpResponseErrorAsync(response, cancellationToken).ConfigureAwait(false);
         }

dotnet/SmooAI.Fetch/SmooFetchBuilder.cs

@@ -76,6 +76,20 @@ public SmooFetchBuilder WithConnectTimeout(TimeSpan connectTimeout)
         return this;
     }
 
+    /// <summary>
+    /// Controls whether redirects are followed. Defaults to <c>true</c>.
+    /// </summary>
+    /// <remarks>
+    /// Set <c>false</c> when a redirect must not be followed: it defeats an SSRF check
+    /// performed on the original host, and RFC 8461 forbids it when fetching an MTA-STS
+    /// policy. The 3xx is then returned rather than thrown.
+    /// </remarks>
+    public SmooFetchBuilder WithFollowRedirects(bool followRedirects)
+    {
+        _options.FollowRedirects = followRedirects;
+        return this;
+    }
+
     /// <summary>Register an async auth-token provider.</summary>
     public SmooFetchBuilder WithAuthTokenProvider(AuthTokenProvider provider, string scheme = "Bearer")
     {
@@ -231,6 +245,11 @@ public SmooFetch Build()
             o.RetryPolicy = _options.RetryPolicy;
             o.Timeout = _options.Timeout;
             o.ConnectTimeout = _options.ConnectTimeout;
+            // NOTE: this method copies options field by field, so anything added
+            // to SmooFetchOptions and NOT listed here is silently dropped —
+            // WithFollowRedirects was honoured by the handler and lost in transit
+            // until a test caught it.
+            o.FollowRedirects = _options.FollowRedirects;
             o.AuthTokenProvider = _options.AuthTokenProvider;
             o.AuthScheme = _options.AuthScheme;
             o.JsonOptions = _options.JsonOptions;

dotnet/SmooAI.Fetch/SmooFetchOptions.cs

@@ -32,6 +32,22 @@ public sealed class SmooFetchOptions
     /// </summary>
     public TimeSpan? ConnectTimeout { get; set; }
 
+    /// <summary>
+    /// Whether redirects are followed automatically. Defaults to <c>true</c>, matching
+    /// <see cref="System.Net.Http.SocketsHttpHandler.AllowAutoRedirect"/>.
+    /// </summary>
+    /// <remarks>
+    /// Set <c>false</c> when a redirect must not be followed. Two cases where following one
+    /// is wrong rather than merely surprising: a caller that resolved the target hostname and
+    /// checked it against an SSRF allowlist has that guard defeated by a 302 to an internal
+    /// address, because the check was performed on the original host; and RFC 8461 forbids
+    /// fetching an MTA-STS policy through a redirect. With <c>false</c> the 3xx is returned
+    /// as an ordinary response to inspect.
+    /// Ignored when <see cref="RequireHttpClientFactory"/> is true (the factory owns the handler);
+    /// configure <c>AllowAutoRedirect</c> on the primary handler at DI registration instead.
+    /// </remarks>
+    public bool FollowRedirects { get; set; } = true;
+
     /// <summary>Optional auth token provider. When set, the returned token is added as <c>Authorization: Bearer {token}</c>.</summary>
     public AuthTokenProvider? AuthTokenProvider { get; set; }
 

go/fetch/builder.go

@@ -17,6 +17,7 @@ type ClientBuilder struct {
 	circuitBreakerOpts *CircuitBreakerOptions
 	circuitBreakerName string
 	hooks              *LifecycleHooks
+	followRedirects    *bool
 	authProvider       AuthTokenProvider
 	authScheme         string
 }
@@ -31,6 +32,28 @@ func NewClientBuilder() *ClientBuilder {
 	}
 }
 
+// WithFollowRedirects controls whether redirects are followed. Defaults to
+// true, matching net/http.
+//
+// Set false when a redirect must not be followed automatically. Two cases where
+// following one is wrong rather than merely surprising:
+//
+//   - The caller resolved the target hostname and checked it against an SSRF
+//     allowlist. A 302 to an internal address defeats that guard entirely,
+//     because the check was performed on the original host.
+//   - RFC 8461 forbids fetching an MTA-STS policy through a redirect.
+//
+// With false the 3xx is returned as an ordinary response to inspect, via
+// http.ErrUseLastResponse.
+//
+// This applies to a caller-supplied *http.Client too, unlike the connect
+// timeout — an SSRF control that silently did not apply because you brought
+// your own client would be worse than no option at all.
+func (b *ClientBuilder) WithFollowRedirects(follow bool) *ClientBuilder {
+	b.followRedirects = &follow
+	return b
+}
+
 // WithHTTPClient sets the underlying *http.Client.
 func (b *ClientBuilder) WithHTTPClient(c *http.Client) *ClientBuilder {
 	b.httpClient = c
@@ -169,15 +192,16 @@ func (b *ClientBuilder) WithContainerOptions(opts FetchContainerOptions) *Client
 // Build constructs the Client from the builder configuration.
 func (b *ClientBuilder) Build() *Client {
 	c := &Client{
-		httpClient:     b.httpClient,
-		baseHeaders:    b.baseHeaders,
-		retry:          b.retryOpts,
-		timeout:        b.timeoutOpts,
-		connectTimeout: b.connectTimeout,
-		rateLimitRetry: b.rateLimitRetryOpts,
-		hooks:          b.hooks,
-		authProvider:   b.authProvider,
-		authScheme:     b.authScheme,
+		httpClient:      b.httpClient,
+		baseHeaders:     b.baseHeaders,
+		retry:           b.retryOpts,
+		timeout:         b.timeoutOpts,
+		connectTimeout:  b.connectTimeout,
+		rateLimitRetry:  b.rateLimitRetryOpts,
+		followRedirects: b.followRedirects,
+		hooks:           b.hooks,
+		authProvider:    b.authProvider,
+		authScheme:      b.authScheme,
 	}
 	if c.authScheme == "" {
 		c.authScheme = "Bearer"
@@ -192,6 +216,15 @@ func (b *ClientBuilder) Build() *Client {
 		}
 	}
 
+	// Applied to whichever client is in use, including a caller-supplied one.
+	// This is a security control, and one that silently did not apply because
+	// the caller brought their own client would be worse than not offering it.
+	if b.followRedirects != nil && !*b.followRedirects {
+		c.httpClient.CheckRedirect = func(*http.Request, []*http.Request) error {
+			return http.ErrUseLastResponse
+		}
+	}
+
 	if c.baseHeaders == nil {
 		c.baseHeaders = make(http.Header)
 	}