diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 023fb1b5729..f6c2ae4499d 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -1511,7 +1511,7 @@ tools: # Guard policy: minimum required integrity level for repository access. Restricts # the agent to users with at least the specified permission level. # (optional) - min-integrity: "reader" + min-integrity: "unapproved" # GitHub App configuration for token minting. When configured, a GitHub App # installation access token is minted at workflow start and used instead of the diff --git a/docs/src/content/docs/reference/github-tools.md b/docs/src/content/docs/reference/github-tools.md index 6702c5c62a7..d9688ca868b 100644 --- a/docs/src/content/docs/reference/github-tools.md +++ b/docs/src/content/docs/reference/github-tools.md @@ -69,7 +69,7 @@ tools: mode: remote toolsets: [default] repos: "all" - min-integrity: reader + min-integrity: unapproved ``` Both `repos` and `min-integrity` are required when either is specified. @@ -96,19 +96,29 @@ tools: - "myorg/*" - "partner/shared-repo" - "myorg/api-*" - min-integrity: writer + min-integrity: approved ``` ### `min-integrity` -Sets the minimum integrity level required for repository access: +Sets the minimum integrity level required for repository access. -| Level | Description | -|-------|-------------| -| `none` | No integrity requirements | -| `reader` | Read-level integrity | -| `writer` | Write-level integrity | -| `merged` | Merged-level integrity | +#### Integrity Level Definitions + +Integrity levels are determined based on the combination of the `author_association` field associated with GitHub objects (issues, pull requests, comments, etc.) and whether an object is reachable from the main branch: + +| Level | Criteria | +|-------|----------| +| `merged` | Objects reachable from the main branch (regardless of authorship) | +| `approved` | Objects with `author_association` of `OWNER`, `MEMBER`, or `COLLABORATOR` | +| `unapproved` | Objects with `author_association` of `CONTRIBUTOR` or `FIRST_TIME_CONTRIBUTOR` | +| `none` | Objects with `author_association` of `FIRST_TIMER` or `NONE` | + +**How it works:** +- **Merged content** has the highest integrity because it has been reviewed and merged into the main branch +- **Approved contributors** (owners, members, collaborators) have established trust relationships with the repository +- **Unapproved contributors** have made contributions but lack formal repository access +- **None level** includes first-time interactions and users with no prior contribution history ### Examples @@ -131,7 +141,7 @@ tools: repos: - "frontend-org/*" - "backend-org/*" - min-integrity: writer + min-integrity: approved ``` ## Lockdown Mode for Public Repositories diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 6323a2a4521..b033d66e4b3 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3037,7 +3037,7 @@ "min-integrity": { "type": "string", "description": "Guard policy: minimum required integrity level for repository access. Restricts the agent to users with at least the specified permission level.", - "enum": ["reader", "writer", "maintainer", "admin"] + "enum": ["none", "unapproved", "approved", "merged"] }, "github-app": { "type": "object", diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index b35f6f69027..22f666171b3 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -268,10 +268,10 @@ type GitHubIntegrityLevel string const ( // GitHubIntegrityNone allows access with no integrity requirements GitHubIntegrityNone GitHubIntegrityLevel = "none" - // GitHubIntegrityReader requires read-level integrity - GitHubIntegrityReader GitHubIntegrityLevel = "reader" - // GitHubIntegrityWriter requires write-level integrity - GitHubIntegrityWriter GitHubIntegrityLevel = "writer" + // GitHubIntegrityUnapproved requires unapproved-level integrity + GitHubIntegrityUnapproved GitHubIntegrityLevel = "unapproved" + // GitHubIntegrityApproved requires approved-level integrity + GitHubIntegrityApproved GitHubIntegrityLevel = "approved" // GitHubIntegrityMerged requires merged-level integrity GitHubIntegrityMerged GitHubIntegrityLevel = "merged" ) diff --git a/pkg/workflow/tools_validation.go b/pkg/workflow/tools_validation.go index b5b8775911a..4b9ac9a6ec1 100644 --- a/pkg/workflow/tools_validation.go +++ b/pkg/workflow/tools_validation.go @@ -87,20 +87,20 @@ func validateGitHubGuardPolicy(tools *Tools, workflowName string) error { // Validate min-integrity field (required when repos is set) if !hasMinIntegrity { toolsValidationLog.Printf("Missing min-integrity in guard policy for workflow: %s", workflowName) - return errors.New("invalid guard policy: 'github.min-integrity' is required. Valid values: 'none', 'reader', 'writer', 'merged'") + return errors.New("invalid guard policy: 'github.min-integrity' is required. Valid values: 'none', 'unapproved', 'approved', 'merged'") } // Validate min-integrity value validIntegrityLevels := map[GitHubIntegrityLevel]bool{ - GitHubIntegrityNone: true, - GitHubIntegrityReader: true, - GitHubIntegrityWriter: true, - GitHubIntegrityMerged: true, + GitHubIntegrityNone: true, + GitHubIntegrityUnapproved: true, + GitHubIntegrityApproved: true, + GitHubIntegrityMerged: true, } if !validIntegrityLevels[github.MinIntegrity] { toolsValidationLog.Printf("Invalid min-integrity level '%s' in workflow: %s", github.MinIntegrity, workflowName) - return errors.New("invalid guard policy: 'github.min-integrity' must be one of: 'none', 'reader', 'writer', 'merged'. Got: '" + string(github.MinIntegrity) + "'") + return errors.New("invalid guard policy: 'github.min-integrity' must be one of: 'none', 'unapproved', 'approved', 'merged'. Got: '" + string(github.MinIntegrity) + "'") } return nil diff --git a/pkg/workflow/tools_validation_test.go b/pkg/workflow/tools_validation_test.go index 432faa366e5..0cae4b958ec 100644 --- a/pkg/workflow/tools_validation_test.go +++ b/pkg/workflow/tools_validation_test.go @@ -265,7 +265,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { toolsMap: map[string]any{ "github": map[string]any{ "repos": "all", - "min-integrity": "reader", + "min-integrity": "unapproved", }, }, shouldError: false, @@ -275,7 +275,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { toolsMap: map[string]any{ "github": map[string]any{ "repos": "public", - "min-integrity": "writer", + "min-integrity": "approved", }, }, shouldError: false, @@ -304,7 +304,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { name: "missing repos field", toolsMap: map[string]any{ "github": map[string]any{ - "min-integrity": "reader", + "min-integrity": "unapproved", }, }, shouldError: true, @@ -336,7 +336,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { toolsMap: map[string]any{ "github": map[string]any{ "repos": "private", - "min-integrity": "reader", + "min-integrity": "unapproved", }, }, shouldError: true, @@ -347,7 +347,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { toolsMap: map[string]any{ "github": map[string]any{ "repos": []any{}, - "min-integrity": "reader", + "min-integrity": "unapproved", }, }, shouldError: true, @@ -358,7 +358,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { toolsMap: map[string]any{ "github": map[string]any{ "repos": []any{"Owner/repo"}, - "min-integrity": "reader", + "min-integrity": "unapproved", }, }, shouldError: true, @@ -369,7 +369,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { toolsMap: map[string]any{ "github": map[string]any{ "repos": []any{"just-a-name"}, - "min-integrity": "reader", + "min-integrity": "unapproved", }, }, shouldError: true, diff --git a/scratchpad/dev.md b/scratchpad/dev.md index 1143c03f50b..5b94cac12c4 100644 --- a/scratchpad/dev.md +++ b/scratchpad/dev.md @@ -1706,7 +1706,7 @@ tools: mode: remote toolsets: [default] repos: "all" # "all", "public", or array of patterns - min-integrity: reader # none | reader | writer | merged + min-integrity: unapproved # none | unapproved | approved | merged ``` Both `repos` and `min-integrity` are required when either is specified under `github:`. @@ -1721,7 +1721,13 @@ Pattern validation rules: - Wildcards are only permitted at the end of the repo name segment - Empty arrays are not allowed -**Integrity Levels**: `none` | `reader` | `writer` | `merged` (case-sensitive) +**Integrity Levels**: `none` | `unapproved` | `approved` | `merged` (case-sensitive) + +Integrity levels are determined by the `author_association` field and main branch reachability: +- `merged`: Objects reachable from the main branch (highest integrity) +- `approved`: `author_association` of `OWNER`, `MEMBER`, or `COLLABORATOR` +- `unapproved`: `author_association` of `CONTRIBUTOR` or `FIRST_TIME_CONTRIBUTOR` +- `none`: `author_association` of `FIRST_TIMER` or `NONE` (lowest integrity) **Validation Location**: `pkg/workflow/tools_validation.go` — `validateGitHubGuardPolicy()` runs during workflow compilation via `compiler_orchestrator_workflow.go` and `compiler_string_api.go`. diff --git a/scratchpad/guard-policies-specification.md b/scratchpad/guard-policies-specification.md index 86ffbe795ee..ff73d677611 100644 --- a/scratchpad/guard-policies-specification.md +++ b/scratchpad/guard-policies-specification.md @@ -42,10 +42,13 @@ Based on the provided JSON schema, the implementation supports: - `"owner/prefix*"` - Repositories with name prefix under owner **Integrity Levels:** -- `"none"` - No min-integrity requirements -- `"reader"` - Read-level integrity -- `"writer"` - Write-level integrity -- `"merged"` - Merged-level integrity + +Integrity levels are based on the combination of the `author_association` field associated with GitHub objects and whether an object is reachable from the main branch: + +- `"merged"` - Objects reachable from the main branch (highest integrity, regardless of authorship) +- `"approved"` - Objects with `author_association` of `OWNER`, `MEMBER`, or `COLLABORATOR` +- `"unapproved"` - Objects with `author_association` of `CONTRIBUTOR` or `FIRST_TIME_CONTRIBUTOR` +- `"none"` - Objects with `author_association` of `FIRST_TIMER` or `NONE` (lowest integrity) ### 3. Frontmatter Syntax @@ -56,7 +59,7 @@ tools: mode: remote toolsets: [default] repos: "all" - min-integrity: reader + min-integrity: unapproved ``` **With Repository Patterns:** @@ -69,7 +72,7 @@ tools: - "myorg/*" - "partner/shared-repo" - "docs/api-*" - min-integrity: writer + min-integrity: approved ``` **Public Repositories Only:** @@ -89,7 +92,7 @@ tools: 2. **Validation** (`tools_validation.go`): - Validates repos format (all/public or valid patterns) - - Validates min-integrity level (none/reader/writer/merged) + - Validates min-integrity level (none/unapproved/approved/merged) - Validates repository pattern syntax (lowercase, valid characters, wildcard placement) - Called during workflow compilation @@ -173,7 +176,7 @@ The design supports future MCP servers (Jira, WorkIQ) through: - Empty arrays not allowed **Integrity Levels:** -- Must be one of: `none`, `reader`, `writer`, `merged` +- Must be one of: `none`, `unapproved`, `approved`, `merged` - Case-sensitive **Required Fields:** @@ -192,7 +195,7 @@ invalid guard policy: repository pattern 'Owner/Repo' must be lowercase invalid guard policy: repository pattern 'owner/re*po' has wildcard in the middle. Wildcards only allowed at the end (e.g., 'prefix*') -invalid guard policy: 'github.min-integrity' must be one of: 'none', 'reader', 'writer', 'merged'. +invalid guard policy: 'github.min-integrity' must be one of: 'none', 'unapproved', 'approved', 'merged'. Got: 'admin' ``` @@ -207,7 +210,7 @@ tools: toolsets: [default] repos: - "myorg/*" - min-integrity: reader + min-integrity: unapproved ``` ### Example 2: Multiple Organizations @@ -221,7 +224,7 @@ tools: - "frontend-org/*" - "backend-org/*" - "shared/infrastructure" - min-integrity: writer + min-integrity: approved ``` ### Example 3: Public Repositories Only @@ -245,7 +248,7 @@ tools: repos: - "myorg/api-*" # Matches api-gateway, api-service, etc. - "myorg/web-*" # Matches web-frontend, web-backend, etc. - min-integrity: writer + min-integrity: approved ``` ## Testing Strategy