Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

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

19 changes: 19 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,25 @@ When developing a new command:
### Go Code Style
- **ALWAYS use `any` instead of `interface{}`** - Use the modern `any` type alias (Go 1.18+) for consistency across the codebase

### Claude Engine Tool Enforcement Security Model

When adding code to `pkg/workflow/claude_engine.go` or `pkg/workflow/claude_tools.go`, be aware of how Claude's permission mode affects tool enforcement:

**Two permission modes are used at runtime:**

1. **`acceptEdits` (default)** — Claude Code honors `--allowed-tools` as the effective tool boundary. Workflow `tools:` and `mcp-servers: allowed:` restrictions are enforced client-side.
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

In acceptEdits mode, --allowed-tools is enforced client-side by Claude, but mcp-servers: allowed: is enforced gateway-side (server-side), not client-side. The current wording implies both tools: and mcp-servers: allowed: are client-side restrictions, which is inaccurate and could confuse contributors making security-sensitive changes.

Suggested change
1. **`acceptEdits` (default)** — Claude Code honors `--allowed-tools` as the effective tool boundary. Workflow `tools:` and `mcp-servers: allowed:` restrictions are enforced client-side.
1. **`acceptEdits` (default)** — Claude Code honors `--allowed-tools` as the effective client-side tool boundary. Workflow `tools:` restrictions are reflected in that `--allowed-tools` setting, while `mcp-servers: allowed:` restrictions are enforced gateway-side by the MCP server configuration.

Copilot uses AI. Check for mistakes.

2. **`bypassPermissions`** — Claude Code silently ignores `--allowed-tools`. Every tool exposed by the MCP gateway is reachable regardless of the workflow's declared tool configuration. This mode is only used when the workflow grants unrestricted bash access (e.g., `bash: "*"`, `bash: [":*"]`, or `bash: null`).
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

This list of unrestricted bash examples should include bash: true (documented elsewhere as equivalent to allowing all bash commands). As written, it suggests bash: true would not trigger bypassPermissions, which conflicts with the documented bash formats and the runtime behavior described in hasBashWildcardInTools.

Suggested change
2. **`bypassPermissions`** — Claude Code silently ignores `--allowed-tools`. Every tool exposed by the MCP gateway is reachable regardless of the workflow's declared tool configuration. This mode is only used when the workflow grants unrestricted bash access (e.g., `bash: "*"`, `bash: [":*"]`, or `bash: null`).
2. **`bypassPermissions`** — Claude Code silently ignores `--allowed-tools`. Every tool exposed by the MCP gateway is reachable regardless of the workflow's declared tool configuration. This mode is only used when the workflow grants unrestricted bash access (e.g., `bash: true`, `bash: "*"`, `bash: [":*"]`, or `bash: null`).

Copilot uses AI. Check for mistakes.

**Security boundary in `bypassPermissions` mode:**
When `bypassPermissions` is active, the **MCP gateway `allowed:` filter is the sole effective tool boundary**. The compiled `tools` field in the gateway configuration (`/tmp/gh-aw/mcp-config/mcp-servers.json`) controls which tools each MCP server exposes. Gateway-side enforcement applies regardless of what the agent requests.

**Implication for code changes:**
- `hasBashWildcardInTools()` in `claude_tools.go` determines which mode is selected — changes here affect the security boundary
- `computeAllowedClaudeToolsString()` builds the `--allowed-tools` string — only effective in `acceptEdits` mode
- `convert_gateway_config_claude.sh` preserves the `tools` field from gateway output — this is what enforces restrictions in `bypassPermissions` mode
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The converter referenced here appears to be outdated/misnamed: the workflow gateway conversion for Claude is implemented in actions/setup/js/convert_gateway_config_claude.cjs (and invoked by actions/setup/sh/start_mcp_gateway.sh), not convert_gateway_config_claude.sh. Consider updating this bullet to point at the .cjs converter (or explicitly mention both, if both are still supported), so contributors look at the code path that actually runs in CI.

Suggested change
- `convert_gateway_config_claude.sh` preserves the `tools` field from gateway output — this is what enforces restrictions in `bypassPermissions` mode
- `actions/setup/js/convert_gateway_config_claude.cjs` (invoked by `actions/setup/sh/start_mcp_gateway.sh`) preserves the `tools` field from gateway output — this is what enforces restrictions in `bypassPermissions` mode

Copilot uses AI. Check for mistakes.
- Do not remove or weaken either enforcement layer; they complement each other for different access scenarios

### Channel Lifecycle Guidelines

