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
7 changes: 7 additions & 0 deletions .changeset/patch-actionable-strict-mode-hints.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions pkg/workflow/strict_mode_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (c *Compiler) validateStrictPermissions(frontmatter map[string]any) error {
for _, scope := range writePermissions {
if perms.IsAllowed(scope, "write") {
strictModeValidationLog.Printf("Write permission validation failed: scope=%s", scope)
return fmt.Errorf("strict mode: write permission '%s: write' is not allowed", scope)
return fmt.Errorf("strict mode: write permission '%s: write' is not allowed for security reasons. Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely. See: https://githubnext.github.io/gh-aw/reference/safe-outputs/", scope)
}
}

Expand All @@ -75,7 +75,7 @@ func (c *Compiler) validateStrictPermissions(frontmatter map[string]any) error {
func (c *Compiler) validateStrictNetwork(networkPermissions *NetworkPermissions) error {
if networkPermissions == nil {
strictModeValidationLog.Printf("Network configuration missing")
return fmt.Errorf("strict mode: 'network' configuration is required")
return fmt.Errorf("strict mode: 'network' configuration is required to prevent unrestricted network access. Add 'network: { allowed: [...] }' or 'network: defaults' to your frontmatter. See: https://githubnext.github.io/gh-aw/reference/network/")
}

// If mode is "defaults", that's acceptable
Expand All @@ -88,7 +88,7 @@ func (c *Compiler) validateStrictNetwork(networkPermissions *NetworkPermissions)
for _, domain := range networkPermissions.Allowed {
if domain == "*" {
strictModeValidationLog.Printf("Network validation failed: wildcard detected")
return fmt.Errorf("strict mode: wildcard '*' is not allowed in network.allowed domains")
return fmt.Errorf("strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access. Specify explicit domains or use ecosystem identifiers like 'python', 'node', 'containers'. See: https://githubnext.github.io/gh-aw/reference/network/#available-ecosystem-identifiers")
}
}

Expand Down Expand Up @@ -127,7 +127,7 @@ func (c *Compiler) validateStrictMCPNetwork(frontmatter map[string]any) error {
if _, hasContainer := serverConfig["container"]; hasContainer {
// Check if network configuration is present
if _, hasNetwork := serverConfig["network"]; !hasNetwork {
return fmt.Errorf("strict mode: custom MCP server '%s' with container must have network configuration", serverName)
return fmt.Errorf("strict mode: custom MCP server '%s' with container must have network configuration for security. Add 'network: { allowed: [...] }' to the server configuration to restrict network access. See: https://githubnext.github.io/gh-aw/reference/network/", serverName)
}
}
}
Expand Down
26 changes: 13 additions & 13 deletions pkg/workflow/strict_mode_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestValidateStrictPermissions(t *testing.T) {
},
},
expectError: true,
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
errorMsg: "strict mode: write permission 'contents: write' is not allowed for security reasons. Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely",
},
{
name: "issues write permission is refused",
Expand All @@ -52,7 +52,7 @@ func TestValidateStrictPermissions(t *testing.T) {
},
},
expectError: true,
errorMsg: "strict mode: write permission 'issues: write' is not allowed",
errorMsg: "strict mode: write permission 'issues: write' is not allowed for security reasons. Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely",
},
{
name: "pull-requests write permission is refused",
Expand All @@ -63,7 +63,7 @@ func TestValidateStrictPermissions(t *testing.T) {
},
},
expectError: true,
errorMsg: "strict mode: write permission 'pull-requests: write' is not allowed",
errorMsg: "strict mode: write permission 'pull-requests: write' is not allowed for security reasons. Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely",
},
{
name: "multiple write permissions fail on first one",
Expand All @@ -89,7 +89,7 @@ func TestValidateStrictPermissions(t *testing.T) {
},
},
expectError: true,
errorMsg: "strict mode: write permission 'issues: write' is not allowed",
errorMsg: "strict mode: write permission 'issues: write' is not allowed for security reasons. Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely",
},
{
name: "other write permissions are allowed (not in sensitive scopes)",
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestValidateStrictPermissions(t *testing.T) {
"permissions": "write",
},
expectError: true,
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
errorMsg: "strict mode: write permission 'contents: write' is not allowed for security reasons. Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely",
},
{
name: "shorthand write-all is refused",
Expand All @@ -126,7 +126,7 @@ func TestValidateStrictPermissions(t *testing.T) {
"permissions": "write-all",
},
expectError: true,
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
errorMsg: "strict mode: write permission 'contents: write' is not allowed for security reasons. Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely",
},
{
name: "empty permissions map is allowed",
Expand Down Expand Up @@ -176,7 +176,7 @@ func TestValidateStrictNetwork(t *testing.T) {
name: "nil network permissions is refused",
networkPermissions: nil,
expectError: true,
errorMsg: "strict mode: 'network' configuration is required",
errorMsg: "strict mode: 'network' configuration is required to prevent unrestricted network access",
},
{
name: "defaults mode is allowed",
Expand All @@ -200,7 +200,7 @@ func TestValidateStrictNetwork(t *testing.T) {
Allowed: []string{"*"},
},
expectError: true,
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains",
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access",
},
{
name: "wildcard among other domains is refused",
Expand All @@ -209,7 +209,7 @@ func TestValidateStrictNetwork(t *testing.T) {
Allowed: []string{"api.example.com", "*", "github.com"},
},
expectError: true,
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains",
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access",
},
{
name: "empty allowed list is allowed",
Expand Down Expand Up @@ -306,7 +306,7 @@ func TestValidateStrictMode(t *testing.T) {
Allowed: []string{"api.example.com"},
},
expectError: true,
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
errorMsg: "strict mode: write permission 'contents: write' is not allowed for security reasons",
},
{
name: "strict mode fails on missing network config",
Expand All @@ -319,7 +319,7 @@ func TestValidateStrictMode(t *testing.T) {
},
networkPermissions: nil,
expectError: true,
errorMsg: "strict mode: 'network' configuration is required",
errorMsg: "strict mode: 'network' configuration is required to prevent unrestricted network access",
},
{
name: "strict mode fails on wildcard network",
Expand All @@ -335,7 +335,7 @@ func TestValidateStrictMode(t *testing.T) {
Allowed: []string{"*"},
},
expectError: true,
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains",
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access",
},
{
name: "strict mode with container MCP requiring network",
Expand All @@ -356,7 +356,7 @@ func TestValidateStrictMode(t *testing.T) {
Allowed: []string{"api.example.com"},
},
expectError: true,
errorMsg: "strict mode: custom MCP server 'my-server' with container must have network configuration",
errorMsg: "strict mode: custom MCP server 'my-server' with container must have network configuration for security",
},
{
name: "strict mode with container MCP and network config",
Expand Down
7 changes: 4 additions & 3 deletions pkg/workflow/validation_strict_mcp_network_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package workflow

import (
"strings"
"testing"
)

Expand Down Expand Up @@ -82,9 +83,9 @@ func TestValidateStrictMCPNetwork_ContainerWithoutNetwork(t *testing.T) {
if err == nil {
t.Error("Expected error for container without network configuration, got nil")
}
expectedMsg := "strict mode: custom MCP server 'my-server' with container must have network configuration"
if err != nil && err.Error() != expectedMsg {
t.Errorf("Expected error message %q, got: %q", expectedMsg, err.Error())
expectedMsg := "strict mode: custom MCP server 'my-server' with container must have network configuration for security"
if err != nil && !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected error message to contain %q, got: %q", expectedMsg, err.Error())
}
}

Expand Down
Loading