Skip to content

Commit aac1bb8

Browse files
committed
fix(web): reject repository-scoped access tokens in skill tools
Scoped tokens are documented to grant access to selected repositories only, so the skill tools now reject that principal inside each handler and the MCP server skips registering them for scoped sessions. The handler check is the real gate: MCP sessions are keyed by owner, not principal, so a session created with a full credential can later be driven by a scoped token.
1 parent 1c8368d commit aac1bb8

10 files changed

Lines changed: 94 additions & 16 deletions

File tree

docs/docs/features/mcp-server.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ Parameters:
457457

458458
Creates a new agent skill: a reusable, named instruction set the user can invoke in Ask Sourcebot as a `/<slug>` slash command or have loaded automatically when a request matches its description. The skill is personal to the authenticated user and enabled immediately. The result includes a link to the skill in **Settings → Skills**.
459459

460-
Requires an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions do not see this tool.
460+
Requires an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions and repository-scoped access tokens cannot use this tool.
461461

462462
Parameters:
463463
| Name | Required | Description |
@@ -471,7 +471,7 @@ Parameters:
471471

472472
Updates an existing agent skill in place. Omitted content fields keep their current values. Personal skills are editable by their owner. Shared skills are editable only by the user who created them, and only while enabled. Skills synced from a repository file are rejected and must be edited in **Settings → Skills**. This tool never enables/disables a skill and never moves it between the personal and shared catalogs.
473473

474-
Requires an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions do not see this tool.
474+
Requires an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions and repository-scoped access tokens cannot use this tool.
475475

476476
Parameters:
477477
| Name | Required | Description |
@@ -487,7 +487,7 @@ Parameters:
487487

488488
Lists the agent skills visible to the authenticated user: their personal skills plus the organization's shared skill catalog. Each row includes `slug` and `scope` (the identifier pair `update_skill` needs), `enabled`, `isSynced` (linked to a repository file), `canEdit` (whether `update_skill` can edit it), and, on shared rows, `adopted`. Skill instructions are never included.
489489

490-
Requires an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions do not see this tool.
490+
Requires an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions and repository-scoped access tokens cannot use this tool.
491491

492492
Parameters:
493493
| Name | Required | Description |

packages/web/src/app/api/(server)/ee/mcp/route.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
7575
}
7676

