Skip to content

Fix schema/code inconsistencies: remove engine.firewall dead code, error_patterns, and cleanup-script #23423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Mar 29, 2026
Merged

Fix schema/code inconsistencies: remove engine.firewall dead code, error_patterns, and cleanup-script #23423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b0ae50
Initial plan
Copilot Mar 29, 2026
5da3af6
fix: remove engine.firewall dead code, error_patterns, and cleanup-sc…
Copilot Mar 29, 2026
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
35 changes: 0 additions & 35 deletions pkg/cli/workflows/example-custom-error-patterns.md
View file
Open in desktop

This file was deleted.

38 changes: 0 additions & 38 deletions pkg/parser/schemas/main_workflow_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3042,10 +3042,6 @@
"description": "HTTPS URL pattern with optional wildcards (e.g., 'https://github.com/githubnext/*')"
},
"examples": [["https://github.com/githubnext/*", "https://api.github.com/repos/*"]]
},
"cleanup-script": {
"type": "string",
"description": "Path to cleanup script run after AWF shuts down (default: ./scripts/ci/cleanup.sh)"
}
},
"additionalProperties": false
Expand Down Expand Up @@ -9163,40 +9159,6 @@
"type": "string"
}
},
"error_patterns": {
"type": "array",
"description": "Custom error patterns for validating agent logs",
"items": {
"type": "object",
"description": "Error pattern definition",
"properties": {
"id": {
"type": "string",
"description": "Unique identifier for this error pattern"
},
"pattern": {
"type": "string",
"description": "Ecma script regular expression pattern to match log lines"
},
"level_group": {
"type": "integer",
"minimum": 0,
"description": "Capture group index (1-based) that contains the error level. Use 0 to infer from pattern content."
},
"message_group": {
"type": "integer",
"minimum": 0,
"description": "Capture group index (1-based) that contains the error message. Use 0 to use the entire match."
},
"description": {
"type": "string",
"description": "Human-readable description of what this pattern matches"
}
},
"required": ["pattern"],
"additionalProperties": false
}
},
"config": {
"type": "string",
"description": "Additional TOML configuration text that will be appended to the generated config.toml in the action (codex engine only)"
Expand Down
43 changes: 2 additions & 41 deletions pkg/workflow/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ type EngineConfig struct {
Env map[string]string
Config string
Args []string
Firewall *FirewallConfig // AWF firewall configuration
Agent string // Agent identifier for copilot --agent flag (copilot engine only)
APITarget string // Custom API endpoint hostname (e.g., "api.acme.ghe.com" or "api.enterprise.githubcopilot.com")
Agent string // Agent identifier for copilot --agent flag (copilot engine only)
APITarget string // Custom API endpoint hostname (e.g., "api.acme.ghe.com" or "api.enterprise.githubcopilot.com")

// Inline definition fields (populated when engine.runtime is specified in frontmatter)
IsInlineDefinition bool // true when the engine is defined inline via engine.runtime + optional engine.provider
Expand Down Expand Up @@ -277,44 +276,6 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng
}
}

// Extract optional 'firewall' field (object format)
if firewall, hasFirewall := engineObj["firewall"]; hasFirewall {
if firewallObj, ok := firewall.(map[string]any); ok {
firewallConfig := &FirewallConfig{}

// Extract enabled field (defaults to true for copilot)
if enabled, hasEnabled := firewallObj["enabled"]; hasEnabled {
if enabledBool, ok := enabled.(bool); ok {
firewallConfig.Enabled = enabledBool
}
}

// Extract version field (empty = latest)
if version, hasVersion := firewallObj["version"]; hasVersion {
if versionStr, ok := version.(string); ok {
firewallConfig.Version = versionStr
}
}

// Extract log-level field (default: "debug")
if logLevel, hasLogLevel := firewallObj["log-level"]; hasLogLevel {
if logLevelStr, ok := logLevel.(string); ok {
firewallConfig.LogLevel = logLevelStr
}
}

// Extract cleanup-script field (default: "./scripts/ci/cleanup.sh")
if cleanupScript, hasCleanupScript := firewallObj["cleanup-script"]; hasCleanupScript {
if cleanupScriptStr, ok := cleanupScript.(string); ok {
firewallConfig.CleanupScript = cleanupScriptStr
}
}

config.Firewall = firewallConfig
engineLog.Print("Extracted firewall configuration")
}
}

