diff --git a/.github/workflows/agentics-maintenance.yml b/.github/workflows/agentics-maintenance.yml index 6eb35db87c3..1553b40cedc 100644 --- a/.github/workflows/agentics-maintenance.yml +++ b/.github/workflows/agentics-maintenance.yml @@ -350,7 +350,7 @@ jobs: - name: Compile workflows run: | - ./gh-aw compile --validate --verbose + ./gh-aw compile --validate --validate-images --verbose echo "✓ All workflows compiled successfully" - name: Setup Scripts diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 051f110acd4..500007af209 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -287,6 +287,7 @@ Examples: noCheckUpdate, _ := cmd.Flags().GetBool("no-check-update") scheduleSeed, _ := cmd.Flags().GetString("schedule-seed") safeUpdate, _ := cmd.Flags().GetBool("safe-update") + validateImages, _ := cmd.Flags().GetBool("validate-images") priorManifestFile, _ := cmd.Flags().GetString("prior-manifest-file") verbose, _ := cmd.Flags().GetBool("verbose") if err := validateEngine(engineOverride); err != nil { @@ -343,6 +344,7 @@ Examples: FailFast: failFast, ScheduleSeed: scheduleSeed, SafeUpdate: safeUpdate, + ValidateImages: validateImages, PriorManifestFile: priorManifestFile, } if _, err := cli.CompileWorkflows(cmd.Context(), config); err != nil { @@ -691,6 +693,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all compileCmd.Flags().Bool("no-check-update", false, "Skip checking for gh-aw updates") compileCmd.Flags().String("schedule-seed", "", "Override the repository slug (owner/repo) used as seed for fuzzy schedule scattering (e.g. 'github/gh-aw'). Bypasses git remote detection entirely. Use this when your git remote is not named 'origin' and you have multiple remotes configured") compileCmd.Flags().Bool("safe-update", false, "Force-enable safe update mode independently of strict mode. Safe update mode is normally equivalent to strict mode: it emits a warning prompt when compilations introduce new restricted secrets or unapproved action additions/removals not present in the existing gh-aw-manifest. Use this flag to enable safe update enforcement on a workflow that has strict: false in its frontmatter") + compileCmd.Flags().Bool("validate-images", false, "Require Docker to be available for container image validation. Without this flag, container image validation is silently skipped when Docker is not installed or the daemon is not running") compileCmd.Flags().String("prior-manifest-file", "", "Path to a JSON file containing pre-cached gh-aw-manifests (map[lockFile]*GHAWManifest); used by the MCP server to supply a tamper-proof manifest baseline captured at startup") if err := compileCmd.Flags().MarkHidden("prior-manifest-file"); err != nil { // Non-fatal: flag is registered even if MarkHidden fails diff --git a/pkg/cli/compile_compiler_setup.go b/pkg/cli/compile_compiler_setup.go index 69c5ec70ce7..706425a3f0f 100644 --- a/pkg/cli/compile_compiler_setup.go +++ b/pkg/cli/compile_compiler_setup.go @@ -163,6 +163,13 @@ func configureCompilerFlags(compiler *workflow.Compiler, config CompileConfig) { compileCompilerSetupLog.Print("Safe update mode force-enabled via --safe-update flag: compilations introducing new restricted secrets or unapproved action additions/removals will emit a warning prompt requesting agent review and a PR security note") } + // Set require docker flag: when set, container image validation fails instead of + // silently skipping when Docker is not available. + compiler.SetRequireDocker(config.ValidateImages) + if config.ValidateImages { + compileCompilerSetupLog.Print("Container image validation requires Docker (--validate-images flag)") + } + // Load pre-cached manifests from file (written by MCP server at startup). // These take precedence over git HEAD / filesystem reads for safe update enforcement. if config.PriorManifestFile != "" { diff --git a/pkg/cli/compile_config.go b/pkg/cli/compile_config.go index c0baf4236d5..39ad3962f90 100644 --- a/pkg/cli/compile_config.go +++ b/pkg/cli/compile_config.go @@ -30,6 +30,7 @@ type CompileConfig struct { FailFast bool // Stop at first error instead of collecting all errors ScheduleSeed string // Override repository slug used for fuzzy schedule scattering (e.g. owner/repo) SafeUpdate bool // Force-enable safe update mode regardless of strict mode setting. Safe update mode is normally equivalent to strict mode (active whenever strict mode is active). + ValidateImages bool // Require Docker to be available for container image validation (fail instead of skipping when Docker is unavailable) PriorManifestFile string // Path to a JSON file containing pre-cached manifests (map[lockFile]*GHAWManifest) collected at MCP server startup; takes precedence over git HEAD / filesystem reads for safe update enforcement } diff --git a/pkg/cli/validate_command.go b/pkg/cli/validate_command.go index 01415dc9174..28cb6c81f59 100644 --- a/pkg/cli/validate_command.go +++ b/pkg/cli/validate_command.go @@ -41,6 +41,7 @@ Examples: failFast, _ := cmd.Flags().GetBool("fail-fast") stats, _ := cmd.Flags().GetBool("stats") noCheckUpdate, _ := cmd.Flags().GetBool("no-check-update") + validateImages, _ := cmd.Flags().GetBool("validate-images") verbose, _ := cmd.Flags().GetBool("verbose") if err := validateEngine(engineOverride); err != nil { @@ -66,6 +67,7 @@ Examples: JSONOutput: jsonOutput, FailFast: failFast, Stats: stats, + ValidateImages: validateImages, } if _, err := CompileWorkflows(context.Background(), config); err != nil { return err @@ -79,6 +81,7 @@ Examples: cmd.Flags().Bool("strict", false, "Override frontmatter to enforce strict mode validation for all workflows (enforces action pinning, network config, safe-outputs, refuses write permissions and deprecated fields). Note: Workflows default to strict mode unless frontmatter sets strict: false") cmd.Flags().BoolP("json", "j", false, "Output results in JSON format") cmd.Flags().Bool("fail-fast", false, "Stop at the first validation error instead of collecting all errors") + cmd.Flags().Bool("validate-images", false, "Require Docker to be available for container image validation. Without this flag, container image validation is silently skipped when Docker is not installed or the daemon is not running") cmd.Flags().Bool("stats", false, "Display statistics table sorted by workflow file size (shows jobs, steps, scripts, and shells)") cmd.Flags().Bool("no-check-update", false, "Skip checking for gh-aw updates") diff --git a/pkg/cli/validate_command_test.go b/pkg/cli/validate_command_test.go index 7ecd8a288b0..bd2f7b0d02c 100644 --- a/pkg/cli/validate_command_test.go +++ b/pkg/cli/validate_command_test.go @@ -28,4 +28,5 @@ func TestNewValidateCommand(t *testing.T) { require.NotNil(t, cmd.Flags().Lookup("fail-fast"), "validate command should have a --fail-fast flag") require.NotNil(t, cmd.Flags().Lookup("stats"), "validate command should have a --stats flag") require.NotNil(t, cmd.Flags().Lookup("no-check-update"), "validate command should have a --no-check-update flag") + require.NotNil(t, cmd.Flags().Lookup("validate-images"), "validate command should have a --validate-images flag") } diff --git a/pkg/workflow/argument_injection_test.go b/pkg/workflow/argument_injection_test.go index 7f652e82bfb..e888569cec8 100644 --- a/pkg/workflow/argument_injection_test.go +++ b/pkg/workflow/argument_injection_test.go @@ -110,7 +110,7 @@ func TestValidateDockerImage_RejectsHyphenPrefix(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateDockerImage(tt.image, false) + err := validateDockerImage(tt.image, false, false) if err == nil { t.Errorf("expected error for image %q but got none", tt.image) return diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 746217641a8..fa86b8942da 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -89,6 +89,7 @@ type Compiler struct { skipHeader bool // If true, skip ASCII art header in generated YAML (for Wasm/editor mode) inlinePrompt bool // If true, inline markdown content in YAML instead of using runtime-import macros (for Wasm builds) priorManifests map[string]*GHAWManifest // Pre-cached manifests keyed by lock file path; takes precedence over git HEAD / filesystem reads + requireDocker bool // If true, fail validation when Docker is not available instead of silently skipping } // NewCompiler creates a new workflow compiler with functional options. @@ -145,6 +146,13 @@ func (c *Compiler) SetSkipValidation(skip bool) { c.skipValidation = skip } +// SetRequireDocker configures whether Docker must be available for container image validation. +// When true, validation fails with an error if Docker is not installed or the daemon is not running. +// When false (default), validation is silently skipped when Docker is unavailable. +func (c *Compiler) SetRequireDocker(require bool) { + c.requireDocker = require +} + // SetQuiet configures whether to suppress success messages (for interactive mode) func (c *Compiler) SetQuiet(quiet bool) { c.quiet = quiet diff --git a/pkg/workflow/docker_validation.go b/pkg/workflow/docker_validation.go index b755875e669..75e017036de 100644 --- a/pkg/workflow/docker_validation.go +++ b/pkg/workflow/docker_validation.go @@ -12,15 +12,18 @@ // // - validateDockerImage() - Validates a single Docker image exists and is accessible // -// # Validation Pattern: Warning vs Error +// # Validation Pattern: Graceful Degradation // -// Docker image validation returns errors for all failure cases. The caller -// (validateContainerImages) collects these and surfaces them as compiler warnings: -// - If Docker is not installed, returns an error -// - If the Docker daemon is not running, returns an error (with fast timeout check) +// Docker image validation degrades gracefully when Docker is unavailable. +// The caller (validateContainerImages) collects errors and surfaces them as compiler warnings: +// - If Docker is not installed, validation is silently skipped (debug log only) +// - If the Docker daemon is not running, validation is silently skipped (debug log only) // - If an image cannot be pulled due to authentication (private repo), validation passes // - If an image truly doesn't exist, returns an error -// - Verbose mode provides detailed validation feedback +// - Detailed validation logging is available via debug logging when enabled +// +// This design ensures that `gh aw compile --validate` does not require Docker +// at compile time. Docker availability is a runtime concern. // // # When to Add Validation Here // @@ -84,9 +87,13 @@ func isDockerDaemonRunning() bool { } // validateDockerImage checks if a Docker image exists and is accessible. -// Returns an error if Docker is not installed, the daemon is not running, -// or the image cannot be found. The caller treats these as warnings. -func validateDockerImage(image string, verbose bool) error { +// When Docker is not installed or the daemon is not running, validation is +// silently skipped (returns nil) so that compile-time validation does not +// depend on Docker availability. If requireDocker is true, returns an error +// instead of skipping when Docker is unavailable. Returns an error only when +// Docker is available and the image cannot be found. The caller treats these +// as warnings. +func validateDockerImage(image string, verbose bool, requireDocker bool) error { dockerValidationLog.Printf("Validating Docker image: %s", image) // Reject names starting with '-' to prevent argument injection @@ -94,19 +101,30 @@ func validateDockerImage(image string, verbose bool) error { return fmt.Errorf("container image name '%s' is invalid: names must not start with '-'", image) } - // Check if docker CLI is available on PATH + // Check if docker CLI is available on PATH. + // If Docker is not installed, skip validation silently — compile is a source + // transformation step and should not require Docker at authoring time. + // When requireDocker is true, return an error instead of skipping. _, err := exec.LookPath("docker") if err != nil { - dockerValidationLog.Print("Docker not installed, cannot validate image") - return fmt.Errorf("docker not installed - could not validate container image '%s'. To use container-based MCP servers, install Docker (https://docs.docker.com/get-started/get-docker)", image) + if requireDocker { + return fmt.Errorf("docker not installed - could not validate container image '%s'. Install Docker or omit the --validate-images flag to skip container image validation", image) + } + dockerValidationLog.Print("Docker not installed, skipping container image validation") + return nil } // Check if Docker daemon is actually running (cached check with short timeout). - // This prevents multi-minute hangs when Docker Desktop is installed but not running, - // which is common on macOS development machines. + // If the daemon is not running (common on CI runners like ubuntu-slim, or when + // Docker Desktop is stopped), skip validation silently instead of emitting a + // warning. Image accessibility is a runtime concern, not a compile-time one. + // When requireDocker is true, return an error instead of skipping. if !isDockerDaemonRunning() { - dockerValidationLog.Print("Docker daemon not running, cannot validate image") - return fmt.Errorf("docker daemon not running - could not validate container image '%s'. Start Docker Desktop or the Docker daemon", image) + if requireDocker { + return fmt.Errorf("docker daemon not running - could not validate container image '%s'. Start the Docker daemon or omit the --validate-images flag to skip container image validation", image) + } + dockerValidationLog.Print("Docker daemon not running, skipping container image validation") + return nil } // Try to inspect the image (will succeed if image exists locally) diff --git a/pkg/workflow/docker_validation_graceful_test.go b/pkg/workflow/docker_validation_graceful_test.go new file mode 100644 index 00000000000..0384e22e6db --- /dev/null +++ b/pkg/workflow/docker_validation_graceful_test.go @@ -0,0 +1,114 @@ +//go:build !integration && !js && !wasm + +package workflow + +import ( + "os/exec" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestValidateDockerImage_SkipsWhenDockerUnavailable verifies that +// validateDockerImage degrades gracefully (returns nil) when Docker +// is not installed or the daemon is not running, instead of returning +// an error that surfaces as a spurious warning. +func TestValidateDockerImage_SkipsWhenDockerUnavailable(t *testing.T) { + // If docker is not installed or daemon not running, validation should + // silently pass — no error, no warning. + if _, lookErr := exec.LookPath("docker"); lookErr != nil { + err := validateDockerImage("ghcr.io/some/image:latest", false, false) + assert.NoError(t, err, "should silently skip when Docker is not installed") + return + } + if !isDockerDaemonRunning() { + err := validateDockerImage("ghcr.io/some/image:latest", false, false) + assert.NoError(t, err, "should silently skip when Docker daemon is not running") + return + } + + t.Skip("Docker is available — graceful degradation path not exercised") +} + +// TestValidateDockerImage_StillRejectsHyphenWithoutDocker verifies that +// the argument injection check still works even when Docker is unavailable. +func TestValidateDockerImage_StillRejectsHyphenWithoutDocker(t *testing.T) { + // The hyphen-prefix guard runs before the Docker availability check, + // so it should always reject invalid names regardless of Docker state. + err := validateDockerImage("-malicious", false, false) + require.Error(t, err, "should reject image names starting with hyphen regardless of Docker availability") + assert.Contains(t, err.Error(), "names must not start with '-'", + "error should explain why the name is invalid") +} + +// TestValidateContainerImages_NoWarningWithoutDocker verifies that +// validateContainerImages does not produce errors when Docker is unavailable +// and the workflow references container-based tools. +func TestValidateContainerImages_NoWarningWithoutDocker(t *testing.T) { + if _, lookErr := exec.LookPath("docker"); lookErr == nil && isDockerDaemonRunning() { + t.Skip("Docker is available — graceful degradation path not exercised") + } + + workflowData := &WorkflowData{ + Tools: map[string]any{ + "serena": map[string]any{ + "container": "ghcr.io/github/serena-mcp-server", + "version": "latest", + }, + }, + } + + compiler := NewCompiler() + err := compiler.validateContainerImages(workflowData) + assert.NoError(t, err, "container image validation should silently pass when Docker is unavailable") +} + +// TestValidateDockerImage_RequireDockerFailsWhenUnavailable verifies that +// when requireDocker is true, validateDockerImage returns an error instead +// of silently skipping when Docker is not installed or the daemon is not running. +func TestValidateDockerImage_RequireDockerFailsWhenUnavailable(t *testing.T) { + if _, lookErr := exec.LookPath("docker"); lookErr != nil { + err := validateDockerImage("ghcr.io/some/image:latest", false, true) + require.Error(t, err, "should fail when Docker is not installed and requireDocker is true") + assert.Contains(t, err.Error(), "docker not installed", + "error should mention Docker is not installed") + assert.Contains(t, err.Error(), "--validate-images", + "error should mention the --validate-images flag") + return + } + if !isDockerDaemonRunning() { + err := validateDockerImage("ghcr.io/some/image:latest", false, true) + require.Error(t, err, "should fail when Docker daemon is not running and requireDocker is true") + assert.Contains(t, err.Error(), "docker daemon not running", + "error should mention Docker daemon is not running") + assert.Contains(t, err.Error(), "--validate-images", + "error should mention the --validate-images flag") + return + } + + t.Skip("Docker is available — requireDocker failure path not exercised") +} + +// TestValidateContainerImages_RequireDockerFailsWhenUnavailable verifies that +// when requireDocker is set on the compiler, validateContainerImages returns +// an error when Docker is unavailable. +func TestValidateContainerImages_RequireDockerFailsWhenUnavailable(t *testing.T) { + if _, lookErr := exec.LookPath("docker"); lookErr == nil && isDockerDaemonRunning() { + t.Skip("Docker is available — requireDocker failure path not exercised") + } + + workflowData := &WorkflowData{ + Tools: map[string]any{ + "serena": map[string]any{ + "container": "ghcr.io/github/serena-mcp-server", + "version": "latest", + }, + }, + } + + compiler := NewCompiler() + compiler.SetRequireDocker(true) + err := compiler.validateContainerImages(workflowData) + require.Error(t, err, "container image validation should fail when Docker is unavailable and requireDocker is true") +} diff --git a/pkg/workflow/docker_validation_wasm.go b/pkg/workflow/docker_validation_wasm.go index 86926b543dd..41d0df454c6 100644 --- a/pkg/workflow/docker_validation_wasm.go +++ b/pkg/workflow/docker_validation_wasm.go @@ -2,6 +2,6 @@ package workflow -func validateDockerImage(image string, verbose bool) error { +func validateDockerImage(image string, verbose bool, requireDocker bool) error { return nil } diff --git a/pkg/workflow/maintenance_workflow.go b/pkg/workflow/maintenance_workflow.go index 0ef83099066..76f9238eab9 100644 --- a/pkg/workflow/maintenance_workflow.go +++ b/pkg/workflow/maintenance_workflow.go @@ -598,7 +598,7 @@ jobs: yaml.WriteString(generateInstallCLISteps(actionMode, version, actionTag, resolver)) yaml.WriteString(` - name: Compile workflows run: | - ` + getCLICmdPrefix(actionMode) + ` compile --validate --verbose + ` + getCLICmdPrefix(actionMode) + ` compile --validate --validate-images --verbose echo "✓ All workflows compiled successfully" - name: Setup Scripts diff --git a/pkg/workflow/runtime_validation.go b/pkg/workflow/runtime_validation.go index 468e00ac518..a263fba7f0b 100644 --- a/pkg/workflow/runtime_validation.go +++ b/pkg/workflow/runtime_validation.go @@ -211,7 +211,7 @@ func (c *Compiler) validateContainerImages(workflowData *WorkflowData) error { } // Validate the container image exists using docker - if err := validateDockerImage(containerImage, c.verbose); err != nil { + if err := validateDockerImage(containerImage, c.verbose, c.requireDocker); err != nil { errors = append(errors, fmt.Sprintf("tool '%s': %v", toolName, err)) } else if c.verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("✓ Container image validated: "+containerImage)) diff --git a/pkg/workflow/validation_test.go b/pkg/workflow/validation_test.go index 882e0b6615d..0fa96930b1e 100644 --- a/pkg/workflow/validation_test.go +++ b/pkg/workflow/validation_test.go @@ -116,7 +116,7 @@ func TestValidateDockerImage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateDockerImage(tt.image, false) + err := validateDockerImage(tt.image, false, false) if tt.expectError && err == nil { t.Error("expected error but got none")