Go channels require explicit lifecycle management to prevent goroutine leaks and resource exhaustion. Follow these guidelines when working with channels:
Expand Down
5 changes: 5 additions & 0 deletions docs/src/content/docs/guides/mcps.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ mcp-servers:
allowed: ["search_pages", "get_page"] # or ["*"] to allow all
```

The `allowed:` filter is enforced at the **MCP gateway level** — the gateway only exposes the listed tools to the agent. This enforcement applies regardless of which AI engine or permission mode is in use.

> [!IMPORTANT]
> For Claude workflows that grant unrestricted bash access (`bash: "*"` or `bash: [":*"]`), Claude runs in `bypassPermissions` mode and its `--allowed-tools` flag is silently ignored. In that case, the `allowed:` gateway filter is the **sole effective tool boundary**. Always specify `allowed:` on each `mcp-servers:` entry when tool restrictions matter. See [Claude Tool Enforcement Security Model](/gh-aw/reference/engines/#claude-tool-enforcement-security-model) for details.
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

This IMPORTANT callout enumerates unrestricted bash as only bash: "*" or bash: [":*"], but the docs also describe bash: true (and bash: null) as “allow all commands”. To avoid misleading workflow authors, please expand this list to include the other equivalent unrestricted forms (at least bash: true and bash: null), matching the engines reference section.

Suggested change
> For Claude workflows that grant unrestricted bash access (`bash: "*"` or `bash: [":*"]`), Claude runs in `bypassPermissions` mode and its `--allowed-tools` flag is silently ignored. In that case, the `allowed:` gateway filter is the **sole effective tool boundary**. Always specify `allowed:` on each `mcp-servers:` entry when tool restrictions matter. See [Claude Tool Enforcement Security Model](/gh-aw/reference/engines/#claude-tool-enforcement-security-model) for details.
> For Claude workflows that grant unrestricted bash access (`bash: "*"` or `bash: [":*"]`, as well as equivalent allow-all forms like `bash: true` and `bash: null`), Claude runs in `bypassPermissions` mode and its `--allowed-tools` flag is silently ignored. In that case, the `allowed:` gateway filter is the **sole effective tool boundary**. Always specify `allowed:` on each `mcp-servers:` entry when tool restrictions matter. See [Claude Tool Enforcement Security Model](/gh-aw/reference/engines/#claude-tool-enforcement-security-model) for details.

Copilot uses AI. Check for mistakes.

## Shared MCP Configurations

Pre-configured MCP server specifications are available in [`.github/workflows/shared/mcp/`](https://github.com/github/gh-aw/tree/main/.github/workflows/shared/mcp) and can be copied or imported directly. Examples include:
Expand Down
35 changes: 35 additions & 0 deletions docs/src/content/docs/reference/engines.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,41 @@ timeout-minutes: 60
| `max-turns` | ❌ | ✅ | ❌ | ❌ | ❌ | ❌ | Iteration budget (Claude only) |
| `max-continuations` | ✅ | ❌ | ❌ | ❌ | ❌ | ❌ | Autopilot run budget (Copilot only) |

## Claude Tool Enforcement Security Model

Claude Code uses one of two permission modes at runtime, and which mode is selected determines whether the declared `tools:` allowlist is enforced:

### `acceptEdits` mode (default)

By default, gh-aw starts Claude Code with `--permission-mode acceptEdits`. In this mode, Claude honors the `--allowed-tools` flag. The workflow's declared `tools:` and `mcp-servers: allowed:` configuration is compiled into an explicit allowlist and passed to the Claude CLI. Only the tools listed there are accessible to the agent.

### `bypassPermissions` mode (unrestricted bash)

When the workflow grants unrestricted bash access — `bash: "*"`, `bash: [":*"]`, or `bash: null` — gh-aw switches to `--permission-mode bypassPermissions`. **In this mode, Claude Code silently ignores `--allowed-tools`.** Every tool exposed by the MCP gateway is reachable regardless of the workflow's declared tool configuration.

Comment on lines +350 to +353
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

This list of “unrestricted bash” configurations is incomplete relative to the documented bash: formats elsewhere in the docs (e.g., bash: true is described as equivalent to ['*'] in docs/src/content/docs/reference/tools.md and frontmatter-full.md). Since bash: true also grants unrestricted bash, it should be included here (and in the summary table) as another case that triggers bypassPermissions.

Copilot uses AI. Check for mistakes.
> [!WARNING]
> Do not rely on `tools:` or `mcp-servers: allowed:` for security guarantees when unrestricted bash is granted. In `bypassPermissions` mode, the agent can already run arbitrary shell commands, so `--allowed-tools` provides no meaningful additional boundary.
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The WARNING contradicts the rest of this new section: it says not to rely on mcp-servers: allowed: for security guarantees, but the following “Gateway-side enforcement” subsection states that the gateway allowed: filter is the sole effective tool boundary in bypassPermissions mode. Suggest rewriting the warning to specifically warn against relying on Claude-side tools:/--allowed-tools enforcement when unrestricted bash is granted, while clarifying that gateway allowed: remains the enforced boundary for MCP tools.

Suggested change
> Do not rely on `tools:` or `mcp-servers: allowed:` for security guarantees when unrestricted bash is granted. In `bypassPermissions` mode, the agent can already run arbitrary shell commands, so `--allowed-tools` provides no meaningful additional boundary.
> Do not rely on Claude-side `tools:` / `--allowed-tools` enforcement for security guarantees when unrestricted bash is granted. In `bypassPermissions` mode, the agent can already run arbitrary shell commands, so the Claude CLI allowlist provides no meaningful additional boundary. However, `mcp-servers: allowed:` remains enforced by the MCP gateway for MCP tools and is still the effective boundary there.

Copilot uses AI. Check for mistakes.

### Gateway-side enforcement

The **MCP gateway's `allowed:` filter is the sole effective tool boundary in `bypassPermissions` mode** (and a second layer of enforcement in `acceptEdits` mode). gh-aw compiles the `allowed:` list from each `mcp-servers:` entry into the gateway configuration before the agent starts. The gateway enforces this list server-side, regardless of what the agent requests.

```yaml wrap
mcp-servers:
notion:
container: "mcp/notion"
allowed: ["search_pages", "get_page"] # enforced at gateway level
```

### Summary

| Workflow config | Permission mode | `--allowed-tools` enforced? | Gateway `allowed:` enforced? |
|---|---|:---:|:---:|
| No unrestricted bash | `acceptEdits` | ✅ Yes | ✅ Yes |
| `bash: "*"` / `bash: [":*"]` / `bash: null` | `bypassPermissions` | ❌ No | ✅ Yes |

For workflows that must restrict which MCP tools are accessible, always specify `allowed:` on each `mcp-servers:` entry. This applies regardless of whether unrestricted bash is used.

## Related Documentation

- [Frontmatter](/gh-aw/reference/frontmatter/) - Complete configuration reference
Expand Down
Loading