Skip to content

Commit d47c8e9

Browse files
authored
Add actionable hints to strict mode validation errors (#4013)
1 parent 7d2f5f7 commit d47c8e9

4 files changed

Lines changed: 28 additions & 20 deletions

File tree

.changeset/patch-actionable-strict-mode-hints.md

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/workflow/strict_mode_validation.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (c *Compiler) validateStrictPermissions(frontmatter map[string]any) error {
6363
for _, scope := range writePermissions {
6464
if perms.IsAllowed(scope, "write") {
6565
strictModeValidationLog.Printf("Write permission validation failed: scope=%s", scope)
66-
return fmt.Errorf("strict mode: write permission '%s: write' is not allowed", scope)
66+
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)
6767
}
6868
}
6969

@@ -75,7 +75,7 @@ func (c *Compiler) validateStrictPermissions(frontmatter map[string]any) error {
7575
func (c *Compiler) validateStrictNetwork(networkPermissions *NetworkPermissions) error {
7676
if networkPermissions == nil {
7777
strictModeValidationLog.Printf("Network configuration missing")
78-
return fmt.Errorf("strict mode: 'network' configuration is required")
78+
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/")
7979
}
8080

8181
// If mode is "defaults", that's acceptable
@@ -88,7 +88,7 @@ func (c *Compiler) validateStrictNetwork(networkPermissions *NetworkPermissions)
8888
for _, domain := range networkPermissions.Allowed {
8989
if domain == "*" {
9090
strictModeValidationLog.Printf("Network validation failed: wildcard detected")
91-
return fmt.Errorf("strict mode: wildcard '*' is not allowed in network.allowed domains")
91+
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")
9292
}
9393
}
9494

@@ -127,7 +127,7 @@ func (c *Compiler) validateStrictMCPNetwork(frontmatter map[string]any) error {
127127
if _, hasContainer := serverConfig["container"]; hasContainer {
128128
// Check if network configuration is present
129129
if _, hasNetwork := serverConfig["network"]; !hasNetwork {
130-
return fmt.Errorf("strict mode: custom MCP server '%s' with container must have network configuration", serverName)
130+
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)
131131
}
132132
}
133133
}

pkg/workflow/strict_mode_validation_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestValidateStrictPermissions(t *testing.T) {
4141
},
4242
},
4343
expectError: true,
44-
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
44+
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",
4545
},
4646
{
4747
name: "issues write permission is refused",
@@ -52,7 +52,7 @@ func TestValidateStrictPermissions(t *testing.T) {
5252
},
5353
},
5454
expectError: true,
55-
errorMsg: "strict mode: write permission 'issues: write' is not allowed",
55+
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",
5656
},
5757
{
5858
name: "pull-requests write permission is refused",
@@ -63,7 +63,7 @@ func TestValidateStrictPermissions(t *testing.T) {
6363
},
6464
},
6565
expectError: true,
66-
errorMsg: "strict mode: write permission 'pull-requests: write' is not allowed",
66+
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",
6767
},
6868
{
6969
name: "multiple write permissions fail on first one",
@@ -89,7 +89,7 @@ func TestValidateStrictPermissions(t *testing.T) {
8989
},
9090
},
9191
expectError: true,
92-
errorMsg: "strict mode: write permission 'issues: write' is not allowed",
92+
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",
9393
},
9494
{
9595
name: "other write permissions are allowed (not in sensitive scopes)",
@@ -117,7 +117,7 @@ func TestValidateStrictPermissions(t *testing.T) {
117117
"permissions": "write",
118118
},
119119
expectError: true,
120-
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
120+
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",
121121
},
122122
{
123123
name: "shorthand write-all is refused",
@@ -126,7 +126,7 @@ func TestValidateStrictPermissions(t *testing.T) {
126126
"permissions": "write-all",
127127
},
128128
expectError: true,
129-
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
129+
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",
130130
},
131131
{
132132
name: "empty permissions map is allowed",
@@ -176,7 +176,7 @@ func TestValidateStrictNetwork(t *testing.T) {
176176
name: "nil network permissions is refused",
177177
networkPermissions: nil,
178178
expectError: true,
179-
errorMsg: "strict mode: 'network' configuration is required",
179+
errorMsg: "strict mode: 'network' configuration is required to prevent unrestricted network access",
180180
},
181181
{
182182
name: "defaults mode is allowed",
@@ -200,7 +200,7 @@ func TestValidateStrictNetwork(t *testing.T) {
200200
Allowed: []string{"*"},
201201
},
202202
expectError: true,
203-
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains",
203+
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access",
204204
},
205205
{
206206
name: "wildcard among other domains is refused",
@@ -209,7 +209,7 @@ func TestValidateStrictNetwork(t *testing.T) {
209209
Allowed: []string{"api.example.com", "*", "github.com"},
210210
},
211211
expectError: true,
212-
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains",
212+
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access",
213213
},
214214
{
215215
name: "empty allowed list is allowed",
@@ -306,7 +306,7 @@ func TestValidateStrictMode(t *testing.T) {
306306
Allowed: []string{"api.example.com"},
307307
},
308308
expectError: true,
309-
errorMsg: "strict mode: write permission 'contents: write' is not allowed",
309+
errorMsg: "strict mode: write permission 'contents: write' is not allowed for security reasons",
310310
},
311311
{
312312
name: "strict mode fails on missing network config",
@@ -319,7 +319,7 @@ func TestValidateStrictMode(t *testing.T) {
319319
},
320320
networkPermissions: nil,
321321
expectError: true,
322-
errorMsg: "strict mode: 'network' configuration is required",
322+
errorMsg: "strict mode: 'network' configuration is required to prevent unrestricted network access",
323323
},
324324
{
325325
name: "strict mode fails on wildcard network",
@@ -335,7 +335,7 @@ func TestValidateStrictMode(t *testing.T) {
335335
Allowed: []string{"*"},
336336
},
337337
expectError: true,
338-
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains",
338+
errorMsg: "strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access",
339339
},
340340
{
341341
name: "strict mode with container MCP requiring network",
@@ -356,7 +356,7 @@ func TestValidateStrictMode(t *testing.T) {
356356
Allowed: []string{"api.example.com"},
357357
},
358358
expectError: true,
359-
errorMsg: "strict mode: custom MCP server 'my-server' with container must have network configuration",
359+
errorMsg: "strict mode: custom MCP server 'my-server' with container must have network configuration for security",
360360
},
361361
{
362362
name: "strict mode with container MCP and network config",

pkg/workflow/validation_strict_mcp_network_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package workflow
22

33
import (
4+
"strings"
45
"testing"
56
)
67

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

0 commit comments

Comments
 (0)