diff --git a/.github/workflows/smoke-service-ports.lock.yml b/.github/workflows/smoke-service-ports.lock.yml index f447f6e68c1..0675ab46597 100644 --- a/.github/workflows/smoke-service-ports.lock.yml +++ b/.github/workflows/smoke-service-ports.lock.yml @@ -567,7 +567,7 @@ jobs: env: GH_HOST: github.com - name: Install AWF binary - run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.41 --rootless + run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.41 - name: Determine automatic lockdown mode for GitHub MCP Server id: determine-automatic-lockdown uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) @@ -1001,7 +1001,7 @@ jobs: continue-on-error: true env: AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs - run: bash "${RUNNER_TEMP}/gh-aw/actions/print_firewall_logs.sh" --rootless + run: bash "${RUNNER_TEMP}/gh-aw/actions/print_firewall_logs.sh" - name: Parse token usage for step summary if: always() continue-on-error: true diff --git a/docs/adr/48098-legacy-security-awf-install-non-rootless.md b/docs/adr/48098-legacy-security-awf-install-non-rootless.md new file mode 100644 index 00000000000..deb03bc64f0 --- /dev/null +++ b/docs/adr/48098-legacy-security-awf-install-non-rootless.md @@ -0,0 +1,43 @@ +# ADR-48098: Legacy-Security Mode Installs AWF to Non-Rootless Path + +**Date**: 2026-07-26 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The AWF binary can be installed in two modes: **rootless** (`--rootless`), which places the binary in `~/.local/bin` to minimize privilege surface when `NetworkIsolation` is true; or **non-rootless**, which installs to `/usr/local/bin` for system-wide access. The `legacy-security: enable` sandbox option causes generated workflow steps to invoke AWF as `sudo -E awf`. The `sudo` command enforces its own `secure_path`, which includes `/usr/local/bin` but not `~/.local/bin`. When both `NetworkIsolation` and `LegacySecurity` were true, AWF was installed rootless (`~/.local/bin`) but invoked via `sudo -E awf`, causing `sudo: awf: command not found` at agent startup. The `print_firewall_logs.sh` step had the identical flaw: it conditionally passed `--rootless` based solely on `NetworkIsolation`, ignoring that legacy-security mode grants full sudo access. + +### Decision + +We will skip the `--rootless` install flag whenever `AgentSandboxConfig.LegacySecurity` is true, regardless of the `NetworkIsolation` setting. The condition in `generateAWFInstallationStep` becomes `NetworkIsolation && !Disabled && !LegacySecurity`, and `generateFirewallLogParsingStep` adds the same `!LegacySecurity` guard. This ensures AWF lands in `/usr/local/bin` (on `sudo`'s `secure_path`) in legacy-security mode, making `sudo -E awf` resolvable. + +### Alternatives Considered + +#### Alternative 1: Extend sudo's secure_path to include ~/.local/bin + +Runner provisioning could be modified to add `~/.local/bin` to `sudo`'s `secure_path` via `/etc/sudoers.d/`. This would allow rootless installs to coexist with sudo invocations. However, this requires infrastructure changes outside this codebase's control, and broadening `secure_path` on shared runners increases the attack surface for privilege escalation via user-writable directories. + +#### Alternative 2: Invoke AWF as `sudo -E env PATH=$PATH awf` + +The generated sudo invocation could preserve the full user `PATH` using `sudo -E env PATH=$PATH awf`, avoiding the `secure_path` restriction without changing the install location. This would be a more invasive change touching the AWF invocation pattern across multiple generated steps, increasing regression risk for non-legacy-security paths that already work correctly. + +### Consequences + +#### Positive +- Agent startup no longer fails with `sudo: awf: command not found` in legacy-security mode with network isolation enabled. +- The firewall log parsing step now uses the correct sudo invocation mode, consistent with the AWF install path. + +#### Negative +- AWF is installed to a root-owned directory (`/usr/local/bin`) in legacy-security mode, requiring the install script to run with elevated privileges (this is already the behavior for non-rootless installs). +- The condition `NetworkIsolation && !LegacySecurity` is a compound flag interaction; future maintainers changing either flag's semantics must account for this coupling. + +#### Neutral +- The smoke-service-ports lock file is regenerated to drop the now-incorrect `--rootless` flags, keeping the file consistent with updated compiler behavior. +- Tests explicitly cover the `LegacySecurity=true` + `NetworkIsolation=true` combination for both the install step and the log parsing step, providing a regression guard. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/workflow/copilot_engine_installation.go b/pkg/workflow/copilot_engine_installation.go index aa8a9eb3077..1866dbbd40f 100644 --- a/pkg/workflow/copilot_engine_installation.go +++ b/pkg/workflow/copilot_engine_installation.go @@ -426,7 +426,10 @@ func generateAWFInstallationStep(version string, agentConfig *AgentSandboxConfig // even on standard GitHub-hosted runners where /usr/local is root-owned) and exports // $GITHUB_PATH so the bare awf invocation in later steps resolves correctly. // Also check Disabled to match isAWFNetworkIsolationEnabled() behavior. - if agentConfig != nil && agentConfig.NetworkIsolation && !agentConfig.Disabled { + // + // Exception: legacy-security mode uses `sudo -E awf`, so the binary must be + // installed to /usr/local/bin (the non-rootless path) to be on sudo's secure_path. + if agentConfig != nil && agentConfig.NetworkIsolation && !agentConfig.Disabled && !agentConfig.LegacySecurity { installCmd += " --rootless" } diff --git a/pkg/workflow/engine_firewall_support.go b/pkg/workflow/engine_firewall_support.go index 8a813e3a7de..c91e5a1ff6a 100644 --- a/pkg/workflow/engine_firewall_support.go +++ b/pkg/workflow/engine_firewall_support.go @@ -132,9 +132,11 @@ func generateFirewallLogParsingStep(workflowName string, workflowData *WorkflowD // In network-isolation (rootless) mode, pass --rootless so the script uses // non-interactive sudo (sudo -n) with a non-sudo chmod fallback. In non-network-isolation - // mode, the script uses plain sudo (AWF ran with full sudo access). + // mode, and in legacy-security mode (where AWF ran with full sudo access), the script + // uses plain sudo. scriptArg := "" - if isAWFNetworkIsolationEnabled(workflowData) { + agentCfg := getAgentConfig(workflowData) + if isAWFNetworkIsolationEnabled(workflowData) && !agentCfg.LegacySecurity { scriptArg = " --rootless" } diff --git a/pkg/workflow/engine_firewall_support_test.go b/pkg/workflow/engine_firewall_support_test.go index cca2a28cd5d..d1bc7b725ef 100644 --- a/pkg/workflow/engine_firewall_support_test.go +++ b/pkg/workflow/engine_firewall_support_test.go @@ -330,3 +330,30 @@ func TestGenerateFirewallLogParsingStepWithNetworkIsolationFalse(t *testing.T) { t.Error("Expected no --rootless flag when NetworkIsolation is explicitly false") } } + +func TestGenerateFirewallLogParsingStepLegacySecurityOmitsRootless(t *testing.T) { + // When legacy-security: enable is set, AWF ran with full sudo access, so the + // log parsing script must use plain sudo (no --rootless), even though + // NetworkIsolation defaults to true. + workflowData := &WorkflowData{ + Name: "test-workflow", + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + NetworkIsolation: true, + LegacySecurity: true, + }, + }, + } + step := generateFirewallLogParsingStep("test-workflow", workflowData) + stepContent := strings.Join(step, "\n") + + // Legacy-security mode must NOT pass --rootless to the log parsing script. + if strings.Contains(stepContent, "--rootless") { + t.Error("Expected no --rootless flag when legacy-security: enable is set (AWF has full sudo access)") + } + + if !strings.Contains(stepContent, `bash "${RUNNER_TEMP}/gh-aw/actions/print_firewall_logs.sh"`) { + t.Error("Expected firewall log parsing step to invoke print_firewall_logs.sh") + } +} diff --git a/pkg/workflow/sandbox_custom_agent_test.go b/pkg/workflow/sandbox_custom_agent_test.go index 199662625df..7ff1ef20f7d 100644 --- a/pkg/workflow/sandbox_custom_agent_test.go +++ b/pkg/workflow/sandbox_custom_agent_test.go @@ -99,6 +99,31 @@ func TestCustomAWFConfiguration(t *testing.T) { t.Error("Should not contain --rootless flag when sudo is true (normal mode)") } }) + + t.Run("legacy-security: enable with NetworkIsolation does not pass --rootless", func(t *testing.T) { + // NetworkIsolation defaults to true, but legacy-security mode uses `sudo -E awf` + // which requires awf to be in /usr/local/bin (non-rootless install path). + agentConfig := &AgentSandboxConfig{ + ID: "awf", + NetworkIsolation: true, + LegacySecurity: true, + } + + step := generateAWFInstallationStep("", agentConfig) + stepStr := strings.Join(step, "\n") + + if len(step) == 0 { + t.Error("Expected installation step to be generated for legacy-security mode") + } + + if !strings.Contains(stepStr, "install_awf_binary.sh") { + t.Error("Should contain reference to install_awf_binary.sh script") + } + + if strings.Contains(stepStr, "--rootless") { + t.Error("Should not contain --rootless flag when legacy-security: enable is set (awf must be in /usr/local/bin for sudo)") + } + }) } func TestGetAgentType(t *testing.T) { diff --git a/pkg/workflow/sandbox_network_isolation_rootless_test.go b/pkg/workflow/sandbox_network_isolation_rootless_test.go index 490be615f4c..c275215bfb9 100644 --- a/pkg/workflow/sandbox_network_isolation_rootless_test.go +++ b/pkg/workflow/sandbox_network_isolation_rootless_test.go @@ -139,3 +139,63 @@ This workflow verifies that sudo is omitted by default when sudo is not set (net } }) } + +// TestLegacySecurityInstallNonRootless verifies that when sandbox.agent.legacy-security: enable +// is set, the compiled lock.yml installs awf without --rootless (to /usr/local/bin, which is +// on sudo's secure_path) and invokes it with "sudo -E awf". +func TestLegacySecurityInstallNonRootless(t *testing.T) { + workflowsDir := t.TempDir() + + markdown := `--- +on: + workflow_dispatch: +engine: copilot +strict: false +network: + allowed: + - github.com +sandbox: + agent: + id: awf + sudo: false + legacy-security: enable +--- + +# Test Legacy Security Non-Rootless Install + +This workflow verifies that legacy-security mode installs awf without --rootless. +` + + workflowPath := filepath.Join(workflowsDir, "test-legacy-security.md") + if err := os.WriteFile(workflowPath, []byte(markdown), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Compilation failed: %v", err) + } + + lockPath := filepath.Join(workflowsDir, "test-legacy-security.lock.yml") + lockContent, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read compiled workflow: %v", err) + } + lockStr := string(lockContent) + + // Install step must NOT pass --rootless: awf must land in /usr/local/bin so that + // the subsequent "sudo -E awf" invocation can find it on sudo's secure_path. + if strings.Contains(lockStr, "--rootless") { + t.Error("Expected no '--rootless' flag in install step when legacy-security: enable is set") + } + + // Install step must be present. + if !strings.Contains(lockStr, "install_awf_binary.sh") { + t.Error("Expected install_awf_binary.sh in lock file for legacy-security mode") + } + + // AWF invocation must use sudo -E awf. + if !strings.Contains(lockStr, "sudo -E awf") { + t.Error("Expected 'sudo -E awf' invocation in lock file when legacy-security: enable is set") + } +}