Skip to content

Commit a8312c0

Browse files
committed
fix(mothership): refuse callerless service writes and organization-scoped CLI writes
Mixed server tools reached through sim_cli services (settings, dashboards, tests, search_sources, workspaces) write through internal operations that declare no API scope, so the read grant never bound them. Each tool now declares which validated arguments write, and the router holds those to the current delegated grant. The grant also applies to callerless organization-scoped runs.
1 parent 4fe6118 commit a8312c0

10 files changed

Lines changed: 68 additions & 2 deletions

File tree

‎apps/sim/lib/core/application/oauth-authorization.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ export function requireOAuthOperationScope(
4141
}
4242
}
4343

44+
/**
45+
* Holds a change no scoped operation guards — an internal operation, or one authorized outside the
46+
* operation layer — to the current {@link withDelegatedApiScopes} grant. Outside a grant it allows.
47+
*/
48+
export function requireDelegatedApiScope(scope: OAuthApiScope): void {
49+
const granted = delegatedApiScopes()
50+
if (granted && !oauthScopeSatisfies(granted, scope)) throw new InsufficientScopeError(scope)
51+
}
52+
4453
function grantedApiScopes(principal: Principal): readonly string[] | undefined {
4554
switch (principal.kind) {
4655
case 'oauth_access_token':

‎apps/sim/lib/mothership/agent-cli/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ export async function executeAgentCliRequest(
2323
context: AgentCliExecutionContext
2424
): Promise<AgentCliRawResult> {
2525
const execute = () => executeUncountedAgentCliRequest(request, context)
26-
// Commands authorize the subject's live role; a callerless run's subject is the workspace owner.
26+
// Commands authorize the subject's live role; a callerless inbox run's subject is the owner.
2727
const { result, withheld } = await countWithheldContent(() =>
28-
context.secretActorUserId === null && !context.organizationId
28+
context.secretActorUserId === null
2929
? withDelegatedApiScopes([OAUTH_API_READ_SCOPE], execute)
3030
: execute()
3131
)

‎apps/sim/lib/mothership/agent-cli/project-file-read.integration.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,48 @@ check('sim_cli for a callerless run holds only the read API scopes', async () =>
350350
expect(await projectFiles()).toHaveLength(1)
351351
})
352352

353+
check('sim_cli services for a callerless run refuse the writes they route', async () => {
354+
const f = await fixture()
355+
const service = (name: string, input: Record<string, unknown>, context: ToolExecutionContext) =>
356+
executeSimCli({ request: { invocation: { kind: 'service', name, input } } }, context)
357+
const inboxSecretScope = async () =>
358+
(
359+
await db
360+
.select({ scope: workspace.inboxSecretScope })
361+
.from(workspace)
362+
.where(eq(workspace.id, f.workspaceId))
363+
)[0]?.scope
364+
const before = await inboxSecretScope()
365+
const widen = before === 'all' ? 'selected' : 'all'
366+
const writes: Array<[string, Record<string, unknown>]> = [
367+
[
368+
'settings',
369+
{ scope: 'workspace', action: 'update', section: 'inbox', changes: { secretScope: widen } },
370+
],
371+
]
372+
373+
const callerless: ToolExecutionContext = { ...f.context, workflowId: '', secretActorUserId: null }
374+
for (const [name, input] of writes) {
375+
const refused = await service(name, input, callerless)
376+
expect(refused.success).toBe(false)
377+
expect(refused.error).toContain('api:write')
378+
}
379+
expect(await inboxSecretScope()).toBe(before)
380+
const read = await service(
381+
'settings',
382+
{ scope: 'workspace', action: 'get', section: 'inbox' },
383+
callerless
384+
)
385+
expect(read.success, read.error).toBe(true)
386+
387+
const attributed: ToolExecutionContext = { ...f.context, workflowId: '', userPermission: 'admin' }
388+
for (const [name, input] of writes) {
389+
const written = await service(name, input, attributed)
390+
expect(written.success, `${name}: ${written.error}`).toBe(true)
391+
}
392+
expect(await inboxSecretScope()).toBe(widen)
393+
})
394+
353395
check('sim_cli for a read-only member keeps what that role allows', async () => {
354396
const f = await fixture()
355397
const memberId = await insertUser('Read member')

‎apps/sim/lib/mothership/tools/server/base-tool.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,6 @@ export interface BaseServerTool<TArgs = unknown, TResult = unknown> {
6868
inputSchema?: z.ZodType<TArgs>
6969
/** Optional Zod schema for output validation */
7070
outputSchema?: z.ZodType<TResult>
71+
/** Whether these validated arguments change stored data; a delegated read grant refuses them. */
72+
writes?(args: TArgs): boolean
7173
}

‎apps/sim/lib/mothership/tools/server/dashboards.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ function workspaceFor(input: { workspaceId?: string }, context?: ServerToolConte
2424
export const dashboardsServerTool: BaseServerTool = {
2525
name: 'dashboards',
2626
inputSchema: mothershipDashboardsInputSchema,
27+
writes: (args) => mothershipDashboardsInputSchema.parse(args).action === 'set',
2728
async execute(raw, context) {
2829
const input = mothershipDashboardsInputSchema.parse(raw)
2930
const workspaceId = workspaceFor(input, context)

‎apps/sim/lib/mothership/tools/server/router.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { createLogger } from '@sim/logger'
22
import { isRecordLike } from '@sim/utils/object'
33
import { z } from 'zod'
44
import { getValidationErrorMessage, isZodError } from '@/lib/api/server/validation'
5+
import { OAUTH_API_WRITE_SCOPE } from '@/lib/auth/oauth-provider'
6+
import { requireDelegatedApiScope } from '@/lib/core/application/oauth-authorization'
57
import { OrchestrationError } from '@/lib/core/orchestration/types'
68
import {
79
Ffmpeg,
@@ -156,6 +158,7 @@ export async function routeExecution(
156158
}
157159

158160
assertServerToolNotAborted(context, `User stop signal aborted ${toolName} after validation`)
161+
if (tool.writes?.(args)) requireDelegatedApiScope(OAUTH_API_WRITE_SCOPE)
159162

160163
// Execute. None of the remaining tools resolve blocks or gate discovery, so the old
161164
// custom-block-overlay / block-visibility ALS scopes are gone with the tools that

‎apps/sim/lib/mothership/tools/server/search-sources.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export const organizationSearchSourcesServerTool: BaseServerTool<
3333
name: 'search_sources',
3434
inputSchema: organizationSearchSourcesInputSchema,
3535
outputSchema: organizationSearchSourcesOutputSchema,
36+
writes: (args) => args.action === 'approve',
3637
async execute(args, context) {
3738
if (context?.requestMode !== 'agent' && context?.requestMode !== 'plan')
3839
throw new OrchestrationError('forbidden', 'Search source controls require Mothership')

‎apps/sim/lib/mothership/tools/server/settings.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,10 @@ function settingsRefresh(context: SettingsContext, section: string): ResourceCha
356356
export const settingsServerTool: BaseServerTool<MothershipSettingsInput> = {
357357
name: 'settings',
358358
inputSchema: mothershipSettingsInputSchema,
359+
writes: (input) =>
360+
input.action === 'update' ||
361+
(input.action === 'execute' &&
362+
settingsOperations[`${input.scope}/${input.section}`]?.[input.operation]?.effect === 'write'),
359363
async execute(raw, context) {
360364
const input = mothershipSettingsInputSchema.parse(raw)
361365
const target = await resolveSettingsContext(input.scope, context, input.workspaceId)

‎apps/sim/lib/mothership/tools/server/tests.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,12 @@ function failures(report: TestReport | null) {
4646
})
4747
}
4848

49+
const TEST_READ_ACTIONS = new Set(['list', 'get'])
50+
4951
export const testsServerTool: BaseServerTool = {
5052
name: 'tests',
5153
inputSchema: mothershipTestsInputSchema,
54+
writes: (args) => !TEST_READ_ACTIONS.has(mothershipTestsInputSchema.parse(args).action),
5255
async execute(raw, context) {
5356
const input = mothershipTestsInputSchema.parse(raw)
5457
const workspaceId = workspaceFor(input, context)

‎apps/sim/lib/mothership/tools/server/workspaces.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { createOrganizationWorkspace } from '@/lib/workspaces/application/create
88
export const workspacesServerTool: BaseServerTool = {
99
name: 'workspaces',
1010
inputSchema: mothershipWorkspacesInputSchema,
11+
writes: () => true,
1112
async execute(raw, context) {
1213
try {
1314
const { action: _action, ...input } = mothershipWorkspacesInputSchema.parse(raw)

0 commit comments

Comments
 (0)