diff --git a/README.md b/README.md index 939fbdc..afd6a09 100644 --- a/README.md +++ b/README.md @@ -177,6 +177,11 @@ export MANAGED_AGENT_SANDBOX=docker go run ./cmd/managed-agent orchestrate ``` +`MANAGED_AGENT_SANDBOX` is a strict deployment-level provider selection: +`local` is the offline default and `docker` is the isolated opt-in. Unknown +provider names fail startup. Provider-specific configuration is intentionally +kept out of the Managed Agents public API. + Do not run workers with different model or sandbox configuration on the same Temporal Task Queue. diff --git a/cmd/managed-agent/main.go b/cmd/managed-agent/main.go index 349e9ea..628eec2 100644 --- a/cmd/managed-agent/main.go +++ b/cmd/managed-agent/main.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "os/signal" + "strings" "syscall" "time" @@ -34,6 +35,11 @@ const defaultAddr = "127.0.0.1:8080" // guardModelSandbox. const unsafeLocalSandboxEnv = "MANAGED_AGENT_ALLOW_UNSAFE_LOCAL_SANDBOX" +const ( + sandboxProviderEnv = "MANAGED_AGENT_SANDBOX" + sandboxImageEnv = "MANAGED_AGENT_SANDBOX_IMAGE" +) + // resolveRuntime returns the self-hosted agent core and whether it is backed by // a real (network-backed) model. It uses the real Messages API client when // MANAGED_AGENT_MODEL_BASE_URL and MANAGED_AGENT_MODEL_API_KEY are set, @@ -58,26 +64,52 @@ func resolveRuntime() (rt agentruntime.AgentRuntime, realModel bool, err error) return agentruntime.NewAgentCore(client, domain.NewRandomIDGen()), realModel, nil } -// resolveSandboxProvider selects the sandbox backend from the environment and -// reports whether the selection is the local (non-isolating) provider. The -// default is the offline, dev-grade local provider so the binary runs (and -// tests stay offline) with no configuration. Set MANAGED_AGENT_SANDBOX=docker -// to opt into the Docker-backed provider, which gives real isolation (Linux -// namespaces/cgroups + --network none). MANAGED_AGENT_SANDBOX_IMAGE overrides -// the container image (NewDockerProvider defaults to alpine:latest when empty). +// sandboxProviderRegistry declares the adapters compiled into this worker. +// Factories are lazy: selecting local never initializes Docker, and future +// optional remote adapters will not require credentials unless selected. +func sandboxProviderRegistry() (*sandbox.ProviderRegistry, error) { + return sandbox.NewProviderRegistry( + sandbox.ProviderRegistration{ + Name: sandbox.LocalProviderName, + Factory: func() (sandbox.Provider, error) { + return sandbox.NewLocalProvider(), nil + }, + }, + sandbox.ProviderRegistration{ + Name: sandbox.DockerProviderName, + Factory: func() (sandbox.Provider, error) { + return sandbox.NewDockerProvider(sandbox.DockerConfig{ + DefaultImage: os.Getenv(sandboxImageEnv), + }) + }, + }, + ) +} + +// resolveSandboxProvider selects the process-wide sandbox backend from the +// internal registry and reports whether it is the local non-isolating provider. +// Provider choice is deployment configuration, not part of the Managed Agents +// public API. Unknown names fail closed instead of silently falling back to +// host execution. func resolveSandboxProvider() (p sandbox.Provider, isLocal bool, err error) { - if os.Getenv("MANAGED_AGENT_SANDBOX") == "docker" { - dp, err := sandbox.NewDockerProvider(sandbox.DockerConfig{ - DefaultImage: os.Getenv("MANAGED_AGENT_SANDBOX_IMAGE"), - }) - if err != nil { - return nil, false, err - } - log.Printf("sandbox: docker provider (real isolation)") - return dp, false, nil + name := strings.TrimSpace(os.Getenv(sandboxProviderEnv)) + if name == "" { + name = sandbox.LocalProviderName + } + registry, err := sandboxProviderRegistry() + if err != nil { + return nil, false, err + } + provider, err := registry.Open(name) + if err != nil { + return nil, false, err + } + if name == sandbox.LocalProviderName { + log.Printf("sandbox: local provider (dev-grade guardrail, not a security boundary)") + return provider, true, nil } - log.Printf("sandbox: local provider (dev-grade guardrail, not a security boundary)") - return sandbox.NewLocalProvider(), true, nil + log.Printf("sandbox: %s provider", name) + return provider, false, nil } // guardModelSandbox refuses to start when a real, network-backed model is paired diff --git a/cmd/managed-agent/main_test.go b/cmd/managed-agent/main_test.go index 13990c7..26bc9f2 100644 --- a/cmd/managed-agent/main_test.go +++ b/cmd/managed-agent/main_test.go @@ -54,6 +54,34 @@ func TestResolveSandboxProvider_DefaultsToLocal(t *testing.T) { } } +func TestResolveSandboxProvider_RejectsUnknownSelection(t *testing.T) { + t.Setenv(sandboxProviderEnv, "dockre") + _, _, err := resolveSandboxProvider() + if err == nil { + t.Fatal("resolveSandboxProvider accepted an unknown provider") + } + if !strings.Contains(err.Error(), `unsupported provider "dockre"`) || + !strings.Contains(err.Error(), "available: docker, local") { + t.Fatalf("resolveSandboxProvider error = %q", err) + } +} + +func TestSandboxProviderRegistry_IsLazy(t *testing.T) { + t.Setenv("PATH", "") + t.Setenv(sandboxImageEnv, "unused.invalid/image") + registry, err := sandboxProviderRegistry() + if err != nil { + t.Fatal(err) + } + provider, err := registry.Open(sandbox.LocalProviderName) + if err != nil { + t.Fatalf("opening local initialized optional Docker provider: %v", err) + } + if provider.Name() != sandbox.LocalProviderName { + t.Fatalf("provider name = %q", provider.Name()) + } +} + func TestResolveRuntime_UsesFakeModelWithoutEnv(t *testing.T) { t.Setenv("MANAGED_AGENT_MODEL_BASE_URL", "") t.Setenv("MANAGED_AGENT_MODEL_API_KEY", "") diff --git a/docs/architecture.md b/docs/architecture.md index fe9b20d..46a04af 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -86,7 +86,7 @@ model vendor, sandbox backend, or worker topology. | `internal/store` | Deprecated SQLite compatibility implementation | | `internal/agentruntime` | Model/tool orchestration behind `AgentRuntime` | | `internal/model` | Offline and Messages API model clients | -| `internal/sandbox` | Local and Docker execution providers | +| `internal/sandbox` | Provider registry, lifecycle contract, and local/Docker adapters | The dependency direction points inward: transport and infrastructure depend on application/domain semantics, while the domain has no HTTP, SQL, model-client, diff --git a/docs/getting-started.md b/docs/getting-started.md index 8bffebe..966481a 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -146,6 +146,10 @@ export MANAGED_AGENT_SANDBOX=docker go run ./cmd/managed-agent orchestrate ``` +The provider name is validated strictly. The compiled choices are currently +`local` and `docker`; an unknown value fails worker startup and never silently +falls back to local host execution. + The configured endpoint must expose an Anthropic-shaped `/v1/messages` API. Do not run workers with different model or sandbox configuration on the same Temporal Task Queue. Keep credentials in the environment and never commit them. diff --git a/docs/sandboxes.md b/docs/sandboxes.md index cc395f9..4a558f3 100644 --- a/docs/sandboxes.md +++ b/docs/sandboxes.md @@ -20,7 +20,11 @@ Temporal Activity -> SessionManager -> sandbox.Provider -> sandbox.Sandbox `SessionManager` gives each session one logical sandbox. PostgreSQL persists the provider name and opaque external ID; a restarted worker calls `Attach` instead of creating an empty replacement. `Sandbox` exposes command execution, confined -file access, a workspace root, and teardown. +file access, a workspace root, and teardown. The execution worker selects one +compiled adapter through an internal registry. `MANAGED_AGENT_SANDBOX` accepts +`local` (the default) or `docker`; an unknown name fails startup instead of +falling back to host execution. Provider selection does not add fields to the +Managed Agents Environment or Session APIs. ## Support levels @@ -60,6 +64,12 @@ A backend implements the core lifecycle contract when it can: 5. read and write paths relative to the workspace; 6. destroy the resource idempotently. +These requirements are executable in +`internal/sandbox/sandboxtest`. Both built-in providers run the same suite, +including cross-client Create/Attach, workspace preservation, ownership +rejection, cancellation, and post-delete missing-reference behavior. +Provider-specific tests cover isolation and resource controls separately. + The built-in toolset currently assumes a POSIX-like environment with `/bin/sh`, `find`, and `grep`. A backend that does not provide those commands is not compatible with all executing built-ins yet. @@ -82,9 +92,9 @@ not compatible with all executing built-ins yet. Remote adapters must use the same lifecycle tests. Production deployments still need orphan reconciliation for the create-before-binding crash window, provider -health reporting, and a provider registry suitable for heterogeneous workers. -Pause, snapshot, fork, quotas, and eviction remain optional capabilities rather -than requirements of the core interface. +health reporting, and provider-aware task routing when heterogeneous workers +share a control plane. Pause, snapshot, fork, quotas, and eviction remain +optional capabilities rather than requirements of the core interface. The upstream behavior informing the session/environment distinction is documented in Claude's @@ -100,6 +110,8 @@ A backend contribution should: - keep its external dependency optional and fail fast when explicitly selected but unavailable; +- register a lazy factory under a stable lowercase provider name and pass the + shared `sandboxtest` lifecycle suite; - preserve the session-scoped ownership contract; - document its trust boundary, network defaults, resource controls, host requirements, and unsupported lifecycle features; diff --git a/internal/sandbox/conformance_test.go b/internal/sandbox/conformance_test.go new file mode 100644 index 0000000..abcca6c --- /dev/null +++ b/internal/sandbox/conformance_test.go @@ -0,0 +1,46 @@ +package sandbox_test + +import ( + "os/exec" + "testing" + "time" + + "github.com/yanpgwang/managed-agent-go/internal/sandbox" + "github.com/yanpgwang/managed-agent-go/internal/sandbox/sandboxtest" +) + +func TestLocalProviderConformance(t *testing.T) { + sandboxtest.Run(t, sandboxtest.Config{ + NewProvider: func(*testing.T) sandbox.Provider { + return sandbox.NewLocalProvider() + }, + Spec: sandbox.Spec{Timeout: 30 * time.Second}, + }) +} + +func TestDockerProviderConformance(t *testing.T) { + sandboxtest.Run(t, sandboxtest.Config{ + NewProvider: func(t *testing.T) sandbox.Provider { + t.Helper() + if _, err := exec.LookPath("docker"); err != nil { + t.Skip("docker not installed") + } + if err := exec.Command( + "docker", + "version", + "--format", + "{{.Server.Version}}", + ).Run(); err != nil { + t.Skip("docker daemon not reachable") + } + provider, err := sandbox.NewDockerProvider(sandbox.DockerConfig{ + DefaultImage: "alpine:latest", + }) + if err != nil { + t.Fatal(err) + } + return provider + }, + Spec: sandbox.Spec{Timeout: 30 * time.Second}, + }) +} diff --git a/internal/sandbox/docker.go b/internal/sandbox/docker.go index c12260f..747ec82 100644 --- a/internal/sandbox/docker.go +++ b/internal/sandbox/docker.go @@ -15,7 +15,6 @@ import ( ) const ( - dockerProviderName = "docker" dockerManagedLabel = "io.mango.managed" dockerSessionKeyLabel = "io.mango.session_key" ) @@ -74,7 +73,7 @@ func NewDockerProvider(cfg DockerConfig) (Provider, error) { return &dockerProvider{dockerPath: path, defaultImage: image}, nil } -func (p *dockerProvider) Name() string { return dockerProviderName } +func (p *dockerProvider) Name() string { return DockerProviderName } // runDocker invokes the docker CLI with the given args, capturing stdout/stderr // (capped) and the process exit code. The passed ctx bounds the whole call. diff --git a/internal/sandbox/local.go b/internal/sandbox/local.go index ae60b19..7c15817 100644 --- a/internal/sandbox/local.go +++ b/internal/sandbox/local.go @@ -13,8 +13,6 @@ import ( "time" ) -const localProviderName = "local" - // maxOutput caps the bytes captured from stdout and stderr, independently. const maxOutput = 100_000 @@ -36,7 +34,7 @@ func NewLocalProvider() Provider { return &localProvider{baseDir: filepath.Join(os.TempDir(), "managed-agent-sandboxes")} } -func (p *localProvider) Name() string { return localProviderName } +func (p *localProvider) Name() string { return LocalProviderName } func (p *localProvider) Create( ctx context.Context, @@ -49,11 +47,7 @@ func (p *localProvider) Create( if sessionKey == "" { return Ref{}, nil, errors.New("sandbox: session key is required") } - root := spec.WorkDir - if root == "" { - sum := sha256.Sum256([]byte(sessionKey)) - root = filepath.Join(p.baseDir, fmt.Sprintf("session-%x", sum[:16])) - } + root := p.rootFor(sessionKey, spec) if err := os.MkdirAll(root, 0o700); err != nil { return Ref{}, nil, fmt.Errorf("sandbox: create root: %w", err) } @@ -85,6 +79,20 @@ func (p *localProvider) Attach( ref.Provider, )) } + expectedRoot, err := canonicalLocalPath(p.rootFor(sessionKey, spec)) + if err != nil { + return nil, fmt.Errorf("sandbox: resolve expected local workspace: %w", err) + } + referencedRoot, err := canonicalLocalPath(ref.ID) + if err != nil { + return nil, fmt.Errorf("sandbox: resolve referenced local workspace: %w", err) + } + if referencedRoot != expectedRoot { + return nil, Permanent(fmt.Errorf( + "sandbox: local workspace %q belongs to another session", + ref.ID, + )) + } info, err := os.Stat(ref.ID) if errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("%w: local workspace %q", ErrNotFound, ref.ID) @@ -98,6 +106,45 @@ func (p *localProvider) Attach( return p.attachRoot(ref.ID, spec) } +// canonicalLocalPath resolves symlinks through the deepest existing ancestor. +// This lets Attach validate session ownership before reporting a missing +// deterministic workspace, even when an operator removed the whole local +// provider base directory. +func canonicalLocalPath(value string) (string, error) { + absolute, err := filepath.Abs(value) + if err != nil { + return "", err + } + current := absolute + var missing []string + for { + resolved, err := filepath.EvalSymlinks(current) + if err == nil { + for i := len(missing) - 1; i >= 0; i-- { + resolved = filepath.Join(resolved, missing[i]) + } + return resolved, nil + } + if !errors.Is(err, os.ErrNotExist) { + return "", err + } + parent := filepath.Dir(current) + if parent == current { + return "", err + } + missing = append(missing, filepath.Base(current)) + current = parent + } +} + +func (p *localProvider) rootFor(sessionKey string, spec Spec) string { + if spec.WorkDir != "" { + return spec.WorkDir + } + sum := sha256.Sum256([]byte(sessionKey)) + return filepath.Join(p.baseDir, fmt.Sprintf("session-%x", sum[:16])) +} + func (p *localProvider) attachRoot(root string, spec Spec) (Sandbox, error) { // Resolve symlinks so confinement checks compare canonical paths (e.g. on // darwin /tmp is a symlink to /private/tmp). diff --git a/internal/sandbox/local_test.go b/internal/sandbox/local_test.go index 6939ee7..7a5879c 100644 --- a/internal/sandbox/local_test.go +++ b/internal/sandbox/local_test.go @@ -2,6 +2,9 @@ package sandbox import ( "context" + "errors" + "os" + "path/filepath" "strings" "testing" "time" @@ -98,3 +101,24 @@ func TestLocal_AttachPreservesWorkspace(t *testing.T) { t.Fatalf("attached data = %q, err=%v", data, err) } } + +func TestLocal_AttachReportsNotFoundAfterBaseDirectoryLoss(t *testing.T) { + ctx := context.Background() + base := filepath.Join(t.TempDir(), "provider-base") + provider := &localProvider{baseDir: base} + ref, box, err := provider.Create(ctx, t.Name(), Spec{}) + if err != nil { + t.Fatal(err) + } + if err := box.Destroy(ctx); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(base); err != nil { + t.Fatal(err) + } + + _, err = provider.Attach(ctx, t.Name(), ref, Spec{}) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("Attach after base loss = %v, want ErrNotFound", err) + } +} diff --git a/internal/sandbox/registry.go b/internal/sandbox/registry.go new file mode 100644 index 0000000..b718a73 --- /dev/null +++ b/internal/sandbox/registry.go @@ -0,0 +1,129 @@ +package sandbox + +import ( + "errors" + "fmt" + "sort" + "strings" +) + +const ( + // LocalProviderName and DockerProviderName are persisted in sandbox + // bindings. They are part of the internal storage compatibility contract and + // must remain stable across worker releases. + LocalProviderName = "local" + DockerProviderName = "docker" +) + +// ProviderFactory constructs one provider client from worker-local +// configuration. Factories are invoked only for the selected provider, so an +// optional adapter never requires credentials or dependencies merely because +// it is registered. +type ProviderFactory func() (Provider, error) + +// ProviderRegistration is one deployment-selectable sandbox adapter. +type ProviderRegistration struct { + Name string + Factory ProviderFactory +} + +// ProviderRegistry resolves a deployment-level provider name to its adapter. +// It is deliberately internal to worker composition: provider mechanics do not +// belong in the Managed Agents Environment or Session wire models. +// +// A registry is immutable after construction and safe for concurrent reads. +type ProviderRegistry struct { + factories map[string]ProviderFactory + names []string +} + +// NewProviderRegistry validates and freezes the available provider set without +// invoking any factory. +func NewProviderRegistry(registrations ...ProviderRegistration) (*ProviderRegistry, error) { + if len(registrations) == 0 { + return nil, errors.New("sandbox: provider registry requires at least one registration") + } + registry := &ProviderRegistry{ + factories: make(map[string]ProviderFactory, len(registrations)), + names: make([]string, 0, len(registrations)), + } + for _, registration := range registrations { + if err := validateProviderName(registration.Name); err != nil { + return nil, err + } + if registration.Factory == nil { + return nil, fmt.Errorf( + "sandbox: provider %q has no factory", + registration.Name, + ) + } + if _, exists := registry.factories[registration.Name]; exists { + return nil, fmt.Errorf( + "sandbox: provider %q is registered more than once", + registration.Name, + ) + } + registry.factories[registration.Name] = registration.Factory + registry.names = append(registry.names, registration.Name) + } + sort.Strings(registry.names) + return registry, nil +} + +// Open constructs the selected provider and verifies that the adapter reports +// the same stable name under which it was registered. +func (r *ProviderRegistry) Open(name string) (Provider, error) { + if r == nil { + return nil, errors.New("sandbox: provider registry is required") + } + factory, ok := r.factories[name] + if !ok { + return nil, fmt.Errorf( + "sandbox: unsupported provider %q (available: %s)", + name, + strings.Join(r.names, ", "), + ) + } + provider, err := factory() + if err != nil { + return nil, fmt.Errorf("sandbox: initialize provider %q: %w", name, err) + } + if provider == nil { + return nil, fmt.Errorf("sandbox: provider %q factory returned nil", name) + } + if provider.Name() != name { + return nil, fmt.Errorf( + "sandbox: provider registered as %q reports name %q", + name, + provider.Name(), + ) + } + return provider, nil +} + +// Names returns a sorted copy of the registered provider names. +func (r *ProviderRegistry) Names() []string { + if r == nil { + return nil + } + return append([]string(nil), r.names...) +} + +func validateProviderName(name string) error { + if name == "" { + return errors.New("sandbox: provider name is required") + } + for i := 0; i < len(name); i++ { + c := name[i] + if (c >= 'a' && c <= 'z') || + (i > 0 && c >= '0' && c <= '9') || + (i > 0 && c == '-') { + continue + } + return fmt.Errorf( + "sandbox: invalid provider name %q (use a lowercase ASCII identifier)", + name, + ) + } + return nil +} diff --git a/internal/sandbox/registry_test.go b/internal/sandbox/registry_test.go new file mode 100644 index 0000000..6436453 --- /dev/null +++ b/internal/sandbox/registry_test.go @@ -0,0 +1,161 @@ +package sandbox + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" +) + +type registryTestProvider struct { + name string +} + +func (p *registryTestProvider) Name() string { return p.name } + +func (*registryTestProvider) Create( + context.Context, + string, + Spec, +) (Ref, Sandbox, error) { + panic("not used") +} + +func (*registryTestProvider) Attach( + context.Context, + string, + Ref, + Spec, +) (Sandbox, error) { + panic("not used") +} + +func registryFactory(name string) ProviderFactory { + return func() (Provider, error) { + return ®istryTestProvider{name: name}, nil + } +} + +func TestProviderRegistry_OpensSelectedFactoryLazily(t *testing.T) { + var localCalls, dockerCalls int + registry, err := NewProviderRegistry( + ProviderRegistration{ + Name: LocalProviderName, + Factory: func() (Provider, error) { + localCalls++ + return ®istryTestProvider{name: LocalProviderName}, nil + }, + }, + ProviderRegistration{ + Name: DockerProviderName, + Factory: func() (Provider, error) { + dockerCalls++ + return ®istryTestProvider{name: DockerProviderName}, nil + }, + }, + ) + if err != nil { + t.Fatal(err) + } + if localCalls != 0 || dockerCalls != 0 { + t.Fatal("registry construction invoked an optional provider factory") + } + + provider, err := registry.Open(LocalProviderName) + if err != nil { + t.Fatal(err) + } + if provider.Name() != LocalProviderName { + t.Fatalf("provider name = %q, want %q", provider.Name(), LocalProviderName) + } + if localCalls != 1 || dockerCalls != 0 { + t.Fatalf("factory calls local=%d docker=%d, want 1/0", localCalls, dockerCalls) + } + if got := registry.Names(); !reflect.DeepEqual( + got, + []string{DockerProviderName, LocalProviderName}, + ) { + t.Fatalf("Names() = %v", got) + } +} + +func TestProviderRegistry_RejectsInvalidRegistrations(t *testing.T) { + cases := []struct { + name string + registrations []ProviderRegistration + want string + }{ + {name: "empty", want: "at least one"}, + { + name: "missing name", + registrations: []ProviderRegistration{{ + Factory: registryFactory("local"), + }}, + want: "name is required", + }, + { + name: "non-canonical name", + registrations: []ProviderRegistration{{ + Name: "Open_Sandbox", Factory: registryFactory("Open_Sandbox"), + }}, + want: "invalid provider name", + }, + { + name: "missing factory", + registrations: []ProviderRegistration{{ + Name: "local", + }}, + want: "has no factory", + }, + { + name: "duplicate", + registrations: []ProviderRegistration{ + {Name: "local", Factory: registryFactory("local")}, + {Name: "local", Factory: registryFactory("local")}, + }, + want: "more than once", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := NewProviderRegistry(tc.registrations...) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("NewProviderRegistry() error = %v, want containing %q", err, tc.want) + } + }) + } +} + +func TestProviderRegistry_RejectsUnknownAndBrokenFactories(t *testing.T) { + sentinel := errors.New("bad credentials") + registry, err := NewProviderRegistry( + ProviderRegistration{Name: "local", Factory: registryFactory("local")}, + ProviderRegistration{Name: "broken", Factory: func() (Provider, error) { + return nil, sentinel + }}, + ProviderRegistration{Name: "nil", Factory: func() (Provider, error) { + return nil, nil + }}, + ProviderRegistration{Name: "alias", Factory: registryFactory("different")}, + ) + if err != nil { + t.Fatal(err) + } + + if _, err := registry.Open("missing"); err == nil || + !strings.Contains(err.Error(), "available: alias, broken, local, nil") { + t.Fatalf("unknown provider error = %v", err) + } + if _, err := registry.Open("broken"); !errors.Is(err, sentinel) { + t.Fatalf("factory error = %v, want wrapped sentinel", err) + } + if _, err := registry.Open("nil"); err == nil || + !strings.Contains(err.Error(), "returned nil") { + t.Fatalf("nil factory error = %v", err) + } + if _, err := registry.Open("alias"); err == nil || + !strings.Contains(err.Error(), `registered as "alias" reports name "different"`) { + t.Fatalf("name mismatch error = %v", err) + } +} diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 9332cdc..192509e 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -105,6 +105,8 @@ type Sandbox interface { ReadFile(ctx context.Context, path string) ([]byte, error) WriteFile(ctx context.Context, path string, data []byte) error Root() string + // Destroy is idempotent. Repeating it after the resource is already gone + // must succeed so deletion workflows can safely retry a lost acknowledgement. Destroy(ctx context.Context) error } diff --git a/internal/sandbox/sandboxtest/conformance.go b/internal/sandbox/sandboxtest/conformance.go new file mode 100644 index 0000000..bc880a7 --- /dev/null +++ b/internal/sandbox/sandboxtest/conformance.go @@ -0,0 +1,238 @@ +// Package sandboxtest provides the lifecycle conformance suite every Mango +// sandbox provider must pass. +package sandboxtest + +import ( + "bytes" + "context" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/yanpgwang/managed-agent-go/internal/sandbox" +) + +// Factory returns a fresh client for the same provider deployment. It may call +// t.Skip when an optional daemon or credential is unavailable. +type Factory func(t *testing.T) sandbox.Provider + +// Config describes the portable POSIX surface exercised by the suite. +type Config struct { + NewProvider Factory + Spec sandbox.Spec + ShellPath string +} + +// Run exercises the provider behavior required by SessionManager and the +// built-in tool runtime. Provider-specific isolation and capability tests remain +// alongside the adapter. +func Run(t *testing.T, cfg Config) { + t.Helper() + if cfg.NewProvider == nil { + t.Fatal("sandboxtest: NewProvider is required") + } + if cfg.Spec.Timeout == 0 { + cfg.Spec.Timeout = 30 * time.Second + } + if cfg.ShellPath == "" { + cfg.ShellPath = "/bin/sh" + } + + t.Run("stable_identity", func(t *testing.T) { + first := cfg.NewProvider(t) + second := cfg.NewProvider(t) + if first == nil || second == nil { + t.Fatal("provider factory returned nil") + } + if first.Name() == "" { + t.Fatal("provider name is empty") + } + if second.Name() != first.Name() { + t.Fatalf( + "provider name changed across clients: %q != %q", + first.Name(), + second.Name(), + ) + } + }) + + t.Run("execution_and_files", func(t *testing.T) { + ctx := context.Background() + provider := cfg.NewProvider(t) + _, box, err := provider.Create(ctx, sessionKey(t), cfg.Spec) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = box.Destroy(context.Background()) }) + + if box.Root() == "" { + t.Fatal("sandbox root is empty") + } + content := []byte{'d', 'u', 'r', 'a', 'b', 'l', 'e', 0, '\n'} + if err := box.WriteFile(ctx, "nested/state.bin", content); err != nil { + t.Fatalf("WriteFile: %v", err) + } + got, err := box.ReadFile(ctx, "nested/state.bin") + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !bytes.Equal(got, content) { + t.Fatalf("file round trip = %q, want %q", got, content) + } + + result, err := box.Exec(ctx, sandbox.Command{ + Path: cfg.ShellPath, + Args: []string{"-c", "printf conformance-exec"}, + }) + if err != nil { + t.Fatalf("Exec: %v", err) + } + if result.ExitCode != 0 || string(result.Stdout) != "conformance-exec" { + t.Fatalf("Exec result = %+v", result) + } + result, err = box.Exec(ctx, sandbox.Command{ + Path: cfg.ShellPath, + Args: []string{"-c", "exit 7"}, + }) + if err != nil { + t.Fatalf("Exec non-zero exit: %v", err) + } + if result.ExitCode != 7 { + t.Fatalf("non-zero exit code = %d, want 7", result.ExitCode) + } + + if _, err := box.ReadFile(ctx, "../escape"); err == nil { + t.Fatal("ReadFile accepted a path outside the workspace") + } + if err := box.WriteFile(ctx, "../escape", []byte("x")); err == nil { + t.Fatal("WriteFile accepted a path outside the workspace") + } + + cancelled, cancel := context.WithCancel(ctx) + cancel() + if _, err := box.Exec(cancelled, sandbox.Command{ + Path: cfg.ShellPath, + Args: []string{"-c", "true"}, + }); !errors.Is(err, context.Canceled) { + t.Fatalf("Exec with cancelled context = %v, want context.Canceled", err) + } + }) + + t.Run("idempotent_create_and_restart_attach", func(t *testing.T) { + ctx := context.Background() + session := sessionKey(t) + firstProvider := cfg.NewProvider(t) + firstRef, first, err := firstProvider.Create(ctx, session, cfg.Spec) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = first.Destroy(context.Background()) }) + if firstRef.Provider != firstProvider.Name() || firstRef.ID == "" { + t.Fatalf("invalid durable reference: %+v", firstRef) + } + if err := first.WriteFile(ctx, "restart.txt", []byte("preserved")); err != nil { + t.Fatal(err) + } + + restartedProvider := cfg.NewProvider(t) + sameRef, same, err := restartedProvider.Create(ctx, session, cfg.Spec) + if err != nil { + t.Fatalf("repeated Create: %v", err) + } + if sameRef != firstRef { + t.Fatalf("repeated Create ref = %+v, want %+v", sameRef, firstRef) + } + assertFile(t, ctx, same, "restart.txt", "preserved") + + attached, err := restartedProvider.Attach(ctx, session, firstRef, cfg.Spec) + if err != nil { + t.Fatalf("Attach: %v", err) + } + assertFile(t, ctx, attached, "restart.txt", "preserved") + }) + + t.Run("ownership_and_missing_reference", func(t *testing.T) { + ctx := context.Background() + session := sessionKey(t) + provider := cfg.NewProvider(t) + ref, box, err := provider.Create(ctx, session, cfg.Spec) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = box.Destroy(context.Background()) }) + + if _, err := provider.Attach( + ctx, + session+"-other", + ref, + cfg.Spec, + ); err == nil || !sandbox.IsPermanent(err) { + t.Fatalf("cross-session Attach = %v, want permanent ownership error", err) + } + if _, err := provider.Attach( + ctx, + session, + sandbox.Ref{Provider: "wrong-provider", ID: ref.ID}, + cfg.Spec, + ); err == nil || !sandbox.IsPermanent(err) { + t.Fatalf("wrong-provider Attach = %v, want permanent error", err) + } + + }) + + t.Run("idempotent_destroy", func(t *testing.T) { + ctx := context.Background() + session := sessionKey(t) + provider := cfg.NewProvider(t) + ref, box, err := provider.Create(ctx, session, cfg.Spec) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = box.Destroy(context.Background()) }) + + if err := box.Destroy(ctx); err != nil { + t.Fatalf("first Destroy: %v", err) + } + if err := box.Destroy(ctx); err != nil { + t.Fatalf("repeated Destroy: %v", err) + } + restartedProvider := cfg.NewProvider(t) + if _, err := restartedProvider.Attach( + ctx, + session, + ref, + cfg.Spec, + ); !errors.Is(err, sandbox.ErrNotFound) { + t.Fatalf("Attach after Destroy = %v, want ErrNotFound", err) + } + }) +} + +func assertFile( + t *testing.T, + ctx context.Context, + box sandbox.Sandbox, + path string, + want string, +) { + t.Helper() + got, err := box.ReadFile(ctx, path) + if err != nil { + t.Fatalf("ReadFile(%q): %v", path, err) + } + if string(got) != want { + t.Fatalf("ReadFile(%q) = %q, want %q", path, got, want) + } +} + +func sessionKey(t *testing.T) string { + t.Helper() + name := strings.NewReplacer( + "/", "-", + " ", "-", + "_", "-", + ).Replace(strings.ToLower(t.Name())) + return fmt.Sprintf("sesn-conformance-%s-%d", name, time.Now().UnixNano()) +} diff --git a/internal/sandbox/session_test.go b/internal/sandbox/session_test.go index 98f6ed0..de8562b 100644 --- a/internal/sandbox/session_test.go +++ b/internal/sandbox/session_test.go @@ -433,7 +433,18 @@ func (p *createTrackingProvider) Create( sessionKey string, spec Spec, ) (Ref, Sandbox, error) { + // This resilience double deliberately returns a distinct resource for each + // Create so the manager's losing-bind cleanup path remains testable even + // though conforming production providers are idempotent by session key. + root, err := os.MkdirTemp("", "mango-bind-election-*") + if err != nil { + return Ref{}, nil, err + } + spec.WorkDir = root ref, box, err := p.inner.Create(ctx, sessionKey, spec) + if err != nil { + _ = os.RemoveAll(root) + } if box != nil { p.createdRoot = box.Root() } @@ -447,13 +458,16 @@ func (p *createTrackingProvider) Attach( spec Spec, ) (Sandbox, error) { p.attachments.Add(1) + // The test double's unique local path is its ownership record. + spec.WorkDir = ref.ID return p.inner.Attach(ctx, sessionKey, ref, spec) } func TestSessionManager_DestroysLosingCreateAndAttachesBindingWinner(t *testing.T) { ctx := context.Background() inner := NewLocalProvider() - winnerRef, winnerBox, err := inner.Create(ctx, "winner-resource", Spec{}) + provider := &createTrackingProvider{inner: inner} + winnerRef, winnerBox, err := provider.Create(ctx, "sesn_election", Spec{}) if err != nil { t.Fatal(err) } @@ -462,7 +476,7 @@ func TestSessionManager_DestroysLosingCreateAndAttachesBindingWinner(t *testing. t.Fatal(err) } - provider := &createTrackingProvider{inner: inner} + provider.createdRoot = "" bindings := &authoritativeBindingStore{winner: Binding{ SessionID: "sesn_election", Ref: winnerRef,