diff --git a/backend/__tests__/fixtures/hostedMcp/github-mcp-tools-source-2026-10-15.json b/backend/__tests__/fixtures/hostedMcp/github-mcp-tools-source-2026-10-15.json new file mode 100644 index 0000000000..a2f0855169 --- /dev/null +++ b/backend/__tests__/fixtures/hostedMcp/github-mcp-tools-source-2026-10-15.json @@ -0,0 +1,412 @@ +{ + "sourceCommit": "796afb74711ddc856fe587debc49373e21c8d0f7", + "tools": [ + { + "name": "get_me", + "description": "Get details of the authenticated GitHub user. Use this when a request is about the user's own profile for GitHub. Or when information is missing to build other tool calls.", + "inputSchema": { + "properties": {}, + "type": "object" + }, + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "Get my user profile" + } + }, + { + "name": "list_issues", + "description": "List issues in a GitHub repository. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter.", + "inputSchema": { + "properties": { + "after": { + "description": "Cursor for pagination. Use the cursor from the previous response.", + "type": "string" + }, + "direction": { + "description": "Order direction. If provided, the 'orderBy' also needs to be provided.", + "enum": [ + "ASC", + "DESC" + ], + "type": "string" + }, + "field_filters": { + "description": "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).", + "items": { + "properties": { + "field_name": { + "description": "Name of the custom field (e.g. \"Priority\"). Case-insensitive.", + "type": "string" + }, + "value": { + "description": "Value to filter on. For single-select fields, the option name (e.g. \"P1\"). For dates, YYYY-MM-DD. For numbers, the numeric value as a string. For text, the text value.", + "type": "string" + } + }, + "required": [ + "field_name", + "value" + ], + "type": "object" + }, + "type": "array" + }, + "fields": { + "description": "Subset of fields to return for each issue. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body' and 'field_values' in particular drops the largest per-result data.", + "items": { + "enum": [ + "number", + "title", + "body", + "state", + "user", + "labels", + "assignees", + "comments", + "created_at", + "updated_at", + "field_values" + ], + "type": "string" + }, + "type": "array" + }, + "labels": { + "description": "Filter by labels", + "items": { + "type": "string" + }, + "type": "array" + }, + "orderBy": { + "description": "Order issues by field. If provided, the 'direction' also needs to be provided.", + "enum": [ + "CREATED_AT", + "UPDATED_AT", + "COMMENTS" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "since": { + "description": "Filter by date (ISO 8601 timestamp)", + "type": "string" + }, + "state": { + "description": "Filter by state, by default both open and closed issues are returned when not provided", + "enum": [ + "OPEN", + "CLOSED" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "List issues" + } + }, + { + "name": "issue_read", + "description": "Get information about a specific issue in a GitHub repository.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The number of the issue", + "type": "number" + }, + "method": { + "description": "The read operation to perform on a single issue.\nOptions are:\n1. get - Get issue details. Also returns best-effort hierarchy flags (`has_parent`, `has_children`); `parent` and `sub_issues_summary` are optional relationship summaries, and `closed_by_pull_requests` summarizes the pull requests configured to close the issue as `total_count` plus up to 5 `references`.\n2. get_comments - Get issue comments.\n3. get_sub_issues - Get sub-issues (children) of the issue.\n4. get_parent - Get the parent issue, if this issue is a sub-issue of another.\n5. get_labels - Get labels assigned to the issue.\n", + "enum": [ + "get", + "get_comments", + "get_sub_issues", + "get_parent", + "get_labels" + ], + "type": "string" + }, + "owner": { + "description": "The owner of the repository", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "The name of the repository", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "issue_number" + ], + "type": "object" + }, + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "Get issue details" + } + }, + { + "name": "list_pull_requests", + "description": "List pull requests in a GitHub repository. If the user specifies an author, then DO NOT use this tool and use the search_pull_requests tool instead.", + "inputSchema": { + "properties": { + "base": { + "description": "Filter by base branch", + "type": "string" + }, + "direction": { + "description": "Sort direction", + "enum": [ + "asc", + "desc" + ], + "type": "string" + }, + "fields": { + "description": "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.", + "items": { + "enum": [ + "number", + "title", + "body", + "state", + "draft", + "merged", + "mergeable_state", + "html_url", + "user", + "labels", + "assignees", + "requested_reviewers", + "merged_by", + "head", + "base", + "additions", + "deletions", + "changed_files", + "commits", + "comments", + "created_at", + "updated_at", + "closed_at", + "merged_at", + "milestone" + ], + "type": "string" + }, + "type": "array" + }, + "head": { + "description": "Filter by head user/org and branch", + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sort": { + "description": "Sort by", + "enum": [ + "created", + "updated", + "popularity", + "long-running" + ], + "type": "string" + }, + "state": { + "description": "Filter by state", + "enum": [ + "open", + "closed", + "all" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "List pull requests" + } + }, + { + "name": "pull_request_read", + "description": "Get information on a specific pull request in GitHub repository.", + "inputSchema": { + "properties": { + "after": { + "description": "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.", + "type": "string" + }, + "method": { + "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get combined commit status of a head commit in a pull request.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_commits - Get the list of commits on a pull request. Use with pagination parameters to control the number of results returned.\n 6. get_review_comments - Get review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns thread metadata and comments with nullable current and original line-range coordinates (line, start_line, original_line, original_start_line). Current coordinates are omitted when unavailable, such as for outdated comments. Use cursor-based pagination (perPage, after) to control results.\n 7. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned.\n 8. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned.\n 9. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR.\n", + "enum": [ + "get", + "get_diff", + "get_status", + "get_files", + "get_commits", + "get_review_comments", + "get_reviews", + "get_comments", + "get_check_runs" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "pullNumber": { + "description": "Pull request number", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "pullNumber" + ], + "type": "object" + }, + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "Get details for a single pull request" + } + }, + { + "name": "get_file_contents", + "description": "Get the contents of a file or directory from a GitHub repository", + "inputSchema": { + "properties": { + "fields": { + "description": "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'.", + "items": { + "enum": [ + "type", + "name", + "path", + "size", + "sha", + "url", + "git_url", + "html_url", + "download_url" + ], + "type": "string" + }, + "type": "array" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string", + "x-mcp-header": "owner" + }, + "path": { + "default": "/", + "description": "Path to file/directory", + "type": "string" + }, + "ref": { + "description": "Accepts optional git refs such as `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head`", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string", + "x-mcp-header": "repo" + }, + "sha": { + "description": "Accepts optional commit SHA. If specified, it will be used instead of ref", + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "Get file or directory contents" + } + } + ] +} diff --git a/backend/__tests__/unit/integrations/hostedMcpClientWiring.test.js b/backend/__tests__/unit/integrations/hostedMcpClientWiring.test.js index 64ccd8e157..ad42f5cff1 100644 --- a/backend/__tests__/unit/integrations/hostedMcpClientWiring.test.js +++ b/backend/__tests__/unit/integrations/hostedMcpClientWiring.test.js @@ -3,11 +3,10 @@ * names derived from its id (`hostedMcpClientConfigKeys`). Those names reach * the backend only if the helm chart maps them from `api-keys`, and `api-keys` * carries them only if the ExternalSecret references their Secret Manager keys. - * Nothing else holds the three layers together: an entry added without its - * wiring reads `not_configured` in production while every other test stays - * green (review-checklist rule 7, a phantom cross-layer contract). This pins - * the chain for every pre-registered entry, so the next one cannot ship - * half-wired. + * Nothing else holds the three layers together (review-checklist rule 7). + * GitHub's ExternalSecret entries are off by default for self-hosted instances. + * Dev enables them only after both App client values exist with versions, and + * both backend refs remain optional. */ const fs = require('fs'); const path = require('path'); @@ -17,6 +16,8 @@ const { hostedMcpClientConfigKeys } = require('../../../services/hostedMcpIntake const chart = path.join(__dirname, '../../../../k8s/helm/commonly'); const deployment = fs.readFileSync(path.join(chart, 'templates/core/backend-deployment.yaml'), 'utf8'); const apiKeys = fs.readFileSync(path.join(chart, 'templates/secrets/api-keys.yaml'), 'utf8'); +const values = fs.readFileSync(path.join(chart, 'values.yaml'), 'utf8'); +const devValues = fs.readFileSync(path.join(chart, 'values-dev.yaml'), 'utf8'); // `- name: ENV` mapped from api-keys, optional so an instance with the entry's // flag off still starts. Returns the api-keys key, or null when unmapped. @@ -51,6 +52,13 @@ const wiringTable = preRegistered.length ? preRegistered.map((entry) => [entry.id]) : [['no-pre-registered-entry']]; +test('GitHub client mapping is off by default and enabled only in configured dev', () => { + expect(preRegistered.map((entry) => entry.id)).toContain('github-mcp'); + expect(values).toMatch(/hostedMcp:\n(?:.|\n)*?githubMcp:\n\s+enabled: false/); + expect(devValues).toMatch(/hostedMcp:\n(?:.|\n)*?githubMcp:\n\s+enabled: true/); + expect(apiKeys).toMatch(/{{- if \.Values\.hostedMcp\.githubMcp\.enabled }}[\s\S]*?secretKey: github-mcp-client-id[\s\S]*?secretKey: github-mcp-client-secret[\s\S]*?{{- end }}/); +}); + describe.each(wiringTable)('%s client wiring', (entryId) => { const { clientId, clientSecret } = hostedMcpClientConfigKeys(entryId); diff --git a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js index 00177c5810..845a6d3e96 100644 --- a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js +++ b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js @@ -576,6 +576,20 @@ describe('hosted-mcp connect: callback', () => { expect(update.$set['config.grantedScope']).toBe('read'); }); + it('records an explicit empty scope response without inventing requested scopes', async () => { + // GitHub App user tokens can report scope="" even when offline_access + // was requested. The callback must preserve what the vendor returned. + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ access_token: 'access-1', scope: '' }), + }); + const res = await callback(); + expect(outcome(res).hostedMcp).toBe('connected'); + expect(Integration.findOneAndUpdate).toHaveBeenCalledTimes(3); + const [, update] = Integration.findOneAndUpdate.mock.calls[2]; + expect(update.$set['config.grantedScope']).toBe(''); + }); + it('exchanges a pre-registered callback with instance credentials while retaining PKCE', async () => { const originalEntry = { ...FIXTURE_ENTRY }; const originalId = process.env.GOOGLE_CALENDAR_CLIENT_ID; diff --git a/backend/__tests__/unit/services/hostedMcpEntry.test.js b/backend/__tests__/unit/services/hostedMcpEntry.test.js index 0215ef745d..ab4b2377d6 100644 --- a/backend/__tests__/unit/services/hostedMcpEntry.test.js +++ b/backend/__tests__/unit/services/hostedMcpEntry.test.js @@ -28,6 +28,8 @@ const SENTRY_CAPTURE = require('../../fixtures/hostedMcp/sentry-tools-list-2026- const NOTION_CAPTURE = require('../../fixtures/hostedMcp/notion-tools-list-2026-10-19.json'); const GITLAB_CAPTURE = require('../../fixtures/hostedMcp/gitlab-tools-list-2026-10-19.json'); const SUPABASE_CAPTURE = require('../../fixtures/hostedMcp/supabase-tools-list-2026-10-19.json'); +const GITHUB_MCP_SOURCE = require('../../fixtures/hostedMcp/github-mcp-tools-source-2026-10-15.json'); +const GITHUB_MCP_PIN = require('../../../integrations/hostedMcp/githubMcpSchemas.json'); const ENTRY_TEXT = 'ENTRY TEXT the pin agreed to.'; const VENDOR_TEXT = 'VENDOR TEXT nobody reviewed.'; @@ -584,6 +586,7 @@ describe('the shipped catalogue cannot land half-wired', () => { // any member has connected anything. Its ordering follows the build plan. expect(HOSTED_MCP_ENTRIES.map((entry) => entry.id)).toEqual([ 'linear', 'google-calendar', 'atlassian', 'airtable', 'sentry', 'notion', 'gitlab', 'supabase', + 'github-mcp', ]); const catalogue = [linear(), linear({ id: 'notion', title: 'Notion' })]; expect(findHostedMcpEntry(catalogue, 'notion').title).toBe('Notion'); @@ -874,6 +877,55 @@ describe('the shipped catalogue cannot land half-wired', () => { .toContain('untracked data and schema changes'); }); + test('GitHub pins six read tools from one fixed upstream commit and exposes no writer', () => { + const entry = findHostedMcpEntry(HOSTED_MCP_ENTRIES, 'github-mcp'); + expect(GITHUB_MCP_SOURCE.sourceCommit).toBe('796afb74711ddc856fe587debc49373e21c8d0f7'); + expect(GITHUB_MCP_PIN.sourceCommit).toBe(GITHUB_MCP_SOURCE.sourceCommit); + expect(entry).toMatchObject({ + resource: 'https://api.githubcopilot.com/mcp/', + issuer: 'https://github.com/login/oauth', + client: 'pre-registered', + scopes: ['offline_access'], + tokenEndpointAuthMethod: 'client_secret_post', + revoke: { page: 'https://github.com/settings/applications' }, + }); + const expected = [ + 'get_me', 'list_issues', 'issue_read', 'list_pull_requests', + 'pull_request_read', 'get_file_contents', + ]; + expect(entry.tools.map((tool) => tool.name)).toEqual(expected); + expect(GITHUB_MCP_SOURCE.tools.map((tool) => tool.name)).toEqual(expected); + const source = new Map(GITHUB_MCP_SOURCE.tools.map((tool) => [tool.name, tool])); + for (const tool of entry.tools) { + const upstream = source.get(tool.name); + expect(tool.upstreamName).toBe(tool.name); + expect(tool.class).toBe('read'); + expect(tool.annotations.readOnlyHint).toBe(true); + expect(upstream.annotations.readOnlyHint).toBe(true); + expect(canonicalJson(tool.inputSchema)).toBe(canonicalJson(upstream.inputSchema)); + expect(tool.description).not.toBe(upstream.description); + const propertyNames = []; + const visit = (value) => { + if (!value || typeof value !== 'object') return; + if (value.properties && typeof value.properties === 'object') { + propertyNames.push(...Object.keys(value.properties)); + } + Object.values(value).forEach(visit); + }; + visit(tool.inputSchema); + for (const targetKey of ['url', 'uri', 'fetch_target', 'target_url', 'source_url']) { + expect(propertyNames).not.toContain(targetKey); + } + } + const offered = entryToolProjections(entry); + expect(offered.map((tool) => tool.name)).toEqual(expected.map((name) => `github-mcp.${name}`)); + expect(offered.every((tool) => tool.requiredWriteMode === 'read')).toBe(true); + for (const name of ['create_issue', 'add_issue_comment', 'create_pull_request', 'merge_pull_request']) { + expect(offered.some((tool) => tool.name === `github-mcp.${name}`)).toBe(false); + } + expect(assessEntryTools(entry, GITHUB_MCP_SOURCE.tools).every((row) => row.verdict === 'ok')).toBe(true); + }); + test('the Linear pin is the measured list minus caller-chosen fetch paths', () => { // The latest capture `connector-ops` took (2026-10-14) is committed beside // this suite, so the pin's schemas are compared with the measurement rather @@ -1023,6 +1075,53 @@ describe('the shipped catalogue cannot land half-wired', () => { .toEqual({ page: bothURLs.page }); }); + // TASK-290: GitHub's App user token reaches only the accounts and + // repositories the App is installed on, so the page where a person picks + // them is part of connecting. Each instance registers its own App, so the + // page comes from the instance's config, and a slug GitHub could not have + // issued offers no page at all rather than a link somewhere else. + test('the GitHub access page is the configured App\'s install page, or none', () => { + const { githubMcpAccessPage } = require('../../../integrations/hostedMcp/githubMcp'); + expect(githubMcpAccessPage({ GITHUB_MCP_APP_SLUG: 'commonly-connect' })) + .toBe('https://github.com/apps/commonly-connect/installations/new'); + expect(githubMcpAccessPage({ GITHUB_MCP_APP_SLUG: ' self-host-app ' })) + .toBe('https://github.com/apps/self-host-app/installations/new'); + for (const slug of [undefined, '', 'Commonly', '../evil', 'a/b', 'app?x=1', '-lead', 'trail-', 'evil.example']) { + expect(githubMcpAccessPage({ GITHUB_MCP_APP_SLUG: slug })).toBeUndefined(); + } + const before = process.env.GITHUB_MCP_APP_SLUG; + try { + process.env.GITHUB_MCP_APP_SLUG = 'commonly-connect'; + jest.isolateModules(() => { + const { GITHUB_MCP_ENTRY } = require('../../../integrations/hostedMcp/githubMcp'); + expect(GITHUB_MCP_ENTRY.accessPage).toBe('https://github.com/apps/commonly-connect/installations/new'); + }); + delete process.env.GITHUB_MCP_APP_SLUG; + jest.isolateModules(() => { + const { GITHUB_MCP_ENTRY } = require('../../../integrations/hostedMcp/githubMcp'); + expect(GITHUB_MCP_ENTRY).not.toHaveProperty('accessPage'); + }); + } finally { + if (before === undefined) delete process.env.GITHUB_MCP_APP_SLUG; + else process.env.GITHUB_MCP_APP_SLUG = before; + } + }); + + test('an access page is an absolute https URL or absent', () => { + for (const accessPage of [ + 'github.com/apps/x/installations/new', 'http://github.com/apps/x', '', 'https://a b', 42, + // Pass the prefix check, but name no host. + 'https://#fragment', 'https://?q', + ]) { + expect(() => assertHostedMcpEntries([linear({ accessPage })])) + .toThrow(/access page is not an absolute https URL: linear/); + } + // Controls: an entry with no page, and one with an https page, both load. + expect(() => assertHostedMcpEntries([linear()])).not.toThrow(); + expect(() => assertHostedMcpEntries([linear({ accessPage: 'https://github.com/apps/x/installations/new' })])) + .not.toThrow(); + }); + test('a namespaced name is the entry id and the pinned name', () => { const entry = linear(); expect(hostedMcpToolName(entry, entry.tools[0])).toBe('linear.list_issues'); diff --git a/backend/__tests__/unit/services/hostedMcpIntake.test.js b/backend/__tests__/unit/services/hostedMcpIntake.test.js index 2f209e8a01..59f3ecd9be 100644 --- a/backend/__tests__/unit/services/hostedMcpIntake.test.js +++ b/backend/__tests__/unit/services/hostedMcpIntake.test.js @@ -426,6 +426,28 @@ describe('hosted-mcp intake: pre-registered client authentication', () => { ); }); + it('uses GitHub App client_secret_post for exchange and refresh when metadata omits methods', () => { + const { GITHUB_MCP_ENTRY } = require('../../../integrations/hostedMcp/githubMcp'); + const client = { clientId: 'github-client', clientSecret: 'github-secret' }; + const exchange = buildTokenExchangeRequest(GITHUB_MCP_ENTRY, {}, client, { + redirectUri: 'https://api.commonly.me/api/hosted-mcp/github-mcp/callback', + code: 'code', codeVerifier: 'pkce-verifier', + }); + const refresh = buildRefreshRequest(GITHUB_MCP_ENTRY, {}, client, { refreshToken: 'refresh' }); + for (const request of [exchange, refresh]) { + expect(request.headers.Authorization).toBeUndefined(); + expect(request.body.get('client_id')).toBe(client.clientId); + expect(request.body.get('client_secret')).toBe(client.clientSecret); + expect(request.body.get('resource')).toBe(GITHUB_MCP_ENTRY.resource); + } + expect(exchange.body.get('code_verifier')).toBe('pkce-verifier'); + expect(() => buildRefreshRequest(GITHUB_MCP_ENTRY, { + token_endpoint_auth_methods_supported: ['client_secret_basic'], + }, client, { refreshToken: 'refresh' })).toThrow(expect.objectContaining({ + code: 'client_auth_unsupported', + })); + }); + it('uses client_secret_post when both endpoints advertise it', () => { const staticEntry = entry({ id: 'google-calendar', client: 'pre-registered' }); const metadata = { diff --git a/backend/__tests__/unit/services/installableCatalogService.test.js b/backend/__tests__/unit/services/installableCatalogService.test.js index f2cb1766b7..58fcda7f0f 100644 --- a/backend/__tests__/unit/services/installableCatalogService.test.js +++ b/backend/__tests__/unit/services/installableCatalogService.test.js @@ -243,6 +243,7 @@ it("a hosted entry's tool row carries its connect descriptor and only its own co title: 'Acme', description: 'Tickets in Acme.', connectNotice: 'Ask your admin to allow this connection first.', + accessPage: 'https://acme.example/install', resource: 'https://mcp.acme.example/mcp', issuer: 'https://mcp.acme.example', client: 'cimd', @@ -285,6 +286,8 @@ it("a hosted entry's tool row carries its connect descriptor and only its own co expect(acmeRow).toMatchObject({ connectionType: 'hosted-mcp', entryId: 'acme' }); expect(acmeRow.connectNotice).toBe(acme.connectNotice); expect(linear.connectNotice).toBeUndefined(); + expect(acmeRow.accessPage).toBe(acme.accessPage); + expect(linear.accessPage).toBeUndefined(); // Each row sees its own vendor's connection and not the other's. expect(linear.connections).toEqual([ { connectionId: 'conn-linear', owner: '', repo: '' }, diff --git a/backend/__tests__/unit/services/toolInstallables.test.js b/backend/__tests__/unit/services/toolInstallables.test.js index e04acc86b6..8c546adc5e 100644 --- a/backend/__tests__/unit/services/toolInstallables.test.js +++ b/backend/__tests__/unit/services/toolInstallables.test.js @@ -229,6 +229,24 @@ describe('one tool Installable per catalogue entry', () => { expect(metas['google-calendar'].readiness()).toEqual({ available: false, reason: 'not_configured' }); }); + test('the pending GitHub MCP entry stays unavailable until both App client values exist', () => { + const noConfig = toolInstallableMetas(HOSTED_MCP_ENTRIES, {}); + expect(noConfig['github-mcp']).toMatchObject({ + connectionType: 'hosted-mcp', + entryId: 'github-mcp', + }); + expect(noConfig['github-mcp'].readiness()).toEqual({ available: false, reason: 'not_configured' }); + + const idOnly = toolInstallableMetas(HOSTED_MCP_ENTRIES, { GITHUB_MCP_CLIENT_ID: 'app-client' }); + expect(idOnly['github-mcp'].readiness()).toEqual({ available: false, reason: 'not_configured' }); + + const configured = toolInstallableMetas(HOSTED_MCP_ENTRIES, { + GITHUB_MCP_CLIENT_ID: 'app-client', + GITHUB_MCP_CLIENT_SECRET: 'app-secret', + }); + expect(configured['github-mcp'].readiness()).toEqual({ available: true }); + }); + test('a hosted entry cannot shadow the builtin GitHub Installable meta', () => { expect(() => toolInstallableMetas([{ ...LINEAR_ENTRY, id: 'github' }])) .toThrow(/collides with a builtin tool installable: github/); diff --git a/backend/integrations/hostedMcp/entries.ts b/backend/integrations/hostedMcp/entries.ts index b969b774d3..fe2db399bc 100644 --- a/backend/integrations/hostedMcp/entries.ts +++ b/backend/integrations/hostedMcp/entries.ts @@ -15,6 +15,7 @@ import type { HostedMcpEntry } from '../../services/hostedMcpEntryService'; import { AIRTABLE_ENTRY } from './airtable'; import { ATLASSIAN_ENTRY } from './atlassian'; import { GOOGLE_CALENDAR_ENTRY } from './googleCalendar'; +import { GITHUB_MCP_ENTRY } from './githubMcp'; import { GITLAB_ENTRY } from './gitlab'; import { LINEAR_ENTRY } from './linear'; import { NOTION_ENTRY } from './notion'; @@ -26,6 +27,7 @@ export const HOSTED_MCP_ENTRIES: HostedMcpEntry[] = [ NOTION_ENTRY, GITLAB_ENTRY, SUPABASE_ENTRY, + GITHUB_MCP_ENTRY, ]; /** @@ -182,6 +184,21 @@ export const assertHostedMcpEntries = ( ); } } + // The catalogue hands this page to a person as a link. The prefix check + // alone accepts `https://#fragment`, which has no host to open. + if (entry.accessPage !== undefined) { + let page: URL | null = null; + try { + if (typeof entry.accessPage === 'string' && /^https:\/\/[^\s]+$/.test(entry.accessPage)) { + page = new URL(entry.accessPage); + } + } catch { + page = null; + } + if (!page || page.protocol !== 'https:' || !page.hostname) { + throw new Error(`hosted-mcp entry access page is not an absolute https URL: ${entry.id}`); + } + } // What removal will do with this entry // (services/connectionRemovalService.ts). A `revoke` that names no kind, // both, or a URL that is not https is not a refusal at call time — the diff --git a/backend/integrations/hostedMcp/githubMcp.ts b/backend/integrations/hostedMcp/githubMcp.ts new file mode 100644 index 0000000000..aec0117551 --- /dev/null +++ b/backend/integrations/hostedMcp/githubMcp.ts @@ -0,0 +1,62 @@ +/** + * GitHub's remote MCP server. The six input schemas are pinned from + * github/github-mcp-server at 796afb74711ddc856fe587debc49373e21c8d0f7 + * (the upstream tool snapshots), not from a live account. A consenting + * account's first tools/list remains the drift check before this is offered. + * + * The credential is a separate, public GitHub App. GitHub App user tokens use + * the app's permissions rather than OAuth scopes; offline_access asks GitHub + * for a refresh token only. This entry pins only reads; any write tool must be + * pinned separately with owner confirmation. The App's client id and secret + * are optional instance configuration. Until registration supplies both, the + * catalogue reports not_configured and offers no Connect. + */ +import type { HostedMcpEntry, HostedMcpPinnedTool } from '../../services/hostedMcpEntryService'; +import pin from './githubMcpSchemas.json'; + +const descriptions: Record = { + get_me: 'Read the account profile for this GitHub connection.', + list_issues: 'List issues in a GitHub repository.', + issue_read: 'Read an issue and its comments or related issues.', + list_pull_requests: 'List pull requests in a GitHub repository.', + pull_request_read: 'Read a pull request, its files, reviews or comments.', + get_file_contents: 'Read a file or directory in a GitHub repository.', +}; + +const tools: HostedMcpPinnedTool[] = pin.tools.map((tool) => ({ + name: tool.name, + upstreamName: tool.name, + description: descriptions[tool.name], + class: 'read', + inputSchema: tool.inputSchema, + annotations: { readOnlyHint: true }, +})); + +// Each instance registers its own App, so its install page comes from the +// instance's config, like its client id. A slug GitHub could not have issued +// offers no page rather than a link to somewhere else. +const GITHUB_APP_SLUG = /^[a-z0-9](?:[a-z0-9-]{0,98}[a-z0-9])?$/; + +export const githubMcpAccessPage = (env: NodeJS.ProcessEnv = process.env): string | undefined => { + const slug = String(env.GITHUB_MCP_APP_SLUG || '').trim(); + return GITHUB_APP_SLUG.test(slug) ? `https://github.com/apps/${slug}/installations/new` : undefined; +}; + +const accessPage = githubMcpAccessPage(); + +export const GITHUB_MCP_ENTRY: HostedMcpEntry = { + id: 'github-mcp', + title: 'GitHub', + description: 'Read files, issues and pull requests in the repositories you install the GitHub App on.', + resource: 'https://api.githubcopilot.com/mcp/', + issuer: 'https://github.com/login/oauth', + client: 'pre-registered', + // GitHub's AS metadata omits auth methods. Its GitHub App token guide names + // client_id and client_secret as POST parameters, so pin that method instead + // of applying RFC 8414's Basic default. Confirm on the first live consent. + tokenEndpointAuthMethod: 'client_secret_post', + scopes: ['offline_access'], + revoke: { page: 'https://github.com/settings/applications' }, + ...(accessPage ? { accessPage } : {}), + tools, +}; diff --git a/backend/integrations/hostedMcp/githubMcpSchemas.json b/backend/integrations/hostedMcp/githubMcpSchemas.json new file mode 100644 index 0000000000..8a8393ba1e --- /dev/null +++ b/backend/integrations/hostedMcp/githubMcpSchemas.json @@ -0,0 +1,394 @@ +{ + "sourceCommit": "796afb74711ddc856fe587debc49373e21c8d0f7", + "tools": [ + { + "name": "get_me", + "inputSchema": { + "properties": {}, + "type": "object" + }, + "annotations": { + "readOnlyHint": true + } + }, + { + "name": "list_issues", + "inputSchema": { + "properties": { + "after": { + "description": "Cursor for pagination. Use the cursor from the previous response.", + "type": "string" + }, + "direction": { + "description": "Order direction. If provided, the 'orderBy' also needs to be provided.", + "enum": [ + "ASC", + "DESC" + ], + "type": "string" + }, + "field_filters": { + "description": "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).", + "items": { + "properties": { + "field_name": { + "description": "Name of the custom field (e.g. \"Priority\"). Case-insensitive.", + "type": "string" + }, + "value": { + "description": "Value to filter on. For single-select fields, the option name (e.g. \"P1\"). For dates, YYYY-MM-DD. For numbers, the numeric value as a string. For text, the text value.", + "type": "string" + } + }, + "required": [ + "field_name", + "value" + ], + "type": "object" + }, + "type": "array" + }, + "fields": { + "description": "Subset of fields to return for each issue. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body' and 'field_values' in particular drops the largest per-result data.", + "items": { + "enum": [ + "number", + "title", + "body", + "state", + "user", + "labels", + "assignees", + "comments", + "created_at", + "updated_at", + "field_values" + ], + "type": "string" + }, + "type": "array" + }, + "labels": { + "description": "Filter by labels", + "items": { + "type": "string" + }, + "type": "array" + }, + "orderBy": { + "description": "Order issues by field. If provided, the 'direction' also needs to be provided.", + "enum": [ + "CREATED_AT", + "UPDATED_AT", + "COMMENTS" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "since": { + "description": "Filter by date (ISO 8601 timestamp)", + "type": "string" + }, + "state": { + "description": "Filter by state, by default both open and closed issues are returned when not provided", + "enum": [ + "OPEN", + "CLOSED" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "annotations": { + "readOnlyHint": true + } + }, + { + "name": "issue_read", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The number of the issue", + "type": "number" + }, + "method": { + "description": "The read operation to perform on a single issue.\nOptions are:\n1. get - Get issue details. Also returns best-effort hierarchy flags (`has_parent`, `has_children`); `parent` and `sub_issues_summary` are optional relationship summaries, and `closed_by_pull_requests` summarizes the pull requests configured to close the issue as `total_count` plus up to 5 `references`.\n2. get_comments - Get issue comments.\n3. get_sub_issues - Get sub-issues (children) of the issue.\n4. get_parent - Get the parent issue, if this issue is a sub-issue of another.\n5. get_labels - Get labels assigned to the issue.\n", + "enum": [ + "get", + "get_comments", + "get_sub_issues", + "get_parent", + "get_labels" + ], + "type": "string" + }, + "owner": { + "description": "The owner of the repository", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "The name of the repository", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "issue_number" + ], + "type": "object" + }, + "annotations": { + "readOnlyHint": true + } + }, + { + "name": "list_pull_requests", + "inputSchema": { + "properties": { + "base": { + "description": "Filter by base branch", + "type": "string" + }, + "direction": { + "description": "Sort direction", + "enum": [ + "asc", + "desc" + ], + "type": "string" + }, + "fields": { + "description": "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.", + "items": { + "enum": [ + "number", + "title", + "body", + "state", + "draft", + "merged", + "mergeable_state", + "html_url", + "user", + "labels", + "assignees", + "requested_reviewers", + "merged_by", + "head", + "base", + "additions", + "deletions", + "changed_files", + "commits", + "comments", + "created_at", + "updated_at", + "closed_at", + "merged_at", + "milestone" + ], + "type": "string" + }, + "type": "array" + }, + "head": { + "description": "Filter by head user/org and branch", + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sort": { + "description": "Sort by", + "enum": [ + "created", + "updated", + "popularity", + "long-running" + ], + "type": "string" + }, + "state": { + "description": "Filter by state", + "enum": [ + "open", + "closed", + "all" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "annotations": { + "readOnlyHint": true + } + }, + { + "name": "pull_request_read", + "inputSchema": { + "properties": { + "after": { + "description": "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.", + "type": "string" + }, + "method": { + "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get combined commit status of a head commit in a pull request.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_commits - Get the list of commits on a pull request. Use with pagination parameters to control the number of results returned.\n 6. get_review_comments - Get review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns thread metadata and comments with nullable current and original line-range coordinates (line, start_line, original_line, original_start_line). Current coordinates are omitted when unavailable, such as for outdated comments. Use cursor-based pagination (perPage, after) to control results.\n 7. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned.\n 8. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned.\n 9. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR.\n", + "enum": [ + "get", + "get_diff", + "get_status", + "get_files", + "get_commits", + "get_review_comments", + "get_reviews", + "get_comments", + "get_check_runs" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "pullNumber": { + "description": "Pull request number", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "pullNumber" + ], + "type": "object" + }, + "annotations": { + "readOnlyHint": true + } + }, + { + "name": "get_file_contents", + "inputSchema": { + "properties": { + "fields": { + "description": "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'.", + "items": { + "enum": [ + "type", + "name", + "path", + "size", + "sha", + "url", + "git_url", + "html_url", + "download_url" + ], + "type": "string" + }, + "type": "array" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string", + "x-mcp-header": "owner" + }, + "path": { + "default": "/", + "description": "Path to file/directory", + "type": "string" + }, + "ref": { + "description": "Accepts optional git refs such as `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head`", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string", + "x-mcp-header": "repo" + }, + "sha": { + "description": "Accepts optional commit SHA. If specified, it will be used instead of ref", + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "annotations": { + "readOnlyHint": true + } + } + ] +} diff --git a/backend/services/hostedMcpEntryService.ts b/backend/services/hostedMcpEntryService.ts index abd1c06c1e..b30892aa1d 100644 --- a/backend/services/hostedMcpEntryService.ts +++ b/backend/services/hostedMcpEntryService.ts @@ -80,6 +80,8 @@ export interface HostedMcpEntry { /** The authorization server the protected-resource metadata names. */ issuer: string; client: 'pre-registered' | 'cimd' | 'dcr'; + /** Entry-pinned exception when an AS omits its token auth-method list. */ + tokenEndpointAuthMethod?: 'client_secret_basic' | 'client_secret_post'; scopes: string[]; /** Only the vendor-specific authorization parameters supported by this code. */ authorizationParams?: { @@ -93,6 +95,13 @@ export interface HostedMcpEntry { * https, and only the kind decides whether removal calls the vendor. */ revoke: { page: string; endpoint?: string }; + /** + * Where a person chooses, at the vendor, what this connection can reach. For + * GitHub that is the App's install page: its user token reaches only the + * accounts and repositories the App is installed on. Offered beside Connect + * and on the connected row, so the reach is the person's own choice; https only. + */ + accessPage?: string; tools: HostedMcpPinnedTool[]; } diff --git a/backend/services/hostedMcpIntakeService.ts b/backend/services/hostedMcpIntakeService.ts index 57d247db53..fac8e1ad69 100644 --- a/backend/services/hostedMcpIntakeService.ts +++ b/backend/services/hostedMcpIntakeService.ts @@ -455,10 +455,13 @@ const endpointAuthentication = ( const supported = endpoint === 'token' ? metadata.token_endpoint_auth_methods_supported : metadata.revocation_endpoint_auth_methods_supported; + const pinnedMethod = endpoint === 'token' ? entry.tokenEndpointAuthMethod : undefined; const methods = supported === undefined - ? ['client_secret_basic'] + ? [pinnedMethod ?? 'client_secret_basic'] : (Array.isArray(supported) ? supported : []); - const method = CONFIDENTIAL_CLIENT_METHODS.find((candidate) => methods.includes(candidate)); + const method = pinnedMethod + ? methods.find((candidate) => candidate === pinnedMethod) + : CONFIDENTIAL_CLIENT_METHODS.find((candidate) => methods.includes(candidate)); if (!method) { throw new HostedMcpClientError( 'client_auth_unsupported', diff --git a/backend/services/installable/installableCatalogService.ts b/backend/services/installable/installableCatalogService.ts index e26e6b403c..43a6541c94 100644 --- a/backend/services/installable/installableCatalogService.ts +++ b/backend/services/installable/installableCatalogService.ts @@ -172,6 +172,7 @@ const toolEntriesFor = async (userId: string): Promise<{ entries: unknown[]; hid label: row.name || row.installableId, description: row.description || '', ...(hostedEntry?.connectNotice ? { connectNotice: hostedEntry.connectNotice } : {}), + ...(hostedEntry?.accessPage ? { accessPage: hostedEntry.accessPage } : {}), connectionType: meta.connectionType, ...(meta.entryId ? { entryId: meta.entryId } : {}), available: readiness.available, diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 3dbd8b52a5..f0be28fef1 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2429,6 +2429,7 @@ "connect": "Connect", "connectError": "Could not begin connecting. Try again in a moment.", "connectOwn": "connect your own {{label}} account", + "chooseAccess": "Choose what {{label}} can reach", "connection": "connection", "counts": "Call counts", "days": "in {{count}} days", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index 4755445bb2..b186673209 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -2420,6 +2420,7 @@ "connect": "连接", "connectError": "无法开始连接，请稍后重试。", "connectOwn": "连接你自己的 {{label}} 账号", + "chooseAccess": "选择 {{label}} 可以访问的内容", "connection": "连接", "counts": "调用次数", "days": "{{count}}天后", diff --git a/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx b/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx index 7b59276d5e..0ad161ce6c 100644 --- a/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx @@ -743,6 +743,52 @@ test('a hosted connector shows vendor setup guidance before connect', async () = expect(screen.getByRole('button', { name: 'Connect' })).toBeInTheDocument(); }); +// TASK-290: GitHub's App user token reaches only the repositories the App is +// installed on, so the page that picks them sits beside Connect and stays +// after connecting, while the person decides what to grant. +test('a hosted connector links to where the person chooses its reach, before and after connecting', async () => { + const accessPage = 'https://github.com/apps/commonly-connect/installations/new'; + mockApi([{ ...linearEntry, connections: [], accessPage }]); + const first = renderTools(); + const before = await screen.findByRole('link', { name: 'Choose what Linear can reach' }); + expect(before).toHaveAttribute('href', accessPage); + expect(before).toHaveAttribute('target', '_blank'); + expect(before).toHaveAttribute('rel', 'noopener noreferrer'); + expect(screen.getByRole('button', { name: 'Connect' })).toBeInTheDocument(); + first.unmount(); + + mockApi([{ ...linearEntry, accessPage }]); + renderTools(); + expect(await screen.findByRole('link', { name: 'Choose what Linear can reach' })).toHaveAttribute('href', accessPage); + expect(screen.queryByRole('button', { name: 'Connect' })).toBeNull(); +}); + +// A live grant takes the entry off the offer list, so the page moves to the +// grant's details. Only the granter, who owns the connection, is sent there. +test('a live hosted grant keeps the reach link on its details for the granter only', async () => { + const accessPage = 'https://github.com/apps/commonly-connect/installations/new'; + const entry = { ...linearEntry, accessPage }; + mockTwoEntries({ catalog: [entry], grants: { p1: [grantLinear], p2: [] } }); + const first = renderTools(); + fireEvent.click(await screen.findByRole('button', { name: 'View Linear in Launch pod' })); + const aside = await screen.findByRole('complementary', { name: 'Grant details' }); + expect(within(aside).getByRole('link', { name: 'Choose what Linear can reach' })).toHaveAttribute('href', accessPage); + first.unmount(); + + mockTwoEntries({ catalog: [entry], grants: { p1: [{ ...grantLinear, grantedBy: 'u2' }], p2: [] } }); + renderTools(); + fireEvent.click(await screen.findByRole('button', { name: 'View Linear in Launch pod' })); + const other = await screen.findByRole('complementary', { name: 'Grant details' }); + expect(within(other).queryByRole('link', { name: /can reach/ })).toBeNull(); +}); + +test('a hosted connector without an access page offers no reach link', async () => { + mockApi([{ ...linearEntry, connections: [] }]); + renderTools(); + expect(await screen.findByRole('button', { name: 'Connect' })).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: /can reach/ })).toBeNull(); +}); + test('a hidden hosted entry has no offering while its existing connection can be removed', async () => { mockApi([]); const prior = axios.get.getMockImplementation(); diff --git a/frontend/src/v2/components/V2ConnectorTools.tsx b/frontend/src/v2/components/V2ConnectorTools.tsx index 9e4254da50..588aaf5b00 100644 --- a/frontend/src/v2/components/V2ConnectorTools.tsx +++ b/frontend/src/v2/components/V2ConnectorTools.tsx @@ -72,6 +72,11 @@ export interface ToolCatalogEntry { label: string; description: string; connectNotice?: string; + /** + * Where the person chooses, at the vendor, what the connection can reach + * (GitHub: the App's install page, which picks the repositories). + */ + accessPage?: string; /** * The connection type this row's tools run through, and — on a hosted entry — * the catalogue entry the start route takes. `entryId` is absent on @@ -778,6 +783,15 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) ); }; + // Where the person who connected a hosted entry picks what it reaches (for + // GitHub, the App's install page). Shown on the offer and on the details of + // their live grant, since a grant takes the entry off the offer list. + const renderAccessLink = (entry: ToolCatalogEntry) => ( + + {t('tools.chooseAccess', { defaultValue: 'Choose what {{label}} can reach', label: entry.label })} + + ); + const renderNotYet = (entry: ToolCatalogEntry) => { const canAdd = entry.available && entry.connections.length > 0 && podIds.length > 0; const canInstallGithubApp = isAdmin && entry.installableId === 'github' && entry.available && entry.connections.length === 0; @@ -808,6 +822,9 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) {canConnectHosted && entry.connectNotice && ( {entry.connectNotice} )} + {isHosted && entry.available && entry.accessPage && ( + {renderAccessLink(entry)} + )} {!entry.available && ( @@ -1022,6 +1039,11 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) )} + {/* The granter is the connection's owner (the hosted mint refuses anyone + else), so the page they reach is the one that bounds this grant. */} + {!dead && isGranter(grant) && entry?.connectionType === 'hosted-mcp' && entry.available && entry.accessPage && ( +

{renderAccessLink(entry)}

+ )} {staleAfterChange?.oldGrantId === grant.grantId && (

{t('tools.staleAfterChange', { defaultValue: 'The new grant is live. This old one still is too — its revoke did not go through.' })}

diff --git a/frontend/src/v2/icons/platforms/index.tsx b/frontend/src/v2/icons/platforms/index.tsx index af61d3ae8d..6542a2d0ee 100644 --- a/frontend/src/v2/icons/platforms/index.tsx +++ b/frontend/src/v2/icons/platforms/index.tsx @@ -104,6 +104,7 @@ const GoogleCalendar = () => ( export const PLATFORM_GLYPHS: Record = { github: GitHub, + 'github-mcp': GitHub, linear: Linear, 'google-calendar': GoogleCalendar, atlassian: Atlassian, diff --git a/k8s/helm/commonly/templates/core/backend-deployment.yaml b/k8s/helm/commonly/templates/core/backend-deployment.yaml index 09aadf4afd..65b139516f 100644 --- a/k8s/helm/commonly/templates/core/backend-deployment.yaml +++ b/k8s/helm/commonly/templates/core/backend-deployment.yaml @@ -83,6 +83,8 @@ spec: value: {{ .Values.hostedMcp.hiddenEntries | toJson | quote }} - name: HOSTED_MCP_HIDDEN_ENTRY_USER_ALLOWLIST value: {{ .Values.hostedMcp.hiddenEntryUserAllowlist | default dict | toJson | quote }} + - name: GITHUB_MCP_APP_SLUG + value: {{ .Values.hostedMcp.githubMcp.appSlug | default "" | quote }} {{- if .Values.errorTracking.enabled }} - name: SENTRY_DSN valueFrom: @@ -446,6 +448,21 @@ spec: key: google-calendar-client-secret optional: true + # GitHub hosted-MCP uses its own pre-registered App, separate from the + # sign-in app and room-grant broker. Missing keys leave it unavailable. + - name: GITHUB_MCP_CLIENT_ID + valueFrom: + secretKeyRef: + name: api-keys + key: github-mcp-client-id + optional: true + - name: GITHUB_MCP_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: api-keys + key: github-mcp-client-secret + optional: true + # Slack installable OAuth and per-workspace token encryption. Every # reference is optional so instances that have not enabled Slack's # OAuth ExternalSecret entries continue to start normally. diff --git a/k8s/helm/commonly/templates/ops/ops-runner-deployment.yaml b/k8s/helm/commonly/templates/ops/ops-runner-deployment.yaml index 1e8c169d99..e220d0ebcd 100644 --- a/k8s/helm/commonly/templates/ops/ops-runner-deployment.yaml +++ b/k8s/helm/commonly/templates/ops/ops-runner-deployment.yaml @@ -50,6 +50,8 @@ spec: value: {{ .Values.hostedMcp.hiddenEntries | toJson | quote }} - name: HOSTED_MCP_HIDDEN_ENTRY_USER_ALLOWLIST value: {{ .Values.hostedMcp.hiddenEntryUserAllowlist | default dict | toJson | quote }} + - name: GITHUB_MCP_APP_SLUG + value: {{ .Values.hostedMcp.githubMcp.appSlug | default "" | quote }} {{- if .Values.errorTracking.enabled }} - name: SENTRY_DSN valueFrom: @@ -413,6 +415,21 @@ spec: key: google-calendar-client-secret optional: true + # GitHub hosted-MCP uses its own pre-registered App, separate from the + # sign-in app and room-grant broker. Missing keys leave it unavailable. + - name: GITHUB_MCP_CLIENT_ID + valueFrom: + secretKeyRef: + name: api-keys + key: github-mcp-client-id + optional: true + - name: GITHUB_MCP_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: api-keys + key: github-mcp-client-secret + optional: true + # Slack installable OAuth and per-workspace token encryption. Every # reference is optional so instances that have not enabled Slack's # OAuth ExternalSecret entries continue to start normally. diff --git a/k8s/helm/commonly/templates/secrets/api-keys.yaml b/k8s/helm/commonly/templates/secrets/api-keys.yaml index af28c220d0..cc44620817 100644 --- a/k8s/helm/commonly/templates/secrets/api-keys.yaml +++ b/k8s/helm/commonly/templates/secrets/api-keys.yaml @@ -223,6 +223,16 @@ spec: remoteRef: key: {{ $keyPrefix }}google-calendar-client-secret {{- end }} + {{- if .Values.hostedMcp.githubMcp.enabled }} + # Pre-registered GitHub hosted-MCP client. Create both remote keys with an + # enabled version before enabling; ESO cannot skip a missing remoteRef. + - secretKey: github-mcp-client-id + remoteRef: + key: {{ $keyPrefix }}github-mcp-client-id + - secretKey: github-mcp-client-secret + remoteRef: + key: {{ $keyPrefix }}github-mcp-client-secret + {{- end }} - secretKey: groupme-bot-id remoteRef: key: {{ $keyPrefix }}groupme-bot-id diff --git a/k8s/helm/commonly/values-dev.yaml b/k8s/helm/commonly/values-dev.yaml index 92aa993591..c311e9d930 100644 --- a/k8s/helm/commonly/values-dev.yaml +++ b/k8s/helm/commonly/values-dev.yaml @@ -450,6 +450,9 @@ hostedMcp: # The 10-27 prod walk failed before Airtable's consent screen ("invalid client_id # or mismatched redirect_uri"). Hidden until TASK-318 lands and a walk passes. - airtable + # Hidden until a fresh non-admin account walks connect, grant, call and + # refusal end to end (TASK-290). That account joins the allowlist below. + - github-mcp # Mongo User _id(s) permitted to walk each hidden hosted-MCP entry (#2649). hiddenEntryUserAllowlist: atlassian: @@ -460,6 +463,11 @@ hostedMcp: - "6a47891ee025ce2e0ca1d30b" googleCalendar: enabled: false + # Commonly Connect App's client ID and secret each have an enabled version + # in Secret Manager (verified 2026-10-19). The backend refs remain optional. + githubMcp: + enabled: true + appSlug: commonly-connect # The team's own accounts, left out of the "typed and got no reply" alert, come # from the secret store (INTERNAL_ACCOUNTS, #2373). The key exists, and on diff --git a/k8s/helm/commonly/values.yaml b/k8s/helm/commonly/values.yaml index a2b6cf18b6..2b74cac489 100644 --- a/k8s/helm/commonly/values.yaml +++ b/k8s/helm/commonly/values.yaml @@ -78,6 +78,14 @@ hostedMcp: hiddenEntryUserAllowlist: {} googleCalendar: enabled: false + # Enable only after both pre-registered GitHub App client secrets exist with + # enabled versions in Secret Manager. A missing remoteRef stalls api-keys. + githubMcp: + enabled: false + # The public slug of the same App (github.com/apps/). It names the + # install page where a person picks the repositories the App's user token + # can reach. Empty: no install link is offered. + appSlug: "" # Agent provisioning (RBAC + PVCs) agentProvisioning: