Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
046b17b
fix(web): honor X-Forwarded-* so the real client IP reaches the pipeline
marcelo-maciel Jul 11, 2026
f537e91
Merge branch 'main' into fix/web-forwarded-headers
marcelo-maciel Jul 13, 2026
75475d3
fix(web): bind forwarded-headers trust to configured proxies
marcelo-maciel Jul 13, 2026
85aa03b
fix(web): name the offending setting when trusted-proxy config is mal…
marcelo-maciel Aug 14, 2026
171abea
Merge branch 'main' into fix/web-forwarded-headers
marcelo-maciel Aug 14, 2026
e7dbe6b
build: pin SSH.NET to 2026.0.0 so restore passes while #1333 is open
marcelo-maciel Aug 17, 2026
5def7d9
fix(web): reject a trusted-proxy ForwardLimit below 1 at startup
marcelo-maciel Aug 17, 2026
cb62b10
docs(web): record why X-Forwarded-Host stays out of the flag list
marcelo-maciel Aug 17, 2026
8e0d7de
build(deps): bump Testcontainers to 4.14.0 and SourceLink past their …
marcelo-maciel Sep 14, 2026
6816b7d
fix(infra): pull MinIO from quay.io on a pinned tag, not Docker Hub
marcelo-maciel Sep 14, 2026
6f0b8f8
fix(web): rebuild the forwarded-headers trust list instead of appendi…
marcelo-maciel Sep 17, 2026
e1ab57b
fix(infra): pull minio/mc from quay.io too, not just minio/minio
marcelo-maciel Sep 18, 2026
99e5514
build(deps): drop the dead SSH.NET pin
marcelo-maciel Sep 18, 2026
71000c3
docs(web): say what a ForwardLimit above the real hop count costs
marcelo-maciel Sep 18, 2026
00ec8e8
test(security): read the session this request created, not the newest…
marcelo-maciel Sep 18, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions deploy/docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ This brings up the full stack on a single host:
| `admin` | `fsh/admin:local` | `FSH_ADMIN_PORT` (default 8081) | Operator console (nginx + React) |
| `dashboard` | `fsh/dashboard:local` | `FSH_DASHBOARD_PORT` (default 8082) | Tenant dashboard (nginx + React) |
| `migrator` | `fsh/dbmigrator:local` | — | One-shot: applies EF migrations + seeds the root tenant + creates the default admin user |
| `postgres` | `postgres:17-alpine` | (internal) | Identity, tenant catalog, module schemas |
| `redis` | `redis:7-alpine` | (internal) | HybridCache L2, Data Protection keys, idempotency store |
| `minio` | `minio/minio:latest` | (internal) | S3-compatible blob store for the Files module |
| `postgres` | `postgres:18-alpine` | (internal) | Identity, tenant catalog, module schemas |
| `redis` | `valkey/valkey:9.1.0-alpine` | (internal) | HybridCache L2, Data Protection keys, idempotency store |
| `minio` | `quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z` | (internal) | S3-compatible blob store for the Files module |

The compose file does **not** include a reverse proxy or TLS terminator. You bring your own edge — Cloudflare Tunnel, AWS ALB, Tailscale Funnel, your existing nginx, anything that can route a TLS subdomain to a host:port on this machine.

Expand Down
6 changes: 4 additions & 2 deletions deploy/docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ services:
# - "6379:6379"

minio:
image: minio/minio:latest
# quay.io: minio/minio is gone from Docker Hub. Tag pinned; quay stopped moving :latest.
image: quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z
container_name: fsh-minio
restart: unless-stopped
command: ["server", "/data", "--console-address", ":9001"]
Expand All @@ -79,7 +80,8 @@ services:
# policy is set — objects are served via the API / presigned URLs, not a
# public bucket.
minio-init:
image: minio/mc:latest
# quay.io: minio/mc is gone from Docker Hub too. Tag pinned; quay stopped moving :latest.
image: quay.io/minio/mc:RELEASE.2025-08-13T08-35-41Z
container_name: fsh-minio-init
restart: "no"
depends_on:
Expand Down
74 changes: 74 additions & 0 deletions src/BuildingBlocks/Web/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@
using FSH.Framework.Web.RateLimiting;
using FSH.Framework.Web.Realtime;
using FSH.Framework.Web.Security;
using FSH.Framework.Web.TrustedProxy;
using FSH.Framework.Web.Versioning;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.HttpOverrides;
using Microsoft.AspNetCore.ResponseCompression;
using Microsoft.Extensions.Caching.Distributed;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Hosting;
using Mediator;
using System.Net;

