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
82 changes: 82 additions & 0 deletions docs/adr/26544-byok-copilot-feature-flag-composition.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# ADR-26544: BYOK-Copilot Feature Flag as a Composing Abstraction for Offline Mode Wiring

**Date**: 2026-04-16
**Status**: Draft
**Deciders**: lpcox, Copilot (PR author)

---

## Part 1 — Narrative (Human-Friendly)

### Context

Copilot offline BYOK (Bring Your Own Key) mode requires three separate pieces to work correctly together: injecting a well-known dummy API key (`COPILOT_API_KEY`) into the execution environment to trigger AWF's runtime BYOK-detection path, enabling the `cli-proxy` feature so that `gh` CLI calls are routed through the authenticated DIFC proxy sidecar, and forcing the Copilot CLI installer to use `latest` instead of a pinned version. Previously, workflow authors had to compose all three behaviors manually in their workflow frontmatter, which was error-prone and created a high documentation burden. The system needed a first-class abstraction that bundled the required wiring behind a single, discoverable switch.

### Decision

We will introduce a `byok-copilot` feature flag that, when enabled on a `copilot`-engine workflow, automatically composes the three required behaviors: it injects `COPILOT_API_KEY: dummy-byok-key-for-offline-mode` into the execution environment, implicitly enables the `cli-proxy` feature (for Copilot engine only), and overrides the engine version to `latest` during installation. The implicit `cli-proxy` enablement is implemented inside `isFeatureEnabled` as a special case: when the `cli-proxy` flag is queried, the function first checks whether `byok-copilot` is active on a Copilot engine workflow and returns `true` if so. The dummy API key is a well-known sentinel value; the real credential never leaves the AWF API proxy sidecar.

### Alternatives Considered

#### Alternative 1: Require explicit frontmatter composition

Workflow authors could be required to set `cli-proxy`, `COPILOT_API_KEY`, and the version field individually. This avoids any hidden behavior in the feature flag system, and every enabled behavior is visible at the frontmatter layer. It was rejected because the combination is mandatory for BYOK to work—there is no valid use case for enabling one without the others—and requiring authors to assemble all three manually introduces a category of silent misconfiguration bugs that are hard to diagnose at runtime.

#### Alternative 2: A "preset" system separate from feature flags

A named preset (e.g., `preset: byok-copilot`) could expand to a validated set of options at compile time, distinct from the existing feature flag mechanism. This would avoid adding implicit logic to `isFeatureEnabled` and could be extended to other preset bundles in the future. It was not chosen because it would require introducing a new schema concept (`preset`) alongside the existing `features` map, increasing the surface area of the workflow YAML language without clear benefit over the simpler feature flag approach for a single-flag use case.

#### Alternative 3: Inject all BYOK wiring unconditionally for all Copilot workflows

The AWF runtime could detect BYOK mode automatically (e.g., by the absence of a real API key) and apply all necessary wiring without any explicit flag. This removes the flag entirely and avoids any frontmatter requirement. It was rejected because it would silently alter the execution environment for all Copilot workflows, making it harder to understand why certain environment variables or proxy flags appear, and it couples the BYOK decision to runtime inference rather than explicit author intent.

### Consequences

#### Positive
- Workflow authors enable BYOK mode with a single line (`byok-copilot: true`) instead of assembling three separate, order-sensitive options.
- Misconfiguration is structurally eliminated: all three required behaviors are either all present or all absent.
- The dummy key constant (`CopilotBYOKDummyAPIKey`) is centralised in `pkg/constants`, making the sentinel value discoverable and easy to update if the AWF BYOK detection signal changes.

#### Negative
- `isFeatureEnabled` now contains an implicit cross-flag dependency (`cli-proxy` ← `byok-copilot`). Readers of the feature evaluation code must know this special case exists; it is not visible from the frontmatter alone.
- The implicit `cli-proxy` enablement is engine-scoped (Copilot only), adding a conditional that future maintainers must understand when adding new engines or new flag interactions.
- Forcing `latest` on BYOK install overrides any pinned version the author may have set, which may cause unexpected behavior if a regression is introduced in the latest Copilot CLI release.

