From ddbaa211dfd0e87d2e509ec36df01c1b00bb5381 Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 14:52:16 -0400 Subject: [PATCH 1/4] fix: close host-scoping deny-list evasion via alternate encodings (#635) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CapabilityEnforcer's NormalizeHostForMatch compared requested hosts and configured deny/allow patterns as raw strings, so a deny entry could be evaded by spelling the same address differently: decimal/hex/short-form IPv4 (2130706433, 0x7f.0.0.1, 127.1), a Unicode label separator or zero-width character (evil。com), a bracketed or zone-qualified IPv6 literal ([::1], fe80::1%eth0), or IPv4-mapped IPv6 (::ffff:127.0.0.1 for 127.0.0.1 — found by code review, verified live against the compiled enforcer). Now canonicalizes through Uri.IdnHost (what the real HTTP client actually connects with) plus explicit zone-id truncation and IPv4-mapped-IPv6 collapse, mirroring the normalization CompositeHookExecutor.IsReservedAddress already does for the same address class. Also adds an advisory warning when a configured deny/allow entry can never match anything (a typo'd pattern silently became a permanent no-op before). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../Sandbox/CapabilityEnforcer.HostScoping.cs | 180 ++++++++++++- .../Behaviors/CapabilityEnforcementTests.cs | 247 +++++++++++++++++- 2 files changed, 414 insertions(+), 13 deletions(-) diff --git a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs index 76ef3d0a..b434334d 100644 --- a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs +++ b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs @@ -1,3 +1,4 @@ +using System.Net; using Domain.AI.Sandbox; using Domain.Common; using Domain.Common.Helpers; @@ -41,6 +42,15 @@ public sealed partial class CapabilityEnforcer var deniedPatterns = profile.DeniedHosts.Where(h => h is not null).Select(NormalizeHostForMatch).ToList(); var allowedPatterns = profile.AllowedHosts.Where(h => h is not null).Select(NormalizeHostForMatch).ToList(); + // #635 LOW: a typo'd or malformed deny/allow entry normalizes to a string ValidateHost + // would reject on the requested-host side (SecureInputValidatorHelper.ValidateHost is never + // run on the CONFIGURED side, only the requested side, in ValidateHosts below) — silently + // becoming a permanent no-op with no signal to the operator that their configuration is + // inert. Advisory only: log and keep evaluating, since a malformed pattern is harmless (it + // just never matches anything) rather than a reason to fail every call closed. + WarnIfPatternIsInert(toolName, "DeniedHosts", deniedPatterns); + WarnIfPatternIsInert(toolName, "AllowedHosts", allowedPatterns); + if (ValidateHosts(requestedHosts, deniedPatterns, allowedPatterns) is { } hostViolation) { _logger.LogWarning("Tool {ToolName} host denied: {Host}", toolName, hostViolation); @@ -89,7 +99,7 @@ public sealed partial class CapabilityEnforcer /// private static bool HostPatternMatches(string normalizedHost, string normalizedPattern) { - if (normalizedPattern.StartsWith("*.")) + if (HasWildcardPrefix(normalizedPattern)) { var suffix = normalizedPattern[1..]; return normalizedHost.EndsWith(suffix, StringComparison.OrdinalIgnoreCase) @@ -101,23 +111,169 @@ private static bool HostPatternMatches(string normalizedHost, string normalizedP /// /// Reduces a host value — whether a requested host or a configured deny/allow entry — to a bare, - /// comparable host name. A value that parses as an absolute URI is reduced via - /// itself, which correctly drops a scheme, userinfo (user@), port, and path/query in one - /// step — not an ad-hoc scan for "://", which matches the first occurrence anywhere - /// in the string rather than only a leading scheme and so can be pointed at an unrelated embedded - /// URL later in the value. A value that isn't itself an absolute URI falls back to a trailing-port - /// strip and a root-terminating FQDN dot trim. Applying the identical reduction to both sides of a - /// comparison is what keeps an operator's plain "evil.com" - /// entry matching every equivalent spelling of that same host a caller might supply. + /// comparable host name, canonicalized the same way the actual network client + /// (Uri/SocketsHttpHandler) would resolve it — not an ad-hoc scan for + /// "://", which matches the first occurrence anywhere in the string rather than only a + /// leading scheme and so can be pointed at an unrelated embedded URL later in the value. A leading + /// "*." wildcard prefix is preserved verbatim around the normalized remainder so + /// 's suffix check keeps working. /// + /// + /// #635: a bare-shape value (no leading scheme, no '/') is now routed through the same + /// Uri host parser via a synthetic http:// scheme, then reduced by + /// to the connect-time form — closing four + /// deny-list-evasion classes measured against the real .NET BCL and, for the fourth, against the + /// actual compiled : a decimal/hex/short-form IPv4 + /// literal (2130706433, 0x7f.0.0.1, 127.1) that a raw string compare never + /// equated with its dotted-quad deny entry; a Unicode label separator or zero-width character + /// (evil。com, a trailing U+200B) that Uri.Host preserves verbatim but + /// Uri.IdnHost — what the connecting client actually uses — normalizes to plain ASCII; a + /// bracketed/zone-qualified IPv6 literal ([::1], fe80::1%eth0) that previously had + /// up to four spellings that failed to match each other or a bare deny entry; and an IPv4-mapped + /// IPv6 literal (::ffff:127.0.0.1), collapsed to its IPv4 form in + /// . A value containing '/' is deliberately excluded + /// from synthetic-scheme parsing and keeps the legacy fallback: forcing a + /// path/query-shaped value through a scheme would let its leading segment before the first + /// '/' be treated as a host even though it has no leading scheme naming one, which is + /// exactly the confusion + /// AllowedHost_UrlWithEmbeddedSchemeLaterInString_DoesNotMatchTheEmbeddedHost exists to + /// rule out. Verified (throwaway console app against the pinned BCL, 26/26 cases, plus code + /// review's own live probe of the compiled enforcer): every existing test pair still normalizes + /// identically, every deny-list bypass class named above closes, and no unrelated host + /// (notevil.com, xn--vil-9ma.com, a different IP) collides with another. NOT + /// verified exhaustive — this closes the specific classes above, not every conceivable alternate + /// host encoding; treat a new one, if found, as its own gap rather than assuming this comment's + /// list is complete. Also verified: no tool in this repo issues an outbound network request + /// through anything other than Uri/HttpClient today (no raw socket, no shelled + /// curl) — the one process-spawning tool + /// (Infrastructure.AI.Tools.RestrictedSearchTool) is a read-only, network-incapable local + /// shell sandbox — so this canonicalization matches every real consumer that exists, not just the + /// one the issue measured. + /// private static string NormalizeHostForMatch(string value) { var trimmed = value.Trim(); - if (Uri.TryCreate(trimmed, UriKind.Absolute, out var uri) && !string.IsNullOrEmpty(uri.Host)) - return uri.Host.TrimEnd('.'); + return HasWildcardPrefix(trimmed) + ? "*." + NormalizeBareHost(trimmed[2..]) + : NormalizeBareHost(trimmed); + } + + private const string WildcardPrefix = "*."; + + /// + /// Whether carries the a + /// suffix pattern uses — the single shared check for all three + /// call sites, so an ordinal-vs-culture inconsistency between them can't reopen a mismatch of the + /// kind #635 was filed to close. + /// + private static bool HasWildcardPrefix(string value) => value.StartsWith(WildcardPrefix, StringComparison.Ordinal); + + /// + /// Normalizes a value with any leading "*." wildcard prefix already stripped — either a + /// full absolute URI or a bare host[:port]/IP literal, never a wildcard pattern itself. + /// + private static string NormalizeBareHost(string value) + { + if (Uri.TryCreate(value, UriKind.Absolute, out var uri) && !string.IsNullOrEmpty(uri.Host)) + return CanonicalizeParsedHost(uri); + + if (value.Contains('/')) + return StripPort(value).TrimEnd('.'); + + // Bare IPv6 needs brackets to be syntactically valid inside a URI ("http://::1/" is not a + // parseable URI); a lone ':' is a port separator (StripPort's job below), not IPv6, so only + // wrap when there's more than one — the same signal StripPort itself uses to leave a bare + // IPv6 literal untouched. + var candidate = !value.StartsWith('[') && value.Count(c => c == ':') > 1 ? $"[{value}]" : value; + + return Uri.TryCreate($"http://{candidate}/", UriKind.Absolute, out var synthetic) && !string.IsNullOrEmpty(synthetic.Host) + ? CanonicalizeParsedHost(synthetic) + : StripPort(value).TrimEnd('.'); + } - return StripPort(trimmed).TrimEnd('.'); + /// + /// A value guarantees fails + /// (its explicit embedded-NUL check), + /// returned when throws instead of falling back to the un-normalized + /// . Falling back to Host would silently reintroduce the exact + /// Unicode-label-separator/zero-width-character evasion class #635 exists to close, on precisely + /// the adversarial input that triggered the exception — code review (#635) found the original + /// silent fallback did exactly this with no signal it had fired. This sentinel instead composes + /// with existing, already-logged machinery: on the requested-host side it makes + /// refuse the call outright (already logged as "host denied"); on a + /// configured pattern it makes that one entry permanently unmatchable, which + /// already logs as an inert-configuration warning. Either way + /// the failure is observable through the logging this file already has, rather than adding a new + /// logging path for an exception verified (empirically, not exhaustively) never to occur. + /// + private const string UnnormalizableHostSentinel = "\0"; + + /// + /// Reduces a successfully-parsed to the bracket-free, zone-free, IPv4-collapsed, + /// connect-time canonical host resolves to. + /// + private static string CanonicalizeParsedHost(Uri uri) + { + string host; + try + { + // Uri.IdnHost, not Uri.Host: Host preserves a Unicode label separator or a bracketed IPv6 + // literal verbatim, which is exactly the class of string a raw compare against a deny + // entry misses (#635). Guarded defensively — verified empirically that IdnHost does not + // throw across every adversarial shape tried (overlong labels, invalid punycode-looking + // input, invalid surrogates, invalid percent-encoding), but a security gate must not + // itself become a crash vector on attacker-controlled input regardless. See + // UnnormalizableHostSentinel for why the fallback on an actual throw is NOT uri.Host. + host = uri.IdnHost; + } + catch (Exception) + { + return UnnormalizableHostSentinel; + } + + if (host.StartsWith('[') && host.EndsWith(']')) + host = host[1..^1]; + + // IdnHost retains an IPv6 zone identifier verbatim, but two spellings of the identical zone + // ("%eth0" bare vs. the percent-escaped "%25eth0") do not string-equal each other — truncate + // it outright rather than trying to normalize its encoding. + var zoneIndex = host.IndexOf('%'); + if (zoneIndex >= 0) + host = host[..zoneIndex]; + + // #635 code review: an IPv4-mapped IPv6 literal ("::ffff:127.0.0.1") is well-formed IPv6, and + // IdnHost does not collapse it to the equivalent IPv4 form — so it never string-equals a plain + // IPv4 deny/allow entry for the identical address. Verified live against the compiled + // enforcer: a DeniedHosts=["127.0.0.1"] entry did not refuse a requested "::ffff:127.0.0.1" + // before this. Mirrors the existing normalization in + // Infrastructure.AI.Hooks.CompositeHookExecutor.IsReservedAddress for the identical address + // class, rather than inventing a second way to do the same collapse. + if (IPAddress.TryParse(host, out var ip) && ip.IsIPv4MappedToIPv6) + host = ip.MapToIPv4().ToString(); + + return host.TrimEnd('.'); + } + + /// + /// Logs one warning per already-normalized entry that + /// would reject — #635 LOW: such an entry + /// can never match any requested host (every requested host is validated the same way in + /// ), so it is a silent, permanent no-op in the operator's + /// configuration unless something says so. + /// + private void WarnIfPatternIsInert(string toolName, string configKey, IReadOnlyList patterns) + { + foreach (var pattern in patterns) + { + var checkValue = HasWildcardPrefix(pattern) ? pattern[2..] : pattern; + if (!SecureInputValidatorHelper.ValidateHost(checkValue)) + { + _logger.LogWarning( + "Tool {ToolName} has a {ConfigKey} entry that normalizes to an invalid host and can never match any requested host: {Pattern}", + toolName, configKey, pattern); + } + } } /// diff --git a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs index 5e3ca428..ef782037 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs @@ -56,6 +56,11 @@ private static ITool MinimalIsolationTool() => Mock.Of(t => private static (ToolPermissionProfileResolver Resolver, CapabilityEnforcer Enforcer) Build( SandboxConfig? config = null, + params (string Name, ITool Tool)[] tools) => Build(config, null, tools); + + private static (ToolPermissionProfileResolver Resolver, CapabilityEnforcer Enforcer) Build( + SandboxConfig? config, + Mock>? logger, params (string Name, ITool Tool)[] tools) { var services = new ServiceCollection(); @@ -68,10 +73,17 @@ private static (ToolPermissionProfileResolver Resolver, CapabilityEnforcer Enfor var lookup = new FirstPartyToolLookup( services.BuildServiceProvider(), new HashSet(tools.Select(t => t.Name))); var resolver = new ToolPermissionProfileResolver(lookup, configMock.Object); - var enforcer = new CapabilityEnforcer(resolver, Mock.Of>()); + var enforcer = new CapabilityEnforcer(resolver, (logger ?? new Mock>()).Object); return (resolver, enforcer); } + private static bool LogsMessageContaining(Mock> logger, string substring) => + logger.Invocations.Any(i => + i.Method.Name == nameof(ILogger.Log) && + i.Arguments.Count > 2 && + i.Arguments[2] is not null && + i.Arguments[2]!.ToString()!.Contains(substring, StringComparison.Ordinal)); + // --- Capability Checks --- [Fact] @@ -743,4 +755,237 @@ public async Task HostScopingConfigured_RequestedHostsIsNull_RefusesFailClosed() result.IsSuccess.Should().BeFalse(); } + + // --- #635: NormalizeHostForMatch bypasses --- + + [Theory] + [InlineData("2130706433")] // decimal IPv4 + [InlineData("127.1")] // short-form IPv4 + [InlineData("0x7f.0.0.1")] // hex-octet IPv4 + public async Task DeniedHost_AlternateIPv4Encoding_StillMatchesDottedQuadDenyEntry(string encodedLoopback) + { + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["127.0.0.1"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: [encodedLoopback]); + + result.IsSuccess.Should().BeFalse( + $"'{encodedLoopback}' is the same address as the denied 127.0.0.1 to the real HTTP client"); + } + + [Theory] + [InlineData("evil。com")] // U+3002 ideographic full stop + [InlineData("evil．com")] // U+FF0E fullwidth full stop + [InlineData("evil.com｡")] // U+FF61 halfwidth ideographic full stop (trailing) + [InlineData("evil.com​")] // trailing zero-width space + public async Task DeniedHost_UnicodeLabelSeparatorOrZeroWidthChar_StillMatchesAsciiDenyEntry(string spoofedHost) + { + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["evil.com"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: [spoofedHost]); + + result.IsSuccess.Should().BeFalse( + "Uri.IdnHost — what the real HTTP client connects with — normalizes this to plain \"evil.com\""); + } + + [Fact] + public async Task DeniedHost_UnicodeLabelSeparator_StillDefeatsWildcardDenyEntry() + { + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["*.evil.com"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["a.evil。com"]); + + result.IsSuccess.Should().BeFalse(); + } + + [Fact] + public async Task DeniedHost_BracketedIPv6Literal_StillMatchesBareDenyEntry() + { + // Before #635: the absolute-URI branch of NormalizeHostForMatch returned Uri.Host verbatim, + // which retains brackets ("[::1]") — a bare deny entry of "::1" never matched a requested + // "http://[::1]/". + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["::1"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["http://[::1]/"]); + + result.IsSuccess.Should().BeFalse(); + } + + [Theory] + [InlineData("fe80::1%eth0")] + [InlineData("[fe80::1%25eth0]")] + public async Task DeniedHost_IPv6ZoneIdentifier_StillMatchesBareDenyEntryWithoutZone(string zoneQualifiedHost) + { + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["fe80::1"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: [zoneQualifiedHost]); + + result.IsSuccess.Should().BeFalse( + "a bare vs. percent-escaped zone id spelling of the same interface must normalize identically"); + } + + [Fact] + public async Task AllowedHost_UnrelatedPrefixCollisionHost_StillRefused() + { + // Guard case: the #635 fix must not become OVER-broad — "notevil.com" must never be treated + // as if it were "evil.com" just because #635's synthetic-scheme parsing now runs on more + // bare-shaped values than before. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { AllowedHosts = ["evil.com"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["notevil.com"]); + + result.IsSuccess.Should().BeFalse(); + } + + [Fact] + public async Task AllowedHost_PunycodeLookalike_DoesNotCollideWithUnicodeDenyEntry() + { + // Guard case: a value that is ALREADY in ASCII/punycode form must not be treated as if + // IdnHost-canonicalizing it could make it collide with an unrelated Unicode-derived host. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { AllowedHosts = ["evil.com"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["xn--vil-9ma.com"]); + + result.IsSuccess.Should().BeFalse(); + } + + [Fact] + public async Task DeniedHost_MalformedConfiguredEntry_LogsInertConfigurationWarning() + { + // #635 LOW: a typo'd deny entry that can never match any requested host previously failed + // silently, with no signal to the operator that their configuration does nothing. + var config = new SandboxConfig + { + ToolOverrides = new() + { + ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["evil.com/*"] } + } + }; + var loggerMock = new Mock>(); + var (_, enforcer) = Build(config, loggerMock, ("http_tool", NetworkFileTool())); + + await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["example.com"]); + + LogsMessageContaining(loggerMock, "can never match any requested host").Should().BeTrue(); + } + + [Fact] + public async Task DeniedHost_WellFormedConfiguredEntry_DoesNotLogInertConfigurationWarning() + { + var config = new SandboxConfig + { + ToolOverrides = new() + { + ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["evil.com"] } // well-formed + } + }; + var loggerMock = new Mock>(); + var (_, enforcer) = Build(config, loggerMock, ("http_tool", NetworkFileTool())); + + await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["example.com"]); + + LogsMessageContaining(loggerMock, "can never match any requested host").Should().BeFalse(); + } + + [Theory] + [InlineData("::ffff:127.0.0.1")] // IPv4-mapped IPv6, colon-hex form + [InlineData("[::ffff:127.0.0.1]")] // same, bracketed as a URI would carry it + public async Task DeniedHost_Ipv4MappedIpv6Literal_StillMatchesPlainIPv4DenyEntry(string mappedForm) + { + // #635 code-review (round 2): a well-formed IPv6 literal encoding the SAME address as a + // plain IPv4 deny entry — Uri.IdnHost does not collapse this form, so it needs its own + // explicit normalization step (mirrors CompositeHookExecutor.IsReservedAddress). + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["127.0.0.1"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: [mappedForm]); + + result.IsSuccess.Should().BeFalse( + $"'{mappedForm}' is the same address as the denied 127.0.0.1 to the real HTTP client"); + } + + [Fact] + public async Task DeniedHost_Ipv4MappedIpv6CloudMetadataAddress_StillMatchesDenyEntry() + { + // The concrete exploit shape from #635's own issue text, restated for the IPv6-mapped form. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["169.254.169.254"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["::ffff:169.254.169.254"]); + + result.IsSuccess.Should().BeFalse(); + } + + [Fact] + public async Task AllowedHost_UnrelatedIpv6_DoesNotCollideWithIpv4MappedNormalization() + { + // Guard case: IPv4-mapped-IPv6 normalization must not over-fire on an ordinary IPv6 address + // that is NOT an IPv4-mapped literal. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { AllowedHosts = ["127.0.0.1"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["2001:db8::1"]); + + result.IsSuccess.Should().BeFalse("an unrelated IPv6 address must not be treated as 127.0.0.1"); + } } From 504a6d9710dd48cf4c2415f41a7f04724f67101f Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 15:16:22 -0400 Subject: [PATCH 2/4] fix: exclude userinfo/fragment/query/backslash from host synthetic-parse, correct overstated doc claim (#635) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run-gates.sh's correctness and security gates found the synthetic-scheme parsing added for #635 silently reduced a bare value carrying '@', '#', '?', or '\' to just its leading host segment, where it was previously refused outright as malformed. Harmless today (matches what the real HTTP client would connect to), but any template consumer's future tool that resolves the raw value some other way (DNS, socket, subprocess) would be checked against a different string than it contacts — excluded alongside '/' rather than left to widen silently. Also corrects an inaccurate doc-comment claim the grader gate flagged: "no tool issues a network request through anything other than Uri/HttpClient" is false (several tools run subprocesses via ISandboxExecutor); the actually-verified fact is that zero production tools declare a Host-kind resource parameter, so EnforceHostScoping never runs with a non-empty requestedHosts today regardless. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../Sandbox/CapabilityEnforcer.HostScoping.cs | 28 ++++++++++++++----- .../Behaviors/CapabilityEnforcementTests.cs | 25 +++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs index b434334d..7c744bcd 100644 --- a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs +++ b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs @@ -143,12 +143,16 @@ private static bool HostPatternMatches(string normalizedHost, string normalizedP /// (notevil.com, xn--vil-9ma.com, a different IP) collides with another. NOT /// verified exhaustive — this closes the specific classes above, not every conceivable alternate /// host encoding; treat a new one, if found, as its own gap rather than assuming this comment's - /// list is complete. Also verified: no tool in this repo issues an outbound network request - /// through anything other than Uri/HttpClient today (no raw socket, no shelled - /// curl) — the one process-spawning tool - /// (Infrastructure.AI.Tools.RestrictedSearchTool) is a read-only, network-incapable local - /// shell sandbox — so this canonicalization matches every real consumer that exists, not just the - /// one the issue measured. + /// list is complete. This Uri-based canonicalization matches every tool that exists TODAY + /// — not because every tool connects via Uri/HttpClient (several run subprocesses — + /// terraform, npm, kubectl — via ISandboxExecutor, which could resolve a host differently), + /// but because grepping every ResourceParametersByOperation declaration in this repo found + /// zero production tools that declare a + /// parameter — never runs with a non-empty requestedHosts + /// today regardless of what a given tool's own consumer does with the value. A template consumer + /// adding a host-taking tool that resolves the raw value through something other than Uri + /// (a raw socket, a DNS lookup, a subprocess) would need this file's normalization to actually + /// match — worth re-verifying at that point rather than assuming this comment still holds. /// private static string NormalizeHostForMatch(string value) { @@ -178,7 +182,17 @@ private static string NormalizeBareHost(string value) if (Uri.TryCreate(value, UriKind.Absolute, out var uri) && !string.IsNullOrEmpty(uri.Host)) return CanonicalizeParsedHost(uri); - if (value.Contains('/')) + // run-gates correctness/security review: not just '/' — a bare value carrying '@' (userinfo), + // '#' (fragment), '?' (query), or '\' (a browser/some URI parsers treat this as '/') would + // otherwise be silently reduced to just the leading host segment by the synthetic-scheme + // parse below, where it was refused outright as malformed before #635. Harmless for a tool + // that builds a web request from the result (the real HTTP client resolves the identical + // leading host), but a template consumer's future tool that feeds the raw value to a DNS + // lookup, a raw socket, or a subprocess would be checked against a different string than it + // actually contacts — the same "checked value must match consumed value" hazard #635 exists + // to close, just for a shape no tool in this repo produces today. Kept alongside '/' in the + // legacy StripPort fallback rather than synthetic-parsed. + if (value.IndexOfAny(['/', '@', '#', '?', '\\']) >= 0) return StripPort(value).TrimEnd('.'); // Bare IPv6 needs brackets to be syntactically valid inside a URI ("http://::1/" is not a diff --git a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs index ef782037..c0e6a783 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs @@ -988,4 +988,29 @@ public async Task AllowedHost_UnrelatedIpv6_DoesNotCollideWithIpv4MappedNormaliz result.IsSuccess.Should().BeFalse("an unrelated IPv6 address must not be treated as 127.0.0.1"); } + + [Theory] + [InlineData("evil.com@allowed.com")] // userinfo + [InlineData("allowed.com#evil.com")] // fragment + [InlineData("allowed.com?evil.com")] // query + [InlineData("allowed.com\\evil.com")] // backslash + public async Task DeniedHostOnly_BareValueWithUserinfoFragmentQueryOrBackslash_StillRefused(string ambiguousValue) + { + // run-gates correctness/security review: these shapes were refused outright as malformed + // before #635 (Uri.CheckHostName rejects them). #635's synthetic-scheme parsing must not + // start silently reducing them to just the leading host segment — kept excluded alongside + // '/' so this file's own normalization never disagrees with a consumer that isn't Uri/ + // HttpClient (a raw socket, a DNS lookup, a subprocess) about which host a value names. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["*.evil.com"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: [ambiguousValue]); + + result.IsSuccess.Should().BeFalse($"'{ambiguousValue}' must stay refused as malformed, not silently reduced to a bare host"); + } } From e92b809aad44f154a2cd8602ef94fc190f59a4eb Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 15:43:43 -0400 Subject: [PATCH 3/4] fix: collapse deprecated IPv4-compatible IPv6 form, correct IdnHost-throw claim (#635) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 code review found a second, distinct alternate-IPv6-encoding bypass beyond the IPv4-mapped form already fixed: the older, RFC-4291-deprecated "IPv4-compatible" form (::a.b.c.d, no ffff prefix) still parses successfully and IPAddress.IsIPv4MappedToIPv6 doesn't recognize it, so a plain DeniedHosts=["127.0.0.1"] entry didn't refuse "::127.0.0.1"/"::7f00:1". Fixed with its own guarded helper rather than a blind "first 12 bytes zero" check — naively taking the last 4 bytes of "::1" (loopback) gives "0.0.0.1", not 127.0.0.1, so loopback and the unspecified address are excluded via the same IPAddress.IsLoopback/ IPv6Any checks the BCL itself uses. Also corrects this file's own doc comment: verified live that Uri.IdnHost DOES throw UriFormatException for a mixed valid-character- plus-invalid-Unicode label, contradicting the earlier "does not throw" claim (that test batch only tried pure-invalid labels, which fail earlier and never reach IdnHost). The existing fail-closed sentinel fallback already handled this correctly — this only fixes what the comment claimed about it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../Sandbox/CapabilityEnforcer.HostScoping.cs | 64 ++++++++++++++++--- .../Behaviors/CapabilityEnforcementTests.cs | 64 +++++++++++++++++++ 2 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs index 7c744bcd..3d0106a9 100644 --- a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs +++ b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs @@ -219,7 +219,9 @@ private static string NormalizeBareHost(string value) /// configured pattern it makes that one entry permanently unmatchable, which /// already logs as an inert-configuration warning. Either way /// the failure is observable through the logging this file already has, rather than adding a new - /// logging path for an exception verified (empirically, not exhaustively) never to occur. + /// logging path for an exception verified (round-2 code review) to actually occur — a mixed + /// valid-character-plus-invalid-Unicode label throws from + /// , not merely a hypothetical this sentinel guards against defensively. /// private const string UnnormalizableHostSentinel = "\0"; @@ -234,11 +236,12 @@ private static string CanonicalizeParsedHost(Uri uri) { // Uri.IdnHost, not Uri.Host: Host preserves a Unicode label separator or a bracketed IPv6 // literal verbatim, which is exactly the class of string a raw compare against a deny - // entry misses (#635). Guarded defensively — verified empirically that IdnHost does not - // throw across every adversarial shape tried (overlong labels, invalid punycode-looking - // input, invalid surrogates, invalid percent-encoding), but a security gate must not - // itself become a crash vector on attacker-controlled input regardless. See - // UnnormalizableHostSentinel for why the fallback on an actual throw is NOT uri.Host. + // entry misses (#635). Round-2 code review found IdnHost DOES throw + // UriFormatException for a mixed valid-character-plus-invalid-Unicode label (verified: + // "a￿.com" — a pure-invalid label fails earlier, at Uri.TryCreate itself, and never + // reaches this property; a mixed one reaches it and throws) — correcting this file's + // earlier, narrower empirical claim. See UnnormalizableHostSentinel for why the fallback + // on that throw is NOT uri.Host. host = uri.IdnHost; } catch (Exception) @@ -263,12 +266,57 @@ private static string CanonicalizeParsedHost(Uri uri) // before this. Mirrors the existing normalization in // Infrastructure.AI.Hooks.CompositeHookExecutor.IsReservedAddress for the identical address // class, rather than inventing a second way to do the same collapse. - if (IPAddress.TryParse(host, out var ip) && ip.IsIPv4MappedToIPv6) - host = ip.MapToIPv4().ToString(); + if (IPAddress.TryParse(host, out var ip)) + { + if (ip.IsIPv4MappedToIPv6) + host = ip.MapToIPv4().ToString(); + else if (TryGetDeprecatedIPv4CompatibleForm(ip, out var ipv4)) + host = ipv4.ToString(); + } return host.TrimEnd('.'); } + /// + /// Round-2 code review: the older, RFC 4291-deprecated "IPv4-compatible" IPv6 form + /// (::a.b.c.d, no ffff prefix — distinct from the IPv4-mapped form + /// already handles above) still parses successfully + /// today and is not covered by that property. Verified: ::127.0.0.1 and its equivalent + /// compressed form ::7f00:1 both parse to the identical address, with + /// IsIPv4MappedToIPv6 false for both — so without this, a plain + /// DeniedHosts=["127.0.0.1"] entry does not refuse either spelling. + /// + /// + /// Deliberately NOT a blind "first 12 bytes zero → take the last 4" check: ::1 (loopback) + /// and :: (unspecified) both have an all-zero first-12-byte prefix too, but are reserved + /// addresses with their own distinct meaning, not IPv4-compatible encodings — verified naively + /// extracting the last 4 bytes of ::1 gives 0.0.0.1, not 127.0.0.1, which + /// would be an outright WRONG collapse (misidentifying loopback as an unrelated address), not + /// merely an incomplete one. Excluded via the same well-tested + /// / checks the BCL itself uses, rather than a hand-rolled + /// special-case list. + /// + private static bool TryGetDeprecatedIPv4CompatibleForm(IPAddress ip, out IPAddress ipv4) + { + ipv4 = IPAddress.None; + + if (ip.AddressFamily != System.Net.Sockets.AddressFamily.InterNetworkV6) + return false; + + if (IPAddress.IsLoopback(ip) || ip.Equals(IPAddress.IPv6Any)) + return false; + + var bytes = ip.GetAddressBytes(); + for (var i = 0; i < 12; i++) + { + if (bytes[i] != 0) + return false; + } + + ipv4 = new IPAddress(bytes[12..]); + return true; + } + /// /// Logs one warning per already-normalized entry that /// would reject — #635 LOW: such an entry diff --git a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs index c0e6a783..6b47f9da 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs @@ -1013,4 +1013,68 @@ public async Task DeniedHostOnly_BareValueWithUserinfoFragmentQueryOrBackslash_S result.IsSuccess.Should().BeFalse($"'{ambiguousValue}' must stay refused as malformed, not silently reduced to a bare host"); } + + [Theory] + [InlineData("::127.0.0.1")] // deprecated IPv4-compatible form, expanded + [InlineData("::7f00:1")] // same address, compressed hex form + public async Task DeniedHost_DeprecatedIpv4CompatibleIpv6Literal_StillMatchesPlainIPv4DenyEntry(string compatibleForm) + { + // #635 round-2 code-review: distinct from the IPv4-MAPPED form ("::ffff:a.b.c.d") already + // covered above — this is the older, RFC 4291-deprecated "IPv4-compatible" form (no "ffff"), + // which IPAddress.IsIPv4MappedToIPv6 does NOT recognize. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["127.0.0.1"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: [compatibleForm]); + + result.IsSuccess.Should().BeFalse( + $"'{compatibleForm}' is the same address as the denied 127.0.0.1 to the real HTTP client"); + } + + [Fact] + public async Task AllowedHost_Ipv6Loopback_IsNotMisidentifiedAsAnUnrelatedIPv4Address() + { + // Guard case: naively taking the last 4 bytes of "::1" (loopback) gives "0.0.0.1", NOT + // "127.0.0.1" — the IPv4-compatible-form collapse must exclude loopback/unspecified rather + // than blindly treating any all-zero-prefixed IPv6 address as IPv4-compatible. Configuring + // the WRONG value ("0.0.0.1") the naive bug would produce, rather than the correct one + // ("127.0.0.1"), so this test actually discriminates: refusing "::1" against a + // "127.0.0.1" allow entry is ALSO the correct outcome, just for a different reason, + // so that pairing can't tell a fixed collapse from a differently-broken one. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { AllowedHosts = ["0.0.0.1"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["::1"]); + + result.IsSuccess.Should().BeFalse("\"::1\" (loopback) must not be collapsed into an unrelated \"0.0.0.1\""); + } + + [Fact] + public async Task DeniedHostOnly_RequestedHostTriggersIdnHostException_StillRefused() + { + // #635 round-2 code-review: verified live that Uri.IdnHost throws UriFormatException for a + // mixed valid-character-plus-invalid-Unicode label — the fail-closed sentinel path + // (CanonicalizeParsedHost's catch block) must refuse the call, not silently admit it. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["*.evil.com"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["a￿.com"]); + + result.IsSuccess.Should().BeFalse(); + } } From 698d550756972afd83a16be41f034d340d1d5ace Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 19:25:28 -0400 Subject: [PATCH 4/4] fix: re-normalize host to a fixed point to catch post-IDNA-fold IPv4 literals (#635) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's security-review gate (running independently on the pushed PR) found a second-order bug in the fix itself: a fullwidth Unicode digit ("２", U+FF12) defeats Uri's own up-front IPv4-literal recognition, so "２852039166" classifies as HostNameType.Dns rather than IPv4. IdnHost then IDNA/NFKC-folds the fullwidth digit to plain ASCII as a side effect of DNS-label normalization, producing "2852039166" — a valid legacy decimal encoding of 169.254.169.254 (the cloud metadata address) — but Uri never re-evaluates HostNameType against its own normalized output, so the value was never collapsed to the dotted-quad form the very first commit in this PR already handles for a plain (all-ASCII) decimal literal. Verified live: re-running the same single-pass normalization on its own output resolves this correctly, since the second pass sees pure ASCII digits up front. NormalizeBareHost now re-normalizes to a fixed point (bounded at 4 iterations, not unconditional) rather than a single pass, so this closes the class of bug regardless of how many confusable/normalization layers an adversarial value stacks. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../Sandbox/CapabilityEnforcer.HostScoping.cs | 32 +++++++++++++++++++ .../Behaviors/CapabilityEnforcementTests.cs | 23 +++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs index 3d0106a9..7eb8eded 100644 --- a/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs +++ b/src/Content/Application/Application.AI.Common/Services/Sandbox/CapabilityEnforcer.HostScoping.cs @@ -177,7 +177,39 @@ private static string NormalizeHostForMatch(string value) /// Normalizes a value with any leading "*." wildcard prefix already stripped — either a /// full absolute URI or a bare host[:port]/IP literal, never a wildcard pattern itself. /// + /// + /// CI security-review (post-merge-attempt) found a second-order gap in this normalization: a + /// non-ASCII digit (a fullwidth ２, U+FF12) defeats 's own up-front IPv4- + /// literal recognition — Uri.TryCreate("http://２852039166/") classifies this as + /// HostNameType.Dns, not IPv4, because the leading character isn't an ASCII digit at + /// parse time. 's IdnHost step then IDNA/NFKC-folds the + /// fullwidth digit to plain ASCII as a side effect of DNS-label normalization — producing + /// "2852039166", a pure-ASCII decimal string that IS a legacy decimal-IPv4 encoding of + /// 169.254.169.254 (the cloud metadata address), but Uri never re-evaluates + /// HostNameType against its own normalized output, so the value is never collapsed to the + /// dotted-quad form the very first #635 fix already handles for a plain (all-ASCII) decimal + /// literal. Verified live: feeding NormalizeBareHostOnce's own output back into itself a + /// second time DOES resolve it correctly — the second pass sees pure ASCII digits up front and + /// Uri recognizes them as IPv4 immediately. below re-runs + /// the single-pass normalization to a fixed point (bounded, not unconditional) specifically to + /// catch this class regardless of how many confusable/normalization layers an adversarial value + /// stacks — not just the two observed here. + /// private static string NormalizeBareHost(string value) + { + var current = value; + for (var i = 0; i < 4; i++) + { + var next = NormalizeBareHostOnce(current); + if (next == current) + return next; + current = next; + } + + return current; + } + + private static string NormalizeBareHostOnce(string value) { if (Uri.TryCreate(value, UriKind.Absolute, out var uri) && !string.IsNullOrEmpty(uri.Host)) return CanonicalizeParsedHost(uri); diff --git a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs index 6b47f9da..6050fb16 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Behaviors/CapabilityEnforcementTests.cs @@ -1077,4 +1077,27 @@ public async Task DeniedHostOnly_RequestedHostTriggersIdnHostException_StillRefu result.IsSuccess.Should().BeFalse(); } + + [Fact] + public async Task DeniedHost_FullwidthDigitEncodedDecimalIPv4_StillMatchesDenyEntry() + { + // CI security-review (post-push, before merge): a fullwidth digit ("２", U+FF12) defeats + // Uri's own up-front IPv4-literal recognition ("２852039166" classifies as HostNameType.Dns, + // not IPv4), so a single normalization pass only IDNA/NFKC-folds it to the plain-ASCII + // decimal string "2852039166" — which IS a legacy decimal encoding of 169.254.169.254 (the + // cloud metadata address) but is never re-evaluated as an IP literal. Verified live: the + // exact concrete exploit from #635's own issue text, restated with a fullwidth leading digit. + var config = new SandboxConfig + { + ToolOverrides = new() { ["http_tool"] = new ToolOverrideConfig { DeniedHosts = ["169.254.169.254"] } } + }; + var (_, enforcer) = Build(config, ("http_tool", NetworkFileTool())); + + var result = await enforcer.EnforceAsync( + "http_tool", ToolCapability.FileRead | ToolCapability.NetworkAccess, + requestedHosts: ["２852039166"]); + + result.IsSuccess.Should().BeFalse( + "the fullwidth leading digit must not survive normalization as a way to hide a decimal-IPv4-encoded deny entry"); + } }