Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
4 changes: 2 additions & 2 deletions .github/aw/safe-outputs-content.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ description: Safe-output reference for issue, discussion, comment, and pull requ
- "docs/**/*.md" # e.g. restrict to Markdown docs
excluded-files: # Optional: glob patterns to strip from the patch entirely
- "**/*.lock"
protected-files: request_review # Optional: "request_review" (default), "blocked", "fallback-to-issue", or "allowed"
protected-files: request-review # Optional: "request-review" (default), "blocked", "fallback-to-issue", or "allowed"
allowed-branches: # Optional: glob patterns for allowed source branch names per run
- "feature/*"
allowed-base-branches: # Optional: glob patterns for allowed base branch overrides per run
Expand All @@ -224,7 +224,7 @@ description: Safe-output reference for issue, discussion, comment, and pull requ

**Allowed Source 分支**: When `allowed-branches` is set, the branch used for PR creation (agent-provided `branch` or the current checkout branch when omitted) must match one of the configured glob patterns.

**File Restrictions**: **Always specify `allowed-files`** — this is the primary guardrail for `create-pull-request`. Scope it to specific file extensions (e.g., `"**/*.md"`, `"**/*.ts"`) or directory paths (e.g., `"src/**"`, `"docs/**"`) matching the workflow's purpose. Omitting `allowed-files` allows the agent to touch any file in the repository, which significantly expands blast radius. Use `excluded-files` to additionally strip specific files (e.g. lock files) from the patch before any checks. The `protected-files` field controls handling of sensitive files (package manifests, CI configs, agent instruction files): `request_review` (default — create the PR but submit a `REQUEST_CHANGES` review so a human approves before merge), `blocked` (hard-block), `fallback-to-issue` (push branch and create a review issue), or `allowed` (no restriction — use only when the workflow is explicitly designed to manage these files). Object form is also supported: `protected-files: { policy: fallback-to-issue, exclude: [AGENTS.md] }`.
**File Restrictions**: **Always specify `allowed-files`** — this is the primary guardrail for `create-pull-request`. Scope it to specific file extensions (e.g., `"**/*.md"`, `"**/*.ts"`) or directory paths (e.g., `"src/**"`, `"docs/**"`) matching the workflow's purpose. Omitting `allowed-files` allows the agent to touch any file in the repository, which significantly expands blast radius. Use `excluded-files` to additionally strip specific files (e.g. lock files) from the patch before any checks. The `protected-files` field controls handling of sensitive files (package manifests, CI configs, agent instruction files): `request-review` (default — create the PR but submit a `REQUEST_CHANGES` review so a human approves before merge), `blocked` (hard-block), `fallback-to-issue` (push branch and create a review issue), or `allowed` (no restriction — use only when the workflow is explicitly designed to manage these files). Object form is also supported: `protected-files: { policy: fallback-to-issue, exclude: [AGENTS.md] }`.

**Auto-Expiration**: The `expires` field auto-closes PRs after a time period. Supports integers (days) or relative formats (2h, 7d, 2w, 1m, 1y). Minimum duration: 2 hours. Only for same-repo PRs without target-repo. Generates `agentics-maintenance.yml` workflow.

