From a1bf6c3aa208ea3eb0099b733770e83a01911cb2 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Sat, 1 Aug 2026 00:21:29 +0200 Subject: [PATCH 1/6] feat(governance): add rulesets tools in new governance toolset Add a new non-default "governance" toolset (icon: law) with tools for managing GitHub repository rulesets at the repository, organization, and enterprise levels. Read operations are consolidated behind method-dispatch tools to match the current MCP surface: - repository_ruleset_read (get, list, get_rules_for_branch, list_rule_suites, get_rule_suite) - organization_repository_ruleset_read (get, list) Write operations remain single-purpose tools, split by level because each level requires a distinct OAuth scope for scope-challenge accuracy: - create_repository_ruleset (repo) - create_organization_repository_ruleset (admin:org) - create_enterprise_repository_ruleset (admin:enterprise) Adds the read:enterprise and admin:enterprise scopes and the law octicon as shared governance infrastructure. Supersedes #821. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e886867-a922-419a-b02c-ac643716aea8 --- README.md | 70 ++ docs/remote-server.md | 1 + .../create_enterprise_repository_ruleset.snap | 101 +++ ...reate_organization_repository_ruleset.snap | 101 +++ .../create_repository_ruleset.snap | 105 +++ .../organization_repository_ruleset_read.snap | 44 ++ .../repository_ruleset_read.snap | 92 +++ pkg/github/rulesets.go | 744 ++++++++++++++++++ pkg/github/rulesets_test.go | 486 ++++++++++++ pkg/github/tools.go | 12 + pkg/octicons/icons/law-dark.png | Bin 0 -> 550 bytes pkg/octicons/icons/law-light.png | Bin 0 -> 841 bytes pkg/octicons/required_icons.txt | 1 + pkg/scopes/scopes.go | 21 +- 14 files changed, 1772 insertions(+), 6 deletions(-) create mode 100644 pkg/github/__toolsnaps__/create_enterprise_repository_ruleset.snap create mode 100644 pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap create mode 100644 pkg/github/__toolsnaps__/create_repository_ruleset.snap create mode 100644 pkg/github/__toolsnaps__/organization_repository_ruleset_read.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 d8d8695d2a..608bd70271 100644 --- a/README.md +++ b/README.md @@ -597,6 +597,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 | @@ -883,6 +884,75 @@ The following sets of tools are available:
+law Governance + +- **create_enterprise_repository_ruleset** - Create enterprise repository ruleset + - **Required OAuth Scopes**: `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 (string, required) + - `name`: The name of the ruleset (string, required) + - `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' (string, optional) + +- **create_organization_repository_ruleset** - Create organization repository ruleset + - **Required OAuth Scopes**: `admin:org` + - `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) + - `name`: The name of the ruleset (string, required) + - `org`: Organization name (string, required) + - `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' (string, optional) + +- **create_repository_ruleset** - Create repository ruleset + - **Required OAuth Scopes**: `repo` + - `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) + - `name`: The name of the ruleset (string, required) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `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' (string, optional) + +- **organization_repository_ruleset_read** - Read organization repository rulesets + - **Required OAuth Scopes**: `read:org` + - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` + - `method`: Operation to perform: + - 'get': Get a specific repository ruleset by ID (requires 'ruleset_id'). + - 'list': List all repository rulesets for the organization. (string, required) + - `org`: Organization name (string, required) + - `page`: Page number for pagination (min 1) (number, optional) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `ruleset_id`: Ruleset ID. Required for the 'get' method. (number, optional) + +- **repository_ruleset_read** - Read repository rulesets + - **Required OAuth Scopes**: `repo` + - `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) + - `includes_parents`: Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods. (boolean, optional) + - `method`: Operation to perform: + - 'get': Get a specific ruleset by ID (requires 'ruleset_id'). + - 'list': List all rulesets for the repository. + - 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). + - 'list_rule_suites': List rule suites, the evaluations of rules against pushes. + - 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). (string, required) + - `owner`: Repository owner (string, required) + - `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 (string, required) + - `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_enterprise_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_enterprise_repository_ruleset.snap new file mode 100644 index 0000000000..e4151d4e42 --- /dev/null +++ b/pkg/github/__toolsnaps__/create_enterprise_repository_ruleset.snap @@ -0,0 +1,101 @@ +{ + "annotations": { + "title": "Create enterprise repository ruleset" + }, + "description": "Create a new repository ruleset for an enterprise", + "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", + "enum": [ + "Integration", + "OrganizationAdmin", + "RepositoryRole", + "Team", + "DeployKey" + ], + "type": "string" + }, + "bypass_mode": { + "description": "When the specified actor can bypass the ruleset", + "enum": [ + "always", + "pull_request" + ], + "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", + "type": "string" + }, + "name": { + "description": "The name of the ruleset", + "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'", + "enum": [ + "branch", + "tag", + "push", + "repository" + ], + "type": "string" + } + }, + "required": [ + "enterprise", + "name", + "enforcement", + "rules" + ], + "type": "object" + }, + "name": "create_enterprise_repository_ruleset" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap new file mode 100644 index 0000000000..99b19aa446 --- /dev/null +++ b/pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap @@ -0,0 +1,101 @@ +{ + "annotations": { + "title": "Create organization repository ruleset" + }, + "description": "Create a new repository ruleset for an organization", + "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", + "enum": [ + "Integration", + "OrganizationAdmin", + "RepositoryRole", + "Team", + "DeployKey" + ], + "type": "string" + }, + "bypass_mode": { + "description": "When the specified actor can bypass the ruleset", + "enum": [ + "always", + "pull_request" + ], + "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" + }, + "name": { + "description": "The name of the ruleset", + "type": "string" + }, + "org": { + "description": "Organization name", + "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'", + "enum": [ + "branch", + "tag", + "push", + "repository" + ], + "type": "string" + } + }, + "required": [ + "org", + "name", + "enforcement", + "rules" + ], + "type": "object" + }, + "name": "create_organization_repository_ruleset" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/create_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_repository_ruleset.snap new file mode 100644 index 0000000000..22f8f7771a --- /dev/null +++ b/pkg/github/__toolsnaps__/create_repository_ruleset.snap @@ -0,0 +1,105 @@ +{ + "annotations": { + "title": "Create repository ruleset" + }, + "description": "Create a new ruleset for a repository", + "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", + "enum": [ + "Integration", + "OrganizationAdmin", + "RepositoryRole", + "Team", + "DeployKey" + ], + "type": "string" + }, + "bypass_mode": { + "description": "When the specified actor can bypass the ruleset", + "enum": [ + "always", + "pull_request" + ], + "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" + }, + "name": { + "description": "The name of the ruleset", + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "repo": { + "description": "Repository name", + "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'", + "enum": [ + "branch", + "tag", + "push" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "name", + "enforcement", + "rules" + ], + "type": "object" + }, + "name": "create_repository_ruleset" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/organization_repository_ruleset_read.snap b/pkg/github/__toolsnaps__/organization_repository_ruleset_read.snap new file mode 100644 index 0000000000..1ffe7f3a2c --- /dev/null +++ b/pkg/github/__toolsnaps__/organization_repository_ruleset_read.snap @@ -0,0 +1,44 @@ +{ + "annotations": { + "readOnlyHint": true, + "title": "Read organization repository rulesets" + }, + "description": "Read an organization's repository rulesets. Select the operation with the 'method' parameter.", + "inputSchema": { + "properties": { + "method": { + "description": "Operation to perform:\n- 'get': Get a specific repository ruleset by ID (requires 'ruleset_id').\n- 'list': List all repository rulesets for the organization.", + "enum": [ + "get", + "list" + ], + "type": "string" + }, + "org": { + "description": "Organization name", + "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" + }, + "ruleset_id": { + "description": "Ruleset ID. Required for the 'get' method.", + "type": "number" + } + }, + "required": [ + "method", + "org" + ], + "type": "object" + }, + "name": "organization_repository_ruleset_read" +} \ 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..3166d23270 --- /dev/null +++ b/pkg/github/__toolsnaps__/repository_ruleset_read.snap @@ -0,0 +1,92 @@ +{ + "annotations": { + "readOnlyHint": true, + "title": "Read repository rulesets" + }, + "description": "Read a repository's rulesets and rule suites. Select 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" + }, + "includes_parents": { + "description": "Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods.", + "type": "boolean" + }, + "method": { + "description": "Operation to perform:\n- 'get': Get a specific ruleset by ID (requires 'ruleset_id').\n- 'list': List all rulesets for the repository.\n- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch').\n- 'list_rule_suites': List rule suites, the evaluations of rules against pushes.\n- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id').", + "enum": [ + "get", + "list", + "get_rules_for_branch", + "list_rule_suites", + "get_rule_suite" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "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", + "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": [ + "method", + "owner", + "repo" + ], + "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..77b3a5d554 --- /dev/null +++ b/pkg/github/rulesets.go @@ -0,0 +1,744 @@ +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/v87/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// RepositoryRulesetRead creates a tool for read operations on a repository's +// rulesets and rule suites. The operation is selected 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 a repository's rulesets and rule suites. Select 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{ + "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').\n" + + "- 'list': List all rulesets for the repository.\n" + + "- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch').\n" + + "- 'list_rule_suites': List rule suites, the evaluations of rules against pushes.\n" + + "- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id').", + }, + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "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.", + }, + "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{"method", "owner", "repo"}, + }), + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + method, err := RequiredParam[string](args, "method") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + 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 + } + + client, err := deps.GetClient(ctx) + if err != nil { + return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + switch strings.ToLower(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 + } + }, + ) +} + +// OrganizationRepositoryRulesetRead creates a tool for read operations on an +// organization's repository rulesets. The operation is selected with the +// "method" parameter. +func OrganizationRepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataGovernance, + mcp.Tool{ + Name: "organization_repository_ruleset_read", + Description: t("TOOL_ORGANIZATION_REPOSITORY_RULESET_READ_DESCRIPTION", "Read an organization's repository rulesets. Select the operation with the 'method' parameter."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ORGANIZATION_REPOSITORY_RULESET_READ_USER_TITLE", "Read organization repository rulesets"), + ReadOnlyHint: true, + }, + InputSchema: WithPagination(&jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "method": { + Type: "string", + Enum: []any{"get", "list"}, + Description: "Operation to perform:\n" + + "- 'get': Get a specific repository ruleset by ID (requires 'ruleset_id').\n" + + "- 'list': List all repository rulesets for the organization.", + }, + "org": { + Type: "string", + Description: "Organization name", + }, + "ruleset_id": { + Type: "number", + Description: "Ruleset ID. Required for the 'get' method.", + }, + }, + Required: []string{"method", "org"}, + }), + }, + []scopes.Scope{scopes.ReadOrg}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + method, err := RequiredParam[string](args, "method") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + org, err := RequiredParam[string](args, "org") + 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 strings.ToLower(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("unknown method: %q", 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository ruleset", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rulesets", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rules for branch", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rule suites", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rule suite", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get organization repository ruleset", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list organization repository rulesets", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + return MarshalledTextResult(rulesets), nil +} + +// CreateRepositoryRuleset creates a tool to create a new repository ruleset. +func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.ServerTool { + properties := rulesetWriteProperties([]any{"branch", "tag", "push"}) + properties["owner"] = &jsonschema.Schema{Type: "string", Description: "Repository owner"} + properties["repo"] = &jsonschema.Schema{Type: "string", Description: "Repository name"} + + return NewTool( + ToolsetMetadataGovernance, + mcp.Tool{ + Name: "create_repository_ruleset", + Description: t("TOOL_CREATE_REPOSITORY_RULESET_DESCRIPTION", "Create a new ruleset for a repository"), + 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{"owner", "repo", "name", "enforcement", "rules"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, 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 + } + 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) + } + + created, resp, err := client.Repositories.CreateRuleset(ctx, owner, repo, ruleset) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create repository ruleset", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + return MarshalledTextResult(created), nil, nil + }, + ) +} + +// CreateOrganizationRepositoryRuleset creates a tool to create a new organization repository ruleset. +func CreateOrganizationRepositoryRuleset(t translations.TranslationHelperFunc) inventory.ServerTool { + properties := rulesetWriteProperties([]any{"branch", "tag", "push", "repository"}) + properties["org"] = &jsonschema.Schema{Type: "string", Description: "Organization name"} + + return NewTool( + ToolsetMetadataGovernance, + mcp.Tool{ + Name: "create_organization_repository_ruleset", + Description: t("TOOL_CREATE_ORGANIZATION_REPOSITORY_RULESET_DESCRIPTION", "Create a new repository ruleset for an organization"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_CREATE_ORGANIZATION_REPOSITORY_RULESET_USER_TITLE", "Create organization repository ruleset"), + ReadOnlyHint: false, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: properties, + Required: []string{"org", "name", "enforcement", "rules"}, + }, + }, + []scopes.Scope{scopes.AdminOrg}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + org, err := RequiredParam[string](args, "org") + 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) + } + + created, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, org, ruleset) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create organization repository ruleset", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + return MarshalledTextResult(created), nil, nil + }, + ) +} + +// CreateEnterpriseRepositoryRuleset creates a tool to create a new enterprise repository ruleset. +func CreateEnterpriseRepositoryRuleset(t translations.TranslationHelperFunc) inventory.ServerTool { + properties := rulesetWriteProperties([]any{"branch", "tag", "push", "repository"}) + properties["enterprise"] = &jsonschema.Schema{Type: "string", Description: "Enterprise slug"} + + return NewTool( + ToolsetMetadataGovernance, + mcp.Tool{ + Name: "create_enterprise_repository_ruleset", + Description: t("TOOL_CREATE_ENTERPRISE_REPOSITORY_RULESET_DESCRIPTION", "Create a new repository ruleset for an enterprise"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_CREATE_ENTERPRISE_REPOSITORY_RULESET_USER_TITLE", "Create enterprise repository ruleset"), + ReadOnlyHint: false, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: properties, + Required: []string{"enterprise", "name", "enforcement", "rules"}, + }, + }, + []scopes.Scope{scopes.AdminEnterprise}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + enterprise, err := RequiredParam[string](args, "enterprise") + 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) + } + + created, resp, err := client.Enterprise.CreateRepositoryRuleset(ctx, enterprise, ruleset) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create enterprise repository ruleset", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + return MarshalledTextResult(created), nil, nil + }, + ) +} + +// rulesetWriteProperties returns the shared input schema properties for the +// ruleset creation tools. Callers pass the target values valid for the API +// level and add the owner/repo, org, or enterprise identifier properties. +func rulesetWriteProperties(targets []any) 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: targets, + Description: "The target of the ruleset. Defaults to 'branch'", + }, + "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"}, + Description: "The type of actor that can bypass a ruleset", + }, + "bypass_mode": { + Type: "string", + Enum: []any{"always", "pull_request"}, + Description: "When the specified actor can bypass the ruleset", + }, + }, + }, + }, + } +} + +// 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)) + 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") + } + requestedRuleTypes = append(requestedRuleTypes, ruleType) + } + + 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") + } + 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 it + // does not recognize, which would let a typo create a weaker ruleset than the + // caller requested. Verify every requested rule type survived the round-trip. + appliedRuleTypes, errResult := rulesetAppliedRuleTypes(ruleset.Rules) + if errResult != nil { + return github.RepositoryRuleset{}, errResult + } + for _, ruleType := range requestedRuleTypes { + if !appliedRuleTypes[ruleType] { + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("unsupported or unrecognized rule type: %q", ruleType)) + } + } + + return ruleset, nil +} + +// rulesetAppliedRuleTypes marshals the parsed rules back to the API's array form +// and returns the set of rule types that were actually retained. +func rulesetAppliedRuleTypes(rules *github.RepositoryRulesetRules) (map[string]bool, *mcp.CallToolResult) { + applied := map[string]bool{} + 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"` + } + if err := json.Unmarshal(raw, &ruleObjects); err != nil { + return nil, utils.NewToolResultErrorFromErr("failed to validate ruleset rules", err) + } + for _, rule := range ruleObjects { + applied[rule.Type] = true + } + return applied, nil +} diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go new file mode 100644 index 0000000000..6135739a7e --- /dev/null +++ b/pkg/github/rulesets_test.go @@ -0,0 +1,486 @@ +package github + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/url" + "testing" + + "github.com/google/go-github/v87/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/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{"method", "owner", "repo"}) + + t.Run("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{"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("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{"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("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{"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("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{"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("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{"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("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{"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("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{"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("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{ + "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("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{"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("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{"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") + }) +} + +func Test_OrganizationRepositoryRulesetRead(t *testing.T) { + toolDef := OrganizationRepositoryRulesetRead(translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + + assert.Equal(t, "organization_repository_ruleset_read", toolDef.Tool.Name) + assert.True(t, toolDef.Tool.Annotations.ReadOnlyHint) + + schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok) + assert.ElementsMatch(t, schema.Required, []string{"method", "org"}) + + t.Run("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{"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("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{"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("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{"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("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{"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, "unknown method") + }) +} + +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{"owner", "repo", "name", "enforcement", "rules"}) + + 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{ + "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) +} + +func Test_CreateRepositoryRuleset_UnsupportedRuleType(t *testing.T) { + toolDef := CreateRepositoryRuleset(translations.NullTranslationHelper) + 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{ + "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") +} + +func Test_CreateRepositoryRuleset_InvalidRules(t *testing.T) { + toolDef := CreateRepositoryRuleset(translations.NullTranslationHelper) + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "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") +} + +func Test_CreateOrganizationRepositoryRuleset(t *testing.T) { + toolDef := CreateOrganizationRepositoryRuleset(translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + + assert.Equal(t, "create_organization_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{"org", "name", "enforcement", "rules"}) + + 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{ + "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) +} + +func Test_CreateEnterpriseRepositoryRuleset(t *testing.T) { + toolDef := CreateEnterpriseRepositoryRuleset(translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + + assert.Equal(t, "create_enterprise_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{"enterprise", "name", "enforcement", "rules"}) + + 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{ + "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) +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index f9b51159b5..9730717881 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", @@ -264,6 +269,13 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent // Organization tools SearchOrgs(t), + // Governance tools (rulesets) + RepositoryRulesetRead(t), + OrganizationRepositoryRulesetRead(t), + CreateRepositoryRuleset(t), + CreateOrganizationRepositoryRuleset(t), + CreateEnterpriseRepositoryRuleset(t), + // Pull request tools PullRequestRead(t), ListPullRequests(t), 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/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..20ad3cd5c0 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" @@ -113,12 +121,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 f340ea4a4e33e7daa4042332667f49508c4ee467 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 25 Aug 2026 16:43:44 +0200 Subject: [PATCH 2/6] refactor(governance): consolidate ruleset tools with dynamic scope challenges Rebase onto main (which now includes #3128's per-call OAuth scope checks) and redesign the ruleset tools around that API instead of the old tool-name-keyed challenge middleware that motivated splitting by level. - Collapse the 5 level-specific tools (repository_ruleset_read, organization_repository_ruleset_read, create_repository_ruleset, create_organization_repository_ruleset, create_enterprise_repository_ruleset) into 2: repository_ruleset_read and create_repository_ruleset. Both take a "level" argument (repository/organization/enterprise) and use scopes.DynamicChallenge to return the exact repo / read:org / admin:org / read:enterprise / admin:enterprise challenge for that call, using the scope hierarchy so a broader granted scope still satisfies the challenge. - A missing or unrecognized "level" (or a non-string value) returns no challenge so normal handler argument validation produces the error, instead of prompting for scopes on a malformed call. - Keep repository_ruleset_read and create_repository_ruleset as separate tools since ReadOnlyHint-based read-only filtering depends on that split. - Add enterprise-level "get" and "list" read support (list issued directly via GET /enterprises/{enterprise}/rulesets, matching the existing rule suite pattern, since go-github has no typed wrapper for it) so all three levels have symmetric read coverage. - Re-add read:enterprise/admin:enterprise to pkg/scopes as opt-in (non-default) OAuth scopes, alongside admin:org which no tool had previously requested, and register them in oauthScopeDefinitions so they are advertised in OAuth protected-resource metadata. - Fetch the "law" toolset icon into the new icons_data_uris.txt embed (required_icons.txt already listed it) and bump go-github v87 -> v89 to match main. - Update README/toolsnaps via script/generate-docs and UPDATE_TOOLSNAPS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 69 +- docs/scope-filtering.md | 1 + .../create_enterprise_repository_ruleset.snap | 101 --- ...reate_organization_repository_ruleset.snap | 101 --- .../create_repository_ruleset.snap | 33 +- .../organization_repository_ruleset_read.snap | 44 -- .../repository_ruleset_read.snap | 33 +- pkg/github/rulesets.go | 587 ++++++++------- pkg/github/rulesets_test.go | 689 ++++++++++++------ pkg/github/tools.go | 3 - pkg/http/oauth/oauth_test.go | 10 + pkg/octicons/icons_data_uris.txt | 2 + pkg/scopes/scopes.go | 3 + pkg/scopes/scopes_test.go | 7 + 14 files changed, 906 insertions(+), 777 deletions(-) delete mode 100644 pkg/github/__toolsnaps__/create_enterprise_repository_ruleset.snap delete mode 100644 pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap delete mode 100644 pkg/github/__toolsnaps__/organization_repository_ruleset_read.snap diff --git a/README.md b/README.md index 608bd70271..7febc7763d 100644 --- a/README.md +++ b/README.md @@ -886,64 +886,45 @@ The following sets of tools are available: law Governance -- **create_enterprise_repository_ruleset** - Create enterprise repository ruleset - - **Required OAuth Scopes**: `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 (string, required) - - `name`: The name of the ruleset (string, required) - - `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' (string, optional) - -- **create_organization_repository_ruleset** - Create organization repository ruleset - - **Required OAuth Scopes**: `admin:org` - - `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) - - `name`: The name of the ruleset (string, required) - - `org`: Organization name (string, required) - - `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' (string, optional) - - **create_repository_ruleset** - Create repository ruleset - - **Required OAuth Scopes**: `repo` + - **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) - - `owner`: Repository owner (string, required) - - `repo`: Repository name (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' (string, optional) - -- **organization_repository_ruleset_read** - Read organization repository rulesets - - **Required OAuth Scopes**: `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` - - `method`: Operation to perform: - - 'get': Get a specific repository ruleset by ID (requires 'ruleset_id'). - - 'list': List all repository rulesets for the organization. (string, required) - - `org`: Organization name (string, required) - - `page`: Page number for pagination (min 1) (number, optional) - - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - - `ruleset_id`: Ruleset ID. Required for the 'get' method. (number, optional) + - `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 - - **Required OAuth Scopes**: `repo` + - **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) - - `includes_parents`: Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods. (boolean, 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'). - - 'list': List all rulesets for the repository. - - 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch'). - - 'list_rule_suites': List rule suites, the evaluations of rules against pushes. - - 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id'). (string, required) - - `owner`: Repository owner (string, required) + - '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 (string, required) + - `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) diff --git a/docs/scope-filtering.md b/docs/scope-filtering.md index 2055db1483..7e066fe760 100644 --- a/docs/scope-filtering.md +++ b/docs/scope-filtering.md @@ -63,6 +63,7 @@ Some scopes implicitly include others: - `repo` → includes `public_repo`, `security_events` - `admin:org` → includes `write:org` → includes `read:org` +- `admin:enterprise` → includes `read:enterprise` - `project` → includes `read:project` This means if your token has `repo`, tools requiring `security_events` will also be available. diff --git a/pkg/github/__toolsnaps__/create_enterprise_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_enterprise_repository_ruleset.snap deleted file mode 100644 index e4151d4e42..0000000000 --- a/pkg/github/__toolsnaps__/create_enterprise_repository_ruleset.snap +++ /dev/null @@ -1,101 +0,0 @@ -{ - "annotations": { - "title": "Create enterprise repository ruleset" - }, - "description": "Create a new repository ruleset for an enterprise", - "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", - "enum": [ - "Integration", - "OrganizationAdmin", - "RepositoryRole", - "Team", - "DeployKey" - ], - "type": "string" - }, - "bypass_mode": { - "description": "When the specified actor can bypass the ruleset", - "enum": [ - "always", - "pull_request" - ], - "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", - "type": "string" - }, - "name": { - "description": "The name of the ruleset", - "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'", - "enum": [ - "branch", - "tag", - "push", - "repository" - ], - "type": "string" - } - }, - "required": [ - "enterprise", - "name", - "enforcement", - "rules" - ], - "type": "object" - }, - "name": "create_enterprise_repository_ruleset" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap deleted file mode 100644 index 99b19aa446..0000000000 --- a/pkg/github/__toolsnaps__/create_organization_repository_ruleset.snap +++ /dev/null @@ -1,101 +0,0 @@ -{ - "annotations": { - "title": "Create organization repository ruleset" - }, - "description": "Create a new repository ruleset for an organization", - "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", - "enum": [ - "Integration", - "OrganizationAdmin", - "RepositoryRole", - "Team", - "DeployKey" - ], - "type": "string" - }, - "bypass_mode": { - "description": "When the specified actor can bypass the ruleset", - "enum": [ - "always", - "pull_request" - ], - "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" - }, - "name": { - "description": "The name of the ruleset", - "type": "string" - }, - "org": { - "description": "Organization name", - "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'", - "enum": [ - "branch", - "tag", - "push", - "repository" - ], - "type": "string" - } - }, - "required": [ - "org", - "name", - "enforcement", - "rules" - ], - "type": "object" - }, - "name": "create_organization_repository_ruleset" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/create_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_repository_ruleset.snap index 22f8f7771a..fc2bdd939b 100644 --- a/pkg/github/__toolsnaps__/create_repository_ruleset.snap +++ b/pkg/github/__toolsnaps__/create_repository_ruleset.snap @@ -1,8 +1,10 @@ { "annotations": { + "idempotentHint": false, + "readOnlyHint": false, "title": "Create repository ruleset" }, - "description": "Create a new ruleset for a repository", + "description": "Create a new ruleset at the repository, organization, or enterprise level", "inputSchema": { "properties": { "bypass_actors": { @@ -50,16 +52,33 @@ ], "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", + "description": "Repository owner. Required when level is 'repository'.", "type": "string" }, "repo": { - "description": "Repository name", + "description": "Repository name. Required when level is 'repository'.", "type": "string" }, "rules": { @@ -83,18 +102,18 @@ "type": "array" }, "target": { - "description": "The target of the ruleset. Defaults to 'branch'", + "description": "The target of the ruleset. Defaults to 'branch'. 'repository' is only valid for 'organization' and 'enterprise' level rulesets.", "enum": [ "branch", "tag", - "push" + "push", + "repository" ], "type": "string" } }, "required": [ - "owner", - "repo", + "level", "name", "enforcement", "rules" diff --git a/pkg/github/__toolsnaps__/organization_repository_ruleset_read.snap b/pkg/github/__toolsnaps__/organization_repository_ruleset_read.snap deleted file mode 100644 index 1ffe7f3a2c..0000000000 --- a/pkg/github/__toolsnaps__/organization_repository_ruleset_read.snap +++ /dev/null @@ -1,44 +0,0 @@ -{ - "annotations": { - "readOnlyHint": true, - "title": "Read organization repository rulesets" - }, - "description": "Read an organization's repository rulesets. Select the operation with the 'method' parameter.", - "inputSchema": { - "properties": { - "method": { - "description": "Operation to perform:\n- 'get': Get a specific repository ruleset by ID (requires 'ruleset_id').\n- 'list': List all repository rulesets for the organization.", - "enum": [ - "get", - "list" - ], - "type": "string" - }, - "org": { - "description": "Organization name", - "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" - }, - "ruleset_id": { - "description": "Ruleset ID. Required for the 'get' method.", - "type": "number" - } - }, - "required": [ - "method", - "org" - ], - "type": "object" - }, - "name": "organization_repository_ruleset_read" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/repository_ruleset_read.snap b/pkg/github/__toolsnaps__/repository_ruleset_read.snap index 3166d23270..2a79b4cfc9 100644 --- a/pkg/github/__toolsnaps__/repository_ruleset_read.snap +++ b/pkg/github/__toolsnaps__/repository_ruleset_read.snap @@ -1,9 +1,10 @@ { "annotations": { + "idempotentHint": false, "readOnlyHint": true, "title": "Read repository rulesets" }, - "description": "Read a repository's rulesets and rule suites. Select the operation with the 'method' parameter.", + "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": { @@ -14,12 +15,25 @@ "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.", + "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').\n- 'list': List all rulesets for the repository.\n- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch').\n- 'list_rule_suites': List rule suites, the evaluations of rules against pushes.\n- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id').", + "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", @@ -29,8 +43,12 @@ ], "type": "string" }, + "org": { + "description": "Organization name. Required when level is 'organization'.", + "type": "string" + }, "owner": { - "description": "Repository owner", + "description": "Repository owner. Required when level is 'repository'.", "type": "string" }, "page": { @@ -49,7 +67,7 @@ "type": "string" }, "repo": { - "description": "Repository name", + "description": "Repository name. Required when level is 'repository'.", "type": "string" }, "rule_suite_id": { @@ -82,9 +100,8 @@ } }, "required": [ - "method", - "owner", - "repo" + "level", + "method" ], "type": "object" }, diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index 77b3a5d554..17b398d75a 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -14,20 +14,89 @@ import ( "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/v87/github" + "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" ) -// RepositoryRulesetRead creates a tool for read operations on a repository's -// rulesets and rule suites. The operation is selected with the "method" -// parameter. +// 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 a repository's rulesets and rule suites. Select the operation with the 'method' parameter."), + 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, @@ -35,23 +104,36 @@ func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.Serve 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').\n" + - "- 'list': List all rulesets for the repository.\n" + - "- 'get_rules_for_branch': Get all rules that apply to a branch (requires 'branch').\n" + - "- 'list_rule_suites': List rule suites, the evaluations of rules against pushes.\n" + - "- 'get_rule_suite': Get a specific rule suite by ID (requires 'rule_suite_id').", + "- '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", + Description: "Repository owner. Required when level is 'repository'.", }, "repo": { Type: "string", - Description: "Repository name", + 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", @@ -59,7 +141,7 @@ func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.Serve }, "includes_parents": { Type: "boolean", - Description: "Include rulesets configured at higher levels that also apply. Defaults to true. Used by the 'get' and 'list' methods.", + 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", @@ -88,20 +170,16 @@ func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.Serve Description: "Rule suite ID. Required for the 'get_rule_suite' method.", }, }, - Required: []string{"method", "owner", "repo"}, + Required: []string{"level", "method"}, }), }, - []scopes.Scope{scopes.Repo}, + rulesetReadScopeAccess(), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - method, err := RequiredParam[string](args, "method") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - owner, err := RequiredParam[string](args, "owner") + level, err := RequiredParam[string](args, "level") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - repo, err := RequiredParam[string](args, "repo") + method, err := RequiredParam[string](args, "method") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -111,145 +189,149 @@ func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.Serve return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - switch strings.ToLower(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 + switch strings.ToLower(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 method: %q", method)), nil, nil + return utils.NewToolResultError(fmt.Sprintf("unknown level: %q (expected 'repository', 'organization', or 'enterprise')", level)), nil, nil } }, ) } -// OrganizationRepositoryRulesetRead creates a tool for read operations on an -// organization's repository rulesets. The operation is selected with the -// "method" parameter. -func OrganizationRepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataGovernance, - mcp.Tool{ - Name: "organization_repository_ruleset_read", - Description: t("TOOL_ORGANIZATION_REPOSITORY_RULESET_READ_DESCRIPTION", "Read an organization's repository rulesets. Select the operation with the 'method' parameter."), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_ORGANIZATION_REPOSITORY_RULESET_READ_USER_TITLE", "Read organization repository rulesets"), - ReadOnlyHint: true, - }, - InputSchema: WithPagination(&jsonschema.Schema{ - Type: "object", - Properties: map[string]*jsonschema.Schema{ - "method": { - Type: "string", - Enum: []any{"get", "list"}, - Description: "Operation to perform:\n" + - "- 'get': Get a specific repository ruleset by ID (requires 'ruleset_id').\n" + - "- 'list': List all repository rulesets for the organization.", - }, - "org": { - Type: "string", - Description: "Organization name", - }, - "ruleset_id": { - Type: "number", - Description: "Ruleset ID. Required for the 'get' method.", - }, - }, - Required: []string{"method", "org"}, - }), - }, - []scopes.Scope{scopes.ReadOrg}, - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - method, err := RequiredParam[string](args, "method") +// 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 } - org, err := RequiredParam[string](args, "org") + } + 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 + } +} - client, err := deps.GetClient(ctx) - if err != nil { - return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) - } +// 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 strings.ToLower(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("unknown method: %q", method)), 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. @@ -424,131 +506,86 @@ func ListOrganizationRepositoryRulesets(ctx context.Context, client *github.Clie return MarshalledTextResult(rulesets), nil } -// CreateRepositoryRuleset creates a tool to create a new repository ruleset. -func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.ServerTool { - properties := rulesetWriteProperties([]any{"branch", "tag", "push"}) - properties["owner"] = &jsonschema.Schema{Type: "string", Description: "Repository owner"} - properties["repo"] = &jsonschema.Schema{Type: "string", Description: "Repository name"} - - return NewTool( - ToolsetMetadataGovernance, - mcp.Tool{ - Name: "create_repository_ruleset", - Description: t("TOOL_CREATE_REPOSITORY_RULESET_DESCRIPTION", "Create a new ruleset for a repository"), - 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{"owner", "repo", "name", "enforcement", "rules"}, - }, - }, - []scopes.Scope{scopes.Repo}, - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, 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 - } - 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) - } - - created, resp, err := client.Repositories.CreateRuleset(ctx, owner, repo, ruleset) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create repository ruleset", resp, err), nil, nil - } - defer func() { _ = resp.Body.Close() }() +// 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get enterprise repository ruleset", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() - return MarshalledTextResult(created), nil, nil - }, - ) + return MarshalledTextResult(ruleset), nil } -// CreateOrganizationRepositoryRuleset creates a tool to create a new organization repository ruleset. -func CreateOrganizationRepositoryRuleset(t translations.TranslationHelperFunc) inventory.ServerTool { - properties := rulesetWriteProperties([]any{"branch", "tag", "push", "repository"}) - properties["org"] = &jsonschema.Schema{Type: "string", Description: "Organization name"} - - return NewTool( - ToolsetMetadataGovernance, - mcp.Tool{ - Name: "create_organization_repository_ruleset", - Description: t("TOOL_CREATE_ORGANIZATION_REPOSITORY_RULESET_DESCRIPTION", "Create a new repository ruleset for an organization"), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_CREATE_ORGANIZATION_REPOSITORY_RULESET_USER_TITLE", "Create organization repository ruleset"), - ReadOnlyHint: false, - }, - InputSchema: &jsonschema.Schema{ - Type: "object", - Properties: properties, - Required: []string{"org", "name", "enforcement", "rules"}, - }, - }, - []scopes.Scope{scopes.AdminOrg}, - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - org, err := RequiredParam[string](args, "org") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - ruleset, errResult := buildRepositoryRulesetFromArgs(args) - if errResult != nil { - return errResult, nil, 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() + } - client, err := deps.GetClient(ctx) - if err != nil { - return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) - } + req, err := client.NewRequest(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil + } - created, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, org, ruleset) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create organization repository ruleset", resp, err), nil, nil - } - defer func() { _ = resp.Body.Close() }() + var rulesets any + resp, err := client.Do(req, &rulesets) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list enterprise repository rulesets", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() - return MarshalledTextResult(created), nil, nil - }, - ) + return MarshalledTextResult(rulesets), nil } -// CreateEnterpriseRepositoryRuleset creates a tool to create a new enterprise repository ruleset. -func CreateEnterpriseRepositoryRuleset(t translations.TranslationHelperFunc) inventory.ServerTool { - properties := rulesetWriteProperties([]any{"branch", "tag", "push", "repository"}) - properties["enterprise"] = &jsonschema.Schema{Type: "string", Description: "Enterprise slug"} +// 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_enterprise_repository_ruleset", - Description: t("TOOL_CREATE_ENTERPRISE_REPOSITORY_RULESET_DESCRIPTION", "Create a new repository ruleset for an enterprise"), + 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_ENTERPRISE_REPOSITORY_RULESET_USER_TITLE", "Create enterprise repository ruleset"), + Title: t("TOOL_CREATE_REPOSITORY_RULESET_USER_TITLE", "Create repository ruleset"), ReadOnlyHint: false, }, InputSchema: &jsonschema.Schema{ Type: "object", Properties: properties, - Required: []string{"enterprise", "name", "enforcement", "rules"}, + Required: []string{"level", "name", "enforcement", "rules"}, }, }, - []scopes.Scope{scopes.AdminEnterprise}, + rulesetWriteScopeAccess(), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - enterprise, err := RequiredParam[string](args, "enterprise") + 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 @@ -559,21 +596,55 @@ func CreateEnterpriseRepositoryRuleset(t translations.TranslationHelperFunc) inv return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - created, resp, err := client.Enterprise.CreateRepositoryRuleset(ctx, enterprise, ruleset) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create enterprise repository ruleset", resp, err), nil, nil + switch strings.ToLower(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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create repository ruleset", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create organization repository ruleset", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + 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 err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create enterprise repository ruleset", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + return MarshalledTextResult(created), nil, nil + default: + return utils.NewToolResultError(fmt.Sprintf("unknown level: %q (expected 'repository', 'organization', or 'enterprise')", level)), nil, nil } - defer func() { _ = resp.Body.Close() }() - - return MarshalledTextResult(created), nil, nil }, ) } // rulesetWriteProperties returns the shared input schema properties for the -// ruleset creation tools. Callers pass the target values valid for the API -// level and add the owner/repo, org, or enterprise identifier properties. -func rulesetWriteProperties(targets []any) map[string]*jsonschema.Schema { +// 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", @@ -586,8 +657,8 @@ func rulesetWriteProperties(targets []any) map[string]*jsonschema.Schema { }, "target": { Type: "string", - Enum: targets, - Description: "The target of the ruleset. Defaults to 'branch'", + 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", diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index 6135739a7e..1f2cdbbbf6 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -8,12 +8,13 @@ import ( "net/url" "testing" - "github.com/google/go-github/v87/github" + "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" ) @@ -27,9 +28,9 @@ func Test_RepositoryRulesetRead(t *testing.T) { schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) require.True(t, ok, "InputSchema should be *jsonschema.Schema") - assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo"}) + assert.ElementsMatch(t, schema.Required, []string{"level", "method"}) - t.Run("get defaults includes_parents to true", func(t *testing.T) { + 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) { @@ -39,7 +40,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { })) deps := BaseDeps{Client: client} handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{"method": "get", "owner": "owner", "repo": "repo", "ruleset_id": float64(42)}) + 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) @@ -51,7 +52,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "main protection", returned.Name) }) - t.Run("get forwards explicit includes_parents=false", func(t *testing.T) { + 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) { @@ -61,7 +62,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { })) deps := BaseDeps{Client: client} handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{"method": "get", "owner": "owner", "repo": "repo", "ruleset_id": float64(42), "includes_parents": false}) + 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) @@ -69,11 +70,11 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "false", capturedQuery.Get("includes_parents")) }) - t.Run("get requires ruleset_id", func(t *testing.T) { + 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{"method": "get", "owner": "owner", "repo": "repo"}) + 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) @@ -81,7 +82,19 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Contains(t, getErrorResult(t, result).Text, "ruleset_id") }) - t.Run("list omits includes_parents when not provided", func(t *testing.T) { + 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) { @@ -91,7 +104,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { })) deps := BaseDeps{Client: client} handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{"method": "list", "owner": "owner", "repo": "repo", "perPage": float64(50)}) + 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) @@ -105,7 +118,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "rs1", returned[0].Name) }) - t.Run("list forwards explicit includes_parents=false", func(t *testing.T) { + 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) { @@ -115,7 +128,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { })) deps := BaseDeps{Client: client} handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{"method": "list", "owner": "owner", "repo": "repo", "includes_parents": false}) + 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) @@ -123,13 +136,13 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "false", capturedQuery.Get("includes_parents")) }) - t.Run("get_rules_for_branch", func(t *testing.T) { + 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{"method": "get_rules_for_branch", "owner": "owner", "repo": "repo", "branch": "main"}) + 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) @@ -137,11 +150,11 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Contains(t, getTextResult(t, result).Text, "Creation") }) - t.Run("get_rules_for_branch requires branch", func(t *testing.T) { + 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{"method": "get_rules_for_branch", "owner": "owner", "repo": "repo"}) + 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) @@ -149,7 +162,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Contains(t, getErrorResult(t, result).Text, "branch") }) - t.Run("list_rule_suites forwards filters", func(t *testing.T) { + 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) { @@ -160,6 +173,7 @@ func Test_RepositoryRulesetRead(t *testing.T) { deps := BaseDeps{Client: client} handler := toolDef.Handler(deps) request := createMCPRequest(map[string]any{ + "level": "repository", "method": "list_rule_suites", "owner": "owner", "repo": "repo", @@ -182,13 +196,13 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Equal(t, "25", capturedQuery.Get("per_page")) }) - t.Run("get_rule_suite", func(t *testing.T) { + 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{"method": "get_rule_suite", "owner": "owner", "repo": "repo", "rule_suite_id": float64(101)}) + 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) @@ -196,37 +210,25 @@ func Test_RepositoryRulesetRead(t *testing.T) { assert.Contains(t, getTextResult(t, result).Text, "fail") }) - t.Run("unknown method", func(t *testing.T) { + 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{"method": "frobnicate", "owner": "owner", "repo": "repo"}) + 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") }) -} - -func Test_OrganizationRepositoryRulesetRead(t *testing.T) { - toolDef := OrganizationRepositoryRulesetRead(translations.NullTranslationHelper) - require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) - assert.Equal(t, "organization_repository_ruleset_read", toolDef.Tool.Name) - assert.True(t, toolDef.Tool.Annotations.ReadOnlyHint) - - schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) - require.True(t, ok) - assert.ElementsMatch(t, schema.Required, []string{"method", "org"}) - - t.Run("get", func(t *testing.T) { + 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{"method": "get", "org": "octo", "ruleset_id": float64(7)}) + 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) @@ -237,11 +239,11 @@ func Test_OrganizationRepositoryRulesetRead(t *testing.T) { assert.Equal(t, "org rs", returned.Name) }) - t.Run("get requires ruleset_id", func(t *testing.T) { + 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{"method": "get", "org": "octo"}) + request := createMCPRequest(map[string]any{"level": "organization", "method": "get", "org": "octo"}) result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) @@ -249,13 +251,25 @@ func Test_OrganizationRepositoryRulesetRead(t *testing.T) { assert.Contains(t, getErrorResult(t, result).Text, "ruleset_id") }) - t.Run("list", func(t *testing.T) { + 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{"method": "list", "org": "octo"}) + request := createMCPRequest(map[string]any{"level": "organization", "method": "list", "org": "octo"}) result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) @@ -267,16 +281,99 @@ func Test_OrganizationRepositoryRulesetRead(t *testing.T) { assert.Equal(t, "org rs", returned[0].Name) }) - t.Run("unknown method", func(t *testing.T) { + 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{"method": "frobnicate", "org": "octo"}) + 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, "unknown method") + 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") }) } @@ -289,198 +386,368 @@ func Test_CreateRepositoryRuleset(t *testing.T) { schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) require.True(t, ok) - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "name", "enforcement", "rules"}) - - 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{ - "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), + 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{}, + "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 + 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"` } - } - 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) -} + 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) + }) -func Test_CreateRepositoryRuleset_UnsupportedRuleType(t *testing.T) { - toolDef := CreateRepositoryRuleset(translations.NullTranslationHelper) - 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{ - "owner": "owner", - "repo": "repo", - "name": "x", - "enforcement": "active", - "rules": []any{ - map[string]any{"type": "creation"}, - map[string]any{"type": "totally_made_up_rule"}, - }, + 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") }) - 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("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"}, + }, + }) -func Test_CreateRepositoryRuleset_InvalidRules(t *testing.T) { - toolDef := CreateRepositoryRuleset(translations.NullTranslationHelper) - client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) - deps := BaseDeps{Client: client} - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "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, "totally_made_up_rule") + assert.False(t, called, "request must not be sent when a rule type is unsupported") }) - 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("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", + }) -func Test_CreateOrganizationRepositoryRuleset(t *testing.T) { - toolDef := CreateOrganizationRepositoryRuleset(translations.NullTranslationHelper) - require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + 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") + }) - assert.Equal(t, "create_organization_repository_ruleset", toolDef.Tool.Name) - assert.False(t, toolDef.Tool.Annotations.ReadOnlyHint) + 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"}}, + }) - schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) - require.True(t, ok) - assert.ElementsMatch(t, schema.Required, []string{"org", "name", "enforcement", "rules"}) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) - 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{ - "org": "octo", - "name": "org protection", - "enforcement": "active", - "rules": []any{map[string]any{"type": "creation"}}, + var returned github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, "org protection", returned.Name) }) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) + 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"}}, + }) - var returned github.RepositoryRuleset - require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) - assert.Equal(t, "org protection", returned.Name) -} + 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") + }) -func Test_CreateEnterpriseRepositoryRuleset(t *testing.T) { - toolDef := CreateEnterpriseRepositoryRuleset(translations.NullTranslationHelper) - require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + 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"}}, + }) - assert.Equal(t, "create_enterprise_repository_ruleset", toolDef.Tool.Name) - assert.False(t, toolDef.Tool.Annotations.ReadOnlyHint) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) - schema, ok := toolDef.Tool.InputSchema.(*jsonschema.Schema) - require.True(t, ok) - assert.ElementsMatch(t, schema.Required, []string{"enterprise", "name", "enforcement", "rules"}) + var returned github.RepositoryRuleset + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + assert.Equal(t, "enterprise protection", returned.Name) + }) - 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{ - "enterprise": "acme", - "name": "enterprise protection", - "enforcement": "active", - "rules": []any{map[string]any{"type": "creation"}}, + 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") }) +} - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) +// 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)) + } + }) + } +} - var returned github.RepositoryRuleset - require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) - assert.Equal(t, "enterprise protection", returned.Name) +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 9730717881..4f70ff3c43 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -271,10 +271,7 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent // Governance tools (rulesets) RepositoryRulesetRead(t), - OrganizationRepositoryRulesetRead(t), CreateRepositoryRuleset(t), - CreateOrganizationRepositoryRuleset(t), - CreateEnterpriseRepositoryRuleset(t), // Pull request tools PullRequestRead(t), diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index 52baae3b6c..fa47b6d715 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -588,6 +588,9 @@ func TestSupportedScopes(t *testing.T) { "repo", "delete_repo", "read:org", + "admin:org", + "read:enterprise", + "admin:enterprise", "read:user", "user:email", "read:packages", @@ -610,6 +613,13 @@ func TestDefaultScopesRequiresExplicitDeleteRepoOptIn(t *testing.T) { assert.Contains(t, DefaultScopes, "repo") } +func TestDefaultScopesRequiresExplicitHighPrivilegeOptIn(t *testing.T) { + for _, scope := range []string{"admin:org", "read:enterprise", "admin:enterprise"} { + assert.Contains(t, SupportedScopes, scope) + assert.NotContains(t, DefaultScopes, scope) + } +} + func TestProtectedResourceResponseFormat(t *testing.T) { t.Parallel() 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/scopes/scopes.go b/pkg/scopes/scopes.go index 20ad3cd5c0..2294ac01ef 100644 --- a/pkg/scopes/scopes.go +++ b/pkg/scopes/scopes.go @@ -85,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}, diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index 8240e8454b..b314ec0122 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) { @@ -57,4 +63,5 @@ func TestScopeHierarchy(t *testing.T) { assert.Contains(t, ScopeHierarchy[WritePackages], ReadPackages) assert.Contains(t, ScopeHierarchy[User], ReadUser) assert.Contains(t, ScopeHierarchy[User], UserEmail) + assert.Contains(t, ScopeHierarchy[AdminEnterprise], ReadEnterprise) } From d01f35658a8b93a9b688f308ef1d66a5453b1361 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 25 Aug 2026 16:58:28 +0200 Subject: [PATCH 3/6] fix(governance): address ruleset review feedback Address four Copilot review comments on the consolidated ruleset tools: - Verify caller-supplied rule *parameters* survive the round-trip through github.RepositoryRulesetRules, not just rule *types*. go-github silently drops unrecognized parameter keys during JSON unmarshal (e.g. a "require_code_owners_review" typo for "require_code_owner_review" becomes the weaker default false), so a supplied parameter that vanishes after the round-trip is now rejected by name. A caller-supplied zero value (false, 0, "", empty array/object) is exempted from this check since it's indistinguishable from an omitempty-dropped field. - Reject rulesets with two rules of the same type up front, since RepositoryRulesetRules has one field per type and a duplicate would silently overwrite the first rule during the round-trip rather than erroring or producing two rules. - Add the "exempt" bypass_mode (alongside the existing "always" and "pull_request") to bypass_actors, matching the repository, organization, and enterprise ruleset APIs. - Add "User", "EnterpriseOwner", and "EnterpriseRole" to the bypass_actors actor_type enum. The consolidated create_repository_ruleset tool shares one schema across all three levels, so the enum is now the union of every level's valid actor types (repository and organization already matched; only enterprise adds EnterpriseOwner/EnterpriseRole), with the enterprise-only values called out in the description. An invalid combination (e.g. EnterpriseOwner at the repository level) is left to the GitHub API to reject, consistent with how this tool already defers other cross-field validation. - Add HasAll/ChallengeAll coverage for AdminEnterprise -> ReadEnterprise scope hierarchy resolution (pkg/scopes/scopes_test.go already asserted the static ScopeHierarchy map entry). Regenerated toolsnaps for create_repository_ruleset. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../create_repository_ruleset.snap | 12 +- pkg/github/rulesets.go | 91 ++++++++++-- pkg/github/rulesets_test.go | 139 ++++++++++++++++++ pkg/scopes/scopes_test.go | 5 + 4 files changed, 228 insertions(+), 19 deletions(-) diff --git a/pkg/github/__toolsnaps__/create_repository_ruleset.snap b/pkg/github/__toolsnaps__/create_repository_ruleset.snap index fc2bdd939b..8ee7fbd724 100644 --- a/pkg/github/__toolsnaps__/create_repository_ruleset.snap +++ b/pkg/github/__toolsnaps__/create_repository_ruleset.snap @@ -16,21 +16,25 @@ "type": "number" }, "actor_type": { - "description": "The type of actor that can bypass a ruleset", + "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" + "DeployKey", + "User", + "EnterpriseOwner", + "EnterpriseRole" ], "type": "string" }, "bypass_mode": { - "description": "When the specified actor can bypass the ruleset", + "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" + "pull_request", + "exempt" ], "type": "string" } diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index 17b398d75a..2153ac395f 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -694,13 +694,13 @@ func rulesetWriteProperties() map[string]*jsonschema.Schema { }, "actor_type": { Type: "string", - Enum: []any{"Integration", "OrganizationAdmin", "RepositoryRole", "Team", "DeployKey"}, - Description: "The type of actor that can bypass a ruleset", + 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"}, - Description: "When the specified actor can bypass the ruleset", + 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.", }, }, }, @@ -731,6 +731,8 @@ 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 { ruleMap, ok := rule.(map[string]any) if !ok { @@ -740,7 +742,21 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules 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{ @@ -775,26 +791,66 @@ 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 it - // does not recognize, which would let a typo create a weaker ruleset than the - // caller requested. Verify every requested rule type survived the round-trip. - appliedRuleTypes, errResult := rulesetAppliedRuleTypes(ruleset.Rules) + // 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, survived the round-trip. + appliedRules, errResult := rulesetAppliedRules(ruleset.Rules) if errResult != nil { return github.RepositoryRuleset{}, errResult } for _, ruleType := range requestedRuleTypes { - if !appliedRuleTypes[ruleType] { + appliedParameters, ok := appliedRules[ruleType] + if !ok { return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("unsupported or unrecognized rule type: %q", ruleType)) } + for key, value := range requestedRuleParameters[ruleType] { + if _, ok := appliedParameters[key]; ok { + continue + } + if isZeroJSONValue(value) { + // A caller-supplied zero value (false, 0, "", empty array/object) + // is indistinguishable from an omitted field once round-tripped + // through a struct field tagged `omitempty`, so it isn't proof of + // an unrecognized key. + continue + } + return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("rule %q: unsupported or unrecognized parameter: %q", ruleType, key)) + } } return ruleset, nil } -// rulesetAppliedRuleTypes marshals the parsed rules back to the API's array form -// and returns the set of rule types that were actually retained. -func rulesetAppliedRuleTypes(rules *github.RepositoryRulesetRules) (map[string]bool, *mcp.CallToolResult) { - applied := map[string]bool{} +// 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 set of +// parameter keys 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 } @@ -803,13 +859,18 @@ func rulesetAppliedRuleTypes(rules *github.RepositoryRulesetRules) (map[string]b return nil, utils.NewToolResultErrorFromErr("failed to validate ruleset rules", err) } var ruleObjects []struct { - Type string `json:"type"` + 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 { - applied[rule.Type] = true + parameters := rule.Parameters + if parameters == nil { + parameters = map[string]any{} + } + applied[rule.Type] = parameters } return applied, nil } diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index 1f2cdbbbf6..da3c8843a9 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -529,6 +529,145 @@ func Test_CreateRepositoryRuleset(t *testing.T) { 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("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("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) { diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index b314ec0122..c9a74781e2 100644 --- a/pkg/scopes/scopes_test.go +++ b/pkg/scopes/scopes_test.go @@ -29,10 +29,15 @@ func TestScopeChecks(t *testing.T) { assert.True(t, HasAll([]string{"repo", "workflow"}, Repo, Workflow)) assert.False(t, HasAll([]string{"repo"}, Repo, Workflow)) assert.True(t, HasAll([]string{"admin:org"}, ReadOrg)) + assert.True(t, HasAll([]string{"admin:enterprise"}, ReadEnterprise)) + assert.False(t, HasAll([]string{"read:enterprise"}, AdminEnterprise)) assert.True(t, HasAllScopeNames([]string{"admin:org"}, []string{"read:org"})) + assert.True(t, HasAllScopeNames([]string{"admin:enterprise"}, []string{"read:enterprise"})) assert.False(t, HasAllScopeNames([]string{"repo"}, []string{"repo", "workflow"})) assert.Nil(t, ChallengeAll([]string{"repo", "workflow"}, Repo, Workflow)) assert.Equal(t, []string{"repo", "workflow"}, ChallengeAll([]string{"repo"}, Repo, Workflow)) + assert.Nil(t, ChallengeAll([]string{"admin:enterprise"}, ReadEnterprise)) + assert.Equal(t, []string{"read:enterprise"}, ChallengeAll([]string{"repo"}, ReadEnterprise)) } func TestDynamicChallenge(t *testing.T) { From 01cd420a5b5a3477859f96a197d356afa1aedd6f Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 25 Aug 2026 17:24:58 +0200 Subject: [PATCH 4/6] fix(governance): close scope-challenge bypass and round-trip gaps Address a second round of Copilot review feedback found on the consolidated ruleset tools: - Security fix: the "level" dispatch in both tool handlers lowercased the argument (strings.ToLower(level)) before matching it, while the scope challenge in rulesetReadScopeAccess/rulesetWriteScopeAccess matches "level" with an exact, case-sensitive comparison. A caller could send level: "Organization" (or "Enterprise") and reach the organization/ enterprise-level API call while the OAuth middleware -- which challenges on the same raw argument -- found no case match and required no admin:org / admin:enterprise scope at all. Handler dispatch on "level" is now case-sensitive (matching the schema's lowercase enum exactly), so a mismatched-case value now falls through to "unknown level" in both tools instead of silently executing a privileged call unscoped. "method" case normalization is untouched since it has no bearing on scope selection. Added regression tests in both Test_RepositoryRulesetRead and Test_CreateRepositoryRuleset that assert the API is never called and the scope challenge independently agrees. - Fixed 12 instances where a GitHub API response's body/connection leaked on the error path: defer resp.Body.Close() was installed only after the err != nil check returned, so a non-nil response accompanying an error was never closed. Every call site now guards `if resp != nil { defer ... }` before checking err, matching the existing FetchRepoIsPrivate pattern in pkg/github/repositories.go. - Generalized the rule-parameter round-trip check into a recursive droppedKeyPath helper and applied it to `conditions` as well as rules[].parameters. github.RepositoryRulesetConditions silently drops unrecognized keys the same way github.RepositoryRulesetRules does (e.g. a "ref_names" typo for "ref_name", or a nested "includes" typo for "include"), so an unrecognized condition key at any nesting depth is now rejected by name instead of silently producing a ruleset with weaker or no applicability conditions than requested. Only nested objects are recursed into; array elements stay opaque so API-side reordering can't produce a false positive. Note: committed unsigned -- the local 1Password SSH-signing agent was unavailable (session locked) at commit time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/rulesets.go | 134 +++++++++++++++++++++++------ pkg/github/rulesets_test.go | 166 ++++++++++++++++++++++++++++++++++++ 2 files changed, 272 insertions(+), 28 deletions(-) diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index 2153ac395f..98e3deec74 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -189,7 +189,7 @@ func RepositoryRulesetRead(t translations.TranslationHelperFunc) inventory.Serve return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - switch strings.ToLower(level) { + switch level { case "repository": return repositoryRulesetReadRepository(ctx, client, strings.ToLower(method), args) case "organization": @@ -337,10 +337,12 @@ func repositoryRulesetReadEnterprise(ctx context.Context, client *github.Client, // 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(ruleset), nil } @@ -357,10 +359,12 @@ func ListRepositoryRulesets(ctx context.Context, client *github.Client, owner, r } 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(rulesets), nil } @@ -373,10 +377,12 @@ func GetRepositoryRulesForBranch(ctx context.Context, client *github.Client, own } 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(branchRules), nil } @@ -449,10 +455,12 @@ func ListRepositoryRuleSuites(ctx context.Context, client *github.Client, owner, 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(ruleSuites), nil } @@ -469,10 +477,12 @@ func GetRepositoryRuleSuite(ctx context.Context, client *github.Client, owner, r 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(ruleSuite), nil } @@ -481,10 +491,12 @@ func GetRepositoryRuleSuite(ctx context.Context, client *github.Client, owner, r // 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(ruleset), nil } @@ -498,10 +510,12 @@ func ListOrganizationRepositoryRulesets(ctx context.Context, client *github.Clie } 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(rulesets), nil } @@ -510,10 +524,12 @@ func ListOrganizationRepositoryRulesets(ctx context.Context, client *github.Clie // 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(ruleset), nil } @@ -541,10 +557,12 @@ func ListEnterpriseRepositoryRulesets(ctx context.Context, client *github.Client 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(rulesets), nil } @@ -596,7 +614,7 @@ func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.Ser return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - switch strings.ToLower(level) { + switch level { case "repository": owner, err := RequiredParam[string](args, "owner") if err != nil { @@ -607,10 +625,12 @@ func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.Ser 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(created), nil, nil case "organization": org, err := RequiredParam[string](args, "org") @@ -618,10 +638,12 @@ func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.Ser 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(created), nil, nil case "enterprise": enterprise, err := RequiredParam[string](args, "enterprise") @@ -629,10 +651,12 @@ func CreateRepositoryRuleset(t translations.TranslationHelperFunc) inventory.Ser 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 } - defer func() { _ = resp.Body.Close() }() return MarshalledTextResult(created), nil, nil default: return utils.NewToolResultError(fmt.Sprintf("unknown level: %q (expected 'repository', 'organization', or 'enterprise')", level)), nil, nil @@ -795,7 +819,8 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules // 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, survived the round-trip. + // 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 @@ -805,24 +830,57 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules if !ok { return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("unsupported or unrecognized rule type: %q", ruleType)) } - for key, value := range requestedRuleParameters[ruleType] { - if _, ok := appliedParameters[key]; ok { - continue + 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)) } - if isZeroJSONValue(value) { - // A caller-supplied zero value (false, 0, "", empty array/object) - // is indistinguishable from an omitted field once round-tripped - // through a struct field tagged `omitempty`, so it isn't proof of - // an unrecognized key. - continue - } - return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("rule %q: unsupported or unrecognized parameter: %q", ruleType, key)) + } + } + + // 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 dotted path of the first key that +// did not survive, or "" if every key survived. A caller-supplied key whose +// value 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. Only +// nested objects are recursed into; array elements are treated as opaque so +// that reordering by the API cannot produce a false positive. +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 + } + requestedChild, requestedIsMap := requestedValue.(map[string]any) + appliedChild, appliedIsMap := appliedValue.(map[string]any) + if requestedIsMap && appliedIsMap { + if nested := droppedKeyPath(requestedChild, appliedChild); nested != "" { + return key + "." + nested + } + } + } + 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 @@ -847,8 +905,8 @@ func isZeroJSONValue(v any) bool { } // rulesetAppliedRules marshals the parsed rules back to the API's array form -// and returns, for each rule type that was actually retained, the set of -// parameter keys the corresponding go-github struct recognized. +// 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 { @@ -874,3 +932,23 @@ func rulesetAppliedRules(rules *github.RepositoryRulesetRules) (map[string]map[s } 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 index da3c8843a9..7f1b52153a 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -375,6 +375,36 @@ func Test_RepositoryRulesetRead(t *testing.T) { 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) { @@ -668,6 +698,108 @@ func Test_CreateRepositoryRuleset(t *testing.T) { assert.Equal(t, github.BypassMode("exempt"), *capturedBody.BypassActors[0].BypassMode) }) + 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) { @@ -772,6 +904,40 @@ func Test_CreateRepositoryRuleset(t *testing.T) { 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 From 0c25c924a9c486ae07f85378a00bf967fc8f9572 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 25 Aug 2026 17:37:59 +0200 Subject: [PATCH 5/6] fix(governance): reject unrecognized bypass_actors keys Address the remaining Copilot review comment: github.BypassActor only recognizes actor_id, actor_type, and bypass_mode, and JSON unmarshal silently discards any other key (e.g. a "bypass_modes" typo). Because the API defaults an omitted bypass_mode to "always", a dropped key would silently grant the actor broader bypass rights than requested. Since BypassActor is a small, fixed set of keys (unlike rule parameters or conditions, which vary per rule/condition type and need the round-trip check), each bypass_actors[i] is now validated directly against an allow-list of the three recognized keys before the request is built, and the unrecognized key is named in the error along with its array index. Added a regression test with a "bypass_modes" typo, and confirmed the existing valid-input test (EnterpriseOwner/exempt) still passes. Note: committed unsigned -- the local 1Password SSH-signing agent is still unavailable (session locked). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/rulesets.go | 15 +++++++++++++++ pkg/github/rulesets_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index 98e3deec74..b29473a9c3 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -803,6 +803,21 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules 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 } diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index 7f1b52153a..6821f6c5ee 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -698,6 +698,40 @@ func Test_CreateRepositoryRuleset(t *testing.T) { 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 From 8e78920fdd1fe4891c956dbd5e25ef7bab29614c Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 25 Aug 2026 17:48:17 +0200 Subject: [PATCH 6/6] fix(governance): recurse into rule-parameter array elements Address the final Copilot review comment: droppedKeyPath previously treated array elements as opaque, so an unrecognized key inside an array element (e.g. required_status_checks[].integration_ids, a typo for integration_id) was silently dropped by go-github's JSON unmarshal without being caught -- the resulting rule would then accept a status check from any integration instead of only the one requested. This round-trip is entirely local (our own marshal/unmarshal of a go-github struct, not a remote API response), so slice order and length are deterministic and safe to compare by index. droppedKeyPath now delegates to a new droppedValuePath helper that recurses into both nested objects and array elements, reporting paths like "required_status_checks[0].integration_ids" when a caller-supplied key disappears in either a map or an array position. Added a regression test for the array-nested typo, and a companion test confirming valid array-nested parameters still round-trip without being misflagged. Note: committed unsigned -- the local 1Password SSH-signing agent is still unavailable (session locked). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/rulesets.go | 60 +++++++++++++++++++------ pkg/github/rulesets_test.go | 88 +++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 13 deletions(-) diff --git a/pkg/github/rulesets.go b/pkg/github/rulesets.go index b29473a9c3..96c1ff66e5 100644 --- a/pkg/github/rulesets.go +++ b/pkg/github/rulesets.go @@ -869,13 +869,15 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules } // droppedKeyPath recursively compares a caller-supplied object against its -// round-tripped counterpart and returns the dotted path of the first key that -// did not survive, or "" if every key survived. A caller-supplied key whose -// value 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. Only -// nested objects are recursed into; array elements are treated as opaque so -// that reordering by the API cannot produce a false positive. +// 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] @@ -885,17 +887,49 @@ func droppedKeyPath(requested, applied map[string]any) string { } return key } - requestedChild, requestedIsMap := requestedValue.(map[string]any) - appliedChild, appliedIsMap := appliedValue.(map[string]any) - if requestedIsMap && appliedIsMap { - if nested := droppedKeyPath(requestedChild, appliedChild); nested != "" { - return key + "." + nested - } + 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 diff --git a/pkg/github/rulesets_test.go b/pkg/github/rulesets_test.go index 6821f6c5ee..46c53825df 100644 --- a/pkg/github/rulesets_test.go +++ b/pkg/github/rulesets_test.go @@ -667,6 +667,94 @@ func Test_CreateRepositoryRuleset(t *testing.T) { 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{