fix: compute Docker socket GID separately for shell expansion#26771
fix: compute Docker socket GID separately for shell expansion#26771
Conversation
The $(stat -c '%g' /var/run/docker.sock) command embedded in
MCP_GATEWAY_DOCKER_COMMAND doesn't expand because:
1. The env var value is single-quoted (no expansion when set)
2. start_mcp_gateway.cjs uses spawn() without a shell
3. Docker misparses the literal -c '%g' as --cpu-shares
Fix: Pre-compute DOCKER_SOCK_GID in a shell line, reference it as
${DOCKER_SOCK_GID} in the docker command. Generalize
buildDockerCommandWithExpandableVars to handle any ${VAR} pattern
(not just GITHUB_WORKSPACE).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes MCP gateway Docker command construction by precomputing the Docker socket group ID (GID) in the shell script and expanding it into MCP_GATEWAY_DOCKER_COMMAND before start_mcp_gateway.cjs reads it (since Node spawn() is used without a shell). It also generalizes the Docker command quoting helper to expand arbitrary ${VAR} patterns and updates tests/golden outputs accordingly.
Changes:
- Precompute
DOCKER_SOCK_GIDin generated shell and use it via${DOCKER_SOCK_GID}in the docker--group-addflag. - Generalize
buildDockerCommandWithExpandableVars()to expand any${VAR}pattern (viafindExpandableVars()), and extend unit tests. - Regenerate workflow lock files and golden fixtures to reflect the new exported command string.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/mcp_setup_generator.go | Switches docker command to use ${DOCKER_SOCK_GID} and emits a preceding DOCKER_SOCK_GID=... computation line in the generated script. |
| pkg/workflow/shell.go | Generalizes variable expansion in docker command quoting via findExpandableVars(). |
| pkg/workflow/shell_test.go | Updates/extends unit tests for multiple ${VAR} expansions, including ${DOCKER_SOCK_GID}. |
| pkg/workflow/mcp_setup_generator_test.go | Updates assertions to expect the new ${DOCKER_SOCK_GID} usage and verifies compute-before-use ordering. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden | Golden update reflecting DOCKER_SOCK_GID computation + new exported docker command quoting. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden | Golden update reflecting DOCKER_SOCK_GID computation + new exported docker command quoting. |
| .github/workflows/workflow-skill-extractor.lock.yml | Regenerated lock output to include DOCKER_SOCK_GID computation and updated docker command quoting. |
| .github/workflows/workflow-normalizer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/workflow-health-manager.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/workflow-generator.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/weekly-safe-outputs-spec-review.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/weekly-issue-summary.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/weekly-blog-post-writer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/video-analyzer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/update-astro.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/unbloat-docs.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/ubuntu-image-analyzer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/tidy.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/test-workflow.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/test-quality-sentinel.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/test-project-url-default.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/test-dispatcher.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/terminal-stylist.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/technical-doc-writer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/super-linter.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/sub-issue-closer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/stale-repo-identifier.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/spec-librarian.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/spec-extractor.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/spec-enforcer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/smoke-workflow-call.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/smoke-workflow-call-with-inputs.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/sergo.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/security-review.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/security-compliance.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/scout.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/schema-feature-coverage.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/schema-consistency-checker.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/safe-output-health.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/repo-tree-map.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/repo-audit-analyzer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/release.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/refiner.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/refactoring-cadence.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/q.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/pr-triage-agent.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/pr-nitpick-reviewer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/poem-bot.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/plan.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/pdf-summary.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/org-health-report.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/metrics-collector.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/mergefest.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/layout-spec-maintainer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/jsweep.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/issue-triage-agent.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/issue-monster.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/issue-arborist.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/hourly-ci-cleaner.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/grumpy-reviewer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/gpclean.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/functional-pragmatist.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/firewall.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/firewall-escape.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/example-permissions-warning.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/draft-pr-cleanup.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/discussion-task-miner.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/dictation-prompt.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/dev.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/dev-hawk.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/dependabot-go-checker.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/dependabot-burner.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/delight.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/dead-code-remover.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/craft.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/copilot-token-optimizer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/copilot-pr-prompt-analysis.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/copilot-cli-deep-research.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/copilot-agent-analysis.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/contribution-check.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/constraint-solving-potd.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/codex-github-remote-mcp-test.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/code-simplifier.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/code-scanning-fixer.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/cli-consistency-checker.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/ci-doctor.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/ci-coach.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/changeset.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/breaking-change-checker.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/bot-detection.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/auto-triage-issues.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/artifacts-summary.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/architecture-guardian.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/archie.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/approach-validator.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/ai-moderator.lock.yml | Same regeneration (GID compute + quoting). |
| .github/workflows/ace-editor.lock.yml | Same regeneration (GID compute + quoting). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 198/198 changed files
- Comments generated: 1
| // findExpandableVars returns all unique ${VAR_NAME} patterns in the string. | ||
| func findExpandableVars(s string) []string { | ||
| var vars []string | ||
| seen := make(map[string]bool) | ||
| for { | ||
| start := strings.Index(s, "${") | ||
| if start < 0 { | ||
| break | ||
| } | ||
| end := strings.Index(s[start:], "}") | ||
| if end < 0 { | ||
| break | ||
| } | ||
| varRef := s[start : start+end+1] | ||
| if !seen[varRef] { | ||
| seen[varRef] = true | ||
| vars = append(vars, varRef) | ||
| } | ||
| s = s[start+end+1:] | ||
| } | ||
| return vars |
There was a problem hiding this comment.
findExpandableVars() currently treats any "${...}" substring as expandable, including GitHub Actions expressions that begin with "${{". For inputs containing "${{ ... }}", this will capture an invalid fragment up to the first "}", and buildDockerCommandWithExpandableVars() will then emit broken quoting. Consider restricting matches to valid shell variable names (e.g., ${[A-Za-z_][A-Za-z0-9_]*}) and/or explicitly skipping "${{" sequences.
There was a problem hiding this comment.
Good catch on the $\{\{ expression issue! The fix should also handle cases where GitHub Actions expressions appear within Docker --env flags, since those could contain secrets that shouldn't be expanded prematurely.
📰 BREAKING: Report filed by Smoke Copilot · ● 963.8K
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
|
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Smoke Test Results — run §24541850986 | Author: @lpcox | Assignees:
Overall:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed pkg/workflow/shell.go changes. The findExpandableVars function correctly scans for ${...} patterns but may incorrectly match GitHub Actions expressions (${{ }}). The buildDockerCommandWithExpandableVars function handles shell quoting well for the common case. Recommend adding validation to restrict matches to valid shell variable names.
📰 BREAKING: Report filed by Smoke Copilot · ● 963.8K
| } | ||
| s = s[start+end+1:] | ||
| } | ||
| return vars |
There was a problem hiding this comment.
The findExpandableVars function matches any \$\{...} pattern, including GitHub Actions expressions like $\{\{ secrets.TOKEN }}. This would match $\{\{ secrets.TOKEN } (stopping at the first }) producing a malformed variable reference. Consider filtering to valid shell variable names only (e.g., \$\{[A-Za-z_][A-Za-z0-9_]*}) and skipping $\{\{ prefixes.
| remaining = remaining[nextIdx+len(nextVar):] | ||
| } | ||
| result.WriteString("'") | ||
| return result.String() |
There was a problem hiding this comment.
The buildDockerCommandWithExpandableVars function builds shell-quoted docker commands but processes the command character by character in a loop. If expandableVars contains overlapping patterns or the input has nested braces, the loop may process the same text multiple times. Consider adding a test case for inputs like \$\{VAR1}\$\{VAR1} and \$\{A_\$\{B}} to ensure correct behavior.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
💥 Smoke Test 24541851021 — Claude engine results: Core (#1–12): ✅✅✅✅✅✅✅✅✅ ❌ ✅✅ PARTIAL — Test 10 (Agentic Workflows MCP status) ❌; Tests 16 & 19
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · ● 206K
|
|
||
| export GH_AW_ENGINE="copilot" | ||
| export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network host --group-add $(stat -c '\''%g'\'' /var/run/docker.sock) -v /var/run/docker.sock:/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e MCP_GATEWAY_PAYLOAD_DIR -e MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_GUARD_MIN_INTEGRITY -e GITHUB_MCP_GUARD_REPOS -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -v /tmp/gh-aw/mcp-payloads:/tmp/gh-aw/mcp-payloads:rw -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw ghcr.io/github/gh-aw-mcpg:v0.2.22' | ||
| DOCKER_SOCK_GID=$(stat -c '%g' /var/run/docker.sock 2>/dev/null || echo '0') |
There was a problem hiding this comment.
✅ Good approach: computing DOCKER_SOCK_GID as a separate step avoids the shell-escaping issues with $(stat ...) embedded inside a single-quoted string. This ensures the GID is correctly captured and passed to --group-add.
| export GH_AW_ENGINE="copilot" | ||
| export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network host --group-add $(stat -c '\''%g'\'' /var/run/docker.sock) -v /var/run/docker.sock:/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e MCP_GATEWAY_PAYLOAD_DIR -e MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_GUARD_MIN_INTEGRITY -e GITHUB_MCP_GUARD_REPOS -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -v /tmp/gh-aw/mcp-payloads:/tmp/gh-aw/mcp-payloads:rw -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw ghcr.io/github/gh-aw-mcpg:v0.2.22' | ||
| DOCKER_SOCK_GID=$(stat -c '%g' /var/run/docker.sock 2>/dev/null || echo '0') | ||
| export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network host --group-add '"${DOCKER_SOCK_GID}"' -v /var/run/docker.sock:/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e MCP_GATEWAY_PAYLOAD_DIR -e MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_GUARD_MIN_INTEGRITY -e GITHUB_MCP_GUARD_REPOS -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -v /tmp/gh-aw/mcp-payloads:/tmp/gh-aw/mcp-payloads:rw -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw ghcr.io/github/gh-aw-mcpg:v0.2.22' |
There was a problem hiding this comment.
Consider using \$\{DOCKER_SOCK_GID:-0} parameter expansion instead of the || echo '0' fallback for a slightly more idiomatic shell style. Either way, this change correctly fixes the shell expansion issue in the single-quoted docker command.
Problem
The
$(stat -c '%g' /var/run/docker.sock)command embedded inMCP_GATEWAY_DOCKER_COMMANDdoes not expand at runtime because:start_mcp_gateway.cjsuses Node.jsspawn()without a shell, so$(...)is never interpreted-c '%g'and misparses it as its own--cpu-sharesflag, causinginvalid argumenterrorsFix
DOCKER_SOCK_GID=$(stat -c '%g' /var/run/docker.sock 2>/dev/null || echo '0')before the export${DOCKER_SOCK_GID}in the docker command instead of the inline$(stat ...)subshellbuildDockerCommandWithExpandableVarsto handle any${VAR}pattern (not just${GITHUB_WORKSPACE}), using a newfindExpandableVars()helperChanges
pkg/workflow/mcp_setup_generator.go— Use${DOCKER_SOCK_GID}variable, add GID computation linepkg/workflow/shell.go— GeneralizedbuildDockerCommandWithExpandableVarswithfindExpandableVars()pkg/workflow/shell_test.go— Added test cases for${DOCKER_SOCK_GID}and multiple variable expansionpkg/workflow/mcp_setup_generator_test.go— Updated assertions for new variable pattern.lock.ymlfiles + 2 golden fixtures✨ PR Review Safe Output Test - Run 24541851021