Expand Down
2 changes: 1 addition & 1 deletion .github/aw/safe-outputs-management.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ description: Safe-output reference for update, label, milestone, project, releas
- "src/**"
excluded-files: # Optional: glob patterns to strip from the patch entirely
- "**/*.lock"
protected-files: blocked # Optional: "blocked" (default), "fallback-to-issue", or "allowed"
protected-files: request-review # Optional: "request-review" (default), "blocked", "fallback-to-issue", or "allowed"
max-patch-size: 2048 # Optional: per-output cap on git patch size in KB (overrides global; default: 4096 KB, max: 10240)
```

Expand Down
4 changes: 2 additions & 2 deletions actions/setup/js/manifest_file_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ function checkFileProtection(patchContent, config) {
if (config.protected_files_policy === "fallback-to-issue") {
return { action: "fallback", files: allFound };
}
if (config.protected_files_policy === "request_review") {
if (config.protected_files_policy === "request-review" || config.protected_files_policy === "request_review") {
return { action: "request_review", files: allFound };
}
return { action: "deny", source: "protected", files: allFound };
Expand Down Expand Up @@ -334,7 +334,7 @@ function checkFileProtectionPostApply(actualFiles, config) {
if (config.protected_files_policy === "fallback-to-issue") {
return { action: "fallback", files: allProtected };
}
if (config.protected_files_policy === "request_review") {
if (config.protected_files_policy === "request-review" || config.protected_files_policy === "request_review") {
return { action: "request_review", files: allProtected };
}
return { action: "deny", source: "protected", files: allProtected };
Expand Down
2 changes: 1 addition & 1 deletion actions/setup/md/manifest_protection_request_review.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
> [!CAUTION]
> Protected files were modified in this change.
> This pull request is in `request_review` mode and requires explicit human scrutiny before merge.
> This pull request is in `request-review` mode and requires explicit human scrutiny before merge.
>
> Protected files: {files}
14 changes: 7 additions & 7 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -7709,14 +7709,14 @@
"oneOf": [
{
"type": "string",
"enum": ["blocked", "allowed", "fallback-to-issue", "request_review"],
"description": "Controls protected-file protection. request_review (default): create the PR but prepend a caution block and submit a REQUEST_CHANGES review for manual scrutiny. blocked: hard-block any patch that modifies package manifests (e.g. package.json, go.mod), engine instruction files (e.g. AGENTS.md, CLAUDE.md) or .github/ files. allowed: allow all changes. fallback-to-issue: push the branch but create a review issue instead of a PR, so a human can review the manifest changes before merging.",
"enum": ["blocked", "allowed", "fallback-to-issue", "request-review"],
"description": "Controls protected-file protection. request-review (default): create the PR but prepend a caution block and submit a REQUEST_CHANGES review for manual scrutiny. blocked: hard-block any patch that modifies package manifests (e.g. package.json, go.mod), engine instruction files (e.g. AGENTS.md, CLAUDE.md) or .github/ files. allowed: allow all changes. fallback-to-issue: push the branch but create a review issue instead of a PR, so a human can review the manifest changes before merging.",
"default": "request_review"
Comment thread
Copilot marked this conversation as resolved.
Outdated

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.

The "default" value is still "request_review" (underscore), but "request_review" was removed from the enum in this PR — only "request-review" (hyphen) is now a valid enum member. A JSON Schema validator will flag this default as invalid.

Both occurrences (lines 7714 and 7730) need:

"default": "request-review"

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the pushed schema update: both canonical request-review and legacy request_review are valid enum values, while the schema default is canonical request-review.

},
{
"type": "string",
"pattern": "^\\$\\{\\{.*\\}\\}$",
"description": "GitHub 操作 expression that resolves to 'blocked', 'allowed', 'fallback-to-issue', or 'request_review' at runtime. Use in reusable workflow_call workflows to parameterize the policy per caller."
"description": "GitHub 操作 expression that resolves to 'blocked', 'allowed', 'fallback-to-issue', or 'request-review' at runtime. Use in reusable workflow_call workflows to parameterize the policy per caller."
},
{
"type": "object",
Expand All @@ -7725,14 +7725,14 @@
"oneOf": [
{
"type": "string",
"enum": ["blocked", "allowed", "fallback-to-issue", "request_review"],
"description": "Protection policy. request_review (default): create the PR but prepend a caution block and submit a REQUEST_CHANGES review. blocked: hard-block any patch that modifies protected files. allowed: allow all changes. fallback-to-issue: push the branch but create a review issue instead of a PR.",
"enum": ["blocked", "allowed", "fallback-to-issue", "request-review"],
"description": "Protection policy. request-review (default): create the PR but prepend a caution block and submit a REQUEST_CHANGES review. blocked: hard-block any patch that modifies protected files. allowed: allow all changes. fallback-to-issue: push the branch but create a review issue instead of a PR.",
"default": "request_review"
},
{
"type": "string",
"pattern": "^\\$\\{\\{.*\\}\\}$",
"description": "GitHub 操作 expression that resolves to 'blocked', 'allowed', 'fallback-to-issue', or 'request_review' at runtime."
"description": "GitHub 操作 expression that resolves to 'blocked', 'allowed', 'fallback-to-issue', or 'request-review' at runtime."
}
]
},
Expand All @@ -7749,7 +7749,7 @@
"description": "Object form for granular control over the protected-file set. Use the exclude list to remove specific files from the default protection while keeping the rest."
}
],
"description": "Controls protected-file protection. String form: request_review (default), blocked, allowed, or fallback-to-issue \u2014 or a GitHub 操作 expression for reusable workflows. Object form: { policy, exclude } to customize the protected-file set."
"description": "Controls protected-file protection. String form: request-review (default), blocked, allowed, or fallback-to-issue \u2014 or a GitHub 操作 expression for reusable workflows. Object form: { policy, exclude } to customize the protected-file set."
},
"allowed-files": {
"type": "array",
Expand Down
59 changes: 58 additions & 1 deletion pkg/workflow/compiler_safe_outputs_config_pr_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,63 @@ func TestCreateReportIncompleteIssueTemplatableBool(t *testing.T) {
// TestPRPolicyFieldsExpressionsPassThrough verifies that GitHub 操作 expression strings
// set on protected-files and patch-format are emitted verbatim into the handler config.
// This enables reusable workflow_call workflows to parameterise these policy fields per caller.
func TestProtectedFilesPolicyDefaultsAreHyphenated(t *testing.T) {
t.Parallel()

tests := []struct {
name string
safeOutputs *SafeOutputsConfig
handlerKey string
}{
{
name: "create-pull-request defaults to request-review",
safeOutputs: &SafeOutputsConfig{
CreatePullRequests: &CreatePullRequestsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}},
},
handlerKey: "create_pull_request",
},
{
name: "push-to-pull-request-branch defaults to request-review",
safeOutputs: &SafeOutputsConfig{
PushToPullRequestBranch: &PushToPullRequestBranchConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}},
},
handlerKey: "push_to_pull_request_branch",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compiler := 新建Compiler()
workflowData := &WorkflowData{Name: "Test Workflow", SafeOutputs: tt.safeOutputs}
var steps []string
compiler.addHandlerManagerConfigEnvVar(&steps, workflowData)
require.NotEmpty(t, steps, "should produce config steps")

var configJSON string
for _, step := range steps {
if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") {
parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ")
require.Len(t, parts, 2, "should split env var line")
configJSON = strings.TrimSpace(parts[1])
configJSON = strings.Trim(configJSON, "\"")
configJSON = strings.ReplaceAll(configJSON, "\\\"", "\"")
}
}
require.NotEmpty(t, configJSON, "should have extracted JSON")

var config map[string]map[string]any
require.NoError(t, json.Unmarshal([]byte(configJSON), &config), "config JSON should be valid")

handlerConfig, ok := config[tt.handlerKey]
require.True(t, ok, "should have %s config", tt.handlerKey)

pfPolicy, ok := handlerConfig["protected_files_policy"]
require.True(t, ok, "should have protected_files_policy field")
assert.Equal(t, "request-review", pfPolicy, "default protected_files_policy should use the hyphenated request-review form")
})
}
}

func TestPRPolicyFieldsExpressionsPassThrough(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -364,7 +421,7 @@ func TestCreatePullRequestProtectedFilesPolicyDefault(t *testing.T) {

handlerCfg, ok := config["create_pull_request"]
require.True(t, ok, "create_pull_request handler config should be present")
assert.Equal(t, "request_review", handlerCfg["protected_files_policy"], "default protected-files mode should be request_review")
assert.Equal(t, "request-review", handlerCfg["protected_files_policy"], "default protected-files mode should be request-review")
}

// TestDispatchWorkflowRelayInjectsDispatchCompatibleRef verifies that when a workflow_call
Expand Down
14 changes: 8 additions & 6 deletions pkg/workflow/compiler_safe_outputs_config_protection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,19 @@ func TestProtectedFilesExclude(t *testing.T) {
{
name: "exclude AGENTS.md from create-pull-request",
excludeFiles: []string{"AGENTS.md"},
wantExcludedFromPF: []string{"AGENTS.md"},
wantExcludedFromPF: []string{"AGENTS.md", "CHANGELOG.md"},
wantPresentInPF: []string{"package.json", "go.mod", "CODEOWNERS", "DESIGN.md"},
},
{
name: "exclude multiple files",
excludeFiles: []string{"AGENTS.md", "CLAUDE.md"},
wantExcludedFromPF: []string{"AGENTS.md", "CLAUDE.md"},
wantExcludedFromPF: []string{"AGENTS.md", "CLAUDE.md", "CHANGELOG.md"},
wantPresentInPF: []string{"package.json", "go.mod"},
},
{
name: "empty exclude list leaves defaults intact",
excludeFiles: nil,
wantExcludedFromPF: nil,
wantExcludedFromPF: []string{"CHANGELOG.md"},
wantPresentInPF: []string{"package.json", "go.mod"},
},
}
Expand Down Expand Up @@ -160,6 +160,7 @@ func TestProtectedFilesExcludePushToPRBranch(t *testing.T) {
}
}
assert.NotContains(t, pfStrings, "AGENTS.md", "AGENTS.md should be excluded from protected_files")
assert.NotContains(t, pfStrings, "CHANGELOG.md", "CHANGELOG.md should be excluded by default from protected_files")
assert.Contains(t, pfStrings, "package.json", "package.json should still be in protected_files")

// Dot-folder prefixes are no longer in protected_path_prefixes — they are
Expand Down Expand Up @@ -376,12 +377,13 @@ func TestHandlerConfigInjectsCurrentCheckoutPatchWorkspacePath(t *testing.T) {
}

// TestProtectTopLevelMdFiles verifies that well-known top-level Markdown files
// (README.md, CONTRIBUTING.md, CHANGELOG.md, SECURITY.md, CODE_OF_CONDUCT.md) are
// always included in the protected_files list in both handler configs.
// (README.md, CONTRIBUTING.md, SECURITY.md, CODE_OF_CONDUCT.md) are always
// included in the protected_files list in both handler configs, while CHANGELOG.md
// is intentionally excluded by default to allow routine changelog updates.
func TestProtectTopLevelMdFiles(t *testing.T) {
config := extractHandlerManagerConfigJSON(t)

expectedFiles := []string{"README.md", "CONTRIBUTING.md", "CHANGELOG.md", "SECURITY.md", "CODE_OF_CONDUCT.md"}
expectedFiles := []string{"README.md", "CONTRIBUTING.md", "SECURITY.md", "CODE_OF_CONDUCT.md"}
for _, handlerName := range []string{"create_pull_request", "push_to_pull_request_branch"} {
handlerCfg, ok := config[handlerName]
require.True(t, ok, "%s handler should be present", handlerName)
Expand Down
8 changes: 6 additions & 2 deletions pkg/workflow/create_pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@
CloseOlderConfig `yaml:",inline"` // Shared close-older settings; Enabled is sourced from close-older-pull-requests.
}

// parseCreatePullRequestsConfig handles only create-pull-request (singular) configuration
// parseCreatePullRequestsConfig handles only create-pull-request (singular) configuration.

Check failure on line 124 in pkg/workflow/create_pull_request.go

View workflow job for this annotation

GitHub 操作 / lint-go-golangci

File is not properly formatted (gofmt)
//nolint:funlen // Large config parser keeps all create-pull-request validation in one place for clarity.
func (c *Compiler) parseCreatePullRequestsConfig(outputMap map[string]any) *CreatePullRequestsConfig {
// Check for singular form only
if _, exists := outputMap["create-pull-request"]; !exists {
Expand Down Expand Up @@ -153,7 +154,10 @@
protectedFilesExclude = preprocessProtectedFilesField(configData, createPRLog)

// Validate protected-files string enum after object-form preprocessing.
validateStringEnumField(configData, "protected-files", []string{"blocked", "allowed", "fallback-to-issue", "request_review"}, createPRLog)
validateStringEnumField(configData, "protected-files", []string{"blocked", "allowed", "fallback-to-issue", "request_review", "request-review"}, createPRLog)
if value, ok := configData["protected-files"].(string); ok {
configData["protected-files"] = normaliseProtectedFilesPolicy(value)
}

// Pre-process patch-format: valid values are "bundle" (default) and "am".
validateStringEnumField(configData, "patch-format", []string{"am", "bundle"}, createPRLog)
Expand Down
5 changes: 3 additions & 2 deletions pkg/workflow/push_to_pull_request_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,11 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any)
exclude := preprocessProtectedFilesField(configMap, pushToPullRequestBranchLog)
pushToBranchConfig.ProtectedFilesExclude = exclude
// Validate policy string (no-op if the field was replaced by preprocessor)
manifestFilesEnums := []string{"blocked", "allowed", "fallback-to-issue"}
manifestFilesEnums := []string{"blocked", "allowed", "fallback-to-issue", "request_review", "request-review"}
validateStringEnumField(configMap, "protected-files", manifestFilesEnums, pushToPullRequestBranchLog)
if strVal, ok := configMap["protected-files"].(string); ok {
pushToBranchConfig.ManifestFilesPolicy = &strVal
normalised := normaliseProtectedFilesPolicy(strVal)
pushToBranchConfig.ManifestFilesPolicy = &normalised
Comment on lines +188 to +192
}

// Parse allowed-files: list of glob patterns forming a strict allowlist of eligible files
Expand Down
1 change: 0 additions & 1 deletion pkg/workflow/runtime_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ var securityConfigFiles = []string{
"DESIGN.md", // Captures design-system source of truth consumed by coding agents

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.

Dropping CHANGELOG.md from the default protected-file set is a behavior change with real security impact, but this PR only adjusts compiler-side tests. I can't find a matching runtime test that proves changelog-only patches are now intentionally allowed while other protected top-level docs still trigger protection. Without that coverage, this is one refactor away from becoming an accidental policy regression.

💡 What coverage is missing

The new Go tests only assert the generated config shape. They do not exercise the JS policy evaluator that actually decides whether a patch is blocked / fallback / request-review.

Please add a runtime-level test around checkFileProtection / checkFileProtectionPostApply showing at least:

// CHANGELOG is no longer protected by default
CHANGELOG.md   -> allow

// adjacent sensitive docs still remain protected
README.md      -> request_review or deny
.github/x.yml  -> request_review or deny

That makes the intended policy change explicit and prevents a future mismatch between compile-time config generation and enforcement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added runtime-level coverage in actions/setup/js/manifest_file_helpers.test.cjs: CHANGELOG.md is allowed with the PR handler's filtered protected-file set, while README.md still produces request_review under request-review policy. The shared manifest list continues to protect changelogs for other handlers.

"README.md", // Primary documentation file often imported by agents as context
"CONTRIBUTING.md", // Contribution guidelines; modifying could mislead contributors or agents
"CHANGELOG.md", // Release history; modification could misrepresent project state
"SECURITY.md", // 安全 policy; tampering could suppress vulnerability disclosure
"CODE_OF_CONDUCT.md", // Community conduct policy
}
Expand Down
Loading
Loading