From 1805edbb79279e2d33c581486e3aa8394413e05c Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 03:22:16 +0000 Subject: [PATCH 01/13] Broker installed HTTP MCP credentials in the Session gateway The gateway's MCP config is now the effective agent.MCPBinding list, so installed HTTP bindings reach it with their bearer token and HTTP headers. The MCP relay injects the bearer and each header in place of the Harness's credential headers and same-named headers, and withholds every injected value from response headers and trailers. Non-HTTP bindings, invalid headers and a header that would replace the bearer are rejected. --- apps/daemon/internal/gateway/gateway.go | 56 ++++++++++-------- apps/daemon/internal/gateway/gateway_test.go | 5 +- apps/daemon/internal/gateway/mcp.go | 58 ++++++++++++++----- apps/daemon/internal/gateway/mcp_test.go | 30 ++++++++-- apps/daemon/internal/gateway/relay.go | 30 +++++----- .../internal/gateway/view_linux_test.go | 3 +- contracts/agents-api/model-execution.md | 2 +- 7 files changed, 120 insertions(+), 64 deletions(-) diff --git a/apps/daemon/internal/gateway/gateway.go b/apps/daemon/internal/gateway/gateway.go index 1f906fc8..6e713424 100644 --- a/apps/daemon/internal/gateway/gateway.go +++ b/apps/daemon/internal/gateway/gateway.go @@ -8,15 +8,15 @@ // by hostname. A model listener relays the declared native routes of its // protocol (internal/modelprovider) to the upstream from the agent host and // injects the credential. An MCP listener relays to its binding's server and -// injects the bearer token: an environment-origin binding connects through the -// sandbox's Network service, a service-origin binding from the agent host, and -// the gateway does the TLS either way. The generic proxy carries HTTP CONNECT -// tunnels and plain-HTTP forward requests, and connects only through the -// sandbox's Network service. Redirects reach the Harness unchanged and are -// never followed. Response headers and trailers that carry an injected -// credential are withheld; bodies pass unchanged. The end of the Session -// closes every connection, tunnels and upgraded ones included. The gateway -// logs nothing. +// injects the binding's bearer token and HTTP headers: an environment-origin +// binding connects through the sandbox's Network service, a service-origin +// binding from the agent host, and the gateway does the TLS either way. The +// generic proxy carries HTTP CONNECT tunnels and plain-HTTP forward requests, +// and connects only through the sandbox's Network service. Redirects reach the +// Harness unchanged and are never followed. Response header and trailer values +// that contain an injected credential or header value are withheld; bodies +// pass unchanged. The end of the Session closes every connection, tunnels and +// upgraded ones included. The gateway logs nothing. package gateway import ( @@ -32,6 +32,7 @@ import ( "strconv" "time" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" @@ -43,11 +44,13 @@ type Config struct { // Models are the frozen model upstreams, each under the adapter's name for // it. Names are unique. Models []Model - // MCP are the Session's MCP HTTP bindings. Server labels are unique. - MCP []proto.MCPHTTPServer - // Prompt is the request that declared MCP. Each binding's origin is - // admitted against its placement with ValidateConnectionOrigin before - // anything else. + // MCP are the Session's effective MCP bindings as + // agent.ResolveMCPBindings returns them, bearer tokens and HTTP headers + // included. Each is an HTTP binding, and server labels are unique. + MCP []agent.MCPBinding + // Prompt is the request the bindings were resolved from. Each binding's + // origin is admitted against its placement with + // proto.MCPHTTPServer.ValidateConnectionOrigin before anything else. Prompt proto.PromptRequestPayload // OpenNetwork opens a new Network stream to the Session's sandbox, as // sandboxnet.Connect takes it. Nil means the Session has no sandbox @@ -206,26 +209,29 @@ func build(session context.Context, cfg Config) (*gateway, error) { } labels := map[string]bool{} - for _, s := range cfg.MCP { - if err := s.ValidateConnectionOrigin(cfg.Prompt); err != nil { - return nil, invalid("MCP server %q: %v", s.ServerLabel, err) + for _, b := range cfg.MCP { + if err := (proto.MCPHTTPServer{ConnectionOrigin: b.ConnectionOrigin}).ValidateConnectionOrigin(cfg.Prompt); err != nil { + return nil, invalid("MCP server %q: %v", b.ServerLabel, err) } - if s.ServerLabel == "" || labels[s.ServerLabel] { - return nil, invalid("MCP server label %q is empty or repeated", s.ServerLabel) + if b.ServerLabel == "" || labels[b.ServerLabel] { + return nil, invalid("MCP server label %q is empty or repeated", b.ServerLabel) + } + labels[b.ServerLabel] = true + if b.Transport != "http" { + return nil, invalid("MCP server %q has transport %q, not http", b.ServerLabel, b.Transport) } - labels[s.ServerLabel] = true transport := host - if s.ConnectionOrigin == "environment" { + if b.ConnectionOrigin == "environment" { if sandbox == nil { - return nil, invalid("MCP server %q has environment origin and the Session has no sandbox network", s.ServerLabel) + return nil, invalid("MCP server %q has environment origin and the Session has no sandbox network", b.ServerLabel) } transport = sandbox } - h, suffix, err := newMCPRelay(s, transport) + h, suffix, err := newMCPRelay(b, transport) if err != nil { - return nil, invalid("MCP server %q: %v", s.ServerLabel, err) + return nil, invalid("MCP server %q: %v", b.ServerLabel, err) } - g.listeners = append(g.listeners, listener{role: roleMCP, name: s.ServerLabel, suffix: suffix, handler: h}) + g.listeners = append(g.listeners, listener{role: roleMCP, name: b.ServerLabel, suffix: suffix, handler: h}) } return g, nil } diff --git a/apps/daemon/internal/gateway/gateway_test.go b/apps/daemon/internal/gateway/gateway_test.go index 266b978b..6928b33e 100644 --- a/apps/daemon/internal/gateway/gateway_test.go +++ b/apps/daemon/internal/gateway/gateway_test.go @@ -16,6 +16,7 @@ import ( "testing" "time" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink/relay" @@ -183,7 +184,7 @@ func TestSessionEndEndsBlockedRelays(t *testing.T) { })) defer srv.Close() gw := serveOnLoopback(t, Config{ - MCP: []proto.MCPHTTPServer{{ConnectionOrigin: "service", ServerLabel: "tools", ServerURL: srv.URL + "/mcp"}}, + MCP: []agent.MCPBinding{{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: srv.URL + "/mcp"}}, Prompt: proto.PromptRequestPayload{DisableExecutionEnvironment: true}, }) @@ -235,7 +236,7 @@ func TestRejectedUpgradeClosesTheUpstream(t *testing.T) { })) defer srv.Close() gw := serveOnLoopback(t, Config{ - MCP: []proto.MCPHTTPServer{{ConnectionOrigin: "service", ServerLabel: "tools", ServerURL: srv.URL + "/mcp"}}, + MCP: []agent.MCPBinding{{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: srv.URL + "/mcp"}}, Prompt: proto.PromptRequestPayload{DisableExecutionEnvironment: true}, }) diff --git a/apps/daemon/internal/gateway/mcp.go b/apps/daemon/internal/gateway/mcp.go index 16064e10..8c61ee38 100644 --- a/apps/daemon/internal/gateway/mcp.go +++ b/apps/daemon/internal/gateway/mcp.go @@ -2,40 +2,60 @@ package gateway import ( "errors" + "fmt" "net/http" "net/http/httputil" "net/url" + "slices" - "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "golang.org/x/net/http/httpguts" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" ) // mcpRelay serves one MCP HTTP binding: it relays each request to the // server's origin with the same path and query. When the binding has a bearer -// token, it injects the token and withholds it from response headers and -// trailers. +// token or HTTP headers, it replaces the Harness's credential headers and +// same-named headers with them, and withholds each injected value from +// response headers and trailers. type mcpRelay struct { scheme string host string - token *string + inject http.Header // canonical names, values as sent; empty when the binding has none transport http.RoundTripper } // newMCPRelay returns the binding's handler and the path and query the -// Harness appends to the listener's address. -func newMCPRelay(s proto.MCPHTTPServer, t http.RoundTripper) (*mcpRelay, string, error) { - u, err := url.Parse(s.ServerURL) +// Harness appends to the listener's address. Errors name a header but never +// carry a value. +func newMCPRelay(b agent.MCPBinding, t http.RoundTripper) (*mcpRelay, string, error) { + u, err := url.Parse(b.ServerURL) if err != nil || (u.Scheme != "https" && u.Scheme != "http") || u.Hostname() == "" || u.User != nil || u.Opaque != "" || u.Fragment != "" { return nil, "", errors.New("server URL is not an absolute http or https URL") } - m := &mcpRelay{scheme: u.Scheme, host: u.Host, transport: t} - if s.BearerToken != nil { + m := &mcpRelay{scheme: u.Scheme, host: u.Host, inject: http.Header{}} + var secrets []string + if b.BearerToken != nil { if u.Scheme != "https" { return nil, "", errors.New("a bearer token needs an https server URL") } - token := sentValue(*s.BearerToken) - m.token = &token - m.transport = withhold(t, token) + token := sentValue(*b.BearerToken) + m.inject.Set("Authorization", "Bearer "+token) + secrets = append(secrets, token) + } + for name, value := range b.HTTPHeaders { + key := http.CanonicalHeaderKey(name) + if !httpguts.ValidHeaderFieldName(name) || !httpguts.ValidHeaderFieldValue(value) { + return nil, "", fmt.Errorf("HTTP header %q is not a valid header", name) + } + if m.inject[key] != nil { + return nil, "", fmt.Errorf("HTTP header %q is repeated or replaces the bearer token", name) + } + v := sentValue(value) + m.inject[key] = []string{v} + secrets = append(secrets, v) } + m.transport = withhold(t, secrets...) suffix := u.EscapedPath() if u.RawQuery != "" || u.ForceQuery { suffix += "?" + u.RawQuery @@ -54,9 +74,17 @@ func (m *mcpRelay) ServeHTTP(w http.ResponseWriter, r *http.Request) { reverseProxy(m.transport, func(pr *httputil.ProxyRequest) { pr.Out.URL = upstream pr.Out.Host = "" - if m.token != nil { - stripCredentials(pr.Out.Header) - pr.Out.Header.Set("Authorization", "Bearer "+*m.token) + if len(m.inject) == 0 { + return + } + stripCredentials(pr.Out.Header) + for name := range pr.Out.Header { + if m.inject[http.CanonicalHeaderKey(name)] != nil { + delete(pr.Out.Header, name) + } + } + for name, values := range m.inject { + pr.Out.Header[name] = slices.Clone(values) } }).ServeHTTP(w, r) } diff --git a/apps/daemon/internal/gateway/mcp_test.go b/apps/daemon/internal/gateway/mcp_test.go index 196d213e..09eacc1e 100644 --- a/apps/daemon/internal/gateway/mcp_test.go +++ b/apps/daemon/internal/gateway/mcp_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" ) @@ -15,13 +16,16 @@ func TestMCPBrokersBothOrigins(t *testing.T) { sb := startSandbox(t) seen := make(chan string, 1) srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - seen <- r.Header.Get("Authorization") + " " + r.URL.RequestURI() + seen <- r.Header.Get("Authorization") + " " + strings.Join(r.Header.Values("X-Tenant"), ",") + " " + r.URL.RequestURI() + w.Header().Set("X-Echo", "tenant-secret") + w.Header().Set("X-Plain", "visible") io.WriteString(w, "tools") })) defer srv.Close() token := "vault-token" - binding := func(origin string) proto.MCPHTTPServer { - return proto.MCPHTTPServer{ConnectionOrigin: origin, ServerLabel: "tools", ServerURL: srv.URL + "/mcp", BearerToken: &token} + binding := func(origin string) agent.MCPBinding { + return agent.MCPBinding{ConnectionOrigin: origin, ServerLabel: "tools", Transport: "http", ServerURL: srv.URL + "/mcp", + BearerToken: &token, HTTPHeaders: map[string]string{"x-tenant": " tenant-secret "}} } service := proto.PromptRequestPayload{DisableExecutionEnvironment: true} environment := proto.PromptRequestPayload{LocalEnvironment: &proto.LocalEnvironment{NetworkAccess: "enabled"}} @@ -32,12 +36,13 @@ func TestMCPBrokersBothOrigins(t *testing.T) { dials int32 }{{"service", service, 0}, {"environment", environment, 1}} { before := sb.dials.Load() - eps := serveOnLoopback(t, Config{MCP: []proto.MCPHTTPServer{binding(c.origin)}, Prompt: c.prompt, OpenNetwork: sb.open, RootCAs: trust(srv)}) + eps := serveOnLoopback(t, Config{MCP: []agent.MCPBinding{binding(c.origin)}, Prompt: c.prompt, OpenNetwork: sb.open, RootCAs: trust(srv)}) if !strings.HasPrefix(eps.MCP["tools"], "http://127.0.0.1:") || !strings.HasSuffix(eps.MCP["tools"], "/mcp") { t.Fatalf("%s: Harness URL %q", c.origin, eps.MCP["tools"]) } req, _ := http.NewRequest("POST", eps.MCP["tools"], strings.NewReader(`{"jsonrpc":"2.0"}`)) req.Header.Set("Authorization", "Bearer harness-value") + req.Header.Set("X-TENANT", "harness-value") resp, err := noRedirects.Do(req) if err != nil { t.Fatal(err) @@ -47,9 +52,12 @@ func TestMCPBrokersBothOrigins(t *testing.T) { if resp.StatusCode != 200 || string(body) != "tools" { t.Fatalf("%s: %d %q", c.origin, resp.StatusCode, body) } - if got := <-seen; got != "Bearer vault-token /mcp" { + if got := <-seen; got != "Bearer vault-token tenant-secret /mcp" { t.Errorf("%s: server saw %q", c.origin, got) } + if resp.Header.Get("X-Echo") != "" || resp.Header.Get("X-Plain") != "visible" { + t.Errorf("%s: an injected header value reached the Harness, or a plain one did not", c.origin) + } if n := sb.dials.Load() - before; n != c.dials { t.Errorf("%s: the sandbox made %d connections, want %d", c.origin, n, c.dials) } @@ -57,7 +65,17 @@ func TestMCPBrokersBothOrigins(t *testing.T) { // Origin admission runs first: an environment binding needs an enabled // workspace network. - if _, err := Plan(Config{MCP: []proto.MCPHTTPServer{binding("environment")}, Prompt: service, OpenNetwork: sb.open}); !errors.Is(err, ErrInvalidConfig) { + if _, err := Plan(Config{MCP: []agent.MCPBinding{binding("environment")}, Prompt: service, OpenNetwork: sb.open}); !errors.Is(err, ErrInvalidConfig) { t.Errorf("Plan with a relocated binding: %v", err) } + // The gateway relays HTTP only, and the bearer token owns Authorization. + stdio := binding("service") + stdio.Transport, stdio.ServerURL, stdio.BearerToken, stdio.HTTPHeaders = "stdio", "", nil, nil + twice := binding("service") + twice.HTTPHeaders = map[string]string{"Authorization": "Basic other"} + for name, b := range map[string]agent.MCPBinding{"stdio": stdio, "Authorization twice": twice} { + if _, err := Plan(Config{MCP: []agent.MCPBinding{b}, Prompt: service}); !errors.Is(err, ErrInvalidConfig) { + t.Errorf("Plan with %s: %v", name, err) + } + } } diff --git a/apps/daemon/internal/gateway/relay.go b/apps/daemon/internal/gateway/relay.go index d0bc5e2e..c9f26547 100644 --- a/apps/daemon/internal/gateway/relay.go +++ b/apps/daemon/internal/gateway/relay.go @@ -202,23 +202,23 @@ func sentValue(v string) string { return textproto.TrimString(strings.NewReplacer("\n", " ", "\r", " ").Replace(v)) } -// withhold returns t, or, when secret is set, a transport that keeps secret -// out of the response headers the Harness receives. secret is the value as -// sent. -func withhold(t http.RoundTripper, secret string) http.RoundTripper { - if secret == "" { +// withhold returns t, or, when a secret is set, a transport that keeps each +// secret out of the response headers the Harness receives. Each secret is the +// value as sent; empty ones are ignored. +func withhold(t http.RoundTripper, secrets ...string) http.RoundTripper { + secrets = slices.DeleteFunc(slices.Clone(secrets), func(s string) bool { return s == "" }) + if len(secrets) == 0 { return t } - return withholding{next: t, secret: secret} + return withholding{next: t, secrets: secrets} } -// withholding removes every header and trailer value that contains secret +// withholding removes every header and trailer value that contains a secret // from each response, informational ones included, so an upstream that echoes -// the injected credential in a header does not disclose it. Bodies pass -// unchanged. +// an injected value in a header does not disclose it. Bodies pass unchanged. type withholding struct { - next http.RoundTripper - secret string + next http.RoundTripper + secrets []string } func (t withholding) RoundTrip(r *http.Request) (*http.Response, error) { @@ -243,11 +243,13 @@ func (t withholding) RoundTrip(r *http.Request) (*http.Response, error) { return resp, nil } -// remove deletes each value that contains the secret. A name whose values -// are all removed stays with none, so an announced trailer stays announced. +// remove deletes each value that contains a secret. A name whose values are +// all removed stays with none, so an announced trailer stays announced. func (t withholding) remove(h http.Header) { for name, values := range h { - h[name] = slices.DeleteFunc(values, func(v string) bool { return strings.Contains(v, t.secret) }) + h[name] = slices.DeleteFunc(values, func(v string) bool { + return slices.ContainsFunc(t.secrets, func(s string) bool { return strings.Contains(v, s) }) + }) } } diff --git a/apps/daemon/internal/gateway/view_linux_test.go b/apps/daemon/internal/gateway/view_linux_test.go index efa544cd..d11a2098 100644 --- a/apps/daemon/internal/gateway/view_linux_test.go +++ b/apps/daemon/internal/gateway/view_linux_test.go @@ -21,6 +21,7 @@ import ( "github.com/hanwen/go-fuse/v2/fuse" "golang.org/x/sys/unix" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/sessionview" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" @@ -68,7 +69,7 @@ func TestListenersExistOnlyInTheSession(t *testing.T) { cfg := Config{ Models: []Model{{Name: "main", Provider: modelprovider.Provider{Protocol: modelprovider.Anthropic, BaseURL: "https://127.0.0.1:1", APIKey: upstreamKey}}}, - MCP: []proto.MCPHTTPServer{{ConnectionOrigin: "service", ServerLabel: "tools", ServerURL: "http://127.0.0.1:" + port + "/mcp"}}, + MCP: []agent.MCPBinding{{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: "http://127.0.0.1:" + port + "/mcp"}}, Prompt: proto.PromptRequestPayload{DisableExecutionEnvironment: true}, OpenNetwork: startSandbox(t).open, } diff --git a/contracts/agents-api/model-execution.md b/contracts/agents-api/model-execution.md index 53a35c80..2a785059 100644 --- a/contracts/agents-api/model-execution.md +++ b/contracts/agents-api/model-execution.md @@ -96,7 +96,7 @@ The Harness reaches its frozen upstream through a Session-local credential gatew - The gateway relays only the declared native routes of the provider's protocol, with the request and response unchanged apart from the credential rules below. An undeclared path or method, or an undeclared WebSocket upgrade, is rejected and never reaches the upstream. - It removes every value of each stripped header, matching names case-insensitively, then injects the upstream credential in the protocol's declared header, with the key's surrounding whitespace removed. -- It removes every response header and trailer value that contains the key, in informational responses too. Response bodies pass unchanged, so an upstream that echoes the key in a body discloses it to the Harness. The gateway applies the same rule to the bearer token it injects for an HTTP MCP server. +- It removes every response header and trailer value that contains the key, in informational responses too. Response bodies pass unchanged, so an upstream that echoes the key in a body discloses it to the Harness. For an HTTP MCP server, the gateway injects the binding's bearer token and HTTP headers in place of the Harness's credential headers and same-named headers, and applies the same rule to each injected value. - It never follows a redirect with the credential. - It never converts between protocols. From 3a806b32690afa2314fc038557546f21ae0728fb Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 03:31:37 +0000 Subject: [PATCH 02/13] Serve the gateway's generic proxy only when the view declares one Config.Proxy selects the generic proxy listener. Without it nothing listens at ProxyPort and Endpoints.Proxy is empty, so a ViewProxyNone view has no generic proxy; the model and MCP listeners keep their fixed ports either way. --- apps/daemon/internal/gateway/gateway.go | 38 +++++++++++++------ apps/daemon/internal/gateway/gateway_test.go | 23 +++++++++++ apps/daemon/internal/gateway/proxy_test.go | 2 +- .../internal/gateway/view_linux_test.go | 1 + 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/apps/daemon/internal/gateway/gateway.go b/apps/daemon/internal/gateway/gateway.go index 6e713424..37b4c47c 100644 --- a/apps/daemon/internal/gateway/gateway.go +++ b/apps/daemon/internal/gateway/gateway.go @@ -1,8 +1,8 @@ // Package gateway is the Session gateway on the agent host. Inside the // Session's loopback-only network namespace it serves one listener per frozen -// model upstream, one per MCP HTTP binding and one generic proxy, so the -// Harness never holds an upstream credential and has no network route of its -// own. +// model upstream, one per MCP HTTP binding and, when the view has one, a +// generic proxy, so the Harness never holds an upstream credential and has no +// network route of its own. // // A listener's identity selects its upstream and credential; nothing is routed // by hostname. A model listener relays the declared native routes of its @@ -60,6 +60,10 @@ type Config struct { // RootCAs are the roots the gateway trusts for upstream TLS. Nil means // the system roots. The server name is always the destination's hostname. RootCAs *x509.CertPool + // Proxy serves the generic proxy at ProxyPort. Without it nothing listens + // there and Endpoints.Proxy is empty; the other listeners keep their + // ports. + Proxy bool } // Model is one frozen model upstream. @@ -81,7 +85,8 @@ type Endpoints struct { // MCP maps each binding's server label to the URL the Harness uses: its // listener with the server URL's path and query. MCP map[string]string - // Proxy is the generic proxy's URL, for HTTP and HTTPS proxy settings. + // Proxy is the generic proxy's URL, for HTTP and HTTPS proxy settings, + // or empty when Config.Proxy is unset. Proxy string } @@ -92,7 +97,8 @@ type SessionNetwork struct { } // ProxyPort is the generic proxy's port in the Session's namespace. The model -// listeners take the following ports in Config order, then the MCP listeners. +// listeners take the following ports in Config order, then the MCP listeners, +// whether or not the proxy is served. // The namespace is the Session's own and the gateway listens before the // Harness starts, so the ports are free; fixing them lets the Harness's // environment be built before the namespace exists. @@ -118,7 +124,7 @@ func Plan(cfg Config) (Endpoints, error) { if err != nil { return Endpoints{}, err } - return g.endpoints(fixedPorts(len(g.listeners))), nil + return g.endpoints(g.fixedPorts()), nil } // Start validates cfg, opens its listeners inside the Session's network @@ -134,7 +140,7 @@ func Start(ctx context.Context, n SessionNetwork, cfg Config) (Endpoints, error) if n.Namespace == nil { return Endpoints{}, fmt.Errorf("%w: no namespace", ErrNetwork) } - ports := fixedPorts(len(g.listeners)) + ports := g.fixedPorts() lns, err := listen(n.Namespace, ports) if err != nil { return Endpoints{}, err @@ -143,10 +149,16 @@ func Start(ctx context.Context, n SessionNetwork, cfg Config) (Endpoints, error) return g.endpoints(ports), nil } -func fixedPorts(n int) []int { - ports := make([]int, n) +// fixedPorts returns each listener's port: ProxyPort for the proxy, then the +// following ports in listener order. +func (g *gateway) fixedPorts() []int { + first := ProxyPort + 1 + if len(g.listeners) > 0 && g.listeners[0].role == roleProxy { + first = ProxyPort + } + ports := make([]int, len(g.listeners)) for i := range ports { - ports[i] = ProxyPort + i + ports[i] = first + i } return ports } @@ -169,7 +181,7 @@ type listener struct { } type gateway struct { - listeners []listener // the proxy, then models, then MCP, in Config order + listeners []listener // the proxy when served, then models, then MCP, in Config order transports []*http.Transport } @@ -193,7 +205,9 @@ func build(session context.Context, cfg Config) (*gateway, error) { forward, sandbox = newTransport(cfg.RootCAs, dial), relayTransport(cfg.RootCAs, dial) g.transports = append(g.transports, forward, sandbox) } - g.listeners = append(g.listeners, listener{role: roleProxy, handler: newProxy(cfg.OpenNetwork, forward)}) + if cfg.Proxy { + g.listeners = append(g.listeners, listener{role: roleProxy, handler: newProxy(cfg.OpenNetwork, forward)}) + } names := map[string]bool{} for _, m := range cfg.Models { diff --git a/apps/daemon/internal/gateway/gateway_test.go b/apps/daemon/internal/gateway/gateway_test.go index 6928b33e..4372f388 100644 --- a/apps/daemon/internal/gateway/gateway_test.go +++ b/apps/daemon/internal/gateway/gateway_test.go @@ -18,6 +18,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink/relay" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink/sandboxlinktest" @@ -158,6 +159,28 @@ func startSandbox(t *testing.T) *sandbox { return sb } +func TestPlanKeepsPortsWithoutTheProxy(t *testing.T) { + cfg := Config{ + Models: []Model{{Name: "main", Provider: modelprovider.Provider{Protocol: modelprovider.Anthropic, BaseURL: "https://upstream.test", APIKey: upstreamKey}}}, + MCP: []agent.MCPBinding{{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: "https://tools.test/mcp"}}, + Prompt: proto.PromptRequestPayload{DisableExecutionEnvironment: true}, + } + for _, proxy := range []bool{false, true} { + cfg.Proxy = proxy + eps, err := Plan(cfg) + if err != nil { + t.Fatal(err) + } + want := Endpoints{Placeholder: modelprovider.Placeholder, Models: map[string]string{"main": "http://127.0.0.1:17101"}, MCP: map[string]string{"tools": "http://127.0.0.1:17102/mcp"}} + if proxy { + want.Proxy = "http://127.0.0.1:17100" + } + if fmt.Sprint(eps) != fmt.Sprint(want) { + t.Errorf("proxy %v: %+v", proxy, eps) + } + } +} + func TestSessionEndEndsBlockedRelays(t *testing.T) { // The upstream upgrades the connection and writes until the Harness's // side is full, then reads until the gateway closes its side. diff --git a/apps/daemon/internal/gateway/proxy_test.go b/apps/daemon/internal/gateway/proxy_test.go index c7d9c85a..f6538aa2 100644 --- a/apps/daemon/internal/gateway/proxy_test.go +++ b/apps/daemon/internal/gateway/proxy_test.go @@ -11,7 +11,7 @@ import ( func TestProxyConnectsOnlyThroughTheSandbox(t *testing.T) { sb := startSandbox(t) - eps := serveOnLoopback(t, Config{OpenNetwork: sb.open}) + eps := serveOnLoopback(t, Config{OpenNetwork: sb.open, Proxy: true}) hello := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "hello "+r.RequestURI) }) secure := httptest.NewTLSServer(hello) defer secure.Close() diff --git a/apps/daemon/internal/gateway/view_linux_test.go b/apps/daemon/internal/gateway/view_linux_test.go index d11a2098..bfdf052a 100644 --- a/apps/daemon/internal/gateway/view_linux_test.go +++ b/apps/daemon/internal/gateway/view_linux_test.go @@ -72,6 +72,7 @@ func TestListenersExistOnlyInTheSession(t *testing.T) { MCP: []agent.MCPBinding{{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: "http://127.0.0.1:" + port + "/mcp"}}, Prompt: proto.PromptRequestPayload{DisableExecutionEnvironment: true}, OpenNetwork: startSandbox(t).open, + Proxy: true, } // The Harness's environment is built before the view exists. eps, err := Plan(cfg) From a116d1ec7c75ec38b5fbb8dcff21b43384ebf700 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 03:58:45 +0000 Subject: [PATCH 03/13] Assemble agent-host Sessions in per-Session views Add apps/daemon/internal/agenthost: Run admits a Session before any effect, allocates its uid and Session directory, points the model provider and HTTP MCP at the Session's credential gateway and calls the kind's view Executor factory. Each ViewSession.Launch builds one sessionview view over the world that worldfs serves from the Session's Link attachment, with the gateway in the view's network namespace. The agent host owns the attachment's lease and fails the Session when the attachment closes, a Link request fails without retry or the world is lost. Teardown releases the Executor, the view, the process broker, the attachment, the Session directory and the uid in that order. Sweep removes Session directories a previous agent host left. Other platforms return ErrUnsupported. The process broker stays behind a local seam until the broker lands. clirunner exports DefaultKillTimeout so a view launch uses the same default as Start and FromHandle. Tests: admission rejects before any dial or directory, the view Executor receives the gateway request, Sweep, and a privileged suite (OAC_TEST_AGENTHOST=1) that runs a Session against oac-sandbox-io. --- .../daemon/internal/agent/clirunner/handle.go | 2 +- .../internal/agent/clirunner/process.go | 6 +- apps/daemon/internal/agenthost/admit.go | 246 ++++++++ .../internal/agenthost/admit_linux_test.go | 167 +++++ apps/daemon/internal/agenthost/agenthost.go | 153 +++++ .../agenthost/agenthost_linux_test.go | 110 ++++ apps/daemon/internal/agenthost/broker.go | 52 ++ apps/daemon/internal/agenthost/doc.go | 33 + .../daemon/internal/agenthost/launch_linux.go | 305 ++++++++++ apps/daemon/internal/agenthost/link.go | 260 ++++++++ apps/daemon/internal/agenthost/run_linux.go | 278 +++++++++ apps/daemon/internal/agenthost/run_other.go | 15 + .../internal/agenthost/sessiondir_linux.go | 124 ++++ .../internal/agenthost/view_linux_test.go | 570 ++++++++++++++++++ 14 files changed, 2319 insertions(+), 2 deletions(-) create mode 100644 apps/daemon/internal/agenthost/admit.go create mode 100644 apps/daemon/internal/agenthost/admit_linux_test.go create mode 100644 apps/daemon/internal/agenthost/agenthost.go create mode 100644 apps/daemon/internal/agenthost/agenthost_linux_test.go create mode 100644 apps/daemon/internal/agenthost/broker.go create mode 100644 apps/daemon/internal/agenthost/doc.go create mode 100644 apps/daemon/internal/agenthost/launch_linux.go create mode 100644 apps/daemon/internal/agenthost/link.go create mode 100644 apps/daemon/internal/agenthost/run_linux.go create mode 100644 apps/daemon/internal/agenthost/run_other.go create mode 100644 apps/daemon/internal/agenthost/sessiondir_linux.go create mode 100644 apps/daemon/internal/agenthost/view_linux_test.go diff --git a/apps/daemon/internal/agent/clirunner/handle.go b/apps/daemon/internal/agent/clirunner/handle.go index fd952579..c676efef 100644 --- a/apps/daemon/internal/agent/clirunner/handle.go +++ b/apps/daemon/internal/agent/clirunner/handle.go @@ -41,7 +41,7 @@ func FromHandle(h Handle, opts HandleOptions) (*Process, error) { opts.Parent = context.Background() } if opts.KillTimeout <= 0 { - opts.KillTimeout = 3 * time.Second + opts.KillTimeout = DefaultKillTimeout } ctx, cancel := context.WithCancel(opts.Parent) p := &Process{Stdin: opts.Stdin, Stdout: opts.Stdout, Stderr: opts.Stderr, ctx: ctx, cancel: cancel, done: make(chan struct{}), killAfter: opts.KillTimeout} diff --git a/apps/daemon/internal/agent/clirunner/process.go b/apps/daemon/internal/agent/clirunner/process.go index d9d40efc..fa406401 100644 --- a/apps/daemon/internal/agent/clirunner/process.go +++ b/apps/daemon/internal/agent/clirunner/process.go @@ -11,6 +11,10 @@ import ( "time" ) +// DefaultKillTimeout is the KillTimeout that a zero or negative StartOptions +// or HandleOptions KillTimeout selects. +const DefaultKillTimeout = 3 * time.Second + type StartOptions struct { Parent context.Context Binary string @@ -52,7 +56,7 @@ func Start(opts StartOptions) (*Process, error) { return nil, fmt.Errorf("clirunner: binary required") } if opts.KillTimeout <= 0 { - opts.KillTimeout = 3 * time.Second + opts.KillTimeout = DefaultKillTimeout } if opts.OwnProcessGroup || runtime.GOOS == "windows" { diff --git a/apps/daemon/internal/agenthost/admit.go b/apps/daemon/internal/agenthost/admit.go new file mode 100644 index 00000000..dac181cd --- /dev/null +++ b/apps/daemon/internal/agenthost/admit.go @@ -0,0 +1,246 @@ +package agenthost + +import ( + "context" + "crypto/x509" + "encoding/json" + "fmt" + "maps" + "math" + "os" + "path" + "path/filepath" + "slices" + "strings" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/gateway" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" +) + +// modelName is the gateway's name for the request's model provider. +const modelName = "model_provider" + +// etcFiles are the files the agent host writes for each Session and presents +// at /etc/. +var etcFiles = []string{"passwd", "group", "hosts", "resolv.conf", "nsswitch.conf"} + +// plan is an admitted Session: everything Run derives before any effect. +type plan struct { + view agent.View + gateway gateway.Config + // request is the request the view Executor factory receives. + request proto.PromptRequestPayload + // mcp and proxy are ViewSession.MCP and ViewSession.Proxy. + mcp []agent.MCPBinding + proxy string +} + +// checkConfig validates cfg and loads the roots in its CA directory. +func checkConfig(cfg Config) (*x509.CertPool, error) { + switch { + case !isHostPath(cfg.StateDir): + return nil, invalidConfig("state directory %q is not absolute and clean", cfg.StateDir) + case cfg.UIDs.First == 0 || cfg.UIDs.Count == 0 || uint64(cfg.UIDs.First)+uint64(cfg.UIDs.Count) > math.MaxUint32: + return nil, invalidConfig("uid range %d+%d", cfg.UIDs.First, cfg.UIDs.Count) + case sandboxlink.CheckRelayURL(cfg.RelayURL) != nil: + return nil, invalidConfig("relay URL") + case cfg.RuntimeID.IsZero() || len(cfg.Credential) == 0: + return nil, invalidConfig("no Runtime ID or credential") + case cfg.Harnesses == nil: + return nil, invalidConfig("no Harness declarations") + case !isHostPath(cfg.Shim): + return nil, invalidConfig("shim %q is not absolute and clean", cfg.Shim) + case !isHostPath(cfg.CADir) || cfg.CADir == "/" || agent.ViewReserved(cfg.CADir): + return nil, invalidConfig("CA directory %q", cfg.CADir) + } + return loadRoots(cfg.CADir) +} + +// loadRoots reads every certificate in dir. Each entry is a regular file of +// PEM certificates, so the view presents exactly what the gateway trusts. +func loadRoots(dir string) (*x509.CertPool, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, &Error{Kind: ErrInvalidConfig, Op: "CA directory", Err: err} + } + if len(entries) == 0 { + return nil, invalidConfig("CA directory %s is empty", dir) + } + roots := x509.NewCertPool() + for _, e := range entries { + if !e.Type().IsRegular() { + return nil, invalidConfig("CA entry %s is not a regular file", e.Name()) + } + data, err := os.ReadFile(filepath.Join(dir, e.Name())) + if err != nil { + return nil, &Error{Kind: ErrInvalidConfig, Op: "CA directory", Err: err} + } + if !roots.AppendCertsFromPEM(data) { + return nil, invalidConfig("CA entry %s holds no PEM certificate", e.Name()) + } + } + return roots, nil +} + +// admit checks the Session and derives its plan without touching anything. +// openNetwork is the Session's Network dial for the gateway. +func admit(cfg Config, roots *x509.CertPool, s Session, openNetwork func(context.Context) (sandboxlink.Stream, error)) (*plan, error) { + if err := checkSession(s); err != nil { + return nil, err + } + req := s.Request + view, err := cfg.Harnesses.ResolveView(req.AgentKind) + if err != nil { + return nil, &Error{Kind: ErrUnsupported, Op: "admit", Err: err} + } + local := req.LocalEnvironment + switch { + case req.DisableExecutionEnvironment: + return nil, unsupported("a Session without an execution environment") + case local == nil: + return nil, unsupported("a Session without a workspace") + case !isViewPath(local.WorkspaceRoot): + return nil, invalidSession("workspace %q is not absolute and clean", local.WorkspaceRoot) + case !req.StrictResume: + return nil, unsupported("a Session without strict resume") + case local.Capabilities || len(local.Skills) > 0 || local.CapabilityRoot != "": + return nil, unsupported("installed Capabilities and skills") + case local.NetworkAccess != "enabled" || len(local.AllowedDomains) > 0: + return nil, unsupported("a restricted workspace network") + } + raw, ok := req.AgentOptions["model_provider"] + if !ok { + return nil, unsupported("a Session without a frozen model provider") + } + provider, err := modelprovider.ParseProvider(raw) + if err != nil { + return nil, invalidSession("model provider: %v", err) + } + bindings, err := agent.ResolveMCPBindings(req) + if err != nil { + return nil, invalidSession("MCP: %v", err) + } + for _, b := range bindings { + if b.Transport != "http" { + return nil, unsupported("%s MCP server %q", b.Transport, b.ServerLabel) + } + } + if err := checkLayout(cfg, view); err != nil { + return nil, err + } + gw := gateway.Config{ + Models: []gateway.Model{{Name: modelName, Provider: provider}}, + MCP: bindings, + Prompt: req, + OpenNetwork: openNetwork, + RootCAs: roots, + Proxy: view.Proxy == agent.ViewProxyEnv, + } + endpoints, err := gateway.Plan(gw) + if err != nil { + return nil, &Error{Kind: ErrInvalidSession, Op: "gateway", Err: err} + } + p := &plan{view: view, gateway: gw, proxy: endpoints.Proxy} + if p.request, err = handoff(req, provider, endpoints); err != nil { + return nil, err + } + for _, b := range bindings { + b.ServerURL, b.BearerToken, b.HTTPHeaders = endpoints.MCP[b.ServerLabel], nil, nil + if b.AllowedTools != nil { + tools := slices.Clone(*b.AllowedTools) + b.AllowedTools = &tools + } + p.mcp = append(p.mcp, b) + } + return p, nil +} + +// handoff rewrites the request as a view Executor receives it: the model +// provider is the gateway's listener with the placeholder key, and MCP is +// only in ViewSession.MCP. +func handoff(req proto.PromptRequestPayload, provider modelprovider.Provider, endpoints gateway.Endpoints) (proto.PromptRequestPayload, error) { + provider.BaseURL, provider.APIKey = endpoints.Models[modelName], modelprovider.Placeholder + encoded, err := json.Marshal(provider) + if err != nil { + return req, invalidSession("model provider: %v", err) + } + var option map[string]any + if err := json.Unmarshal(encoded, &option); err != nil { + return req, invalidSession("model provider: %v", err) + } + req.AgentOptions = maps.Clone(req.AgentOptions) + req.AgentOptions["model_provider"] = option + req.MCPHTTPServers = nil + local := *req.LocalEnvironment + local.MCP = nil + req.LocalEnvironment = &local + return req, nil +} + +// checkLayout rejects a view whose overlays, masks or shim paths meet the +// agent host's own overlays: the /etc files and the CA directory. +func checkLayout(cfg Config, view agent.View) error { + own := []string{cfg.CADir} + for _, name := range etcFiles { + own = append(own, "/etc/"+name) + } + claimed := slices.Clone(view.ShimPaths) + for _, o := range view.Overlays { + claimed = append(claimed, o.Path) + } + for _, m := range view.Masks { + claimed = append(claimed, m.Path) + } + for _, p := range claimed { + for _, q := range own { + if p == q || strings.HasPrefix(p, q+"/") || strings.HasPrefix(q, p+"/") { + return &Error{Kind: ErrUnsupported, Op: "admit", Err: fmt.Errorf("%w: view path %s meets the agent host's %s", agent.ErrInvalidView, p, q)} + } + } + } + return nil +} + +func checkSession(s Session) error { + b := s.Binding + r := b.Resource + switch { + case r.TenantID.IsZero() || r.EnvironmentID.IsZero() || r.ID.IsZero() || r.Generation == 0: + return invalidSession("resource") + case b.AttachmentID.IsZero() || b.SessionID.IsZero() || b.AssignmentID.IsZero() || len(b.AttachGrant) == 0: + return invalidSession("binding") + case s.Input == nil || s.Output == nil: + return invalidSession("no input or output channel") + } + for _, env := range []map[string]string{s.Environment.Sandbox, s.Environment.Tool} { + for name, value := range env { + if name == "" || strings.ContainsAny(name, "=\x00") || strings.ContainsRune(value, 0) { + return invalidSession("environment variable %q", name) + } + } + } + return nil +} + +func isHostPath(p string) bool { + return filepath.IsAbs(p) && filepath.Clean(p) == p && !strings.ContainsRune(p, 0) +} + +func isViewPath(p string) bool { + return strings.HasPrefix(p, "/") && path.Clean(p) == p && !strings.ContainsRune(p, 0) +} + +func unsupported(format string, args ...any) error { + return &Error{Kind: ErrUnsupported, Op: "admit", Err: fmt.Errorf("%w: %s", agent.ErrUnsupportedOperation, fmt.Sprintf(format, args...))} +} + +func invalidSession(format string, args ...any) error { + return &Error{Kind: ErrInvalidSession, Op: "admit", Err: fmt.Errorf(format, args...)} +} + +func invalidConfig(format string, args ...any) error { + return &Error{Kind: ErrInvalidConfig, Err: fmt.Errorf(format, args...)} +} diff --git a/apps/daemon/internal/agenthost/admit_linux_test.go b/apps/daemon/internal/agenthost/admit_linux_test.go new file mode 100644 index 00000000..9e3962d2 --- /dev/null +++ b/apps/daemon/internal/agenthost/admit_linux_test.go @@ -0,0 +1,167 @@ +//go:build linux + +package agenthost + +import ( + "context" + "errors" + "maps" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "reflect" + "strings" + "sync/atomic" + "testing" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentplugin" + "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" +) + +var errFactory = errors.New("factory reached") + +// viewFixture registers "viewed", whose factory records what it receives, +// "masked", whose view masks an /etc file the agent host writes, and +// "plain", which declares no view. +type viewFixture struct { + cfg Config + req proto.PromptRequestPayload + session agent.ViewSession + homeSet bool +} + +func newViewFixture(t *testing.T) *viewFixture { + upstream := httptest.NewTLSServer(http.NotFoundHandler()) + upstream.Close() + f := &viewFixture{} + reg := agent.NewRegistry() + view := agent.View{ + Closure: []agent.ViewMount{{Name: "harness", HostDir: t.TempDir()}}, + LocalExec: []string{"/.oac/harness/harness"}, + Proxy: agent.ViewProxyEnv, + Executor: func(_ context.Context, req proto.PromptRequestPayload, s agent.ViewSession) (agent.Executor, error) { + f.req, f.session = req, s + info, err := os.Stat(s.Home.Host) + f.homeSet = err == nil && info.IsDir() + return nil, errFactory + }, + } + register(reg, "viewed", &view, "mcp_servers") + masked := view + masked.Masks = []agent.ViewMask{{Path: "/etc/passwd"}} + register(reg, "masked", &masked) + register(reg, "plain", nil) + f.cfg = newConfig(t, reg, upstream.Certificate()) + return f +} + +func TestAdmissionRejectsBeforeAnyEffect(t *testing.T) { + f := newViewFixture(t) + for name, c := range map[string]struct { + change func(*proto.PromptRequestPayload) + want []error + }{ + "kind without a view": {func(r *proto.PromptRequestPayload) { r.AgentKind = "plain" }, []error{ErrUnsupported, agent.ErrUnsupportedOperation}}, + "view meeting the agent host's /etc": {func(r *proto.PromptRequestPayload) { r.AgentKind = "masked" }, []error{ErrUnsupported, agent.ErrInvalidView}}, + "environment none": {func(r *proto.PromptRequestPayload) { + r.DisableExecutionEnvironment, r.LocalEnvironment = true, nil + }, []error{ErrUnsupported, agent.ErrUnsupportedOperation}}, + "relative workspace": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.WorkspaceRoot = "workspace" }, []error{ErrInvalidSession}}, + "no model provider": {func(r *proto.PromptRequestPayload) { delete(r.AgentOptions, "model_provider") }, []error{ErrUnsupported}}, + "no strict resume": {func(r *proto.PromptRequestPayload) { r.StrictResume = false }, []error{ErrUnsupported}}, + "capabilities": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.Capabilities = true }, []error{ErrUnsupported}}, + "restricted network": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.NetworkAccess = "disabled" }, []error{ErrUnsupported}}, + "allowed domains only": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.AllowedDomains = []string{"example.com"} }, []error{ErrUnsupported}}, + "stdio MCP": {func(r *proto.PromptRequestPayload) { + r.LocalEnvironment.MCP = []proto.EnvironmentMCP{{Server: agentplugin.MCPServer{Name: "tools", Type: "stdio", Command: "tools"}}} + }, []error{ErrUnsupported, agent.ErrUnsupportedOperation}}, + } { + req := request("viewed", "/workspace", "https://model.test", "sk-test") + c.change(&req) + s, _, _ := newSession(newResource(), req) + var dials atomic.Int32 + err := run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }}) + for _, want := range c.want { + if !errors.Is(err, want) { + t.Errorf("%s: Run = %v, want %v", name, err, want) + } + } + if dials.Load() != 0 || len(leftSessions(t, f.cfg)) != 0 { + t.Errorf("%s: %d dials and %d Session directories", name, dials.Load(), len(leftSessions(t, f.cfg))) + } + if _, err := os.Stat(sessionsDir(f.cfg.StateDir)); !errors.Is(err, os.ErrNotExist) { + t.Errorf("%s: the sessions directory exists", name) + } + } +} + +func TestViewExecutorReceivesTheGatewayRequest(t *testing.T) { + f := newViewFixture(t) + bearer := "mcp-secret" + req := request("viewed", "/workspace", "https://model.test", "sk-test") + req.MCPHTTPServers = &[]proto.MCPHTTPServer{{ConnectionOrigin: "environment", ServerLabel: "docs", ServerURL: "https://mcp.test/docs", BearerToken: &bearer}} + original := maps.Clone(req.AgentOptions) + s, _, _ := newSession(newResource(), req) + var dials atomic.Int32 + err := run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }}) + if !errors.Is(err, ErrExecutor) || !errors.Is(err, errFactory) { + t.Fatalf("Run = %v, want the factory's error as ErrExecutor", err) + } + provider, err := modelprovider.ParseProvider(f.req.AgentOptions["model_provider"]) + if err != nil || provider.BaseURL != "http://127.0.0.1:17101" || provider.APIKey != modelprovider.Placeholder || provider.Protocol != modelprovider.Anthropic { + t.Errorf("model provider %+v, %v; want the gateway with the placeholder", provider.BaseURL, err) + } + if !reflect.DeepEqual(req.AgentOptions, original) { + t.Error("the Session's request changed") + } + if f.req.MCPHTTPServers != nil || f.req.LocalEnvironment == nil || f.req.LocalEnvironment.MCP != nil || f.req.LocalEnvironment.WorkspaceRoot != "/workspace" { + t.Error("the request still carries MCP or lost its workspace") + } + mcp := f.session.MCP + if len(mcp) != 1 || mcp[0].ServerLabel != "docs" || mcp[0].ServerURL != "http://127.0.0.1:17102/docs" || mcp[0].BearerToken != nil || mcp[0].HTTPHeaders != nil { + t.Errorf("ViewSession.MCP = %+v, want one credential-free gateway binding", mcp) + } + if f.session.Proxy != "http://127.0.0.1:17100" || f.session.Home.View != "/.oac/home" || !f.homeSet || f.session.Launch == nil { + t.Errorf("ViewSession proxy %q, home %+v (present %v)", f.session.Proxy, f.session.Home, f.homeSet) + } + if !strings.HasPrefix(f.session.Home.Host, sessionsDir(f.cfg.StateDir)+string(filepath.Separator)) { + t.Errorf("home %s is outside the Session directories", f.session.Home.Host) + } + if dials.Load() != 0 || len(leftSessions(t, f.cfg)) != 0 { + t.Errorf("%d dials and %d Session directories after Run", dials.Load(), len(leftSessions(t, f.cfg))) + } + + // A connection option reaches the factory's handoff check, which rejects + // it before the adapter runs. + req = request("viewed", "/workspace", "https://model.test", "sk-test") + req.AgentOptions["mcp_servers"] = map[string]any{} + s, _, _ = newSession(newResource(), req) + f.req = proto.PromptRequestPayload{} + err = run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }}) + if !errors.Is(err, ErrUnsupported) || !errors.Is(err, agent.ErrViewHandoff) || f.req.AgentKind != "" { + t.Errorf("Run with a connection option = %v, want ErrUnsupported and ErrViewHandoff before the adapter", err) + } + if dials.Load() != 0 || len(leftSessions(t, f.cfg)) != 0 { + t.Errorf("%d dials and %d Session directories after Run", dials.Load(), len(leftSessions(t, f.cfg))) + } +} + +func TestSweepRemovesLeftoverSessions(t *testing.T) { + cfg := Config{StateDir: t.TempDir()} + if err := Sweep(cfg); err != nil { + t.Fatalf("Sweep without sessions: %v", err) + } + left := filepath.Join(sessionsDir(cfg.StateDir), "left", "home") + if err := os.MkdirAll(left, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(left, "history"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if err := Sweep(cfg); err != nil || len(leftSessions(t, cfg)) != 0 { + t.Fatalf("Sweep = %v, %d Session directories left", err, len(leftSessions(t, cfg))) + } +} diff --git a/apps/daemon/internal/agenthost/agenthost.go b/apps/daemon/internal/agenthost/agenthost.go new file mode 100644 index 00000000..20008fff --- /dev/null +++ b/apps/daemon/internal/agenthost/agenthost.go @@ -0,0 +1,153 @@ +package agenthost + +import ( + "crypto/tls" + "errors" + "log/slog" + "strings" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +// Config is the agent host's own configuration, shared by its Sessions. It +// holds a credential: keep it in memory and never log it. +type Config struct { + // StateDir is an absolute host directory private to the agent host. Each + // Session's directory is StateDir/sessions/. + StateDir string + // UIDs is the range Session uids are allocated from; each Session's gid + // equals its uid. Only one agent host runs per kernel, and nothing else + // uses the range. + UIDs UIDRange + // RelayURL and TLS reach the Link relay, as sandboxlink.DialAttach takes + // them. A nil TLS uses the system roots. + RelayURL string + TLS *tls.Config + // RuntimeID and Credential authenticate the agent host to the relay. + RuntimeID sandboxwire.ID + Credential []byte + // Harnesses holds the Harness declarations. A kind runs only when it + // declares an agent.View. + Harnesses *agent.Registry + // Shim is the absolute host path of the static oac-process-shim binary. + Shim string + // CADir is an absolute host directory of regular PEM files: the roots the + // agent host trusts. The gateway trusts exactly these for upstream TLS, + // and the view presents the directory read-only at the same path. + CADir string + // Log receives each view's presentation report. Nil discards it. + Log *slog.Logger +} + +// UIDRange is Count ids from First. First is nonzero. +type UIDRange struct { + First, Count uint32 +} + +// Session is one Session the agent host runs. +type Session struct { + // Binding is the Session's Link attachment. + Binding Binding + // Environment is what processes forwarded to the sandbox receive. + Environment Environment + // Request is the Session's frozen request. Its Input is ignored; each + // Turn's input arrives on Input. + Request proto.PromptRequestPayload + // Input carries one Turn each. Run runs them in order and ends the + // Session once Input is closed and the last Turn has settled. + Input <-chan Input + // Output receives every Turn's envelopes. Run never closes it. + Output chan<- proto.Envelope +} + +// Binding is the identity of the Session's Link attachment, as each Open +// carries it. +type Binding struct { + Resource sandboxlink.ResourceRef + AttachmentID sandboxwire.ID + SessionID sandboxwire.ID + AssignmentID sandboxwire.ID + AssignmentEpoch uint64 + // AttachGrant authorizes each Open and renewal. It is secret. + AttachGrant []byte +} + +// Environment is the remote environment policy of processes forwarded to the +// sandbox. +type Environment struct { + // Sandbox holds the Environment's fixed values, such as HOME, PATH, + // TMPDIR and LANG in the sandbox. + Sandbox map[string]string + // Tool is the Environment's tool environment. + Tool map[string]string +} + +// Input is one Turn: its run ID and its input. +type Input struct { + RunID string + Message proto.MessageInput +} + +// Error kinds. Every error Run and Sweep return matches one of them with +// errors.Is. +var ( + // ErrUnsupported is a platform other than Linux, or a Session that asks + // for what the agent host does not run. A Session's error also matches + // agent.ErrUnsupportedKind, agent.ErrUnsupportedOperation, + // agent.ErrViewHandoff, or agent.ErrInvalidView for a view whose paths + // meet the agent host's own overlays. + ErrUnsupported = errors.New("agenthost: unsupported") + // ErrInvalidConfig is a Config that Run and Sweep reject. + ErrInvalidConfig = errors.New("agenthost: invalid configuration") + // ErrInvalidSession is a malformed Session. + ErrInvalidSession = errors.New("agenthost: invalid session") + // ErrCapacity means every Session uid is in use. + ErrCapacity = errors.New("agenthost: no free session uid") + // ErrSessionExists means the Session's directory already exists. + ErrSessionExists = errors.New("agenthost: session directory exists") + // ErrExecutor is a view Executor factory that failed. + ErrExecutor = errors.New("agenthost: view executor failed") + // ErrLink is a Link attachment that failed or ended. + ErrLink = errors.New("agenthost: link attachment failed") + // ErrWorld is a world that no longer shows the sandbox faithfully, or + // that cannot show that its attachment holds nothing. + ErrWorld = errors.New("agenthost: world lost") + // ErrLaunch is a view that could not be launched. + ErrLaunch = errors.New("agenthost: launch failed") + // ErrProcessBroker is a process broker that could not start. + ErrProcessBroker = errors.New("agenthost: process broker failed") + // ErrTurn is a Turn that failed or left its Executor unusable. + ErrTurn = errors.New("agenthost: turn failed") + // ErrTeardown is a Session resource that could not be released. + ErrTeardown = errors.New("agenthost: teardown incomplete") +) + +// Error is a typed agent host failure. It matches Kind and, when present, +// Err. Its message never includes a credential. +type Error struct { + Kind error + Op string + Err error +} + +func (e *Error) Error() string { + var b strings.Builder + b.WriteString(e.Kind.Error()) + if e.Op != "" { + b.WriteString(": " + e.Op) + } + if e.Err != nil { + b.WriteString(": " + e.Err.Error()) + } + return b.String() +} + +func (e *Error) Unwrap() []error { + if e.Err == nil { + return []error{e.Kind} + } + return []error{e.Kind, e.Err} +} diff --git a/apps/daemon/internal/agenthost/agenthost_linux_test.go b/apps/daemon/internal/agenthost/agenthost_linux_test.go new file mode 100644 index 00000000..8dee3a0f --- /dev/null +++ b/apps/daemon/internal/agenthost/agenthost_linux_test.go @@ -0,0 +1,110 @@ +//go:build linux + +package agenthost + +import ( + "context" + "crypto/x509" + "encoding/pem" + "errors" + "os" + "path/filepath" + "sync/atomic" + "testing" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/sessionview" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto/prototest" + "github.com/MiniMax-AI/OpenAgentCore/internal/harnessconfig" + "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +// The test binary is also the privileged suite's Harness inside the view. +func TestMain(m *testing.M) { + sessionview.Init() + if os.Getenv(harnessEnv) != "" { + os.Exit(runHarness(os.Args[1:])) + } + os.Exit(m.Run()) +} + +// newConfig returns a Config whose CA directory holds ca. +func newConfig(t *testing.T, reg *agent.Registry, ca *x509.Certificate) Config { + t.Helper() + dir := t.TempDir() + if err := os.Chmod(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "ca.pem"), pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: ca.Raw}), 0o644); err != nil { + t.Fatal(err) + } + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + return Config{StateDir: t.TempDir(), UIDs: UIDRange{First: 70000, Count: 8}, RelayURL: "ws://127.0.0.1:9", RuntimeID: sandboxwire.NewID(), + Credential: []byte("runtime-credential"), Harnesses: reg, Shim: exe, CADir: dir} +} + +// register declares kind with view, or without one when view is nil. +func register(reg *agent.Registry, kind string, view *agent.View, connection ...string) { + info := proto.SupportedAgentKind{Kind: kind, Available: true, Capabilities: prototest.Capabilities(proto.AgentKindCapabilities{})} + declaration := agent.Declaration{Info: info, ConnectionOptions: connection, + Configuration: harnessconfig.Configuration{Providers: []harnessconfig.Provider{{Protocol: string(modelprovider.Anthropic)}}}} + reg.Register(declaration, agent.Runtime{Info: info, View: view, + Session: func(context.Context, proto.PromptRequestPayload, chan<- proto.Envelope) (agent.Session, error) { + return nil, errors.New("not used") + }}) +} + +// request is a Session request the agent host admits. +func request(kind, workspace, baseURL, key string) proto.PromptRequestPayload { + return proto.PromptRequestPayload{ + AgentKind: kind, + StrictResume: true, + AgentOptions: map[string]any{"model": "m", "model_provider": map[string]any{"protocol": "anthropic", "base_url": baseURL, "api_key": key}}, + LocalEnvironment: &proto.LocalEnvironment{WorkspaceRoot: workspace, NetworkAccess: "enabled"}, + } +} + +// newSession returns a Session for req on a fresh attachment of resource. +func newSession(resource sandboxlink.ResourceRef, req proto.PromptRequestPayload) (Session, chan Input, chan proto.Envelope) { + in, out := make(chan Input), make(chan proto.Envelope, 16) + return Session{ + Binding: Binding{Resource: resource, AttachmentID: sandboxwire.NewID(), SessionID: sandboxwire.NewID(), + AssignmentID: sandboxwire.NewID(), AssignmentEpoch: 1, AttachGrant: []byte("grant-" + sandboxwire.NewID().String())}, + Request: req, Input: in, Output: out, + }, in, out +} + +func newResource() sandboxlink.ResourceRef { + return sandboxlink.ResourceRef{TenantID: sandboxwire.NewID(), EnvironmentID: sandboxwire.NewID(), Kind: sandboxlink.ResourceAllocation, + ID: sandboxwire.NewID(), Generation: 1} +} + +// countingDial counts dials and connects nothing. +func countingDial(n *atomic.Int32) dialFunc { + return func(context.Context, func(sandboxlink.AttachmentClosed)) (attachLink, error) { + n.Add(1) + return nil, errors.New("no relay in this test") + } +} + +// noBroker is a process broker that serves nothing. +type noBroker struct{} + +func (noBroker) Start(brokerConfig) error { return nil } +func (noBroker) Close() error { return nil } + +// leftSessions lists what remains under the state directory's sessions. +func leftSessions(t *testing.T, cfg Config) []os.DirEntry { + t.Helper() + entries, err := os.ReadDir(sessionsDir(cfg.StateDir)) + if err != nil && !errors.Is(err, os.ErrNotExist) { + t.Fatal(err) + } + return entries +} diff --git a/apps/daemon/internal/agenthost/broker.go b/apps/daemon/internal/agenthost/broker.go new file mode 100644 index 00000000..de5d0866 --- /dev/null +++ b/apps/daemon/internal/agenthost/broker.go @@ -0,0 +1,52 @@ +package agenthost + +import ( + "context" + "errors" + "io" + "time" +) + +// processBroker runs the commands the view's shims forward to the sandbox +// over the Process service. One broker serves a Session from its first launch +// until teardown. +type processBroker interface { + // Start begins serving the Session's run directory. + Start(brokerConfig) error + // Close cancels and releases the remote operations that remain and stops + // serving. + Close() error +} + +// brokerConfig is what a Session's broker serves. +type brokerConfig struct { + // RunDir is the host directory the view presents read-only at + // agent.ViewPrivateRoot/agent.ViewRunName. + RunDir string + // UID and GID are the Session's. + UID, GID uint32 + // Names maps each shim name to the program it runs in the sandbox, found + // on the remote PATH; Paths maps each view path the shim is bound over to + // the same sandbox path. + Names, Paths map[string]string + // Pass names the Harness variables a forwarded process keeps + // (agent.View.ForwardEnv). + Pass []string + // Sandbox and Tool are the Session's Environment. + Sandbox, Tool map[string]string + // Dial opens a Process stream on the Session's attachment. + Dial func(context.Context) (io.ReadWriteCloser, error) + // CancelGrace is the grace of a Cancel the broker sends on its own. + CancelGrace time.Duration +} + +// errNoBroker is unavailableBroker's Start error. +var errNoBroker = errors.New("no process broker in this build") + +// unavailableBroker is the broker this build has. Its Start fails, so a +// Session admits, prepares its Executor and fails its first launch with +// ErrProcessBroker. +type unavailableBroker struct{} + +func (unavailableBroker) Start(brokerConfig) error { return errNoBroker } +func (unavailableBroker) Close() error { return nil } diff --git a/apps/daemon/internal/agenthost/doc.go b/apps/daemon/internal/agenthost/doc.go new file mode 100644 index 00000000..a1934920 --- /dev/null +++ b/apps/daemon/internal/agenthost/doc.go @@ -0,0 +1,33 @@ +// Package agenthost runs Sessions whose Harness runs on the agent host, next to +// Core, while its tools, files and network act in the sandbox through the +// Session's Link attachment. It needs Linux; elsewhere Run and Sweep return +// ErrUnsupported. +// +// Run runs one Session. It admits the Session before any effect: the kind +// must declare an agent.View, and the request must use only what a view runs. +// It then allocates the Session uid, creates the Session directory under +// Config.StateDir, rewrites the request so the model provider and HTTP MCP +// reach the network only through the Session's gateway, and calls the view's +// Executor factory. The first ViewSession.Launch starts the process broker; +// each Launch builds one sessionview view, of which one at a time is live, +// over the world that worldfs serves from the attachment's File service, with +// the gateway listening in the view's network namespace. +// +// Each view presents the closure directories read-only and executable, the +// Session home read-write and noexec, the broker's run directory read-only, +// the agent host's /etc/passwd, group, hosts, resolv.conf and nsswitch.conf, +// the agent host's CA directory at its host path, then the adapter's overlays +// and masks and the process shim. Everything else is the world. +// +// The agent host owns the Session's Link attachment: it opens each stream +// with the Session's binding, renews the lease and fails the Session when the +// relay closes the attachment, a Link request fails in a way that is not +// retryable, or the world is lost or may still hold state. A failure cancels +// the running Turn and closes the live view. Teardown releases, in order, the +// Executor, the view, the process broker, the Link attachment, the Session +// directory and the uid. Sweep removes the Session directories a previous +// agent host left; Run's owner calls it at startup. +// +// The Harness view protocol is in contracts/agents-api/harness-onboarding.md +// and the gateway's in contracts/agents-api/model-execution.md. +package agenthost diff --git a/apps/daemon/internal/agenthost/launch_linux.go b/apps/daemon/internal/agenthost/launch_linux.go new file mode 100644 index 00000000..d516bb3e --- /dev/null +++ b/apps/daemon/internal/agenthost/launch_linux.go @@ -0,0 +1,305 @@ +//go:build linux + +package agenthost + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "slices" + "syscall" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent/clirunner" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/gateway" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/sessionview" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/worldfs" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" +) + +// worldExport is the File service export that holds the sandbox's world, as +// docs/sandbox-bootstrap.md defines it. +const worldExport sandboxlink.ExportID = "world" + +// liveView is the Session's one live view slot. +type liveView struct { + view *sessionview.View // nil while the view is being built + closed bool // the Session ended while the view was being built +} + +// closeLive closes the live view. It runs when the Session's context ends. +func (s *session) closeLive() { + s.mu.Lock() + lv := s.live + var v *sessionview.View + if lv != nil { + lv.closed, v = true, lv.view + } + s.mu.Unlock() + if v != nil { + v.Close() + } +} + +// launch is ViewSession.Launch: it builds one view and runs opts.Binary in it. +func (s *session) launch(opts clirunner.StartOptions) (*clirunner.Process, error) { + switch { + case !slices.Contains(s.plan.view.LocalExec, opts.Binary): + return nil, &Error{Kind: ErrLaunch, Err: fmt.Errorf("%q is not a LocalExec path", opts.Binary)} + case !isViewPath(opts.Dir): + return nil, &Error{Kind: ErrLaunch, Err: fmt.Errorf("directory %q is not absolute and clean", opts.Dir)} + case !opts.OwnProcessGroup: + return nil, &Error{Kind: ErrLaunch, Err: errors.New("a view process runs in its own process group")} + } + if opts.Parent == nil { + opts.Parent = context.Background() + } + if opts.KillTimeout <= 0 { + opts.KillTimeout = clirunner.DefaultKillTimeout + } + s.mu.Lock() + switch { + case s.ctx.Err() != nil: + s.mu.Unlock() + return nil, &Error{Kind: ErrLaunch, Err: errors.New("the Session is ending")} + case s.live != nil: + s.mu.Unlock() + return nil, &Error{Kind: ErrLaunch, Err: errors.New("the Session already has a live view")} + } + lv := &liveView{} + s.live = lv + s.views.Add(1) + s.mu.Unlock() + p, err := s.start(lv, opts) + if err != nil { + s.release(lv) + } + return p, err +} + +// release frees the view slot and ends the launch's count. +func (s *session) release(lv *liveView) { + s.mu.Lock() + if s.live == lv { + s.live = nil + } + s.mu.Unlock() + s.views.Done() +} + +func (s *session) start(lv *liveView, opts clirunner.StartOptions) (*clirunner.Process, error) { + if err := s.startBroker(opts.KillTimeout); err != nil { + return nil, err + } + if err := s.dir.chownHome(s.uid); err != nil { + return nil, &Error{Kind: ErrLaunch, Op: "home", Err: err} + } + ends, err := newStdio(opts.NeedStdin) + if err != nil { + return nil, &Error{Kind: ErrLaunch, Op: "stdio", Err: err} + } + // The gateway serves until the view has ended. + viewCtx, stopGateway := context.WithCancel(context.Background()) + world := worldfs.New(worldExport, s.openFile) + spec := s.spec(viewCtx, world, opts, ends) + + // Construction ends with the Session or with the caller. + startCtx, cancel := context.WithCancel(s.ctx) + stop := context.AfterFunc(opts.Parent, cancel) + v, err := sessionview.Start(startCtx, spec) + stop() + cancel() + ends.closeChild() + if err != nil { + stopGateway() + ends.closeParent() + if errors.Is(err, worldfs.ErrAttachmentDirty) { + // Only ending the attachment releases what the world may hold. + err = &Error{Kind: ErrWorld, Op: "launch", Err: err} + s.fail(err) + return nil, err + } + return nil, &Error{Kind: ErrLaunch, Err: err} + } + p := v.Presentation() + s.log.Info("agent host view started", "binary", opts.Binary, "targets", p.Targets, "links", p.Links, "synthesized", p.Synthesized) + + s.mu.Lock() + lv.view = v + closed := lv.closed + s.mu.Unlock() + if closed { + v.Close() + stopGateway() + ends.closeParent() + return nil, &Error{Kind: ErrLaunch, Err: errors.New("the Session is ending")} + } + process, err := clirunner.FromHandle(viewHandle{v}, clirunner.HandleOptions{Parent: opts.Parent, Stdin: ends.stdin(), + Stdout: ends.parent[1], Stderr: ends.parent[2], KillTimeout: opts.KillTimeout}) + if err != nil { + v.Close() + stopGateway() + ends.closeParent() + return nil, &Error{Kind: ErrLaunch, Err: err} + } + ended := make(chan struct{}) + go func() { + select { + case <-world.Lost(): + s.fail(&Error{Kind: ErrWorld, Op: "world", Err: world.Err()}) + case <-ended: + } + }() + go func() { + defer s.release(lv) + _, _ = v.Wait() + stopGateway() + close(ended) + }() + return process, nil +} + +// startBroker starts the Session's process broker at its first launch. +func (s *session) startBroker(grace time.Duration) error { + s.brokerMu.Lock() + defer s.brokerMu.Unlock() + if s.broker != nil { + return nil + } + view := s.plan.view + names := make(map[string]string, len(view.Shims)) + for _, n := range view.Shims { + names[n] = n + } + paths := make(map[string]string, len(view.ShimPaths)) + for _, p := range view.ShimPaths { + paths[p] = p + } + b := s.deps.broker() + err := b.Start(brokerConfig{RunDir: s.dir.entry(runEntry), UID: s.uid, GID: s.uid, Names: names, Paths: paths, + Pass: slices.Clone(view.ForwardEnv), Sandbox: s.in.Environment.Sandbox, Tool: s.in.Environment.Tool, + Dial: s.openProcess, CancelGrace: grace}) + if err != nil { + err = &Error{Kind: ErrProcessBroker, Err: err} + s.fail(err) + return err + } + s.broker = b + return nil +} + +// spec builds the view: the closure, home and run directories, the agent +// host's /etc files and CA directory, the adapter's overlays and masks, the +// shim and the gateway in the view's network namespace. +func (s *session) spec(viewCtx context.Context, world *worldfs.World, opts clirunner.StartOptions, ends *stdio) sessionview.Spec { + view := s.plan.view + var private []sessionview.PrivateDir + for _, m := range view.Closure { + private = append(private, sessionview.PrivateDir{Name: m.Name, HostDir: m.HostDir, Exec: true}) + } + private = append(private, + sessionview.PrivateDir{Name: agent.ViewHomeName, HostDir: s.dir.entry(homeEntry), Writable: true}, + sessionview.PrivateDir{Name: agent.ViewRunName, HostDir: s.dir.entry(runEntry)}) + var overlays []sessionview.Overlay + for _, name := range etcFiles { + overlays = append(overlays, sessionview.Overlay{Path: "/etc/" + name, Source: s.dir.entry(etcEntry, name)}) + } + overlays = append(overlays, sessionview.Overlay{Path: s.cfg.CADir, Source: s.cfg.CADir}) + for _, o := range view.Overlays { + overlays = append(overlays, sessionview.Overlay{Path: o.Path, Source: o.Source, Exec: o.Exec}) + } + for _, m := range view.Masks { + source := s.dir.entry(maskEntry, "file") + if m.Dir { + source = s.dir.entry(maskEntry, "dir") + } + overlays = append(overlays, sessionview.Overlay{Path: m.Path, Source: source}) + } + return sessionview.Spec{ + World: world.Serve, + Private: private, + Overlays: overlays, + Shim: sessionview.Shim{Binary: s.cfg.Shim, Names: view.Shims, Paths: view.ShimPaths}, + Process: sessionview.Process{Path: opts.Binary, Args: append([]string{opts.Binary}, opts.Args...), Env: opts.Env, + Dir: opts.Dir, UID: s.uid, GID: s.uid, Stdin: ends.child[0], Stdout: ends.child[1], Stderr: ends.child[2], + Grace: opts.KillTimeout}, + Network: sessionview.Network{Setup: func(netns *os.File) error { + _, err := gateway.Start(viewCtx, gateway.SessionNetwork{Namespace: netns}, s.plan.gateway) + return err + }}, + StagingParent: s.dir.entry(stagingEntry), + } +} + +// stdio holds the view process's stdio: the child ends sessionview passes to +// the process and the parent ends the clirunner.Process owns. Without a stdin +// pipe the child's stdin is /dev/null and there is no parent end. +type stdio struct { + child, parent [3]*os.File +} + +func newStdio(needStdin bool) (*stdio, error) { + e := &stdio{} + if needStdin { + r, w, err := os.Pipe() + if err != nil { + return nil, err + } + e.child[0], e.parent[0] = r, w + } else { + null, err := os.Open(os.DevNull) + if err != nil { + return nil, err + } + e.child[0] = null + } + for i := 1; i < 3; i++ { + r, w, err := os.Pipe() + if err != nil { + e.closeChild() + e.closeParent() + return nil, err + } + e.child[i], e.parent[i] = w, r + } + return e, nil +} + +func (e *stdio) stdin() io.WriteCloser { + if e.parent[0] == nil { + return nil + } + return e.parent[0] +} + +func (e *stdio) closeChild() { closeFiles(e.child[:]) } +func (e *stdio) closeParent() { closeFiles(e.parent[:]) } + +func closeFiles(files []*os.File) { + for _, f := range files { + if f != nil { + f.Close() + } + } +} + +// viewHandle is a view as a clirunner.Handle. +type viewHandle struct{ v *sessionview.View } + +func (h viewHandle) Signal(sig syscall.Signal) error { return h.v.Signal(sig) } + +func (h viewHandle) Wait() (int, error) { + exit, err := h.v.Wait() + switch { + case err != nil: + return -1, err + case exit.Signal != 0: + return -1, nil + } + return exit.Code, nil +} + +func (h viewHandle) Close() error { return h.v.Close() } diff --git a/apps/daemon/internal/agenthost/link.go b/apps/daemon/internal/agenthost/link.go new file mode 100644 index 00000000..257aec48 --- /dev/null +++ b/apps/daemon/internal/agenthost/link.go @@ -0,0 +1,260 @@ +package agenthost + +import ( + "context" + "errors" + "fmt" + "sync" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +// attachLink is the part of *sandboxlink.AttachLink the Session uses. +type attachLink interface { + OpenService(context.Context, sandboxlink.Open) (sandboxlink.Stream, sandboxlink.Opened, error) + Renew(context.Context, sandboxlink.RenewAttachment) (sandboxlink.AttachmentRenewed, error) + CloseAttachment(context.Context, sandboxwire.ID) error + Done() <-chan struct{} + Close() error +} + +// dialFunc connects an attach link; onClosed is its OnAttachmentClosed. +type dialFunc func(ctx context.Context, onClosed func(sandboxlink.AttachmentClosed)) (attachLink, error) + +func relayDial(cfg Config) dialFunc { + return func(ctx context.Context, onClosed func(sandboxlink.AttachmentClosed)) (attachLink, error) { + return sandboxlink.DialAttach(ctx, sandboxlink.AttachConfig{URL: cfg.RelayURL, TLS: cfg.TLS, RuntimeID: cfg.RuntimeID, + Credential: cfg.Credential, OnAttachmentClosed: onClosed}) + } +} + +const ( + // retryWait is the pause between attempts of a renewal or close that + // failed with a retryable error. + retryWait = 250 * time.Millisecond + // closeBound bounds CloseAttachment at teardown, redials included. + closeBound = 10 * time.Second +) + +// linkOwner owns the Session's Link attachment. It dials the relay when a +// stream is first needed and again after the link drops, pins the service +// instance the first Opened reports, renews the lease before it passes, and +// fails the Session on any Link failure that is not retryable and on the +// relay closing the attachment. +type linkOwner struct { + dial dialFunc + binding Binding + fail func(error) + + dialMu sync.Mutex // serializes dials + mu sync.Mutex + link attachLink + // instance is the service instance of the first Opened; zero before. + instance sandboxwire.ID + lease time.Time + // opened records that an Open was sent, so the attachment may exist. + opened bool + closing bool + renewer chan struct{} // closed when the renewal loop returns; nil before it starts + stop context.CancelFunc +} + +func newLinkOwner(dial dialFunc, b Binding, fail func(error)) *linkOwner { + return &linkOwner{dial: dial, binding: b, fail: fail} +} + +// current returns the live link, dialing a new one when there is none. +func (l *linkOwner) current(ctx context.Context) (attachLink, error) { + l.dialMu.Lock() + defer l.dialMu.Unlock() + l.mu.Lock() + link := l.link + l.mu.Unlock() + if link != nil { + select { + case <-link.Done(): + link.Close() + default: + return link, nil + } + } + link, err := l.dial(ctx, l.closed) + if err != nil { + return nil, err + } + l.mu.Lock() + l.link = link + l.mu.Unlock() + return link, nil +} + +// open opens a stream of service on the Session's attachment. +func (l *linkOwner) open(ctx context.Context, service sandboxlink.Service, version uint16) (sandboxlink.Stream, error) { + l.mu.Lock() + closing := l.closing + l.mu.Unlock() + if closing { + return nil, &Error{Kind: ErrLink, Op: "open " + service.String(), Err: errors.New("the Session is ending")} + } + link, err := l.current(ctx) + if err != nil { + return nil, l.observe("dial", err) + } + l.mu.Lock() + if l.closing { + l.mu.Unlock() + return nil, &Error{Kind: ErrLink, Op: "open " + service.String(), Err: errors.New("the Session is ending")} + } + l.opened = true + expected := l.instance + l.mu.Unlock() + b := l.binding + st, opened, err := link.OpenService(ctx, sandboxlink.Open{Service: service, Version: version, Resource: b.Resource, + ExpectedServerInstanceID: expected, AttachmentID: b.AttachmentID, SessionID: b.SessionID, + AssignmentID: b.AssignmentID, AssignmentEpoch: b.AssignmentEpoch, AttachGrant: b.AttachGrant}) + if err != nil { + return nil, l.observe("open "+service.String(), err) + } + l.mu.Lock() + defer l.mu.Unlock() + if l.instance.IsZero() { + l.instance = opened.ServerInstanceID + } + if opened.LeaseExpiresAt.After(l.lease) { + l.lease = opened.LeaseExpiresAt + } + if l.renewer == nil && !l.closing { + ctx, stop := context.WithCancel(context.Background()) + l.renewer, l.stop = make(chan struct{}), stop + go l.renew(ctx) + } + return st, nil +} + +// observe fails the Session on a Link failure that is not retryable and +// returns err as a typed error. +func (l *linkOwner) observe(op string, err error) error { + err = &Error{Kind: ErrLink, Op: op, Err: err} + if !retryable(err) { + l.fail(err) + } + return err +} + +// retryable reports whether a failed Link request may succeed later: a Link +// failure whose code says so, or a transport failure. +func retryable(err error) bool { + var le *sandboxlink.Error + return !errors.As(err, &le) || le.Code.Retryable() +} + +// closed is the link's OnAttachmentClosed. It never blocks. +func (l *linkOwner) closed(c sandboxlink.AttachmentClosed) { + if c.AttachmentID == l.binding.AttachmentID { + l.fail(&Error{Kind: ErrLink, Op: "attachment", Err: fmt.Errorf("the relay closed the attachment (reason %d)", c.Reason)}) + } +} + +// renew extends the lease at half its remaining time until ctx ends. A +// renewal that fails retryably is tried again until the lease passes. +func (l *linkOwner) renew(ctx context.Context) { + defer close(l.renewer) + for { + l.mu.Lock() + lease := l.lease + l.mu.Unlock() + timer := time.NewTimer(time.Until(lease) / 2) + select { + case <-ctx.Done(): + timer.Stop() + return + case <-timer.C: + } + for { + if !time.Now().Before(lease) { + l.fail(&Error{Kind: ErrLink, Op: "renew", Err: sandboxlink.LeaseExpired}) + return + } + attempt, cancel := context.WithDeadline(ctx, lease) + renewed, err := l.renewOnce(attempt) + cancel() + if ctx.Err() != nil { + return + } + if err == nil { + l.mu.Lock() + if renewed.LeaseExpiresAt.After(l.lease) { + l.lease = renewed.LeaseExpiresAt + } + l.mu.Unlock() + break + } + if err = l.observe("renew", err); !retryable(err) { + return + } + if !sleep(ctx, retryWait) { + return + } + } + } +} + +func (l *linkOwner) renewOnce(ctx context.Context) (sandboxlink.AttachmentRenewed, error) { + link, err := l.current(ctx) + if err != nil { + return sandboxlink.AttachmentRenewed{}, err + } + return link.Renew(ctx, sandboxlink.RenewAttachment{AttachmentID: l.binding.AttachmentID, AttachGrant: l.binding.AttachGrant}) +} + +// close stops renewal, closes the attachment when an Open may have created +// it, and closes the link. Later opens fail. +func (l *linkOwner) close() error { + l.mu.Lock() + l.closing = true + opened, renewer, stop := l.opened, l.renewer, l.stop + l.mu.Unlock() + if stop != nil { + stop() + <-renewer + } + var err error + if opened { + ctx, cancel := context.WithTimeout(context.Background(), closeBound) + for { + var link attachLink + if link, err = l.current(ctx); err == nil { + err = link.CloseAttachment(ctx, l.binding.AttachmentID) + } + if err == nil || !retryable(err) || !sleep(ctx, retryWait) { + break + } + } + cancel() + } + l.mu.Lock() + link := l.link + l.link = nil + l.mu.Unlock() + if link != nil { + link.Close() + } + if err != nil { + return &Error{Kind: ErrTeardown, Op: "close attachment", Err: err} + } + return nil +} + +// sleep waits d and reports whether ctx is still live. +func sleep(ctx context.Context, d time.Duration) bool { + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-ctx.Done(): + return false + case <-timer.C: + return true + } +} diff --git a/apps/daemon/internal/agenthost/run_linux.go b/apps/daemon/internal/agenthost/run_linux.go new file mode 100644 index 00000000..87a68942 --- /dev/null +++ b/apps/daemon/internal/agenthost/run_linux.go @@ -0,0 +1,278 @@ +//go:build linux + +package agenthost + +import ( + "context" + "errors" + "fmt" + "io" + "io/fs" + "log/slog" + "os" + "path/filepath" + "sync" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxfs" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxnet" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxprocess" +) + +const ( + // turnBuffer is how many envelopes a Turn may emit ahead of Output. + turnBuffer = 64 + // settleBound bounds a cancelled Turn's settlement. + settleBound = 30 * time.Second + // executorCloseBound bounds Executor.Close at teardown. + executorCloseBound = 30 * time.Second +) + +// deps are the parts tests replace. +type deps struct { + dial dialFunc + broker func() processBroker +} + +// Run runs one Session until Input is closed, ctx ends or the Session fails, +// then tears it down. It returns nil after Input closed and every Turn +// settled, ctx's error when ctx ended the Session, and otherwise the error +// that ended it, joined with any teardown failure. +func Run(ctx context.Context, cfg Config, s Session) error { + return run(ctx, cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return unavailableBroker{} }}) +} + +// Sweep removes every Session directory under cfg.StateDir. Run's owner calls +// it at startup, before any Session runs. +func Sweep(cfg Config) error { + if !isHostPath(cfg.StateDir) { + return invalidConfig("state directory %q is not absolute and clean", cfg.StateDir) + } + dir := sessionsDir(cfg.StateDir) + entries, err := os.ReadDir(dir) + if errors.Is(err, fs.ErrNotExist) { + return nil + } + if err != nil { + return &Error{Kind: ErrTeardown, Op: "sweep", Err: err} + } + var errs []error + for _, e := range entries { + errs = append(errs, os.RemoveAll(filepath.Join(dir, e.Name()))) + } + if err := errors.Join(errs...); err != nil { + return &Error{Kind: ErrTeardown, Op: "sweep", Err: err} + } + return nil +} + +// session is one running Session. +type session struct { + cfg Config + in Session + deps deps + plan *plan + link *linkOwner + log *slog.Logger + uid uint32 + dir sessionDir + + // ctx ends when the Session fails or tears down. + ctx context.Context + cancel context.CancelFunc + + failMu sync.Mutex + failure error + + mu sync.Mutex + // live is the one view that may run; nil when none does. + live *liveView + // views counts launches and their views until each is torn down. + views sync.WaitGroup + + brokerMu sync.Mutex + broker processBroker // started at the first launch +} + +func run(ctx context.Context, cfg Config, in Session, d deps) error { + roots, err := checkConfig(cfg) + if err != nil { + return err + } + s := &session{cfg: cfg, in: in, deps: d, log: cfg.Log} + if s.log == nil { + s.log = slog.New(slog.DiscardHandler) + } + s.ctx, s.cancel = context.WithCancel(ctx) + defer s.cancel() + s.link = newLinkOwner(d.dial, in.Binding, s.fail) + if s.plan, err = admit(cfg, roots, in, s.openNetwork); err != nil { + return err + } + if s.uid, err = allocUID(cfg.UIDs); err != nil { + return err + } + if s.dir, err = createSessionDir(cfg.StateDir, in.Binding.SessionID, s.uid); err != nil { + freeUID(s.uid) + return err + } + context.AfterFunc(s.ctx, s.closeLive) + + exec, err := s.plan.view.Executor(s.ctx, s.plan.request, agent.ViewSession{ + Home: agent.ViewDir{Host: s.dir.entry(homeEntry), View: agent.ViewPrivateRoot + "/" + agent.ViewHomeName}, + Proxy: s.plan.proxy, + MCP: s.plan.mcp, + Launch: s.launch, + }) + if err != nil { + err = executorError(err) + } else { + err = s.drive(exec) + } + // The Session's failure and the end of ctx explain whatever followed them. + if failure := s.failed(); failure != nil { + err = failure + } else if ctx.Err() != nil { + err = ctx.Err() + } + return errors.Join(err, s.teardown(exec)) +} + +func executorError(err error) error { + if errors.Is(err, agent.ErrUnsupportedOperation) || errors.Is(err, agent.ErrViewHandoff) { + return &Error{Kind: ErrUnsupported, Op: "executor", Err: err} + } + return &Error{Kind: ErrExecutor, Err: err} +} + +// fail records the Session's first failure and ends the Session: its live +// view closes and its Turn is cancelled. +func (s *session) fail(err error) { + s.failMu.Lock() + if s.failure == nil { + s.failure = err + } + s.failMu.Unlock() + s.cancel() +} + +func (s *session) failed() error { + s.failMu.Lock() + defer s.failMu.Unlock() + return s.failure +} + +// drive runs each Turn from Input in order until Input is closed, a Turn +// fails or the Session ends. +func (s *session) drive(exec agent.Executor) error { + for { + select { + case <-s.ctx.Done(): + return nil + case in, ok := <-s.in.Input: + if !ok { + return nil + } + if err := s.turn(exec, in); err != nil { + return err + } + } + } +} + +// turn runs one Turn, forwards its envelopes to Output and waits for its +// settlement. When the Session ends first it cancels the Turn. +func (s *session) turn(exec agent.Executor, in Input) error { + out := make(chan proto.Envelope, turnBuffer) + turn, err := exec.StartTurn(s.ctx, in.RunID, in.Message, out) + if turn == nil { + if err == nil { + err = errors.New("no Turn") + } + return &Error{Kind: ErrTurn, Op: "start", Err: err} + } + forwarded := make(chan struct{}) + go func() { + defer close(forwarded) + for e := range out { + select { + case s.in.Output <- e: + case <-s.ctx.Done(): + } + } + }() + settled, serr := turn.AwaitSettlement(s.ctx) + if s.ctx.Err() != nil { + ctx, cancel := context.WithTimeout(context.Background(), settleBound) + turn.Cancel(ctx) + settled, serr = turn.AwaitSettlement(ctx) + cancel() + } + if serr != nil { + return &Error{Kind: ErrTurn, Op: "settle", Err: serr} + } + <-forwarded + switch { + case err != nil: + return &Error{Kind: ErrTurn, Op: "start", Err: err} + case !settled.Reusable: + return &Error{Kind: ErrTurn, Op: "settle", Err: fmt.Errorf("the Executor is not reusable: %s", settled.Reason)} + } + return nil +} + +// teardown releases the Session in order: the Executor, the view, the +// process broker, the Link attachment, the Session directory and the uid. +func (s *session) teardown(exec agent.Executor) error { + var errs []error + if exec != nil { + ctx, cancel := context.WithTimeout(context.Background(), executorCloseBound) + if err := exec.Close(ctx); err != nil { + errs = append(errs, &Error{Kind: ErrTeardown, Op: "close executor", Err: err}) + } + cancel() + } + // Ending the Session closes the live view and refuses new launches. Under + // mu, every launch that passed its check has already counted itself. + s.mu.Lock() + s.cancel() + s.mu.Unlock() + s.views.Wait() + s.brokerMu.Lock() + broker := s.broker + s.brokerMu.Unlock() + if broker != nil { + if err := broker.Close(); err != nil { + errs = append(errs, &Error{Kind: ErrTeardown, Op: "close process broker", Err: err}) + } + } + errs = append(errs, s.link.close()) + if err := os.RemoveAll(string(s.dir)); err != nil { + errs = append(errs, &Error{Kind: ErrTeardown, Op: "remove session directory", Err: err}) + } + freeUID(s.uid) + return errors.Join(errs...) +} + +func (s *session) openFile(ctx context.Context) (io.ReadWriteCloser, error) { + st, err := s.link.open(ctx, sandboxlink.ServiceFile, sandboxfs.Version) + if err != nil { + return nil, err + } + return st, nil +} + +func (s *session) openProcess(ctx context.Context) (io.ReadWriteCloser, error) { + st, err := s.link.open(ctx, sandboxlink.ServiceProcess, sandboxprocess.Version) + if err != nil { + return nil, err + } + return st, nil +} + +func (s *session) openNetwork(ctx context.Context) (sandboxlink.Stream, error) { + return s.link.open(ctx, sandboxlink.ServiceNetwork, sandboxnet.Version) +} diff --git a/apps/daemon/internal/agenthost/run_other.go b/apps/daemon/internal/agenthost/run_other.go new file mode 100644 index 00000000..40b9d80e --- /dev/null +++ b/apps/daemon/internal/agenthost/run_other.go @@ -0,0 +1,15 @@ +//go:build !linux + +package agenthost + +import "context" + +// Run reports that the agent host needs Linux. +func Run(context.Context, Config, Session) error { + return &Error{Kind: ErrUnsupported, Op: "run"} +} + +// Sweep reports that the agent host needs Linux. +func Sweep(Config) error { + return &Error{Kind: ErrUnsupported, Op: "sweep"} +} diff --git a/apps/daemon/internal/agenthost/sessiondir_linux.go b/apps/daemon/internal/agenthost/sessiondir_linux.go new file mode 100644 index 00000000..7fe9f4b7 --- /dev/null +++ b/apps/daemon/internal/agenthost/sessiondir_linux.go @@ -0,0 +1,124 @@ +//go:build linux + +package agenthost + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sync" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +// uids holds the Session uids in use. Only one agent host runs per kernel, so +// the set is process-wide. +var uids = struct { + sync.Mutex + used map[uint32]bool +}{used: map[uint32]bool{}} + +func allocUID(r UIDRange) (uint32, error) { + uids.Lock() + defer uids.Unlock() + for i := range r.Count { + if id := r.First + i; !uids.used[id] { + uids.used[id] = true + return id, nil + } + } + return 0, &Error{Kind: ErrCapacity} +} + +func freeUID(id uint32) { + uids.Lock() + defer uids.Unlock() + delete(uids.used, id) +} + +// The entries of a Session directory. +const ( + homeEntry = agent.ViewHomeName // the Session home, owned by the Session uid + runEntry = agent.ViewRunName // the process broker's run directory + etcEntry = "etc" // the /etc files + maskEntry = "mask" // an empty file and an empty directory that masks present + stagingEntry = "staging" // sessionview's staging parent +) + +func sessionsDir(stateDir string) string { return filepath.Join(stateDir, "sessions") } + +// sessionDir is one Session's host directory, private to the agent host. +type sessionDir string + +func (d sessionDir) entry(name ...string) string { + return filepath.Join(append([]string{string(d)}, name...)...) +} + +// createSessionDir creates the Session directory with its home, run, etc, +// mask and staging entries. +func createSessionDir(stateDir string, id sandboxwire.ID, uid uint32) (sessionDir, error) { + parent := sessionsDir(stateDir) + if err := os.MkdirAll(parent, 0o700); err != nil { + return "", &Error{Kind: ErrInvalidConfig, Op: "state directory", Err: err} + } + d := sessionDir(filepath.Join(parent, id.String())) + if err := os.Mkdir(string(d), 0o700); errors.Is(err, fs.ErrExist) { + return "", &Error{Kind: ErrSessionExists, Err: err} + } else if err != nil { + return "", &Error{Kind: ErrInvalidConfig, Op: "session directory", Err: err} + } + if err := d.populate(uid); err != nil { + os.RemoveAll(string(d)) + return "", &Error{Kind: ErrInvalidConfig, Op: "session directory", Err: err} + } + return d, nil +} + +func (d sessionDir) populate(uid uint32) error { + dirs := []struct { + name string + mode fs.FileMode + }{{homeEntry, 0o700}, {runEntry, 0o755}, {etcEntry, 0o755}, {maskEntry, 0o755}, {filepath.Join(maskEntry, "dir"), 0o555}, {stagingEntry, 0o700}} + for _, e := range dirs { + if err := os.Mkdir(d.entry(e.name), e.mode); err != nil { + return err + } + } + home := agent.ViewPrivateRoot + "/" + agent.ViewHomeName + files := map[string]string{ + filepath.Join(maskEntry, "file"): "", + filepath.Join(etcEntry, "passwd"): fmt.Sprintf("root:x:0:0:root:/root:/usr/sbin/nologin\noac:x:%d:%d:oac:%s:/bin/bash\nnobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin\n", + uid, uid, home), + filepath.Join(etcEntry, "group"): fmt.Sprintf("root:x:0:\noac:x:%d:\nnogroup:x:65534:\n", uid), + filepath.Join(etcEntry, "hosts"): "127.0.0.1 localhost\n::1 localhost\n", + filepath.Join(etcEntry, "resolv.conf"): "", + filepath.Join(etcEntry, "nsswitch.conf"): "passwd: files\ngroup: files\nshadow: files\nhosts: files\n", + } + for name, content := range files { + if err := os.WriteFile(d.entry(name), []byte(content), 0o444); err != nil { + return err + } + } + return nil +} + +// chownHome gives the Session uid the home tree, including what the view +// Executor factory wrote there. It runs while no view is live, so no Session +// process changes the tree meanwhile, and os.Root keeps every change inside +// it. +func (d sessionDir) chownHome(uid uint32) error { + root, err := os.OpenRoot(d.entry(homeEntry)) + if err != nil { + return err + } + defer root.Close() + return fs.WalkDir(root.FS(), ".", func(name string, _ fs.DirEntry, err error) error { + if err != nil { + return err + } + return root.Lchown(name, int(uid), int(uid)) + }) +} diff --git a/apps/daemon/internal/agenthost/view_linux_test.go b/apps/daemon/internal/agenthost/view_linux_test.go new file mode 100644 index 00000000..d6eedad6 --- /dev/null +++ b/apps/daemon/internal/agenthost/view_linux_test.go @@ -0,0 +1,570 @@ +//go:build linux + +package agenthost + +import ( + "bytes" + "context" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "io" + "io/fs" + "net" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent/clirunner" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/sessionview" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxbootstrap" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxfs" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink/relay" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink/sandboxlinktest" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +// The view suite needs root with CAP_SYS_ADMIN and CAP_NET_ADMIN, /dev/fuse, +// no AppArmor confinement and a static oac-sandbox-io, whose world is the +// container's /. Run it in a throwaway container: +// +// CGO_ENABLED=0 go build -o /tmp/oac-sandbox-io ./apps/sandboxio/cmd/oac-sandbox-io +// CGO_ENABLED=0 go test -c -o /tmp/agenthost.test ./apps/daemon/internal/agenthost +// docker run --rm --cap-add SYS_ADMIN --cap-add NET_ADMIN --device /dev/fuse --security-opt apparmor=unconfined \ +// -e OAC_TEST_AGENTHOST=1 -e OAC_TEST_SANDBOXIO=/sandboxio -v /tmp/oac-sandbox-io:/sandboxio:ro \ +// -v /tmp/agenthost.test:/t.test:ro debian:bookworm-slim /t.test -test.v +const ( + gateEnv = "OAC_TEST_AGENTHOST" + sandboxIOEnv = "OAC_TEST_SANDBOXIO" + harnessEnv = "OAC_AGENTHOST_HARNESS" + modelEnv = "OAC_AGENTHOST_MODEL" + caEnv = "OAC_AGENTHOST_CA" + harnessPath = "/.oac/harness/harness" + upstreamKey = "sk-agenthost-upstream" + wait = 20 * time.Second +) + +func TestSessionRunsInAViewOverItsAttachment(t *testing.T) { + if os.Getenv(gateEnv) != "1" { + t.Skipf("set %s=1 and run the test binary as root in a privileged container; see the comment above", gateEnv) + } + if err := sessionview.Probe(); err != nil { + t.Fatalf("Probe: %v", err) + } + sb := startSandbox(t, os.Getenv(sandboxIOEnv)) + // The model upstream reports whether each request carried the real key. + keyed := make(chan bool, 4) + upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + keyed <- r.Header.Get("X-Api-Key") == upstreamKey + io.WriteString(w, "answer") + })) + defer upstream.Close() + + reg := agent.NewRegistry() + cfg := newConfig(t, reg, upstream.Certificate()) + cfg.RelayURL = sb.url + closure := t.TempDir() + if err := os.Chmod(closure, 0o755); err != nil { + t.Fatal(err) + } + copyExecutable(t, filepath.Join(closure, "harness")) + register(reg, "test", &agent.View{ + Closure: []agent.ViewMount{{Name: "harness", HostDir: closure}}, + Masks: []agent.ViewMask{{Path: "/etc/ld.so.preload"}, {Path: "/etc/hostname"}, {Path: "/etc/apt", Dir: true}}, + LocalExec: []string{harnessPath}, + Proxy: agent.ViewProxyNone, + Executor: func(_ context.Context, req proto.PromptRequestPayload, s agent.ViewSession) (agent.Executor, error) { + provider, err := modelprovider.ParseProvider(req.AgentOptions["model_provider"]) + if err != nil { + return nil, err + } + return &testExecutor{session: s, dir: req.LocalEnvironment.WorkspaceRoot, + env: []string{harnessEnv + "=1", modelEnv + "=" + provider.BaseURL, caEnv + "=" + cfg.CADir}}, nil + }, + }) + sb.auth.AddRuntime(cfg.Credential, cfg.RuntimeID) + sb.ready(t, cfg) + workspace, err := os.MkdirTemp("/tmp", "agenthost-workspace-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(workspace) + if err := os.Chmod(workspace, 0o777); err != nil { + t.Fatal(err) + } + + t.Run("one Session", func(t *testing.T) { + s := startSession(t, cfg, sb, request("test", workspace, upstream.URL, upstreamKey), 2*time.Second) + r := s.turn(t, "check") + for _, name := range harnessChecks { + if msg, ok := r.Checks[name]; !ok || msg != "" { + t.Errorf("%s: %q", name, msg) + } + } + if r.Exit != "" { + t.Errorf("Harness: %s; stderr %s", r.Exit, r.Stderr) + } + select { + case ok := <-keyed: + if !ok { + t.Error("the upstream did not receive the configured key") + } + default: + t.Error("no request reached the upstream") + } + if data, err := os.ReadFile(filepath.Join(workspace, "renamed")); err != nil || string(data) != "world" { + t.Errorf("the renamed world file holds %q, %v", data, err) + } + // Only renewal keeps the attachment past its 2 second lease. + time.Sleep(3 * time.Second) + if r := s.turn(t, "touch"); r.Checks["touch"] != "" || r.Exit != "" { + t.Errorf("touch after the first lease: %+v", r) + } + if data, err := os.ReadFile(filepath.Join(workspace, "touched")); err != nil || string(data) != "renewed" { + t.Errorf("the file written after the first lease holds %q, %v", data, err) + } + close(s.in) + if err := s.wait(t); err != nil { + t.Fatalf("Run = %v", err) + } + if err := sb.renew(t, cfg, s.binding); !errors.Is(err, sandboxlink.LeaseExpired) { + t.Errorf("Renew after teardown = %v, want LeaseExpired for a closed attachment", err) + } + checkReleased(t, cfg) + }) + + t.Run("a restarted sandbox service fails the Session", func(t *testing.T) { + s := startSession(t, cfg, sb, request("test", workspace, upstream.URL, upstreamKey), time.Minute) + s.send(t, "wait") + beat := filepath.Join(workspace, "beat") + for deadline := time.Now().Add(wait); ; time.Sleep(50 * time.Millisecond) { + if _, err := os.Stat(beat); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatal("the Harness never wrote to the world") + } + } + sb.stop() + sb.start(t) + err := s.wait(t) + t.Logf("Run = %v", err) + if !errors.Is(err, ErrLink) && !errors.Is(err, ErrWorld) { + t.Errorf("Run = %v, want ErrLink or ErrWorld", err) + } + if errors.Is(err, ErrTeardown) { + t.Errorf("teardown incomplete: %v", err) + } + checkReleased(t, cfg) + }) +} + +// sandbox is a relay and the oac-sandbox-io serving its one resource. +type sandbox struct { + auth *sandboxlinktest.Authority + url string + bin string + bootstrap string + resource sandboxlink.ResourceRef + cmd *exec.Cmd +} + +func startSandbox(t *testing.T, bin string) *sandbox { + if bin == "" { + t.Fatalf("set %s to a static oac-sandbox-io", sandboxIOEnv) + } + auth := sandboxlinktest.NewAuthority() + rl, err := relay.New(relay.Config{Authority: auth}) + if err != nil { + t.Fatal(err) + } + srv := httptest.NewServer(rl) + t.Cleanup(srv.Close) + t.Cleanup(func() { rl.Close() }) + url := "ws://" + strings.TrimPrefix(srv.URL, "http://") + in := sandboxbootstrap.Input{Version: sandboxbootstrap.Version, LinkURL: url, Credential: "serve-credential", Resource: sandboxbootstrap.Resource{ + TenantID: uuid.NewString(), EnvironmentID: uuid.NewString(), Kind: "allocation", ID: uuid.NewString(), Generation: 1}} + raw, err := in.Marshal() + if err != nil { + t.Fatal(err) + } + bootstrap := filepath.Join(t.TempDir(), "bootstrap.json") + if err := os.WriteFile(bootstrap, raw, 0o600); err != nil { + t.Fatal(err) + } + sb := &sandbox{auth: auth, url: url, bin: bin, bootstrap: bootstrap, resource: in.Resource.Ref()} + auth.AddServe([]byte(in.Credential), sandboxlink.ServePeer{PeerID: sandboxwire.NewID(), Resource: sb.resource}) + sb.start(t) + t.Cleanup(sb.stop) + return sb +} + +func (sb *sandbox) start(t *testing.T) { + cmd := exec.Command(sb.bin, "--bootstrap-file", sb.bootstrap) + cmd.Stdout, cmd.Stderr = os.Stderr, os.Stderr + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + sb.cmd = cmd +} + +func (sb *sandbox) stop() { + if sb.cmd != nil { + sb.cmd.Process.Signal(syscall.SIGTERM) + sb.cmd.Wait() + sb.cmd = nil + } +} + +func (sb *sandbox) grant(b Binding, runtimeID sandboxwire.ID, lease time.Duration) { + sb.auth.AddGrant(b.AttachGrant, sandboxlinktest.Grant{RuntimeID: runtimeID, Resource: b.Resource, SessionID: b.SessionID, + AssignmentID: b.AssignmentID, AssignmentEpoch: b.AssignmentEpoch, Lease: lease, + Services: []sandboxlink.Service{sandboxlink.ServiceFile, sandboxlink.ServiceProcess, sandboxlink.ServiceNetwork}}) +} + +func (sb *sandbox) dial(t *testing.T, cfg Config) *sandboxlink.AttachLink { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), wait) + defer cancel() + link, err := sandboxlink.DialAttach(ctx, sandboxlink.AttachConfig{URL: sb.url, RuntimeID: cfg.RuntimeID, Credential: cfg.Credential}) + if err != nil { + t.Fatal(err) + } + return link +} + +// ready waits until the relay holds oac-sandbox-io as the resource's serve +// peer, so that an Open reaches it. +func (sb *sandbox) ready(t *testing.T, cfg Config) { + t.Helper() + probe, _, _ := newSession(sb.resource, proto.PromptRequestPayload{}) + b := probe.Binding + sb.grant(b, cfg.RuntimeID, time.Minute) + link := sb.dial(t, cfg) + defer link.Close() + ctx, cancel := context.WithTimeout(context.Background(), wait) + defer cancel() + for { + st, _, err := link.OpenService(ctx, sandboxlink.Open{Service: sandboxlink.ServiceFile, Version: sandboxfs.Version, Resource: b.Resource, + AttachmentID: b.AttachmentID, SessionID: b.SessionID, AssignmentID: b.AssignmentID, AssignmentEpoch: b.AssignmentEpoch, AttachGrant: b.AttachGrant}) + if err == nil { + st.Close() + break + } + if !errors.Is(err, sandboxlink.ServiceUnavailable) || ctx.Err() != nil { + t.Fatalf("oac-sandbox-io is not serving: %v", err) + } + time.Sleep(50 * time.Millisecond) + } + if err := link.CloseAttachment(ctx, b.AttachmentID); err != nil { + t.Fatal(err) + } +} + +// renew renews b's attachment from a fresh link. +func (sb *sandbox) renew(t *testing.T, cfg Config, b Binding) error { + link := sb.dial(t, cfg) + defer link.Close() + ctx, cancel := context.WithTimeout(context.Background(), wait) + defer cancel() + _, err := link.Renew(ctx, sandboxlink.RenewAttachment{AttachmentID: b.AttachmentID, AttachGrant: b.AttachGrant}) + return err +} + +// sessionRun is a running Session under test. +type sessionRun struct { + binding Binding + in chan Input + out chan proto.Envelope + done chan error +} + +func startSession(t *testing.T, cfg Config, sb *sandbox, req proto.PromptRequestPayload, lease time.Duration) *sessionRun { + s, in, out := newSession(sb.resource, req) + sb.grant(s.Binding, cfg.RuntimeID, lease) + r := &sessionRun{binding: s.Binding, in: in, out: out, done: make(chan error, 1)} + go func() { + r.done <- run(context.Background(), cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return noBroker{} }}) + }() + return r +} + +func (r *sessionRun) send(t *testing.T, mode string) { + t.Helper() + select { + case r.in <- Input{RunID: mode, Message: proto.TextInput(mode)}: + case err := <-r.done: + t.Fatalf("Run ended before the %s Turn: %v", mode, err) + case <-time.After(wait): + t.Fatalf("Run took no %s Turn", mode) + } +} + +func (r *sessionRun) turn(t *testing.T, mode string) report { + t.Helper() + r.send(t, mode) + select { + case e := <-r.out: + var rep report + if err := json.Unmarshal(e.Payload, &rep); err != nil { + t.Fatal(err) + } + return rep + case err := <-r.done: + t.Fatalf("Run ended during the %s Turn: %v", mode, err) + case <-time.After(wait): + t.Fatalf("no %s report", mode) + } + return report{} +} + +func (r *sessionRun) wait(t *testing.T) error { + t.Helper() + select { + case err := <-r.done: + return err + case <-time.After(3 * wait): + t.Fatal("Run did not return") + return nil + } +} + +// checkReleased checks that no Session directory, mount or process remains. +func checkReleased(t *testing.T, cfg Config) { + t.Helper() + if left := leftSessions(t, cfg); len(left) != 0 { + t.Errorf("%d Session directories remain", len(left)) + } + mounts, err := os.ReadFile("/proc/self/mountinfo") + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(mounts), cfg.StateDir) { + t.Error("a mount under the state directory remains") + } + procs, err := os.ReadDir("/proc") + if err != nil { + t.Fatal(err) + } + for _, p := range procs { + if _, err := strconv.Atoi(p.Name()); err != nil { + continue + } + status, err := os.ReadFile(filepath.Join("/proc", p.Name(), "status")) + if err != nil { + continue + } + for line := range strings.Lines(string(status)) { + if fields := strings.Fields(line); len(fields) > 1 && fields[0] == "Uid:" { + if uid, _ := strconv.ParseUint(fields[1], 10, 32); uid >= uint64(cfg.UIDs.First) && uid < uint64(cfg.UIDs.First+cfg.UIDs.Count) { + t.Errorf("process %s runs with Session uid %d", p.Name(), uid) + } + } + } + } +} + +func copyExecutable(t *testing.T, dst string) { + t.Helper() + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(exe) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(dst, data, 0o755); err != nil { + t.Fatal(err) + } +} + +// testExecutor runs one Harness view per Turn; the Turn's input text is the +// Harness mode. +type testExecutor struct { + session agent.ViewSession + dir string + env []string +} + +func (e *testExecutor) StartTurn(_ context.Context, runID string, input proto.MessageInput, out chan<- proto.Envelope) (agent.Turn, error) { + mode := *input[0].Content[0].Text + p, err := e.session.Launch(clirunner.StartOptions{Binary: harnessPath, Args: []string{mode}, Dir: e.dir, Env: e.env, + OwnProcessGroup: true, KillTimeout: time.Second}) + if err != nil { + return nil, err + } + turn := &testTurn{p: p, settled: make(chan struct{})} + go turn.run(runID, out) + return turn, nil +} + +func (e *testExecutor) Close(context.Context) error { return nil } + +// report is a Turn's one envelope: the Harness's checks, its stderr and how +// it exited. +type report struct { + Checks map[string]string `json:"checks"` + Stderr string `json:"stderr"` + Exit string `json:"exit"` +} + +type testTurn struct { + p *clirunner.Process + settled chan struct{} +} + +func (t *testTurn) run(runID string, out chan<- proto.Envelope) { + defer close(t.settled) + defer close(out) + var stderr bytes.Buffer + copied := make(chan struct{}) + go func() { + io.Copy(&stderr, t.p.Stderr) + close(copied) + }() + stdout, _ := io.ReadAll(t.p.Stdout) + <-copied + var r report + json.Unmarshal(stdout, &r.Checks) + if err := t.p.Wait(); err != nil { + r.Exit = err.Error() + } + r.Stderr = stderr.String() + payload, _ := json.Marshal(r) + out <- proto.Envelope{Type: proto.TypeOutputMessage, ID: runID, Payload: payload} +} + +func (t *testTurn) Cancel(context.Context) error { + t.p.Cancel() + return nil +} + +func (t *testTurn) CancellationOutcome() proto.DonePayload { return proto.DonePayload{} } + +func (t *testTurn) SteerWithReceipt(context.Context, proto.PromptSteerPayload, func()) error { + return agent.ErrUnsupportedOperation +} + +func (t *testTurn) AwaitSettlement(ctx context.Context) (agent.TurnSettlement, error) { + select { + case <-t.settled: + return agent.TurnSettlement{Reusable: true}, nil + case <-ctx.Done(): + return agent.TurnSettlement{}, ctx.Err() + } +} + +var harnessChecks = []string{"world rename", "model through the gateway", "no direct route", "world is noexec", "masks", "home", "passwd", "CA directory"} + +// runHarness runs inside the view, in the workspace, and prints a JSON map +// from each check to its failure, empty when it passed. +func runHarness(args []string) int { + if len(args) != 1 { + return 2 + } + checks := map[string]func() error{} + switch args[0] { + case "check": + checks = map[string]func() error{ + "world rename": func() error { + if err := os.WriteFile("staged", []byte("world"), 0o644); err != nil { + return err + } + return os.Rename("staged", "renamed") + }, + "model through the gateway": func() error { + req, _ := http.NewRequest("POST", os.Getenv(modelEnv)+"/v1/messages", strings.NewReader("{}")) + req.Header.Set("X-Api-Key", modelprovider.Placeholder) + resp, err := (&http.Client{Timeout: wait}).Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + if body, _ := io.ReadAll(resp.Body); resp.StatusCode != http.StatusOK || string(body) != "answer" { + return fmt.Errorf("answered %d %q", resp.StatusCode, body) + } + return nil + }, + "no direct route": func() error { + c, err := net.DialTimeout("tcp", "192.0.2.1:80", 2*time.Second) + if err == nil { + c.Close() + return errors.New("connected outside the gateway") + } + if !errors.Is(err, syscall.ENETUNREACH) { + return fmt.Errorf("dial: %v, want ENETUNREACH", err) + } + return nil + }, + "world is noexec": func() error { + if err := exec.Command("/bin/true").Run(); !errors.Is(err, fs.ErrPermission) { + return fmt.Errorf("exec of a world binary: %v, want a permission error", err) + } + return nil + }, + "masks": func() error { + for _, p := range []string{"/etc/ld.so.preload", "/etc/hostname"} { + if data, err := os.ReadFile(p); err != nil || len(data) != 0 { + return fmt.Errorf("%s holds %d bytes, %v", p, len(data), err) + } + } + if entries, err := os.ReadDir("/etc/apt"); err != nil || len(entries) != 0 { + return fmt.Errorf("/etc/apt holds %d entries, %v", len(entries), err) + } + return nil + }, + "home": func() error { + return os.WriteFile(agent.ViewPrivateRoot+"/"+agent.ViewHomeName+"/probe", []byte("x"), 0o600) + }, + "passwd": func() error { + data, err := os.ReadFile("/etc/passwd") + if want := fmt.Sprintf("oac:x:%d:%d:oac:/.oac/home:/bin/bash\n", os.Getuid(), os.Getgid()); err != nil || !strings.Contains(string(data), want) { + return fmt.Errorf("/etc/passwd lacks %q: %v", want, err) + } + return nil + }, + "CA directory": func() error { + data, err := os.ReadFile(filepath.Join(os.Getenv(caEnv), "ca.pem")) + if block, _ := pem.Decode(data); err != nil || block == nil { + return fmt.Errorf("no CA certificate: %v", err) + } + return nil + }, + } + case "touch": + checks["touch"] = func() error { return os.WriteFile("touched", []byte("renewed"), 0o644) } + case "wait": + // Beat in the world until the view ends. + for i := 0; i < 600; i++ { + os.WriteFile("beat", []byte(strconv.Itoa(i)), 0o644) + os.ReadDir(".") + time.Sleep(100 * time.Millisecond) + } + default: + return 2 + } + report := map[string]string{} + for name, check := range checks { + report[name] = "" + if err := check(); err != nil { + report[name] = err.Error() + } + } + json.NewEncoder(os.Stdout).Encode(report) + return 0 +} From 6e7753d961aa26e80401949b9e2c67b62425fa33 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 03:59:23 +0000 Subject: [PATCH 04/13] Document the agent host in the repository map and MCP bindings Add the agenthost row to the repository map and state, where the effective MCP bindings are described, that an agent-host view's gateway holds HTTP binding credentials. --- contracts/agents-api/environments.md | 2 +- docs/development.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/agents-api/environments.md b/contracts/agents-api/environments.md index a20e10e8..f27733d2 100644 --- a/contracts/agents-api/environments.md +++ b/contracts/agents-api/environments.md @@ -367,7 +367,7 @@ Environment MCP needs enabled network. Duplicate server identities are rejected. ### Effective bindings -The Runtime resolves public HTTP declarations and installed Plugin MCP through `agent.ResolveMCPBindings` before adapter projection. Each transient binding keeps its connection origin, transport, nullable tool allowlist, required flag, credential authority and installed stdio identity. Bindings are never persisted or logged; duplicate identities and unavailable selected credentials are rejected. MiniMax reads its session-private native runtime-name registry for exact first-frame identities and cross-checks completed native results for both transports; adapters never fabricate a delayed start event or guess identities. +The Runtime resolves public HTTP declarations and installed Plugin MCP through `agent.ResolveMCPBindings` before adapter projection. Each transient binding keeps its connection origin, transport, nullable tool allowlist, required flag, credential authority and installed stdio identity. Bindings are never persisted or logged; duplicate identities and unavailable selected credentials are rejected. MiniMax reads its session-private native runtime-name registry for exact first-frame identities and cross-checks completed native results for both transports; adapters never fabricate a delayed start event or guess identities. In an agent-host view, the Session's [credential gateway](model-execution.md#credential-gateway) holds each HTTP binding's bearer token and headers, and the Harness receives credential-free loopback URLs ([Endpoints and proxy](harness-onboarding.md#endpoints-and-proxy)). ### Public MCP connection origin diff --git a/docs/development.md b/docs/development.md index b6dea0f6..a4b2e1a7 100644 --- a/docs/development.md +++ b/docs/development.md @@ -73,6 +73,7 @@ For frontend development, run `pnpm dev:web` using the fixture or Core connectio | `apps/sandboxio` | Sandbox I/O service binary `oac-sandbox-io` and its Linux protocol services | [Sandbox bootstrap](sandbox-bootstrap.md#responsibilities-and-readiness), [File access protocol](file-access-protocol.md#the-linux-service), [Process protocol](process-protocol.md#implement-a-service), [Network protocol](sandbox-network-protocol.md#implement-a-service) | | `apps/daemon/internal/dispatch` | Runtime preparation, Executor reuse, Turn and cleanup ownership | [Harness lifecycle](../contracts/agents-api/harness-onboarding.md#required-adapter-interfaces) | | `apps/daemon/internal/agent` | Native harness adapters | [Native references](../contracts/agents-api/harness-onboarding.md#native-references) | +| `apps/daemon/internal/agenthost` | Agent-host Sessions: admission, Session directory, Link attachment, views and teardown | [Run in an agent-host view](../contracts/agents-api/harness-onboarding.md#run-in-an-agent-host-view) | | `services/core/internal/sandbox` | Provider interfaces and managed compute lifecycle | [Provider onboarding](sandbox-provider.md) | | `services/web` | Console login and the server-side management proxy | [Console server](web/console-server.md) | | `apps/web` and `packages/agents-client` | Console UI and typed clients | [Web guide](../apps/web/README.md) | From c9d3891c807019da5dc01ef29ad9e73f73689d14 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 04:16:50 +0000 Subject: [PATCH 05/13] Keep MCP credentials and queries off plaintext and Harness URLs The gateway now rejects, before the Session starts, an HTTP MCP binding that injects a bearer token or any HTTP header over a non-https server URL, so no injected value crosses the sandbox's network in plaintext. An MCP listener serves only its server URL's path. The Harness's URL carries no query, since a query may hold a credential; the listener relays each request to exactly the server URL, query included, and refuses a request with a query (400) or for another path (404). model-execution.md states both rules under Credential gateway. --- apps/daemon/internal/gateway/gateway.go | 8 ++-- apps/daemon/internal/gateway/mcp.go | 56 ++++++++++++++---------- apps/daemon/internal/gateway/mcp_test.go | 47 +++++++++++++++++++- contracts/agents-api/model-execution.md | 2 + 4 files changed, 84 insertions(+), 29 deletions(-) diff --git a/apps/daemon/internal/gateway/gateway.go b/apps/daemon/internal/gateway/gateway.go index 37b4c47c..1a759486 100644 --- a/apps/daemon/internal/gateway/gateway.go +++ b/apps/daemon/internal/gateway/gateway.go @@ -10,7 +10,9 @@ // injects the credential. An MCP listener relays to its binding's server and // injects the binding's bearer token and HTTP headers: an environment-origin // binding connects through the sandbox's Network service, a service-origin -// binding from the agent host, and the gateway does the TLS either way. The +// binding from the agent host, and the gateway does the TLS either way. An MCP +// listener serves only its server URL's path, without a query, and relays to +// exactly the server URL; a binding that injects a value needs https. The // generic proxy carries HTTP CONNECT tunnels and plain-HTTP forward requests, // and connects only through the sandbox's Network service. Redirects reach the // Harness unchanged and are never followed. Response header and trailer values @@ -83,7 +85,7 @@ type Endpoints struct { // base URL's path. Models map[string]string // MCP maps each binding's server label to the URL the Harness uses: its - // listener with the server URL's path and query. + // listener with the server URL's path and no query. MCP map[string]string // Proxy is the generic proxy's URL, for HTTP and HTTPS proxy settings, // or empty when Config.Proxy is unset. @@ -176,7 +178,7 @@ const ( type listener struct { role role name string // model name or MCP server label - suffix string // MCP: the server URL's path and query + suffix string // MCP: the server URL's path handler http.Handler } diff --git a/apps/daemon/internal/gateway/mcp.go b/apps/daemon/internal/gateway/mcp.go index 8c61ee38..57cdfae7 100644 --- a/apps/daemon/internal/gateway/mcp.go +++ b/apps/daemon/internal/gateway/mcp.go @@ -13,32 +13,35 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" ) -// mcpRelay serves one MCP HTTP binding: it relays each request to the -// server's origin with the same path and query. When the binding has a bearer -// token or HTTP headers, it replaces the Harness's credential headers and -// same-named headers with them, and withholds each injected value from -// response headers and trailers. +// mcpRelay serves one MCP HTTP binding at its server URL's path and relays +// each request to exactly the server URL, query included. The Harness's URL +// carries no query, since a query may hold a credential, so a request with a +// query or for another path is refused. When the binding has a bearer token or +// HTTP headers, it replaces the Harness's credential headers and same-named +// headers with them, and withholds each injected value from response headers +// and trailers. type mcpRelay struct { - scheme string - host string + upstream url.URL // the binding's server URL + path string // the escaped path the Harness requests inject http.Header // canonical names, values as sent; empty when the binding has none transport http.RoundTripper } -// newMCPRelay returns the binding's handler and the path and query the -// Harness appends to the listener's address. Errors name a header but never -// carry a value. +// newMCPRelay returns the binding's handler and the path the Harness appends +// to the listener's address. A binding with a bearer token or HTTP headers +// needs an https server URL, so no injected value crosses a network in +// plaintext. Errors name a header but never carry a value. func newMCPRelay(b agent.MCPBinding, t http.RoundTripper) (*mcpRelay, string, error) { u, err := url.Parse(b.ServerURL) if err != nil || (u.Scheme != "https" && u.Scheme != "http") || u.Hostname() == "" || u.User != nil || u.Opaque != "" || u.Fragment != "" { return nil, "", errors.New("server URL is not an absolute http or https URL") } - m := &mcpRelay{scheme: u.Scheme, host: u.Host, inject: http.Header{}} + m := &mcpRelay{upstream: *u, path: u.EscapedPath(), inject: http.Header{}} + if m.path == "" { + m.path = "/" + } var secrets []string if b.BearerToken != nil { - if u.Scheme != "https" { - return nil, "", errors.New("a bearer token needs an https server URL") - } token := sentValue(*b.BearerToken) m.inject.Set("Authorization", "Bearer "+token) secrets = append(secrets, token) @@ -55,24 +58,29 @@ func newMCPRelay(b agent.MCPBinding, t http.RoundTripper) (*mcpRelay, string, er m.inject[key] = []string{v} secrets = append(secrets, v) } - m.transport = withhold(t, secrets...) - suffix := u.EscapedPath() - if u.RawQuery != "" || u.ForceQuery { - suffix += "?" + u.RawQuery + if len(m.inject) > 0 && u.Scheme != "https" { + return nil, "", errors.New("a bearer token or HTTP headers need an https server URL") } - return m, suffix, nil + m.transport = withhold(t, secrets...) + return m, u.EscapedPath(), nil } func (m *mcpRelay) ServeHTTP(w http.ResponseWriter, r *http.Request) { - path, query, hasQuery, ok := requestTarget(r) - if !ok { + path, _, hasQuery, ok := requestTarget(r) + switch { + case !ok: http.Error(w, "origin-form request target required", http.StatusBadRequest) return + case hasQuery: + http.Error(w, "the MCP endpoint takes no query", http.StatusBadRequest) + return + case path != m.path: + http.NotFound(w, r) + return } - upstream := &url.URL{Scheme: m.scheme, Host: m.host, Path: r.URL.Path, RawPath: path, - RawQuery: query, ForceQuery: hasQuery && query == ""} reverseProxy(m.transport, func(pr *httputil.ProxyRequest) { - pr.Out.URL = upstream + upstream := m.upstream + pr.Out.URL = &upstream pr.Out.Host = "" if len(m.inject) == 0 { return diff --git a/apps/daemon/internal/gateway/mcp_test.go b/apps/daemon/internal/gateway/mcp_test.go index 09eacc1e..79454e8a 100644 --- a/apps/daemon/internal/gateway/mcp_test.go +++ b/apps/daemon/internal/gateway/mcp_test.go @@ -73,9 +73,52 @@ func TestMCPBrokersBothOrigins(t *testing.T) { stdio.Transport, stdio.ServerURL, stdio.BearerToken, stdio.HTTPHeaders = "stdio", "", nil, nil twice := binding("service") twice.HTTPHeaders = map[string]string{"Authorization": "Basic other"} - for name, b := range map[string]agent.MCPBinding{"stdio": stdio, "Authorization twice": twice} { - if _, err := Plan(Config{MCP: []agent.MCPBinding{b}, Prompt: service}); !errors.Is(err, ErrInvalidConfig) { + // An injected value never crosses a network in plaintext. + plain := binding("environment") + plain.ServerURL, plain.BearerToken, plain.HTTPHeaders = "http://mcp.test/mcp", nil, map[string]string{"X-Api-Key": "header-secret"} + for name, c := range map[string]struct { + b agent.MCPBinding + prompt proto.PromptRequestPayload + }{"stdio": {stdio, service}, "Authorization twice": {twice, service}, "headers over http": {plain, environment}} { + _, err := Plan(Config{MCP: []agent.MCPBinding{c.b}, Prompt: c.prompt, OpenNetwork: sb.open}) + if !errors.Is(err, ErrInvalidConfig) || strings.Contains(err.Error(), "secret") { t.Errorf("Plan with %s: %v", name, err) } } } + +// The Harness's URL carries no query; the listener relays to exactly the +// server URL and refuses any other target. +func TestMCPServesOnlyItsServerURL(t *testing.T) { + seen := make(chan string, 8) + srv := httptest.NewTLSServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + select { + case seen <- r.URL.RequestURI(): + default: + } + })) + defer srv.Close() + b := agent.MCPBinding{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: srv.URL + "/mcp?tenant=a"} + eps := serveOnLoopback(t, Config{MCP: []agent.MCPBinding{b}, Prompt: proto.PromptRequestPayload{DisableExecutionEnvironment: true}, RootCAs: trust(srv)}) + harness := eps.MCP["tools"] + if !strings.HasPrefix(harness, "http://127.0.0.1:") || !strings.HasSuffix(harness, "/mcp") || strings.Contains(harness, "?") { + t.Fatalf("Harness URL %q", harness) + } + base := strings.TrimSuffix(harness, "/mcp") + for _, c := range []struct { + url string + status int + }{{harness, 200}, {harness + "?tenant=b", 400}, {harness + "?", 400}, {base + "/other", 404}, {base + "/mcp/", 404}} { + resp, err := noRedirects.Get(c.url) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + if resp.StatusCode != c.status { + t.Errorf("GET %s: %d, want %d", strings.TrimPrefix(c.url, base), resp.StatusCode, c.status) + } + } + if got := <-seen; got != "/mcp?tenant=a" || len(seen) != 0 { + t.Errorf("the server saw %q and %d more requests", got, len(seen)) + } +} diff --git a/contracts/agents-api/model-execution.md b/contracts/agents-api/model-execution.md index 2a785059..88d8c792 100644 --- a/contracts/agents-api/model-execution.md +++ b/contracts/agents-api/model-execution.md @@ -99,6 +99,8 @@ The Harness reaches its frozen upstream through a Session-local credential gatew - It removes every response header and trailer value that contains the key, in informational responses too. Response bodies pass unchanged, so an upstream that echoes the key in a body discloses it to the Harness. For an HTTP MCP server, the gateway injects the binding's bearer token and HTTP headers in place of the Harness's credential headers and same-named headers, and applies the same rule to each injected value. - It never follows a redirect with the credential. - It never converts between protocols. +- An HTTP MCP binding with a bearer token or HTTP headers needs an `https` server URL. The gateway rejects any other before the Session starts, so no injected value crosses a network in plaintext. +- The Harness's URL for an HTTP MCP binding is its listener with the server URL's path and no query, since a query may hold a credential. The listener relays each request to exactly the server URL, query included. It refuses a request that carries a query with 400 and one for another path with 404. [`internal/modelprovider/config.go`](../../internal/modelprovider/config.go) declares each protocol's routes and credential header, the stripped headers and the placeholder. Its `LookupRoute` matches a request against the routes, and `UpstreamPath` joins a matched route to the upstream base URL. A Harness that calls a route the table does not declare needs a protocol change, not a gateway exception. From c33527c2f727be1aa92c12c9ba029544424e7166 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 04:21:26 +0000 Subject: [PATCH 06/13] Settle agent-host view ends, uid reuse and Session results - Sweep sends SIGKILL, through a pidfd after rereading the uids, to every process whose real, effective, saved or fs uid lies in Config.UIDs, rescans /proc until none remains and returns ErrTeardown after a bound. Allocation skips a uid any running process holds. The package comment states the residual races. - A view's owning handle ends the Session's ownership inside Wait: the gateway stops, a lost world or a world whose Stop failed fails the Session as ErrWorld, and the view slot is freed before the clirunner.Process reports the end, so an immediate relaunch finds the slot free. A world Stop error after a failed sessionview.Start fails the Session too. Run reports every such error. - Run tears down first and decides its result afterwards, so a failure recorded while Executor.Close waits counts. The link owner ignores what the Link reports once it begins closing the attachment. - Admission validates the Open the Session will send with Link encoding, so a zero epoch, an invalid resource kind or an oversized grant is ErrInvalidSession before any effect. --- apps/daemon/internal/agenthost/admit.go | 23 ++- .../internal/agenthost/admit_linux_test.go | 43 ++-- apps/daemon/internal/agenthost/agenthost.go | 2 +- .../agenthost/agenthost_linux_test.go | 27 +++ apps/daemon/internal/agenthost/doc.go | 28 ++- .../daemon/internal/agenthost/launch_linux.go | 142 +++++++++----- apps/daemon/internal/agenthost/link.go | 28 ++- apps/daemon/internal/agenthost/procs_linux.go | 183 ++++++++++++++++++ apps/daemon/internal/agenthost/run_linux.go | 70 +++++-- .../internal/agenthost/session_linux_test.go | 170 ++++++++++++++++ .../internal/agenthost/sessiondir_linux.go | 9 +- .../internal/agenthost/view_linux_test.go | 22 ++- 12 files changed, 638 insertions(+), 109 deletions(-) create mode 100644 apps/daemon/internal/agenthost/procs_linux.go create mode 100644 apps/daemon/internal/agenthost/session_linux_test.go diff --git a/apps/daemon/internal/agenthost/admit.go b/apps/daemon/internal/agenthost/admit.go index dac181cd..c53caa9b 100644 --- a/apps/daemon/internal/agenthost/admit.go +++ b/apps/daemon/internal/agenthost/admit.go @@ -17,7 +17,9 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/gateway" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxfs" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" ) // modelName is the gateway's name for the request's model provider. @@ -43,7 +45,7 @@ func checkConfig(cfg Config) (*x509.CertPool, error) { switch { case !isHostPath(cfg.StateDir): return nil, invalidConfig("state directory %q is not absolute and clean", cfg.StateDir) - case cfg.UIDs.First == 0 || cfg.UIDs.Count == 0 || uint64(cfg.UIDs.First)+uint64(cfg.UIDs.Count) > math.MaxUint32: + case !cfg.UIDs.valid(): return nil, invalidConfig("uid range %d+%d", cfg.UIDs.First, cfg.UIDs.Count) case sandboxlink.CheckRelayURL(cfg.RelayURL) != nil: return nil, invalidConfig("relay URL") @@ -204,15 +206,13 @@ func checkLayout(cfg Config, view agent.View) error { return nil } +// checkSession checks the Session's own fields. Its binding is valid when the +// Open the Session sends is, as Link encoding checks it. func checkSession(s Session) error { - b := s.Binding - r := b.Resource - switch { - case r.TenantID.IsZero() || r.EnvironmentID.IsZero() || r.ID.IsZero() || r.Generation == 0: - return invalidSession("resource") - case b.AttachmentID.IsZero() || b.SessionID.IsZero() || b.AssignmentID.IsZero() || len(b.AttachGrant) == 0: - return invalidSession("binding") - case s.Input == nil || s.Output == nil: + if _, err := sandboxlink.Encode(1, s.Binding.open(sandboxlink.ServiceFile, sandboxfs.Version, sandboxwire.ID{})); err != nil { + return invalidSession("binding: %v", err) + } + if s.Input == nil || s.Output == nil { return invalidSession("no input or output channel") } for _, env := range []map[string]string{s.Environment.Sandbox, s.Environment.Tool} { @@ -225,6 +225,11 @@ func checkSession(s Session) error { return nil } +// valid reports whether r is a nonempty range of nonzero uids. +func (r UIDRange) valid() bool { + return r.First != 0 && r.Count != 0 && uint64(r.First)+uint64(r.Count) <= math.MaxUint32 +} + func isHostPath(p string) bool { return filepath.IsAbs(p) && filepath.Clean(p) == p && !strings.ContainsRune(p, 0) } diff --git a/apps/daemon/internal/agenthost/admit_linux_test.go b/apps/daemon/internal/agenthost/admit_linux_test.go index 9e3962d2..679457ba 100644 --- a/apps/daemon/internal/agenthost/admit_linux_test.go +++ b/apps/daemon/internal/agenthost/admit_linux_test.go @@ -19,6 +19,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/internal/agentplugin" "github.com/MiniMax-AI/OpenAgentCore/internal/modelprovider" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" ) var errFactory = errors.New("factory reached") @@ -83,7 +84,7 @@ func TestAdmissionRejectsBeforeAnyEffect(t *testing.T) { c.change(&req) s, _, _ := newSession(newResource(), req) var dials atomic.Int32 - err := run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }}) + err := run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }, procs: &fakeProcesses{}}) for _, want := range c.want { if !errors.Is(err, want) { t.Errorf("%s: Run = %v, want %v", name, err, want) @@ -96,17 +97,34 @@ func TestAdmissionRejectsBeforeAnyEffect(t *testing.T) { t.Errorf("%s: the sessions directory exists", name) } } + // A binding that Link encoding refuses is refused before any effect. + for name, change := range map[string]func(*Binding){ + "zero assignment epoch": func(b *Binding) { b.AssignmentEpoch = 0 }, + "invalid resource kind": func(b *Binding) { b.Resource.Kind = 0 }, + "oversized attach grant": func(b *Binding) { b.AttachGrant = make([]byte, sandboxlink.MaxGrantBytes+1) }, + } { + s, _, _ := newSession(newResource(), request("viewed", "/workspace", "https://model.test", "sk-test")) + change(&s.Binding) + var dials atomic.Int32 + err := run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }, procs: &fakeProcesses{}}) + if !errors.Is(err, ErrInvalidSession) || dials.Load() != 0 { + t.Errorf("%s: Run = %v after %d dials, want ErrInvalidSession", name, err, dials.Load()) + } + if _, err := os.Stat(sessionsDir(f.cfg.StateDir)); !errors.Is(err, os.ErrNotExist) { + t.Errorf("%s: the sessions directory exists", name) + } + } } func TestViewExecutorReceivesTheGatewayRequest(t *testing.T) { f := newViewFixture(t) bearer := "mcp-secret" req := request("viewed", "/workspace", "https://model.test", "sk-test") - req.MCPHTTPServers = &[]proto.MCPHTTPServer{{ConnectionOrigin: "environment", ServerLabel: "docs", ServerURL: "https://mcp.test/docs", BearerToken: &bearer}} + req.MCPHTTPServers = &[]proto.MCPHTTPServer{{ConnectionOrigin: "environment", ServerLabel: "docs", ServerURL: "https://mcp.test/docs?tenant=a", BearerToken: &bearer}} original := maps.Clone(req.AgentOptions) s, _, _ := newSession(newResource(), req) var dials atomic.Int32 - err := run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }}) + err := run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }, procs: &fakeProcesses{}}) if !errors.Is(err, ErrExecutor) || !errors.Is(err, errFactory) { t.Fatalf("Run = %v, want the factory's error as ErrExecutor", err) } @@ -140,7 +158,7 @@ func TestViewExecutorReceivesTheGatewayRequest(t *testing.T) { req.AgentOptions["mcp_servers"] = map[string]any{} s, _, _ = newSession(newResource(), req) f.req = proto.PromptRequestPayload{} - err = run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }}) + err = run(context.Background(), f.cfg, s, deps{dial: countingDial(&dials), broker: func() processBroker { return noBroker{} }, procs: &fakeProcesses{}}) if !errors.Is(err, ErrUnsupported) || !errors.Is(err, agent.ErrViewHandoff) || f.req.AgentKind != "" { t.Errorf("Run with a connection option = %v, want ErrUnsupported and ErrViewHandoff before the adapter", err) } @@ -148,20 +166,3 @@ func TestViewExecutorReceivesTheGatewayRequest(t *testing.T) { t.Errorf("%d dials and %d Session directories after Run", dials.Load(), len(leftSessions(t, f.cfg))) } } - -func TestSweepRemovesLeftoverSessions(t *testing.T) { - cfg := Config{StateDir: t.TempDir()} - if err := Sweep(cfg); err != nil { - t.Fatalf("Sweep without sessions: %v", err) - } - left := filepath.Join(sessionsDir(cfg.StateDir), "left", "home") - if err := os.MkdirAll(left, 0o700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(left, "history"), []byte("x"), 0o600); err != nil { - t.Fatal(err) - } - if err := Sweep(cfg); err != nil || len(leftSessions(t, cfg)) != 0 { - t.Fatalf("Sweep = %v, %d Session directories left", err, len(leftSessions(t, cfg))) - } -} diff --git a/apps/daemon/internal/agenthost/agenthost.go b/apps/daemon/internal/agenthost/agenthost.go index 20008fff..0828db72 100644 --- a/apps/daemon/internal/agenthost/agenthost.go +++ b/apps/daemon/internal/agenthost/agenthost.go @@ -20,7 +20,7 @@ type Config struct { StateDir string // UIDs is the range Session uids are allocated from; each Session's gid // equals its uid. Only one agent host runs per kernel, and nothing else - // uses the range. + // uses the range: Sweep kills every process that holds one of its uids. UIDs UIDRange // RelayURL and TLS reach the Link relay, as sandboxlink.DialAttach takes // them. A nil TLS uses the system roots. diff --git a/apps/daemon/internal/agenthost/agenthost_linux_test.go b/apps/daemon/internal/agenthost/agenthost_linux_test.go index 8dee3a0f..08f4cd40 100644 --- a/apps/daemon/internal/agenthost/agenthost_linux_test.go +++ b/apps/daemon/internal/agenthost/agenthost_linux_test.go @@ -9,6 +9,8 @@ import ( "errors" "os" "path/filepath" + "slices" + "sync" "sync/atomic" "testing" @@ -108,3 +110,28 @@ func leftSessions(t *testing.T, cfg Config) []os.DirEntry { } return entries } + +// fakeProcesses is a process table whose processes end when killed, unless +// they are stubborn. +type fakeProcesses struct { + mu sync.Mutex + procs []hostProcess + stubborn bool + killed []int +} + +func (f *fakeProcesses) list() ([]hostProcess, error) { + f.mu.Lock() + defer f.mu.Unlock() + return slices.Clone(f.procs), nil +} + +func (f *fakeProcesses) kill(p hostProcess, r UIDRange) error { + f.mu.Lock() + defer f.mu.Unlock() + f.killed = append(f.killed, p.pid) + if !f.stubborn { + f.procs = slices.DeleteFunc(f.procs, func(q hostProcess) bool { return q.pid == p.pid }) + } + return nil +} diff --git a/apps/daemon/internal/agenthost/doc.go b/apps/daemon/internal/agenthost/doc.go index a1934920..ece326fa 100644 --- a/apps/daemon/internal/agenthost/doc.go +++ b/apps/daemon/internal/agenthost/doc.go @@ -22,11 +22,29 @@ // The agent host owns the Session's Link attachment: it opens each stream // with the Session's binding, renews the lease and fails the Session when the // relay closes the attachment, a Link request fails in a way that is not -// retryable, or the world is lost or may still hold state. A failure cancels -// the running Turn and closes the live view. Teardown releases, in order, the -// Executor, the view, the process broker, the Link attachment, the Session -// directory and the uid. Sweep removes the Session directories a previous -// agent host left; Run's owner calls it at startup. +// retryable, or the world is lost or did not stop cleanly, which leaves what +// the attachment holds uncertain. A failure cancels the running Turn and +// closes the live view. A view's end is settled before its clirunner.Process +// reports it: the gateway has stopped, the world's end is recorded and the +// view slot is free. Teardown releases, in order, the Executor, the view, the +// process broker, the Link attachment, the Session directory and the uid; +// Run decides its result only afterwards, so a failure recorded during +// teardown counts, and from the close of the attachment on, what the Link +// reports changes nothing. +// +// Sweep runs at startup, before any Session. It sends SIGKILL to every +// process whose real, effective, saved or file-system uid lies in +// Config.UIDs, scans /proc again until none remains or a bound passes, and +// then removes the Session directories a previous agent host left. Allocation +// also skips a uid that a running process holds. A process in the range runs +// with no capabilities and no_new_privs, so it can only fork more processes +// of its own uid: each scan finds what the previous round's processes +// started, and a uid that no process holds at allocation stays free until the +// Session starts one. The signal goes through a pidfd and only after the +// uids are read again, so a pid reused since the scan is never signalled. A +// zombie runs nothing and is ignored. A process that the agent host's /proc +// does not show, such as one in a sibling PID namespace, is outside these +// guarantees, which is why nothing else may use the range. // // The Harness view protocol is in contracts/agents-api/harness-onboarding.md // and the gateway's in contracts/agents-api/model-execution.md. diff --git a/apps/daemon/internal/agenthost/launch_linux.go b/apps/daemon/internal/agenthost/launch_linux.go index d516bb3e..a87ebb1c 100644 --- a/apps/daemon/internal/agenthost/launch_linux.go +++ b/apps/daemon/internal/agenthost/launch_linux.go @@ -9,6 +9,7 @@ import ( "io" "os" "slices" + "sync" "syscall" "time" @@ -26,15 +27,29 @@ const worldExport sandboxlink.ExportID = "world" // liveView is the Session's one live view slot. type liveView struct { - view *sessionview.View // nil while the view is being built - closed bool // the Session ended while the view was being built + view runningView // nil while the view is being built + closed bool // the Session ended while the view was being built +} + +// runningView is the part of *sessionview.View the Session owns. +type runningView interface { + Signal(syscall.Signal) error + Wait() (sessionview.Exit, error) + Close() error +} + +// viewWorld is the part of *worldfs.World the Session watches. +type viewWorld interface { + Stop() error + Lost() <-chan struct{} + Err() error } // closeLive closes the live view. It runs when the Session's context ends. func (s *session) closeLive() { s.mu.Lock() lv := s.live - var v *sessionview.View + var v runningView if lv != nil { lv.closed, v = true, lv.view } @@ -73,11 +88,7 @@ func (s *session) launch(opts clirunner.StartOptions) (*clirunner.Process, error s.live = lv s.views.Add(1) s.mu.Unlock() - p, err := s.start(lv, opts) - if err != nil { - s.release(lv) - } - return p, err + return s.start(lv, opts) } // release frees the view slot and ends the launch's count. @@ -90,15 +101,20 @@ func (s *session) release(lv *liveView) { s.views.Done() } +// start builds the view for lv. Until the view runs, each failure releases +// lv; from then on the view's owner does. func (s *session) start(lv *liveView, opts clirunner.StartOptions) (*clirunner.Process, error) { if err := s.startBroker(opts.KillTimeout); err != nil { + s.release(lv) return nil, err } if err := s.dir.chownHome(s.uid); err != nil { + s.release(lv) return nil, &Error{Kind: ErrLaunch, Op: "home", Err: err} } ends, err := newStdio(opts.NeedStdin) if err != nil { + s.release(lv) return nil, &Error{Kind: ErrLaunch, Op: "stdio", Err: err} } // The gateway serves until the view has ended. @@ -116,52 +132,100 @@ func (s *session) start(lv *liveView, opts clirunner.StartOptions) (*clirunner.P if err != nil { stopGateway() ends.closeParent() - if errors.Is(err, worldfs.ErrAttachmentDirty) { - // Only ending the attachment releases what the world may hold. - err = &Error{Kind: ErrWorld, Op: "launch", Err: err} - s.fail(err) - return nil, err + defer s.release(lv) + // sessionview stops a world that served; Stop reports how that went. + if serr := world.Stop(); serr != nil || errors.Is(err, worldfs.ErrAttachmentDirty) { + return nil, s.worldEnded("launch", errors.Join(err, serr)) } return nil, &Error{Kind: ErrLaunch, Err: err} } p := v.Presentation() s.log.Info("agent host view started", "binary", opts.Binary, "targets", p.Targets, "links", p.Links, "synthesized", p.Synthesized) + return s.own(lv, v, world, stopGateway, opts, ends) +} +// own hands a started view to the clirunner.Process the adapter receives. +func (s *session) own(lv *liveView, v runningView, world viewWorld, stopGateway func(), opts clirunner.StartOptions, ends *stdio) (*clirunner.Process, error) { + h := &ownedView{s: s, lv: lv, v: v, world: world, stopGateway: stopGateway, ended: make(chan struct{}), watched: make(chan struct{})} + go h.watch() s.mu.Lock() lv.view = v closed := lv.closed s.mu.Unlock() if closed { v.Close() - stopGateway() + h.Wait() ends.closeParent() return nil, &Error{Kind: ErrLaunch, Err: errors.New("the Session is ending")} } - process, err := clirunner.FromHandle(viewHandle{v}, clirunner.HandleOptions{Parent: opts.Parent, Stdin: ends.stdin(), + process, err := clirunner.FromHandle(h, clirunner.HandleOptions{Parent: opts.Parent, Stdin: ends.stdin(), Stdout: ends.parent[1], Stderr: ends.parent[2], KillTimeout: opts.KillTimeout}) if err != nil { v.Close() - stopGateway() + h.Wait() ends.closeParent() return nil, &Error{Kind: ErrLaunch, Err: err} } - ended := make(chan struct{}) - go func() { - select { - case <-world.Lost(): - s.fail(&Error{Kind: ErrWorld, Op: "world", Err: world.Err()}) - case <-ended: - } - }() - go func() { - defer s.release(lv) - _, _ = v.Wait() - stopGateway() - close(ended) - }() return process, nil } +// ownedView is a running view as a clirunner.Handle. Its Wait ends the +// Session's ownership of the view before it returns, so the end the adapter +// observes through the Process comes after it: the gateway has stopped, a +// lost world or one that did not stop cleanly has failed the Session, and +// the view slot is free for the next Launch. +type ownedView struct { + s *session + lv *liveView + v runningView + world viewWorld + stopGateway func() + ended chan struct{} // closed once the view has ended + watched chan struct{} // closed when watch returns + once sync.Once +} + +// watch fails the Session as soon as the world is lost while the view runs. +func (h *ownedView) watch() { + defer close(h.watched) + select { + case <-h.world.Lost(): + h.s.fail(&Error{Kind: ErrWorld, Op: "world", Err: h.world.Err()}) + case <-h.ended: + } +} + +func (h *ownedView) Signal(sig syscall.Signal) error { return h.v.Signal(sig) } + +func (h *ownedView) Close() error { return h.v.Close() } + +func (h *ownedView) Wait() (int, error) { + exit, err := h.v.Wait() + h.once.Do(h.end) + switch { + case err != nil: + return -1, err + case exit.Signal != 0: + return -1, nil + } + return exit.Code, nil +} + +// end releases the view once it has ended and its world has stopped. +func (h *ownedView) end() { + h.stopGateway() + close(h.ended) + <-h.watched + if lost := h.world.Err(); lost != nil { + h.s.fail(&Error{Kind: ErrWorld, Op: "world", Err: lost}) + } + // The view has stopped its world; Stop reports how that went. + if err := h.world.Stop(); err != nil { + h.s.worldEnded("stop world", err) + } + h.s.release(h.lv) +} + // startBroker starts the Session's process broker at its first launch. func (s *session) startBroker(grace time.Duration) error { s.brokerMu.Lock() @@ -285,21 +349,3 @@ func closeFiles(files []*os.File) { } } } - -// viewHandle is a view as a clirunner.Handle. -type viewHandle struct{ v *sessionview.View } - -func (h viewHandle) Signal(sig syscall.Signal) error { return h.v.Signal(sig) } - -func (h viewHandle) Wait() (int, error) { - exit, err := h.v.Wait() - switch { - case err != nil: - return -1, err - case exit.Signal != 0: - return -1, nil - } - return exit.Code, nil -} - -func (h viewHandle) Close() error { return h.v.Close() } diff --git a/apps/daemon/internal/agenthost/link.go b/apps/daemon/internal/agenthost/link.go index 257aec48..51ca9396 100644 --- a/apps/daemon/internal/agenthost/link.go +++ b/apps/daemon/internal/agenthost/link.go @@ -65,6 +65,13 @@ func newLinkOwner(dial dialFunc, b Binding, fail func(error)) *linkOwner { return &linkOwner{dial: dial, binding: b, fail: fail} } +// open is the Open that carries b for service. +func (b Binding) open(service sandboxlink.Service, version uint16, expected sandboxwire.ID) sandboxlink.Open { + return sandboxlink.Open{Service: service, Version: version, Resource: b.Resource, ExpectedServerInstanceID: expected, + AttachmentID: b.AttachmentID, SessionID: b.SessionID, AssignmentID: b.AssignmentID, AssignmentEpoch: b.AssignmentEpoch, + AttachGrant: b.AttachGrant} +} + // current returns the live link, dialing a new one when there is none. func (l *linkOwner) current(ctx context.Context) (attachLink, error) { l.dialMu.Lock() @@ -110,10 +117,7 @@ func (l *linkOwner) open(ctx context.Context, service sandboxlink.Service, versi l.opened = true expected := l.instance l.mu.Unlock() - b := l.binding - st, opened, err := link.OpenService(ctx, sandboxlink.Open{Service: service, Version: version, Resource: b.Resource, - ExpectedServerInstanceID: expected, AttachmentID: b.AttachmentID, SessionID: b.SessionID, - AssignmentID: b.AssignmentID, AssignmentEpoch: b.AssignmentEpoch, AttachGrant: b.AttachGrant}) + st, opened, err := link.OpenService(ctx, l.binding.open(service, version, expected)) if err != nil { return nil, l.observe("open "+service.String(), err) } @@ -138,11 +142,21 @@ func (l *linkOwner) open(ctx context.Context, service sandboxlink.Service, versi func (l *linkOwner) observe(op string, err error) error { err = &Error{Kind: ErrLink, Op: op, Err: err} if !retryable(err) { - l.fail(err) + l.report(err) } return err } +// report fails the Session with err unless close has begun: from then on the +// Session's own close of the attachment explains whatever the Link reports. +func (l *linkOwner) report(err error) { + l.mu.Lock() + defer l.mu.Unlock() + if !l.closing { + l.fail(err) + } +} + // retryable reports whether a failed Link request may succeed later: a Link // failure whose code says so, or a transport failure. func retryable(err error) bool { @@ -153,7 +167,7 @@ func retryable(err error) bool { // closed is the link's OnAttachmentClosed. It never blocks. func (l *linkOwner) closed(c sandboxlink.AttachmentClosed) { if c.AttachmentID == l.binding.AttachmentID { - l.fail(&Error{Kind: ErrLink, Op: "attachment", Err: fmt.Errorf("the relay closed the attachment (reason %d)", c.Reason)}) + l.report(&Error{Kind: ErrLink, Op: "attachment", Err: fmt.Errorf("the relay closed the attachment (reason %d)", c.Reason)}) } } @@ -174,7 +188,7 @@ func (l *linkOwner) renew(ctx context.Context) { } for { if !time.Now().Before(lease) { - l.fail(&Error{Kind: ErrLink, Op: "renew", Err: sandboxlink.LeaseExpired}) + l.report(&Error{Kind: ErrLink, Op: "renew", Err: sandboxlink.LeaseExpired}) return } attempt, cancel := context.WithDeadline(ctx, lease) diff --git a/apps/daemon/internal/agenthost/procs_linux.go b/apps/daemon/internal/agenthost/procs_linux.go new file mode 100644 index 00000000..e1bd0d87 --- /dev/null +++ b/apps/daemon/internal/agenthost/procs_linux.go @@ -0,0 +1,183 @@ +//go:build linux + +package agenthost + +import ( + "bufio" + "bytes" + "errors" + "fmt" + "io/fs" + "os" + "strconv" + "strings" + "time" + + "golang.org/x/sys/unix" +) + +const ( + // sweepBound bounds how long Sweep waits for the processes it killed. + sweepBound = 10 * time.Second + // sweepPoll is the pause between Sweep's scans. + sweepPoll = 50 * time.Millisecond +) + +// hostProcess is one process /proc lists, with its real, effective, saved and +// file-system uids. +type hostProcess struct { + pid int + uids [4]uint32 +} + +// in reports whether p holds a uid in r. +func (p hostProcess) in(r UIDRange) bool { + for _, id := range p.uids { + if id >= r.First && id-r.First < r.Count { + return true + } + } + return false +} + +// processTable lists and kills the host's processes. Tests replace it. +type processTable interface { + // list returns every process that still runs; a zombie runs nothing and + // is left out. + list() ([]hostProcess, error) + // kill sends SIGKILL to the process p names when it still holds a uid in + // r, and never to a process that reused p's pid. + kill(p hostProcess, r UIDRange) error +} + +// heldUIDs returns the uids in r that a running process holds. +func heldUIDs(procs processTable, r UIDRange) (map[uint32]bool, error) { + list, err := procs.list() + if err != nil { + return nil, err + } + held := map[uint32]bool{} + for _, p := range list { + for _, id := range p.uids { + if id >= r.First && id-r.First < r.Count { + held[id] = true + } + } + } + return held, nil +} + +// endProcesses kills every process that holds a uid in r, scanning again +// until none remains or bound passes. A process in r can only fork processes +// of its own uid, so each scan finds what the previous round's processes +// started. +func endProcesses(procs processTable, r UIDRange, bound time.Duration) error { + deadline := time.Now().Add(bound) + for { + list, err := procs.list() + if err != nil { + return err + } + var held []hostProcess + for _, p := range list { + if p.in(r) { + held = append(held, p) + } + } + if len(held) == 0 { + return nil + } + if !time.Now().Before(deadline) { + return fmt.Errorf("%d processes still hold Session uids after %s", len(held), bound) + } + for _, p := range held { + if err := procs.kill(p, r); err != nil { + return err + } + } + time.Sleep(sweepPoll) + } +} + +// procfs is the host's /proc. +type procfs struct{} + +func (procfs) list() ([]hostProcess, error) { + entries, err := os.ReadDir("/proc") + if err != nil { + return nil, err + } + var list []hostProcess + for _, e := range entries { + pid, err := strconv.Atoi(e.Name()) + if err != nil || pid <= 0 { + continue + } + p, running, err := readProcess(pid) + if err != nil { + return nil, err + } + if running { + list = append(list, p) + } + } + return list, nil +} + +func (procfs) kill(p hostProcess, r UIDRange) error { + fd, err := unix.PidfdOpen(p.pid, 0) + if errors.Is(err, unix.ESRCH) { + return nil + } + if err != nil { + return fmt.Errorf("pidfd of %d: %w", p.pid, err) + } + defer unix.Close(fd) + // The pid may name another process since the scan. The uids read after + // the pidfd opened are its process's while it runs; once it has ended, + // the signal reaches nothing. + q, running, err := readProcess(p.pid) + if err != nil || !running || !q.in(r) { + return err + } + if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) { + return fmt.Errorf("kill %d: %w", p.pid, err) + } + return nil +} + +// readProcess reads pid's uids from /proc//status. running is false when +// the process has ended or is a zombie. +func readProcess(pid int) (p hostProcess, running bool, err error) { + data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/status") + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ESRCH) { + return p, false, nil + } + if err != nil { + return p, false, err + } + p.pid = pid + var state string + var uids []string + sc := bufio.NewScanner(bytes.NewReader(data)) + for sc.Scan() { + key, value, _ := strings.Cut(sc.Text(), ":") + switch key { + case "State": + state = strings.TrimSpace(value) + case "Uid": + uids = strings.Fields(value) + } + } + if len(uids) != 4 { + return p, false, fmt.Errorf("/proc/%d/status has no uids", pid) + } + for i, s := range uids { + id, err := strconv.ParseUint(s, 10, 32) + if err != nil { + return p, false, fmt.Errorf("/proc/%d/status uid %q", pid, s) + } + p.uids[i] = uint32(id) + } + return p, !strings.HasPrefix(state, "Z") && !strings.HasPrefix(state, "X"), nil +} diff --git a/apps/daemon/internal/agenthost/run_linux.go b/apps/daemon/internal/agenthost/run_linux.go index 87a68942..9e65ee35 100644 --- a/apps/daemon/internal/agenthost/run_linux.go +++ b/apps/daemon/internal/agenthost/run_linux.go @@ -35,21 +35,35 @@ const ( type deps struct { dial dialFunc broker func() processBroker + procs processTable } // Run runs one Session until Input is closed, ctx ends or the Session fails, // then tears it down. It returns nil after Input closed and every Turn // settled, ctx's error when ctx ended the Session, and otherwise the error -// that ended it, joined with any teardown failure. +// that ended it, joined with any view cleanup and teardown failure. A failure +// recorded during teardown counts. func Run(ctx context.Context, cfg Config, s Session) error { - return run(ctx, cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return unavailableBroker{} }}) + return run(ctx, cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return unavailableBroker{} }, procs: procfs{}}) } -// Sweep removes every Session directory under cfg.StateDir. Run's owner calls -// it at startup, before any Session runs. +// Sweep ends every process that holds a uid in cfg.UIDs, then removes every +// Session directory under cfg.StateDir. Run's owner calls it at startup, +// before any Session runs. It returns ErrTeardown when a process still holds +// a Session uid after a bounded wait. func Sweep(cfg Config) error { - if !isHostPath(cfg.StateDir) { + return sweep(cfg, procfs{}, sweepBound) +} + +func sweep(cfg Config, procs processTable, bound time.Duration) error { + switch { + case !isHostPath(cfg.StateDir): return invalidConfig("state directory %q is not absolute and clean", cfg.StateDir) + case !cfg.UIDs.valid(): + return invalidConfig("uid range %d+%d", cfg.UIDs.First, cfg.UIDs.Count) + } + if err := endProcesses(procs, cfg.UIDs, bound); err != nil { + return &Error{Kind: ErrTeardown, Op: "sweep processes", Err: err} } dir := sessionsDir(cfg.StateDir) entries, err := os.ReadDir(dir) @@ -85,7 +99,8 @@ type session struct { cancel context.CancelFunc failMu sync.Mutex - failure error + failure error // the first failure, which ended the Session + cleanup []error // each world that did not stop cleanly mu sync.Mutex // live is the one view that may run; nil when none does. @@ -112,7 +127,7 @@ func run(ctx context.Context, cfg Config, in Session, d deps) error { if s.plan, err = admit(cfg, roots, in, s.openNetwork); err != nil { return err } - if s.uid, err = allocUID(cfg.UIDs); err != nil { + if s.uid, err = allocUID(cfg.UIDs, d.procs); err != nil { return err } if s.dir, err = createSessionDir(cfg.StateDir, in.Binding.SessionID, s.uid); err != nil { @@ -132,13 +147,31 @@ func run(ctx context.Context, cfg Config, in Session, d deps) error { } else { err = s.drive(exec) } - // The Session's failure and the end of ctx explain whatever followed them. - if failure := s.failed(); failure != nil { + return s.finish(exec, err, ctx.Err()) +} + +// finish tears the Session down and only then decides its result, so a +// failure recorded during teardown counts: the Session's first failure, else +// ended, the end of Run's ctx, else err. Teardown has joined every view and +// stopped the link owner's reports, so nothing changes the result later. +func (s *session) finish(exec agent.Executor, err, ended error) error { + terr := s.teardown(exec) + s.failMu.Lock() + failure, cleanup := s.failure, s.cleanup + s.failMu.Unlock() + switch { + case failure != nil: err = failure - } else if ctx.Err() != nil { - err = ctx.Err() + case ended != nil: + err = ended } - return errors.Join(err, s.teardown(exec)) + errs := []error{err} + for _, c := range cleanup { + if c != failure { + errs = append(errs, c) + } + } + return errors.Join(append(errs, terr)...) } func executorError(err error) error { @@ -159,10 +192,17 @@ func (s *session) fail(err error) { s.cancel() } -func (s *session) failed() error { +// worldEnded records a world that did not stop cleanly, or that cannot show +// that its attachment holds nothing. Only ending the attachment settles its +// state, so the Session fails, and Run reports the error even after another +// failure. +func (s *session) worldEnded(op string, err error) error { + e := &Error{Kind: ErrWorld, Op: op, Err: err} s.failMu.Lock() - defer s.failMu.Unlock() - return s.failure + s.cleanup = append(s.cleanup, e) + s.failMu.Unlock() + s.fail(e) + return e } // drive runs each Turn from Input in order until Input is closed, a Turn diff --git a/apps/daemon/internal/agenthost/session_linux_test.go b/apps/daemon/internal/agenthost/session_linux_test.go new file mode 100644 index 00000000..f13154c7 --- /dev/null +++ b/apps/daemon/internal/agenthost/session_linux_test.go @@ -0,0 +1,170 @@ +//go:build linux + +package agenthost + +import ( + "context" + "errors" + "log/slog" + "os" + "path/filepath" + "reflect" + "sync" + "syscall" + "testing" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent/clirunner" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/sessionview" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +func TestSweepEndsSessionProcessesBeforeRemovingDirectories(t *testing.T) { + cfg := Config{StateDir: t.TempDir(), UIDs: UIDRange{First: 70000, Count: 8}} + if err := sweep(cfg, &fakeProcesses{}, time.Second); err != nil { + t.Fatalf("Sweep without sessions: %v", err) + } + leave := func() { + left := filepath.Join(sessionsDir(cfg.StateDir), "left", "home") + if err := os.MkdirAll(left, 0o700); err != nil { + t.Fatal(err) + } + } + leave() + // Any of the four uids places a process in the range. + procs := &fakeProcesses{procs: []hostProcess{{pid: 10, uids: [4]uint32{1000, 1000, 1000, 70003}}, {pid: 11, uids: [4]uint32{1000, 1000, 1000, 1000}}}} + if err := sweep(cfg, procs, time.Second); err != nil || !reflect.DeepEqual(procs.killed, []int{10}) || len(leftSessions(t, cfg)) != 0 { + t.Fatalf("Sweep = %v, killed %v, %d Session directories left", err, procs.killed, len(leftSessions(t, cfg))) + } + leave() + procs = &fakeProcesses{procs: []hostProcess{{pid: 12, uids: [4]uint32{70000, 70000, 70000, 70000}}}, stubborn: true} + if err := sweep(cfg, procs, 100*time.Millisecond); !errors.Is(err, ErrTeardown) || len(leftSessions(t, cfg)) != 1 { + t.Fatalf("Sweep with a process that outlives the bound = %v, %d Session directories left", err, len(leftSessions(t, cfg))) + } +} + +func TestAllocationSkipsUIDsThatProcessesHold(t *testing.T) { + r := UIDRange{First: 71000, Count: 2} + procs := &fakeProcesses{procs: []hostProcess{{pid: 10, uids: [4]uint32{1000, 71000, 1000, 1000}}}} + id, err := allocUID(r, procs) + if err != nil || id != 71001 { + t.Fatalf("allocUID = %d, %v; want 71001", id, err) + } + defer freeUID(id) + if _, err := allocUID(r, procs); !errors.Is(err, ErrCapacity) { + t.Fatalf("allocUID with every uid taken = %v", err) + } + // The host's table shows this process with its own uids. + self := uint32(os.Getuid()) + if held, err := heldUIDs(procfs{}, UIDRange{First: self, Count: 1}); err != nil || !held[self] { + t.Fatalf("/proc shows uid %d held: %v, %v", self, held[self], err) + } +} + +func TestViewEndReleasesTheSlotBeforeTheProcessEnds(t *testing.T) { + errDetach := errors.New("detach failed") + for name, stop := range map[string]error{"clean world": nil, "failed detach": errDetach} { + s := newOwnerSession(t) + lv := &liveView{} + s.live = lv + s.views.Add(1) + ends, err := newStdio(false) + if err != nil { + t.Fatal(err) + } + ends.closeChild() + v := &fakeView{exit: make(chan struct{})} + p, err := s.own(lv, v, fakeWorld{stop: stop}, func() {}, clirunner.StartOptions{Parent: context.Background(), KillTimeout: time.Second}, ends) + if err != nil { + t.Fatal(err) + } + v.Close() + _ = p.Wait() + s.mu.Lock() + live := s.live + s.mu.Unlock() + if live != nil { + t.Errorf("%s: the view slot is taken after Process.Wait", name) + } + failed := s.ctx.Err() != nil + err = s.finish(nil, nil, nil) + switch { + case stop == nil && (err != nil || failed): + t.Errorf("%s: Run = %v, Session failed %v", name, err, failed) + case stop != nil && (!errors.Is(err, ErrWorld) || !errors.Is(err, errDetach) || !failed): + t.Errorf("%s: Run = %v, Session failed %v; want ErrWorld with the detach error", name, err, failed) + } + } +} + +func TestFailureDuringTeardownCounts(t *testing.T) { + s := newOwnerSession(t) + // The relay revokes the attachment while Executor.Close waits. + revoke := func() { s.link.closed(sandboxlink.AttachmentClosed{AttachmentID: s.link.binding.AttachmentID}) } + if err := s.finish(closingExecutor(revoke), nil, nil); !errors.Is(err, ErrLink) { + t.Fatalf("Run = %v, want the revocation", err) + } + // Once the link owner closes the attachment, the Link's reports explain nothing. + s = newOwnerSession(t) + if err := s.finish(nil, nil, nil); err != nil { + t.Fatal(err) + } + s.link.closed(sandboxlink.AttachmentClosed{AttachmentID: s.link.binding.AttachmentID}) + if s.ctx.Err() == nil { + t.Fatal("teardown left the Session's context live") + } + s.failMu.Lock() + defer s.failMu.Unlock() + if s.failure != nil { + t.Fatalf("a report after close failed the Session: %v", s.failure) + } +} + +// newOwnerSession is a Session with no directory, uid or link. +func newOwnerSession(t *testing.T) *session { + s := &session{log: slog.New(slog.DiscardHandler)} + s.ctx, s.cancel = context.WithCancel(context.Background()) + t.Cleanup(s.cancel) + s.link = newLinkOwner(nil, Binding{AttachmentID: sandboxwire.NewID()}, s.fail) + return s +} + +// fakeView is a view that ends when closed. +type fakeView struct { + exit chan struct{} + once sync.Once +} + +func (v *fakeView) Signal(syscall.Signal) error { return nil } + +func (v *fakeView) Wait() (sessionview.Exit, error) { + <-v.exit + return sessionview.Exit{}, nil +} + +func (v *fakeView) Close() error { + v.once.Do(func() { close(v.exit) }) + return nil +} + +// fakeWorld is a world that is never lost and whose Stop returns stop. +type fakeWorld struct{ stop error } + +func (w fakeWorld) Stop() error { return w.stop } +func (fakeWorld) Lost() <-chan struct{} { return nil } +func (fakeWorld) Err() error { return nil } + +// closingExecutor runs itself when closed. +type closingExecutor func() + +func (closingExecutor) StartTurn(context.Context, string, proto.MessageInput, chan<- proto.Envelope) (agent.Turn, error) { + return nil, errors.New("no Turns") +} + +func (e closingExecutor) Close(context.Context) error { + e() + return nil +} diff --git a/apps/daemon/internal/agenthost/sessiondir_linux.go b/apps/daemon/internal/agenthost/sessiondir_linux.go index 7fe9f4b7..42d1ae8c 100644 --- a/apps/daemon/internal/agenthost/sessiondir_linux.go +++ b/apps/daemon/internal/agenthost/sessiondir_linux.go @@ -21,11 +21,16 @@ var uids = struct { used map[uint32]bool }{used: map[uint32]bool{}} -func allocUID(r UIDRange) (uint32, error) { +// allocUID returns a uid in r that no Session uses and no process holds. +func allocUID(r UIDRange, procs processTable) (uint32, error) { + held, err := heldUIDs(procs, r) + if err != nil { + return 0, &Error{Kind: ErrInvalidConfig, Op: "processes", Err: err} + } uids.Lock() defer uids.Unlock() for i := range r.Count { - if id := r.First + i; !uids.used[id] { + if id := r.First + i; !uids.used[id] && !held[id] { uids.used[id] = true return id, nil } diff --git a/apps/daemon/internal/agenthost/view_linux_test.go b/apps/daemon/internal/agenthost/view_linux_test.go index d6eedad6..81b5e9eb 100644 --- a/apps/daemon/internal/agenthost/view_linux_test.go +++ b/apps/daemon/internal/agenthost/view_linux_test.go @@ -171,6 +171,26 @@ func TestSessionRunsInAViewOverItsAttachment(t *testing.T) { } checkReleased(t, cfg) }) + + t.Run("Sweep ends a lingering Session process", func(t *testing.T) { + id := cfg.UIDs.First + 1 + cmd := exec.Command("/bin/sleep", "60") + cmd.SysProcAttr = &syscall.SysProcAttr{Credential: &syscall.Credential{Uid: id, Gid: id}} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + if err := Sweep(cfg); err != nil { + t.Fatalf("Sweep = %v", err) + } + select { + case <-done: + case <-time.After(wait): + cmd.Process.Kill() + t.Fatal("the process with a Session uid still runs after Sweep") + } + }) } // sandbox is a relay and the oac-sandbox-io serving its one resource. @@ -298,7 +318,7 @@ func startSession(t *testing.T, cfg Config, sb *sandbox, req proto.PromptRequest sb.grant(s.Binding, cfg.RuntimeID, lease) r := &sessionRun{binding: s.Binding, in: in, out: out, done: make(chan error, 1)} go func() { - r.done <- run(context.Background(), cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return noBroker{} }}) + r.done <- run(context.Background(), cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return noBroker{} }, procs: procfs{}}) }() return r } From 41803b1e7da101a1aef93f6a5fdfd92436e14bea Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 04:39:58 +0000 Subject: [PATCH 07/13] Treat an MCP server URL's query as a credential in the gateway A binding whose server URL has a query now needs https, like one that injects a bearer token or headers, and the gateway withholds the query and each parameter value, as sent and decoded, from response headers and trailers. A server URL with userinfo gets its own error. --- apps/daemon/internal/gateway/gateway.go | 11 +++--- apps/daemon/internal/gateway/mcp.go | 47 +++++++++++++++++++----- apps/daemon/internal/gateway/mcp_test.go | 24 +++++++++--- contracts/agents-api/model-execution.md | 4 +- 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/apps/daemon/internal/gateway/gateway.go b/apps/daemon/internal/gateway/gateway.go index 1a759486..702cb553 100644 --- a/apps/daemon/internal/gateway/gateway.go +++ b/apps/daemon/internal/gateway/gateway.go @@ -12,11 +12,12 @@ // binding connects through the sandbox's Network service, a service-origin // binding from the agent host, and the gateway does the TLS either way. An MCP // listener serves only its server URL's path, without a query, and relays to -// exactly the server URL; a binding that injects a value needs https. The -// generic proxy carries HTTP CONNECT tunnels and plain-HTTP forward requests, -// and connects only through the sandbox's Network service. Redirects reach the -// Harness unchanged and are never followed. Response header and trailer values -// that contain an injected credential or header value are withheld; bodies +// exactly the server URL. The server URL's query is a credential: a binding +// with a query or an injected value needs https. The generic proxy carries +// HTTP CONNECT tunnels and plain-HTTP forward requests, and connects only +// through the sandbox's Network service. Redirects reach the Harness unchanged +// and are never followed. Response header and trailer values that contain an +// injected credential, header value or MCP query value are withheld; bodies // pass unchanged. The end of the Session closes every connection, tunnels and // upgraded ones included. The gateway logs nothing. package gateway diff --git a/apps/daemon/internal/gateway/mcp.go b/apps/daemon/internal/gateway/mcp.go index 57cdfae7..49369407 100644 --- a/apps/daemon/internal/gateway/mcp.go +++ b/apps/daemon/internal/gateway/mcp.go @@ -7,6 +7,7 @@ import ( "net/http/httputil" "net/url" "slices" + "strings" "golang.org/x/net/http/httpguts" @@ -14,10 +15,11 @@ import ( ) // mcpRelay serves one MCP HTTP binding at its server URL's path and relays -// each request to exactly the server URL, query included. The Harness's URL -// carries no query, since a query may hold a credential, so a request with a -// query or for another path is refused. When the binding has a bearer token or -// HTTP headers, it replaces the Harness's credential headers and same-named +// each request to exactly the server URL, query included. The server URL's +// query is credential material: the Harness's URL carries none, so a request +// with a query or for another path is refused, and response headers and +// trailers never carry it. When the binding has a bearer token or HTTP +// headers, it replaces the Harness's credential headers and same-named // headers with them, and withholds each injected value from response headers // and trailers. type mcpRelay struct { @@ -28,13 +30,17 @@ type mcpRelay struct { } // newMCPRelay returns the binding's handler and the path the Harness appends -// to the listener's address. A binding with a bearer token or HTTP headers -// needs an https server URL, so no injected value crosses a network in -// plaintext. Errors name a header but never carry a value. +// to the listener's address. A binding with a bearer token, HTTP headers or a +// query needs an https server URL, so no credential crosses a network in +// plaintext, and a server URL with userinfo is rejected. Errors name a header +// but never carry a value. func newMCPRelay(b agent.MCPBinding, t http.RoundTripper) (*mcpRelay, string, error) { u, err := url.Parse(b.ServerURL) - if err != nil || (u.Scheme != "https" && u.Scheme != "http") || u.Hostname() == "" || u.User != nil || u.Opaque != "" || u.Fragment != "" { + switch { + case err != nil || (u.Scheme != "https" && u.Scheme != "http") || u.Hostname() == "" || u.Opaque != "" || u.Fragment != "": return nil, "", errors.New("server URL is not an absolute http or https URL") + case u.User != nil: + return nil, "", errors.New("server URL carries userinfo; use a bearer token or HTTP headers") } m := &mcpRelay{upstream: *u, path: u.EscapedPath(), inject: http.Header{}} if m.path == "" { @@ -58,13 +64,34 @@ func newMCPRelay(b agent.MCPBinding, t http.RoundTripper) (*mcpRelay, string, er m.inject[key] = []string{v} secrets = append(secrets, v) } - if len(m.inject) > 0 && u.Scheme != "https" { - return nil, "", errors.New("a bearer token or HTTP headers need an https server URL") + if u.RawQuery != "" { + secrets = append(secrets, queryValues(u.RawQuery)...) + } + if len(secrets) > 0 && u.Scheme != "https" { + return nil, "", errors.New("a bearer token, HTTP headers or a query need an https server URL") } m.transport = withhold(t, secrets...) return m, u.EscapedPath(), nil } +// queryValues returns what a raw query discloses: the query itself and each +// parameter's value, as sent and decoded. A parameter without "=" is its own +// value. +func queryValues(raw string) []string { + values := []string{raw} + for _, param := range strings.Split(raw, "&") { + _, v, ok := strings.Cut(param, "=") + if !ok { + v = param + } + values = append(values, v) + if d, err := url.QueryUnescape(v); err == nil && d != v { + values = append(values, d) + } + } + return values +} + func (m *mcpRelay) ServeHTTP(w http.ResponseWriter, r *http.Request) { path, _, hasQuery, ok := requestTarget(r) switch { diff --git a/apps/daemon/internal/gateway/mcp_test.go b/apps/daemon/internal/gateway/mcp_test.go index 79454e8a..3c3c14de 100644 --- a/apps/daemon/internal/gateway/mcp_test.go +++ b/apps/daemon/internal/gateway/mcp_test.go @@ -73,13 +73,17 @@ func TestMCPBrokersBothOrigins(t *testing.T) { stdio.Transport, stdio.ServerURL, stdio.BearerToken, stdio.HTTPHeaders = "stdio", "", nil, nil twice := binding("service") twice.HTTPHeaders = map[string]string{"Authorization": "Basic other"} - // An injected value never crosses a network in plaintext. + // No credential crosses a network in plaintext, and userinfo is none. plain := binding("environment") plain.ServerURL, plain.BearerToken, plain.HTTPHeaders = "http://mcp.test/mcp", nil, map[string]string{"X-Api-Key": "header-secret"} + query := binding("environment") + query.ServerURL, query.BearerToken, query.HTTPHeaders = "http://mcp.test/mcp?api_key=query-secret", nil, nil + userinfo := binding("service") + userinfo.ServerURL = "https://user:info-secret@mcp.test/mcp" for name, c := range map[string]struct { b agent.MCPBinding prompt proto.PromptRequestPayload - }{"stdio": {stdio, service}, "Authorization twice": {twice, service}, "headers over http": {plain, environment}} { + }{"stdio": {stdio, service}, "Authorization twice": {twice, service}, "headers over http": {plain, environment}, "a query over http": {query, environment}, "userinfo": {userinfo, service}} { _, err := Plan(Config{MCP: []agent.MCPBinding{c.b}, Prompt: c.prompt, OpenNetwork: sb.open}) if !errors.Is(err, ErrInvalidConfig) || strings.Contains(err.Error(), "secret") { t.Errorf("Plan with %s: %v", name, err) @@ -88,17 +92,22 @@ func TestMCPBrokersBothOrigins(t *testing.T) { } // The Harness's URL carries no query; the listener relays to exactly the -// server URL and refuses any other target. +// server URL, refuses any other target and keeps the query out of response +// headers. func TestMCPServesOnlyItsServerURL(t *testing.T) { seen := make(chan string, 8) - srv := httptest.NewTLSServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { select { case seen <- r.URL.RequestURI(): default: } + w.Header().Set("Location", r.URL.RequestURI()) + w.Header().Set("X-Key", r.URL.Query().Get("key")) + w.Header().Set("X-Plain", "visible") })) defer srv.Close() - b := agent.MCPBinding{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: srv.URL + "/mcp?tenant=a"} + const query = "tenant=acme&key=query%2Bsecret" + b := agent.MCPBinding{ConnectionOrigin: "service", ServerLabel: "tools", Transport: "http", ServerURL: srv.URL + "/mcp?" + query} eps := serveOnLoopback(t, Config{MCP: []agent.MCPBinding{b}, Prompt: proto.PromptRequestPayload{DisableExecutionEnvironment: true}, RootCAs: trust(srv)}) harness := eps.MCP["tools"] if !strings.HasPrefix(harness, "http://127.0.0.1:") || !strings.HasSuffix(harness, "/mcp") || strings.Contains(harness, "?") { @@ -117,8 +126,11 @@ func TestMCPServesOnlyItsServerURL(t *testing.T) { if resp.StatusCode != c.status { t.Errorf("GET %s: %d, want %d", strings.TrimPrefix(c.url, base), resp.StatusCode, c.status) } + if c.status == 200 && (resp.Header.Get("Location") != "" || resp.Header.Get("X-Key") != "" || resp.Header.Get("X-Plain") != "visible") { + t.Errorf("response headers %v", resp.Header) + } } - if got := <-seen; got != "/mcp?tenant=a" || len(seen) != 0 { + if got := <-seen; got != "/mcp?"+query || len(seen) != 0 { t.Errorf("the server saw %q and %d more requests", got, len(seen)) } } diff --git a/contracts/agents-api/model-execution.md b/contracts/agents-api/model-execution.md index 88d8c792..29ff3fa4 100644 --- a/contracts/agents-api/model-execution.md +++ b/contracts/agents-api/model-execution.md @@ -99,8 +99,8 @@ The Harness reaches its frozen upstream through a Session-local credential gatew - It removes every response header and trailer value that contains the key, in informational responses too. Response bodies pass unchanged, so an upstream that echoes the key in a body discloses it to the Harness. For an HTTP MCP server, the gateway injects the binding's bearer token and HTTP headers in place of the Harness's credential headers and same-named headers, and applies the same rule to each injected value. - It never follows a redirect with the credential. - It never converts between protocols. -- An HTTP MCP binding with a bearer token or HTTP headers needs an `https` server URL. The gateway rejects any other before the Session starts, so no injected value crosses a network in plaintext. -- The Harness's URL for an HTTP MCP binding is its listener with the server URL's path and no query, since a query may hold a credential. The listener relays each request to exactly the server URL, query included. It refuses a request that carries a query with 400 and one for another path with 404. +- An HTTP MCP server URL's query is a credential too. A binding with a bearer token, HTTP headers or a query needs an `https` server URL, and a server URL with userinfo is rejected. The gateway rejects any other before the Session starts, so no credential crosses a network in plaintext. +- The Harness's URL for an HTTP MCP binding is its listener with the server URL's path and no query. The listener relays each request to exactly the server URL, query included. It refuses a request that carries a query with 400 and one for another path with 404. It removes every response header and trailer value that contains the query or one of its parameter values, as sent or decoded, so a short value also withholds any header value that contains it. [`internal/modelprovider/config.go`](../../internal/modelprovider/config.go) declares each protocol's routes and credential header, the stripped headers and the placeholder. Its `LookupRoute` matches a request against the routes, and `UpstreamPath` joins a matched route to the upstream base URL. A Harness that calls a route the table does not declare needs a protocol change, not a gateway exception. From d4d98c19210b0ec17e1904b97d390981e6016928 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 04:47:03 +0000 Subject: [PATCH 08/13] End leftover Session processes with their view's PID namespace Sweep and uid allocation now scan every thread, so a process whose leader thread is a zombie still counts. Sweep ends a task in a view by killing its PID namespace's init, found by walking up the task's ancestors with pidfds, so a process forked while the scan runs dies with the namespace. A task in the agent host's own namespace is killed directly and reported as ErrTeardown if it survives the bound. --- apps/daemon/internal/agenthost/agenthost.go | 3 +- .../agenthost/agenthost_linux_test.go | 20 +- apps/daemon/internal/agenthost/doc.go | 33 ++- apps/daemon/internal/agenthost/procs_linux.go | 272 +++++++++++++----- apps/daemon/internal/agenthost/run_linux.go | 9 +- .../internal/agenthost/session_linux_test.go | 14 +- .../internal/agenthost/view_linux_test.go | 151 ++++++++++ 7 files changed, 398 insertions(+), 104 deletions(-) diff --git a/apps/daemon/internal/agenthost/agenthost.go b/apps/daemon/internal/agenthost/agenthost.go index 0828db72..95f83b31 100644 --- a/apps/daemon/internal/agenthost/agenthost.go +++ b/apps/daemon/internal/agenthost/agenthost.go @@ -20,7 +20,8 @@ type Config struct { StateDir string // UIDs is the range Session uids are allocated from; each Session's gid // equals its uid. Only one agent host runs per kernel, and nothing else - // uses the range: Sweep kills every process that holds one of its uids. + // uses the range: Sweep kills every process that holds one of its uids, + // with its whole PID namespace when that is not the agent host's. UIDs UIDRange // RelayURL and TLS reach the Link relay, as sandboxlink.DialAttach takes // them. A nil TLS uses the system roots. diff --git a/apps/daemon/internal/agenthost/agenthost_linux_test.go b/apps/daemon/internal/agenthost/agenthost_linux_test.go index 08f4cd40..a4c64e99 100644 --- a/apps/daemon/internal/agenthost/agenthost_linux_test.go +++ b/apps/daemon/internal/agenthost/agenthost_linux_test.go @@ -24,12 +24,16 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" ) -// The test binary is also the privileged suite's Harness inside the view. +// The test binary is also the privileged suite's Harness inside the view and +// its process with a zombie leader. func TestMain(m *testing.M) { sessionview.Init() if os.Getenv(harnessEnv) != "" { os.Exit(runHarness(os.Args[1:])) } + if os.Getenv(zombieLeaderEnv) != "" { + runZombieLeader() + } os.Exit(m.Run()) } @@ -115,23 +119,23 @@ func leftSessions(t *testing.T, cfg Config) []os.DirEntry { // they are stubborn. type fakeProcesses struct { mu sync.Mutex - procs []hostProcess + list []task stubborn bool - killed []int + ended []int } -func (f *fakeProcesses) list() ([]hostProcess, error) { +func (f *fakeProcesses) tasks() ([]task, error) { f.mu.Lock() defer f.mu.Unlock() - return slices.Clone(f.procs), nil + return slices.Clone(f.list), nil } -func (f *fakeProcesses) kill(p hostProcess, r UIDRange) error { +func (f *fakeProcesses) end(t task, r UIDRange) error { f.mu.Lock() defer f.mu.Unlock() - f.killed = append(f.killed, p.pid) + f.ended = append(f.ended, t.tgid) if !f.stubborn { - f.procs = slices.DeleteFunc(f.procs, func(q hostProcess) bool { return q.pid == p.pid }) + f.list = slices.DeleteFunc(f.list, func(q task) bool { return q.tgid == t.tgid }) } return nil } diff --git a/apps/daemon/internal/agenthost/doc.go b/apps/daemon/internal/agenthost/doc.go index ece326fa..1d95d205 100644 --- a/apps/daemon/internal/agenthost/doc.go +++ b/apps/daemon/internal/agenthost/doc.go @@ -32,19 +32,26 @@ // teardown counts, and from the close of the attachment on, what the Link // reports changes nothing. // -// Sweep runs at startup, before any Session. It sends SIGKILL to every -// process whose real, effective, saved or file-system uid lies in -// Config.UIDs, scans /proc again until none remains or a bound passes, and -// then removes the Session directories a previous agent host left. Allocation -// also skips a uid that a running process holds. A process in the range runs -// with no capabilities and no_new_privs, so it can only fork more processes -// of its own uid: each scan finds what the previous round's processes -// started, and a uid that no process holds at allocation stays free until the -// Session starts one. The signal goes through a pidfd and only after the -// uids are read again, so a pid reused since the scan is never signalled. A -// zombie runs nothing and is ignored. A process that the agent host's /proc -// does not show, such as one in a sibling PID namespace, is outside these -// guarantees, which is why nothing else may use the range. +// Sweep runs at startup, before any Session. It ends every task, each thread +// of each process, whose real, effective, saved or file-system uid lies in +// Config.UIDs, scans /proc again until none runs or a bound passes, and only +// then removes the Session directories a previous agent host left. +// Allocation also skips a uid that any running task holds. Session processes +// run only in a view's PID namespace, so Sweep ends a view's task by killing +// the namespace's init: the kernel then kills every process in the +// namespace, and nothing can fork into it any more. The init is the task's +// nearest ancestor whose pid in its namespace is 1; no process in a view can +// enter another namespace, so each ancestor up to it shares the namespace. +// Each step of the walk pins the parent with a pidfd and confirms that the +// child still has that parent, and each signal goes through a pidfd, so a +// reused pid is never followed or signalled. A task with a Session uid in +// the agent host's own namespace is not a Session's: Sweep kills its process +// and returns ErrTeardown if one still runs after the bound. A scan misses a +// process only while a parent that exits at once forks it; the view still +// ends once any of its tasks is caught, and a view's launcher ends the view +// when the agent host that started it goes away. A task that the agent +// host's /proc does not show, such as one in a sibling PID namespace, is +// outside these guarantees, which is why nothing else may use the range. // // The Harness view protocol is in contracts/agents-api/harness-onboarding.md // and the gateway's in contracts/agents-api/model-execution.md. diff --git a/apps/daemon/internal/agenthost/procs_linux.go b/apps/daemon/internal/agenthost/procs_linux.go index e1bd0d87..f56525ca 100644 --- a/apps/daemon/internal/agenthost/procs_linux.go +++ b/apps/daemon/internal/agenthost/procs_linux.go @@ -23,43 +23,43 @@ const ( sweepPoll = 50 * time.Millisecond ) -// hostProcess is one process /proc lists, with its real, effective, saved and -// file-system uids. -type hostProcess struct { - pid int - uids [4]uint32 +// task is one running task, a thread of a process, as /proc lists it. +type task struct { + tgid, tid int + uids [4]uint32 // real, effective, saved and file-system } -// in reports whether p holds a uid in r. -func (p hostProcess) in(r UIDRange) bool { - for _, id := range p.uids { - if id >= r.First && id-r.First < r.Count { - return true - } - } - return false +// in reports whether t holds a uid in r. +func (t task) in(r UIDRange) bool { return holds(t.uids, r) } + +func holds(uids [4]uint32, r UIDRange) bool { + return r.has(uids[0]) || r.has(uids[1]) || r.has(uids[2]) || r.has(uids[3]) } -// processTable lists and kills the host's processes. Tests replace it. +func (r UIDRange) has(id uint32) bool { return id >= r.First && id-r.First < r.Count } + +// processTable lists the host's tasks and ends them. Tests replace it. type processTable interface { - // list returns every process that still runs; a zombie runs nothing and - // is left out. - list() ([]hostProcess, error) - // kill sends SIGKILL to the process p names when it still holds a uid in - // r, and never to a process that reused p's pid. - kill(p hostProcess, r UIDRange) error + // tasks returns every task that runs, each thread of each process; a + // zombie runs nothing and is left out. + tasks() ([]task, error) + // end kills, while t still holds a uid in r, the init of t's PID + // namespace when that is a view's, which ends every process in it, and + // t's process when t is in the agent host's own namespace. It never + // signals a process that reused a pid. + end(t task, r UIDRange) error } -// heldUIDs returns the uids in r that a running process holds. +// heldUIDs returns the uids in r that a running task holds. func heldUIDs(procs processTable, r UIDRange) (map[uint32]bool, error) { - list, err := procs.list() + tasks, err := procs.tasks() if err != nil { return nil, err } held := map[uint32]bool{} - for _, p := range list { - for _, id := range p.uids { - if id >= r.First && id-r.First < r.Count { + for _, t := range tasks { + for _, id := range t.uids { + if r.has(id) { held[id] = true } } @@ -67,31 +67,34 @@ func heldUIDs(procs processTable, r UIDRange) (map[uint32]bool, error) { return held, nil } -// endProcesses kills every process that holds a uid in r, scanning again -// until none remains or bound passes. A process in r can only fork processes -// of its own uid, so each scan finds what the previous round's processes -// started. +// endProcesses ends every task that holds a uid in r and scans again until +// none runs or bound passes. func endProcesses(procs processTable, r UIDRange, bound time.Duration) error { deadline := time.Now().Add(bound) for { - list, err := procs.list() + tasks, err := procs.tasks() if err != nil { return err } - var held []hostProcess - for _, p := range list { - if p.in(r) { - held = append(held, p) + var held []task + for _, t := range tasks { + if t.in(r) { + held = append(held, t) } } if len(held) == 0 { return nil } if !time.Now().Before(deadline) { - return fmt.Errorf("%d processes still hold Session uids after %s", len(held), bound) + return fmt.Errorf("%d tasks still hold Session uids after %s", len(held), bound) } - for _, p := range held { - if err := procs.kill(p, r); err != nil { + ended := map[int]bool{} + for _, t := range held { + if ended[t.tgid] { + continue + } + ended[t.tgid] = true + if err := procs.end(t, r); err != nil { return err } } @@ -99,85 +102,210 @@ func endProcesses(procs processTable, r UIDRange, bound time.Duration) error { } } -// procfs is the host's /proc. +// procfs is the host's /proc, which must be the agent host's own PID +// namespace's. type procfs struct{} -func (procfs) list() ([]hostProcess, error) { - entries, err := os.ReadDir("/proc") +// errGone is a process or task that has ended. +var errGone = errors.New("ended") + +func (procfs) tasks() ([]task, error) { + self, err := readStatus("/proc/self/status") + if err != nil { + return nil, err + } + if len(self.nspid) != 1 { + return nil, errors.New("/proc is not the agent host's PID namespace's") + } + pids, err := os.ReadDir("/proc") if err != nil { return nil, err } - var list []hostProcess - for _, e := range entries { - pid, err := strconv.Atoi(e.Name()) + var list []task + for _, p := range pids { + pid, err := strconv.Atoi(p.Name()) if err != nil || pid <= 0 { continue } - p, running, err := readProcess(pid) + tids, err := os.ReadDir(procPath(pid, "task")) + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ESRCH) { + continue + } if err != nil { return nil, err } - if running { - list = append(list, p) + for _, e := range tids { + tid, err := strconv.Atoi(e.Name()) + if err != nil { + continue + } + s, err := readStatus(procPath(pid, "task", e.Name(), "status")) + if errors.Is(err, errGone) { + continue + } + if err != nil { + return nil, err + } + if s.running() { + list = append(list, task{tgid: pid, tid: tid, uids: s.uids}) + } } } return list, nil } -func (procfs) kill(p hostProcess, r UIDRange) error { - fd, err := unix.PidfdOpen(p.pid, 0) +func (procfs) end(t task, r UIDRange) error { + fd, err := unix.PidfdOpen(t.tgid, 0) if errors.Is(err, unix.ESRCH) { return nil } if err != nil { - return fmt.Errorf("pidfd of %d: %w", p.pid, err) + return fmt.Errorf("pidfd of %d: %w", t.tgid, err) } - defer unix.Close(fd) - // The pid may name another process since the scan. The uids read after - // the pidfd opened are its process's while it runs; once it has ended, - // the signal reaches nothing. - q, running, err := readProcess(p.pid) - if err != nil || !running || !q.in(r) { + // The pid may name another process since the scan. What /proc shows under + // it belongs to the process fd pins while that process exists, which each + // read confirms afterwards; once it has ended, the signal reaches nothing. + s, err := readStatus(procPath(t.tgid, "task", strconv.Itoa(t.tid), "status")) + if err == nil { + err = exists(fd) + } + if err == nil && (!s.running() || !holds(s.uids, r)) { + err = errGone + } + if err == nil && len(s.nspid) > 1 { + fd, err = namespaceInit(fd, t.tgid, len(s.nspid)) + } + if errors.Is(err, errGone) { + return nil + } + if err != nil { return err } + defer unix.Close(fd) if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) { - return fmt.Errorf("kill %d: %w", p.pid, err) + return fmt.Errorf("kill: %w", err) } return nil } -// readProcess reads pid's uids from /proc//status. running is false when -// the process has ended or is a zombie. -func readProcess(pid int) (p hostProcess, running bool, err error) { - data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/status") +// namespaceInit takes fd, the pidfd of process pid, and returns a pidfd of the +// init of its PID namespace, depth namespaces below /proc's. On an error it +// closes every pidfd. The init is the process's nearest ancestor whose pid in +// its own namespace is 1: no process in a view can enter another namespace, +// so every ancestor up to the init shares the namespace. Each step pins the +// parent and then confirms that the child still exists and still has that +// parent, so the walk never follows a reused pid. +func namespaceInit(fd, pid, depth int) (int, error) { + for { + s, err := readStatus(procPath(pid, "status")) + if err == nil { + err = exists(fd) + } + if err == nil && (len(s.nspid) != depth || s.ppid <= 0) { + err = fmt.Errorf("process %d is outside its view's PID namespace", pid) + } + if err != nil { + unix.Close(fd) + return -1, err + } + if s.nspid[depth-1] == 1 { + return fd, nil + } + parent, err := unix.PidfdOpen(s.ppid, 0) + if errors.Is(err, unix.ESRCH) { + continue // the parent has ended and the process has a new one + } + if err != nil { + unix.Close(fd) + return -1, fmt.Errorf("pidfd of %d: %w", s.ppid, err) + } + again, err := readStatus(procPath(pid, "status")) + if err == nil { + err = exists(fd) + } + if err != nil { + unix.Close(parent) + unix.Close(fd) + return -1, err + } + if again.ppid != s.ppid { + unix.Close(parent) + continue + } + unix.Close(fd) + fd, pid = parent, s.ppid + } +} + +// exists returns nil while the process fd pins exists, as a zombie too, and +// errGone once it has been reaped. +func exists(fd int) error { + err := unix.PidfdSendSignal(fd, 0, nil, 0) + if errors.Is(err, unix.ESRCH) { + return errGone + } + return err +} + +func procPath(pid int, name ...string) string { + return "/proc/" + strconv.Itoa(pid) + "/" + strings.Join(name, "/") +} + +// status is what a status file in /proc reports. +type status struct { + state string + ppid int + uids [4]uint32 + nspid []int // the pid in each PID namespace, from /proc's to the task's own +} + +func (s status) running() bool { + return !strings.HasPrefix(s.state, "Z") && !strings.HasPrefix(s.state, "X") +} + +// readStatus reads a status file in /proc. It returns errGone when the +// process or task has ended. +func readStatus(path string) (status, error) { + var s status + data, err := os.ReadFile(path) if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ESRCH) { - return p, false, nil + return s, errGone } if err != nil { - return p, false, err + return s, err } - p.pid = pid - var state string var uids []string sc := bufio.NewScanner(bytes.NewReader(data)) for sc.Scan() { key, value, _ := strings.Cut(sc.Text(), ":") switch key { case "State": - state = strings.TrimSpace(value) + s.state = strings.TrimSpace(value) + case "PPid": + if s.ppid, err = strconv.Atoi(strings.TrimSpace(value)); err != nil { + return s, fmt.Errorf("%s: PPid %q", path, value) + } case "Uid": uids = strings.Fields(value) + case "NSpid": + for _, f := range strings.Fields(value) { + n, err := strconv.Atoi(f) + if err != nil { + return s, fmt.Errorf("%s: NSpid %q", path, value) + } + s.nspid = append(s.nspid, n) + } } } - if len(uids) != 4 { - return p, false, fmt.Errorf("/proc/%d/status has no uids", pid) + if len(uids) != 4 || len(s.nspid) == 0 { + return s, fmt.Errorf("%s has no uids or NSpid", path) } - for i, s := range uids { - id, err := strconv.ParseUint(s, 10, 32) + for i, f := range uids { + id, err := strconv.ParseUint(f, 10, 32) if err != nil { - return p, false, fmt.Errorf("/proc/%d/status uid %q", pid, s) + return s, fmt.Errorf("%s: uid %q", path, f) } - p.uids[i] = uint32(id) + s.uids[i] = uint32(id) } - return p, !strings.HasPrefix(state, "Z") && !strings.HasPrefix(state, "X"), nil + return s, nil } diff --git a/apps/daemon/internal/agenthost/run_linux.go b/apps/daemon/internal/agenthost/run_linux.go index 9e65ee35..63b3c158 100644 --- a/apps/daemon/internal/agenthost/run_linux.go +++ b/apps/daemon/internal/agenthost/run_linux.go @@ -47,10 +47,11 @@ func Run(ctx context.Context, cfg Config, s Session) error { return run(ctx, cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return unavailableBroker{} }, procs: procfs{}}) } -// Sweep ends every process that holds a uid in cfg.UIDs, then removes every -// Session directory under cfg.StateDir. Run's owner calls it at startup, -// before any Session runs. It returns ErrTeardown when a process still holds -// a Session uid after a bounded wait. +// Sweep ends every process that holds a uid in cfg.UIDs, with its view's PID +// namespace, then removes every Session directory under cfg.StateDir. Run's +// owner calls it at startup, before any Session runs, with /proc showing the +// agent host's own PID namespace. It returns ErrTeardown when a task still +// holds a Session uid after a bounded wait. func Sweep(cfg Config) error { return sweep(cfg, procfs{}, sweepBound) } diff --git a/apps/daemon/internal/agenthost/session_linux_test.go b/apps/daemon/internal/agenthost/session_linux_test.go index f13154c7..f36e306b 100644 --- a/apps/daemon/internal/agenthost/session_linux_test.go +++ b/apps/daemon/internal/agenthost/session_linux_test.go @@ -34,13 +34,14 @@ func TestSweepEndsSessionProcessesBeforeRemovingDirectories(t *testing.T) { } } leave() - // Any of the four uids places a process in the range. - procs := &fakeProcesses{procs: []hostProcess{{pid: 10, uids: [4]uint32{1000, 1000, 1000, 70003}}, {pid: 11, uids: [4]uint32{1000, 1000, 1000, 1000}}}} - if err := sweep(cfg, procs, time.Second); err != nil || !reflect.DeepEqual(procs.killed, []int{10}) || len(leftSessions(t, cfg)) != 0 { - t.Fatalf("Sweep = %v, killed %v, %d Session directories left", err, procs.killed, len(leftSessions(t, cfg))) + // Any of the four uids of any thread places a process in the range. + outside := [4]uint32{1000, 1000, 1000, 1000} + procs := &fakeProcesses{list: []task{{tgid: 10, tid: 10, uids: outside}, {tgid: 10, tid: 12, uids: [4]uint32{1000, 1000, 1000, 70003}}, {tgid: 11, tid: 11, uids: outside}}} + if err := sweep(cfg, procs, time.Second); err != nil || !reflect.DeepEqual(procs.ended, []int{10}) || len(leftSessions(t, cfg)) != 0 { + t.Fatalf("Sweep = %v, ended %v, %d Session directories left", err, procs.ended, len(leftSessions(t, cfg))) } leave() - procs = &fakeProcesses{procs: []hostProcess{{pid: 12, uids: [4]uint32{70000, 70000, 70000, 70000}}}, stubborn: true} + procs = &fakeProcesses{list: []task{{tgid: 12, tid: 12, uids: [4]uint32{70000, 70000, 70000, 70000}}}, stubborn: true} if err := sweep(cfg, procs, 100*time.Millisecond); !errors.Is(err, ErrTeardown) || len(leftSessions(t, cfg)) != 1 { t.Fatalf("Sweep with a process that outlives the bound = %v, %d Session directories left", err, len(leftSessions(t, cfg))) } @@ -48,7 +49,8 @@ func TestSweepEndsSessionProcessesBeforeRemovingDirectories(t *testing.T) { func TestAllocationSkipsUIDsThatProcessesHold(t *testing.T) { r := UIDRange{First: 71000, Count: 2} - procs := &fakeProcesses{procs: []hostProcess{{pid: 10, uids: [4]uint32{1000, 71000, 1000, 1000}}}} + // A thread holds the uid; its process's leader does not. + procs := &fakeProcesses{list: []task{{tgid: 10, tid: 10, uids: [4]uint32{1000, 1000, 1000, 1000}}, {tgid: 10, tid: 11, uids: [4]uint32{1000, 71000, 1000, 1000}}}} id, err := allocUID(r, procs) if err != nil || id != 71001 { t.Fatalf("allocUID = %d, %v; want 71001", id, err) diff --git a/apps/daemon/internal/agenthost/view_linux_test.go b/apps/daemon/internal/agenthost/view_linux_test.go index 81b5e9eb..29819f4d 100644 --- a/apps/daemon/internal/agenthost/view_linux_test.go +++ b/apps/daemon/internal/agenthost/view_linux_test.go @@ -17,6 +17,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strconv" "strings" "syscall" @@ -24,6 +25,7 @@ import ( "time" "github.com/google/uuid" + "golang.org/x/sys/unix" "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent/clirunner" @@ -191,6 +193,155 @@ func TestSessionRunsInAViewOverItsAttachment(t *testing.T) { t.Fatal("the process with a Session uid still runs after Sweep") } }) + + t.Run("Sweep ends a view whose leader thread has exited", func(t *testing.T) { + id := cfg.UIDs.First + 2 + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + cmd := inPIDNamespace(id, exe) + cmd.Env = append(os.Environ(), zombieLeaderEnv+"=1") + done := startView(t, cmd) + until(t, "a zombie leader with a running thread", func() bool { return zombieLeaderHolds(id) }) + if got, err := allocUID(UIDRange{First: id, Count: 1}, procfs{}); !errors.Is(err, ErrCapacity) { + freeUID(got) + t.Errorf("allocUID beside a running thread = %d, %v", got, err) + } + sweepEnds(t, cfg, id, done) + }) + + t.Run("Sweep ends a view whose processes fork", func(t *testing.T) { + id := cfg.UIDs.First + 3 + // Each subshell forks a sleep and exits at once. + done := startView(t, inPIDNamespace(id, "/bin/sh", "-c", "while :; do (sleep 60 &); sleep 0.01; done")) + until(t, "forked processes", func() bool { return processesHolding(id) >= 3 }) + sweepEnds(t, cfg, id, done) + }) +} + +// inPIDNamespace returns a command that runs argv with uid under a root init +// in a new PID namespace, as a view does. The init starts argv again whenever +// it ends, so only ending the namespace ends it. +func inPIDNamespace(uid uint32, argv ...string) *exec.Cmd { + script := `while :; do setpriv --reuid="$0" --regid="$0" --clear-groups -- "$@"; done` + cmd := exec.Command("/bin/sh", append([]string{"-c", script, strconv.Itoa(int(uid))}, argv...)...) + cmd.SysProcAttr = &syscall.SysProcAttr{Cloneflags: syscall.CLONE_NEWPID} + return cmd +} + +// startView starts cmd and returns its end. +func startView(t *testing.T, cmd *exec.Cmd) <-chan error { + t.Helper() + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + t.Cleanup(func() { cmd.Process.Kill() }) + return done +} + +// sweepEnds checks that Sweep ends the view whose init done reports and every +// process that holds id. +func sweepEnds(t *testing.T, cfg Config, id uint32, done <-chan error) { + t.Helper() + if err := Sweep(cfg); err != nil { + t.Fatalf("Sweep = %v", err) + } + select { + case <-done: + case <-time.After(wait): + t.Fatal("the view's init still runs after Sweep") + } + if n := processesHolding(id); n != 0 { + t.Errorf("%d processes hold uid %d after Sweep", n, id) + } +} + +func until(t *testing.T, what string, ok func() bool) { + t.Helper() + deadline := time.Now().Add(wait) + for !ok() { + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for %s", what) + } + time.Sleep(10 * time.Millisecond) + } +} + +// processesHolding counts the processes with a running thread whose real uid +// is id. +func processesHolding(id uint32) int { + procs := map[int]bool{} + for _, t := range threads() { + if t.running && t.uids[0] == id { + procs[t.tgid] = true + } + } + return len(procs) +} + +// zombieLeaderHolds reports whether a process whose leader thread is a zombie +// runs a thread whose real uid is id. +func zombieLeaderHolds(id uint32) bool { + zombie, holding := map[int]bool{}, map[int]bool{} + for _, t := range threads() { + switch { + case t.tid == t.tgid && !t.running: + zombie[t.tgid] = true + case t.running && t.uids[0] == id: + holding[t.tgid] = true + } + } + for tgid := range holding { + if zombie[tgid] { + return true + } + } + return false +} + +type thread struct { + tgid, tid int + uids [4]uint32 + running bool +} + +// threads lists every thread in /proc, zombies included. +func threads() []thread { + var list []thread + pids, _ := os.ReadDir("/proc") + for _, p := range pids { + tgid, err := strconv.Atoi(p.Name()) + if err != nil { + continue + } + tids, _ := os.ReadDir(procPath(tgid, "task")) + for _, e := range tids { + tid, _ := strconv.Atoi(e.Name()) + if s, err := readStatus(procPath(tgid, "task", e.Name(), "status")); err == nil { + list = append(list, thread{tgid: tgid, tid: tid, uids: s.uids, running: s.running()}) + } + } + } + return list +} + +// zombieLeaderEnv makes the test binary a process whose leader thread exits +// while another thread runs on. +const zombieLeaderEnv = "OAC_AGENTHOST_ZOMBIE_LEADER" + +func runZombieLeader() { + runtime.LockOSThread() + started := make(chan struct{}) + go func() { + runtime.LockOSThread() + close(started) + time.Sleep(time.Hour) + }() + <-started + unix.RawSyscall(unix.SYS_EXIT, 0, 0, 0) // ends this thread only } // sandbox is a relay and the oac-sandbox-io serving its one resource. From 67699146bd7a1255d1ee9e0666eb577d2bb72f18 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 05:00:51 +0000 Subject: [PATCH 09/13] Drive agent-host Turns as dispatch does and keep a Session whose Executor will not close Each Turn's output consumer starts before StartTurn, and a nil Turn's channel is closed. The Turn's Done waits for settlement and any required Executor.Close, after an Error envelope when the Turn failed, and nothing is published when Close fails. Teardown joins the forwarder, so nothing reaches Output after Run returns. A failed Close ends the views and is retried once; if it still fails, Run returns ErrTeardown and keeps the Session directory and the uid for the next Sweep. The driving mirrors dispatch's prepared execution. --- apps/daemon/internal/agenthost/doc.go | 15 +- apps/daemon/internal/agenthost/run_linux.go | 278 +++++++++++++++--- .../internal/agenthost/session_linux_test.go | 203 +++++++++++++ .../internal/agenthost/view_linux_test.go | 27 +- 4 files changed, 477 insertions(+), 46 deletions(-) diff --git a/apps/daemon/internal/agenthost/doc.go b/apps/daemon/internal/agenthost/doc.go index 1d95d205..9e7c66fa 100644 --- a/apps/daemon/internal/agenthost/doc.go +++ b/apps/daemon/internal/agenthost/doc.go @@ -30,7 +30,20 @@ // process broker, the Link attachment, the Session directory and the uid; // Run decides its result only afterwards, so a failure recorded during // teardown counts, and from the close of the attachment on, what the Link -// reports changes nothing. +// reports changes nothing. When Executor.Close fails, teardown ends the +// views, which kills their processes, and retries Close once. If Close +// fails again, the Executor may still use the Session directory: Run returns +// ErrTeardown and keeps the directory and the uid, which stays in use until +// the agent host exits, and the next agent host's Sweep reclaims both. +// +// Run drives each Turn as the daemon's dispatch drives a prepared execution. +// One output consumer starts before StartTurn and forwards the Turn's +// envelopes to Output in order. The Turn's Done waits until the Turn has +// settled and, when the Turn leaves the Executor unusable, until the +// Executor has closed; a failed Turn publishes an Error envelope before it. +// When Close fails, nothing more is published. A Turn that fails or leaves +// the Executor unusable ends the Session, and nothing is sent to Output +// after Run returns. // // Sweep runs at startup, before any Session. It ends every task, each thread // of each process, whose real, effective, saved or file-system uid lies in diff --git a/apps/daemon/internal/agenthost/run_linux.go b/apps/daemon/internal/agenthost/run_linux.go index 63b3c158..d039fd59 100644 --- a/apps/daemon/internal/agenthost/run_linux.go +++ b/apps/daemon/internal/agenthost/run_linux.go @@ -25,10 +25,9 @@ import ( const ( // turnBuffer is how many envelopes a Turn may emit ahead of Output. turnBuffer = 64 - // settleBound bounds a cancelled Turn's settlement. - settleBound = 30 * time.Second - // executorCloseBound bounds Executor.Close at teardown. - executorCloseBound = 30 * time.Second + // nativeBound bounds a Turn's settlement and each Executor.Close, as + // dispatch's preparedCancelTimeout does. + nativeBound = 10 * time.Second ) // deps are the parts tests replace. @@ -111,6 +110,11 @@ type session struct { brokerMu sync.Mutex broker processBroker // started at the first launch + + // The goroutine that runs drive and then teardown owns these. + fwd *forwarder // the last Turn's forwarder + execClosed bool // a Close of the Executor succeeded + execErr error // the last Close's error } func run(ctx context.Context, cfg Config, in Session, d deps) error { @@ -224,57 +228,245 @@ func (s *session) drive(exec agent.Executor) error { } } -// turn runs one Turn, forwards its envelopes to Output and waits for its -// settlement. When the Session ends first it cancels the Turn. +// The Turn driving below mirrors the daemon's prepared execution in +// apps/daemon/internal/dispatch: startPreparedExecution +// (preparation_start.go), forwardPreparedOutput, runPreparedRelease and +// forwardPreparedTerminal (prepared_handoff.go), as harness-onboarding.md's +// "What the Runtime does around a Turn" describes them. A Session has no +// steering, functions or interactions, so no admitted operation joins the +// release; the end of the Session stands in for the connection's shutdown. + +// forwarder is a Turn's one output consumer. It starts before StartTurn and +// drains out: it forwards each envelope to Output in order until the Session +// ends, and keeps the Turn's Done for turn to publish after settlement. +type forwarder struct { + runID string + out chan proto.Envelope + // ended closes at the Turn's terminal observation: its Done, a protocol + // error or the close of out. + ended chan struct{} + // abort closes when the Turn is to be cancelled: a protocol error, a + // failed start or the end of the Session. + abort chan struct{} + // stop makes the forwarder return without draining further. + stop chan struct{} + // done closes when the forwarder has returned and sends nothing more. + done chan struct{} + + endOnce, abortOnce, stopOnce sync.Once + + mu sync.Mutex + terminal *proto.Envelope + protocolErr error +} + +func newForwarder(runID string) *forwarder { + return &forwarder{runID: runID, out: make(chan proto.Envelope, turnBuffer), + ended: make(chan struct{}), abort: make(chan struct{}), stop: make(chan struct{}), done: make(chan struct{})} +} + +func (f *forwarder) end() { f.endOnce.Do(func() { close(f.ended) }) } +func (f *forwarder) cancel() { f.abortOnce.Do(func() { close(f.abort) }) } +func (f *forwarder) halt() { f.stopOnce.Do(func() { close(f.stop) }) } + +func (f *forwarder) aborted() bool { + select { + case <-f.abort: + return true + default: + return false + } +} + +// forward runs f until out closes or f is halted. +func (s *session) forward(f *forwarder) { + defer close(f.done) + for { + select { + case <-f.stop: + return + case e, ok := <-f.out: + if !ok { + f.end() + return + } + f.mu.Lock() + if e.ID != f.runID || f.terminal != nil { + if f.protocolErr == nil { + f.protocolErr = errors.New("executor output crossed the Turn boundary") + } + f.mu.Unlock() + f.end() + f.cancel() + continue + } + if e.Type == proto.TypeDone { + f.terminal = &e + f.mu.Unlock() + f.end() + continue + } + f.mu.Unlock() + if s.ctx.Err() == nil { + select { + case s.in.Output <- e: + case <-s.ctx.Done(): + } + } + } + } +} + +// turn runs one Turn. Its forwarder starts before StartTurn. Once the Turn's +// output ends, or the Turn is to be cancelled, turn awaits its settlement, +// closes the Executor when the Turn leaves it unusable and only then +// publishes the Turn's Done, after an Error envelope when the Turn failed. +// When Close fails, the Executor keeps the Turn and nothing is published. +// When the Session has ended, nothing is published and turn returns nil. func (s *session) turn(exec agent.Executor, in Input) error { - out := make(chan proto.Envelope, turnBuffer) - turn, err := exec.StartTurn(s.ctx, in.RunID, in.Message, out) + f := newForwarder(in.RunID) + s.fwd = f + go s.forward(f) + defer context.AfterFunc(s.ctx, f.cancel)() + turn, startErr := exec.StartTurn(s.ctx, in.RunID, in.Message, f.out) if turn == nil { - if err == nil { - err = errors.New("no Turn") + // out stays with the caller. The failed Turn ends the Session, and + // teardown closes the Executor. + close(f.out) + <-f.done + if startErr == nil { + startErr = errors.New("no Turn") + } + return &Error{Kind: ErrTurn, Op: "start", Err: startErr} + } + if startErr != nil { + f.cancel() + } + select { + case <-f.ended: + case <-f.abort: + } + settlement, nativeErr := settle(turn, f.abort) + if nativeErr == nil { + // Settlement confirms that out is closed. + select { + case <-f.done: + case <-s.ctx.Done(): } - return &Error{Kind: ErrTurn, Op: "start", Err: err} } - forwarded := make(chan struct{}) + f.mu.Lock() + terminal, protocolErr := f.terminal, f.protocolErr + if terminal == nil && protocolErr == nil && !f.aborted() { + protocolErr = errors.New("executor output ended without a terminal result") + } + f.mu.Unlock() + if nativeErr != nil || !settlement.Reusable || startErr != nil || protocolErr != nil || s.ctx.Err() != nil { + if err := s.closeExecutor(exec); err != nil { + return &Error{Kind: ErrTurn, Op: "close executor", Err: errors.Join(nativeErr, err)} + } + } + // A confirmed Close confirms that out is closed too. + select { + case <-f.done: + case <-s.ctx.Done(): + return nil + } + + var failure string + var result error + switch { + case nativeErr != nil: + failure, result = "executor Turn settlement failed", &Error{Kind: ErrTurn, Op: "settle", Err: nativeErr} + case protocolErr != nil: + failure, result = protocolErr.Error(), &Error{Kind: ErrTurn, Op: "output", Err: protocolErr} + case startErr != nil: + failure, result = "executor Turn could not start", &Error{Kind: ErrTurn, Op: "start", Err: startErr} + case !settlement.Reusable: + result = &Error{Kind: ErrTurn, Op: "settle", Err: fmt.Errorf("the Executor is not reusable: %s", settlement.Reason)} + } + if failure != "" { + e, err := proto.NewEnvelope(proto.TypeError, in.RunID, proto.ErrorPayload{Error: failure}) + if err != nil { + return errors.Join(result, err) + } + s.publish(e) + } + if terminal == nil { + e, err := proto.NewEnvelope(proto.TypeDone, in.RunID, proto.DonePayload{}) + if err != nil { + return errors.Join(result, err) + } + terminal = &e + } + s.publish(*terminal) + if s.ctx.Err() != nil { + return nil + } + return result +} + +// settle awaits turn's settlement for at most nativeBound. When abort closes +// first it cancels the Turn, and a failed Cancel ends the wait; natural +// completion never calls Cancel. +func settle(turn agent.Turn, abort <-chan struct{}) (agent.TurnSettlement, error) { + ctx, cancel := context.WithTimeout(context.Background(), nativeBound) + defer cancel() + cancelled := make(chan error, 1) + settled := make(chan struct{}) go func() { - defer close(forwarded) - for e := range out { - select { - case s.in.Output <- e: - case <-s.ctx.Done(): + select { + case <-abort: + err := turn.Cancel(ctx) + if err != nil { + cancel() } + cancelled <- err + case <-settled: + cancelled <- nil } }() - settled, serr := turn.AwaitSettlement(s.ctx) + settlement, err := turn.AwaitSettlement(ctx) + close(settled) + return settlement, errors.Join(err, <-cancelled) +} + +// publish sends e to Output while the Session runs. +func (s *session) publish(e proto.Envelope) { if s.ctx.Err() != nil { - ctx, cancel := context.WithTimeout(context.Background(), settleBound) - turn.Cancel(ctx) - settled, serr = turn.AwaitSettlement(ctx) - cancel() + return } - if serr != nil { - return &Error{Kind: ErrTurn, Op: "settle", Err: serr} + select { + case s.in.Output <- e: + case <-s.ctx.Done(): } - <-forwarded - switch { - case err != nil: - return &Error{Kind: ErrTurn, Op: "start", Err: err} - case !settled.Reusable: - return &Error{Kind: ErrTurn, Op: "settle", Err: fmt.Errorf("the Executor is not reusable: %s", settled.Reason)} +} + +// closeExecutor closes exec for at most nativeBound, until a Close succeeds. +// A failed Close retains the Executor's resources, and a later call retries +// it. +func (s *session) closeExecutor(exec agent.Executor) error { + if exec == nil || s.execClosed { + return nil } - return nil + ctx, cancel := context.WithTimeout(context.Background(), nativeBound) + defer cancel() + s.execErr = exec.Close(ctx) + s.execClosed = s.execErr == nil + return s.execErr } // teardown releases the Session in order: the Executor, the view, the // process broker, the Link attachment, the Session directory and the uid. +// When Close fails, teardown ends the views, which kills each view's +// processes, and retries Close once. If that fails too, the Executor may +// still use the Session directory: teardown returns ErrTeardown and keeps +// the directory and the uid, which stays in use until the agent host exits; +// the next agent host's Sweep reclaims both. func (s *session) teardown(exec agent.Executor) error { var errs []error - if exec != nil { - ctx, cancel := context.WithTimeout(context.Background(), executorCloseBound) - if err := exec.Close(ctx); err != nil { - errs = append(errs, &Error{Kind: ErrTeardown, Op: "close executor", Err: err}) - } - cancel() + closeErr := s.execErr + if closeErr == nil { + closeErr = s.closeExecutor(exec) } // Ending the Session closes the live view and refuses new launches. Under // mu, every launch that passed its check has already counted itself. @@ -282,6 +474,14 @@ func (s *session) teardown(exec agent.Executor) error { s.cancel() s.mu.Unlock() s.views.Wait() + if closeErr != nil { + closeErr = s.closeExecutor(exec) + } + // The Session has ended, so the forwarder sends nothing more. + if f := s.fwd; f != nil { + f.halt() + <-f.done + } s.brokerMu.Lock() broker := s.broker s.brokerMu.Unlock() @@ -291,6 +491,10 @@ func (s *session) teardown(exec agent.Executor) error { } } errs = append(errs, s.link.close()) + if closeErr != nil { + errs = append(errs, &Error{Kind: ErrTeardown, Op: "close executor", Err: closeErr}) + return errors.Join(errs...) + } if err := os.RemoveAll(string(s.dir)); err != nil { errs = append(errs, &Error{Kind: ErrTeardown, Op: "remove session directory", Err: err}) } diff --git a/apps/daemon/internal/agenthost/session_linux_test.go b/apps/daemon/internal/agenthost/session_linux_test.go index f36e306b..68067d52 100644 --- a/apps/daemon/internal/agenthost/session_linux_test.go +++ b/apps/daemon/internal/agenthost/session_linux_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "reflect" "sync" + "sync/atomic" "syscall" "testing" "time" @@ -125,15 +126,217 @@ func TestFailureDuringTeardownCounts(t *testing.T) { } } +func TestTurnDrainsOutputFromStart(t *testing.T) { + s := newOwnerSession(t) + output := make(chan proto.Envelope, 2*turnBuffer) + s.in.Output = output + // The adapter emits more than out holds before StartTurn returns. + turn := &fakeTurn{} + exec := &fakeExecutor{start: func(runID string, out chan<- proto.Envelope) (agent.Turn, error) { + for range turnBuffer + 1 { + out <- proto.Envelope{Type: proto.TypeOutputMessage, ID: runID} + } + out <- doneEnvelope(runID) + close(out) + return turn, nil + }} + if err := within(t, func() error { return s.turn(exec, Input{RunID: "r"}) }); err != nil { + t.Fatalf("turn = %v", err) + } + if got := drainAll(output); len(got) != turnBuffer+2 || got[len(got)-1].Type != proto.TypeDone || turn.cancelled.Load() || exec.closes != 0 { + t.Fatalf("Output got %d envelopes; Turn cancelled %v; Executor closed %d times", len(got), turn.cancelled.Load(), exec.closes) + } + // A nil Turn leaves out with its caller, which closes it. + var kept chan<- proto.Envelope + exec = &fakeExecutor{start: func(_ string, out chan<- proto.Envelope) (agent.Turn, error) { + kept = out + return nil, errors.New("refused") + }} + if err := within(t, func() error { return s.turn(exec, Input{RunID: "r"}) }); !errors.Is(err, ErrTurn) || !isClosed(kept) { + t.Fatalf("turn without a Turn = %v; out closed %v", err, isClosed(kept)) + } +} + +func TestTurnPublishesDoneAfterSettlementAndClose(t *testing.T) { + s := newOwnerSession(t) + output := make(chan proto.Envelope, 4) + s.in.Output = output + published := -1 + exec := &fakeExecutor{ + start: func(runID string, out chan<- proto.Envelope) (agent.Turn, error) { + out <- doneEnvelope(runID) + close(out) + return &fakeTurn{settleErr: errors.New("settlement lost")}, nil + }, + close: func() error { + published = len(output) + return nil + }, + } + err := within(t, func() error { return s.turn(exec, Input{RunID: "r"}) }) + got := drainAll(output) + if !errors.Is(err, ErrTurn) || published != 0 || len(got) != 2 || got[0].Type != proto.TypeError || got[1].Type != proto.TypeDone { + t.Fatalf("turn = %v; %d envelopes published before Close; Output got %v, want Error then Done", err, published, got) + } +} + +func TestFailedCloseKeepsTheSessionDirectoryAndUID(t *testing.T) { + errStuck := errors.New("close stuck") + for name, recovers := range map[string]bool{"Close fails until the view ends": true, "Close keeps failing": false} { + s := newOwnerSession(t) + output := make(chan proto.Envelope, 4) + s.in.Output = output + // Each case takes its own uid. + uid, err := allocUID(UIDRange{First: 72000, Count: 2}, &fakeProcesses{}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { freeUID(uid) }) + s.uid = uid + if s.dir, err = createSessionDir(t.TempDir(), sandboxwire.NewID(), uid); err != nil { + t.Fatal(err) + } + v := &fakeView{exit: make(chan struct{})} + s.live = &liveView{view: v} + s.views.Add(1) + go func() { + v.Wait() + s.views.Done() + }() + // The Turn's settlement fails, and its adapter never closes out. + var kept chan<- proto.Envelope + exec := &fakeExecutor{ + start: func(runID string, out chan<- proto.Envelope) (agent.Turn, error) { + kept = out + out <- proto.Envelope{Type: proto.TypeOutputMessage, ID: runID} + out <- doneEnvelope(runID) + return &fakeTurn{settleErr: errors.New("settlement lost")}, nil + }, + close: func() error { + select { + case <-v.exit: + if recovers { + return nil + } + default: + } + return errStuck + }, + } + err = within(t, func() error { return s.finish(exec, s.turn(exec, Input{RunID: "r"}), nil) }) + _, statErr := os.Stat(string(s.dir)) + uids.Lock() + used := uids.used[uid] + uids.Unlock() + switch { + case !errors.Is(err, ErrTurn) || exec.closes != 2 || len(output) != 1: + t.Errorf("%s: Run = %v; Executor closed %d times; Output got %d envelopes, want only the output message", name, err, exec.closes, len(output)) + case recovers && (errors.Is(err, ErrTeardown) || statErr == nil || used): + t.Errorf("%s: Run = %v; directory kept %v; uid in use %v", name, err, statErr == nil, used) + case !recovers && (!errors.Is(err, ErrTeardown) || !errors.Is(err, errStuck) || statErr != nil || !used): + t.Errorf("%s: Run = %v; directory kept %v; uid in use %v; want ErrTeardown keeping both", name, err, statErr == nil, used) + } + // The forwarder has returned, so nothing reaches Output any more. + select { + case <-s.fwd.done: + default: + t.Errorf("%s: the forwarder outlives Run", name) + } + kept <- proto.Envelope{Type: proto.TypeOutputMessage, ID: "r"} + } +} + // newOwnerSession is a Session with no directory, uid or link. func newOwnerSession(t *testing.T) *session { s := &session{log: slog.New(slog.DiscardHandler)} s.ctx, s.cancel = context.WithCancel(context.Background()) t.Cleanup(s.cancel) + context.AfterFunc(s.ctx, s.closeLive) s.link = newLinkOwner(nil, Binding{AttachmentID: sandboxwire.NewID()}, s.fail) return s } +// within runs f and fails t unless f returns within a bound. +func within(t *testing.T, f func() error) error { + t.Helper() + done := make(chan error, 1) + go func() { done <- f() }() + select { + case err := <-done: + return err + case <-time.After(5 * time.Second): + t.Fatal("blocked") + return nil + } +} + +// drainAll returns what ch holds. +func drainAll(ch chan proto.Envelope) []proto.Envelope { + var list []proto.Envelope + for len(ch) > 0 { + list = append(list, <-ch) + } + return list +} + +// isClosed reports whether ch is closed. +func isClosed(ch chan<- proto.Envelope) (closed bool) { + defer func() { closed = recover() != nil }() + select { + case ch <- proto.Envelope{}: + default: + } + return false +} + +func doneEnvelope(runID string) proto.Envelope { + e, err := proto.NewEnvelope(proto.TypeDone, runID, proto.DonePayload{}) + if err != nil { + panic(err) + } + return e +} + +// fakeExecutor starts each Turn with start; Close returns close's result. +type fakeExecutor struct { + start func(runID string, out chan<- proto.Envelope) (agent.Turn, error) + close func() error + closes int +} + +func (e *fakeExecutor) StartTurn(_ context.Context, runID string, _ proto.MessageInput, out chan<- proto.Envelope) (agent.Turn, error) { + return e.start(runID, out) +} + +func (e *fakeExecutor) Close(context.Context) error { + e.closes++ + if e.close == nil { + return nil + } + return e.close() +} + +// fakeTurn settles at once: reusable, or with settleErr. +type fakeTurn struct { + settleErr error + cancelled atomic.Bool +} + +func (t *fakeTurn) Cancel(context.Context) error { + t.cancelled.Store(true) + return nil +} + +func (t *fakeTurn) CancellationOutcome() proto.DonePayload { return proto.DonePayload{} } + +func (t *fakeTurn) SteerWithReceipt(context.Context, proto.PromptSteerPayload, func()) error { + return agent.ErrUnsupportedOperation +} + +func (t *fakeTurn) AwaitSettlement(context.Context) (agent.TurnSettlement, error) { + return agent.TurnSettlement{Reusable: t.settleErr == nil}, t.settleErr +} + // fakeView is a view that ends when closed. type fakeView struct { exit chan struct{} diff --git a/apps/daemon/internal/agenthost/view_linux_test.go b/apps/daemon/internal/agenthost/view_linux_test.go index 29819f4d..888f73fc 100644 --- a/apps/daemon/internal/agenthost/view_linux_test.go +++ b/apps/daemon/internal/agenthost/view_linux_test.go @@ -485,22 +485,31 @@ func (r *sessionRun) send(t *testing.T, mode string) { } } +// turn runs a Turn in mode and returns its report, which its Done follows. func (r *sessionRun) turn(t *testing.T, mode string) report { t.Helper() r.send(t, mode) + var rep report + if err := json.Unmarshal(r.next(t, mode).Payload, &rep); err != nil { + t.Fatal(err) + } + if e := r.next(t, mode); e.Type != proto.TypeDone { + t.Fatalf("the %s Turn sent %s after its report, want its Done", mode, e.Type) + } + return rep +} + +func (r *sessionRun) next(t *testing.T, mode string) proto.Envelope { + t.Helper() select { case e := <-r.out: - var rep report - if err := json.Unmarshal(e.Payload, &rep); err != nil { - t.Fatal(err) - } - return rep + return e case err := <-r.done: t.Fatalf("Run ended during the %s Turn: %v", mode, err) case <-time.After(wait): - t.Fatalf("no %s report", mode) + t.Fatalf("the %s Turn sent nothing", mode) } - return report{} + return proto.Envelope{} } func (r *sessionRun) wait(t *testing.T) error { @@ -586,7 +595,7 @@ func (e *testExecutor) StartTurn(_ context.Context, runID string, input proto.Me func (e *testExecutor) Close(context.Context) error { return nil } -// report is a Turn's one envelope: the Harness's checks, its stderr and how +// report is a Turn's report envelope: the Harness's checks, its stderr and how // it exited. type report struct { Checks map[string]string `json:"checks"` @@ -618,6 +627,8 @@ func (t *testTurn) run(runID string, out chan<- proto.Envelope) { r.Stderr = stderr.String() payload, _ := json.Marshal(r) out <- proto.Envelope{Type: proto.TypeOutputMessage, ID: runID, Payload: payload} + done, _ := proto.NewEnvelope(proto.TypeDone, runID, proto.DonePayload{}) + out <- done } func (t *testTurn) Cancel(context.Context) error { From 371c4ff58a3f1d9a55fda8ad1795725ee042ff8b Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 05:19:00 +0000 Subject: [PATCH 10/13] End leftover views by their launchers before the uid sweep A process in a view could fork and exit as each uid scan passed, so the scan could miss a view entirely. sessionview.EndLeftoverViews finds each view by its launcher, the process whose command line is exactly the launcher's and which is PID 1 of a PID namespace directly below /proc's, kills it through a pidfd confirmed after opening, and waits with a bound for it to exit; the kernel kills the rest of the view first. Sweep calls it before the uid scan, which now only ends host-namespace leftovers, and drops the namespace-init walk. procfs.end closes its pidfd on every path. --- apps/daemon/internal/agenthost/agenthost.go | 4 +- apps/daemon/internal/agenthost/doc.go | 37 ++-- apps/daemon/internal/agenthost/procs_linux.go | 73 +------- apps/daemon/internal/agenthost/run_linux.go | 19 +- .../internal/agenthost/session_linux_test.go | 15 +- .../internal/agenthost/view_linux_test.go | 49 +++-- .../internal/sessionview/leftover_linux.go | 177 ++++++++++++++++++ .../sessionview/leftover_linux_test.go | 65 +++++++ .../internal/sessionview/leftover_other.go | 8 + 9 files changed, 330 insertions(+), 117 deletions(-) create mode 100644 apps/daemon/internal/sessionview/leftover_linux.go create mode 100644 apps/daemon/internal/sessionview/leftover_linux_test.go create mode 100644 apps/daemon/internal/sessionview/leftover_other.go diff --git a/apps/daemon/internal/agenthost/agenthost.go b/apps/daemon/internal/agenthost/agenthost.go index 95f83b31..cdaf7545 100644 --- a/apps/daemon/internal/agenthost/agenthost.go +++ b/apps/daemon/internal/agenthost/agenthost.go @@ -20,8 +20,8 @@ type Config struct { StateDir string // UIDs is the range Session uids are allocated from; each Session's gid // equals its uid. Only one agent host runs per kernel, and nothing else - // uses the range: Sweep kills every process that holds one of its uids, - // with its whole PID namespace when that is not the agent host's. + // uses the range or starts session views: Sweep ends every view and kills + // every process that holds one of the range's uids. UIDs UIDRange // RelayURL and TLS reach the Link relay, as sandboxlink.DialAttach takes // them. A nil TLS uses the system roots. diff --git a/apps/daemon/internal/agenthost/doc.go b/apps/daemon/internal/agenthost/doc.go index 9e7c66fa..fdd62407 100644 --- a/apps/daemon/internal/agenthost/doc.go +++ b/apps/daemon/internal/agenthost/doc.go @@ -45,26 +45,23 @@ // the Executor unusable ends the Session, and nothing is sent to Output // after Run returns. // -// Sweep runs at startup, before any Session. It ends every task, each thread -// of each process, whose real, effective, saved or file-system uid lies in -// Config.UIDs, scans /proc again until none runs or a bound passes, and only -// then removes the Session directories a previous agent host left. -// Allocation also skips a uid that any running task holds. Session processes -// run only in a view's PID namespace, so Sweep ends a view's task by killing -// the namespace's init: the kernel then kills every process in the -// namespace, and nothing can fork into it any more. The init is the task's -// nearest ancestor whose pid in its namespace is 1; no process in a view can -// enter another namespace, so each ancestor up to it shares the namespace. -// Each step of the walk pins the parent with a pidfd and confirms that the -// child still has that parent, and each signal goes through a pidfd, so a -// reused pid is never followed or signalled. A task with a Session uid in -// the agent host's own namespace is not a Session's: Sweep kills its process -// and returns ErrTeardown if one still runs after the bound. A scan misses a -// process only while a parent that exits at once forks it; the view still -// ends once any of its tasks is caught, and a view's launcher ends the view -// when the agent host that started it goes away. A task that the agent -// host's /proc does not show, such as one in a sibling PID namespace, is -// outside these guarantees, which is why nothing else may use the range. +// Sweep runs at startup, before any Session. Session processes run only in +// views, and each view's launcher is PID 1 of the view's PID namespace, so +// Sweep first ends every view a previous agent host left with +// sessionview.EndLeftoverViews: killing a launcher kills every process in its +// view, whatever its uids, and the launcher exits only once its view is +// empty. Nothing in a view can start a view or leave its namespace, so no +// Session process remains once every launcher has exited. Sweep then kills +// each process with a task, any thread, whose real, effective, saved or +// file-system uid lies in Config.UIDs. Such a process runs outside every view +// and is not a Session's: Sweep scans /proc again until none runs or a bound +// passes and then returns ErrTeardown; one that forks and exits as each scan +// passes can escape the scans. Only then does Sweep remove the Session +// directories a previous agent host left. Each signal goes through a pidfd, +// so a reused pid is never signalled. Allocation also skips a uid that any +// running task holds. A view or task that the agent host's /proc does not +// show, such as one in a sibling PID namespace, is outside these guarantees, +// which is why nothing else may use the range or start views. // // The Harness view protocol is in contracts/agents-api/harness-onboarding.md // and the gateway's in contracts/agents-api/model-execution.md. diff --git a/apps/daemon/internal/agenthost/procs_linux.go b/apps/daemon/internal/agenthost/procs_linux.go index f56525ca..54075534 100644 --- a/apps/daemon/internal/agenthost/procs_linux.go +++ b/apps/daemon/internal/agenthost/procs_linux.go @@ -43,9 +43,7 @@ type processTable interface { // tasks returns every task that runs, each thread of each process; a // zombie runs nothing and is left out. tasks() ([]task, error) - // end kills, while t still holds a uid in r, the init of t's PID - // namespace when that is a view's, which ends every process in it, and - // t's process when t is in the agent host's own namespace. It never + // end kills t's process while t still holds a uid in r. It never // signals a process that reused a pid. end(t task, r UIDRange) error } @@ -162,81 +160,27 @@ func (procfs) end(t task, r UIDRange) error { if err != nil { return fmt.Errorf("pidfd of %d: %w", t.tgid, err) } + defer unix.Close(fd) // The pid may name another process since the scan. What /proc shows under - // it belongs to the process fd pins while that process exists, which each - // read confirms afterwards; once it has ended, the signal reaches nothing. + // it belongs to the process fd pins while that process exists, which the + // signal 0 confirms afterwards; once it has ended, the kill reaches + // nothing. s, err := readStatus(procPath(t.tgid, "task", strconv.Itoa(t.tid), "status")) if err == nil { err = exists(fd) } - if err == nil && (!s.running() || !holds(s.uids, r)) { - err = errGone - } - if err == nil && len(s.nspid) > 1 { - fd, err = namespaceInit(fd, t.tgid, len(s.nspid)) - } - if errors.Is(err, errGone) { + if errors.Is(err, errGone) || (err == nil && (!s.running() || !holds(s.uids, r))) { return nil } if err != nil { return err } - defer unix.Close(fd) if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) { return fmt.Errorf("kill: %w", err) } return nil } -// namespaceInit takes fd, the pidfd of process pid, and returns a pidfd of the -// init of its PID namespace, depth namespaces below /proc's. On an error it -// closes every pidfd. The init is the process's nearest ancestor whose pid in -// its own namespace is 1: no process in a view can enter another namespace, -// so every ancestor up to the init shares the namespace. Each step pins the -// parent and then confirms that the child still exists and still has that -// parent, so the walk never follows a reused pid. -func namespaceInit(fd, pid, depth int) (int, error) { - for { - s, err := readStatus(procPath(pid, "status")) - if err == nil { - err = exists(fd) - } - if err == nil && (len(s.nspid) != depth || s.ppid <= 0) { - err = fmt.Errorf("process %d is outside its view's PID namespace", pid) - } - if err != nil { - unix.Close(fd) - return -1, err - } - if s.nspid[depth-1] == 1 { - return fd, nil - } - parent, err := unix.PidfdOpen(s.ppid, 0) - if errors.Is(err, unix.ESRCH) { - continue // the parent has ended and the process has a new one - } - if err != nil { - unix.Close(fd) - return -1, fmt.Errorf("pidfd of %d: %w", s.ppid, err) - } - again, err := readStatus(procPath(pid, "status")) - if err == nil { - err = exists(fd) - } - if err != nil { - unix.Close(parent) - unix.Close(fd) - return -1, err - } - if again.ppid != s.ppid { - unix.Close(parent) - continue - } - unix.Close(fd) - fd, pid = parent, s.ppid - } -} - // exists returns nil while the process fd pins exists, as a zombie too, and // errGone once it has been reaped. func exists(fd int) error { @@ -254,7 +198,6 @@ func procPath(pid int, name ...string) string { // status is what a status file in /proc reports. type status struct { state string - ppid int uids [4]uint32 nspid []int // the pid in each PID namespace, from /proc's to the task's own } @@ -281,10 +224,6 @@ func readStatus(path string) (status, error) { switch key { case "State": s.state = strings.TrimSpace(value) - case "PPid": - if s.ppid, err = strconv.Atoi(strings.TrimSpace(value)); err != nil { - return s, fmt.Errorf("%s: PPid %q", path, value) - } case "Uid": uids = strings.Fields(value) case "NSpid": diff --git a/apps/daemon/internal/agenthost/run_linux.go b/apps/daemon/internal/agenthost/run_linux.go index d039fd59..1fd9ef82 100644 --- a/apps/daemon/internal/agenthost/run_linux.go +++ b/apps/daemon/internal/agenthost/run_linux.go @@ -15,6 +15,7 @@ import ( "time" "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/sessionview" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxfs" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" @@ -46,22 +47,26 @@ func Run(ctx context.Context, cfg Config, s Session) error { return run(ctx, cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return unavailableBroker{} }, procs: procfs{}}) } -// Sweep ends every process that holds a uid in cfg.UIDs, with its view's PID -// namespace, then removes every Session directory under cfg.StateDir. Run's -// owner calls it at startup, before any Session runs, with /proc showing the -// agent host's own PID namespace. It returns ErrTeardown when a task still -// holds a Session uid after a bounded wait. +// Sweep ends every view a previous agent host left, then every process that +// holds a uid in cfg.UIDs, then removes every Session directory under +// cfg.StateDir. Run's owner calls it at startup, before any Session runs, +// with /proc showing the agent host's own PID namespace. It returns +// ErrTeardown when a view or a process with a Session uid still runs after a +// bounded wait. func Sweep(cfg Config) error { - return sweep(cfg, procfs{}, sweepBound) + return sweep(cfg, sessionview.EndLeftoverViews, procfs{}, sweepBound) } -func sweep(cfg Config, procs processTable, bound time.Duration) error { +func sweep(cfg Config, endViews func(time.Duration) error, procs processTable, bound time.Duration) error { switch { case !isHostPath(cfg.StateDir): return invalidConfig("state directory %q is not absolute and clean", cfg.StateDir) case !cfg.UIDs.valid(): return invalidConfig("uid range %d+%d", cfg.UIDs.First, cfg.UIDs.Count) } + if err := endViews(bound); err != nil { + return &Error{Kind: ErrTeardown, Op: "sweep views", Err: err} + } if err := endProcesses(procs, cfg.UIDs, bound); err != nil { return &Error{Kind: ErrTeardown, Op: "sweep processes", Err: err} } diff --git a/apps/daemon/internal/agenthost/session_linux_test.go b/apps/daemon/internal/agenthost/session_linux_test.go index 68067d52..a000afb4 100644 --- a/apps/daemon/internal/agenthost/session_linux_test.go +++ b/apps/daemon/internal/agenthost/session_linux_test.go @@ -23,9 +23,10 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" ) -func TestSweepEndsSessionProcessesBeforeRemovingDirectories(t *testing.T) { +func TestSweepEndsViewsAndProcessesBeforeRemovingDirectories(t *testing.T) { cfg := Config{StateDir: t.TempDir(), UIDs: UIDRange{First: 70000, Count: 8}} - if err := sweep(cfg, &fakeProcesses{}, time.Second); err != nil { + viewsEnded := func(time.Duration) error { return nil } + if err := sweep(cfg, viewsEnded, &fakeProcesses{}, time.Second); err != nil { t.Fatalf("Sweep without sessions: %v", err) } leave := func() { @@ -38,14 +39,20 @@ func TestSweepEndsSessionProcessesBeforeRemovingDirectories(t *testing.T) { // Any of the four uids of any thread places a process in the range. outside := [4]uint32{1000, 1000, 1000, 1000} procs := &fakeProcesses{list: []task{{tgid: 10, tid: 10, uids: outside}, {tgid: 10, tid: 12, uids: [4]uint32{1000, 1000, 1000, 70003}}, {tgid: 11, tid: 11, uids: outside}}} - if err := sweep(cfg, procs, time.Second); err != nil || !reflect.DeepEqual(procs.ended, []int{10}) || len(leftSessions(t, cfg)) != 0 { + if err := sweep(cfg, viewsEnded, procs, time.Second); err != nil || !reflect.DeepEqual(procs.ended, []int{10}) || len(leftSessions(t, cfg)) != 0 { t.Fatalf("Sweep = %v, ended %v, %d Session directories left", err, procs.ended, len(leftSessions(t, cfg))) } leave() procs = &fakeProcesses{list: []task{{tgid: 12, tid: 12, uids: [4]uint32{70000, 70000, 70000, 70000}}}, stubborn: true} - if err := sweep(cfg, procs, 100*time.Millisecond); !errors.Is(err, ErrTeardown) || len(leftSessions(t, cfg)) != 1 { + if err := sweep(cfg, viewsEnded, procs, 100*time.Millisecond); !errors.Is(err, ErrTeardown) || len(leftSessions(t, cfg)) != 1 { t.Fatalf("Sweep with a process that outlives the bound = %v, %d Session directories left", err, len(leftSessions(t, cfg))) } + errStuck := errors.New("a launcher still runs") + procs = &fakeProcesses{list: []task{{tgid: 13, tid: 13, uids: [4]uint32{70000, 70000, 70000, 70000}}}} + viewsStuck := func(time.Duration) error { return errStuck } + if err := sweep(cfg, viewsStuck, procs, time.Second); !errors.Is(err, ErrTeardown) || !errors.Is(err, errStuck) || procs.ended != nil || len(leftSessions(t, cfg)) != 1 { + t.Fatalf("Sweep with a view that outlives the bound = %v, ended %v, %d Session directories left", err, procs.ended, len(leftSessions(t, cfg))) + } } func TestAllocationSkipsUIDsThatProcessesHold(t *testing.T) { diff --git a/apps/daemon/internal/agenthost/view_linux_test.go b/apps/daemon/internal/agenthost/view_linux_test.go index 888f73fc..9e9cebdb 100644 --- a/apps/daemon/internal/agenthost/view_linux_test.go +++ b/apps/daemon/internal/agenthost/view_linux_test.go @@ -200,7 +200,7 @@ func TestSessionRunsInAViewOverItsAttachment(t *testing.T) { if err != nil { t.Fatal(err) } - cmd := inPIDNamespace(id, exe) + cmd := inView(id, exe) cmd.Env = append(os.Environ(), zombieLeaderEnv+"=1") done := startView(t, cmd) until(t, "a zombie leader with a running thread", func() bool { return zombieLeaderHolds(id) }) @@ -211,23 +211,38 @@ func TestSessionRunsInAViewOverItsAttachment(t *testing.T) { sweepEnds(t, cfg, id, done) }) - t.Run("Sweep ends a view whose processes fork", func(t *testing.T) { + t.Run("Sweep ends a view that the uid scan misses", func(t *testing.T) { id := cfg.UIDs.First + 3 - // Each subshell forks a sleep and exits at once. - done := startView(t, inPIDNamespace(id, "/bin/sh", "-c", "while :; do (sleep 60 &); sleep 0.01; done")) - until(t, "forked processes", func() bool { return processesHolding(id) >= 3 }) - sweepEnds(t, cfg, id, done) + done := startView(t, inView(id, "/bin/sleep", "600")) + until(t, "a Session process", func() bool { return processesHolding(id) == 1 }) + left := filepath.Join(sessionsDir(cfg.StateDir), "left") + if err := os.MkdirAll(left, 0o700); err != nil { + t.Fatal(err) + } + // The scan finds no process with a Session uid, as when the last one + // forks and exits as the scan passes. + if err := sweep(cfg, sessionview.EndLeftoverViews, &fakeProcesses{}, wait); err != nil { + t.Fatalf("Sweep = %v", err) + } + select { + case <-done: + case <-time.After(wait): + t.Fatal("the view's launcher still runs after Sweep") + } + if n := processesHolding(id); n != 0 || len(leftSessions(t, cfg)) != 0 { + t.Errorf("%d processes hold uid %d and %d Session directories remain after Sweep", n, id, len(leftSessions(t, cfg))) + } }) } -// inPIDNamespace returns a command that runs argv with uid under a root init -// in a new PID namespace, as a view does. The init starts argv again whenever -// it ends, so only ending the namespace ends it. -func inPIDNamespace(uid uint32, argv ...string) *exec.Cmd { - script := `while :; do setpriv --reuid="$0" --regid="$0" --clear-groups -- "$@"; done` - cmd := exec.Command("/bin/sh", append([]string{"-c", script, strconv.Itoa(int(uid))}, argv...)...) - cmd.SysProcAttr = &syscall.SysProcAttr{Cloneflags: syscall.CLONE_NEWPID} - return cmd +// inView returns a command that runs argv, which holds no shell syntax, with +// uid in a view-like PID namespace: its root init has sessionview's launcher +// command line and starts argv again whenever it ends, so only ending the +// namespace ends it. +func inView(uid uint32, argv ...string) *exec.Cmd { + script := fmt.Sprintf("while :; do setpriv --reuid=%d --regid=%d --clear-groups -- %s; done\n", uid, uid, strings.Join(argv, " ")) + return &exec.Cmd{Path: "/bin/sh", Args: []string{"oac-sessionview"}, Stdin: strings.NewReader(script), + SysProcAttr: &syscall.SysProcAttr{Cloneflags: syscall.CLONE_NEWPID}} } // startView starts cmd and returns its end. @@ -242,8 +257,8 @@ func startView(t *testing.T, cmd *exec.Cmd) <-chan error { return done } -// sweepEnds checks that Sweep ends the view whose init done reports and every -// process that holds id. +// sweepEnds checks that Sweep ends the view whose launcher done reports and +// every process that holds id. func sweepEnds(t *testing.T, cfg Config, id uint32, done <-chan error) { t.Helper() if err := Sweep(cfg); err != nil { @@ -252,7 +267,7 @@ func sweepEnds(t *testing.T, cfg Config, id uint32, done <-chan error) { select { case <-done: case <-time.After(wait): - t.Fatal("the view's init still runs after Sweep") + t.Fatal("the view's launcher still runs after Sweep") } if n := processesHolding(id); n != 0 { t.Errorf("%d processes hold uid %d after Sweep", n, id) diff --git a/apps/daemon/internal/sessionview/leftover_linux.go b/apps/daemon/internal/sessionview/leftover_linux.go new file mode 100644 index 00000000..e6e8dbac --- /dev/null +++ b/apps/daemon/internal/sessionview/leftover_linux.go @@ -0,0 +1,177 @@ +//go:build linux + +package sessionview + +import ( + "bufio" + "bytes" + "errors" + "fmt" + "io/fs" + "os" + "strconv" + "strings" + "time" + + "golang.org/x/sys/unix" +) + +// EndLeftoverViews ends every view that /proc shows. A daemon that starts views calls it at startup, before it starts any, to end the views a previous instance left; nothing else on the host may start views. +// +// A view's launcher is the process whose command line is exactly the launcher's and which is PID 1 of a PID namespace directly below /proc's. EndLeftoverViews kills each launcher, and the kernel then kills every other process in its view; the launcher exits only once its view is empty. Each launcher is pinned with a pidfd and confirmed again before the kill, so a reused pid is never signalled. It waits up to bound for every launcher to exit and returns an [ErrLauncher] error when one has not. /proc must show the caller's own PID namespace. +func EndLeftoverViews(bound time.Duration) error { + self, err := nspid("/proc/self/status") + if err != nil { + return leftoverError(err) + } + if len(self) != 1 { + return leftoverError(errors.New("/proc is not this process's PID namespace's")) + } + entries, err := os.ReadDir("/proc") + if err != nil { + return leftoverError(err) + } + var launchers []int + defer func() { + for _, fd := range launchers { + unix.Close(fd) + } + }() + for _, e := range entries { + pid, err := strconv.Atoi(e.Name()) + if err != nil || pid <= 0 { + continue + } + fd, err := killLauncher(pid) + if err != nil { + return leftoverError(err) + } + if fd >= 0 { + launchers = append(launchers, fd) + } + } + left, err := awaitExits(launchers, bound) + if err != nil { + return leftoverError(err) + } + if left > 0 { + return leftoverError(fmt.Errorf("%d launchers still run after %s", left, bound)) + } + return nil +} + +func leftoverError(err error) error { + return &Error{Kind: ErrLauncher, Op: "end leftover views", Err: err} +} + +// killLauncher kills pid when it is a launcher and returns its pidfd, or -1 when pid is not a launcher or has ended. +func killLauncher(pid int) (int, error) { + if ok, err := isLauncher(pid); err != nil || !ok { + return -1, err + } + fd, err := unix.PidfdOpen(pid, 0) + if errors.Is(err, unix.ESRCH) { + return -1, nil + } + if err != nil { + return -1, fmt.Errorf("pidfd of %d: %w", pid, err) + } + // What /proc shows under pid belongs to the process fd pins while that process exists, which the signal 0 confirms afterwards. + ok, err := isLauncher(pid) + if err == nil && ok { + err = unix.PidfdSendSignal(fd, 0, nil, 0) + if err == nil { + err = unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0) + } + } + switch { + case errors.Is(err, unix.ESRCH) || (err == nil && !ok): + unix.Close(fd) + return -1, nil + case err != nil: + unix.Close(fd) + return -1, fmt.Errorf("kill launcher %d: %w", pid, err) + } + return fd, nil +} + +// isLauncher reports whether pid is a launcher. A process that has ended, or a zombie, which has no command line, is not. +func isLauncher(pid int) (bool, error) { + dir := "/proc/" + strconv.Itoa(pid) + "/" + cmdline, err := os.ReadFile(dir + "cmdline") + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ESRCH) { + return false, nil + } + if err != nil { + return false, err + } + if string(cmdline) != launcherArg0+"\x00" { + return false, nil + } + ns, err := nspid(dir + "status") + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ESRCH) { + return false, nil + } + if err != nil { + return false, err + } + return len(ns) == 2 && ns[1] == 1, nil +} + +// nspid reads the NSpid line of a status file in /proc: the pid in each PID namespace, from /proc's to the process's own. +func nspid(path string) ([]int, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + sc := bufio.NewScanner(bytes.NewReader(data)) + for sc.Scan() { + key, value, _ := strings.Cut(sc.Text(), ":") + if key != "NSpid" { + continue + } + var ids []int + for _, f := range strings.Fields(value) { + id, err := strconv.Atoi(f) + if err != nil { + return nil, fmt.Errorf("%s: NSpid %q", path, value) + } + ids = append(ids, id) + } + if len(ids) > 0 { + return ids, nil + } + } + return nil, fmt.Errorf("%s has no NSpid", path) +} + +// awaitExits waits up to bound for the process behind each pidfd to exit and returns how many have not. +func awaitExits(fds []int, bound time.Duration) (int, error) { + deadline := time.Now().Add(bound) + pending := fds + for len(pending) > 0 { + wait := time.Until(deadline) + if wait <= 0 { + break + } + polls := make([]unix.PollFd, len(pending)) + for i, fd := range pending { + polls[i] = unix.PollFd{Fd: int32(fd), Events: unix.POLLIN} + } + _, err := unix.Poll(polls, int(wait.Milliseconds())+1) + if errors.Is(err, unix.EINTR) { + continue + } + if err != nil { + return len(pending), err + } + var next []int + for i, p := range polls { + if p.Revents == 0 { + next = append(next, pending[i]) + } + } + pending = next + } + return len(pending), nil +} diff --git a/apps/daemon/internal/sessionview/leftover_linux_test.go b/apps/daemon/internal/sessionview/leftover_linux_test.go new file mode 100644 index 00000000..ebb8dd7b --- /dev/null +++ b/apps/daemon/internal/sessionview/leftover_linux_test.go @@ -0,0 +1,65 @@ +//go:build linux + +package sessionview + +import ( + "bufio" + "context" + "fmt" + "os" + "os/exec" + "strings" + "testing" + "time" +) + +// TestEndLeftoverViewsEndsLiveViews checks that EndLeftoverViews ends a running view with every process in it and leaves a process that only shares the launcher's command line. +func TestEndLeftoverViewsEndsLiveViews(t *testing.T) { + requireView(t) + f := newFixture(t) + w := &loopbackWorld{dir: f.world} + token := fmt.Sprintf("oac-leftover-%d", time.Now().UnixNano()) + v, err := Start(context.Background(), f.spec(w, "wait", "OAC_VIEW_TOKEN="+token)) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer v.Close() + if line, err := bufio.NewReader(v.Stdout()).ReadString('\n'); err != nil || line != "ready\n" { + t.Fatalf("harness said %q, %v", line, err) + } + if n := processesWith(t, token); n != 1 { + t.Fatalf("%d grandchildren in the view, want 1", n) + } + // It has the launcher's command line but is no PID 1 of a view. + other := exec.Command("/bin/sh") + other.Args = []string{launcherArg0} + other.Stdin = strings.NewReader("sleep 60\n") + if err := other.Start(); err != nil { + t.Fatal(err) + } + defer func() { + other.Process.Kill() + other.Wait() + }() + + if err := EndLeftoverViews(5 * time.Second); err != nil { + t.Fatalf("EndLeftoverViews: %v", err) + } + waited := make(chan struct{}) + go func() { + v.Wait() + close(waited) + }() + select { + case <-waited: + case <-time.After(5 * time.Second): + t.Fatal("the view still runs") + } + if n := processesWith(t, token); n != 0 { + t.Errorf("%d processes of the view survived", n) + } + // A killed child stays a zombie until other.Wait reaps it. + if status, err := os.ReadFile(fmt.Sprintf("/proc/%d/status", other.Process.Pid)); err != nil || strings.Contains(string(status), "State:\tZ") { + t.Errorf("a process outside any view was killed: %v", err) + } +} diff --git a/apps/daemon/internal/sessionview/leftover_other.go b/apps/daemon/internal/sessionview/leftover_other.go new file mode 100644 index 00000000..9c740f34 --- /dev/null +++ b/apps/daemon/internal/sessionview/leftover_other.go @@ -0,0 +1,8 @@ +//go:build !linux + +package sessionview + +import "time" + +// EndLeftoverViews reports that views need Linux. +func EndLeftoverViews(time.Duration) error { return ErrUnsupported } From 3ac016c17fc53665de86de7b55662eaa794b0e3c Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 05:19:07 +0000 Subject: [PATCH 11/13] Reject function tools at agent host admission A function call waits for a result, and a view Session's Input carries only new Turns, so the result could never arrive. Admission now rejects function tools and tool search with ErrUnsupported before any effect. --- apps/daemon/internal/agenthost/admit.go | 3 +++ apps/daemon/internal/agenthost/admit_linux_test.go | 1 + apps/daemon/internal/agenthost/doc.go | 3 ++- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/daemon/internal/agenthost/admit.go b/apps/daemon/internal/agenthost/admit.go index c53caa9b..e4a320f9 100644 --- a/apps/daemon/internal/agenthost/admit.go +++ b/apps/daemon/internal/agenthost/admit.go @@ -112,6 +112,9 @@ func admit(cfg Config, roots *x509.CertPool, s Session, openNetwork func(context return nil, unsupported("installed Capabilities and skills") case local.NetworkAccess != "enabled" || len(local.AllowedDomains) > 0: return nil, unsupported("a restricted workspace network") + case len(req.FunctionTools) > 0 || req.ToolSearch: + // A function call waits for a result that Input cannot deliver. + return nil, unsupported("function tools and their discovery") } raw, ok := req.AgentOptions["model_provider"] if !ok { diff --git a/apps/daemon/internal/agenthost/admit_linux_test.go b/apps/daemon/internal/agenthost/admit_linux_test.go index 679457ba..a4a3cbba 100644 --- a/apps/daemon/internal/agenthost/admit_linux_test.go +++ b/apps/daemon/internal/agenthost/admit_linux_test.go @@ -76,6 +76,7 @@ func TestAdmissionRejectsBeforeAnyEffect(t *testing.T) { "capabilities": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.Capabilities = true }, []error{ErrUnsupported}}, "restricted network": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.NetworkAccess = "disabled" }, []error{ErrUnsupported}}, "allowed domains only": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.AllowedDomains = []string{"example.com"} }, []error{ErrUnsupported}}, + "function tools": {func(r *proto.PromptRequestPayload) { r.FunctionTools = []proto.FunctionTool{{Name: "lookup"}} }, []error{ErrUnsupported, agent.ErrUnsupportedOperation}}, "stdio MCP": {func(r *proto.PromptRequestPayload) { r.LocalEnvironment.MCP = []proto.EnvironmentMCP{{Server: agentplugin.MCPServer{Name: "tools", Type: "stdio", Command: "tools"}}} }, []error{ErrUnsupported, agent.ErrUnsupportedOperation}}, diff --git a/apps/daemon/internal/agenthost/doc.go b/apps/daemon/internal/agenthost/doc.go index fdd62407..14077098 100644 --- a/apps/daemon/internal/agenthost/doc.go +++ b/apps/daemon/internal/agenthost/doc.go @@ -4,7 +4,8 @@ // ErrUnsupported. // // Run runs one Session. It admits the Session before any effect: the kind -// must declare an agent.View, and the request must use only what a view runs. +// must declare an agent.View, and the request must use only what a view runs +// and no function tools, whose results Input cannot carry. // It then allocates the Session uid, creates the Session directory under // Config.StateDir, rewrites the request so the model provider and HTTP MCP // reach the network only through the Session's gateway, and calls the view's From 35a8f4a9ea31e10aa14b5697641093f48a4a58e3 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 05:19:12 +0000 Subject: [PATCH 12/13] Keep the terminal that arrives while the Executor closes Close may deliver the Turn's Done, for example when StartTurn failed but left native work running. turn now reads the forwarder's terminal again after the forwarder has finished, so that Done is published instead of being replaced by a synthesized one. --- apps/daemon/internal/agenthost/run_linux.go | 4 +++- .../internal/agenthost/session_linux_test.go | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/apps/daemon/internal/agenthost/run_linux.go b/apps/daemon/internal/agenthost/run_linux.go index 1fd9ef82..70c02310 100644 --- a/apps/daemon/internal/agenthost/run_linux.go +++ b/apps/daemon/internal/agenthost/run_linux.go @@ -370,12 +370,14 @@ func (s *session) turn(exec agent.Executor, in Input) error { return &Error{Kind: ErrTurn, Op: "close executor", Err: errors.Join(nativeErr, err)} } } - // A confirmed Close confirms that out is closed too. + // A confirmed Close confirms that out is closed too. What arrived during + // Close counts: Close may deliver the Turn's Done. select { case <-f.done: case <-s.ctx.Done(): return nil } + terminal = f.terminal var failure string var result error diff --git a/apps/daemon/internal/agenthost/session_linux_test.go b/apps/daemon/internal/agenthost/session_linux_test.go index a000afb4..51e951ef 100644 --- a/apps/daemon/internal/agenthost/session_linux_test.go +++ b/apps/daemon/internal/agenthost/session_linux_test.go @@ -185,6 +185,29 @@ func TestTurnPublishesDoneAfterSettlementAndClose(t *testing.T) { if !errors.Is(err, ErrTurn) || published != 0 || len(got) != 2 || got[0].Type != proto.TypeError || got[1].Type != proto.TypeDone { t.Fatalf("turn = %v; %d envelopes published before Close; Output got %v, want Error then Done", err, published, got) } + // A Turn that failed to start delivers its Done during Close. + s = newOwnerSession(t) + s.in.Output = output + native, err := proto.NewEnvelope(proto.TypeDone, "r", proto.DonePayload{Content: "native"}) + if err != nil { + t.Fatal(err) + } + var kept chan<- proto.Envelope + exec = &fakeExecutor{ + start: func(_ string, out chan<- proto.Envelope) (agent.Turn, error) { + kept = out + return &fakeTurn{settleErr: errors.New("settlement lost")}, errors.New("start failed") + }, + close: func() error { + kept <- native + close(kept) + return nil + }, + } + err = within(t, func() error { return s.turn(exec, Input{RunID: "r"}) }) + if got := drainAll(output); !errors.Is(err, ErrTurn) || len(got) != 2 || got[0].Type != proto.TypeError || !reflect.DeepEqual(got[1], native) { + t.Fatalf("turn = %v; Output got %v, want Error then the Done from Close", err, got) + } } func TestFailedCloseKeepsTheSessionDirectoryAndUID(t *testing.T) { From 4338269a1dc1e426961dfc666c9b4e5157d6116d Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Thu, 1 Oct 2026 05:35:05 +0000 Subject: [PATCH 13/13] Remove agent-host restart recovery until views have owned cgroups Ending a previous agent host's views by launcher command line and PID namespace cannot prove ownership, and an exiting launcher is already invisible to the scan. Restart recovery returns with an owned cgroup per view; until then the agent host recovers nothing an earlier agent-host process left, and a new one must not reuse its StateDir or UIDs. Sweep, sessionview.EndLeftoverViews and the /proc kill and wait code go. Allocation keeps its thread scan, which only detects a uid conflict. A Session whose Executor will not close keeps its directory, and its uid stays in use until the agent host exits. --- apps/daemon/internal/agenthost/agenthost.go | 12 +- .../agenthost/agenthost_linux_test.go | 26 +-- apps/daemon/internal/agenthost/doc.go | 30 +-- apps/daemon/internal/agenthost/procs_linux.go | 117 +----------- apps/daemon/internal/agenthost/run_linux.go | 47 +---- apps/daemon/internal/agenthost/run_other.go | 5 - .../internal/agenthost/session_linux_test.go | 33 ---- .../internal/agenthost/view_linux_test.go | 108 +---------- .../internal/sessionview/leftover_linux.go | 177 ------------------ .../sessionview/leftover_linux_test.go | 65 ------- .../internal/sessionview/leftover_other.go | 8 - 11 files changed, 32 insertions(+), 596 deletions(-) delete mode 100644 apps/daemon/internal/sessionview/leftover_linux.go delete mode 100644 apps/daemon/internal/sessionview/leftover_linux_test.go delete mode 100644 apps/daemon/internal/sessionview/leftover_other.go diff --git a/apps/daemon/internal/agenthost/agenthost.go b/apps/daemon/internal/agenthost/agenthost.go index cdaf7545..7ad0558f 100644 --- a/apps/daemon/internal/agenthost/agenthost.go +++ b/apps/daemon/internal/agenthost/agenthost.go @@ -14,14 +14,17 @@ import ( // Config is the agent host's own configuration, shared by its Sessions. It // holds a credential: keep it in memory and never log it. +// +// The agent host does not yet recover Sessions that an earlier agent-host +// process left. Until it does, an agent-host process must not reuse the +// StateDir or the UIDs of an earlier one. type Config struct { // StateDir is an absolute host directory private to the agent host. Each // Session's directory is StateDir/sessions/. StateDir string // UIDs is the range Session uids are allocated from; each Session's gid // equals its uid. Only one agent host runs per kernel, and nothing else - // uses the range or starts session views: Sweep ends every view and kills - // every process that holds one of the range's uids. + // uses the range or starts session views. UIDs UIDRange // RelayURL and TLS reach the Link relay, as sandboxlink.DialAttach takes // them. A nil TLS uses the system roots. @@ -92,8 +95,7 @@ type Input struct { Message proto.MessageInput } -// Error kinds. Every error Run and Sweep return matches one of them with -// errors.Is. +// Error kinds. Every error Run returns matches one of them with errors.Is. var ( // ErrUnsupported is a platform other than Linux, or a Session that asks // for what the agent host does not run. A Session's error also matches @@ -101,7 +103,7 @@ var ( // agent.ErrViewHandoff, or agent.ErrInvalidView for a view whose paths // meet the agent host's own overlays. ErrUnsupported = errors.New("agenthost: unsupported") - // ErrInvalidConfig is a Config that Run and Sweep reject. + // ErrInvalidConfig is a Config that Run rejects. ErrInvalidConfig = errors.New("agenthost: invalid configuration") // ErrInvalidSession is a malformed Session. ErrInvalidSession = errors.New("agenthost: invalid session") diff --git a/apps/daemon/internal/agenthost/agenthost_linux_test.go b/apps/daemon/internal/agenthost/agenthost_linux_test.go index a4c64e99..8c7243f6 100644 --- a/apps/daemon/internal/agenthost/agenthost_linux_test.go +++ b/apps/daemon/internal/agenthost/agenthost_linux_test.go @@ -9,8 +9,6 @@ import ( "errors" "os" "path/filepath" - "slices" - "sync" "sync/atomic" "testing" @@ -115,27 +113,9 @@ func leftSessions(t *testing.T, cfg Config) []os.DirEntry { return entries } -// fakeProcesses is a process table whose processes end when killed, unless -// they are stubborn. +// fakeProcesses is a process table that lists fixed tasks. type fakeProcesses struct { - mu sync.Mutex - list []task - stubborn bool - ended []int + list []task } -func (f *fakeProcesses) tasks() ([]task, error) { - f.mu.Lock() - defer f.mu.Unlock() - return slices.Clone(f.list), nil -} - -func (f *fakeProcesses) end(t task, r UIDRange) error { - f.mu.Lock() - defer f.mu.Unlock() - f.ended = append(f.ended, t.tgid) - if !f.stubborn { - f.list = slices.DeleteFunc(f.list, func(q task) bool { return q.tgid == t.tgid }) - } - return nil -} +func (f *fakeProcesses) tasks() ([]task, error) { return f.list, nil } diff --git a/apps/daemon/internal/agenthost/doc.go b/apps/daemon/internal/agenthost/doc.go index 14077098..26014d6b 100644 --- a/apps/daemon/internal/agenthost/doc.go +++ b/apps/daemon/internal/agenthost/doc.go @@ -1,12 +1,14 @@ // Package agenthost runs Sessions whose Harness runs on the agent host, next to // Core, while its tools, files and network act in the sandbox through the -// Session's Link attachment. It needs Linux; elsewhere Run and Sweep return +// Session's Link attachment. It needs Linux; elsewhere Run returns // ErrUnsupported. // // Run runs one Session. It admits the Session before any effect: the kind // must declare an agent.View, and the request must use only what a view runs -// and no function tools, whose results Input cannot carry. -// It then allocates the Session uid, creates the Session directory under +// and no function tools, whose results Input cannot carry. It then allocates +// the Session uid, skipping each uid that a running thread holds as its real, +// effective, saved or file-system uid; this check only detects a conflict and +// never ends a process. It creates the Session directory under // Config.StateDir, rewrites the request so the model provider and HTTP MCP // reach the network only through the Session's gateway, and calls the view's // Executor factory. The first ViewSession.Launch starts the process broker; @@ -34,8 +36,8 @@ // reports changes nothing. When Executor.Close fails, teardown ends the // views, which kills their processes, and retries Close once. If Close // fails again, the Executor may still use the Session directory: Run returns -// ErrTeardown and keeps the directory and the uid, which stays in use until -// the agent host exits, and the next agent host's Sweep reclaims both. +// ErrTeardown and keeps the directory, and the uid stays in use until the +// agent host exits. // // Run drives each Turn as the daemon's dispatch drives a prepared execution. // One output consumer starts before StartTurn and forwards the Turn's @@ -46,24 +48,6 @@ // the Executor unusable ends the Session, and nothing is sent to Output // after Run returns. // -// Sweep runs at startup, before any Session. Session processes run only in -// views, and each view's launcher is PID 1 of the view's PID namespace, so -// Sweep first ends every view a previous agent host left with -// sessionview.EndLeftoverViews: killing a launcher kills every process in its -// view, whatever its uids, and the launcher exits only once its view is -// empty. Nothing in a view can start a view or leave its namespace, so no -// Session process remains once every launcher has exited. Sweep then kills -// each process with a task, any thread, whose real, effective, saved or -// file-system uid lies in Config.UIDs. Such a process runs outside every view -// and is not a Session's: Sweep scans /proc again until none runs or a bound -// passes and then returns ErrTeardown; one that forks and exits as each scan -// passes can escape the scans. Only then does Sweep remove the Session -// directories a previous agent host left. Each signal goes through a pidfd, -// so a reused pid is never signalled. Allocation also skips a uid that any -// running task holds. A view or task that the agent host's /proc does not -// show, such as one in a sibling PID namespace, is outside these guarantees, -// which is why nothing else may use the range or start views. -// // The Harness view protocol is in contracts/agents-api/harness-onboarding.md // and the gateway's in contracts/agents-api/model-execution.md. package agenthost diff --git a/apps/daemon/internal/agenthost/procs_linux.go b/apps/daemon/internal/agenthost/procs_linux.go index 54075534..fe7ba350 100644 --- a/apps/daemon/internal/agenthost/procs_linux.go +++ b/apps/daemon/internal/agenthost/procs_linux.go @@ -11,41 +11,23 @@ import ( "os" "strconv" "strings" - "time" "golang.org/x/sys/unix" ) -const ( - // sweepBound bounds how long Sweep waits for the processes it killed. - sweepBound = 10 * time.Second - // sweepPoll is the pause between Sweep's scans. - sweepPoll = 50 * time.Millisecond -) - // task is one running task, a thread of a process, as /proc lists it. type task struct { tgid, tid int uids [4]uint32 // real, effective, saved and file-system } -// in reports whether t holds a uid in r. -func (t task) in(r UIDRange) bool { return holds(t.uids, r) } - -func holds(uids [4]uint32, r UIDRange) bool { - return r.has(uids[0]) || r.has(uids[1]) || r.has(uids[2]) || r.has(uids[3]) -} - func (r UIDRange) has(id uint32) bool { return id >= r.First && id-r.First < r.Count } -// processTable lists the host's tasks and ends them. Tests replace it. +// processTable lists the host's tasks. Tests replace it. type processTable interface { // tasks returns every task that runs, each thread of each process; a // zombie runs nothing and is left out. tasks() ([]task, error) - // end kills t's process while t still holds a uid in r. It never - // signals a process that reused a pid. - end(t task, r UIDRange) error } // heldUIDs returns the uids in r that a running task holds. @@ -65,56 +47,13 @@ func heldUIDs(procs processTable, r UIDRange) (map[uint32]bool, error) { return held, nil } -// endProcesses ends every task that holds a uid in r and scans again until -// none runs or bound passes. -func endProcesses(procs processTable, r UIDRange, bound time.Duration) error { - deadline := time.Now().Add(bound) - for { - tasks, err := procs.tasks() - if err != nil { - return err - } - var held []task - for _, t := range tasks { - if t.in(r) { - held = append(held, t) - } - } - if len(held) == 0 { - return nil - } - if !time.Now().Before(deadline) { - return fmt.Errorf("%d tasks still hold Session uids after %s", len(held), bound) - } - ended := map[int]bool{} - for _, t := range held { - if ended[t.tgid] { - continue - } - ended[t.tgid] = true - if err := procs.end(t, r); err != nil { - return err - } - } - time.Sleep(sweepPoll) - } -} - -// procfs is the host's /proc, which must be the agent host's own PID -// namespace's. +// procfs is the host's /proc. type procfs struct{} // errGone is a process or task that has ended. var errGone = errors.New("ended") func (procfs) tasks() ([]task, error) { - self, err := readStatus("/proc/self/status") - if err != nil { - return nil, err - } - if len(self.nspid) != 1 { - return nil, errors.New("/proc is not the agent host's PID namespace's") - } pids, err := os.ReadDir("/proc") if err != nil { return nil, err @@ -152,45 +91,6 @@ func (procfs) tasks() ([]task, error) { return list, nil } -func (procfs) end(t task, r UIDRange) error { - fd, err := unix.PidfdOpen(t.tgid, 0) - if errors.Is(err, unix.ESRCH) { - return nil - } - if err != nil { - return fmt.Errorf("pidfd of %d: %w", t.tgid, err) - } - defer unix.Close(fd) - // The pid may name another process since the scan. What /proc shows under - // it belongs to the process fd pins while that process exists, which the - // signal 0 confirms afterwards; once it has ended, the kill reaches - // nothing. - s, err := readStatus(procPath(t.tgid, "task", strconv.Itoa(t.tid), "status")) - if err == nil { - err = exists(fd) - } - if errors.Is(err, errGone) || (err == nil && (!s.running() || !holds(s.uids, r))) { - return nil - } - if err != nil { - return err - } - if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) { - return fmt.Errorf("kill: %w", err) - } - return nil -} - -// exists returns nil while the process fd pins exists, as a zombie too, and -// errGone once it has been reaped. -func exists(fd int) error { - err := unix.PidfdSendSignal(fd, 0, nil, 0) - if errors.Is(err, unix.ESRCH) { - return errGone - } - return err -} - func procPath(pid int, name ...string) string { return "/proc/" + strconv.Itoa(pid) + "/" + strings.Join(name, "/") } @@ -199,7 +99,6 @@ func procPath(pid int, name ...string) string { type status struct { state string uids [4]uint32 - nspid []int // the pid in each PID namespace, from /proc's to the task's own } func (s status) running() bool { @@ -226,18 +125,10 @@ func readStatus(path string) (status, error) { s.state = strings.TrimSpace(value) case "Uid": uids = strings.Fields(value) - case "NSpid": - for _, f := range strings.Fields(value) { - n, err := strconv.Atoi(f) - if err != nil { - return s, fmt.Errorf("%s: NSpid %q", path, value) - } - s.nspid = append(s.nspid, n) - } } } - if len(uids) != 4 || len(s.nspid) == 0 { - return s, fmt.Errorf("%s has no uids or NSpid", path) + if len(uids) != 4 { + return s, fmt.Errorf("%s has no uids", path) } for i, f := range uids { id, err := strconv.ParseUint(f, 10, 32) diff --git a/apps/daemon/internal/agenthost/run_linux.go b/apps/daemon/internal/agenthost/run_linux.go index 70c02310..0cb07bfb 100644 --- a/apps/daemon/internal/agenthost/run_linux.go +++ b/apps/daemon/internal/agenthost/run_linux.go @@ -7,15 +7,12 @@ import ( "errors" "fmt" "io" - "io/fs" "log/slog" "os" - "path/filepath" "sync" "time" "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" - "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/sessionview" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxfs" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" @@ -47,47 +44,6 @@ func Run(ctx context.Context, cfg Config, s Session) error { return run(ctx, cfg, s, deps{dial: relayDial(cfg), broker: func() processBroker { return unavailableBroker{} }, procs: procfs{}}) } -// Sweep ends every view a previous agent host left, then every process that -// holds a uid in cfg.UIDs, then removes every Session directory under -// cfg.StateDir. Run's owner calls it at startup, before any Session runs, -// with /proc showing the agent host's own PID namespace. It returns -// ErrTeardown when a view or a process with a Session uid still runs after a -// bounded wait. -func Sweep(cfg Config) error { - return sweep(cfg, sessionview.EndLeftoverViews, procfs{}, sweepBound) -} - -func sweep(cfg Config, endViews func(time.Duration) error, procs processTable, bound time.Duration) error { - switch { - case !isHostPath(cfg.StateDir): - return invalidConfig("state directory %q is not absolute and clean", cfg.StateDir) - case !cfg.UIDs.valid(): - return invalidConfig("uid range %d+%d", cfg.UIDs.First, cfg.UIDs.Count) - } - if err := endViews(bound); err != nil { - return &Error{Kind: ErrTeardown, Op: "sweep views", Err: err} - } - if err := endProcesses(procs, cfg.UIDs, bound); err != nil { - return &Error{Kind: ErrTeardown, Op: "sweep processes", Err: err} - } - dir := sessionsDir(cfg.StateDir) - entries, err := os.ReadDir(dir) - if errors.Is(err, fs.ErrNotExist) { - return nil - } - if err != nil { - return &Error{Kind: ErrTeardown, Op: "sweep", Err: err} - } - var errs []error - for _, e := range entries { - errs = append(errs, os.RemoveAll(filepath.Join(dir, e.Name()))) - } - if err := errors.Join(errs...); err != nil { - return &Error{Kind: ErrTeardown, Op: "sweep", Err: err} - } - return nil -} - // session is one running Session. type session struct { cfg Config @@ -467,8 +423,7 @@ func (s *session) closeExecutor(exec agent.Executor) error { // When Close fails, teardown ends the views, which kills each view's // processes, and retries Close once. If that fails too, the Executor may // still use the Session directory: teardown returns ErrTeardown and keeps -// the directory and the uid, which stays in use until the agent host exits; -// the next agent host's Sweep reclaims both. +// the directory and the uid, which stays in use until the agent host exits. func (s *session) teardown(exec agent.Executor) error { var errs []error closeErr := s.execErr diff --git a/apps/daemon/internal/agenthost/run_other.go b/apps/daemon/internal/agenthost/run_other.go index 40b9d80e..f6758ae5 100644 --- a/apps/daemon/internal/agenthost/run_other.go +++ b/apps/daemon/internal/agenthost/run_other.go @@ -8,8 +8,3 @@ import "context" func Run(context.Context, Config, Session) error { return &Error{Kind: ErrUnsupported, Op: "run"} } - -// Sweep reports that the agent host needs Linux. -func Sweep(Config) error { - return &Error{Kind: ErrUnsupported, Op: "sweep"} -} diff --git a/apps/daemon/internal/agenthost/session_linux_test.go b/apps/daemon/internal/agenthost/session_linux_test.go index 51e951ef..daa5e3c3 100644 --- a/apps/daemon/internal/agenthost/session_linux_test.go +++ b/apps/daemon/internal/agenthost/session_linux_test.go @@ -7,7 +7,6 @@ import ( "errors" "log/slog" "os" - "path/filepath" "reflect" "sync" "sync/atomic" @@ -23,38 +22,6 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" ) -func TestSweepEndsViewsAndProcessesBeforeRemovingDirectories(t *testing.T) { - cfg := Config{StateDir: t.TempDir(), UIDs: UIDRange{First: 70000, Count: 8}} - viewsEnded := func(time.Duration) error { return nil } - if err := sweep(cfg, viewsEnded, &fakeProcesses{}, time.Second); err != nil { - t.Fatalf("Sweep without sessions: %v", err) - } - leave := func() { - left := filepath.Join(sessionsDir(cfg.StateDir), "left", "home") - if err := os.MkdirAll(left, 0o700); err != nil { - t.Fatal(err) - } - } - leave() - // Any of the four uids of any thread places a process in the range. - outside := [4]uint32{1000, 1000, 1000, 1000} - procs := &fakeProcesses{list: []task{{tgid: 10, tid: 10, uids: outside}, {tgid: 10, tid: 12, uids: [4]uint32{1000, 1000, 1000, 70003}}, {tgid: 11, tid: 11, uids: outside}}} - if err := sweep(cfg, viewsEnded, procs, time.Second); err != nil || !reflect.DeepEqual(procs.ended, []int{10}) || len(leftSessions(t, cfg)) != 0 { - t.Fatalf("Sweep = %v, ended %v, %d Session directories left", err, procs.ended, len(leftSessions(t, cfg))) - } - leave() - procs = &fakeProcesses{list: []task{{tgid: 12, tid: 12, uids: [4]uint32{70000, 70000, 70000, 70000}}}, stubborn: true} - if err := sweep(cfg, viewsEnded, procs, 100*time.Millisecond); !errors.Is(err, ErrTeardown) || len(leftSessions(t, cfg)) != 1 { - t.Fatalf("Sweep with a process that outlives the bound = %v, %d Session directories left", err, len(leftSessions(t, cfg))) - } - errStuck := errors.New("a launcher still runs") - procs = &fakeProcesses{list: []task{{tgid: 13, tid: 13, uids: [4]uint32{70000, 70000, 70000, 70000}}}} - viewsStuck := func(time.Duration) error { return errStuck } - if err := sweep(cfg, viewsStuck, procs, time.Second); !errors.Is(err, ErrTeardown) || !errors.Is(err, errStuck) || procs.ended != nil || len(leftSessions(t, cfg)) != 1 { - t.Fatalf("Sweep with a view that outlives the bound = %v, ended %v, %d Session directories left", err, procs.ended, len(leftSessions(t, cfg))) - } -} - func TestAllocationSkipsUIDsThatProcessesHold(t *testing.T) { r := UIDRange{First: 71000, Count: 2} // A thread holds the uid; its process's leader does not. diff --git a/apps/daemon/internal/agenthost/view_linux_test.go b/apps/daemon/internal/agenthost/view_linux_test.go index 9e9cebdb..3331d495 100644 --- a/apps/daemon/internal/agenthost/view_linux_test.go +++ b/apps/daemon/internal/agenthost/view_linux_test.go @@ -174,106 +174,30 @@ func TestSessionRunsInAViewOverItsAttachment(t *testing.T) { checkReleased(t, cfg) }) - t.Run("Sweep ends a lingering Session process", func(t *testing.T) { + t.Run("allocation skips a uid that a thread holds under an exited leader", func(t *testing.T) { id := cfg.UIDs.First + 1 - cmd := exec.Command("/bin/sleep", "60") - cmd.SysProcAttr = &syscall.SysProcAttr{Credential: &syscall.Credential{Uid: id, Gid: id}} - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - done := make(chan error, 1) - go func() { done <- cmd.Wait() }() - if err := Sweep(cfg); err != nil { - t.Fatalf("Sweep = %v", err) - } - select { - case <-done: - case <-time.After(wait): - cmd.Process.Kill() - t.Fatal("the process with a Session uid still runs after Sweep") - } - }) - - t.Run("Sweep ends a view whose leader thread has exited", func(t *testing.T) { - id := cfg.UIDs.First + 2 exe, err := os.Executable() if err != nil { t.Fatal(err) } - cmd := inView(id, exe) + cmd := exec.Command(exe) cmd.Env = append(os.Environ(), zombieLeaderEnv+"=1") - done := startView(t, cmd) + cmd.SysProcAttr = &syscall.SysProcAttr{Credential: &syscall.Credential{Uid: id, Gid: id}} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + defer func() { + cmd.Process.Kill() + cmd.Wait() + }() until(t, "a zombie leader with a running thread", func() bool { return zombieLeaderHolds(id) }) if got, err := allocUID(UIDRange{First: id, Count: 1}, procfs{}); !errors.Is(err, ErrCapacity) { freeUID(got) t.Errorf("allocUID beside a running thread = %d, %v", got, err) } - sweepEnds(t, cfg, id, done) - }) - - t.Run("Sweep ends a view that the uid scan misses", func(t *testing.T) { - id := cfg.UIDs.First + 3 - done := startView(t, inView(id, "/bin/sleep", "600")) - until(t, "a Session process", func() bool { return processesHolding(id) == 1 }) - left := filepath.Join(sessionsDir(cfg.StateDir), "left") - if err := os.MkdirAll(left, 0o700); err != nil { - t.Fatal(err) - } - // The scan finds no process with a Session uid, as when the last one - // forks and exits as the scan passes. - if err := sweep(cfg, sessionview.EndLeftoverViews, &fakeProcesses{}, wait); err != nil { - t.Fatalf("Sweep = %v", err) - } - select { - case <-done: - case <-time.After(wait): - t.Fatal("the view's launcher still runs after Sweep") - } - if n := processesHolding(id); n != 0 || len(leftSessions(t, cfg)) != 0 { - t.Errorf("%d processes hold uid %d and %d Session directories remain after Sweep", n, id, len(leftSessions(t, cfg))) - } }) } -// inView returns a command that runs argv, which holds no shell syntax, with -// uid in a view-like PID namespace: its root init has sessionview's launcher -// command line and starts argv again whenever it ends, so only ending the -// namespace ends it. -func inView(uid uint32, argv ...string) *exec.Cmd { - script := fmt.Sprintf("while :; do setpriv --reuid=%d --regid=%d --clear-groups -- %s; done\n", uid, uid, strings.Join(argv, " ")) - return &exec.Cmd{Path: "/bin/sh", Args: []string{"oac-sessionview"}, Stdin: strings.NewReader(script), - SysProcAttr: &syscall.SysProcAttr{Cloneflags: syscall.CLONE_NEWPID}} -} - -// startView starts cmd and returns its end. -func startView(t *testing.T, cmd *exec.Cmd) <-chan error { - t.Helper() - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - done := make(chan error, 1) - go func() { done <- cmd.Wait() }() - t.Cleanup(func() { cmd.Process.Kill() }) - return done -} - -// sweepEnds checks that Sweep ends the view whose launcher done reports and -// every process that holds id. -func sweepEnds(t *testing.T, cfg Config, id uint32, done <-chan error) { - t.Helper() - if err := Sweep(cfg); err != nil { - t.Fatalf("Sweep = %v", err) - } - select { - case <-done: - case <-time.After(wait): - t.Fatal("the view's launcher still runs after Sweep") - } - if n := processesHolding(id); n != 0 { - t.Errorf("%d processes hold uid %d after Sweep", n, id) - } -} - func until(t *testing.T, what string, ok func() bool) { t.Helper() deadline := time.Now().Add(wait) @@ -285,18 +209,6 @@ func until(t *testing.T, what string, ok func() bool) { } } -// processesHolding counts the processes with a running thread whose real uid -// is id. -func processesHolding(id uint32) int { - procs := map[int]bool{} - for _, t := range threads() { - if t.running && t.uids[0] == id { - procs[t.tgid] = true - } - } - return len(procs) -} - // zombieLeaderHolds reports whether a process whose leader thread is a zombie // runs a thread whose real uid is id. func zombieLeaderHolds(id uint32) bool { diff --git a/apps/daemon/internal/sessionview/leftover_linux.go b/apps/daemon/internal/sessionview/leftover_linux.go deleted file mode 100644 index e6e8dbac..00000000 --- a/apps/daemon/internal/sessionview/leftover_linux.go +++ /dev/null @@ -1,177 +0,0 @@ -//go:build linux - -package sessionview - -import ( - "bufio" - "bytes" - "errors" - "fmt" - "io/fs" - "os" - "strconv" - "strings" - "time" - - "golang.org/x/sys/unix" -) - -// EndLeftoverViews ends every view that /proc shows. A daemon that starts views calls it at startup, before it starts any, to end the views a previous instance left; nothing else on the host may start views. -// -// A view's launcher is the process whose command line is exactly the launcher's and which is PID 1 of a PID namespace directly below /proc's. EndLeftoverViews kills each launcher, and the kernel then kills every other process in its view; the launcher exits only once its view is empty. Each launcher is pinned with a pidfd and confirmed again before the kill, so a reused pid is never signalled. It waits up to bound for every launcher to exit and returns an [ErrLauncher] error when one has not. /proc must show the caller's own PID namespace. -func EndLeftoverViews(bound time.Duration) error { - self, err := nspid("/proc/self/status") - if err != nil { - return leftoverError(err) - } - if len(self) != 1 { - return leftoverError(errors.New("/proc is not this process's PID namespace's")) - } - entries, err := os.ReadDir("/proc") - if err != nil { - return leftoverError(err) - } - var launchers []int - defer func() { - for _, fd := range launchers { - unix.Close(fd) - } - }() - for _, e := range entries { - pid, err := strconv.Atoi(e.Name()) - if err != nil || pid <= 0 { - continue - } - fd, err := killLauncher(pid) - if err != nil { - return leftoverError(err) - } - if fd >= 0 { - launchers = append(launchers, fd) - } - } - left, err := awaitExits(launchers, bound) - if err != nil { - return leftoverError(err) - } - if left > 0 { - return leftoverError(fmt.Errorf("%d launchers still run after %s", left, bound)) - } - return nil -} - -func leftoverError(err error) error { - return &Error{Kind: ErrLauncher, Op: "end leftover views", Err: err} -} - -// killLauncher kills pid when it is a launcher and returns its pidfd, or -1 when pid is not a launcher or has ended. -func killLauncher(pid int) (int, error) { - if ok, err := isLauncher(pid); err != nil || !ok { - return -1, err - } - fd, err := unix.PidfdOpen(pid, 0) - if errors.Is(err, unix.ESRCH) { - return -1, nil - } - if err != nil { - return -1, fmt.Errorf("pidfd of %d: %w", pid, err) - } - // What /proc shows under pid belongs to the process fd pins while that process exists, which the signal 0 confirms afterwards. - ok, err := isLauncher(pid) - if err == nil && ok { - err = unix.PidfdSendSignal(fd, 0, nil, 0) - if err == nil { - err = unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0) - } - } - switch { - case errors.Is(err, unix.ESRCH) || (err == nil && !ok): - unix.Close(fd) - return -1, nil - case err != nil: - unix.Close(fd) - return -1, fmt.Errorf("kill launcher %d: %w", pid, err) - } - return fd, nil -} - -// isLauncher reports whether pid is a launcher. A process that has ended, or a zombie, which has no command line, is not. -func isLauncher(pid int) (bool, error) { - dir := "/proc/" + strconv.Itoa(pid) + "/" - cmdline, err := os.ReadFile(dir + "cmdline") - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ESRCH) { - return false, nil - } - if err != nil { - return false, err - } - if string(cmdline) != launcherArg0+"\x00" { - return false, nil - } - ns, err := nspid(dir + "status") - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ESRCH) { - return false, nil - } - if err != nil { - return false, err - } - return len(ns) == 2 && ns[1] == 1, nil -} - -// nspid reads the NSpid line of a status file in /proc: the pid in each PID namespace, from /proc's to the process's own. -func nspid(path string) ([]int, error) { - data, err := os.ReadFile(path) - if err != nil { - return nil, err - } - sc := bufio.NewScanner(bytes.NewReader(data)) - for sc.Scan() { - key, value, _ := strings.Cut(sc.Text(), ":") - if key != "NSpid" { - continue - } - var ids []int - for _, f := range strings.Fields(value) { - id, err := strconv.Atoi(f) - if err != nil { - return nil, fmt.Errorf("%s: NSpid %q", path, value) - } - ids = append(ids, id) - } - if len(ids) > 0 { - return ids, nil - } - } - return nil, fmt.Errorf("%s has no NSpid", path) -} - -// awaitExits waits up to bound for the process behind each pidfd to exit and returns how many have not. -func awaitExits(fds []int, bound time.Duration) (int, error) { - deadline := time.Now().Add(bound) - pending := fds - for len(pending) > 0 { - wait := time.Until(deadline) - if wait <= 0 { - break - } - polls := make([]unix.PollFd, len(pending)) - for i, fd := range pending { - polls[i] = unix.PollFd{Fd: int32(fd), Events: unix.POLLIN} - } - _, err := unix.Poll(polls, int(wait.Milliseconds())+1) - if errors.Is(err, unix.EINTR) { - continue - } - if err != nil { - return len(pending), err - } - var next []int - for i, p := range polls { - if p.Revents == 0 { - next = append(next, pending[i]) - } - } - pending = next - } - return len(pending), nil -} diff --git a/apps/daemon/internal/sessionview/leftover_linux_test.go b/apps/daemon/internal/sessionview/leftover_linux_test.go deleted file mode 100644 index ebb8dd7b..00000000 --- a/apps/daemon/internal/sessionview/leftover_linux_test.go +++ /dev/null @@ -1,65 +0,0 @@ -//go:build linux - -package sessionview - -import ( - "bufio" - "context" - "fmt" - "os" - "os/exec" - "strings" - "testing" - "time" -) - -// TestEndLeftoverViewsEndsLiveViews checks that EndLeftoverViews ends a running view with every process in it and leaves a process that only shares the launcher's command line. -func TestEndLeftoverViewsEndsLiveViews(t *testing.T) { - requireView(t) - f := newFixture(t) - w := &loopbackWorld{dir: f.world} - token := fmt.Sprintf("oac-leftover-%d", time.Now().UnixNano()) - v, err := Start(context.Background(), f.spec(w, "wait", "OAC_VIEW_TOKEN="+token)) - if err != nil { - t.Fatalf("Start: %v", err) - } - defer v.Close() - if line, err := bufio.NewReader(v.Stdout()).ReadString('\n'); err != nil || line != "ready\n" { - t.Fatalf("harness said %q, %v", line, err) - } - if n := processesWith(t, token); n != 1 { - t.Fatalf("%d grandchildren in the view, want 1", n) - } - // It has the launcher's command line but is no PID 1 of a view. - other := exec.Command("/bin/sh") - other.Args = []string{launcherArg0} - other.Stdin = strings.NewReader("sleep 60\n") - if err := other.Start(); err != nil { - t.Fatal(err) - } - defer func() { - other.Process.Kill() - other.Wait() - }() - - if err := EndLeftoverViews(5 * time.Second); err != nil { - t.Fatalf("EndLeftoverViews: %v", err) - } - waited := make(chan struct{}) - go func() { - v.Wait() - close(waited) - }() - select { - case <-waited: - case <-time.After(5 * time.Second): - t.Fatal("the view still runs") - } - if n := processesWith(t, token); n != 0 { - t.Errorf("%d processes of the view survived", n) - } - // A killed child stays a zombie until other.Wait reaps it. - if status, err := os.ReadFile(fmt.Sprintf("/proc/%d/status", other.Process.Pid)); err != nil || strings.Contains(string(status), "State:\tZ") { - t.Errorf("a process outside any view was killed: %v", err) - } -} diff --git a/apps/daemon/internal/sessionview/leftover_other.go b/apps/daemon/internal/sessionview/leftover_other.go deleted file mode 100644 index 9c740f34..00000000 --- a/apps/daemon/internal/sessionview/leftover_other.go +++ /dev/null @@ -1,8 +0,0 @@ -//go:build !linux - -package sessionview - -import "time" - -// EndLeftoverViews reports that views need Linux. -func EndLeftoverViews(time.Duration) error { return ErrUnsupported }