diff --git a/.github/workflows/test-claude-create-issue.lock.yml b/.github/workflows/test-claude-create-issue.lock.yml index bd775ffd5db..9c58c79dec9 100644 --- a/.github/workflows/test-claude-create-issue.lock.yml +++ b/.github/workflows/test-claude-create-issue.lock.yml @@ -16,12 +16,119 @@ run-name: "Test Claude Create Issue" jobs: test-claude-create-issue: runs-on: ubuntu-latest - permissions: read-all + permissions: {} outputs: output: ${{ steps.collect_output.outputs.output }} steps: - name: Checkout repository uses: actions/checkout@v5 + - name: Generate Network Permissions Hook + run: | + mkdir -p .claude/hooks + cat > .claude/hooks/network_permissions.py << 'EOF' + #!/usr/bin/env python3 + """ + Network permissions validator for Claude Code engine. + Generated by gh-aw from engine network permissions configuration. + """ + + import json + import sys + import urllib.parse + import re + + # Domain whitelist (populated during generation) + ALLOWED_DOMAINS = [] + + def extract_domain(url_or_query): + """Extract domain from URL or search query.""" + if not url_or_query: + return None + + if url_or_query.startswith(('http://', 'https://')): + return urllib.parse.urlparse(url_or_query).netloc.lower() + + # Check for domain patterns in search queries + match = re.search(r'site:([a-zA-Z0-9.-]+\.[a-zA-Z]{2,})', url_or_query) + if match: + return match.group(1).lower() + + return None + + def is_domain_allowed(domain): + """Check if domain is allowed.""" + if not domain: + # If no domain detected, allow only if not under deny-all policy + return bool(ALLOWED_DOMAINS) # False if empty list (deny-all), True if has domains + + # Empty allowed domains means deny all + if not ALLOWED_DOMAINS: + return False + + for pattern in ALLOWED_DOMAINS: + regex = pattern.replace('.', r'\.').replace('*', '.*') + if re.match(f'^{regex}$', domain): + return True + return False + + # Main logic + try: + data = json.load(sys.stdin) + tool_name = data.get('tool_name', '') + tool_input = data.get('tool_input', {}) + + if tool_name not in ['WebFetch', 'WebSearch']: + sys.exit(0) # Allow other tools + + target = tool_input.get('url') or tool_input.get('query', '') + domain = extract_domain(target) + + # For WebSearch, apply domain restrictions consistently + # If no domain detected in search query, check if restrictions are in place + if tool_name == 'WebSearch' and not domain: + # Since this hook is only generated when network permissions are configured, + # empty ALLOWED_DOMAINS means deny-all policy + if not ALLOWED_DOMAINS: # Empty list means deny all + print(f"Network access blocked: deny-all policy in effect", file=sys.stderr) + print(f"No domains are allowed for WebSearch", file=sys.stderr) + sys.exit(2) # Block under deny-all policy + else: + print(f"Network access blocked for WebSearch: no specific domain detected", file=sys.stderr) + print(f"Allowed domains: {', '.join(ALLOWED_DOMAINS)}", file=sys.stderr) + sys.exit(2) # Block general searches when domain allowlist is configured + + if not is_domain_allowed(domain): + print(f"Network access blocked for domain: {domain}", file=sys.stderr) + print(f"Allowed domains: {', '.join(ALLOWED_DOMAINS)}", file=sys.stderr) + sys.exit(2) # Block with feedback to Claude + + sys.exit(0) # Allow + + except Exception as e: + print(f"Network validation error: {e}", file=sys.stderr) + sys.exit(2) # Block on errors + + EOF + chmod +x .claude/hooks/network_permissions.py + - name: Generate Claude Settings + run: | + cat > .claude/settings.json << 'EOF' + { + "hooks": { + "PreToolUse": [ + { + "matcher": "WebFetch|WebSearch", + "hooks": [ + { + "type": "command", + "command": ".claude/hooks/network_permissions.py" + } + ] + } + ] + } + } + EOF - name: Setup agent output id: setup_agent_output uses: actions/github-script@v7 @@ -222,6 +329,7 @@ jobs: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} mcp_config: /tmp/mcp-config/mcp-servers.json prompt_file: /tmp/aw-prompts/prompt.txt + settings: .claude/settings.json timeout_minutes: 5 env: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} diff --git a/.github/workflows/test-claude-create-issue.md b/.github/workflows/test-claude-create-issue.md index 629e4c9be17..c15a7c12921 100644 --- a/.github/workflows/test-claude-create-issue.md +++ b/.github/workflows/test-claude-create-issue.md @@ -4,7 +4,7 @@ on: engine: id: claude - +strict: true safe-outputs: create-issue: title-prefix: "[claude-test] " diff --git a/docs/frontmatter.md b/docs/frontmatter.md index c8601a1fff5..e71eeff659f 100644 --- a/docs/frontmatter.md +++ b/docs/frontmatter.md @@ -22,6 +22,7 @@ The YAML frontmatter supports standard GitHub Actions properties plus additional - `tools`: Available tools and MCP servers for the AI engine - `cache`: Cache configuration for workflow dependencies - `safe-outputs`: [Safe Output Processing](safe-outputs.md) for automatic issue creation and comment posting. +- `strict`: Enable strict mode to enforce deny-by-default permissions for engine and MCP servers ## Trigger Events (`on:`) @@ -283,6 +284,58 @@ engine: - "*.safe-domain.org" ``` +## Strict Mode (`strict:`) + +Strict mode enforces deny-by-default permissions for both engine and MCP servers even when no explicit permissions are configured. This provides a zero-trust security model that adheres to security best practices. + +```yaml +strict: true # Enable strict mode (default: false) +``` + +### Behavior + +When strict mode is enabled: + +1. **No explicit network permissions**: Automatically enforces deny-all policy + ```yaml + strict: true + engine: claude + # No engine.permissions.network specified + # Result: All network access is denied (same as empty allowed list) + ``` + +2. **Explicit network permissions**: Uses the specified permissions normally + ```yaml + strict: true + engine: + id: claude + permissions: + network: + allowed: ["api.github.com"] + # Result: Only api.github.com is accessible + ``` + +3. **Strict mode disabled**: Maintains backwards-compatible behavior + ```yaml + strict: false # or omitted entirely + engine: claude + # No engine.permissions.network specified + # Result: Unrestricted network access (backwards compatible) + ``` + +### Use Cases + +- **Security-first workflows**: When you want to ensure no accidental network access +- **Compliance requirements**: For environments requiring deny-by-default policies +- **Zero-trust environments**: When explicit permissions should always be required +- **Migration assistance**: Gradually migrate existing workflows to explicit permissions + +### Compatibility + +- Only applies to engines that support network permissions (currently Claude) +- Non-Claude engines ignore strict mode setting +- Backwards compatible when `strict: false` or omitted + ## Safe Outputs Configuration (`safe-outputs:`) See [Safe Outputs Processing](safe-outputs.md) for automatic issue creation, comment posting and other safe outputs. diff --git a/pkg/cli/templates/instructions.md b/pkg/cli/templates/instructions.md index 1d8cdb6e884..e6046d329b6 100644 --- a/pkg/cli/templates/instructions.md +++ b/pkg/cli/templates/instructions.md @@ -76,6 +76,11 @@ The YAML frontmatter supports these fields: - "*.trusted-domain.com" ``` +- **`strict:`** - Enable strict mode for deny-by-default permissions (boolean, default: false) + ```yaml + strict: true # Enforce deny-all network permissions when no explicit permissions set + ``` + - **`tools:`** - Tool configuration for coding agent - `github:` - GitHub API tools - `claude:` - Claude-specific tools diff --git a/pkg/parser/schema_test.go b/pkg/parser/schema_test.go index 52905021065..f94100c4e93 100644 --- a/pkg/parser/schema_test.go +++ b/pkg/parser/schema_test.go @@ -474,6 +474,31 @@ func TestValidateMainWorkflowFrontmatterWithSchema(t *testing.T) { wantErr: true, errContains: "additional properties 'invalid_prop' not allowed", }, + { + name: "valid strict mode true", + frontmatter: map[string]any{ + "on": "push", + "strict": true, + }, + wantErr: false, + }, + { + name: "valid strict mode false", + frontmatter: map[string]any{ + "on": "push", + "strict": false, + }, + wantErr: false, + }, + { + name: "invalid strict mode as string", + frontmatter: map[string]any{ + "on": "push", + "strict": "true", + }, + wantErr: true, + errContains: "want boolean", + }, { name: "valid claude engine with network permissions", frontmatter: map[string]any{ diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 206b4d1e817..c4452bb0a94 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -972,6 +972,10 @@ } ] }, + "strict": { + "type": "boolean", + "description": "Enable strict mode to enforce deny-by-default permissions for engine and MCP servers even when permissions are not explicitly set" + }, "safe-outputs": { "type": "object", "description": "Output configuration for automatic safe outputs", diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index f618b2235f7..2343a5a5a63 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -464,6 +464,26 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) // Extract AI engine setting from frontmatter engineSetting, engineConfig := c.extractEngineConfig(result.Frontmatter) + // Extract strict mode setting from frontmatter + strictMode := c.extractStrictMode(result.Frontmatter) + + // Apply strict mode: inject deny-all network permissions if strict mode is enabled + // and no explicit network permissions are configured + if strictMode && engineConfig != nil && engineConfig.ID == "claude" { + if engineConfig.Permissions == nil || engineConfig.Permissions.Network == nil { + // Initialize permissions structure if needed + if engineConfig.Permissions == nil { + engineConfig.Permissions = &EnginePermissions{} + } + if engineConfig.Permissions.Network == nil { + // Inject deny-all network permissions (empty allowed list) + engineConfig.Permissions.Network = &NetworkPermissions{ + Allowed: []string{}, // Empty list means deny-all + } + } + } + } + // Override with command line AI engine setting if provided if c.engineOverride != "" { originalEngineSetting := engineSetting @@ -702,7 +722,7 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) } // Apply defaults - c.applyDefaults(workflowData, markdownPath) + c.applyDefaults(workflowData, markdownPath, strictMode) // Apply pull request draft filter if specified c.applyPullRequestDraftFilter(workflowData, result.Frontmatter) @@ -904,7 +924,7 @@ func (c *Compiler) extractCommandName(frontmatter map[string]any) string { } // applyDefaults applies default values for missing workflow sections -func (c *Compiler) applyDefaults(data *WorkflowData, markdownPath string) { +func (c *Compiler) applyDefaults(data *WorkflowData, markdownPath string, strictMode bool) { // Check if this is a command trigger workflow (by checking if user specified "on.command") isCommandTrigger := false if data.On == "" { @@ -1002,7 +1022,16 @@ func (c *Compiler) applyDefaults(data *WorkflowData, markdownPath string) { } if data.Permissions == "" { - data.Permissions = `permissions: read-all` + if strictMode { + // In strict mode, default to empty permissions instead of read-all + data.Permissions = `permissions: {}` + } else { + // Default behavior: use read-all permissions + data.Permissions = `permissions: read-all` + } + } else if strictMode { + // In strict mode, validate permissions and warn about write permissions + c.validatePermissionsInStrictMode(data.Permissions) } // Generate concurrency configuration using the dedicated concurrency module diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index 2ea683599ab..10fd4db59d1 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -2933,6 +2933,251 @@ func TestGenerateJobName(t *testing.T) { } } +func TestStrictModeNetworkPermissions(t *testing.T) { + compiler := NewCompiler(false, "", "test") + + tmpDir := t.TempDir() + + t.Run("strict mode disabled with no permissions (default behavior)", func(t *testing.T) { + testContent := `--- +on: push +engine: claude +strict: false +--- + +# Test Workflow + +This is a test workflow without network permissions. +` + testFile := filepath.Join(tmpDir, "no-strict-workflow.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + err := compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected compilation error: %v", err) + } + + // Read the compiled output + lockFile := filepath.Join(tmpDir, "no-strict-workflow.lock.yml") + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // Should not contain network hook setup (no restrictions) + if strings.Contains(string(lockContent), "Generate Network Permissions Hook") { + t.Error("Should not contain network hook setup when strict mode is disabled and no permissions set") + } + if strings.Contains(string(lockContent), ".claude/settings.json") { + t.Error("Should not reference settings.json when strict mode is disabled and no permissions set") + } + }) + + t.Run("strict mode enabled with no explicit permissions (should enforce deny-all)", func(t *testing.T) { + testContent := `--- +on: push +engine: claude +strict: true +--- + +# Test Workflow + +This is a test workflow with strict mode but no explicit network permissions. +` + testFile := filepath.Join(tmpDir, "strict-no-perms-workflow.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + err := compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected compilation error: %v", err) + } + + // Read the compiled output + lockFile := filepath.Join(tmpDir, "strict-no-perms-workflow.lock.yml") + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // Should contain network hook setup (deny-all enforcement) + if !strings.Contains(string(lockContent), "Generate Network Permissions Hook") { + t.Error("Should contain network hook setup when strict mode is enabled") + } + if !strings.Contains(string(lockContent), ".claude/settings.json") { + t.Error("Should reference settings.json when strict mode is enabled") + } + // Should have empty ALLOWED_DOMAINS array for deny-all + if !strings.Contains(string(lockContent), "ALLOWED_DOMAINS = []") { + t.Error("Should have empty ALLOWED_DOMAINS array for deny-all policy") + } + }) + + t.Run("strict mode enabled with explicit network permissions (should use explicit permissions)", func(t *testing.T) { + testContent := `--- +on: push +engine: + id: claude + permissions: + network: + allowed: ["example.com", "api.github.com"] +strict: true +--- + +# Test Workflow + +This is a test workflow with strict mode and explicit network permissions. +` + testFile := filepath.Join(tmpDir, "strict-with-perms-workflow.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + err := compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected compilation error: %v", err) + } + + // Read the compiled output + lockFile := filepath.Join(tmpDir, "strict-with-perms-workflow.lock.yml") + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // Should contain network hook setup with specified domains + if !strings.Contains(string(lockContent), "Generate Network Permissions Hook") { + t.Error("Should contain network hook setup when strict mode is enabled with explicit permissions") + } + if !strings.Contains(string(lockContent), `"example.com"`) { + t.Error("Should contain example.com in allowed domains") + } + if !strings.Contains(string(lockContent), `"api.github.com"`) { + t.Error("Should contain api.github.com in allowed domains") + } + }) + + t.Run("strict mode not specified (should default to false)", func(t *testing.T) { + testContent := `--- +on: push +engine: claude +--- + +# Test Workflow + +This is a test workflow without strict mode specified. +` + testFile := filepath.Join(tmpDir, "no-strict-field-workflow.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + err := compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected compilation error: %v", err) + } + + // Read the compiled output + lockFile := filepath.Join(tmpDir, "no-strict-field-workflow.lock.yml") + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // Should not contain network hook setup (default behavior) + if strings.Contains(string(lockContent), "Generate Network Permissions Hook") { + t.Error("Should not contain network hook setup when strict mode is not specified") + } + }) + + t.Run("strict mode with non-claude engine (should be ignored)", func(t *testing.T) { + testContent := `--- +on: push +engine: codex +strict: true +--- + +# Test Workflow + +This is a test workflow with strict mode and codex engine. +` + testFile := filepath.Join(tmpDir, "strict-codex-workflow.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + err := compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected compilation error: %v", err) + } + + // Read the compiled output + lockFile := filepath.Join(tmpDir, "strict-codex-workflow.lock.yml") + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // Should not contain claude-specific network hook setup + if strings.Contains(string(lockContent), "Generate Network Permissions Hook") { + t.Error("Should not contain network hook setup for non-claude engines") + } + }) +} + +func TestExtractStrictMode(t *testing.T) { + compiler := NewCompiler(false, "", "test") + + tests := []struct { + name string + frontmatter map[string]any + expected bool + }{ + { + name: "strict mode true", + frontmatter: map[string]any{"strict": true}, + expected: true, + }, + { + name: "strict mode false", + frontmatter: map[string]any{"strict": false}, + expected: false, + }, + { + name: "strict mode not specified", + frontmatter: map[string]any{"on": "push"}, + expected: false, + }, + { + name: "strict mode as string (should default to false)", + frontmatter: map[string]any{"strict": "true"}, + expected: false, + }, + { + name: "empty frontmatter", + frontmatter: map[string]any{}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := compiler.extractStrictMode(tt.frontmatter) + if result != tt.expected { + t.Errorf("extractStrictMode() = %v, want %v", result, tt.expected) + } + }) + } +} + func TestMCPImageField(t *testing.T) { // Create temporary directory for test files tmpDir, err := os.MkdirTemp("", "mcp-container-test") diff --git a/pkg/workflow/engine.go b/pkg/workflow/engine.go index 0feb416b553..d1f62d09660 100644 --- a/pkg/workflow/engine.go +++ b/pkg/workflow/engine.go @@ -1,6 +1,8 @@ package workflow -import "fmt" +import ( + "fmt" +) // EngineConfig represents the parsed engine configuration type EngineConfig struct { diff --git a/pkg/workflow/strict.go b/pkg/workflow/strict.go new file mode 100644 index 00000000000..0e30ef20410 --- /dev/null +++ b/pkg/workflow/strict.go @@ -0,0 +1,29 @@ +package workflow + +import ( + "fmt" + "strings" + + "github.com/githubnext/gh-aw/pkg/console" +) + +// extractStrictMode extracts strict mode setting from frontmatter +func (c *Compiler) extractStrictMode(frontmatter map[string]any) bool { + if strict, exists := frontmatter["strict"]; exists { + if strictBool, ok := strict.(bool); ok { + return strictBool + } + } + return false // Default to false if not specified or not a boolean +} + +// validatePermissionsInStrictMode checks permissions in strict mode and warns about write permissions +func (c *Compiler) validatePermissionsInStrictMode(permissions string) { + if permissions == "" { + return + } + hasWritePermissions := strings.Contains(permissions, "write") + if hasWritePermissions { + fmt.Println(console.FormatWarningMessage("Strict mode: Found 'write' permissions. Consider using 'read' permissions only for better security.")) + } +}