From 05d9ff132b3a8b6691686e4985e12c7400ca6716 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 21 Aug 2026 02:15:32 +0200 Subject: [PATCH] feat(auth): add OAuth scope policies Model authorization as alternative paths with conjunctive requirements and per-requirement scope alternatives. Resolve call-specific policies from tool arguments for precise PAT filtering and OAuth challenges. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 26e41558-43f9-42b2-8569-8489957c2b0a --- README.md | 202 ++++++++-------- cmd/github-mcp-server/generate_docs.go | 78 ++++--- cmd/github-mcp-server/list_scopes.go | 63 ++--- cmd/github-mcp-server/main_test.go | 18 +- docs/feature-flags.md | 70 +++--- docs/insiders-features.md | 16 +- docs/scope-filtering.md | 15 +- pkg/github/actions.go | 8 +- pkg/github/code_quality.go | 2 +- pkg/github/code_scanning.go | 4 +- pkg/github/context_tools.go | 6 +- pkg/github/copilot.go | 6 +- pkg/github/dependabot.go | 4 +- pkg/github/dependencies.go | 18 +- pkg/github/discussions.go | 10 +- pkg/github/feature_flags_test.go | 2 +- pkg/github/find_duplicate.go | 2 +- pkg/github/find_duplicate_test.go | 3 +- pkg/github/gists.go | 8 +- pkg/github/git.go | 2 +- pkg/github/header_params_test.go | 4 + pkg/github/issue_dependencies.go | 4 +- pkg/github/issue_fields.go | 3 +- pkg/github/issue_fields_test.go | 4 +- pkg/github/issues.go | 18 +- pkg/github/issues_granular.go | 24 +- pkg/github/labels.go | 6 +- pkg/github/notifications.go | 12 +- pkg/github/projects.go | 6 +- pkg/github/pullrequests.go | 20 +- pkg/github/pullrequests_granular.go | 20 +- pkg/github/repositories.go | 122 +++++----- pkg/github/repositories_test.go | 3 +- pkg/github/repository_path.go | 79 +++++++ pkg/github/repository_path_test.go | 145 ++++++++++++ pkg/github/scope_filter.go | 50 ++-- pkg/github/scope_filter_test.go | 62 ++--- pkg/github/search.go | 10 +- pkg/github/secret_scanning.go | 4 +- pkg/github/security_advisories.go | 8 +- pkg/github/tool_scopes.go | 44 ++++ pkg/github/tool_scopes_test.go | 73 ++++++ pkg/github/ui_tools.go | 3 +- pkg/http/middleware/scope_challenge.go | 14 +- pkg/http/middleware/scope_challenge_test.go | 123 ++++++++++ pkg/inventory/server_tool.go | 57 +++-- pkg/inventory/server_tool_test.go | 2 + pkg/scopes/map.go | 110 +++------ pkg/scopes/map_test.go | 246 ++++---------------- pkg/scopes/scopes.go | 208 +++++++++-------- pkg/scopes/scopes_test.go | 203 +++++----------- 51 files changed, 1252 insertions(+), 972 deletions(-) create mode 100644 pkg/github/repository_path.go create mode 100644 pkg/github/repository_path_test.go create mode 100644 pkg/github/tool_scopes.go create mode 100644 pkg/github/tool_scopes_test.go diff --git a/README.md b/README.md index 9527478a0f..34592be048 100644 --- a/README.md +++ b/README.md @@ -626,7 +626,7 @@ The following sets of tools are available: workflow Actions - **actions_get** - Get details of GitHub Actions resources (workflows, workflow runs, jobs, and artifacts) - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `method`: The method to execute (string, required) - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) @@ -638,7 +638,7 @@ The following sets of tools are available: (string, required) - **actions_list** - List GitHub Actions workflows in a repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `method`: The action to perform (string, required) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (default: 1) (number, optional) @@ -653,7 +653,7 @@ The following sets of tools are available: - `workflow_runs_filter`: Filters for workflow runs. **ONLY** used when method is 'list_workflow_runs' (object, optional) - **actions_run_trigger** - Trigger GitHub Actions workflow actions - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `inputs`: Inputs the workflow accepts. Only used for 'run_workflow' method. (object, optional) - `method`: The method to execute (string, required) - `owner`: Repository owner (string, required) @@ -663,7 +663,7 @@ The following sets of tools are available: - `workflow_id`: The workflow ID (numeric) or workflow file name (e.g., main.yml, ci.yaml). Required for 'run_workflow' method. (string, optional) - **get_job_logs** - Get GitHub Actions workflow job logs - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `failed_only`: When true, gets logs for all failed jobs in the workflow run specified by run_id. Requires run_id to be provided. (boolean, optional) - `job_id`: The unique identifier of the workflow job. Required when getting logs for a single job. (number, optional) - `owner`: Repository owner (string, required) @@ -679,7 +679,7 @@ The following sets of tools are available: code-square Code Quality - **get_code_quality_finding** - Get code quality finding - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `findingNumber`: The number of the finding. (number, required) - `owner`: The owner of the repository. (string, required) - `repo`: The name of the repository. (string, required) @@ -691,15 +691,15 @@ The following sets of tools are available: codescan Code Security - **get_code_scanning_alert** - Get code scanning alert - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `alertNumber`: The number of the alert. (number, required) - `owner`: The owner of the repository. (string, required) - `repo`: The name of the repository. (string, required) - **list_code_scanning_alerts** - List code scanning alerts - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `owner`: The owner of the repository. (string, required) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -719,14 +719,14 @@ The following sets of tools are available: - No parameters required - **get_team_members** - Get team members - - **Required OAuth Scopes**: `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` + - **OAuth Scope Policy**: (`admin:org` OR `read:org` OR `write:org`) + - **Preferred OAuth Challenge**: `read:org` - `org`: Organization login (owner) that contains the team. (string, required) - `team_slug`: Team slug (string, required) - **get_teams** - Get teams - - **Required OAuth Scopes**: `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` + - **OAuth Scope Policy**: (`admin:org` OR `read:org` OR `write:org`) + - **Preferred OAuth Challenge**: `read:org` - `user`: Username to get teams for. If not provided, uses the authenticated user. (string, optional) @@ -736,7 +736,7 @@ The following sets of tools are available: copilot Copilot - **assign_copilot_to_issue** - Assign Copilot to issue - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `base_ref`: Git reference (e.g., branch) that the agent will start its work from. If not specified, defaults to the repository's default branch (string, optional) - `custom_instructions`: Optional custom instructions to guide the agent beyond the issue body. Use this to provide additional context, constraints, or guidance that is not captured in the issue description (string, optional) - `issue_number`: Issue number (number, required) @@ -744,7 +744,7 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **request_copilot_review** - Request Copilot review - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number (number, required) - `repo`: Repository name (string, required) @@ -756,7 +756,7 @@ The following sets of tools are available: copilot Copilot Issue Intents - **assign_copilot_to_issue_with_intent** - Assign Copilot to issue with intent - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `base_ref`: Git reference (e.g., branch) that the agent will start its work from. If not specified, defaults to the repository's default branch. Ignored when is_suggestion is true (string, optional) - `confidence`: How confident you are in this choice. 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, required) - `custom_instructions`: Optional custom instructions to guide the agent beyond the issue body. Ignored when is_suggestion is true (string, optional) @@ -773,15 +773,15 @@ The following sets of tools are available: dependabot Dependabot - **get_dependabot_alert** - Get dependabot alert - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `alertNumber`: The number of the alert. (number, required) - `owner`: The owner of the repository. (string, required) - `repo`: The name of the repository. (string, required) - **list_dependabot_alerts** - List dependabot alerts - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) - `owner`: The owner of the repository. (string, required) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -796,7 +796,7 @@ The following sets of tools are available: comment-discussion Discussions - **discussion_comment_write** - Manage discussion comments - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: Comment content (required for 'add', 'reply', and 'update' methods) (string, optional) - `commentNodeID`: The Node ID of the discussion comment (required for 'reply', 'update', 'delete', 'mark_answer', and 'unmark_answer' methods). For 'reply', this is the top-level comment to reply to; GitHub Discussions only support one level of nesting. (string, optional) - `discussionNumber`: Discussion number (required for 'add' and 'reply' methods) (number, optional) @@ -813,13 +813,13 @@ The following sets of tools are available: - `repo`: Repository name (required for 'add' and 'reply' methods) (string, optional) - **get_discussion** - Get discussion - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `discussionNumber`: Discussion Number (number, required) - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - **get_discussion_comments** - Get discussion comments - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) - `discussionNumber`: Discussion Number (number, required) - `includeReplies`: When true, each top-level comment will include its replies nested within it (up to 100 replies per comment, which is the GitHub API maximum). Defaults to false. (boolean, optional) @@ -828,12 +828,12 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **list_discussion_categories** - List discussion categories - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (string, required) - `repo`: Repository name. If not provided, discussion categories will be queried at the organisation level. (string, optional) - **list_discussions** - List discussions - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) - `category`: Optional filter by discussion category ID. If provided, only discussions with this category are listed. (string, optional) - `direction`: Order direction. (string, optional) @@ -849,7 +849,7 @@ The following sets of tools are available: logo-gist Gists - **create_gist** - Create Gist - - **Required OAuth Scopes**: `gist` + - **OAuth Scope Policy**: `gist` - `content`: Content for simple single-file gist creation (string, required) - `description`: Description of the gist (string, optional) - `filename`: Filename for simple single-file gist creation (string, required) @@ -865,7 +865,7 @@ The following sets of tools are available: - `username`: GitHub username (omit for authenticated user's gists) (string, optional) - **update_gist** - Update Gist - - **Required OAuth Scopes**: `gist` + - **OAuth Scope Policy**: `gist` - `content`: Content for the file (string, required) - `description`: Updated description of the gist (string, optional) - `filename`: Filename to update or create (string, required) @@ -878,7 +878,7 @@ The following sets of tools are available: git-branch Git - **get_repository_tree** - Get repository tree - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (username or organization) (string, required) - `path_filter`: Optional path prefix to filter the tree results (e.g., 'src/' to only show files in the src directory) (string, optional) - `recursive`: Setting this parameter to true returns the objects or subtrees referenced by the tree. Default is false (boolean, optional) @@ -892,7 +892,7 @@ The following sets of tools are available: issue-opened Issues - **add_issue_comment** - Add comment to issue or pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: Comment content. Required unless reaction is provided. (string, optional) - `comment_id`: The numeric ID of the issue or pull request comment to react to. Use this for reactions to comments; omit it to react to the issue or pull request itself. Cannot be combined with body. (integer, optional) - `issue_number`: Issue or pull request number to comment on or react to. (number, required) @@ -901,13 +901,13 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **get_label** - Get a specific label from a repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `name`: Label name. (string, required) - `owner`: Repository owner (username or organization name) (string, required) - `repo`: Repository name (string, required) - **issue_read** - Get issue details - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The number of the issue (number, required) - `method`: The read operation to perform on a single issue. Options are: @@ -923,7 +923,7 @@ The following sets of tools are available: - `repo`: The name of the repository (string, required) - **issue_write** - Create or update issue/pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `assignees`: Usernames to assign to this issue (string[], optional) - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Required when state_reason is 'duplicate'. (number, optional) @@ -944,19 +944,19 @@ The following sets of tools are available: - `type`: Type of this issue. For updates, pass null to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - **list_issue_fields** - List issue fields - - **Required OAuth Scopes (any of)**: `repo`, `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` + - **OAuth Scope Policy**: `repo` OR (`admin:org` OR `read:org` OR `write:org`) + - **Preferred OAuth Challenge**: `repo` - `owner`: The account owner of the repository or organization. The name is not case sensitive. (string, required) - `repo`: The name of the repository. When provided, returns fields for this specific repository (inherited from its organization). When omitted, returns org-level fields directly. (string, optional) - **list_issue_types** - List available issue types - - **Required OAuth Scopes (any of)**: `repo`, `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` + - **OAuth Scope Policy**: `repo` OR (`admin:org` OR `read:org` OR `write:org`) + - **Preferred OAuth Challenge**: `repo` - `owner`: The account owner of the repository or organization. (string, required) - `repo`: The name of the repository. When provided, returns issue types for this specific repository. When omitted, returns org-level issue types directly. (string, optional) - **list_issues** - List issues - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) - `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional) - `field_filters`: Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date). (object[], optional) @@ -970,7 +970,7 @@ The following sets of tools are available: - `state`: Filter by state, by default both open and closed issues are returned when not provided (string, optional) - **search_issues** - Search issues - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `fields`: Subset of fields to return for each issue result. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body', 'reactions', and 'labels' in particular drops the largest per-result data. (string[], optional) - `order`: Sort order (string, optional) - `owner`: Optional repository owner. If provided with repo, only issues for this repository are listed. (string, optional) @@ -981,7 +981,7 @@ The following sets of tools are available: - `sort`: Sort field by number of matches of categories, defaults to best match (string, optional) - **sub_issue_write** - Change sub-issue - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after_id`: The ID of the sub-issue to be prioritized after (either after_id OR before_id should be specified) (number, optional) - `before_id`: The ID of the sub-issue to be prioritized before (either after_id OR before_id should be specified) (number, optional) - `issue_number`: The number of the parent issue (number, required) @@ -1004,13 +1004,13 @@ The following sets of tools are available: tag Labels - **get_label** - Get a specific label from a repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `name`: Label name. (string, required) - `owner`: Repository owner (username or organization name) (string, required) - `repo`: Repository name (string, required) - **label_write** - Write operations on repository labels - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `color`: Label color as 6-character hex code without '#' prefix (e.g., 'f29513'). Required for 'create', optional for 'update'. (string, optional) - `description`: Label description text. Optional for 'create' and 'update'. (string, optional) - `method`: Operation to perform: 'create', 'update', or 'delete' (string, required) @@ -1020,7 +1020,7 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **list_label** - List labels from a repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (username or organization name) - required for all operations (string, required) - `repo`: Repository name - required for all operations (string, required) @@ -1031,16 +1031,16 @@ The following sets of tools are available: bell Notifications - **dismiss_notification** - Dismiss notification - - **Required OAuth Scopes**: `notifications` + - **OAuth Scope Policy**: `notifications` - `state`: The new state of the notification (read/done) (string, required) - `threadID`: The ID of the notification thread (string, required) - **get_notification_details** - Get notification details - - **Required OAuth Scopes**: `notifications` + - **OAuth Scope Policy**: `notifications` - `notificationID`: The ID of the notification (string, required) - **list_notifications** - List notifications - - **Required OAuth Scopes**: `notifications` + - **OAuth Scope Policy**: `notifications` - `before`: Only show notifications updated before the given time (ISO 8601 format) (string, optional) - `filter`: Filter notifications to, use default unless specified. Read notifications are ones that have already been acknowledged by the user. Participating notifications are those that the user is directly involved in, such as issues or pull requests they have commented on or created. (string, optional) - `owner`: Optional repository owner. If provided with repo, only notifications for this repository are listed. (string, optional) @@ -1050,18 +1050,18 @@ The following sets of tools are available: - `since`: Only show notifications updated after the given time (ISO 8601 format) (string, optional) - **manage_notification_subscription** - Manage notification subscription - - **Required OAuth Scopes**: `notifications` + - **OAuth Scope Policy**: `notifications` - `action`: Action to perform: ignore, watch, or delete the notification subscription. (string, required) - `notificationID`: The ID of the notification thread. (string, required) - **manage_repository_notification_subscription** - Manage repository notification subscription - - **Required OAuth Scopes**: `notifications` + - **OAuth Scope Policy**: `notifications` - `action`: Action to perform: ignore, watch, or delete the repository notification subscription. (string, required) - `owner`: The account owner of the repository. (string, required) - `repo`: The name of the repository. (string, required) - **mark_all_notifications_read** - Mark all notifications as read - - **Required OAuth Scopes**: `notifications` + - **OAuth Scope Policy**: `notifications` - `lastReadAt`: Describes the last point that notifications were checked (optional). Default: Now (string, optional) - `owner`: Optional repository owner. If provided with repo, only notifications for this repository are marked as read. (string, optional) - `repo`: Optional repository name. If provided with owner, only notifications for this repository are marked as read. (string, optional) @@ -1073,8 +1073,8 @@ The following sets of tools are available: organization Organizations - **search_orgs** - Search organizations - - **Required OAuth Scopes**: `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` + - **OAuth Scope Policy**: (`admin:org` OR `read:org` OR `write:org`) + - **Preferred OAuth Challenge**: `read:org` - `order`: Sort order (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -1088,8 +1088,8 @@ The following sets of tools are available: project Projects - **projects_get** - Get details of GitHub Projects resources - - **Required OAuth Scopes**: `read:project` - - **Accepted OAuth Scopes**: `project`, `read:project` + - **OAuth Scope Policy**: (`project` OR `read:project`) + - **Preferred OAuth Challenge**: `read:project` - `field_id`: The field's ID. Required for 'get_project_field' method. (number, optional) - `field_names`: Specific list of field names to include in the response when getting a project item (e.g. ["Status", "Priority"]). Resolved server-side to field IDs — pass this instead of 'fields' when you only know the human-readable names. Mutually exclusive with 'fields' — provide one, not both. Only used for 'get_project_item' method. (string[], optional) - `fields`: Specific list of field IDs to include in the response when getting a project item (e.g. ["102589", "985201", "169875"]). If neither 'fields' nor 'field_names' is provided, only the title field is included. Mutually exclusive with 'field_names' — provide one, not both. Only used for 'get_project_item' method. (string[], optional) @@ -1102,8 +1102,8 @@ The following sets of tools are available: - `view_id`: The node ID of the project view. Required for 'get_project_view' method. (string, optional) - **projects_list** - List GitHub Projects resources - - **Required OAuth Scopes**: `read:project` - - **Accepted OAuth Scopes**: `project`, `read:project` + - **OAuth Scope Policy**: (`project` OR `read:project`) + - **Preferred OAuth Challenge**: `read:project` - `after`: Forward pagination cursor from previous pageInfo.nextCursor. (string, optional) - `before`: Backward pagination cursor from previous pageInfo.prevCursor (rare). (string, optional) - `field_names`: Field names to include when listing project items (e.g. ["Status", "Priority"]). Resolved server-side to field IDs — pass this instead of 'fields' when you only know the human-readable names. Names that fail to resolve return a structured error. Mutually exclusive with 'fields' — provide one, not both. Only used for 'list_project_items' method. (string[], optional) @@ -1116,7 +1116,7 @@ The following sets of tools are available: - `query`: Filter/query string. For list_projects: filter by title text and state (e.g. "roadmap is:open"). For list_project_items: advanced filtering using GitHub's project filtering syntax. (string, optional) - **projects_write** - Manage GitHub Projects - - **Required OAuth Scopes**: `project` + - **OAuth Scope Policy**: `project` - `body`: The body of the status update (markdown). Used for 'create_project_status_update' method. (string, optional) - `field_name`: The name of the iteration field (e.g. 'Sprint'). Required for 'create_iteration_field' method. (string, optional) - `filter`: Saved view filter; omit on update to preserve it, or pass null to clear it. (string | null, optional) @@ -1151,7 +1151,7 @@ The following sets of tools are available: git-pull-request Pull Requests - **add_comment_to_pending_review** - Add review comment to the requester's latest pending pull request review - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: The text of the review comment (string, required) - `line`: The line of the blob in the pull request diff that the comment applies to. For multi-line comments, the last line of the range (number, optional) - `owner`: Repository owner (string, required) @@ -1164,7 +1164,7 @@ The following sets of tools are available: - `subjectType`: The level at which the comment is targeted (string, required) - **add_reply_to_pull_request_comment** - Add reply to pull request comment - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: The text of the reply. Required unless reaction is provided. (string, optional) - `commentId`: The numeric ID of the pull request review comment to reply or react to. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...). (number, required) - `owner`: Repository owner (string, required) @@ -1173,7 +1173,7 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **create_pull_request** - Open new pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `base`: Branch to merge into (string, required) - `body`: PR description (string, optional) - `draft`: Create as draft PR (boolean, optional) @@ -1185,7 +1185,7 @@ The following sets of tools are available: - `title`: PR title (string, required) - **list_pull_requests** - List pull requests - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `base`: Filter by base branch (string, optional) - `direction`: Sort direction (string, optional) - `fields`: Subset of fields to return for each pull request. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body' in particular drops the largest per-result data. (string[], optional) @@ -1198,7 +1198,7 @@ The following sets of tools are available: - `state`: Filter by state (string, optional) - **merge_pull_request** - Merge pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `commit_message`: Extra detail for merge commit (string, optional) - `commit_title`: Title for merge commit (string, optional) - `merge_method`: Merge method (string, optional) @@ -1207,7 +1207,7 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **pull_request_read** - Get details for a single pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after`: Cursor for pagination, used only by the get_review_comments method. Pass the endCursor from the previous page's PageInfo to fetch the next page. (string, optional) - `method`: Action to specify what pull request data needs to be retrieved from GitHub. Possible options: @@ -1228,7 +1228,7 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **pull_request_review_write** - Write operations (create, submit, delete) on pull request reviews - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: Review comment text (string, optional) - `commitID`: SHA of commit to review (string, optional) - `event`: Review action to perform. (string, optional) @@ -1239,7 +1239,7 @@ The following sets of tools are available: - `threadId`: The node ID of the review thread (e.g., PRRT_kwDOxxx). Required for resolve_thread and unresolve_thread methods. Get thread IDs from pull_request_read with method get_review_comments. (string, optional) - **search_pull_requests** - Search pull requests - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `fields`: Subset of fields to return for each pull request result. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body', 'reactions', and 'labels' in particular drops the largest per-result data. (string[], optional) - `order`: Sort order (string, optional) - `owner`: Optional repository owner. If provided with repo, only pull requests for this repository are listed. (string, optional) @@ -1250,7 +1250,7 @@ The following sets of tools are available: - `sort`: Sort field by number of matches of categories, defaults to best match (string, optional) - **update_pull_request** - Edit pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `base`: New base branch name (string, optional) - `body`: New description (string, optional) - `draft`: Mark pull request as draft (true) or ready for review (false) (boolean, optional) @@ -1263,7 +1263,7 @@ The following sets of tools are available: - `title`: New title (string, optional) - **update_pull_request_branch** - Update pull request branch - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `expectedHeadSha`: The expected SHA of the pull request's HEAD ref (string, optional) - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number (number, required) @@ -1276,14 +1276,14 @@ The following sets of tools are available: repo Repositories - **create_branch** - Create branch - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `branch`: Name for new branch (string, required) - `from_branch`: Source branch (defaults to repo default) (string, optional) - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - **create_or_update_file** - Create or update file - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `allow_symlink_write`: Set true to update a symbolic link itself; content must be its new target path. (boolean, optional) - `branch`: Branch to create/update the file in (string, required) - `content`: Content of the file, exactly as it should appear once written. Do not base64-encode it; this server does that before calling the REST API. (string, required) @@ -1294,7 +1294,7 @@ The following sets of tools are available: - `sha`: The blob SHA of the file being replaced. Required if the file already exists. (string, optional) - **create_repository** - Create repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `autoInit`: Initialize with README (boolean, optional) - `description`: Repository description (string, optional) - `name`: Repository name (string, required) @@ -1302,7 +1302,7 @@ The following sets of tools are available: - `private`: Whether the repository should be private. Defaults to true (private) when omitted. (boolean, optional) - **delete_file** - Delete file - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `branch`: Branch to delete the file from (string, required) - `message`: Commit message (string, required) - `owner`: Repository owner (username or organization) (string, required) @@ -1310,18 +1310,18 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **delete_repository** - Delete repository - - **Required OAuth Scopes (all required)**: `delete_repo`, `repo` + - **OAuth Scope Policy**: `delete_repo` AND `repo` - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **fork_repository** - Fork repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `organization`: Organization to fork to (string, optional) - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - **get_commit** - Get commit details - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `detail`: Level of detail to include for changed files. "none" omits stats and files entirely. "stats" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. "full_patch" additionally includes the unified diff content for each file and can be very large. (string, optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) @@ -1330,7 +1330,7 @@ The following sets of tools are available: - `sha`: Commit SHA, branch name, or tag name (string, required) - **get_file_contents** - Get file or directory contents - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `fields`: Subset of fields to return for each entry when the path is a directory. If omitted, all fields are returned. Ignored when the path is a single file. Use this to reduce response size when listing directories and you only need specific fields, e.g. just 'name' and 'type'. (string[], optional) - `owner`: Repository owner (username or organization) (string, required) - `path`: Path to file/directory (string, optional) @@ -1339,31 +1339,31 @@ The following sets of tools are available: - `sha`: Accepts optional commit SHA. If specified, it will be used instead of ref (string, optional) - **get_latest_release** - Get latest release - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - **get_release_by_tag** - Get a release by tag name - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `tag`: Tag name (e.g., 'v1.0.0') (string, required) - **get_tag** - Get tag details - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `tag`: Tag name (string, required) - **list_branches** - List branches - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `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) - `repo`: Repository name (string, required) - **list_commits** - List commits - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `author`: Author username or email address to filter commits by (string, optional) - `fields`: Subset of fields to return for each commit. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields, e.g. just 'sha' and 'html_url'. (string[], optional) - `owner`: Repository owner (string, required) @@ -1376,7 +1376,7 @@ The following sets of tools are available: - `until`: Only commits before this date will be returned (ISO 8601 format: YYYY-MM-DDTHH:MM:SSZ or YYYY-MM-DD) (string, optional) - **list_releases** - List releases - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `fields`: Subset of fields to return for each release. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body' in particular drops the largest per-release data. (string[], optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) @@ -1384,7 +1384,7 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **list_repository_collaborators** - List repository collaborators - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `affiliation`: Filter by affiliation. Can be one of: 'outside' (outside collaborators), 'direct' (all with permissions regardless of org membership), 'all' (all collaborators). Default: 'all' (string, optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (default 1, min 1) (number, optional) @@ -1392,14 +1392,14 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **list_tags** - List tags - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `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) - `repo`: Repository name (string, required) - **push_files** - Push files to repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `branch`: Branch to push to (string, required) - `files`: Array of file objects to push, each object with path (string) and content (string) (object[], required) - `message`: Commit message (string, required) @@ -1407,7 +1407,7 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - **search_code** - Search code - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `fields`: Subset of fields to return for each code search result. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'repository' and 'text_matches' in particular drops the largest per-result data. (string[], optional) - `order`: Sort order for results (string, optional) - `page`: Page number for pagination (min 1) (number, optional) @@ -1416,7 +1416,7 @@ The following sets of tools are available: - `sort`: Sort field ('indexed' only) (string, optional) - **search_commits** - Search commits - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `order`: Sort order (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -1424,7 +1424,7 @@ The following sets of tools are available: - `sort`: Sort by author or committer date (defaults to best match) (string, optional) - **search_repositories** - Search repositories - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `minimal_output`: Return minimal repository information (default: true). When false, returns full GitHub API repository objects. (boolean, optional) - `order`: Sort order (string, optional) - `page`: Page number for pagination (min 1) (number, optional) @@ -1439,15 +1439,15 @@ The following sets of tools are available: shield-lock Secret Protection - **get_secret_scanning_alert** - Get secret scanning alert - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `alertNumber`: The number of the alert. (number, required) - `owner`: The owner of the repository. (string, required) - `repo`: The name of the repository. (string, required) - **list_secret_scanning_alerts** - List secret scanning alerts - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `owner`: The owner of the repository. (string, required) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -1463,13 +1463,13 @@ The following sets of tools are available: shield Security Advisories - **get_global_security_advisory** - Get a global security advisory - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `ghsaId`: GitHub Security Advisory ID (format: GHSA-xxxx-xxxx-xxxx). (string, required) - **list_global_security_advisories** - List global security advisories - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `affects`: Filter advisories by affected package or version (e.g. "package1,package2@1.0.0"). (string, optional) - `cveId`: Filter by CVE ID. (string, optional) - `cwes`: Filter by Common Weakness Enumeration IDs (e.g. ["79", "284", "22"]). (string[], optional) @@ -1483,16 +1483,16 @@ The following sets of tools are available: - `updated`: Filter by update date or date range (ISO 8601 date or range). (string, optional) - **list_org_repository_security_advisories** - List org repository security advisories - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `direction`: Sort direction. (string, optional) - `org`: The organization login. (string, required) - `sort`: Sort field. (string, optional) - `state`: Filter by advisory state. (string, optional) - **list_repository_security_advisories** - List repository security advisories - - **Required OAuth Scopes**: `security_events` - - **Accepted OAuth Scopes**: `repo`, `security_events` + - **OAuth Scope Policy**: (`repo` OR `security_events`) + - **Preferred OAuth Challenge**: `security_events` - `direction`: Sort direction. (string, optional) - `owner`: The owner of the repository. (string, required) - `repo`: The name of the repository. (string, required) @@ -1506,7 +1506,7 @@ The following sets of tools are available: star Stargazers - **list_starred_repositories** - List starred repositories - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `direction`: The direction to sort the results by. (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -1514,12 +1514,12 @@ The following sets of tools are available: - `username`: Username to list starred repositories for. Defaults to the authenticated user. (string, optional) - **star_repository** - Star repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - **unstar_repository** - Unstar repository - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) @@ -1530,7 +1530,7 @@ The following sets of tools are available: people Users - **search_users** - Search users - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `order`: Sort order (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 2eb1d35743..13e38a8b6f 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -219,21 +219,10 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) { // Tool name (no icon - section header already has the toolset icon) fmt.Fprintf(buf, "- **%s** - %s\n", tool.Tool.Name, tool.Tool.Annotations.Title) - // OAuth scopes if present - if len(tool.RequiredScopes) > 0 { - scopeList := "`" + strings.Join(tool.RequiredScopes, "`, `") + "`" - switch { - case len(tool.RequiredScopeGroups) > 1: - fmt.Fprintf(buf, " - **Required OAuth Scopes (all required)**: %s\n", scopeList) - case len(tool.RequiredScopes) > 1: - fmt.Fprintf(buf, " - **Required OAuth Scopes (any of)**: %s\n", scopeList) - default: - fmt.Fprintf(buf, " - **Required OAuth Scopes**: %s\n", scopeList) - } - - // Only show accepted scopes if they differ from required scopes - if len(tool.AcceptedScopes) > 0 && !scopesEqual(tool.RequiredScopes, tool.AcceptedScopes) { - fmt.Fprintf(buf, " - **Accepted OAuth Scopes**: `%s`\n", strings.Join(tool.AcceptedScopes, "`, `")) + if policy := formatScopePolicy(tool.ScopePolicy); policy != "" { + fmt.Fprintf(buf, " - **OAuth Scope Policy**: %s\n", policy) + if challenge := preferredChallengeScopes(tool.ScopePolicy); scopePolicyNeedsChallengeDetail(tool.ScopePolicy) && len(challenge) > 0 { + fmt.Fprintf(buf, " - **Preferred OAuth Challenge**: `%s`\n", strings.Join(challenge, "`, `")) } } @@ -322,26 +311,57 @@ func schemaTypeString(schema *jsonschema.Schema) string { return strings.Join(types, " | ") } -// scopesEqual checks if two scope slices contain the same elements (order-independent) -func scopesEqual(a, b []string) bool { - if len(a) != len(b) { - return false +func formatScopePolicy(policy inventory.ScopePolicy) string { + paths := make([]string, 0, len(policy.AnyOf)) + for _, path := range policy.AnyOf { + requirements := make([]string, 0, len(path.AllOf)) + for _, requirement := range path.AllOf { + alternatives := requirement.AnyOf + if len(alternatives) == 0 && requirement.ChallengeScope != "" { + alternatives = []string{requirement.ChallengeScope} + } + quoted := make([]string, len(alternatives)) + for i, scope := range alternatives { + quoted[i] = "`" + scope + "`" + } + if len(quoted) > 1 { + requirements = append(requirements, "("+strings.Join(quoted, " OR ")+")") + } else if len(quoted) == 1 { + requirements = append(requirements, quoted[0]) + } + } + if len(requirements) > 0 { + paths = append(paths, strings.Join(requirements, " AND ")) + } } + return strings.Join(paths, " OR ") +} - // Create a map for quick lookup - aMap := make(map[string]bool, len(a)) - for _, scope := range a { - aMap[scope] = true +func preferredChallengeScopes(policy inventory.ScopePolicy) []string { + if len(policy.AnyOf) == 0 { + return nil } - - // Check if all elements in b are in a - for _, scope := range b { - if !aMap[scope] { - return false + result := make([]string, 0, len(policy.AnyOf[0].AllOf)) + for _, requirement := range policy.AnyOf[0].AllOf { + if requirement.ChallengeScope != "" && !slices.Contains(result, requirement.ChallengeScope) { + result = append(result, requirement.ChallengeScope) } } + return result +} - return true +func scopePolicyNeedsChallengeDetail(policy inventory.ScopePolicy) bool { + if len(policy.AnyOf) > 1 { + return true + } + for _, path := range policy.AnyOf { + for _, requirement := range path.AllOf { + if len(requirement.AnyOf) != 1 || requirement.AnyOf[0] != requirement.ChallengeScope { + return true + } + } + } + return false } // indentMultilineDescription adds the specified indent to all lines after the first line. diff --git a/cmd/github-mcp-server/list_scopes.go b/cmd/github-mcp-server/list_scopes.go index d8b8bf3922..8b35059aa6 100644 --- a/cmd/github-mcp-server/list_scopes.go +++ b/cmd/github-mcp-server/list_scopes.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "slices" "sort" "strings" @@ -17,11 +18,11 @@ import ( // ToolScopeInfo contains scope information for a single tool. type ToolScopeInfo struct { - Name string `json:"name"` - Toolset string `json:"toolset"` - ReadOnly bool `json:"read_only"` - RequiredScopes []string `json:"required_scopes"` - AcceptedScopes []string `json:"accepted_scopes,omitempty"` + Name string `json:"name"` + Toolset string `json:"toolset"` + ReadOnly bool `json:"read_only"` + ScopePolicy inventory.ScopePolicy `json:"scope_policy"` + ChallengeScopes []string `json:"challenge_scopes,omitempty"` } // ScopesOutput is the full output structure for the list-scopes command. @@ -36,12 +37,12 @@ type ScopesOutput struct { var listScopesCmd = &cobra.Command{ Use: "list-scopes", - Short: "List required OAuth scopes for enabled tools", - Long: `List the required OAuth scopes for all enabled tools. + Short: "List OAuth scope policies for enabled tools", + Long: `List the OAuth scope policies for all enabled tools. This command creates an inventory based on the same flags as the stdio command -and outputs the required OAuth scopes for each enabled tool. This is useful for -determining what scopes a token needs to use specific tools. +and outputs the authorization paths and preferred challenge scopes for each +enabled tool. The output format can be controlled with the --output flag: - text (default): Human-readable text output @@ -153,30 +154,28 @@ func collectToolScopes(inv *inventory.Inventory, readOnly bool) ScopesOutput { for _, serverTool := range availableTools { tool := serverTool.Tool - // Get scope information directly from ServerTool - requiredScopes := serverTool.RequiredScopes - acceptedScopes := serverTool.AcceptedScopes + challengeScopes := allChallengeScopes(serverTool.ScopePolicy) // Determine if tool is read-only isReadOnly := serverTool.IsReadOnly() toolInfo := ToolScopeInfo{ - Name: tool.Name, - Toolset: string(serverTool.Toolset.ID), - ReadOnly: isReadOnly, - RequiredScopes: requiredScopes, - AcceptedScopes: acceptedScopes, + Name: tool.Name, + Toolset: string(serverTool.Toolset.ID), + ReadOnly: isReadOnly, + ScopePolicy: serverTool.ScopePolicy, + ChallengeScopes: challengeScopes, } tools = append(tools, toolInfo) // Track unique scopes - for _, s := range requiredScopes { + for _, s := range challengeScopes { scopeSet[s] = true toolsByScope[s] = append(toolsByScope[s], tool.Name) } // Track scopes by tool - scopesByTool[tool.Name] = requiredScopes + scopesByTool[tool.Name] = challengeScopes } // Sort tools by name @@ -225,7 +224,7 @@ func outputSummary(output ScopesOutput) error { return nil } - fmt.Println("Required OAuth scopes for enabled tools:") + fmt.Println("OAuth scope policies for enabled tools:") fmt.Println() for _, scope := range output.UniqueScopes { fmt.Printf(" %s\n", formatScopeDisplay(scope)) @@ -235,8 +234,8 @@ func outputSummary(output ScopesOutput) error { } func outputText(output ScopesOutput) error { - fmt.Printf("OAuth Scopes for Enabled Tools\n") - fmt.Printf("==============================\n\n") + fmt.Printf("OAuth Scope Policies for Enabled Tools\n") + fmt.Printf("======================================\n\n") fmt.Printf("Enabled Toolsets: %s\n", strings.Join(output.EnabledToolsets, ", ")) fmt.Printf("Read-Only Mode: %v\n\n", output.ReadOnly) @@ -265,8 +264,8 @@ func outputText(output ScopesOutput) error { } scopeStr := "(no scope required)" - if len(tool.RequiredScopes) > 0 { - scopeStr = strings.Join(tool.RequiredScopes, ", ") + if policy := formatScopePolicy(tool.ScopePolicy); policy != "" { + scopeStr = policy } fmt.Printf(" %s %s: %s\n", rwIndicator, tool.Name, scopeStr) @@ -278,9 +277,9 @@ func outputText(output ScopesOutput) error { fmt.Println("## Summary") fmt.Println() if len(output.UniqueScopes) == 0 { - fmt.Println("No OAuth scopes required for enabled tools.") + fmt.Println("No OAuth scopes are used by enabled tools.") } else { - fmt.Println("Unique scopes required:") + fmt.Println("Unique preferred challenge scopes:") for _, scope := range output.UniqueScopes { fmt.Printf(" • %s\n", formatScopeDisplay(scope)) } @@ -292,3 +291,15 @@ func outputText(output ScopesOutput) error { return nil } + +func allChallengeScopes(policy inventory.ScopePolicy) []string { + var result []string + for _, path := range policy.AnyOf { + for _, requirement := range path.AllOf { + if requirement.ChallengeScope != "" && !slices.Contains(result, requirement.ChallengeScope) { + result = append(result, requirement.ChallengeScope) + } + } + } + return result +} diff --git a/cmd/github-mcp-server/main_test.go b/cmd/github-mcp-server/main_test.go index ad927701a2..43411e3394 100644 --- a/cmd/github-mcp-server/main_test.go +++ b/cmd/github-mcp-server/main_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/spf13/viper" @@ -63,21 +64,20 @@ func TestWriteToolDocScopeSemantics(t *testing.T) { want string }{ { - name: "legacy multi-scope tools use any-of", + name: "alternative scope paths", tool: inventory.ServerTool{ - Tool: mcp.Tool{Name: "legacy", Annotations: &mcp.ToolAnnotations{Title: "Legacy"}}, - RequiredScopes: []string{"repo", "read:org"}, + Tool: mcp.Tool{Name: "alternative", Annotations: &mcp.ToolAnnotations{Title: "Alternative"}}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.Repo, scopes.ReadOrg), }, - want: "**Required OAuth Scopes (any of)**", + want: "`repo` OR (`admin:org` OR `read:org` OR `write:org`)", }, { - name: "conjunctive scope groups use all-required", + name: "conjunctive requirements", tool: inventory.ServerTool{ - Tool: mcp.Tool{Name: "conjunctive", Annotations: &mcp.ToolAnnotations{Title: "Conjunctive"}}, - RequiredScopes: []string{"delete_repo", "repo"}, - RequiredScopeGroups: [][]string{{"delete_repo"}, {"repo"}}, + Tool: mcp.Tool{Name: "conjunctive", Annotations: &mcp.ToolAnnotations{Title: "Conjunctive"}}, + ScopePolicy: scopes.AllOfScopePolicy(scopes.DeleteRepo, scopes.Repo), }, - want: "**Required OAuth Scopes (all required)**", + want: "`delete_repo` AND `repo`", }, } diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 0de5bdd722..b3c0d4b80d 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -35,7 +35,7 @@ runtime behavior (such as output formatting) won't appear here. ### `remote_mcp_ui_apps` - **create_pull_request** - Open new pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - **MCP App UI**: `ui://github-mcp-server/pr-write` - `base`: Branch to merge into (string, required) - `body`: PR description (string, optional) @@ -52,7 +52,7 @@ runtime behavior (such as output formatting) won't appear here. - No parameters required - **issue_write** - Create or update issue/pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - **MCP App UI**: `ui://github-mcp-server/issue-write` - `assignees`: Usernames to assign to this issue (string[], optional) - `body`: Issue body content (string, optional) @@ -74,14 +74,14 @@ runtime behavior (such as output formatting) won't appear here. - `type`: Type of this issue. For updates, pass null to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - **ui_get** - Get UI data - - **Required OAuth Scopes (any of)**: `repo`, `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` + - **OAuth Scope Policy**: `repo` OR (`admin:org` OR `read:org` OR `write:org`) + - **Preferred OAuth Challenge**: `repo` - `method`: The type of data to fetch (string, required) - `owner`: Repository owner (required for all methods) (string, required) - `repo`: Repository name (required for labels, assignees, milestones, branches, issue fields, reviewers) (string, optional) - **update_pull_request** - Edit pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - **MCP App UI**: `ui://github-mcp-server/pr-edit` - `base`: New base branch name (string, optional) - `body`: New description (string, optional) @@ -97,21 +97,21 @@ runtime behavior (such as output formatting) won't appear here. ### `issues_granular` - **add_issue_comment_reaction** - Add Reaction to Issue or Pull Request Comment - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `comment_id`: The issue or pull request comment ID (number, required) - `content`: The emoji reaction type (string, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **add_issue_reaction** - Add Reaction to Issue or Pull Request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `content`: The emoji reaction type (string, required) - `issue_number`: The issue number (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **add_sub_issue** - Add Sub-Issue - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The parent issue number (number, required) - `owner`: Repository owner (username or organization) (string, required) - `replace_parent`: If true, reparent the sub-issue if it already has a parent (boolean, optional) @@ -119,21 +119,21 @@ runtime behavior (such as output formatting) won't appear here. - `sub_issue_id`: The ID of the sub-issue to add. ID is not the same as issue number (number, required) - **create_issue** - Create Issue - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: Issue body content (optional) (string, optional) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - `title`: Issue title (string, required) - **remove_sub_issue** - Remove Sub-Issue - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The parent issue number (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - `sub_issue_id`: The ID of the sub-issue to remove. ID is not the same as issue number (number, required) - **reprioritize_sub_issue** - Reprioritize Sub-Issue - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after_id`: The ID of the sub-issue to place this after (either after_id OR before_id should be specified) (number, optional) - `before_id`: The ID of the sub-issue to place this before (either after_id OR before_id should be specified) (number, optional) - `issue_number`: The parent issue number (number, required) @@ -142,42 +142,42 @@ runtime behavior (such as output formatting) won't appear here. - `sub_issue_id`: The ID of the sub-issue to reorder. ID is not the same as issue number (number, required) - **set_issue_fields** - Set Issue Fields - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `fields`: Array of issue field values to set. Each element must have a 'field_id' (string, the GraphQL node ID of the field) and exactly one value field: 'text_value' for text fields, 'number_value' for number fields, 'date_value' (ISO 8601 date string) for date fields, or 'single_select_option_id' (the GraphQL node ID of the option) for single select fields. Set 'delete' to true to remove a field value. (object[], required) - `issue_number`: The issue number to update (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **update_issue_assignees** - Update Issue Assignees - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `assignees`: GitHub usernames to assign to this issue. ([], required) - `issue_number`: The issue number to update (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **update_issue_body** - Update Issue Body - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: The new body content for the issue (string, required) - `issue_number`: The issue number to update (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **update_issue_labels** - Update Issue Labels - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The issue number to update (number, required) - `labels`: Labels to apply to this issue. ([], required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **update_issue_milestone** - Update Issue Milestone - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The issue number to update (number, required) - `milestone`: The milestone number to set on the issue (integer, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **update_issue_state** - Update Issue State - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `confidence`: How confident you are in this choice. Use 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, optional) - `duplicate_of`: The issue number of the canonical issue this issue duplicates. Only valid when state_reason is 'duplicate'. Required when is_suggestion is true and state_reason is 'duplicate'. The issue number is resolved to a database ID before being sent to the API. (number, optional) - `is_suggestion`: If true, this state change is sent to the API as a suggestion (suggest:true) rather than an applied change. Whether the change is applied or recorded as a proposal is determined by the API. (boolean, optional) @@ -189,14 +189,14 @@ runtime behavior (such as output formatting) won't appear here. - `state_reason`: The reason for the state change (only for closed state) (string, optional) - **update_issue_title** - Update Issue Title - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The issue number to update (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - `title`: The new title for the issue (string, required) - **update_issue_type** - Update Issue Type - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `confidence`: How confident you are in this choice. Use 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, optional) - `is_suggestion`: If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the type is applied or recorded as a proposal is determined by the API. (boolean, optional) - `issue_number`: The issue number to update (number, required) @@ -208,7 +208,7 @@ runtime behavior (such as output formatting) won't appear here. ### `pull_requests_granular` - **add_pull_request_review_comment** - Add Pull Request Review Comment - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: The comment body (string, required) - `line`: The line number in the diff to comment on (optional) (number, optional) - `owner`: Repository owner (username or organization) (string, required) @@ -221,14 +221,14 @@ runtime behavior (such as output formatting) won't appear here. - `subjectType`: The subject type of the comment (string, required) - **add_pull_request_review_comment_reaction** - Add Pull Request Review Comment Reaction - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `comment_id`: The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...). (number, required) - `content`: The emoji reaction type (string, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **create_pull_request_review** - Create Pull Request Review - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: The review body text (optional) (string, optional) - `commitID`: The SHA of the commit to review (optional, defaults to latest) (string, optional) - `event`: The review action to perform. If omitted, creates a pending review. (string, optional) @@ -237,24 +237,24 @@ runtime behavior (such as output formatting) won't appear here. - `repo`: Repository name (string, required) - **delete_pending_pull_request_review** - Delete Pending Pull Request Review - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (username or organization) (string, required) - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) - **request_pull_request_reviewers** - Request Pull Request Reviewers - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (username or organization) (string, required) - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], required) - **resolve_review_thread** - Resolve Review Thread - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `threadID`: The node ID of the review thread to resolve (e.g., PRRT_kwDOxxx) (string, required) - **submit_pending_pull_request_review** - Submit Pending Pull Request Review - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: The review body text (optional) (string, optional) - `event`: The review action to perform (string, required) - `owner`: Repository owner (username or organization) (string, required) @@ -262,32 +262,32 @@ runtime behavior (such as output formatting) won't appear here. - `repo`: Repository name (string, required) - **unresolve_review_thread** - Unresolve Review Thread - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `threadID`: The node ID of the review thread to unresolve (e.g., PRRT_kwDOxxx) (string, required) - **update_pull_request_body** - Update Pull Request Body - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `body`: The new body content for the pull request (string, required) - `owner`: Repository owner (username or organization) (string, required) - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) - **update_pull_request_draft_state** - Update Pull Request Draft State - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `draft`: Set to true to convert to draft, false to mark as ready for review (boolean, required) - `owner`: Repository owner (username or organization) (string, required) - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) - **update_pull_request_state** - Update Pull Request State - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (username or organization) (string, required) - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) - `state`: The new state for the pull request (string, required) - **update_pull_request_title** - Update Pull Request Title - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `owner`: Repository owner (username or organization) (string, required) - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) @@ -296,7 +296,7 @@ runtime behavior (such as output formatting) won't appear here. ### `file_blame` - **get_file_blame** - Get file blame information - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) - `end_line`: Optional 1-based ending line of the window of interest. Must be >= start_line when both are provided. (number, optional) - `owner`: Repository owner (username or organization) (string, required) @@ -309,7 +309,7 @@ runtime behavior (such as output formatting) won't appear here. ### `issue_dependencies` - **issue_dependency_read** - Read issue dependencies - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The number of the issue (number, required) - `method`: The read operation to perform on a single issue's dependencies. Options are: @@ -322,7 +322,7 @@ runtime behavior (such as output formatting) won't appear here. - `repo`: The name of the repository (string, required) - **issue_dependency_write** - Change issue dependency - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The number of the subject issue (number, required) - `method`: The action to perform. Options are: @@ -341,7 +341,7 @@ runtime behavior (such as output formatting) won't appear here. ### `duplicate_detection` - **find_duplicate** - Find duplicate issues - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `confidence_threshold`: Minimum similarity threshold a candidate must meet to be returned; higher values are stricter. When omitted, the API's high-precision default is used. The scale is defined by the API, so no client-side bounds are enforced. (number, optional) - `issue_number`: The number of the existing issue to find duplicates for (number, required) - `owner`: The owner of the repository (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 350522bf5e..303a9fe8bb 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -29,7 +29,7 @@ The list below is generated from the Go source. It covers tool **inventory and s ### `remote_mcp_ui_apps` - **create_pull_request** - Open new pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - **MCP App UI**: `ui://github-mcp-server/pr-write` - `base`: Branch to merge into (string, required) - `body`: PR description (string, optional) @@ -46,7 +46,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - No parameters required - **issue_write** - Create or update issue/pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - **MCP App UI**: `ui://github-mcp-server/issue-write` - `assignees`: Usernames to assign to this issue (string[], optional) - `body`: Issue body content (string, optional) @@ -68,14 +68,14 @@ The list below is generated from the Go source. It covers tool **inventory and s - `type`: Type of this issue. For updates, pass null to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - **ui_get** - Get UI data - - **Required OAuth Scopes (any of)**: `repo`, `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` + - **OAuth Scope Policy**: `repo` OR (`admin:org` OR `read:org` OR `write:org`) + - **Preferred OAuth Challenge**: `repo` - `method`: The type of data to fetch (string, required) - `owner`: Repository owner (required for all methods) (string, required) - `repo`: Repository name (required for labels, assignees, milestones, branches, issue fields, reviewers) (string, optional) - **update_pull_request** - Edit pull request - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - **MCP App UI**: `ui://github-mcp-server/pr-edit` - `base`: New base branch name (string, optional) - `body`: New description (string, optional) @@ -91,7 +91,7 @@ The list below is generated from the Go source. It covers tool **inventory and s ### `file_blame` - **get_file_blame** - Get file blame information - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) - `end_line`: Optional 1-based ending line of the window of interest. Must be >= start_line when both are provided. (number, optional) - `owner`: Repository owner (username or organization) (string, required) @@ -104,7 +104,7 @@ The list below is generated from the Go source. It covers tool **inventory and s ### `issue_dependencies` - **issue_dependency_read** - Read issue dependencies - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The number of the issue (number, required) - `method`: The read operation to perform on a single issue's dependencies. Options are: @@ -117,7 +117,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `repo`: The name of the repository (string, required) - **issue_dependency_write** - Change issue dependency - - **Required OAuth Scopes**: `repo` + - **OAuth Scope Policy**: `repo` - `issue_number`: The number of the subject issue (number, required) - `method`: The action to perform. Options are: diff --git a/docs/scope-filtering.md b/docs/scope-filtering.md index f29d631ca1..6f55d0111f 100644 --- a/docs/scope-filtering.md +++ b/docs/scope-filtering.md @@ -22,6 +22,19 @@ When the server starts with a classic PAT, it makes a lightweight HTTP HEAD requ With OAuth, the remote server can dynamically request additional scopes as needed. With PATs, scopes are fixed at token creation, so the server proactively hides tools you can't use. +## Scope Policies + +Tool scopes are modeled as authorization paths rather than a flat list: + +- A tool may support multiple paths, and satisfying any path is sufficient. +- Every independent requirement within the selected path must be satisfied. +- A requirement may accept multiple alternative scopes, including broader scopes from the hierarchy. +- Argument-dependent tools resolve the exact path at call time before an OAuth challenge is issued. + +For example, repository deletion requires `repo` **and** `delete_repo`, while listing issue fields requires `repo` for a repository request or `read:org` for an organization request. Updating an ordinary file requires `repo`; updating a workflow file requires both `repo` and `workflow`. + +PAT filtering runs before call arguments are known, so it keeps a tool when any supported path is usable. OAuth challenges resolve the actual arguments and request only the preferred missing scopes for that call. + ## OAuth Scope Challenges (Remote Server) When using the [remote MCP server](./remote-server.md) with OAuth authentication, the server uses a different approach called **scope challenges**. Instead of hiding tools upfront, all tools are available, and the server requests additional scopes on-demand when you try to use a tool that requires them. @@ -58,7 +71,7 @@ Some scopes implicitly include others: This means if your token has `repo`, tools requiring `security_events` will also be available. -Each tool in the [README](../README.md#tools) lists its required and accepted OAuth scopes. +Each tool in the [README](../README.md#tools) lists its OAuth scope policy and preferred challenge scopes. ## Public Repository Access diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 0a1db9d387..b72502c1d2 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -323,7 +323,7 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an Required: []string{"method", "owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -453,7 +453,7 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an Required: []string{"method", "owner", "repo", "resource_id"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -581,7 +581,7 @@ func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerToo Required: []string{"method", "owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -693,7 +693,7 @@ For single job logs, provide job_id. For all failed jobs in a run, provide run_i Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/code_quality.go b/pkg/github/code_quality.go index 41c791182b..b16ac90d78 100644 --- a/pkg/github/code_quality.go +++ b/pkg/github/code_quality.go @@ -46,7 +46,7 @@ func GetCodeQualityFinding(t translations.TranslationHelperFunc) inventory.Serve Required: []string{"owner", "repo", "findingNumber"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/code_scanning.go b/pkg/github/code_scanning.go index 74a1c7e0fa..d9834858cd 100644 --- a/pkg/github/code_scanning.go +++ b/pkg/github/code_scanning.go @@ -46,7 +46,7 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server Required: []string{"owner", "repo", "alertNumber"}, }, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { @@ -146,7 +146,7 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv }, InputSchema: schema, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index b4c7098c56..aa521487e6 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -62,7 +62,7 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool { }, }, }, - nil, + scopes.UnscopedScopePolicy(), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, _ map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -143,7 +143,7 @@ func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool { }, }, }, - []scopes.Scope{scopes.ReadOrg}, + scopes.AnyOfScopePolicy(scopes.ReadOrg), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { user, err := OptionalParam[string](args, "user") if err != nil { @@ -251,7 +251,7 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"org", "team_slug"}, }, }, - []scopes.Scope{scopes.ReadOrg}, + scopes.AnyOfScopePolicy(scopes.ReadOrg), 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 { diff --git a/pkg/github/copilot.go b/pkg/github/copilot.go index 4b3225e221..80d4fe7d57 100644 --- a/pkg/github/copilot.go +++ b/pkg/github/copilot.go @@ -201,7 +201,7 @@ func AssignCopilotToIssue(t translations.TranslationHelperFunc) inventory.Server Required: []string{"owner", "repo", "issue_number"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, request *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { var params struct { Owner string `mapstructure:"owner"` @@ -565,7 +565,7 @@ func AssignCopilotToIssueWithIntent(t translations.TranslationHelperFunc) invent Required: []string{"owner", "repo", "issue_number", "rationale", "confidence", "is_suggestion"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, request *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { // Presence-check is_suggestion before decoding: mapstructure defaults a // missing bool to false, which would silently launch Copilot instead of @@ -867,7 +867,7 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/dependabot.go b/pkg/github/dependabot.go index f56a33134a..18baf5c867 100644 --- a/pkg/github/dependabot.go +++ b/pkg/github/dependabot.go @@ -47,7 +47,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo Required: []string{"owner", "repo", "alertNumber"}, }, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { @@ -139,7 +139,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server }, InputSchema: schema, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index d52b5f4aeb..81f4aabc85 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -15,7 +15,6 @@ import ( "github.com/github/github-mcp-server/pkg/observability" "github.com/github/github-mcp-server/pkg/observability/metrics" "github.com/github/github-mcp-server/pkg/raw" - "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" "github.com/github/github-mcp-server/pkg/utils" gogithub "github.com/google/go-github/v89/github" @@ -230,21 +229,18 @@ func (d BaseDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool { // The handler function receives deps extracted from context via MustDepsFromContext. // Ensure ContextWithDeps is called to inject deps before any tool handlers are invoked. // -// requiredScopes specifies the minimum OAuth scopes needed for this tool. -// AcceptedScopes are automatically derived using the scope hierarchy (e.g., if -// public_repo is required, repo is also accepted since repo grants public_repo). +// scopePolicy describes every OAuth scope combination that can authorize this tool. func NewTool[In, Out any]( toolset inventory.ToolsetMetadata, tool mcp.Tool, - requiredScopes []scopes.Scope, + scopePolicy inventory.ScopePolicy, handler func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args In) (*mcp.CallToolResult, Out, error), ) inventory.ServerTool { st := inventory.NewServerToolWithContextHandler(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest, args In) (*mcp.CallToolResult, Out, error) { deps := MustDepsFromContext(ctx) return handler(ctx, deps, req, args) }) - st.RequiredScopes = scopes.ToStringSlice(requiredScopes...) - st.AcceptedScopes = scopes.ExpandScopes(requiredScopes...) + st.ScopePolicy = scopePolicy return st } @@ -254,20 +250,18 @@ func NewTool[In, Out any]( // The handler function receives deps extracted from context via MustDepsFromContext. // Ensure ContextWithDeps is called to inject deps before any tool handlers are invoked. // -// requiredScopes specifies the minimum OAuth scopes needed for this tool. -// AcceptedScopes are automatically derived using the scope hierarchy. +// scopePolicy describes every OAuth scope combination that can authorize this tool. func NewToolFromHandler( toolset inventory.ToolsetMetadata, tool mcp.Tool, - requiredScopes []scopes.Scope, + scopePolicy inventory.ScopePolicy, handler func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest) (*mcp.CallToolResult, error), ) inventory.ServerTool { st := inventory.NewServerTool(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { deps := MustDepsFromContext(ctx) return handler(ctx, deps, req) }) - st.RequiredScopes = scopes.ToStringSlice(requiredScopes...) - st.AcceptedScopes = scopes.ExpandScopes(requiredScopes...) + st.ScopePolicy = scopePolicy return st } diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 9d9d02f82a..68e7356799 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -165,7 +165,7 @@ func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"owner"}, }), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -312,7 +312,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "discussionNumber"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { // Decode params var params struct { @@ -424,7 +424,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve Required: []string{"owner", "repo", "discussionNumber"}, }), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { // Decode params var params struct { @@ -639,7 +639,7 @@ Options are: Required: []string{"method"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1015,7 +1015,7 @@ func ListDiscussionCategories(t translations.TranslationHelperFunc) inventory.Se Required: []string{"owner"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index 0b73ddeb3b..d98fdfdfb4 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -41,7 +41,7 @@ func HelloWorldTool(t translations.TranslationHelperFunc) inventory.ServerTool { ReadOnlyHint: true, }, }, - []scopes.Scope{}, + scopes.UnscopedScopePolicy(), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, _ map[string]any) (*mcp.CallToolResult, any, error) { // Check feature flag to determine greeting style diff --git a/pkg/github/find_duplicate.go b/pkg/github/find_duplicate.go index e32179ce8d..38185693ba 100644 --- a/pkg/github/find_duplicate.go +++ b/pkg/github/find_duplicate.go @@ -83,7 +83,7 @@ func FindDuplicate(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/find_duplicate_test.go b/pkg/github/find_duplicate_test.go index be5c3dda81..59cbcd908e 100644 --- a/pkg/github/find_duplicate_test.go +++ b/pkg/github/find_duplicate_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/jsonschema-go/jsonschema" "github.com/stretchr/testify/assert" @@ -26,7 +27,7 @@ func Test_FindDuplicate(t *testing.T) { assert.Equal(t, "find_duplicate", tool.Name) assert.NotEmpty(t, tool.Description) assert.True(t, tool.Annotations.ReadOnlyHint) - assert.ElementsMatch(t, serverTool.RequiredScopes, []string{"repo"}) + assert.Equal(t, scopes.AnyOfScopePolicy(scopes.Repo), serverTool.ScopePolicy) schema := tool.InputSchema.(*jsonschema.Schema) assert.Contains(t, schema.Properties, "owner") diff --git a/pkg/github/gists.go b/pkg/github/gists.go index 0ea883ec7d..29ac108dfc 100644 --- a/pkg/github/gists.go +++ b/pkg/github/gists.go @@ -43,7 +43,7 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool { }, }), }, - nil, + scopes.UnscopedScopePolicy(), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { username, err := OptionalParam[string](args, "username") if err != nil { @@ -129,7 +129,7 @@ func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"gist_id"}, }, }, - nil, + scopes.UnscopedScopePolicy(), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { gistID, err := RequiredParam[string](args, "gist_id") if err != nil { @@ -202,7 +202,7 @@ func CreateGist(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"filename", "content"}, }, }, - []scopes.Scope{scopes.Gist}, + scopes.AnyOfScopePolicy(scopes.Gist), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { description, err := OptionalParam[string](args, "description") if err != nil { @@ -303,7 +303,7 @@ func UpdateGist(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"gist_id", "filename", "content"}, }, }, - []scopes.Scope{scopes.Gist}, + scopes.AnyOfScopePolicy(scopes.Gist), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { gistID, err := RequiredParam[string](args, "gist_id") if err != nil { diff --git a/pkg/github/git.go b/pkg/github/git.go index 5b0c9b79b2..0445e2ce3d 100644 --- a/pkg/github/git.go +++ b/pkg/github/git.go @@ -78,7 +78,7 @@ func GetRepositoryTree(t translations.TranslationHelperFunc) inventory.ServerToo Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/header_params_test.go b/pkg/github/header_params_test.go index ed08f27a79..cbd0771ca4 100644 --- a/pkg/github/header_params_test.go +++ b/pkg/github/header_params_test.go @@ -29,6 +29,10 @@ func TestAllToolsRoutingParamsGetHeaders(t *testing.T) { if !ok || schema == nil { continue } + if pathSchema := schema.Properties["path"]; pathSchema != nil { + require.NotContainsf(t, pathSchema.Extra, "x-mcp-header", + "tool %q path must remain in MCP arguments", tool.Name) + } for prop, header := range inventory.HeaderParams { ps, ok := schema.Properties[prop] if !ok || ps == nil { diff --git a/pkg/github/issue_dependencies.go b/pkg/github/issue_dependencies.go index d0b596a66b..2c37f571af 100644 --- a/pkg/github/issue_dependencies.go +++ b/pkg/github/issue_dependencies.go @@ -62,7 +62,7 @@ Options are: }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -249,7 +249,7 @@ Options are: Required: []string{"method", "type", "owner", "repo", "issue_number", "related_issue_number"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/issue_fields.go b/pkg/github/issue_fields.go index e06bdbfcf9..cd1798d71a 100644 --- a/pkg/github/issue_fields.go +++ b/pkg/github/issue_fields.go @@ -129,7 +129,7 @@ func ListIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"owner"}, }, }, - []scopes.Scope{scopes.Repo, scopes.ReadOrg}, + scopes.AnyOfScopePolicy(scopes.Repo, scopes.ReadOrg), 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 { @@ -167,6 +167,7 @@ func ListIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool } return result, nil, nil }) + st.ScopeResolver = repositoryOrOrganizationScopePolicy return st } diff --git a/pkg/github/issue_fields_test.go b/pkg/github/issue_fields_test.go index 2c2b26ee2a..e79abf8302 100644 --- a/pkg/github/issue_fields_test.go +++ b/pkg/github/issue_fields_test.go @@ -7,6 +7,7 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/jsonschema-go/jsonschema" "github.com/shurcooL/githubv4" @@ -26,8 +27,7 @@ func Test_ListIssueFields(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "owner") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "repo") assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"owner"}) - assert.ElementsMatch(t, serverTool.RequiredScopes, []string{"repo", "read:org"}) - assert.ElementsMatch(t, serverTool.AcceptedScopes, []string{"repo", "read:org", "write:org", "admin:org"}) + assert.Equal(t, scopes.AnyOfScopePolicy(scopes.Repo, scopes.ReadOrg), serverTool.ScopePolicy) queryStruct := issueFieldsRepoQuery{} defaultVars := map[string]any{ diff --git a/pkg/github/issues.go b/pkg/github/issues.go index d6cc55e1ed..b549292e31 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -831,7 +831,7 @@ func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1256,7 +1256,7 @@ func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, // ListIssueTypes creates a tool to list defined issue types for an organization or repository. // This can be used to understand supported issue type values for creating or updating issues. func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "list_issue_types", @@ -1280,7 +1280,7 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner"}, }, }, - []scopes.Scope{scopes.Repo, scopes.ReadOrg}, + scopes.AnyOfScopePolicy(scopes.Repo, scopes.ReadOrg), 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 { @@ -1354,6 +1354,8 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoMetadata(true)) return result, nil, nil }) + st.ScopeResolver = repositoryOrOrganizationScopePolicy + return st } // AddIssueComment creates a tool to add a comment or reaction to an issue. @@ -1401,7 +1403,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"owner", "repo", "issue_number"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1620,7 +1622,7 @@ func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"method", "owner", "repo", "issue_number", "sub_issue_id"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1882,7 +1884,7 @@ func SearchIssues(t translations.TranslationHelperFunc, opts ...ToolOption) inve }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { options := []searchOption{ifcSearchPostProcessOption(ctx, deps)} fields, err := OptionalStringArrayParam(args, "fields") @@ -2511,7 +2513,7 @@ Options are: Required: []string{"method", "owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { method, err := RequiredParam[string](args, "method") if err != nil { @@ -3044,7 +3046,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index fb5ff32242..7bb269ce7f 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -67,7 +67,7 @@ func issueUpdateTool( Required: required, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -148,7 +148,7 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT Required: []string{"owner", "repo", "title"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -304,7 +304,7 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor Required: []string{"owner", "repo", "issue_number", "assignees"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -522,7 +522,7 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S Required: []string{"owner", "repo", "issue_number", "labels"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -735,7 +735,7 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser Required: []string{"owner", "repo", "issue_number", "issue_type"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -909,7 +909,7 @@ func GranularUpdateIssueState(t translations.TranslationHelperFunc) inventory.Se Required: []string{"owner", "repo", "issue_number", "state"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1067,7 +1067,7 @@ func GranularAddSubIssue(t translations.TranslationHelperFunc) inventory.ServerT Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1137,7 +1137,7 @@ func GranularRemoveSubIssue(t translations.TranslationHelperFunc) inventory.Serv Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1214,7 +1214,7 @@ func GranularReprioritizeSubIssue(t translations.TranslationHelperFunc) inventor Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1379,7 +1379,7 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv Required: []string{"owner", "repo", "issue_number", "fields"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1580,7 +1580,7 @@ func GranularAddIssueReaction(t translations.TranslationHelperFunc) inventory.Se Required: []string{"owner", "repo", "issue_number", "content"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1662,7 +1662,7 @@ func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inven Required: []string{"owner", "repo", "comment_id", "content"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/labels.go b/pkg/github/labels.go index b8ea92f892..f02adb3964 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -52,7 +52,7 @@ func GetLabel(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "name"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -154,7 +154,7 @@ func ListLabels(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -274,7 +274,7 @@ func LabelWrite(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"method", "owner", "repo", "name"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { // Get and validate required parameters method, err := RequiredParam[string](args, "method") diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index cd71a6c9d8..1c3f6b31ef 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -63,7 +63,7 @@ func ListNotifications(t translations.TranslationHelperFunc) inventory.ServerToo }, }), }, - []scopes.Scope{scopes.Notifications}, + scopes.AnyOfScopePolicy(scopes.Notifications), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -189,7 +189,7 @@ func DismissNotification(t translations.TranslationHelperFunc) inventory.ServerT Required: []string{"threadID", "state"}, }, }, - []scopes.Scope{scopes.Notifications}, + scopes.AnyOfScopePolicy(scopes.Notifications), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -267,7 +267,7 @@ func MarkAllNotificationsRead(t translations.TranslationHelperFunc) inventory.Se }, }, }, - []scopes.Scope{scopes.Notifications}, + scopes.AnyOfScopePolicy(scopes.Notifications), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -352,7 +352,7 @@ func GetNotificationDetails(t translations.TranslationHelperFunc) inventory.Serv Required: []string{"notificationID"}, }, }, - []scopes.Scope{scopes.Notifications}, + scopes.AnyOfScopePolicy(scopes.Notifications), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -433,7 +433,7 @@ func ManageNotificationSubscription(t translations.TranslationHelperFunc) invent Required: []string{"notificationID", "action"}, }, }, - []scopes.Scope{scopes.Notifications}, + scopes.AnyOfScopePolicy(scopes.Notifications), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -534,7 +534,7 @@ func ManageRepositoryNotificationSubscription(t translations.TranslationHelperFu Required: []string{"owner", "repo", "action"}, }, }, - []scopes.Scope{scopes.Notifications}, + scopes.AnyOfScopePolicy(scopes.Notifications), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 57a5dc145b..a9c9edf472 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -357,7 +357,7 @@ Use this tool to list projects for a user or organization, or list project field Required: []string{"method", "owner"}, }, }, - []scopes.Scope{scopes.ReadProject}, + scopes.AnyOfScopePolicy(scopes.ReadProject), 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 { @@ -521,7 +521,7 @@ Use this tool to get details about individual projects, project fields, project Required: []string{"method"}, }, }, - []scopes.Scope{scopes.ReadProject}, + scopes.AnyOfScopePolicy(scopes.ReadProject), 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 { @@ -891,7 +891,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"method", "owner"}, }, }, - []scopes.Scope{scopes.Project}, + scopes.AnyOfScopePolicy(scopes.Project), 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 { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 8801ec2894..925ed4a4df 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -78,7 +78,7 @@ Possible options: }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -706,7 +706,7 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo Required: []string{"owner", "repo", "title", "head", "base"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { owner, err := RequiredParam[string](args, "owner") if err != nil { @@ -923,7 +923,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { owner, err := RequiredParam[string](args, "owner") if err != nil { @@ -1216,7 +1216,7 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1377,7 +1377,7 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1529,7 +1529,7 @@ func MergePullRequest(t translations.TranslationHelperFunc) inventory.ServerTool }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1651,7 +1651,7 @@ func SearchPullRequests(t translations.TranslationHelperFunc) inventory.ServerTo }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { options := []searchOption{ifcSearchPostProcessOption(ctx, deps)} fields, err := OptionalStringArrayParam(args, "fields") @@ -1700,7 +1700,7 @@ func UpdatePullRequestBranch(t translations.TranslationHelperFunc) inventory.Ser }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1834,7 +1834,7 @@ Available methods: }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { var params PullRequestReviewWriteParams if err := mapstructure.WeakDecode(args, ¶ms); err != nil { @@ -2327,7 +2327,7 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index c727beb6e5..80b3aa2daf 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -62,7 +62,7 @@ func prUpdateTool( Required: required, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -195,7 +195,7 @@ func GranularUpdatePullRequestDraftState(t translations.TranslationHelperFunc) i Required: []string{"owner", "repo", "pullNumber", "draft"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -304,7 +304,7 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i Required: []string{"owner", "repo", "pullNumber", "reviewers"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -397,7 +397,7 @@ func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inven Required: []string{"owner", "repo", "pullNumber"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -465,7 +465,7 @@ func GranularSubmitPendingPullRequestReview(t translations.TranslationHelperFunc Required: []string{"owner", "repo", "pullNumber", "event"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -527,7 +527,7 @@ func GranularDeletePendingPullRequestReview(t translations.TranslationHelperFunc Required: []string{"owner", "repo", "pullNumber"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -589,7 +589,7 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i Required: []string{"owner", "repo", "pullNumber", "path", "body", "subjectType"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -694,7 +694,7 @@ func GranularResolveReviewThread(t translations.TranslationHelperFunc) inventory Required: []string{"threadID"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { threadID, err := RequiredParam[string](args, "threadID") if err != nil { @@ -738,7 +738,7 @@ func GranularUnresolveReviewThread(t translations.TranslationHelperFunc) invento Required: []string{"threadID"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { threadID, err := RequiredParam[string](args, "threadID") if err != nil { @@ -796,7 +796,7 @@ func GranularAddPullRequestReviewCommentReaction(t translations.TranslationHelpe Required: []string{"owner", "repo", "comment_id", "content"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 446d7ca551..66767040d4 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -61,7 +61,7 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "sha"}, }), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -188,7 +188,7 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -337,7 +337,7 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo"}, }), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -405,7 +405,7 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { // CreateOrUpdateFile creates a tool to create or update a file in a GitHub repository. func CreateOrUpdateFile(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + tool := NewTool( ToolsetMetadataRepos, mcp.Tool{ Name: "create_or_update_file", @@ -461,7 +461,7 @@ SHA MUST be provided for existing file updates. Required: []string{"owner", "repo", "path", "content", "message", "branch"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -475,6 +475,10 @@ SHA MUST be provided for existing file updates. if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + path, err = validateRelativePath(path) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("invalid path: %s", err)), nil, nil + } content, err := RequiredParam[string](args, "content") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil @@ -518,8 +522,6 @@ SHA MUST be provided for existing file updates. return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - path = strings.TrimPrefix(path, "/") - // SHA validation using Contents API to fetch current file metadata (blob SHA) getOpts := &github.RepositoryContentGetOptions{Ref: branch} @@ -623,6 +625,8 @@ SHA MUST be provided for existing file updates. return MarshalledTextResult(minimalResponse), nil, nil }, ) + tool.ScopeResolver = workflowScopePolicyForPath + return tool } // CreateRepository creates a tool to create a new GitHub repository. @@ -664,7 +668,7 @@ func CreateRepository(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"name"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { name, err := RequiredParam[string](args, "name") if err != nil { @@ -774,7 +778,7 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.DeleteRepo, scopes.Repo}, + scopes.AllOfScopePolicy(scopes.DeleteRepo, scopes.Repo), func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { owner, err := RequiredParam[string](args, "owner") if err != nil { @@ -906,7 +910,6 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool ) tool.MinimumProtocolVersion = inventory.ProtocolVersionMultiRoundTrip tool.RequiredElicitationMode = inventory.ElicitationModeForm - tool.RequiredScopeGroups = scopes.ExpandScopeGroups(scopes.DeleteRepo, scopes.Repo) return tool } @@ -985,7 +988,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1222,7 +1225,7 @@ func ForkRepository(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1292,7 +1295,7 @@ func ForkRepository(t translations.TranslationHelperFunc) inventory.ServerTool { // The approach implemented here gets automatic commit signing when used with either the github-actions user or as an app, // both of which suit an LLM well. func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + tool := NewTool( ToolsetMetadataRepos, mcp.Tool{ Name: "delete_file", @@ -1329,7 +1332,7 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "path", "message", "branch"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1343,6 +1346,10 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + path, err = validateRelativePath(path) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("invalid path: %s", err)), nil, nil + } message, err := RequiredParam[string](args, "message") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil @@ -1473,6 +1480,8 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultText(string(r)), nil, nil }, ) + tool.ScopeResolver = workflowScopePolicyForPath + return tool } // CreateBranch creates a tool to create a new branch. @@ -1509,7 +1518,7 @@ func CreateBranch(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "branch"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1590,7 +1599,7 @@ func CreateBranch(t translations.TranslationHelperFunc) inventory.ServerTool { // PushFiles creates a tool to push multiple files in a single commit to a GitHub repository. func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + tool := NewTool( ToolsetMetadataRepos, mcp.Tool{ Name: "push_files", @@ -1641,7 +1650,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "branch", "files", "message"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1666,6 +1675,35 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError("files parameter must be an array of objects with path and content"), nil, nil } + entries := make([]*github.TreeEntry, 0, len(filesObj)) + for _, file := range filesObj { + fileMap, ok := file.(map[string]any) + if !ok { + return utils.NewToolResultError("each file must be an object with path and content"), nil, nil + } + + filePath, ok := fileMap["path"].(string) + if !ok || filePath == "" { + return utils.NewToolResultError("each file must have a path"), nil, nil + } + filePath, err = validateRelativePath(filePath) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("invalid file path: %s", err)), nil, nil + } + + content, ok := fileMap["content"].(string) + if !ok { + return utils.NewToolResultError("each file must have content"), nil, nil + } + + entries = append(entries, &github.TreeEntry{ + Path: github.Ptr(filePath), + Mode: github.Ptr("100644"), + Type: github.Ptr("blob"), + Content: github.Ptr(content), + }) + } + client, err := deps.GetClient(ctx) if err != nil { return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) @@ -1739,34 +1777,6 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool { baseCommit = base } - // Create tree entries for all files (or remaining files if empty repo) - var entries []*github.TreeEntry - - for _, file := range filesObj { - fileMap, ok := file.(map[string]any) - if !ok { - return utils.NewToolResultError("each file must be an object with path and content"), nil, nil - } - - path, ok := fileMap["path"].(string) - if !ok || path == "" { - return utils.NewToolResultError("each file must have a path"), nil, nil - } - - content, ok := fileMap["content"].(string) - if !ok { - return utils.NewToolResultError("each file must have content"), nil, nil - } - - // Create a tree entry for the file - entries = append(entries, &github.TreeEntry{ - Path: github.Ptr(path), - Mode: github.Ptr("100644"), // Regular file mode - Type: github.Ptr("blob"), - Content: github.Ptr(content), - }) - } - // Create a new tree with the file entries (baseCommit is now guaranteed to exist) newTree, resp, err := client.Git.CreateTree(ctx, owner, repo, *baseCommit.Tree.SHA, entries) if err != nil { @@ -1821,6 +1831,8 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultText(string(r)), nil, nil }, ) + tool.ScopeResolver = workflowScopePolicyForFiles + return tool } // ListTags creates a tool to list tags in a GitHub repository. @@ -1849,7 +1861,7 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo"}, }), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -1944,7 +1956,7 @@ func GetTag(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "tag"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -2060,7 +2072,7 @@ func ListReleases(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -2176,7 +2188,7 @@ func GetLatestRelease(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -2254,7 +2266,7 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"owner", "repo", "tag"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -2342,7 +2354,7 @@ func ListStarredRepositories(t translations.TranslationHelperFunc) inventory.Ser }, }), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { username, err := OptionalParam[string](args, "username") if err != nil { @@ -2482,7 +2494,7 @@ func StarRepository(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -2547,7 +2559,7 @@ func UnstarRepository(t translations.TranslationHelperFunc) inventory.ServerTool Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -2754,7 +2766,7 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"owner", "repo", "path"}, }), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { @@ -3043,7 +3055,7 @@ func ListRepositoryCollaborators(t translations.TranslationHelperFunc) inventory return schema }(), }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(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 { diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index e45c3c4f47..9ea49165c1 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -3599,8 +3599,7 @@ func Test_DeleteRepository(t *testing.T) { assert.True(t, *tool.Annotations.DestructiveHint) assert.Equal(t, inventory.ProtocolVersionMultiRoundTrip, serverTool.MinimumProtocolVersion) assert.Equal(t, inventory.ElicitationModeForm, serverTool.RequiredElicitationMode) - assert.ElementsMatch(t, []string{string(scopes.DeleteRepo), string(scopes.Repo)}, serverTool.RequiredScopes) - assert.Len(t, serverTool.RequiredScopeGroups, 2) + assert.Equal(t, scopes.AllOfScopePolicy(scopes.DeleteRepo, scopes.Repo), serverTool.ScopePolicy) t.Run("requests exact repository name through elicitation", func(t *testing.T) { client := NewMockedHTTPClient( diff --git a/pkg/github/repository_path.go b/pkg/github/repository_path.go new file mode 100644 index 0000000000..7cd5541f3f --- /dev/null +++ b/pkg/github/repository_path.go @@ -0,0 +1,79 @@ +package github + +import ( + "fmt" + "path" + "slices" + "strings" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" +) + +const workflowPathPrefix = ".github/workflows/" + +func validateRelativePath(value string) (string, error) { + value = strings.TrimPrefix(value, "/") + if value == "" { + return "", fmt.Errorf("path must not be empty") + } + if path.IsAbs(value) { + return "", fmt.Errorf("path must be relative") + } + if strings.Contains(value, `\`) { + return "", fmt.Errorf("path must use forward slashes") + } + if slices.Contains(strings.Split(value, "/"), "..") { + return "", fmt.Errorf("path must not contain parent directory traversal") + } + + cleaned := path.Clean(value) + if cleaned == "." { + return "", fmt.Errorf("path must identify a file") + } + return cleaned, nil +} + +func isWorkflowPath(value string) bool { + return strings.HasPrefix(value, workflowPathPrefix) && len(value) > len(workflowPathPrefix) +} + +func workflowScopePolicyForPath(arguments map[string]any) inventory.ScopePolicy { + value, ok := arguments["path"].(string) + if !ok { + return scopes.UnscopedScopePolicy() + } + cleaned, err := validateRelativePath(value) + if err != nil { + return scopes.UnscopedScopePolicy() + } + if !isWorkflowPath(cleaned) { + return scopes.AllOfScopePolicy(scopes.Repo) + } + return scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow) +} + +func workflowScopePolicyForFiles(arguments map[string]any) inventory.ScopePolicy { + files, ok := arguments["files"].([]any) + if !ok { + return scopes.UnscopedScopePolicy() + } + for _, file := range files { + fileMap, ok := file.(map[string]any) + if !ok { + return scopes.UnscopedScopePolicy() + } + value, ok := fileMap["path"].(string) + if !ok { + return scopes.UnscopedScopePolicy() + } + cleaned, err := validateRelativePath(value) + if err != nil { + return scopes.UnscopedScopePolicy() + } + if isWorkflowPath(cleaned) { + return scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow) + } + } + return scopes.AllOfScopePolicy(scopes.Repo) +} diff --git a/pkg/github/repository_path_test.go b/pkg/github/repository_path_test.go new file mode 100644 index 0000000000..7f52e17fa4 --- /dev/null +++ b/pkg/github/repository_path_test.go @@ -0,0 +1,145 @@ +package github + +import ( + "context" + "testing" + + "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/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateRelativePath(t *testing.T) { + tests := []struct { + name string + value string + want string + wantErr string + }{ + {name: "file", value: "docs/readme.md", want: "docs/readme.md"}, + {name: "normalizes leading slash", value: "/docs/readme.md", want: "docs/readme.md"}, + {name: "normalizes dot segment", value: "./.github/workflows/ci.yml", want: ".github/workflows/ci.yml"}, + {name: "normalizes duplicate separator", value: ".github//workflows/ci.yml", want: ".github/workflows/ci.yml"}, + {name: "empty", value: "", wantErr: "must not be empty"}, + {name: "current directory", value: ".", wantErr: "must identify a file"}, + {name: "double leading slash", value: "//.github/workflows/ci.yml", wantErr: "must be relative"}, + {name: "parent traversal", value: "docs/../.github/workflows/ci.yml", wantErr: "parent directory traversal"}, + {name: "leading traversal", value: "../.github/workflows/ci.yml", wantErr: "parent directory traversal"}, + {name: "backslash traversal", value: `docs\..\.github\workflows\ci.yml`, wantErr: "forward slashes"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := validateRelativePath(tt.value) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestFileWriteWorkflowScopeResolvers(t *testing.T) { + tests := []struct { + name string + tool inventory.ServerTool + args map[string]any + want inventory.ScopePolicy + }{ + { + name: "create regular file", + tool: CreateOrUpdateFile(translations.NullTranslationHelper), + args: map[string]any{"path": "docs/readme.md"}, + want: scopes.AllOfScopePolicy(scopes.Repo), + }, + { + name: "create workflow", + tool: CreateOrUpdateFile(translations.NullTranslationHelper), + args: map[string]any{"path": ".github/workflows/ci.yml"}, + want: scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow), + }, + { + name: "delete normalized workflow", + tool: DeleteFile(translations.NullTranslationHelper), + args: map[string]any{"path": "./.github/workflows/ci.yml"}, + want: scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow), + }, + { + name: "reject traversal instead of resolving it", + tool: DeleteFile(translations.NullTranslationHelper), + args: map[string]any{"path": "docs/../.github/workflows/ci.yml"}, + want: scopes.UnscopedScopePolicy(), + }, + { + name: "push regular files", + tool: PushFiles(translations.NullTranslationHelper), + args: map[string]any{"files": []any{map[string]any{"path": "README.md"}}}, + want: scopes.AllOfScopePolicy(scopes.Repo), + }, + { + name: "push includes workflow", + tool: PushFiles(translations.NullTranslationHelper), + args: map[string]any{"files": []any{ + map[string]any{"path": "README.md"}, + map[string]any{"path": ".github/workflows/ci.yml"}, + }}, + want: scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.NotNil(t, tt.tool.ScopeResolver) + assert.Equal(t, tt.want, tt.tool.ScopeResolver(tt.args)) + }) + } +} + +func TestFileWriteToolsRejectUnsafePathsBeforeAPICalls(t *testing.T) { + tests := []struct { + name string + tool inventory.ServerTool + args map[string]any + }{ + { + name: "create or update", + tool: CreateOrUpdateFile(translations.NullTranslationHelper), + args: map[string]any{ + "owner": "owner", "repo": "repo", "path": "../workflow.yml", + "content": "content", "message": "message", "branch": "main", + }, + }, + { + name: "delete", + tool: DeleteFile(translations.NullTranslationHelper), + args: map[string]any{ + "owner": "owner", "repo": "repo", "path": "//.github/workflows/ci.yml", + "message": "message", "branch": "main", + }, + }, + { + name: "push", + tool: PushFiles(translations.NullTranslationHelper), + args: map[string]any{ + "owner": "owner", "repo": "repo", "branch": "main", "message": "message", + "files": []any{map[string]any{"path": `..\workflow.yml`, "content": "content"}}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + deps := BaseDeps{} + request := createMCPRequest(tt.args) + result, err := tt.tool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "path") + }) + } +} diff --git a/pkg/github/scope_filter.go b/pkg/github/scope_filter.go index add0ee3b18..029fdcdce0 100644 --- a/pkg/github/scope_filter.go +++ b/pkg/github/scope_filter.go @@ -15,19 +15,35 @@ var repoScopesSet = map[string]bool{ string(scopes.PublicRepo): true, } -// onlyRequiresRepoScopes returns true if all of the tool's accepted scopes -// are repo-related scopes (repo, public_repo). Such tools work on public -// repositories without needing any scope. -func onlyRequiresRepoScopes(acceptedScopes []string) bool { - if len(acceptedScopes) == 0 { - return false - } - for _, scope := range acceptedScopes { - if !repoScopesSet[scope] { - return false +// hasRepoOnlyScopeAlternative returns true if at least one authorization path +// uses only repo-related scopes. Such a path may work on a public repository +// without any token scope. +func hasRepoOnlyScopeAlternative(policy inventory.ScopePolicy) bool { + for _, alternative := range policy.AnyOf { + if len(alternative.AllOf) == 0 { + continue + } + repoOnly := true + for _, requirement := range alternative.AllOf { + accepted := requirement.AnyOf + if len(accepted) == 0 { + accepted = []string{requirement.ChallengeScope} + } + for _, scope := range accepted { + if !repoScopesSet[scope] { + repoOnly = false + break + } + } + if !repoOnly { + break + } + } + if repoOnly { + return true } } - return true + return false } // CreateToolScopeFilter creates an inventory.ToolFilter that filters tools @@ -41,9 +57,9 @@ func onlyRequiresRepoScopes(acceptedScopes []string) bool { // token is known at startup and won't change during the session. // // The filter returns true (include tool) if: -// - The tool has no scope requirements (AcceptedScopes is empty) +// - The tool has an unscoped authorization path // - The tool is read-only and only requires repo/public_repo scopes (works on public repos) -// - The token has at least one of the tool's accepted scopes +// - The token satisfies every requirement in any authorization path // // Example usage: // @@ -55,13 +71,11 @@ func onlyRequiresRepoScopes(acceptedScopes []string) bool { // inventory := github.NewInventory(t).WithFilter(filter).Build() func CreateToolScopeFilter(tokenScopes []string) inventory.ToolFilter { return func(_ context.Context, tool *inventory.ServerTool) (bool, error) { + policy := tool.ScopePolicy // Read-only tools requiring only repo/public_repo work on public repos without any scope - if tool.Tool.Annotations != nil && tool.Tool.Annotations.ReadOnlyHint && onlyRequiresRepoScopes(tool.AcceptedScopes) { + if tool.Tool.Annotations != nil && tool.Tool.Annotations.ReadOnlyHint && hasRepoOnlyScopeAlternative(policy) { return true, nil } - if len(tool.RequiredScopeGroups) > 0 { - return scopes.HasRequiredScopeGroups(tokenScopes, tool.RequiredScopeGroups), nil - } - return scopes.HasRequiredScopes(tokenScopes, tool.AcceptedScopes), nil + return scopes.ScopePolicySatisfied(tokenScopes, policy), nil } } diff --git a/pkg/github/scope_filter_test.go b/pkg/github/scope_filter_test.go index 934a848a2d..19cda810e9 100644 --- a/pkg/github/scope_filter_test.go +++ b/pkg/github/scope_filter_test.go @@ -5,6 +5,7 @@ import ( "testing" "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/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" @@ -12,20 +13,18 @@ import ( ) func TestCreateToolScopeFilter(t *testing.T) { - // Create test tools with various scope requirements toolNoScopes := &inventory.ServerTool{ - Tool: mcp.Tool{Name: "no_scopes_tool"}, - AcceptedScopes: nil, + Tool: mcp.Tool{Name: "no_scopes_tool"}, } toolEmptyScopes := &inventory.ServerTool{ - Tool: mcp.Tool{Name: "empty_scopes_tool"}, - AcceptedScopes: []string{}, + Tool: mcp.Tool{Name: "empty_scopes_tool"}, + ScopePolicy: scopes.UnscopedScopePolicy(), } toolRepoScope := &inventory.ServerTool{ - Tool: mcp.Tool{Name: "repo_tool"}, - AcceptedScopes: []string{"repo"}, + Tool: mcp.Tool{Name: "repo_tool"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.Repo), } toolRepoScopeReadOnly := &inventory.ServerTool{ @@ -33,12 +32,12 @@ func TestCreateToolScopeFilter(t *testing.T) { Name: "repo_tool_readonly", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, }, - AcceptedScopes: []string{"repo"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.Repo), } toolPublicRepoScope := &inventory.ServerTool{ - Tool: mcp.Tool{Name: "public_repo_tool"}, - AcceptedScopes: []string{"public_repo", "repo"}, // repo is parent, also accepted + Tool: mcp.Tool{Name: "public_repo_tool"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.PublicRepo), } toolPublicRepoScopeReadOnly := &inventory.ServerTool{ @@ -46,23 +45,22 @@ func TestCreateToolScopeFilter(t *testing.T) { Name: "public_repo_tool_readonly", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, }, - AcceptedScopes: []string{"public_repo", "repo"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.PublicRepo), } toolGistScope := &inventory.ServerTool{ - Tool: mcp.Tool{Name: "gist_tool"}, - AcceptedScopes: []string{"gist"}, + Tool: mcp.Tool{Name: "gist_tool"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.Gist), } toolMultiScope := &inventory.ServerTool{ - Tool: mcp.Tool{Name: "multi_scope_tool"}, - AcceptedScopes: []string{"repo", "admin:org"}, + Tool: mcp.Tool{Name: "multi_scope_tool"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.Repo, scopes.AdminOrg), } toolConjunctiveScopes := &inventory.ServerTool{ - Tool: mcp.Tool{Name: "conjunctive_scope_tool"}, - AcceptedScopes: []string{"delete_repo", "repo"}, - RequiredScopeGroups: [][]string{{"delete_repo"}, {"repo"}}, + Tool: mcp.Tool{Name: "conjunctive_scope_tool"}, + ScopePolicy: scopes.AllOfScopePolicy(scopes.DeleteRepo, scopes.Repo), } tests := []struct { @@ -166,19 +164,19 @@ func TestCreateToolScopeFilter_Integration(t *testing.T) { // Test integration with inventory builder tools := []inventory.ServerTool{ { - Tool: mcp.Tool{Name: "public_tool"}, - Toolset: inventory.ToolsetMetadata{ID: "test"}, - AcceptedScopes: nil, // No scopes required + Tool: mcp.Tool{Name: "public_tool"}, + Toolset: inventory.ToolsetMetadata{ID: "test"}, + ScopePolicy: scopes.UnscopedScopePolicy(), }, { - Tool: mcp.Tool{Name: "repo_tool"}, - Toolset: inventory.ToolsetMetadata{ID: "test"}, - AcceptedScopes: []string{"repo"}, + Tool: mcp.Tool{Name: "repo_tool"}, + Toolset: inventory.ToolsetMetadata{ID: "test"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.Repo), }, { - Tool: mcp.Tool{Name: "gist_tool"}, - Toolset: inventory.ToolsetMetadata{ID: "test"}, - AcceptedScopes: []string{"gist"}, + Tool: mcp.Tool{Name: "gist_tool"}, + Toolset: inventory.ToolsetMetadata{ID: "test"}, + ScopePolicy: scopes.AnyOfScopePolicy(scopes.Gist), }, } @@ -209,8 +207,9 @@ func TestCreateToolScopeFilter_Integration(t *testing.T) { assert.NotContains(t, toolNames, "gist_tool") } -func TestCreateToolScopeFilterPreservesExistingMultiScopeSemantics(t *testing.T) { +func TestCreateToolScopeFilterSupportsAlternativePaths(t *testing.T) { filter := CreateToolScopeFilter([]string{"repo"}) + unscopedFilter := CreateToolScopeFilter(nil) tools := []inventory.ServerTool{ ListIssueFields(translations.NullTranslationHelper), ListIssueTypes(translations.NullTranslationHelper), @@ -220,7 +219,10 @@ func TestCreateToolScopeFilterPreservesExistingMultiScopeSemantics(t *testing.T) for i := range tools { allowed, err := filter(context.Background(), &tools[i]) require.NoError(t, err) - assert.True(t, allowed, "%s should remain visible with a repo-only token", tools[i].Tool.Name) - assert.Empty(t, tools[i].RequiredScopeGroups, "%s should retain legacy any-of scope semantics", tools[i].Tool.Name) + assert.True(t, allowed, "%s should be visible with a repo-only token", tools[i].Tool.Name) + + allowed, err = unscopedFilter(context.Background(), &tools[i]) + require.NoError(t, err) + assert.True(t, allowed, "%s should remain visible when a public-repository path is plausible", tools[i].Tool.Name) } } diff --git a/pkg/github/search.go b/pkg/github/search.go index 3160209318..2b295615c8 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -58,7 +58,7 @@ func SearchRepositories(t translations.TranslationHelperFunc) inventory.ServerTo }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { query, err := RequiredParam[string](args, "query") if err != nil { @@ -229,7 +229,7 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { query, err := RequiredParam[string](args, "query") if err != nil { @@ -474,7 +474,7 @@ func SearchUsers(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { return userOrOrgHandler(ctx, "user", deps, args) }, @@ -516,7 +516,7 @@ func SearchOrgs(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.ReadOrg}, + scopes.AnyOfScopePolicy(scopes.ReadOrg), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { return userOrOrgHandler(ctx, "org", deps, args) }, @@ -558,7 +558,7 @@ func SearchCommits(t translations.TranslationHelperFunc) inventory.ServerTool { }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.AnyOfScopePolicy(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { query, err := RequiredParam[string](args, "query") if err != nil { diff --git a/pkg/github/secret_scanning.go b/pkg/github/secret_scanning.go index de444faea7..285ddc31c3 100644 --- a/pkg/github/secret_scanning.go +++ b/pkg/github/secret_scanning.go @@ -47,7 +47,7 @@ func GetSecretScanningAlert(t translations.TranslationHelperFunc) inventory.Serv Required: []string{"owner", "repo", "alertNumber"}, }, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { @@ -142,7 +142,7 @@ func ListSecretScanningAlerts(t translations.TranslationHelperFunc) inventory.Se }, InputSchema: schema, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { diff --git a/pkg/github/security_advisories.go b/pkg/github/security_advisories.go index 4f718de767..4e796f66f0 100644 --- a/pkg/github/security_advisories.go +++ b/pkg/github/security_advisories.go @@ -85,7 +85,7 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor }, }, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -254,7 +254,7 @@ func ListRepositorySecurityAdvisories(t translations.TranslationHelperFunc) inve Required: []string{"owner", "repo"}, }, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { @@ -349,7 +349,7 @@ func GetGlobalSecurityAdvisory(t translations.TranslationHelperFunc) inventory.S Required: []string{"ghsaId"}, }, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { client, err := deps.GetClient(ctx) if err != nil { @@ -425,7 +425,7 @@ func ListOrgRepositorySecurityAdvisories(t translations.TranslationHelperFunc) i Required: []string{"org"}, }, }, - []scopes.Scope{scopes.SecurityEvents}, + scopes.AnyOfScopePolicy(scopes.SecurityEvents), 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 { diff --git a/pkg/github/tool_scopes.go b/pkg/github/tool_scopes.go new file mode 100644 index 0000000000..68e38d31b2 --- /dev/null +++ b/pkg/github/tool_scopes.go @@ -0,0 +1,44 @@ +package github + +import ( + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" +) + +func repositoryOrOrganizationScopePolicy(arguments map[string]any) inventory.ScopePolicy { + if owner, ok := arguments["owner"].(string); !ok || owner == "" { + return scopes.UnscopedScopePolicy() + } + repoValue, hasRepo := arguments["repo"] + if !hasRepo || repoValue == nil || repoValue == "" { + return scopes.AllOfScopePolicy(scopes.ReadOrg) + } + repo, ok := repoValue.(string) + if !ok { + return scopes.UnscopedScopePolicy() + } + if repo != "" { + return scopes.AllOfScopePolicy(scopes.Repo) + } + return scopes.AllOfScopePolicy(scopes.ReadOrg) +} + +func uiGetScopePolicy(arguments map[string]any) inventory.ScopePolicy { + if owner, ok := arguments["owner"].(string); !ok || owner == "" { + return scopes.UnscopedScopePolicy() + } + method, ok := arguments["method"].(string) + if !ok { + return scopes.UnscopedScopePolicy() + } + if method == "issue_types" { + return scopes.AllOfScopePolicy(scopes.ReadOrg) + } + switch method { + case "labels", "assignees", "milestones", "branches", "issue_fields", "reviewers": + if repo, ok := arguments["repo"].(string); ok && repo != "" { + return scopes.AllOfScopePolicy(scopes.Repo) + } + } + return scopes.UnscopedScopePolicy() +} diff --git a/pkg/github/tool_scopes_test.go b/pkg/github/tool_scopes_test.go new file mode 100644 index 0000000000..37688eb6f6 --- /dev/null +++ b/pkg/github/tool_scopes_test.go @@ -0,0 +1,73 @@ +package github + +import ( + "testing" + + "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/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConditionalToolScopePolicies(t *testing.T) { + tests := []struct { + name string + tool inventory.ServerTool + arguments map[string]any + allowed []string + disallowed []string + }{ + { + name: "issue fields repository route", + tool: ListIssueFields(translations.NullTranslationHelper), + arguments: map[string]any{"owner": "octo", "repo": "repo"}, + allowed: []string{"repo"}, + disallowed: []string{"read:org"}, + }, + { + name: "issue fields organization route", + tool: ListIssueFields(translations.NullTranslationHelper), + arguments: map[string]any{"owner": "octo"}, + allowed: []string{"read:org"}, + disallowed: []string{"repo"}, + }, + { + name: "issue types repository route", + tool: ListIssueTypes(translations.NullTranslationHelper), + arguments: map[string]any{"owner": "octo", "repo": "repo"}, + allowed: []string{"repo"}, + disallowed: []string{"read:org"}, + }, + { + name: "issue types organization route", + tool: ListIssueTypes(translations.NullTranslationHelper), + arguments: map[string]any{"owner": "octo"}, + allowed: []string{"admin:org"}, + disallowed: []string{"repo"}, + }, + { + name: "ui repository method", + tool: UIGet(translations.NullTranslationHelper), + arguments: map[string]any{"method": "labels", "owner": "octo", "repo": "repo"}, + allowed: []string{"repo"}, + disallowed: []string{"read:org"}, + }, + { + name: "ui organization method", + tool: UIGet(translations.NullTranslationHelper), + arguments: map[string]any{"method": "issue_types", "owner": "octo"}, + allowed: []string{"write:org"}, + disallowed: []string{"repo"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.NotNil(t, tt.tool.ScopeResolver) + policy := tt.tool.ScopeResolver(tt.arguments) + assert.True(t, scopes.ScopePolicySatisfied(tt.allowed, policy)) + assert.False(t, scopes.ScopePolicySatisfied(tt.disallowed, policy)) + }) + } +} diff --git a/pkg/github/ui_tools.go b/pkg/github/ui_tools.go index 3fa4afcb15..136da45b67 100644 --- a/pkg/github/ui_tools.go +++ b/pkg/github/ui_tools.go @@ -68,7 +68,7 @@ func UIGet(t translations.TranslationHelperFunc) inventory.ServerTool { Required: []string{"method", "owner"}, }, }, - []scopes.Scope{scopes.Repo, scopes.ReadOrg}, + scopes.AnyOfScopePolicy(scopes.Repo, 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 { @@ -100,6 +100,7 @@ func UIGet(t translations.TranslationHelperFunc) inventory.ServerTool { } }) st.FeatureFlagEnable = MCPAppsFeatureFlag + st.ScopeResolver = uiGetScopePolicy return st } diff --git a/pkg/http/middleware/scope_challenge.go b/pkg/http/middleware/scope_challenge.go index 4ac3bab906..a74e0561c7 100644 --- a/pkg/http/middleware/scope_challenge.go +++ b/pkg/http/middleware/scope_challenge.go @@ -43,6 +43,7 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter // Try to use pre-parsed MCP method info first (performance optimization) // This avoids re-parsing the JSON body if WithMCPParse middleware ran earlier var toolName string + var arguments map[string]any if methodInfo, ok := ghcontext.MCPMethod(ctx); ok && methodInfo != nil { // Only check tools/call requests if methodInfo.Method != "tools/call" { @@ -50,6 +51,7 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter return } toolName = methodInfo.ItemName + arguments = methodInfo.Arguments } else { // Fallback: parse the request body directly body, err := io.ReadAll(r.Body) @@ -85,6 +87,7 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter } toolName = mcpRequest.Params.Name + arguments = mcpRequest.Params.Arguments } toolScopeInfo, err := scopes.GetToolScopeInfo(toolName) if err != nil { @@ -97,6 +100,7 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter next.ServeHTTP(w, r) return } + toolScopeInfo = toolScopeInfo.Resolve(arguments) // Get OAuth scopes for Token. First check if scopes are already in context, then fetch from GitHub if not present. // This allows Remote Server to pass scope info to avoid redundant GitHub API calls. @@ -114,13 +118,13 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter r = r.WithContext(ctx) // Check if user has the required scopes - if toolScopeInfo.HasAcceptedScope(activeScopes...) { + if toolScopeInfo.Satisfies(activeScopes...) { next.ServeHTTP(w, r) return } // User lacks required scopes - get the scopes they need - requiredScopes := toolScopeInfo.GetRequiredScopesSlice() + challengeScopes := toolScopeInfo.ChallengeScopes(activeScopes...) // Build the resource metadata URL using the shared utility // GetEffectiveResourcePath returns the original path (e.g., /mcp or /mcp/x/all) @@ -129,15 +133,15 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter resourceMetadataURL := oauth.BuildResourceMetadataURL(r, oauthCfg, resourcePath) // Build recommended scopes: existing scopes + required scopes - recommendedScopes := make([]string, 0, len(activeScopes)+len(requiredScopes)) + recommendedScopes := make([]string, 0, len(activeScopes)+len(challengeScopes)) recommendedScopes = append(recommendedScopes, activeScopes...) - recommendedScopes = append(recommendedScopes, requiredScopes...) + recommendedScopes = append(recommendedScopes, challengeScopes...) // Build the WWW-Authenticate header value wwwAuthenticateHeader := fmt.Sprintf(`Bearer error="insufficient_scope", scope=%q, resource_metadata=%q, error_description=%q`, strings.Join(recommendedScopes, " "), resourceMetadataURL, - "Additional scopes required: "+strings.Join(requiredScopes, ", "), + "Additional scopes required: "+strings.Join(challengeScopes, ", "), ) // Send scope challenge response with the superset of existing and required scopes diff --git a/pkg/http/middleware/scope_challenge_test.go b/pkg/http/middleware/scope_challenge_test.go index 9ce67bc7cd..41ed564552 100644 --- a/pkg/http/middleware/scope_challenge_test.go +++ b/pkg/http/middleware/scope_challenge_test.go @@ -1,6 +1,8 @@ package middleware import ( + "bytes" + "context" "io" "net/http" "net/http/httptest" @@ -9,6 +11,8 @@ import ( ghcontext "github.com/github/github-mcp-server/pkg/context" "github.com/github/github-mcp-server/pkg/http/oauth" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -82,3 +86,122 @@ func TestWithScopeChallenge_MaxBodySize(t *testing.T) { assert.Equal(t, body, capturedBody, "body should be preserved for downstream handlers") }) } + +func TestWithScopeChallengeResolvesScopesFromParsedArguments(t *testing.T) { + setDynamicScopeTestMap(t) + + tests := []struct { + name string + arguments map[string]any + wantStatus int + wantNext bool + }{ + { + name: "regular file only requires repo", + arguments: map[string]any{"path": "README.md"}, + wantStatus: http.StatusNoContent, + wantNext: true, + }, + { + name: "non-ASCII workflow path without header requires workflow", + arguments: map[string]any{"path": ".github/workflows/构建.yml"}, + wantStatus: http.StatusForbidden, + }, + { + name: "workflow in array requires workflow", + arguments: map[string]any{"files": []any{ + map[string]any{"path": "README.md"}, + map[string]any{"path": ".github/workflows/ci.yml"}, + }}, + wantStatus: http.StatusForbidden, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + nextCalled := false + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + nextCalled = true + w.WriteHeader(http.StatusNoContent) + }) + handler := WithScopeChallenge(&oauth.Config{}, &mockScopeFetcher{})(next) + + request := httptest.NewRequest(http.MethodPost, "/mcp", nil) + assert.Empty(t, request.Header.Get("Mcp-Param-path")) + ctx := scopeChallengeContext(request.Context()) + ctx = ghcontext.WithMCPMethodInfo(ctx, &ghcontext.MCPMethodInfo{ + Method: "tools/call", + ItemName: "write_file", + Arguments: tt.arguments, + }) + request = request.WithContext(ctx) + + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + + assert.Equal(t, tt.wantStatus, response.Code) + assert.Equal(t, tt.wantNext, nextCalled) + if tt.wantStatus == http.StatusForbidden { + challenge := response.Header().Get("WWW-Authenticate") + assert.Contains(t, challenge, `scope="repo workflow"`) + assert.Contains(t, challenge, "Additional scopes required: workflow") + } + }) + } +} + +func TestWithScopeChallengeResolvesScopesFromFallbackBody(t *testing.T) { + setDynamicScopeTestMap(t) + + nextCalled := false + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + nextCalled = true + w.WriteHeader(http.StatusNoContent) + }) + handler := WithScopeChallenge(&oauth.Config{}, &mockScopeFetcher{})(next) + + body := []byte(`{"jsonrpc":"2.0","method":"tools/call","params":{"name":"write_file","arguments":{"path":".github/workflows/ci.yml"}}}`) + request := httptest.NewRequest(http.MethodPost, "/mcp", bytes.NewReader(body)) + request = request.WithContext(scopeChallengeContext(request.Context())) + + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + + assert.Equal(t, http.StatusForbidden, response.Code) + assert.False(t, nextCalled) + assert.Contains(t, response.Header().Get("WWW-Authenticate"), "Additional scopes required: workflow") +} + +func setDynamicScopeTestMap(t *testing.T) { + t.Helper() + scopes.SetGlobalToolScopeMap(scopes.ToolScopeMap{ + "write_file": { + ScopePolicy: scopes.AllOfScopePolicy(scopes.Repo), + ScopeResolver: func(arguments map[string]any) inventory.ScopePolicy { + if path, _ := arguments["path"].(string); strings.HasPrefix(path, ".github/workflows/") { + return scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow) + } + files, _ := arguments["files"].([]any) + for _, file := range files { + fileMap, _ := file.(map[string]any) + if path, _ := fileMap["path"].(string); strings.HasPrefix(path, ".github/workflows/") { + return scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow) + } + } + return scopes.AllOfScopePolicy(scopes.Repo) + }, + }, + }) + t.Cleanup(func() { + scopes.SetGlobalToolScopeMap(nil) + }) +} + +func scopeChallengeContext(ctx context.Context) context.Context { + ctx = ghcontext.WithTokenInfo(ctx, &ghcontext.TokenInfo{ + Token: "oauth-token", + TokenType: utils.TokenTypeOAuthAccessToken, + }) + ctx = ghcontext.WithTokenScopes(ctx, []string{"repo"}) + return ctx +} diff --git a/pkg/inventory/server_tool.go b/pkg/inventory/server_tool.go index 1d8cbcf885..59392756fe 100644 --- a/pkg/inventory/server_tool.go +++ b/pkg/inventory/server_tool.go @@ -19,6 +19,30 @@ import ( // should define their own typed dependencies struct and type-assert as needed. type HandlerFunc func(deps any) mcp.ToolHandler +// ScopeRequirement is one independently required OAuth capability. Any scope +// in AnyOf satisfies it; ChallengeScope is advertised when the requirement is +// not satisfied. +type ScopeRequirement struct { + ChallengeScope string `json:"challenge"` + AnyOf []string `json:"any_of"` +} + +// ScopePath is one complete way to authorize an operation. Every +// requirement in AllOf must be satisfied. +type ScopePath struct { + AllOf []ScopeRequirement `json:"all_of"` +} + +// ScopePolicy describes all supported ways to authorize an operation. Any +// path is sufficient. The first path is the preferred path for +// an OAuth challenge when call-time resolution leaves multiple choices. +type ScopePolicy struct { + AnyOf []ScopePath `json:"any_of"` +} + +// ScopeResolver returns the complete OAuth scope policy for a specific call. +type ScopeResolver func(arguments map[string]any) ScopePolicy + // ToolHandlerMiddleware wraps an MCP tool handler. Middleware is applied from // right to left, so the first middleware passed to RegisterFunc executes first. type ToolHandlerMiddleware func(next mcp.ToolHandler) mcp.ToolHandler @@ -90,18 +114,12 @@ type ServerTool struct { // list or call this tool. Empty means the tool does not require elicitation. RequiredElicitationMode ElicitationMode - // RequiredScopes specifies the minimum OAuth scopes required for this tool. - // These are the scopes that must be present for the tool to function. - RequiredScopes []string - - // AcceptedScopes specifies all OAuth scopes that can be used with this tool. - // This includes the required scopes plus any higher-level scopes that provide - // the necessary permissions due to scope hierarchy. - AcceptedScopes []string + // ScopePolicy describes every scope combination that can authorize this tool. + // It is used for argument-free availability checks. + ScopePolicy ScopePolicy - // RequiredScopeGroups contains one group of accepted alternatives for each - // independently required OAuth scope. Every group must be satisfied. - RequiredScopeGroups [][]string + // ScopeResolver returns the exact policy for a call based on its arguments. + ScopeResolver ScopeResolver } // IsReadOnly returns true if this tool is marked as read-only via annotations. @@ -139,21 +157,20 @@ func (st *ServerTool) RegisterFunc(s *mcp.Server, deps any, middleware ...ToolHa if len(toolCopy.Icons) == 0 { toolCopy.Icons = st.Toolset.Icons() } - // Project routing-relevant params to standard MCP-Param-* headers (SEP-2243) - // so a remote proxy can read owner/repo from headers instead of re-parsing the - // JSON-RPC body. No-op for tools without these params. + // Project owner/repo routing params to standard MCP-Param-* headers (SEP-2243) + // so a remote proxy can route requests without re-parsing the JSON-RPC body. + // No-op for tools without these params. AnnotateHeaderParams(&toolCopy) s.AddTool(&toolCopy, handler) } -// HeaderParams maps tool input properties to the MCP-Param-* header name a -// header-aware proxy reads, avoiding a second parse of the request body. New -// routing-relevant params should be added here so projection stays automatic -// for every tool; the enforcement test in pkg/github guards full coverage. +// HeaderParams maps owner/repo input properties to the MCP-Param-* headers a +// header-aware proxy reads for repository routing. The enforcement test in +// pkg/github guards full coverage. var HeaderParams = map[string]string{"owner": "owner", "repo": "repo"} -// AnnotateHeaderParams returns a copy of tool whose routing-relevant input -// properties (per HeaderParams) carry an "x-mcp-header" annotation, which the +// AnnotateHeaderParams returns a copy of tool whose owner/repo input properties +// carry an "x-mcp-header" annotation, which the // SDK projects onto Mcp-Param-{name} request headers. It never mutates the // input tool's schema or any map shared with the original tool definition: // callers shallow-copy ServerTool.Tool, so the *jsonschema.Schema (and its diff --git a/pkg/inventory/server_tool_test.go b/pkg/inventory/server_tool_test.go index 9e32b5f30c..08bb8fe553 100644 --- a/pkg/inventory/server_tool_test.go +++ b/pkg/inventory/server_tool_test.go @@ -167,6 +167,7 @@ func TestAnnotateHeaderParams(t *testing.T) { Properties: map[string]*jsonschema.Schema{ "owner": {Type: "string"}, "repo": {Type: "string"}, + "path": {Type: "string"}, "detail": {Type: "string"}, }, }} @@ -174,6 +175,7 @@ func TestAnnotateHeaderParams(t *testing.T) { schema := tool.InputSchema.(*jsonschema.Schema) assert.Equal(t, "owner", schema.Properties["owner"].Extra["x-mcp-header"]) assert.Equal(t, "repo", schema.Properties["repo"].Extra["x-mcp-header"]) + assert.Nil(t, schema.Properties["path"].Extra) assert.Nil(t, schema.Properties["detail"].Extra) // No-op for tools without owner/repo and when InputSchema is not a *jsonschema.Schema diff --git a/pkg/scopes/map.go b/pkg/scopes/map.go index e13345f0f7..018e432bc0 100644 --- a/pkg/scopes/map.go +++ b/pkg/scopes/map.go @@ -1,21 +1,19 @@ package scopes -import "github.com/github/github-mcp-server/pkg/inventory" +import ( + "github.com/github/github-mcp-server/pkg/inventory" +) // ToolScopeMap maps tool names to their scope requirements. type ToolScopeMap map[string]*ToolScopeInfo // ToolScopeInfo contains scope information for a single tool. type ToolScopeInfo struct { - // RequiredScopes contains the scopes that are directly required by this tool. - RequiredScopes []string + // Policy describes every supported authorization path for the tool. + ScopePolicy inventory.ScopePolicy - // AcceptedScopes contains all scopes that satisfy the requirements (including parent scopes). - AcceptedScopes []string - - // RequiredScopeGroups contains accepted alternatives for each independently - // required scope. Every group must be satisfied. - RequiredScopeGroups [][]string + // ScopeResolver returns the exact policy for a call. + ScopeResolver inventory.ScopeResolver } // globalToolScopeMap is populated from inventory when SetToolScopeMapFromInventory is called @@ -52,7 +50,6 @@ func GetToolScopeInfo(toolName string) (*ToolScopeInfo, error) { } // GetToolScopeMapFromInventory builds a tool scope map from an inventory. -// This extracts scope information from ServerTool.RequiredScopes and ServerTool.AcceptedScopes. func GetToolScopeMapFromInventory(inv *inventory.Inventory) ToolScopeMap { result := make(ToolScopeMap) @@ -61,11 +58,10 @@ func GetToolScopeMapFromInventory(inv *inventory.Inventory) ToolScopeMap { allTools := inv.AllTools() for i := range allTools { tool := &allTools[i] - if len(tool.RequiredScopes) > 0 || len(tool.AcceptedScopes) > 0 { + if len(tool.ScopePolicy.AnyOf) > 0 || tool.ScopeResolver != nil { result[tool.Tool.Name] = &ToolScopeInfo{ - RequiredScopes: tool.RequiredScopes, - AcceptedScopes: tool.AcceptedScopes, - RequiredScopeGroups: tool.RequiredScopeGroups, + ScopePolicy: tool.ScopePolicy, + ScopeResolver: tool.ScopeResolver, } } } @@ -73,83 +69,29 @@ func GetToolScopeMapFromInventory(inv *inventory.Inventory) ToolScopeMap { return result } -// HasAcceptedScope checks if any of the provided user scopes satisfy the tool's requirements. -func (t *ToolScopeInfo) HasAcceptedScope(userScopes ...string) bool { - if t != nil && len(t.RequiredScopeGroups) > 0 { - return HasRequiredScopeGroups(userScopes, t.RequiredScopeGroups) - } - if t == nil || len(t.AcceptedScopes) == 0 { - return true // No scopes required - } - - userScopeSet := make(map[string]bool) - for _, scope := range userScopes { - userScopeSet[scope] = true +// Resolve returns the scope requirements for a specific call. +func (t *ToolScopeInfo) Resolve(arguments map[string]any) *ToolScopeInfo { + if t == nil || t.ScopeResolver == nil { + return t } - - for _, scope := range t.AcceptedScopes { - if userScopeSet[scope] { - return true - } - } - return false + resolved := *t + resolved.ScopePolicy = t.ScopeResolver(arguments) + return &resolved } -// MissingScopes returns the required scopes that are not present in the user's scopes. -func (t *ToolScopeInfo) MissingScopes(userScopes ...string) []string { - if t == nil || len(t.RequiredScopes) == 0 { - return nil - } - - // Create a set of user scopes for O(1) lookup - userScopeSet := make(map[string]bool, len(userScopes)) - for _, s := range userScopes { - userScopeSet[s] = true - } - - if len(t.RequiredScopeGroups) > 0 { - userScopeSet := expandScopeSet(userScopes) - var missing []string - for i, group := range t.RequiredScopeGroups { - satisfied := false - for _, scope := range group { - if userScopeSet[scope] { - satisfied = true - break - } - } - if !satisfied && i < len(t.RequiredScopes) { - missing = append(missing, t.RequiredScopes[i]) - } - } - return missing - } - - // Check if any accepted scope is present - hasAccepted := false - for _, scope := range t.AcceptedScopes { - if userScopeSet[scope] { - hasAccepted = true - break - } - } - - if hasAccepted { - return nil // User has sufficient scopes +// Satisfies checks whether the provided scopes satisfy the tool policy. +func (t *ToolScopeInfo) Satisfies(userScopes ...string) bool { + if t == nil { + return true } - - // Return required scopes as the minimum needed - missing := make([]string, len(t.RequiredScopes)) - copy(missing, t.RequiredScopes) - return missing + return ScopePolicySatisfied(userScopes, t.ScopePolicy) } -// GetRequiredScopesSlice returns the required scopes as a slice of strings. -func (t *ToolScopeInfo) GetRequiredScopesSlice() []string { +// ChallengeScopes returns the preferred scopes needed for the first declared +// authorization alternative. +func (t *ToolScopeInfo) ChallengeScopes(userScopes ...string) []string { if t == nil { return nil } - scopes := make([]string, len(t.RequiredScopes)) - copy(scopes, t.RequiredScopes) - return scopes + return ChallengeScopesForPolicy(userScopes, t.ScopePolicy) } diff --git a/pkg/scopes/map_test.go b/pkg/scopes/map_test.go index 3c5a7ede72..f58e259283 100644 --- a/pkg/scopes/map_test.go +++ b/pkg/scopes/map_test.go @@ -3,223 +3,65 @@ package scopes import ( "testing" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestGetToolScopeMap(t *testing.T) { - // Reset and set up a test map - SetGlobalToolScopeMap(ToolScopeMap{ - "test_tool": &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - }) - - m, err := GetToolScopeMap() +func TestGetToolScopeMapFromInventory(t *testing.T) { + inv, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{ + {Tool: testTool("scoped"), Toolset: testToolset(), ScopePolicy: AnyOfScopePolicy(ReadOrg)}, + {Tool: testTool("unscoped"), Toolset: testToolset()}, + }). + WithToolsets([]string{"test"}). + Build() require.NoError(t, err) - require.NotNil(t, m) - require.Greater(t, len(m), 0, "expected at least one tool in the scope map") - testTool, ok := m["test_tool"] - require.True(t, ok, "expected test_tool to be in the scope map") - assert.Contains(t, testTool.RequiredScopes, "read:org") - assert.Contains(t, testTool.AcceptedScopes, "read:org") - assert.Contains(t, testTool.AcceptedScopes, "admin:org") + scopeMap := GetToolScopeMapFromInventory(inv) + require.Contains(t, scopeMap, "scoped") + assert.Equal(t, AnyOfScopePolicy(ReadOrg), scopeMap["scoped"].ScopePolicy) + assert.NotContains(t, scopeMap, "unscoped") } -func TestGetToolScopeInfo(t *testing.T) { - // Set up test scope map - SetGlobalToolScopeMap(ToolScopeMap{ - "search_orgs": &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - }) +func TestToolScopeInfo(t *testing.T) { + info := &ToolScopeInfo{ScopePolicy: AllOfScopePolicy(Repo, Workflow)} - info, err := GetToolScopeInfo("search_orgs") - require.NoError(t, err) - require.NotNil(t, info) - - // Non-existent tool should return nil - info, err = GetToolScopeInfo("nonexistent_tool") - require.NoError(t, err) - assert.Nil(t, info) + assert.True(t, info.Satisfies("repo", "workflow")) + assert.False(t, info.Satisfies("repo")) + assert.Equal(t, []string{"workflow"}, info.ChallengeScopes("repo")) + assert.True(t, (*ToolScopeInfo)(nil).Satisfies()) + assert.Nil(t, (*ToolScopeInfo)(nil).ChallengeScopes()) } -func TestToolScopeInfo_HasAcceptedScope(t *testing.T) { - testCases := []struct { - name string - scopeInfo *ToolScopeInfo - userScopes []string - expected bool - }{ - { - name: "has exact required scope", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - userScopes: []string{"read:org"}, - expected: true, - }, - { - name: "has parent scope (admin:org grants read:org)", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - userScopes: []string{"admin:org"}, - expected: true, - }, - { - name: "has parent scope (write:org grants read:org)", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - userScopes: []string{"write:org"}, - expected: true, - }, - { - name: "missing required scope", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - userScopes: []string{"repo"}, - expected: false, - }, - { - name: "no scope required", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{}, - AcceptedScopes: []string{}, - }, - userScopes: []string{}, - expected: true, - }, - { - name: "nil scope info", - scopeInfo: nil, - userScopes: []string{}, - expected: true, - }, - { - name: "repo scope for tool requiring repo", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"repo"}, - AcceptedScopes: []string{"repo"}, - }, - userScopes: []string{"repo"}, - expected: true, - }, - { - name: "missing repo scope", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"repo"}, - AcceptedScopes: []string{"repo"}, - }, - userScopes: []string{"public_repo"}, - expected: false, - }, - { - name: "satisfies all required groups", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"delete_repo", "repo"}, - AcceptedScopes: []string{"delete_repo", "repo"}, - RequiredScopeGroups: [][]string{{"delete_repo"}, {"repo"}}, - }, - userScopes: []string{"delete_repo", "repo"}, - expected: true, - }, - { - name: "missing one required group", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"delete_repo", "repo"}, - AcceptedScopes: []string{"delete_repo", "repo"}, - RequiredScopeGroups: [][]string{{"delete_repo"}, {"repo"}}, - }, - userScopes: []string{"delete_repo"}, - expected: false, +func TestToolScopeInfoResolve(t *testing.T) { + base := &ToolScopeInfo{ + ScopePolicy: AnyOfScopePolicy(Repo, ReadOrg), + ScopeResolver: func(arguments map[string]any) inventory.ScopePolicy { + if arguments["workflow"] == true { + return AllOfScopePolicy(Repo, Workflow) + } + return AllOfScopePolicy(Repo) }, } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - result := tc.scopeInfo.HasAcceptedScope(tc.userScopes...) - assert.Equal(t, tc.expected, result) - }) - } + resolved := base.Resolve(map[string]any{"workflow": true}) + require.NotSame(t, base, resolved) + assert.True(t, resolved.Satisfies("repo", "workflow")) + assert.False(t, resolved.Satisfies("repo")) + assert.Equal(t, []string{"workflow"}, resolved.ChallengeScopes("repo")) + + assert.True(t, base.Satisfies("read:org")) + regular := base.Resolve(map[string]any{"workflow": false}) + assert.True(t, regular.Satisfies("repo")) + assert.False(t, regular.Satisfies("read:org")) } -func TestToolScopeInfo_MissingScopes(t *testing.T) { - testCases := []struct { - name string - scopeInfo *ToolScopeInfo - userScopes []string - expectedLen int - expectedScopes []string - }{ - { - name: "has required scope - no missing", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - userScopes: []string{"read:org"}, - expectedLen: 0, - expectedScopes: nil, - }, - { - name: "missing scope", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"read:org"}, - AcceptedScopes: []string{"read:org", "write:org", "admin:org"}, - }, - userScopes: []string{"repo"}, - expectedLen: 1, - expectedScopes: []string{"read:org"}, - }, - { - name: "no scope required - no missing", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{}, - AcceptedScopes: []string{}, - }, - userScopes: []string{}, - expectedLen: 0, - expectedScopes: nil, - }, - { - name: "nil scope info - no missing", - scopeInfo: nil, - userScopes: []string{}, - expectedLen: 0, - expectedScopes: nil, - }, - { - name: "reports only missing required groups", - scopeInfo: &ToolScopeInfo{ - RequiredScopes: []string{"delete_repo", "repo"}, - AcceptedScopes: []string{"delete_repo", "repo"}, - RequiredScopeGroups: [][]string{{"delete_repo"}, {"repo"}}, - }, - userScopes: []string{"delete_repo"}, - expectedLen: 1, - expectedScopes: []string{"repo"}, - }, - } +func testTool(name string) mcp.Tool { + return mcp.Tool{Name: name} +} - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - missing := tc.scopeInfo.MissingScopes(tc.userScopes...) - assert.Len(t, missing, tc.expectedLen) - if tc.expectedScopes != nil { - for _, expected := range tc.expectedScopes { - assert.Contains(t, missing, expected) - } - } - }) - } +func testToolset() inventory.ToolsetMetadata { + return inventory.ToolsetMetadata{ID: "test"} } diff --git a/pkg/scopes/scopes.go b/pkg/scopes/scopes.go index 05a7774cd0..5a653d95be 100644 --- a/pkg/scopes/scopes.go +++ b/pkg/scopes/scopes.go @@ -1,8 +1,9 @@ package scopes import ( - "slices" "sort" + + "github.com/github/github-mcp-server/pkg/inventory" ) // Scope represents a GitHub OAuth scope. @@ -122,49 +123,6 @@ var ScopeHierarchy = map[Scope][]Scope{ User: {ReadUser, UserEmail}, } -// ScopeSet represents a set of OAuth scopes. -type ScopeSet map[Scope]bool - -// NewScopeSet creates a new ScopeSet from the given scopes. -func NewScopeSet(scopes ...Scope) ScopeSet { - set := make(ScopeSet) - for _, scope := range scopes { - set[scope] = true - } - return set -} - -// ToSlice converts a ScopeSet to a slice of Scope values. -func (s ScopeSet) ToSlice() []Scope { - scopes := make([]Scope, 0, len(s)) - for scope := range s { - scopes = append(scopes, scope) - } - // Sort for deterministic output - slices.Sort(scopes) - return scopes -} - -// ToStringSlice converts a ScopeSet to a slice of string values. -// The returned slice is sorted for deterministic output. -func (s ScopeSet) ToStringSlice() []string { - scopes := make([]string, 0, len(s)) - for scope := range s { - scopes = append(scopes, string(scope)) - } - sort.Strings(scopes) - return scopes -} - -// ToStringSlice converts a slice of Scopes to a slice of strings. -func ToStringSlice(scopes ...Scope) []string { - result := make([]string, len(scopes)) - for i, scope := range scopes { - result[i] = string(scope) - } - return result -} - // ExpandScopes takes a list of required scopes and returns all accepted scopes // including parent scopes from the hierarchy. // For example, if "public_repo" is required, "repo" is also accepted since @@ -182,11 +140,16 @@ func ExpandScopes(required ...Scope) []string { accepted[string(scope)] = true } - // Add parent scopes that grant access to required scopes - for parent, children := range ScopeHierarchy { - for _, child := range children { - if accepted[string(child)] { - accepted[string(parent)] = true + // Add every ancestor scope that grants access to a required scope. Iterate + // to a fixed point so provider hierarchies can be arbitrary DAGs. + for changed := true; changed; { + changed = false + for parent, children := range ScopeHierarchy { + for _, child := range children { + if accepted[string(child)] && !accepted[string(parent)] { + accepted[string(parent)] = true + changed = true + } } } } @@ -200,15 +163,49 @@ func ExpandScopes(required ...Scope) []string { return result } -// ExpandScopeGroups returns one accepted-scope group for each independently -// required scope. A token must satisfy every group, while any scope within a -// group is sufficient because parent scopes grant the same permission. -func ExpandScopeGroups(required ...Scope) [][]string { - groups := make([][]string, 0, len(required)) +// NewScopeRequirement creates one capability requirement. The challenge scope +// and each explicitly accepted alternative are expanded to include ancestors. +func NewScopeRequirement(challenge Scope, alternatives ...Scope) inventory.ScopeRequirement { + alternatives = append([]Scope{challenge}, alternatives...) + return inventory.ScopeRequirement{ + ChallengeScope: string(challenge), + AnyOf: ExpandScopes(alternatives...), + } +} + +// AnyOfScopePolicy creates a policy where any one supplied scope is sufficient. +func AnyOfScopePolicy(required ...Scope) inventory.ScopePolicy { + if len(required) == 0 { + return UnscopedScopePolicy() + } + paths := make([]inventory.ScopePath, 0, len(required)) + for _, scope := range required { + paths = append(paths, inventory.ScopePath{ + AllOf: []inventory.ScopeRequirement{NewScopeRequirement(scope)}, + }) + } + return inventory.ScopePolicy{AnyOf: paths} +} + +// AllOfScopePolicy creates a policy where every supplied scope is required. +func AllOfScopePolicy(required ...Scope) inventory.ScopePolicy { + requirements := make([]inventory.ScopeRequirement, 0, len(required)) for _, scope := range required { - groups = append(groups, ExpandScopes(scope)) + requirements = append(requirements, NewScopeRequirement(scope)) + } + if len(requirements) == 0 { + return UnscopedScopePolicy() + } + return inventory.ScopePolicy{ + AnyOf: []inventory.ScopePath{{AllOf: requirements}}, + } +} + +// UnscopedScopePolicy creates an explicit policy that requires no OAuth scope. +func UnscopedScopePolicy() inventory.ScopePolicy { + return inventory.ScopePolicy{ + AnyOf: []inventory.ScopePath{{}}, } - return groups } // expandScopeSet returns a set of all scopes granted by the given scopes, @@ -217,62 +214,83 @@ func ExpandScopeGroups(required ...Scope) [][]string { // and "security_events" since "repo" grants access to those child scopes. func expandScopeSet(scopes []string) map[string]bool { expanded := make(map[string]bool, len(scopes)) - for _, scope := range scopes { + queue := append([]string(nil), scopes...) + for len(queue) > 0 { + scope := queue[0] + queue = queue[1:] + if expanded[scope] { + continue + } expanded[scope] = true - // Add child scopes granted by this scope - if children, ok := ScopeHierarchy[Scope(scope)]; ok { - for _, child := range children { - expanded[string(child)] = true + for _, child := range ScopeHierarchy[Scope(scope)] { + if !expanded[string(child)] { + queue = append(queue, string(child)) } } } return expanded } -// HasRequiredScopes checks if tokenScopes satisfy the acceptedScopes requirement. -// A tool's acceptedScopes includes both the required scopes AND parent scopes -// that implicitly grant the required permissions (via ExpandScopes). -// -// For PAT filtering: if ANY of the acceptedScopes are granted by the token -// (directly or via scope hierarchy), the tool should be visible. -// -// Returns true if the tool should be visible to the token holder. -func HasRequiredScopes(tokenScopes []string, acceptedScopes []string) bool { - // No scopes required = always allowed - if len(acceptedScopes) == 0 { +// ScopePolicySatisfied reports whether the token satisfies at least one +// complete authorization alternative. +func ScopePolicySatisfied(tokenScopes []string, policy inventory.ScopePolicy) bool { + if len(policy.AnyOf) == 0 { return true } - - // Expand token scopes to include child scopes they grant - grantedScopes := expandScopeSet(tokenScopes) - - // Check if any accepted scope is granted by the token - for _, accepted := range acceptedScopes { - if grantedScopes[accepted] { + granted := expandScopeSet(tokenScopes) + for _, path := range policy.AnyOf { + satisfied := true + for _, requirement := range path.AllOf { + if !scopeRequirementSatisfied(granted, requirement) { + satisfied = false + break + } + } + if satisfied { return true } } return false } -// HasRequiredScopeGroups reports whether the token satisfies every independent -// required-scope group. -func HasRequiredScopeGroups(tokenScopes []string, groups [][]string) bool { - if len(groups) == 0 { - return true +// ChallengeScopesForPolicy returns the preferred challenge scopes for the first +// declared authorization alternative. Alternative order is policy, not an +// inferred least-privilege ranking. +func ChallengeScopesForPolicy(tokenScopes []string, policy inventory.ScopePolicy) []string { + if len(policy.AnyOf) == 0 { + return nil } - grantedScopes := expandScopeSet(tokenScopes) - for _, group := range groups { - satisfied := false - for _, accepted := range group { - if grantedScopes[accepted] { - satisfied = true - break - } + if ScopePolicySatisfied(tokenScopes, policy) { + return nil + } + granted := expandScopeSet(tokenScopes) + path := policy.AnyOf[0] + missing := make([]string, 0, len(path.AllOf)) + seen := make(map[string]bool) + for _, requirement := range path.AllOf { + if scopeRequirementSatisfied(granted, requirement) || requirement.ChallengeScope == "" || seen[requirement.ChallengeScope] { + continue } - if !satisfied { - return false + seen[requirement.ChallengeScope] = true + missing = append(missing, requirement.ChallengeScope) + } + return missing +} + +func scopeRequirementSatisfied(granted map[string]bool, requirement inventory.ScopeRequirement) bool { + if requirement.ChallengeScope == "" { + return true + } + if granted[requirement.ChallengeScope] { + return true + } + if len(requirement.AnyOf) == 0 { + return false + } + for _, scope := range requirement.AnyOf { + if granted[scope] { + return true } } - return true + return false } diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index bf5269da17..513a0ae263 100644 --- a/pkg/scopes/scopes_test.go +++ b/pkg/scopes/scopes_test.go @@ -4,6 +4,7 @@ import ( "sort" "testing" + "github.com/github/github-mcp-server/pkg/inventory" "github.com/stretchr/testify/assert" ) @@ -112,14 +113,6 @@ func TestExpandScopes(t *testing.T) { } } -func TestHasRequiredScopeGroups(t *testing.T) { - groups := ExpandScopeGroups(DeleteRepo, Repo) - - assert.True(t, HasRequiredScopeGroups([]string{"delete_repo", "repo"}, groups)) - assert.False(t, HasRequiredScopeGroups([]string{"delete_repo"}, groups)) - assert.False(t, HasRequiredScopeGroups([]string{"repo"}, groups)) -} - func TestOAuthScopeCatalog(t *testing.T) { supported := SupportedOAuthScopes() defaults := DefaultOAuthScopes() @@ -133,35 +126,66 @@ func TestOAuthScopeCatalog(t *testing.T) { assert.NotContains(t, defaults, string(Codespace)) } -func TestToStringSlice(t *testing.T) { - tests := []struct { - name string - scopes []Scope - expected []string - }{ - { - name: "empty returns empty", - scopes: []Scope{}, - expected: []string{}, - }, - { - name: "single scope", - scopes: []Scope{Repo}, - expected: []string{"repo"}, - }, - { - name: "multiple scopes", - scopes: []Scope{Repo, Gist, ReadOrg}, - expected: []string{"repo", "gist", "read:org"}, - }, - } +func TestScopePolicy(t *testing.T) { + t.Run("any authorization path is sufficient", func(t *testing.T) { + policy := AnyOfScopePolicy(Repo, ReadOrg) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := ToStringSlice(tt.scopes...) - assert.Equal(t, tt.expected, result) - }) - } + assert.True(t, ScopePolicySatisfied([]string{"repo"}, policy)) + assert.True(t, ScopePolicySatisfied([]string{"admin:org"}, policy)) + assert.Nil(t, ChallengeScopesForPolicy([]string{"admin:org"}, policy)) + assert.False(t, ScopePolicySatisfied([]string{"gist"}, policy)) + assert.Equal(t, []string{"repo"}, ChallengeScopesForPolicy([]string{"gist"}, policy)) + }) + + t.Run("every requirement in a path must be satisfied", func(t *testing.T) { + policy := AllOfScopePolicy(Repo, Workflow) + + assert.True(t, ScopePolicySatisfied([]string{"repo", "workflow"}, policy)) + assert.False(t, ScopePolicySatisfied([]string{"repo"}, policy)) + assert.Equal(t, []string{"workflow"}, ChallengeScopesForPolicy([]string{"repo"}, policy)) + }) + + t.Run("challenge uses declared path preference", func(t *testing.T) { + policy := inventory.ScopePolicy{ + AnyOf: []inventory.ScopePath{ + {AllOf: []inventory.ScopeRequirement{ + NewScopeRequirement(Repo), + NewScopeRequirement(Workflow), + }}, + {AllOf: []inventory.ScopeRequirement{NewScopeRequirement(ReadOrg)}}, + }, + } + + assert.Equal(t, []string{"repo", "workflow"}, ChallengeScopesForPolicy([]string{"gist"}, policy)) + }) + + t.Run("canonical challenge scope always satisfies its requirement", func(t *testing.T) { + policy := inventory.ScopePolicy{ + AnyOf: []inventory.ScopePath{{ + AllOf: []inventory.ScopeRequirement{{ + ChallengeScope: "narrow", + AnyOf: []string{"broad"}, + }}, + }}, + } + + assert.True(t, ScopePolicySatisfied([]string{"narrow"}, policy)) + }) + + t.Run("accepted alternatives apply to one requirement only", func(t *testing.T) { + policy := inventory.ScopePolicy{ + AnyOf: []inventory.ScopePath{{ + AllOf: []inventory.ScopeRequirement{ + NewScopeRequirement(PublicRepo), + NewScopeRequirement(Workflow), + }, + }}, + } + + assert.True(t, ScopePolicySatisfied([]string{"repo", "workflow"}, policy)) + assert.False(t, ScopePolicySatisfied([]string{"repo"}, policy)) + assert.Equal(t, []string{"workflow"}, ChallengeScopesForPolicy([]string{"repo"}, policy)) + }) } func TestScopeHierarchy(t *testing.T) { @@ -249,110 +273,3 @@ func TestExpandScopeSet(t *testing.T) { }) } } - -func TestHasRequiredScopes(t *testing.T) { - tests := []struct { - name string - tokenScopes []string - acceptedScopes []string - expected bool - }{ - { - name: "no accepted scopes - always allowed", - tokenScopes: []string{}, - acceptedScopes: []string{}, - expected: true, - }, - { - name: "nil accepted scopes - always allowed", - tokenScopes: []string{"repo"}, - acceptedScopes: nil, - expected: true, - }, - { - name: "token has exact required scope", - tokenScopes: []string{"repo"}, - acceptedScopes: []string{"repo"}, - expected: true, - }, - { - name: "token has parent scope that grants access", - tokenScopes: []string{"repo"}, - acceptedScopes: []string{"public_repo"}, - expected: true, - }, - { - name: "token has parent scope for security_events", - tokenScopes: []string{"repo"}, - acceptedScopes: []string{"security_events"}, - expected: true, - }, - { - name: "token has admin:org which grants read:org", - tokenScopes: []string{"admin:org"}, - acceptedScopes: []string{"read:org"}, - expected: true, - }, - { - name: "token has write:org which grants read:org", - tokenScopes: []string{"write:org"}, - acceptedScopes: []string{"read:org"}, - expected: true, - }, - { - name: "token missing required scope", - tokenScopes: []string{"gist"}, - acceptedScopes: []string{"repo"}, - expected: false, - }, - { - name: "token has child but not parent - fails", - tokenScopes: []string{"public_repo"}, - acceptedScopes: []string{"repo"}, - expected: false, - }, - { - name: "multiple token scopes - one matches", - tokenScopes: []string{"gist", "repo"}, - acceptedScopes: []string{"public_repo"}, - expected: true, - }, - { - name: "multiple accepted scopes - token has one", - tokenScopes: []string{"repo"}, - acceptedScopes: []string{"repo", "admin:org"}, - expected: true, - }, - { - name: "empty token scopes - fails when scopes required", - tokenScopes: []string{}, - acceptedScopes: []string{"repo"}, - expected: false, - }, - { - name: "user scope grants read:user", - tokenScopes: []string{"user"}, - acceptedScopes: []string{"read:user"}, - expected: true, - }, - { - name: "user scope grants user:email", - tokenScopes: []string{"user"}, - acceptedScopes: []string{"user:email"}, - expected: true, - }, - { - name: "write:packages grants read:packages", - tokenScopes: []string{"write:packages"}, - acceptedScopes: []string{"read:packages"}, - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := HasRequiredScopes(tt.tokenScopes, tt.acceptedScopes) - assert.Equal(t, tt.expected, result) - }) - } -}