Skip to content

feat: phase 1 – gh CLI proxy sidecar with mcpg DIFC proxy#1730

Merged
lpcox merged 9 commits intomainfrom
copilot/design-gh-cli-proxy-sidecar
Apr 7, 2026
Merged

feat: phase 1 – gh CLI proxy sidecar with mcpg DIFC proxy#1730
lpcox merged 9 commits intomainfrom
copilot/design-gh-cli-proxy-sidecar

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 6, 2026

Agents currently access GitHub exclusively through the MCP protocol, which bloats context with 30+ tool schemas (~8–12k tokens/turn) and forces unnatural tool-call framing. The root constraint: gh CLI requires GH_TOKEN, which AWF deliberately excludes from the agent container.

This implements Phase 1 of the CLI proxy sidecar design: a new awf-cli-proxy container at 172.30.0.50 that runs an mcpg DIFC proxy (holds GH_TOKEN, enforces guard policies) alongside a Node.js HTTP server. The agent gets a /usr/local/bin/gh wrapper that forwards invocations over HTTP — no token exposure, same DIFC guarantees.

New container: containers/cli-proxy/

  • Dockerfile — Multi-stage: copies mcpg binary from a configurable image (default ghcr.io/github/gh-aw-mcpg:v0.2.15, overridable via --cli-proxy-mcpg-image); layers gh CLI + Node.js on Alpine; runs as non-root user cliproxy
  • entrypoint.sh — Fails closed if GH_TOKEN is absent; starts mcpg proxy (--tls, --guards-mode filter, --trusted-bots github-actions[bot],github-actions,dependabot[bot],copilot, port 127.0.0.1:18443), waits for TLS cert, then starts HTTP server under a supervisor loop (Node in background + wait) so SIGTERM/SIGINT gracefully stop both Node and mcpg
  • server.js/exec (POST) and /health (GET); subcommand allowlist enforced with action-level blocks; execFile (not shell) prevents injection; configurable timeout + output cap; request body size limit (1 MB, returns 413 on overflow)
  • server.test.js — 61 unit tests covering read-only/writable modes, flag-before-subcommand parsing, always-denied meta-commands, codespace/cache/org write-action blocking, subcommand-index bypass prevention, and api read-only denial

Read-only by default (opt-in write via --cli-proxy-writable):

ALLOWED:  issue list/view, pr list/view/diff/checks, repo list/view, search, run list/view, ...
DENIED:   api (raw passthrough; use typed subcommands instead), pr create/merge,
          issue create/close, repo create/delete, secret set,
          codespace create/delete, cache delete, org invite, ...
ALWAYS:   auth, config, extension (even in writable mode)

Note: gh api is excluded from read-only mode because it is a raw HTTP passthrough that accepts -X POST/PUT/DELETE and can perform arbitrary mutations regardless of action-level blocks. It remains available in writable mode.

AWF integration

  • src/types.tsCLI_PROXY_PORT = 11000; new config fields: enableCliProxy, cliProxyWritable, cliProxyPolicy, cliProxyMcpgImage, githubToken
  • src/docker-manager.ts — cli-proxy service at 172.30.0.50; AWF_CLI_PROXY_IP propagated to iptables-init before service definition (same timing requirement as api-proxy); GITHUB_TOKEN/GH_TOKEN excluded from agent env when cli-proxy enabled (tokens held securely in mcpg; safe because actions/checkout runs before awf and tools needing GITHUB_TOKEN should use GITHUB_MCP_SERVER_TOKEN); cli-proxy log preservation on cleanup; cliProxyIp added to network config; passes MCPG_IMAGE Docker build arg when using --build-local
  • src/cli.ts--enable-cli-proxy, --cli-proxy-writable, --cli-proxy-policy, --cli-proxy-mcpg-image flags; reads GITHUB_TOKEN || GH_TOKEN for the proxy token; warns when neither is set; predownload subcommand supports --enable-cli-proxy
  • containers/agent/setup-iptables.sh — NAT RETURN rule for AWF_CLI_PROXY_IP (parallel to api-proxy pattern)
  • .github/workflows/build.yml — CLI proxy unit tests added to CI pipeline alongside the existing api-proxy test step

Compiler-controlled mcpg version

The AWF compiler (gh-aw) sets --cli-proxy-mcpg-image to control which mcpg version is pulled and run inside the cli-proxy sidecar. This is only used when building locally with --build-local; the pre-built GHCR cli-proxy image already has mcpg bundled at release time. The Dockerfile uses ARG MCPG_IMAGE=ghcr.io/github/gh-aw-mcpg:v0.2.15 so the default is deterministic and the compiler can override it for version pinning or testing new mcpg releases.

# Compiler pins a specific mcpg version when building locally
awf --build-local --enable-cli-proxy --cli-proxy-mcpg-image ghcr.io/github/gh-aw-mcpg:v0.3.0 ...

Agent-side gh wrapper

gh-cli-proxy-wrapper.sh installed in the agent image; activated at runtime (both chroot and non-chroot) when AWF_CLI_PROXY_URL is set by copying to /tmp/awf-lib/gh and prepending to PATH. The wrapper captures HTTP status separately from the response body so 4xx policy errors (e.g. 403 on a blocked command) surface the actual error message rather than a generic "unavailable" message:

