Skip to content

Commit 2ef8727

Browse files
refactor: complete Engine abstraction — move install, invocation, and paths behind Engine enum (#301)
* refactor: remove dead $HOME/.copilot mcp-config copy from templates The AWF invocation explicitly passes --additional-mcp-config @/tmp/awf-tools/mcp-config.json, so the copy to $HOME/.copilot/ mcp-config.json was never read by copilot inside the AWF container. Removes from both base.yml and 1es-base.yml: - mkdir -p "$HOME/.copilot" - cp/chmod of mcp-config.json to $HOME/.copilot/ Part of #287 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: move engine log paths behind Engine::log_dir() Add Engine::log_dir() method returning the engine-specific log directory (~/.copilot/logs for Copilot). Replace 6 hardcoded ~/.copilot/logs paths in both templates with {{ engine_log_dir }} marker, substituted at compile time. Part of #287 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: move engine install steps behind Engine::install_steps() Add Engine::install_steps(engine_config) that generates the full NuGet install YAML for the Copilot CLI. Replaces 4 hardcoded install blocks (Agent + Detection jobs in both templates) with a single {{ engine_install_steps }} marker. - Move COPILOT_CLI_VERSION constant from common.rs to engine.rs - Wire engine.version front matter field: uses custom version if set, falls back to COPILOT_CLI_VERSION - Return empty string when engine.command is set (skip NuGet install) - Remove {{ copilot_version }} marker (absorbed into install_steps) - Remove engine.version "not yet wired" warning Part of #287 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: move engine invocation behind Engine::invocation() Add Engine::invocation() that generates the complete AWF command string including binary path, prompt delivery flag, and MCP config flag. The engine now controls how prompts are provided (e.g. --prompt "$(cat ...)") and how MCP config is referenced. Template changes: - Replace hardcoded AWF commands with {{ engine_run }} (Agent job) and {{ engine_run_detection }} (Detection job) - Remove {{ copilot_params }} marker (absorbed into invocation) Code changes: - Add copilot_invocation() helper in engine.rs - Replace copilot_params variable with engine_run/engine_run_detection - Rename 20 test functions: test_copilot_params_* → test_engine_args_* - Update compiler_tests.rs marker assertion Part of #287 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: update AGENTS.md, test docs, and version updater for Engine refactor - AGENTS.md: document new markers (engine_install_steps, engine_run, engine_run_detection, engine_env, engine_log_dir), mark copilot_version as removed, remove duplicate engine_env section - Version updater: point COPILOT_CLI_VERSION at src/engine.rs - Test docs: rename copilot_params references to engine_args/engine_run - compiler_tests.rs: update assertion messages Part of #287 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * remove max-turns config... specific to claude code * feat: wire engine.command, agent, api-target, args, and env into pipeline Wire the five remaining engine front matter fields that were previously parsed but emitted 'not yet wired' warnings: - engine.command: overrides the engine binary path in the AWF invocation; skips NuGet install (already handled). Validated for shell-safe chars. - engine.agent: adds --agent <name> to the Copilot CLI invocation for custom agent file selection from .github/agents/. - engine.api-target: adds --api-target <hostname> for GHES/GHEC endpoints and adds the hostname to the AWF network allowlist. - engine.args: appends user-provided CLI arguments after compiler-generated args. Subject to strict char validation and blocked from overriding compiler-controlled flags (--prompt, --allow-tool, --disable-builtin-mcps, --no-ask-user, --ask-user, --additional-mcp-config, --allow-all-tools, --allow-all-paths). - engine.env: merges user environment variables into the sandbox step env block. Validates key names, blocks compiler-controlled keys (GITHUB_TOKEN, PATH, BASH_ENV, LD_PRELOAD, etc.), and rejects ADO expression/command injection in values. All five warnings are removed. AGENTS.md updated to reflect wired status. 33 new tests added covering happy paths, validation, and security edge cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: prevent panic on empty env key and validate engine.version - Fix empty env key panic: key.chars().next().unwrap() was called before the emptiness check, causing a panic on empty YAML keys ("": value). Now uses let-else to bail with a clear error. - Validate engine.version: the version string was embedded unvalidated into NuGet command arguments, allowing injection (e.g., spaces, quotes, additional -Source flags). Now validated with is_valid_version allowlist [A-Za-z0-9._-] before use. install_steps() returns Result to propagate the validation error. - 5 new tests for both fixes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: handle engine.version "latest" and quote engine_log_dir in templates - engine.version: "latest" now omits the -Version flag entirely so NuGet installs the newest available version, instead of passing "-Version latest" which NuGet rejects at runtime. - Quote {{ engine_log_dir }} in base.yml and 1es-base.yml shell scripts to future-proof against paths with spaces. - Add explanatory comment on the intentionally case-insensitive BLOCKED_ENV_KEYS comparison (prevents accidental shadowing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b3ecca7 commit 2ef8727

15 files changed

Lines changed: 845 additions & 307 deletions

File tree

.github/workflows/update-awf-version.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ There are four items to check:
2929
| Item | Upstream Source | Local Path |
3030
|------|---------------|------------|
3131
| `AWF_VERSION` | [github/gh-aw-firewall](https://github.com/github/gh-aw-firewall) latest release | `src/compile/common.rs` |
32-
| `COPILOT_CLI_VERSION` | [github/copilot-cli](https://github.com/github/copilot-cli) latest release | `src/compile/common.rs` |
32+
| `COPILOT_CLI_VERSION` | [github/copilot-cli](https://github.com/github/copilot-cli) latest release | `src/engine.rs` |
3333
| `MCPG_VERSION` | [github/gh-aw-mcpg](https://github.com/github/gh-aw-mcpg) latest release | `src/compile/common.rs` |
3434
| `ecosystem_domains.json` | [github/gh-aw](https://github.com/github/gh-aw) `pkg/workflow/data/ecosystem_domains.json` on `main` | `src/data/ecosystem_domains.json` |
3535

@@ -45,7 +45,7 @@ Fetch the latest release of the upstream repository. Record the tag name, stripp
4545

4646
### Step 2: Read the Current Version
4747

48-
Read the file `src/compile/common.rs` in this repository and find the corresponding constant:
48+
Read the file `src/compile/common.rs` (for `AWF_VERSION`, `MCPG_VERSION`) or `src/engine.rs` (for `COPILOT_CLI_VERSION`) in this repository and find the corresponding constant:
4949

5050
- `pub const AWF_VERSION: &str = "...";`
5151
- `pub const COPILOT_CLI_VERSION: &str = "...";`
@@ -89,7 +89,7 @@ If the latest version is newer than the current constant:
8989
```markdown
9090
## Dependency Update
9191

92-
Updates the pinned `COPILOT_CLI_VERSION` constant in `src/compile/common.rs` from `<old-version>` to `<latest-version>`.
92+
Updates the pinned `COPILOT_CLI_VERSION` constant in `src/engine.rs` from `<old-version>` to `<latest-version>`.
9393

9494
### Release
9595

.vscode/mcp.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"servers": {
3+
"github-agentic-workflows": {
4+
"command": "gh",
5+
"args": [
6+
"aw",
7+
"mcp-server"
8+
]
9+
}
10+
}
11+
}

.vscode/settings.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"github.copilot.enable": {
3+
"markdown": true
4+
}
5+
}

AGENTS.md

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -346,16 +346,13 @@ engine:
346346
| `id` | string | `copilot` | Engine identifier. Currently only `copilot` (GitHub Copilot CLI) is supported. |
347347
| `model` | string | `claude-opus-4.5` | AI model to use. Options include `claude-sonnet-4.5`, `gpt-5.2-codex`, `gemini-3-pro-preview`, etc. |
348348
| `timeout-minutes` | integer | *(none)* | Maximum time in minutes the agent job is allowed to run. Sets `timeoutInMinutes` on the `Agent` job in the generated pipeline. |
349-
| `version` | string | *(none)* | Engine CLI version to install (e.g., `"0.0.422"`, `"latest"`). **Not yet wired** — parsed but ignored with a warning. |
350-
| `agent` | string | *(none)* | Custom agent file identifier (Copilot only). **Not yet wired** — parsed but ignored with a warning. |
351-
| `api-target` | string | *(none)* | Custom API endpoint hostname for GHES/GHEC (e.g., `"api.acme.ghe.com"`). **Not yet wired** — parsed but ignored with a warning. |
352-
| `args` | list | `[]` | Custom CLI arguments injected before the prompt. **Not yet wired** — parsed but ignored with a warning. |
353-
| `env` | map | *(none)* | Engine-specific environment variables. **Not yet wired** — parsed but ignored with a warning. |
354-
| `command` | string | *(none)* | Custom engine executable path (skips default installation). **Not yet wired** — parsed but ignored with a warning. |
349+
| `version` | string | *(none)* | Engine CLI version to install (e.g., `"0.0.422"`, `"latest"`). Overrides the pinned `COPILOT_CLI_VERSION`. Set to `"latest"` to use the newest available version. |
350+
| `agent` | string | *(none)* | Custom agent file identifier (Copilot only). Adds `--agent <name>` to the CLI invocation, selecting a custom agent from `.github/agents/`. |
351+
| `api-target` | string | *(none)* | Custom API endpoint hostname for GHES/GHEC (e.g., `"api.acme.ghe.com"`). Adds `--api-target <hostname>` to the CLI invocation and adds the hostname to the AWF network allowlist. |
352+
| `args` | list | `[]` | Custom CLI arguments appended after compiler-generated args. Subject to shell-safety validation and blocked from overriding compiler-controlled flags (`--prompt`, `--allow-tool`, `--disable-builtin-mcps`, etc.). |
353+
| `env` | map | *(none)* | Engine-specific environment variables merged into the sandbox step's `env:` block. Keys must be valid env var names; values must not contain ADO expressions (`$(`, `${{`) or pipeline command injection (`##vso[`). Compiler-controlled keys (`GITHUB_TOKEN`, `PATH`, `BASH_ENV`, etc.) are blocked. |
354+
| `command` | string | *(none)* | Custom engine executable path (skips default NuGet installation). The path must be accessible inside the AWF container (e.g., `/tmp/...` or workspace-mounted paths). |
355355

356-
> **Deprecated:** `max-turns` is still accepted in front matter for backwards compatibility but is ignored at compile time (a warning is emitted). It was specific to Claude Code and is not supported by Copilot CLI.
357-
358-
> **Note:** Fields marked "not yet wired" are accepted in the schema for forward compatibility with gh-aw but produce a compile-time warning. Pipeline wiring for these fields is incremental.
359356

360357
#### `timeout-minutes`
361358

@@ -562,7 +559,7 @@ The compiler transforms the input into valid Azure DevOps pipeline YAML based on
562559
- **Standalone**: Uses `src/data/base.yml`
563560
- **1ES**: Uses `src/data/1es-base.yml`
564561

565-
Explicit markings are embedded in these templates that the compiler is allowed to replace e.g. `{{ copilot_params }}` denotes parameters which are passed to the copilot command line tool. The compiler should not replace sections denoted by `${{ some content }}`. What follows is a mapping of markings to responsibilities (primarily for the standalone template).
562+
Explicit markings are embedded in these templates that the compiler is allowed to replace e.g. `{{ engine_run }}` denotes the full engine invocation command. The compiler should not replace sections denoted by `${{ some content }}`. What follows is a mapping of markings to responsibilities (primarily for the standalone template).
566563

567564
## {{ parameters }}
568565

@@ -650,18 +647,57 @@ This distinction allows resources (like templates) to be available as pipeline r
650647

651648
Should be replaced with the human-readable name from the front matter (e.g., "Daily Code Review"). This is used for display purposes like stage names.
652649

653-
## {{ copilot_params }}
650+
## {{ engine_install_steps }}
651+
652+
Should be replaced with engine-specific pipeline steps to install the engine binary. Generated by `Engine::install_steps()`. For Copilot, this produces:
653+
- `NuGetAuthenticate@1` task
654+
- `NuGetCommand@2` task to install `Microsoft.Copilot.CLI.linux-x64` (uses `engine.version` if set, otherwise `COPILOT_CLI_VERSION` constant)
655+
- Bash step to copy binary to `/tmp/awf-tools/copilot`
656+
- Bash step to verify installation
657+
658+
Returns empty when `engine.command` is set (user provides own binary).
654659

655-
Additional params provided to copilot CLI. The compiler generates:
660+
## {{ engine_run }}
661+
662+
Should be replaced with the full AWF `--` command string for the Agent job. Generated by `Engine::invocation()`. For Copilot, this produces:
663+
```
664+
<command_path> --prompt "$(cat /tmp/awf-tools/agent-prompt.md)" --additional-mcp-config @/tmp/awf-tools/mcp-config.json <engine args> <user args>
665+
```
666+
667+
The binary path defaults to `/tmp/awf-tools/copilot` but can be overridden via `engine.command`. The engine controls how the prompt is delivered (`--prompt "$(cat ...)"`), and how MCP config is referenced (`--additional-mcp-config @...`).
668+
669+
Engine args include:
656670
- `--model <model>` - AI model from `engine` front matter field (default: claude-opus-4.5)
671+
- `--agent <name>` - Custom agent file from `engine.agent` (selects from `.github/agents/`)
672+
- `--api-target <hostname>` - Custom API endpoint from `engine.api-target` (GHES/GHEC)
657673
- `--no-ask-user` - Prevents interactive prompts
658674
- `--disable-builtin-mcps` - Disables all built-in Copilot CLI MCPs (single flag, no argument)
659675
- `--allow-all-tools` - When bash is omitted (default) or has a wildcard (`":*"` or `"*"`), allows all tools instead of individual `--allow-tool` flags
660676
- `--allow-tool <tool>` - When bash is NOT wildcard, explicitly allows configured tools (github, safeoutputs, write, and shell commands from the `bash:` field plus any runtime-required commands)
661677
- `--allow-all-paths` - When `edit` tool is enabled (default), allows the agent to write to any file path
678+
- Custom args from `engine.args` — appended after compiler-generated args (subject to shell-safety validation and blocked flag checks)
662679

663680
MCP servers are handled entirely by the MCP Gateway (MCPG) and are not passed as copilot CLI params.
664681

682+
## {{ engine_run_detection }}
683+
684+
Same as `{{ engine_run }}` but for the Detection (threat analysis) job. Uses a different prompt path (`/tmp/awf-tools/threat-analysis-prompt.md`) and no MCP config.
685+
686+
## {{ engine_env }}
687+
688+
Generates engine-specific environment variable entries for the AWF sandbox step via `Engine::env()`. For the Copilot engine, this produces:
689+
690+
- `GITHUB_TOKEN: $(GITHUB_TOKEN)` — GitHub authentication
691+
- `GITHUB_READ_ONLY: 1` — Restricts GitHub API to read-only access
692+
- `COPILOT_OTEL_ENABLED`, `COPILOT_OTEL_EXPORTER_TYPE`, `COPILOT_OTEL_FILE_EXPORTER_PATH` — OpenTelemetry file-based tracing for agent statistics
693+
- Custom env vars from `engine.env` — merged after compiler-controlled vars (YAML-quoted, validated for safety)
694+
695+
ADO access tokens (`AZURE_DEVOPS_EXT_PAT`, `SYSTEM_ACCESSTOKEN`) are not part of this marker — they are injected separately by `{{ acquire_ado_token }}` and extension pipeline variable mappings when `permissions.read` is configured.
696+
697+
## {{ engine_log_dir }}
698+
699+
Should be replaced with the engine's log directory path, generated by `Engine::log_dir()`. For Copilot: `~/.copilot/logs`. Used by log collection steps to copy engine logs to pipeline artifacts.
700+
665701
## {{ pool }}
666702

667703
Should be replaced with the agent pool name from the `pool` front matter field. Defaults to `AZS-1ES-L-MMS-ubuntu-22.04` if not specified.
@@ -845,8 +881,9 @@ This ensures the Copilot CLI config reflects MCPG's actual runtime state rather
845881
Should be replaced with the comma-separated domain list for AWF's `--allow-domains` flag. The list includes:
846882
1. Core Azure DevOps/GitHub endpoints (from `allowed_hosts.rs`)
847883
2. MCP-specific endpoints for each enabled MCP
848-
3. Ecosystem identifier expansions from `network.allowed:` (e.g., `python` → PyPI/pip domains)
849-
4. User-specified additional hosts from `network.allowed:` front matter
884+
3. Engine-required hosts (e.g., `engine.api-target` hostname for GHES/GHEC)
885+
4. Ecosystem identifier expansions from `network.allowed:` (e.g., `python` → PyPI/pip domains)
886+
5. User-specified additional hosts from `network.allowed:` front matter
850887

851888
The output is formatted as a comma-separated string (e.g., `github.com,*.dev.azure.com,api.github.com`).
852889

@@ -884,16 +921,6 @@ The step:
884921

885922
If `permissions.read` is not configured, this marker is replaced with an empty string.
886923

887-
## {{ engine_env }}
888-
889-
Generates engine-specific environment variable entries for the AWF sandbox step via `Engine::env()`. For the Copilot engine, this produces:
890-
891-
- `GITHUB_TOKEN: $(GITHUB_TOKEN)` — GitHub authentication
892-
- `GITHUB_READ_ONLY: 1` — Restricts GitHub API to read-only access
893-
- `COPILOT_OTEL_ENABLED`, `COPILOT_OTEL_EXPORTER_TYPE`, `COPILOT_OTEL_FILE_EXPORTER_PATH` — OpenTelemetry file-based tracing for agent statistics
894-
895-
ADO access tokens (`AZURE_DEVOPS_EXT_PAT`, `SYSTEM_ACCESSTOKEN`) are not part of this marker — they are injected separately by `{{ acquire_ado_token }}` and extension pipeline variable mappings when `permissions.read` is configured.
896-
897924
## {{ acquire_write_token }}
898925

899926
Generates an `AzureCLI@2` step that acquires a write-capable ADO-scoped access token from the ARM service connection specified in `permissions.write`. This token is used only by the executor in Stage 3 (`Execution` job) and is never exposed to the agent.
@@ -951,12 +978,7 @@ Should be replaced with the domain the AWF-sandboxed agent uses to reach MCPG on
951978

952979
## {{ copilot_version }}
953980

954-
Should be replaced with the pinned version of the `Microsoft.Copilot.CLI.linux-x64` NuGet package (defined as `COPILOT_CLI_VERSION` constant in `src/compile/common.rs`). This version is used in the pipeline step that installs the Copilot CLI tool from Azure Artifacts.
955-
956-
The generated pipelines install the package from:
957-
```
958-
https://pkgs.dev.azure.com/msazuresphere/_packaging/Guardian1ESPTUpstreamOrgFeed/nuget/v3/index.json
959-
```
981+
**Removed.** This marker has been absorbed into `{{ engine_install_steps }}`. The `COPILOT_CLI_VERSION` constant now lives in `src/engine.rs` and is used internally by `Engine::install_steps()`. The version can be overridden per-agent via `engine: { id: copilot, version: "..." }` in front matter.
960982

961983
### 1ES-Specific Template Markers
962984

0 commit comments

Comments
 (0)