Skip to content
Merged
4 changes: 0 additions & 4 deletions .github/workflows/contribution-check.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion .github/workflows/contribution-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,4 +252,8 @@ If any subagent call failed (❓), also apply `outdated`.
- Close the previous report issue when creating a new one (`close-older-issues: true`).
- Be constructive in assessments — these reports help maintainers prioritize, not gatekeep.

{{#import shared/noop-reminder.md}}
**Important**: If no action is needed after completing your analysis, you **MUST** call the `noop` safe-output tool with a brief explanation. Failing to call any safe-output tool is the most common cause of safe-output workflow failures.

```json
{"noop": {"message": "No action needed: [brief explanation of what was analyzed and why]"}}
```
4 changes: 3 additions & 1 deletion .github/workflows/daily-fact.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion .github/workflows/smoke-claude.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions .github/workflows/smoke-crush.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion .github/workflows/smoke-crush.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,8 @@ Add a **very brief** comment (max 5-10 lines) to the current pull request with:

If all tests pass, use the `add_labels` safe-output tool to add the label `smoke-crush` to the pull request.

{{#import shared/noop-reminder.md}}
**Important**: If no action is needed after completing your analysis, you **MUST** call the `noop` safe-output tool with a brief explanation. Failing to call any safe-output tool is the most common cause of safe-output workflow failures.

```json
{"noop": {"message": "No action needed: [brief explanation of what was analyzed and why]"}}
```
2 changes: 0 additions & 2 deletions .github/workflows/workflow-skill-extractor.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion .github/workflows/workflow-skill-extractor.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,8 @@ Use this priority rubric:
- Keep recommendations concrete and actionable.
- If no action is needed, call `noop` with a brief explanation.

{{#import shared/noop-reminder.md}}
**Important**: If no action is needed after completing your analysis, you **MUST** call the `noop` safe-output tool with a brief explanation. Failing to call any safe-output tool is the most common cause of safe-output workflow failures.

```json
{"noop": {"message": "No action needed: [brief explanation of what was analyzed and why]"}}
```
11 changes: 9 additions & 2 deletions pkg/workflow/mcp_setup_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,9 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,

var containerCmd strings.Builder
containerCmd.WriteString("docker run -i --rm --network host")
// Use runner UID/GID so gateway-created /tmp logs remain readable by downstream
// redaction/upload steps; keep a supplementary docker.sock group for daemon access.
containerCmd.WriteString(" --user ${MCP_GATEWAY_UID}:${MCP_GATEWAY_GID}")
containerCmd.WriteString(" --group-add ${DOCKER_SOCK_GID}")
containerCmd.WriteString(" -v /var/run/docker.sock:/var/run/docker.sock") // Enable docker-in-docker for MCP gateway
// Pass required gateway environment variables
Expand Down Expand Up @@ -902,11 +905,15 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
}
}

// Compute the Docker socket group ID so the MCPG container can access /var/run/docker.sock
// Compute the runner user/group IDs and Docker socket group ID before constructing
// the docker command so these values are expanded to literals in the exported command.
yaml.WriteString(" MCP_GATEWAY_UID=$(id -u 2>/dev/null || echo '0')\n")
yaml.WriteString(" MCP_GATEWAY_GID=$(id -g 2>/dev/null || echo '0')\n")
yaml.WriteString(" DOCKER_SOCK_GID=$(stat -c '%g' /var/run/docker.sock 2>/dev/null || echo '0')\n")

// Build the export command with proper quoting that allows variable expansion
// We need to break out of quotes for shell variables like ${GITHUB_WORKSPACE} and ${DOCKER_SOCK_GID}
// We need to break out of quotes for shell variables like
// ${GITHUB_WORKSPACE}, ${MCP_GATEWAY_UID}, ${MCP_GATEWAY_GID}, and ${DOCKER_SOCK_GID}
cmdWithExpandableVars := buildDockerCommandWithExpandableVars(containerCmd.String())
yaml.WriteString(" export MCP_GATEWAY_DOCKER_COMMAND=" + cmdWithExpandableVars + "\n")
yaml.WriteString(" \n")
Expand Down
27 changes: 21 additions & 6 deletions pkg/workflow/mcp_setup_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,9 +477,9 @@ Test that TAVILY_API_KEY is passed to gateway container.
"Docker command should include -e TAVILY_API_KEY before the container image")
}

// TestMCPGatewayDockerCommandIncludesDockerSocketGroup verifies the gateway docker command
// adds the docker socket group as a supplementary group for non-root execution.
func TestMCPGatewayDockerCommandIncludesDockerSocketGroup(t *testing.T) {
// TestMCPGatewayDockerCommandUsesRunnerIdentityAndSocketGroup verifies the gateway docker command
// computes and uses runner UID/GID and docker socket group values in the generated command.
func TestMCPGatewayDockerCommandUsesRunnerIdentityAndSocketGroup(t *testing.T) {
frontmatter := `---
on: workflow_dispatch
engine: copilot
Expand Down Expand Up @@ -508,16 +508,31 @@ tools:
require.NoError(t, err, "Failed to read output file")
yamlStr := string(content)

userSnippet := `--user '"${MCP_GATEWAY_UID}"':'"${MCP_GATEWAY_GID}"'`
groupAddSnippet := `--group-add '"${DOCKER_SOCK_GID}"'`
mountSnippet := `-v /var/run/docker.sock:/var/run/docker.sock`
gidComputeSnippet := `DOCKER_SOCK_GID=$(stat -c '%g' /var/run/docker.sock 2>/dev/null || echo '0')`
require.Contains(t, yamlStr, gidComputeSnippet,
uidComputeSnippet := `MCP_GATEWAY_UID=$(id -u 2>/dev/null || echo '0')`
runnerGIDComputeSnippet := `MCP_GATEWAY_GID=$(id -g 2>/dev/null || echo '0')`
socketGIDComputeSnippet := `DOCKER_SOCK_GID=$(stat -c '%g' /var/run/docker.sock 2>/dev/null || echo '0')`
require.Contains(t, yamlStr, uidComputeSnippet,
"Shell should compute MCP_GATEWAY_UID before docker command")
require.Contains(t, yamlStr, runnerGIDComputeSnippet,
"Shell should compute MCP_GATEWAY_GID before docker command")
require.Contains(t, yamlStr, userSnippet,
"Docker command should include runner UID/GID user mapping")
require.Contains(t, yamlStr, socketGIDComputeSnippet,
"Shell should compute DOCKER_SOCK_GID before docker command")
require.Contains(t, yamlStr, groupAddSnippet,
"Docker command should include docker socket supplementary group mapping")
require.Contains(t, yamlStr, mountSnippet,
"Docker command should mount the Docker socket")
require.Less(t, strings.Index(yamlStr, gidComputeSnippet), strings.Index(yamlStr, groupAddSnippet),
require.Less(t, strings.Index(yamlStr, uidComputeSnippet), strings.Index(yamlStr, userSnippet),
"MCP_GATEWAY_UID should be computed before it is used in the docker command")
require.Less(t, strings.Index(yamlStr, runnerGIDComputeSnippet), strings.Index(yamlStr, userSnippet),
"MCP_GATEWAY_GID should be computed before it is used in the docker command")
require.Less(t, strings.Index(yamlStr, userSnippet), strings.Index(yamlStr, groupAddSnippet),
"Docker command should include user mapping before supplementary group mapping")
require.Less(t, strings.Index(yamlStr, socketGIDComputeSnippet), strings.Index(yamlStr, groupAddSnippet),
"DOCKER_SOCK_GID should be computed before it is used in the docker command")
require.Less(t, strings.Index(yamlStr, groupAddSnippet), strings.Index(yamlStr, mountSnippet),
"Docker command should add supplementary group before mounting the Docker socket")
Expand Down
Loading