# Agent sees:
$ gh pr list --repo owner/repo --json number,title
# → HTTP POST 172.30.0.50:11000/exec
# → cli-proxy executes gh with GH_HOST=localhost:18443
# → mcpg injects GH_TOKEN, enforces guard policy, logs to JSONL
# → Squid enforces domain allowlist on outbound traffic

Tests

  • 21 new docker-manager.test.ts tests covering service generation, environment propagation, iptables-init env, writable mode, guard policy passthrough, image resolution, MCPG_IMAGE build arg passthrough, and agent depends_on
  • 61 containers/cli-proxy/server.test.js tests, covering read-only/writable modes, codespace/cache/org write-action blocking, subcommand-index indexOf-bypass fix, and api read-only denial
  • Updated startContainers test to include awf-cli-proxy in the pre-run container removal list

Copilot AI linked an issue Apr 6, 2026 that may be closed by this pull request
32 tasks
Copilot AI added 2 commits April 6, 2026 22:49
- Add containers/cli-proxy/ with Dockerfile, server.js, entrypoint.sh,
  healthcheck.sh, package.json, and server.test.js (49 unit tests)
- Add CLI_PROXY_PORT constant and enableCliProxy/cliProxyWritable/
  cliProxyPolicy/githubToken config fields to src/types.ts
- Add cli-proxy service definition in src/docker-manager.ts:
  - IP 172.30.0.50 on awf-net
  - Passes GH_TOKEN to mcpg DIFC proxy; never exposed to agent
  - AWF_CLI_PROXY_URL set in agent environment
  - GITHUB_TOKEN/GH_TOKEN excluded from agent when cli-proxy enabled
  - AWF_CLI_PROXY_IP propagated to iptables-init
  - Log preservation in cleanup
- Add --enable-cli-proxy, --cli-proxy-writable, --cli-proxy-policy flags
  to src/cli.ts; add --enable-cli-proxy to predownload subcommand
- Add AWF_CLI_PROXY_IP iptables RETURN rule in setup-iptables.sh
- Add gh-cli-proxy-wrapper.sh to agent container; activated at runtime
  via AWF_CLI_PROXY_URL in both chroot and non-chroot modes
- Add 18 new docker-manager tests for CLI proxy service; update existing
  container removal test to include awf-cli-proxy

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/b5f7487a-4993-4737-a01e-8f091a8e84c7
- Fix test name: 'should not include cli-proxy when cliProxyIp is missing'
- Fix gh-cli-proxy-wrapper.sh: capture curl exit code in CURL_EXIT variable
  before checking it (avoids $? being overwritten by shell); remove set -e
- Fix entrypoint.sh: clarify log message that /tmp/awf-lib/gh is the path
  inside the chroot (not the host-prefixed /host/tmp/awf-lib/gh path)
- Fix server.js: sanitize error message in catch block to avoid stack trace
  exposure (CodeQL js/stack-trace-exposure)

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/b5f7487a-4993-4737-a01e-8f091a8e84c7
Copilot AI changed the title [WIP] Design gh CLI proxy sidecar with mcpg DIFC proxy feat: Phase 1 – gh CLI proxy sidecar with mcpg DIFC proxy Apr 6, 2026
Copilot AI requested a review from lpcox April 6, 2026 22:54
Copilot finished work on behalf of lpcox April 6, 2026 22:54
@lpcox lpcox marked this pull request as ready for review April 6, 2026 22:58
@lpcox lpcox requested a review from Mossaka as a code owner April 6, 2026 22:58
Copilot AI review requested due to automatic review settings April 6, 2026 22:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an AWF “gh CLI proxy” sidecar (mcpg DIFC proxy + HTTP exec server) so agents can invoke gh without receiving GitHub tokens, while maintaining guard/audit controls.

Changes:

  • Introduces containers/cli-proxy/ image (mcpg + Node HTTP server) and an agent-side gh wrapper that forwards commands over HTTP.
  • Extends AWF config/CLI flags and docker-compose generation to deploy and wire the cli-proxy sidecar (IP 172.30.0.50, port 11000) and update iptables NAT exceptions.
  • Adds/updates unit tests around docker-compose generation and image predownload resolution.
Show a summary per file
File Description
src/types.ts Adds CLI proxy port constant and WrapperConfig fields for enabling/configuring the sidecar.
src/docker-manager.ts Adds compose service generation, env propagation, token exclusion from agent env, and log preservation for cli-proxy.
src/docker-manager.test.ts Adds coverage for cli-proxy service generation and container cleanup list updates.
src/commands/predownload.ts Adds optional predownload of the cli-proxy image.
src/cli.ts Adds CLI flags and config wiring for enabling/configuring the cli-proxy sidecar.
containers/cli-proxy/Dockerfile Builds the new sidecar image (mcpg + gh + Node server).
containers/cli-proxy/entrypoint.sh Starts mcpg (if token present) and the HTTP server.
containers/cli-proxy/server.js Implements /health and /exec with subcommand/action policy enforcement.
containers/cli-proxy/server.test.js Adds unit tests for argument validation/policy logic.
containers/cli-proxy/package.json Defines cli-proxy Node package scripts/deps for tests.
containers/cli-proxy/healthcheck.sh Adds a container healthcheck script.
containers/agent/setup-iptables.sh Adds NAT RETURN rule to allow agent→cli-proxy traffic.
containers/agent/gh-cli-proxy-wrapper.sh Adds the agent-side gh wrapper that forwards invocations to the sidecar.
containers/agent/entrypoint.sh Activates the gh wrapper in both chroot and non-chroot modes when enabled.
containers/agent/Dockerfile Installs the gh wrapper into the agent image.