#### Neutral
- The glossary entry for `features` has been updated to document `byok-copilot` semantics.
- The dummy key value (`dummy-byok-key-for-offline-mode`) is opaque to the AWF container; the real credential path is entirely in the AWF API proxy sidecar and unchanged by this PR.
- Tests for the new flag are co-located with the existing engine tests, following the project's existing test organization pattern.

---

## Part 2 — Normative Specification (RFC 2119)

> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).

### Feature Flag Behavior

1. Implementations **MUST** treat `byok-copilot` as a valid `FeatureFlag` constant defined in `pkg/constants/feature_constants.go`.
2. When `byok-copilot` is enabled on a workflow with `engine: copilot`, implementations **MUST** inject the value of `CopilotBYOKDummyAPIKey` as the `COPILOT_API_KEY` environment variable into the Copilot execution step.
3. When `byok-copilot` is enabled on a workflow with `engine: copilot`, implementations **MUST** treat the `cli-proxy` feature flag as enabled, regardless of whether it is explicitly set in the workflow frontmatter.
4. The implicit `cli-proxy` enablement described above **MUST NOT** apply to workflows using engines other than `copilot`.
5. When `byok-copilot` is enabled, implementations **MUST** override the Copilot CLI install version to `latest`, ignoring any `engine.version` value set in the workflow frontmatter.

### Dummy Key Constant

1. The placeholder API key used for BYOK detection **MUST** be sourced from the `CopilotBYOKDummyAPIKey` constant in `pkg/constants/engine_constants.go`.
2. Implementations **MUST NOT** embed the dummy key value as a string literal outside of the constants package.
3. The dummy key **MUST NOT** be treated as a real credential; the real AWF API proxy credential **SHALL** remain isolated in the sidecar and **MUST NOT** be injected into the workflow container environment.

### Feature Composition Logic

1. Implicit feature enablement rules (such as `byok-copilot` enabling `cli-proxy`) **MUST** be implemented inside the `isFeatureEnabled` function in `pkg/workflow/features.go`.
2. Each implicit enablement rule **MUST** be scoped to the specific engine or context for which it applies and **MUST NOT** affect unrelated engines or feature flags.
3. Implementations **SHOULD** log a message when an implicit feature enablement rule is triggered, to aid in runtime diagnostics.

### Conformance

An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24490726928) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/glossary.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ See [Engines Reference](/gh-aw/reference/engines/).

### Feature Flags (`features:`)

