From cf99bb63726332f99a02de451ed9a35fee0cc298 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 27 Aug 2026 16:07:15 +0200 Subject: [PATCH 1/6] feat(governance): add repository ruleset tools with multi-level scope challenge Reimplements the repository ruleset support from #821 (issue #820) onto the current inventory-based tool architecture, consolidated into two level-aware tools in a new non-default `governance` toolset. - `repository_ruleset_read`: read rulesets, branch rules, and rule suites at repository, organization, or enterprise level. - `create_repository_ruleset`: create a ruleset at any of the three levels. A `level` argument selects the scope, and a DynamicChallenge up-scopes the required OAuth scope accordingly (repo -> read:org/admin:org -> read:enterprise/admin:enterprise), so the default surface only asks for repo scope. Ruleset creation round-trips the request through go-github's RepositoryRuleset unmarshalling and rejects rule types, parameters, conditions, or bypass-actor keys that are silently dropped, preventing typos from creating a weaker-than-intended ruleset. Co-authored-by: Patrick Knight Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e886867-a922-419a-b02c-ac643716aea8 --- README.md | 51 + docs/remote-server.md | 1 + .../create_repository_ruleset.snap | 128 ++ .../repository_ruleset_read.snap | 109 ++ pkg/github/rulesets.go | 1003 ++++++++++++++ pkg/github/rulesets_test.go | 1180 +++++++++++++++++ pkg/github/tools.go | 9 + pkg/http/oauth/oauth_test.go | 3 + pkg/octicons/icons/law-dark.png | Bin 0 -> 550 bytes pkg/octicons/icons/law-light.png | Bin 0 -> 841 bytes pkg/octicons/icons_data_uris.txt | 2 + pkg/octicons/required_icons.txt | 1 + pkg/scopes/scopes.go | 24 +- 13 files changed, 2505 insertions(+), 6 deletions(-) create mode 100644 pkg/github/__toolsnaps__/create_repository_ruleset.snap create mode 100644 pkg/github/__toolsnaps__/repository_ruleset_read.snap create mode 100644 pkg/github/rulesets.go create mode 100644 pkg/github/rulesets_test.go create mode 100644 pkg/octicons/icons/law-dark.png create mode 100644 pkg/octicons/icons/law-light.png diff --git a/README.md b/README.md index 1b9f693a34..30ea93473f 100644 --- a/README.md +++ b/README.md @@ -586,6 +586,7 @@ The following sets of tools are available: | comment-discussion | `discussions` | GitHub Discussions related tools | | logo-gist | `gists` | GitHub Gist related tools | | git-branch | `git` | GitHub Git API related tools for low-level Git operations | +| law | `governance` | Repository governance tools for managing rulesets at the repository, organization, and enterprise levels | | issue-opened | `issues` | GitHub Issues related tools | | tag | `labels` | GitHub Labels related tools | | bell | `notifications` | GitHub Notifications related tools | @@ -872,6 +873,56 @@ The following sets of tools are available:
+law Governance + +- **create_repository_ruleset** - Create repository ruleset + - **OAuth Challenge Scopes**: `repo`, `admin:org`, `admin:enterprise` + - `bypass_actors`: The actors that can bypass the rules in this ruleset (object[], optional) + - `conditions`: Conditions for when this ruleset applies, e.g. {"ref_name": {"include": ["refs/heads/main"], "exclude": []}} (object, optional) + - `enforcement`: The enforcement level of the ruleset. 'evaluate' allows admins to test rules before enforcing them (string, required) + - `enterprise`: Enterprise slug. Required when level is 'enterprise'. (string, optional) + - `level`: The level at which the ruleset is configured: + - 'repository': A ruleset on a single repository (requires 'owner' and 'repo'). + - 'organization': A ruleset covering repositories in an organization (requires 'org'). + - 'enterprise': A ruleset covering repositories across an enterprise (requires 'enterprise'). (string, required) + - `name`: The name of the ruleset (string, required) + - `org`: Organization name. Required when level is 'organization'. (string, optional) + - `owner`: Repository owner. Required when level is 'repository'. (string, optional) + - `repo`: Repository name. Required when level is 'repository'. (string, optional) + - `rules`: An array of rules within the ruleset. Each rule is an object with a 'type' (e.g. 'creation', 'deletion', 'non_fast_forward', 'required_signatures', 'pull_request', 'required_status_checks') and, for rules that need configuration, a 'parameters' object (object[], required) + - `target`: The target of the ruleset. Defaults to 'branch'. 'repository' is only valid for 'organization' and 'enterprise' level rulesets. (string, optional) + +- **repository_ruleset_read** - Read repository rulesets + - **OAuth Challenge Scopes**: `repo`, `read:org`, `read:enterprise` + - `actor_name`: The handle for the GitHub user account to filter rule suites on. Used by the 'list_rule_suites' method. (string, optional) + - `branch`: Branch name. Required for the 'get_rules_for_branch' method. (string, optional) + - `enterprise`: Enterprise slug. Required when level is 'enterprise'. (string, optional) + - `includes_parents`: Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods at the repository level. (boolean, optional) + - `level`: The level at which the ruleset is configured: + - 'repository': A ruleset on a single repository (requires 'owner' and 'repo'). + - 'organization': A ruleset covering repositories in an organization (requires 'org'). + - 'enterprise': A ruleset covering repositories across an enterprise (requires 'enterprise'). (string, required) + - `method`: Operation to perform: + - 'get': Get a specific ruleset by ID (requires 'ruleset_id'). Supported at every level. + - 'list': List all rulesets. Supported at every level. + - 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). Repository level only. + - 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository level only. + - 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository level only. (string, required) + - `org`: Organization name. Required when level is 'organization'. (string, optional) + - `owner`: Repository owner. Required when level is 'repository'. (string, optional) + - `page`: Page number for pagination (min 1) (number, optional) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `ref`: The name of the ref (branch, tag, etc.) to filter rule suites by. Used by the 'list_rule_suites' method. (string, optional) + - `repo`: Repository name. Required when level is 'repository'. (string, optional) + - `rule_suite_id`: Rule suite ID. Required for the 'get_rule_suite' method. (number, optional) + - `rule_suite_result`: The rule suite result to filter by. Used by the 'list_rule_suites' method. (string, optional) + - `ruleset_id`: Ruleset ID. Required for the 'get' method. (number, optional) + - `time_period`: The time period to filter rule suites by. Used by the 'list_rule_suites' method. (string, optional) + +
+ +
+ issue-opened Issues - **add_issue_comment** - Add comment to issue or pull request diff --git a/docs/remote-server.md b/docs/remote-server.md index d8587a3116..78b5c906ac 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -30,6 +30,7 @@ Below is a table of available toolsets for the remote GitHub MCP Server. Each to | comment-discussion
`discussions` | GitHub Discussions related tools | https://api.githubcopilot.com/mcp/x/discussions | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-discussions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdiscussions%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/discussions/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-discussions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdiscussions%2Freadonly%22%7D) | | logo-gist
`gists` | GitHub Gist related tools | https://api.githubcopilot.com/mcp/x/gists | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/gists/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%2Freadonly%22%7D) | | git-branch
`git` | GitHub Git API related tools for low-level Git operations | https://api.githubcopilot.com/mcp/x/git | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-git&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgit%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/git/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-git&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgit%2Freadonly%22%7D) | +| law
`governance` | Repository governance tools for managing rulesets at the repository, organization, and enterprise levels | https://api.githubcopilot.com/mcp/x/governance | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-governance&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgovernance%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/governance/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-governance&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgovernance%2Freadonly%22%7D) | | issue-opened
`issues` | GitHub Issues related tools | https://api.githubcopilot.com/mcp/x/issues | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/issues/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%2Freadonly%22%7D) | | tag
`labels` | GitHub Labels related tools | https://api.githubcopilot.com/mcp/x/labels | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-labels&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Flabels%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/labels/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-labels&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Flabels%2Freadonly%22%7D) | | bell
`notifications` | GitHub Notifications related tools | https://api.githubcopilot.com/mcp/x/notifications | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/notifications/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%2Freadonly%22%7D) | diff --git a/pkg/github/__toolsnaps__/create_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_repository_ruleset.snap new file mode 100644 index 0000000000..8ee7fbd724 --- /dev/null +++ b/pkg/github/__toolsnaps__/create_repository_ruleset.snap @@ -0,0 +1,128 @@ +{ + "annotations": { + "idempotentHint": false, + "readOnlyHint": false, + "title": "Create repository ruleset" + }, + "description": "Create a new ruleset at the repository, organization, or enterprise level", + "inputSchema": { + "properties": { + "bypass_actors": { + "description": "The actors that can bypass the rules in this ruleset", + "items": { + "properties": { + "actor_id": { + "description": "The ID of the actor that can bypass a ruleset", + "type": "number" + }, + "actor_type": { + "description": "The type of actor that can bypass a ruleset. 'EnterpriseOwner' and 'EnterpriseRole' are only valid for 'enterprise' level rulesets.", + "enum": [ + "Integration", + "OrganizationAdmin", + "RepositoryRole", + "Team", + "DeployKey", + "User", + "EnterpriseOwner", + "EnterpriseRole" + ], + "type": "string" + }, + "bypass_mode": { + "description": "When the specified actor can bypass the ruleset. 'pull_request' only applies to branch rulesets and is not valid for the 'DeployKey' actor type. 'exempt' means rules are not run for that actor and no bypass audit entry is created.", + "enum": [ + "always", + "pull_request", + "exempt" + ], + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + }, + "conditions": { + "description": "Conditions for when this ruleset applies, e.g. {\"ref_name\": {\"include\": [\"refs/heads/main\"], \"exclude\": []}}", + "type": "object" + }, + "enforcement": { + "description": "The enforcement level of the ruleset. 'evaluate' allows admins to test rules before enforcing them", + "enum": [ + "disabled", + "active", + "evaluate" + ], + "type": "string" + }, + "enterprise": { + "description": "Enterprise slug. Required when level is 'enterprise'.", + "type": "string" + }, + "level": { + "description": "The level at which the ruleset is configured:\n- 'repository': A ruleset on a single repository (requires 'owner' and 'repo').\n- 'organization': A ruleset covering repositories in an organization (requires 'org').\n- 'enterprise': A ruleset covering repositories across an enterprise (requires 'enterprise').", + "enum": [ + "repository", + "organization", + "enterprise" + ], + "type": "string" + }, + "name": { + "description": "The name of the ruleset", + "type": "string" + }, + "org": { + "description": "Organization name. Required when level is 'organization'.", + "type": "string" + }, + "owner": { + "description": "Repository owner. Required when level is 'repository'.", + "type": "string" + }, + "repo": { + "description": "Repository name. Required when level is 'repository'.", + "type": "string" + }, + "rules": { + "description": "An array of rules within the ruleset. Each rule is an object with a 'type' (e.g. 'creation', 'deletion', 'non_fast_forward', 'required_signatures', 'pull_request', 'required_status_checks') and, for rules that need configuration, a 'parameters' object", + "items": { + "properties": { + "parameters": { + "description": "Parameters for rule types that require additional configuration", + "type": "object" + }, + "type": { + "description": "The type of rule, e.g. 'creation', 'deletion', 'non_fast_forward', 'required_signatures', 'pull_request', 'required_status_checks'", + "type": "string" + } + }, + "required": [ + "type" + ], + "type": "object" + }, + "type": "array" + }, + "target": { + "description": "The target of the ruleset. Defaults to 'branch'. 'repository' is only valid for 'organization' and 'enterprise' level rulesets.", + "enum": [ + "branch", + "tag", + "push", + "repository" + ], + "type": "string" + } + }, + "required": [ + "level", + "name", + "enforcement", + "rules" + ], + "type": "object" + }, + "name": "create_repository_ruleset" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/repository_ruleset_read.snap b/pkg/github/__toolsnaps__/repository_ruleset_read.snap new file mode 100644 index 0000000000..2a79b4cfc9 --- /dev/null +++ b/pkg/github/__toolsnaps__/repository_ruleset_read.snap @@ -0,0 +1,109 @@ +{ + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "Read repository rulesets" + }, + "description": "Read rulesets and rule suites at the repository, organization, or enterprise level. Select the level with the 'level' parameter and the operation with the 'method' parameter.", + "inputSchema": { + "properties": { + "actor_name": { + "description": "The handle for the GitHub user account to filter rule suites on. Used by the 'list_rule_suites' method.", + "type": "string" + }, + "branch": { + "description": "Branch name. Required for the 'get_rules_for_branch' method.", + "type": "string" + }, + "enterprise": { + "description": "Enterprise slug. Required when level is 'enterprise'.", + "type": "string" + }, + "includes_parents": { + "description": "Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods at the repository level.", + "type": "boolean" + }, + "level": { + "description": "The level at which the ruleset is configured:\n- 'repository': A ruleset on a single repository (requires 'owner' and 'repo').\n- 'organization': A ruleset covering repositories in an organization (requires 'org').\n- 'enterprise': A ruleset covering repositories across an enterprise (requires 'enterprise').", + "enum": [ + "repository", + "organization", + "enterprise" + ], + "type": "string" + }, + "method": { + "description": "Operation to perform:\n- 'get': Get a specific ruleset by ID (requires 'ruleset_id'). Supported at every level.\n- 'list': List all rulesets. Supported at every level.\n- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). Repository level only.\n- 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository level only.\n- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository level only.", + "enum": [ + "get", + "list", + "get_rules_for_branch", + "list_rule_suites", + "get_rule_suite" + ], + "type": "string" + }, + "org": { + "description": "Organization name. Required when level is 'organization'.", + "type": "string" + }, + "owner": { + "description": "Repository owner. Required when level is 'repository'.", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "ref": { + "description": "The name of the ref (branch, tag, etc.) to filter rule suites by. Used by the 'list_rule_suites' method.", + "type": "string" + }, + "repo": { + "description": "Repository name. Required when level is 'repository'.", + "type": "string" + }, + "rule_suite_id": { + "description": "Rule suite ID. Required for the 'get_rule_suite' method.", + "type": "number" + }, + "rule_suite_result": { + "description": "The rule suite result to filter by. Used by the 'list_rule_suites' method.", + "enum": [ + "pass", + "fail", + "bypass", + "all" + ], + "type": "string" + }, + "ruleset_id": { + "description": "Ruleset ID. Required for the 'get' method.", + "type": "number" + }, + "time_period": { + "description": "The time period to filter rule suites by. Used by the 'list_rule_suites' method.", + "enum": [ + "hour", + "day", + "week", + "month" + ], + "type": "string" + } + }, + "required": [ + "level", + "method" + ], + "type": "object" + }, + "name": "repository_ruleset_read" +} \ No newline at end of file diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go new file mode 100644 index 0000000000..96c1ff66e5 --- /dev/null +++ b/pkg/github/rulesets.go @@ -0,0 +1,1003 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "strconv" + "strings" + + ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/go-github/v89/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// rulesetLevelDescription documents the "level" parameter shared by the +// ruleset read and write tools. +const rulesetLevelDescription = "The level at which the ruleset is configured:\n" + + "- 'repository': A ruleset on a single repository (requires 'owner' and 'repo').\n" + + "- 'organization': A ruleset covering repositories in an organization (requires 'org').\n" + + "- 'enterprise': A ruleset covering repositories across an enterprise (requires 'enterprise')." + +// rulesetReadScopeAccess declares the exhaustive scope challenge policy for +// repository_ruleset_read. The exact scope challenged depends on the "level" +// argument: repository reads need "repo", organization reads need "read:org", +// and enterprise reads need "read:enterprise". A missing or unrecognized +// level returns no challenge so normal handler validation produces the error. +func rulesetReadScopeAccess() inventory.ScopeAccess { + return scopes.DynamicChallenge( + []scopes.Scope{scopes.Repo, scopes.ReadOrg, scopes.ReadEnterprise}, + func([]string) bool { + // Repository-level reads may target public repositories, so the + // tool stays visible even for tokens without any of these scopes. + return true + }, + func(arguments map[string]any, activeScopes []string) []string { + level, ok := arguments["level"].(string) + if !ok { + return nil + } + switch level { + case "repository": + return scopes.ChallengeAll(activeScopes, scopes.Repo) + case "organization": + return scopes.ChallengeAll(activeScopes, scopes.ReadOrg) + case "enterprise": + return scopes.ChallengeAll(activeScopes, scopes.ReadEnterprise) + default: + return nil + } + }, + ) +} + +// rulesetWriteScopeAccess declares the exhaustive scope challenge policy for +// create_repository_ruleset. The exact scope challenged depends on the +// "level" argument: repository writes need "repo", organization writes need +// "admin:org", and enterprise writes need "admin:enterprise". A missing or +// unrecognized level returns no challenge so normal handler validation +// produces the error. +func rulesetWriteScopeAccess() inventory.ScopeAccess { + return scopes.DynamicChallenge( + []scopes.Scope{scopes.Repo, scopes.AdminOrg, scopes.AdminEnterprise}, + func([]string) bool { return true }, + func(arguments map[string]any, activeScopes []string) []string { + level, ok := arguments["level"].(string) + if !ok { + return nil + } + switch level { + case "repository": + return scopes.ChallengeAll(activeScopes, scopes.Repo) + case "organization": + return scopes.ChallengeAll(activeScopes, scopes.AdminOrg) + case "enterprise": + return scopes.ChallengeAll(activeScopes, scopes.AdminEnterprise) + default: + return nil + } + }, + ) +} + +// RepositoryRulesetRead creates a tool for read operations on rulesets and +// rule suites at the repository, organization, or enterprise level. The +// level is selected with the "level" parameter and the operation with the +// "method" parameter. +func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataGovernance, + mcp.Tool{ + Name: "repository_ruleset_read", + Description: t("TOOL_REPOSITORY_RULESET_READ_DESCRIPTION", "Read rulesets and rule suites at the repository, organization, or enterprise level. Select the level with the 'level' parameter and the operation with the 'method' parameter."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REPOSITORY_RULESET_READ_USER_TITLE", "Read repository rulesets"), + ReadOnlyHint: true, + }, + InputSchema: WithPagination(&jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "level": { + Type: "string", + Enum: []any{"repository", "organization", "enterprise"}, + Description: rulesetLevelDescription, + }, + "method": { + Type: "string", + Enum: []any{"get", "list", "get_rules_for_branch", "list_rule_suites", "get_rule_suite"}, + Description: "Operation to perform:\n" + + "- 'get': Get a specific ruleset by ID (requires 'ruleset_id'). Supported at every level.\n" + + "- 'list': List all rulesets. Supported at every level.\n" + + "- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). Repository level only.\n" + + "- 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository level only.\n" + + "- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository level only.", + }, + "owner": { + Type: "string", + Description: "Repository owner. Required when level is 'repository'.", + }, + "repo": { + Type: "string", + Description: "Repository name. Required when level is 'repository'.", + }, + "org": { + Type: "string", + Description: "Organization name. Required when level is 'organization'.", + }, + "enterprise": { + Type: "string", + Description: "Enterprise slug. Required when level is 'enterprise'.", + }, + "ruleset_id": { + Type: "number", + Description: "Ruleset ID. Required for the 'get' method.", + }, + "includes_parents": { + Type: "boolean", + Description: "Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods at the repository level.", + }, + "branch": { + Type: "string", + Description: "Branch name. Required for the 'get_rules_for_branch' method.", + }, + "ref": { + Type: "string", + Description: "The name of the ref (branch, tag, etc.) to filter rule suites by. Used by the 'list_rule_suites' method.", + }, + "time_period": { + Type: "string", + Enum: []any{"hour", "day", "week", "month"}, + Description: "The time period to filter rule suites by. Used by the 'list_rule_suites' method.", + }, + "actor_name": { + Type: "string", + Description: "The handle for the GitHub user account to filter rule suites on. Used by the 'list_rule_suites' method.", + }, + "rule_suite_result": { + Type: "string", + Enum: []any{"pass", "fail", "bypass", "all"}, + Description: "The rule suite result to filter by. Used by the 'list_rule_suites' method.", + }, + "rule_suite_id": { + Type: "number", + Description: "Rule suite ID. Required for the 'get_rule_suite' method.", + }, + }, + Required: []string{"level", "method"}, + }), + }, + rulesetReadScopeAccess(), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + level, err := RequiredParam[string](args, "level") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + method, err := RequiredParam[string](args, "method") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + switch level { + case "repository": + return repositoryRulesetReadRepository(ctx, client, strings.ToLower(method), args) + case "organization": + return repositoryRulesetReadOrganization(ctx, client, strings.ToLower(method), args) + case "enterprise": + return repositoryRulesetReadEnterprise(ctx, client, strings.ToLower(method), args) + default: + return utils.NewToolResultError(fmt.Sprintf("unknown level: %q (expected 'repository', 'organization', or 'enterprise')", level)), nil, nil + } + }, + ) +} + +// repositoryRulesetReadRepository handles repository_ruleset_read calls with level="repository". +func repositoryRulesetReadRepository(ctx context.Context, client *github.Client, method string, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + switch method { + case "get": + rulesetID, err := RequiredBigInt(args, "ruleset_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + // GetRuleset always sends includes_parents; default to the + // GitHub API default of true when the caller omits it. + includesParents := true + if _, ok := args["includes_parents"]; ok { + includesParents, err = OptionalParam[bool](args, "includes_parents") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + } + result, err := GetRepositoryRuleset(ctx, client, owner, repo, rulesetID, includesParents) + return result, nil, err + case "list": + pagination, err := OptionalPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + var includesParents *bool + if _, ok := args["includes_parents"]; ok { + v, err := OptionalParam[bool](args, "includes_parents") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + includesParents = &v + } + result, err := ListRepositoryRulesets(ctx, client, owner, repo, includesParents, pagination) + return result, nil, err + case "get_rules_for_branch": + branch, err := RequiredParam[string](args, "branch") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pagination, err := OptionalPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := GetRepositoryRulesForBranch(ctx, client, owner, repo, branch, pagination) + return result, nil, err + case "list_rule_suites": + filters, err := ruleSuiteFiltersFromArgs(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pagination, err := OptionalPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := ListRepositoryRuleSuites(ctx, client, owner, repo, filters, pagination) + return result, nil, err + case "get_rule_suite": + ruleSuiteID, err := RequiredBigInt(args, "rule_suite_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := GetRepositoryRuleSuite(ctx, client, owner, repo, ruleSuiteID) + return result, nil, err + default: + return utils.NewToolResultError(fmt.Sprintf("unknown method: %q", method)), nil, nil + } +} + +// repositoryRulesetReadOrganization handles repository_ruleset_read calls with level="organization". +func repositoryRulesetReadOrganization(ctx context.Context, client *github.Client, method string, args map[string]any) (*mcp.CallToolResult, any, error) { + org, err := RequiredParam[string](args, "org") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + switch method { + case "get": + rulesetID, err := RequiredBigInt(args, "ruleset_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := GetOrganizationRepositoryRuleset(ctx, client, org, rulesetID) + return result, nil, err + case "list": + pagination, err := OptionalPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := ListOrganizationRepositoryRulesets(ctx, client, org, pagination) + return result, nil, err + default: + return utils.NewToolResultError(fmt.Sprintf("method %q is not supported for level \"organization\"; supported methods: get, list", method)), nil, nil + } +} + +// repositoryRulesetReadEnterprise handles repository_ruleset_read calls with level="enterprise". +func repositoryRulesetReadEnterprise(ctx context.Context, client *github.Client, method string, args map[string]any) (*mcp.CallToolResult, any, error) { + enterprise, err := RequiredParam[string](args, "enterprise") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + switch method { + case "get": + rulesetID, err := RequiredBigInt(args, "ruleset_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := GetEnterpriseRepositoryRuleset(ctx, client, enterprise, rulesetID) + return result, nil, err + case "list": + pagination, err := OptionalPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := ListEnterpriseRepositoryRulesets(ctx, client, enterprise, pagination) + return result, nil, err + default: + return utils.NewToolResultError(fmt.Sprintf("method %q is not supported for level \"enterprise\"; supported methods: get, list", method)), nil, nil + } +} + +// GetRepositoryRuleset gets a specific repository ruleset by ID. +func GetRepositoryRuleset(ctx context.Context, client *github.Client, owner, repo string, rulesetID int64, includesParents bool) (*mcp.CallToolResult, error) { + ruleset, resp, err := client.Repositories.GetRuleset(ctx, owner, repo, rulesetID, includesParents) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository ruleset", resp, err), nil + } + + return MarshalledTextResult(ruleset), nil +} + +// ListRepositoryRulesets lists all rulesets for a repository. When +// includesParents is nil GitHub's default behaviour (include parents) is used. +func ListRepositoryRulesets(ctx context.Context, client *github.Client, owner, repo string, includesParents *bool, pagination PaginationParams) (*mcp.CallToolResult, error) { + opts := &github.RepositoryListRulesetsOptions{ + ListOptions: github.ListOptions{ + Page: pagination.Page, + PerPage: pagination.PerPage, + }, + IncludesParents: includesParents, + } + + rulesets, resp, err := client.Repositories.GetAllRulesets(ctx, owner, repo, opts) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rulesets", resp, err), nil + } + + return MarshalledTextResult(rulesets), nil +} + +// GetRepositoryRulesForBranch gets all rules that apply to a specific branch. +func GetRepositoryRulesForBranch(ctx context.Context, client *github.Client, owner, repo, branch string, pagination PaginationParams) (*mcp.CallToolResult, error) { + opts := &github.ListOptions{ + Page: pagination.Page, + PerPage: pagination.PerPage, + } + + branchRules, resp, err := client.Repositories.ListRulesForBranch(ctx, owner, repo, branch, opts) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rules for branch", resp, err), nil + } + + return MarshalledTextResult(branchRules), nil +} + +// ruleSuiteFilters holds the optional filters for listing rule suites. +type ruleSuiteFilters struct { + Ref string + TimePeriod string + ActorName string + RuleSuiteResult string +} + +func ruleSuiteFiltersFromArgs(args map[string]any) (ruleSuiteFilters, error) { + ref, err := OptionalParam[string](args, "ref") + if err != nil { + return ruleSuiteFilters{}, err + } + timePeriod, err := OptionalParam[string](args, "time_period") + if err != nil { + return ruleSuiteFilters{}, err + } + actorName, err := OptionalParam[string](args, "actor_name") + if err != nil { + return ruleSuiteFilters{}, err + } + ruleSuiteResult, err := OptionalParam[string](args, "rule_suite_result") + if err != nil { + return ruleSuiteFilters{}, err + } + return ruleSuiteFilters{ + Ref: ref, + TimePeriod: timePeriod, + ActorName: actorName, + RuleSuiteResult: ruleSuiteResult, + }, nil +} + +// ListRepositoryRuleSuites lists rule suites (evaluations of rules against +// pushes) for a repository. Rule suites are not supported by go-github, so the +// request is issued directly. +func ListRepositoryRuleSuites(ctx context.Context, client *github.Client, owner, repo string, filters ruleSuiteFilters, pagination PaginationParams) (*mcp.CallToolResult, error) { + apiURL := fmt.Sprintf("repos/%s/%s/rulesets/rule-suites", owner, repo) + query := url.Values{} + if filters.Ref != "" { + query.Set("ref", filters.Ref) + } + if filters.TimePeriod != "" { + query.Set("time_period", filters.TimePeriod) + } + if filters.ActorName != "" { + query.Set("actor_name", filters.ActorName) + } + if filters.RuleSuiteResult != "" { + query.Set("rule_suite_result", filters.RuleSuiteResult) + } + if pagination.Page > 0 { + query.Set("page", strconv.Itoa(pagination.Page)) + } + if pagination.PerPage > 0 { + query.Set("per_page", strconv.Itoa(pagination.PerPage)) + } + if len(query) > 0 { + apiURL += "?" + query.Encode() + } + + req, err := client.NewRequest(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil + } + + var ruleSuites any + resp, err := client.Do(req, &ruleSuites) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rule suites", resp, err), nil + } + + return MarshalledTextResult(ruleSuites), nil +} + +// GetRepositoryRuleSuite gets details of a specific repository rule suite, +// including the evaluation results for each rule. Rule suites are not supported +// by go-github, so the request is issued directly. +func GetRepositoryRuleSuite(ctx context.Context, client *github.Client, owner, repo string, ruleSuiteID int64) (*mcp.CallToolResult, error) { + apiURL := fmt.Sprintf("repos/%s/%s/rulesets/rule-suites/%d", owner, repo, ruleSuiteID) + req, err := client.NewRequest(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil + } + + var ruleSuite any + resp, err := client.Do(req, &ruleSuite) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rule suite", resp, err), nil + } + + return MarshalledTextResult(ruleSuite), nil +} + +// GetOrganizationRepositoryRuleset gets a specific organization repository +// ruleset by ID. +func GetOrganizationRepositoryRuleset(ctx context.Context, client *github.Client, org string, rulesetID int64) (*mcp.CallToolResult, error) { + ruleset, resp, err := client.Organizations.GetRepositoryRuleset(ctx, org, rulesetID) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get organization repository ruleset", resp, err), nil + } + + return MarshalledTextResult(ruleset), nil +} + +// ListOrganizationRepositoryRulesets lists all repository rulesets for an +// organization. +func ListOrganizationRepositoryRulesets(ctx context.Context, client *github.Client, org string, pagination PaginationParams) (*mcp.CallToolResult, error) { + opts := &github.ListOptions{ + Page: pagination.Page, + PerPage: pagination.PerPage, + } + + rulesets, resp, err := client.Organizations.ListAllRepositoryRulesets(ctx, org, opts) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list organization repository rulesets", resp, err), nil + } + + return MarshalledTextResult(rulesets), nil +} + +// GetEnterpriseRepositoryRuleset gets a specific enterprise repository +// ruleset by ID. +func GetEnterpriseRepositoryRuleset(ctx context.Context, client *github.Client, enterprise string, rulesetID int64) (*mcp.CallToolResult, error) { + ruleset, resp, err := client.Enterprise.GetRepositoryRuleset(ctx, enterprise, rulesetID) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get enterprise repository ruleset", resp, err), nil + } + + return MarshalledTextResult(ruleset), nil +} + +// ListEnterpriseRepositoryRulesets lists all repository rulesets for an +// enterprise. Listing enterprise rulesets is not supported by go-github, so +// the request is issued directly. +func ListEnterpriseRepositoryRulesets(ctx context.Context, client *github.Client, enterprise string, pagination PaginationParams) (*mcp.CallToolResult, error) { + apiURL := fmt.Sprintf("enterprises/%s/rulesets", enterprise) + query := url.Values{} + if pagination.Page > 0 { + query.Set("page", strconv.Itoa(pagination.Page)) + } + if pagination.PerPage > 0 { + query.Set("per_page", strconv.Itoa(pagination.PerPage)) + } + if len(query) > 0 { + apiURL += "?" + query.Encode() + } + + req, err := client.NewRequest(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil + } + + var rulesets any + resp, err := client.Do(req, &rulesets) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list enterprise repository rulesets", resp, err), nil + } + + return MarshalledTextResult(rulesets), nil +} + +// CreateRepositoryRuleset creates a tool to create a new repository ruleset +// at the repository, organization, or enterprise level. The level is +// selected with the "level" parameter. +func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.ServerTool { + properties := rulesetWriteProperties() + properties["level"] = &jsonschema.Schema{ + Type: "string", + Enum: []any{"repository", "organization", "enterprise"}, + Description: rulesetLevelDescription, + } + properties["owner"] = &jsonschema.Schema{Type: "string", Description: "Repository owner. Required when level is 'repository'."} + properties["repo"] = &jsonschema.Schema{Type: "string", Description: "Repository name. Required when level is 'repository'."} + properties["org"] = &jsonschema.Schema{Type: "string", Description: "Organization name. Required when level is 'organization'."} + properties["enterprise"] = &jsonschema.Schema{Type: "string", Description: "Enterprise slug. Required when level is 'enterprise'."} + + return NewTool( + ToolsetMetadataGovernance, + mcp.Tool{ + Name: "create_repository_ruleset", + Description: t("TOOL_CREATE_REPOSITORY_RULESET_DESCRIPTION", "Create a new ruleset at the repository, organization, or enterprise level"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_CREATE_REPOSITORY_RULESET_USER_TITLE", "Create repository ruleset"), + ReadOnlyHint: false, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: properties, + Required: []string{"level", "name", "enforcement", "rules"}, + }, + }, + rulesetWriteScopeAccess(), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + level, err := RequiredParam[string](args, "level") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + ruleset, errResult := buildRepositoryRulesetFromArgs(args) + if errResult != nil { + return errResult, nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + switch level { + case "repository": + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + created, resp, err := client.Repositories.CreateRuleset(ctx, owner, repo, ruleset) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create repository ruleset", resp, err), nil, nil + } + return MarshalledTextResult(created), nil, nil + case "organization": + org, err := RequiredParam[string](args, "org") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + created, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, org, ruleset) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create organization repository ruleset", resp, err), nil, nil + } + return MarshalledTextResult(created), nil, nil + case "enterprise": + enterprise, err := RequiredParam[string](args, "enterprise") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + created, resp, err := client.Enterprise.CreateRepositoryRuleset(ctx, enterprise, ruleset) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create enterprise repository ruleset", resp, err), nil, nil + } + return MarshalledTextResult(created), nil, nil + default: + return utils.NewToolResultError(fmt.Sprintf("unknown level: %q (expected 'repository', 'organization', or 'enterprise')", level)), nil, nil + } + }, + ) +} + +// rulesetWriteProperties returns the shared input schema properties for the +// ruleset creation tool. Callers add the level-specific identifier +// properties (owner/repo, org, or enterprise). +func rulesetWriteProperties() map[string]*jsonschema.Schema { + return map[string]*jsonschema.Schema{ + "name": { + Type: "string", + Description: "The name of the ruleset", + }, + "enforcement": { + Type: "string", + Enum: []any{"disabled", "active", "evaluate"}, + Description: "The enforcement level of the ruleset. 'evaluate' allows admins to test rules before enforcing them", + }, + "target": { + Type: "string", + Enum: []any{"branch", "tag", "push", "repository"}, + Description: "The target of the ruleset. Defaults to 'branch'. 'repository' is only valid for 'organization' and 'enterprise' level rulesets.", + }, + "rules": { + Type: "array", + Description: "An array of rules within the ruleset. Each rule is an object with a 'type' (e.g. 'creation', 'deletion', 'non_fast_forward', 'required_signatures', 'pull_request', 'required_status_checks') and, for rules that need configuration, a 'parameters' object", + Items: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "type": { + Type: "string", + Description: "The type of rule, e.g. 'creation', 'deletion', 'non_fast_forward', 'required_signatures', 'pull_request', 'required_status_checks'", + }, + "parameters": { + Type: "object", + Description: "Parameters for rule types that require additional configuration", + }, + }, + Required: []string{"type"}, + }, + }, + "conditions": { + Type: "object", + Description: "Conditions for when this ruleset applies, e.g. {\"ref_name\": {\"include\": [\"refs/heads/main\"], \"exclude\": []}}", + }, + "bypass_actors": { + Type: "array", + Description: "The actors that can bypass the rules in this ruleset", + Items: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "actor_id": { + Type: "number", + Description: "The ID of the actor that can bypass a ruleset", + }, + "actor_type": { + Type: "string", + Enum: []any{"Integration", "OrganizationAdmin", "RepositoryRole", "Team", "DeployKey", "User", "EnterpriseOwner", "EnterpriseRole"}, + Description: "The type of actor that can bypass a ruleset. 'EnterpriseOwner' and 'EnterpriseRole' are only valid for 'enterprise' level rulesets.", + }, + "bypass_mode": { + Type: "string", + Enum: []any{"always", "pull_request", "exempt"}, + Description: "When the specified actor can bypass the ruleset. 'pull_request' only applies to branch rulesets and is not valid for the 'DeployKey' actor type. 'exempt' means rules are not run for that actor and no bypass audit entry is created.", + }, + }, + }, + }, + } +} + +// buildRepositoryRulesetFromArgs assembles a github.RepositoryRuleset from the +// shared ruleset creation arguments. It returns a non-nil *mcp.CallToolResult +// describing the problem when the arguments are invalid. +func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRuleset, *mcp.CallToolResult) { + name, err := RequiredParam[string](args, "name") + if err != nil { + return github.RepositoryRuleset{}, utils.NewToolResultError(err.Error()) + } + enforcement, err := RequiredParam[string](args, "enforcement") + if err != nil { + return github.RepositoryRuleset{}, utils.NewToolResultError(err.Error()) + } + target, err := OptionalParam[string](args, "target") + if err != nil { + return github.RepositoryRuleset{}, utils.NewToolResultError(err.Error()) + } + + rules, ok := args["rules"].([]any) + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError("rules parameter must be an array of rule objects") + } + + requestedRuleTypes := make([]string, 0, len(rules)) + requestedRuleParameters := make(map[string]map[string]any, len(rules)) + seenRuleTypes := make(map[string]bool, len(rules)) + for _, rule := range rules { + ruleMap, ok := rule.(map[string]any) + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError("each rule must be an object with a 'type' field") + } + ruleType, ok := ruleMap["type"].(string) + if !ok || ruleType == "" { + return github.RepositoryRuleset{}, utils.NewToolResultError("each rule must have a non-empty string 'type' field") + } + if seenRuleTypes[ruleType] { + // github.RepositoryRulesetRules has a single field per rule type, so a + // second rule of the same type would silently overwrite the first + // during the round-trip below rather than producing two rules. + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("duplicate rule type: %q (a ruleset may only have one rule of each type)", ruleType)) + } + seenRuleTypes[ruleType] = true + requestedRuleTypes = append(requestedRuleTypes, ruleType) + if parameters, exists := ruleMap["parameters"]; exists && parameters != nil { + parametersMap, ok := parameters.(map[string]any) + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("rule %q: parameters must be an object", ruleType)) + } + requestedRuleParameters[ruleType] = parametersMap + } + } + + payload := map[string]any{ + "name": name, + "enforcement": enforcement, + "rules": rules, + } + if target != "" { + payload["target"] = target + } + if conditions, exists := args["conditions"]; exists && conditions != nil { + conditionsMap, ok := conditions.(map[string]any) + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError("conditions parameter must be an object") + } + payload["conditions"] = conditionsMap + } + if bypassActors, exists := args["bypass_actors"]; exists && bypassActors != nil { + bypassActorsArr, ok := bypassActors.([]any) + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError("bypass_actors parameter must be an array of objects") + } + for i, actor := range bypassActorsArr { + actorMap, ok := actor.(map[string]any) + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d] must be an object", i)) + } + // github.BypassActor recognizes only these three keys; any other key + // (e.g. a "bypass_modes" typo) is silently discarded by JSON + // unmarshal, which would grant the actor the default "always" bypass + // mode instead of the caller's intended value. + for key := range actorMap { + if key != "actor_id" && key != "actor_type" && key != "bypass_mode" { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d]: unsupported or unrecognized key: %q", i, key)) + } + } + } + payload["bypass_actors"] = bypassActorsArr + } + + raw, err := json.Marshal(payload) + if err != nil { + return github.RepositoryRuleset{}, utils.NewToolResultErrorFromErr("failed to build ruleset request", err) + } + var ruleset github.RepositoryRuleset + if err := json.Unmarshal(raw, &ruleset); err != nil { + return github.RepositoryRuleset{}, utils.NewToolResultErrorFromErr("failed to parse ruleset request", err) + } + + // github.RepositoryRulesetRules.UnmarshalJSON silently discards rule types and + // rule parameters it does not recognize, which would let a typo (e.g. + // "require_code_owners_review" instead of "require_code_owner_review") create + // a weaker ruleset than the caller requested. Verify every requested rule + // type, and every supplied parameter key within it (recursively), survived + // the round-trip. + appliedRules, errResult := rulesetAppliedRules(ruleset.Rules) + if errResult != nil { + return github.RepositoryRuleset{}, errResult + } + for _, ruleType := range requestedRuleTypes { + appliedParameters, ok := appliedRules[ruleType] + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("unsupported or unrecognized rule type: %q", ruleType)) + } + if requested := requestedRuleParameters[ruleType]; requested != nil { + if droppedPath := droppedKeyPath(requested, appliedParameters); droppedPath != "" { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("rule %q: unsupported or unrecognized parameter: %q", ruleType, droppedPath)) + } + } + } + + // github.RepositoryRulesetConditions has the same silent-drop behavior for + // unrecognized keys (e.g. "ref_names" instead of "ref_name"), so verify the + // requested conditions survived the round-trip the same way. + if requestedConditions, ok := payload["conditions"].(map[string]any); ok { + appliedConditions, errResult := rulesetAppliedConditions(ruleset.Conditions) + if errResult != nil { + return github.RepositoryRuleset{}, errResult + } + if droppedPath := droppedKeyPath(requestedConditions, appliedConditions); droppedPath != "" { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("conditions: unsupported or unrecognized key: %q", droppedPath)) + } + } + + return ruleset, nil +} + +// droppedKeyPath recursively compares a caller-supplied object against its +// round-tripped counterpart and returns the path of the first key or array +// element that did not survive (e.g. "required_status_checks[0].integration_id"), +// or "" if everything survived. A caller-supplied value that is a JSON zero +// value (false, 0, "", or an empty array/object) is exempt, since it is +// indistinguishable from a field omitted by a `json:",omitempty"` struct tag +// on the far side of the round-trip. This round-trip is entirely local (our +// own JSON marshal/unmarshal of a go-github struct, not a remote API +// response), so slice order and length are preserved deterministically and +// array elements are safe to compare by index. +func droppedKeyPath(requested, applied map[string]any) string { + for key, requestedValue := range requested { + appliedValue, ok := applied[key] + if !ok { + if isZeroJSONValue(requestedValue) { + continue + } + return key + } + if nested := droppedValuePath(requestedValue, appliedValue); nested != "" { + return key + nested + } + } + return "" +} + +// droppedValuePath recurses into map and array values on behalf of +// droppedKeyPath. It returns a path suffix beginning with "." (object key) or +// "[i]" (array index), or "" when requested and applied agree closely enough. +func droppedValuePath(requested, applied any) string { + switch requestedTyped := requested.(type) { + case map[string]any: + appliedMap, ok := applied.(map[string]any) + if !ok { + return "" + } + if nested := droppedKeyPath(requestedTyped, appliedMap); nested != "" { + return "." + nested + } + return "" + case []any: + appliedArr, ok := applied.([]any) + if !ok { + return "" + } + for i, requestedElem := range requestedTyped { + if i >= len(appliedArr) { + if isZeroJSONValue(requestedElem) { + continue + } + return fmt.Sprintf("[%d]", i) + } + if nested := droppedValuePath(requestedElem, appliedArr[i]); nested != "" { + return fmt.Sprintf("[%d]%s", i, nested) + } + } + return "" + default: + return "" + } +} + +// isZeroJSONValue reports whether v is the JSON zero value for its type +// (false, 0, "", nil, or an empty array/object). Such values are +// indistinguishable from an omitted field once round-tripped through a Go +// struct field tagged `omitempty`. +func isZeroJSONValue(v any) bool { + switch value := v.(type) { + case nil: + return true + case bool: + return !value + case float64: + return value == 0 + case string: + return value == "" + case []any: + return len(value) == 0 + case map[string]any: + return len(value) == 0 + default: + return false + } +} + +// rulesetAppliedRules marshals the parsed rules back to the API's array form +// and returns, for each rule type that was actually retained, the parameter +// object the corresponding go-github struct recognized. +func rulesetAppliedRules(rules *github.RepositoryRulesetRules) (map[string]map[string]any, *mcp.CallToolResult) { + applied := map[string]map[string]any{} + if rules == nil { + return applied, nil + } + raw, err := json.Marshal(rules) + if err != nil { + return nil, utils.NewToolResultErrorFromErr("failed to validate ruleset rules", err) + } + var ruleObjects []struct { + Type string `json:"type"` + Parameters map[string]any `json:"parameters"` + } + if err := json.Unmarshal(raw, &ruleObjects); err != nil { + return nil, utils.NewToolResultErrorFromErr("failed to validate ruleset rules", err) + } + for _, rule := range ruleObjects { + parameters := rule.Parameters + if parameters == nil { + parameters = map[string]any{} + } + applied[rule.Type] = parameters + } + return applied, nil +} + +// rulesetAppliedConditions marshals the parsed conditions back to the API's +// object form and returns the keys that were actually retained. +func rulesetAppliedConditions(conditions *github.RepositoryRulesetConditions) (map[string]any, *mcp.CallToolResult) { + if conditions == nil { + return map[string]any{}, nil + } + raw, err := json.Marshal(conditions) + if err != nil { + return nil, utils.NewToolResultErrorFromErr("failed to validate ruleset conditions", err) + } + var applied map[string]any + if err := json.Unmarshal(raw, &applied); err != nil { + return nil, utils.NewToolResultErrorFromErr("failed to validate ruleset conditions", err) + } + if applied == nil { + applied = map[string]any{} + } + return applied, nil +} diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go new file mode 100644 index 0000000000..46c53825df --- /dev/null +++ b/pkg/github/rulesets_test.go @@ -0,0 +1,1180 @@ +package github + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/url" + "testing" + + "github.com/google/go-github/v89/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/translations" +) + +func Test_RepositoryRulesetRead(t *testing.T) { + toolDef := RepositoryRulesetRead(translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + + assert.Equal(t, "repository_ruleset_read", toolDef.Tool.Name) + assert.NotEmpty(t, toolDef.Tool.Description) + assert.True(t, toolDef.Tool.Annotations.ReadOnlyHint) + + schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok, "InputSchema should be *jsonschema.Schema") + assert.ElementsMatch(t, schema.Required, []string{"level", "method"}) + + t.Run("repository level: get defaults includes_parents to true", func(t *testing.T) { + var capturedQuery url.Values + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/rulesets/{ruleset_id}": func(w http.ResponseWriter, r *http.Request) { + capturedQuery = r.URL.Query() + mockResponse(t, http.StatusOK, &github.RepositoryRuleset{Name: "main protection", Enforcement: "active"})(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "get", "owner": "owner", "repo": "repo", "ruleset_id": float64(42)}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Equal(t, "true", capturedQuery.Get("includes_parents")) + + var returned github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, "main protection", returned.Name) + }) + + t.Run("repository level: get forwards explicit includes_parents=false", func(t *testing.T) { + var capturedQuery url.Values + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/rulesets/{ruleset_id}": func(w http.ResponseWriter, r *http.Request) { + capturedQuery = r.URL.Query() + mockResponse(t, http.StatusOK, &github.RepositoryRuleset{Name: "rs"})(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "get", "owner": "owner", "repo": "repo", "ruleset_id": float64(42), "includes_parents": false}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Equal(t, "false", capturedQuery.Get("includes_parents")) + }) + + t.Run("repository level: get requires ruleset_id", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "get", "owner": "owner", "repo": "repo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "ruleset_id") + }) + + t.Run("repository level: requires owner and repo", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "list"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "owner") + }) + + t.Run("repository level: list omits includes_parents when not provided", func(t *testing.T) { + var capturedQuery url.Values + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { + capturedQuery = r.URL.Query() + mockResponse(t, http.StatusOK, []*github.RepositoryRuleset{{Name: "rs1"}})(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "list", "owner": "owner", "repo": "repo", "perPage": float64(50)}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.False(t, capturedQuery.Has("includes_parents"), "includes_parents must not be sent when omitted") + assert.Equal(t, "50", capturedQuery.Get("per_page")) + + var returned []*github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + require.Len(t, returned, 1) + assert.Equal(t, "rs1", returned[0].Name) + }) + + t.Run("repository level: list forwards explicit includes_parents=false", func(t *testing.T) { + var capturedQuery url.Values + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { + capturedQuery = r.URL.Query() + mockResponse(t, http.StatusOK, []*github.RepositoryRuleset{})(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "list", "owner": "owner", "repo": "repo", "includes_parents": false}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Equal(t, "false", capturedQuery.Get("includes_parents")) + }) + + t.Run("repository level: get_rules_for_branch", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/rules/branches/{branch}": mockResponse(t, http.StatusOK, []map[string]any{{"type": "creation"}}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "get_rules_for_branch", "owner": "owner", "repo": "repo", "branch": "main"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "Creation") + }) + + t.Run("repository level: get_rules_for_branch requires branch", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "get_rules_for_branch", "owner": "owner", "repo": "repo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "branch") + }) + + t.Run("repository level: list_rule_suites forwards filters", func(t *testing.T) { + var capturedQuery url.Values + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/rulesets/rule-suites": func(w http.ResponseWriter, r *http.Request) { + capturedQuery = r.URL.Query() + mockResponse(t, http.StatusOK, []map[string]any{{"id": 101, "result": "pass"}})(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "method": "list_rule_suites", + "owner": "owner", + "repo": "repo", + "ref": "refs/heads/main", + "time_period": "week", + "actor_name": "octocat", + "rule_suite_result": "pass", + "perPage": float64(25), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "pass") + + assert.Equal(t, "refs/heads/main", capturedQuery.Get("ref")) + assert.Equal(t, "week", capturedQuery.Get("time_period")) + assert.Equal(t, "octocat", capturedQuery.Get("actor_name")) + assert.Equal(t, "pass", capturedQuery.Get("rule_suite_result")) + assert.Equal(t, "25", capturedQuery.Get("per_page")) + }) + + t.Run("repository level: get_rule_suite", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/rulesets/rule-suites/{rule_suite_id}": mockResponse(t, http.StatusOK, map[string]any{"id": 101, "result": "fail"}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "get_rule_suite", "owner": "owner", "repo": "repo", "rule_suite_id": float64(101)}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "fail") + }) + + t.Run("repository level: unknown method", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "repository", "method": "frobnicate", "owner": "owner", "repo": "repo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "unknown method") + }) + + t.Run("organization level: get", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /orgs/{org}/rulesets/{ruleset_id}": mockResponse(t, http.StatusOK, &github.RepositoryRuleset{Name: "org rs", Enforcement: "active"}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "organization", "method": "get", "org": "octo", "ruleset_id": float64(7)}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var returned github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, "org rs", returned.Name) + }) + + t.Run("organization level: get requires ruleset_id", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "organization", "method": "get", "org": "octo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "ruleset_id") + }) + + t.Run("organization level: requires org", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "organization", "method": "list"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "org") + }) + + t.Run("organization level: list", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /orgs/{org}/rulesets": mockResponse(t, http.StatusOK, []*github.RepositoryRuleset{{Name: "org rs"}}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "organization", "method": "list", "org": "octo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var returned []*github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + require.Len(t, returned, 1) + assert.Equal(t, "org rs", returned[0].Name) + }) + + t.Run("organization level: repository-only method defers to normal validation", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "organization", "method": "get_rules_for_branch", "org": "octo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "not supported for level \"organization\"") + }) + + t.Run("organization level: unknown method", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "organization", "method": "frobnicate", "org": "octo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "not supported for level") + }) + + t.Run("enterprise level: get", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /enterprises/{enterprise}/rulesets/{ruleset_id}": mockResponse(t, http.StatusOK, &github.RepositoryRuleset{Name: "enterprise rs", Enforcement: "active"}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "enterprise", "method": "get", "enterprise": "acme", "ruleset_id": float64(9)}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var returned github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, "enterprise rs", returned.Name) + }) + + t.Run("enterprise level: requires enterprise", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "enterprise", "method": "list"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "enterprise") + }) + + t.Run("enterprise level: list", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /enterprises/{enterprise}/rulesets": mockResponse(t, http.StatusOK, []*github.RepositoryRuleset{{Name: "enterprise rs"}}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "enterprise", "method": "list", "enterprise": "acme"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var returned []*github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + require.Len(t, returned, 1) + assert.Equal(t, "enterprise rs", returned[0].Name) + }) + + t.Run("enterprise level: repository-only method defers to normal validation", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "enterprise", "method": "list_rule_suites", "enterprise": "acme"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "not supported for level \"enterprise\"") + }) + + t.Run("unknown level defers to normal validation", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "planet", "method": "list"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "unknown level") + }) + + t.Run("mismatched-case level is rejected rather than silently normalized", func(t *testing.T) { + // The scope challenge in rulesetReadScopeAccess matches "level" with an + // exact, case-sensitive comparison. If the handler instead normalized case + // (e.g. via strings.ToLower) before dispatching, a caller could send + // "Organization" to reach the organization-level read while the OAuth + // middleware -- which sees the raw, un-normalized argument -- would find no + // case matching "organization" and issue no scope challenge at all, + // letting an under-scoped token read organization rulesets for free. + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /orgs/{org}/rulesets": func(w http.ResponseWriter, r *http.Request) { + called = true + mockResponse(t, http.StatusOK, []*github.RepositoryRuleset{})(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "Organization", "method": "list", "org": "octo"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "unknown level") + assert.False(t, called, "a mismatched-case level must not reach the organization-level API call") + + // The scope challenge must independently agree: it must not treat + // "Organization" as a recognized level either. + assert.Empty(t, toolDef.ScopeAccess.Challenge(map[string]any{"level": "Organization"}, nil)) + }) +} + +func Test_CreateRepositoryRuleset(t *testing.T) { + toolDef := CreateRepositoryRuleset(translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + + assert.Equal(t, "create_repository_ruleset", toolDef.Tool.Name) + assert.False(t, toolDef.Tool.Annotations.ReadOnlyHint) + + schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok) + assert.ElementsMatch(t, schema.Required, []string{"level", "name", "enforcement", "rules"}) + + t.Run("repository level", func(t *testing.T) { + var capturedBody github.RepositoryRuleset + var capturedRaw []byte + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + capturedRaw = body + _ = json.Unmarshal(body, &capturedBody) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(body) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "main protection", + "enforcement": "active", + "target": "branch", + "rules": []any{ + map[string]any{"type": "creation"}, + map[string]any{"type": "deletion"}, + map[string]any{ + "type": "pull_request", + "parameters": map[string]any{ + "required_approving_review_count": float64(2), + }, + }, + }, + "conditions": map[string]any{ + "ref_name": map[string]any{ + "include": []any{"refs/heads/main"}, + "exclude": []any{}, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + assert.Equal(t, "main protection", capturedBody.Name) + assert.Equal(t, github.RulesetEnforcement("active"), capturedBody.Enforcement) + require.NotNil(t, capturedBody.Rules) + + // Verify the outbound body preserves all requested rules and the pull_request + // parameters, rather than silently dropping them in the JSON round-trip. + var outbound struct { + Rules []struct { + Type string `json:"type"` + Parameters map[string]any `json:"parameters"` + } `json:"rules"` + Conditions struct { + RefName struct { + Include []string `json:"include"` + } `json:"ref_name"` + } `json:"conditions"` + } + require.NoError(t, json.Unmarshal(capturedRaw, &outbound)) + + sentTypes := make([]string, 0, len(outbound.Rules)) + var pullRequestParams map[string]any + for _, rule := range outbound.Rules { + sentTypes = append(sentTypes, rule.Type) + if rule.Type == "pull_request" { + pullRequestParams = rule.Parameters + } + } + assert.ElementsMatch(t, []string{"creation", "deletion", "pull_request"}, sentTypes) + require.NotNil(t, pullRequestParams) + assert.EqualValues(t, 2, pullRequestParams["required_approving_review_count"]) + assert.Equal(t, []string{"refs/heads/main"}, outbound.Conditions.RefName.Include) + }) + + t.Run("repository level requires owner and repo", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "name": "x", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "owner") + }) + + t.Run("unsupported rule type", func(t *testing.T) { + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{ + map[string]any{"type": "creation"}, + map[string]any{"type": "totally_made_up_rule"}, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "totally_made_up_rule") + assert.False(t, called, "request must not be sent when a rule type is unsupported") + }) + + t.Run("invalid rules", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": "not-an-array", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "rules parameter must be an array") + }) + + t.Run("duplicate rule type", func(t *testing.T) { + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{ + map[string]any{"type": "creation"}, + map[string]any{"type": "creation"}, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "duplicate rule type") + assert.False(t, called, "request must not be sent when rules contain a duplicate type") + }) + + t.Run("unrecognized rule parameter is rejected even though the rule type is valid", func(t *testing.T) { + // "require_code_owners_review" is a plausible typo for the real + // pull_request parameter "require_code_owner_review". go-github's + // generated UnmarshalJSON silently drops unknown parameter keys, so + // without this check the ruleset would be created with the weaker + // default (false) instead of surfacing the mistake. + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{ + map[string]any{ + "type": "pull_request", + "parameters": map[string]any{ + "require_code_owners_review": true, // typo: should be require_code_owner_review + }, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "require_code_owners_review") + assert.False(t, called, "request must not be sent when a rule parameter is unrecognized") + }) + + t.Run("zero-valued parameters are not flagged as unrecognized", func(t *testing.T) { + // Scalar fields without `omitempty` (like required_approving_review_count) + // always round-trip, but slice fields with `omitempty` (like + // allowed_merge_methods) vanish from the response when empty. An + // explicit zero value supplied by the caller must not be misread as an + // unsupported parameter key. + var capturedBody []byte + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { + capturedBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(capturedBody) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{ + map[string]any{ + "type": "pull_request", + "parameters": map[string]any{ + "required_approving_review_count": float64(0), + "allowed_merge_methods": []any{}, + }, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if result.IsError { + t.Fatalf("unexpected error: %s", getErrorResult(t, result).Text) + } + assert.NotEmpty(t, capturedBody) + }) + + t.Run("unrecognized key inside a rule parameter array element is rejected", func(t *testing.T) { + // "integration_ids" is a plausible typo for the real per-check field + // "integration_id" on required_status_checks[]. Unlike the top-level + // rule/condition round-trip, this array is produced by our own local + // JSON marshal/unmarshal of the go-github struct (not a remote API + // response), so element order is guaranteed stable and comparing by + // index is safe. Without this check, the typo would silently vanish and + // the resulting rule would accept a status check from any integration + // instead of only the one requested. + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{ + map[string]any{ + "type": "required_status_checks", + "parameters": map[string]any{ + "required_status_checks": []any{ + map[string]any{ + "context": "ci", + "integration_ids": float64(42), // typo: should be integration_id + }, + }, + "strict_required_status_checks_policy": true, + }, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "required_status_checks[0].integration_ids") + assert.False(t, called, "request must not be sent when a nested array element key is unrecognized") + }) + + t.Run("valid rule parameter array elements round-trip and are not misflagged", func(t *testing.T) { + var capturedBody []byte + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { + capturedBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(capturedBody) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{ + map[string]any{ + "type": "required_status_checks", + "parameters": map[string]any{ + "required_status_checks": []any{ + map[string]any{ + "context": "ci", + "integration_id": float64(42), + }, + }, + "strict_required_status_checks_policy": true, + }, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if result.IsError { + t.Fatalf("unexpected error: %s", getErrorResult(t, result).Text) + } + assert.NotEmpty(t, capturedBody) + }) + + t.Run("bypass_actors accepts exempt bypass mode and enterprise actor types", func(t *testing.T) { + var capturedBody github.RepositoryRuleset + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /enterprises/{enterprise}/rulesets": func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(body, &capturedBody) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(body) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "enterprise", + "enterprise": "acme", + "name": "enterprise protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + "bypass_actors": []any{ + map[string]any{"actor_type": "EnterpriseOwner", "bypass_mode": "exempt"}, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + require.Len(t, capturedBody.BypassActors, 1) + assert.Equal(t, github.BypassActorType("EnterpriseOwner"), *capturedBody.BypassActors[0].ActorType) + assert.Equal(t, github.BypassMode("exempt"), *capturedBody.BypassActors[0].BypassMode) + }) + + t.Run("unrecognized bypass_actors key is rejected", func(t *testing.T) { + // "bypass_modes" is a plausible typo for "bypass_mode". github.BypassActor + // only recognizes actor_id/actor_type/bypass_mode, so an unknown key is + // silently discarded during JSON unmarshal -- and because the API + // defaults an omitted bypass_mode to "always", the resulting actor would + // get broader bypass rights than the caller requested. + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + "bypass_actors": []any{ + map[string]any{"actor_type": "Team", "actor_id": float64(1), "bypass_modes": "pull_request"}, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "bypass_modes") + assert.False(t, called, "request must not be sent when a bypass_actors key is unrecognized") + }) + + t.Run("unrecognized top-level condition key is rejected", func(t *testing.T) { + // "ref_names" is a plausible typo for the real condition key "ref_name". + // github.RepositoryRulesetConditions silently drops unknown keys during + // JSON unmarshal, so without this check the ruleset would be created with + // no ref_name condition at all (applying to every ref) instead of + // surfacing the mistake. + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + "conditions": map[string]any{ + "ref_names": map[string]any{ // typo: should be ref_name + "include": []any{"refs/heads/main"}, + "exclude": []any{}, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "ref_names") + assert.False(t, called, "request must not be sent when a condition key is unrecognized") + }) + + t.Run("unrecognized nested condition key is rejected", func(t *testing.T) { + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + "conditions": map[string]any{ + "ref_name": map[string]any{ + "includes": []any{"refs/heads/main"}, // typo: should be include + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "ref_name.includes") + assert.False(t, called, "request must not be sent when a nested condition key is unrecognized") + }) + + t.Run("valid conditions round-trip and are not misflagged", func(t *testing.T) { + var capturedBody []byte + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { + capturedBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(capturedBody) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + "conditions": map[string]any{ + "ref_name": map[string]any{ + "include": []any{"refs/heads/main"}, + "exclude": []any{}, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if result.IsError { + t.Fatalf("unexpected error: %s", getErrorResult(t, result).Text) + } + assert.NotEmpty(t, capturedBody) + }) + + t.Run("organization level", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /orgs/{org}/rulesets": func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(body) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "organization", + "org": "octo", + "name": "org protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var returned github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, "org protection", returned.Name) + }) + + t.Run("organization level requires org", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "organization", + "name": "org protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "org") + }) + + t.Run("enterprise level", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /enterprises/{enterprise}/rulesets": func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(body) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "enterprise", + "enterprise": "acme", + "name": "enterprise protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var returned github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, "enterprise protection", returned.Name) + }) + + t.Run("enterprise level requires enterprise", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "enterprise", + "name": "enterprise protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "enterprise") + }) + + t.Run("unknown level", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "planet", + "name": "x", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "unknown level") + }) + + t.Run("mismatched-case level is rejected rather than silently normalized", func(t *testing.T) { + // Mirrors the read-tool regression above: rulesetWriteScopeAccess only + // recognizes an exact, lowercase "organization"/"enterprise" match. If the + // handler normalized case before dispatching, "Organization" would reach + // client.Organizations.CreateRepositoryRuleset while the OAuth middleware + // -- which challenges on the raw argument -- would see no case match and + // require no admin:org scope at all, letting an under-scoped token create + // organization-wide rulesets for free. + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /orgs/{org}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "Organization", + "org": "octo", + "name": "org protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "unknown level") + assert.False(t, called, "a mismatched-case level must not reach the organization-level create call") + + assert.Empty(t, toolDef.ScopeAccess.Challenge(map[string]any{"level": "Organization"}, nil)) + }) +} + +// Test_RulesetScopeChallenges verifies that the ruleset read and write tools +// challenge for the exact scope implied by the "level" argument, and defer to +// normal handler validation (no challenge) when "level" is missing or not a +// string. +func Test_RulesetScopeChallenges(t *testing.T) { + tests := []struct { + name string + tool inventory.ServerTool + arguments map[string]any + allowed []string + disallowed []string + }{ + { + name: "read repository level", + tool: RepositoryRulesetRead(translations.NullTranslationHelper), + arguments: map[string]any{"level": "repository", "method": "get"}, + allowed: []string{"repo"}, + disallowed: []string{"read:org"}, + }, + { + name: "read organization level", + tool: RepositoryRulesetRead(translations.NullTranslationHelper), + arguments: map[string]any{"level": "organization", "method": "list"}, + allowed: []string{"read:org"}, + disallowed: []string{"repo"}, + }, + { + name: "read enterprise level", + tool: RepositoryRulesetRead(translations.NullTranslationHelper), + arguments: map[string]any{"level": "enterprise", "method": "get"}, + allowed: []string{"read:enterprise"}, + disallowed: []string{"repo", "read:org"}, + }, + { + name: "read missing level defers to validation", + tool: RepositoryRulesetRead(translations.NullTranslationHelper), + arguments: map[string]any{"method": "get"}, + allowed: nil, + disallowed: nil, + }, + { + name: "read unknown level defers to validation", + tool: RepositoryRulesetRead(translations.NullTranslationHelper), + arguments: map[string]any{"level": "planet", "method": "get"}, + allowed: nil, + disallowed: nil, + }, + { + name: "write repository level", + tool: CreateRepositoryRuleset(translations.NullTranslationHelper), + arguments: map[string]any{"level": "repository"}, + allowed: []string{"repo"}, + disallowed: []string{"admin:org"}, + }, + { + name: "write organization level", + tool: CreateRepositoryRuleset(translations.NullTranslationHelper), + arguments: map[string]any{"level": "organization"}, + allowed: []string{"admin:org"}, + disallowed: []string{"repo"}, + }, + { + name: "write enterprise level", + tool: CreateRepositoryRuleset(translations.NullTranslationHelper), + arguments: map[string]any{"level": "enterprise"}, + allowed: []string{"admin:enterprise"}, + disallowed: []string{"repo", "admin:org"}, + }, + { + name: "write missing level defers to validation", + tool: CreateRepositoryRuleset(translations.NullTranslationHelper), + arguments: map[string]any{}, + allowed: nil, + disallowed: nil, + }, + { + name: "write malformed level defers to validation", + tool: CreateRepositoryRuleset(translations.NullTranslationHelper), + arguments: map[string]any{"level": 123}, + allowed: nil, + disallowed: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.NotNil(t, tt.tool.ScopeAccess.Challenge) + assert.True(t, tt.tool.ScopeAccess.Dynamic) + assert.True(t, tt.tool.ScopeAccess.Visible(nil)) + assert.Empty(t, tt.tool.ScopeAccess.Challenge(tt.arguments, tt.allowed)) + if tt.disallowed == nil { + assert.Empty(t, tt.tool.ScopeAccess.Challenge(tt.arguments, nil)) + } else { + assert.NotEmpty(t, tt.tool.ScopeAccess.Challenge(tt.arguments, tt.disallowed)) + } + }) + } +} + +func Test_RulesetScopeMetadataIsExhaustive(t *testing.T) { + tests := []struct { + tool inventory.ServerTool + maxScopes []string + }{ + {tool: RepositoryRulesetRead(translations.NullTranslationHelper), maxScopes: []string{"repo", "read:org", "read:enterprise"}}, + {tool: CreateRepositoryRuleset(translations.NullTranslationHelper), maxScopes: []string{"repo", "admin:org", "admin:enterprise"}}, + } + + for _, tt := range tests { + t.Run(tt.tool.Tool.Name, func(t *testing.T) { + assert.True(t, tt.tool.ScopeAccess.Dynamic) + assert.Equal(t, tt.maxScopes, tt.tool.ScopeAccess.Scopes) + assert.NotNil(t, tt.tool.ScopeAccess.Challenge) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index ca46deadd2..a6ffbecfee 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -73,6 +73,11 @@ var ( Description: "GitHub Organization related tools", Icon: "organization", } + ToolsetMetadataGovernance = inventory.ToolsetMetadata{ + ID: "governance", + Description: "Repository governance tools for managing rulesets at the repository, organization, and enterprise levels", + Icon: "law", + } ToolsetMetadataActions = inventory.ToolsetMetadata{ ID: "actions", Description: "GitHub Actions workflows and CI/CD operations", @@ -265,6 +270,10 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent // Organization tools SearchOrgs(t), + // Governance tools (rulesets) + RepositoryRulesetRead(t), + CreateRepositoryRuleset(t), + // Pull request tools PullRequestRead(t), ListPullRequests(t), diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index 39c7e953b4..d2176880de 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -653,6 +653,9 @@ func TestSupportedScopes(t *testing.T) { "repo", "delete_repo", "read:org", + "admin:org", + "read:enterprise", + "admin:enterprise", "read:user", "user:email", "read:packages", diff --git a/pkg/octicons/icons/law-dark.png b/pkg/octicons/icons/law-dark.png new file mode 100644 index 0000000000000000000000000000000000000000..3c7127574873c01e6c225a4743485ad0d63e356c GIT binary patch literal 550 zcmV+>0@?kEP)q*CT@h}L+?9wIQc91Dnzn_<)p-YFUL7AKHl?n2Xi^e5^xB$s?MpO-H4X@MBUeCgFWg4^?R;&QN5~GQ-)jy zb^=>~Rp4DpX(hLM6CkBj&*evA@*40lrSzfC*1gf~77apaFy-&i^F2^c1KV@y{Cm*G oQcCH5A?NrbhsDc6;{OzW0sdf(YKjW*u>b%707*qoM6N<$f`5AW&;S4c literal 0 HcmV?d00001 diff --git a/pkg/octicons/icons/law-light.png b/pkg/octicons/icons/law-light.png new file mode 100644 index 0000000000000000000000000000000000000000..3121c5b6547e0440a7eb2fe8fa3ed132600e174c GIT binary patch literal 841 zcmV-P1GfB$P)Th+xnZTCgp>w4w(oJxDLv`~lW%ch`*TtS+rWS}^$AY@C^= z2a|0Qvt84k#P4Z%-sk)MJTtsJ122`Fck~_L?t|eqei@8P3{T`Pg;c+cg1BZ1n~t`~{ddT5R`{u~#endV8A~SsBsOj|F%Y!yR#2U|40{aLMMc55o6Zl?& z@O-hDb3OOj$ugOY4;*b>`Q`q0-0Wc$Zcg}@1r_^L zv6!1Bdb+OVe;w8*4Sm2;o+kd`x!J>oLSeKD0EkAT4FVqld0>&5=kKW(UX?xeKF|o{ zQT9fo(T2&RD!`uKxD!}{N;l{r!oGAm{T$c0%R=}B$Y{tuV2M4yamOGH9G(df%GzQfeb)feD&LkCRF7FprzrVpFm2XNKZY0 zgwg}t@UFD}3?M3fz{=+K#HK1BAnZl?D;T?TcET3|rv=uxbtKo+3U7(6dP`xQpnY&I z08szI55R~~=>;&&@l>n8R-nIJE{{!`t?=;~;BFYwU2}TOr@;N7^2&FUHP7=Zhph{_u@_{?Dm_Q&+{*dLofn`0xo$qSm6&GPkZ5F8yBft^Z9O!@7M)$P;!9R7m?p+x=H(O5aRiOGAkeXhrJ{_CUdl{G&@Z#%lQ6L*- T*E9Y<00000NkvXXu0mjfWvYxK literal 0 HcmV?d00001 diff --git a/pkg/octicons/icons_data_uris.txt b/pkg/octicons/icons_data_uris.txt index 1083af68b8..b5899f7589 100644 --- a/pkg/octicons/icons_data_uris.txt +++ b/pkg/octicons/icons_data_uris.txt @@ -30,6 +30,8 @@ git-pull-request-dark data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYA git-pull-request-light data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAACwUlEQVRIie2Vz28UZRjHP993pi0QIC3YahNjirtmd3bS3Q1eUHvQEPUiEv8A4kXjwRJ78MCFBLjBBRKCHowHE38cNCbGGx6IUoKiodtNpoNmTJp4oSJNQ3pw29l5POxus2wo3QTwxPc0887zfD7zvu9kXnjEUfdNrjj5vJOmMP4e9JrfR1G02tuQD8tvgpck0dxCPwK30ViqnJTcr4bOmfRlI/PrhUJ5313woDpDpu8ss7f6nYHrvDnGcYlPsoY/bKaXwHY3HWfvgmNnMX0zvMM7069A3c3pkEYWa7UVgFxQPSfs7SSeH3k2rEy5jMubMBoG1yQ+SBbm53of+gCybMkk/H8VAFdbZisZLAFsJ11oyL+BUURcwrjWAZixXeIwxs/5UuVAr0QAYRjubGR+HWy3mb6QCIBXQe8nce0jgIkwfMo3/xLG085x8I9ofkMyUa0O+w2rgS0mcf3lboEDiKJo1cvsIDDr4D1DhTb8407hYhTdTJW+AvrdMnuhG9Je1m9BBzbfjXbyQcXypeqJLQt7+0rVE/mgYr3j7l7FDzOPBY8FDx6vc1EolPeNjI5/Jpgw7Lm9o+Pry//c/K0PhnLFyrSMDxE79jwxvn9079gvt28vrUD7V1EoFHalbltd2C7DfS4sAF4DTSdx7cL96LliZVriPPADuL+geRh0Z8il5SiKVn2ATENvCCYw78U/b8xdBcgHlYuGHQXuK5A4ClxM4vnXW8Lqp5JdWWt6h4CvWnsgxgDSbRZ3Gg0tCJ7sY4nGwDZOt/WBZtzN9FswLgM2sGann5mcPDaw5pWEHQH7cUu86SdkR3LF6tfrA814MNVpwNrM1leUxPXrSKfMeHcwdctyNotY8c3NbMX3LJsB3ZHsymDqlkHvYHYyievXWxPpSj4o75eYIuPWZof+vRKG4c61pncIx6gZsx34/5L/ACy3ElqUYhuvAAAAAElFTkSuQmCC issue-opened-dark data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAABxElEQVRIibWVvW4TURCFv0vlNEDcIHAkKAERJOIKSjoUArwBPwUFFaKIIngAJARCPIgdh4cgRBYt6ZIAEYEqdhpEw0fhCbkKa68dxyNtsfNzzpnZu3NhwpYGBdUqcA+4A1wEZiK0DawD74FWSml3JAJ1CngGLAIngU1gFfgZKWeAG8AFoAu8At6mlH6VtqTOqJ/UP2pDnRuQO6c27VlbrQ0Dvq121Fulag7q5qPmW18SdSqUd9Qrw4Jn9bNR21YrRQkvYixDKy/AuB3jWjocqKpdtXFU8AxrOTqZzp2PgvnaMRDUA+tB7mypG+OCZ3hbahPgRPguAR9LihaicEu9WcKxClzOi/fU1wPAk7rjgX0uEfNG3cs7mJjtE+wA5/olpZQEHgNf6K2NJyW4NeD7v7c4WptjSc0svlMjdzyM2fbdOyOA7x/T+7mzGj9H8xgIWuquevpw4HmsivkxwBdC/WJRsBKLqqPOHgH8aqybtcJlF0m1WLndUToJ5V31q9r3NOYk7Wh1Wa0PyK3HzA3l/4H3uzIrwFNgCThF7/x/AH5EylngOnAe6AAvgXcppd9DEWRE08DdeIou/RVgJaXUGYQzUfsL+zmwV7BtIq0AAAAASUVORK5CYII= issue-opened-light data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAAC2UlEQVRIicWVMUyTaRjHf/+vVUpOJOdilOYoUvVreq2CDmLOwdlAS5xMbrrhBifjYG7QjYuJw108nZx1huLgYlw0QshxChUK2koxHHcuCmgsFfieG0or3kGPSoz/8X3f5/97nvfN8z7wmaVqm9FodFfR/EnMugy5giCAwYwgI9Ff9Hl9L9Lp1zUBgsGO+rqGwnlhF4CdwBSyAUwvS1G220zHBSFgHtmVxYWvfp2ZGSj8LyAcjgfZphTQZtAr7OdsZvSP9RIJR+LtQhcNusF+d1aUfPp05M8NAavmg8AOk3cmN56+s1Hpa7U/cviUYbcw3jgex9ZCKoBgsKM+0PDuAdBq2He5zOiTzZiXtc+NxRw598GeLRcWTuTz+UUAp3ygrqFwHmgzeWdqNQd4PpFOy/gedNQfaDz3UQXRaHRX0fNPGdzNZUZO12q+VuHIoV7g5Hu/1/IinX7tABTNnwR2SvRsxRzAzOsBGuuWfAkoX5FZFzCVHR95tFVAbiI9DEwj6wLwl5YVMTRYLTDsxjuRrgGYYz/kxkbvbXRWMOBB24cKYI9ks1X8hXQDaAaa5XG9WjImZgV71wI+m8qAv8y0t1pSmP0ITANTmHO2qqvRZDALlTdgHKyjWkx2YvQ2cHtTacMxYBhWK5DoF4TCkXj7Jg02VKsbOwI0Y+qvALZrOQXMC13cKkDSJWBuJUCqAhgbG3uF7IpBcn/k8KlPNQ+78U5QQuhy/vHjuQoAYPndwi9gw4bd2ufGYrWatxz8No50ExhaKsz9Vl6vAPL5/KKzoiTGG0fOg1oqCbvxTp/juw/M+zynu/yTwjoD58CBQ02ez/pAR4E+M69ntf3/o1Y3dqR050oAQz7P6Z6cfPRRw647MkOhUMAfaDyH+AloBKYNPRT292rQHoMOSp09J3TZlt5ezWazxX97VR3638RiX9ct+RImSyBcrDT0ETMyMp4ptRIgVX7QL6J/ALSUEwJ5rdg2AAAAAElFTkSuQmCC +law-dark data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAAB20lEQVRIieWVz4uNYRTHP8+9t9ya7qiZNUWpmQ0LkaLGRcOOQpaUWbBipxE3439gacNiyg4LG12xRoNSmqZJidkxKTPl+lg4V++89+eblXxX7znne77fc56etwf+K6i31VtFeioFPSYL8gcbqMeAXRFujdyViBdSSo+LmraFx9V5B2NeHR8kNqWud2leU2fVSvCaajO+K1Fb69K3rk5lDcbUhnonCO/Va+pEbpA/BpncpHo9egyNhjrWbZObQVpVN3epdxhEflT9Gr1z2VopQ6oCF4C3QA043/dcN2IGGI3ei6HVMcVMTHBQfaout8++3wZqWV2KWj00OodTF9TXalJPBPHkEAangns84lfqGzVlSUeCdDbikrqoPhvC4HlsUI74XGgdzpIeqSvZs1MvB3FvLwN1d3AuZXKb1E/qw3Zih9pSG7nJauoX9W4fg3vdbpx6Q/2pTlSAaX7fpv3qGWAZGAEOAC3gKL0xDfwAZtUXwDdgG1AHEnAItarOqZ/diJb6RN3XZ4M96gP1e653Ub2qVlOGXAZ2AluAVeBdSmklt3oTIKVUz+VHYvIa8DGl9KHP1r3R7RYNQmkw5e/Q8z1QTwPbc+n8e9DGUkrpfiHn+COHxctC4v8UfgH+YI1qigrwsQAAAABJRU5ErkJggg== +law-light data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAAC/klEQVRIieWVz2sUZxjHP887UzcqRmixelC6SSZhN9HdtQUvKq5SouKhFBo8qAextJU2Jj14sCKlgiiIYNAUWrAtvbSiByk9SC/Z/AHWbHfXjOusLq1CWjD422ycmacHk20Ss13TnsTvaXje5/v9PDO8PAMvlZx46ksnnuifi8eeG0LjIHNy1O1u6UhskVCSE90fPOXwNYAazZYKv138T4BYLPaaL5F+YHudGc7aWvnYdd3bNQFObPUGJPwFmDfjvILqF8uXvXo8k8n4Tjw5AOANZzem02n75sjofkQ+ByIzfOOo6fTcy4M2QMQaz1UC+wgQRdgNXBP4XoLwfLGYcz332ckymYwPHG1d+eYFguA9hV1AK8q3QDlijecADEChUBj13OxhFW5O+JcFFftUsZibJXq6ruV/HdYnDX3AUp4m/uG52cOFQmG0CgCIRqMNAh8BeWCRmefvqRdelT32PtAI5FH2RqPRhsmjKsCe37gTWIJKN+ggwr50Ov0c17jLQvgEyGjIPmCJPb9xxzMAwXQDOc8dGlTMSeCNWyOj79SLdzquvgs0IXKydDU7AAyB9DJxgQxAW3z124omVOUEoKXhoZ9ASqFITz2AhNID3PCutP4MICp9wMrm9sSmKiAg7EX5Kxi7c3bCFwKnBda3dSTX1Apvia16S2GdoH1wLgBQ/8EPwIhR6QUwzfFUq8BWoL9cLo9Nmq3w8RngbhjSXXN6zKfA/aDyyneTNc/zKoh8BWxra1sVsw10AgZhbUs8ud2IlkNhYRDIOiAANtf+PnQCvon4B5z21CUleCBqNaFsBCS0rE22//jOGXvB4tdRPhT4UVUQBYQQJYPowVr5oZFtRvUQSg+qDYIBFJASogf9R3e/mbKLuiynvZhAdAWBuRfY/vCNfP7PqYFTV8XUeiKRWPjwCU2i1qLQMreuFy79XvOt/01OPDkwCXlemfot/08117XTnupCtXlG97T/wT91ue5dGTo3W07tVaD6GZCaXqs+HZvRexmYFfDi629RIBtl1zP+PwAAAABJRU5ErkJggg== logo-gist-dark data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACYAAAAYCAYAAACWTY9zAAAABmJLR0QA/wD/AP+gvaeTAAACoElEQVRIie2WX2hXZRjHP8/4hW5UzoGFja2wdiF00S7EotAgIcKbMBoIu7HuuhANC3aRCTIQgmBBsEZ3s8hoIwomrLuB4E0YhqWFrjHY0NY2rGHh6uPFecXj2flt+/2mXoRfOHDO+z5/Ps973vO8B+7r/yi1U+1XL6gL6j/qZfWU+r7aca9YIgf1LtAL/AuMAOeA68AjQCfwLDAeEU8l+wpwCDgfEV/fFTq120zfq49XsWlVu3PPu5LPX2vI+4L6RrXJJnVanVEfqyHow+rnas8awH5Vx6pN7k2Vv1dvgnq1HFgD8Eq6//IOJ21QN6rr6w1wRp2t0/c3tb8w1qmeVK95S3+oJ9K2aVF7037+L9ldzF1dABWgHRivqypoBppyUB3AGHAFOAZMAY3Ao8DzwDqytzQBfAIcBa4CH+Rinr0J9mCaLFuRfcADheHvIqJaIftSzOci4soyBQ2k+O8A0xExUDSokPUyqwToAx4qjL1O9RVuA2ZWgFqVGoAFslUrUzvQkq7XVhHvZ6BVfUttWtF6BbBJ4ImyyYiYj4i5iJgDVtNI+4BvgI+BP9Vx9Vv1iNpWK9hPwCa1FK4WRcRCRLxKVmgXMEh2xPUAP6pbawEbTfd71gqWA5yIiKGIOJxAdwIbgDdrARsG5oC31eY7BVcAPQ38DZQdeQ2lYBExDxwBWoGv1JYaci6S+0OpJnUHsB74pTA1C2xRiy2JCkBEfJT+Kg4CF9QTZHtvkeyL3Qy8XJLzErBdfSYiflD3A9sSwO9kPfBpoBuYAT4t+H8BfAgMqceB+YgYpSj1JXVYnfd2Takj6oH861ZfVCfVwfS8Wx1Tr+Z8Z9XP1CdL8kWKOZ5sZ0qKX+LUlA7hyipslxzUyb/YnJeL0ahW66f3taJuAAWd129KkzycAAAAAElFTkSuQmCC logo-gist-light data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACYAAAAYCAYAAACWTY9zAAAABmJLR0QA/wD/AP+gvaeTAAAD2ElEQVRIie2WS2xUVRjHf9/tHSgjRCA+C5am3gFmhnIHJypEY4aVujAYfCUGF8SFRhONgiRsiInxFRIiGhcSxQVEhYhhRQyycuMjUWaG0mnLLQ9BUORRiK20nTl/F1DSDJe2VOLC+F/dc873/c/vfDfnAf/rPyIb2Qgy4SI5njNjKTAb8BG9GBHSbmfeZwcrxQP/KlgqnVsj9CZQM9jlYL+JIYxbgEXAYrBDUaUYABQKBf/Yb2dXy6yzp1Lceb3BfIAgE66Q9C7wcw1v+aHK3iP1gXPnhrOcr6XD7eO/nytgvG2oD5g6kclbs+H9npgbdZQ2XwHWlM8n6a+uB51ucA2PRF17j8eZdHeXfgW2Drfd0KQflRj43OT2TQQKwHN8KjgBXAl2Q9/QMpndZti6rqtAxSmKfjgPPD1RqLHkyexhAM9p+/X2bm5rm9HS0tI4kWQL0uFeYE5UKc281uQgHR6W+Lqns/T85b5MuAjHWxgFYBjqDLCnP+mvnHHhQuNALbEK00Nc3FQDGCP/1Nqoo7TdB5qBQxNZFTDdjORwozWdSyF9i8dJnN4R3nHPc1PkvFsxd58/ODh5aKjRk1c7guwjQ29gnJez9cMeDa5Whou7cipwPm7WVCZc6ZwlRvb5ct90dZVjF+IZKxG+fC3pKZdPjrKgTQBBOnxNcKKns7ipPsAHDENx2RIbzTRtZF+tgSe4WoWlOxCnxoAalzygz1z8OVSdbM2Dvps56LuZJh4b081UwZgVpHMvNOXzyTHjR5EPHJXREjd4uFjsHf4O5od/jmWW9NnYP6R7gA+T/dUP7kyHvxi0Y/aTvOonPe3tR8cL5gEdwM3z5+di4a5F5XK5L6qUH63htZj0JLAFVENaa7WGfakFd6XH6+Vj7EY8VUXLgQ3/FA7g0pV2BNgBEKQXLgb7Tq72LLB6PB5edZJ9BZzFeLUll5t+PcDqFVXK3wMXJGuqHzPDiwU7XCz2mul1YFZiQF9ms9lrOGhVpe7pFKdUuu0BoNGM7rqhMxKt+Xw+UZ/jAxzoKL8fZMI5Eq8MOL8rSIfbwDokqoamGna7TA/WJ5vsoIx7U9lc7sD+YjGVWfiSZHcbdGP84SDhwQKJFcApBt3HdRZfGGw491d1R5AJt+LojTpLuy+DAUQdpVWtmYW7GmQvClaAbrRLtXDohGFFYHM1YXsu1wtbg7RFTquAZ5DXI/Q4sAwxzQDBWUw78dy6qNJ+bCRVVCm9F6RzkvSywTZMp4GbRi1/Uz6fbG5rm1EoFPxRA4G4i7opn0/OmzdvWlx8nGbPXjIlm81O6F33vwD+Bhvyhr7wtSBQAAAAAElFTkSuQmCC mark-github-dark data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAAB8ElEQVRIibWVu09UQRSHv9k1IgW7AY0RBWKsTGx9ND4qS5F/wEYLDb0xAY0UxkdrZ2NHYWfsaYyVT4iJwZpNjJooLBQEYz6LvZsdxmH3LtFfN3PO+X5zzzwu/GeFbkF1BJgCJoHjwFgRagDLwAvgeQjhR1+u6qA6q67ZW6vqjDpYFn5YfV0CnOqDOtELPqY2dgFvq6Ee6daWd1HyvPqyBPSV+jQav1H3tbkhMpgF7hXDLaAeQthUzwFXgE/AF0BgFDgBPAshLKhV4CcwVNTPhBAexKsfcfuGbqhdT1imA1+j+lV1GKBSxKeAWpRfTca94HuB+BTVgcuxwWRS8zCEsFbWIISwBcwl0x2m+jnZuKNl4RHjQMJYjoPNJFju0vxt8itiNKHTomqSO7wLeA3YE01VYoPvSf7Jfg2AU8n4W2zwPglO78Igrekw1enMDb1fXKCuUivq7Uz99Tiprq6rvwuzhSLpo3pLvZABn1Vv2nrkUjWLPdlWMFcEF9WD6tuo4EnG4HEG3Nad3KcOqEtRe/bb+ic8Uo9l8i/tAF9UB3bq54StJ3dTvaGOqofM3IuiRalW1PH8bnUKx4tVxLqYyTuf5Czl4JV0IoSwApwB7gLr7enMWtpzG7TeodNFbXmpNfWq6YloxYbUa2q9L+i/1h8/EAGdUrF9ZQAAAABJRU5ErkJggg== diff --git a/pkg/octicons/required_icons.txt b/pkg/octicons/required_icons.txt index 15dc444956..ebc5d99048 100644 --- a/pkg/octicons/required_icons.txt +++ b/pkg/octicons/required_icons.txt @@ -29,6 +29,7 @@ git-commit git-merge git-pull-request issue-opened +law logo-gist mark-github organization diff --git a/pkg/scopes/scopes.go b/pkg/scopes/scopes.go index d845cc6dc8..2294ac01ef 100644 --- a/pkg/scopes/scopes.go +++ b/pkg/scopes/scopes.go @@ -31,6 +31,14 @@ const ( // AdminOrg grants full control of organizations and teams AdminOrg Scope = "admin:org" + // ReadEnterprise grants read-only access to enterprise profile data, including + // enterprise-level custom properties + ReadEnterprise Scope = "read:enterprise" + + // AdminEnterprise grants full control of enterprises, including enterprise-level + // rulesets and custom properties + AdminEnterprise Scope = "admin:enterprise" + // Gist grants write access to gists Gist Scope = "gist" @@ -77,6 +85,9 @@ var oauthScopeDefinitions = []oauthScopeDefinition{ {scope: Repo, byDefault: true}, {scope: DeleteRepo}, {scope: ReadOrg, byDefault: true}, + {scope: AdminOrg}, + {scope: ReadEnterprise}, + {scope: AdminEnterprise}, {scope: ReadUser, byDefault: true}, {scope: UserEmail, byDefault: true}, {scope: ReadPackages, byDefault: true}, @@ -113,12 +124,13 @@ func oauthScopes(defaultOnly bool) []string { // A parent scope implicitly grants access to all child scopes. // For example, "repo" grants access to "public_repo" and "security_events". var ScopeHierarchy = map[Scope][]Scope{ - Repo: {PublicRepo, SecurityEvents}, - AdminOrg: {WriteOrg, ReadOrg}, - WriteOrg: {ReadOrg}, - Project: {ReadProject}, - WritePackages: {ReadPackages}, - User: {ReadUser, UserEmail}, + Repo: {PublicRepo, SecurityEvents}, + AdminOrg: {WriteOrg, ReadOrg}, + AdminEnterprise: {ReadEnterprise}, + WriteOrg: {ReadOrg}, + Project: {ReadProject}, + WritePackages: {ReadPackages}, + User: {ReadUser, UserEmail}, } // RequireAll creates scope checks for a tool that always needs the given scopes. From 4443619ee113b7161f260a5f9714d95b790c07b5 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 1 Sep 2026 12:56:02 +0200 Subject: [PATCH 2/6] fix(governance): complete ruleset routing and validation Add organization rule-suite routing and current filters, harden schema validation, and keep scope metadata usable for fixed-scope tokens and library callers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 6 +- .../create_repository_ruleset.snap | 5 + .../repository_ruleset_read.snap | 17 +- pkg/github/rulesets.go | 172 +++++++++++------- pkg/github/rulesets_test.go | 139 ++++++++------ pkg/scopes/scopes_test.go | 11 ++ 6 files changed, 228 insertions(+), 122 deletions(-) diff --git a/README.md b/README.md index 30ea93473f..9d32100497 100644 --- a/README.md +++ b/README.md @@ -897,6 +897,7 @@ The following sets of tools are available: - `actor_name`: The handle for the GitHub user account to filter rule suites on. Used by the 'list_rule_suites' method. (string, optional) - `branch`: Branch name. Required for the 'get_rules_for_branch' method. (string, optional) - `enterprise`: Enterprise slug. Required when level is 'enterprise'. (string, optional) + - `evaluate_status`: Filter rule suites by ruleset evaluation mode. Used by the 'list_rule_suites' method. (string, optional) - `includes_parents`: Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods at the repository level. (boolean, optional) - `level`: The level at which the ruleset is configured: - 'repository': A ruleset on a single repository (requires 'owner' and 'repo'). @@ -906,14 +907,15 @@ The following sets of tools are available: - 'get': Get a specific ruleset by ID (requires 'ruleset_id'). Supported at every level. - 'list': List all rulesets. Supported at every level. - 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). Repository level only. - - 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository level only. - - 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository level only. (string, required) + - 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository and organization levels only. + - 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository and organization levels only. (string, required) - `org`: Organization name. Required when level is 'organization'. (string, optional) - `owner`: Repository owner. Required when level is 'repository'. (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `ref`: The name of the ref (branch, tag, etc.) to filter rule suites by. Used by the 'list_rule_suites' method. (string, optional) - `repo`: Repository name. Required when level is 'repository'. (string, optional) + - `repository_name`: Repository name to filter rule suites by. Used by the 'list_rule_suites' method at the organization level. (string, optional) - `rule_suite_id`: Rule suite ID. Required for the 'get_rule_suite' method. (number, optional) - `rule_suite_result`: The rule suite result to filter by. Used by the 'list_rule_suites' method. (string, optional) - `ruleset_id`: Ruleset ID. Required for the 'get' method. (number, optional) diff --git a/pkg/github/__toolsnaps__/create_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_repository_ruleset.snap index 8ee7fbd724..82b596dc58 100644 --- a/pkg/github/__toolsnaps__/create_repository_ruleset.snap +++ b/pkg/github/__toolsnaps__/create_repository_ruleset.snap @@ -10,6 +10,7 @@ "bypass_actors": { "description": "The actors that can bypass the rules in this ruleset", "items": { + "additionalProperties": false, "properties": { "actor_id": { "description": "The ID of the actor that can bypass a ruleset", @@ -39,6 +40,9 @@ "type": "string" } }, + "required": [ + "actor_type" + ], "type": "object" }, "type": "array" @@ -88,6 +92,7 @@ "rules": { "description": "An array of rules within the ruleset. Each rule is an object with a 'type' (e.g. 'creation', 'deletion', 'non_fast_forward', 'required_signatures', 'pull_request', 'required_status_checks') and, for rules that need configuration, a 'parameters' object", "items": { + "additionalProperties": false, "properties": { "parameters": { "description": "Parameters for rule types that require additional configuration", diff --git a/pkg/github/__toolsnaps__/repository_ruleset_read.snap b/pkg/github/__toolsnaps__/repository_ruleset_read.snap index 2a79b4cfc9..47b4c3742d 100644 --- a/pkg/github/__toolsnaps__/repository_ruleset_read.snap +++ b/pkg/github/__toolsnaps__/repository_ruleset_read.snap @@ -4,7 +4,7 @@ "readOnlyHint": true, "title": "Read repository rulesets" }, - "description": "Read rulesets and rule suites at the repository, organization, or enterprise level. Select the level with the 'level' parameter and the operation with the 'method' parameter.", + "description": "Read rulesets at the repository, organization, or enterprise level, and rule suites at the repository or organization level. Select the level with the 'level' parameter and the operation with the 'method' parameter.", "inputSchema": { "properties": { "actor_name": { @@ -19,6 +19,15 @@ "description": "Enterprise slug. Required when level is 'enterprise'.", "type": "string" }, + "evaluate_status": { + "description": "Filter rule suites by ruleset evaluation mode. Used by the 'list_rule_suites' method.", + "enum": [ + "all", + "active", + "evaluate" + ], + "type": "string" + }, "includes_parents": { "description": "Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods at the repository level.", "type": "boolean" @@ -33,7 +42,7 @@ "type": "string" }, "method": { - "description": "Operation to perform:\n- 'get': Get a specific ruleset by ID (requires 'ruleset_id'). Supported at every level.\n- 'list': List all rulesets. Supported at every level.\n- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). Repository level only.\n- 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository level only.\n- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository level only.", + "description": "Operation to perform:\n- 'get': Get a specific ruleset by ID (requires 'ruleset_id'). Supported at every level.\n- 'list': List all rulesets. Supported at every level.\n- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). Repository level only.\n- 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository and organization levels only.\n- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository and organization levels only.", "enum": [ "get", "list", @@ -70,6 +79,10 @@ "description": "Repository name. Required when level is 'repository'.", "type": "string" }, + "repository_name": { + "description": "Repository name to filter rule suites by. Used by the 'list_rule_suites' method at the organization level.", + "type": "string" + }, "rule_suite_id": { "description": "Rule suite ID. Required for the 'get_rule_suite' method.", "type": "number" diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index 96c1ff66e5..b9a2890f9f 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -67,7 +67,11 @@ func rulesetReadScopeAccess() inventory.ScopeAccess { func rulesetWriteScopeAccess() inventory.ScopeAccess { return scopes.DynamicChallenge( []scopes.Scope{scopes.Repo, scopes.AdminOrg, scopes.AdminEnterprise}, - func([]string) bool { return true }, + func(activeScopes []string) bool { + return scopes.HasAll(activeScopes, scopes.Repo) || + scopes.HasAll(activeScopes, scopes.AdminOrg) || + scopes.HasAll(activeScopes, scopes.AdminEnterprise) + }, func(arguments map[string]any, activeScopes []string) []string { level, ok := arguments["level"].(string) if !ok { @@ -87,16 +91,13 @@ func rulesetWriteScopeAccess() inventory.ScopeAccess { ) } -// RepositoryRulesetRead creates a tool for read operations on rulesets and -// rule suites at the repository, organization, or enterprise level. The -// level is selected with the "level" parameter and the operation with the -// "method" parameter. +// RepositoryRulesetRead creates a tool for ruleset and rule-suite reads. func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( ToolsetMetadataGovernance, mcp.Tool{ Name: "repository_ruleset_read", - Description: t("TOOL_REPOSITORY_RULESET_READ_DESCRIPTION", "Read rulesets and rule suites at the repository, organization, or enterprise level. Select the level with the 'level' parameter and the operation with the 'method' parameter."), + Description: t("TOOL_REPOSITORY_RULESET_READ_DESCRIPTION", "Read rulesets at the repository, organization, or enterprise level, and rule suites at the repository or organization level. Select the level with the 'level' parameter and the operation with the 'method' parameter."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_REPOSITORY_RULESET_READ_USER_TITLE", "Read repository rulesets"), ReadOnlyHint: true, @@ -116,8 +117,8 @@ func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.Serve "- 'get': Get a specific ruleset by ID (requires 'ruleset_id'). Supported at every level.\n" + "- 'list': List all rulesets. Supported at every level.\n" + "- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). Repository level only.\n" + - "- 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository level only.\n" + - "- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository level only.", + "- 'list_rule_suites': List rule suites, the evaluations of rules against pushes. Repository and organization levels only.\n" + + "- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). Repository and organization levels only.", }, "owner": { Type: "string", @@ -160,11 +161,20 @@ func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.Serve Type: "string", Description: "The handle for the GitHub user account to filter rule suites on. Used by the 'list_rule_suites' method.", }, + "repository_name": { + Type: "string", + Description: "Repository name to filter rule suites by. Used by the 'list_rule_suites' method at the organization level.", + }, "rule_suite_result": { Type: "string", Enum: []any{"pass", "fail", "bypass", "all"}, Description: "The rule suite result to filter by. Used by the 'list_rule_suites' method.", }, + "evaluate_status": { + Type: "string", + Enum: []any{"all", "active", "evaluate"}, + Description: "Filter rule suites by ruleset evaluation mode. Used by the 'list_rule_suites' method.", + }, "rule_suite_id": { Type: "number", Description: "Rule suite ID. Required for the 'get_rule_suite' method.", @@ -220,8 +230,7 @@ func repositoryRulesetReadRepository(ctx context.Context, client *github.Client, if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - // GetRuleset always sends includes_parents; default to the - // GitHub API default of true when the caller omits it. + // GetRuleset always sends includes_parents, so preserve GitHub's true default. includesParents := true if _, ok := args["includes_parents"]; ok { includesParents, err = OptionalParam[bool](args, "includes_parents") @@ -302,8 +311,26 @@ func repositoryRulesetReadOrganization(ctx context.Context, client *github.Clien } result, err := ListOrganizationRepositoryRulesets(ctx, client, org, pagination) return result, nil, err + case "list_rule_suites": + filters, err := ruleSuiteFiltersFromArgs(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pagination, err := OptionalPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := ListOrganizationRuleSuites(ctx, client, org, filters, pagination) + return result, nil, err + case "get_rule_suite": + ruleSuiteID, err := RequiredBigInt(args, "rule_suite_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := GetOrganizationRuleSuite(ctx, client, org, ruleSuiteID) + return result, nil, err default: - return utils.NewToolResultError(fmt.Sprintf("method %q is not supported for level \"organization\"; supported methods: get, list", method)), nil, nil + return utils.NewToolResultError(fmt.Sprintf("method %q is not supported for level \"organization\"; supported methods: get, list, list_rule_suites, get_rule_suite", method)), nil, nil } } @@ -387,48 +414,73 @@ func GetRepositoryRulesForBranch(ctx context.Context, client *github.Client, own return MarshalledTextResult(branchRules), nil } -// ruleSuiteFilters holds the optional filters for listing rule suites. -type ruleSuiteFilters struct { +// RuleSuiteFilters holds the optional filters for listing rule suites. +type RuleSuiteFilters struct { Ref string + RepositoryName string TimePeriod string ActorName string RuleSuiteResult string + EvaluateStatus string } -func ruleSuiteFiltersFromArgs(args map[string]any) (ruleSuiteFilters, error) { +func ruleSuiteFiltersFromArgs(args map[string]any) (RuleSuiteFilters, error) { ref, err := OptionalParam[string](args, "ref") if err != nil { - return ruleSuiteFilters{}, err + return RuleSuiteFilters{}, err + } + repositoryName, err := OptionalParam[string](args, "repository_name") + if err != nil { + return RuleSuiteFilters{}, err } timePeriod, err := OptionalParam[string](args, "time_period") if err != nil { - return ruleSuiteFilters{}, err + return RuleSuiteFilters{}, err } actorName, err := OptionalParam[string](args, "actor_name") if err != nil { - return ruleSuiteFilters{}, err + return RuleSuiteFilters{}, err } ruleSuiteResult, err := OptionalParam[string](args, "rule_suite_result") if err != nil { - return ruleSuiteFilters{}, err + return RuleSuiteFilters{}, err + } + evaluateStatus, err := OptionalParam[string](args, "evaluate_status") + if err != nil { + return RuleSuiteFilters{}, err } - return ruleSuiteFilters{ + return RuleSuiteFilters{ Ref: ref, + RepositoryName: repositoryName, TimePeriod: timePeriod, ActorName: actorName, RuleSuiteResult: ruleSuiteResult, + EvaluateStatus: evaluateStatus, }, nil } // ListRepositoryRuleSuites lists rule suites (evaluations of rules against // pushes) for a repository. Rule suites are not supported by go-github, so the // request is issued directly. -func ListRepositoryRuleSuites(ctx context.Context, client *github.Client, owner, repo string, filters ruleSuiteFilters, pagination PaginationParams) (*mcp.CallToolResult, error) { - apiURL := fmt.Sprintf("repos/%s/%s/rulesets/rule-suites", owner, repo) +func ListRepositoryRuleSuites(ctx context.Context, client *github.Client, owner, repo string, filters RuleSuiteFilters, pagination PaginationParams) (*mcp.CallToolResult, error) { + apiURL := fmt.Sprintf("repos/%s/%s/rulesets/rule-suites", url.PathEscape(owner), url.PathEscape(repo)) + return listRuleSuites(ctx, client, apiURL, "failed to list repository rule suites", filters, false, pagination) +} + +// ListOrganizationRuleSuites lists rule suites for an organization. +func ListOrganizationRuleSuites(ctx context.Context, client *github.Client, org string, filters RuleSuiteFilters, pagination PaginationParams) (*mcp.CallToolResult, error) { + apiURL := fmt.Sprintf("orgs/%s/rulesets/rule-suites", url.PathEscape(org)) + return listRuleSuites(ctx, client, apiURL, "failed to list organization rule suites", filters, true, pagination) +} + +func listRuleSuites(ctx context.Context, client *github.Client, apiURL, errorMessage string, filters RuleSuiteFilters, includeRepositoryName bool, pagination PaginationParams) (*mcp.CallToolResult, error) { query := url.Values{} if filters.Ref != "" { query.Set("ref", filters.Ref) } + if includeRepositoryName && filters.RepositoryName != "" { + query.Set("repository_name", filters.RepositoryName) + } if filters.TimePeriod != "" { query.Set("time_period", filters.TimePeriod) } @@ -438,6 +490,9 @@ func ListRepositoryRuleSuites(ctx context.Context, client *github.Client, owner, if filters.RuleSuiteResult != "" { query.Set("rule_suite_result", filters.RuleSuiteResult) } + if filters.EvaluateStatus != "" { + query.Set("evaluate_status", filters.EvaluateStatus) + } if pagination.Page > 0 { query.Set("page", strconv.Itoa(pagination.Page)) } @@ -459,17 +514,25 @@ func ListRepositoryRuleSuites(ctx context.Context, client *github.Client, owner, defer func() { _ = resp.Body.Close() }() } if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rule suites", resp, err), nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, errorMessage, resp, err), nil } return MarshalledTextResult(ruleSuites), nil } -// GetRepositoryRuleSuite gets details of a specific repository rule suite, -// including the evaluation results for each rule. Rule suites are not supported -// by go-github, so the request is issued directly. +// GetRepositoryRuleSuite gets details of a specific repository rule suite. func GetRepositoryRuleSuite(ctx context.Context, client *github.Client, owner, repo string, ruleSuiteID int64) (*mcp.CallToolResult, error) { - apiURL := fmt.Sprintf("repos/%s/%s/rulesets/rule-suites/%d", owner, repo, ruleSuiteID) + apiURL := fmt.Sprintf("repos/%s/%s/rulesets/rule-suites/%d", url.PathEscape(owner), url.PathEscape(repo), ruleSuiteID) + return getRuleSuite(ctx, client, apiURL, "failed to get repository rule suite") +} + +// GetOrganizationRuleSuite gets details of a specific organization rule suite. +func GetOrganizationRuleSuite(ctx context.Context, client *github.Client, org string, ruleSuiteID int64) (*mcp.CallToolResult, error) { + apiURL := fmt.Sprintf("orgs/%s/rulesets/rule-suites/%d", url.PathEscape(org), ruleSuiteID) + return getRuleSuite(ctx, client, apiURL, "failed to get organization rule suite") +} + +func getRuleSuite(ctx context.Context, client *github.Client, apiURL, errorMessage string) (*mcp.CallToolResult, error) { req, err := client.NewRequest(ctx, http.MethodGet, apiURL, nil) if err != nil { return utils.NewToolResultErrorFromErr("failed to create request", err), nil @@ -481,7 +544,7 @@ func GetRepositoryRuleSuite(ctx context.Context, client *github.Client, owner, r defer func() { _ = resp.Body.Close() }() } if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rule suite", resp, err), nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, errorMessage, resp, err), nil } return MarshalledTextResult(ruleSuite), nil @@ -538,7 +601,7 @@ func GetEnterpriseRepositoryRuleset(ctx context.Context, client *github.Client, // enterprise. Listing enterprise rulesets is not supported by go-github, so // the request is issued directly. func ListEnterpriseRepositoryRulesets(ctx context.Context, client *github.Client, enterprise string, pagination PaginationParams) (*mcp.CallToolResult, error) { - apiURL := fmt.Sprintf("enterprises/%s/rulesets", enterprise) + apiURL := fmt.Sprintf("enterprises/%s/rulesets", url.PathEscape(enterprise)) query := url.Values{} if pagination.Page > 0 { query.Set("page", strconv.Itoa(pagination.Page)) @@ -555,7 +618,7 @@ func ListEnterpriseRepositoryRulesets(ctx context.Context, client *github.Client return utils.NewToolResultErrorFromErr("failed to create request", err), nil } - var rulesets any + var rulesets []*github.RepositoryRuleset resp, err := client.Do(req, &rulesets) if resp != nil { defer func() { _ = resp.Body.Close() }() @@ -688,7 +751,8 @@ func rulesetWriteProperties() map[string]*jsonschema.Schema { Type: "array", Description: "An array of rules within the ruleset. Each rule is an object with a 'type' (e.g. 'creation', 'deletion', 'non_fast_forward', 'required_signatures', 'pull_request', 'required_status_checks') and, for rules that need configuration, a 'parameters' object", Items: &jsonschema.Schema{ - Type: "object", + Type: "object", + AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}}, Properties: map[string]*jsonschema.Schema{ "type": { Type: "string", @@ -710,7 +774,8 @@ func rulesetWriteProperties() map[string]*jsonschema.Schema { Type: "array", Description: "The actors that can bypass the rules in this ruleset", Items: &jsonschema.Schema{ - Type: "object", + Type: "object", + AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}}, Properties: map[string]*jsonschema.Schema{ "actor_id": { Type: "number", @@ -727,6 +792,7 @@ func rulesetWriteProperties() map[string]*jsonschema.Schema { Description: "When the specified actor can bypass the ruleset. 'pull_request' only applies to branch rulesets and is not valid for the 'DeployKey' actor type. 'exempt' means rules are not run for that actor and no bypass audit entry is created.", }, }, + Required: []string{"actor_type"}, }, }, } @@ -757,19 +823,22 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules requestedRuleTypes := make([]string, 0, len(rules)) requestedRuleParameters := make(map[string]map[string]any, len(rules)) seenRuleTypes := make(map[string]bool, len(rules)) - for _, rule := range rules { + for i, rule := range rules { ruleMap, ok := rule.(map[string]any) if !ok { return github.RepositoryRuleset{}, utils.NewToolResultError("each rule must be an object with a 'type' field") } + for key := range ruleMap { + if key != "type" && key != "parameters" { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("rules[%d]: unsupported or unrecognized key: %q", i, key)) + } + } ruleType, ok := ruleMap["type"].(string) if !ok || ruleType == "" { return github.RepositoryRuleset{}, utils.NewToolResultError("each rule must have a non-empty string 'type' field") } if seenRuleTypes[ruleType] { - // github.RepositoryRulesetRules has a single field per rule type, so a - // second rule of the same type would silently overwrite the first - // during the round-trip below rather than producing two rules. + // go-github stores only one rule per type. return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("duplicate rule type: %q (a ruleset may only have one rule of each type)", ruleType)) } seenRuleTypes[ruleType] = true @@ -808,10 +877,6 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules if !ok { return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d] must be an object", i)) } - // github.BypassActor recognizes only these three keys; any other key - // (e.g. a "bypass_modes" typo) is silently discarded by JSON - // unmarshal, which would grant the actor the default "always" bypass - // mode instead of the caller's intended value. for key := range actorMap { if key != "actor_id" && key != "actor_type" && key != "bypass_mode" { return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d]: unsupported or unrecognized key: %q", i, key)) @@ -830,12 +895,7 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules return github.RepositoryRuleset{}, utils.NewToolResultErrorFromErr("failed to parse ruleset request", err) } - // github.RepositoryRulesetRules.UnmarshalJSON silently discards rule types and - // rule parameters it does not recognize, which would let a typo (e.g. - // "require_code_owners_review" instead of "require_code_owner_review") create - // a weaker ruleset than the caller requested. Verify every requested rule - // type, and every supplied parameter key within it (recursively), survived - // the round-trip. + // Reject fields silently discarded by go-github's custom ruleset unmarshaler. appliedRules, errResult := rulesetAppliedRules(ruleset.Rules) if errResult != nil { return github.RepositoryRuleset{}, errResult @@ -852,9 +912,6 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules } } - // github.RepositoryRulesetConditions has the same silent-drop behavior for - // unrecognized keys (e.g. "ref_names" instead of "ref_name"), so verify the - // requested conditions survived the round-trip the same way. if requestedConditions, ok := payload["conditions"].(map[string]any); ok { appliedConditions, errResult := rulesetAppliedConditions(ruleset.Conditions) if errResult != nil { @@ -868,16 +925,8 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules return ruleset, nil } -// droppedKeyPath recursively compares a caller-supplied object against its -// round-tripped counterpart and returns the path of the first key or array -// element that did not survive (e.g. "required_status_checks[0].integration_id"), -// or "" if everything survived. A caller-supplied value that is a JSON zero -// value (false, 0, "", or an empty array/object) is exempt, since it is -// indistinguishable from a field omitted by a `json:",omitempty"` struct tag -// on the far side of the round-trip. This round-trip is entirely local (our -// own JSON marshal/unmarshal of a go-github struct, not a remote API -// response), so slice order and length are preserved deterministically and -// array elements are safe to compare by index. +// droppedKeyPath returns the first caller-supplied field omitted by the local +// go-github JSON round-trip. JSON zero values may disappear through omitempty. func droppedKeyPath(requested, applied map[string]any) string { for key, requestedValue := range requested { appliedValue, ok := applied[key] @@ -894,9 +943,6 @@ func droppedKeyPath(requested, applied map[string]any) string { return "" } -// droppedValuePath recurses into map and array values on behalf of -// droppedKeyPath. It returns a path suffix beginning with "." (object key) or -// "[i]" (array index), or "" when requested and applied agree closely enough. func droppedValuePath(requested, applied any) string { switch requestedTyped := requested.(type) { case map[string]any: @@ -930,10 +976,6 @@ func droppedValuePath(requested, applied any) string { } } -// isZeroJSONValue reports whether v is the JSON zero value for its type -// (false, 0, "", nil, or an empty array/object). Such values are -// indistinguishable from an omitted field once round-tripped through a Go -// struct field tagged `omitempty`. func isZeroJSONValue(v any) bool { switch value := v.(type) { case nil: diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index 46c53825df..4cf89d70be 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -181,6 +181,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { "time_period": "week", "actor_name": "octocat", "rule_suite_result": "pass", + "evaluate_status": "evaluate", "perPage": float64(25), }) @@ -193,6 +194,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "week", capturedQuery.Get("time_period")) assert.Equal(t, "octocat", capturedQuery.Get("actor_name")) assert.Equal(t, "pass", capturedQuery.Get("rule_suite_result")) + assert.Equal(t, "evaluate", capturedQuery.Get("evaluate_status")) assert.Equal(t, "25", capturedQuery.Get("per_page")) }) @@ -281,7 +283,46 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "org rs", returned[0].Name) }) - t.Run("organization level: repository-only method defers to normal validation", func(t *testing.T) { + t.Run("organization level: list_rule_suites forwards organization filters", func(t *testing.T) { + var capturedQuery url.Values + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /orgs/{org}/rulesets/rule-suites": func(w http.ResponseWriter, r *http.Request) { + capturedQuery = r.URL.Query() + mockResponse(t, http.StatusOK, []map[string]any{{"id": 101, "repository_name": "repo"}})(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "organization", + "method": "list_rule_suites", + "org": "octo", + "repository_name": "repo", + "evaluate_status": "active", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Equal(t, "repo", capturedQuery.Get("repository_name")) + assert.Equal(t, "active", capturedQuery.Get("evaluate_status")) + }) + + t.Run("organization level: get_rule_suite", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /orgs/{org}/rulesets/rule-suites/{rule_suite_id}": mockResponse(t, http.StatusOK, map[string]any{"id": 101, "result": "pass"}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "organization", "method": "get_rule_suite", "org": "octo", "rule_suite_id": float64(101)}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "pass") + }) + + t.Run("organization level: repository-only method is rejected", func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) deps := BaseDeps{Client: client} handler := toolDef.Handler(deps) @@ -352,7 +393,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "enterprise rs", returned[0].Name) }) - t.Run("enterprise level: repository-only method defers to normal validation", func(t *testing.T) { + t.Run("enterprise level: rule suite method is rejected", func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) deps := BaseDeps{Client: client} handler := toolDef.Handler(deps) @@ -377,13 +418,6 @@ func Test_RepositoryRulesetRead(t *testing.T) { }) t.Run("mismatched-case level is rejected rather than silently normalized", func(t *testing.T) { - // The scope challenge in rulesetReadScopeAccess matches "level" with an - // exact, case-sensitive comparison. If the handler instead normalized case - // (e.g. via strings.ToLower) before dispatching, a caller could send - // "Organization" to reach the organization-level read while the OAuth - // middleware -- which sees the raw, un-normalized argument -- would find no - // case matching "organization" and issue no scope challenge at all, - // letting an under-scoped token read organization rulesets for free. called := false client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "GET /orgs/{org}/rulesets": func(w http.ResponseWriter, r *http.Request) { @@ -401,8 +435,6 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Contains(t, getErrorResult(t, result).Text, "unknown level") assert.False(t, called, "a mismatched-case level must not reach the organization-level API call") - // The scope challenge must independently agree: it must not treat - // "Organization" as a recognized level either. assert.Empty(t, toolDef.ScopeAccess.Challenge(map[string]any{"level": "Organization"}, nil)) }) } @@ -465,8 +497,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { assert.Equal(t, github.RulesetEnforcement("active"), capturedBody.Enforcement) require.NotNil(t, capturedBody.Rules) - // Verify the outbound body preserves all requested rules and the pull_request - // parameters, rather than silently dropping them in the JSON round-trip. var outbound struct { Rules []struct { Type string `json:"type"` @@ -540,6 +570,37 @@ func Test_CreateRepositoryRuleset(t *testing.T) { assert.False(t, called, "request must not be sent when a rule type is unsupported") }) + t.Run("unrecognized rule key", func(t *testing.T) { + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "x", + "enforcement": "active", + "rules": []any{ + map[string]any{ + "type": "pull_request", + "configuration": map[string]any{"required_approving_review_count": float64(2)}, + }, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "configuration") + assert.False(t, called) + }) + t.Run("invalid rules", func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) deps := BaseDeps{Client: client} @@ -589,11 +650,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { }) t.Run("unrecognized rule parameter is rejected even though the rule type is valid", func(t *testing.T) { - // "require_code_owners_review" is a plausible typo for the real - // pull_request parameter "require_code_owner_review". go-github's - // generated UnmarshalJSON silently drops unknown parameter keys, so - // without this check the ruleset would be created with the weaker - // default (false) instead of surfacing the mistake. called := false client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { @@ -627,11 +683,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { }) t.Run("zero-valued parameters are not flagged as unrecognized", func(t *testing.T) { - // Scalar fields without `omitempty` (like required_approving_review_count) - // always round-trip, but slice fields with `omitempty` (like - // allowed_merge_methods) vanish from the response when empty. An - // explicit zero value supplied by the caller must not be misread as an - // unsupported parameter key. var capturedBody []byte client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { @@ -668,14 +719,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { }) t.Run("unrecognized key inside a rule parameter array element is rejected", func(t *testing.T) { - // "integration_ids" is a plausible typo for the real per-check field - // "integration_id" on required_status_checks[]. Unlike the top-level - // rule/condition round-trip, this array is produced by our own local - // JSON marshal/unmarshal of the go-github struct (not a remote API - // response), so element order is guaranteed stable and comparing by - // index is safe. Without this check, the typo would silently vanish and - // the resulting rule would accept a status check from any integration - // instead of only the one requested. called := false client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { @@ -787,11 +830,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { }) t.Run("unrecognized bypass_actors key is rejected", func(t *testing.T) { - // "bypass_modes" is a plausible typo for "bypass_mode". github.BypassActor - // only recognizes actor_id/actor_type/bypass_mode, so an unknown key is - // silently discarded during JSON unmarshal -- and because the API - // defaults an omitted bypass_mode to "always", the resulting actor would - // get broader bypass rights than the caller requested. called := false client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { @@ -821,11 +859,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { }) t.Run("unrecognized top-level condition key is rejected", func(t *testing.T) { - // "ref_names" is a plausible typo for the real condition key "ref_name". - // github.RepositoryRulesetConditions silently drops unknown keys during - // JSON unmarshal, so without this check the ruleset would be created with - // no ref_name condition at all (applying to every ref) instead of - // surfacing the mistake. called := false client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { @@ -1028,13 +1061,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { }) t.Run("mismatched-case level is rejected rather than silently normalized", func(t *testing.T) { - // Mirrors the read-tool regression above: rulesetWriteScopeAccess only - // recognizes an exact, lowercase "organization"/"enterprise" match. If the - // handler normalized case before dispatching, "Organization" would reach - // client.Organizations.CreateRepositoryRuleset while the OAuth middleware - // -- which challenges on the raw argument -- would see no case match and - // require no admin:org scope at all, letting an under-scoped token create - // organization-wide rulesets for free. called := false client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "POST /orgs/{org}/rulesets": func(w http.ResponseWriter, _ *http.Request) { @@ -1062,10 +1088,6 @@ func Test_CreateRepositoryRuleset(t *testing.T) { }) } -// Test_RulesetScopeChallenges verifies that the ruleset read and write tools -// challenge for the exact scope implied by the "level" argument, and defer to -// normal handler validation (no challenge) when "level" is missing or not a -// string. func Test_RulesetScopeChallenges(t *testing.T) { tests := []struct { name string @@ -1150,7 +1172,6 @@ func Test_RulesetScopeChallenges(t *testing.T) { t.Run(tt.name, func(t *testing.T) { require.NotNil(t, tt.tool.ScopeAccess.Challenge) assert.True(t, tt.tool.ScopeAccess.Dynamic) - assert.True(t, tt.tool.ScopeAccess.Visible(nil)) assert.Empty(t, tt.tool.ScopeAccess.Challenge(tt.arguments, tt.allowed)) if tt.disallowed == nil { assert.Empty(t, tt.tool.ScopeAccess.Challenge(tt.arguments, nil)) @@ -1161,6 +1182,18 @@ func Test_RulesetScopeChallenges(t *testing.T) { } } +func Test_RulesetScopeVisibility(t *testing.T) { + readAccess := RepositoryRulesetRead(translations.NullTranslationHelper).ScopeAccess + writeAccess := CreateRepositoryRuleset(translations.NullTranslationHelper).ScopeAccess + + assert.True(t, readAccess.Visible(nil)) + assert.False(t, writeAccess.Visible(nil)) + assert.True(t, writeAccess.Visible([]string{"repo"})) + assert.True(t, writeAccess.Visible([]string{"admin:org"})) + assert.True(t, writeAccess.Visible([]string{"admin:enterprise"})) + assert.False(t, writeAccess.Visible([]string{"read:org", "read:enterprise"})) +} + func Test_RulesetScopeMetadataIsExhaustive(t *testing.T) { tests := []struct { tool inventory.ServerTool diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index 62bfc6178e..e9deda9953 100644 --- a/pkg/scopes/scopes_test.go +++ b/pkg/scopes/scopes_test.go @@ -17,6 +17,12 @@ func TestOAuthScopeCatalog(t *testing.T) { assert.NotContains(t, defaults, string(Workflow)) assert.Contains(t, supported, string(Codespace)) assert.NotContains(t, defaults, string(Codespace)) + assert.Contains(t, supported, string(AdminOrg)) + assert.NotContains(t, defaults, string(AdminOrg)) + assert.Contains(t, supported, string(ReadEnterprise)) + assert.NotContains(t, defaults, string(ReadEnterprise)) + assert.Contains(t, supported, string(AdminEnterprise)) + assert.NotContains(t, defaults, string(AdminEnterprise)) } func TestScopeChecks(t *testing.T) { @@ -25,8 +31,12 @@ func TestScopeChecks(t *testing.T) { assert.False(t, HasAll([]string{"repo"}, Repo, Workflow)) assert.True(t, HasAll([]string{"admin:org"}, ReadOrg)) assert.True(t, HasAllScopeNames([]string{"admin:org"}, []string{"read:org"})) + assert.True(t, HasAll([]string{"admin:enterprise"}, ReadEnterprise)) + assert.True(t, HasAllScopeNames([]string{"admin:enterprise"}, []string{"read:enterprise"})) + assert.False(t, HasAll([]string{"read:enterprise"}, AdminEnterprise)) assert.False(t, HasAllScopeNames([]string{"repo"}, []string{"repo", "workflow"})) assert.Nil(t, ChallengeAll([]string{"repo", "workflow"}, Repo, Workflow)) + assert.Nil(t, ChallengeAll([]string{"admin:enterprise"}, ReadEnterprise)) assert.Equal(t, []string{"repo", "workflow"}, ChallengeAll([]string{"repo"}, Repo, Workflow)) } @@ -53,6 +63,7 @@ func TestScopeHierarchy(t *testing.T) { assert.Contains(t, ScopeHierarchy[Repo], SecurityEvents) assert.Contains(t, ScopeHierarchy[AdminOrg], WriteOrg) assert.Contains(t, ScopeHierarchy[AdminOrg], ReadOrg) + assert.Contains(t, ScopeHierarchy[AdminEnterprise], ReadEnterprise) assert.Contains(t, ScopeHierarchy[WriteOrg], ReadOrg) assert.Contains(t, ScopeHierarchy[Project], ReadProject) assert.Contains(t, ScopeHierarchy[WritePackages], ReadPackages) From 0ebe8d7b251fa25a93bdcdc38f97a9331aa260fb Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 2 Sep 2026 12:11:26 +0200 Subject: [PATCH 3/6] fix(governance): escape ruleset branch path Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/rulesets.go | 2 +- pkg/github/rulesets_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index b9a2890f9f..5d85ca1350 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -403,7 +403,7 @@ func GetRepositoryRulesForBranch(ctx context.Context, client *github.Client, own PerPage: pagination.PerPage, } - branchRules, resp, err := client.Repositories.ListRulesForBranch(ctx, owner, repo, branch, opts) + branchRules, resp, err := client.Repositories.ListRulesForBranch(ctx, owner, repo, url.PathEscape(branch), opts) if resp != nil { defer func() { _ = resp.Body.Close() }() } diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index 4cf89d70be..f3473d3d2b 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -439,6 +439,38 @@ func Test_RepositoryRulesetRead(t *testing.T) { }) } +func Test_GetRepositoryRulesForBranchEscapesBranch(t *testing.T) { + tests := []struct { + name string + branch string + escapedPath string + }{ + { + name: "slash", + branch: "release/1.0", + escapedPath: "/repos/owner/repo/rules/branches/release%2F1.0", + }, + { + name: "special characters", + branch: "release/#1%ready", + escapedPath: "/repos/owner/repo/rules/branches/release%2F%231%25ready", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandler(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, tt.escapedPath, r.URL.EscapedPath()) + mockResponse(t, http.StatusOK, []map[string]any{{"type": "creation"}})(w, r) + })) + + result, err := GetRepositoryRulesForBranch(t.Context(), client, "owner", "repo", tt.branch, PaginationParams{}) + require.NoError(t, err) + require.False(t, result.IsError) + }) + } +} + func Test_CreateRepositoryRuleset(t *testing.T) { toolDef := CreateRepositoryRuleset(translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) From 4ddbb27a8837f2242b6e6415b1016fd8a3e77b38 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 2 Sep 2026 12:29:33 +0200 Subject: [PATCH 4/6] fix(governance): decode enterprise ruleset lists Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/rulesets.go | 11 +++-- pkg/github/rulesets_test.go | 85 +++++++++++++++++++++++++++++++------ 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index 5d85ca1350..d17add5af7 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -597,6 +597,11 @@ func GetEnterpriseRepositoryRuleset(ctx context.Context, client *github.Client, return MarshalledTextResult(ruleset), nil } +type enterpriseRepositoryRulesetsResponse struct { + TotalCount int `json:"total_count"` + Rulesets []*github.RepositoryRuleset `json:"rulesets"` +} + // ListEnterpriseRepositoryRulesets lists all repository rulesets for an // enterprise. Listing enterprise rulesets is not supported by go-github, so // the request is issued directly. @@ -618,8 +623,8 @@ func ListEnterpriseRepositoryRulesets(ctx context.Context, client *github.Client return utils.NewToolResultErrorFromErr("failed to create request", err), nil } - var rulesets []*github.RepositoryRuleset - resp, err := client.Do(req, &rulesets) + var result enterpriseRepositoryRulesetsResponse + resp, err := client.Do(req, &result) if resp != nil { defer func() { _ = resp.Body.Close() }() } @@ -627,7 +632,7 @@ func ListEnterpriseRepositoryRulesets(ctx context.Context, client *github.Client return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list enterprise repository rulesets", resp, err), nil } - return MarshalledTextResult(rulesets), nil + return MarshalledTextResult(result), nil } // CreateRepositoryRuleset creates a tool to create a new repository ruleset diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index f3473d3d2b..8e32b4c8ba 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -376,21 +376,80 @@ func Test_RepositoryRulesetRead(t *testing.T) { }) t.Run("enterprise level: list", func(t *testing.T) { - client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - "GET /enterprises/{enterprise}/rulesets": mockResponse(t, http.StatusOK, []*github.RepositoryRuleset{{Name: "enterprise rs"}}), - })) - deps := BaseDeps{Client: client} - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{"level": "enterprise", "method": "list", "enterprise": "acme"}) + t.Run("success preserves total count and pagination", func(t *testing.T) { + var capturedQuery url.Values + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /enterprises/{enterprise}/rulesets": func(w http.ResponseWriter, r *http.Request) { + capturedQuery = r.URL.Query() + mockResponse(t, http.StatusOK, map[string]any{ + "total_count": 17, + "rulesets": []*github.RepositoryRuleset{{Name: "enterprise rs"}}, + })(w, r) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "enterprise", + "method": "list", + "enterprise": "acme", + "page": float64(2), + "perPage": float64(1), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Equal(t, "2", capturedQuery.Get("page")) + assert.Equal(t, "1", capturedQuery.Get("per_page")) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) + var returned struct { + TotalCount int `json:"total_count"` + Rulesets []*github.RepositoryRuleset `json:"rulesets"` + } + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, 17, returned.TotalCount) + require.Len(t, returned.Rulesets, 1) + assert.Equal(t, "enterprise rs", returned.Rulesets[0].Name) + }) - var returned []*github.RepositoryRuleset - require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) - require.Len(t, returned, 1) - assert.Equal(t, "enterprise rs", returned[0].Name) + t.Run("empty response preserves object shape", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /enterprises/{enterprise}/rulesets": mockResponse(t, http.StatusOK, map[string]any{ + "total_count": 0, + "rulesets": []*github.RepositoryRuleset{}, + }), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "enterprise", "method": "list", "enterprise": "acme"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var returned struct { + TotalCount int `json:"total_count"` + Rulesets []*github.RepositoryRuleset `json:"rulesets"` + } + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Zero(t, returned.TotalCount) + assert.Empty(t, returned.Rulesets) + }) + + t.Run("API error", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /enterprises/{enterprise}/rulesets": mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "Internal Server Error"}), + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{"level": "enterprise", "method": "list", "enterprise": "acme"}) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "failed to list enterprise repository rulesets") + }) }) t.Run("enterprise level: rule suite method is rejected", func(t *testing.T) { From 1c2d8567da2eea622f03cdfb782e394b1cb151c4 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 2 Sep 2026 13:53:40 +0200 Subject: [PATCH 5/6] fix(governance): reject unknown ruleset create fields Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../create_repository_ruleset.snap | 1 + pkg/github/rulesets.go | 7 ++- pkg/github/rulesets_test.go | 55 +++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/pkg/github/__toolsnaps__/create_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_repository_ruleset.snap index 82b596dc58..d835b87dc3 100644 --- a/pkg/github/__toolsnaps__/create_repository_ruleset.snap +++ b/pkg/github/__toolsnaps__/create_repository_ruleset.snap @@ -6,6 +6,7 @@ }, "description": "Create a new ruleset at the repository, organization, or enterprise level", "inputSchema": { + "additionalProperties": false, "properties": { "bypass_actors": { "description": "The actors that can bypass the rules in this ruleset", diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index d17add5af7..e5624b0d06 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -660,9 +660,10 @@ func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.Ser ReadOnlyHint: false, }, InputSchema: &jsonschema.Schema{ - Type: "object", - Properties: properties, - Required: []string{"level", "name", "enforcement", "rules"}, + Type: "object", + AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}}, + Properties: properties, + Required: []string{"level", "name", "enforcement", "rules"}, }, }, rulesetWriteScopeAccess(), diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index 8e32b4c8ba..bae10562cb 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "io" + "maps" "net/http" "net/url" "testing" @@ -540,6 +541,60 @@ func Test_CreateRepositoryRuleset(t *testing.T) { schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) require.True(t, ok) assert.ElementsMatch(t, schema.Required, []string{"level", "name", "enforcement", "rules"}) + require.NotNil(t, schema.AdditionalProperties) + require.NotNil(t, schema.AdditionalProperties.Not) + + propertyNames := make([]string, 0, len(schema.Properties)) + for name := range schema.Properties { + propertyNames = append(propertyNames, name) + } + assert.ElementsMatch(t, []string{ + "level", + "owner", + "repo", + "org", + "enterprise", + "name", + "enforcement", + "target", + "rules", + "conditions", + "bypass_actors", + }, propertyNames) + + resolvedSchema, err := schema.Resolve(nil) + require.NoError(t, err) + validArgs := map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "org": "org", + "enterprise": "enterprise", + "name": "main protection", + "enforcement": "active", + "target": "branch", + "rules": []any{map[string]any{"type": "creation"}}, + "conditions": map[string]any{"ref_name": map[string]any{"include": []any{"refs/heads/main"}}}, + "bypass_actors": []any{ + map[string]any{"actor_type": "OrganizationAdmin", "bypass_mode": "always"}, + }, + } + require.NoError(t, resolvedSchema.Validate(validArgs)) + + for _, test := range []struct { + field string + typo string + }{ + {field: "target", typo: "targte"}, + {field: "conditions", typo: "conditons"}, + } { + t.Run("rejects misspelled "+test.field, func(t *testing.T) { + args := maps.Clone(validArgs) + args[test.typo] = args[test.field] + delete(args, test.field) + require.Error(t, resolvedSchema.Validate(args)) + }) + } t.Run("repository level", func(t *testing.T) { var capturedBody github.RepositoryRuleset From b89714aa8596dd8e3810582b7b51ab631b3dc279 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 2 Sep 2026 15:17:28 +0200 Subject: [PATCH 6/6] fix(governance): require explicit ruleset bypass mode Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../create_repository_ruleset.snap | 3 +- pkg/github/rulesets.go | 11 ++- pkg/github/rulesets_test.go | 82 +++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/pkg/github/__toolsnaps__/create_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_repository_ruleset.snap index d835b87dc3..81f2cadcb2 100644 --- a/pkg/github/__toolsnaps__/create_repository_ruleset.snap +++ b/pkg/github/__toolsnaps__/create_repository_ruleset.snap @@ -42,7 +42,8 @@ } }, "required": [ - "actor_type" + "actor_type", + "bypass_mode" ], "type": "object" }, diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index e5624b0d06..ac05282aaa 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -798,7 +798,7 @@ func rulesetWriteProperties() map[string]*jsonschema.Schema { Description: "When the specified actor can bypass the ruleset. 'pull_request' only applies to branch rulesets and is not valid for the 'DeployKey' actor type. 'exempt' means rules are not run for that actor and no bypass audit entry is created.", }, }, - Required: []string{"actor_type"}, + Required: []string{"actor_type", "bypass_mode"}, }, }, } @@ -888,6 +888,15 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d]: unsupported or unrecognized key: %q", i, key)) } } + bypassMode, ok := actorMap["bypass_mode"].(string) + if !ok { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d].bypass_mode is required and must be a string", i)) + } + switch github.BypassMode(bypassMode) { + case github.BypassModeAlways, github.BypassModePullRequest, github.BypassModeExempt: + default: + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d].bypass_mode must be one of \"always\", \"pull_request\", or \"exempt\"", i)) + } } payload["bypass_actors"] = bypassActorsArr } diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index bae10562cb..37de10f112 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -564,6 +564,11 @@ func Test_CreateRepositoryRuleset(t *testing.T) { resolvedSchema, err := schema.Resolve(nil) require.NoError(t, err) + bypassActorSchema := schema.Properties["bypass_actors"].Items + require.NotNil(t, bypassActorSchema) + assert.ElementsMatch(t, []string{"actor_type", "bypass_mode"}, bypassActorSchema.Required) + assert.ElementsMatch(t, []any{"always", "pull_request", "exempt"}, bypassActorSchema.Properties["bypass_mode"].Enum) + validArgs := map[string]any{ "level": "repository", "owner": "owner", @@ -581,6 +586,15 @@ func Test_CreateRepositoryRuleset(t *testing.T) { } require.NoError(t, resolvedSchema.Validate(validArgs)) + t.Run("bypass actor requires an explicit documented mode", func(t *testing.T) { + args := maps.Clone(validArgs) + args["bypass_actors"] = []any{map[string]any{"actor_type": "OrganizationAdmin"}} + require.Error(t, resolvedSchema.Validate(args)) + + args["bypass_actors"] = []any{map[string]any{"actor_type": "OrganizationAdmin", "bypass_mode": "never"}} + require.Error(t, resolvedSchema.Validate(args)) + }) + for _, test := range []struct { field string typo string @@ -944,6 +958,74 @@ func Test_CreateRepositoryRuleset(t *testing.T) { assert.NotEmpty(t, capturedBody) }) + for _, bypassMode := range []string{"always", "pull_request", "exempt"} { + t.Run("bypass_actors serializes "+bypassMode+" exactly", func(t *testing.T) { + var capturedRaw []byte + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) { + capturedRaw, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write(capturedRaw) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "main protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + "bypass_actors": []any{ + map[string]any{"actor_type": "OrganizationAdmin", "bypass_mode": bypassMode}, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var outbound struct { + BypassActors []struct { + BypassMode string `json:"bypass_mode"` + } `json:"bypass_actors"` + } + require.NoError(t, json.Unmarshal(capturedRaw, &outbound)) + require.Len(t, outbound.BypassActors, 1) + assert.Equal(t, bypassMode, outbound.BypassActors[0].BypassMode) + }) + } + + t.Run("bypass_actors without bypass_mode is rejected before request", func(t *testing.T) { + called := false + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusCreated) + }, + })) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "level": "repository", + "owner": "owner", + "repo": "repo", + "name": "main protection", + "enforcement": "active", + "rules": []any{map[string]any{"type": "creation"}}, + "bypass_actors": []any{ + map[string]any{"actor_type": "OrganizationAdmin"}, + }, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "bypass_actors[0].bypass_mode") + assert.False(t, called) + }) + t.Run("bypass_actors accepts exempt bypass mode and enterprise actor types", func(t *testing.T) { var capturedBody github.RepositoryRuleset client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{