Remove anonymous bash tool syntax, require explicit configuration#14222
Remove anonymous bash tool syntax, require explicit configuration#14222
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot have the codemox update to base: true instad of base: false. |
Updated codemod to convert anonymous bash: syntax to bash: true (enable all commands) instead of bash: false. This is more semantically equivalent to the original behavior where anonymous bash: was converted to default commands. - Updated getBashAnonymousRemovalCodemod to use bash: true - Renamed replaceBashAnonymousWithFalse to replaceBashAnonymousWithTrue - Updated all tests to expect bash: true - Re-migrated 12 workflows from bash: false to bash: true - Recompiled all workflow lock files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done! Updated codemod to use Changes in commit 84c208a:
|
🛡️ Security Posture AnalysisThis PR contains changes that may affect the security posture. Please review the following concerns: 🟠 Tool Security Relaxation: Unrestricted Bash accessLocation: Change Detected: - # --allow-tool shell(cat)
- # --allow-tool shell(date)
- # --allow-tool shell(echo)
- # --allow-tool shell(grep)
- # --allow-tool shell(head)
- # --allow-tool shell(ls)
- # --allow-tool shell(pwd)
- # --allow-tool shell(sort)
- # --allow-tool shell(tail)
- # --allow-tool shell(uniq)
- # --allow-tool shell(wc)
- # --allow-tool shell(yq)
+ # --allow-tool BashSecurity Impact: The workflow now allows the general Recommendation: Keep the Bash tool restricted to the explicit commands the workflow requires or add explicit justification and compensating controls for granting unfettered Bash access. Summary
Note: This is an automated analysis. Please verify these findings and determine if the changes are intentional and justified.
|
There was a problem hiding this comment.
Pull request overview
This PR removes support for the ambiguous anonymous bash: tool syntax in workflow frontmatter, requiring explicit configuration and providing a gh aw fix codemod to migrate existing workflows.
Changes:
- Updated tool parsing/validation to reject
tools.bash: null(anonymousbash:) and to supportbash: true|false. - Added compiler-time validation to fail before default tool expansion can transform
nilbash into defaults. - Added a codemod and migrated existing workflows/lockfiles away from anonymous
bash:.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/tools_validation_test.go | Adds unit tests for explicit/anonymous bash validation and boolean parsing. |
| pkg/workflow/tools_validation.go | Introduces validation that rejects anonymous bash: configs early. |
| pkg/workflow/tools_parser.go | Updates bash tool parsing to reject nil and accept booleans. |
| pkg/workflow/compiler_orchestrator_workflow.go | Runs bash config validation before defaults are applied. |
| pkg/workflow/bash_anonymous_validation_test.go | Adds end-to-end compiler tests for rejecting anonymous bash and accepting explicit forms. |
| pkg/cli/fix_codemods_test.go | Updates codemod registry tests for the new codemod entry. |
| pkg/cli/fix_codemods.go | Registers the new “bash-anonymous-removal” codemod. |
| pkg/cli/codemod_bash_anonymous_test.go | Adds tests for the anonymous-bash codemod behavior. |
| pkg/cli/codemod_bash_anonymous.go | Implements codemod to rewrite anonymous bash: into an explicit configuration. |
| .github/workflows/video-analyzer.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/video-analyzer.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/technical-doc-writer.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/technical-doc-writer.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/q.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/q.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/mcp-inspector.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/mcp-inspector.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/lockfile-stats.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/lockfile-stats.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/daily-secrets-analysis.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/daily-secrets-analysis.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/daily-malicious-code-scan.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/daily-malicious-code-scan.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/daily-code-metrics.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/daily-code-metrics.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/daily-cli-performance.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/daily-cli-performance.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/code-scanning-fixer.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/code-scanning-fixer.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/artifacts-summary.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/artifacts-summary.lock.yml | Recompiled lockfile reflecting updated tools config. |
| .github/workflows/archie.md | Migrates workflow tools config from anonymous bash: to explicit. |
| .github/workflows/archie.lock.yml | Recompiled lockfile reflecting updated tools config. |
Comments suppressed due to low confidence (1)
pkg/cli/codemod_bash_anonymous.go:95
- replaceBashAnonymousWithTrue only rewrites lines where the trimmed content is exactly
bash:. YAML likebash: # comment(still a nil value) will not be rewritten even though the frontmatter map indicatesbashis nil, so the codemod will silently not apply. Consider handlingbash:lines that have only whitespace and/or an inline comment after the colon, preserving the comment when addingtrue.
// Replace bash: with bash: true if in tools block
if inToolsBlock && (trimmed == "bash:" || startsWith(trimmed, "bash: ")) {
// Check if it's just 'bash:' with nothing after the colon
if trimmed == "bash:" {
indent := getIndentation(line)
result = append(result, indent+"bash: true")
modified = true
bashAnonymousCodemodLog.Printf("Replaced 'bash:' with 'bash: true'")
continue
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check if bash is present in the raw map but Bash field is nil | ||
| // This indicates the anonymous syntax (bash:) was used | ||
| if rawMap := tools.ToMap(); rawMap != nil { | ||
| if _, hasBash := rawMap["bash"]; hasBash && tools.Bash == nil { | ||
| toolsValidationLog.Printf("Invalid bash tool configuration in workflow: %s", workflowName) | ||
| return fmt.Errorf("invalid bash tool configuration: anonymous syntax 'bash:' is not supported. Use 'bash: true' (enable all commands), 'bash: false' (disable), or 'bash: [\"cmd1\", \"cmd2\"]' (specific commands). Run 'gh aw fix' to automatically migrate") | ||
| } |
There was a problem hiding this comment.
validateBashToolConfig currently treats any case where rawMap["bash"] exists but tools.Bash == nil as the anonymous bash: syntax. However parseBashTool also returns nil for other invalid types (e.g., bash: "foo"), so those would get a misleading “anonymous syntax” error and an incorrect suggestion to run gh aw fix. Consider checking the raw value: if rawMap["bash"] == nil then report anonymous syntax; otherwise report an invalid bash config type/value with a message that lists the accepted forms (bool or array).
| Name: "Replace anonymous bash tool syntax with explicit true", | ||
| Description: "Replaces 'bash:' (anonymous/nil syntax) with 'bash: true' to make configuration explicit", | ||
| IntroducedIn: "0.9.0", | ||
| Apply: func(content string, frontmatter map[string]any) (string, bool, error) { | ||
| // Check if tools.bash exists | ||
| toolsValue, hasTools := frontmatter["tools"] | ||
| if !hasTools { | ||
| return content, false, nil | ||
| } | ||
|
|
||
| toolsMap, ok := toolsValue.(map[string]any) | ||
| if !ok { | ||
| return content, false, nil | ||
| } | ||
|
|
||
| // Check if bash field exists and is nil | ||
| bashValue, hasBash := toolsMap["bash"] | ||
| if !hasBash { | ||
| return content, false, nil | ||
| } | ||
|
|
||
| // Only modify if bash is nil (anonymous syntax) | ||
| if bashValue != nil { | ||
| return content, false, nil | ||
| } | ||
|
|
||
| // Parse frontmatter to get raw lines | ||
| frontmatterLines, markdown, err := parseFrontmatterLines(content) | ||
| if err != nil { | ||
| return content, false, err | ||
| } | ||
|
|
||
| // Replace the bash field from anonymous to explicit true | ||
| modifiedLines, modified := replaceBashAnonymousWithTrue(frontmatterLines) | ||
| if !modified { | ||
| return content, false, nil | ||
| } | ||
|
|
||
| // Reconstruct the content | ||
| newContent := reconstructContent(modifiedLines, markdown) | ||
| bashAnonymousCodemodLog.Print("Applied bash anonymous removal, replaced with 'bash: true'") | ||
| return newContent, true, nil |
There was a problem hiding this comment.
This codemod replaces anonymous bash: with bash: true, but compilation treats bash: true as wildcard/all commands while the previous anonymous form was compiled to constants.DefaultBashTools (via applyDefaultTools handling bash == nil). That means the migration can broaden shell permissions compared to the old behavior. If the goal is semantic equivalence, migrate to an explicit list of the default commands (or adjust the compiler semantics for bash: true).
This issue also appears on line 85 of the same file.
| Name: "Replace anonymous bash tool syntax with explicit true", | |
| Description: "Replaces 'bash:' (anonymous/nil syntax) with 'bash: true' to make configuration explicit", | |
| IntroducedIn: "0.9.0", | |
| Apply: func(content string, frontmatter map[string]any) (string, bool, error) { | |
| // Check if tools.bash exists | |
| toolsValue, hasTools := frontmatter["tools"] | |
| if !hasTools { | |
| return content, false, nil | |
| } | |
| toolsMap, ok := toolsValue.(map[string]any) | |
| if !ok { | |
| return content, false, nil | |
| } | |
| // Check if bash field exists and is nil | |
| bashValue, hasBash := toolsMap["bash"] | |
| if !hasBash { | |
| return content, false, nil | |
| } | |
| // Only modify if bash is nil (anonymous syntax) | |
| if bashValue != nil { | |
| return content, false, nil | |
| } | |
| // Parse frontmatter to get raw lines | |
| frontmatterLines, markdown, err := parseFrontmatterLines(content) | |
| if err != nil { | |
| return content, false, err | |
| } | |
| // Replace the bash field from anonymous to explicit true | |
| modifiedLines, modified := replaceBashAnonymousWithTrue(frontmatterLines) | |
| if !modified { | |
| return content, false, nil | |
| } | |
| // Reconstruct the content | |
| newContent := reconstructContent(modifiedLines, markdown) | |
| bashAnonymousCodemodLog.Print("Applied bash anonymous removal, replaced with 'bash: true'") | |
| return newContent, true, nil | |
| Name: "Disable anonymous bash tool codemod", | |
| Description: "Previously replaced 'bash:' (anonymous/nil syntax) with 'bash: true', but is now disabled to avoid broadening shell permissions.", | |
| IntroducedIn: "0.9.0", | |
| Apply: func(content string, frontmatter map[string]any) (string, bool, error) { | |
| // This codemod is intentionally a no-op. | |
| // The original implementation replaced anonymous "bash:" with "bash: true", | |
| // which caused compilation to treat bash as a wildcard for all commands | |
| // and could silently broaden shell permissions compared to the previous | |
| // semantics (where bash == nil maps to the default bash tools). | |
| // To preserve existing behavior and avoid expanding permissions, we no longer | |
| // modify the content here. | |
| return content, false, nil |
| getDiscussionFlagRemovalCodemod(), | ||
| getMCPModeToTypeCodemod(), | ||
| getInstallScriptURLCodemod(), | ||
| getBashAnonymousRemovalCodemod(), // Replace bash: with bash: false |
There was a problem hiding this comment.
The trailing comment says “Replace bash: with bash: false”, but the codemod actually replaces anonymous bash: with bash: true. This is misleading for future maintainers—update the comment to match the actual behavior (or change the codemod if false was intended).
| getBashAnonymousRemovalCodemod(), // Replace bash: with bash: false | |
| getBashAnonymousRemovalCodemod(), // Replace anonymous bash: with bash: true |
Anonymous
bash:syntax (nil value) was ambiguous and silently converted to default commands during compilation. This removes support and requires explicit configuration.Changes
Parser (
tools_parser.go)parseBashTool()returnsnilfor anonymous syntax, rejects at parse timebash: true(all commands),bash: false(disabled)Validation (
tools_validation.go,compiler_orchestrator_workflow.go)validateBashToolConfig()inParseWorkflowFile()beforeapplyDefaultTools()gh aw fixCodemod (
codemod_bash_anonymous.go)bash:→bash: truebash: true(enable all commands) to match the original behavior where anonymous syntax was converted to default commandsgh aw fixcommandWorkflows
bash: trueSupported Syntax
Migration
Compilation fails with:
Running
gh aw fixautomatically convertsbash:tobash: true, which enables all commands and is semantically equivalent to the original anonymous syntax behavior.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.