7777
const response = await sew(() =>
78-
withOptionalAuth(async ({ user }) => {
78+
withOptionalAuth(async ({ user, principal }) => {
7979
if (env.EXPERIMENT_ASK_GH_ENABLED === 'true' && !user) {
8080
return notAuthenticated();
8181
}
@@ -111,7 +111,13 @@ export const POST = apiHandler(async (request: NextRequest) => {
111111
},
112112
});
113113

114-
const mcpServer = await createMcpServer({ isAuthenticated: ownerId !== null });
114+
// Repository-scoped access tokens never get the skill management
115+
// tools: their documented authorization boundary is the selected
116+
// repositories only. The tools also reject scoped principals
117+
// per-request, since a session is keyed by owner, not principal.
118+
const mcpServer = await createMcpServer({
119+
canManageSkills: ownerId !== null && principal?.source !== 'scoped_access_token',
120+
});
115121
await mcpServer.connect(transport);
116122

117123
return transport.handleRequest(request);

packages/web/src/ee/features/mcp/server.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131

3232
const dedent = _dedent.withOptions({ alignValues: true });
3333

34-
export async function createMcpServer({ isAuthenticated }: { isAuthenticated: boolean }): Promise<McpServer> {
34+
export async function createMcpServer({ canManageSkills }: { canManageSkills: boolean }): Promise<McpServer> {
3535
// Defense-in-depth: the MCP server is a paid feature. The /api/ee/mcp route
3636
// gates on the `mcp` entitlement before calling this; this assertion
3737
// backstops that contract so the server can't be constructed on a
@@ -64,10 +64,13 @@ export async function createMcpServer({ isAuthenticated }: { isAuthenticated: bo
6464
registerMcpTool(server, findSymbolReferencesDefinition, toolContext);
6565

6666
// The skill management tools require an authenticated user (skills are
67-
// per-user) and the Ask feature. Registration is per session and the
68-
// session owner is fixed, so this gate is exact; withAuth inside each
69-
// tool's execute remains the real enforcement.
70-
if (isAuthenticated && await hasEntitlement('ask')) {
67+
// per-user) whose credential is not repository-scoped (scoped access
68+
// tokens grant repo access only, never account-level skill management),
69+
// plus the Ask feature. Registration is best-effort UX: sessions are keyed
70+
// by owner, not principal, so the per-request checks inside each tool's
71+
// execute (withAuth + the scoped-token rejection) remain the real
72+
// enforcement.
73+
if (canManageSkills && await hasEntitlement('ask')) {
7174
registerMcpTool(server, createSkillDefinition, toolContext);
7275
registerMcpTool(server, updateSkillDefinition, toolContext);
7376
registerMcpTool(server, listSkillsDefinition, toolContext);

packages/web/src/features/tools/createSkill.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ beforeEach(() => {
5454
org: { id: 1 },
5555
user: { id: "user-1" },
5656
prisma: {},
57+
principal: { source: "api_key" },
5758
};
5859
mocks.withAuth.mockImplementation(async (callback: (context: unknown) => unknown) => callback(mocks.authContext));
5960
mocks.checkAskEntitlement.mockResolvedValue(null);
@@ -104,6 +105,19 @@ describe("createSkillDefinition", () => {
104105
.rejects.toThrow("Authentication is required to create skills.");
105106
});
106107

108+
test("rejects a repository-scoped access token without creating", async () => {
109+
mocks.authContext = {
110+
org: { id: 1 },
111+
user: { id: "user-1" },
112+
prisma: {},
113+
principal: { source: "scoped_access_token" },
114+
};
115+
116+
await expect(createSkillDefinition.execute(validInput, { source: "sourcebot-mcp-server" }))
117+
.rejects.toThrow("Repository-scoped access tokens cannot manage skills.");
118+
expect(mocks.createPersonalAgentSkillForContext).not.toHaveBeenCalled();
119+
});
120+
107121
test("throws the entitlement message when Ask is not available", async () => {
108122
mocks.checkAskEntitlement.mockResolvedValue({
109123
statusCode: StatusCodes.FORBIDDEN,

packages/web/src/features/tools/createSkill.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { sew } from "@/middleware/sew";
77
import { withAuth } from "@/middleware/withAuth";
88
import { ToolDefinition } from "./types";
99
import { logger } from "./logger";
10-
import { skillSettingsUrl, toSkillToolError, toSkillAnalyticsSource } from "./skillToolShared";
10+
import { scopedTokensCannotManageSkills, skillSettingsUrl, toSkillToolError, toSkillAnalyticsSource } from "./skillToolShared";
1111
import description from "./createSkill.txt";
1212

1313
// Plain described strings rather than the piped slug schema: the AI SDK
@@ -45,7 +45,11 @@ export const createSkillDefinition: ToolDefinition<"create_skill", typeof create
4545
}
4646

4747
const result = await sew(() =>
48-
withAuth(async ({ org, user, prisma }) => {
48+
withAuth(async ({ org, user, prisma, principal }) => {
49+
if (principal.source === 'scoped_access_token') {
50+
return scopedTokensCannotManageSkills();
51+
}
52+
4953
const askError = await checkAskEntitlement();
5054
if (askError) {
5155
return askError;

packages/web/src/features/tools/listSkills.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ beforeEach(() => {
6868
org: { id: 1 },
6969
user: { id: "user-1" },
7070
prisma,
71+
principal: { source: "api_key" },
7172
};
7273
mocks.withAuth.mockImplementation(async (callback: (context: unknown) => unknown) => callback(mocks.authContext));
7374
mocks.checkAskEntitlement.mockResolvedValue(null);
@@ -120,6 +121,19 @@ describe("listSkillsDefinition", () => {
120121
expect(result.metadata).toEqual({ count: 0 });
121122
});
122123

124+
test("rejects a repository-scoped access token without listing", async () => {
125+
mocks.authContext = {
126+
org: { id: 1 },
127+
user: { id: "user-1" },
128+
prisma,
129+
principal: { source: "scoped_access_token" },
130+
};
131+
132+
await expect(listSkillsDefinition.execute({}, { source: "sourcebot-mcp-server" }))
133+
.rejects.toThrow("Repository-scoped access tokens cannot manage skills.");
134+
expect(prisma.agentSkill.findMany).not.toHaveBeenCalled();
135+
});
136+
123137
test("the scope filter limits the query to one catalog", async () => {
124138
prisma.agentSkill.findMany.mockResolvedValueOnce([personalRow()]);
125139

packages/web/src/features/tools/listSkills.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { sew } from "@/middleware/sew";
66
import { withAuth } from "@/middleware/withAuth";
77
import { ToolDefinition } from "./types";
88
import { logger } from "./logger";
9-
import { toSkillToolError } from "./skillToolShared";
9+
import { scopedTokensCannotManageSkills, toSkillToolError } from "./skillToolShared";
1010
import description from "./listSkills.txt";
1111

1212
const listSkillsShape = {
@@ -29,7 +29,11 @@ export const listSkillsDefinition: ToolDefinition<"list_skills", typeof listSkil
2929
logger.debug('list_skills', input);
3030

3131
const result = await sew(() =>
32-
withAuth(async ({ org, user, prisma }) => {
32+
withAuth(async ({ org, user, prisma, principal }) => {
33+
if (principal.source === 'scoped_access_token') {
34+
return scopedTokensCannotManageSkills();
35+
}
36+
3337
const askError = await checkAskEntitlement();
3438
if (askError) {
3539
return askError;

packages/web/src/features/tools/skillToolShared.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { env } from "@sourcebot/shared";
22
import { ErrorCode } from "@/lib/errorCodes";
33
import type { AskSkillAnalyticsSource } from "@/lib/posthogEvents";
44
import type { ServiceError } from "@/lib/serviceError";
5+
import { StatusCodes } from "http-status-codes";
56

67
// Helpers shared by the skill management tools (create_skill, update_skill,
78
// list_skills).
@@ -11,6 +12,18 @@ export const toSkillAnalyticsSource = (source: string | undefined): AskSkillAnal
1112
? source
1213
: 'sourcebot-ask-agent';
1314

15+
// Repository-scoped access tokens grant access to selected repositories only;
16+
// account-level skill management is outside their documented authorization
17+
// boundary. Every skill tool rejects them in its handler regardless of
18+
// registration-time gating: an MCP session is keyed by its owner, not its
19+
// principal, so a session created with a full credential can later be driven
20+
// by a scoped token for the same user.
21+
export const scopedTokensCannotManageSkills = (): ServiceError => ({
22+
statusCode: StatusCodes.FORBIDDEN,
23+
errorCode: ErrorCode.INSUFFICIENT_PERMISSIONS,
24+
message: "Repository-scoped access tokens cannot manage skills.",
25+
});
26+
1427
// The settings page supports ?skill= deep links that open the given skill.
1528
export const skillSettingsUrl = (skillId: string): string =>
1629
`${env.AUTH_URL.replace(/\/$/, '')}/settings/skills?skill=${encodeURIComponent(skillId)}`;

packages/web/src/features/tools/updateSkill.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ beforeEach(() => {
4747
org: { id: 1 },
4848
user: { id: "user-1" },
4949
prisma: {},
50+
principal: { source: "api_key" },
5051
};
5152
mocks.withAuth.mockImplementation(async (callback: (context: unknown) => unknown) => callback(mocks.authContext));
5253
mocks.checkAskEntitlement.mockResolvedValue(null);
@@ -121,6 +122,21 @@ describe("updateSkillDefinition", () => {
121122
)).rejects.toThrow("Authentication is required to update skills.");
122123
});
123124

125+
test("rejects a repository-scoped access token without updating", async () => {
126+
mocks.authContext = {
127+
org: { id: 1 },
128+
user: { id: "user-1" },
129+
prisma: {},
130+
principal: { source: "scoped_access_token" },
131+
};
132+
133+
await expect(updateSkillDefinition.execute(
134+
{ slug: "review-pr", scope: "personal", name: "Renamed" },
135+
{ source: "sourcebot-mcp-server" },
136+
)).rejects.toThrow("Repository-scoped access tokens cannot manage skills.");
137+
expect(mocks.updateAgentSkillForContext).not.toHaveBeenCalled();
138+
});
139+
124140
test("returns the updated skill as JSON output plus UI metadata with the tool's scope", async () => {
125141
const result = await updateSkillDefinition.execute(
126142
{ slug: "review-pr", scope: "personal", name: "Review PR" },

packages/web/src/features/tools/updateSkill.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { sew } from "@/middleware/sew";
77
import { withAuth } from "@/middleware/withAuth";
88
import { ToolDefinition } from "./types";
99
import { logger } from "./logger";
10-
import { skillSettingsUrl, toSkillToolError, toSkillAnalyticsSource } from "./skillToolShared";
10+
import { scopedTokensCannotManageSkills, skillSettingsUrl, toSkillToolError, toSkillAnalyticsSource } from "./skillToolShared";
1111
import description from "./updateSkill.txt";
1212

1313
// Same plain-string style as create_skill: the merged result is validated in
@@ -43,7 +43,11 @@ export const updateSkillDefinition: ToolDefinition<"update_skill", typeof update
4343
const { slug, scope, name, newSlug, description: newDescription, instructions } = input;
4444

4545
const result = await sew(() =>
46-
withAuth(async ({ org, user, prisma }) => {
46+
withAuth(async ({ org, user, prisma, principal }) => {
47+
if (principal.source === 'scoped_access_token') {
48+
return scopedTokensCannotManageSkills();
49+
}
50+
4751
const askError = await checkAskEntitlement();
4852
if (askError) {
4953
return askError;

0 commit comments

Comments
 (0)