Copilot's findings

Tip

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

  • Files reviewed: 15/16 changed files
  • Comments generated: 9

Comment on lines +48 to +55
# Create non-root user
RUN addgroup -S cliproxy && adduser -S cliproxy -G cliproxy

# Expose port for agent→cli-proxy HTTP communication
# 11000 - gh exec endpoint and health check
EXPOSE 11000

ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image creates a non-root user (cliproxy) but never switches to it, so both mcpg and the HTTP server will run as root. This also contradicts the comment in src/docker-manager.ts that the CLI proxy runs as non-root. Add a USER cliproxy (or equivalent) before the entrypoint and ensure required paths (/var/log/cli-proxy, /tmp/proxy-tls) remain writable.

Copilot uses AI. Check for mistakes.
Comment on lines +33 to +67
const ALLOWED_SUBCOMMANDS_READONLY = new Set([
'api',
'browse',
'cache',
'codespace',
'gist',
'issue',
'label',
'org',
'pr',
'release',
'repo',
'run',
'search',
'secret',
'variable',
'workflow',
]);

/**
* Actions that are blocked within their parent subcommand in read-only mode.
* Maps subcommand -> Set of blocked action verbs.
*/
const BLOCKED_ACTIONS_READONLY = new Map([
['gist', new Set(['create', 'delete', 'edit'])],
['issue', new Set(['create', 'close', 'delete', 'edit', 'lock', 'pin', 'reopen', 'transfer', 'unpin'])],
['label', new Set(['create', 'delete', 'edit'])],
['pr', new Set(['checkout', 'close', 'create', 'edit', 'lock', 'merge', 'ready', 'reopen', 'review', 'update-branch'])],
['release', new Set(['create', 'delete', 'delete-asset', 'edit', 'upload'])],
['repo', new Set(['archive', 'create', 'delete', 'edit', 'fork', 'rename', 'set-default', 'sync', 'unarchive'])],
['run', new Set(['cancel', 'delete', 'download', 'rerun'])],
['secret', new Set(['delete', 'set'])],
['variable', new Set(['delete', 'set'])],
['workflow', new Set(['disable', 'enable', 'run'])],
]);
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In read-only mode, some allowed top-level subcommands (e.g. codespace, cache, org) can still perform write operations, but there are no action-level blocks for them in BLOCKED_ACTIONS_READONLY. That means commands like gh codespace create or gh cache delete would currently be permitted in read-only mode. Either remove these subcommands from the read-only allowlist or add an explicit blocked-actions set for each to prevent state-changing actions.

