feat(execution): add Copilot-native evaluation sandbox - #545
Open
Chris (cwkendall) wants to merge 3 commits into
Open
feat(execution): add Copilot-native evaluation sandbox#545Chris (cwkendall) wants to merge 3 commits into
Chris (cwkendall) wants to merge 3 commits into
Conversation
Delegate OS enforcement to Copilot CLI and MXC while Waza supplies least-privilege task workspace, skill, network, credential, and environment policy. Preserve existing execution when sandboxing is omitted. Closes microsoft#544 Amp-Thread-ID: https://ampcode.com/threads/T-019ffe44-0a2f-7568-a1ff-4a62816cedf2 Co-authored-by: Amp <amp@ampcode.com>
Chris (cwkendall)
requested review from
Charles Lowell (chlowell),
Richard Park (richardpark-msft) and
Shayne Boyer (spboyer)
as code owners
August 19, 2026 03:10
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds schemaVersion 1.3 support to opt Copilot SDK evaluations into Copilot CLI’s native OS sandbox, including path-policy derivation, environment sanitization, and documentation updates.
Changes:
- Introduces
config.sandbox(schema 1.3+) across schema, spec parsing/validation, execution requests, and grader context propagation. - Implements sandbox enforcement in the Copilot executor: workspace/skill path policy, bypass/managed-approval rejection, and sanitized CLI process environment handling.
- Updates docs/examples to schemaVersion 1.3 and adds extensive tests (unit + opt-in live canaries).
Reviewed changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| site/src/content/docs/reference/schema.mdx | Bumps schemaVersion references to 1.3 and documents config.sandbox. |
| site/src/content/docs/reference/schema-changes.md | Updates current versions to 1.3 and adds a 1.3 changelog entry. |
| site/src/content/docs/reference/cli.mdx | Updates waza migrate docs to reference schema 1.3. |
| site/src/content/docs/guides/snapshot-replay.mdx | Updates cross-artifact schemaVersion references to 1.3. |
| site/src/content/docs/guides/eval-yaml.mdx | Documents config.sandbox and adds a sandbox guide section. |
| schemas/eval.schema.json | Adds config.sandbox schema + conditional constraint tying sandbox to executor: copilot-sdk. |
| internal/validation/schema_test.go | Adds schema validation test ensuring sandbox requires copilot-sdk. |
| internal/orchestration/runner_test.go | Updates execution request test to include sandbox propagation. |
| internal/orchestration/runner.go | Propagates sandbox into execution requests and grader context. |
| internal/models/spec_test.go | Adds tests for sandbox loading, strict field rejection, and validation rules. |
| internal/models/spec.go | Adds SandboxConfig, strict YAML unmarshalling, and schema/executor validation. |
| internal/models/schema_version.go | Bumps CurrentSchemaVersion to 1.3 with commentary. |
| internal/graders/prompt_grader_test.go | Verifies sandbox is passed through prompt grader execution requests. |
| internal/graders/prompt_grader.go | Propagates sandbox into prompt grader execution requests. |
| internal/graders/grader.go | Adds sandbox field to grader context. |
| internal/execution/workspace_test.go | Adds test that workspace capture rejects symlinks. |
| internal/execution/sdkclient_test.go | Adds tests for env sanitization, auth precedence, and shared-client keying. |
| internal/execution/sdkclient.go | Adds sanitized CLI env handling, proxy filtering, and shared-client key changes. |
| internal/execution/engine.go | Extends execution request with sandbox config. |
| internal/execution/copilot_test.go | Adds sandbox policy/path validation tests and opt-in live canary tests. |
| internal/execution/copilot_engine_test.go | Adds tests for sandbox prerequisites and shutdown cleanup behavior. |
| internal/execution/copilot_client_wrappers.go | Adds session wrapper method to configure Copilot sandbox + permissions. |
| internal/execution/copilot.go | Implements sandbox configuration flow, permission wrapping, workspace handling, and symlink-safe capture. |
| examples/custom-agent/eval.yaml | Updates example to schema 1.3 and enables sandbox with explicit skill dirs. |
| examples/custom-agent/README.md | Documents the example as a runnable sandbox isolation demo. |
| docs/design/195-copilot-native-eval-sandbox.md | Adds design doc for Copilot-native sandboxing approach and contracts. |
| docs/PRD.md | Updates date and adds an epic entry for sandboxing model-visible commands. |
| docs/INTEGRATION-TESTING.md | Documents how to enable and test the native command sandbox. |
| docs/GUIDE.md | Adds user-facing guide section on configuring sandboxed evals. |
| cmd/waza/cmd_run.go | Enables environment sanitization automatically when sandbox is enabled. |
| cmd/waza/cmd_migrate_test.go | Makes migrate tests track models.CurrentSchemaVersion dynamically. |
| README.md | Updates schema 1.3 references and adds Copilot-native sandbox documentation. |
| AGENTS.md | Updates fixture isolation description to account for sandboxed workspace roots. |
Files not reviewed (2)
- cmd/waza/copilot_client_wrapper_mocks_test.go: Generated file
- internal/execution/copilot_client_wrapper_mocks_test.go: Generated file
Suppressed comments (1)
site/src/content/docs/reference/cli.mdx:1
- This sentence mixes “current schema version” with the fact that
waza migrateonly performs changes on future major migrations. To avoid confusion (since “current” varies by artifact type and minor bumps don’t require migration), consider wording like: “The current major schema version is1(current minor1.3), so … no file changes are made.”
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Canonicalize the denied temporary root, inspect bypass requests through typed SDK fields, make workspace capture race-safe without per-file path resolution, and reject unsupported schema majors. Keep the existing custom-agent example unchanged. Issue: microsoft#544 Amp-Thread-ID: https://ampcode.com/threads/T-019ffe44-0a2f-7568-a1ff-4a62816cedf2 Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- cmd/waza/copilot_client_wrapper_mocks_test.go: Generated file
- internal/execution/copilot_client_wrapper_mocks_test.go: Generated file
Suppressed comments (2)
schemas/eval.schema.json:128
- The conditional schema enforces
config.executor == "copilot-sdk"only whenexecutoris present, because thethenbranch does not require theexecutorproperty. Ifexecutoris optional elsewhere in the schema,{config: {sandbox: ...}}could pass validation without asserting the executor constraint. Consider addingrequired: ["executor"]inside thethen’sconfigsubschema (or otherwise ensuring the conditional both requires and constrainsexecutorwhensandboxis present).
"allOf": [
{
"if": {
"properties": {
"config": {
"required": [
"sandbox"
]
}
}
},
"then": {
"properties": {
"config": {
"properties": {
"executor": {
"const": "copilot-sdk"
}
}
}
}
}
}
],
internal/execution/sdkclient.go:158
sanitizedCLIEnvallocates and populates large allowlist maps on every call. Since the allowlist is static, consider hoistingallowed(andproxyVariables) to package-level vars to reduce per-call allocations and work, especially if the shared client is created repeatedly in tests or in multi-engine scenarios.
func sanitizedCLIEnv(environ []string) []string {
allowed := map[string]bool{
"PATH": true, "HOME": true, "USER": true, "USERNAME": true,
"LOGNAME": true, "SHELL": true, "TERM": true, "COLORTERM": true,
"TMPDIR": true, "TEMP": true, "TMP": true, "LANG": true, "TZ": true,
"HTTP_PROXY": true, "HTTPS_PROXY": true, "ALL_PROXY": true, "NO_PROXY": true,
"SSL_CERT_FILE": true, "SSL_CERT_DIR": true, "NODE_EXTRA_CA_CERTS": true,
"REQUESTS_CA_BUNDLE": true, "CURL_CA_BUNDLE": true, "NIX_SSL_CERT_FILE": true,
"GIT_SSL_CAINFO": true, "COPILOT_HOME": true,
"LC_ALL": true, "LC_COLLATE": true, "LC_CTYPE": true, "LC_MESSAGES": true,
"LC_MONETARY": true, "LC_NUMERIC": true, "LC_TIME": true, "LC_PAPER": true,
"LC_NAME": true, "LC_ADDRESS": true, "LC_TELEPHONE": true,
"LC_MEASUREMENT": true, "LC_IDENTIFICATION": true,
"XDG_CONFIG_HOME": true, "XDG_CACHE_HOME": true, "XDG_DATA_HOME": true,
"XDG_STATE_HOME": true, "XDG_RUNTIME_DIR": true,
"XDG_DATA_DIRS": true, "XDG_CONFIG_DIRS": true,
"SYSTEMROOT": true, "COMSPEC": true, "PATHEXT": true, "USERPROFILE": true,
"APPDATA": true, "LOCALAPPDATA": true,
}
Match the existing documentation style, make path overlap checks filesystem-aware, and keep the sandbox environment allowlist platform-correct. Issue: microsoft#544 Amp-Thread-ID: https://ampcode.com/threads/T-019ffe44-0a2f-7568-a1ff-4a62816cedf2 Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- cmd/waza/copilot_client_wrapper_mocks_test.go: Generated file
- internal/execution/copilot_client_wrapper_mocks_test.go: Generated file
Suppressed comments (2)
internal/execution/copilot.go:198
copilotClientOptionsis called with an empty CLI path in the customNewCopilotClientbranch, which makes the behavior diverge from the production path-resolution logic insharedClientOptions(COPILOT_CLI_PATH / embedded CLI lookup + “refuse PATH fallback”). Consider either (a) resolving the CLI path here too (same rules assharedClientOptions) so custom client factories get consistent, fully-initialized options, or (b) documenting clearly that custom factories are responsible for settingStdioConnection.Pathand accepting any resulting PATH fallback behavior.
copilotOptions := copilotClientOptions("error", cliArgs, "", sanitizeEnvironment)
client = options.NewCopilotClient(copilotOptions)
internal/execution/copilot_client_wrappers.go:135
- These failures return generic errors without enough context to debug operational issues (e.g., which workspace/skill dirs were being configured, or whether the rejection was for options vs permissions). Consider including key context (workspaceDir, number of readonly dirs) and, if available from the RPC response types, any diagnostic/message field returned by Copilot so operators can understand why a host rejected the sandbox policy.
updated, err := w.inner.RPC.Options.Update(ctx, options)
if err != nil {
return err
}
if !updated.Success {
return fmt.Errorf("copilot rejected the sandbox configuration")
}
if permissions == nil {
return nil
}
configured, err := w.inner.RPC.Permissions.Configure(ctx, permissions)
if err != nil {
return err
}
if !configured.Success {
return fmt.Errorf("copilot rejected the workspace permission boundary")
}
Comment on lines
+105
to
+128
| "allOf": [ | ||
| { | ||
| "if": { | ||
| "properties": { | ||
| "config": { | ||
| "required": [ | ||
| "sandbox" | ||
| ] | ||
| } | ||
| } | ||
| }, | ||
| "then": { | ||
| "properties": { | ||
| "config": { | ||
| "properties": { | ||
| "executor": { | ||
| "const": "copilot-sdk" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ], |
|
Chris (@cwkendall) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated no new comments.
Files not reviewed (2)
- cmd/waza/copilot_client_wrapper_mocks_test.go: Generated file
- internal/execution/copilot_client_wrapper_mocks_test.go: Generated file
Suppressed comments (6)
internal/models/spec.go:129
- The JSON schema requires
config.sandbox.enabled, but the Go model will acceptsandbox: {}(or a missingenabled) and silently treat it as disabled becauseEnabledis a non-pointer bool. To keep runtime behavior consistent with the published schema/docs, enforce presence ofenabledwhen the sandbox block exists (e.g., makeEnableda*booland validate non-nil, or haveUnmarshalYAMLexplicitly error when theenabledkey is missing in a mapping node).
// SandboxConfig controls Copilot CLI's native sandbox for model-visible tools.
type SandboxConfig struct {
Enabled bool `yaml:"enabled" json:"enabled"`
AllowDevToolCaches bool `yaml:"allow_dev_tool_caches,omitempty" json:"allow_dev_tool_caches,omitempty"`
AllowOutboundNetwork bool `yaml:"allow_outbound_network,omitempty" json:"allow_outbound_network,omitempty"`
AllowLocalNetwork bool `yaml:"allow_local_network,omitempty" json:"allow_local_network,omitempty"`
GitAuth bool `yaml:"git_auth,omitempty" json:"git_auth,omitempty"`
GHAuth bool `yaml:"gh_auth,omitempty" json:"gh_auth,omitempty"`
ReadonlyPaths []string `yaml:"readonly_paths,omitempty" json:"readonly_paths,omitempty"`
ReadwritePaths []string `yaml:"readwrite_paths,omitempty" json:"readwrite_paths,omitempty"`
}
func (s *SandboxConfig) UnmarshalYAML(node *yaml.Node) error {
allowed := map[string]bool{
"enabled": true, "allow_dev_tool_caches": true,
"allow_outbound_network": true, "allow_local_network": true,
"git_auth": true, "gh_auth": true,
"readonly_paths": true, "readwrite_paths": true,
}
if node.Kind == yaml.MappingNode {
for i := 0; i < len(node.Content); i += 2 {
if field := node.Content[i].Value; !allowed[field] {
return fmt.Errorf("unknown sandbox field %q", field)
}
}
}
type rawSandboxConfig SandboxConfig
var decoded rawSandboxConfig
if err := node.Decode(&decoded); err != nil {
return err
}
*s = SandboxConfig(decoded)
return nil
internal/execution/copilot.go:191
copilotClientOptions(..., cliPath="", ...)sets up acopilot.StdioConnectionwithout a resolved CLI path. If any non-test caller providesNewCopilotClientand then starts a real CLI process, this can reintroduce PATH fallback / ambiguous binary selection, which undermines the “refusing PATH fallback” posture used elsewhere. Consider either (a) resolvingcliPaththe same way assharedClientOptions(COPILOT_CLI_PATH or embedded) before building options, or (b) explicitly documenting/enforcing that custom client factories must provide a fully specified connection path.
sanitizeEnvironment := options != nil && options.SanitizeEnvironment
internal/execution/copilot.go:198
copilotClientOptions(..., cliPath="", ...)sets up acopilot.StdioConnectionwithout a resolved CLI path. If any non-test caller providesNewCopilotClientand then starts a real CLI process, this can reintroduce PATH fallback / ambiguous binary selection, which undermines the “refusing PATH fallback” posture used elsewhere. Consider either (a) resolvingcliPaththe same way assharedClientOptions(COPILOT_CLI_PATH or embedded) before building options, or (b) explicitly documenting/enforcing that custom client factories must provide a fully specified connection path.
client = SharedClient(SharedClientOptions{CLIArgs: cliArgs, SanitizeEnvironment: sanitizeEnvironment})
} else {
copilotOptions := copilotClientOptions("error", cliArgs, "", sanitizeEnvironment)
client = options.NewCopilotClient(copilotOptions)
schemas/eval.schema.json:128
- The
ifschema does not requireconfigto be present. In JSON Schema,propertiesdoes not fail when a property is missing, so theifcondition can evaluate as “true” for instances withoutconfig, causing thethenbranch (executor const) to apply more broadly than intended. A concrete fix is to add"required": ["config"]inside theif, and (optionally) also constrainconfigto be an object in that condition to avoid surprising matches.
"allOf": [
{
"if": {
"properties": {
"config": {
"required": [
"sandbox"
]
}
}
},
"then": {
"properties": {
"config": {
"properties": {
"executor": {
"const": "copilot-sdk"
}
}
}
}
}
}
],
internal/execution/copilot_client_wrappers.go:126
- These failures are hard to diagnose because they don’t include which RPC was rejected and don’t surface any available response details. Consider including the operation name (Options.Update vs Permissions.Configure) and key context (e.g., workspaceDir), and if the RPC responses expose any error/reason fields, append them to the error message to make troubleshooting sandbox policy failures actionable.
updated, err := w.inner.RPC.Options.Update(ctx, options)
if err != nil {
return err
}
if !updated.Success {
return fmt.Errorf("copilot rejected the sandbox configuration")
}
internal/execution/copilot_client_wrappers.go:136
- These failures are hard to diagnose because they don’t include which RPC was rejected and don’t surface any available response details. Consider including the operation name (Options.Update vs Permissions.Configure) and key context (e.g., workspaceDir), and if the RPC responses expose any error/reason fields, append them to the error message to make troubleshooting sandbox policy failures actionable.
configured, err := w.inner.RPC.Permissions.Configure(ctx, permissions)
if err != nil {
return err
}
if !configured.Success {
return fmt.Errorf("copilot rejected the workspace permission boundary")
}
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in
config.sandboxpolicy forcopilot-sdkevaluations so model-visible commands run inside Copilot CLI/MXC's native OS sandbox. Waza remains a thin policy adapter: it declares the task workspace, skills, explicit prerequisites, network/auth posture, and non-interactive approval policy while Copilot owns Seatbelt, bubblewrap, and ProcessContainer enforcement. Omitting the block preserves existing evaluation behaviour.The design and ownership boundary are documented in
docs/design/195-copilot-native-eval-sandbox.md.Related issue
Closes #544
Agent handoff
internal/execution/copilot.go,internal/execution/copilot_client_wrappers.go,internal/execution/sdkclient.go,internal/models/spec.go,schemas/eval.schema.json, anddocs/design/195-copilot-native-eval-sandbox.md.Type of change
Validation
go test ./...make lintorgolangci-lint run-go vet ./...passes; the pinned golangci-lint binary was unavailable locally, so the required CI lint job remains the authoritative gate.npm --prefix site run build)web/changed - not applicable; no dashboard files changed.go generate ./internal/execution ./cmd/wazago build -o bin/waza ./cmd/wazagit diff --checkDocumentation
site/docs updated, if CLI, YAML, dashboard, or validator behavior changedRisk and rollback
config.sandboxretain their previous behaviour.Notes for reviewers
Please focus on the ownership boundary in the design document, the native RPC policy mapping in
sessionSandboxConfiguration, the process-level environment keying insdkclient.go, and the live/concurrent isolation tests. The implementation deliberately avoids custom Seatbelt profiles, bubblewrap wrappers, shell parsing, chroot, or container orchestration.