CLI proxy: start difc-proxy on host, pass --difc-proxy-host to AWF#25366
CLI proxy: start difc-proxy on host, pass --difc-proxy-host to AWF#25366
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cd08abe8-65f6-4cd4-aca7-a2cfa59d7e81 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…roxy on host When features.cli-proxy is enabled, the compiler now: 1. Starts a difc-proxy container on the host before AWF execution 2. Passes --difc-proxy-host host.docker.internal:18443 and --difc-proxy-ca-cert /tmp/gh-aw/difc-proxy-tls/ca.crt to AWF 3. Injects GH_TOKEN into the AWF step env with --exclude-env GH_TOKEN 4. Stops the CLI proxy container after AWF execution Removed deprecated flags: --enable-cli-proxy, --cli-proxy-policy. Minimum AWF version bumped to v0.26.0 for CLI proxy support. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cd08abe8-65f6-4cd4-aca7-a2cfa59d7e81 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
- Handle empty policy gracefully in start_cli_proxy.sh (proxy starts without guard filtering when no policy is configured) - Exit with error when proxy fails to start (prevents AWF from running with a non-functional proxy) - Rename hasCliProxyNeeded to isCliProxyNeeded for naming consistency Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cd08abe8-65f6-4cd4-aca7-a2cfa59d7e81 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR shifts AWF CLI-proxy support from an AWF-managed container to a host-managed difc-proxy container, updating AWF invocation flags and ensuring GH_TOKEN is available for the proxy while excluded from the agent container.
Changes:
- Add host-side start/stop lifecycle for the CLI proxy and inject those steps around engine execution.
- Replace deprecated AWF CLI-proxy flags with
--difc-proxy-host/--difc-proxy-ca-certand bump the minimum AWF version gate. - Inject
GH_TOKENinto AWF execution step env (post secret-filtering) across engines; update various workflow lockfiles for quoting consistency.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/gemini_engine.go | Injects GH_TOKEN into filtered step env for CLI-proxy mode. |
| pkg/workflow/copilot_engine_execution.go | Injects GH_TOKEN into filtered step env for CLI-proxy mode. |
| pkg/workflow/compiler_yaml_main_job.go | Adds start/stop CLI-proxy steps around engine execution. |
| pkg/workflow/compiler_difc_proxy.go | Adds CLI-proxy gating + YAML step generation for host-managed proxy lifecycle. |
| pkg/workflow/codex_engine.go | Injects GH_TOKEN into filtered step env for CLI-proxy mode. |
| pkg/workflow/claude_engine.go | Injects GH_TOKEN into filtered step env for CLI-proxy mode. |
| pkg/workflow/awf_helpers.go | Switches AWF flags to --difc-proxy-*, adds GH_TOKEN exclusion, adds env injection helper, updates version gate docs. |
| pkg/workflow/awf_helpers_test.go | Updates tests to expect new --difc-proxy-* flags and new version gating semantics. |
| pkg/constants/version_constants.go | Bumps AWFCliProxyMinVersion to v0.26.0. |
| pkg/constants/feature_constants.go | Updates cli-proxy feature flag documentation to reflect host-managed difc-proxy + new flags. |
| actions/setup/sh/start_cli_proxy.sh | New script to start host-side difc-proxy for AWF CLI-proxy sidecar. |
| actions/setup/sh/stop_cli_proxy.sh | New script to stop/remove the host-side CLI proxy container. |
| .github/workflows/refactoring-cadence.lock.yml | Quotes ${RUNNER_TEMP} paths consistently in bash invocations. |
| .github/workflows/design-decision-gate.lock.yml | Quotes ${RUNNER_TEMP} paths consistently in bash invocations. |
| .github/workflows/deep-report.lock.yml | Lockfile regeneration + minor env expression tweak (REPO_NAME). |
| .github/workflows/architecture-guardian.lock.yml | Quotes ${RUNNER_TEMP} paths consistently in bash invocations. |
| .github/workflows/approach-validator.lock.yml | Quotes ${RUNNER_TEMP} paths consistently in bash invocations. |
| .changeset/minor-add-cli-proxy-feature-flag.md | Updates changeset to mention new flags/host proxy (but still contains outdated statement). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/workflow/awf_helpers.go:649
awfSupportsCliProxycurrently returnstruewhenfirewallConfigis nil/empty, butconstants.DefaultFirewallVersionis stillv0.25.16whileAWFCliProxyMinVersionis nowv0.26.0. In the common case where sandbox/firewall is enabled viasandbox:(sonetwork.firewallis absent), this will incorrectly emit--difc-proxy-host/--difc-proxy-ca-certagainst AWFv0.25.16and cause AWF to fail at startup. Fix by either (a) bumpingDefaultFirewallVersionto >=v0.26.0, or (b) makingawfSupportsCliProxycompare against the effective default version instead of assuming it always meets the minimum (and update the associated tests).
// Special cases:
// - No version override (firewallConfig is nil or has no Version): use DefaultFirewallVersion
// which is always ≥ AWFCliProxyMinVersion → returns true.
// - "latest": always returns true (latest is always a new release).
// - Any semver string ≥ AWFCliProxyMinVersion: returns true.
// - Any semver string < AWFCliProxyMinVersion: returns false.
// - Non-semver string (e.g. a branch name): returns false (conservative).
func awfSupportsCliProxy(firewallConfig *FirewallConfig) bool {
var versionStr string
if firewallConfig != nil && firewallConfig.Version != "" {
versionStr = firewallConfig.Version
} else {
// No override → use the default, which is always ≥ the minimum.
return true
}
- Files reviewed: 18/18 changed files
- Comments generated: 5
pkg/workflow/awf_helpers.go
Outdated
| // The token is excluded from the agent container via --exclude-env GH_TOKEN. | ||
| // | ||
| // #nosec G101 -- This is NOT a hardcoded credential. It is a GitHub Actions expression | ||
| // template that is resolved at runtime by the GitHub Actions runner. | ||
| func addCliProxyGHTokenToEnv(env map[string]string, workflowData *WorkflowData) { | ||
| if isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) && isFirewallEnabled(workflowData) { |
There was a problem hiding this comment.
addCliProxyGHTokenToEnv injects GH_TOKEN into the AWF step env whenever the feature flag + firewall are enabled, but it does not check whether the effective AWF version supports --exclude-env. When AWF < v0.25.3, BuildAWFArgs will still pass --env-all and skip --exclude-env, which would leak this injected GH_TOKEN into the agent container. Consider gating the injection on awfSupportsExcludeEnv(getFirewallConfig(workflowData)) (and likely the CLI proxy version gate as well) so we never introduce a new secret into the container on older AWF versions.
This issue also appears on line 635 of the same file.
| // The token is excluded from the agent container via --exclude-env GH_TOKEN. | |
| // | |
| // #nosec G101 -- This is NOT a hardcoded credential. It is a GitHub Actions expression | |
| // template that is resolved at runtime by the GitHub Actions runner. | |
| func addCliProxyGHTokenToEnv(env map[string]string, workflowData *WorkflowData) { | |
| if isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) && isFirewallEnabled(workflowData) { | |
| // The token is excluded from the agent container via --exclude-env GH_TOKEN, so only | |
| // inject it when the effective AWF version supports both cli-proxy flags and | |
| // --exclude-env. | |
| // | |
| // #nosec G101 -- This is NOT a hardcoded credential. It is a GitHub Actions expression | |
| // template that is resolved at runtime by the GitHub Actions runner. | |
| func addCliProxyGHTokenToEnv(env map[string]string, workflowData *WorkflowData) { | |
| firewallConfig := getFirewallConfig(workflowData) | |
| if isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) && | |
| isFirewallEnabled(workflowData) && | |
| awfSupportsCliProxy(firewallConfig) && | |
| awfSupportsExcludeEnv(firewallConfig) { |
| // DefaultFirewallVersion is the default version of the gh-aw-firewall (AWF) binary | ||
| const DefaultFirewallVersion Version = "v0.25.16" | ||
|
|
||
| // AWFExcludeEnvMinVersion is the minimum AWF version that supports the --exclude-env flag. | ||
| // Workflows pinning an older AWF version must not emit --exclude-env flags or the run will fail. | ||
| const AWFExcludeEnvMinVersion Version = "v0.25.3" | ||
|
|
||
| // AWFCliProxyMinVersion is the minimum AWF version that supports the --enable-cli-proxy flag. | ||
| // Workflows pinning an older AWF version must not emit --enable-cli-proxy flags or the run will fail. | ||
| const AWFCliProxyMinVersion Version = "v0.25.14" | ||
| // AWFCliProxyMinVersion is the minimum AWF version that supports the CLI proxy flags | ||
| // (--difc-proxy-host, --difc-proxy-ca-cert). Workflows pinning an older AWF version | ||
| // must not emit CLI proxy flags or the run will fail. | ||
| const AWFCliProxyMinVersion Version = "v0.26.0" | ||
|
|
There was a problem hiding this comment.
AWFCliProxyMinVersion is now v0.26.0 but DefaultFirewallVersion remains v0.25.16. With the current version-gate logic (and tests), this combination can cause CLI proxy flags to be emitted for workflows that rely on the default AWF version, even though that default is below the minimum. Either bump DefaultFirewallVersion to >= v0.26.0 or adjust the gating logic/tests to treat the default as not supporting CLI proxy yet.
| want: true, | ||
| }, | ||
| { | ||
| name: "v0.25.14 supports --enable-cli-proxy (exact minimum version)", | ||
| firewallConfig: &FirewallConfig{Version: "v0.25.14"}, | ||
| name: "v0.26.0 supports CLI proxy flags (exact minimum version)", | ||
| firewallConfig: &FirewallConfig{Version: "v0.26.0"}, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "v0.26.0 supports --enable-cli-proxy", | ||
| firewallConfig: &FirewallConfig{Version: "v0.26.0"}, | ||
| name: "v0.27.0 supports CLI proxy flags", | ||
| firewallConfig: &FirewallConfig{Version: "v0.27.0"}, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "v0.25.13 does not support --enable-cli-proxy", | ||
| firewallConfig: &FirewallConfig{Version: "v0.25.13"}, | ||
| name: "v0.25.16 does not support CLI proxy flags", | ||
| firewallConfig: &FirewallConfig{Version: "v0.25.16"}, | ||
| want: false, |
There was a problem hiding this comment.
The version-gate tests here assume awfSupportsCliProxy(nil) / empty version implies support via the default, but DefaultFirewallVersion is currently v0.25.16 while the minimum is v0.26.0. If the default stays at v0.25.16, the expected behavior should be that the default does not support CLI proxy flags; alternatively bump the default version and keep these expectations. Either way, align this test table with the chosen default/version-gate semantics so unsupported flags aren’t emitted by default.
| docker run -d --name awmg-cli-proxy --network host \ | ||
| -e GH_TOKEN \ | ||
| -e GITHUB_SERVER_URL \ | ||
| -e DEBUG='*' \ | ||
| -v "$TLS_DIR:$TLS_DIR" \ | ||
| -v "$MCP_LOG_DIR:$MCP_LOG_DIR" \ | ||
| "$CONTAINER_IMAGE" proxy \ |
There was a problem hiding this comment.
start_cli_proxy.sh is not idempotent: docker run --name awmg-cli-proxy will fail if a container with that name already exists (e.g., prior job cancellation on a self-hosted runner, or a previous failed cleanup). Consider removing any existing container (and/or checking for it) before starting so the step can recover reliably.
| Add `cli-proxy` feature flag that injects `--enable-cli-proxy` and `--cli-proxy-policy` into the AWF command, giving agents secure read-only `gh` CLI access without exposing `GITHUB_TOKEN` (requires firewall v0.25.14+). | ||
| Replaced with `--difc-proxy-host` and `--difc-proxy-ca-cert` flags, starting difc-proxy on the host before AWF (requires firewall v0.26.0+). |
There was a problem hiding this comment.
This changeset still states the cli-proxy feature injects the deprecated --enable-cli-proxy/--cli-proxy-policy flags. Since the implementation now uses --difc-proxy-host/--difc-proxy-ca-cert and host-managed proxy lifecycle, update the text to describe the current behavior (rather than appending a second sentence that contradicts the first).
| Add `cli-proxy` feature flag that injects `--enable-cli-proxy` and `--cli-proxy-policy` into the AWF command, giving agents secure read-only `gh` CLI access without exposing `GITHUB_TOKEN` (requires firewall v0.25.14+). | |
| Replaced with `--difc-proxy-host` and `--difc-proxy-ca-cert` flags, starting difc-proxy on the host before AWF (requires firewall v0.26.0+). | |
| Add `cli-proxy` feature flag that injects `--difc-proxy-host` and `--difc-proxy-ca-cert` into the AWF command, starting `difc-proxy` on the host before AWF and giving agents secure read-only `gh` CLI access without exposing `GITHUB_TOKEN` (requires firewall v0.26.0+). |
|
@copilot re-compile agentic workflows. |
- Bump DefaultFirewallVersion to v0.26.0 to align with AWFCliProxyMinVersion - Gate addCliProxyGHTokenToEnv on awfSupportsCliProxy and awfSupportsExcludeEnv to prevent leaking GH_TOKEN into the agent container on older AWF versions - Make start_cli_proxy.sh idempotent by removing any leftover container first - Update changeset to describe current behavior (difc-proxy-host flags) - Recompile all agentic workflows with updated DefaultFirewallVersion Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e59645aa-2981-470c-bd44-1075fd88317a Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Done in f48eddc. Recompiled all agentic workflows after bumping Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot apply changes based on the comments in this thread |
|
@copilot resolve the merge conflicts in this pull request |
All 5 review comments from that thread were already addressed in f48eddc:
All lock files were also recompiled with the new Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…oxy-implementation # Conflicts: # .github/workflows/api-consumption-report.lock.yml # .github/workflows/approach-validator.lock.yml # .github/workflows/architecture-guardian.lock.yml # .github/workflows/audit-workflows.lock.yml # .github/workflows/daily-firewall-report.lock.yml # .github/workflows/daily-performance-summary.lock.yml # .github/workflows/deep-report.lock.yml # .github/workflows/design-decision-gate.lock.yml # .github/workflows/docs-noob-tester.lock.yml # .github/workflows/poem-bot.lock.yml # .github/workflows/refactoring-cadence.lock.yml # .github/workflows/technical-doc-writer.lock.yml # .github/workflows/unbloat-docs.lock.yml Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Merge conflicts resolved in 3ac5110. All 13 conflicting Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
lpcox
left a comment
There was a problem hiding this comment.
Review: PR #25366 — CLI proxy: start difc-proxy on host, pass --difc-proxy-host to AWF
This is the compiler-side counterpart to gh-aw-firewall PR #1809. The architecture is sound — moving the difc-proxy from AWF's restrictive sandbox to the host eliminates the mcpg crash issues we hit in PR #1801.
What looks good
- Clean separation:
start_cli_proxy.sh/stop_cli_proxy.shfollow the existingstart_difc_proxy.shpattern exactly - GH_TOKEN flow:
addCliProxyGHTokenToEnv→ AWF step env →--exclude-env GH_TOKENprevents agent exposure. Gated on bothawfSupportsCliProxyANDawfSupportsExcludeEnv(as requested in the auto-review) - Port conflict avoidance: DIFC proxy stops before CLI proxy starts in
generateMainJobStepsordering, so both can use port 18443 without conflict - Idempotent start:
docker rm -f awmg-cli-proxybeforedocker runhandles leftover containers - Stop step has
if: always()+continue-on-error: true— correct cleanup semantics - Test updates are thorough — version gates, flag assertions, negative tests for deprecated flags
isCliProxyNeeded()cleanly factors the 3-way check (feature flag + firewall enabled + version support)- Guard policy stays on the host proxy (via
CLI_PROXY_POLICYenv instart_cli_proxy.sh), correctly removed from AWF args
Issues
1. start_cli_proxy.sh: unquoted $POLICY_ARGS — shell word-splitting risk
POLICY_ARGS=""
if [ -n "$POLICY" ]; then
POLICY_ARGS="--policy $POLICY"
fi
docker run ... $POLICY_ARGS \If CLI_PROXY_POLICY JSON ever contains spaces (e.g., pretty-printed JSON), this would word-split and break. The existing start_difc_proxy.sh handles this correctly with --policy "$POLICY". Recommend using an array:
POLICY_ARGS=()
if [ -n "$POLICY" ]; then
POLICY_ARGS=(--policy "$POLICY")
fi
docker run ... "${POLICY_ARGS[@]}" \Currently safe because getDIFCProxyPolicyJSON generates compact JSON, but an array is more robust and matches the quoting in start_difc_proxy.sh.
2. Release coordination: DefaultFirewallVersion v0.26.0
187 lock files now reference AWF v0.26.0 images/binaries. This PR must not merge before AWF v0.26.0 is published to GHCR, or every workflow will fail at install_awf_binary.sh v0.26.0 and download_docker_images.sh ...agent:0.26.0.
Recommend documenting this in the PR body: "Merge order: AWF PR #1809 must be released as v0.26.0 before this PR can merge."
3. addCliProxyGHTokenToEnv docstring is misleading
The comment says "AWF reads it from the step env and the cli-proxy sidecar uses it for upstream API authentication." Neither is accurate:
- AWF does not read GH_TOKEN — it receives it via
--env-alland immediately strips it via--exclude-env - The cli-proxy sidecar does not use GH_TOKEN — it connects to the host difc-proxy which already has the token from
start_cli_proxy.sh
The actual purpose is: ensure GH_TOKEN is in the AWF step env so --env-all picks it up and --exclude-env GH_TOKEN can reliably strip it from the agent container. Suggest:
// addCliProxyGHTokenToEnv adds GH_TOKEN to the AWF step environment when the
// cli-proxy feature is enabled. The token is NOT used by AWF or its cli-proxy
// sidecar directly — the host difc-proxy (started by start_cli_proxy.sh) already
// has it. However, --env-all passes all step env vars into the agent container,
// so we explicitly set GH_TOKEN here to ensure --exclude-env GH_TOKEN can
// reliably strip it regardless of how the token enters the environment.4. Missing dedicated tests for isCliProxyNeeded() and addCliProxyGHTokenToEnv()
Both functions are tested indirectly (via step generation and BuildAWFArgs), but table-driven unit tests would catch edge cases:
isCliProxyNeededwith feature disabled, firewall disabled, old AWF versionaddCliProxyGHTokenToEnvverifying it does NOT inject GH_TOKEN when cli-proxy is disabled or AWF is too old
Not blocking, but would improve confidence.
Summary
The implementation correctly handles the architectural change. The four items above are: (1) a shell safety fix, (2) a release process note, (3) a comment accuracy fix, and (4) optional test improvements. Items 1 and 3 are minor code fixes; items 2 and 4 are process/coverage notes.
The AWF side (PR #1809) is complementary and ready — I've reviewed and committed fixes there too.
…25400) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f16055db-4d7a-479e-acae-0713caf5344d Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
@copilot update the firewall version to v0.25.17 |
… docstring Agent-Logs-Url: https://github.com/github/gh-aw/sessions/35642b32-32d1-4a2d-bea7-8041bed78e77 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed items 1 and 3 in 7365f86:
Item 2 (release coordination) is noted — will add merge-order guidance to the PR body. Item 4 (dedicated tests) tracked for follow-up.
Done in 7365f86. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…eScript compatibility (#25406) * feat: increase logging in copilot driver for silent startup failures (#issue) (#25390) * feat(logging): add debug logging to 5 CLI files for improved troubleshooting (#25393) * fix: add parentheses to JSDoc type cast in copilot_driver.cjs for TypeScript compatibility Agent-Logs-Url: https://github.com/github/gh-aw/sessions/34f7e8b3-df09-41bc-b786-8bb4b22ebb7e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
AWF's internal mcpg container for cli-proxy crashes reliably under its restrictive sandbox (cap_drop ALL, pids_limit 50, mem_limit 256m, non-host network). This moves difc-proxy management to the gh-aw compiler, running it on the host with
--network hostwhere it already works reliably.Merge order: gh-aw-firewall PR #1809 must be released as v0.26.0 before this PR can merge, since
AWFCliProxyMinVersionrequires v0.26.0 for CLI proxy flags.AWF flag changes
Replaced deprecated flags with new ones that point AWF at the external proxy:
--enable-cli-proxy,--cli-proxy-policy,--cli-proxy-mcpg-image→ removed--difc-proxy-host,--difc-proxy-ca-cert→ addedAWFCliProxyMinVersionbumped tov0.26.0DefaultFirewallVersionset tov0.25.17(below CLI proxy minimum — cli-proxy flags are only emitted when workflows explicitly pinv0.26.0+)New host-side proxy lifecycle
actions/setup/sh/start_cli_proxy.sh— startsawmg-cli-proxycontainer on host (port 18443, TLS to/tmp/gh-aw/difc-proxy-tls/), with optional guard policy. Idempotent: removes any leftover container before starting. Fails the step if proxy doesn't become healthy within 30s. Uses bash arrays for safe shell quoting of policy JSON.actions/setup/sh/stop_cli_proxy.sh—docker rm -fcleanup, runs withif: always().compiler_yaml_main_job.goimmediately before/after engine execution.GH_TOKEN injection
When cli-proxy is enabled,
GH_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN || github.token }}is added to the AWF step env across all engines (copilot, claude, codex, gemini) via sharedaddCliProxyGHTokenToEnv()helper. The token is NOT used by AWF or its cli-proxy sidecar directly — the host difc-proxy already has it. The injection ensures--env-allpicks it up so--exclude-env GH_TOKENcan reliably strip it from the agent container. The injection is gated on bothawfSupportsCliProxyandawfSupportsExcludeEnvto prevent leaking the token into the agent container on older AWF versions.Architecture