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
4 changes: 2 additions & 2 deletions AGENTS.md

Large diffs are not rendered by default.

72 changes: 64 additions & 8 deletions packages/api/src/agent-side/tools.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {beforeEach, describe, expect, mock, test} from 'bun:test';
import {Err, Ok} from 'ts-results-es';
import {Err, Ok, type Result} from 'ts-results-es';

import {ApiErr} from '@proxy/utils';
import {ApiErr, type ApiError} from '@proxy/utils';
import {findIntegration, integrationFor, type Integration} from '@proxy/integrations';

const getAgent = mock();
Expand All @@ -12,10 +12,15 @@ const logAgentRequest = mock();

mock.module('@proxy/db/audit', () => ({logAgentRequest}));

const listConnections = mock();
const updateConnectionCredential = mock();
const listConnectionsWithCredential = mock();

mock.module('@proxy/db/connection', () => ({listConnections, updateConnectionCredential}));
mock.module('@proxy/db/connection', () => ({listConnectionsWithCredential, updateConnectionCredential: mock(), getConnectionCredential: mock(async () => Ok(null))}));

// Readable stand-ins for encryption, so a test can write the credential it means.
mock.module('../utils/secret-crypto', () => ({
encryptSecret: (plaintext: string): Result<string, ApiError> => Ok(`enc:${plaintext}`),
decryptSecret: (stored: string): Result<string, ApiError> => Ok(stored.replace(/^enc:/, '')),
}));

const loadConnection = mock();

Expand Down Expand Up @@ -49,6 +54,16 @@ const notionConnection = {
credential: 'v1.x.y.z',
};

// Read files is on by default, but the person unticked Drive on Google's consent screen.
const googleConnection = {
id: 'google-1',
integrationId: 'google',
account: 'alex@example.com',
createdAt: new Date(),
defaults: [{actionId: 'readFiles'}],
credential: `enc:${JSON.stringify({accessToken: 'access-1', refreshToken: 'refresh-1', expiresAt: null, scopes: ['openid', 'https://www.googleapis.com/auth/userinfo.email'], email: 'alex@example.com'})}`,
};

