From 9923c1083b379afa6b16949cdefe40cf173f738a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 7 Sep 2026 15:42:15 +0000 Subject: [PATCH] Add workspace strict mode configuration Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../src/content/docs/reference/frontmatter.md | 2 ++ pkg/cli/compile_orchestrator.go | 16 ++++++++++++- .../compile_permissions_integration_test.go | 24 +++++++++++++++++++ pkg/parser/schemas/repo_config_schema.json | 5 ++++ pkg/workflow/compiler_repo_config_test.go | 18 ++++++++++++++ pkg/workflow/compiler_yaml_policy.go | 6 ++++- .../pull_request_target_validation.go | 10 ++------ .../pull_request_target_validation_test.go | 8 ++++--- pkg/workflow/repo_config.go | 6 +++++ pkg/workflow/repo_config_test.go | 17 +++++++++++++ 10 files changed, 99 insertions(+), 13 deletions(-) diff --git a/docs/src/content/docs/reference/frontmatter.md b/docs/src/content/docs/reference/frontmatter.md index a79930d2255..ae02cc780ac 100644 --- a/docs/src/content/docs/reference/frontmatter.md +++ b/docs/src/content/docs/reference/frontmatter.md @@ -771,6 +771,8 @@ strict: false # Disable enhanced security validation for development/testing Workflows compiled with `strict: false` cannot run on public repositories. The workflow fails at runtime with an error message prompting recompilation with strict mode. +To prevent workflows from opting out, set `"strict": true` at the top level of `.github/workflows/aw.json`. This enforces strict mode for every `gh aw compile` invocation in the repository. The repository setting only accepts `true`; `"strict": false` is invalid. + See [Network Permissions - Strict Mode Validation](/gh-aw/reference/network/#strict-mode-validation) for details on network validation and [CLI Commands](/gh-aw/setup/cli/#compile) for compilation options. ## Learn More diff --git a/pkg/cli/compile_orchestrator.go b/pkg/cli/compile_orchestrator.go index 3066b337497..583233e72a7 100644 --- a/pkg/cli/compile_orchestrator.go +++ b/pkg/cli/compile_orchestrator.go @@ -9,6 +9,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/workflow" ) @@ -17,7 +18,8 @@ var compileOrchestratorLog = logger.New("cli:compile_orchestrator") var compileUpdateContainerPins = updateContainerPins // CompileWorkflows compiles workflows based on the provided configuration -func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.WorkflowData, error) { +func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.WorkflowData, error) { //nolint:largefunc // Existing compilation lifecycle remains centralized. + config = applyWorkspaceStrictMode(config) compileOrchestratorLog.Printf("Starting workflow compilation: files=%d, validate=%v, watch=%v, noEmit=%v", len(config.MarkdownFiles), config.Validate, config.Watch, config.NoEmit) @@ -155,6 +157,18 @@ func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.Wo return compileAllFilesInDirectory(ctx, compiler, config, workflowDir, stats, &validationResults) } +func applyWorkspaceStrictMode(config CompileConfig) CompileConfig { + gitRoot, err := gitutil.FindGitRoot() + if err != nil { + gitRoot = "" + } + repoConfig, err := workflow.LoadRepoConfig(gitRoot) + if err == nil && repoConfig.Strict { + config.Strict = true + } + return config +} + func maybeForceRefreshContainerPins(ctx context.Context, config CompileConfig, workflowDir string) error { if !config.ForceRefreshContainerPins { return nil diff --git a/pkg/cli/compile_permissions_integration_test.go b/pkg/cli/compile_permissions_integration_test.go index c197b58c5a0..dbec739f5eb 100644 --- a/pkg/cli/compile_permissions_integration_test.go +++ b/pkg/cli/compile_permissions_integration_test.go @@ -113,3 +113,27 @@ engine: copilot assert.Contains(t, string(output), "strict mode: write permission 'contents: write' is not allowed", "compile without --strict should enable strict validation by default") } + +func TestCompileWorkspaceStrictModeOverridesFrontmatter(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + require.NoError(t, os.WriteFile(filepath.Join(setup.workflowsDir, "aw.json"), []byte(`{"strict":true}`), 0644)) + workflowPath := filepath.Join(setup.workflowsDir, "workspace-strict.md") + workflow := `--- +on: push +strict: false +permissions: + contents: write +engine: copilot +--- + +# Workspace strict +` + require.NoError(t, os.WriteFile(workflowPath, []byte(workflow), 0644)) + + cmd := exec.Command(setup.binaryPath, "compile", workflowPath) + output, err := cmd.CombinedOutput() + require.Error(t, err, "workspace strict mode should reject a workflow opt-out") + assert.Contains(t, string(output), "strict mode: write permission 'contents: write' is not allowed") +} diff --git a/pkg/parser/schemas/repo_config_schema.json b/pkg/parser/schemas/repo_config_schema.json index df51870f147..69ff35cf78a 100644 --- a/pkg/parser/schemas/repo_config_schema.json +++ b/pkg/parser/schemas/repo_config_schema.json @@ -52,6 +52,11 @@ "description": "Enable or disable the builtin centralized /help slash command handler. Defaults to true when omitted. Set to false to disable.", "type": "boolean" }, + "strict": { + "description": "Enforce strict mode for every workflow compiled in this repository. Only true is accepted so workflows cannot opt out with frontmatter.", + "type": "boolean", + "const": true + }, "utc": { "description": "Project home UTC offset used when rendering local times in CLI output. Must be a numeric UTC offset such as +00:00 or -08:00.", "type": "string", diff --git a/pkg/workflow/compiler_repo_config_test.go b/pkg/workflow/compiler_repo_config_test.go index 6198064befa..995f9816879 100644 --- a/pkg/workflow/compiler_repo_config_test.go +++ b/pkg/workflow/compiler_repo_config_test.go @@ -93,6 +93,24 @@ func TestCompilerLoadRepoConfig_CachesResult(t *testing.T) { } } +func TestCompilerRepoConfigStrictOverridesFrontmatter(t *testing.T) { + gitRoot := t.TempDir() + workflowsDir := filepath.Join(gitRoot, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0o755); err != nil { + t.Fatalf("Failed to create workflows directory: %v", err) + } + if err := os.WriteFile(filepath.Join(workflowsDir, "aw.json"), []byte(`{"strict":true}`), 0o600); err != nil { + t.Fatalf("Failed to write aw.json: %v", err) + } + + compiler := NewCompiler() + compiler.gitRoot = gitRoot + + if !compiler.effectiveStrictMode(map[string]any{"strict": false}) { + t.Fatal("Expected repository strict mode to override workflow frontmatter") + } +} + func TestCompilerLoadRepoConfig_CachesError(t *testing.T) { gitRoot := t.TempDir() workflowsDir := filepath.Join(gitRoot, ".github", "workflows") diff --git a/pkg/workflow/compiler_yaml_policy.go b/pkg/workflow/compiler_yaml_policy.go index 4bee0b2dcc2..bf5829e0bd7 100644 --- a/pkg/workflow/compiler_yaml_policy.go +++ b/pkg/workflow/compiler_yaml_policy.go @@ -5,7 +5,7 @@ import "github.com/github/gh-aw/pkg/logger" var compilerYAMLPolicyLog = logger.New("workflow:compiler_yaml_policy") // effectiveStrictMode computes the effective strict mode for a workflow. -// Priority: CLI flag (c.strictMode) > frontmatter strict field > default (true). +// Priority: CLI flag or repository config > frontmatter strict field > default (true). // This should be used when emitting metadata/env vars to correctly reflect the // workflow's strictness as inferred from the source (frontmatter). func (c *Compiler) effectiveStrictMode(frontmatter map[string]any) bool { @@ -14,6 +14,10 @@ func (c *Compiler) effectiveStrictMode(frontmatter map[string]any) bool { compilerYAMLPolicyLog.Print("Strict mode enabled by CLI flag") return true } + if repoConfig, err := c.loadRepoConfig(); err == nil && repoConfig.Strict { + compilerYAMLPolicyLog.Print("Strict mode enforced by repository config") + return true + } if strictVal, exists := frontmatter["strict"]; exists { if strictBool, ok := strictVal.(bool); ok { compilerYAMLPolicyLog.Printf("Strict mode resolved from frontmatter strict field: %v", strictBool) diff --git a/pkg/workflow/pull_request_target_validation.go b/pkg/workflow/pull_request_target_validation.go index d31a4209e37..6daabe6b631 100644 --- a/pkg/workflow/pull_request_target_validation.go +++ b/pkg/workflow/pull_request_target_validation.go @@ -62,7 +62,7 @@ var pullRequestTargetGitHubExpressionPattern = regexp.MustCompile(`^\$\{\{\s*([^ // When the workflow frontmatter sets strict: false, effectiveStrictMode is lowered so the // dangerous-trigger strict-only warning is skipped; the insecure-checkout check still runs // and emits a non-strict warning when checkout is not explicitly disabled. -func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, markdownPath string) error { +func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, markdownPath string) error { //nolint:largefunc // Trigger checks share parsed state and diagnostics. // Fast path: skip expensive YAML parsing when the On field cannot possibly contain // a pull_request_target trigger. This avoids yaml.Unmarshal on every // validateWorkflowData call for the common case of non-pull_request_target workflows. @@ -98,13 +98,7 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, return nil } - effectiveStrictMode := c.strictMode - if workflowData.RawFrontmatter != nil { - if strictBool, ok := workflowData.RawFrontmatter["strict"].(bool); ok && !strictBool { - pullRequestTargetLog.Print("Frontmatter strict: false detected, disabling strict mode error for pull_request_target validation") - effectiveStrictMode = false - } - } + effectiveStrictMode := c.effectiveStrictMode(workflowData.RawFrontmatter) // In strict mode, always emit a warning that pull_request_target is a very dangerous trigger, // regardless of whether checkout is disabled. The workflow still runs with full write diff --git a/pkg/workflow/pull_request_target_validation_test.go b/pkg/workflow/pull_request_target_validation_test.go index b37b5232146..a38a72db7eb 100644 --- a/pkg/workflow/pull_request_target_validation_test.go +++ b/pkg/workflow/pull_request_target_validation_test.go @@ -77,6 +77,7 @@ Test workflow content.`, { name: "pull_request_target with trusted checkout - non-strict - no warnings no error", frontmatter: `--- +strict: false on: pull_request_target: types: [opened] @@ -341,7 +342,7 @@ Test workflow content.`, warningCount: 1, // dangerous-trigger warning }, { - name: "pull_request_target with no checkout key - strict CLI + frontmatter strict false - insecure checkout warning", + name: "pull_request_target with no checkout key - strict CLI overrides frontmatter strict false", frontmatter: `--- strict: false on: @@ -358,9 +359,10 @@ permissions: Test workflow content.`, filename: "prt-checkout-enabled-strict-frontmatter-opt-out.md", strictMode: true, - expectError: false, + expectError: true, expectWarning: true, - warningCount: 1, // strict: false lowers to non-strict mode (skips dangerous-trigger warning), but insecure-checkout warning still emits + errorContains: "pull_request_target trigger with checkout enabled is extremely insecure", + warningCount: 1, // dangerous-trigger warning }, { name: "pull_request trigger (not target) - strict - no diagnostic", diff --git a/pkg/workflow/repo_config.go b/pkg/workflow/repo_config.go index 3cf26113f36..161e71ab764 100644 --- a/pkg/workflow/repo_config.go +++ b/pkg/workflow/repo_config.go @@ -143,6 +143,10 @@ func (m *MaintenanceConfig) IsJobDisabled(jobName string) bool { // RepoConfig is the parsed representation of aw.json. type RepoConfig struct { + // Strict enforces strict mode for every workflow compiled in the repository. + // The schema only permits true when this field is present. + Strict bool + // GHES enables GitHub Enterprise Server compatibility mode. // When true, the compiler uses artifact action versions supported by GHES. GHES bool @@ -225,6 +229,7 @@ func (r *RepoConfig) UnmarshalJSON(data []byte) error { //nolint:largefunc // Po // Use an intermediate struct with json.RawMessage to defer maintenance and // auto_upgrade parsing. var raw struct { + Strict bool `json:"strict,omitempty"` GHES bool `json:"ghes,omitempty"` HelpCommand *bool `json:"help_command,omitempty"` // nil = use default (enabled) UTC string `json:"utc,omitempty"` @@ -237,6 +242,7 @@ func (r *RepoConfig) UnmarshalJSON(data []byte) error { //nolint:largefunc // Po return err } + r.Strict = raw.Strict r.GHES = raw.GHES r.HelpCommand = raw.HelpCommand r.UTC = strings.TrimSpace(raw.UTC) diff --git a/pkg/workflow/repo_config_test.go b/pkg/workflow/repo_config_test.go index 9898089e85e..abc6b8d58f1 100644 --- a/pkg/workflow/repo_config_test.go +++ b/pkg/workflow/repo_config_test.go @@ -62,6 +62,23 @@ func TestLoadRepoConfig_EmptyObject(t *testing.T) { assert.True(t, cfg.IsHelpCommandEnabled(), "help command should be enabled by default") } +func TestLoadRepoConfig_StrictTrue(t *testing.T) { + dir := t.TempDir() + writeAWJSON(t, dir, `{"strict": true}`) + + cfg, err := LoadRepoConfig(dir) + require.NoError(t, err, "aw.json should accept strict: true") + assert.True(t, cfg.Strict, "strict mode should be enforced") +} + +func TestLoadRepoConfig_StrictFalseRejected(t *testing.T) { + dir := t.TempDir() + writeAWJSON(t, dir, `{"strict": false}`) + + _, err := LoadRepoConfig(dir) + assert.Error(t, err, "aw.json should reject strict: false") +} + func TestLoadRepoConfig_HelpCommandFalse(t *testing.T) { dir := t.TempDir() writeAWJSON(t, dir, `{"help_command": false}`)