diff --git a/.env.example b/.env.example index 0548468..8ba5afd 100644 --- a/.env.example +++ b/.env.example @@ -28,6 +28,13 @@ API_PORT=8080 # Belt-and-suspenders: also pin the host(s) Kestrel will answer for, so forged # Host headers are rejected outright. Overrides the appsettings.json "*" default. # AllowedHosts=apply.example.com +# +# Forwarded client/protocol headers are accepted only from a trusted proxy. +# Same-host loopback proxies need no configuration. For a proxy container or +# remote load balancer, set its exact source IP or CIDR network (not the public +# visitor addresses). Never use 0.0.0.0/0 or ::/0. +# FORWARDED_HEADERS_KNOWN_PROXY=172.17.0.1 +# FORWARDED_HEADERS_KNOWN_NETWORK=172.18.0.0/16 # Poller cadence (seconds). The poller container drains the on-demand poll queue every # DRAIN_INTERVAL and runs a full multi-tenant poll every POLL_INTERVAL. diff --git a/BACKLOG.md b/BACKLOG.md index ec7b160..242d409 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -8,7 +8,7 @@ or `SPRINTS.md` are not committed backlog until they have a corresponding issue. ## Security and stability - [x] [#51 — Close the Python poller link-check DNS rebinding gap](https://github.com/CryptoJones/OSApplyTrack/issues/51) -- [ ] [#49 — Restrict forwarded-header trust to configured proxies](https://github.com/CryptoJones/OSApplyTrack/issues/49) +- [x] [#49 — Restrict forwarded-header trust to configured proxies](https://github.com/CryptoJones/OSApplyTrack/issues/49) - [ ] [#50 — Add global JSON body caps and per-field/cardinality limits](https://github.com/CryptoJones/OSApplyTrack/issues/50) - [ ] [#52 — Serialize overlapping tenant poll runs](https://github.com/CryptoJones/OSApplyTrack/issues/52) diff --git a/README.md b/README.md index 8d83b9e..e572caf 100644 --- a/README.md +++ b/README.md @@ -192,6 +192,7 @@ All configuration is environment variables (see [`.env.example`](./.env.example) | `POLL_INTERVAL` | `3600` | Seconds between full multi-tenant polls. | | `ConnectionStrings__Postgres` | _(compose default)_ | Override to point the API at an external Postgres. | | `DATABASE_URL` | _(compose default)_ | Override to point the poller at an external Postgres (libpq URL). | +| `FORWARDED_HEADERS_KNOWN_PROXY` / `FORWARDED_HEADERS_KNOWN_NETWORK` | _(empty)_ | Source IP or CIDR of a trusted reverse proxy when it is not on loopback. | | `APPLYTRACK_DIR` | `./applications` | Default folder the `import-md` command reads when `--dir` is omitted. | | `Llm__BaseUrl` / `Llm__Model` / `Llm__ApiKey` | _(empty)_ | Instance-default cover-letter LLM — any OpenAI-compatible endpoint (a local Ollama/vLLM/LM Studio model or a hosted provider). `ApiKey` is blank for a keyless local model. Each tenant can override these in **Settings · AI**, including a reusable multi-line signature. See [Cover letters](#cover-letters). | | `APPLYTRACK_SECRETS_KEY` | _(empty)_ | Master key (AES-256-GCM) that encrypts each tenant's **own** stored LLM API key at rest. Leave unset to disable per-tenant keys — the instance default above is still used. | @@ -383,8 +384,12 @@ OSApplyTrack is built to face the public internet behind a reverse proxy: private/loopback/link-local/reserved addresses and re-checks every redirect hop, so a hostile listing URL can't pivot into your network. - **Behind HTTPS.** Front the API with a TLS-terminating reverse proxy (Caddy, - nginx, or `tailscale serve`). The API honors `X-Forwarded-Proto`, so the session - cookie's `Secure` flag is set automatically. Don't expose Kestrel directly. + nginx, or `tailscale serve`). Same-host loopback proxies are trusted by default. + For a proxy container or remote load balancer, set + `FORWARDED_HEADERS_KNOWN_PROXY` to its source IP or + `FORWARDED_HEADERS_KNOWN_NETWORK` to its CIDR. Forwarded headers from every other + address are ignored, so a direct caller cannot forge its rate-limit IP or HTTPS + state. Don't expose Kestrel directly. - **Change the default password.** For any deployment reachable beyond `localhost`, change `POSTGRES_PASSWORD` (and the matching connection string) from the bundled development default before first boot — the documented value is not a diff --git a/api/ApplyTrack.Api.Tests/ConfigurationTests.cs b/api/ApplyTrack.Api.Tests/ConfigurationTests.cs index 3b1ad20..f789aa9 100644 --- a/api/ApplyTrack.Api.Tests/ConfigurationTests.cs +++ b/api/ApplyTrack.Api.Tests/ConfigurationTests.cs @@ -3,6 +3,8 @@ using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; +using System.Net; namespace ApplyTrack.Api.Tests; @@ -56,4 +58,50 @@ public async Task App_boots_with_custom_MigrationTimeoutSeconds() var response = await client.GetAsync("/health"); Assert.Equal(System.Net.HttpStatusCode.OK, response.StatusCode); } + + [Fact] + public void Forwarded_headers_keep_safe_loopback_defaults() + { + var configuration = new ConfigurationBuilder().Build(); + var options = ForwardedHeadersConfiguration.Create(configuration); + + Assert.Equal(1, options.ForwardLimit); + Assert.NotEmpty(options.KnownProxies); + Assert.DoesNotContain(IPAddress.Any, options.KnownProxies); + Assert.DoesNotContain(IPAddress.IPv6Any, options.KnownProxies); + } + + [Fact] + public void Forwarded_headers_add_configured_proxy_and_network() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["ForwardedHeaders:KnownProxies:0"] = "192.0.2.10", + ["ForwardedHeaders:KnownNetworks:0"] = "198.51.100.0/24", + }) + .Build(); + + var options = ForwardedHeadersConfiguration.Create(configuration); + + Assert.Contains(IPAddress.Parse("192.0.2.10"), options.KnownProxies); + Assert.Contains(IPNetwork.Parse("198.51.100.0/24"), options.KnownIPNetworks); + } + + [Theory] + [InlineData("ForwardedHeaders:KnownProxies:0", "not-an-ip")] + [InlineData("ForwardedHeaders:KnownNetworks:0", "192.0.2.1/not-a-prefix")] + public void Forwarded_headers_reject_invalid_configuration(string key, string value) + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { [key] = value }) + .Build(); + + var error = Assert.Throws(() => + { + _ = ForwardedHeadersConfiguration.Create(configuration); + }); + + Assert.Contains(value, error.Message); + } } diff --git a/api/ApplyTrack.Api.Tests/RateLimitTests.cs b/api/ApplyTrack.Api.Tests/RateLimitTests.cs index c75147d..18576e2 100644 --- a/api/ApplyTrack.Api.Tests/RateLimitTests.cs +++ b/api/ApplyTrack.Api.Tests/RateLimitTests.cs @@ -4,15 +4,17 @@ using System.Net; using System.Text; using ApplyTrack.Api.Auth; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; namespace ApplyTrack.Api.Tests; /// -/// Asserts that the rate-limit partition key uses the client IP from forwarded -/// headers (via UseForwardedHeaders → RemoteIpAddress) rather than raw header -/// parsing, and that it falls back to the direct connection IP otherwise. +/// Asserts that untrusted direct requests cannot replace the rate-limit partition +/// with X-Forwarded-For, while explicitly trusted proxies retain per-client buckets. /// Each test boots a fresh factory so rate-limit state is unshared. /// [Collection(PostgresCollection.Name)] @@ -25,8 +27,7 @@ public class RateLimitTests : IAsyncLifetime public Task InitializeAsync() { - _factory = new WebApplicationFactory().WithWebHostBuilder(b => - b.UseSetting("ConnectionStrings:Postgres", _pg.ConnectionString)); + _factory = CreateFactory(trustProxy: false); return Task.CompletedTask; } @@ -34,11 +35,23 @@ public Task InitializeAsync() private static StringContent Json(string body) => new(body, Encoding.UTF8, "application/json"); + private WebApplicationFactory CreateFactory(bool trustProxy) => + new WebApplicationFactory().WithWebHostBuilder(b => + { + b.UseSetting("ConnectionStrings:Postgres", _pg.ConnectionString); + if (trustProxy) + b.UseSetting("ForwardedHeaders:KnownProxies:0", "192.0.2.10"); + b.ConfigureTestServices(services => + services.AddSingleton( + new RemoteIpStartupFilter(IPAddress.Parse("192.0.2.10")))); + }); + /// Seeds a fresh session and returns an authenticated client. - private async Task AuthenticatedClient() + private async Task AuthenticatedClient( + WebApplicationFactory? factory = null) { var (_, sid) = await TestAuth.SeedSessionAsync(_pg.ConnectionString); - var client = _factory.CreateClient(); + var client = (factory ?? _factory).CreateClient(); client.DefaultRequestHeaders.Add("Cookie", $"{AuthCookie.Name}={sid}"); return client; } @@ -91,11 +104,8 @@ public async Task Rate_limit_without_forwarded_header_uses_remote_ip() } [Fact] - public async Task Distinct_forwarded_ips_get_independent_rate_limit_buckets() + public async Task Distinct_spoofed_forwarded_ips_share_the_direct_connection_bucket() { - // The point of the PR: the partition key is the *forwarded* client IP, so - // exhausting one client's budget must not throttle a different client behind - // the same proxy. Same authenticated session, two different X-Forwarded-For. var client = await AuthenticatedClient(); client.DefaultRequestHeaders.Add("X-Forwarded-For", "203.0.113.1"); @@ -107,11 +117,44 @@ public async Task Distinct_forwarded_ips_get_independent_rate_limit_buckets() var exhausted = await client.PostAsync("/api/poll", Json("{}")); Assert.Equal(HttpStatusCode.TooManyRequests, exhausted.StatusCode); - // A request forwarded from a different client IP lands in a separate bucket - // and is still allowed — proving partitioning is per forwarded IP, not global. + client.DefaultRequestHeaders.Remove("X-Forwarded-For"); + client.DefaultRequestHeaders.Add("X-Forwarded-For", "198.51.100.2"); + var other = await client.PostAsync("/api/poll", Json("{}")); + Assert.Equal(HttpStatusCode.TooManyRequests, other.StatusCode); + } + + [Fact] + public async Task Distinct_forwarded_ips_from_a_configured_proxy_get_independent_buckets() + { + await using var trustedFactory = CreateFactory(trustProxy: true); + var client = await AuthenticatedClient(trustedFactory); + + client.DefaultRequestHeaders.Add("X-Forwarded-For", "203.0.113.1"); + for (var i = 0; i < 15; i++) + { + var res = await client.PostAsync("/api/poll", Json("{}")); + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + } + var exhausted = await client.PostAsync("/api/poll", Json("{}")); + Assert.Equal(HttpStatusCode.TooManyRequests, exhausted.StatusCode); + client.DefaultRequestHeaders.Remove("X-Forwarded-For"); client.DefaultRequestHeaders.Add("X-Forwarded-For", "198.51.100.2"); var other = await client.PostAsync("/api/poll", Json("{}")); Assert.Equal(HttpStatusCode.OK, other.StatusCode); } + + private sealed class RemoteIpStartupFilter(IPAddress remoteIp) : IStartupFilter + { + public Action Configure(Action next) => + app => + { + app.Use(async (context, nextMiddleware) => + { + context.Connection.RemoteIpAddress = remoteIp; + await nextMiddleware(); + }); + next(app); + }; + } } diff --git a/api/ApplyTrack.Api/ApplyTrack.Api.csproj b/api/ApplyTrack.Api/ApplyTrack.Api.csproj index 2ab5976..2c7c15a 100644 --- a/api/ApplyTrack.Api/ApplyTrack.Api.csproj +++ b/api/ApplyTrack.Api/ApplyTrack.Api.csproj @@ -5,7 +5,7 @@ enable enable ApplyTrack.Api - 1.11.2 + 1.11.3 Aaron K. Clark Copyright 2026 Aaron K. Clark Apache-2.0 diff --git a/api/ApplyTrack.Api/ForwardedHeadersConfiguration.cs b/api/ApplyTrack.Api/ForwardedHeadersConfiguration.cs new file mode 100644 index 0000000..87ad10a --- /dev/null +++ b/api/ApplyTrack.Api/ForwardedHeadersConfiguration.cs @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Aaron K. Clark + +using System.Net; +using Microsoft.AspNetCore.HttpOverrides; + +namespace ApplyTrack.Api; + +/// +/// Builds a fail-closed forwarded-header policy. ASP.NET Core's loopback defaults +/// remain trusted for same-host proxies; every other proxy or network must be named +/// explicitly in configuration. +/// +public static class ForwardedHeadersConfiguration +{ + public static ForwardedHeadersOptions Create(IConfiguration configuration) + { + var options = new ForwardedHeadersOptions + { + ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto, + ForwardLimit = 1, + }; + + foreach (var value in Values(configuration, "ForwardedHeaders:KnownProxies")) + { + if (!IPAddress.TryParse(value, out var address)) + throw new InvalidOperationException( + $"ForwardedHeaders:KnownProxies contains invalid IP address '{value}'."); + options.KnownProxies.Add(address); + } + + foreach (var value in Values(configuration, "ForwardedHeaders:KnownNetworks")) + { + if (!System.Net.IPNetwork.TryParse(value, out var network)) + throw new InvalidOperationException( + $"ForwardedHeaders:KnownNetworks contains invalid CIDR network '{value}'."); + options.KnownIPNetworks.Add(network); + } + + return options; + } + + private static IEnumerable Values(IConfiguration configuration, string section) => + configuration.GetSection(section).GetChildren() + .Select(item => item.Value?.Trim()) + .Where(value => !string.IsNullOrWhiteSpace(value)) + .Select(value => value!); +} diff --git a/api/ApplyTrack.Api/Program.cs b/api/ApplyTrack.Api/Program.cs index bafb790..d578c9c 100644 --- a/api/ApplyTrack.Api/Program.cs +++ b/api/ApplyTrack.Api/Program.cs @@ -144,18 +144,12 @@ static string ClientPartition(HttpContext ctx) => builder.Configuration["MigrationTimeoutSeconds"], defaultValue: 60)); Migrator.Upgrade(connectionString, migrationTimeout); -// Behind a TLS-terminating reverse proxy (Caddy/nginx/`tailscale serve` — the usual -// self-host front), honor X-Forwarded-Proto so Request.IsHttps is true and the session -// cookie keeps its Secure flag. The app is meant to sit behind that proxy, so forwarded -// headers are accepted from any hop — don't expose Kestrel directly to the internet. -// Plain-HTTP local/dev sends no such header, so this is a no-op there. -var forwarded = new ForwardedHeadersOptions -{ - ForwardedHeaders = ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedFor, -}; -forwarded.KnownIPNetworks.Clear(); -forwarded.KnownProxies.Clear(); -app.UseForwardedHeaders(forwarded); +// Honor forwarded client/protocol data only from a known reverse proxy. ASP.NET +// Core's loopback defaults cover same-host Caddy/nginx/`tailscale serve`; container +// gateways and remote proxies must be named under ForwardedHeaders:KnownProxies or +// KnownNetworks. An untrusted direct caller's headers are ignored, so they cannot +// forge Request.IsHttps or rotate the per-IP rate-limit partition. +app.UseForwardedHeaders(ForwardedHeadersConfiguration.Create(builder.Configuration)); // Enforce request-body size limits on the two endpoints that accept large uploads, // checked on Content-Length before the body is read, so it works under TestServer too. diff --git a/api/ApplyTrack.Api/appsettings.json b/api/ApplyTrack.Api/appsettings.json index 0573245..ba80158 100644 --- a/api/ApplyTrack.Api/appsettings.json +++ b/api/ApplyTrack.Api/appsettings.json @@ -6,6 +6,10 @@ } }, "AllowedHosts": "*", + "ForwardedHeaders": { + "KnownProxies": [], + "KnownNetworks": [] + }, "ConnectionStrings": { "Postgres": "Host=localhost;Port=5432;Database=applytrack;Username=applytrack;Password=JanewayDidNothingWrong" } diff --git a/docker-compose.yml b/docker-compose.yml index eec2bb9..ed44a7b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -45,6 +45,10 @@ services: Email__Password: ${Email__Password:-} Email__From: ${Email__From:-} Email__FromName: ${Email__FromName:-OSApplyTrack} + # Same-host proxies are trusted by default. Set either value only when the + # proxy reaches this container from a different address or bridge network. + ForwardedHeaders__KnownProxies__0: ${FORWARDED_HEADERS_KNOWN_PROXY:-} + ForwardedHeaders__KnownNetworks__0: ${FORWARDED_HEADERS_KNOWN_NETWORK:-} # 8080 is contested (vLLM, llama.cpp, and half of all dev tools default to it). # If it's already taken, Docker fails the bind loudly ("address already in use") # rather than silently winning — relocate by setting API_PORT in .env. diff --git a/pyproject.toml b/pyproject.toml index d0db01b..673822c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "applytrack-poller" -version = "1.11.2" +version = "1.11.3" description = "Discovery poller for OSApplyTrack — fetches and scores remote job leads into shared Postgres." requires-python = ">=3.10" license = { text = "Apache-2.0" } diff --git a/uv.lock b/uv.lock index 9e93a55..b4000b0 100644 --- a/uv.lock +++ b/uv.lock @@ -23,7 +23,7 @@ wheels = [ [[package]] name = "applytrack-poller" -version = "1.11.2" +version = "1.11.3" source = { editable = "." } dependencies = [ { name = "defusedxml" },