namespace FSH.Framework.Web;

Expand Down Expand Up @@ -63,6 +66,72 @@ public static IHostApplicationBuilder AddHeroPlatform(this IHostApplicationBuild
}

builder.Services.AddHttpContextAccessor();

// The app runs behind a reverse proxy (e.g. cloudflared → Caddy → app), so the real client IP
// and scheme arrive via X-Forwarded-*. Without this, RemoteIpAddress is the proxy's container
// IP, which collapses the rate-limit partition into one bucket and records useless audit IPs.
// Trust is bound to the configured ingress CIDRs/proxies (see TrustedProxyOptions): forwarded
// headers from any other source are ignored, so a client reaching the app directly cannot forge
// its IP/scheme. With nothing configured, the framework default (loopback only) stands.
var trustedProxy = builder.Configuration
.GetSection(nameof(TrustedProxyOptions)).Get<TrustedProxyOptions>() ?? new TrustedProxyOptions();
builder.Services.Configure<ForwardedHeadersOptions>(forwarded =>
{
// A hop count below 1 is never what an operator means, and neither bad value announces itself:
// 0 truncates the unwind loop to zero iterations, so forwarded headers stop being processed with
// no error, while a negative value overflows the middleware's buffer allocation and 500s every
// request - including requests carrying no forwarded headers at all. Fail the boot instead.
if (trustedProxy.ForwardLimit < 1)
{
throw new InvalidOperationException(
$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.ForwardLimit)} is {trustedProxy.ForwardLimit}, which is not a valid proxy hop count: it must be at least 1 (one hop per proxy in front of the app).");
}

forwarded.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto;
forwarded.ForwardLimit = trustedProxy.ForwardLimit;

// The trust list is always rebuilt from scratch, never appended to. Whatever is in the
// options when this runs depends on who configured them first, and with
// ASPNETCORE_FORWARDEDHEADERS_ENABLED=true that is ForwardedHeadersOptionsSetup, which
// empties both lists. An empty list is not "trust nobody" in ForwardedHeadersMiddleware:
// it only validates the peer when at least one entry exists, so empty means the app
// rewrites RemoteIpAddress from an X-Forwarded-For sent by anyone at all.
forwarded.KnownProxies.Clear();
forwarded.KnownIPNetworks.Clear();

if (trustedProxy.KnownProxies.Length == 0 && trustedProxy.KnownNetworks.Length == 0)
{
// Nothing configured: restate the framework's own default rather than inherit it,
// for the same reason. Local development runs behind Kestrel on loopback and still
// needs its forwarded headers honoured.
forwarded.KnownProxies.Add(IPAddress.IPv6Loopback);
forwarded.KnownIPNetworks.Add(new System.Net.IPNetwork(IPAddress.Loopback, 8));
return;
}

foreach (var proxy in trustedProxy.KnownProxies)
{
if (!IPAddress.TryParse(proxy, out var address))
{
throw new InvalidOperationException(
$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.KnownProxies)} contains \"{proxy}\", which is not a valid IP address (for example \"10.0.0.5\").");
}

forwarded.KnownProxies.Add(address);
}

foreach (var network in trustedProxy.KnownNetworks)
{
if (!System.Net.IPNetwork.TryParse(network, out var parsedNetwork))
{
throw new InvalidOperationException(
$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.KnownNetworks)} contains \"{network}\", which is not a valid CIDR network (for example \"10.0.0.0/8\").");
}

forwarded.KnownIPNetworks.Add(parsedNetwork);
}
});

builder.Services.AddHeroDatabaseOptions(builder.Configuration);
builder.Services.AddHeroRateLimiting(builder.Configuration);