A frontmatter section that enables experimental or optional compiler and runtime behaviors as key-value pairs. Feature flags provide controlled access to new capabilities before they become defaults or are fully stabilized. Common flags include `action-mode` (controls how custom action references are compiled), `copilot-requests` (enables GitHub Actions token authentication for Copilot; currently in **private preview** — will not work unless your account has been onboarded), `mcp-gateway` (enables the MCP gateway proxy), `integrity-reactions` (enables reaction-based integrity promotion and demotion), and `cli-proxy` (enables CLI proxy mode for integrity enforcement at the network boundary). See [Frontmatter Reference](/gh-aw/reference/frontmatter/#feature-flags-features).
A frontmatter section that enables experimental or optional compiler and runtime behaviors as key-value pairs. Feature flags provide controlled access to new capabilities before they become defaults or are fully stabilized. Common flags include `action-mode` (controls how custom action references are compiled), `copilot-requests` (enables GitHub Actions token authentication for Copilot; currently in **private preview** — will not work unless your account has been onboarded), `byok-copilot` (enables Copilot offline BYOK mode with dummy `COPILOT_API_KEY`, API proxy sidecar, implicit `cli-proxy`, and latest Copilot CLI install), `mcp-gateway` (enables the MCP gateway proxy), `integrity-reactions` (enables reaction-based integrity promotion and demotion), and `cli-proxy` (enables CLI proxy mode for integrity enforcement at the network boundary). See [Frontmatter Reference](/gh-aw/reference/frontmatter/#feature-flags-features).

### Fuzzy Scheduling

Expand Down
5 changes: 5 additions & 0 deletions pkg/constants/engine_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ const (
// CopilotCLIIntegrationIDValue is the value of the integration ID for agentic workflows.
CopilotCLIIntegrationIDValue = "agentic-workflows"

// CopilotBYOKDummyAPIKey is the placeholder API key used to trigger AWF's
// runtime BYOK detection for Copilot offline mode. The real credential remains
// isolated in the AWF API proxy sidecar.
CopilotBYOKDummyAPIKey = "dummy-byok-key-for-offline-mode"

// ClaudeCLIModelEnvVar is the native environment variable name supported by the Claude Code CLI
// for selecting the model. Setting this env var is equivalent to passing --model to the CLI.
ClaudeCLIModelEnvVar = "ANTHROPIC_MODEL"
Expand Down
11 changes: 11 additions & 0 deletions pkg/constants/feature_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ const (
// features:
// copilot-integration-id: true
CopilotIntegrationIDFeatureFlag FeatureFlag = "copilot-integration-id"
// ByokCopilotFeatureFlag enables Copilot CLI offline BYOK mode.
// When enabled with engine: copilot, the compiler:
// - injects a dummy COPILOT_API_KEY into the agent env to trigger AWF BYOK runtime behavior
// - implicitly enables the cli-proxy feature
// - installs the latest Copilot CLI version (un-pinned)
//
// Workflow frontmatter usage:
//
// features:
// byok-copilot: true
ByokCopilotFeatureFlag FeatureFlag = "byok-copilot"
// IntegrityReactionsFeatureFlag enables reaction-based integrity promotion/demotion
// in the MCPG allow-only policy. When enabled, the compiler injects
// endorsement-reactions and disapproval-reactions fields into the allow-only policy.
Expand Down
2 changes: 1 addition & 1 deletion pkg/constants/version_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const DefaultGeminiVersion Version = "0.37.2"
const DefaultGitHubMCPServerVersion Version = "v0.32.0"

// DefaultFirewallVersion is the default version of the gh-aw-firewall (AWF) binary
const DefaultFirewallVersion Version = "v0.25.20"
const DefaultFirewallVersion Version = "v0.25.21"

// AWFExcludeEnvMinVersion is the minimum AWF version that supports the --exclude-env flag.
// Workflows pinning an older AWF version must not emit --exclude-env flags or the run will fail.
Expand Down
26 changes: 26 additions & 0 deletions pkg/workflow/awf_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"
"testing"

"github.com/github/gh-aw/pkg/constants"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -790,6 +791,31 @@ func TestBuildAWFArgsCliProxy(t *testing.T) {
assert.NotContains(t, argsStr, "--cli-proxy-policy", "Should not include deprecated --cli-proxy-policy")
})

t.Run("includes cli-proxy flags when byok-copilot is enabled", func(t *testing.T) {
config := AWFCommandConfig{
EngineName: "copilot",
WorkflowData: &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "copilot",
},
NetworkPermissions: &NetworkPermissions{
Firewall: &FirewallConfig{Enabled: true, Version: "v0.26.0"},
},
Features: map[string]any{
string(constants.ByokCopilotFeatureFlag): true,
},
},
AllowedDomains: "github.com",
}

args := BuildAWFArgs(config)
argsStr := strings.Join(args, " ")

assert.Contains(t, argsStr, "--difc-proxy-host", "Should include --difc-proxy-host when byok-copilot is enabled")
assert.Contains(t, argsStr, "--difc-proxy-ca-cert", "Should include --difc-proxy-ca-cert when byok-copilot is enabled")
})

t.Run("does not include deprecated flags even with guard policy configured", func(t *testing.T) {
config := AWFCommandConfig{
EngineName: "copilot",
Expand Down
7 changes: 7 additions & 0 deletions pkg/workflow/copilot_engine_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,13 @@ touch %s
copilotExecLog.Printf("Added %d custom env vars from agent config", len(agentConfig.Env))
}

// Inject the dummy COPILOT_API_KEY AFTER all env merges so that legacy/manual
// wiring in engine.env or agent.env cannot accidentally overwrite the sentinel
// value that triggers AWF's runtime BYOK detection path.
if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) {
env["COPILOT_API_KEY"] = constants.CopilotBYOKDummyAPIKey
}

// Add HTTP MCP header secrets to env for passthrough
headerSecrets := collectHTTPMCPHeaderSecrets(workflowData.Tools)
for varName, secretExpr := range headerSecrets {
Expand Down
4 changes: 4 additions & 0 deletions pkg/workflow/copilot_engine_installation.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" {
copilotVersion = workflowData.EngineConfig.Version
}
if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) {
copilotVersion = "latest"
copilotInstallLog.Print("byok-copilot enabled: forcing Copilot CLI install version to latest")
}

// Use the installer script for global installation
copilotInstallLog.Print("Using new installer script for Copilot installation")
Expand Down
24 changes: 24 additions & 0 deletions pkg/workflow/copilot_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,30 @@ func TestCopilotEngineEnvOverridesTokenExpression(t *testing.T) {
})
}

