fix isolation startup lifecycle and MCP transport wrapping

This commit is contained in:
lxowalle 2026-04-08 16:48:55 +08:00
parent 4626989407
commit a009e76da2
7 changed files with 57 additions and 28 deletions

View file

@ -99,6 +99,9 @@ func postStartPlatformIsolation(cmd *exec.Cmd, isolation config.IsolationConfig,
return nil return nil
} }
func cleanupPendingPlatformResources(cmd *exec.Cmd) {
}
// buildLinuxBwrapArgs translates the mount plan into the bubblewrap command // buildLinuxBwrapArgs translates the mount plan into the bubblewrap command
// line that re-executes the original process inside the isolated mount view. // line that re-executes the original process inside the isolated mount view.
func buildLinuxBwrapArgs( func buildLinuxBwrapArgs(

View file

@ -17,3 +17,6 @@ func applyPlatformIsolation(cmd *exec.Cmd, isolation config.IsolationConfig, roo
func postStartPlatformIsolation(cmd *exec.Cmd, isolation config.IsolationConfig, root string) error { func postStartPlatformIsolation(cmd *exec.Cmd, isolation config.IsolationConfig, root string) error {
return nil return nil
} }
func cleanupPendingPlatformResources(cmd *exec.Cmd) {
}

View file

@ -101,9 +101,9 @@ func postStartPlatformIsolation(cmd *exec.Cmd, isolation config.IsolationConfig,
} }
return fmt.Errorf("open process for job assignment: %w", err) return fmt.Errorf("open process for job assignment: %w", err)
} }
defer windows.CloseHandle(proc)
if err := windows.AssignProcessToJobObject(job, proc); err != nil { if err := windows.AssignProcessToJobObject(job, proc); err != nil {
_ = windows.CloseHandle(proc)
_ = windows.CloseHandle(job) _ = windows.CloseHandle(job)
if resources.token != 0 { if resources.token != 0 {
_ = resources.token.Close() _ = resources.token.Close()
@ -120,6 +120,20 @@ func postStartPlatformIsolation(cmd *exec.Cmd, isolation config.IsolationConfig,
return nil return nil
} }
func cleanupPendingPlatformResources(cmd *exec.Cmd) {
if cmd == nil {
return
}
resourcesAny, ok := windowsPendingResources.LoadAndDelete(cmd)
if !ok {
return
}
resources, _ := resourcesAny.(windowsProcessResources)
if resources.token != 0 {
_ = resources.token.Close()
}
}
func reapWindowsProcessResources(pid int, proc windows.Handle, job windows.Handle) { func reapWindowsProcessResources(pid int, proc windows.Handle, job windows.Handle) {
_, _ = windows.WaitForSingleObject(proc, windows.INFINITE) _, _ = windows.WaitForSingleObject(proc, windows.INFINITE)
_ = windows.CloseHandle(proc) _ = windows.CloseHandle(proc)

View file

@ -42,7 +42,6 @@ type UserEnv struct {
var ( var (
isolationMu sync.RWMutex isolationMu sync.RWMutex
currentIsolation = config.DefaultConfig().Isolation currentIsolation = config.DefaultConfig().Isolation
currentWorkspace = config.DefaultConfig().WorkspacePath()
) )
// Configure updates the process-wide isolation state used by subsequent child // Configure updates the process-wide isolation state used by subsequent child
@ -53,11 +52,9 @@ func Configure(cfg *config.Config) {
if cfg == nil { if cfg == nil {
defaults := config.DefaultConfig() defaults := config.DefaultConfig()
currentIsolation = defaults.Isolation currentIsolation = defaults.Isolation
currentWorkspace = defaults.WorkspacePath()
return return
} }
currentIsolation = cfg.Isolation currentIsolation = cfg.Isolation
currentWorkspace = filepath.Clean(cfg.WorkspacePath())
} }
// CurrentConfig returns the currently active isolation settings. // CurrentConfig returns the currently active isolation settings.
@ -67,14 +64,6 @@ func CurrentConfig() config.IsolationConfig {
return currentIsolation return currentIsolation
} }
// CurrentWorkspace returns the workspace path currently associated with the
// configured runtime state.
func CurrentWorkspace() string {
isolationMu.RLock()
defer isolationMu.RUnlock()
return currentWorkspace
}
// ResolveInstanceRoot resolves the instance root used to build the isolated // ResolveInstanceRoot resolves the instance root used to build the isolated
// filesystem and redirected user environment. // filesystem and redirected user environment.
func ResolveInstanceRoot() (string, error) { func ResolveInstanceRoot() (string, error) {
@ -111,11 +100,7 @@ func InstanceDirs(root string) []string {
filepath.Join(root, "runtime-user-env", "cache"), filepath.Join(root, "runtime-user-env", "cache"),
filepath.Join(root, "runtime-user-env", "state"), filepath.Join(root, "runtime-user-env", "state"),
} }
workspace := CurrentWorkspace() dirs = append(dirs, filepath.Join(root, pkg.WorkspaceName))
if workspace == "" {
workspace = filepath.Join(root, pkg.WorkspaceName)
}
dirs = append(dirs, workspace)
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
dirs = append(dirs, dirs = append(dirs,
filepath.Join(root, "runtime-user-env", "AppData", "Roaming"), filepath.Join(root, "runtime-user-env", "AppData", "Roaming"),
@ -291,8 +276,10 @@ func BuildLinuxMountPlan(root string, overrides []config.ExposePath) []MountRule
// BuildWindowsAccessRules derives the host-path access policy used by the // BuildWindowsAccessRules derives the host-path access policy used by the
// Windows restricted-token backend. // Windows restricted-token backend.
func BuildWindowsAccessRules(root string, overrides []config.ExposePath) []AccessRule { func BuildWindowsAccessRules(root string, overrides []config.ExposePath) []AccessRule {
rules := []AccessRule{{Path: root, Mode: "rw"}} merged := MergeExposePaths(nil, overrides)
for _, item := range MergeExposePaths(nil, overrides) { rules := make([]AccessRule, 0, len(merged)+1)
rules = append(rules, AccessRule{Path: root, Mode: "rw"})
for _, item := range merged {
rules = append(rules, AccessRule{Path: item.Source, Mode: item.Mode}) rules = append(rules, AccessRule{Path: item.Source, Mode: item.Mode})
} }
return rules return rules
@ -367,6 +354,7 @@ func Start(cmd *exec.Cmd) error {
return err return err
} }
if err := cmd.Start(); err != nil { if err := cmd.Start(); err != nil {
cleanupPendingPlatformResources(cmd)
return err return err
} }
isolation := CurrentConfig() isolation := CurrentConfig()
@ -393,6 +381,7 @@ func Run(cmd *exec.Cmd) error {
return err return err
} }
if err := cmd.Start(); err != nil { if err := cmd.Start(); err != nil {
cleanupPendingPlatformResources(cmd)
return err return err
} }
isolation := CurrentConfig() isolation := CurrentConfig()

View file

@ -7,6 +7,7 @@ import (
"runtime" "runtime"
"testing" "testing"
"github.com/sipeed/picoclaw/pkg"
"github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/config"
) )
@ -35,6 +36,30 @@ func TestPrepareInstanceRoot_CreatesDirectories(t *testing.T) {
} }
} }
func TestInstanceDirs_UsesInstanceWorkspaceNotGlobalState(t *testing.T) {
root := filepath.Join(t.TempDir(), "instance")
cfg := config.DefaultConfig()
cfg.Isolation.Enabled = true
cfg.Agents.Defaults.Workspace = filepath.Join(t.TempDir(), "external-workspace")
Configure(cfg)
t.Cleanup(func() { Configure(config.DefaultConfig()) })
dirs := InstanceDirs(root)
wantWorkspace := filepath.Join(root, pkg.WorkspaceName)
found := false
for _, dir := range dirs {
if dir == wantWorkspace {
found = true
}
if dir == cfg.WorkspacePath() {
t.Fatalf("InstanceDirs() should not depend on process-wide workspace state: %q", dir)
}
}
if !found {
t.Fatalf("InstanceDirs() missing instance workspace dir %q", wantWorkspace)
}
}
func TestIsSupportedOn(t *testing.T) { func TestIsSupportedOn(t *testing.T) {
tests := []struct { tests := []struct {
goos string goos string

View file

@ -220,5 +220,7 @@ func (c *isolatedIOConn) Close() error {
return c.closeErr return c.closeErr
} }
var _ sdkmcp.Transport = (*isolatedCommandTransport)(nil) var (
var _ sdkmcp.Connection = (*isolatedIOConn)(nil) _ sdkmcp.Transport = (*isolatedCommandTransport)(nil)
_ sdkmcp.Connection = (*isolatedIOConn)(nil)
)

View file

@ -16,7 +16,6 @@ import (
"github.com/modelcontextprotocol/go-sdk/mcp" "github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/config"
"github.com/sipeed/picoclaw/pkg/isolation"
"github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/logger"
) )
@ -366,12 +365,6 @@ func (m *Manager) ConnectServer(
env = append(env, fmt.Sprintf("%s=%s", k, v)) env = append(env, fmt.Sprintf("%s=%s", k, v))
} }
cmd.Env = env cmd.Env = env
// Apply the shared isolation preparation before the MCP SDK takes over the
// stdio transport so stdio servers see the same isolated environment.
if err := isolation.PrepareCommand(cmd); err != nil {
return fmt.Errorf("prepare stdio MCP isolation: %w", err)
}
transport = &isolatedCommandTransport{Command: cmd} transport = &isolatedCommandTransport{Command: cmd}
default: default:
return fmt.Errorf( return fmt.Errorf(