Expand Down Expand Up @@ -150,6 +219,11 @@ public static WebApplication UseHeroPlatform(this WebApplication app, Action<Fsh
var openApiEnabled = options.UseOpenApi && IsOpenApiEnabled(app.Configuration);

app.UseExceptionHandler();

// Apply forwarded headers before anything reads the client IP or scheme (HTTPS redirect,
// rate limiting, auth, audit) so they all see the real client, not the reverse proxy.
app.UseForwardedHeaders();

app.UseResponseCompression();

// CORS MUST run before UseHttpsRedirection: preflight OPTIONS can't follow an HTTP→HTTPS redirect, so
Expand Down
42 changes: 42 additions & 0 deletions src/BuildingBlocks/Web/TrustedProxy/TrustedProxyOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
namespace FSH.Framework.Web.TrustedProxy;

/// <summary>
/// Trusted reverse-proxy configuration for X-Forwarded-* processing. Behind an ingress
/// (e.g. cloudflared → Caddy → app) the real client IP and scheme arrive via forwarded headers;
/// these settings bound which upstream sources are trusted so a client reaching the app from
/// outside the proxy network cannot forge its own IP/scheme. When no proxies or networks are
/// configured, the framework default (loopback only) stands and forwarded headers from any other
/// source are ignored.
/// <para>
/// Only X-Forwarded-For and X-Forwarded-Proto are honoured. X-Forwarded-Host is deliberately left
/// out: rewriting Request.Host from a header is a host-header injection primitive, and the endpoints
/// that build a public URL from the request (user registration and confirmation e-mails) would then
/// send links pointing wherever the header said. The trade-off is that Request.Host keeps the
/// internal host behind a proxy, and those links carry it.
/// </para>
/// </summary>
public sealed class TrustedProxyOptions
{
/// <summary>Individual upstream proxy IP addresses whose X-Forwarded-* headers are trusted.</summary>
public string[] KnownProxies { get; init; } = [];

/// <summary>Trusted upstream networks in CIDR notation (e.g. "10.0.0.0/8", "172.16.0.0/12").</summary>
public string[] KnownNetworks { get; init; } = [];

/// <summary>
/// Number of proxy hops to unwind from X-Forwarded-For. Must match the real ingress hop count
/// (cloudflared → Caddy → app is 2). The framework default of 1 reads only the rightmost hop,
/// which yields the nearest proxy's IP (or an attacker-injected value) in a multi-hop topology.
/// Must be at least 1: anything lower is rejected at startup, since 0 would silently stop
/// forwarded-header processing and a negative value would fail every request.
/// <para>
/// Setting it higher than the real hop count is what turns this into a vulnerability: the
/// middleware trusts one entry per hop, counting from the right, and only the peer itself is
/// checked against the trust list. A limit of 2 with a single proxy in front means the value the
/// proxy appended is discarded in favour of the one the client sent, so the caller picks its own
/// RemoteIpAddress and every IP-based rate limit and audit entry follows it. Count the proxies
/// that actually rewrite the header, not the ones in the diagram.
/// </para>
/// </summary>
public int ForwardLimit { get; init; } = 1;
}
10 changes: 6 additions & 4 deletions src/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
<!-- Pack default lives in Directory.Build.props (false; source-ownership model). -->
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
<!-- 10.x: the 8.x line pulls Microsoft.Build.Tasks.Git 8.0.0, unpatched for NU1902. -->
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="10.0.401" />
</ItemGroup>
<ItemGroup Label="Aspire">
<PackageVersion Include="Aspire.Hosting.JavaScript" Version="13.4.0" />
Expand Down Expand Up @@ -122,9 +123,10 @@
<PackageVersion Include="xunit" Version="2.9.3" />
<PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" />
<PackageVersion Include="Microsoft.AspNetCore.Mvc.Testing" Version="10.0.8" />
<PackageVersion Include="Testcontainers.PostgreSql" Version="4.11.0" />
<PackageVersion Include="Testcontainers.Redis" Version="4.11.0" />
<PackageVersion Include="Testcontainers.Minio" Version="4.11.0" />
<!-- 4.14.0+: earlier versions pull SSH.NET 2025.1.0, NU1903. -->
<PackageVersion Include="Testcontainers.PostgreSql" Version="4.14.0" />
<PackageVersion Include="Testcontainers.Redis" Version="4.14.0" />
<PackageVersion Include="Testcontainers.Minio" Version="4.14.0" />
</ItemGroup>
<ItemGroup Label="AWS">
<PackageVersion Include="AWSSDK.S3" Version="4.0.23.4" />
Expand Down
5 changes: 5 additions & 0 deletions src/Host/FSH.Starter.Api/appsettings.Production.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@
"Ip": { "PermitLimit": 300, "WindowSeconds": 60, "QueueLimit": 0 },
"Auth": { "PermitLimit": 10, "WindowSeconds": 60, "QueueLimit": 0 }
},
"TrustedProxyOptions": {
"KnownProxies": [],
"KnownNetworks": [],
"ForwardLimit": 1
},
"Storage": {
"Provider": "local"
}
Expand Down
5 changes: 5 additions & 0 deletions src/Host/FSH.Starter.Api/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@
"QueueLimit": 0
}
},
"TrustedProxyOptions": {
"KnownProxies": [],
"KnownNetworks": [],
"ForwardLimit": 1
},
"Storage": {
"Provider": "local"
},
Expand Down
5 changes: 5 additions & 0 deletions src/Host/FSH.Starter.AppHost/AppHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@
var minioUser = builder.AddParameter("minio-user", "minioadmin");
var minioPassword = builder.AddParameter("minio-password", "minioadmin", secret: true);