func TestCopilotEngineByokFeatureSetsDummyAPIKey(t *testing.T) {
engine := NewCopilotEngine()
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "copilot",
},
Features: map[string]any{
string(constants.ByokCopilotFeatureFlag): true,
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
if len(steps) != 1 {
t.Fatalf("Expected 1 step, got %d", len(steps))
}

stepContent := strings.Join([]string(steps[0]), "\n")
expected := "COPILOT_API_KEY: " + constants.CopilotBYOKDummyAPIKey
if !strings.Contains(stepContent, expected) {
t.Errorf("Expected byok-copilot to inject dummy COPILOT_API_KEY, got:\n%s", stepContent)
}
}

func TestCopilotEngineDriverScript(t *testing.T) {
engine := NewCopilotEngine()

Expand Down
35 changes: 35 additions & 0 deletions pkg/workflow/copilot_installer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,38 @@ func TestCopilotInstallerExpressionVersion_ViaEngineConfig(t *testing.T) {
t.Errorf("Expression should NOT be embedded directly in shell command, got:\n%s", installStep)
}
}

func TestCopilotInstallerByokFeatureUsesLatestVersion(t *testing.T) {
engine := NewCopilotEngine()
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Version: "1.0.0",
},
Features: map[string]any{
string(constants.ByokCopilotFeatureFlag): true,
},
}

steps := engine.GetInstallationSteps(workflowData)

var installStep string
for _, step := range steps {
stepContent := strings.Join(step, "\n")
if strings.Contains(stepContent, "install_copilot_cli.sh") {
installStep = stepContent
break
}
}

if installStep == "" {
t.Fatal("Could not find install step with install_copilot_cli.sh")
}

if !strings.Contains(installStep, `install_copilot_cli.sh" latest`) {
t.Errorf("Expected byok-copilot to force latest Copilot CLI install, got:\n%s", installStep)
}
if strings.Contains(installStep, `install_copilot_cli.sh" 1.0.0`) {
t.Errorf("Expected byok-copilot to ignore pinned version, got:\n%s", installStep)
}
}
12 changes: 12 additions & 0 deletions pkg/workflow/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ func isFeatureEnabled(flag constants.FeatureFlag, workflowData *WorkflowData) bo
flagLower := strings.ToLower(strings.TrimSpace(string(flag)))
featuresLog.Printf("Checking if feature is enabled: %s", flagLower)

// byok-copilot implicitly enables cli-proxy for copilot engine workflows.
// This keeps the byok-copilot frontmatter minimal while ensuring gh CLI
// access is routed through the authenticated DIFC proxy.
if flag == constants.CliProxyFeatureFlag &&
workflowData != nil &&
workflowData.EngineConfig != nil &&
strings.EqualFold(workflowData.EngineConfig.ID, string(constants.CopilotEngine)) &&
isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) {
featuresLog.Print("cli-proxy implicitly enabled by byok-copilot feature flag")
return true
}

// First, check if the feature is explicitly set in frontmatter
if workflowData != nil && workflowData.Features != nil {
if value, exists := workflowData.Features[flagLower]; exists {
Expand Down
Loading
Loading