From 1a181e9ebaf0be11c9d4ba46b8f7feeba4266052 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Thu, 8 Oct 2026 10:57:54 +0800 Subject: [PATCH] Refuse non-public destinations in http.request http.request sent through the default HttpClient: no address check and auto-followed redirects. Its URL is routinely built from trigger text or an upstream model's output, and any team Member can author one, so a workflow could read the worker's loopback services and cloud metadata (every spelling: localhost, 2130706433, 0x7f000001, 127.1, [::1], [::ffff:127.0.0.1]) and return the response as node outputs. A 302 from an allowed host to loopback was followed, and the author's X-Api-Key, PRIVATE-TOKEN and Cookie headers went along to the new origin, since the built-in follower strips only Authorization. The node now uses a named client whose ConnectCallback resolves the host and dials only public addresses, or ones an operator committed to OutboundHttp:AllowedInternalDestinations (hosts or CIDRs). Checking at connect time covers every address spelling, every redirect hop and DNS rebinding in one place, because the resolution checked is the one dialled. IPv4 reuses the sandbox egress predicate; IPv6 is global unicast only, and v4-mapped, v4-compatible, NAT64 and 6to4 forms are judged by the IPv4 address they carry. The worker's HTTP(S)_PROXY is still honoured, as the default client did, so a worker that reaches the internet only through a proxy keeps working. A connection to the proxy is dialled as operator infrastructure; a request the proxy carries has its host resolved and checked on the worker before it is sent. The proxy resolves the name again, a rebinding window only the proxy's own egress rules can close. The handler keeps no cookie jar: the factory shares one primary handler across every run and team, so a jar sent one run's session on another's requests. An author-written Host header is dropped, because Host and the SNI derived from it pick the virtual host behind an admitted name. Redirects are followed by a handler, at most 10 hops (the built-in allows 50) and never https->http. A hop that changes scheme, host or port drops every request header, a same-host http->https move included; the author keeps them by writing the https URL. A redirect that would carry the body to another origin (307/308, or 300/301/302 of anything but a POST) is returned as the node's 3xx, since the body holds the same secrets the headers do. The allowlist is parsed while the host is built, so a malformed entry stops the host instead of failing every http.request call at run time. A refusal is an IFailure (outbound_destination_refused) and fails the node as non-retryable. http.request is the only workflow-controlled outbound HTTP: git.* nodes call the bound provider, llm.complete the credential's base URL, pack import already allowlists https hosts with redirects off, and chat.post_message makes no outbound call. --- backend/src/CodeSpace.Api/Startup.cs | 6 + backend/src/CodeSpace.Api/appsettings.json | 4 + .../OutboundHttp/GuardedDestinationHandler.cs | 47 +++ .../GuardedHttpClientRegistration.cs | 48 +++ .../OriginScopedRedirectHandler.cs | 78 +++++ .../OutboundDestinationAllowlist.cs | 78 +++++ .../OutboundHttp/OutboundDestinationGuard.cs | 100 ++++++ .../OutboundDestinationRefusedException.cs | 23 ++ .../Services/OutboundHttp/PublicAddress.cs | 55 +++ .../Nodes/Builtin/HttpRequestNode.cs | 15 +- .../OutboundHttpAllowlistSetting.cs | 26 ++ .../Failures/FailureCodes.cs | 3 + .../HttpRequestClientHostE2ETests.cs | 63 ++++ .../Infrastructure/PostgresFixture.cs | 4 + .../HttpRequestDestinationFlowTests.cs | 210 ++++++++++++ .../Infrastructure/LoopbackResponder.cs | 128 +++++++ .../appsettings.json | 4 + .../Architecture/FailureTaxonomyTests.cs | 1 + .../OutboundHttp/GuardedHttpClientTests.cs | 324 ++++++++++++++++++ .../OutboundHttp/LoopbackResponder.cs | 137 ++++++++ .../OriginScopedRedirectHandlerTests.cs | 169 +++++++++ .../OutboundDestinationAllowlistTests.cs | 98 ++++++ .../OutboundDestinationGuardTests.cs | 77 +++++ .../OutboundHttp/PublicAddressTests.cs | 63 ++++ 24 files changed, 1760 insertions(+), 1 deletion(-) create mode 100644 backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedDestinationHandler.cs create mode 100644 backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedHttpClientRegistration.cs create mode 100644 backend/src/CodeSpace.Core/Services/OutboundHttp/OriginScopedRedirectHandler.cs create mode 100644 backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationAllowlist.cs create mode 100644 backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationGuard.cs create mode 100644 backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationRefusedException.cs create mode 100644 backend/src/CodeSpace.Core/Services/OutboundHttp/PublicAddress.cs create mode 100644 backend/src/CodeSpace.Core/Settings/OutboundHttp/OutboundHttpAllowlistSetting.cs create mode 100644 backend/tests/CodeSpace.E2ETests/Workflows/HttpRequestClientHostE2ETests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/HttpRequestDestinationFlowTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/LoopbackResponder.cs create mode 100644 backend/tests/CodeSpace.UnitTests/OutboundHttp/GuardedHttpClientTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/OutboundHttp/LoopbackResponder.cs create mode 100644 backend/tests/CodeSpace.UnitTests/OutboundHttp/OriginScopedRedirectHandlerTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationAllowlistTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationGuardTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/OutboundHttp/PublicAddressTests.cs diff --git a/backend/src/CodeSpace.Api/Startup.cs b/backend/src/CodeSpace.Api/Startup.cs index 0b8de4926..a1c3feecc 100644 --- a/backend/src/CodeSpace.Api/Startup.cs +++ b/backend/src/CodeSpace.Api/Startup.cs @@ -5,6 +5,7 @@ using CodeSpace.Core.Services.Workflows.Llm; using CodeSpace.Api.Filters; using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.OutboundHttp; using CodeSpace.Core.Services.Identity; using CodeSpace.Core.Settings; using Microsoft.AspNetCore.Diagnostics.HealthChecks; @@ -70,6 +71,11 @@ public void ConfigureServices(IServiceCollection services) // generation, no connection-lifetime tuning, no transient resilience. Register the hardened, resilient named // clients (generous tunable budget + SocketsHttpHandler + retry/Retry-After) — shared with the resilience test. services.AddLlmHttpClients(); + // http.request sends to a URL that trigger text or an upstream model can write, so its client reaches only + // public addresses or ones committed to OutboundHttp:AllowedInternalDestinations, checked on every redirect hop, + // and a cross-origin redirect carries none of the author's headers. The allowlist is parsed here, at boot, so a + // malformed entry stops the host instead of failing every http.request call. + services.AddGuardedHttpClient(nameof(CodeSpace.Core.Services.Workflows.Nodes.Builtin.HttpRequestNode), Configuration); // Periodic background work is uniformly IRecurringJob (Rule 14) — there is deliberately no AddHostedService // here. A hosted service runs its own timer on EVERY pod including the public API, outside the mediator // pipeline and invisible to the dashboard; a recurring job runs only where a Hangfire server does (the Worker diff --git a/backend/src/CodeSpace.Api/appsettings.json b/backend/src/CodeSpace.Api/appsettings.json index 2f8420aad..866bc98fb 100644 --- a/backend/src/CodeSpace.Api/appsettings.json +++ b/backend/src/CodeSpace.Api/appsettings.json @@ -63,6 +63,10 @@ "StoreDirectory": "", "LocalRwxShared": false }, + "OutboundHttp": { + "_doc": "AllowedInternalDestinations lists the INTERNAL destinations the http.request workflow node may reach: host names (exact match, no subdomains), IP addresses or CIDR ranges, e.g. [\"internal-api.corp\", \"10.20.0.0/16\"]. Everything else that is loopback, private, link-local, CGNAT, unique-local or a cloud metadata address is refused, on every redirect hop, however the address is spelled. Empty by default: a workflow URL can be written by trigger text or a model, so nothing on the worker's own network is reachable until an operator commits it here. Read once at boot; a malformed entry stops the host from starting. The node honours the worker's HTTP(S)_PROXY / NO_PROXY: a direct connection is checked as it is made, a proxied request by resolving its host on the worker before it is sent. The proxy resolves that name again, so a name whose DNS answer changes in between can still reach whatever the proxy can: keep the proxy itself from reaching internal addresses.", + "AllowedInternalDestinations": [] + }, "Shutdown": { "_doc": "Graceful-shutdown drain budget in seconds. The orchestrator's grace period MUST be at least this or the process is SIGKILLed before in-flight background work drains (k8s: terminationGracePeriodSeconds). 30 matches k8s's own default.", "DrainSeconds": 30 diff --git a/backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedDestinationHandler.cs b/backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedDestinationHandler.cs new file mode 100644 index 000000000..7c2cfb41e --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedDestinationHandler.cs @@ -0,0 +1,47 @@ +using System.Net.Sockets; + +namespace CodeSpace.Core.Services.OutboundHttp; + +/// +/// The per-request half of the destination guard (the connect-time half is ), +/// run on every hop, redirects included. +/// +/// A request is addressed to the authority in its URL, the one the guard checks: an author-written Host +/// header is dropped, because Host — and the TLS SNI the handler derives from it — is what picks the virtual host +/// behind an address, so an operator who admits one name on a shared ingress would otherwise have admitted every other +/// name behind it. Browsers' fetch treats Host as a forbidden request header the same way. +/// +/// A request the operator's proxy will carry has its destination checked here, before it is sent: the socket the +/// handler opens goes to the proxy, so the connect-time check never sees where the request is going. A refusal has the +/// shape a connect-time one has — an whose inner exception is the +/// — so callers handle the two alike. +/// +public sealed class GuardedDestinationHandler : DelegatingHandler +{ + private readonly OutboundDestinationGuard _guard; + + public GuardedDestinationHandler(OutboundDestinationGuard guard) { _guard = guard; } + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + request.Headers.Host = null; + + await PermitProxiedDestinationAsync(request.RequestUri!, cancellationToken).ConfigureAwait(false); + + return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + } + + private async Task PermitProxiedDestinationAsync(Uri destination, CancellationToken cancellationToken) + { + if (_guard.ProxyFor(destination) == null) return; + + try + { + await _guard.PermitAsync(destination.IdnHost, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is OutboundDestinationRefusedException or SocketException) + { + throw new HttpRequestException($"{ex.Message} ({destination.IdnHost}:{destination.Port})", ex); + } + } +} diff --git a/backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedHttpClientRegistration.cs b/backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedHttpClientRegistration.cs new file mode 100644 index 000000000..aa828b833 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OutboundHttp/GuardedHttpClientRegistration.cs @@ -0,0 +1,48 @@ +using CodeSpace.Core.Settings.OutboundHttp; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; + +namespace CodeSpace.Core.Services.OutboundHttp; + +/// +/// Registers a named HttpClient whose every connection goes through , whose every +/// request goes through , and whose redirects are followed by +/// — the client for outbound HTTP whose URL a workflow author, a trigger +/// payload or an upstream model can write. Extracted so the host (Startup), the integration fixture and the tests all +/// exercise this one registration rather than re-deriving it. +/// +public static class GuardedHttpClientRegistration +{ + /// + /// The host's registration: the operator's committed allowlist, parsed HERE — once, while the host is being built — + /// so a malformed entry stops the host from starting rather than failing every http.request call at run time; and + /// the process's proxy (: HTTP(S)_PROXY / NO_PROXY, or the OS setting). + /// + public static IServiceCollection AddGuardedHttpClient(this IServiceCollection services, string name, IConfiguration configuration) => + services.AddGuardedHttpClient(name, new OutboundDestinationGuard(new OutboundHttpAllowlistSetting(configuration).Value, HttpClient.DefaultProxy)); + + public static IServiceCollection AddGuardedHttpClient(this IServiceCollection services, string name, OutboundDestinationGuard guard) + { + services.AddHttpClient(name) + .ConfigurePrimaryHttpMessageHandler(() => CreatePrimaryHandler(guard)) + .AddHttpMessageHandler(() => new OriginScopedRedirectHandler()) + .AddHttpMessageHandler(() => new GuardedDestinationHandler(guard)); + + return services; + } + + /// + /// Redirects off (the handler above follows them). Cookies off: the factory shares one primary handler across every + /// client it hands out for its lifetime — across runs and teams — so a handler-owned cookie jar would send one run's + /// session on another's requests; the only cookies sent are the ones an author writes. The guard's proxy, whose + /// connections the connect callback recognises and dials as the operator's own. HTTP/3 never engages — it bypasses + /// the connect callback, but requests default to HTTP/1.1 and are never upgraded. + /// + private static SocketsHttpHandler CreatePrimaryHandler(OutboundDestinationGuard guard) => new() + { + AllowAutoRedirect = false, + UseCookies = false, + Proxy = guard.Proxy, + ConnectCallback = guard.ConnectAsync, + }; +} diff --git a/backend/src/CodeSpace.Core/Services/OutboundHttp/OriginScopedRedirectHandler.cs b/backend/src/CodeSpace.Core/Services/OutboundHttp/OriginScopedRedirectHandler.cs new file mode 100644 index 000000000..ae1fd0d3e --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OutboundHttp/OriginScopedRedirectHandler.cs @@ -0,0 +1,78 @@ +using System.Net; + +namespace CodeSpace.Core.Services.OutboundHttp; + +/// +/// Follows redirects for a guarded client in place of the handler's built-in follower, which strips only +/// Authorization when a redirect changes origin: every other header the author wrote — X-Api-Key, +/// PRIVATE-TOKEN, Cookie, often resolved from a team secret — went on to whatever host the first one +/// pointed at. Here a hop that changes scheme, host or port drops EVERY request header; a same-origin hop keeps them, +/// since that is the origin they were written for. That includes an http→https move on the same host: it arrives +/// without the author's headers, and the author keeps them by writing the https URL. A redirect that would carry the +/// request BODY to another origin (a 307/308, or a 300/301/302 of anything but a POST) is not followed at all, because +/// the body holds the same secrets — a token request's client_secret, a webhook's payload — and re-sending the +/// request without it would be a different request; the 3xx is returned for the workflow to act on. Each hop connects +/// through the same primary handler, so the destination guard checks it again. +/// +/// Followed rather than returned as a 3xx because http.request has always followed redirects, and a workflow +/// calling an API that moved behind a same-origin or body-less hop (a CDN, a canonical path) would otherwise start +/// failing. Like the built-in follower: 300/301/302/303/307/308 with a Location are followed; 303, and +/// 300/301/302 after a POST, continue as a body-less GET; an https→http downgrade or a non-http scheme is not followed, +/// and the 3xx is returned. Unlike it, at most hops (the built-in allows 50) are followed, +/// after which the last 3xx is returned as is. +/// +public sealed class OriginScopedRedirectHandler : DelegatingHandler +{ + public const int MaxRedirects = 10; + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + + for (var hop = 0; hop < MaxRedirects && ReadFollowableTarget(request, response) is { } target; hop++) + { + Retarget(request, target, response.StatusCode); + response.Dispose(); + + response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + } + + return response; + } + + private static Uri? ReadFollowableTarget(HttpRequestMessage request, HttpResponseMessage response) + { + if (!IsRedirect(response.StatusCode) || response.Headers.Location is not { } location) return null; + + var target = location.IsAbsoluteUri ? location : new Uri(request.RequestUri!, location); + + return IsFollowable(request.RequestUri!, target) && !CarriesBodyAcrossOrigin(request, target, response.StatusCode) ? target : null; + } + + private static bool IsRedirect(HttpStatusCode status) => (int)status is 300 or 301 or 302 or 303 or 307 or 308; + + /// https anywhere, or http from http — never a downgrade, never another scheme. + private static bool IsFollowable(Uri current, Uri target) => target.Scheme == Uri.UriSchemeHttps || (target.Scheme == Uri.UriSchemeHttp && current.Scheme == Uri.UriSchemeHttp); + + private static bool CarriesBodyAcrossOrigin(HttpRequestMessage request, Uri target, HttpStatusCode status) => request.Content != null && !ContinuesAsGet(status, request.Method) && !IsSameOrigin(request.RequestUri!, target); + + private static void Retarget(HttpRequestMessage request, Uri target, HttpStatusCode status) + { + if (!IsSameOrigin(request.RequestUri!, target)) request.Headers.Clear(); + + if (ContinuesAsGet(status, request.Method)) DropBody(request); + + request.RequestUri = target; + } + + private static bool IsSameOrigin(Uri current, Uri target) => current.Scheme == target.Scheme && current.Port == target.Port && string.Equals(current.IdnHost, target.IdnHost, StringComparison.OrdinalIgnoreCase); + + private static bool ContinuesAsGet(HttpStatusCode status, HttpMethod method) => status == HttpStatusCode.SeeOther || ((int)status is 300 or 301 or 302 && method == HttpMethod.Post); + + private static void DropBody(HttpRequestMessage request) + { + request.Method = HttpMethod.Get; + request.Content?.Dispose(); + request.Content = null; + } +} diff --git a/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationAllowlist.cs b/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationAllowlist.cs new file mode 100644 index 000000000..5624d4d0e --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationAllowlist.cs @@ -0,0 +1,78 @@ +using System.Net; +using CodeSpace.Core.Settings.OutboundHttp; + +namespace CodeSpace.Core.Services.OutboundHttp; + +/// +/// The internal destinations an operator has deliberately opened to workflow-driven outbound HTTP — a host name, an +/// address, or a CIDR per entry, committed in OutboundHttp:AllowedInternalDestinations. A host entry admits +/// whatever that exact name resolves to (no implicit subdomains), and only that name's virtual host — the request is +/// addressed to its URL's authority, never an author-written Host; an address or CIDR entry admits the addresses +/// inside it, whatever name reached them. Empty by default: nothing internal is reachable until an operator commits it. +/// +/// Parsing is strict. An entry that is none of the three fails the setting — and so the host's boot — instead of +/// being skipped, because a skipped entry is a destination the operator believes is open and is not, found only when a +/// workflow fails. +/// +public sealed class OutboundDestinationAllowlist +{ + public static readonly OutboundDestinationAllowlist Empty = new(new HashSet(StringComparer.OrdinalIgnoreCase), Array.Empty()); + + private readonly IReadOnlySet _hosts; + private readonly IReadOnlyList _networks; + + private OutboundDestinationAllowlist(IReadOnlySet hosts, IReadOnlyList networks) + { + _hosts = hosts; + _networks = networks; + } + + public static OutboundDestinationAllowlist Parse(IEnumerable entries) + { + var hosts = new HashSet(StringComparer.OrdinalIgnoreCase); + var networks = new List(); + + foreach (var entry in entries.Where(e => !string.IsNullOrWhiteSpace(e)).Select(e => e.Trim())) + { + if (TryParseNetwork(entry) is { } network) + { + networks.Add(network); + continue; + } + + hosts.Add(ParseHostName(entry)); + } + + return new OutboundDestinationAllowlist(hosts, networks); + } + + /// Whether , reached through , is an operator-admitted destination. + public bool Admits(string host, IPAddress address) + { + var normalized = address.IsIPv4MappedToIPv6 ? address.MapToIPv4() : address; + + return _hosts.Contains(host) || _networks.Any(network => network.Contains(normalized)); + } + + private static IPNetwork? TryParseNetwork(string entry) + { + if (IPAddress.TryParse(entry, out var address)) return new IPNetwork(address, address.GetAddressBytes().Length * 8); + + if (!entry.Contains('/')) return null; + + if (IPNetwork.TryParse(entry, out var network)) return network; + + throw Malformed(entry); + } + + private static string ParseHostName(string entry) + { + if (Uri.CheckHostName(entry) != UriHostNameType.Dns) + throw Malformed(entry); + + return entry; + } + + private static InvalidOperationException Malformed(string entry) => + new($"'{entry}' in {OutboundHttpAllowlistSetting.ConfigurationKey} is not a host name, an IP address or a CIDR range (for example internal-api.corp, 10.1.2.3 or 10.0.0.0/8). Fix or remove the entry; it is refused rather than ignored so a destination you believe is open is not silently closed."); +} diff --git a/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationGuard.cs b/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationGuard.cs new file mode 100644 index 000000000..54b9ab3b0 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationGuard.cs @@ -0,0 +1,100 @@ +using System.Net; +using System.Net.Sockets; + +namespace CodeSpace.Core.Services.OutboundHttp; + +/// +/// The destination check for workflow-driven outbound HTTP. As a it +/// resolves the host the handler is about to connect to, keeps only the addresses that are public or operator-admitted, +/// and dials EXACTLY those — so the check and the connection can never disagree about where the socket goes. +/// +/// Checking at connect time rather than on the URL is what makes it hold. Every spelling of an address +/// (2130706433, 0x7f000001, 127.1, [::ffff:127.0.0.1]) has already been reduced to the +/// address being dialled; every redirect hop opens its own connection through this same callback; and a name whose +/// DNS answer changes between a check and the connect (rebinding) has no gap to exploit, because there is one +/// resolution and it is the one dialled. A name answering with both public and internal addresses is dialled at its +/// public ones only. +/// +/// The operator's proxy (; in production , i.e. +/// HTTP(S)_PROXY / NO_PROXY or the OS setting — what the node's client honoured before this guard existed) is kept, so a +/// worker that reaches the internet only through it still can. A connection TO that proxy is the operator's own +/// infrastructure and is dialled unchecked; the destination of a request it carries is invisible here, so +/// checks it per request through before it is sent. That +/// check resolves the name on the worker — a name the worker cannot resolve fails as a lookup failure, since it cannot +/// be judged — and the proxy resolves it again, so a name whose answer changes in between is the one window this guard +/// does not close; the proxy's own egress rules are the backstop there. A destination the proxy bypasses connects +/// directly, through this callback, with no such window. +/// +public sealed class OutboundDestinationGuard +{ + private readonly OutboundDestinationAllowlist _allowlist; + private readonly Func> _resolve; + + /// Name resolution; the system resolver unless a test stubs it. + public OutboundDestinationGuard(OutboundDestinationAllowlist allowlist, IWebProxy proxy, Func>? resolve = null) + { + _allowlist = allowlist; + _resolve = resolve ?? Dns.GetHostAddressesAsync; + Proxy = proxy; + } + + /// The proxy the guarded handler sends through; applies it. + public IWebProxy Proxy { get; } + + public async ValueTask ConnectAsync(SocketsHttpConnectionContext context, CancellationToken cancellationToken) + { + var endpoint = context.DnsEndPoint; + + var addresses = IsProxyEndpoint(endpoint, context.InitialRequestMessage.RequestUri) + ? await ResolveAsync(endpoint.Host, cancellationToken).ConfigureAwait(false) + : await PermitAsync(endpoint.Host, cancellationToken).ConfigureAwait(false); + + return await DialAsync(addresses, endpoint.Port, cancellationToken).ConfigureAwait(false); + } + + /// The addresses may be dialled at, resolved now; throws when none is public or admitted. + public async Task PermitAsync(string host, CancellationToken cancellationToken) => Permit(host, await ResolveAsync(host, cancellationToken).ConfigureAwait(false)); + + /// The subset of that may be dialled at; throws when nothing is left. + public IPAddress[] Permit(string host, IReadOnlyList resolved) + { + var permitted = resolved.Where(address => PublicAddress.IsPublic(address) || _allowlist.Admits(host, address)).ToArray(); + + if (permitted.Length == 0) + throw new OutboundDestinationRefusedException(host); + + return permitted; + } + + /// The proxy a request to is sent through, or null when it connects directly — the same lookup makes. + public Uri? ProxyFor(Uri destination) => Proxy.IsBypassed(destination) ? null : Proxy.GetProxy(destination); + + /// Whether this connection is the handler opening one to the proxy that carries its request, rather than to a destination. + private bool IsProxyEndpoint(DnsEndPoint endpoint, Uri? request) => request != null && ProxyFor(request) is { } proxy && proxy.Port == endpoint.Port && string.Equals(proxy.IdnHost, endpoint.Host, StringComparison.OrdinalIgnoreCase); + + /// An address literal (bracketed IPv6 included) is its own resolution; a name goes to DNS. A lookup failure propagates as it always did, so an unknown host still reads as one. + private async Task ResolveAsync(string host, CancellationToken cancellationToken) + { + if (IPAddress.TryParse(host, out var literal)) return new[] { literal }; + + return await _resolve(host, cancellationToken).ConfigureAwait(false); + } + + /// The handler's own default dial, restricted to the given addresses: a dual-mode socket tries each in order until one connects. + private static async ValueTask DialAsync(IPAddress[] addresses, int port, CancellationToken cancellationToken) + { + var socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true }; + + try + { + await socket.ConnectAsync(addresses, port, cancellationToken).ConfigureAwait(false); + + return new NetworkStream(socket, ownsSocket: true); + } + catch + { + socket.Dispose(); + throw; + } + } +} diff --git a/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationRefusedException.cs b/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationRefusedException.cs new file mode 100644 index 000000000..f29e06060 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OutboundHttp/OutboundDestinationRefusedException.cs @@ -0,0 +1,23 @@ +using CodeSpace.Core.Settings.OutboundHttp; +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Core.Services.OutboundHttp; + +/// +/// A guarded connection refused because its host resolved to no address the guard may dial. Thrown from the handler's +/// connect callback, so the caller sees it as the inner exception of the the request +/// fails with. The message names the host and the setting that admits it, never the addresses it resolved to: it is +/// shown to the run viewer, and echoing an internal address back would make the guard a resolver for the worker's DNS. +/// +public sealed class OutboundDestinationRefusedException : Exception, IFailure +{ + public OutboundDestinationRefusedException(string host) + : base($"Refused to connect to '{host}': it does not resolve to a public address. Workflow HTTP calls may not reach the worker's loopback, link-local, private or metadata addresses; an operator admits an intentional internal destination by committing its host or CIDR to {OutboundHttpAllowlistSetting.ConfigurationKey}.") + { + } + + /// Well-formed and understood, but blocked by the destination rule: the URL or the operator's allowlist must change, and a retry cannot help. + public FailureKind Kind => FailureKind.Unprocessable; + + public string Code => FailureCodes.OutboundDestinationRefused; +} diff --git a/backend/src/CodeSpace.Core/Services/OutboundHttp/PublicAddress.cs b/backend/src/CodeSpace.Core/Services/OutboundHttp/PublicAddress.cs new file mode 100644 index 000000000..3c164daef --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OutboundHttp/PublicAddress.cs @@ -0,0 +1,55 @@ +using System.Net; +using System.Net.Sockets; +using CodeSpace.Core.Services.Agents.Sandbox.Isolation; + +namespace CodeSpace.Core.Services.OutboundHttp; + +/// +/// Whether an address is a GLOBALLY ROUTABLE destination — the only kind workflow-driven outbound HTTP may dial +/// without an operator saying otherwise. Everything else reaches the worker itself: its loopback services, its cloud +/// metadata endpoint (169.254.169.254, fd00:ec2::254, Aliyun's 100.100.100.200), or the network it sits on. +/// +/// IPv4 reuses the sandbox egress predicate (), so the +/// two outbound surfaces cannot drift apart. IPv6 is global unicast (2000::/3) minus documentation and Teredo; +/// that excludes ::, ::1, link-local, site-local, unique-local and multicast in one test. An IPv6 address +/// that EMBEDS an IPv4 one — v4-mapped, v4-compatible, NAT64 64:ff9b::/96, 6to4 2002::/16 — is judged by +/// the IPv4 address inside it: a dual-stack socket dials ::ffff:127.0.0.1 straight to 127.0.0.1, and an +/// IPv6-only cluster reaches public IPv4 hosts through NAT64, which must keep working. +/// +public static class PublicAddress +{ + private static readonly byte[] MappedPrefix = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff }; + private static readonly byte[] CompatiblePrefix = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; + private static readonly byte[] Nat64Prefix = { 0, 0x64, 0xff, 0x9b, 0, 0, 0, 0, 0, 0, 0, 0 }; + private static readonly byte[] SixToFourPrefix = { 0x20, 0x02 }; + private static readonly byte[] DocumentationPrefix = { 0x20, 0x01, 0x0d, 0xb8 }; + private static readonly byte[] TeredoPrefix = { 0x20, 0x01, 0, 0 }; + + public static bool IsPublic(IPAddress address) => address.AddressFamily switch + { + AddressFamily.InterNetwork => EgressHostResolver.IsGloballyRoutableIpv4(address), + AddressFamily.InterNetworkV6 => IsPublicIpv6(address.GetAddressBytes()), + _ => false, + }; + + private static bool IsPublicIpv6(byte[] bytes) + { + if (EmbeddedIpv4(bytes) is { } embedded) return EgressHostResolver.IsGloballyRoutableIpv4(embedded); + + return IsGlobalUnicast(bytes) && !StartsWith(bytes, DocumentationPrefix) && !StartsWith(bytes, TeredoPrefix); + } + + /// The IPv4 address an IPv6 address carries, when it is one of the forms that route to that IPv4 address; null otherwise. :: and ::1 fall in the v4-compatible block and come out as 0.0.0.0 / 0.0.0.1, which the IPv4 check refuses. + private static IPAddress? EmbeddedIpv4(byte[] bytes) + { + if (StartsWith(bytes, MappedPrefix) || StartsWith(bytes, CompatiblePrefix) || StartsWith(bytes, Nat64Prefix)) return new IPAddress(bytes[12..16]); + + if (StartsWith(bytes, SixToFourPrefix)) return new IPAddress(bytes[2..6]); + + return null; + } + + private static bool IsGlobalUnicast(byte[] bytes) => (bytes[0] & 0xe0) == 0x20; + + private static bool StartsWith(byte[] bytes, byte[] prefix) => bytes.AsSpan().StartsWith(prefix); +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/HttpRequestNode.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/HttpRequestNode.cs index ac8683300..e0ae4285f 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/HttpRequestNode.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/HttpRequestNode.cs @@ -2,6 +2,7 @@ using System.Net.Http.Headers; using System.Text; using System.Text.Json; +using CodeSpace.Core.Services.OutboundHttp; using CodeSpace.Messages.Enums; using Microsoft.Extensions.Logging; @@ -25,6 +26,13 @@ namespace CodeSpace.Core.Services.Workflows.Nodes.Builtin; /// ok — true iff 2xx /// body — response body (parsed JSON when parseJson and content-type matches, else string) /// headers — response headers as object +/// +/// The URL is whatever its template resolves to — often trigger text or an upstream node's output — so the node's +/// client is the guarded one (): it reaches only public addresses or ones an +/// operator committed to OutboundHttp:AllowedInternalDestinations, directly or through the worker's +/// HTTP(S)_PROXY; it sends no cookies but the ones the author writes, and no author Host header; a redirect to +/// another origin — an http→https move on the same host included — carries none of the author's headers; and a +/// redirect that would carry the body to another origin is returned as the node's 3xx rather than followed. /// public sealed class HttpRequestNode : INodeRuntime { @@ -43,7 +51,7 @@ public sealed class HttpRequestNode : INodeRuntime Category = "Tools", Kind = NodeKind.Regular, IconKey = "globe", - Description = "Make an HTTP request to any URL. Universal escape hatch when no bespoke node exists.", + Description = "Make an HTTP request to any public URL. Universal escape hatch when no bespoke node exists. Internal addresses (loopback, private, cloud metadata) are refused unless an operator allowlists them. A redirect to another origin, http→https included, arrives without your headers — write the final URL to keep them — and one that would send your body to another origin comes back as the 3xx.", // HTTP requests with mutating verbs (POST/PUT/PATCH/DELETE) are side-effecting. GET // is technically safe to retry, but the manifest is a node-level marker and the // operator can't promise which verb their template will pick. Mark the whole node @@ -153,6 +161,11 @@ public async Task RunAsync(NodeRunContext context, CancellationToken // (Observability already emitted external_call.failed; re-translate to NodeResult.Fail.) return NodeResult.Fail($"HTTP request timed out after {timeoutSeconds}s."); } + catch (HttpRequestException ex) when (ex.InnerException is OutboundDestinationRefusedException) + { + // Refused by destination policy: deterministic, so a retry policy must not spend attempts on it. + return NodeResult.Fail($"HTTP request failed: {ex.Message}", retryable: false); + } catch (HttpRequestException ex) { return NodeResult.Fail($"HTTP request failed: {ex.Message}"); diff --git a/backend/src/CodeSpace.Core/Settings/OutboundHttp/OutboundHttpAllowlistSetting.cs b/backend/src/CodeSpace.Core/Settings/OutboundHttp/OutboundHttpAllowlistSetting.cs new file mode 100644 index 000000000..8fc42ce88 --- /dev/null +++ b/backend/src/CodeSpace.Core/Settings/OutboundHttp/OutboundHttpAllowlistSetting.cs @@ -0,0 +1,26 @@ +using CodeSpace.Core.Services.OutboundHttp; +using Microsoft.Extensions.Configuration; + +namespace CodeSpace.Core.Settings.OutboundHttp; + +/// +/// The operator's committed allowlist of INTERNAL destinations workflow-driven outbound HTTP (the http.request +/// node) may reach — host names, addresses and CIDR ranges that the destination guard would otherwise refuse as +/// loopback, private, link-local or otherwise non-public. Read once, while the host is being built +/// (GuardedHttpClientRegistration.AddGuardedHttpClient), so a malformed entry stops the host from starting. It is +/// a list in appsettings, changed by committing a value, not a switch: the guard itself cannot be turned off. +/// +/// The key's literal value is pinned by a unit test: renaming it would silently empty every deployment's +/// allowlist that used the old name, and their intentional internal targets would start failing. +/// +public class OutboundHttpAllowlistSetting : IConfigurationSetting +{ + public const string ConfigurationKey = "OutboundHttp:AllowedInternalDestinations"; + + public OutboundHttpAllowlistSetting(IConfiguration configuration) + { + Value = OutboundDestinationAllowlist.Parse(configuration.GetSection(ConfigurationKey).Get() ?? Array.Empty()); + } + + public OutboundDestinationAllowlist Value { get; set; } +} diff --git a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs index a9f1214f3..390642e98 100644 --- a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs +++ b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs @@ -69,6 +69,9 @@ public static class FailureCodes public const string RerunUpstreamNotReusable = "rerun_upstream_not_reusable"; public const string PackImportFailed = "pack_import_failed"; + /// A workflow's outbound HTTP call was refused at connect time because its host resolves only to loopback, private, link-local, metadata or other non-public addresses. Remedy: point the call at a public destination, or have an operator commit the host or CIDR to OutboundHttp:AllowedInternalDestinations — a retry cannot help. + public const string OutboundDestinationRefused = "outbound_destination_refused"; + /// A settled cell's outputs were redacted for persistence and their encrypted recovery payload is not readable, so the originals exist nowhere. Remedy: re-run the workflow from that node — a resume can only ever offer the redaction placeholder. public const string WorkflowOutputsUnrecoverable = "workflow_outputs_unrecoverable"; diff --git a/backend/tests/CodeSpace.E2ETests/Workflows/HttpRequestClientHostE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Workflows/HttpRequestClientHostE2ETests.cs new file mode 100644 index 000000000..f295a4c84 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Workflows/HttpRequestClientHostE2ETests.cs @@ -0,0 +1,63 @@ +using CodeSpace.Core.Services.OutboundHttp; +using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.Core.Settings.OutboundHttp; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.E2ETests.Workflows; + +/// +/// HIGH fidelity (Rule 12): the http.request client as the REAL host registers it — Program.CreateHostBuilder + +/// Startup, the API's committed appsettings.json, real sockets against a loopback responder. Every other +/// tier builds its own registration, so none of them would notice the host losing its AddGuardedHttpClient line +/// (a merge-conflict casualty in a busy file): CreateClient(nameof(HttpRequestNode)) would fall back to the +/// default client — redirects followed, no address check — and every other test would stay green. These pin that +/// registration, the committed empty allowlist, and that the allowlist is read while the host is being built. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class HttpRequestClientHostE2ETests : IClassFixture +{ + private const string MetadataPath = "/latest/meta-data/iam/security-credentials/role"; + + private readonly WebhookApiFactory _factory; + + public HttpRequestClientHostE2ETests(WebhookApiFactory factory) { _factory = factory; } + + [Theory] + [InlineData("127.0.0.1")] + [InlineData("localhost")] + public async Task The_hosts_http_request_client_refuses_the_workers_loopback(string host) + { + await using var metadata = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{\"Token\":\"FAKE-LOOPBACK-SECRET\"}")); + using var client = _factory.Services.GetRequiredService().CreateClient(nameof(HttpRequestNode)); + client.Timeout = TimeSpan.FromSeconds(10); + + var ex = await Should.ThrowAsync(() => client.GetAsync($"http://{host}:{metadata.Port}{MetadataPath}")); + + ex.InnerException.ShouldBeOfType(customMessage: $"the host's '{nameof(HttpRequestNode)}' client did not refuse http://{host} through the destination guard ({ex.Message}). Check Startup.ConfigureServices still calls AddGuardedHttpClient for it, and that CodeSpace.Api/appsettings.json commits an empty {OutboundHttpAllowlistSetting.ConfigurationKey}."); + metadata.Accepted.ShouldBe(0, $"no socket may reach the worker's loopback through http://{host}"); + } + + [Fact] + public void A_malformed_committed_entry_stops_the_host_from_starting() + { + // Built WITHOUT the fixture's database: the host must fail while its services are being registered, before + // anything would touch one. + using var unbooted = new WebhookApiFactory(); + using var factory = unbooted.WithWebHostBuilder(builder => builder.ConfigureAppConfiguration((_, configuration) => configuration.AddInMemoryCollection(new Dictionary { [$"{OutboundHttpAllowlistSetting.ConfigurationKey}:0"] = "internal-api.corp:8080" }))); + + var ex = Should.Throw(() => factory.Services); + + Causes(ex).ShouldContain(cause => cause is InvalidOperationException && cause.Message.Contains("internal-api.corp:8080"), customMessage: $"the host must refuse to start on a malformed {OutboundHttpAllowlistSetting.ConfigurationKey} entry, naming it — otherwise it is found only when every http.request call fails at run time. Got: {ex}"); + } + + private static IEnumerable Causes(Exception ex) + { + for (Exception? cause = ex; cause != null; cause = cause.InnerException) yield return cause; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs index 0cc358224..c470b79d5 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs @@ -3,6 +3,7 @@ using Autofac.Extensions.DependencyInjection; using CodeSpace.Core; using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.OutboundHttp; using CodeSpace.Core.Settings; using CodeSpace.IntegrationTests.Settings; using MediatR; @@ -198,6 +199,9 @@ private Autofac.IContainer BuildIocContainer(string connectionString) // is unaffected; a longer timeout is inert for any client that never makes a real slow call. services.AddHttpClient(nameof(CodeSpace.Core.Services.Workflows.Llm.Anthropic.AnthropicClient), c => c.Timeout = TimeSpan.FromSeconds(180)); services.AddHttpClient(nameof(CodeSpace.Core.Services.Workflows.Llm.OpenAi.OpenAiClient), c => c.Timeout = TimeSpan.FromSeconds(180)); + // The production http.request client, so every engine test that runs the node goes through the destination + // guard. The suite's committed allowlist (appsettings.json) admits 127.0.0.1 for its loopback responders. + services.AddGuardedHttpClient(nameof(CodeSpace.Core.Services.Workflows.Nodes.Builtin.HttpRequestNode), configuration); var builder = new ContainerBuilder(); builder.Populate(services); diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/HttpRequestDestinationFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/HttpRequestDestinationFlowTests.cs new file mode 100644 index 000000000..9355a2042 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/HttpRequestDestinationFlowTests.cs @@ -0,0 +1,210 @@ +using System.Net; +using System.Net.Sockets; +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.OutboundHttp; +using CodeSpace.Core.Services.Workflows.Engine; +using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.Core.Settings.OutboundHttp; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Dtos.Workflows; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// http.request's destination guard through the whole engine: a workflow created by the mediator, a run seeded with +/// an untrusted trigger payload, resolving {{trigger.body}} into +/// the URL, the real sending through the PRODUCTION guarded-client registration, and the +/// outcome read back from workflow_run_node and the ledger. The responders are raw loopback sockets that count +/// accepted connections, standing in for a metadata endpoint or an admin port on the worker. +/// +/// Each test runs the engine in a child scope whose carries its own committed +/// allowlist (the fixture's root admits 127.0.0.1 for the suite's other loopback-calling tests), so a refusal here is +/// the guard's and not an artefact of the suite's configuration. The proxy rows build their guard with a loopback +/// responder as the operator's proxy and a stubbed resolver, so no lookup or connection leaves the machine. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class HttpRequestDestinationFlowTests +{ + private const string MetadataPath = "/latest/meta-data/iam/security-credentials/role"; + private const string FakeCredential = "{\"AccessKeyId\":\"FAKE-AKID\",\"Token\":\"FAKE-LOOPBACK-SECRET\"}"; + + private readonly PostgresFixture _fixture; + + public HttpRequestDestinationFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Theory] + [InlineData("127.0.0.1")] + [InlineData("localhost")] + [InlineData("2130706433")] + [InlineData("[::1]")] + [InlineData("[::ffff:127.0.0.1]")] + public async Task A_loopback_url_built_from_trigger_text_fails_the_node_without_reaching_loopback(string host) + { + await using var metadata = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + + var runId = await RunHttpWorkflowAsync("{{trigger.body}}", $"http://{host}:{metadata.Port}{MetadataPath}"); + + var node = await ReadCallNodeAsync(runId); + node.Status.ShouldBe(NodeStatus.Failure); + node.Error.ShouldNotBeNull().ShouldContain("Refused to connect"); + node.OutputsJson.ShouldNotContain("FAKE-LOOPBACK-SECRET"); + + (await ReadExternalCallErrorAsync(runId)).ShouldContain("Refused to connect", customMessage: "the ledger's external_call.failed must say the destination was refused, so the run-detail page explains the failure"); + metadata.Accepted.ShouldBe(0, $"no socket may reach the loopback responder through http://{host}"); + } + + [Fact] + public async Task A_redirect_to_loopback_from_an_allowlisted_host_fails_the_node_at_the_redirect_hop() + { + await using var metadata = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + await using var front = LoopbackResponder.Start(_ => LoopbackResponder.Redirect($"http://127.0.0.1:{metadata.Port}{MetadataPath}")); + + var runId = await RunHttpWorkflowAsync($"http://localhost:{front.Port}/{{{{trigger.body}}}}", "feature-x", "localhost"); + + var node = await ReadCallNodeAsync(runId); + node.Status.ShouldBe(NodeStatus.Failure); + node.Error.ShouldNotBeNull().ShouldContain("Refused to connect to '127.0.0.1'"); + + front.Requests.Count.ShouldBe(1, "the allowlisted first hop was made"); + metadata.Accepted.ShouldBe(0, "the redirect target is loopback and not allowlisted — no socket may reach it"); + } + + [Fact] + public async Task An_allowlisted_internal_destination_still_answers_in_full() + { + await using var service = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + + var runId = await RunHttpWorkflowAsync("{{trigger.body}}", $"http://127.0.0.1:{service.Port}{MetadataPath}", "127.0.0.0/8"); + + var node = await ReadCallNodeAsync(runId); + node.Status.ShouldBe(NodeStatus.Success, node.Error); + + var outputs = JsonDocument.Parse(node.OutputsJson).RootElement; + outputs.GetProperty("status").GetInt32().ShouldBe(200); + outputs.GetProperty("body").GetProperty("Token").GetString().ShouldBe("FAKE-LOOPBACK-SECRET", "an operator-admitted destination's response reaches the node outputs as before"); + service.Accepted.ShouldBe(1); + } + + [Fact] + public async Task A_public_destination_reached_through_the_operators_proxy_answers_in_full() + { + // A worker whose only route out is HTTP(S)_PROXY: the public-resolving name is checked on the worker, the proxy + // fetches it, and the answer reaches the node outputs. 203.0.113.10 is TEST-NET-3 — public to the guard, routed + // nowhere — so nothing here could reach a real host even if the proxy were bypassed. + await using var proxy = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{\"via\":\"operator-proxy\"}")); + + var runId = await RunHttpWorkflowAsync("{{trigger.body}}", "http://api.example.test/v1/items", ProxiedGuard(proxy)); + + var node = await ReadCallNodeAsync(runId); + node.Status.ShouldBe(NodeStatus.Success, node.Error); + JsonDocument.Parse(node.OutputsJson).RootElement.GetProperty("body").GetProperty("via").GetString().ShouldBe("operator-proxy"); + proxy.Requests.Single().ShouldStartWith("GET http://api.example.test/v1/items HTTP/1.1"); + } + + [Fact] + public async Task An_internal_destination_the_proxy_would_carry_fails_the_node_before_the_proxy_is_asked() + { + await using var proxy = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + + var runId = await RunHttpWorkflowAsync("{{trigger.body}}", $"http://metadata.example.test{MetadataPath}", ProxiedGuard(proxy)); + + var node = await ReadCallNodeAsync(runId); + node.Status.ShouldBe(NodeStatus.Failure); + node.Error.ShouldNotBeNull().ShouldContain("Refused to connect to 'metadata.example.test'"); + node.OutputsJson.ShouldNotContain("FAKE-LOOPBACK-SECRET"); + proxy.Accepted.ShouldBe(0, "the proxy must never be asked to fetch the metadata address"); + } + + // ─── Helpers ──────────────────────────────────────────────────────────────── + + /// The production registration from a committed allowlist of , as the host builds it. + private Task RunHttpWorkflowAsync(string urlTemplate, string triggerBody, params string[] allowed) => RunHttpWorkflowAsync(urlTemplate, triggerBody, services => services.AddGuardedHttpClient(nameof(HttpRequestNode), CommittedAllowlist(allowed))); + + private Task RunHttpWorkflowAsync(string urlTemplate, string triggerBody, OutboundDestinationGuard guard) => RunHttpWorkflowAsync(urlTemplate, triggerBody, services => services.AddGuardedHttpClient(nameof(HttpRequestNode), guard)); + + private async Task RunHttpWorkflowAsync(string urlTemplate, string triggerBody, Action registerGuardedClient) + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var workflowId = await CreateHttpWorkflowAsync(teamId, userId, urlTemplate); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId, payloadJson: JsonSerializer.Serialize(new { body = triggerBody })); + + var services = new ServiceCollection(); + registerGuardedClient(services); + + await using var guarded = services.BuildServiceProvider(); + using var scope = _fixture.BeginScope(b => b.RegisterInstance(guarded.GetRequiredService()).As()); + + await scope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + + return runId; + } + + private static IConfiguration CommittedAllowlist(string[] allowed) => new ConfigurationBuilder() + .AddInMemoryCollection(allowed.Select((entry, i) => KeyValuePair.Create($"{OutboundHttpAllowlistSetting.ConfigurationKey}:{i}", (string?)entry))) + .Build(); + + /// Nothing allowlisted; as the operator's HTTP(S)_PROXY; two stubbed names, one public and one the metadata address. + private static OutboundDestinationGuard ProxiedGuard(LoopbackResponder proxy) => new(OutboundDestinationAllowlist.Empty, new WebProxy($"http://127.0.0.1:{proxy.Port}"), (host, _) => host switch + { + "api.example.test" => Task.FromResult(new[] { IPAddress.Parse("203.0.113.10") }), + "metadata.example.test" => Task.FromResult(new[] { IPAddress.Parse("169.254.169.254") }), + _ => Task.FromException(new SocketException((int)SocketError.HostNotFound)), + }); + + private async Task CreateHttpWorkflowAsync(Guid teamId, Guid userId, string urlTemplate) + { + using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); + var mediator = scope.Resolve(); + + var def = new WorkflowDefinition + { + SchemaVersion = 1, + Nodes = new List + { + new() { Id = "start", TypeKey = "trigger.pr.opened", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "call", TypeKey = "http.request", Config = WorkflowsTestSeed.Json("""{"timeoutSeconds":5}"""), Inputs = WorkflowsTestSeed.Json(JsonSerializer.Serialize(new { url = urlTemplate, method = "GET" })) }, + new() { Id = "end", TypeKey = "builtin.terminal", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + }, + Edges = new List + { + new() { From = "start", To = "call" }, + new() { From = "call", To = "end" }, + }, + }; + + return await mediator.Send(new CodeSpace.Messages.Commands.Workflows.CreateWorkflowCommand + { + Name = "http-destination-" + Guid.NewGuid().ToString("N")[..8], + Description = null, + Definition = def, + Activations = new List(), + Enabled = true, + }); + } + + private async Task ReadCallNodeAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().WorkflowRunNode.AsNoTracking().SingleAsync(n => n.RunId == runId && n.NodeId == "call"); + } + + private async Task ReadExternalCallErrorAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + + var failed = await scope.Resolve().WorkflowRunRecord.AsNoTracking().SingleAsync(r => r.RunId == runId && r.RecordType == WorkflowRunRecordTypes.ExternalCallFailed); + + return JsonDocument.Parse(failed.PayloadJson).RootElement.GetProperty("error").GetString() ?? ""; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/LoopbackResponder.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/LoopbackResponder.cs new file mode 100644 index 000000000..fd1c92261 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/LoopbackResponder.cs @@ -0,0 +1,128 @@ +using System.Collections.Concurrent; +using System.Net; +using System.Net.Sockets; +using System.Text; + +namespace CodeSpace.IntegrationTests.Workflows.Infrastructure; + +/// +/// A raw HTTP/1.1 responder on 127.0.0.1 AND ::1 at one ephemeral port, standing in for a service on the worker's own +/// loopback (a metadata endpoint, an admin port). It counts ACCEPTED CONNECTIONS, not just requests, so a test can +/// prove the guard refused before any socket reached it — a refused-at-connect destination and one that answered +/// would otherwise look alike from the client. It accepts any Host header, so every spelling of loopback reaches it. +/// +public sealed class LoopbackResponder : IAsyncDisposable +{ + private readonly List _listeners; + private readonly Func _respond; + private readonly CancellationTokenSource _stop = new(); + private readonly ConcurrentQueue _requests = new(); + private int _accepted; + + private LoopbackResponder(List listeners, Func respond) + { + _listeners = listeners; + _respond = respond; + Port = ((IPEndPoint)listeners[0].LocalEndpoint).Port; + + foreach (var listener in listeners) _ = AcceptLoopAsync(listener); + } + + public int Port { get; } + + public int Accepted => Volatile.Read(ref _accepted); + + /// Each request's head (request line + headers) in arrival order. + public IReadOnlyCollection Requests => _requests; + + /// Start a responder whose reply to each request head is 's raw HTTP response. + public static LoopbackResponder Start(Func respond) + { + var v4 = new TcpListener(IPAddress.Loopback, 0); + v4.Start(); + + return new LoopbackResponder(new List { v4 }.Concat(TryListenIpv6(((IPEndPoint)v4.LocalEndpoint).Port)).ToList(), respond); + } + + public static string Ok(string body) => $"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {Encoding.UTF8.GetByteCount(body)}\r\nConnection: close\r\n\r\n{body}"; + + public static string Redirect(string location) => $"HTTP/1.1 302 Found\r\nLocation: {location}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"; + + /// The IPv6 twin on the same port, when this host has IPv6 loopback. Without it a [::1] request would fail with "connection refused" even if the guard let it through — hiding the very bug the test looks for. + private static IEnumerable TryListenIpv6(int port) + { + if (!Socket.OSSupportsIPv6) return Array.Empty(); + + try + { + var v6 = new TcpListener(IPAddress.IPv6Loopback, port); + v6.Start(); + return new[] { v6 }; + } + catch (SocketException) + { + return Array.Empty(); + } + } + + private async Task AcceptLoopAsync(TcpListener listener) + { + while (!_stop.IsCancellationRequested) + { + TcpClient client; + + try { client = await listener.AcceptTcpClientAsync(_stop.Token).ConfigureAwait(false); } + catch (Exception) { return; } + + Interlocked.Increment(ref _accepted); + _ = ServeAsync(client); + } + } + + private async Task ServeAsync(TcpClient client) + { + using var _ = client; + + try + { + var stream = client.GetStream(); + var head = await ReadHeadAsync(stream).ConfigureAwait(false); + + _requests.Enqueue(head); + + await stream.WriteAsync(Encoding.UTF8.GetBytes(_respond(head)), _stop.Token).ConfigureAwait(false); + } + catch (Exception) + { + // A client that hung up mid-request is not this responder's failure to report. + } + } + + private async Task ReadHeadAsync(NetworkStream stream) + { + var received = new StringBuilder(); + var buffer = new byte[8192]; + + while (!received.ToString().Contains("\r\n\r\n")) + { + var read = await stream.ReadAsync(buffer, _stop.Token).ConfigureAwait(false); + + if (read == 0) break; + + received.Append(Encoding.ASCII.GetString(buffer, 0, read)); + } + + return received.ToString(); + } + + public ValueTask DisposeAsync() + { + _stop.Cancel(); + + foreach (var listener in _listeners) listener.Stop(); + + _stop.Dispose(); + + return ValueTask.CompletedTask; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/appsettings.json b/backend/tests/CodeSpace.IntegrationTests/appsettings.json index 9930e4e8d..302b12e49 100644 --- a/backend/tests/CodeSpace.IntegrationTests/appsettings.json +++ b/backend/tests/CodeSpace.IntegrationTests/appsettings.json @@ -3,6 +3,10 @@ "AdminConnectionString": "Host=localhost;Port=5432;Username=postgres;Password=123456;Database=postgres", "TestDatabaseNamePrefix": "codespace_test_" }, + "OutboundHttp": { + "_doc": "The suite's http.request tests call loopback responders on 127.0.0.1, which the destination guard refuses unless an operator admits them. This is that admission for the test host only; the guard's own tests run under their own allowlists.", + "AllowedInternalDestinations": [ "127.0.0.1" ] + }, "Logging": { "LogLevel": { "Default": "Warning", diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs index 50e78035a..9b30a0c89 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs @@ -84,6 +84,7 @@ public void The_wire_codes_are_pinned() FailureCodes.RerunBlockedUnsupportedNode.ShouldBe("rerun_blocked_unsupported_node"); FailureCodes.RerunUpstreamNotReusable.ShouldBe("rerun_upstream_not_reusable"); FailureCodes.PackImportFailed.ShouldBe("pack_import_failed"); + FailureCodes.OutboundDestinationRefused.ShouldBe("outbound_destination_refused"); FailureCodes.WorkflowOutputsUnrecoverable.ShouldBe("workflow_outputs_unrecoverable"); FailureCodes.StorageCredentialInvalid.ShouldBe("storage_credential_invalid"); FailureCodes.StorageCredentialConflict.ShouldBe("storage_credential_conflict"); diff --git a/backend/tests/CodeSpace.UnitTests/OutboundHttp/GuardedHttpClientTests.cs b/backend/tests/CodeSpace.UnitTests/OutboundHttp/GuardedHttpClientTests.cs new file mode 100644 index 000000000..0178a0f14 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OutboundHttp/GuardedHttpClientTests.cs @@ -0,0 +1,324 @@ +using System.Net; +using System.Net.Sockets; +using System.Text.Json; +using CodeSpace.Core.Services.OutboundHttp; +using CodeSpace.Core.Services.Workflows.Nodes; +using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.Core.Services.Workflows.Runtime; +using CodeSpace.Core.Settings.OutboundHttp; +using CodeSpace.Messages.Enums; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; + +namespace CodeSpace.UnitTests.OutboundHttp; + +/// +/// The PRODUCTION registration () driven over real sockets against loopback +/// responders — the shapes the SSRF audit proved reachable through http.request, now refused. Each refusal also asserts +/// the responder ACCEPTED NO CONNECTION: a test that only checked for a failed request would pass just as well if the +/// guard let the connection through and the request then failed for some other reason. +/// +/// Each guard is built here with an explicit proxy — none, or a loopback responder playing the operator's proxy — +/// so a developer's own HTTP(S)_PROXY cannot change what these assert. Names other than localhost go to a stubbed +/// resolver (), so no lookup leaves the process. +/// +[Trait("Category", "Unit")] +public class GuardedHttpClientTests +{ + private const string MetadataPath = "/latest/meta-data/iam/security-credentials/role"; + private const string FakeCredential = "{\"AccessKeyId\":\"FAKE-AKID\",\"Token\":\"FAKE-LOOPBACK-SECRET\"}"; + + [Theory] + [InlineData("localhost")] + [InlineData("127.0.0.1")] + [InlineData("2130706433")] // decimal + [InlineData("0x7f000001")] // hex + [InlineData("127.1")] // short form + [InlineData("0177.0.0.1")] // octal + [InlineData("0.0.0.0")] // "this host" — connects to loopback + [InlineData("[::1]")] + [InlineData("[::]")] + [InlineData("[::ffff:127.0.0.1]")] // v4-mapped + [InlineData("[0:0:0:0:0:ffff:7f00:1]")] // v4-mapped, long hex form + public async Task Every_spelling_of_loopback_is_refused_before_a_connection_is_made(string host) + { + await using var metadata = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + using var factory = GuardedFactory(); + + var ex = await Should.ThrowAsync(() => factory.Client.GetAsync($"http://{host}:{metadata.Port}{MetadataPath}")); + + ex.InnerException.ShouldBeOfType($"http://{host} must be refused by the destination guard, not fail some other way: {ex.Message}"); + metadata.Accepted.ShouldBe(0, $"no socket may reach the loopback responder through http://{host}"); + } + + [Theory] + [InlineData("127.0.0.0/8", "127.0.0.1")] // CIDR entry + [InlineData("localhost", "localhost")] // host entry: admits ::1 and 127.0.0.1 — the dial tries both + public async Task An_allowlisted_internal_destination_answers_in_full(string entry, string host) + { + await using var service = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + using var factory = GuardedFactory(entry); + + using var response = await factory.Client.GetAsync($"http://{host}:{service.Port}{MetadataPath}"); + + response.IsSuccessStatusCode.ShouldBeTrue(); + (await response.Content.ReadAsStringAsync()).ShouldBe(FakeCredential); + } + + [Fact] + public async Task A_redirect_from_an_admitted_host_to_loopback_is_refused_at_the_redirect_hop() + { + // The audit's P3: the author pins a host, the host answers 302 to the worker's loopback. The first hop is + // admitted (the operator allowlisted that host by name); the second must be refused when it connects. + await using var metadata = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + await using var front = LoopbackResponder.Start(_ => LoopbackResponder.Redirect($"http://127.0.0.1:{metadata.Port}{MetadataPath}")); + using var factory = GuardedFactory("localhost"); + + var ex = await Should.ThrowAsync(() => factory.Client.GetAsync($"http://localhost:{front.Port}/feature-x")); + + ex.InnerException.ShouldBeOfType(ex.Message); + front.Requests.Count.ShouldBe(1, "the admitted first hop was made"); + metadata.Accepted.ShouldBe(0, "the redirect target is loopback and not allowlisted — no socket may reach it"); + } + + [Fact] + public async Task A_cross_origin_redirect_delivers_none_of_the_author_headers() + { + await using var target = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{\"ok\":true}")); + await using var front = LoopbackResponder.Start(_ => LoopbackResponder.Redirect($"http://127.0.0.1:{target.Port}/data")); + using var factory = GuardedFactory("localhost", "127.0.0.1"); + + using var request = new HttpRequestMessage(HttpMethod.Get, $"http://localhost:{front.Port}/start"); + request.Headers.TryAddWithoutValidation("X-Api-Key", "FAKE-TEAM-API-KEY"); + request.Headers.TryAddWithoutValidation("PRIVATE-TOKEN", "FAKE-PRIVATE-TOKEN"); + request.Headers.TryAddWithoutValidation("Cookie", "sid=FAKE-COOKIE"); + + using var response = await factory.Client.SendAsync(request); + + response.IsSuccessStatusCode.ShouldBeTrue(); + front.Requests.Single().ShouldContain("X-Api-Key: FAKE-TEAM-API-KEY", customMessage: "the origin the author wrote the headers for receives them"); + + var redirected = target.Requests.Single(); + redirected.ShouldNotContain("FAKE-TEAM-API-KEY"); + redirected.ShouldNotContain("FAKE-PRIVATE-TOKEN"); + redirected.ShouldNotContain("FAKE-COOKIE"); + } + + [Fact] + public async Task The_http_request_node_refuses_a_loopback_url_built_from_trigger_text() + { + // The audit's P1, end to end through the real node: the URL is the untrusted {{trigger.body}}, resolved by the + // engine's own VariableResolver. + await using var metadata = LoopbackResponder.Start(_ => LoopbackResponder.Ok(FakeCredential)); + using var factory = GuardedFactory(); + + var result = await new HttpRequestNode(factory.Factory).RunAsync(Context(new { url = "{{trigger.body}}", method = "GET" }, $"http://127.0.0.1:{metadata.Port}{MetadataPath}"), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Failure); + result.Error.ShouldNotBeNull().ShouldContain("Refused to connect to '127.0.0.1'"); + result.Retryable.ShouldBeFalse("a refused destination is deterministic; retrying it only burns attempts"); + result.Outputs.ShouldNotContainKey("body", "nothing from the loopback responder may become a node output"); + metadata.Accepted.ShouldBe(0); + } + + [Fact] + public async Task A_cookie_one_run_was_given_is_never_sent_by_another() + { + // Every CreateClient(nameof(HttpRequestNode)) in the process shares one primary handler for its lifetime, across + // runs and teams. With the handler's own cookie jar, run B's call carried the session run A had just been given. + await using var service = LoopbackResponder.Start(head => head.StartsWith("GET /login", StringComparison.Ordinal) ? LoopbackResponder.Ok("{}", "Set-Cookie: sid=FAKE-TEAM-A-SESSION; Path=/") : LoopbackResponder.Ok("{}")); + using var factory = GuardedFactory("localhost"); + + var teamA = await new HttpRequestNode(factory.Factory).RunAsync(Context(new { url = $"http://localhost:{service.Port}/login", method = "GET" }), CancellationToken.None); + var teamB = await new HttpRequestNode(factory.Factory).RunAsync(Context(new { url = $"http://localhost:{service.Port}/me", method = "GET" }), CancellationToken.None); + var authored = await new HttpRequestNode(factory.Factory).RunAsync(Context(new { url = $"http://localhost:{service.Port}/mine", method = "GET", headers = new Dictionary { ["Cookie"] = "sid=FAKE-AUTHORED" } }), CancellationToken.None); + + new[] { teamA.Status, teamB.Status, authored.Status }.ShouldAllBe(status => status == NodeStatus.Success); + RequestTo(service, "/me").ShouldNotContain("Cookie:", customMessage: "a cookie set for one run must never ride along on another run's request"); + RequestTo(service, "/mine").ShouldContain("Cookie: sid=FAKE-AUTHORED", customMessage: "a Cookie header the author wrote still reaches the origin it was written for"); + RequestTo(service, "/mine").ShouldNotContain("FAKE-TEAM-A-SESSION"); + } + + [Fact] + public async Task An_author_host_header_cannot_reach_another_virtual_host_behind_an_admitted_name() + { + // An operator admits ONE name. Host (and the TLS SNI the handler derives from it) is what picks the virtual host + // behind that address, so an author-written Host would reach every other vhost on a shared ingress. + await using var ingress = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{}")); + using var factory = GuardedFactory("localhost"); + + var result = await new HttpRequestNode(factory.Factory).RunAsync(Context(new { url = $"http://localhost:{ingress.Port}/admin", method = "GET", headers = new Dictionary { ["Host"] = "secret-admin.internal" } }), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Success, result.Error); + RequestTo(ingress, "/admin").ShouldContain($"Host: localhost:{ingress.Port}", customMessage: "the request is addressed to the authority the guard checked"); + RequestTo(ingress, "/admin").ShouldNotContain("secret-admin.internal"); + } + + [Fact] + public async Task A_cross_origin_307_returns_the_redirect_instead_of_sending_the_body_on() + { + // The audit's body twin of the header leak: a token endpoint answers 307 to another origin, and a 307 re-sends + // the body — the client_secret the author wrote for the first origin, often from a team secret. + await using var collector = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{}")); + await using var front = LoopbackResponder.Start(_ => LoopbackResponder.Redirect($"http://127.0.0.1:{collector.Port}/collect", 307)); + using var factory = GuardedFactory("localhost", "127.0.0.1"); + + var result = await new HttpRequestNode(factory.Factory).RunAsync(Context(new { url = $"http://localhost:{front.Port}/token", method = "POST", body = new { client_secret = "FAKE-BODY-SECRET" } }), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Success, result.Error); + result.Outputs["status"].GetInt32().ShouldBe(307, "the redirect comes back to the workflow, which can follow it deliberately"); + collector.Accepted.ShouldBe(0, "no socket may carry the author's body to the redirect's origin"); + } + + [Theory] + [InlineData("::1", "v4-first.example.test", "127.0.0.1", "::1")] + [InlineData("127.0.0.0/8", "v6-first.example.test", "::1", "127.0.0.1")] + public async Task A_mixed_answer_is_dialled_only_at_the_addresses_the_guard_permits(string allowed, string host, string refused, string permitted) + { + // The DNS-rebinding shape at socket level: one name answering with an address the guard refuses AND one it + // admits, the refused one FIRST. Both listen on the same port, so only the dial decides where the socket goes — + // a guard that checked the answer but dialled all of it would land on the refused address. + await using var service = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{}")); + + if (!service.ListensOnIpv6) return; // a host without IPv6 loopback cannot stage a two-family answer + + using var factory = GuardedFactory(new OutboundDestinationGuard(OutboundDestinationAllowlist.Parse(new[] { allowed }), Direct, TestDns)); + + using var response = await factory.Client.GetAsync($"http://{host}:{service.Port}/"); + + response.IsSuccessStatusCode.ShouldBeTrue(); + service.AcceptedOn(IPAddress.Parse(refused)).ShouldBe(0, $"{host} answered {refused} first, and the guard refuses it — no socket may be dialled there"); + service.AcceptedOn(IPAddress.Parse(permitted)).ShouldBe(1); + } + + [Fact] + public async Task A_public_destination_is_reached_through_the_operators_proxy() + { + // A worker that reaches the internet only through HTTP(S)_PROXY. The proxy's own address is internal (loopback + // here) and is dialled as the operator's proxy; the public destination behind it answers. 203.0.113.10 is + // TEST-NET-3: public to the guard and routed nowhere, so a regression that dialled it directly reaches no host. + await using var proxy = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{\"via\":\"operator-proxy\"}")); + using var factory = GuardedFactory(ProxiedGuard(proxy)); + + using var response = await factory.Client.GetAsync("http://api.example.test/v1/items"); + + (await response.Content.ReadAsStringAsync()).ShouldBe("{\"via\":\"operator-proxy\"}"); + proxy.Requests.Single().ShouldStartWith("GET http://api.example.test/v1/items HTTP/1.1", customMessage: "the proxy is asked for the destination itself"); + } + + [Theory] + [InlineData("http://metadata.example.test/latest/meta-data/")] + [InlineData("https://internal.example.test/admin")] // would be a CONNECT tunnel + [InlineData("http://127.0.0.1:8080/admin")] + [InlineData("http://[::ffff:10.0.0.5]/admin")] + public async Task A_request_the_proxy_would_carry_to_an_internal_address_is_refused_before_it_is_sent(string url) + { + // Through a proxy the connect-time check sees only the proxy, so the destination is checked before the request + // leaves: the proxy must never be asked to fetch it. + await using var proxy = LoopbackResponder.Start(_ => LoopbackResponder.Ok("{}")); + using var factory = GuardedFactory(ProxiedGuard(proxy)); + + var ex = await Should.ThrowAsync(() => factory.Client.GetAsync(url)); + + ex.InnerException.ShouldBeOfType(ex.Message); + proxy.Accepted.ShouldBe(0, $"the proxy must not be asked for {url}"); + } + + [Fact] + public async Task A_redirect_through_the_proxy_to_an_internal_address_is_refused_at_that_hop() + { + await using var proxy = LoopbackResponder.Start(_ => LoopbackResponder.Redirect("http://metadata.example.test/latest/meta-data/")); + using var factory = GuardedFactory(ProxiedGuard(proxy)); + + var ex = await Should.ThrowAsync(() => factory.Client.GetAsync("http://api.example.test/feature-x")); + + ex.InnerException.ShouldBeOfType(ex.Message); + proxy.Requests.Count.ShouldBe(1, "only the public first hop went through the proxy"); + } + + [Fact] + public void A_malformed_committed_entry_fails_the_registration_not_the_first_request() + { + // The host registers the client from configuration at boot, so a bad entry stops the host from starting instead + // of failing every http.request — public ones included — at run time. + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary { [$"{OutboundHttpAllowlistSetting.ConfigurationKey}:0"] = "internal-api.corp:8080" }).Build(); + + var ex = Should.Throw(() => new ServiceCollection().AddGuardedHttpClient(nameof(HttpRequestNode), configuration)); + + ex.Message.ShouldContain("internal-api.corp:8080"); + } + + private static string RequestTo(LoopbackResponder responder, string path) => responder.Requests.Single(head => head.StartsWith($"GET {path} ", StringComparison.Ordinal)); + + private static NodeRunContext Context(object inputs, string triggerBody = "") + { + var raw = JsonSerializer.SerializeToElement(inputs); + var scope = new NodeRunScope { Trigger = new Dictionary { ["body"] = JsonSerializer.SerializeToElement(triggerBody) } }; + var empty = JsonSerializer.SerializeToElement(new { }); + + return new NodeRunContext + { + Inputs = VariableResolver.ResolveBag(raw, scope), + Config = new Dictionary { ["timeoutSeconds"] = JsonSerializer.SerializeToElement(5) }, + RawConfig = empty, + RawInputs = raw, + Scope = scope, + Logger = NullLogger.Instance, + Observability = NodeObservability.NoOp, + }; + } + + /// No proxy address, so bypasses every destination and each request connects directly through the guard. + private static readonly IWebProxy Direct = new WebProxy(); + + /// The names these tests resolve. 203.0.113.10 is TEST-NET-3: public to the guard, routed nowhere. + private static readonly Dictionary StubbedNames = new() + { + ["api.example.test"] = new[] { "203.0.113.10" }, + ["metadata.example.test"] = new[] { "169.254.169.254" }, + ["internal.example.test"] = new[] { "10.0.0.5" }, + ["v4-first.example.test"] = new[] { "127.0.0.1", "::1" }, + ["v6-first.example.test"] = new[] { "::1", "127.0.0.1" }, + }; + + private static Task TestDns(string host, CancellationToken _) => StubbedNames.TryGetValue(host, out var answer) ? Task.FromResult(answer.Select(IPAddress.Parse).ToArray()) : Task.FromException(new SocketException((int)SocketError.HostNotFound)); + + /// A guard whose operator proxy is , as HTTP(S)_PROXY would name it, with nothing allowlisted. + private static OutboundDestinationGuard ProxiedGuard(LoopbackResponder proxy) => new(OutboundDestinationAllowlist.Empty, new WebProxy($"http://127.0.0.1:{proxy.Port}"), TestDns); + + /// The production registration over a guard admitting , connecting directly and resolving through the system resolver. + private static GuardedClientFactory GuardedFactory(params string[] allowed) => GuardedFactory(new OutboundDestinationGuard(OutboundDestinationAllowlist.Parse(allowed), Direct)); + + private static GuardedClientFactory GuardedFactory(OutboundDestinationGuard guard) + { + var services = new ServiceCollection(); + services.AddGuardedHttpClient(nameof(HttpRequestNode), guard); + + return new GuardedClientFactory(services.BuildServiceProvider()); + } + + private sealed class GuardedClientFactory : IDisposable + { + private readonly ServiceProvider _provider; + + public GuardedClientFactory(ServiceProvider provider) + { + _provider = provider; + Factory = provider.GetRequiredService(); + Client = Factory.CreateClient(nameof(HttpRequestNode)); + Client.Timeout = TimeSpan.FromSeconds(10); + } + + public IHttpClientFactory Factory { get; } + + public HttpClient Client { get; } + + public void Dispose() + { + Client.Dispose(); + _provider.Dispose(); + } + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OutboundHttp/LoopbackResponder.cs b/backend/tests/CodeSpace.UnitTests/OutboundHttp/LoopbackResponder.cs new file mode 100644 index 000000000..4e8232a3d --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OutboundHttp/LoopbackResponder.cs @@ -0,0 +1,137 @@ +using System.Collections.Concurrent; +using System.Net; +using System.Net.Sockets; +using System.Text; + +namespace CodeSpace.UnitTests.OutboundHttp; + +/// +/// A raw HTTP/1.1 responder on 127.0.0.1 AND ::1 at one ephemeral port, standing in for a service on the worker's own +/// loopback (a metadata endpoint, an admin port) or for an operator's proxy. It counts ACCEPTED CONNECTIONS, not just +/// requests, and per listening address, so a test can prove the guard refused before any socket reached it — a +/// refused-at-connect destination and one that answered would otherwise look alike from the client — and which of the +/// two addresses a dial landed on. It accepts any Host header, so every spelling of loopback reaches it. +/// +internal sealed class LoopbackResponder : IAsyncDisposable +{ + private readonly List _listeners; + private readonly Func _respond; + private readonly CancellationTokenSource _stop = new(); + private readonly ConcurrentQueue _requests = new(); + private readonly int[] _accepted; + + private LoopbackResponder(List listeners, Func respond) + { + _listeners = listeners; + _respond = respond; + _accepted = new int[listeners.Count]; + Port = ((IPEndPoint)listeners[0].LocalEndpoint).Port; + + for (var i = 0; i < listeners.Count; i++) _ = AcceptLoopAsync(listeners[i], i); + } + + public int Port { get; } + + public int Accepted => Enumerable.Range(0, _accepted.Length).Sum(i => Volatile.Read(ref _accepted[i])); + + /// Connections accepted on the listener bound to ( or ). + public int AcceptedOn(IPAddress address) => Volatile.Read(ref _accepted[_listeners.FindIndex(listener => ((IPEndPoint)listener.LocalEndpoint).Address.Equals(address))]); + + /// Whether the ::1 twin is listening — false on a host without IPv6 loopback. + public bool ListensOnIpv6 => _listeners.Count > 1; + + /// Each request's head (request line + headers) in arrival order. + public IReadOnlyCollection Requests => _requests; + + /// Start a responder whose reply to each request head is 's raw HTTP response. + public static LoopbackResponder Start(Func respond) + { + var v4 = new TcpListener(IPAddress.Loopback, 0); + v4.Start(); + + return new LoopbackResponder(new List { v4 }.Concat(TryListenIpv6(((IPEndPoint)v4.LocalEndpoint).Port)).ToList(), respond); + } + + /// A 200 with a JSON body; is one more raw header line (a Set-Cookie, say). + public static string Ok(string body, string? extraHeader = null) => $"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n{(extraHeader == null ? "" : extraHeader + "\r\n")}Content-Length: {Encoding.UTF8.GetByteCount(body)}\r\nConnection: close\r\n\r\n{body}"; + + public static string Redirect(string location, int status = 302) => $"HTTP/1.1 {status} Redirect\r\nLocation: {location}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"; + + /// The IPv6 twin on the same port, when this host has IPv6 loopback. Without it a [::1] request would fail with "connection refused" even if the guard let it through — hiding the very bug the test looks for. + private static IEnumerable TryListenIpv6(int port) + { + if (!Socket.OSSupportsIPv6) return Array.Empty(); + + try + { + var v6 = new TcpListener(IPAddress.IPv6Loopback, port); + v6.Start(); + return new[] { v6 }; + } + catch (SocketException) + { + return Array.Empty(); + } + } + + private async Task AcceptLoopAsync(TcpListener listener, int index) + { + while (!_stop.IsCancellationRequested) + { + TcpClient client; + + try { client = await listener.AcceptTcpClientAsync(_stop.Token).ConfigureAwait(false); } + catch (Exception) { return; } + + Interlocked.Increment(ref _accepted[index]); + _ = ServeAsync(client); + } + } + + private async Task ServeAsync(TcpClient client) + { + using var _ = client; + + try + { + var stream = client.GetStream(); + var head = await ReadHeadAsync(stream).ConfigureAwait(false); + + _requests.Enqueue(head); + + await stream.WriteAsync(Encoding.UTF8.GetBytes(_respond(head)), _stop.Token).ConfigureAwait(false); + } + catch (Exception) + { + // A client that hung up mid-request is not this responder's failure to report. + } + } + + private async Task ReadHeadAsync(NetworkStream stream) + { + var received = new StringBuilder(); + var buffer = new byte[8192]; + + while (!received.ToString().Contains("\r\n\r\n")) + { + var read = await stream.ReadAsync(buffer, _stop.Token).ConfigureAwait(false); + + if (read == 0) break; + + received.Append(Encoding.ASCII.GetString(buffer, 0, read)); + } + + return received.ToString(); + } + + public ValueTask DisposeAsync() + { + _stop.Cancel(); + + foreach (var listener in _listeners) listener.Stop(); + + _stop.Dispose(); + + return ValueTask.CompletedTask; + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OutboundHttp/OriginScopedRedirectHandlerTests.cs b/backend/tests/CodeSpace.UnitTests/OutboundHttp/OriginScopedRedirectHandlerTests.cs new file mode 100644 index 000000000..034e21c27 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OutboundHttp/OriginScopedRedirectHandlerTests.cs @@ -0,0 +1,169 @@ +using System.Net; +using System.Text; +using CodeSpace.Core.Services.OutboundHttp; +using Shouldly; + +namespace CodeSpace.UnitTests.OutboundHttp; + +/// +/// The redirect follower in isolation, over a scripted inner handler that records every hop it is asked to send. The +/// built-in follower forwarded every author header except Authorization to a different origin; these pin that +/// a cross-origin hop now carries none of them, and that the rest of the built-in behaviour (method rewrite, no +/// https→http downgrade, a bounded hop count) is unchanged. +/// +[Trait("Category", "Unit")] +public class OriginScopedRedirectHandlerTests +{ + [Theory] + [InlineData(HttpStatusCode.Found, "http://front.example.test/a", "http://other.example.test/b")] // host changes + [InlineData(HttpStatusCode.Found, "http://front.example.test/a", "http://front.example.test:8080/b")] // port changes + [InlineData(HttpStatusCode.Found, "http://front.example.test/a", "https://front.example.test/b")] // scheme changes — an http→https move on the same host too: the author re-points the URL at https to keep them + [InlineData(HttpStatusCode.TemporaryRedirect, "http://front.example.test/a", "http://other.example.test/b")] // a body-less 307 is still followed + public async Task A_cross_origin_redirect_carries_none_of_the_author_headers(HttpStatusCode status, string from, string to) + { + var inner = new ScriptedHandler(Redirect(status, to), Ok()); + + using var response = await Send(inner, AuthoredRequest(HttpMethod.Get, from)); + + response.StatusCode.ShouldBe(HttpStatusCode.OK); + inner.Hops[1].Uri.ShouldBe(new Uri(to)); + inner.Hops[1].Headers.ShouldBeEmpty("a hop to a different origin must not carry X-Api-Key, PRIVATE-TOKEN, Cookie or Authorization"); + } + + [Fact] + public async Task A_same_origin_redirect_keeps_the_author_headers() + { + var inner = new ScriptedHandler(Redirect(HttpStatusCode.MovedPermanently, "/v2/items"), Ok()); + + using var response = await Send(inner, AuthoredRequest(HttpMethod.Get, "http://api.example.test/v1/items")); + + response.StatusCode.ShouldBe(HttpStatusCode.OK); + inner.Hops[1].Uri.ShouldBe(new Uri("http://api.example.test/v2/items"), "a relative Location resolves against the current URL"); + inner.Hops[1].Headers.Keys.ShouldBe(new[] { "Authorization", "Cookie", "PRIVATE-TOKEN", "X-Api-Key" }, ignoreOrder: true); + } + + [Theory] + [InlineData(HttpStatusCode.SeeOther, "POST", "GET", false)] + [InlineData(HttpStatusCode.Found, "POST", "GET", false)] + [InlineData(HttpStatusCode.MovedPermanently, "POST", "GET", false)] + [InlineData(HttpStatusCode.SeeOther, "PUT", "GET", false)] + [InlineData(HttpStatusCode.TemporaryRedirect, "POST", "POST", true)] + [InlineData(HttpStatusCode.PermanentRedirect, "PUT", "PUT", true)] + [InlineData(HttpStatusCode.Found, "PUT", "PUT", true)] + public async Task A_redirect_keeps_or_rewrites_the_method_like_the_builtin_follower(HttpStatusCode status, string method, string expectedMethod, bool keepsBody) + { + var inner = new ScriptedHandler(Redirect(status, "/next"), Ok()); + var request = AuthoredRequest(new HttpMethod(method), "http://api.example.test/start"); + request.Content = new StringContent("{\"a\":1}", Encoding.UTF8, "application/json"); + + using var response = await Send(inner, request); + + inner.Hops[1].Method.ShouldBe(expectedMethod); + (inner.Hops[1].Body != null).ShouldBe(keepsBody); + } + + [Theory] + [InlineData(HttpStatusCode.TemporaryRedirect, "POST")] + [InlineData(HttpStatusCode.PermanentRedirect, "PUT")] + [InlineData(HttpStatusCode.Found, "PUT")] // 300/301/302 keep the body for every method but POST + [InlineData(HttpStatusCode.MovedPermanently, "PATCH")] + public async Task A_redirect_that_would_carry_the_body_to_another_origin_is_returned_unfollowed(HttpStatusCode status, string method) + { + // Dropping the headers is not enough when the method keeps its body: an OAuth token request or a webhook POST + // carries its secret there. Re-sending it without the body would be a different request, so the 3xx comes back. + var inner = new ScriptedHandler(Redirect(status, "http://collector.example.test/collect"), Ok()); + var request = AuthoredRequest(new HttpMethod(method), "http://api.example.test/token"); + request.Content = new StringContent("{\"client_secret\":\"FAKE-BODY-SECRET\"}", Encoding.UTF8, "application/json"); + + using var response = await Send(inner, request); + + response.StatusCode.ShouldBe(status); + inner.Hops.Count.ShouldBe(1, "the body never reaches the other origin"); + } + + [Theory] + [InlineData("https://api.example.test/a", "http://api.example.test/b")] // downgrade + [InlineData("http://api.example.test/a", "ftp://api.example.test/b")] // not http + [InlineData("http://api.example.test/a", "file:///etc/passwd")] // not http + public async Task A_redirect_the_builtin_follower_refuses_is_returned_unfollowed(string from, string to) + { + var inner = new ScriptedHandler(Redirect(HttpStatusCode.Found, to)); + + using var response = await Send(inner, AuthoredRequest(HttpMethod.Get, from)); + + response.StatusCode.ShouldBe(HttpStatusCode.Found); + inner.Hops.Count.ShouldBe(1); + } + + [Fact] + public async Task A_redirect_loop_stops_after_the_hop_limit_and_returns_the_last_redirect() + { + var inner = new ScriptedHandler(Enumerable.Repeat(Redirect(HttpStatusCode.Found, "/again"), 50).ToArray()); + + using var response = await Send(inner, AuthoredRequest(HttpMethod.Get, "http://api.example.test/start")); + + response.StatusCode.ShouldBe(HttpStatusCode.Found); + inner.Hops.Count.ShouldBe(OriginScopedRedirectHandler.MaxRedirects + 1, "the first request plus MaxRedirects follow-ups, then stop"); + } + + [Fact] + public async Task A_response_that_is_not_a_redirect_is_returned_as_is() + { + var inner = new ScriptedHandler(() => new HttpResponseMessage(HttpStatusCode.NotModified)); + + using var response = await Send(inner, AuthoredRequest(HttpMethod.Get, "http://api.example.test/x")); + + response.StatusCode.ShouldBe(HttpStatusCode.NotModified); + inner.Hops.Count.ShouldBe(1); + } + + private static async Task Send(ScriptedHandler inner, HttpRequestMessage request) + { + using var invoker = new HttpMessageInvoker(new OriginScopedRedirectHandler { InnerHandler = inner }); + + return await invoker.SendAsync(request, CancellationToken.None); + } + + private static HttpRequestMessage AuthoredRequest(HttpMethod method, string url) + { + var request = new HttpRequestMessage(method, url); + + request.Headers.TryAddWithoutValidation("X-Api-Key", "FAKE-TEAM-API-KEY"); + request.Headers.TryAddWithoutValidation("PRIVATE-TOKEN", "FAKE-PRIVATE-TOKEN"); + request.Headers.TryAddWithoutValidation("Cookie", "sid=FAKE-COOKIE"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer FAKE-BEARER"); + + return request; + } + + private static Func Redirect(HttpStatusCode status, string location) => () => + { + var response = new HttpResponseMessage(status); + response.Headers.Location = new Uri(location, UriKind.RelativeOrAbsolute); + return response; + }; + + private static Func Ok() => () => new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("ok") }; + + private sealed record Hop(Uri Uri, string Method, Dictionary Headers, string? Body); + + /// Answers each hop with the next scripted response, recording what the hop carried. + private sealed class ScriptedHandler : HttpMessageHandler + { + private readonly Queue> _responses; + + public ScriptedHandler(params Func[] responses) { _responses = new Queue>(responses); } + + public List Hops { get; } = new(); + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + var headers = request.Headers.ToDictionary(h => h.Key, h => string.Join(",", h.Value)); + var body = request.Content == null ? null : await request.Content.ReadAsStringAsync(cancellationToken); + + Hops.Add(new Hop(request.RequestUri!, request.Method.Method, headers, body)); + + return _responses.Dequeue()(); + } + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationAllowlistTests.cs b/backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationAllowlistTests.cs new file mode 100644 index 000000000..6b5c21d45 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationAllowlistTests.cs @@ -0,0 +1,98 @@ +using System.Net; +using CodeSpace.Core.Services.OutboundHttp; +using CodeSpace.Core.Settings.OutboundHttp; +using Microsoft.Extensions.Configuration; +using Shouldly; + +namespace CodeSpace.UnitTests.OutboundHttp; + +/// +/// The operator's committed allowlist is the only way an internal destination becomes reachable, so its parsing is +/// strict: an entry that is neither a host name, an address nor a CIDR fails the setting rather than being skipped, +/// because a skipped entry is a destination the operator believes is admitted and is not. +/// +[Trait("Category", "Unit")] +public class OutboundDestinationAllowlistTests +{ + [Fact] + public void ConfigurationKey_ConstantNamePinned() + { + // Renaming this key silently empties every deployment's allowlist that pinned the old name, and their + // intentional internal targets start failing. Hard-pin it so a rename is a visible decision. + OutboundHttpAllowlistSetting.ConfigurationKey.ShouldBe("OutboundHttp:AllowedInternalDestinations"); + } + + [Theory] + [InlineData("internal-api.corp", "internal-api.corp", "10.1.2.3")] // host entry admits whatever the name resolves to + [InlineData("Internal-API.corp", "internal-api.corp", "10.1.2.3")] // host names compare case-insensitively + [InlineData("10.0.0.0/8", "anything.corp", "10.200.0.7")] // CIDR entry admits an address inside it + [InlineData("10.1.2.3", "10.1.2.3", "10.1.2.3")] // a bare address is a single-address network + [InlineData("fd00::/8", "v6.corp", "fd12::5")] // IPv6 CIDR + [InlineData("::1", "[::1]", "::1")] // bare IPv6 address + [InlineData("10.0.0.0/8", "[::ffff:10.1.2.3]", "::ffff:10.1.2.3")] // a v4-mapped address matches its IPv4 network + public void An_allowlisted_destination_is_admitted(string entry, string host, string address) + { + var allowlist = OutboundDestinationAllowlist.Parse(new[] { entry }); + + allowlist.Admits(host, IPAddress.Parse(address)).ShouldBeTrue($"'{entry}' must admit {host} → {address}"); + } + + [Theory] + [InlineData("internal-api.corp", "other.corp", "10.1.2.3")] // a different host name + [InlineData("internal-api.corp", "api.internal-api.corp", "10.1.2.3")] // no implicit subdomains + [InlineData("10.0.0.0/8", "anything.corp", "11.0.0.1")] // outside the CIDR + [InlineData("10.1.2.3", "10.1.2.4", "10.1.2.4")] // a neighbour of a single address + [InlineData("127.0.0.1", "[::1]", "::1")] // IPv4 loopback does not admit IPv6 loopback + public void A_destination_outside_the_allowlist_is_not_admitted(string entry, string host, string address) + { + var allowlist = OutboundDestinationAllowlist.Parse(new[] { entry }); + + allowlist.Admits(host, IPAddress.Parse(address)).ShouldBeFalse($"'{entry}' must not admit {host} → {address}"); + } + + [Fact] + public void The_empty_allowlist_admits_nothing() + { + OutboundDestinationAllowlist.Empty.Admits("localhost", IPAddress.Loopback).ShouldBeFalse(); + } + + [Theory] + [InlineData("not a host")] + [InlineData("http://internal.corp")] + [InlineData("10.0.0.0/99")] + [InlineData("internal.corp:8080")] + public void A_malformed_entry_fails_the_setting(string entry) + { + var ex = Should.Throw(() => OutboundDestinationAllowlist.Parse(new[] { entry })); + + ex.Message.ShouldContain(entry); + ex.Message.ShouldContain(OutboundHttpAllowlistSetting.ConfigurationKey); + } + + [Fact] + public void The_setting_reads_the_committed_array_once() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + [$"{OutboundHttpAllowlistSetting.ConfigurationKey}:0"] = " internal-api.corp ", + [$"{OutboundHttpAllowlistSetting.ConfigurationKey}:1"] = "", + [$"{OutboundHttpAllowlistSetting.ConfigurationKey}:2"] = "192.168.10.0/24", + }) + .Build(); + + var allowlist = new OutboundHttpAllowlistSetting(configuration).Value; + + allowlist.Admits("internal-api.corp", IPAddress.Parse("10.9.9.9")).ShouldBeTrue("a trimmed host entry is admitted"); + allowlist.Admits("printer.lan", IPAddress.Parse("192.168.10.40")).ShouldBeTrue("the CIDR entry is admitted"); + allowlist.Admits("localhost", IPAddress.Loopback).ShouldBeFalse("a blank entry admits nothing"); + } + + [Fact] + public void An_absent_setting_is_the_empty_allowlist() + { + var allowlist = new OutboundHttpAllowlistSetting(new ConfigurationBuilder().Build()).Value; + + allowlist.Admits("localhost", IPAddress.Loopback).ShouldBeFalse(); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationGuardTests.cs b/backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationGuardTests.cs new file mode 100644 index 000000000..510050926 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OutboundHttp/OutboundDestinationGuardTests.cs @@ -0,0 +1,77 @@ +using System.Net; +using CodeSpace.Core.Services.OutboundHttp; +using CodeSpace.Core.Settings.OutboundHttp; +using CodeSpace.Messages.Failures; +using Shouldly; + +namespace CodeSpace.UnitTests.OutboundHttp; + +/// +/// The guard's decision over a host's resolved addresses, with the resolution stubbed. That the guard then dials +/// exactly the addresses returns — so what it keeps here is what a +/// connection can reach — is pinned at socket level by +/// . +/// +[Trait("Category", "Unit")] +public class OutboundDestinationGuardTests +{ + [Fact] + public void A_name_resolving_to_public_addresses_is_dialled_at_those_addresses() + { + var resolved = Addresses("93.184.216.34", "2606:2800:220:1:248:1893:25c8:1946"); + + var permitted = Guard(OutboundDestinationAllowlist.Empty).Permit("api.example.test", resolved); + + permitted.ShouldBe(resolved); + } + + [Fact] + public void A_mixed_answer_is_dialled_only_at_its_public_addresses() + { + // The DNS-rebinding shape: one name answering with both a public and an internal address. Only the public one + // may be dialled — the connection cannot land on the internal one however the answers are ordered. + var permitted = Guard(OutboundDestinationAllowlist.Empty).Permit("rebind.example.test", Addresses("127.0.0.1", "93.184.216.34", "169.254.169.254")); + + permitted.ShouldBe(Addresses("93.184.216.34")); + } + + [Theory] + [InlineData("metadata.example.test", "169.254.169.254")] + [InlineData("localhost", "::1", "127.0.0.1")] + [InlineData("lan.example.test", "10.0.0.5", "fd00::5")] + [InlineData("nothing.example.test")] + public void A_name_resolving_only_to_internal_addresses_is_refused(string host, params string[] resolved) + { + var ex = Should.Throw(() => Guard(OutboundDestinationAllowlist.Empty).Permit(host, Addresses(resolved))); + + ex.Message.ShouldContain(host); + ex.Message.ShouldContain(OutboundHttpAllowlistSetting.ConfigurationKey, customMessage: "the refusal must name the setting that admits an intentional internal target"); + ((IFailure)ex).Kind.ShouldBe(FailureKind.Unprocessable); + ((IFailure)ex).Code.ShouldBe(FailureCodes.OutboundDestinationRefused); + } + + [Fact] + public void The_refusal_does_not_disclose_the_internal_addresses_a_name_resolved_to() + { + // The message reaches the run viewer; echoing 10.20.30.40 back would turn http.request into a resolver for the + // worker's internal DNS. + var ex = Should.Throw(() => Guard(OutboundDestinationAllowlist.Empty).Permit("db.internal.test", Addresses("10.20.30.40"))); + + ex.Message.ShouldNotContain("10.20.30.40"); + } + + [Theory] + [InlineData("internal-api.corp", "internal-api.corp", "10.1.2.3")] + [InlineData("127.0.0.0/8", "localhost", "127.0.0.1")] + [InlineData("fd00::/8", "v6.corp", "fd00::7")] + public void An_allowlisted_internal_destination_is_dialled(string entry, string host, string address) + { + var permitted = Guard(OutboundDestinationAllowlist.Parse(new[] { entry })).Permit(host, Addresses(address)); + + permitted.ShouldBe(Addresses(address)); + } + + private static OutboundDestinationGuard Guard(OutboundDestinationAllowlist allowlist) => new(allowlist, new WebProxy()); + + private static IPAddress[] Addresses(params string[] addresses) => addresses.Select(IPAddress.Parse).ToArray(); +} diff --git a/backend/tests/CodeSpace.UnitTests/OutboundHttp/PublicAddressTests.cs b/backend/tests/CodeSpace.UnitTests/OutboundHttp/PublicAddressTests.cs new file mode 100644 index 000000000..c82ebce4b --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OutboundHttp/PublicAddressTests.cs @@ -0,0 +1,63 @@ +using System.Net; +using CodeSpace.Core.Services.OutboundHttp; +using Shouldly; + +namespace CodeSpace.UnitTests.OutboundHttp; + +/// +/// Pins which resolved addresses a guarded client may dial. Every refused row is a way to reach the worker's own +/// loopback, its cloud metadata service, or its LAN; the IPv6 rows include each form that EMBEDS an IPv4 address, +/// because a dual-stack socket connects ::ffff:127.0.0.1 straight to 127.0.0.1. +/// +[Trait("Category", "Unit")] +public class PublicAddressTests +{ + [Theory] + [InlineData("127.0.0.1")] // loopback + [InlineData("127.255.255.254")] // the whole of 127/8 + [InlineData("0.0.0.0")] // "this host" — connects to loopback on Linux and macOS + [InlineData("10.1.2.3")] // RFC 1918 + [InlineData("172.16.0.1")] // RFC 1918 + [InlineData("192.168.1.1")] // RFC 1918 + [InlineData("169.254.169.254")] // link-local: AWS/GCP/Azure metadata + [InlineData("100.100.100.200")] // CGNAT range: Aliyun metadata + [InlineData("100.64.0.1")] // CGNAT + [InlineData("198.18.0.1")] // benchmarking, holds the sandbox's per-run /30s + [InlineData("224.0.0.1")] // multicast + [InlineData("255.255.255.255")] // broadcast + [InlineData("::")] // unspecified + [InlineData("::1")] // IPv6 loopback + [InlineData("::ffff:127.0.0.1")] // v4-mapped loopback + [InlineData("::ffff:169.254.169.254")] // v4-mapped metadata + [InlineData("::ffff:10.0.0.1")] // v4-mapped RFC 1918 + [InlineData("::127.0.0.1")] // v4-compatible loopback + [InlineData("64:ff9b::7f00:1")] // NAT64 of 127.0.0.1 + [InlineData("64:ff9b::a9fe:a9fe")] // NAT64 of 169.254.169.254 + [InlineData("2002:7f00:1::1")] // 6to4 of 127.0.0.1 + [InlineData("2002:a9fe:a9fe::1")] // 6to4 of 169.254.169.254 + [InlineData("fe80::1")] // link-local + [InlineData("fec0::1")] // deprecated site-local + [InlineData("fc00::1")] // unique local + [InlineData("fd00:ec2::254")] // unique local: AWS metadata over IPv6 + [InlineData("ff02::1")] // multicast + [InlineData("2001:db8::1")] // documentation + [InlineData("2001::1")] // Teredo, which carries an obfuscated IPv4 + public void A_non_public_address_is_refused(string address) + { + PublicAddress.IsPublic(IPAddress.Parse(address)).ShouldBeFalse($"{address} reaches the worker's own loopback, metadata or network"); + } + + [Theory] + [InlineData("93.184.216.34")] + [InlineData("8.8.8.8")] + [InlineData("1.1.1.1")] + [InlineData("2606:4700:4700::1111")] + [InlineData("2001:4860:4860::8888")] + [InlineData("::ffff:8.8.8.8")] // v4-mapped public stays public + [InlineData("64:ff9b::808:808")] // NAT64 of a public address: how an IPv6-only cluster reaches IPv4 hosts + [InlineData("2002:808:808::1")] // 6to4 of a public address + public void A_public_address_is_allowed(string address) + { + PublicAddress.IsPublic(IPAddress.Parse(address)).ShouldBeTrue($"{address} is a globally routable destination"); + } +}