// quay.io: minio/minio is gone from Docker Hub. Tag pinned; quay stopped moving :latest.
var minio = builder.AddContainer("minio", "minio/minio")
.WithImageRegistry("quay.io")
.WithImageTag("RELEASE.2025-09-07T16-13-09Z")
.WithArgs("server", "/data", "--console-address", ":9001")
.WithHttpEndpoint(port: 9000, targetPort: 9000, name: "api")
.WithHttpEndpoint(port: 9001, targetPort: 9001, name: "console")
Expand All @@ -73,6 +76,8 @@
""").ReplaceLineEndings("\n");

var minioInit = builder.AddContainer("minio-init", "minio/mc")
.WithImageRegistry("quay.io")
.WithImageTag("RELEASE.2025-08-13T08-35-41Z")
.WithEntrypoint("/bin/sh")
.WithArgs("-c", minioInitScript)
.WithEnvironment("MC_USER", minioUser)
Expand Down
64 changes: 64 additions & 0 deletions src/Tests/Framework.Tests/Web/ForwardedHeadersHostDefaultsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using FSH.Framework.Web;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Shouldly;
using Xunit;

namespace Framework.Tests.Web;

/// <summary>
/// The sibling of <see cref="TrustedProxyOptionsBindingTests"/> for the one host shape that class cannot
/// reach. It builds through <c>Host.CreateApplicationBuilder</c>, which never runs
/// ConfigureWebDefaults, so the framework's loopback defaults are always still in place when
/// AddHeroPlatform looks at them. A web host started with FORWARDEDHEADERS_ENABLED registers
/// ForwardedHeadersOptionsSetup, which empties both trust lists — and an empty trust list is not
/// "trust nobody" in ForwardedHeadersMiddleware, it is "check nobody": the middleware only validates
/// the peer when at least one entry exists. That is the configuration this pins.
/// </summary>
public sealed class ForwardedHeadersHostDefaultsTests
{
private static ForwardedHeadersOptions ResolveWithAspNetForwarding()
{
// Passed as a command-line arg rather than an environment variable: host configuration reads
// both, and an env var would leak into every other test running in this process.
var builder = WebApplication.CreateBuilder(new WebApplicationOptions
{
Args = ["--FORWARDEDHEADERS_ENABLED=true"],
EnvironmentName = "Development",
});
builder.AddHeroPlatform();

// The premise of this whole test: ASP.NET registered its own setup for these options. If the
// flag ever stops reaching host configuration, the assertions below would pass for the wrong
// reason — nothing cleared the lists, so nothing had to restore them.
builder.Services.Any(d =>
d.ServiceType == typeof(IConfigureOptions<ForwardedHeadersOptions>) &&
d.ImplementationType?.Name == "ForwardedHeadersOptionsSetup")
.ShouldBeTrue("FORWARDEDHEADERS_ENABLED did not reach host configuration");

// Not builder.Build(): the host validates the whole container, and the modules that supply
// ICurrentUser and friends are not registered here. Only the options matter.
using var provider = builder.Services.BuildServiceProvider();
return provider.GetRequiredService<IOptions<ForwardedHeadersOptions>>().Value;
}

[Fact]
public void ForwardedHeaders_Should_TrustSomeone_When_NothingConfiguredAndAspNetForwardingEnabled()
{
// Act
var options = ResolveWithAspNetForwarding();

// Assert — with both lists empty the middleware skips the peer check entirely and rewrites
// RemoteIpAddress from X-Forwarded-For sent by anyone at all.
(options.KnownProxies.Count + options.KnownIPNetworks.Count).ShouldBeGreaterThan(
0,
"an empty trust list makes ForwardedHeadersMiddleware accept X-Forwarded-For from any peer");

// And what is restored is the framework's own default, not a trust policy of our own
// invention: with nothing configured the app must trust exactly loopback, no wider.
var frameworkDefaults = new ForwardedHeadersOptions();
options.KnownProxies.ShouldBe(frameworkDefaults.KnownProxies);
options.KnownIPNetworks.ShouldBe(frameworkDefaults.KnownIPNetworks);
}
}
Loading
Loading