feat: omit sudo from generated lock.yml when network-isolation is enabled; bump firewall to v0.27.10 and mcpg to v0.3.30

[Copilot]

When sandbox.agent.network-isolation: true is set, AWF uses container-based egress (not host-iptables) so it can run rootless — required for ARC/Kubernetes runners where passwordless sudo is unavailable. The compiler was still emitting sudo unconditionally.

Changes

Rootless execution for network-isolation workflows

• install_awf_binary.sh — adds --rootless flag; replaces all sudo calls with a maybe_sudo() helper that becomes a no-op when the flag is passed
• copilot_engine_installation.go — generateAWFInstallationStep appends --rootless when agentConfig.NetworkIsolation is true
• awf_helpers.go — GetAWFCommandPrefix returns "awf" instead of "sudo -E awf" when network-isolation is active
• engine_firewall_support.go — generateFirewallLogParsingStep now accepts workflowData and skips the sudo chmod -R a+rX permission-fix when network-isolation is enabled (rootless AWF doesn't create root-owned files)

Legacy (non-isolation) topology is unchanged — still uses sudo -E awf.

Version bumps

Image	Old	New
ghcr.io/github/gh-aw-firewall/*	v0.27.9	v0.27.10
ghcr.io/github/gh-aw-mcpg	v0.3.29	v0.3.30

251 lock files recompiled; ghcr.io/github/gh-aw-mcpg:v0.3.30 pin added to actions-lock.json.

With network-isolation enabled, the generated lock now looks like:

# Install AWF binary (no sudo)
run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.10 --rootless

# Run AWF (no sudo -E)
run: |
  awf --config ... --build-local -- /bin/bash -c '...'

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #41269 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold).

[github-actions]

🧪 Test quality analysis by Test Quality Sentinel · 86.3 AIC · ⌖ 13.3 AIC · ⊞ 8.4K · ◷

[github-actions]

✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%).

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 80/100 — Excellent

Analyzed 9 test case(s): 9 design, 0 implementation, 0 guideline violation(s).

📊 Metrics & Test Classification (9 tests analyzed)

Metric	Value
New/modified tests analyzed	9
✅ Design tests (behavioral contracts)	9 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	6 (67%)
Duplicate test clusters	0
Test inflation detected	Yes — awf_helpers_test.go +60 lines vs awf_helpers.go +7 lines (8.6:1 ratio); see note below
🚨 Coding-guideline violations	0 (Go mock libraries / missing build tags / no assertion messages)

Test	File	Classification	Issues Detected
TestGetAWFCommandPrefixNetworkIsolation/returns awf (no sudo) when network-isolation is enabled	pkg/workflow/awf_helpers_test.go	✅ Design	—
TestGetAWFCommandPrefixNetworkIsolation/returns sudo -E awf when network-isolation is disabled	pkg/workflow/awf_helpers_test.go	✅ Design	—
TestGetAWFCommandPrefixNetworkIsolation/returns sudo -E awf when no sandbox config is set	pkg/workflow/awf_helpers_test.go	✅ Design	Edge case: nil SandboxConfig
TestGetAWFCommandPrefixNetworkIsolation/custom command takes precedence over network-isolation	pkg/workflow/awf_helpers_test.go	✅ Design	Edge case: override precedence
TestGenerateFirewallLogParsingStepNetworkIsolationOmitsSudo	pkg/workflow/engine_firewall_support_test.go	✅ Design	—
TestCustomAWFConfiguration/network-isolation agent config passes --rootless to install script	pkg/workflow/sandbox_custom_agent_test.go	✅ Design	—
TestCustomAWFConfiguration/non-isolation agent config does not pass --rootless	pkg/workflow/sandbox_custom_agent_test.go	✅ Design	Negative case
TestNetworkIsolationRootless/network-isolation workflow omits sudo from awf invocation and install (integration)	pkg/workflow/sandbox_network_isolation_rootless_test.go	✅ Design	Full compilation check, multiple absence assertions
TestNetworkIsolationRootless/legacy workflow (no network-isolation) still uses sudo -E awf (integration)	pkg/workflow/sandbox_network_isolation_rootless_test.go	✅ Design	Regression guard: legacy sudo preserved

Go: 9 (*_test.go); JavaScript: 0. Other languages detected but not scored.

Score breakdown: Behavioral (40/40) + Edge Cases (20/30) + Duplication (20/20) + Proportional Growth (0/10 — inflation flag on awf_helpers_test.go)

Inflation note: awf_helpers_test.go added 60 lines vs awf_helpers.go +7 lines (8.6:1 ratio). This triggers the mechanical inflation penalty but does not reflect poor quality — GetAWFCommandPrefix is a compact function and the 4-subtest coverage (nil config, disabled, enabled, custom command override) is appropriate.

Build tags: All four test files carry correct //go:build tags (!integration for unit tests, integration for the new end-to-end file). ✅

Verdict

✅ Check passed. 0% implementation tests (threshold: 30%). All 9 new tests verify observable behavioral contracts — output strings, compile-time lock.yml contents, and flag-passing semantics — with no mock-library usage and no missing build tags.

[github-actions]

Skills-Based Review 🧠

Applied /diagnose, /tdd, and /grill-with-docs — leaving as a comment (no blocking issues, but a few items worth addressing).

📋 Key Themes & Highlights

Key Themes

• Threat-detection gap: buildInstallAWFForExternalDetectorStep always passes nil for agentConfig, meaning threat-detection workflows can never receive --rootless. If this is intentional, a comment should say so.
• /usr/local/bin writability assumption: maybe_sudo removes the privilege escalation but the install target is still a system directory. The script relies on the container running as root (or the dir being group-writable), which is a reasonable ARC assumption but is undocumented.
• Warning to stdout: The unknown-argument warning in install_awf_binary.sh goes to stdout rather than stderr.
• Integration-only end-to-end test: TestNetworkIsolationRootless runs only under //go:build integration. Unit tests provide solid function-level coverage, but the full compile round-trip is only exercised in integration mode.

Positive Highlights

• ✅ maybe_sudo helper is a clean, self-contained abstraction — easy to audit
• ✅ Strong unit test suite: TestGetAWFCommandPrefixNetworkIsolation, TestGenerateFirewallLogParsingStepNetworkIsolationOmitsSudo, and the install-step tests all cover both the new rootless and legacy sudo paths
• ✅ Legacy topology is explicitly unchanged, minimising regression risk
• ✅ isAWFNetworkIsolationEnabled in firewall.go is nil-safe throughout the call chain
• ✅ Clear PR description with before/after lock snippets and a well-structured changeset entry

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 120.5 AIC · ⌖ 8.06 AIC · ⊞ 6.6K

[github-actions]

[/grill-with-docs] Warning messages should go to stderr (>&2), not stdout. Emitting warnings on stdout can pollute the version-check output or cause callers that parse stdout to misinterpret the output.

💡 One-line fix

# before
*) echo "WARNING: Unknown argument: $arg" ;;

# after
*) echo "WARNING: Unknown argument: $arg" >&2 ;;

This is consistent with the ERROR: messages in the same script, which also go to stderr (or would benefit from doing so).

[github-actions]

[/diagnose] In rootless mode maybe_sudo becomes a no-op, but AWF_INSTALL_DIR is still hardcoded to /usr/local/bin. On runners where the current user cannot write to /usr/local/bin (and sudo is unavailable), the mkdir/cp/tee/mv calls will fail with a Permission denied error that may be hard to diagnose.

💡 Consider an early-exit guard or a user-writable fallback

Add an early writability check when --rootless is active:

if [ "$ROOTLESS" = "true" ] && [ ! -w "$AWF_INSTALL_DIR" ]; then
  echo "ERROR: rootless install requires $AWF_INSTALL_DIR to be writable by the current user." >&2
  exit 1
fi

Alternatively document the assumption that ARC/Kubernetes containers run as root inside the container (which would make /usr/local/bin writable without sudo), so future maintainers understand why no fallback is needed.

[github-actions]

[/tdd] The end-to-end compilation test is gated behind //go:build integration, so it won't run in standard go test ./... CI. The unit tests (TestGetAWFCommandPrefixNetworkIsolation, TestGenerateFirewallLogParsingStepNetworkIsolationOmitsSudo, etc.) do provide good function-level coverage — this note is just to confirm the build tag is intentional and that integration tests are part of the regular test pipeline for this feature.

📋 Context

If the integration test is the only place that tests the full compile-to-lock-file round-trip for the rootless path, any regression in how WorkflowData is wired through the compiler chain (outside the individual functions) would go undetected in fast CI. Consider whether a simpler golden-file unit test (no full compile) would give comparable confidence without the integration build constraint.

[github-actions]

[/tdd] TestGenerateFirewallLogParsingStepFixesFirewallPermissions passes nil for workflowData to cover the legacy sudo-chmod path, which exercises the nil-safety of isAWFNetworkIsolationEnabled. However, there is no test for a non-nil workflowData with NetworkIsolation: false — i.e., confirming that an explicitly configured, non-isolated sandbox still emits sudo chmod.

💡 Suggested additional sub-test

t.Run("non-isolation workflow with explicit NetworkIsolation=false still chmod with sudo", func(t *testing.T) {
    workflowData := &WorkflowData{
        Name: "test-workflow",
        SandboxConfig: &SandboxConfig{
            Agent: &AgentSandboxConfig{
                ID:               "awf",
                NetworkIsolation: false,
            },
        },
    }
    step := generateFirewallLogParsingStep("test-workflow", workflowData)
    stepContent := strings.Join(step, "\n")
    if !strings.Contains(stepContent, "sudo chmod -R a+rX") {
        t.Error("Expected sudo chmod when NetworkIsolation is explicitly false")
    }
})

This closes the gap between nil (no config) and false (config present, not isolated).

[Copilot]

Pull request overview

Updates the gh-aw workflow compiler to support rootless AWF execution when sandbox.agent.network-isolation: true, ensuring generated lock workflows don’t require sudo in environments where it’s unavailable (e.g., ARC/Kubernetes runners). Also bumps the default firewall and MCP gateway versions and refreshes generated fixtures/lockfiles accordingly.

Changes:

• Emit rootless AWF install/run behavior under network-isolation (no sudo / sudo -E) and skip the firewall log permission-fix that assumed root-owned files.
• Add/adjust tests to assert the new rootless compilation output behavior and update golden fixtures.
• Bump defaults to firewall v0.27.10 and MCP gateway v0.3.30, updating related pinned-image metadata inputs.

Show a summary per file

File	Description
actions/setup/sh/install_awf_binary.sh	Adds --rootless mode and routes privileged operations through maybe_sudo() (plus adjusted install verification).
pkg/workflow/copilot_engine_installation.go	Appends --rootless to the AWF install step when network-isolation is enabled.
pkg/workflow/awf_helpers.go	Uses awf (no sudo -E) as the command prefix when network-isolation is enabled.
pkg/workflow/engine_firewall_support.go	Skips sudo chmod -R a+rX ... in the firewall log parsing step for network-isolation workflows.
pkg/workflow/compiler_yaml_main_job.go	Plumbs workflowData into firewall log parsing step generation.
pkg/workflow/engine_firewall_support_test.go	Updates existing test for new function signature and adds coverage for the network-isolation sudo-omit behavior.
pkg/workflow/awf_helpers_test.go	Adds unit coverage for GetAWFCommandPrefix behavior under network-isolation and precedence rules.
pkg/workflow/sandbox_custom_agent_test.go	Adds assertions that the install step includes/excludes --rootless based on agent config.
pkg/workflow/sandbox_network_isolation_rootless_test.go	New integration-tagged test that compiles minimal workflows and asserts sudo omission/presence in output.
pkg/constants/version_constants.go	Bumps default firewall and MCP gateway versions.
pkg/actionpins/data/action_pins.json	Adds MCP gateway v0.3.30 container pin.
pkg/workflow/data/action_pins.json	Adds MCP gateway v0.3.30 container pin (workflow-side pin data).
.github/aw/actions-lock.json	Adds MCP gateway v0.3.30 pin to the repo cache.
.changeset/patch-network-isolation-rootless-sudo-omit.md	Documents the behavioral change and version bumps for release notes.
.github/workflows/test-workflow.lock.yml	Regenerated lock workflow reflecting version bumps and (intended) sudo/rootless changes.
.github/workflows/codex-github-remote-mcp-test.lock.yml	Regenerated lock workflow reflecting version bumps and (intended) sudo/rootless changes.
.github/workflows/bot-detection.lock.yml	Regenerated lock workflow reflecting version bumps and (intended) sudo/rootless changes.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_AllEngines/pi.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden	Updates golden output for new AWF/firewall/MCPG versions.
pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden	Updates golden output for new AWF/firewall/MCPG versions.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 37/274 changed files
• Comments generated: 7

[lpcox]

@copilot address review feedback

[pelikhan]

@copilot run pr-finisher skill

[github-actions]

REQUEST_CHANGES — 3 medium issues across the rootless AWF path

The core logic is structurally correct (network-isolation → rootless install, rootless runtime, no sudo chmod on logs). Three issues need to be addressed before merge.

🔍 Blocking issues summary

1. Inconsistent rootless guard in generateAWFInstallationStep (copilot_engine_installation.go:278)
The function checks agentConfig.NetworkIsolation directly, bypassing isAWFNetworkIsolationEnabled(), which also checks agentConfig.Disabled. The inconsistency is mitigated by caller-side isFirewallEnabled() guards today, but the raw *AgentSandboxConfig signature has no such protection, making this a latent bug for future callers. All other rootless decision points in this package use the helper.

2. Weak 'AWF is invoked' assertion in integration test (sandbox_network_isolation_rootless_test.go:61)
strings.Contains(lockStr, "awf ") also matches awf logs summary in the firewall log-parsing step and would pass even if the main AWF invocation were absent. The assertion needs a pattern that is specific to the primary execution command.

3. No preflight writability check in rootless install (install_awf_binary.sh:154)
--rootless assumes /usr/local/bin and /usr/local/lib/awf are writable by the unprivileged user. On standard GitHub-hosted runners and GPU runners (already called out in the script comments) this assumption fails silently mid-install with a generic EACCES. A preflight [ -w "${AWF_INSTALL_DIR}" ] check would surface the misconfiguration immediately with a clear message.

Non-blocking note (awf_helpers.go:725): the rootless branch hardcodes the string "awf" rather than deriving it from constants.AWFDefaultCommand. Low risk today but creates a silent divergence point if the constant ever changes.

🔎 Code quality review by PR Code Quality Reviewer · 164.4 AIC · ⌖ 9.82 AIC · ⊞ 5.2K

[github-actions]

generateAWFInstallationStep checks agentConfig.NetworkIsolation directly instead of using isAWFNetworkIsolationEnabled(), diverging from the codebase-wide pattern and creating a latent split-state if Disabled=true.

💡 Detail and suggested fix

Every other rootless decision in this package goes through isAWFNetworkIsolationEnabled(), which also guards against agentConfig.Disabled. With this direct check, the combination Disabled=true && NetworkIsolation=true produces a split state: the install script receives --rootless (no sudo), while GetAWFCommandPrefix — which uses the helper — returns "sudo -E awf".

This is mitigated today because all call sites first check isFirewallEnabled(), which short-circuits on disabled agents. But the function accepts a raw *AgentSandboxConfig with no such guard, making it a trap for future callers.

Suggested fix — mirror the helper's Disabled guard:

if agentConfig != nil && agentConfig.NetworkIsolation && !agentConfig.Disabled {
    installCmd += " --rootless"
}

Or refactor to accept *WorkflowData and delegate to isAWFNetworkIsolationEnabled(), matching the signatures of GetAWFCommandPrefix and generateFirewallLogParsingStep.

[github-actions]

The "awf " substring check will pass even if the primary AWF invocation is absent, providing false confidence that the main agent command runs rootlessly.

💡 Detail and suggested fix

strings.Contains(lockStr, "awf ") matches any occurrence of awf in the compiled lock file — including awf logs summary in the firewall log-parsing step. If the main AWF execution line were accidentally dropped from the output, this assertion would still pass.

The check should target a pattern that can only come from the main agent invocation, for example the full command that GetAWFCommandPrefix produces without a sudo prefix:

// Verify the main AWF invocation is present and rootless
if !strings.Contains(lockStr, "\n        run: awf ") && !strings.Contains(lockStr, "run: |\n          awf ") {
    t.Error("Expected rootless awf

[github-actions]

Rootless mode writes to root-owned system directories without first verifying write access, producing a cryptic EACCES mid-install on any runner where /usr/local/bin or /usr/local/lib/awf is not pre-chowned to the unprivileged user.

💡 Detail and suggested fix

The existing comments already call out that /usr/local/bin may be inaccessible to non-root users on GPU runners. The rootless path now runs all mkdir/cp/mv/tee without privilege elevation. If the environment does not pre-grant write access to these directories (which is only guaranteed in ARC/Kubernetes containers, not on GPU or standard GitHub-hosted runners), the script will abort mid-install with an opaque Permission denied and leave a partial installation.

Add a preflight check so the error message is actionable:

maybe_sudo() {
  if [ "$ROOTLESS" = "true" ]; then
    "$@"
  else
    sudo "$@"
  fi
}

# After ROOTLESS is determined, before any install work:
if [ "$ROOTLESS" = "true" ]; then
  if ! [ -w "${AWF_INSTALL_DIR}" ] || ! { mkdir -p "${AWF_LIB_DIR}" 2>/dev/null && [ -w "${AWF_LIB_DIR}" ]; }; then
    echo "ERROR: --rootless requires write access to ${AWF_INSTALL_DIR} and ${AWF_LIB_DIR}."
    echo "       These directories are root-owned on standard runners; --rootless is intended"
    echo "       only for ARC/Kubernetes containers where they are pre-chowned to the runner user."
    exit 1
  fi
fi

This surfaces the misconfiguration immediately rather than halfway through the install.

[github-actions]

"awf" is a magic string literal; if constants.AWFDefaultCommand is ever renamed or given a full path, this branch silently diverges.

💡 Detail and suggested fix

constants.AWFDefaultCommand = "sudo -E awf". The rootless branch hardcodes "awf" independently rather than deriving it from the constant. If the binary is renamed (e.g. to awf2) or given an absolute path in the constant, the sudo-prefixed path automatically picks up the change, but this branch stays stale and silently uses the wrong binary name.

Derive the binary name from the constant:

import "strings"

// Strip the "sudo -E " prefix, keeping just the binary name.
const sudoPrefix = "sudo -E "
if isAWFNetworkIsolationEnabled(workflowData) {
    awfHelpersLog.Print("Using rootless AWF command (network-isolation mode)")
    return strings.TrimPrefix(string(constants.AWFDefaultCommand), sudoPrefix)
}

Or add a separate constants.AWFBinaryName = "awf" constant and reference it from both paths. Either way, the binary name is expressed once.

[Copilot]

@copilot address review feedback

Addressed in commit ff8d8b8.

I fixed the review items by:

• updating the legacy-mode verification comment in install_awf_binary.sh to match actual sudo env behavior,
• removing hard-coded firewall version args from the two sandbox_custom_agent_test.go subtests,
• changing sandbox_network_isolation_rootless_test.go to //go:build !integration so it runs in unit test suites,
• restoring digest pinning for firewall (agent/api-proxy/squid:0.27.10) and MCPG (v0.3.30) via actions-lock.json + regenerated lock files.

[lpcox]

@copilot address this review feedback and also make sure that all linting and unit tests pass #41269 (review)