diff --git a/README.md b/README.md index a48ac5584..0c4e1cf39 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,7 @@ Currently applied to: - `list_pipelines`, `get_pipeline` - `list_releases` - `list_commits`, `get_commit` +- `get_current_user`, `get_user`, `get_users`, `search_users` A spike measurement against `list_projects` with 5 owned projects went from **~32 KB → ~3 KB** by switching to the compact default. Because it's allow-list based, the compact output stays compact when GitLab adds new fields upstream. diff --git a/src/schemas/users.ts b/src/schemas/users.ts index 4fcfef7f1..3dd55afd8 100644 --- a/src/schemas/users.ts +++ b/src/schemas/users.ts @@ -43,3 +43,35 @@ export const GitLabUserSchema = z .passthrough(); export type GitLabUser = z.infer; + +export const GitLabUserListSchema = z.array(GitLabUserSchema); + +/** + * Slim shape (Phase 3b / DOT-560): identity-only fields safe to expose for + * any GitLab user. Deliberately excludes every privacy-sensitive field + * GitLab returns for the current user (email, last_sign_in_at, is_admin, + * two_factor_enabled, confirmed_at, current_sign_in_at, private_profile, + * last_activity_on, theme_id, color_scheme_id, projects_limit, external) + * and the bulky public-profile fields (bio, location, organization, + * job_title, work_information, pronouns, followers, following). Callers + * opt back in via `fields: "all"` or a custom `fields: [...]` allow-list. + * + * `bot` is included because the LLM often needs to know whether to treat + * the user as a service account vs. a person. Always safe to expose. + */ +export const UserSlimShape = { + id: true, + username: true, + name: true, + state: true, + avatar_url: true, + web_url: true, + bot: true, +} as const; + +export const GitLabUserSlimSchema = GitLabUserSchema.pick(UserSlimShape); +export type GitLabUserSlim = z.infer; + +export const USER_SLIM_FIELDS = Object.keys(UserSlimShape) as ReadonlyArray< + keyof typeof UserSlimShape +>; diff --git a/src/tools/users.ts b/src/tools/users.ts index f8e6a0cf4..7dd7cee43 100644 --- a/src/tools/users.ts +++ b/src/tools/users.ts @@ -1,22 +1,31 @@ import type { McpServer, RegisteredTool } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; import { parseGitLabResponse } from "../schemas/parse.js"; -import { GitLabUserSchema } from "../schemas/users.js"; +import { GitLabUserListSchema, GitLabUserSchema, USER_SLIM_FIELDS } from "../schemas/users.js"; import { buildQueryString, defaultClient, resolveProjectId } from "../utils/gitlab-client.js"; import type { Logger } from "../utils/logger.js"; +import { projectField, projectFields } from "../utils/projection.js"; +import { fieldsParam } from "../utils/schema-helpers.js"; const GetUsersSchema = z.object({ usernames: z.array(z.string()).describe("List of usernames to look up"), + fields: fieldsParam("user").optional(), }); const GetUserSchema = z.object({ user_id: z.number().describe("User ID"), + fields: fieldsParam("user").optional(), }); const SearchUsersSchema = z.object({ search: z.string().describe("Search query"), page: z.number().optional().describe("Page number"), per_page: z.number().optional().describe("Results per page"), + fields: fieldsParam("user").optional(), +}); + +const GetCurrentUserSchema = z.object({ + fields: fieldsParam("user").optional(), }); const ListEventsSchema = z.object({ @@ -101,9 +110,11 @@ export function registerUserTools(server: McpServer, logger: Logger): Map(`/users${query}`); + const raw = await defaultClient.get(`/users${query}`); + const users = parseGitLabResponse(GitLabUserListSchema, raw, "get_users", logger); // Always assign — null when the username didn't resolve. Omitting // missing keys would collapse signal: callers couldn't tell whether // a key wasn't asked for or actually didn't exist on GitLab. - results[username] = users.length > 0 ? users[0] : null; + if (users.length === 0) { + results[username] = null; + continue; + } + results[username] = projectField( + users[0] as unknown as Record, + USER_SLIM_FIELDS, + args.fields, + ); } return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; @@ -133,9 +153,11 @@ export function registerUserTools(server: McpServer, logger: Logger): Map, + USER_SLIM_FIELDS, + args.fields, + ); + return { content: [{ type: "text", text: JSON.stringify(projected, null, 2) }] }; }, ); toolRef2.disable(); @@ -156,11 +183,13 @@ export function registerUserTools(server: McpServer, logger: Logger): Map { const args = SearchUsersSchema.parse(params); - const query = buildQueryString(args); + const { fields, ...queryParams } = args; + const query = buildQueryString(queryParams); - const users = await defaultClient.get(`/users${query}`); - return { content: [{ type: "text", text: JSON.stringify(users, null, 2) }] }; + const raw = await defaultClient.get(`/users${query}`); + const users = parseGitLabResponse( + GitLabUserListSchema, + raw, + "search_users", + logger, + ) as unknown as Record[]; + const projected = projectFields(users, USER_SLIM_FIELDS, fields); + return { content: [{ type: "text", text: JSON.stringify(projected, null, 2) }] }; }, ); toolRef3.disable(); @@ -366,17 +403,25 @@ export function registerUserTools(server: McpServer, logger: Logger): Map { + async (params) => { + const args = GetCurrentUserSchema.parse(params); const raw = await defaultClient.get(`/user`); const user = parseGitLabResponse(GitLabUserSchema, raw, "get_current_user", logger); - return { content: [{ type: "text", text: JSON.stringify(user, null, 2) }] }; + const projected = projectField( + user as unknown as Record, + USER_SLIM_FIELDS, + args.fields, + ); + return { content: [{ type: "text", text: JSON.stringify(projected, null, 2) }] }; }, ); toolRef8.disable(); diff --git a/tests/schemas/users.test.ts b/tests/schemas/users.test.ts index 935a199b4..a7950972a 100644 --- a/tests/schemas/users.test.ts +++ b/tests/schemas/users.test.ts @@ -1,5 +1,12 @@ import { describe, expect, it } from "bun:test"; -import { GitLabUserSchema } from "../../src/schemas/users.js"; +import { + GitLabUserListSchema, + GitLabUserSchema, + GitLabUserSlimSchema, + USER_SLIM_FIELDS, + UserSlimShape, +} from "../../src/schemas/users.js"; +import { projectField } from "../../src/utils/projection.js"; import getCurrentUserFixture from "../fixtures/users/get_current_user.json"; import getUserFixture from "../fixtures/users/get_user.json"; @@ -28,3 +35,101 @@ describe("GitLabUserSchema", () => { expect(result.success).toBe(false); }); }); + +describe("GitLabUserSlimSchema (Phase 3b / DOT-560)", () => { + it("parses the get_user fixture and types every slim field", () => { + const result = GitLabUserSlimSchema.safeParse(getUserFixture); + expect(result.success).toBe(true); + if (result.success) { + for (const k of Object.keys(UserSlimShape)) { + if (k in (getUserFixture as Record)) { + expect(k in result.data).toBe(true); + } + } + } + }); + + it("USER_SLIM_FIELDS exactly mirrors UserSlimShape keys", () => { + expect([...USER_SLIM_FIELDS].sort()).toEqual(Object.keys(UserSlimShape).sort()); + }); + + it("projectField with USER_SLIM_FIELDS keeps only slim keys at runtime", () => { + // Use get_current_user fixture (has the admin/2FA fields populated) + const slim = projectField( + getCurrentUserFixture as Record, + USER_SLIM_FIELDS, + undefined, + ); + const allowed = new Set(USER_SLIM_FIELDS); + for (const k of Object.keys(slim)) { + expect(allowed.has(k)).toBe(true); + } + }); + + it("USER_SLIM_FIELDS does NOT include privacy-sensitive fields (guardrail)", () => { + // Pin the privacy invariant. If a future change adds any of these to + // UserSlimShape, this test fails loudly and forces a deliberate decision. + const privacyFields = [ + "email", + "public_email", + "last_sign_in_at", + "confirmed_at", + "last_activity_on", + "two_factor_enabled", + "current_sign_in_at", + "external", + "private_profile", + "is_admin", + "theme_id", + "color_scheme_id", + "projects_limit", + ]; + const slimSet = new Set(USER_SLIM_FIELDS); + for (const field of privacyFields) { + expect(slimSet.has(field)).toBe(false); + } + }); + + it("projectField on the current-user fixture drops every privacy-sensitive field", () => { + const slim = projectField( + getCurrentUserFixture as Record, + USER_SLIM_FIELDS, + undefined, + ) as Record; + for (const field of [ + "email", + "last_sign_in_at", + "confirmed_at", + "last_activity_on", + "two_factor_enabled", + "current_sign_in_at", + "external", + "private_profile", + "is_admin", + "theme_id", + "color_scheme_id", + "projects_limit", + ]) { + expect(slim[field]).toBeUndefined(); + } + }); + + it("parses an array of users via GitLabUserListSchema", () => { + const result = GitLabUserListSchema.safeParse([getUserFixture, getCurrentUserFixture]); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data).toHaveLength(2); + } + }); + + it("token budget: projected user is materially smaller than the full payload", () => { + const fullBytes = Buffer.byteLength(JSON.stringify(getCurrentUserFixture), "utf8"); + const slim = projectField( + getCurrentUserFixture as Record, + USER_SLIM_FIELDS, + undefined, + ); + const slimBytes = Buffer.byteLength(JSON.stringify(slim), "utf8"); + expect(slimBytes).toBeLessThanOrEqual(Math.floor(fullBytes * 0.8)); + }); +}); diff --git a/tests/users.test.ts b/tests/users.test.ts index 35f61441a..eeda12cba 100644 --- a/tests/users.test.ts +++ b/tests/users.test.ts @@ -192,4 +192,298 @@ describe("User Tools Handlers", () => { expect(parsed.authenticated).toBe(false); }); }); + + describe("get_current_user field projection (Phase 3b / DOT-560)", () => { + const fullUser = { + id: 42, + name: "Ismar", + username: "ismart", + state: "active", + avatar_url: null, + web_url: "https://gitlab.example/ismart", + created_at: "2020-01-01T00:00:00Z", + bot: false, + // Privacy-sensitive — must be absent from default slim output: + email: "ismart@example.com", + last_sign_in_at: "2026-05-15T08:00:00Z", + confirmed_at: "2020-01-01T00:00:00Z", + last_activity_on: "2026-05-15", + two_factor_enabled: true, + current_sign_in_at: "2026-05-15T08:00:00Z", + is_admin: false, + private_profile: false, + external: false, + }; + + function mockUserResponse(user: Record) { + // @ts-expect-error - mock doesn't need full fetch signature + globalThis.fetch = mock(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(JSON.stringify(user)), + } as Response), + ); + } + + it("returns identity-only fields by default (privacy fields stripped)", async () => { + mockUserResponse(fullUser); + const result = await client.callTool({ name: "get_current_user", arguments: {} }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data.username).toBe("ismart"); + expect(data.bot).toBe(false); + // Privacy guardrail + expect(data.email).toBeUndefined(); + expect(data.last_sign_in_at).toBeUndefined(); + expect(data.is_admin).toBeUndefined(); + expect(data.two_factor_enabled).toBeUndefined(); + expect(data.current_sign_in_at).toBeUndefined(); + expect(data.private_profile).toBeUndefined(); + }); + + it('exposes privacy fields when fields="all"', async () => { + mockUserResponse(fullUser); + const result = await client.callTool({ + name: "get_current_user", + arguments: { fields: "all" }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data.email).toBe("ismart@example.com"); + expect(data.is_admin).toBe(false); + expect(data.two_factor_enabled).toBe(true); + }); + + it("returns exactly the requested fields for a custom list", async () => { + mockUserResponse(fullUser); + const result = await client.callTool({ + name: "get_current_user", + arguments: { fields: ["id", "username", "email"] }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(Object.keys(data).sort()).toEqual(["email", "id", "username"]); + expect(data.email).toBe("ismart@example.com"); + }); + }); + + describe("get_user field projection (Phase 3b / DOT-560)", () => { + const fullUser = { + id: 7, + name: "Alice", + username: "alice", + state: "active", + avatar_url: null, + web_url: "https://gitlab.example/alice", + bot: false, + bio: "Engineer", + location: "Berlin", + organization: "ACME", + job_title: "SWE", + pronouns: "they/them", + work_information: "info", + followers: 12, + following: 5, + created_at: "2021-06-01T00:00:00Z", + }; + + function mockUserResponse(user: Record) { + // @ts-expect-error - mock doesn't need full fetch signature + globalThis.fetch = mock(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(JSON.stringify(user)), + } as Response), + ); + } + + it("returns only identity-default fields by default", async () => { + mockUserResponse(fullUser); + const result = await client.callTool({ + name: "get_user", + arguments: { user_id: 7 }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data.id).toBe(7); + expect(data.username).toBe("alice"); + // Bloat dropped + expect(data.bio).toBeUndefined(); + expect(data.organization).toBeUndefined(); + expect(data.followers).toBeUndefined(); + expect(data.created_at).toBeUndefined(); + }); + + it('returns the full payload when fields="all"', async () => { + mockUserResponse(fullUser); + const result = await client.callTool({ + name: "get_user", + arguments: { user_id: 7, fields: "all" }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data.bio).toBe("Engineer"); + expect(data.followers).toBe(12); + }); + + it("returns exactly the requested fields for a custom list", async () => { + mockUserResponse(fullUser); + const result = await client.callTool({ + name: "get_user", + arguments: { user_id: 7, fields: ["id", "name"] }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(Object.keys(data).sort()).toEqual(["id", "name"]); + }); + }); + + describe("get_users field projection (Phase 3b / DOT-560)", () => { + function mockPerUsername(byUsername: Record | null>) { + // @ts-expect-error - mock doesn't need full fetch signature + globalThis.fetch = mock((url: string) => { + const match = url.match(/username=([^&]+)/); + const username = match ? decodeURIComponent(match[1]) : ""; + const user = byUsername[username]; + return Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(JSON.stringify(user ? [user] : [])), + } as Response); + }); + } + + const alice = { + id: 1, + username: "alice", + name: "Alice", + state: "active", + avatar_url: null, + web_url: "https://gitlab.example/alice", + bot: false, + bio: "Engineer", + organization: "ACME", + }; + + it("slims each value in the map by default", async () => { + mockPerUsername({ alice, unknown: null }); + const result = await client.callTool({ + name: "get_users", + arguments: { usernames: ["alice", "unknown"] }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data.alice.username).toBe("alice"); + expect(data.alice.bio).toBeUndefined(); + expect(data.alice.organization).toBeUndefined(); + // null preserved for unresolved usernames + expect(data.unknown).toBeNull(); + }); + + it('preserves bloat when fields="all"', async () => { + mockPerUsername({ alice }); + const result = await client.callTool({ + name: "get_users", + arguments: { usernames: ["alice"], fields: "all" }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data.alice.bio).toBe("Engineer"); + expect(data.alice.organization).toBe("ACME"); + }); + + it("returns exactly the requested fields for a custom list", async () => { + mockPerUsername({ alice }); + const result = await client.callTool({ + name: "get_users", + arguments: { usernames: ["alice"], fields: ["id", "username"] }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(Object.keys(data.alice).sort()).toEqual(["id", "username"]); + }); + }); + + describe("search_users field projection (Phase 3b / DOT-560)", () => { + function mockUsersResponse(users: Record[]) { + // @ts-expect-error - mock doesn't need full fetch signature + globalThis.fetch = mock(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(JSON.stringify(users)), + } as Response), + ); + } + + const matches = [ + { + id: 1, + username: "alice", + name: "Alice", + state: "active", + avatar_url: null, + web_url: "https://gitlab.example/alice", + bot: false, + bio: "Engineer", + organization: "ACME", + }, + { + id: 2, + username: "alex", + name: "Alex", + state: "active", + avatar_url: null, + web_url: "https://gitlab.example/alex", + bot: false, + bio: "Designer", + organization: "Globex", + }, + ]; + + it("slims each match by default", async () => { + mockUsersResponse(matches); + const result = await client.callTool({ + name: "search_users", + arguments: { search: "al" }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data).toHaveLength(2); + expect(data[0].username).toBe("alice"); + expect(data[0].bio).toBeUndefined(); + expect(data[1].organization).toBeUndefined(); + }); + + it('preserves bloat when fields="all"', async () => { + mockUsersResponse(matches); + const result = await client.callTool({ + name: "search_users", + arguments: { search: "al", fields: "all" }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(data[0].bio).toBe("Engineer"); + }); + + it("respects a caller-supplied field allow-list", async () => { + mockUsersResponse(matches); + const result = await client.callTool({ + name: "search_users", + arguments: { search: "al", fields: ["id", "username"] }, + }); + const data = JSON.parse((result.content as TextContent)[0].text); + expect(Object.keys(data[0]).sort()).toEqual(["id", "username"]); + }); + + it("does not forward the fields param to GitLab as a query string", async () => { + let capturedUrl: string | undefined; + // @ts-expect-error - mock doesn't need full fetch signature + globalThis.fetch = mock((url: string) => { + capturedUrl = url; + return Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(JSON.stringify(matches)), + } as Response); + }); + await client.callTool({ + name: "search_users", + arguments: { search: "al", fields: ["id"] }, + }); + // `fields` is a server-side projection param, not a GitLab API param + expect(capturedUrl).not.toContain("fields="); + }); + }); });