Copilot uses AI. Check for mistakes.
Comment on lines +134 to +145
// Check action-level blocklist
const blockedActions = BLOCKED_ACTIONS_READONLY.get(subcommand);
if (blockedActions) {
// The action is the first non-flag argument after the subcommand
const subcommandIndex = args.indexOf(subcommand);
const action = args.slice(subcommandIndex + 1).find(a => !a.startsWith('-'));
if (action && blockedActions.has(action)) {
return {
valid: false,
error: `Action '${subcommand} ${action}' is not allowed in read-only mode. Enable write mode with --cli-proxy-writable.`,
};
}
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action detection uses args.indexOf(subcommand) to find the subcommand position. If the subcommand string appears earlier as a flag value (e.g. gh --repo pr pr merge 1), indexOf will point at the flag value and the blocked-action check can be bypassed. Track and reuse the actual subcommand index found during the scan instead of calling indexOf.

Copilot uses AI. Check for mistakes.
Comment on lines +158 to +164
function readBody(req) {
return new Promise((resolve, reject) => {
const chunks = [];
req.on('data', chunk => chunks.push(chunk));
req.on('end', () => resolve(Buffer.concat(chunks)));
req.on('error', reject);
});
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readBody() buffers the entire request body without any size limit. A large POST to /exec could cause memory exhaustion in the sidecar. Enforce a maximum request size (e.g., reject once a byte limit is exceeded) and return 413 for oversized bodies.

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +43
RESPONSE=$(curl -sf \
--max-time 60 \
-X POST "${CLI_PROXY}/exec" \
-H "Content-Type: application/json" \
--data-binary "$(printf '{"args":%s,"cwd":%s,"stdin":"%s"}' \
"$ARGS_JSON" \
"$(printf '%s' "$CWD" | jq -Rs .)" \
"$STDIN_DATA")")
CURL_EXIT=$?

if [ "$CURL_EXIT" -ne 0 ] || [ -z "$RESPONSE" ]; then
echo "gh: CLI proxy unavailable at ${CLI_PROXY}" >&2
exit 1
fi
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrapper uses curl -f, so any non-2xx response (e.g. 403 when a command is blocked) makes curl exit non-zero and discards the JSON error body. As a result, users see “CLI proxy unavailable” instead of the actual policy error. Capture HTTP status and response body (without -f) and surface .error for 4xx/5xx responses.

Copilot uses AI. Check for mistakes.
src/cli.ts Outdated
enableCliProxy: options.enableCliProxy,
cliProxyWritable: options.cliProxyWritable,
cliProxyPolicy: options.cliProxyPolicy,
githubToken: process.env.GITHUB_TOKEN,
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

githubToken is read only from process.env.GITHUB_TOKEN. Many users of gh set GH_TOKEN instead, and this PR also explicitly excludes GH_TOKEN from the agent env when the sidecar is enabled. Consider reading process.env.GITHUB_TOKEN || process.env.GH_TOKEN (and/or honoring a dedicated CLI flag) so enabling the CLI proxy works with either token variable.

Suggested change
githubToken: process.env.GITHUB_TOKEN,
githubToken: process.env.GITHUB_TOKEN || process.env.GH_TOKEN,

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +80
# Cleanup handler: kill mcpg when the server exits or receives a signal
cleanup() {
echo "[cli-proxy] Shutting down..."
if [ -n "$MCPG_PID" ]; then
kill "$MCPG_PID" 2>/dev/null || true
fi
exit 0
}
trap cleanup INT TERM

# Start the Node.js HTTP server (foreground)
echo "[cli-proxy] Starting HTTP server on port 11000..."
exec node /app/server.js
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entrypoint sets a trap to kill the mcpg background process, but then execs Node as PID 1. After exec, the shell (and its traps) no longer exist, so the cleanup handler won’t run on SIGTERM/SIGINT. If graceful shutdown of mcpg is desired, run a simple supervisor loop (or avoid exec and wait on Node) so signals can trigger cleanup.

Suggested change
# Cleanup handler: kill mcpg when the server exits or receives a signal
cleanup() {
echo "[cli-proxy] Shutting down..."
if [ -n "$MCPG_PID" ]; then
kill "$MCPG_PID" 2>/dev/null || true
fi
exit 0
}
trap cleanup INT TERM
# Start the Node.js HTTP server (foreground)
echo "[cli-proxy] Starting HTTP server on port 11000..."
exec node /app/server.js
NODE_PID=""
# Cleanup handler: stop child processes when the server exits or receives a signal
cleanup() {
echo "[cli-proxy] Shutting down..."
if [ -n "$NODE_PID" ]; then
kill "$NODE_PID" 2>/dev/null || true
wait "$NODE_PID" 2>/dev/null || true
fi
if [ -n "$MCPG_PID" ]; then
kill "$MCPG_PID" 2>/dev/null || true
wait "$MCPG_PID" 2>/dev/null || true
fi
}
trap 'cleanup; exit 0' INT TERM
# Start the Node.js HTTP server under shell supervision so traps remain active
echo "[cli-proxy] Starting HTTP server on port 11000..."
node /app/server.js &
NODE_PID=$!
if wait "$NODE_PID"; then
NODE_EXIT=0
else
NODE_EXIT=$?
fi
cleanup
exit "$NODE_EXIT"

Copilot uses AI. Check for mistakes.

echo "[cli-proxy] gh CLI configured to route through mcpg proxy at ${GH_HOST}"
else
echo "[cli-proxy] WARNING: GH_TOKEN not set - mcpg proxy disabled, gh CLI will not authenticate"
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When GH_TOKEN is missing, the entrypoint disables mcpg and starts the HTTP server anyway. That means gh commands run without DIFC guard enforcement/audit logging (and may still access public GitHub endpoints), which undermines the “same DIFC guarantees” goal. Consider failing closed (exit non-zero) or having the server reject /exec until mcpg is running, when --enable-cli-proxy is enabled but no token is provided.

Suggested change
echo "[cli-proxy] WARNING: GH_TOKEN not set - mcpg proxy disabled, gh CLI will not authenticate"
echo "[cli-proxy] ERROR: GH_TOKEN not set - refusing to start HTTP server without mcpg DIFC enforcement"
exit 1

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +12
"scripts": {
"start": "node server.js",
"test": "jest --verbose --ci"
},
"devDependencies": {
"jest": "^30.2.0"
},
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package defines unit tests (npm test), but the repository’s CI workflow currently only runs container tests for containers/api-proxy (see .github/workflows/build.yml:62-66). As a result, changes to cli-proxy (server.js/validateArgs policy) won’t be exercised in CI. Add a CI step to run cd containers/cli-proxy && npm ci && npm test (or otherwise wire these tests into the main test pipeline).

Copilot uses AI. Check for mistakes.
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Apr 6, 2026

@copilot apply changes based on the comments in this thread

- Dockerfile: Add USER cliproxy so mcpg and HTTP server run as non-root;
  fix log and TLS dir ownership to be writable by cliproxy before USER switch
- entrypoint.sh: Fail closed when GH_TOKEN is missing (exit 1) instead of
  starting an unenforced server; fix signal handling by running Node in the
  background with wait/trap so cleanup kills both Node and mcpg on SIGTERM
- server.js: Add blocked write actions for codespace (create/delete/edit/stop/
  ports), cache (delete), and org (invite) in BLOCKED_ACTIONS_READONLY;
  track subcommandIndex during the scan instead of using args.indexOf() to
  prevent bypass when the subcommand string appears earlier as a flag value;
  add MAX_REQUEST_BODY_BYTES limit in readBody (returns null + 413 on overflow)
- gh-cli-proxy-wrapper.sh: Remove curl -f; capture HTTP status code with -w
  and response body with -o so 4xx policy errors (403) surface the actual
  .error message instead of "CLI proxy unavailable"
- cli.ts: Read GITHUB_TOKEN || GH_TOKEN for githubToken; update warning to
  mention both env var names
- build.yml: Add 'Run CLI proxy unit tests' step parallel to api-proxy step
- server.test.js: Add 11 new tests covering cache/codespace/org blocked
  actions and the subcommandIndex indexOf-bypass fix (58 tests total)

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/4d1512c6-a5af-49dc-9693-2f2c1af0b173
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 6, 2026

@copilot apply changes based on the comments in this thread

Applied all changes from the review in commit 1b3964e:

  • Dockerfile: Added USER cliproxy so both mcpg and the HTTP server run as non-root. Fixed directory ownership (/var/log/cli-proxy, /tmp/proxy-tls) before the USER switch so the non-root process can write to them.
  • entrypoint.sh: Fails closed with exit 1 when GH_TOKEN is missing — the server no longer starts without DIFC enforcement. Fixed signal handling by running Node in the background and using wait + trap, so SIGTERM/SIGINT properly trigger cleanup of both Node and mcpg (avoids the exec-kills-trap problem).
  • server.js: Added write-action blocks for codespace (create/delete/edit/stop/ports), cache (delete), and org (invite) in BLOCKED_ACTIONS_READONLY. Fixed the indexOf bypass by tracking subcommandIndex during the scan rather than calling args.indexOf(subcommand) after the fact. Added MAX_REQUEST_BODY_BYTES limit in readBody — returns null and sends 413 when the body exceeds the limit.
  • gh-cli-proxy-wrapper.sh: Replaced curl -sf with -s -o RESPONSE_FILE -w "%{http_code}" so the response body is captured even on 4xx. Policy errors (403) now surface the actual .error message from the server rather than "CLI proxy unavailable".
  • cli.ts: githubToken now reads GITHUB_TOKEN || GH_TOKEN; the missing-token warning mentions both env var names.
  • build.yml: Added a "Run CLI proxy unit tests" CI step (parallel to the existing api-proxy step).
  • server.test.js: 11 new tests covering cache/codespace/org blocked actions and the subcommandIndex indexOf-bypass fix (58 tests total).

Copilot finished work on behalf of lpcox April 6, 2026 23:12
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Apr 6, 2026

Review: Phase 1 CLI Proxy Sidecar

Excellent implementation — the architecture closely follows the design doc, the test coverage is solid (49 server tests + 18 docker-manager tests), and the security hardening is thorough (cap_drop ALL, no-new-privileges, resource limits, execFile not shell). A few issues to address:


🔴 High: gh api in read-only mode can perform mutations

api is in ALLOWED_SUBCOMMANDS_READONLY, but gh api is a raw HTTP passthrough that can perform POST/PUT/DELETE. For example:

gh api -X POST /repos/owner/repo/issues -f title="Created via api"

This bypasses the entire subcommand allowlist. The mcpg guard policies provide a second layer, but the server.js allowlist gives a false sense of security.

Options:

  1. Remove api from the read-only set entirely (most secure; agents use gh issue list, gh pr list etc. instead)
  2. Parse -X/--method flag and only allow GET in read-only mode
  3. Accept that mcpg is the real enforcement layer and document this in a code comment

I'd recommend option 1 — agents rarely need raw gh api when the typed subcommands are available.


🟡 Medium: Missing --trusted-bots in mcpg invocation

The reference start_difc_proxy.sh passes --trusted-bots github-actions[bot],github-actions,dependabot[bot],copilot to mcpg. The cli-proxy entrypoint.sh omits this flag. Without it, guard policies may reject legitimate bot-authored content (e.g., Dependabot PRs, Copilot-authored commits).

 mcpg proxy \
   --policy "${AWF_GH_GUARD_POLICY}" \
   --listen 127.0.0.1:18443 \
   --tls \
   --tls-dir /tmp/proxy-tls \
   --guards-mode filter \
+  --trusted-bots github-actions[bot],github-actions,dependabot[bot],copilot \
   --log-dir /var/log/cli-proxy/mcpg &

🟡 Medium: Dockerfile pins ghcr.io/github/gh-aw-mcpg:latest

The multi-stage FROM uses :latest, making builds non-deterministic. Should pin to a specific version (e.g., :v0.2.2 or whatever is current) for reproducibility. The api-proxy Dockerfile pins its base images.

-FROM ghcr.io/github/gh-aw-mcpg:latest AS mcpg-source
+FROM ghcr.io/github/gh-aw-mcpg:v0.2.2 AS mcpg-source

🟢 Low: Token exclusion from agent env

When enableCliProxy is true, GITHUB_TOKEN/GH_TOKEN are added to EXCLUDED_ENV_VARS. This is the right thing for gh CLI isolation, but worth noting that unlike api-proxy (which excludes LLM keys), this excludes a token used by many tools. In practice it should be fine since actions/checkout runs before AWF and GITHUB_MCP_SERVER_TOKEN is a separate env var, but a comment noting this design choice would help future readers.


🟢 Nit: Section comment ordering in cli.ts

The // -- Logging & Debug -- section comment was replaced by // -- CLI Proxy --, but the logging options that follow lost their section header. Minor, but the existing code has clear section delimiters.


✅ Things done well

  • Security model is correct: GH_TOKEN → mcpg proxy → guard policies → gh CLI → Squid. Agent never sees the token.
  • Both chroot and non-chroot wrapper activation paths handled correctly (PATH prepending with /tmp/awf-lib)
  • Protected env keys in server.js (GH_HOST, GH_TOKEN, GITHUB_TOKEN, NODE_EXTRA_CA_CERTS) prevents agent from overriding security-critical vars via the env field
  • iptables RETURN rule correctly placed (parallel to api-proxy pattern)
  • Log preservation follows the existing Squid/API-proxy pattern exactly
  • Startup order: squid (healthy) → cli-proxy (healthy) → agent. Correct.
  • Default guard policy falls back to {"repos":["$GITHUB_REPOSITORY"],"min-integrity":"public"} when no explicit policy is provided. Good default.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Apr 6, 2026

@copilot apply changes based on the comments in #1730 (comment)

- server.js: Remove 'api' from ALLOWED_SUBCOMMANDS_READONLY; raw HTTP
  passthrough can mutate via -X POST/PUT/DELETE. 'api' allowed in writable
  mode. Add explanatory comment. (61 tests total)
- entrypoint.sh: Add --trusted-bots flag to mcpg so guard policies don't
  reject bot-authored content (Dependabot, Copilot, github-actions)
- Dockerfile: Pin ghcr.io/github/gh-aw-mcpg :latest -> :v0.2.2 for
  reproducible builds
- docker-manager.ts: Expand GITHUB_TOKEN exclusion comment explaining
  design intent and why it is safe in practice
- cli.ts: Restore // -- Logging & Debug -- section header before --log-level
- server.test.js: Add 3 new tests (61 total)

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/2e1f5421-3e78-44f2-a1c0-d2920f65a173
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 6, 2026

@copilot apply changes based on the comments in #1730 (comment)

Applied all actionable items from the review in commit d99431d:

  • gh api in read-only mode: Removed api from ALLOWED_SUBCOMMANDS_READONLY (option 1 — most secure). Added a comment explaining the rationale. api remains available in writable mode. Updated tests to assert denial in read-only mode and allowance in writable mode (61 tests total).
  • --trusted-bots: Added --trusted-bots "github-actions[bot],github-actions,dependabot[bot],copilot" to the mcpg invocation in entrypoint.sh.
  • Image pin: Changed ghcr.io/github/gh-aw-mcpg:latest:v0.2.2 in Dockerfile.
  • Token exclusion comment: Expanded the comment in docker-manager.ts to explain the design intent and why excluding GITHUB_TOKEN is safe in practice.
  • Section comment: Restored // -- Logging & Debug -- before --log-level in cli.ts.

Copilot finished work on behalf of lpcox April 6, 2026 23:26
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Apr 6, 2026

@copilot make the gh-aw-mcpg container image an awf command-line parameter that the compiler needs to set. update the issue to reflec that the compiler will need to set this since it decides which mcpg version to pull and run.

Make the gh-aw-mcpg source image configurable via CLI so the AWF compiler
can control which mcpg version is pulled and run in the cli-proxy sidecar:

- Dockerfile: use ARG MCPG_IMAGE (default ghcr.io/github/gh-aw-mcpg:v0.2.2)
- src/types.ts: add cliProxyMcpgImage to WrapperConfig
- src/cli.ts: add --cli-proxy-mcpg-image flag under CLI Proxy section
- src/docker-manager.ts: pass MCPG_IMAGE build arg when building locally
- src/docker-manager.test.ts: 3 new tests (320 total)

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/15ba6c64-db6c-4c47-b813-21878fb57168
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 6, 2026

@copilot make the gh-aw-mcpg container image an awf command-line parameter that the compiler needs to set. update the issue to reflec that the compiler will need to set this since it decides which mc...

Done in commit 3f51a22. The mcpg source image is now controlled via --cli-proxy-mcpg-image <image> (default: ghcr.io/github/gh-aw-mcpg:v0.2.2):

  • Dockerfile: FROM ghcr.io/github/gh-aw-mcpg:v0.2.2ARG MCPG_IMAGE=ghcr.io/github/gh-aw-mcpg:v0.2.2 / FROM ${MCPG_IMAGE} — overridable at build time.
  • src/types.ts: cliProxyMcpgImage?: string added to WrapperConfig; JSDoc notes it only applies to --build-local (pre-built GHCR images already have mcpg bundled).
  • src/cli.ts: --cli-proxy-mcpg-image <image> flag added under the // -- CLI Proxy -- section; help text explains the compiler use case.
  • src/docker-manager.ts: passes MCPG_IMAGE as a Docker build arg when --build-local + cliProxyMcpgImage are set; omitted otherwise (Dockerfile ARG default takes effect).
  • 3 new tests added (320 total).

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Security Review — PR #1730 (CLI proxy sidecar)

Overall the PR is well-structured with good security foundations: execFile (no shell), protected env keys, token isolation in mcpg, capability dropping, resource limits, and an allowlist/blocklist pattern. One security issue was found that deserves attention.


🔴 Flag-Parsing Bypass for ALWAYS_DENIED_SUBCOMMANDS (Write Mode)

File: containers/cli-proxy/server.jsvalidateArgs() (~line 136)

The subcommand-detection loop skips over any --flag (without =) and the following argument, assuming it is the flag's value:

if (!arg.includes('=') && i + 1 < args.length && !args[i + 1].startsWith('-')) {
  // Flag with a separate value (e.g., --repo owner/repo): skip both
  i += 2;   // ← skips flag AND next arg
}
```

This logic correctly handles `gh --repo owner/repo pr list`, but it misclassifies **boolean global flags** (flags that take no value) as value-taking flags. This means the argument immediately after a boolean flag is silently dropped from consideration, including the words `auth`, `config`, and `extension`.

**Concrete bypass in write mode:**

```
args: ["--version", "auth", "login"]

Walk-through:

  1. --version — no =, next arg auth doesn't start with - → parser skips both (treats auth as --version's value)
  2. Remaining: ["login"] → detected subcommand is login
  3. login is not in ALWAYS_DENIED_SUBCOMMANDS → passes validation ✓
  4. execFile('gh', ['--version', 'auth', 'login']) is called — auth login is the actual command executed

The same pattern applies to -h / --help before config or extension.

Is this immediately exploitable? --version and --help cause gh to print and exit before processing the subcommand, so the immediate practical risk is mitigated. However:

  • This is a clear violation of the stated invariant: "auth, config, extension are always denied, even in writable mode"
  • If a future gh release adds a boolean global flag that does not cause early exit, the bypass becomes fully exploitable
  • The flaw is not covered by the existing test suite (tests check the indexOf bypass but not the boolean-flag skip case)

Recommended fix: Enumerate the known value-taking global flags explicitly, and treat everything else as boolean (one-skip). For example:

const VALUE_FLAGS = new Set(['--repo', '-R', '--jq', '-q', '--template', '-t', '--json', '--limit', '-L', '--branch', '-b']);

if (arg.startsWith('-')) {
  const isValueFlag = VALUE_FLAGS.has(arg) || (arg.includes('='));
  if (!isValueFlag && i + 1 < args.length && !args[i + 1].startsWith('-')) {
    // boolean flag followed by a non-flag arg — check if next arg IS the value
    // (conservative: only skip if we know this flag takes a value)
    i += 1;  // skip just the flag
  } else if (!arg.includes('=') && VALUE_FLAGS.has(arg)) {
    i += 2;  // flag + value
  } else {
    i += 1;
  }
}

Or add a regression test asserting that ["--version", "auth", "login"] is rejected:

it('should deny auth even when preceded by boolean flag --version', () => {
  const result = validateArgs(['--version', 'auth', 'login'], true);
  expect(result.valid).toBe(false);
});

ℹ️ Informational: No Authentication on /exec Endpoint

File: containers/cli-proxy/server.js — line 362

server.listen(CLI_PROXY_PORT, '0.0.0.0', () => { ... });

The HTTP server accepts unauthenticated requests from any container in awf-net. In the current topology, that means Squid (172.30.0.10) and the API proxy (172.30.0.30) could both call /exec and execute gh commands with the held GH_TOKEN.

This mirrors the api-proxy design (also unauthenticated within the network) and the blast radius is limited by the isolated Docker network. Noting it here as a defense-in-depth consideration — a simple shared-secret header (generated at container startup, passed to the agent via AWF_CLI_PROXY_TOKEN) would eliminate the intra-network trust requirement entirely.

No action required for this item; flagging for awareness.

Generated by Security Guard for issue #1730 · ● 200.7K ·

@lpcox lpcox disabled auto-merge April 7, 2026 00:00
@lpcox lpcox changed the title feat: Phase 1 – gh CLI proxy sidecar with mcpg DIFC proxy feat: phase 1 – gh CLI proxy sidecar with mcpg DIFC proxy Apr 7, 2026
Add tests for:
- resolveImages with enableCliProxy
- resolveImages with both enableApiProxy and enableCliProxy
- predownloadCommand with enableCliProxy
- handlePredownloadAction forwarding enableCliProxy

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 86.20% 86.10% 📉 -0.10%
Statements 86.07% 85.97% 📉 -0.10%
Functions 87.41% 87.41% ➡️ +0.00%
Branches 78.56% 78.65% 📈 +0.09%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 61.8% → 61.3% (-0.48%) 62.3% → 61.8% (-0.47%)
src/docker-manager.ts 86.6% → 86.7% (+0.08%) 86.1% → 86.2% (+0.10%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Extract CLI proxy logging into testable emitCliProxyStatusLogs()
function (same pattern as emitApiProxyTargetWarnings). Add tests for:
- disabled/undefined cli proxy (no-op)
- enabled with token present (read-only and writable modes)
- enabled with missing token (warning messages)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 86.20% 86.23% 📈 +0.03%
Statements 86.07% 86.11% 📈 +0.04%
Functions 87.41% 87.45% 📈 +0.04%
Branches 78.56% 78.86% 📈 +0.30%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/docker-manager.ts 86.6% → 86.7% (+0.08%) 86.1% → 86.2% (+0.10%)
src/cli.ts 61.8% → 62.1% (+0.26%) 62.3% → 62.6% (+0.31%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Smoke Test Results

  • ✅ GitHub MCP: "chore: upgrade workflows to gh-aw-actions v0.67.2", "fix(smoke-services): add GitHub Actions services block and use --allow-host-service-ports"
  • ✅ Playwright: GitHub page title contains "GitHub"
  • ✅ File write: /tmp/gh-aw/agent/smoke-test-claude-24057582147.txt created
  • ✅ Bash: File verified via cat

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

🔥 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP ⚠️ (pre-step data unavailable)
File write/read (smoke-test-copilot-24057582192.txt)

Overall: PASS (2/2 verifiable tests passed)

PR: feat: phase 1 – gh CLI proxy sidecar with mcpg DIFC proxy
Author: @Copilot · Assignees: @lpcox, @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Smoke Test: GitHub Actions Services Connectivity ✅

All checks passed against host.docker.internal:

Check Result
Redis PING (port 6379) PONG
pg_isready (port 5432) ✅ accepting connections
psql SELECT 1 (smoketest DB) ✅ returned 1

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Security Review — Action-Level Blocking Bypass

Severity: Medium

Summary

The validateArgs() function in containers/cli-proxy/server.js correctly uses flag-skipping logic to find the subcommand (lines 116–131), but uses a simpler scan to find the action (line 155) that does not skip flag values. This creates a bypass for all action-level blocks in read-only mode.

Vulnerable Code

containers/cli-proxy/server.js, line 155:

const action = args.slice(subcommandIndex + 1).find(a => !a.startsWith('-'));
```

`find(a => !a.startsWith('-'))` returns the first argument that doesn't begin with `-`, which includes **flag values** (e.g., `owner/repo` in `--repo owner/repo`). When a flag with a non-flag-like value appears before the real action, the flag's value is mistakenly treated as the action.

### Proof-of-Concept Bypass

In read-only mode, `pr create` is in `BLOCKED_ACTIONS_READONLY`. However:

```
args = ['pr', '--repo', 'owner/repo', 'create']
  1. subcommandIndex = 0 (pr)
  2. args.slice(1) = ['--repo', 'owner/repo', 'create']
  3. .find(a => !a.startsWith('-'))'owner/repo' ← (flag value, not the action)
  4. blockedActions.has('owner/repo')false✅ allowed

The agent can then execute gh pr create despite the read-only policy. This works for any blocked action where a preceding flag takes a non-flag-like value.

Affected blocked actions (non-exhaustive):

  • pr create/merge/review/…gh pr --repo owner/repo create
  • issue create/close/editgh issue --repo owner/repo create
  • secret set/deletegh secret --repo owner/repo set MY_SECRET
  • run cancel/rerungh run --workflow ci.yml cancel
  • workflow run/enable/disablegh workflow --repo owner/repo run ci.yml

Missing Test Coverage

No test covers ['pr', '--repo', 'owner/repo', 'create'] (flag+value before a blocked action). The existing test for "flags before action" (['pr', '--json', 'number', 'list']) uses a non-blocked action (list), so it does not catch the bypass.

Suggested Fix

Apply the same flag-skipping logic used for subcommand detection to the action scan:

// Instead of:
const action = args.slice(subcommandIndex + 1).find(a => !a.startsWith('-'));

// Use:
function findFirstNonFlagArg(args) {
  let i = 0;
  while (i < args.length) {
    const arg = args[i];
    if (arg.startsWith('-')) {
      // Skip flag and its value (if next arg is not a flag)
      if (!arg.includes('=') && i + 1 < args.length && !args[i + 1].startsWith('-')) {
        i += 2;
      } else {
        i += 1;
      }
    } else {
      return arg;
    }
  }
  return undefined;
}

const action = findFirstNonFlagArg(args.slice(subcommandIndex + 1));

And add a regression test:

it('should deny pr create when a flag with a value precedes the action', () => {
  // gh pr --repo owner/repo create → must be blocked
  const result = validateArgs(['pr', '--repo', 'owner/repo', 'create'], false);
  expect(result.valid).toBe(false);
});

Notes

  • The mcpg DIFC proxy provides an independent enforcement layer, which limits the real-world blast radius.
  • The vulnerability only affects the validateArgs read-only guard in the Node HTTP server — execFile (no shell) and token isolation are unaffected.
  • All other security properties reviewed (token isolation, subcommand allowlist, injection prevention, resource limits, capability dropping, iptables rules) look correct.

Generated by Security Guard for issue #1730 · ● 239K ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.14.1 v20.20.2 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environment.

Tested by Smoke Chroot

@lpcox lpcox merged commit 93903cf into main Apr 7, 2026
62 of 64 checks passed
@lpcox lpcox deleted the copilot/design-gh-cli-proxy-sidecar branch April 7, 2026 00:39
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Smoke test matrix (run 24058930701):

  • feat: phase 1 – gh CLI proxy sidecar with mcpg DIFC proxy ✅
  • chore: upgrade workflows to gh-aw-actions v0.67.2 ✅
  • safeinputs-gh PR query ❌ (tool unavailable)
  • Playwright title check ❌ (EACCES on /tmp/gh-aw/mcp-logs/playwright)
  • Tavily search ❌ (tool unavailable)
  • File write/read ✅
  • Discussion oracle comment ❌ (github-discussion-query unavailable)
  • npm ci && npm run build ✅
    Overall status: FAIL

🔮 The oracle has spoken through Smoke Codex

Copilot AI added a commit that referenced this pull request Apr 7, 2026
Add build-cli-proxy job to release.yml following the same pattern as
api-proxy: build+push to GHCR, cosign signing, SBOM generation, and
SBOM attestation. Also update docs/releasing.md to document the new
cli-proxy image.

The predownload logic already includes cli-proxy support from PR #1730.

Fixes #1746

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/d6d42f21-06c7-45b4-8d7f-f60f1d597851

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
lpcox added a commit that referenced this pull request Apr 8, 2026
* Initial plan

* fix: add cli-proxy container image to release workflow

Add build-cli-proxy job to release.yml following the same pattern as
api-proxy: build+push to GHCR, cosign signing, SBOM generation, and
SBOM attestation. Also update docs/releasing.md to document the new
cli-proxy image.

The predownload logic already includes cli-proxy support from PR #1730.

Fixes #1746

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/d6d42f21-06c7-45b4-8d7f-f60f1d597851

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Design: gh CLI proxy sidecar with mcpg DIFC proxy

3 participants