// Reads cards of its own, and doesn't read the mailbox.
const agent = {
id: 'agent-1',
Expand All @@ -75,15 +90,15 @@ const send = mock();
const sendNew = mock();
const connector = {list: mock(), get: mock(), create: mock(), update: mock(), remove: mock(), commands: {archive, send}, newCommands: {sendNew}};

function useConnection(connection: typeof infoConnection | typeof emailConnection | typeof notionConnection) {
function useConnection(connection: typeof infoConnection | typeof emailConnection | typeof notionConnection | typeof googleConnection) {
loadConnection.mockResolvedValue(Ok({connection, integration: integration(connection.integrationId), connector}));
}

beforeEach(() => {
mock.clearAllMocks();
getAgent.mockResolvedValue(Ok(agent));
logAgentRequest.mockResolvedValue(Ok(undefined));
listConnections.mockResolvedValue(Ok([infoConnection, emailConnection]));
listConnectionsWithCredential.mockResolvedValue(Ok([infoConnection, emailConnection, googleConnection]));
connector.list.mockResolvedValue(Ok({records: [card], nextPage: null}));
connector.get.mockResolvedValue(Ok(card));
useConnection(infoConnection);
Expand All @@ -101,6 +116,47 @@ describe('listAgentConnections', () => {
});
});

describe("actions the connection's sign-in didn't cover", () => {
test('are not among what the agent can do, whatever is ticked', async () => {
getAgent.mockResolvedValue(Ok({...agent, grants: [{connectionId: 'google-1', actionId: 'readFiles', allowed: true}]}));

const {listAgentConnections} = await import('./tools');
const result = await listAgentConnections(signedIn);

expect(result.unwrap().connections.map((connection) => connection.id)).toEqual(['info-1', 'mail-1']);
});

test('offer no tools', async () => {
useConnection(googleConnection);

const {listAgentTools} = await import('./tools');
const result = await listAgentTools(signedIn, 'google-1');

expect(result.unwrap()).toEqual([]);
});

test('running one is refused, saying how to allow it, and logged as denied', async () => {
useConnection(googleConnection);

const {runAgentTool} = await import('./tools');
const result = await runAgentTool(caller, 'google-1', 'files_list', {search: 'budget'});

expect(result.unwrapErr()).toMatchObject({kind: 'validation_error', message: "Google didn't allow Read files at sign-in. Sign in to Google again to allow it."});
expect(connector.list).not.toHaveBeenCalled();
expect(logAgentRequest.mock.calls[0]?.[0]).toMatchObject({connectionId: 'google-1', collectionId: 'files', action: 'list', query: 'budget', outcome: 'denied'});
});

test("one the agent's own setting turns off anyway is plainly forbidden", async () => {
getAgent.mockResolvedValue(Ok({...agent, grants: [{connectionId: 'google-1', actionId: 'readFiles', allowed: false}]}));
useConnection(googleConnection);

const {runAgentTool} = await import('./tools');
const result = await runAgentTool(caller, 'google-1', 'files_list', {});

expect(result.unwrapErr().kind).toBe('forbidden');
});
});

describe('listAgentTools', () => {
test("the tools of the connection's actions the agent has", async () => {
useConnection(emailConnection);
Expand Down Expand Up @@ -351,7 +407,7 @@ describe('custom MCP servers', () => {

beforeEach(() => {
loadConnection.mockResolvedValue(Ok({connection: customConnection, integration: integrationFor({integrationId: 'mcp', tools: [search, publish]}), connector: null}));
listConnections.mockResolvedValue(Ok([customConnection]));
listConnectionsWithCredential.mockResolvedValue(Ok([customConnection]));
callCustomTool.mockResolvedValue(Ok('3 results'));
});

Expand Down
33 changes: 24 additions & 9 deletions packages/api/src/agent-side/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {Err, Ok, type Result} from 'ts-results-es';
import {ApiErr, type ApiError, Do, requirePresent} from '@proxy/utils';
import {getAgent, type AgentRow, type SignedInAgent} from '@proxy/db/agent';
import {logAgentRequest} from '@proxy/db/audit';
import {listConnections, type ConnectionRow} from '@proxy/db/connection';
import {listConnectionsWithCredential, type ConnectionRow} from '@proxy/db/connection';
import {
allowedTools,
applies,
Expand All @@ -22,6 +22,7 @@ import {
} from '@proxy/integrations';

import {callCustomTool} from '../connections/custom/custom-mcp';
import {unavailableActions, unavailableError} from '../connections/unavailable';
import type {Connector, DataRecord, RecordPage, RecordTarget, RecordValues} from '../records/connector';
import {parseRecordValues} from '../records/record-values';
import {loadConnection} from '../records/target';
Expand Down Expand Up @@ -53,12 +54,14 @@ export type ToolResult = RecordPage | {record: DataRecord | null} | {text: strin
// How much of a custom tool's params the activity log keeps.
const MAX_LOGGED_PARAMS = 200;

export function actionsOf(agent: AgentRow, connection: ConnectionRow, integration: Integration): string[] {
/** What the agent can do with the connection, none of the `unavailable` actions its sign-in didn't cover among them. */
export function actionsOf(agent: AgentRow, connection: ConnectionRow, integration: Integration, unavailable: string[]): string[] {
const own: OwnSettings = Object.fromEntries(agent.grants.filter((grant) => grant.connectionId === connection.id).map((grant) => [grant.actionId, grant.allowed]));
return effectiveActions(
integration,
connection.defaults.map((stored) => stored.actionId),
own,
unavailable,
);
}

Expand All @@ -73,13 +76,13 @@ export function requireAgent(signedIn: SignedInAgent): Promise<Result<AgentRow,
export function listAgentConnections(signedIn: SignedInAgent): Promise<Result<{agent: {id: string; providerId: string; name: string}; connections: AgentConnection[]}, ApiError>> {
return Do(async ($) => {
const agent = $(await requireAgent(signedIn));
const connections = $(await listConnections(signedIn.orgId))
const connections = $(await listConnectionsWithCredential(signedIn.orgId))
.flatMap((connection) => {
const integration = integrationOf(connection);
if (!integration) {
return [];
}
const actions = actionsOf(agent, connection, integration);
const actions = actionsOf(agent, connection, integration, unavailableActions(connection));
return [
{
id: connection.id,
Expand All @@ -101,7 +104,7 @@ export function listAgentTools(signedIn: SignedInAgent, connectionId: string): P
return Do(async ($) => {
const agent = $(await requireAgent(signedIn));
const {connection, integration} = $(await loadConnection(signedIn.orgId, connectionId));
return allowedTools(integration, actionsOf(agent, connection, integration));
return allowedTools(integration, actionsOf(agent, connection, integration, unavailableActions(connection)));
});
}

Expand Down Expand Up @@ -132,7 +135,8 @@ function log(run: ToolRun, outcome: 'allowed' | 'denied', title: string | null,
/**
* Runs one of the connection's tools with `params`, as its input schema describes them. A
* connection outside the organization, or a tool its integration doesn't have, is not found and
* not logged; a tool whose action the agent doesn't have is refused and logged as denied.
* not logged; a tool whose action the agent doesn't have is refused and logged as denied, saying
* how to allow it where the connection's sign-in didn't cover the action.
* `readOnly` limits the run to tools that only read, or only to those that change something, as
* the MCP server's `read` and `run` do; any other is refused, unlogged.
*/
Expand All @@ -153,14 +157,25 @@ export function runAgentTool(caller: AgentCaller, connectionId: string, toolName

// A list's search is logged, a refused one's too, so its params are read first.
const listParams = tool.kind === 'list' ? $(parseToolParams(tool, params)) : null;
if (!actionsOf(agent, connection, integration).includes(tool.action)) {
const unavailable = unavailableActions(connection);
if (!actionsOf(agent, connection, integration, unavailable).includes(tool.action)) {
$(await log(run, 'denied', null, searchOf(listParams)));
return $(Err(ApiErr.forbidden()));
return $(Err(refusal(integration, agent, connection, unavailable, tool.action)));
}
return $(await execute(run, listParams ?? $(parseToolParams(tool, params))));
});
}

// Why an action is refused: the sign-in, naming the action it didn't cover, where signing in again
// would allow it as the agent's settings stand; else plainly forbidden.
function refusal(integration: Integration, agent: AgentRow, connection: ConnectionRow, unavailable: string[], actionId: string): ApiError {
if (!actionsOf(agent, connection, integration, []).includes(actionId)) {
return ApiErr.forbidden();
}
const required = integration.actions.find((action) => action.id === actionId)?.requires;
return unavailableError(integration, unavailable.includes(actionId) || required === undefined ? actionId : required);
}

function searchOf(params: Record<string, string> | null): string | null {
return params?.search?.trim() || null;
}
Expand Down Expand Up @@ -206,7 +221,7 @@ function runServerTool(
outcome,
via: caller.via,
});
if (!actionsOf(agent, connection, integration).includes(tool.action)) {
if (!actionsOf(agent, connection, integration, unavailableActions(connection)).includes(tool.action)) {
$(await logRun('denied'));
return $(Err(ApiErr.forbidden()));
}
Expand Down
35 changes: 29 additions & 6 deletions packages/api/src/agents/route.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {beforeEach, describe, expect, mock, test} from 'bun:test';
import {Err, Ok} from 'ts-results-es';
import {Err, Ok, type Result} from 'ts-results-es';

import {ApiErr} from '@proxy/utils';
import {ApiErr, type ApiError} from '@proxy/utils';

const listAgents = mock();
const getAgent = mock();
Expand All @@ -14,9 +14,14 @@ const isProviderConflict = mock(() => false);

mock.module('@proxy/db/agent', () => ({listAgents, getAgent, createAgent, updateAgent, deleteAgent, setAgentGrants, isUsernameConflict, isProviderConflict}));

const getConnection = mock();
const getConnectionWithCredential = mock();

mock.module('@proxy/db/connection', () => ({getConnection}));
mock.module('@proxy/db/connection', () => ({getConnectionWithCredential}));

// Readable stand-ins for encryption, so a test can write the credential it means.
mock.module('../utils/secret-crypto', () => ({
decryptSecret: (stored: string): Result<string, ApiError> => Ok(stored.replace(/^enc:/, '')),
}));

const requireUserOrgId = mock();

Expand Down Expand Up @@ -44,7 +49,9 @@ beforeEach(() => {
createAgent.mockResolvedValue(Ok(agentRow));
updateAgent.mockResolvedValue(Ok(true));
setAgentGrants.mockResolvedValue(Ok(undefined));
getConnection.mockResolvedValue(Ok({id: 'conn-1', integrationId: 'email', account: 'alex@example.com', createdAt: new Date(), tools: null, name: null, defaults: []}));
getConnectionWithCredential.mockResolvedValue(
Ok({id: 'conn-1', integrationId: 'email', account: 'alex@example.com', createdAt: new Date(), tools: null, name: null, defaults: [], credential: 'enc:{}'}),
);
isUsernameConflict.mockReturnValue(false);
isProviderConflict.mockReturnValue(false);
});
Expand Down Expand Up @@ -179,8 +186,24 @@ describe('handleSetAgentGrantsRoute', () => {
expect(setAgentGrants).not.toHaveBeenCalled();
});

test("refuses turning on an action the connection's sign-in didn't cover, but not turning it off", async () => {
const credential = {accessToken: 'access-1', refreshToken: 'refresh-1', expiresAt: null, scopes: ['openid'], email: 'alex@example.com'};
getConnectionWithCredential.mockResolvedValue(
Ok({id: 'conn-1', integrationId: 'google', account: 'alex@example.com', createdAt: new Date(), defaults: [], credential: `enc:${JSON.stringify(credential)}`}),
);

const {handleSetAgentGrantsRoute} = await import('./route');
const on = await handleSetAgentGrantsRoute(makeRequest(params, {actions: {readFiles: true}}));
expect(on.unwrapErr()).toMatchObject({kind: 'validation_error', message: "Google didn't allow Read files at sign-in. Sign in to Google again to allow it."});
expect(setAgentGrants).not.toHaveBeenCalled();

const off = await handleSetAgentGrantsRoute(makeRequest(params, {actions: {readFiles: false}}));
expect(off.isOk()).toBe(true);
expect(setAgentGrants).toHaveBeenCalledWith({agentId: 'agent-1', connectionId: 'conn-1', actions: {readFiles: false}});
});

test('refuses a connection outside the organization', async () => {
getConnection.mockResolvedValue(Ok(null));
getConnectionWithCredential.mockResolvedValue(Ok(null));

const {handleSetAgentGrantsRoute} = await import('./route');
const result = await handleSetAgentGrantsRoute(makeRequest(params, {actions: {read: true}}));
Expand Down
8 changes: 5 additions & 3 deletions packages/api/src/agents/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import {z} from 'zod';

import {ApiErr, type ApiError, Do, parseSchema, requirePresent} from '@proxy/utils';
import {deleteAgent, getAgent, listAgents, setAgentGrants, updateAgent} from '@proxy/db/agent';
import {getConnection} from '@proxy/db/connection';
import {getConnectionWithCredential} from '@proxy/db/connection';
import {deleteOAuthGrant, listAgentOAuthGrants} from '@proxy/db/oauth';

import {refuseUnavailable, unavailableActions} from '../connections/unavailable';
import type {AuthenticatedRequest} from '../server/middleware/require-auth';
import {parseActionChanges, requireIntegration} from '../utils/connection-actions';
import {requireUserOrgId} from '../utils/user-org';
Expand Down Expand Up @@ -92,17 +93,18 @@ export function handleDeleteAgentRoute(request: AuthenticatedRequest): Promise<R
/**
* Sets the agent's own settings for actions of a connection, such as `{actions: {send: false}}`;
* `null` returns an action to the connection's default. An action the integration doesn't have is
* refused.
* refused, and so is turning on one the connection's sign-in didn't cover.
*/
export function handleSetAgentGrantsRoute(request: AuthenticatedRequest): Promise<Result<AgentResponse, ApiError>> {
return Do(async ($) => {
const orgId = $(await requireUserOrgId(request));
const agentId = agentIdOf(request);
const connectionId = request.params.connectionId ?? '';
$(await requireAgent(orgId, agentId));
const connection = $(requirePresent($(await getConnection(orgId, connectionId)), ApiErr.notFound('connection', connectionId)));
const connection = $(requirePresent($(await getConnectionWithCredential(orgId, connectionId)), ApiErr.notFound('connection', connectionId)));
const integration = $(requireIntegration(connection));
const actions = $(parseActionChanges(integration, z.boolean().nullable(), request.body));
$(refuseUnavailable(integration, unavailableActions(connection), actions));

$(await setAgentGrants({agentId, connectionId, actions}));
return $(await requireAgent(orgId, agentId));
Expand Down
14 changes: 9 additions & 5 deletions packages/api/src/connections/connect-flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ type ReturnTo = z.infer<typeof returnToSchema>;

type Route = (request: AuthenticatedRequest, res: Response) => Promise<void>;

/** What the provider's sign-in is sent: where it comes back to, the state, and the PKCE challenge. */
export type SignInRequest = {redirectUri: string; state: string; challenge: string};
/**
* What the provider's sign-in is sent: where it comes back to, the state, the PKCE challenge, and
* the account a sign-in again is for (`?account=`), as a connection's page passes it, for a provider
* that can open its sign-in on it.
*/
export type SignInRequest = {redirectUri: string; state: string; challenge: string; account: string | null};

/** The provider's sign-in page, and what to keep in the cookie for the callback. */
type SignInStart<E> = (request: SignInRequest) => Promise<Result<{url: string; kept: E}, ApiError>>;
Expand Down Expand Up @@ -146,12 +150,12 @@ export function signInFlow<E, C>(args: SignInArgs<E, C>) {
}

/** Keeps the handshake, and what `start` keeps beside it, in a cookie on `res`, and answers the provider's sign-in page, for the browser to go to. */
function beginSignIn(res: Response, begin: {userId: string; returnTo: ReturnTo | undefined; start: SignInStart<E>}): Promise<Result<string, ApiError>> {
function beginSignIn(res: Response, begin: {userId: string; returnTo: ReturnTo | undefined; account: string | null; start: SignInStart<E>}): Promise<Result<string, ApiError>> {
return Do(async ($) => {
const appUrl = $(requirePresent(env.APP_URL, ApiErr.internalError(new Error('APP_URL is not configured'))));
const state = randomBytes(16).toString('hex');
const {verifier, challenge} = makePkce();
const {url, kept} = $(await begin.start({redirectUri: redirectUri(appUrl), state, challenge}));
const {url, kept} = $(await begin.start({redirectUri: redirectUri(appUrl), state, challenge, account: begin.account}));
const payload: StatePayload = {state, verifier, userId: begin.userId, ...(begin.returnTo ? {returnTo: begin.returnTo} : {}), kept};
const cookie = $(encryptSecret(JSON.stringify(payload)));
res.cookie(stateCookieName, cookie, {...stateCookieOptions(), maxAge: stateCookieMaxAgeMs});
Expand Down Expand Up @@ -225,7 +229,7 @@ export function signInRoutes<E, C>(args: SignInArgs<E, C> & {start: SignInStart<
}

const returnTo = returnToSchema.safeParse(queryParam(request, 'returnTo')).data;
const begun = await flow.beginSignIn(res, {userId: request.user.userId, returnTo, start: args.start});
const begun = await flow.beginSignIn(res, {userId: request.user.userId, returnTo, account: queryParam(request, 'account'), start: args.start});
if (begun.isErr()) {
flow.redirectToConnectError(res, appUrl, returnTo, begun.error);
return;
Expand Down
6 changes: 5 additions & 1 deletion packages/api/src/connections/connection-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ export type ConnectionResponse = {
// What the person called the connection, where they named it; null for every other integration,
// which is called by its integration.
name: string | null;
// The actions the connection's sign-in didn't cover, in the catalog's order: no agent gets them
// until the person signs in again and allows them.
unavailable: string[];
};

export function toConnectionResponse(row: ConnectionRow): ConnectionResponse {
export function toConnectionResponse(row: ConnectionRow, unavailable: string[]): ConnectionResponse {
const integration = integrationOf(row);
const actions = integration?.actions ?? [];
return {
Expand All @@ -28,5 +31,6 @@ export function toConnectionResponse(row: ConnectionRow): ConnectionResponse {
defaults: actions.map((action) => action.id).filter((id) => row.defaults.some((stored) => stored.actionId === id)),
tools: integration?.mcpTools ?? null,
name: row.name,
unavailable,
};
}
Loading
Loading