Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions apps/web/src/routers/user-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,32 @@ let testUser: User;
let surveyTestUser: User;
let skipTestUser: User;

describe('user router - getMe', () => {
it('getMe returns isAdmin: true for an admin user', async () => {
const admin = await insertTestUser({ is_admin: true });
const caller = await createCallerForUser(admin.id);

await expect(caller.user.getMe()).resolves.toEqual({
success: true,
id: admin.id,
email: admin.google_user_email,
isAdmin: true,
});
});

it('getMe returns isAdmin: false for a non-admin user', async () => {
const user = await insertTestUser();
const caller = await createCallerForUser(user.id);

await expect(caller.user.getMe()).resolves.toEqual({
success: true,
id: user.id,
email: user.google_user_email,
isAdmin: false,
});
});
});

describe('user router - updateProfile', () => {
beforeAll(async () => {
testUser = await insertTestUser({
Expand Down
6 changes: 5 additions & 1 deletion apps/web/src/routers/user-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,11 @@ async function computeNotificationCapabilities(userId: string): Promise<Notifica
export const userRouter = createTRPCRouter({
// Account linking routes
getMe: baseProcedure.query(async ({ ctx }) => {
return successResult({ id: ctx.user.id, email: ctx.user.google_user_email });
return successResult({
id: ctx.user.id,
email: ctx.user.google_user_email,
isAdmin: ctx.user.is_admin,
});
}),

getAuthProviders: baseProcedure.query(async ({ ctx }) => {
Expand Down
132 changes: 129 additions & 3 deletions apps/web/src/scripts/mcp-catalog/catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
generateMissingSummaries,
isDenylistedPath,
parseKiloCompletion,
procedureRequiresAdmin,
readCommittedSummaries,
runKiloCompletion,
type CatalogLeaf,
Expand Down Expand Up @@ -99,7 +100,9 @@ describe('mcp-catalog catalog', () => {
);
// Every non-internal query and mutation with a summary is a row; a
// denylisted segment anywhere in the path and subscriptions are dropped,
// summary or not. `testConnection` stays because `test` is exact-only.
// summary or not. `testConnection` stays because `test` is exact-only,
// and the summary-less `debug.ping` is published and waits for the LLM
// pass because `debug` stays published and guarded.
expect(rows.map(row => row.path)).toEqual([
'organizations.subscription.cancel',
'user.deleteAccount',
Expand All @@ -114,18 +117,19 @@ describe('mcp-catalog catalog', () => {
'mutation',
'query',
]);
expect(missing).toEqual([]);
expect(missing.map(leaf => leaf.path)).toEqual(['debug.ping']);
});

it('treats an internal segment anywhere in the path as internal', () => {
expect(isDenylistedPath('admin.users.list')).toBe(true);
expect(isDenylistedPath('organizations.admin.grantCredit')).toBe(true);
expect(isDenylistedPath('codingPlans.adminInsights')).toBe(true);
expect(isDenylistedPath('debug.ping')).toBe(true);
expect(isDenylistedPath('test.echo')).toBe(true);
expect(isDenylistedPath('slack.devRemoveDbRowOnly')).toBe(true);
expect(isDenylistedPath('user.getProfile')).toBe(false);
expect(isDenylistedPath('slack.testConnection')).toBe(false);
// `debug` is published and carries the guard marker, so it is not internal.
expect(isDenylistedPath('debug.ping')).toBe(false);
});

it('fails on a zero-row catalog instead of emitting an empty one', () => {
Expand Down Expand Up @@ -245,6 +249,128 @@ describe('mcp-catalog catalog', () => {
);
expect(rows[0]?.summary).toBe(summary);
});

it('marks a real admin-guarded procedure with the trailing admin marker', () => {
const { rows } = buildCatalogRows(
withMutation([queryLeaf('organizations.seatPurchases')]),
new Map([['organizations.seatPurchases', 'Lists seat purchases.']])
);
const row = rows[0];
expect(row?.admin).toBe(true);
// Key order stays deterministic for the dump's byte-for-byte check.
expect(Object.keys(row ?? {})).toEqual([
'path',
'kind',
'summary',
'inputSchema',
'tags',
'searchBlob',
'admin',
]);
});

it('leaves the admin key off a real non-admin procedure', () => {
const { rows } = buildCatalogRows(
withMutation([queryLeaf('user.getBalance')]),
new Map([['user.getBalance', 'Returns the credit balance.']])
);
expect(rows[0]).not.toHaveProperty('admin');
expect(Object.keys(rows[0] ?? {})).toEqual([
'path',
'kind',
'summary',
'inputSchema',
'tags',
'searchBlob',
]);
});

it('keeps a debug leaf and marks it debug without an admin mark', () => {
const { rows, missing } = buildCatalogRows(
withMutation([queryLeaf('debug.ping')]),
new Map([['debug.ping', 'Pings the debug router.']])
);
const row = rows[0];
// The fixture's own leaves are published; the mutation added to satisfy
// the drift guard carries no summary, so it waits for the LLM pass.
expect(missing.map(leaf => leaf.path)).toEqual(['user.updateProfile']);
expect(row?.debug).toBe(true);
expect(row).not.toHaveProperty('admin');
// Key order stays deterministic for the dump's byte-for-byte check.
expect(Object.keys(row ?? {})).toEqual([
'path',
'kind',
'summary',
'inputSchema',
'tags',
'searchBlob',
'debug',
]);
});

it('marks an admin-guarded debug procedure with both marks', () => {
const { rows } = buildCatalogRows(
withMutation([queryLeaf('debug.badInputError')]),
new Map([['debug.badInputError', 'Echoes a short string back from the debug router.']])
);
const row = rows[0];
expect(row?.debug).toBe(true);
expect(row?.admin).toBe(true);
// Both marks are emitted at the tail in a fixed order, admin first.
expect(Object.keys(row ?? {})).toEqual([
'path',
'kind',
'summary',
'inputSchema',
'tags',
'searchBlob',
'admin',
'debug',
]);
});

it('still drops an admin leaf', () => {
const { rows, missing } = buildCatalogRows(
withMutation([queryLeaf('admin.users.list'), queryLeaf('user.getProfile')]),
new Map([
['admin.users.list', 'Lists users.'],
['user.getProfile', 'Returns the profile of a user.'],
])
);
expect(rows.map(row => row.path)).toEqual(['user.getProfile']);
expect(missing.map(leaf => leaf.path)).toEqual(['user.updateProfile']);
});

it('still drops a test leaf', () => {
const { rows, missing } = buildCatalogRows(
withMutation([queryLeaf('test.echo'), queryLeaf('user.getProfile')]),
new Map([
['test.echo', 'Echoes a payload.'],
['user.getProfile', 'Returns the profile of a user.'],
])
);
expect(rows.map(row => row.path)).toEqual(['user.getProfile']);
expect(missing.map(leaf => leaf.path)).toEqual(['user.updateProfile']);
});
});

describe('procedureRequiresAdmin', () => {
const cases: Array<[string | null, boolean]> = [
['adminProcedure.input(z.object({})).query(async () => ({}))', true],
['creditManagerProcedure.query(async () => ({}))', true],
['superadminProcedure.query(async () => ({}))', true],
['sessionViewerProcedure.query(async () => ({}))', true],
['baseProcedure.query(async () => ({}))', false],
['protectedProcedure.query(async () => ({}))', false],
// A guard elsewhere in the chain does not make the procedure admin-only.
['baseProcedure.use(adminProcedure).query(async () => ({}))', false],
['notAdminProcedure.query(async () => ({}))', false],
[null, false],
];

it.each(cases)('treats %s as admin-guarded: %s', (source, expected) => {
expect(procedureRequiresAdmin(source)).toBe(expected);
});
});

describe('CATALOG_JSON_DISPLAY_PATH', () => {
Expand Down
76 changes: 61 additions & 15 deletions apps/web/src/scripts/mcp-catalog/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
* entry point.
*
* Queries and mutations are published wholesale; paths with an internal-only
* segment (`admin`, `debug`, `dev`, `test`) and subscriptions stay internal.
* segment (`admin`, `dev`, `test`) and subscriptions stay internal. `debug`
* paths are published too, marked with `debug: true` for the guard.
*/
import { spawnSync } from 'node:child_process';
import { readFileSync, statSync } from 'node:fs';
Expand Down Expand Up @@ -38,25 +39,33 @@ export const ROOT_ROUTER_PATH = join(__dirname, '..', '..', 'routers', 'root-rou

/**
* Whether a procedure path is internal-only. A path is internal when any
* segment is `admin`, `debug`, `dev`, or `test`, or starts with `admin`,
* `debug`, or `dev` — the routers name admin and dev-only procedures `adminX`
* and `devX` (for example `organizations.admin.grantCredit` and
* `slack.devRemoveDbRowOnly`). `test` stays exact-only so user-facing calls
* such as `slack.testConnection` remain published. Subscriptions are never
* exported regardless of this check.
* segment is `test` or starts with `admin` or `dev` — the routers name admin
* and dev-only procedures `adminX` and `devX` (for example
* `organizations.admin.grantCredit` and `slack.devRemoveDbRowOnly`). `test`
* stays exact-only so user-facing calls such as `slack.testConnection` remain
* published, and `debug` stays published too: its rows carry a `debug: true`
* marker and are guarded wherever they are offered or called. Subscriptions
* are never exported regardless of this check.
*/
export function isDenylistedPath(path: string): boolean {
return path.split('.').some(segment => {
const lower = segment.toLowerCase();
return (
lower === 'test' ||
lower.startsWith('admin') ||
lower.startsWith('debug') ||
lower.startsWith('dev')
);
return lower === 'test' || lower.startsWith('admin') || lower.startsWith('dev');
});
}

/**
* Procedure builders that reject non-admin users. `apps/web/src/lib/trpc/init.ts`
* builds every admin-only procedure from `adminProcedure`, and the other three
* variants chain on it, so a chain whose head is any of these is admin-guarded.
*/
export const ADMIN_GUARD_PROCEDURES = [
'adminProcedure',
'creditManagerProcedure',
'superadminProcedure',
'sessionViewerProcedure',
] as const;

/** Instruction every generated summary must follow. */
export const SUMMARY_INSTRUCTION =
'Write a 1-2 sentence search-friendly summary of what this call does, in words an agent would type to find it. No implementation detail. Not a restatement of the path.';
Expand Down Expand Up @@ -90,6 +99,17 @@ export type CatalogRow = {
inputSchema: Record<string, unknown>;
tags: string[];
searchBlob: string;
/**
* Emitted only on rows whose procedure sits behind an admin guard;
* absent means every grant may use the row.
*/
admin?: true;
/**
* Emitted only on rows whose procedure's top-level segment is `debug`;
* absent means the row is not a debug endpoint. A debug row behind an admin
* guard carries both marks.
*/
debug?: true;
};

/** Failure while generating summaries. `retryable` failures name the failed batch. */
Expand Down Expand Up @@ -249,7 +269,7 @@ function deriveTags(segments: string[], schemaKeys: string[]): string[] {
return tags;
}

function shapeRow(leaf: CatalogLeaf, summary: string): CatalogRow {
function shapeRow(leaf: CatalogLeaf, summary: string, admin: boolean, debug: boolean): CatalogRow {
const segments = leaf.path.split('.');
const inputSchema = toInputSchema(leaf.inputs);
const schemaKeys = topSchemaKeys(inputSchema);
Expand All @@ -261,6 +281,10 @@ function shapeRow(leaf: CatalogLeaf, summary: string): CatalogRow {
inputSchema,
tags,
searchBlob: [leaf.path, summary, ...tags, ...schemaKeys].filter(Boolean).join(' '),
// Emitted at the tail and only when true, in a fixed order, so the dump's
// byte-for-byte check stays deterministic for every other row.
...(admin ? { admin: true } : {}),
...(debug ? { debug: true } : {}),
};
}

Expand All @@ -283,6 +307,9 @@ export function buildCatalogRows(
summaries: Map<string, string> = new Map()
): { rows: CatalogRow[]; missing: CatalogLeaf[] } {
let exposedMutations = 0;
// Resolved once for the whole catalog: the guard marker is decided from each
// procedure's own extracted source, never the whole router file.
const topLevelFiles = extractTopLevelRouterFiles();
const rows: CatalogRow[] = [];
const missing: CatalogLeaf[] = [];
for (const leaf of leaves) {
Expand All @@ -291,7 +318,14 @@ export function buildCatalogRows(
if (leaf.type === 'mutation') exposedMutations += 1;
const summary = summaries.get(leaf.path);
if (typeof summary === 'string' && summary !== '') {
rows.push(shapeRow(leaf, summary));
// An extraction miss counts as non-admin: never hide an endpoint by accident.
const admin = procedureRequiresAdmin(
extractProcedureSource(leaf.path, topLevelFiles)?.source ?? null
);
// The debug mark comes from the leaf's own top-level segment, so it never
// depends on static extraction succeeding.
const debug = leaf.path.split('.')[0] === 'debug';
rows.push(shapeRow(leaf, summary, admin, debug));
} else {
missing.push(leaf);
}
Expand Down Expand Up @@ -597,6 +631,18 @@ export function extractProcedureSource(
return { file: currentFile, source: block };
}

/**
* True when a procedure's extracted value expression is guarded by an admin
* procedure builder. The guard must head the chain (`adminProcedure.input(…)`),
* so a base procedure that merely mentions a guard in its body is not marked;
* a null source (extraction failure) is not admin, because hiding a non-admin
* endpoint would be the worse mistake.
*/
export function procedureRequiresAdmin(source: string | null): boolean {
if (source === null) return false;
return ADMIN_GUARD_PROCEDURES.some(guard => new RegExp(`^\\s*${guard}\\b`).test(source));
}

function capContext(text: string, limit: number): string {
return text.length <= limit ? text : `${text.slice(0, limit)}\n// … truncated`;
}
Expand Down
2 changes: 1 addition & 1 deletion services/ai-attribution/drizzle/meta/_journal.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
"breakpoints": true
}
]
}
}
Loading