Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions docs/src/content/docs/reference/frontmatter.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion pkg/cli/compile_orchestrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -17,7 +18,8 @@ var compileOrchestratorLog = logger.New("cli:compile_orchestrator")
var compileUpdateContainerPins = updateContainerPins

// CompileWorkflows compiles workflows based on the provided configuration

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pkg/cli/compile_orchestrator.go:L20: yagni: workspace strict config loading embedded in CLI orchestrator. Reuse compiler-level strict resolution only; remove applyWorkspaceStrictMode from CLI.

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)

Expand Down Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions pkg/cli/compile_permissions_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
5 changes: 5 additions & 0 deletions pkg/parser/schemas/repo_config_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 18 additions & 0 deletions pkg/workflow/compiler_repo_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 5 additions & 1 deletion pkg/workflow/compiler_yaml_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Comment on lines +17 to +20
if strictVal, exists := frontmatter["strict"]; exists {
if strictBool, ok := strictVal.(bool); ok {
compilerYAMLPolicyLog.Printf("Strict mode resolved from frontmatter strict field: %v", strictBool)
Expand Down
10 changes: 2 additions & 8 deletions pkg/workflow/pull_request_target_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines 62 to 64
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.
Expand Down Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions pkg/workflow/pull_request_target_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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:
Expand All @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions pkg/workflow/repo_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"`
Expand All @@ -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)
Expand Down
17 changes: 17 additions & 0 deletions pkg/workflow/repo_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
Expand Down
Loading