// Extract optional 'api-target' field (custom API endpoint for any engine)
if apiTarget, hasAPITarget := engineObj["api-target"]; hasAPITarget {
if apiTargetStr, ok := apiTarget.(string); ok && apiTargetStr != "" {
Expand Down
13 changes: 6 additions & 7 deletions pkg/workflow/firewall.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ var firewallLog = logger.New("workflow:firewall")
// FirewallConfig represents AWF (gh-aw-firewall) configuration for network egress control.
// These settings are specific to the AWF sandbox and do not apply to Sandbox Runtime (SRT).
type FirewallConfig struct {
Enabled bool `yaml:"enabled,omitempty"` // Enable/disable AWF (default: true for copilot when network restrictions present)
Version string `yaml:"version,omitempty"` // AWF version (empty = latest)
Args []string `yaml:"args,omitempty"` // Additional arguments to pass to AWF
LogLevel string `yaml:"log_level,omitempty"` // AWF log level (default: "info")
CleanupScript string `yaml:"cleanup_script,omitempty"` // Cleanup script path (default: "./scripts/ci/cleanup.sh")
SSLBump bool `yaml:"ssl_bump,omitempty"` // AWF-only: Enable SSL Bump for HTTPS content inspection (allows URL path filtering)
AllowURLs []string `yaml:"allow_urls,omitempty"` // AWF-only: URL patterns to allow for HTTPS (requires SSLBump), e.g., "https://github.com/githubnext/*"
Enabled bool `yaml:"enabled,omitempty"` // Enable/disable AWF (default: true for copilot when network restrictions present)
Version string `yaml:"version,omitempty"` // AWF version (empty = latest)
Args []string `yaml:"args,omitempty"` // Additional arguments to pass to AWF
LogLevel string `yaml:"log_level,omitempty"` // AWF log level (default: "info")
SSLBump bool `yaml:"ssl_bump,omitempty"` // AWF-only: Enable SSL Bump for HTTPS content inspection (allows URL path filtering)
AllowURLs []string `yaml:"allow_urls,omitempty"` // AWF-only: URL patterns to allow for HTTPS (requires SSLBump), e.g., "https://github.com/githubnext/*"
Comment on lines +16 to +21
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The FirewallConfig struct tags use snake_case keys (log_level, ssl_bump, allow_urls) and also include an enabled field, but the schema and extraction logic use kebab-case (log-level, ssl-bump, allow-urls) and the firewall object variant in main_workflow_schema.json does not allow enabled as a property (see schema around pkg/parser/schemas/main_workflow_schema.json:3011-3047). If this struct is ever marshaled back to YAML/JSON (e.g., via FrontmatterConfig.ToMap), it can generate config that fails schema validation and/or drop fields when parsing via JSON unmarshalling. Consider aligning the serialized field names with the schema (kebab-case) and ensuring enabled is represented via the non-object firewall variants (boolean/null/string) rather than emitted inside the object.

Suggested change
Enabled bool `yaml:"enabled,omitempty"` // Enable/disable AWF (default: true for copilot when network restrictions present)
Version string `yaml:"version,omitempty"` // AWF version (empty = latest)
Args []string `yaml:"args,omitempty"` // Additional arguments to pass to AWF
LogLevel string `yaml:"log_level,omitempty"` // AWF log level (default: "info")
SSLBump bool `yaml:"ssl_bump,omitempty"` // AWF-only: Enable SSL Bump for HTTPS content inspection (allows URL path filtering)
AllowURLs []string `yaml:"allow_urls,omitempty"` // AWF-only: URL patterns to allow for HTTPS (requires SSLBump), e.g., "https://github.com/githubnext/*"
Enabled bool `yaml:"-"` // Enable/disable AWF (default: true for copilot when network restrictions present)
Version string `yaml:"version,omitempty"` // AWF version (empty = latest)
Args []string `yaml:"args,omitempty"` // Additional arguments to pass to AWF
LogLevel string `yaml:"log-level,omitempty"` // AWF log level (default: "info")
SSLBump bool `yaml:"ssl-bump,omitempty"` // AWF-only: Enable SSL Bump for HTTPS content inspection (allows URL path filtering)
AllowURLs []string `yaml:"allow-urls,omitempty"` // AWF-only: URL patterns to allow for HTTPS (requires SSLBump), e.g., "https://github.com/githubnext/*"

Copilot uses AI. Check for mistakes.
}

// isFirewallDisabledBySandboxAgent checks if the firewall is disabled via sandbox.agent: false
Expand Down
Loading