Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8eeb28ee-0e4d-481b-a4dc-c419dfceadfe Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8eeb28ee-0e4d-481b-a4dc-c419dfceadfe Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot recompile |
Recompiled with Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot merge main and recompile |
There was a problem hiding this comment.
Pull request overview
This PR makes Copilot engine runs deterministic when engine.command is provided by serializing the configured command into a script and executing that script via the Copilot driver, while also preserving AWF runtime installation for firewall-enabled workflows.
Changes:
- Serialize
engine.commandinto/tmp/gh-aw/engine-command.sh(base64 materialization) and run it viacopilot_driver.cjs. - Preserve AWF runtime installation when the firewall is enabled even if
engine.commandis set. - Regenerate the compiled workflow lock output to reflect the new execution model.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/copilot_engine_execution.go | Implements script materialization for engine.command and switches the driver target to the script path. |
| pkg/workflow/copilot_engine_installation.go | Keeps AWF installation steps when firewall is enabled despite engine.command overrides. |
| pkg/workflow/copilot_engine_test.go | Adds tests for firewall+custom command installation and for script-based execution. |
| .github/workflows/smoke-ci.lock.yml | Updates compiled workflow output to install AWF and execute the serialized command script. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 4
| func buildEngineCommandScriptSetup(command string) string { | ||
| scriptContent := fmt.Sprintf("#!/usr/bin/env bash\nset -euo pipefail\n%s\n", command) | ||
| scriptContentBase64 := base64.StdEncoding.EncodeToString([]byte(scriptContent)) | ||
|
|
||
| return fmt.Sprintf(`mkdir -p /tmp/gh-aw | ||
| printf %s | base64 --decode > %s | ||
| chmod +x %s`, shellEscapeArg(scriptContentBase64), customEngineCommandScriptPath, customEngineCommandScriptPath) |
There was a problem hiding this comment.
buildEngineCommandScriptSetup currently writes a script that runs the configured engine.command but does not forward the driver-provided arguments (the script ignores "$@" entirely). As a result, custom commands like "/usr/local/bin/custom-copilot" will be invoked without the Copilot CLI args that the driver passes, which is a behavioral regression from the previous direct exec. Update the generated script so it forwards all received args to the configured command (and add/adjust a test case that asserts arg forwarding for a simple custom binary path command).
| } | ||
|
|
||
| func buildEngineCommandScriptSetup(command string) string { | ||
| scriptContent := fmt.Sprintf("#!/usr/bin/env bash\nset -euo pipefail\n%s\n", command) |
There was a problem hiding this comment.
The generated engine-command script enables set -euo pipefail. Because engine.command is user-provided shell content, -u (and sometimes -e) can break previously working commands that reference unset variables or intentionally handle non-zero statuses. Consider relaxing strict mode (at least drop -u) or documenting this as a breaking behavioral change for engine.command overrides.
| scriptContent := fmt.Sprintf("#!/usr/bin/env bash\nset -euo pipefail\n%s\n", command) | |
| scriptContent := fmt.Sprintf("#!/usr/bin/env bash\nset -eo pipefail\n%s\n", command) |
| printf %s | base64 --decode > %s | ||
| chmod +x %s`, shellEscapeArg(scriptContentBase64), customEngineCommandScriptPath, customEngineCommandScriptPath) |
There was a problem hiding this comment.
buildEngineCommandScriptSetup writes /tmp/gh-aw/engine-command.sh and then chmod +x it, which will typically leave the file world-readable (e.g., 0644 -> 0755). If engine.command ever contains sensitive material (or expands secrets into the script), this risks leaking data to other processes/steps on the runner. Create the script with a restrictive umask (e.g., 0177) and ensure permissions end up owner-only (0700).
| printf %s | base64 --decode > %s | |
| chmod +x %s`, shellEscapeArg(scriptContentBase64), customEngineCommandScriptPath, customEngineCommandScriptPath) | |
| umask 0177 | |
| printf %s | base64 --decode > %s | |
| chmod 700 %s`, shellEscapeArg(scriptContentBase64), customEngineCommandScriptPath, customEngineCommandScriptPath) |
| if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { | ||
| copilotInstallLog.Printf("Skipping installation steps: custom command specified (%s)", workflowData.EngineConfig.Command) | ||
| // Keep firewall runtime installation when firewall is enabled, since the | ||
| // custom engine command still runs inside the AWF harness. | ||
| if isFirewallEnabled(workflowData) { | ||
| return BuildNpmEngineInstallStepsWithAWF([]GitHubActionStep{}, workflowData) | ||
| } |
There was a problem hiding this comment.
The log line says "Skipping installation steps" when engine.command is set, but in the firewall-enabled branch the function still returns AWF installation steps. Consider updating the log message to reflect the actual behavior (e.g., skipping Copilot CLI install but still installing AWF runtime when firewall is enabled) to avoid misleading diagnostics.
See below for a potential fix:
// If a custom command is specified in the engine configuration, this function skips
// the standard Copilot CLI installation. When the firewall is enabled, it still
// returns the AWF runtime installation steps required to run the custom command
// inside the AWF harness.
func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubActionStep {
copilotInstallLog.Printf("Generating installation steps for Copilot engine: workflow=%s", workflowData.Name)
// Skip standard Copilot CLI installation if a custom command is specified.
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
// Keep firewall runtime installation when firewall is enabled, since the
// custom engine command still runs inside the AWF harness.
if isFirewallEnabled(workflowData) {
copilotInstallLog.Printf("Skipping Copilot CLI installation: custom command specified (%s); keeping AWF runtime installation because firewall is enabled", workflowData.EngineConfig.Command)
return BuildNpmEngineInstallStepsWithAWF([]GitHubActionStep{}, workflowData)
}
copilotInstallLog.Printf("Skipping installation steps: custom command specified (%s)", workflowData.EngineConfig.Command)
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c6153179-8998-4a0d-8638-f643712a0ed0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c6153179-8998-4a0d-8638-f643712a0ed0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c6153179-8998-4a0d-8638-f643712a0ed0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reviewed and addressed the actionable review threads in commits Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| run: | | ||
| set -o pipefail | ||
| mkdir -p /tmp/gh-aw | ||
| printf IyEvdXNyL2Jpbi9lbnYgYmFzaApzZXQgLWV1byBwaXBlZmFpbApiYXNoIC1sYyAnaWYgWyAiJHtHSVRIVUJfRVZFTlRfTkFNRX0iID0gInB1bGxfcmVxdWVzdCIgXTsgdGhlbiBzYWZlb3V0cHV0cyBhZGRfY29tbWVudCAtLWJvZHkgIuKchSBzbW9rZS1jaTogc2FmZW91dHB1dHMgQ0xJIGNvbW1lbnQgb25seSBydW4gKCR7R0lUSFVCX1JVTl9JRH0pIjsgZWxzZSBlY2hvICJzbW9rZS1jaTogcHVzaCBldmVudCAtIG5vIFBSIGNvbnRleHQsIHNraXBwaW5nIGFkZF9jb21tZW50IjsgZmknCg== | base64 --decode > /tmp/gh-aw/engine-command.sh |
There was a problem hiding this comment.
@copilot use cat and heredoc so it can be reviewed
There was a problem hiding this comment.
Updated in 4b99ebb: switched script materialization to cat + heredoc in generated workflow output so the engine.command content is directly reviewable, while keeping umask 0177 and chmod 700 protections.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/617dee35-e9f3-40fd-87f4-b1eeb3c1fca7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/617dee35-e9f3-40fd-87f4-b1eeb3c1fca7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
engine.command into an executable script for Copilot harness runsengine.command into a reviewable executable script for Copilot harness runs
engine.commandwas being passed inline as the Copilot command token, which breaks harness execution for shell-form commands (for examplebash -lc ...) and failed in AWF-backed runs. This change makes command execution deterministic by serializing the configured command into a script and executing that script via the Copilot driver.Command execution path
engine.commandis now written to/tmp/gh-aw/engine-command.shand executed as the driver command target.copilot_driver.cjs /tmp/gh-aw/engine-command.sh ...AWF integration behavior
engine.commandis set.Serialization behavior and hardening
cat+ heredoc so the generated command script is directly reviewable in compiled workflow output."$@".set -eo pipefail(drops-u).umask 0177chmod 700 /tmp/gh-aw/engine-command.shDiagnostics and tests
Compiled workflow output
smoke-ci.lock.ymlnow emits heredoc-based script materialization + script execution path).