From f4e1251f4e8c82ceb5b92338870f75b4dd1ee28e Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Tue, 29 Sep 2026 19:52:26 +0800 Subject: [PATCH] Retire the gateway path to the model broker Since #2044 every brokered child with a network of its own reaches its broker through the codespace-mcp relay and a per-run Unix socket, so every lease already binds loopback. What was left existed only for a namespaced run launched before that: a re-bind without a socket path bound every address, the source gate admitted 10/8, the setup result carried the veth gateway, and a fixed log line named each such re-bind so a deployment could tell when none was left. A re-bind now binds loopback like every open, the source gate admits loopback alone, and SetupResult.HostIp, the request's ChildInNetworkNamespace flag and the legacy log line are gone. A handle that still records a network namespace and no broker socket is not re-bound at all: nothing listens at its gateway any more, so a re-bind would clear the posture that says its model access is gone. Such a straggler lands as ModelCredentialLeaseLost and its agent is stopped. The legacy E2E arm is flipped rather than dropped. The same pre-guard namespace on a 10.x lease, re-bound without a socket, now has its child refused at the gateway while a listener bound at that address and port is answered, so the kernel says the gateway path is closed, not only the listener's prefix. The inet teardown line stays: the veth guard is an inet table of the same name, and a sealed survivor's table may still need deleting. The teardown-by-name arm now asserts the kernel lists nothing of the run afterwards, which the removed legacy arm used to check. --- .github/workflows/sandbox-isolation.yml | 23 ++- .../Services/Agents/AgentRunExecutor.cs | 20 +- .../Broker/LoopbackModelCredentialBroker.cs | 117 +++--------- .../Sandbox/Isolation/FilteredEgressNetns.cs | 14 +- .../Agents/ModelCredentialRebindRequest.cs | 13 +- .../Agents/SandboxHandle.cs | 21 +-- .../CodeSpace.Messages/Agents/SandboxSpec.cs | 9 +- .../AgentRunExecutorCredentialBrokerTests.cs | 95 ++++++++-- .../FilteredEgressNetnsE2ETests.cs | 63 +++++-- .../ModelCredentialBrokerNetnsE2ETests.cs | 171 ++++++++---------- .../Agents/ModelCredentialBrokerTests.cs | 105 +++++------ 11 files changed, 318 insertions(+), 333 deletions(-) diff --git a/.github/workflows/sandbox-isolation.yml b/.github/workflows/sandbox-isolation.yml index 5239cfb9d..99bd26bf9 100644 --- a/.github/workflows/sandbox-isolation.yml +++ b/.github/workflows/sandbox-isolation.yml @@ -235,7 +235,7 @@ jobs: passed=$(grep -oE 'passed="[0-9]+"' "$trx" | head -1 | grep -oE '[0-9]+') echo "executed=${executed:-0} passed=${passed:-0}" if [ "${executed:-0}" -lt 90 ]; then - echo "::error::Expected >=90 sandbox isolation tests to run (bwrap/prlimit confinement + cap-drop + cgroup-namespace re-root + egress-allowlist filter + cgroup resource cap + durable-launch cgroup wiring + argv/envp per-string kernel ceiling + a prompt past it riding stdin + a read-only workspace mount + a network-off run reaching its broker through the relay and nothing else + an allowlist run relayed to its broker + a read-only reviewer reading its diff with the real CLIs + a bwrap probe that runs the launch argv + the MCP helper bound file by file behind a read-only socket dir + a CLI reaching its broker socket through the relay + a severed child reaching its broker over the lease socket across a worker restart + a pre-relay namespaced run re-bound at its gateway and torn down with its seal + an allowlist run's veth guarded both ways, a flow the worker opened before the run included + forwarding a root worker may not write named before an allowlist is planned + an allowlist run's port 53 open only at the resolvers of the resolv.conf its namespace reads, and still to a resolver address the worker's own NAT rewrites before its forward and its input hooks + a target repository's own CLI config kept out of the run with the real CLIs, every repository's memory read in a multi-repo workspace included), but only ${executed:-0} did — the Category=Sandbox filter matched too few (trait regression?). If a case was deliberately removed, lower this number in the same PR." + echo "::error::Expected >=90 sandbox isolation tests to run (bwrap/prlimit confinement + cap-drop + cgroup-namespace re-root + egress-allowlist filter + cgroup resource cap + durable-launch cgroup wiring + argv/envp per-string kernel ceiling + a prompt past it riding stdin + a read-only workspace mount + a network-off run reaching its broker through the relay and nothing else + an allowlist run relayed to its broker + a read-only reviewer reading its diff with the real CLIs + a bwrap probe that runs the launch argv + the MCP helper bound file by file behind a read-only socket dir + a CLI reaching its broker socket through the relay + a severed child reaching its broker over the lease socket across a worker restart + a pre-relay namespaced run's gateway answering nothing after its re-bind + an allowlist run's veth guarded both ways, a flow the worker opened before the run included + forwarding a root worker may not write named before an allowlist is planned + an allowlist run's port 53 open only at the resolvers of the resolv.conf its namespace reads, and still to a resolver address the worker's own NAT rewrites before its forward and its input hooks + a target repository's own CLI config kept out of the run with the real CLIs, every repository's memory read in a multi-repo workspace included), but only ${executed:-0} did — the Category=Sandbox filter matched too few (trait regression?). If a case was deliberately removed, lower this number in the same PR." exit 1 fi @@ -283,22 +283,21 @@ jobs: assert len(cases) == rows and all(r.get('outcome') == 'Passed' for r in cases), f'{method}: all {rows} case(s) must pass' print('All 6 sealed-egress arms ran and passed.') - # The allowlist plan's host-routing arms, the guard on its veth, its DNS pinned to the resolvers of the resolv.conf - # its namespace reads and still answered where the worker's own NAT rewrites one, the relayed allowlist launch, - # and the forwarding probe's read-only arm return early without ip and nft, or off root; require their markers. - # The IPv6 arm prints its marker only where the veth has a link-local. - for marker in ('[filtered-egress-e2e] ran restart-reissue', '[filtered-egress-e2e] ran policy-route-discard-dst', '[filtered-egress-e2e] ran worker-shut', '[filtered-egress-e2e] ran ipv6-link-local', '[filtered-egress-e2e] ran shadowed-peer-refused', '[filtered-egress-e2e] ran established-flow-refused', '[filtered-egress-e2e] ran pmtu-upload', '[filtered-egress-e2e] ran stale-guard-replaced', '[filtered-egress-e2e] ran forwarding-read-only', '[filtered-egress-e2e] ran dns-pinned', '[filtered-egress-e2e] ran resolv-conf-view', '[filtered-egress-e2e] ran dns-dnat', '[filtered-egress-e2e] ran dns-redirect', '[durable-egress-e2e] ran allowlist-relay'): + # The allowlist plan's host-routing arms, the guard on its veth, the teardown by name, its DNS pinned to the + # resolvers of the resolv.conf its namespace reads and still answered where the worker's own NAT rewrites one, the + # relayed allowlist launch, and the forwarding probe's read-only arm return early without ip and nft, or off root; + # require their markers. The IPv6 arm prints its marker only where the veth has a link-local. + for marker in ('[filtered-egress-e2e] ran teardown-by-name', '[filtered-egress-e2e] ran restart-reissue', '[filtered-egress-e2e] ran policy-route-discard-dst', '[filtered-egress-e2e] ran worker-shut', '[filtered-egress-e2e] ran ipv6-link-local', '[filtered-egress-e2e] ran shadowed-peer-refused', '[filtered-egress-e2e] ran established-flow-refused', '[filtered-egress-e2e] ran pmtu-upload', '[filtered-egress-e2e] ran stale-guard-replaced', '[filtered-egress-e2e] ran forwarding-read-only', '[filtered-egress-e2e] ran dns-pinned', '[filtered-egress-e2e] ran resolv-conf-view', '[filtered-egress-e2e] ran dns-dnat', '[filtered-egress-e2e] ran dns-redirect', '[durable-egress-e2e] ran allowlist-relay'): assert marker in text, f'"{marker}" is missing — this lane is root with ip and nft, so the allowlist plan must run' - for method in ('FilteredEgressNetnsE2ETests.A_30_still_held_by_a_run_that_outlived_its_worker_is_not_handed_to_the_next_run', 'FilteredEgressNetnsE2ETests.A_host_whose_policy_rule_discards_the_run_s_replies_fails_the_setup_and_leaks_nothing', 'FilteredEgressNetnsE2ETests.An_allowlist_run_reaches_neither_the_worker_s_gateway_nor_its_address_while_dns_on_the_worker_and_the_allowlist_still_answer', 'FilteredEgressNetnsE2ETests.An_allowlist_run_has_no_path_to_the_worker_over_the_veth_s_ipv6_link_local_either', 'FilteredEgressNetnsE2ETests.A_peer_the_worker_reaches_at_the_run_s_address_is_refused_and_the_sandbox_receives_nothing', 'FilteredEgressNetnsE2ETests.A_flow_the_worker_opened_to_the_shadowed_peer_before_the_run_is_neither_handed_to_the_sandbox_nor_answered_from_it', 'FilteredEgressNetnsE2ETests.An_upload_across_a_narrower_uplink_completes_because_the_worker_s_frag_needed_reaches_the_run', 'FilteredEgressNetnsE2ETests.A_guard_an_earlier_teardown_left_behind_is_replaced_not_added_to', 'FilteredEgressNetnsE2ETests.Forwarding_a_root_worker_may_not_write_is_named_before_an_allowlist_is_planned_on_it', 'FilteredEgressNetnsE2ETests.An_allowlist_run_reaches_port_53_only_at_the_resolver_its_resolv_conf_names', 'FilteredEgressNetnsE2ETests.A_namespace_the_setup_builds_reads_the_worker_s_resolv_conf_and_opens_port_53_to_its_resolvers_alone', 'FilteredEgressNetnsE2ETests.A_resolver_address_the_worker_dnats_before_its_forward_hook_still_answers_the_run', 'FilteredEgressNetnsE2ETests.A_resolver_address_the_worker_redirects_to_itself_still_answers_the_run', 'DurableLaunchEgressE2ETests.An_allowlist_run_reaches_its_broker_through_the_relay_and_its_allowlist_still_holds'): + for method in ('FilteredEgressNetnsE2ETests.The_durable_setup_teardown_split_enforces_the_filter_and_teardown_is_reconstructable_from_runId', 'FilteredEgressNetnsE2ETests.A_30_still_held_by_a_run_that_outlived_its_worker_is_not_handed_to_the_next_run', 'FilteredEgressNetnsE2ETests.A_host_whose_policy_rule_discards_the_run_s_replies_fails_the_setup_and_leaks_nothing', 'FilteredEgressNetnsE2ETests.An_allowlist_run_reaches_neither_the_worker_s_gateway_nor_its_address_while_dns_on_the_worker_and_the_allowlist_still_answer', 'FilteredEgressNetnsE2ETests.An_allowlist_run_has_no_path_to_the_worker_over_the_veth_s_ipv6_link_local_either', 'FilteredEgressNetnsE2ETests.A_peer_the_worker_reaches_at_the_run_s_address_is_refused_and_the_sandbox_receives_nothing', 'FilteredEgressNetnsE2ETests.A_flow_the_worker_opened_to_the_shadowed_peer_before_the_run_is_neither_handed_to_the_sandbox_nor_answered_from_it', 'FilteredEgressNetnsE2ETests.An_upload_across_a_narrower_uplink_completes_because_the_worker_s_frag_needed_reaches_the_run', 'FilteredEgressNetnsE2ETests.A_guard_an_earlier_teardown_left_behind_is_replaced_not_added_to', 'FilteredEgressNetnsE2ETests.Forwarding_a_root_worker_may_not_write_is_named_before_an_allowlist_is_planned_on_it', 'FilteredEgressNetnsE2ETests.An_allowlist_run_reaches_port_53_only_at_the_resolver_its_resolv_conf_names', 'FilteredEgressNetnsE2ETests.A_namespace_the_setup_builds_reads_the_worker_s_resolv_conf_and_opens_port_53_to_its_resolvers_alone', 'FilteredEgressNetnsE2ETests.A_resolver_address_the_worker_dnats_before_its_forward_hook_still_answers_the_run', 'FilteredEgressNetnsE2ETests.A_resolver_address_the_worker_redirects_to_itself_still_answers_the_run', 'DurableLaunchEgressE2ETests.An_allowlist_run_reaches_its_broker_through_the_relay_and_its_allowlist_still_holds'): cases = [r for r in results if method in r.get('testName', '')] assert len(cases) == 1 and cases[0].get('outcome') == 'Passed', f'{method}: must pass' - print('All 14 allowlist-plan arms ran and passed.') + print('All 15 allowlist-plan arms ran and passed.') - # The broker's socket, relay-revoke and legacy-gateway arms return early without bwrap or ip/nft; require each - # marker, the legacy survivor's teardown of the seal it carried included. - for arm in ('socket-channel', 'revoke-network-off', 'revoke-allowlist', 'legacy-gateway-rebind', 'legacy-sealed-teardown'): + # The broker's socket, relay-revoke and retired-gateway arms return early without bwrap or ip/nft; require each marker. + for arm in ('socket-channel', 'revoke-network-off', 'revoke-allowlist', 'gateway-retired'): assert f'[broker-socket-e2e] ran {arm}' in text, f'broker E2E arm "{arm}" did not run — this lane is root with bwrap, ip and nft' - print('All 5 broker arms ran.') + print('All 4 broker arms ran.') # The bwrap probe E2E returns early off root, which reads as Passed; require each arm's marker. for arm in ('masked-proc', 'unmasked-proc'): diff --git a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs index c8940e74f..7ea3ebe85 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs @@ -4568,12 +4568,16 @@ private async Task RebindModelCredentialLeaseAsync(ModelAccessContext cont } /// - /// The re-bind this run's handle makes possible, or null when it makes none. Three gates, each of which would + /// The re-bind this run's handle makes possible, or null when it makes none. Four gates, each of which would /// otherwise produce a lease that answers the wrong thing: /// /// The address. Port + route + bearer must all be recorded. A handle stamped before they were is a /// run whose port nobody wrote down, and that is the mixed-version deploy case: it keeps the typed landing. /// + /// The door. See — a child that calls an address no broker + /// serves any more is not restored by a lease on loopback, and a re-bind that took would clear the posture that says + /// its model access is gone. + /// /// The host. The agent calls a port on the machine it was launched on. Binding that number HERE, on a /// worker that is not that machine, would answer nobody at all — while clearing the posture that says the run's /// access is gone. Same predicate the runner uses before answering any other pid-derived question, and it admits @@ -4584,22 +4588,32 @@ private async Task RebindModelCredentialLeaseAsync(ModelAccessContext cont /// The credential. See — a resolve that landed on a different /// ROW is not a restoration. /// - /// Takes what it reads rather than the whole context, so the three gates are directly testable (Rule 1 — + /// Takes what it reads rather than the whole context, so the four gates are directly testable (Rule 1 — /// four parameters, under the cap). /// internal static ModelCredentialRebindRequest? RebindRequestFor(AgentRunOwnerToken owner, Guid teamId, SandboxHandle handle, ResolvedModelCredential? upstream) { if (handle.ModelBrokerRunToken is not { Length: > 0 } token || handle.ModelBrokerRoute is not { Length: > 0 } route || handle.ModelBrokerPort is not { } port) return null; + if (CallsItsBrokerAtAGateway(handle)) return null; if (!LocalProcessRunner.PidAnswerableHere(handle)) return null; if (upstream is not { } resolved || !FrontsTheSameCredential(handle, resolved)) return null; return new() { RunId = owner.RunId, TeamId = teamId, Epoch = owner.Epoch, Port = port, PathId = route, RunToken = token, Upstream = resolved, Ttl = Credentials.ModelCredentialLease.Ttl, - SocketPath = handle.ModelBrokerSocketPath, ChildInNetworkNamespace = handle.EgressNetnsKey is { Length: > 0 }, + SocketPath = handle.ModelBrokerSocketPath, }; } + /// + /// Whether this handle's child reaches its broker at its network namespace's gateway rather than through a socket: + /// a namespace recorded and no broker socket. Only a run launched before a namespaced child reached its broker + /// through a socket has that shape — every brokered launch into a network of its own since mints one or is refused + /// — and nothing listens at a gateway any more, since every lease binds loopback. So such a run keeps the typed + /// landing, and its agent is stopped rather than left calling nothing. + /// + private static bool CallsItsBrokerAtAGateway(SandboxHandle handle) => handle.EgressNetnsKey is { Length: > 0 } && handle.ModelBrokerSocketPath is null; + /// /// Whether the credential this pass resolved is the SAME one the launch's lease fronted — the row id when the /// launch named one (both null is the operator-global key, which has no row) and the provider tag either way. diff --git a/backend/src/CodeSpace.Core/Services/Agents/Credentials/Broker/LoopbackModelCredentialBroker.cs b/backend/src/CodeSpace.Core/Services/Agents/Credentials/Broker/LoopbackModelCredentialBroker.cs index e7316d670..a77c4a596 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Credentials/Broker/LoopbackModelCredentialBroker.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Credentials/Broker/LoopbackModelCredentialBroker.cs @@ -34,20 +34,14 @@ namespace CodeSpace.Core.Services.Agents.Credentials.Broker; /// allowlist — cannot reach the host's 127.0.0.1, so such a lease is also served on a per-run Unix socket /// () whose directory is bound into the sandbox, and the codespace-mcp relay /// inside answers the CLI's 127.0.0.1:<port> and carries each call to that socket. The runner resolves -/// to loopback for every child. The one exception is the re-bind of a -/// handle written before the socket existed whose child is in a network namespace of its own: that child reaches this -/// worker at its namespace's veth gateway, so on a host that can build such namespaces that re-bind binds every -/// address (http://+:port/, which also matches any Host header), and the broker logs -/// for each one. A handle without a socket whose child shares the worker's network is -/// re-bound on loopback, where that child calls. +/// to loopback for every child, and a re-bind binds loopback too, with +/// its socket or without one. /// /// What guards it. The 256-bit bearer, checked in constant time, is the capability; the run's route /// segment is a 128-bit CSPRNG id, so a caller cannot even find another run's route by holding its id; the lease /// expires without renewal and is withdrawn outright on cancel. The source-address gate below is defence in depth, -/// not the guarantee: it admits loopback and the 10.0.0.0/8 space the subnet allocator handed per-run /30s out -/// of before it moved to 198.19.64.0–198.19.191.255. The gateway-addressed survivors above call from there; every run -/// since reaches this broker from loopback. So a deployment whose pods sit in a 10/8 network has neighbours that can -/// REACH the port — and still cannot use it without a live run's token. +/// not the guarantee: it admits loopback alone, which is where every child's call arrives from — directly, or spliced +/// from its socket by this worker. /// /// Streaming is load-bearing. Both harnesses stream (SSE), so the proxy reads response headers only /// () and flushes every chunk it copies. A buffered relay would @@ -149,7 +143,7 @@ internal void BreakListenerForTest(Guid runId) if (_byRun.TryGetValue(runId, out var lease)) CloseQuietly(lease.Listener); } - /// Test seam: the prefix a LIVE lease's listener is bound to (http://127.0.0.1:<port>/, or http://+:<port>/ for a wide bind), or null for a run with no lease — the one observation that says which addresses a lease exposes, on a host that has no second address to call it from. + /// Test seam: the prefix a LIVE lease's listener is bound to (http://127.0.0.1:<port>/), or null for a run with no lease — the one observation that says which addresses a lease exposes, on a host that has no second address to call it from. internal string? ListenerPrefixForTest(Guid runId) => _byRun.TryGetValue(runId, out var lease) ? lease.Listener.Prefixes.Single() : null; /// Test seam: kill the Unix-socket acceptor behind a LIVE lease without going through a revoke — 's counterpart for the lease's other door, whose failure must end the claim just the same. @@ -258,19 +252,16 @@ public Task RebindAsync(ModelCredentialRebindRequest request, Cancellation // THE SAME ADDRESS, already up, on this very worker: a re-attach of a run this process never stopped // serving (a reclaim after the observation lease lapsed while the old pass was between heartbeats). There - // is nothing to re-open, and trying is actively destructive — BindPort would walk the candidate hosts - // against a port THIS PROCESS holds, and on Linux the wide bind fails while the loopback one succeeds - // underneath it, so Install would close the live WIDE listener and leave a sealed netns run talking to an - // address it cannot reach, with true returned and the posture cleared. Adopt it instead. + // is nothing to re-open, and trying is actively destructive — BindPort would ask for a port THIS PROCESS + // holds: refused, it would end a live run typed; granted, Install would close the very listener and socket + // the child is calling. Adopt it instead. if (IsSameAddress(held, request)) return AdoptHeldLease(held, request); } // The PORT first, the socket only once it holds: the port is the lock between two workers on one host. A worker // still serving this run holds it, so this re-bind is refused here — before anything at the socket's path is // touched — and the live worker's sandboxed children keep their door. - var hosts = CandidateHosts(request.SocketPath, request.ChildInNetworkNamespace); - - if (BindPort(request.Port, hosts, out var bindFailure) is not { } bound) return RefuseRebind(request, "its port could not be bound here — something else is holding it, or this host refused the bind", bindFailure); + if (BindPort(request.Port, out var bindFailure) is not { } bound) return RefuseRebind(request, "its port could not be bound here — something else is holding it, or this host refused the bind", bindFailure); if (!TryReopenChannel(request, bound.Port, out var channel, out var socketFailure)) { @@ -285,13 +276,11 @@ public Task RebindAsync(ModelCredentialRebindRequest request, Cancellation }, request.Ttl); _logger.LogInformation("Model credential RE-BOUND for agent run {RunId} on port {Port} (team {TeamId}, epoch {Epoch}) until {ExpiresAt:O}; its detached agent's next model call is answered here", lease.RunId, lease.Port, lease.TeamId, lease.Epoch, lease.ExpiresAt); - LogIfLegacyRebind(request); - WarnIfUnreachableFromNetns(lease, hosts, bound.Host); return Task.FromResult(true); } - /// Re-open the socket a re-bind names, on the port it now holds — true with no channel for a handle that recorded none (the legacy re-bind, TCP alone as before), false when the socket could not be bound. + /// Re-open the socket a re-bind names, on the port it now holds — true with no channel for a handle that recorded none (a child on the worker's own network, which calls loopback), false when the socket could not be bound. private bool TryReopenChannel(ModelCredentialRebindRequest request, int port, out BrokerSocketChannel? channel, out Exception? failure) { channel = null; @@ -304,32 +293,15 @@ private bool TryReopenChannel(ModelCredentialRebindRequest request, int port, ou return channel is not null; } - /// The fixed words of the line a legacy re-bind logs — pinned by a test, because retiring the gateway path to the broker waits on this line going quiet. - internal const string LegacyRebindMarker = "legacy model-broker re-bind"; - - /// - /// Say so when a re-bind restored a child in a network namespace of its own that has no socket to come in through - /// ( without a - /// ): it reaches this worker at its namespace's gateway, the - /// only reason a lease still binds wide. The line is how a deployment learns that no such run is left, so that the - /// wide bind can go — which is why a run on the worker's own network, calling loopback, never logs it. - /// - private void LogIfLegacyRebind(ModelCredentialRebindRequest request) - { - if (request.SocketPath is not null || !request.ChildInNetworkNamespace) return; - - _logger.LogInformation("Agent run {RunId}: " + LegacyRebindMarker + " on port {Port} — its handle records a network namespace and no broker socket, so its child still reaches this worker at that namespace's gateway", request.RunId, request.Port); - } - /// Whether a lease this worker already holds IS the address the request is asking for — same port, same route, same bearer, same socket. All four, because any one of them differing means the child would be talking to something other than what the handle recorded. private static bool IsSameAddress(Lease held, ModelCredentialRebindRequest request) => held.Port == request.Port && string.Equals(held.PathId, request.PathId, StringComparison.Ordinal) && McpRunToken.Matches(held.Token, request.RunToken) && string.Equals(held.SocketPath, request.SocketPath, StringComparison.Ordinal); /// /// Take over a lease this worker is ALREADY serving at the re-attach's epoch, without touching its listener. The - /// socket stays exactly as it was bound — which is the point, since re-binding it is what would move a wide bind - /// down to loopback — and only the two things a new claimant owns change: the fence the lease answers renewals on, - /// and its window. + /// socket stays exactly as it was bound — which is the point, since re-binding it would close the address the child + /// is calling — and only the two things a new claimant owns change: the fence the lease answers renewals on, and its + /// window. /// /// The epoch move is what makes the adoption real rather than cosmetic: is fenced, /// so a lease left at the previous attempt's epoch would refuse every heartbeat the re-attach sends and lapse two @@ -482,19 +454,6 @@ private void SweepLapsedLeases() /// The address a lease's child is handed: this run's own port and route, with the reachable host left as a token for the runner to substitute at launch (see ). private static string BaseUrlFor(Lease lease) => $"http://{SandboxSpec.ModelBrokerHostToken}:{lease.Port}/{lease.PathId}"; - /// - /// A legacy re-bind on a host that CAN build per-run network namespaces, but that refused the wide bind, serves - /// only a child on the worker's own network: a namespaced child with no socket reaches this worker at its namespace - /// gateway, and nothing is listening there. Said out loud because the failure it produces is a model call that - /// times out, which reads like a provider problem rather than a bind that fell back. - /// - private void WarnIfUnreachableFromNetns(Lease lease, IReadOnlyList candidateHosts, string host) - { - if (host == candidateHosts[0]) return; // it bound what it wanted: the wide bind where one can be needed, loopback where nothing needs more - - _logger.LogWarning("Agent run {RunId}: its model-credential broker fell back to a loopback-only bind on a host that builds filtered-egress namespaces; a deny-by-default egress run cannot reach it there", lease.RunId); - } - /// /// Bind a FRESH ephemeral port for a new lease, on loopback only. A child on the worker's own network calls it /// there; a child in a network of its own reaches it through the lease's socket, spliced to this same loopback @@ -515,37 +474,16 @@ private static (HttpListener Listener, int Port)? BindFresh(out Exception? failu } /// - /// Bind ONE GIVEN port — a re-bind's whole job. No fresh-port retry, deliberately: the address is not this - /// process's to choose, it is the one a detached agent already holds, so a substitute would answer nobody. On the - /// hosts names for the request, because the child whose port this is still reaches the - /// worker the way it did when it was launched. + /// Bind ONE GIVEN port, on loopback — a re-bind's whole job. No fresh-port retry, deliberately: the address is not + /// this process's to choose, it is the one a detached agent already holds, so a substitute would answer nobody. + /// Loopback for every re-bind, as for every open: a child on the worker's own network calls it there, and a child in + /// a network of its own comes in through the lease's socket, spliced to this same listener. /// - private static (HttpListener Listener, int Port, string Host)? BindPort(int port, IReadOnlyList candidateHosts, out Exception? failure) - { - failure = null; - - foreach (var host in candidateHosts) - if (TryBind(host, port, out failure) is { } listener) return (listener, port, host); - - return null; - } - - /// The prefix host that binds every address AND matches any Host header — what a per-run namespace's child necessarily sends, since it addresses the worker by its gateway IP. - private const string AnyHost = "+"; + private static (HttpListener Listener, int Port)? BindPort(int port, out Exception? failure) => + TryBind(LoopbackHost, port, out failure) is { } listener ? (listener, port) : null; private const string LoopbackHost = "127.0.0.1"; - /// - /// Where a RE-BIND binds, in order of preference. A request with a socket path binds loopback: its namespaced child - /// comes in through the socket, spliced to loopback, so a wide bind would only hand its port to the host's - /// neighbours. So does one whose child shares the worker's network, which calls loopback. Only the legacy re-bind — - /// a child in a network namespace of its own on a handle written before the socket existed — keeps the wide bind - /// first where a per-run network namespace can exist, because such a child reaches this worker at its namespace - /// gateway. That branch goes once no such handle is left in flight (). - /// - private static IReadOnlyList CandidateHosts(string? socketPath, bool childInNetworkNamespace) => - socketPath is null && childInNetworkNamespace && FilteredEgressNetns.IsSupported ? new[] { AnyHost, LoopbackHost } : new[] { LoopbackHost }; - /// /// Bind ONE prefix, or null plus the reason it could not. EVERY exception counts as "did not bind" — deliberately /// wider than an enumerated list, and the width is the fix. @@ -559,7 +497,7 @@ private static IReadOnlyList CandidateHosts(string? socketPath, bool chi /// the one nobody ran this on; a bind that threw is a bind that did not happen, whatever it threw. /// /// The reason travels out rather than being dropped, so the caller's Warning can name it: the difference - /// between "something else holds this port" and "this host refuses the wide bind" is the difference between + /// between "something else holds this port" and "this host refuses the prefix" is the difference between /// waiting and reconfiguring. /// private static HttpListener? TryBind(string host, int port, out Exception? failure) @@ -653,7 +591,7 @@ private void DropIfStillServing(Lease lease, string door, Exception? reason) _logger.LogWarning(reason, "Agent run {RunId}: the {Door} of its brokered model lease on port {Port} stopped accepting, so the lease was dropped rather than left claiming an address nothing answers", lease.RunId, door, lease.Port); } - /// The lease's door on 127.0.0.1:<port> (or every address, for a legacy wide bind), as names it. + /// The lease's door on 127.0.0.1:<port>, as names it. private const string ListenerDoor = "TCP listener"; /// The lease's per-run Unix socket (), as names it. @@ -741,15 +679,12 @@ private void RefusePath(HttpListenerResponse response, Lease lease, string path) } /// - /// Whether a request's source could be one of this host's sandboxes: loopback (a run sharing the host network, or a - /// relayed one, whose socket is spliced to the port from this worker) or the 10.0.0.0/8 space - /// carved per-run /30s out of before it moved to 198.19.64.0–198.19.191.255, - /// which a namespaced run launched before the relay still calls from. A run holding a /30 from the new pool is - /// relayed, so it never calls from that /30. Defence in depth behind the token — see the type remarks on what it - /// does and does not buy. + /// Whether a request's source could be one of this host's sandboxes: loopback, and only loopback — a run sharing the + /// host network calls from there, and a relayed one's socket is spliced to the port from this worker. A run holding a + /// per-run /30 is relayed, so it never calls from that /30. Defence in depth behind the token — see the type remarks + /// on what it does and does not buy. /// - internal static bool IsPlausibleSandboxSource(IPAddress? source) => - source is not null && (IPAddress.IsLoopback(source) || (source.AddressFamily == AddressFamily.InterNetwork && source.GetAddressBytes()[0] == 10)); + internal static bool IsPlausibleSandboxSource(IPAddress? source) => source is not null && IPAddress.IsLoopback(source); private static void Refuse(HttpListenerResponse response, HttpStatusCode status) { diff --git a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Isolation/FilteredEgressNetns.cs b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Isolation/FilteredEgressNetns.cs index f9497ccf9..fa736c65f 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Isolation/FilteredEgressNetns.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Isolation/FilteredEgressNetns.cs @@ -29,7 +29,7 @@ public static class FilteredEgressNetns private static readonly CapabilityProbe Filter = new(() => FilterProbe(IpForwardPath), () => System.Diagnostics.Stopwatch.GetElapsedTime(ProcessStart), ProbeRetryInterval); - /// True when ip + nft are present (the binaries the plan drives). Whether this process may USE them to filter a run is , which a confining host's launch keys on; this alone gates what needs only the binaries — tearing a namespace down by name, the legacy re-bind's wide bind — and the allowlist plan of a host where bubblewrap does not confine, whose setup filters the run or aborts its launch (LocalProcessRunner.FiltersAllowlist). A failed probe is retried like the filter probe (): a fork that failed once at boot must not disable any of them for the process lifetime. + /// True when ip + nft are present (the binaries the plan drives). Whether this process may USE them to filter a run is , which a confining host's launch keys on; this alone gates what needs only the binaries — tearing a namespace down by name — and the allowlist plan of a host where bubblewrap does not confine, whose setup filters the run or aborts its launch (LocalProcessRunner.FiltersAllowlist). A failed probe is retried like the filter probe (): a fork that failed once at boot must not disable any of them for the process lifetime. public static bool IsSupported => Tools.Holds; /// @@ -118,16 +118,6 @@ public sealed record SetupResult /// The ip netns exec <ns> prefix a caller prepends to run its command inside the filtered netns. Empty when setup failed. public IReadOnlyList ExecPrefix { get; init; } = Array.Empty(); - /// - /// The HOST-side veth address of this run's /30 (FilteredEgressPlan.HostIp) — the namespace's default - /// gateway. Null when setup failed. It is not knowable before the /30 is reserved HERE, so it is returned. A - /// packet to the host's own address is delivered locally, so the plan's forward-hook filter never sees it; the - /// plan's guard on the veth does, and admits nothing there but replies and DNS to a resolver the run's resolv.conf - /// names (FilteredEgressPlan.BuildVethGuardRuleset). A child launched before that guard still reaches this - /// worker at this address, which is how a run launched before the relay reached its broker. - /// - public string? HostIp { get; init; } - public string? SetupError { get; init; } } @@ -208,7 +198,7 @@ internal static async Task ApplyAsync(string runId, FilteredEgressP return new SetupResult { SetupOk = false, SetupError = $"nft -f - → exit {nftExit}: {Trim(nftOut)}" }; } - return new SetupResult { SetupOk = true, ExecPrefix = plan.ExecPrefix, HostIp = plan.HostIp }; + return new SetupResult { SetupOk = true, ExecPrefix = plan.ExecPrefix }; } catch (Exception ex) { diff --git a/backend/src/CodeSpace.Messages/Agents/ModelCredentialRebindRequest.cs b/backend/src/CodeSpace.Messages/Agents/ModelCredentialRebindRequest.cs index 51b060bb7..14669cbe6 100644 --- a/backend/src/CodeSpace.Messages/Agents/ModelCredentialRebindRequest.cs +++ b/backend/src/CodeSpace.Messages/Agents/ModelCredentialRebindRequest.cs @@ -45,19 +45,8 @@ public sealed record ModelCredentialRebindRequest /// The Unix socket the launch's lease was also served on, read off the same handle, or null when it recorded none. /// With a path, is bound on LOOPBACK FIRST and the socket re-opened at this exact path only once /// that bind holds: the port is the lock between two workers on one host, so a worker that loses it never touches - /// the other's socket. Null binds loopback too, unless holds: that is the - /// legacy re-bind — the recorded port wide first, for a child that reaches this worker at its namespace gateway. + /// the other's socket. Null binds loopback alone, for a child on the worker's own network, which calls it there. /// [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull)] public string? SocketPath { get; init; } - - /// - /// Whether the same handle recorded a per-run network namespace for the child (SandboxHandle.EgressNetnsKey). - /// Without a , such a child reaches this worker at its namespace's gateway rather than on - /// loopback — the one kind of run that still needs the legacy re-bind, so the broker binds wide for exactly these - /// re-binds and names them in a fixed log line that the retirement of the gateway path waits on. A run on the - /// worker's own network calls loopback and is not one of them, whatever its handle lacks: its re-bind binds - /// loopback alone. - /// - public bool ChildInNetworkNamespace { get; init; } } diff --git a/backend/src/CodeSpace.Messages/Agents/SandboxHandle.cs b/backend/src/CodeSpace.Messages/Agents/SandboxHandle.cs index 65c7f0c71..fd34b3171 100644 --- a/backend/src/CodeSpace.Messages/Agents/SandboxHandle.cs +++ b/backend/src/CodeSpace.Messages/Agents/SandboxHandle.cs @@ -170,17 +170,14 @@ public sealed record SandboxHandle /// re-attach, this port is unbound. A local process that grabs it and ACCEPTS — rather than merely holding it, /// which only makes the re-bind refuse and the run land typed — receives the child's next request: its bearer, its /// prompt, and whatever the child does with the reply it sends back. Nothing in the address stops that, because - /// the address is all the child has. What bounds it: the squatter must be ON THIS HOST (the child reaches loopback - /// or its own netns gateway), must win the race for one specific ephemeral port, and gains a per-run broker bearer + /// the address is all the child has. What bounds it: the squatter must be ON THIS HOST (the child reaches loopback, + /// directly or through its relay), must win the race for one specific ephemeral port, and gains a per-run broker bearer /// that authenticates to nothing else and expires — not the tenant's key, which never leaves the broker. /// /// Per-lease ports change the shape of that exposure rather than its nature: the number of squattable - /// addresses is now the number of concurrent brokered runs instead of one per worker. Every lease binds loopback; a - /// child in a network of its own reaches it through . Only the re-bind of a - /// handle with no socket path — a namespaced run launched before the socket existed, which reaches the worker at its - /// namespace gateway — still binds WIDE (+) where the host builds filtered-egress namespaces. Reaching any of - /// them still buys nothing without a live run's bearer, and the broker refuses any source outside loopback and the - /// 10/8 space the allocator carved those runs' /30s from before it moved to 198.19.64.0–198.19.191.255. + /// addresses is now the number of concurrent brokered runs instead of one per worker. Every lease and every re-bind + /// binds loopback; a child in a network of its own reaches it through . Reaching + /// any of them still buys nothing without a live run's bearer, and the broker refuses any source but loopback. /// public int? ModelBrokerPort { get; init; } @@ -203,9 +200,11 @@ public sealed record SandboxHandle /// The per-run Unix socket this run's brokered lease was ALSO served on, recorded so a re-attach re-opens it at the /// same path — the path a sandbox's directory bind still points at. The re-bind takes /// on loopback FIRST and touches the socket only once it holds that port, because the port is the lock between two - /// workers on one host. Null when the lease had no socket, which is every handle written before this field existed: - /// such a handle takes the legacy re-bind, wide first. Omitted from the JSON when null, so a handle without one is - /// byte-identical to one written before the field. + /// workers on one host. Null when the lease had no socket — a child on the worker's own network, which calls + /// loopback, and every handle written before this field existed. A handle with an and + /// none is a namespaced run launched before the socket existed, whose child calls its namespace gateway; a re-attach + /// does not re-bind it. Omitted from the JSON when null, so a handle without one is byte-identical to one written + /// before the field. /// [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? ModelBrokerSocketPath { get; init; } diff --git a/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs b/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs index c22ef1bd5..e05adb158 100644 --- a/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs +++ b/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs @@ -202,11 +202,10 @@ public sealed record SandboxSpec /// /// The token that stands in for the HOST ADDRESS of the model-credential broker inside an - /// value (the base URL a brokered run's CLI calls). Only the RUNNER knows that address, - /// and only at launch: a deny-by-default egress run executes inside a per-run network namespace whose /30 is - /// reserved DURING the launch — after the broker lease was opened and its base URL was projected — so the child - /// reaches the worker at that namespace's own gateway IP, while a run sharing the host network reaches it on - /// loopback. The runner substitutes whichever applies, so the token NEVER survives into the child. + /// value (the base URL a brokered run's CLI calls). The RUNNER substitutes it at launch, + /// with loopback for every child: a run sharing the host network calls the broker there, and one in a network of + /// its own calls the codespace-mcp relay there, which carries the call to the lease's socket. So the token + /// NEVER survives into the child. /// /// The same shape as , and for the same reason: a pure /// IAgentHarness.BuildInvocation (and, here, a pure credential projection) cannot know a per-launch diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs index 886562637..16006ec3d 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs @@ -902,13 +902,6 @@ public async Task A_reattach_that_cannot_re_bind_the_address_still_lands_the_run { if (OperatingSystem.IsWindows()) return; - // BEFORE anything is launched. The fixture below holds LOOPBACK, the only candidate host a worker without - // filtered-egress namespaces tries; on a host that builds them the broker prefers the wide bind this fixture - // does not hold, so the refusal would not be falsifiable. Skipping here rather than after the drain, because - // the drain deliberately leaves a real `sleep 600` child ALIVE: a guard further down would return having - // leaked a ten-minute detached process and a run row stuck Running on every netns-capable dev box. - if (CodeSpace.Core.Services.Agents.Sandbox.Isolation.FilteredEgressNetns.IsSupported) return; - var teamId = await SeedTeamAsync(); var credId = await SeedModelCredentialAsync(teamId, BrokeredProvider, "sk-rebind-refused-fixture"); var runId = await CreateRunWithCredentialAsync(teamId, credId); @@ -942,10 +935,85 @@ await WaitUntilAsync(() => !ProcessIsAlive(handle.ProcessId), TimeSpan.FromSecon $"the agent (pid {handle.ProcessId}) was still alive after its run was landed lease-lost; an agent that cannot call a model must be stopped, not just recorded — diagnose with `ps -p {handle.ProcessId} -o pid,stat,etime,command`"); } + [Fact] + public async Task A_reattach_of_a_run_whose_child_calls_its_namespace_gateway_asks_for_no_re_bind_and_lands_it_typed() + { + if (OperatingSystem.IsWindows()) return; + + // A Trusted run shares the worker's network, so its lease has no socket on any host. Its handle, read back from + // the row with a network namespace added, is the one a run launched before its broker had a socket left behind: + // its child calls that namespace's gateway, where no broker listens any more. + var teamId = await SeedTeamAsync(); + var credId = await SeedModelCredentialAsync(teamId, BrokeredProvider, "sk-gateway-survivor-fixture"); + var runId = await CreateTaskRunAsync(teamId, new AgentTask { Goal = "scripted", Harness = "scripted-projector", Model = "test-model", ModelCredentialId = credId, Autonomy = AgentAutonomyLevel.Trusted, Permissions = AgentAutonomyPolicy.Derive(AgentAutonomyLevel.Trusted) }); + + var harness = new BrokerableScriptedHarness(BrokeredProvider, "sleep 600"); + var (handle, _) = await DrainLeavingTheAgentRunningAsync(runId, harness); + + try + { + await StageGatewayAddressedHandleAsync(runId, handle); + + using var workerB = new RecordingBroker(LoopbackModelCredentialBroker.ForTest(new AlwaysOkUpstream())); + using var bounded = new CancellationTokenSource(TimeSpan.FromSeconds(90)); // a re-bind that took would leave the agent sleeping: this ends the pass, not the suite + var reservation = await ReserveReattachAfterLapseAsync(runId); + + await ReattachUntilStoppedAsync(reservation, harness, workerB, bounded.Token); + + workerB.Rebinds.ShouldBeEmpty("a re-bind binds loopback, and this run's child calls its namespace's gateway: asking for one would read as 'the address is back' and clear the posture that says its model access is gone"); + workerB.HasLease(runId).ShouldBeFalse("and nothing may be installed for it"); + + using var verify = _fixture.BeginScope(); + var run = await verify.Resolve().GetAsync(runId, CancellationToken.None); + + run.Status.ShouldBe(AgentRunStatus.Failed, "a run no worker can restore is ended, not left burning its wall clock on calls nothing answers"); + JsonSerializer.Deserialize(run.ResultJson!, AgentJson.Options)!.ExitReason.ShouldBe(CodeSpace.Messages.Failures.FailureCodes.ModelCredentialLeaseLost, "under the typed code any other unrestorable run lands with"); + JsonSerializer.Deserialize(run.SandboxConfinementJson!, AgentJson.Options)!.ModelCredentialLeaseLost.ShouldBeTrue("the posture says its model access is gone, which is now true"); + + await WaitUntilAsync(() => !ProcessIsAlive(handle.ProcessId), TimeSpan.FromSeconds(15), + $"the agent (pid {handle.ProcessId}) was still alive after its run was landed lease-lost — diagnose with `ps -p {handle.ProcessId} -o pid,stat,etime,command`"); + } + finally { KillQuietly(handle.ProcessId); } + } + + /// + /// Give a drained run's persisted handle a network namespace, keeping its port, route and bearer and no broker socket: + /// the handle a namespaced run launched before its broker had a socket wrote. Through SQL, as the older handle shapes + /// in these suites are staged; and into the native launch's receipt when the launch wrote one, because the runner + /// answers for a handle only while its namespace key matches the receipt's, and such a run's two agreed. + /// + private async Task StageGatewayAddressedHandleAsync(Guid runId, SandboxHandle handle) + { + var netnsKey = Guid.NewGuid().ToString("N"); + var receipt = Path.Combine(handle.SpoolDirectory, NativeLaunchProtocol.DirectoryName, NativeLaunchProtocol.ReceiptFile); + + if (File.Exists(receipt)) + { + var recorded = JsonSerializer.Deserialize(await File.ReadAllTextAsync(receipt), NativeLaunchProtocol.Json).ShouldNotBeNull(); + await File.WriteAllTextAsync(receipt, JsonSerializer.Serialize(recorded with { EgressNetnsKey = netnsKey }, NativeLaunchProtocol.Json)); + } + + using (var scope = _fixture.BeginScope()) + await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE agent_run SET runner_handle = runner_handle || jsonb_build_object('egressNetnsKey', {netnsKey}::text) WHERE id = {runId}"); + + var staged = HandleOf(runId).ShouldNotBeNull(); + + staged.EgressNetnsKey.ShouldBe(netnsKey, "fixture: the staged handle must record the namespace"); + staged.ModelBrokerSocketPath.ShouldBeNull("fixture: and no broker socket, or this pins the socket re-bind instead"); + staged.ModelBrokerPort.ShouldBe(handle.ModelBrokerPort, "fixture: and still the port, route and bearer a re-bind would need"); + } + + /// Best-effort: stop an agent a failed assertion left running, so a red run does not leak a ten-minute sleep. + private static void KillQuietly(int pid) + { + try { using var process = Process.GetProcessById(pid); process.Kill(entireProcessTree: true); } + catch (Exception exception) when (exception is ArgumentException or InvalidOperationException or System.ComponentModel.Win32Exception) { /* already gone */ } + } + [Theory] [InlineData(true)] // a network-off run: its lease is served over a socket, the handle records it, and the re-attach re-opens it - [InlineData(false)] // a network-on run: no socket, so the handle is exactly a pre-field handle, and the re-attach takes the legacy re-bind - public async Task A_reattach_re_opens_the_broker_socket_its_handle_recorded_and_a_handle_without_one_takes_the_legacy_rebind(bool socket) + [InlineData(false)] // a network-on run: no socket, so the handle is exactly a pre-field handle, and the re-attach re-binds loopback alone + public async Task A_reattach_re_opens_the_broker_socket_its_handle_recorded_and_a_handle_without_one_is_served_on_loopback(bool socket) { if (OperatingSystem.IsWindows()) return; @@ -974,7 +1042,8 @@ public async Task A_reattach_re_opens_the_broker_socket_its_handle_recorded_and_ if (standIn is not null) socketPath.ShouldBe(standIn, "exactly the path the lease bound"); if (!socket) RunnerHandleJsonOf(runId).ShouldNotContain("modelBrokerSocketPath", customMessage: "a handle whose lease had no socket must be stored exactly as one written before the field existed"); - using var workerB = new RecordingBroker(LoopbackModelCredentialBroker.ForTest(new AlwaysOkUpstream())); + var brokerB = LoopbackModelCredentialBroker.ForTest(new AlwaysOkUpstream()); + using var workerB = new RecordingBroker(brokerB); var reservation = await ReserveReattachAfterLapseAsync(runId); var reattach = ReattachUntilStoppedAsync(reservation, harness, workerB); @@ -983,9 +1052,9 @@ public async Task A_reattach_re_opens_the_broker_socket_its_handle_recorded_and_ var rebind = workerB.Rebinds.ShouldHaveSingleItem(); rebind.SocketPath.ShouldBe(socketPath, - customMessage: "the re-attach read the handle back from the row and must ask for the socket it recorded — or, for a handle that recorded none, for none, which is what keeps a gateway-addressed child on the wide legacy re-bind"); - rebind.ChildInNetworkNamespace.ShouldBeFalse( - "this run shares the worker's network and its child calls loopback, so its re-bind is not the legacy gateway kind — reading it as one would keep the line the gateway path's retirement waits on firing for every shared-network run, forever"); + customMessage: "the re-attach read the handle back from the row and must ask for the socket it recorded — or, for a handle that recorded none, for none"); + brokerB.ListenerPrefixForTest(runId).ShouldBe($"http://127.0.0.1:{handle.ModelBrokerPort}/", + $"worker B serves the re-bound port on loopback alone, with a socket or without one — every child calls it there, directly or through its relay (namespaces possible here: {CodeSpace.Core.Services.Agents.Sandbox.Isolation.FilteredEgressNetns.IsSupported})"); (await ReachesUpstreamAsync(childBaseUrl, runToken)).ShouldBeTrue("the original loopback address answers again on worker B"); if (socketPath is not null) diff --git a/backend/tests/CodeSpace.SandboxTests/FilteredEgressNetnsE2ETests.cs b/backend/tests/CodeSpace.SandboxTests/FilteredEgressNetnsE2ETests.cs index ce42f51a2..923ca8e34 100644 --- a/backend/tests/CodeSpace.SandboxTests/FilteredEgressNetnsE2ETests.cs +++ b/backend/tests/CodeSpace.SandboxTests/FilteredEgressNetnsE2ETests.cs @@ -28,8 +28,9 @@ namespace CodeSpace.SandboxTests; /// address or the veth's IPv6 link-local) while DNS on the worker and the allowlist still answer; a peer the worker /// reaches at an address the run's /30 shadows is refused and the sandbox receives nothing, a flow the worker opened to /// that peer before the run included; an upload across a narrower uplink still completes; and a guard an earlier round -/// left behind is replaced, not added to. So does the one about forwarding a root worker may not turn on, and the four -/// about DNS: port 53 is open only at the resolver the run's resolv.conf names; the namespace the production setup +/// left behind is replaced, not added to. So do the one about forwarding a root worker may not turn on, the durable +/// teardown by name, after which the kernel lists nothing of the run (no namespace, no forward table, no guard), and the +/// four about DNS: port 53 is open only at the resolver the run's resolv.conf names; the namespace the production setup /// builds reads the worker's own resolv.conf, whose resolvers alone its tables admit; and a resolver address the /// worker's own NAT rewrites — DNATed before the forward table, REDIRECTed to the worker before the guard — still /// answers the run. @@ -73,6 +74,7 @@ public async Task The_durable_setup_teardown_split_enforces_the_filter_and_teard if (!FilteredEgressNetns.IsSupported) return; var runId = Guid.NewGuid().ToString("N"); + var names = NamesOf(runId); var setup = await FilteredEgressNetns.SetupAsync(runId, new[] { Allowed }, timeoutSeconds: 20, CancellationToken.None); try @@ -83,12 +85,21 @@ public async Task The_durable_setup_teardown_split_enforces_the_filter_and_teard // Run curl INSIDE the netns via the ExecPrefix — exactly how the durable launch will prefix its command chain. (await RunViaPrefixAsync(setup.ExecPrefix, Allowed)).ShouldBe(0, "the ALLOWED host is reachable through the set-up netns"); (await RunViaPrefixAsync(setup.ExecPrefix, Denied)).ShouldNotBe(0, "the DENIED host is dropped — SetupAsync's netns enforces the filter"); + (await RunHostExitAsync(["nft", "list", "table", "inet", names.Namespace])).ShouldBe(0, "control: the guard on the run's veth is there before its run ends"); } finally { await FilteredEgressNetns.TeardownAsync(runId, CancellationToken.None); // reconstructed from runId alone — the reap/crash-resume contract } + // The kernel's own listing is the evidence the guard went, not the re-setup below: the plan's ruleset replaces a + // guard table of the same name, so a re-setup succeeds over one a teardown left behind. + (await RunHostExitAsync(["nft", "list", "table", "inet", names.Namespace])).ShouldNotBe(0, $"the guard on the run's veth must go with its run, or every allowlist run leaks an nft table on the worker — check `nft list tables | grep {names.Namespace}`"); + (await RunHostExitAsync(["nft", "list", "table", "ip", names.Namespace])).ShouldNotBe(0, "and so must its forward table"); + (await RunHostExitAsync(["ip", "netns", "pids", names.Namespace])).ShouldNotBe(0, $"and its namespace — check `ip netns list | grep {names.Namespace}`"); + + output.WriteLine($"{RanMarker} teardown-by-name table={names.Namespace}"); + // Teardown actually freed the runId-derived names: a second SetupAsync with the SAME runId succeeds (it would // collide on the still-present ns/table otherwise). This is the leak-free guarantee. var resetup = await FilteredEgressNetns.SetupAsync(runId, new[] { Allowed }, timeoutSeconds: 20, CancellationToken.None); @@ -119,9 +130,13 @@ public async Task A_30_still_held_by_a_run_that_outlived_its_worker_is_not_hande try { second.SetupOk.ShouldBeTrue($"the next run's allowlist namespace must set up: {second.SetupError}"); - second.HostIp.ShouldNotBe(first.HostIp, "the survivor's /30 is still on its veth; handing it out again routes one run's replies into the other's namespace"); - output.WriteLine($"{RanMarker} restart-reissue survivor={first.HostIp} next={second.HostIp}"); + var survivorGateway = await GatewayOfAsync(survivor); + var nextGateway = await GatewayOfAsync(next); + + nextGateway.ShouldNotBe(survivorGateway, "the survivor's /30 is still on its veth; handing it out again routes one run's replies into the other's namespace"); + + output.WriteLine($"{RanMarker} restart-reissue survivor={survivorGateway} next={nextGateway}"); } finally { await FilteredEgressNetns.TeardownAsync(next, CancellationToken.None); } } @@ -197,18 +212,19 @@ public async Task An_allowlist_run_reaches_neither_the_worker_s_gateway_nor_its_ { setup.SetupOk.ShouldBeTrue($"the allowlist namespace must set up on this host: {setup.SetupError}"); - using var unlisted = new WorkerResolver(setup.HostIp!); - var guarded = await ProbeTheWorkerAsync(setup, workerIp, listener); + var gateway = await GatewayOfAsync(runId); + using var unlisted = new WorkerResolver(gateway); + var guarded = await ProbeTheWorkerAsync(setup, gateway, workerIp, listener); - guarded["gateway"].ShouldNotBe("open", $"a listener on the worker must not be reachable at the run's gateway {setup.HostIp} (its API, every other run's lease); probe: {Describe(guarded)}"); + guarded["gateway"].ShouldNotBe("open", $"a listener on the worker must not be reachable at the run's gateway {gateway} (its API, every other run's lease); probe: {Describe(guarded)}"); guarded["worker"].ShouldNotBe("open", $"nor at the worker's own address {workerIp}; probe: {Describe(guarded)}"); guarded["dns_udp"].ShouldBe("answered", $"the resolver the run's resolv.conf names, which the worker serves on its own address, still answers over UDP; probe: {Describe(guarded)}"); guarded["dns_tcp"].ShouldBe("answered", $"and over TCP; probe: {Describe(guarded)}"); - guarded["gateway_53"].ShouldNotBe("open", $"but port 53 at the gateway {setup.HostIp}, which the resolv.conf does not name, is shut — check `nft list table inet {FilteredEgressPlan.NamespaceFor(runId)}` names {workerIp} on each port-53 accept; probe: {Describe(guarded)}"); + guarded["gateway_53"].ShouldNotBe("open", $"but port 53 at the gateway {gateway}, which the resolv.conf does not name, is shut — check `nft list table inet {FilteredEgressPlan.NamespaceFor(runId)}` names {workerIp} on each port-53 accept; probe: {Describe(guarded)}"); guarded["allowed"].ShouldBe("open", $"the allowlisted {Allowed} is still reachable through the namespace's NAT; probe: {Describe(guarded)}"); (await RunHostExitAsync(["nft", "delete", "table", "inet", FilteredEgressPlan.NamespaceFor(runId)])).ShouldBe(0, "control setup: the guard must be there to delete"); - var unguarded = await ProbeTheWorkerAsync(setup, workerIp, listener); + var unguarded = await ProbeTheWorkerAsync(setup, gateway, workerIp, listener); unguarded["gateway"].ShouldBe("open", $"control: with the guard gone the listener answers at the gateway, or the refusal above proved nothing about the guard; probe: {Describe(unguarded)}"); unguarded["worker"].ShouldBe("open", $"control: and at the worker's own address; probe: {Describe(unguarded)}"); @@ -417,7 +433,7 @@ public async Task A_guard_an_earlier_teardown_left_behind_is_replaced_not_added_ var setup = await FilteredEgressNetns.SetupAsync(runId, new[] { Allowed }, view.Path, timeoutSeconds: 20, CancellationToken.None); setup.SetupOk.ShouldBeTrue($"the allowlist namespace must set up over the stale table: {setup.SetupError}"); - var probe = await ProbeTheWorkerAsync(setup, workerIp, listener); + var probe = await ProbeTheWorkerAsync(setup, await GatewayOfAsync(runId), workerIp, listener); probe["dns_udp"].ShouldBe("answered", $"this setup's DNS rule must not sit behind the earlier guard's drop — check `nft list table inet {names.Namespace}` holds one input chain of four rules; probe: {Describe(probe)}"); probe["gateway"].ShouldNotBe("open", $"and its guard still stands; probe: {Describe(probe)}"); @@ -572,20 +588,21 @@ public async Task A_resolver_address_the_worker_redirects_to_itself_still_answer { setup.SetupOk.ShouldBeTrue($"the allowlist namespace must set up on this host: {setup.SetupError}"); - using var proxy = new WorkerResolver(setup.HostIp!); - var rewritten = await ProbePort53Async(setup.ExecPrefix, service, setup.HostIp!); + var gateway = await GatewayOfAsync(runId); + using var proxy = new WorkerResolver(gateway); + var rewritten = await ProbePort53Async(setup.ExecPrefix, service, gateway); rewritten["view"].ShouldBe(File.ReadAllText(view.Path), $"fixture: the namespace must read the resolv.conf the rules were built from; probe: {Describe(rewritten)}"); - rewritten["lookup"].ShouldBe(PinnedAnswer, $"a lookup through the resolv.conf, which names {service}, must be answered by the proxy the worker redirects it to at {setup.HostIp} — check each port-53 accept in `nft list table inet {FilteredEgressPlan.NamespaceFor(runId)}` matches `ct original ip daddr`; probe: {Describe(rewritten)}"); + rewritten["lookup"].ShouldBe(PinnedAnswer, $"a lookup through the resolv.conf, which names {service}, must be answered by the proxy the worker redirects it to at {gateway} — check each port-53 accept in `nft list table inet {FilteredEgressPlan.NamespaceFor(runId)}` matches `ct original ip daddr`; probe: {Describe(rewritten)}"); rewritten["resolver_udp"].ShouldBe("answered", $"and DNS to {service} directly over UDP; probe: {Describe(rewritten)}"); rewritten["resolver_tcp"].ShouldBe("answered", $"and over TCP; probe: {Describe(rewritten)}"); - rewritten["unlisted_tcp"].ShouldNotBe("open", $"but port 53 at the gateway {setup.HostIp}, which the file does not name, stays shut though the proxy listens there; probe: {Describe(rewritten)}"); + rewritten["unlisted_tcp"].ShouldNotBe("open", $"but port 53 at the gateway {gateway}, which the file does not name, stays shut though the proxy listens there; probe: {Describe(rewritten)}"); rewritten["unlisted_udp"].ShouldNotBe("answered", $"and a datagram to it unanswered; probe: {Describe(rewritten)}"); (await redirect.DeleteAsync()).ShouldBe(0, "control setup: the worker's REDIRECT must be there to delete"); var direct = await LookupAsync(setup.ExecPrefix); - direct.ShouldNotBe(PinnedAnswer, $"control: with the REDIRECT gone the lookup must not reach the proxy at {setup.HostIp}, or the answer above did not come through the worker's rewrite"); + direct.ShouldNotBe(PinnedAnswer, $"control: with the REDIRECT gone the lookup must not reach the proxy at {gateway}, or the answer above did not come through the worker's rewrite"); output.WriteLine($"{RanMarker} dns-redirect {Describe(rewritten)} control: lookup={direct}".Replace('\n', ' ')); } @@ -716,9 +733,9 @@ public void Dispose() } /// From inside the namespace: the worker's listener at the gateway and at its own address, DNS to the worker over UDP and TCP, port 53 at the gateway, and the allowlisted IP. - private static async Task> ProbeTheWorkerAsync(FilteredEgressNetns.SetupResult setup, string workerIp, TcpListener listener) + private static async Task> ProbeTheWorkerAsync(FilteredEgressNetns.SetupResult setup, string gateway, string workerIp, TcpListener listener) { - var stdout = await RunHostAsync(setup.ExecPrefix.Concat(["python3", "-c", WorkerProbeScript, setup.HostIp!, workerIp, PortOf(listener), Allowed]).ToList()); + var stdout = await RunHostAsync(setup.ExecPrefix.Concat(["python3", "-c", WorkerProbeScript, gateway, workerIp, PortOf(listener), Allowed]).ToList()); var line = stdout.Split('\n').Select(l => l.Trim()).LastOrDefault(l => l.StartsWith('{')); return line is null ? new Dictionary { ["gateway"] = $"no probe output: {stdout}", ["worker"] = "?", ["dns_udp"] = "?", ["dns_tcp"] = "?", ["gateway_53"] = "?", ["allowed"] = "?" } : JsonSerializer.Deserialize>(line)!; @@ -743,6 +760,18 @@ private static async Task> ProbePort53Async(IReadOnly /// The run's namespace, table and veth names: the run key's alone, so a plan on any lease names them as the setup did. private static FilteredEgressPlan NamesOf(string runId) => FilteredEgressPlan.Build(runId, Array.Empty(), new EgressSubnetAllocator.Lease { Cidr = "0.0.0.0/30", HostIp = "0.0.0.1", NsIp = "0.0.0.2" }, []); + /// The run's gateway: the IPv4 address on the host end of its veth (), as the kernel holds it — the /30 the setup reserved, read where it took effect. + private static async Task GatewayOfAsync(string runId) + { + var veth = NamesOf(runId).VethHost; + var listed = await RunHostAsync(["ip", "-4", "-o", "addr", "show", "dev", veth]); + var words = listed.Split(' ', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + var inet = Array.IndexOf(words, "inet"); + + (inet >= 0 && inet + 1 < words.Length).ShouldBeTrue($"fixture: the run's host veth {veth} must carry an IPv4 address; `ip -4 -o addr show dev {veth}` printed: {listed}"); + return words[inet + 1].Split('/')[0]; + } + /// Connect from the worker, send bytes and half-close, and return the peer's one-line answer — or the socket error that stopped it, or timeout. private static async Task AskAsync(string host, int port, int bytes) { diff --git a/backend/tests/CodeSpace.SandboxTests/ModelCredentialBrokerNetnsE2ETests.cs b/backend/tests/CodeSpace.SandboxTests/ModelCredentialBrokerNetnsE2ETests.cs index 3da825c20..a65dfa45a 100644 --- a/backend/tests/CodeSpace.SandboxTests/ModelCredentialBrokerNetnsE2ETests.cs +++ b/backend/tests/CodeSpace.SandboxTests/ModelCredentialBrokerNetnsE2ETests.cs @@ -18,8 +18,8 @@ namespace CodeSpace.SandboxTests; /// read-only into the sandbox (, /// which needs only bubblewrap and so runs as root and as the unprivileged worker uid alike), through the relay the /// production chain puts in front of the CLI, for a network-off and an allowlist child, until the lease is revoked — -/// and, for a run launched before the relay, at its namespace's gateway on a legacy re-bind, until its teardown by name -/// removes the namespace and the seal it carried. +/// and, for a run launched before the relay whose child calls its namespace's gateway, that a re-bind leaves nothing +/// answering there (). /// /// The second claim is the flip side: the bearer the sandbox holds is NOT the tenant's key. Sent straight to the /// provider it buys nothing, so a token that escapes a run is not a credential. @@ -276,69 +276,72 @@ public async Task A_namespaced_run_reaches_its_broker_and_is_refused_the_moment_ finally { if (allowlist) await FilteredEgressNetns.TeardownAsync(netnsKey, CancellationToken.None); } } + /// + /// 🟢 High fidelity (Rule 12): the gateway path to the broker is gone, observed on a live kernel. The run it served + /// was launched before the relay: its child froze its broker's address at its namespace's GATEWAY, and its handle + /// recorded a port, a route and a bearer but no socket. It is staged as it was, on a 10.x lease as the old allocator + /// handed them out, in the allowlist plan's namespace as it was built before its veth was guarded + /// (), so the only thing that decides whether the gateway answers is the broker's bind. + /// + /// A socketless re-bind on the next worker binds loopback like every lease: the worker reaches it there, and + /// the child at its gateway is refused. The control then binds the SAME port at the gateway and is answered, so the + /// refusal is the broker's bind and not a namespace that could not reach its gateway at all. + /// [Fact] - public async Task A_gateway_addressed_run_launched_before_the_relay_is_re_bound_wide_and_reaches_its_broker() + public async Task A_gateway_addressed_child_reaches_nothing_after_a_socketless_rebind() { - // The in-flight survivor this deploy must not strand: a namespaced run launched by the code before the relay, - // whose child froze a base URL at its namespace's GATEWAY and whose handle recorded no socket. Its re-bind is the - // legacy one: the recorded port on every address, wide first, where this host can build namespaces; and the - // broker's source gate admits the child's 10.x address. The namespace is the allowlist plan's as it was built - // before its veth was guarded (PreGuardPlan), applied on a 10.x lease as the old allocator handed them out, so - // the arm keeps meaning what it means when the pool moves. A network-off survivor was sealed through that veth - // by an inet table of its own, which only the teardown by name still deletes, so the arm stages that table too - // and ends by tearing the namespace down. if (!FilteredEgressNetns.IsSupported) return; // the root lane, with ip + nft, is authoritative - FilteredEgressNetns.CanFilter.ShouldBeTrue($"ip and nft are here, but this process could not build an allowlist namespace ({FilteredEgressNetns.FilterUnavailableReason}) — the survivor this arm stands for could not exist either"); + FilteredEgressNetns.CanFilter.ShouldBeTrue($"ip and nft are here, but this process could not build an allowlist namespace ({FilteredEgressNetns.FilterUnavailableReason}) — the survivor this arm stands for could not be staged"); var runId = Guid.NewGuid(); - var teamId = Guid.NewGuid(); var netnsKey = Guid.NewGuid().ToString("N"); - var third = RandomNumberGenerator.GetInt32(0, 64) * 4; - var second = RandomNumberGenerator.GetInt32(0, 256); - var lease = new EgressSubnetAllocator.Lease { Cidr = $"10.254.{second}.{third}/30", HostIp = $"10.254.{second}.{third + 1}", NsIp = $"10.254.{second}.{third + 2}" }; + var lease = LegacyLease(); try { - var plan = PreGuardPlan(netnsKey, lease); - var setup = await FilteredEgressNetns.ApplyAsync(netnsKey, plan, timeoutSeconds: 20, CancellationToken.None); - setup.SetupOk.ShouldBeTrue($"the allowlist-plan netns must set up on {lease.Cidr}; setup error: {setup.SetupError}"); - - // What the survivor's handle recorded: a port, a route and a bearer — and no socket. - var brokered = new BrokeredModelCredential("unused", McpRunTokenMint(), DateTimeOffset.UtcNow) { RebindPort = FreeLoopbackPort(), RebindRoute = McpPathIdMint() }; - var url = $"http://{setup.HostIp}:{brokered.RebindPort}/{brokered.RebindRoute}/v1/messages"; - var sealTable = plan.Namespace; + var setup = await FilteredEgressNetns.ApplyAsync(netnsKey, PreGuardPlan(netnsKey, lease), timeoutSeconds: 20, CancellationToken.None); + setup.SetupOk.ShouldBeTrue($"the pre-guard allowlist netns must set up on {lease.Cidr}; setup error: {setup.SetupError}"); - (await RunHostAsync(["nft", "-f", "-"], RetiredSealRuleset(sealTable, plan.VethHost, plan.HostIp, brokered.RebindPort!.Value))).ShouldBe(0, "setup: the seal a network-off survivor carries must load on this host"); - (await RunHostAsync(["nft", "list", "table", "inet", sealTable])).ShouldBe(0, "control: the survivor's seal is there before its run ends"); - - (await CurlAsync(setup.ExecPrefix, url, brokered.RunToken)).Exit.ShouldNotBe(0, "precondition: the worker that minted the address is gone, so nothing answers at the gateway"); + // What the survivor's handle recorded: a port, a route and a bearer, and no socket. + var port = FreeLoopbackPort(); + var route = CodeSpace.Core.Services.Agents.Mcp.McpRunToken.MintPathId(); + var token = CodeSpace.Core.Services.Agents.Mcp.McpRunToken.Mint(); + var gatewayUrl = $"http://{lease.HostIp}:{port}/{route}/v1/messages"; using var workerB = LoopbackModelCredentialBroker.ForTest(new AlwaysOkUpstream()); - (await workerB.RebindAsync(RebindOf(brokered, runId, teamId, epoch: 2) with { ChildInNetworkNamespace = true }, CancellationToken.None)).ShouldBeTrue("the new worker must re-open the survivor's recorded address"); - workerB.ListenerPrefixForTest(runId).ShouldBe($"http://+:{brokered.RebindPort}/", "a re-bind with no socket on a host that builds namespaces takes the wide bind, first"); + (await workerB.RebindAsync(SocketlessRebind(runId, port, route, token), CancellationToken.None)).ShouldBeTrue("the next worker must re-open the recorded port"); + workerB.ListenerPrefixForTest(runId).ShouldBe($"http://127.0.0.1:{port}/", "a re-bind without a socket binds loopback like every lease, on a host that builds namespaces too"); - (await CurlInNetnsAsync(setup.ExecPrefix, url, brokered.RunToken)).ShouldBe("200", - customMessage: $"the survivor must reach its re-bound broker at its gateway {setup.HostIp}; if curl cannot connect the re-bind took loopback instead of the wide bind — check by hand: `ip netns exec {FilteredEgressPlan.NamespaceFor(netnsKey)} curl -v {url}`"); + (await CurlAsync($"http://127.0.0.1:{port}/{route}/v1/messages", token)).ShouldBe((0, "200"), customMessage: "control: the re-bound lease answers where it binds, so the refusal below is about where it binds"); - output.WriteLine($"{RanMarker} legacy-gateway-rebind lease={lease.Cidr}"); + var (gatewayExit, gatewayStatus) = await CurlAsync(gatewayUrl, token, setup.ExecPrefix); - // The run's terminal path: the teardown by name, reconstructed from the run key alone, as a later worker does. - await FilteredEgressNetns.TeardownAsync(netnsKey, CancellationToken.None); + gatewayExit.ShouldBe(7, + customMessage: $"a child calling its gateway must be refused (curl exit 7) — got exit {gatewayExit}, status '{gatewayStatus}'. Exit 0 means the broker answered at the gateway, so a wide bind is back; check by hand: `ip netns exec {FilteredEgressPlan.NamespaceFor(netnsKey)} curl -v {gatewayUrl}`"); - (await RunHostAsync(["nft", "list", "table", "inet", sealTable])).ShouldNotBe(0, $"the survivor's seal must go with its run, or every sealed run in flight at the deploy leaks an nft table on the worker — check `nft list tables | grep {sealTable}`"); - (await RunHostAsync(["ip", "netns", "pids", sealTable])).ShouldNotBe(0, $"and so must its namespace — check `ip netns list | grep {sealTable}`"); + (await AnsweredAtGatewayAsync(lease.HostIp, port, setup.ExecPrefix)).ShouldBe("204", + customMessage: $"control: a listener bound at the gateway on the same port must be answered from inside, or the refusal above proved nothing about the broker's bind — check `ip netns exec {FilteredEgressPlan.NamespaceFor(netnsKey)} ip route`"); - output.WriteLine($"{RanMarker} legacy-sealed-teardown table={sealTable}"); + output.WriteLine($"{RanMarker} gateway-retired uid={EffectiveUid()} lease={lease.Cidr} gatewayExit={gatewayExit}"); } finally { await FilteredEgressNetns.TeardownAsync(netnsKey, CancellationToken.None); } } + /// A /30 from the range the allocator walked (10.1.1.0 up to 10.254.254.252) before it moved to 198.19.64.0–198.19.191.255, which is where a run launched before the relay still sits. Its top /16, random within it, so repeated runs rarely share one. + private static EgressSubnetAllocator.Lease LegacyLease() + { + var second = RandomNumberGenerator.GetInt32(1, 255); + var third = RandomNumberGenerator.GetInt32(0, 64) * 4; + + return new() { Cidr = $"10.254.{second}.{third}/30", HostIp = $"10.254.{second}.{third + 1}", NsIp = $"10.254.{second}.{third + 2}" }; + } + /// /// The allowlist plan a run launched before the guard on its veth was built with: the same setup and forward table /// (), and no inet table of the plan's own. A new namespace drops what - /// its child sends the worker, gateway included; a survivor's does not, which is how it still reaches its broker there. + /// its child sends the worker, gateway included; a survivor's does not, which is how it reached its broker there. /// private static FilteredEgressPlan PreGuardPlan(string netnsKey, EgressSubnetAllocator.Lease lease) { @@ -347,43 +350,42 @@ private static FilteredEgressPlan PreGuardPlan(string netnsKey, EgressSubnetAllo return plan with { NftRuleset = FilteredEgressPlan.BuildNftRuleset(plan.Namespace, lease.Cidr, Array.Empty(), Array.Empty()) }; } - /// - /// The inet table the veth seal (#2035) loaded for a network-off run: input from the run's veth only to the broker's - /// port at the gateway, nothing forwarded. A copy of the ruleset that change shipped, kept only to stage what a run - /// sealed before the relay still carries; the plan that built it is gone, so there is no live source for it to drift - /// from, and the teardown under test deletes the table by its name whatever it holds. - /// - private static string RetiredSealRuleset(string table, string vethHost, string hostIp, int brokerPort) => string.Join("\n", new[] + /// The re-bind a survivor's handle yields: its recorded port, route and bearer, and no socket path. + private static ModelCredentialRebindRequest SocketlessRebind(Guid runId, int port, string route, string token) => new() { - $"table inet {table} {{", - " chain input {", - " type filter hook input priority 0;", - $" iifname \"{vethHost}\" ct state established,related accept", - $" iifname \"{vethHost}\" ip daddr {hostIp} tcp dport {brokerPort} accept", - $" iifname \"{vethHost}\" drop", - " }", - " chain forward {", - " type filter hook forward priority 0;", - $" iifname \"{vethHost}\" drop", - " }", - "}", - }) + "\n"; - - /// Run one host command as this worker, feeding when given, and return its exit code. - private static async Task RunHostAsync(IReadOnlyList argv, string? stdin = null) + RunId = runId, TeamId = Guid.NewGuid(), Epoch = 2, Port = port, PathId = route, RunToken = token, + Upstream = new() { Provider = "Anthropic", ApiKey = "sk-e2e-upstream-key" }, Ttl = TimeSpan.FromMinutes(5), + }; + + /// Bind at the gateway , answer one request 204, and return the status the namespace's curl saw — or curl's exit code when it saw none. + private static async Task AnsweredAtGatewayAsync(string hostIp, int port, IReadOnlyList execPrefix) { - var psi = new ProcessStartInfo { FileName = argv[0], UseShellExecute = false, RedirectStandardInput = true, RedirectStandardOutput = true, RedirectStandardError = true }; - foreach (var argument in argv.Skip(1)) psi.ArgumentList.Add(argument); + using var control = new System.Net.HttpListener(); + control.Prefixes.Add($"http://{hostIp}:{port}/"); + control.Start(); - using var process = Process.Start(psi)!; + _ = AnswerOnceAsync(control); - await process.StandardInput.WriteAsync(stdin ?? ""); - process.StandardInput.Close(); - await process.StandardOutput.ReadToEndAsync(); - await process.StandardError.ReadToEndAsync(); - await process.WaitForExitAsync(); + var (exit, status) = await CurlAsync($"http://{hostIp}:{port}/", "control", execPrefix); + + return exit == 0 ? status : $"curl exit {exit}"; + } + + private static async Task AnswerOnceAsync(System.Net.HttpListener listener) + { + var context = await listener.GetContextAsync(); - return process.ExitCode; + context.Response.StatusCode = 204; + context.Response.Close(); + } + + /// A port free on loopback right now, allocated by the kernel (Rule 12.8). + private static int FreeLoopbackPort() + { + using var probe = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + probe.Start(); + + return ((System.Net.IPEndPoint)probe.LocalEndpoint).Port; } /// @@ -434,18 +436,6 @@ private static (int Exit, string Status) RunChainToExit(IReadOnlyList ar return (process.ExitCode, stdout.Result.Trim()); } - private static int FreeLoopbackPort() - { - using var probe = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); - probe.Start(); - - return ((System.Net.IPEndPoint)probe.LocalEndpoint).Port; - } - - private static string McpRunTokenMint() => CodeSpace.Core.Services.Agents.Mcp.McpRunToken.Mint(); - - private static string McpPathIdMint() => CodeSpace.Core.Services.Agents.Mcp.McpRunToken.MintPathId(); - private static ModelCredentialLeaseRequest LeaseFor(Guid runId, Guid teamId) => new() { RunId = runId, TeamId = teamId, Epoch = 1, Upstream = new() { Provider = "Anthropic", ApiKey = "sk-e2e-upstream-key" }, Ttl = TimeSpan.FromMinutes(5) }; @@ -469,7 +459,7 @@ public async Task A_run_token_presented_to_the_provider_directly_is_refused() if (brokered is null) return; - var (exit, status) = await CurlAsync(null, $"https://{ProviderHost}/v1/messages", brokered.RunToken); + var (exit, status) = await CurlAsync($"https://{ProviderHost}/v1/messages", brokered.RunToken); if (exit != 0) return; // no egress from this runner at all — nothing to observe, and the CI job with network is authoritative @@ -486,17 +476,8 @@ private static string ReachableUrl(BrokeredModelCredential brokered) return LocalProcessRunner.ResolveModelBrokerHost(spec).Environment["URL"]; } - private static async Task CurlInNetnsAsync(IReadOnlyList execPrefix, string url, string token) - { - var (exit, status) = await CurlAsync(execPrefix, url, token); - - exit.ShouldBe(0, $"curl itself failed inside the namespace (exit {exit}) — that is a reachability failure, not a refusal. Diagnose with: `{string.Join(' ', execPrefix)} curl -v {url}`"); - - return status; - } - - /// POST to with the bearer and report (curl exit code, HTTP status). Run behind when one is given, so the request originates INSIDE the namespace. - private static async Task<(int Exit, string Status)> CurlAsync(IReadOnlyList? execPrefix, string url, string token) + /// POST to with the bearer and report (curl exit code, HTTP status) — from this worker, or from inside a namespace behind when one is given. + private static async Task<(int Exit, string Status)> CurlAsync(string url, string token, IReadOnlyList? execPrefix = null) { var argv = (execPrefix ?? Array.Empty()).Concat(new[] { diff --git a/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs index be2ca9405..d0ac58d98 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs @@ -132,27 +132,35 @@ public async Task A_new_lease_listens_on_loopback_only_even_where_a_per_run_name } [Theory] - [InlineData(false, true)] // the legacy re-bind: a handle written before the socket, whose namespaced child calls the gateway - [InlineData(false, false)] // no socket and no namespace: a child on the worker's own network (network on, or an unconfined host) calls loopback - [InlineData(true, true)] // a handle with a socket: its child comes in through it, spliced to loopback - public async Task Only_a_rebind_of_a_namespaced_child_without_a_socket_binds_wide_and_only_where_a_namespace_can_exist(bool socket, bool netns) + [InlineData(false)] // no socket: a child on the worker's own network, calling loopback — the row that bound every address while namespaced runs from before the relay were still in flight + [InlineData(true)] // a socket: its child comes in through it, spliced to loopback + public async Task A_rebind_binds_loopback_only_with_a_socket_or_without_even_where_a_per_run_namespace_can_exist(bool socket) { - // Mutation: key the wide bind on the missing socket alone, and the no-namespace row goes red on a host that - // builds filtered-egress namespaces — a Trusted run's loopback lease would come back on every address. + // Mutation: bind the recorded port on every address again for a re-bind without a socket, and the first row + // goes red. if (socket && !Socket.OSSupportsUnixDomainSockets) return; using var sockets = new BrokerSockets(); using var broker = LoopbackModelCredentialBroker.ForTest(new StubUpstream()); var port = ReserveLoopbackPort(); - var request = RebindOn(port, epoch: 3) with { SocketPath = socket ? sockets.NewPath() : null, ChildInNetworkNamespace = netns }; + var request = RebindOn(port, epoch: 3) with { SocketPath = socket ? sockets.NewPath() : null }; if (!await broker.RebindAsync(request, CancellationToken.None)) return; // the port was taken in between — nothing to observe - var wide = !socket && netns && FilteredEgressNetns.IsSupported; - - broker.ListenerPrefixForTest(request.RunId).ShouldBe(wide ? $"http://+:{port}/" : $"http://127.0.0.1:{port}/", $"only a veth-sealed or allowlist run launched before the relay reaches this worker at its namespace gateway, so its re-bind alone keeps the wide bind until they drain; every other child calls loopback (namespaces possible here: {FilteredEgressNetns.IsSupported})"); + broker.ListenerPrefixForTest(request.RunId).ShouldBe($"http://127.0.0.1:{port}/", $"a re-bind binds loopback alone, as an open does: no child calls this worker anywhere else, so any wider bind only hands the port to the host's neighbours (namespaces possible here: {FilteredEgressNetns.IsSupported})"); } + [Theory] + [InlineData("127.0.0.1", true)] // a child on the worker's own network, or a relayed one whose socket this worker splices to the port + [InlineData("::1", true)] + [InlineData("10.1.1.2", false)] // the 10/8 space per-run /30s were carved from before the pool moved, admitted while runs launched before the relay still called from it + [InlineData("198.19.64.2", false)] // a /30 from today's pool: its run is relayed, so it never calls from there + [InlineData("192.168.1.20", false)] // a neighbour on the host's network + [InlineData(null, false)] + public void Only_loopback_is_a_plausible_sandbox_source(string? source, bool plausible) => + LoopbackModelCredentialBroker.IsPlausibleSandboxSource(source is null ? null : IPAddress.Parse(source)).ShouldBe(plausible, + customMessage: "every child's call reaches the broker from loopback — directly, or spliced from its socket by this worker — so any other source is a neighbour probing the port, refused before the bearer is even weighed"); + /// A loopback port free at the moment of asking — a re-bind names its port, it never picks one. private static int ReserveLoopbackPort() { @@ -503,11 +511,10 @@ public async Task Rebind_of_an_address_this_worker_already_serves_adopts_it_inst (await broker.RebindAsync(RebindOf(live, runId, epoch: 9), CancellationToken.None)).ShouldBeTrue( "an address that is already up IS restored — answering false here would make a re-attach end a run whose model access never went anywhere"); - // Re-binding would have walked the candidate hosts against a port THIS PROCESS holds; on Linux the wide bind - // fails while loopback succeeds underneath it, so Install would close the live WIDE listener and a sealed - // netns run would lose its broker while this returned true. The address has to be untouched. + // Re-binding would have asked for a port THIS PROCESS holds: refused, a live run ends typed; granted, Install + // closes the listener the child is calling while this returns true. The address has to be untouched. (await CallAsync(live, "/v1/messages", live.RunToken)).StatusCode.ShouldBe(HttpStatusCode.OK, - customMessage: "the address must still answer after the adoption — a re-bind that closed and re-opened it would drop a sealed run's wide bind down to loopback, which reads as success here and as a dead run in production"); + customMessage: "the address must still answer after the adoption — a re-bind that closed and re-opened it would read as success here and as a dead run in production"); upstream.Calls.ShouldBe(1); (await broker.RenewAsync(runId, 9, CancellationToken.None)).ShouldBeTrue( @@ -942,48 +949,6 @@ public async Task A_lease_whose_socket_acceptor_died_stops_being_claimed() logger.Warnings.ShouldContain(line => line.Contains(runId.ToString(), StringComparison.Ordinal), "and the drop says so, naming the run"); } - [Theory] - [InlineData(true, false, true)] // a namespace and no socket: a child that reaches this worker at its namespace's gateway — the survivor the gateway path's retirement waits on - [InlineData(true, true, false)] // a namespace and a socket: its child comes in through the socket - [InlineData(false, false, false)] // no namespace: a child on the worker's own network, calling loopback — every shared-network run, and every run on a host that builds no namespaces - [InlineData(false, true, false)] // a socket and no namespace - public async Task Only_a_rebind_of_a_handle_with_a_network_namespace_and_no_socket_says_it_is_a_legacy_rebind(bool netns, bool socket, bool legacyLine) - { - if (!Socket.OSSupportsUnixDomainSockets) return; - - using var sockets = new BrokerSockets(); - using var occupied = new OccupiedPort(); - var port = occupied.Port; - occupied.Dispose(); // a port nothing holds: the re-bind must take, so the only thing that differs between the rows is the line - - var owner = new AgentRunOwnerToken(Guid.NewGuid(), Guid.NewGuid(), 8); - var credentialId = Guid.NewGuid(); - var handle = new SandboxHandle - { - Kind = "local", ProcessId = 1, SpoolDirectory = "/tmp", Deadline = DateTimeOffset.UtcNow, LaunchHost = LocalProcessRunner.CurrentHost, - ModelBrokerPort = port, ModelBrokerRoute = McpRunToken.MintPathId(), ModelBrokerRunToken = McpRunToken.Mint(), ModelBrokerProvider = "Anthropic", ModelBrokerCredentialId = credentialId, - EgressNetnsKey = netns ? owner.RunId.ToString("N") : null, ModelBrokerSocketPath = socket ? sockets.NewPath() : null, - }; - var logger = new CapturingLogger(); - using var broker = LoopbackModelCredentialBroker.ForTest(new StubUpstream(), logger: logger); - - // The executor's reading of the handle, then the broker's re-bind of it — the whole chain the line is decided on. - var request = AgentRunExecutor.RebindRequestFor(owner, Guid.NewGuid(), handle, new() { Provider = "Anthropic", CredentialId = credentialId }).ShouldNotBeNull(); - - (await broker.RebindAsync(request, CancellationToken.None)).ShouldBeTrue("precondition: the re-bind took"); - - logger.Informations.Any(line => line.Contains(LoopbackModelCredentialBroker.LegacyRebindMarker, StringComparison.Ordinal)).ShouldBe(legacyLine, - customMessage: "the gateway path to the broker is retired only once no legacy re-bind has happened for a while, and this line is what says one did. It must name exactly the runs that retirement would cut off — a child in a network namespace with no socket to come in through. Missing there, the retirement reads silence and strands them; present for a run on the worker's own network (which calls loopback) or one with a socket, it never reads silence at all"); - } - - [Fact] - public void The_legacy_rebind_marker_is_pinned() - { - // Retiring the gateway path waits on this line going quiet in the deployment's logs, so the words are what an - // operator's query matches. A rename is a decision to change that query, not a refactor. - LoopbackModelCredentialBroker.LegacyRebindMarker.ShouldBe("legacy model-broker re-bind"); - } - /// /// POST one model call to a lease THROUGH its Unix socket — the way a sandboxed child reaches it — addressed as that /// child addresses it (127.0.0.1:<port>/<route>), so the Host header the loopback listener matches @@ -1076,6 +1041,27 @@ public void A_rebind_is_only_built_for_a_handle_whose_agent_this_host_can_reach( customMessage: "the agent calls a port on the machine it was LAUNCHED on. Binding that number on a different worker answers nobody at all — and because the caller reads a built request as 'the address is back', it would also clear the posture that says this run has no model, leaving a dead run recorded as healthy and never landed"); } + [Theory] + [InlineData(true, false, false)] // a namespace and no socket: a run launched before the relay, whose child calls its namespace's gateway, where nothing listens any more + [InlineData(true, true, true)] // a namespace and a socket: its child comes in through the socket + [InlineData(false, false, true)] // no namespace: a child on the worker's own network, calling loopback + [InlineData(false, true, true)] // a socket and no namespace + public void A_rebind_is_not_built_for_a_handle_whose_child_calls_its_namespace_gateway(bool netns, bool socket, bool built) + { + var credentialId = Guid.NewGuid(); + var handle = new SandboxHandle + { + Kind = "local", ProcessId = 1, SpoolDirectory = "/tmp", Deadline = DateTimeOffset.UtcNow, LaunchHost = LocalProcessRunner.CurrentHost, + ModelBrokerPort = 44444, ModelBrokerRoute = "route-id", ModelBrokerRunToken = "a-recorded-run-token", ModelBrokerProvider = "Anthropic", ModelBrokerCredentialId = credentialId, + EgressNetnsKey = netns ? "a-recorded-netns-key" : null, ModelBrokerSocketPath = socket ? "/spool/k/broker/segment/s" : null, + }; + + var request = AgentRunExecutor.RebindRequestFor(new(Guid.NewGuid(), Guid.NewGuid(), 8), Guid.NewGuid(), handle, new() { Provider = "Anthropic", CredentialId = credentialId }); + + (request is not null).ShouldBe(built, + customMessage: "every lease binds loopback, so a child that calls its namespace's gateway reaches nothing a re-bind could open. Built anyway, the re-bind would read as 'the address is back' and clear the posture that says the run lost its model access, leaving an agent that can make no call recorded as healthy; declined, the run lands typed and its agent is stopped"); + } + [Theory] [InlineData("/spool/k/broker/segment/s")] [InlineData(null)] @@ -1092,7 +1078,7 @@ public void A_rebind_carries_the_socket_its_handle_recorded(string? socketPath) var request = AgentRunExecutor.RebindRequestFor(new(Guid.NewGuid(), Guid.NewGuid(), 8), Guid.NewGuid(), handle, new() { Provider = "Anthropic", CredentialId = credentialId }).ShouldNotBeNull(); request.SocketPath.ShouldBe(socketPath, - customMessage: "the re-bind must re-open the socket the launch's lease served, or a sandboxed child whose only door is that socket is left calling nothing; and a handle that recorded none must ask for none, which is what keeps it on the legacy wide re-bind its gateway-addressed child needs"); + customMessage: "the re-bind must re-open the socket the launch's lease served, or a sandboxed child whose only door is that socket is left calling nothing; and a handle that recorded none must ask for none, since its child calls loopback"); } /// Stands in for this worker's own host identity inside [InlineData], which cannot carry a runtime value. @@ -1188,9 +1174,6 @@ private sealed class CapturingLogger : Microsoft.Extensions.Logging.ILogger Warnings { get; } = []; - /// The Information lines, kept apart from so the assertions on those stay exactly what they were. - public List Informations { get; } = []; - /// Released AFTER each warning is recorded, so a waiter that acquires it reads a that already holds that line. public SemaphoreSlim Warned { get; } = new(0); @@ -1199,8 +1182,6 @@ private sealed class CapturingLogger : Microsoft.Extensions.Logging.ILogger(Microsoft.Extensions.Logging.LogLevel logLevel, Microsoft.Extensions.Logging.EventId eventId, TState state, Exception? exception, Func formatter) { - if (logLevel == Microsoft.Extensions.Logging.LogLevel.Information) Informations.Add(formatter(state, exception)); - if (logLevel < Microsoft.Extensions.Logging.LogLevel.Warning) return; Warnings.Add(formatter(state, exception));