-
Notifications
You must be signed in to change notification settings - Fork 495
Default sandbox.agent.sudo to false (network isolation) #42354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b4d12c4
b601dba
460d754
489d02a
3cb86c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # ADR-42354: Default sandbox.agent.sudo to False (Network Isolation) | ||
|
|
||
| **Date**: 2026-06-29 | ||
| **Status**: Draft | ||
| **Deciders**: Unknown | ||
|
|
||
| --- | ||
|
|
||
| ### Context | ||
|
|
||
| The `sandbox.agent` configuration controls how the AWF (Agentic Workflow Firewall) process is launched when running AI agents in GitHub Actions workflows. Previously, omitting the `sudo` field in `sandbox.agent` frontmatter was equivalent to `sudo: true`, causing AWF to be invoked as `sudo -E awf` — granting the agent elevated host-level access by default. This "permissive by default" posture conflicted with the security principle of least privilege. Any workflow that did not explicitly configure `sudo: false` unknowingly ran in a more privileged mode. The goal of this change is to make network isolation (rootless mode) the safe default, requiring explicit opt-in for elevated access. | ||
|
|
||
| ### Decision | ||
|
|
||
| We will change the global default for `sandbox.agent.sudo` from `true` (host-access/sudo mode) to `false` (network isolation/rootless mode). When `sudo` is omitted from the frontmatter, `NetworkIsolation=true` will be set and AWF will run without `sudo`. Explicitly setting `sudo: true` will still work but will emit a compile-time error in strict mode and a warning in non-strict mode, signaling that the field is deprecated and its use should be intentional. | ||
|
|
||
| ### Alternatives Considered | ||
|
|
||
| #### Alternative 1: Keep sudo: true as the Default (Status Quo) | ||
|
|
||
| The existing behavior could be retained, requiring operators to explicitly set `sudo: false` to enable network isolation. This was rejected because security-by-default is strongly preferable: most workflows do not require host-level access, and relying on operators to opt into a safer mode leaves a large surface area exposed by inaction or oversight. | ||
|
|
||
| #### Alternative 2: Remove the sudo Option Entirely and Always Use Network Isolation | ||
|
|
||
| The `sudo` field could be removed from the schema so that all workflows unconditionally run in rootless/network-isolation mode. This was rejected because some legitimate workflows may currently depend on `sudo: true` for reasons not yet eliminated. A hard removal without a deprecation path would be a breaking change with no escape hatch; the warning/error feedback mechanism preserves discoverability while signaling the direction of travel. | ||
|
|
||
| ### Consequences | ||
|
|
||
| #### Positive | ||
| - Workflows that omit `sudo` now default to the more secure rootless network-isolation mode, reducing the default attack surface for AI agents. | ||
| - Explicit `sudo: true` usage is surfaced at compile time (error in strict mode, warning otherwise), giving operators visibility into elevated-privilege configurations. | ||
| - Aligns the sandbox defaults with the security principle of least privilege. | ||
|
|
||
| #### Negative | ||
| - Existing workflows that omit `sudo` and relied on the old default (`sudo -E awf`) will silently switch to rootless mode, which may break workflows that require host-level access or sudo networking. | ||
| - The `SudoExplicitlyEnabled` sentinel field adds complexity to `AgentSandboxConfig`, requiring callers and test code to distinguish between "sudo not set" and "sudo set to false." | ||
| - All golden files and tests that previously asserted `sudo -E awf` as the default output must be updated, increasing the scope of a seemingly small default change. | ||
|
|
||
| #### Neutral | ||
| - The change does not alter the YAML serialization format; `sudo: true` and `sudo: false` remain valid frontmatter values. | ||
| - The deprecation path for `sudo: true` (strict error vs. non-strict warning) introduces two distinct enforcement modes whose behavior differences may need to be documented for operators. | ||
|
|
||
| --- | ||
|
|
||
| *ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,21 +46,22 @@ type SandboxConfig struct { | |
|
|
||
| // AgentSandboxConfig represents the agent sandbox configuration | ||
| type AgentSandboxConfig struct { | ||
| ID string `yaml:"id,omitempty"` // Agent ID: "awf" or "srt" (replaces Type in new object format) | ||
| Type SandboxType `yaml:"type,omitempty"` // Sandbox type: "awf" or "srt" (legacy, use ID instead) | ||
| Version string `yaml:"version,omitempty"` // AWF version override used to install and run the matching firewall version | ||
| Platform string `yaml:"platform,omitempty"` // AWF platform.type override (github.com, ghes, ghec, ghec-self-hosted) | ||
| NetworkIsolation bool `yaml:"sudo,omitempty"` // Internal: true = isolation mode (AWF --network-isolation). Frontmatter sudo: false maps to NetworkIsolation=true; sudo: true or omitted maps to NetworkIsolation=false. | ||
| Disabled bool `yaml:"-"` // True when agent is explicitly set to false (disables firewall). This is a runtime flag, not serialized to YAML. | ||
| DisableReason string `yaml:"-"` // Operator-authored justification from dangerously-disable-sandbox-agent feature; available for diagnostics and audit logging. | ||
| Config *SandboxRuntimeConfig `yaml:"config,omitempty"` // Custom SRT config (optional) | ||
| Command string `yaml:"command,omitempty"` // Custom command to replace AWF or SRT installation | ||
| Args []string `yaml:"args,omitempty"` // Additional arguments to append to the command | ||
| Env map[string]string `yaml:"env,omitempty"` // Environment variables to set on the step | ||
| Mounts []string `yaml:"mounts,omitempty"` // Container mounts to add for AWF (format: "source:dest:mode") | ||
| Memory string `yaml:"memory,omitempty"` // Memory limit for the AWF container (e.g., "4g", "8g") | ||
| ModelFallback *TemplatableBool `yaml:"model-fallback,omitempty"` // AWF API proxy model fallback enable/disable flag (optional) | ||
| Targets map[string]*AgentAPIProxyTargetConfig `yaml:"targets,omitempty"` // Per-provider API proxy target overrides keyed by provider name (e.g. "openai", "anthropic") | ||
| ID string `yaml:"id,omitempty"` // Agent ID: "awf" or "srt" (replaces Type in new object format) | ||
| Type SandboxType `yaml:"type,omitempty"` // Sandbox type: "awf" or "srt" (legacy, use ID instead) | ||
| Version string `yaml:"version,omitempty"` // AWF version override used to install and run the matching firewall version | ||
| Platform string `yaml:"platform,omitempty"` // AWF platform.type override (github.com, ghes, ghec, ghec-self-hosted) | ||
| NetworkIsolation bool `yaml:"sudo,omitempty"` // Internal: true = isolation mode (AWF --network-isolation). Frontmatter sudo: false (or omitted) maps to NetworkIsolation=true; sudo: true maps to NetworkIsolation=false. | ||
| SudoExplicitlyEnabled bool `yaml:"-"` // True when sudo: true was explicitly set in frontmatter. Used to emit an error (strict) or warning (non-strict) at compile time. | ||
| Disabled bool `yaml:"-"` // True when agent is explicitly set to false (disables firewall). This is a runtime flag, not serialized to YAML. | ||
| DisableReason string `yaml:"-"` // Operator-authored justification from dangerously-disable-sandbox-agent feature; available for diagnostics and audit logging. | ||
| Config *SandboxRuntimeConfig `yaml:"config,omitempty"` // Custom SRT config (optional) | ||
| Command string `yaml:"command,omitempty"` // Custom command to replace AWF or SRT installation | ||
| Args []string `yaml:"args,omitempty"` // Additional arguments to append to the command | ||
| Env map[string]string `yaml:"env,omitempty"` // Environment variables to set on the step | ||
| Mounts []string `yaml:"mounts,omitempty"` // Container mounts to add for AWF (format: "source:dest:mode") | ||
| Memory string `yaml:"memory,omitempty"` // Memory limit for the AWF container (e.g., "4g", "8g") | ||
| ModelFallback *TemplatableBool `yaml:"model-fallback,omitempty"` // AWF API proxy model fallback enable/disable flag (optional) | ||
| Targets map[string]*AgentAPIProxyTargetConfig `yaml:"targets,omitempty"` // Per-provider API proxy target overrides keyed by provider name (e.g. "openai", "anthropic") | ||
| } | ||
|
|
||
| // AgentAPIProxyTargetConfig configures a single LLM provider's API proxy target. | ||
|
|
@@ -169,7 +170,8 @@ func applySandboxDefaults(sandboxConfig *SandboxConfig, engineConfig *EngineConf | |
| sandboxLog.Print("No sandbox config found, creating default with agent: awf") | ||
| sandboxConfig = &SandboxConfig{ | ||
| Agent: &AgentSandboxConfig{ | ||
| Type: SandboxTypeAWF, | ||
| Type: SandboxTypeAWF, | ||
| NetworkIsolation: true, // Default: sudo: false (network isolation enabled) | ||
| }, | ||
| } | ||
| ensureDefaultAgentWritePath(sandboxConfig) | ||
|
|
@@ -188,7 +190,8 @@ func applySandboxDefaults(sandboxConfig *SandboxConfig, engineConfig *EngineConf | |
| if sandboxConfig.Agent == nil { | ||
| sandboxLog.Print("Sandbox config exists without agent, setting default agent: awf") | ||
| sandboxConfig.Agent = &AgentSandboxConfig{ | ||
| Type: SandboxTypeAWF, | ||
| Type: SandboxTypeAWF, | ||
| NetworkIsolation: true, // Default: sudo: false (network isolation enabled) | ||
| } | ||
| ensureDefaultAgentWritePath(sandboxConfig) | ||
| return sandboxConfig | ||
|
|
@@ -205,6 +208,12 @@ func applySandboxDefaults(sandboxConfig *SandboxConfig, engineConfig *EngineConf | |
| sandboxConfig.Agent.Type = SandboxTypeAWF | ||
| } | ||
|
|
||
| // Apply the default sudo: false (network isolation) when sudo was not explicitly | ||
| // set to true in frontmatter. This ensures network isolation is the default. | ||
| if !sandboxConfig.Agent.SudoExplicitlyEnabled { | ||
| sandboxConfig.Agent.NetworkIsolation = true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The legacy When 💡 Suggested fixEither apply the new default inside the legacy early-return path, or emit a deprecation warning so operators know their config is outside the new policy: if sandboxConfig.Type != "" {
sandboxLog.Printf("Sandbox config uses legacy Type field: %s, preserving it", sandboxConfig.Type)
// Apply network isolation default to Agent if present.
if sandboxConfig.Agent != nil && !sandboxConfig.Agent.SudoExplicitlyEnabled {
sandboxConfig.Agent.NetworkIsolation = true
}
ensureDefaultAgentWritePath(sandboxConfig)
return sandboxConfig
}If the legacy path is intentionally excluded, document that explicitly in the function comment so reviewers and operators can audit their workflow inventory.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The However, the early-return path at line 183–186 (when if sandboxConfig.Type != "" {
ensureDefaultAgentWritePath(sandboxConfig)
return sandboxConfig // ← bypasses the NetworkIsolation default below
}For frontmatter-sourced configs this is safe because A defensive fix or a comment explaining the invariant would prevent future regressions: if sandboxConfig.Type != "" {
// NetworkIsolation is already set by extractAgentSandboxConfig; skip re-defaulting.
ensureDefaultAgentWritePath(sandboxConfig)\n return sandboxConfig\n}\n```\n\n@copilot please address this. |
||
| } | ||
|
|
||
| ensureDefaultAgentWritePath(sandboxConfig) | ||
| return sandboxConfig | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The object-format branch now correctly defaults
NetworkIsolation = truehere, but the string-format branch (line ~164, unchanged) still returns&AgentSandboxConfig{Type: agentType}withNetworkIsolation = false.The default is fixed up later by
applySandboxDefaultsvia the!SudoExplicitlyEnabledguard, so current behavior is correct. However, any future code readingagentConfig.NetworkIsolationfrom the pre-defaultssandboxConfig(e.g., insiderunPostEngineValidations) would seefalsefor string-format agents, which contradicts the stated default.Consider making the default eager and consistent:
@copilot please address this.