🔒 Implement markdown security scanner for workflows (#15208)

* reject dodgy workflows

* Update pkg/workflow/markdown_security_scanner.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update pkg/workflow/markdown_security_scanner.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update pkg/workflow/markdown_security_scanner.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update pkg/workflow/compiler_orchestrator_engine.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update pkg/workflow/markdown_security_scanner.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* reject dodgy workflows

* reject dodgy workflows

* Initial plan (#15225)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

* add AddOptions

* fix tests

* fix code

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>

Author: 3 people

docs/src/content/docs/reference/markdown.md

@@ -98,6 +98,10 @@ Agentic markdown supports GitHub Actions expression substitutions and conditiona
 
 This design enables rapid iteration on AI instructions while maintaining strict compilation requirements for security-sensitive frontmatter configuration. See [Editing Workflows](/gh-aw/guides/editing-workflows/) for complete guidance on when recompilation is needed versus when you can edit directly.
 
+## Security Scanning
+
+The markdown body of workflows (excluding frontmatter) is automatically scanned for malicious content when added via `gh aw add`, during trial mode, and at compile time for imported files. The scanner rejects workflows containing: Unicode abuse (zero-width characters, bidirectional overrides), hidden content (suspicious HTML comments, CSS-hidden elements), obfuscated links (data URIs, `javascript:` URLs, IP-based URLs, URL shorteners), dangerous HTML tags (`<script>`, `<iframe>`, `<object>`, `<form>`, event handlers), embedded executable content (SVG scripts, executable MIME data URIs), and social engineering patterns (prompt injection, base64-encoded commands, pipe-to-shell patterns). These checks cannot be overridden.
+
 ## Related Documentation
 
 - [Editing Workflows](/gh-aw/guides/editing-workflows/) - When to recompile vs edit directly

pkg/cli/add_command.go

@@ -109,7 +109,10 @@ func TestAddWorkflows(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := AddWorkflows(tt.workflows, tt.number, false, "", "", false, "", false, false, false, "", false, "")
+			opts := AddOptions{
+				Number: tt.number,
+			}
+			_, err := AddWorkflows(tt.workflows, opts)
 
 			if tt.expectError {
 				require.Error(t, err, "Expected error for test case: %s", tt.name)
@@ -171,22 +174,13 @@ func TestAddResolvedWorkflows(t *testing.T) {
 				},
 			}
 
+			opts := AddOptions{
+				Number: tt.number,
+			}
 			_, err := AddResolvedWorkflows(
 				[]string{"test/repo/test-workflow"},
 				resolved,
-				tt.number,
-				false, // verbose
-				false, // quiet
-				"",    // engineOverride
-				"",    // name
-				false, // force
-				"",    // appendText
-				false, // createPR
-				false, // push
-				false, // noGitattributes
-				"",    // workflowDir
-				false, // noStopAfter
-				"",    // stopAfter
+				opts,
 			)
 
 			if tt.expectError {

pkg/cli/add_command_test.go

@@ -68,7 +68,8 @@ func TestAddWorkflowsFromCurrentRepository(t *testing.T) {
 			// Clear cache before each test
 			ClearCurrentRepoSlugCache()
 
-			_, err := AddWorkflows(tt.workflowSpecs, 1, false, "", "", false, "", false, false, false, "", false, "")
+			opts := AddOptions{Number: 1}
+			_, err := AddWorkflows(tt.workflowSpecs, opts)
 
 			if tt.expectError {
 				if err == nil {
@@ -151,7 +152,8 @@ func TestAddWorkflowsFromCurrentRepositoryMultiple(t *testing.T) {
 			// Clear cache before each test
 			ClearCurrentRepoSlugCache()
 
-			_, err := AddWorkflows(tt.workflowSpecs, 1, false, "", "", false, "", false, false, false, "", false, "")
+			opts := AddOptions{Number: 1}
+			_, err := AddWorkflows(tt.workflowSpecs, opts)
 
 			if tt.expectError {
 				if err == nil {
@@ -192,7 +194,8 @@ func TestAddWorkflowsFromCurrentRepositoryNotInGitRepo(t *testing.T) {
 
 	// When not in a git repo, the check should be skipped (can't determine current repo)
 	// The function should proceed and fail for other reasons (e.g., workflow not found)
-	_, err = AddWorkflows([]string{"some-owner/some-repo/workflow"}, 1, false, "", "", false, "", false, false, false, "", false, "")
+	opts := AddOptions{Number: 1}
+	_, err = AddWorkflows([]string{"some-owner/some-repo/workflow"}, opts)
 
 	// Should NOT get the "cannot add workflows from the current repository" error
 	if err != nil && strings.Contains(err.Error(), "cannot add workflows from the current repository") {

pkg/cli/add_current_repo_test.go

@@ -84,7 +84,8 @@ This is a test workflow.`
 		os.Remove(".gitattributes")
 
 		// Call addWorkflowsNormal with noGitattributes=false
-		err := addWorkflowsNormal([]*WorkflowSpec{spec}, 1, false, false, "", "", false, "", false, false, false, "", false, "")
+		opts := AddOptions{Number: 1}
+		err := addWorkflowsNormal([]*WorkflowSpec{spec}, opts)
 		if err != nil {
 			// We expect this to fail because we don't have a full workflow setup,
 			// but gitattributes should still be updated before the error
@@ -113,8 +114,9 @@ This is a test workflow.`
 		// Remove any existing .gitattributes
 		os.Remove(".gitattributes")
 
+		opts := AddOptions{Number: 1, NoGitattributes: true}
 		// Call addWorkflowsNormal with noGitattributes=true
-		err := addWorkflowsNormal([]*WorkflowSpec{spec}, 1, false, false, "", "", false, "", false, true, false, "", false, "")
+		err := addWorkflowsNormal([]*WorkflowSpec{spec}, opts)
 		if err != nil {
 			// We expect this to fail because we don't have a full workflow setup
 			t.Logf("Expected error during workflow addition: %v", err)
@@ -135,8 +137,9 @@ This is a test workflow.`
 			t.Fatalf("Failed to create .gitattributes: %v", err)
 		}
 
+		opts := AddOptions{Number: 1, NoGitattributes: true}
 		// Call addWorkflowsNormal with noGitattributes=true
-		err := addWorkflowsNormal([]*WorkflowSpec{spec}, 1, false, false, "", "", false, "", false, true, false, "", false, "")
+		err := addWorkflowsNormal([]*WorkflowSpec{spec}, opts)
 		if err != nil {
 			// We expect this to fail because we don't have a full workflow setup
 			t.Logf("Expected error during workflow addition: %v", err)

pkg/cli/add_gitattributes_test.go

@@ -19,9 +19,25 @@ func (c *AddInteractiveConfig) applyChanges(ctx context.Context, workflowFiles,
 
 	// Add the workflow using existing implementation with --create-pull-request
 	// Pass the resolved workflows to avoid re-fetching them
-	// Pass quiet=true to suppress detailed output (already shown earlier in interactive mode)
+	// Pass Quiet=true to suppress detailed output (already shown earlier in interactive mode)
 	// This returns the result including PR number and HasWorkflowDispatch
-	result, err := AddResolvedWorkflows(c.WorkflowSpecs, c.resolvedWorkflows, 1, c.Verbose, true, c.EngineOverride, "", false, "", true, false, c.NoGitattributes, c.WorkflowDir, c.NoStopAfter, c.StopAfter)
+	opts := AddOptions{
+		Number:                 1,
+		Verbose:                c.Verbose,
+		Quiet:                  true,
+		EngineOverride:         c.EngineOverride,
+		Name:                   "",
+		Force:                  false,
+		AppendText:             "",
+		CreatePR:               true,
+		Push:                   false,
+		NoGitattributes:        c.NoGitattributes,
+		WorkflowDir:            c.WorkflowDir,
+		NoStopAfter:            c.NoStopAfter,
+		StopAfter:              c.StopAfter,
+		DisableSecurityScanner: false,
+	}
+	result, err := AddResolvedWorkflows(c.WorkflowSpecs, c.resolvedWorkflows, opts)
 	if err != nil {
 		return fmt.Errorf("failed to add workflow: %w", err)
 	}

pkg/cli/add_interactive_git.go

@@ -477,7 +477,8 @@ on: push
 
 	// Test 1: Non-wildcard duplicate should return error
 	t.Run("non_wildcard_duplicate_returns_error", func(t *testing.T) {
-		err := addWorkflowWithTracking(spec, 1, false, false, "", "", false, "", nil, false, "", false, "")
+		opts := AddOptions{Number: 1}
+		err := addWorkflowWithTracking(spec, nil, opts)
 		if err == nil {
 			t.Error("Expected error for non-wildcard duplicate, got nil")
 		}
@@ -488,15 +489,17 @@ on: push
 
 	// Test 2: Wildcard duplicate should return nil (skip with warning)
 	t.Run("wildcard_duplicate_returns_nil", func(t *testing.T) {
-		err := addWorkflowWithTracking(spec, 1, false, false, "", "", false, "", nil, true, "", false, "")
+		opts := AddOptions{Number: 1, FromWildcard: true}
+		err := addWorkflowWithTracking(spec, nil, opts)
 		if err != nil {
 			t.Errorf("Expected nil for wildcard duplicate (should skip), got error: %v", err)
 		}
 	})
 
 	// Test 3: Wildcard duplicate with force flag should succeed
 	t.Run("wildcard_duplicate_with_force_succeeds", func(t *testing.T) {
-		err := addWorkflowWithTracking(spec, 1, false, false, "", "", true, "", nil, true, "", false, "")
+		opts := AddOptions{Number: 1, Force: true, FromWildcard: true}
+		err := addWorkflowWithTracking(spec, nil, opts)
 		// This should succeed or return nil
 		if err != nil && strings.Contains(err.Error(), "already exists") {
 			t.Errorf("Expected success with force flag, got 'already exists' error: %v", err)

pkg/cli/add_wildcard_test.go

@@ -13,7 +13,7 @@ import (
 var addWorkflowPRLog = logger.New("cli:add_workflow_pr")
 
 // addWorkflowsWithPR handles workflow addition with PR creation and returns the PR number and URL.
-func addWorkflowsWithPR(workflows []*WorkflowSpec, number int, verbose bool, quiet bool, engineOverride string, name string, force bool, appendText string, push bool, noGitattributes bool, fromWildcard bool, workflowDir string, noStopAfter bool, stopAfter string) (int, string, error) {
+func addWorkflowsWithPR(workflows []*WorkflowSpec, opts AddOptions) (int, string, error) {
 	addWorkflowPRLog.Printf("Adding %d workflow(s) with PR creation", len(workflows))
 
 	// Get current branch for restoration later
@@ -31,7 +31,7 @@ func addWorkflowsWithPR(workflows []*WorkflowSpec, number int, verbose bool, qui
 
 	addWorkflowPRLog.Printf("Creating temporary branch: %s", branchName)
 
-	if err := createAndSwitchBranch(branchName, verbose); err != nil {
+	if err := createAndSwitchBranch(branchName, opts.Verbose); err != nil {
 		return 0, "", fmt.Errorf("failed to create branch %s: %w", branchName, err)
 	}
 
@@ -43,33 +43,36 @@ func addWorkflowsWithPR(workflows []*WorkflowSpec, number int, verbose bool, qui
 
 	// Ensure we switch back to original branch on exit
 	defer func() {
-		if switchErr := switchBranch(currentBranch, verbose); switchErr != nil && verbose {
+		if switchErr := switchBranch(currentBranch, opts.Verbose); switchErr != nil && opts.Verbose {
 			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to switch back to branch %s: %v", currentBranch, switchErr)))
 		}
 	}()
 
 	// Add workflows using the normal function logic
 	addWorkflowPRLog.Print("Adding workflows to repository")
-	if err := addWorkflowsNormal(workflows, number, verbose, quiet, engineOverride, name, force, appendText, push, noGitattributes, fromWildcard, workflowDir, noStopAfter, stopAfter); err != nil {
+	// Disable security scanner for PR creation to use workflow settings
+	prOpts := opts
+	prOpts.DisableSecurityScanner = false
+	if err := addWorkflowsNormal(workflows, prOpts); err != nil {
 		addWorkflowPRLog.Printf("Failed to add workflows: %v", err)
 		// Rollback on error
-		if rollbackErr := tracker.RollbackAllFiles(verbose); rollbackErr != nil && verbose {
+		if rollbackErr := tracker.RollbackAllFiles(opts.Verbose); rollbackErr != nil && opts.Verbose {
 			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to rollback files: %v", rollbackErr)))
 		}
 		return 0, "", fmt.Errorf("failed to add workflows: %w", err)
 	}
 
 	// Stage all files before creating PR
 	addWorkflowPRLog.Print("Staging workflow files")
-	if err := tracker.StageAllFiles(verbose); err != nil {
-		if rollbackErr := tracker.RollbackAllFiles(verbose); rollbackErr != nil && verbose {
+	if err := tracker.StageAllFiles(opts.Verbose); err != nil {
+		if rollbackErr := tracker.RollbackAllFiles(opts.Verbose); rollbackErr != nil && opts.Verbose {
 			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to rollback files: %v", rollbackErr)))
 		}
 		return 0, "", fmt.Errorf("failed to stage workflow files: %w", err)
 	}
 
-	// Update .gitattributes and stage it if modified
-	if err := stageGitAttributesIfChanged(); err != nil && verbose {
+	// Update .gitattributes and stage it if changed
+	if err := stageGitAttributesIfChanged(); err != nil && opts.Verbose {
 		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to stage .gitattributes: %v", err)))
 	}
 
@@ -92,29 +95,29 @@ func addWorkflowsWithPR(workflows []*WorkflowSpec, number int, verbose bool, qui
 		prBody = fmt.Sprintf("Add agentic workflows: %s", joinedNames)
 	}
 
-	if err := commitChanges(commitMessage, verbose); err != nil {
-		if rollbackErr := tracker.RollbackAllFiles(verbose); rollbackErr != nil && verbose {
+	if err := commitChanges(commitMessage, opts.Verbose); err != nil {
+		if rollbackErr := tracker.RollbackAllFiles(opts.Verbose); rollbackErr != nil && opts.Verbose {
 			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to rollback files: %v", rollbackErr)))
 		}
 		return 0, "", fmt.Errorf("failed to commit files: %w", err)
 	}
 
 	// Push branch
 	addWorkflowPRLog.Printf("Pushing branch %s to remote", branchName)
-	if err := pushBranch(branchName, verbose); err != nil {
+	if err := pushBranch(branchName, opts.Verbose); err != nil {
 		addWorkflowPRLog.Printf("Failed to push branch: %v", err)
-		if rollbackErr := tracker.RollbackAllFiles(verbose); rollbackErr != nil && verbose {
+		if rollbackErr := tracker.RollbackAllFiles(opts.Verbose); rollbackErr != nil && opts.Verbose {
 			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to rollback files: %v", rollbackErr)))
 		}
 		return 0, "", fmt.Errorf("failed to push branch %s: %w", branchName, err)
 	}
 
 	// Create PR
 	addWorkflowPRLog.Printf("Creating pull request: %s", prTitle)
-	prNumber, prURL, err := createPR(branchName, prTitle, prBody, verbose)
+	prNumber, prURL, err := createPR(branchName, prTitle, prBody, opts.Verbose)
 	if err != nil {
 		addWorkflowPRLog.Printf("Failed to create PR: %v", err)
-		if rollbackErr := tracker.RollbackAllFiles(verbose); rollbackErr != nil && verbose {
+		if rollbackErr := tracker.RollbackAllFiles(opts.Verbose); rollbackErr != nil && opts.Verbose {
 			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to rollback files: %v", rollbackErr)))
 		}
 		return 0, "", fmt.Errorf("failed to create PR: %w", err)
@@ -125,7 +128,7 @@ func addWorkflowsWithPR(workflows []*WorkflowSpec, number int, verbose bool, qui
 	// Success - no rollback needed
 
 	// Switch back to original branch
-	if err := switchBranch(currentBranch, verbose); err != nil {
+	if err := switchBranch(currentBranch, opts.Verbose); err != nil {
 		return prNumber, prURL, fmt.Errorf("failed to switch back to branch %s: %w", currentBranch, err)
 	}
 

pkg/cli/add_workflow_pr.go

@@ -65,7 +65,7 @@ This is a test workflow.
 	}
 
 	// Test the local installation function
-	err = installLocalWorkflowInTrialMode(originalDir, tempDir, spec, "", false)
+	err = installLocalWorkflowInTrialMode(originalDir, tempDir, spec, "", false, &TrialOptions{DisableSecurityScanner: false})
 	if err != nil {
 		t.Fatalf("Failed to install local workflow: %v", err)
 	}

pkg/cli/local_workflow_trial_test.go

@@ -49,19 +49,20 @@ type RepoConfig struct {
 
 // TrialOptions contains all configuration options for running workflow trials
 type TrialOptions struct {
-	Repos          RepoConfig
-	DeleteHostRepo bool
-	ForceDelete    bool
-	Quiet          bool
-	DryRun         bool
-	TimeoutMinutes int
-	TriggerContext string
-	RepeatCount    int
-	AutoMergePRs   bool
-	EngineOverride string
-	AppendText     string
-	PushSecrets    bool
-	Verbose        bool
+	Repos                  RepoConfig
+	DeleteHostRepo         bool
+	ForceDelete            bool
+	Quiet                  bool
+	DryRun                 bool
+	TimeoutMinutes         int
+	TriggerContext         string
+	RepeatCount            int
+	AutoMergePRs           bool
+	EngineOverride         string
+	AppendText             string
+	PushSecrets            bool
+	Verbose                bool
+	DisableSecurityScanner bool
 }
 
 // NewTrialCommand creates the trial command
@@ -132,6 +133,7 @@ Trial results are saved both locally (in trials/ directory) and in the host repo
 			appendText, _ := cmd.Flags().GetString("append")
 			pushSecrets, _ := cmd.Flags().GetBool("use-local-secrets")
 			verbose, _ := cmd.Root().PersistentFlags().GetBool("verbose")
+			disableSecurityScanner, _ := cmd.Flags().GetBool("disable-security-scanner")
 
 			if err := validateEngine(engineOverride); err != nil {
 				return err
@@ -147,18 +149,19 @@ Trial results are saved both locally (in trials/ directory) and in the host repo
 					CloneRepo:   cloneRepoSpec,
 					HostRepo:    hostRepoSpec,
 				},
-				DeleteHostRepo: deleteHostRepo,
-				ForceDelete:    forceDeleteHostRepo,
-				Quiet:          yes,
-				DryRun:         dryRun,
-				TimeoutMinutes: timeout,
-				TriggerContext: triggerContext,
-				RepeatCount:    repeatCount,
-				AutoMergePRs:   autoMergePRs,
-				EngineOverride: engineOverride,
-				AppendText:     appendText,
-				PushSecrets:    pushSecrets,
-				Verbose:        verbose,
+				DeleteHostRepo:         deleteHostRepo,
+				ForceDelete:            forceDeleteHostRepo,
+				Quiet:                  yes,
+				DryRun:                 dryRun,
+				TimeoutMinutes:         timeout,
+				TriggerContext:         triggerContext,
+				RepeatCount:            repeatCount,
+				AutoMergePRs:           autoMergePRs,
+				EngineOverride:         engineOverride,
+				AppendText:             appendText,
+				PushSecrets:            pushSecrets,
+				Verbose:                verbose,
+				DisableSecurityScanner: disableSecurityScanner,
 			}
 
 			if err := RunWorkflowTrials(cmd.Context(), workflowSpecs, opts); err != nil {
@@ -192,6 +195,7 @@ Trial results are saved both locally (in trials/ directory) and in the host repo
 	addEngineFlag(cmd)
 	cmd.Flags().String("append", "", "Append extra content to the end of agentic workflow on installation")
 	cmd.Flags().Bool("use-local-secrets", false, "Use local environment API key secrets for trial execution (pushes and cleans up secrets in repository)")
+	cmd.Flags().Bool("disable-security-scanner", false, "Disable security scanning of workflow markdown content")
 	cmd.MarkFlagsMutuallyExclusive("host-repo", "repo")
 	cmd.MarkFlagsMutuallyExclusive("logical-repo", "clone-repo")
 
@@ -440,7 +444,7 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp
 			fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("=== Running trial for workflow: %s ===", parsedSpec.WorkflowName)))
 
 			// Install workflow with trial mode compilation
-			if err := installWorkflowInTrialMode(ctx, tempDir, parsedSpec, logicalRepoSlug, cloneRepoSlug, hostRepoSlug, secretTracker, opts.EngineOverride, opts.AppendText, opts.PushSecrets, directTrialMode, opts.Verbose); err != nil {
+			if err := installWorkflowInTrialMode(ctx, tempDir, parsedSpec, logicalRepoSlug, cloneRepoSlug, hostRepoSlug, secretTracker, opts.EngineOverride, opts.AppendText, opts.PushSecrets, directTrialMode, opts.Verbose, &opts); err != nil {
 				return fmt.Errorf("failed to install workflow '%s' in trial mode: %w", parsedSpec.WorkflowName, err)
 			}