Skip to content

Commit 1fe1010

Browse files
CopilotlpcoxCopilotgithub-advanced-security[bot]
authored
Redesign cli-proxy: connect to external DIFC proxy started by compiler (#1809)
* Initial plan * feat: redesign cli-proxy to connect to external DIFC proxy Replace internal mcpg container management with external DIFC proxy connection. The gh-aw compiler now starts mcpg on the host; AWF only launches the cli-proxy container and connects via TCP tunnel. New CLI flags: - --difc-proxy-host <host:port>: connect to external DIFC proxy - --difc-proxy-ca-cert <path>: TLS CA cert from DIFC proxy Removed CLI flags: - --enable-cli-proxy - --cli-proxy-writable - --cli-proxy-policy - --cli-proxy-mcpg-image Key changes: - Remove mcpg service from Docker Compose generation - cli-proxy gets its own IP (172.30.0.50) with extra_hosts - TCP tunnel forwards localhost to host DIFC proxy for TLS - Write control delegated to DIFC guard policy - Remove cli-proxy-tls named volume (CA cert bind-mounted) - Update all tests (1374 pass) Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a38ec447-a23c-4104-be85-bd7328c48559 * docs: update gh-cli-proxy-design.md for external DIFC proxy Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a38ec447-a23c-4104-be85-bd7328c48559 * fix: address review feedback for cli-proxy redesign - Add port validation and error logging to tcp-tunnel.js - Make NODE_EXTRA_CA_CERTS conditional on cert existence - Fix CLI help text for --difc-proxy-ca-cert Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a38ec447-a23c-4104-be85-bd7328c48559 * fix: apply PR review feedback for cli-proxy redesign - Use URL-based parser for difcProxyHost (IPv6 support) - Add host iptables rule for cli-proxy → DIFC proxy port - Add 'alias' to ALWAYS_DENIED_SUBCOMMANDS (shell exec) - Fail fast when CA cert not found in entrypoint.sh - Add server.on('error') handler in tcp-tunnel.js - Rename --enable-cli-proxy to --difc-proxy in predownload Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/11e4f4ef-6a07-4318-9e6f-aba436bd9590 * refactor: extract parseDifcProxyHost to shared utility Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/11e4f4ef-6a07-4318-9e6f-aba436bd9590 * fix: remove CA cert poll loop, update stale flag comments The CA cert is now a bind mount (not generated at runtime by mcpg), so it's immediately available at container start. Replace the 30s polling loop with a single existence check for faster startup. Also update stale comments referencing --enable-cli-proxy to --difc-proxy-host in setup-iptables.sh and gh-cli-proxy-wrapper.sh. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding 'CodeQL / Log injection' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Landon Cox <landon.cox@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
1 parent acffaea commit 1fe1010

20 files changed

+516
-910
lines changed

‎containers/agent/gh-cli-proxy-wrapper.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# /usr/local/bin/gh-cli-proxy-wrapper
33
# Forwards gh CLI invocations to the CLI proxy sidecar over HTTP.
44
# This wrapper is installed at /usr/local/bin/gh in the agent container
5-
# when --enable-cli-proxy is active, so it takes precedence over any
5+
# when --difc-proxy-host is active, so it takes precedence over any
66
# host-mounted gh binary at /host/usr/bin/gh.
77
#
88
# Dependencies: curl, jq (both available in the agent container)

‎containers/agent/setup-iptables.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ if [ -n "$AWF_API_PROXY_IP" ]; then
174174
fi
175175

176176
# Allow traffic to CLI proxy sidecar (when enabled)
177-
# AWF_CLI_PROXY_IP is set by docker-manager.ts when --enable-cli-proxy is used
177+
# AWF_CLI_PROXY_IP is set by docker-manager.ts when --difc-proxy-host is used
178178
if [ -n "$AWF_CLI_PROXY_IP" ]; then
179179
echo "[iptables] Allow traffic to CLI proxy sidecar (${AWF_CLI_PROXY_IP})..."
180180
iptables -t nat -A OUTPUT -d "$AWF_CLI_PROXY_IP" -j RETURN

‎containers/cli-proxy/Dockerfile‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
# CLI Proxy sidecar for AWF - provides gh CLI access via mcpg DIFC proxy
1+
# CLI Proxy sidecar for AWF - provides gh CLI access via external DIFC proxy
22
#
33
# This container runs the HTTP exec server (port 11000) that receives gh
4-
# invocations from the agent container. The mcpg DIFC proxy runs as a
5-
# separate docker-compose service (awf-cli-proxy-mcpg) using the official
6-
# gh-aw-mcpg image directly — no binary extraction needed. GH_HOST is
7-
# set to the mcpg container so all gh CLI traffic flows through the proxy.
4+
# invocations from the agent container. The DIFC proxy (mcpg) runs
5+
# externally on the host, started by the gh-aw compiler. A TCP tunnel
6+
# forwards localhost traffic to the external proxy for TLS hostname matching.
87
FROM node:22-alpine
98

109
# Install gh CLI and curl for healthchecks/wrapper
@@ -25,7 +24,7 @@ COPY package*.json ./
2524
RUN npm ci --omit=dev
2625

2726
# Copy application files
28-
COPY server.js ./
27+
COPY server.js tcp-tunnel.js ./
2928
COPY entrypoint.sh /usr/local/bin/entrypoint.sh
3029
COPY healthcheck.sh /usr/local/bin/healthcheck.sh
3130

@@ -38,7 +37,7 @@ RUN addgroup -S cliproxy && adduser -S cliproxy -G cliproxy
3837
RUN mkdir -p /var/log/cli-proxy && \
3938
chown -R cliproxy:cliproxy /var/log/cli-proxy
4039

41-
# Create /tmp/proxy-tls directory owned by cliproxy for shared mcpg TLS certs
40+
# Create /tmp/proxy-tls directory owned by cliproxy for mounted DIFC proxy CA cert
4241
RUN mkdir -p /tmp/proxy-tls && chown cliproxy:cliproxy /tmp/proxy-tls
4342

4443
# Switch to non-root user

‎containers/cli-proxy/entrypoint.sh‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,61 @@
11
#!/bin/bash
22
# CLI Proxy sidecar entrypoint
33
#
4-
# The mcpg DIFC proxy runs as a separate docker-compose service
5-
# (awf-cli-proxy-mcpg). This container shares mcpg's network namespace
6-
# (network_mode: service:cli-proxy-mcpg), so localhost resolves to mcpg.
7-
# This ensures the TLS cert's SAN (localhost + 127.0.0.1) matches the
8-
# hostname used by the gh CLI, avoiding TLS hostname verification failures.
4+
# Connects to an external DIFC proxy (mcpg) started by the gh-aw compiler
5+
# on the host. Uses a TCP tunnel to forward localhost:${DIFC_PORT} to
6+
# ${DIFC_HOST}:${DIFC_PORT}, so the gh CLI can connect via localhost
7+
# (matching the DIFC proxy's TLS cert SAN for localhost/127.0.0.1).
98
set -e
109

1110
echo "[cli-proxy] Starting CLI proxy sidecar..."
1211

1312
NODE_PID=""
13+
TUNNEL_PID=""
1414

15-
# cli-proxy shares mcpg's network namespace, so mcpg is always at localhost.
16-
# AWF_MCPG_PORT is set by docker-manager.ts.
17-
MCPG_PORT="${AWF_MCPG_PORT:-18443}"
15+
# External DIFC proxy host and port, set by docker-manager.ts
16+
DIFC_HOST="${AWF_DIFC_PROXY_HOST:-host.docker.internal}"
17+
DIFC_PORT="${AWF_DIFC_PROXY_PORT:-18443}"
1818

19-
echo "[cli-proxy] mcpg proxy at localhost:${MCPG_PORT}"
19+
echo "[cli-proxy] External DIFC proxy at ${DIFC_HOST}:${DIFC_PORT}"
2020

21-
# Wait for TLS cert to appear in the shared volume (max 30s)
22-
echo "[cli-proxy] Waiting for mcpg TLS certificate..."
23-
i=0
24-
while [ $i -lt 30 ]; do
25-
if [ -f /tmp/proxy-tls/ca.crt ]; then
26-
echo "[cli-proxy] TLS certificate available"
27-
break
28-
fi
29-
sleep 1
30-
i=$((i + 1))
31-
done
21+
# Start the TCP tunnel: localhost:${DIFC_PORT} → ${DIFC_HOST}:${DIFC_PORT}
22+
# This allows the gh CLI to connect via localhost, matching the cert's SAN.
23+
echo "[cli-proxy] Starting TCP tunnel: localhost:${DIFC_PORT} → ${DIFC_HOST}:${DIFC_PORT}"
24+
node /app/tcp-tunnel.js "${DIFC_PORT}" "${DIFC_HOST}" "${DIFC_PORT}" &
25+
TUNNEL_PID=$!
3226

27+
# Verify CA cert is available (bind-mounted from host by docker-manager.ts).
28+
# Unlike the old architecture where mcpg generated the cert at runtime, the
29+
# external DIFC proxy has already created the cert before AWF starts, so the
30+
# bind mount makes it immediately available — no polling needed.
3331
if [ ! -f /tmp/proxy-tls/ca.crt ]; then
34-
echo "[cli-proxy] ERROR: mcpg TLS certificate not found within 30s"
32+
echo "[cli-proxy] ERROR: DIFC proxy TLS certificate not found at /tmp/proxy-tls/ca.crt"
33+
echo "[cli-proxy] Ensure --difc-proxy-ca-cert points to a valid CA cert file on the host"
3534
exit 1
3635
fi
36+
echo "[cli-proxy] TLS certificate available"
3737

38-
# Configure gh CLI to route through the mcpg proxy (TLS, self-signed CA)
39-
# Uses localhost because cli-proxy shares mcpg's network namespace — the
40-
# self-signed cert's SAN covers localhost, so TLS hostname verification passes.
41-
export GH_HOST="localhost:${MCPG_PORT}"
42-
export NODE_EXTRA_CA_CERTS="/tmp/proxy-tls/ca.crt"
38+
# Configure gh CLI to route through the DIFC proxy via the TCP tunnel
39+
# Uses localhost because the tunnel makes the DIFC proxy appear on localhost,
40+
# matching the self-signed cert's SAN.
41+
export GH_HOST="localhost:${DIFC_PORT}"
4342
export GH_REPO="${GH_REPO:-$GITHUB_REPOSITORY}"
43+
# The CA cert is guaranteed to exist at this point (we exit above if missing)
44+
export NODE_EXTRA_CA_CERTS="/tmp/proxy-tls/ca.crt"
4445

45-
echo "[cli-proxy] gh CLI configured to route through mcpg proxy at ${GH_HOST}"
46+
echo "[cli-proxy] gh CLI configured to route through DIFC proxy at ${GH_HOST}"
4647

47-
# Cleanup handler: stop the Node HTTP server on signal
48+
# Cleanup handler: stop the Node HTTP server and TCP tunnel on signal
4849
cleanup() {
4950
echo "[cli-proxy] Shutting down..."
5051
if [ -n "$NODE_PID" ]; then
5152
kill "$NODE_PID" 2>/dev/null || true
5253
wait "$NODE_PID" 2>/dev/null || true
5354
fi
55+
if [ -n "$TUNNEL_PID" ]; then
56+
kill "$TUNNEL_PID" 2>/dev/null || true
57+
wait "$TUNNEL_PID" 2>/dev/null || true
58+
fi
5459
}
5560
trap 'cleanup; exit 0' INT TERM
5661

‎containers/cli-proxy/server.js‎

Lines changed: 14 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@
77
* POST /exec - Execute a gh CLI command and return stdout/stderr/exitCode
88
*
99
* Security:
10-
* - Subcommand allowlist enforced (read-only mode by default)
1110
* - Args are exec'd directly via execFile (no shell, no injection)
1211
* - Per-command timeout (default 30s)
1312
* - Max output size limit to prevent memory exhaustion
13+
* - Meta-commands (auth, config, extension) are always denied
1414
*
15-
* The gh CLI running inside this container has GH_HOST set to the mcpg proxy
16-
* (localhost:18443), so it never sees GH_TOKEN directly.
15+
* The gh CLI running inside this container has GH_HOST set to the DIFC proxy
16+
* (localhost:18443 via TCP tunnel), so it never sees GH_TOKEN directly.
17+
* Write control is handled by the DIFC guard policy, not by this server.
1718
*/
1819

1920
const http = require('http');
@@ -23,77 +24,26 @@ const CLI_PROXY_PORT = parseInt(process.env.AWF_CLI_PROXY_PORT || '11000', 10);
2324
const COMMAND_TIMEOUT_MS = parseInt(process.env.AWF_CLI_PROXY_TIMEOUT_MS || '30000', 10);
2425
const MAX_OUTPUT_BYTES = parseInt(process.env.AWF_CLI_PROXY_MAX_OUTPUT_BYTES || String(10 * 1024 * 1024), 10);
2526

26-
// When AWF_CLI_PROXY_WRITABLE=true, allow write operations
27-
const WRITABLE_MODE = process.env.AWF_CLI_PROXY_WRITABLE === 'true';
28-
29-
/**
30-
* Subcommands allowed in read-only mode.
31-
* These commands only retrieve data and do not modify any GitHub resources.
32-
*
33-
* Note: 'api' is intentionally excluded even in read-only mode because it is a raw
34-
* HTTP passthrough that can perform arbitrary POST/PUT/DELETE mutations via -X/--method.
35-
* Agents should use typed subcommands (gh issue list, gh pr view, etc.) instead.
36-
* In writable mode, 'api' is permitted since the operator has explicitly opted in.
37-
*/
38-
const ALLOWED_SUBCOMMANDS_READONLY = new Set([
39-
'browse',
40-
'cache',
41-
'codespace',
42-
'gist',
43-
'issue',
44-
'label',
45-
'org',
46-
'pr',
47-
'release',
48-
'repo',
49-
'run',
50-
'search',
51-
'secret',
52-
'variable',
53-
'workflow',
54-
]);
55-
56-
/**
57-
* Actions that are blocked within their parent subcommand in read-only mode.
58-
* Maps subcommand -> Set of blocked action verbs.
59-
*/
60-
const BLOCKED_ACTIONS_READONLY = new Map([
61-
// cache: delete is a write operation
62-
['cache', new Set(['delete'])],
63-
// codespace: create, delete, edit, stop, ports forward are write operations
64-
['codespace', new Set(['create', 'delete', 'edit', 'stop', 'ports'])],
65-
['gist', new Set(['create', 'delete', 'edit'])],
66-
['issue', new Set(['create', 'close', 'delete', 'edit', 'lock', 'pin', 'reopen', 'transfer', 'unpin'])],
67-
['label', new Set(['create', 'delete', 'edit'])],
68-
// org: invite changes org membership
69-
['org', new Set(['invite'])],
70-
['pr', new Set(['checkout', 'close', 'create', 'edit', 'lock', 'merge', 'ready', 'reopen', 'review', 'update-branch'])],
71-
['release', new Set(['create', 'delete', 'delete-asset', 'edit', 'upload'])],
72-
['repo', new Set(['archive', 'create', 'delete', 'edit', 'fork', 'rename', 'set-default', 'sync', 'unarchive'])],
73-
['run', new Set(['cancel', 'delete', 'download', 'rerun'])],
74-
['secret', new Set(['delete', 'set'])],
75-
['variable', new Set(['delete', 'set'])],
76-
['workflow', new Set(['disable', 'enable', 'run'])],
77-
]);
78-
7927
/**
80-
* Meta-commands that are always denied, even in write mode.
28+
* Meta-commands that are always denied.
8129
* These modify gh itself rather than GitHub resources.
8230
*/
8331
const ALWAYS_DENIED_SUBCOMMANDS = new Set([
32+
'alias',
8433
'auth',
8534
'config',
8635
'extension',
8736
]);
8837

8938
/**
90-
* Validates the gh CLI arguments against the subcommand allowlist.
39+
* Validates the gh CLI arguments.
40+
* Write control is handled by the DIFC guard policy — this server only
41+
* blocks meta-commands that modify gh CLI itself.
9142
*
9243
* @param {string[]} args - The argument array (excluding 'gh' itself)
93-
* @param {boolean} writable - Whether write operations are permitted
9444
* @returns {{ valid: boolean, error?: string }}
9545
*/
96-
function validateArgs(args, writable) {
46+
function validateArgs(args) {
9747
if (!Array.isArray(args)) {
9848
return { valid: false, error: 'args must be an array' };
9949
}
@@ -105,13 +55,7 @@ function validateArgs(args, writable) {
10555
}
10656

10757
// Find the subcommand by scanning through args, skipping flags and their values.
108-
// Handles patterns like: gh --repo owner/repo pr list
109-
// Strategy: when we see --flag (without =), assume the next non-flag-like arg is its value.
110-
// We also track the subcommand's index so that subsequent action detection doesn't
111-
// accidentally pick up a flag value that happens to equal the subcommand string
112-
// (e.g. gh --repo pr pr merge 1 would be wrongly parsed by indexOf).
11358
let subcommand = null;
114-
let subcommandIndex = -1;
11559
let i = 0;
11660
while (i < args.length) {
11761
const arg = args[i];
@@ -125,7 +69,6 @@ function validateArgs(args, writable) {
12569
}
12670
} else {
12771
subcommand = arg;
128-
subcommandIndex = i;
12972
break;
13073
}
13174
}
@@ -140,28 +83,6 @@ function validateArgs(args, writable) {
14083
return { valid: false, error: `Subcommand '${subcommand}' is not permitted` };
14184
}
14285

143-
if (!writable) {
144-
// Read-only mode: check allowlist
145-
if (!ALLOWED_SUBCOMMANDS_READONLY.has(subcommand)) {
146-
return { valid: false, error: `Subcommand '${subcommand}' is not allowed in read-only mode. Enable write mode with --cli-proxy-writable.` };
147-
}
148-
149-
// Check action-level blocklist
150-
const blockedActions = BLOCKED_ACTIONS_READONLY.get(subcommand);
151-
if (blockedActions) {
152-
// The action is the first non-flag argument after the subcommand.
153-
// Use the tracked subcommandIndex (not indexOf) to avoid false matches when
154-
// the subcommand string also appears as a flag value earlier in the args array.
155-
const action = args.slice(subcommandIndex + 1).find(a => !a.startsWith('-'));
156-
if (action && blockedActions.has(action)) {
157-
return {
158-
valid: false,
159-
error: `Action '${subcommand} ${action}' is not allowed in read-only mode. Enable write mode with --cli-proxy-writable.`,
160-
};
161-
}
162-
}
163-
}
164-
16586
return { valid: true };
16687
}
16788

@@ -221,7 +142,7 @@ function sendError(res, statusCode, message) {
221142
* Handle GET /health
222143
*/
223144
function handleHealth(res) {
224-
const body = JSON.stringify({ status: 'ok', service: 'cli-proxy', writable: WRITABLE_MODE });
145+
const body = JSON.stringify({ status: 'ok', service: 'cli-proxy' });
225146
res.writeHead(200, {
226147
'Content-Type': 'application/json',
227148
'Content-Length': Buffer.byteLength(body),
@@ -261,7 +182,7 @@ async function handleExec(req, res) {
261182
const { args, cwd, stdin, env: extraEnv } = body;
262183

263184
// Validate args
264-
const validation = validateArgs(args, WRITABLE_MODE);
185+
const validation = validateArgs(args);
265186
if (!validation.valid) {
266187
return sendError(res, 403, validation.error);
267188
}
@@ -364,7 +285,7 @@ if (require.main === module) {
364285
});
365286

366287
server.listen(CLI_PROXY_PORT, '0.0.0.0', () => {
367-
console.log(`[cli-proxy] HTTP server listening on port ${CLI_PROXY_PORT} (writable=${WRITABLE_MODE})`);
288+
console.log(`[cli-proxy] HTTP server listening on port ${CLI_PROXY_PORT}`);
368289
});
369290

370291
server.on('error', err => {
@@ -373,4 +294,4 @@ if (require.main === module) {
373294
});
374295
}
375296

376-
module.exports = { validateArgs, ALLOWED_SUBCOMMANDS_READONLY, BLOCKED_ACTIONS_READONLY, ALWAYS_DENIED_SUBCOMMANDS };
297+
module.exports = { validateArgs, ALWAYS_DENIED_SUBCOMMANDS };

0 commit comments

Comments
 (0)