diff --git a/.github/workflows/backend-e2e.yml b/.github/workflows/backend-e2e.yml index 64c760f9d..74f1a88e0 100644 --- a/.github/workflows/backend-e2e.yml +++ b/.github/workflows/backend-e2e.yml @@ -173,13 +173,15 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install bubblewrap + util-linux (prlimit) + - name: Install bubblewrap + util-linux (prlimit) + iproute2/nftables # The headline E2E spawns a REAL agent process through the production runner, which wraps it in # bubblewrap + prlimit (the production confinement posture). Running as root in the SDK container, so - # no sudo. ca-certificates so the container can fetch packages; util-linux for prlimit. + # no sudo. ca-certificates so the container can fetch packages; util-linux for prlimit; iproute2 and + # nftables so a network-off brokered agent is SEALED to its broker, as a confining host must, rather than + # refused as sandbox_sealed_egress_unavailable. run: | apt-get update - apt-get install -y --no-install-recommends bubblewrap util-linux ca-certificates + apt-get install -y --no-install-recommends bubblewrap util-linux ca-certificates iproute2 nftables echo "bwrap: $(bwrap --version)" echo "prlimit: $(prlimit --version)" diff --git a/.github/workflows/sandbox-isolation.yml b/.github/workflows/sandbox-isolation.yml index cf04a9b90..e29c0d4e5 100644 --- a/.github/workflows/sandbox-isolation.yml +++ b/.github/workflows/sandbox-isolation.yml @@ -224,8 +224,8 @@ jobs: executed=$(grep -oE 'executed="[0-9]+"' "$trx" | head -1 | grep -oE '[0-9]+') passed=$(grep -oE 'passed="[0-9]+"' "$trx" | head -1 | grep -oE '[0-9]+') echo "executed=${executed:-0} passed=${passed:-0}" - if [ "${executed:-0}" -lt 65 ]; then - echo "::error::Expected >=65 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 sealed to its broker + a read-only reviewer reading its diff with the real CLIs), 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." + if [ "${executed:-0}" -lt 66 ]; then + echo "::error::Expected >=66 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 sealed to its broker + a read-only reviewer reading its diff with the real CLIs), 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 @@ -258,12 +258,12 @@ jobs: print(f'All {len(arms)} reviewer E2E arms ran and passed.') # The sealed-egress E2E returns early on a host that cannot seal, which reads as Passed; require each arm's marker. - for arm in ('durable', 'non-durable', 'ipv6', 'restart-reissue', 'policy-route-discard'): + for arm in ('durable', 'non-durable', 'ipv6', 'restart-reissue', 'policy-route-discard', 'setup-failure'): assert f'[sealed-egress-e2e] ran {arm}' in text, f'sealed-egress E2E arm "{arm}" did not run — this lane is root with bwrap, ip and nft, so it must seal' - for method, rows in (('A_network_off_brokered_run_reaches_its_broker_and_nothing_else', 2), ('A_sealed_namespace_drops_the_gateway_over_ipv6_link_local_too', 1), ('A_30_still_held_by_a_run_that_outlived_its_worker_is_not_handed_to_the_next_run', 1), ('A_host_whose_policy_rule_discards_the_run_s_replies_fails_the_setup_and_leaks_nothing', 1)): + for method, rows in (('A_network_off_brokered_run_reaches_its_broker_and_nothing_else', 2), ('A_sealed_namespace_drops_the_gateway_over_ipv6_link_local_too', 1), ('A_30_still_held_by_a_run_that_outlived_its_worker_is_not_handed_to_the_next_run', 1), ('A_host_whose_policy_rule_discards_the_run_s_replies_fails_the_setup_and_leaks_nothing', 1), ('A_sealed_setup_that_fails_on_this_host_refuses_the_launch_typed_and_leaks_nothing', 1)): cases = [r for r in results if 'SealedEgressE2ETests.' + method in r.get('testName', '')] assert len(cases) == rows and all(r.get('outcome') == 'Passed' for r in cases), f'{method}: all {rows} case(s) must pass' - print('All 5 sealed-egress arms ran and passed.') + print('All 6 sealed-egress arms ran and passed.') print('All 10 batch/stream kernel cases passed.') PY diff --git a/backend/Dockerfile.worker b/backend/Dockerfile.worker index bf08929d6..192756ab7 100644 --- a/backend/Dockerfile.worker +++ b/backend/Dockerfile.worker @@ -34,7 +34,8 @@ # The same namespace machinery SEALS a network-off run whose model is brokered to that broker (no route, no NAT, no # DNS, one gateway port). It is taken only where bubblewrap confines AND FilteredEgressNetns.CanSeal has proved, by # building a throwaway namespace, that this process may: root + CAP_NET_ADMIN + CAP_SYS_ADMIN, no sysctl needed. A -# pod without them keeps severing such a run — which also severs it from its broker, so it reaches no model. +# pod without them that DOES confine refuses such a run before it spends anything (sandbox_sealed_egress_unavailable), +# because severing it instead would cut it off from its broker and leave it reaching no model. # # CONFINEMENT ARMING: bubblewrap is installed here so the capability is PRESENT, but the fail-closed guard # Sandbox:RequireConfinement is left to the DEPLOYMENT to arm (k8s pod / compose) on a host that grants user diff --git a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs index 346920cd8..662ec191b 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs @@ -484,6 +484,11 @@ public async Task ExecuteAsync(Guid agentRunId, CancellationToken cancellationTo spec = BuildSpec(RunCold(effectiveTask)); } + // A spec this runner could only launch with a network that leaves the agent unable to work — a network-off + // brokered run on a host that confines but cannot seal — is refused HERE, the last moment a refusal costs + // nothing: before the local acceptance is prepared, the spend admitted or a process started. + (runner as ISandboxEgressAdmission)?.EnsureEgressAdmissible(spec, brokeredCredential?.ReachableFromNamespace ?? false); + // Verification is judged against the contract the envelope persisted — never a goal amended for the // dispatch alone (this cold hint, or an unreadable checkpoint's) — or the contract hash cannot match. var contract = effectiveTask with { Goal = task.Goal }; 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 22a00f800..bb6c682d7 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Credentials/Broker/LoopbackModelCredentialBroker.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Credentials/Broker/LoopbackModelCredentialBroker.cs @@ -173,7 +173,7 @@ private static HttpMessageHandler DefaultUpstreamHandler() => _logger.LogDebug("Model credential brokered for agent run {RunId} on port {Port} (team {TeamId}, epoch {Epoch}) until {ExpiresAt:O}", lease.RunId, lease.Port, lease.TeamId, lease.Epoch, lease.ExpiresAt); WarnIfUnreachableFromNetns(lease, bound.Host); - return Task.FromResult(new(BaseUrlFor(lease), lease.Token, lease.ExpiresAt) { RebindPort = lease.Port, RebindRoute = lease.PathId }); + return Task.FromResult(new(BaseUrlFor(lease), lease.Token, lease.ExpiresAt) { RebindPort = lease.Port, RebindRoute = lease.PathId, ReachableFromNamespace = bound.Host == AnyHost }); } /// diff --git a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Exceptions/SealedEgressUnavailableException.cs b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Exceptions/SealedEgressUnavailableException.cs new file mode 100644 index 000000000..a29051085 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Exceptions/SealedEgressUnavailableException.cs @@ -0,0 +1,50 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Core.Services.Agents.Sandbox.Exceptions; + +/// +/// A network-off run whose model is brokered, REFUSED because this host would confine it but cannot seal its network +/// to that broker. Severing it instead — what a host that cannot seal would otherwise do — cuts the broker off along +/// with everything else, so the agent reaches no model and the run burns its whole timeout and the CLI's retries on +/// failures that read like a provider outage. Refusing names the wall instead, and does it before anything is spent. +/// +/// Unavailable, like : nothing about the launch can be +/// changed to make it work, and the identical launch succeeds untouched once an operator grants the worker what a +/// sealed namespace needs — or, for a setup step that failed on a host that can seal (), +/// fixes what that step names. says which it was, in the same words the message uses. +/// +public sealed class SealedEgressUnavailableException : Exception, IFailure +{ + /// The worker lacks the ip or nft binary a sealed namespace is built with. + public const string CauseMissingTools = "ip or nft is not installed on this worker"; + + /// The binaries are there, but this process could not build a throwaway namespace. + public const string CauseNoPrivilege = "this worker may not create a network namespace (it needs root with CAP_NET_ADMIN and CAP_SYS_ADMIN)"; + + /// The run's model broker could only listen on loopback, which a sealed namespace cannot reach. + public const string CauseBrokerLoopbackOnly = "the run's model broker could only listen on loopback, which a sealed namespace cannot reach"; + + /// What an operator does about a worker that cannot build a sealed namespace at all. + private const string GrantRemedy = "Grant the worker what a sealed namespace needs (see backend/Dockerfile.worker, EGRESS FILTERING); a retry on this host helps only once it can build one, which it re-checks at most once a minute."; + + /// What an operator does about one setup step that failed on a worker that can build a sealed namespace. + private const string SetupRemedy = "This worker can build a sealed namespace, but a step of this one failed on its host: fix what that step names (a route or policy rule that discards the run's /30, or a namespace or veth left behind under the run's name). Every launch runs the setup afresh."; + + public SealedEgressUnavailableException(string cause) : this(cause, GrantRemedy) { } + + private SealedEgressUnavailableException(string cause, string remedy) + : base($"This run's network is off and its model is reached through its broker; on this worker that is enforced with a network namespace sealed to that broker, but one cannot be built here: {cause}. Refusing to launch an agent that could not reach its model. {remedy}") + { + Cause = cause; + } + + /// A sealed setup that failed at launch on a host that proved it can seal — a name collision, a route the kernel will not send the run's replies down — carrying the failed step's own error. + public static SealedEgressUnavailableException SetupFailed(string? setupError) => new($"the sealed namespace's setup failed: {setupError}", SetupRemedy); + + /// Which wall the host hit — one of the Cause* constants, or a failed setup step's own error. + public string Cause { get; } + + FailureKind IFailure.Kind => FailureKind.Unavailable; + string IFailure.Code => FailureCodes.SandboxSealedEgressUnavailable; + string? IFailure.ClientMessage => "This host cannot give a network-off run a sealed route to its model."; +} diff --git a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/ISandboxEgressAdmission.cs b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/ISandboxEgressAdmission.cs new file mode 100644 index 000000000..4262a75e9 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/ISandboxEgressAdmission.cs @@ -0,0 +1,24 @@ +using CodeSpace.Messages.Agents; + +namespace CodeSpace.Core.Services.Agents.Sandbox; + +/// +/// Optional capability a sandbox runner MAY implement alongside (Rule 7 / ISP — a sibling +/// interface, never a widening of the base contract): refuse, BEFORE anything is spent, a spec this runner could only +/// launch with a network that leaves the agent unable to do its work. The runner is the one place that knows what it +/// can actually build on this host, and the executor asks it at the one moment a refusal still costs nothing — after +/// the spec is built, before the spend is admitted and the process started. +/// +/// The first such spec is a network-off run whose model is brokered () +/// on a host that confines but cannot seal: severing it would cut its broker off too. A runner without this capability +/// simply launches, exactly as it always has. +/// +public interface ISandboxEgressAdmission +{ + /// + /// Throws an naming the wall when this runner cannot give + /// the egress it needs; returns otherwise. + /// is whether the run's broker listens where a per-run namespace can reach it (BrokeredModelCredential.ReachableFromNamespace). + /// + void EnsureEgressAdmissible(SandboxSpec spec, bool modelBrokerReachableFromNamespace); +} diff --git a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.Durable.cs b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.Durable.cs index b292ac753..3a85afb0c 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.Durable.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.Durable.cs @@ -4,6 +4,7 @@ using System.Text; using CodeSpace.Core.Services.Agents.AgentRunLogging; using CodeSpace.Core.Services.Agents.Mcp; +using CodeSpace.Core.Services.Agents.Sandbox.Exceptions; using CodeSpace.Core.Services.Agents.Sandbox.Isolation; using CodeSpace.Messages.Agents; using CodeSpace.NativeLaunch; @@ -354,12 +355,36 @@ private static bool TryFileLength(string path, out long length) { var setup = await FilteredEgressNetns.SetupSealedAsync(spoolKey, brokerPort, EgressSetupTimeoutSeconds, ct).ConfigureAwait(false); + // The same refusal EnsureEgressAdmissible raises before any spend, for the rarer case the probe could not + // foresee — a setup step that fails on a host that proved it can seal (a name collision, a kernel refusal). if (!setup.SetupOk) - throw new InvalidOperationException($"Sealed-egress netns setup failed (fail-closed — run aborted rather than launched with a network it was not given): {setup.SetupError}"); + throw SealedEgressUnavailableException.SetupFailed(setup.SetupError); return (setup.ExecPrefix, spoolKey, setup.HostIp); } + /// + /// Refuse, before anything is spent, a network-off brokered run this host would confine but cannot seal — the + /// mirror of , which would otherwise quietly sever it from its broker. A spec with + /// no broker port, or a host that does not confine, is admitted untouched: nothing about its launch changes. + /// + public void EnsureEgressAdmissible(SandboxSpec spec, bool modelBrokerReachableFromNamespace) + { + if (spec.ModelBrokerPort is null || BubblewrapSandbox.Available is null) return; + + if (SealRefusal(FilteredEgressNetns.IsSupported, FilteredEgressNetns.CanSeal, modelBrokerReachableFromNamespace) is not { } cause) return; + + // The probe keeps the step that failed and its output; an operator needs that more than the category. + throw new SealedEgressUnavailableException(cause == SealedEgressUnavailableException.CauseNoPrivilege && FilteredEgressNetns.SealUnavailableReason is { } probe ? $"{cause}; the probe: {probe}" : cause); + } + + /// Why a confining host cannot seal a brokered network-off run, or null when it can. Pure over the host's three facts, so every cause is testable on a host that has none of them. + internal static string? SealRefusal(bool haveTools, bool canSeal, bool brokerReachableFromNamespace) => + !haveTools ? SealedEgressUnavailableException.CauseMissingTools + : !canSeal ? SealedEgressUnavailableException.CauseNoPrivilege + : !brokerReachableFromNamespace ? SealedEgressUnavailableException.CauseBrokerLoopbackOnly + : null; + /// /// Create this run's cgroup-v2 resource-cap leaf (B4) when a memory/cpu cap is requested AND the operator delegated /// a root () on a cgroup-v2 host — returning the self-add prefix the diff --git a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.cs b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.cs index a85f962af..2810a4e1b 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Sandbox/Runners/LocalProcessRunner.cs @@ -23,7 +23,7 @@ namespace CodeSpace.Core.Services.Agents.Sandbox.Runners; /// Caller cancellation is honoured distinctly from the spec timeout: it terminates the process and rethrows /// (the durable path differs — see its remarks: cancellation stops observing without killing). /// -public sealed partial class LocalProcessRunner : ISandboxRunner, ISandboxStreamRunner, ISandboxDurableRunner, ISandboxLaunchIdentityRunner, ISandboxDurableLogSource, ISandboxDurableDiagnosticSource, ISingletonDependency +public sealed partial class LocalProcessRunner : ISandboxRunner, ISandboxStreamRunner, ISandboxDurableRunner, ISandboxLaunchIdentityRunner, ISandboxDurableLogSource, ISandboxDurableDiagnosticSource, ISandboxEgressAdmission, ISingletonDependency { /// This runner's registry key. The runner-local spelling of the shared — same constant, so there is one literal. public const string LocalKind = SandboxKinds.Local; diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs index b24266e5b..de0cedde6 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs @@ -1774,6 +1774,8 @@ private static void AppendFailedVerdict(StringBuilder builder, SupervisorAgentRe "RETRY this exact subtask so it runs on a live worker; do NOT re-plan it and do NOT amend its check — there is nothing wrong with either.", Messages.Failures.FailureCodes.ModelCredentialBrokerUnavailable => "RETRY this exact subtask once, in case another worker can broker its model credential; if it ends the same way again, 'ask_human' — that is a deployment setting only an operator can change. Either way do NOT re-plan it and do NOT amend its check — there is nothing wrong with either.", + Messages.Failures.FailureCodes.SandboxSealedEgressUnavailable => + "RETRY this exact subtask once, in case another worker can seal its network to its model broker; if it ends the same way again, 'ask_human' — that is a deployment setting only an operator can change. Either way do NOT re-plan it and do NOT amend its check — there is nothing wrong with either.", _ => "This is an infrastructure fault with no recorded remedy: 'ask_human' to rule. Do NOT re-plan it and do NOT amend its check — neither is where the fault is.", }; diff --git a/backend/src/CodeSpace.Messages/Agents/BrokeredModelCredential.cs b/backend/src/CodeSpace.Messages/Agents/BrokeredModelCredential.cs index 12d904474..b6ff3f66f 100644 --- a/backend/src/CodeSpace.Messages/Agents/BrokeredModelCredential.cs +++ b/backend/src/CodeSpace.Messages/Agents/BrokeredModelCredential.cs @@ -33,4 +33,12 @@ public sealed record BrokeredModelCredential(string BaseUrl, string RunToken, Da /// The unguessable route segment of , for the same reason as : a re-bind has to install the run's OWN route, never mint a fresh one, or the address the agent holds resolves to nothing. Null exactly when is. public string? RebindRoute { get; init; } + + /// + /// Whether the lease listens on every address, so a child inside a per-run network namespace — which reaches the + /// worker at its namespace gateway, never on loopback — can reach it. False (the default) is the fail-closed + /// answer: a broker that could only bind loopback, or one that does not say, cannot serve a sealed network-off run, + /// and such a run is refused before launch rather than left calling an address nothing answers. + /// + public bool ReachableFromNamespace { get; init; } } diff --git a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs index 04d8bbe7a..492b5e929 100644 --- a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs +++ b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs @@ -117,6 +117,9 @@ public static class FailureCodes /// This host cannot reserve a filtered-egress run's own /30 subnet, so the run is refused rather than handed one nothing reserved. Remedy: make the reservation directory under the agent-run spool root writable by the worker — a retry on the same host cannot help. public const string SandboxEgressReservationUnavailable = "sandbox_egress_reservation_unavailable"; + /// A network-off run whose model is brokered was refused before it spent anything, because this host would confine it but cannot seal its network to that broker — no ip/nft, no privilege to build a namespace, or a broker that could only listen on loopback — and severing it instead would leave its agent unable to reach any model. Remedy: grant the worker what a sealed namespace needs (root with CAP_NET_ADMIN and CAP_SYS_ADMIN, ip and nft; see backend/Dockerfile.worker) — a retry on the same host helps only once it can build one, which it re-checks at most once a minute. + public const string SandboxSealedEgressUnavailable = "sandbox_sealed_egress_unavailable"; + /// A run's model credential could not be brokered on a deployment that requires confinement, so the run is refused rather than handed the tenant's long-lived provider key. Remedy: make the worker able to bind a broker listener, use a harness that honours a base-URL override, or store an upstream endpoint on the credential — a retry on the same host cannot help. public const string ModelCredentialBrokerUnavailable = "model_credential_broker_unavailable"; @@ -125,7 +128,8 @@ public static class FailureCodes /// /// The codes that, worn as an agent attempt's AgentRunResult.ExitReason, say the attempt died on OUR - /// INFRASTRUCTURE — the worker went away, the broker could not bind — rather than on anything the agent did or + /// INFRASTRUCTURE — the worker went away, the broker could not bind, the host could not seal a network-off run to + /// its broker — rather than on anything the agent did or /// the model answered. A post-hoc grader that meets one of these is grading an attempt whose check never ran and /// never could have, so no further agent pass can change its verdict. /// @@ -134,12 +138,13 @@ public static class FailureCodes /// answers about the work. Membership here is the narrower claim that the run never got to be about the work at /// all. Pinned member-by-member by a unit test — adding or removing one changes what a post-hoc grade can be. /// - /// Membership settles the CLASSIFICATION, never the REMEDY, and the two members already differ on + /// Membership settles the CLASSIFICATION, never the REMEDY, and the members already differ on /// the second. is a worker that went away mid-run: the identical attempt /// on a live worker simply succeeds, so its repair is a retry and nothing else. /// is a worker that could not broker at all on a deployment mandating confinement — a retry helps only if it /// lands somewhere that CAN broker, and if the deployment itself is misconfigured no number of attempts will - /// (its own remedy line above says as much). Both are equally "not about the work", which is all this set + /// (its own remedy line above says as much); is the same kind of + /// deployment answer about the network. All are equally "not about the work", which is all this set /// claims; what to DO about each is the prompt renderer's question, and /// LlmSupervisorDecider.EndedByDeploymentSteer answers it per exit reason rather than per class. /// @@ -152,6 +157,7 @@ public static class FailureCodes { ModelCredentialLeaseLost, ModelCredentialBrokerUnavailable, + SandboxSealedEgressUnavailable, }; /// diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs index 3a60b1c21..abb5c5b86 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentRunExecutorCredentialBrokerTests.cs @@ -168,6 +168,57 @@ public async Task A_sealed_launch_s_record_survives_the_column_and_reads_back_as .ShouldBe("Network: off (Standard) — confined: egress sealed to the run's model broker"); } + [Fact] + public async Task A_network_off_brokered_run_its_runner_cannot_seal_is_refused_before_it_spends_or_launches() + { + if (OperatingSystem.IsWindows()) return; + + // The ORDER is the claim: the runner is asked while a refusal still costs nothing. A run owned by a workflow run + // records a spend row the moment it is admitted — even with no cap — so an absent row proves the refusal came + // first; a null launch proves no process started; and the lease must be withdrawn like any finished run's. + var teamId = await SeedTeamAsync(); + var credId = await SeedModelCredentialAsync(teamId, BrokeredProvider, "sk-unsealable-fixture"); + var workflowRunId = await SeedCappedWorkflowRunAsync(teamId, capUsd: null); + var runId = await CreateTaskRunInWorkflowAsync(teamId, workflowRunId, new AgentTask { Goal = "scripted", Harness = "scripted-projector", Model = "claude-opus-4-8", ModelCredentialId = credId, MaxCostUsd = 5m }); + var runner = new SealRefusingRunner(); + + using var broker = new LoopbackModelCredentialBroker(); + var harness = new BrokerableScriptedHarness(BrokeredProvider, "echo done"); + + await ExecuteAsync(runId, harness, runners: new SandboxRunnerRegistry(new ISandboxRunner[] { runner }), credentialBroker: broker); + + using var scope = _fixture.BeginScope(); + var run = await scope.Resolve().GetAsync(runId, CancellationToken.None); + var result = JsonSerializer.Deserialize(run.ResultJson!, AgentJson.Options)!; + var leasePort = new Uri(harness.BuiltTask!.Environment["SCRIPTED_BASE_URL"].Replace(SandboxSpec.ModelBrokerHostToken, "127.0.0.1", StringComparison.Ordinal)).Port; + + run.Status.ShouldBe(AgentRunStatus.Failed); + result.ExitReason.ShouldBe(CodeSpace.Messages.Failures.FailureCodes.SandboxSealedEgressUnavailable, "the refusal lands under its own code, which the supervisor steers on"); + runner.Asked.ShouldHaveSingleItem().Port.ShouldBe(leasePort, "the runner is asked about the spec the run would have launched, lease port and all"); + runner.Asked[0].Reachable.ShouldBe(CodeSpace.Core.Services.Agents.Sandbox.Isolation.FilteredEgressNetns.IsSupported, "and is told whether the lease bound where a namespace can reach it"); + runner.Launched.ShouldBeNull("a refused run starts no process"); + (await scope.Resolve().BudgetReservation.AsNoTracking().Where(r => r.TeamId == teamId).ToListAsync()) + .ShouldBeEmpty("a refusal before admission claims nothing — an admitted run here would have recorded an unbudgeted row"); + broker.HasLease(runId).ShouldBeFalse("the lease is withdrawn when the refused run ends, like any other"); + } + + [Fact] + public async Task An_unbrokered_network_off_run_is_admitted_by_a_runner_that_cannot_seal() + { + if (OperatingSystem.IsWindows()) return; + + // Nothing to seal, nothing to refuse: a run with no broker lease — model-less, or keyless — launches exactly as + // it did before, even where a brokered one would be refused. + var teamId = await SeedTeamAsync(); + var runId = await CreateScriptedRunAsync(teamId); + var runner = new SealRefusingRunner(); + + await ExecuteAsync(runId, new ScriptedHarness("printf 'one\\n'"), runners: new SandboxRunnerRegistry(new ISandboxRunner[] { runner })); + + runner.Asked.ShouldHaveSingleItem().Port.ShouldBeNull(); + runner.Launched.ShouldNotBeNull("an unbrokered network-off run is launched, not refused"); + } + [Fact] public async Task A_run_whose_credential_cannot_be_brokered_discloses_the_direct_injection() { @@ -1369,6 +1420,51 @@ private sealed class AddresslessBroker(LoopbackModelCredentialBroker inner) : IM /// Its variable names are deliberately its own — the assertion is about which KIND of value lands, not about /// Anthropic's or OpenAI's spellings, which their own harness pin tests own. /// + private async Task CreateTaskRunInWorkflowAsync(Guid teamId, Guid workflowRunId, AgentTask task) + { + using var scope = await WorkflowsTestSeed.BeginSeedOperatorScopeAsync(_fixture, teamId); + var run = await scope.Resolve().CreateAsync(task, teamId, workflowRunId, null, iterationKey: "", cancellationToken: CancellationToken.None); + return run.Id; + } + + /// A durable runner that refuses admission to any spec carrying a broker port — the shape the local runner takes on a host that confines but cannot seal — recording what it was asked and what it launched. + private sealed class SealRefusingRunner : ISandboxRunner, ISandboxDurableRunner, ISandboxEgressAdmission + { + public string Kind => LocalProcessRunner.LocalKind; + + public List<(int? Port, bool Reachable)> Asked { get; } = new(); + + public SandboxSpec? Launched { get; private set; } + + public void EnsureEgressAdmissible(SandboxSpec spec, bool modelBrokerReachableFromNamespace) + { + Asked.Add((spec.ModelBrokerPort, modelBrokerReachableFromNamespace)); + + if (spec.ModelBrokerPort is not null) throw new CodeSpace.Core.Services.Agents.Sandbox.Exceptions.SealedEgressUnavailableException(CodeSpace.Core.Services.Agents.Sandbox.Exceptions.SealedEgressUnavailableException.CauseNoPrivilege); + } + + public Task RunAsync(SandboxSpec spec, CancellationToken cancellationToken) => + throw new NotSupportedException("The executor must take the durable path."); + + public Task LaunchAsync(SandboxSpec spec, string spoolKey, CancellationToken cancellationToken) + { + Launched = spec; + + var spoolDirectory = LocalProcessRunner.SpoolDirectoryFor(spoolKey); + Directory.CreateDirectory(spoolDirectory); + + return Task.FromResult(new SandboxHandle { Kind = Kind, ProcessId = System.Environment.ProcessId, SpoolDirectory = spoolDirectory, Deadline = DateTimeOffset.UtcNow.AddMinutes(5) }); + } + + public Task AttachAsync(SandboxHandle handle, Func onStdoutFrame, CancellationToken cancellationToken, Func? onCheckpoint = null) => + Task.FromResult(new SandboxResult { Status = SandboxStatus.Success, ExitCode = 0, Stdout = "", Stderr = "" }); + + public Task ProbeAsync(SandboxHandle handle, CancellationToken cancellationToken) => + Task.FromResult(new SandboxProbe { State = SandboxRunState.Exited, ExitCode = 0 }); + + public Task TerminateAsync(SandboxHandle handle, CancellationToken cancellationToken) => Task.FromResult(SandboxTerminateResult.Killed); + } + private sealed class BrokerableScriptedHarness(string provider, string script) : IAgentHarness, IModelCredentialProjector, IBrokeredModelCredentialProjector { public const string KeyEnvVar = "SCRIPTED_MODEL_KEY"; diff --git a/backend/tests/CodeSpace.SandboxTests/SealedEgressE2ETests.cs b/backend/tests/CodeSpace.SandboxTests/SealedEgressE2ETests.cs index bfcc2c417..4d101a31b 100644 --- a/backend/tests/CodeSpace.SandboxTests/SealedEgressE2ETests.cs +++ b/backend/tests/CodeSpace.SandboxTests/SealedEgressE2ETests.cs @@ -6,6 +6,7 @@ using System.Text.Json; using CodeSpace.Core.Services.Agents.Credentials.Broker; using CodeSpace.Core.Services.Agents.Sandbox; +using CodeSpace.Core.Services.Agents.Sandbox.Exceptions; using CodeSpace.Core.Services.Agents.Sandbox.Isolation; using CodeSpace.Core.Services.Agents.Sandbox.Runners; using CodeSpace.Messages.Agents; @@ -198,6 +199,44 @@ public async Task A_host_whose_policy_rule_discards_the_run_s_replies_fails_the_ } } + [Fact] + public async Task A_sealed_setup_that_fails_on_this_host_refuses_the_launch_typed_and_leaks_nothing() + { + if (!Seals()) return; + + // A host that proved it can seal can still fail one run's setup — a name collision, a kernel refusal. The launch + // must then refuse under the same typed wall the executor's pre-spend admission raises, naming the failed step, + // and tear down whatever the partial setup built. Occupying the host veth's name makes the plan's own + // `ip link add` fail exactly as a collision would. + var key = Guid.NewGuid().ToString("N"); + var names = FilteredEgressPlan.BuildSealed(key, 9, new EgressSubnetAllocator.Lease { Cidr = "0.0.0.0/30", HostIp = "0.0.0.1", NsIp = "0.0.0.2" }); + + // A veth, not a dummy: the veth driver is what the plan itself needs, so it is loaded wherever sealing works at all. + (await RunHostExitAsync(["ip", "link", "add", names.VethHost, "type", "veth", "peer", "name", "csp-" + names.VethHost[4..]])).ShouldBe(0, $"fixture: could not occupy {names.VethHost}"); + + try + { + using var broker = LoopbackModelCredentialBroker.ForTest(new AlwaysOkUpstream()); + var brokered = (await broker.OpenAsync(Lease(), CancellationToken.None)).ShouldNotBeNull(); + var spec = new SandboxSpec { Command = "/bin/true", AllowNetwork = false, ModelBrokerPort = brokered.RebindPort, TimeoutSeconds = 30 }; + _spoolDirs.Add(LocalProcessRunner.SpoolDirectoryFor(key)); + + var thrown = await Should.ThrowAsync(() => new LocalProcessRunner().LaunchAsync(spec, key, CancellationToken.None)); + var refusal = (thrown as SealedEgressUnavailableException ?? thrown.InnerException as SealedEgressUnavailableException).ShouldNotBeNull($"the launch must refuse typed, not as {thrown.GetType().Name}: {thrown.Message}"); + + ((CodeSpace.Messages.Failures.IFailure)refusal).Code.ShouldBe(CodeSpace.Messages.Failures.FailureCodes.SandboxSealedEgressUnavailable); + refusal.Cause.ShouldContain("ip link add", customMessage: $"the refusal must name the setup step that failed: {refusal.Cause}"); + (await NetnsExistsAsync(names.Namespace)).ShouldBeFalse("a failed setup must tear down the namespace it had already created"); + + output.WriteLine($"{RanMarker} setup-failure cause={refusal.Cause}"); + } + finally + { + await RunHostExitAsync(["ip", "link", "del", names.VethHost]); // best-effort: the failed setup's teardown may already have removed it (and its peer with it) + await FilteredEgressNetns.TeardownAsync(key, CancellationToken.None); + } + } + public void Dispose() { foreach (var dir in _spoolDirs) diff --git a/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs index b59a4316d..f688cf42c 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialBrokerTests.cs @@ -108,6 +108,20 @@ public async Task A_live_lease_relays_the_call_with_the_tenants_key_attached_ser upstream.SeenHeaderValues.ShouldNotContain(brokered.RunToken, "the per-run bearer authenticates to the broker only; forwarding it leaks a capability the provider has no use for"); } + [Fact] + public async Task A_lease_says_whether_a_per_run_namespace_can_reach_it() + { + // A sealed network-off run reaches the worker at its namespace gateway, never on loopback, so the lease has to + // say whether it took the wide bind. The wide bind is tried only where namespaces can exist; everywhere else + // it binds loopback and must say it is NOT reachable, which is what refuses a sealed launch before it spends. + using var broker = LoopbackModelCredentialBroker.ForTest(new StubUpstream()); + + var brokered = await broker.OpenAsync(LeaseFor(Guid.NewGuid()), CancellationToken.None); + if (brokered is null) return; // this host cannot bind a listener at all — nothing to assert + + brokered.ReachableFromNamespace.ShouldBe(FilteredEgressNetns.IsSupported, "reachable from a namespace exactly when the broker bound every address, which it tries only where a namespace can exist"); + } + [Fact] public async Task A_revoked_lease_is_refused() { diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs index 9656307fa..aa4c404e8 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs @@ -494,6 +494,7 @@ public void The_user_prompt_renders_an_infra_classed_rejection_as_unverified_nev [Theory] [InlineData(FailureCodes.ModelCredentialLeaseLost)] [InlineData(FailureCodes.ModelCredentialBrokerUnavailable)] + [InlineData(FailureCodes.SandboxSealedEgressUnavailable)] public void An_attempt_this_deployment_ended_is_steered_at_a_retry_never_at_re_planning_its_check(string exitReason) { // The contradiction this arm removes: the shared infra steer says "Do NOT retry the agent … Re-plan this @@ -539,6 +540,10 @@ public void The_two_deployment_exit_reasons_get_the_remedy_each_one_actually_has LlmSupervisorDecider.EndedByDeploymentSteer("some_future_infra_exit") .ShouldBe("This is an infrastructure fault with no recorded remedy: 'ask_human' to rule. Do NOT re-plan it and do NOT amend its check — neither is where the fault is."); + // A host that cannot seal is the same kind of deployment answer as one that cannot broker, about the network. + LlmSupervisorDecider.EndedByDeploymentSteer(FailureCodes.SandboxSealedEgressUnavailable) + .ShouldBe("RETRY this exact subtask once, in case another worker can seal its network to its model broker; if it ends the same way again, 'ask_human' — that is a deployment setting only an operator can change. Either way do NOT re-plan it and do NOT amend its check — there is nothing wrong with either."); + leaseLost.ShouldNotBe(brokerDown, "one remedy text for two different faults is how a bounded repair becomes an unbounded loop"); leaseLost.ShouldNotContain("ask_human", Case.Sensitive, "a live worker is the whole repair — escalating a rolling restart to a human is noise"); brokerDown.ShouldContain("ask_human", Case.Sensitive, "only an operator can change a confinement setting"); diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs index 7c2d97a64..95d85e745 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs @@ -106,6 +106,7 @@ public void The_wire_codes_are_pinned() FailureCodes.UnscopedModelCall.ShouldBe("unscoped_model_call"); FailureCodes.ModelCredentialBrokerUnavailable.ShouldBe("model_credential_broker_unavailable"); FailureCodes.ModelCredentialLeaseLost.ShouldBe("model_credential_lease_lost"); + FailureCodes.SandboxSealedEgressUnavailable.ShouldBe("sandbox_sealed_egress_unavailable"); } [Fact] @@ -115,7 +116,7 @@ public void The_infra_exit_reasons_are_pinned_member_by_member() // deployment ended it". Too wide and a genuine failure stops buying the retries that could fix it; too // narrow and a worker restart buys a stronger model. Either way the drift is silent, so the set is spelled // out here as literals rather than compared against the constants it is built from. - FailureCodes.InfraExitReasons.ShouldBe(new[] { "model_credential_lease_lost", "model_credential_broker_unavailable" }, ignoreOrder: true, + FailureCodes.InfraExitReasons.ShouldBe(new[] { "model_credential_lease_lost", "model_credential_broker_unavailable", "sandbox_sealed_egress_unavailable" }, ignoreOrder: true, customMessage: "adding an exit reason here changes what a post-hoc grade can be — state the new member's producer in the PR, add it to this list deliberately, and give it its own remedy arm in LlmSupervisorDecider.EndedByDeploymentSteer (membership settles the classification, never the remedy)"); } diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/SealedEgressAdmissionTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/SealedEgressAdmissionTests.cs new file mode 100644 index 000000000..e5739ca1c --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/SealedEgressAdmissionTests.cs @@ -0,0 +1,71 @@ +using CodeSpace.Core.Services.Agents.Sandbox.Exceptions; +using CodeSpace.Core.Services.Agents.Sandbox.Isolation; +using CodeSpace.Core.Services.Agents.Sandbox.Runners; +using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Failures; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// Pins the local runner's refusal of a network-off brokered run it would confine but cannot seal — the admission the +/// executor asks for before anything is spent. The executor asking at all, and landing the refusal typed with no spend +/// and no process, is pinned one tier up in AgentRunExecutorTests; the kernel refusing a real setup is pinned in +/// the sandbox lane. +/// +[Trait("Category", "Unit")] +public class SealedEgressAdmissionTests +{ + [Theory] + [InlineData(false, false, false, SealedEgressUnavailableException.CauseMissingTools)] + [InlineData(true, false, true, SealedEgressUnavailableException.CauseNoPrivilege)] + [InlineData(true, true, false, SealedEgressUnavailableException.CauseBrokerLoopbackOnly)] + [InlineData(true, true, true, null)] + public void Each_wall_a_confining_host_can_hit_is_named(bool haveTools, bool canSeal, bool brokerReachable, string? expected) => + LocalProcessRunner.SealRefusal(haveTools, canSeal, brokerReachable).ShouldBe(expected); + + [Fact] + public void A_spec_with_no_broker_port_is_admitted_on_any_host() => + // Nothing to seal: every unbrokered or network-on run launches exactly as it always did, even on a host that could not seal. + Should.NotThrow(() => new LocalProcessRunner().EnsureEgressAdmissible(new SandboxSpec { Command = "agent" }, modelBrokerReachableFromNamespace: false)); + + [Fact] + public void A_brokered_network_off_spec_is_refused_exactly_where_this_host_confines_but_cannot_seal() + { + // Honest on either host: unconfined hosts (macOS dev, a pod without userns) admit it untouched, because nothing + // there would have severed it; a confining host refuses it unless it can seal and the broker is reachable. + var spec = new SandboxSpec { Command = "agent", ModelBrokerPort = 43121 }; + var refusal = BubblewrapSandbox.Available is null ? null : LocalProcessRunner.SealRefusal(FilteredEgressNetns.IsSupported, FilteredEgressNetns.CanSeal, brokerReachableFromNamespace: false); + + var thrown = Record.Exception(() => new LocalProcessRunner().EnsureEgressAdmissible(spec, modelBrokerReachableFromNamespace: false)); + + if (refusal is null) thrown.ShouldBeNull(); + else thrown.ShouldBeOfType().Cause.ShouldStartWith(refusal, customMessage: "the cause leads with the wall, then the probe's own account of it"); + } + + [Fact] + public void The_refusal_is_an_unavailable_failure_that_names_its_cause_and_its_remedy() + { + IFailure failure = new SealedEgressUnavailableException(SealedEgressUnavailableException.CauseNoPrivilege); + + failure.Kind.ShouldBe(FailureKind.Unavailable, "nothing about the launch can change to make it work — only the host can"); + failure.Code.ShouldBe(FailureCodes.SandboxSealedEgressUnavailable); + failure.ClientMessage.ShouldBe("This host cannot give a network-off run a sealed route to its model."); + ((Exception)failure).Message.ShouldContain(SealedEgressUnavailableException.CauseNoPrivilege, customMessage: "the operator must be told which wall it was"); + ((Exception)failure).Message.ShouldContain("a retry on this host helps only once it can build one", customMessage: "a probe failure can be transient, so the remedy must not promise a retry is hopeless"); + } + + [Fact] + public void A_setup_step_that_failed_on_a_host_that_can_seal_is_told_to_fix_that_step() + { + // The probe proved this worker can build a namespace, so granting it more and waiting for a re-probe fixes + // nothing: the failed step (a route that discards the run's /30, say) fails every launch until it is fixed. + var refusal = SealedEgressUnavailableException.SetupFailed("ip route get 10.1.1.2 from 10.1.1.1 → exit 2: RTNETLINK answers: No route to host"); + + ((IFailure)refusal).Code.ShouldBe(FailureCodes.SandboxSealedEgressUnavailable, "the same wall as every other refusal, so the supervisor steers it the same way"); + refusal.Cause.ShouldBe("the sealed namespace's setup failed: ip route get 10.1.1.2 from 10.1.1.1 → exit 2: RTNETLINK answers: No route to host"); + refusal.Message.ShouldContain("fix what that step names", customMessage: "the remedy points at the failed step"); + refusal.Message.ShouldNotContain("Dockerfile.worker", customMessage: "the worker already has what a sealed namespace needs"); + refusal.Message.ShouldNotContain("once a minute", customMessage: "the setup runs afresh on every launch; no probe is waited for"); + } +}