diff --git a/.github/workflows/daily-team-status.lock.yml b/.github/workflows/daily-team-status.lock.yml index 7183d1331d8..aff54b163e7 100644 --- a/.github/workflows/daily-team-status.lock.yml +++ b/.github/workflows/daily-team-status.lock.yml @@ -2242,8 +2242,7 @@ jobs: with: script: | async function main() { - const path = require("path"); - const crypto = require("crypto"); + const path = require("path"); const redactedDomains = []; function getRedactedDomains() { return [...redactedDomains]; @@ -2438,6 +2437,7 @@ jobs: return s.replace(/\b(fixes?|closes?|resolves?|fix|close|resolve)\s+#(\w+)/gi, (match, action, ref) => `\`${action} #${ref}\``); } } + const crypto = require("crypto"); const TEMPORARY_ID_PATTERN = /#(aw_[0-9a-f]{12})/gi; function generateTemporaryId() { return "aw_" + crypto.randomBytes(6).toString("hex"); @@ -3854,8 +3854,8 @@ jobs: return lines.join("\n"); } function runLogParser(options) { - const fs = require("fs"); - const path = require("path"); + const fs = require("fs"); + const path = require("path"); const { parseLog, parserName, supportsDirectories = false } = options; try { const logPath = process.env.GH_AW_AGENT_OUTPUT; @@ -4467,9 +4467,9 @@ jobs: function main() { - const fs = require("fs"); + const fs = require("fs"); - const path = require("path"); + const path = require("path"); try { diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index c4fb5534d39..42223a57b58 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -5985,7 +5985,7 @@ jobs: format: cyclonedx-json output-file: sbom.cdx.json - name: Upload SBOM artifacts - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: sbom-artifacts path: | diff --git a/docs/src/content/docs/reference/sandbox.md b/docs/src/content/docs/reference/sandbox.md index c7b932558bc..39b77e656e0 100644 --- a/docs/src/content/docs/reference/sandbox.md +++ b/docs/src/content/docs/reference/sandbox.md @@ -119,12 +119,37 @@ sandbox: DEBUG_LEVEL: "verbose" ``` +##### Custom Mounts + +Add custom container mounts to make host paths available inside the AWF container: + +```yaml wrap +sandbox: + agent: + id: awf + mounts: + - "/host/data:/data:ro" + - "/usr/local/bin/custom-tool:/usr/local/bin/custom-tool:ro" + - "/tmp/cache:/cache:rw" +``` + +Mount syntax follows Docker's format: `source:destination:mode` +- `source`: Path on the host system +- `destination`: Path inside the container +- `mode`: Either `ro` (read-only) or `rw` (read-write) + +Custom mounts are useful for: +- Providing access to datasets or configuration files +- Making custom tools available in the container +- Sharing cache directories between host and container + | Field | Type | Description | |-------|------|-------------| | `id` | `string` | Agent identifier: `awf` or `srt` | | `command` | `string` | Custom command to replace AWF binary installation | | `args` | `string[]` | Additional arguments appended to the command | | `env` | `object` | Environment variables set on the execution step | +| `mounts` | `string[]` | Container mounts using syntax `source:destination:mode` | When `command` is specified, the standard AWF installation is skipped and your custom command is used instead. diff --git a/pkg/cli/workflows/test-custom-mounts.md b/pkg/cli/workflows/test-custom-mounts.md new file mode 100644 index 00000000000..f775766478d --- /dev/null +++ b/pkg/cli/workflows/test-custom-mounts.md @@ -0,0 +1,16 @@ +--- +name: Test Custom Mounts +on: workflow_dispatch +engine: copilot +sandbox: + agent: + id: awf + mounts: + - "/host/data:/data:ro" + - "/usr/local/bin/custom-tool:/usr/local/bin/custom-tool:ro" +network: + allowed: + - defaults +--- + +Test workflow to verify custom mounts configuration. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 238c64e24d0..4caa7e64b2e 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1809,6 +1809,16 @@ "type": "string" } }, + "mounts": { + "type": "array", + "description": "Container mounts to add when using AWF. Each mount is specified using Docker mount syntax: 'source:destination:mode' where mode can be 'ro' (read-only) or 'rw' (read-write). Example: '/host/path:/container/path:ro'", + "items": { + "type": "string", + "pattern": "^[^:]+:[^:]+:(ro|rw)$", + "description": "Mount specification in format 'source:destination:mode'" + }, + "examples": [["/host/data:/data:ro", "/usr/local/bin/custom-tool:/usr/local/bin/custom-tool:ro"]] + }, "config": { "type": "object", "description": "Custom Sandbox Runtime configuration (only applies when type is 'srt'). Note: Network configuration is controlled by the top-level 'network' field, not here.", diff --git a/pkg/workflow/copilot_engine.go b/pkg/workflow/copilot_engine.go index 9cf3b4ab193..f327a260adf 100644 --- a/pkg/workflow/copilot_engine.go +++ b/pkg/workflow/copilot_engine.go @@ -356,6 +356,19 @@ func (e *CopilotEngine) GetExecutionSteps(workflowData *WorkflowData, logFile st awfArgs = append(awfArgs, "--mount", "/usr/bin/yq:/usr/bin/yq:ro") copilotLog.Print("Added gh CLI binary mount to AWF container") + // Add custom mounts from agent config if specified + if agentConfig != nil && len(agentConfig.Mounts) > 0 { + // Sort mounts for consistent output + sortedMounts := make([]string, len(agentConfig.Mounts)) + copy(sortedMounts, agentConfig.Mounts) + sort.Strings(sortedMounts) + + for _, mount := range sortedMounts { + awfArgs = append(awfArgs, "--mount", mount) + } + copilotLog.Printf("Added %d custom mounts from agent config", len(sortedMounts)) + } + awfArgs = append(awfArgs, "--allow-domains", allowedDomains) awfArgs = append(awfArgs, "--log-level", awfLogLevel) awfArgs = append(awfArgs, "--proxy-logs-dir", "/tmp/gh-aw/sandbox/firewall/logs") diff --git a/pkg/workflow/frontmatter_extraction.go b/pkg/workflow/frontmatter_extraction.go index 53946d2f615..e86fd0f1a41 100644 --- a/pkg/workflow/frontmatter_extraction.go +++ b/pkg/workflow/frontmatter_extraction.go @@ -838,6 +838,17 @@ func (c *Compiler) extractAgentSandboxConfig(agentVal any) *AgentSandboxConfig { } } + // Extract mounts (container mounts for AWF) + if mountsVal, hasMounts := agentObj["mounts"]; hasMounts { + if mountsSlice, ok := mountsVal.([]any); ok { + for _, mount := range mountsSlice { + if mountStr, ok := mount.(string); ok { + agentConfig.Mounts = append(agentConfig.Mounts, mountStr) + } + } + } + } + return agentConfig } diff --git a/pkg/workflow/sandbox.go b/pkg/workflow/sandbox.go index c5b63f3bc4e..93b536efee3 100644 --- a/pkg/workflow/sandbox.go +++ b/pkg/workflow/sandbox.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "strings" "github.com/githubnext/gh-aw/pkg/logger" ) @@ -41,6 +42,7 @@ type AgentSandboxConfig struct { Command string `yaml:"command,omitempty"` // Custom command to replace AWF or SRT installation Args []string `yaml:"args,omitempty"` // Additional arguments to append to the command Env map[string]string `yaml:"env,omitempty"` // Environment variables to set on the step + Mounts []string `yaml:"mounts,omitempty"` // Container mounts to add for AWF (format: "source:dest:mode") } // SandboxRuntimeConfig represents the Anthropic Sandbox Runtime configuration @@ -226,6 +228,41 @@ func generateSRTConfigJSON(workflowData *WorkflowData) (string, error) { return string(jsonBytes), nil } +// validateMountsSyntax validates that mount strings follow the correct syntax +// Expected format: "source:destination:mode" where mode is either "ro" or "rw" +func validateMountsSyntax(mounts []string) error { + for i, mount := range mounts { + // Split the mount string by colons + parts := strings.Split(mount, ":") + + // Must have exactly 3 parts: source, destination, mode + if len(parts) != 3 { + return fmt.Errorf("invalid mount syntax at index %d: '%s'. Expected format: 'source:destination:mode' (e.g., '/host/path:/container/path:ro')", i, mount) + } + + source := parts[0] + dest := parts[1] + mode := parts[2] + + // Validate that source and destination are not empty + if source == "" { + return fmt.Errorf("invalid mount at index %d: source path is empty in '%s'", i, mount) + } + if dest == "" { + return fmt.Errorf("invalid mount at index %d: destination path is empty in '%s'", i, mount) + } + + // Validate mode is either "ro" or "rw" + if mode != "ro" && mode != "rw" { + return fmt.Errorf("invalid mount at index %d: mode must be 'ro' (read-only) or 'rw' (read-write), got '%s' in '%s'", i, mode, mount) + } + + sandboxLog.Printf("Validated mount %d: source=%s, dest=%s, mode=%s", i, source, dest, mode) + } + + return nil +} + // validateSandboxConfig validates the sandbox configuration // Returns an error if the configuration is invalid func validateSandboxConfig(workflowData *WorkflowData) error { @@ -235,6 +272,14 @@ func validateSandboxConfig(workflowData *WorkflowData) error { sandboxConfig := workflowData.SandboxConfig + // Validate mounts syntax if specified + agentConfig := getAgentConfig(workflowData) + if agentConfig != nil && len(agentConfig.Mounts) > 0 { + if err := validateMountsSyntax(agentConfig.Mounts); err != nil { + return err + } + } + // Validate that SRT is only used with Copilot engine if isSRTEnabled(workflowData) { // Check if the sandbox-runtime feature flag is enabled diff --git a/pkg/workflow/sandbox_mounts_test.go b/pkg/workflow/sandbox_mounts_test.go new file mode 100644 index 00000000000..764a523d6c9 --- /dev/null +++ b/pkg/workflow/sandbox_mounts_test.go @@ -0,0 +1,378 @@ +package workflow + +import ( + "strings" + "testing" +) + +// TestValidateMountsSyntax tests the mount syntax validation function +func TestValidateMountsSyntax(t *testing.T) { + tests := []struct { + name string + mounts []string + wantErr bool + errMsg string + }{ + { + name: "valid read-only mount", + mounts: []string{"/host/path:/container/path:ro"}, + wantErr: false, + }, + { + name: "valid read-write mount", + mounts: []string{"/host/path:/container/path:rw"}, + wantErr: false, + }, + { + name: "multiple valid mounts", + mounts: []string{ + "/host/data:/data:ro", + "/usr/local/bin/tool:/usr/local/bin/tool:ro", + "/tmp/cache:/cache:rw", + }, + wantErr: false, + }, + { + name: "empty mounts list", + mounts: []string{}, + wantErr: false, + }, + { + name: "invalid mount - too few parts", + mounts: []string{"/host/path:/container/path"}, + wantErr: true, + errMsg: "invalid mount syntax", + }, + { + name: "invalid mount - too many parts", + mounts: []string{"/host/path:/container/path:ro:extra"}, + wantErr: true, + errMsg: "invalid mount syntax", + }, + { + name: "invalid mount - empty source", + mounts: []string{":/container/path:ro"}, + wantErr: true, + errMsg: "source path is empty", + }, + { + name: "invalid mount - empty destination", + mounts: []string{"/host/path::ro"}, + wantErr: true, + errMsg: "destination path is empty", + }, + { + name: "invalid mount - invalid mode", + mounts: []string{"/host/path:/container/path:xyz"}, + wantErr: true, + errMsg: "mode must be 'ro' (read-only) or 'rw' (read-write)", + }, + { + name: "invalid mount - uppercase mode", + mounts: []string{"/host/path:/container/path:RO"}, + wantErr: true, + errMsg: "mode must be 'ro' (read-only) or 'rw' (read-write)", + }, + { + name: "mixed valid and invalid mounts", + mounts: []string{ + "/host/data:/data:ro", + "/invalid:mount", + }, + wantErr: true, + errMsg: "invalid mount syntax at index 1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateMountsSyntax(tt.mounts) + + if tt.wantErr && err == nil { + t.Errorf("validateMountsSyntax() expected error but got none") + } + + if !tt.wantErr && err != nil { + t.Errorf("validateMountsSyntax() unexpected error: %v", err) + } + + if tt.wantErr && err != nil && tt.errMsg != "" { + if !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("validateMountsSyntax() error message = %v, want to contain %v", err.Error(), tt.errMsg) + } + } + }) + } +} + +// TestSandboxConfigWithMounts tests that sandbox configuration with mounts is validated +func TestSandboxConfigWithMounts(t *testing.T) { + tests := []struct { + name string + data *WorkflowData + wantErr bool + errMsg string + }{ + { + name: "valid mounts in agent config", + data: &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + Mounts: []string{ + "/host/data:/data:ro", + "/usr/local/bin/tool:/usr/local/bin/tool:ro", + }, + }, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + }, + wantErr: false, + }, + { + name: "no mounts in agent config", + data: &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + }, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + }, + wantErr: false, + }, + { + name: "invalid mount syntax in agent config", + data: &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + Mounts: []string{ + "/host/data:/data:ro", + "/invalid:mount", // Invalid - only 2 parts + }, + }, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + }, + wantErr: true, + errMsg: "invalid mount syntax", + }, + { + name: "invalid mode in mount", + data: &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + Mounts: []string{ + "/host/data:/data:invalid", + }, + }, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + }, + wantErr: true, + errMsg: "mode must be 'ro' (read-only) or 'rw' (read-write)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSandboxConfig(tt.data) + + if tt.wantErr && err == nil { + t.Errorf("validateSandboxConfig() expected error but got none") + } + + if !tt.wantErr && err != nil { + t.Errorf("validateSandboxConfig() unexpected error: %v", err) + } + + if tt.wantErr && err != nil && tt.errMsg != "" { + if !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("validateSandboxConfig() error message = %v, want to contain %v", err.Error(), tt.errMsg) + } + } + }) + } +} + +// TestCopilotEngineWithCustomMounts tests that custom mounts are included in AWF command +func TestCopilotEngineWithCustomMounts(t *testing.T) { + t.Run("custom mounts are included in AWF command", func(t *testing.T) { + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + Mounts: []string{ + "/host/data:/data:ro", + "/usr/local/bin/custom-tool:/usr/local/bin/custom-tool:ro", + }, + }, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + engine := NewCopilotEngine() + steps := engine.GetExecutionSteps(workflowData, "test.log") + + if len(steps) == 0 { + t.Fatal("Expected at least one execution step") + } + + stepContent := strings.Join(steps[0], "\n") + + // Check that custom mounts are included + if !strings.Contains(stepContent, "--mount /host/data:/data:ro") { + t.Error("Expected command to contain custom mount '--mount /host/data:/data:ro'") + } + + if !strings.Contains(stepContent, "--mount /usr/local/bin/custom-tool:/usr/local/bin/custom-tool:ro") { + t.Error("Expected command to contain custom mount '--mount /usr/local/bin/custom-tool:/usr/local/bin/custom-tool:ro'") + } + + // Verify standard mounts are still present + if !strings.Contains(stepContent, "--mount /tmp:/tmp:rw") { + t.Error("Expected command to still contain standard mount '--mount /tmp:/tmp:rw'") + } + }) + + t.Run("no custom mounts when not specified", func(t *testing.T) { + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + engine := NewCopilotEngine() + steps := engine.GetExecutionSteps(workflowData, "test.log") + + if len(steps) == 0 { + t.Fatal("Expected at least one execution step") + } + + stepContent := strings.Join(steps[0], "\n") + + // Verify standard mounts are present + if !strings.Contains(stepContent, "--mount /tmp:/tmp:rw") { + t.Error("Expected command to contain standard mount '--mount /tmp:/tmp:rw'") + } + + // Custom mount should not be present + if strings.Contains(stepContent, "--mount /host/data:/data:ro") { + t.Error("Did not expect custom mount in output when not configured") + } + }) + + t.Run("custom mounts are sorted alphabetically", func(t *testing.T) { + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + Mounts: []string{ + "/var/log:/logs:ro", + "/data:/data:rw", + "/usr/bin/tool:/usr/bin/tool:ro", + "/etc/config:/etc/config:ro", + }, + }, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + engine := NewCopilotEngine() + steps := engine.GetExecutionSteps(workflowData, "test.log") + + if len(steps) == 0 { + t.Fatal("Expected at least one execution step") + } + + stepContent := strings.Join(steps[0], "\n") + + // Find the positions of each mount in the output + dataPos := strings.Index(stepContent, "--mount /data:/data:rw") + etcPos := strings.Index(stepContent, "--mount /etc/config:/etc/config:ro") + usrPos := strings.Index(stepContent, "--mount /usr/bin/tool:/usr/bin/tool:ro") + varPos := strings.Index(stepContent, "--mount /var/log:/logs:ro") + + // Verify all mounts are present + if dataPos == -1 { + t.Error("Expected to find mount '/data:/data:rw'") + } + if etcPos == -1 { + t.Error("Expected to find mount '/etc/config:/etc/config:ro'") + } + if usrPos == -1 { + t.Error("Expected to find mount '/usr/bin/tool:/usr/bin/tool:ro'") + } + if varPos == -1 { + t.Error("Expected to find mount '/var/log:/logs:ro'") + } + + // Verify mounts are in alphabetical order + // Expected order: /data, /etc, /usr, /var + if dataPos != -1 && etcPos != -1 && dataPos >= etcPos { + t.Error("Expected '/data:/data:rw' to appear before '/etc/config:/etc/config:ro'") + } + if etcPos != -1 && usrPos != -1 && etcPos >= usrPos { + t.Error("Expected '/etc/config:/etc/config:ro' to appear before '/usr/bin/tool:/usr/bin/tool:ro'") + } + if usrPos != -1 && varPos != -1 && usrPos >= varPos { + t.Error("Expected '/usr/bin/tool:/usr/bin/tool:ro' to appear before '/var/log:/logs:ro'") + } + }) +}