Skip to content

Commit d8ebcc0

Browse files
improvement(tools): prevent internal request self-hops (#7190)
* improvement(tools): prevent internal request self-hops * fix(executor): preserve background execution principals * fix(executor): authorize files with workflow principals * fix(auth): bound derived workflow delegations * fix(tools): close boundary review gaps * fix(tools): close indirect boundary gaps * fix(tools): resolve helper-built self hops * fix(tools): normalize relative self hop paths * fix(tools): fail closed on opaque URL helpers * fix(tools): reject dynamic Sim-origin paths * fix(tools): enforce external request origin * fix(tools): close remaining self-hop bypasses * fix(tools): block self-hosted loopback aliases
1 parent 1969779 commit d8ebcc0

81 files changed

Lines changed: 5218 additions & 1136 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/skills/add-block/SKILL.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,10 @@ After creating the block, you MUST validate it against every tool it references:
10521052
4. **Verify conditions** — each subBlock should only show for the operations that actually use it
10531053
5. **Verify `{Service}BlockMeta` is exported** with at least 7 templates, each having `icon`, `title`, `prompt`, `modules`, `category`, and `tags`
10541054
6. **If any tool outputs are still unknown**, explicitly tell the user instead of guessing block outputs
1055+
7. **Verify the tool execution boundary** — blocks never create or call API routes. Every referenced
1056+
tool must already be either a registered `InternalToolConfig.operation` or an absolute external
1057+
HTTP(S) `ToolConfig.request`. If transport needs to change, use the `add-tools` skill; do not add a
1058+
same-origin `/api/...` hop from the block.
10551059

10561060
## Option Lists: `selectorKey` or `options`, never a per-block fetcher
10571061

.agents/skills/add-integration/SKILL.md

Lines changed: 59 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ apps/sim/tools/{service}/
6060

6161
### Key Patterns
6262

63+
Choose the tool boundary before writing the declaration:
64+
65+
- Use `InternalToolConfig.operation` for same-process Sim/provider work. Put the handler under
66+
`apps/sim/lib/internal/{service}/execute-tool.ts` and register every ID in
67+
`apps/sim/lib/internal/tool-operations/registry.server.ts`.
68+
- Use `ToolConfig.request` only for an absolute external HTTP(S) provider endpoint.
69+
70+
Never point a tool at `/api/...`, construct an absolute URL back to Sim, declare
71+
`request.internal`, or add an API route merely to reuse code, normalize files, or authorize
72+
resources. A real external/browser route and an in-process tool may share the same operation, but
73+
neither calls the other. Follow the full transport and handler rules in the `add-tools` skill.
74+
6375
**types.ts:**
6476
```typescript
6577
import type { ToolResponse } from '@/tools/types'
@@ -82,7 +94,7 @@ export interface {Service}Response extends ToolResponse {
8294

8395
**Tool file pattern:**
8496
```typescript
85-
export const {service}{Action}Tool: ToolConfig<Params, Response> = {
97+
export const {service}{Action}Tool: InternalToolConfig<Params, Response> = {
8698
id: '{service}_{action}',
8799
name: '{Service} {Action}',
88100
description: '...',
@@ -95,16 +107,11 @@ export const {service}{Action}Tool: ToolConfig<Params, Response> = {
95107
// ... other params
96108
},
97109

98-
request: { url, method, headers, body },
99-
100-
transformResponse: async (response) => {
101-
const data = await response.json()
102-
return {
103-
success: true,
104-
output: {
105-
field: data.field ?? null, // Always handle nullables
106-
},
107-
}
110+
operation: {
111+
input: (params) => ({
112+
accessToken: params.accessToken,
113+
// Map only the semantic operation input.
114+
}),
108115
},
109116

110117
outputs: { /* ... */ },
@@ -135,7 +142,8 @@ and leave the field unannotated.
135142
sent with their normal request semantics. A URL, domain, resource ID, control field, or opaque
136143
payload is not model-visible merely because the provider is AI-backed or may process the
137144
referenced resource later.
138-
- **Text or structured content consumed by an AI model:** declare `request.modelInput` with
145+
- **Text or structured content consumed by an AI model:** declare `request.modelInput` for an
146+
external provider request or `operation.modelInput` for an in-process operation, with
139147
`mode: 'project'` and select only the exact model-visible fields. The shared executor replaces
140148
activated Sim secrets with canonical `{{NAME}}` labels before request formatting. For nested or
141149
JSON-string fields, use a small shared selector plus `applyProjected`; verify that selecting the
@@ -144,20 +152,19 @@ and leave the field unannotated.
144152
top-level param in `request.modelInput`. Project the private copy before the existing request
145153
formatter parses it; keep formatter behavior deterministic when a whole-value placeholder is not
146154
valid in the serialized grammar. Do not introduce a second hard-rejection path.
147-
- **Opaque model input owned by an authenticated internal route** such as inline audio, image,
148-
video, or document bytes: add `privateProvenance` to a projected request, or use
155+
- **Opaque model input owned by an in-process operation** such as inline audio, image, video, or
156+
document bytes: add `privateProvenance` to the operation model-input declaration, or use
149157
`mode: 'private-provenance'` when there is no textual projection. Do not select storage keys,
150-
paths, signed URLs, or ordinary remote URLs as byte provenance; the owning route must authorize
151-
stored bytes independently at model egress. The route must call
158+
paths, signed URLs, or ordinary remote URLs as byte provenance; the owning operation must
159+
authorize stored bytes independently at model egress. The operation must call
152160
`validateOpaqueModelInputProvenance` before downloading or sending content to the model and must
153161
apply the workspace-file provenance guard before reading a persisted workspace file.
154162
- **Sim-owned durable storage or internal execution handoff** that can later enter a workflow/model
155163
(table cells, Agent memory, knowledge documents/chunks, workspace-file contents, or child-workflow
156-
input): transport encrypted field-scoped provenance with `request.secretProvenance`. The
157-
authenticated receiver validates the exact selection and scope, strips the private envelope, and
158-
persists, imports, or propagates it at the owning boundary. Preserve shared legacy behavior for
159-
headerless internal calls and rows/files whose provenance marker is `NULL`; never invent a
160-
tool-local migration rule.
164+
input): transport encrypted field-scoped provenance with `operation.secretProvenance`. The
165+
operation validates the exact selection and trusted scope, then persists, imports, or propagates
166+
it at the owning boundary. Preserve shared legacy behavior for rows/files whose provenance marker
167+
is `NULL`; never invent a tool-local migration rule.
161168

162169
Hard rules:
163170

@@ -166,7 +173,8 @@ Hard rules:
166173
transport and strips private metadata from functional results.
167174
- Never attach private provenance to an external URL or to `directExecution`. Project proven
168175
model-visible external fields with `request.modelInput`; otherwise preserve ordinary request
169-
semantics. Use an authenticated internal route when encrypted provenance must cross the boundary.
176+
semantics. Use a registered in-process operation when encrypted provenance must cross the
177+
boundary.
170178
- Never sanitize arbitrary third-party tool results. Projection applies only to secrets activated
171179
by Sim's resolved-secret provenance for that execution/tool call.
172180
- Do not add provenance merely because a value is persisted, returned by a tool, or appears in a
@@ -596,6 +604,10 @@ If creating V2 versions (API-aligned outputs):
596604
- [ ] Created `tools/{service}/` directory
597605
- [ ] Created `types.ts` with all interfaces
598606
- [ ] Created tool file for each operation
607+
- [ ] Chose exactly one boundary per tool: registered `InternalToolConfig.operation` or absolute
608+
external HTTP(S) `ToolConfig.request`
609+
- [ ] No tool points to `/api/...`, constructs a URL back to Sim, declares `request.internal`, or
610+
has an HTTP fallback for an in-process operation
599611
- [ ] All params have correct visibility
600612
- [ ] All nullable fields use `?? null`
601613
- [ ] All optional outputs have `optional: true`
@@ -607,6 +619,8 @@ If creating V2 versions (API-aligned outputs):
607619
external resource locators and control inputs retain their request semantics
608620
- [ ] Confirmed ordinary third-party tool results are not generically sanitized
609621
- [ ] Added provenance compatibility and fail-closed boundary tests where applicable
622+
- [ ] `bun run check:tool-request-boundary` passes
623+
- [ ] Internal-operation registry completeness test passes for every operation-backed tool
610624

611625
### Block
612626
- [ ] Created `blocks/blocks/{service}.ts`
@@ -721,7 +735,8 @@ interface UserFile {
721735

722736
### File Input Pattern (Uploads)
723737

724-
For tools that accept file uploads, **always route through an internal API endpoint** rather than calling external APIs directly. This ensures proper file content retrieval.
738+
File authorization, normalization, storage reads, provider upload, and response mapping belong in a
739+
registered in-process operation. Do not create an internal API route for file tools.
725740

726741
#### 1. Block SubBlocks for File Input
727742

@@ -757,137 +772,36 @@ Use the basic/advanced mode pattern:
757772

758773
#### 2. Normalize File Input in Block Config
759774

760-
In `tools.config.tool`, use `normalizeFileInput` to handle all input variants:
775+
`tools.config.tool` selects the tool before variable resolution and must not mutate or coerce input.
776+
Use `tools.config.params`, which runs after variable resolution, to normalize all file variants:
761777

762778
```typescript
763779
import { normalizeFileInput } from '@/blocks/utils'
764780

765781
tools: {
766782
config: {
767-
tool: (params) => {
768-
// Normalize file from basic (uploadFile), advanced (fileRef), or legacy (fileContent)
769-
const normalizedFile = normalizeFileInput(
770-
params.uploadFile || params.fileRef || params.fileContent,
771-
{ single: true }
772-
)
773-
if (normalizedFile) {
774-
params.file = normalizedFile
775-
}
776-
return `{service}_${params.operation}`
783+
tool: (params) => `{service}_${params.operation}`,
784+
params: (params) => {
785+
// Serialization collapses the basic/advanced pair into the canonical `file` key.
786+
const normalizedFile = normalizeFileInput(params.file, { single: true })
787+
return normalizedFile ? { file: normalizedFile } : {}
777788
},
778789
},
779790
}
780791
```
781792

782-
#### 3. Create Special Internal Tool Execution Route
783-
784-
Create `apps/sim/app/api/tools/{service}/{action}/route.ts`. This raw route pattern is only for an integration's provider-execution boundary when it needs special file normalization, large-body handling, or protocol behavior. It is not the pattern for CRUD or other operations on protected Sim resources. For those, use the `migrate-application-operation` skill and an authorized application use case with the ordinary internal/v2 route builders.
785-
786-
Internal tool routes are HTTP boundaries and follow the same contract policy as public routes — define the request/response shape in `apps/sim/lib/api/contracts/tools/{service}.ts` (or an existing aggregate) and validate with canonical helpers from `@/lib/api/server`. Never write a route-local Zod schema. Authenticate and perform cheap admission before parsing or downloading files.
793+
#### 3. Define and register the in-process operation
787794

788795
```typescript
789-
// apps/sim/lib/api/contracts/tools/{service}.ts
790-
import { z } from 'zod'
791-
import { defineRouteContract } from '@/lib/api/contracts'
792-
import { FileInputSchema } from '@/lib/uploads/utils/file-schemas'
793-
794-
export const {service}UploadBodySchema = z.object({
795-
accessToken: z.string(),
796-
file: FileInputSchema.optional().nullable(),
797-
fileContent: z.string().optional().nullable(),
798-
// ... other params
799-
})
800-
801-
export const {service}UploadResponseSchema = z.object({
802-
success: z.boolean(),
803-
output: z.object({ id: z.string(), url: z.string() }).optional(),
804-
error: z.string().optional(),
805-
})
806-
807-
export const {service}UploadContract = defineRouteContract({
808-
method: 'POST',
809-
path: '/api/tools/{service}/upload',
810-
body: {service}UploadBodySchema,
811-
response: { mode: 'json', schema: {service}UploadResponseSchema },
812-
})
813-
814-
export type {Service}UploadBody = z.input<typeof {service}UploadBodySchema>
815-
export type {Service}UploadResponse = z.output<typeof {service}UploadResponseSchema>
816-
```
817-
818-
```typescript
819-
// apps/sim/app/api/tools/{service}/upload/route.ts
820-
import { createLogger } from '@sim/logger'
821-
import { NextResponse, type NextRequest } from 'next/server'
822-
import { {service}UploadContract } from '@/lib/api/contracts/tools/{service}'
823-
import { parseRequest } from '@/lib/api/server'
824-
import { checkInternalAuth } from '@/lib/auth/hybrid'
825-
import { generateRequestId } from '@/lib/core/utils/request'
826-
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
827-
import { type RawFileInput } from '@/lib/uploads/utils/file-schemas'
828-
import { processFilesToUserFiles } from '@/lib/uploads/utils/file-utils'
829-
import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
830-
831-
const logger = createLogger('{Service}UploadAPI')
832-
833-
export const POST = withRouteHandler(async (request: NextRequest) => {
834-
const requestId = generateRequestId()
835-
836-
// Auth always runs BEFORE parseRequest — never validate untrusted input before authenticating.
837-
const authResult = await checkInternalAuth(request, { requireWorkflowId: false })
838-
if (!authResult.success) {
839-
return NextResponse.json({ success: false, error: 'Unauthorized' }, { status: 401 })
840-
}
841-
842-
const parsed = await parseRequest({service}UploadContract, request, {})
843-
if (!parsed.success) return parsed.response
844-
const data = parsed.data.body
845-
846-
let fileBuffer: Buffer
847-
let fileName: string
848-
849-
// Prefer UserFile input, fall back to legacy base64
850-
if (data.file) {
851-
const userFiles = processFilesToUserFiles([data.file as RawFileInput], requestId, logger)
852-
if (userFiles.length === 0) {
853-
return NextResponse.json({ success: false, error: 'Invalid file' }, { status: 400 })
854-
}
855-
const userFile = userFiles[0]
856-
fileBuffer = await downloadFileFromStorage(userFile, requestId, logger)
857-
fileName = userFile.name
858-
} else if (data.fileContent) {
859-
// Legacy: base64 string (backwards compatibility)
860-
fileBuffer = Buffer.from(data.fileContent, 'base64')
861-
fileName = 'file'
862-
} else {
863-
return NextResponse.json({ success: false, error: 'File required' }, { status: 400 })
864-
}
865-
866-
// Now call external API with fileBuffer
867-
const response = await fetch('https://api.{service}.com/upload', {
868-
method: 'POST',
869-
headers: { Authorization: `Bearer ${data.accessToken}` },
870-
body: new Uint8Array(fileBuffer), // Convert Buffer for fetch
871-
})
872-
873-
// ... handle response
874-
})
875-
```
876-
877-
#### 4. Update Tool to Use Internal Route
878-
879-
```typescript
880-
export const {service}UploadTool: ToolConfig<Params, Response> = {
796+
export const {service}UploadTool: InternalToolConfig<Params, Response> = {
881797
id: '{service}_upload',
882798
// ...
883799
params: {
884800
file: { type: 'file', required: false, visibility: 'user-or-llm' },
885801
fileContent: { type: 'string', required: false, visibility: 'hidden' }, // Legacy
886802
},
887-
request: {
888-
url: '/api/tools/{service}/upload', // Internal route
889-
method: 'POST',
890-
body: (params) => ({
803+
operation: {
804+
input: (params) => ({
891805
accessToken: params.accessToken,
892806
file: params.file,
893807
fileContent: params.fileContent,
@@ -896,6 +810,13 @@ export const {service}UploadTool: ToolConfig<Params, Response> = {
896810
}
897811
```
898812

813+
Implement `apps/sim/lib/internal/{service}/execute-tool.ts` and keep the file/provider work in typed
814+
operations beside it. The handler validates `request.input`, derives storage authority only from
815+
trusted `request.context`, authorizes every stored file before reading bytes, forwards
816+
`request.signal`, enforces declared and actual byte caps, and returns the canonical tool response.
817+
Register `{service}_upload` in `apps/sim/lib/internal/tool-operations/registry.server.ts` and add a
818+
registry/direct-handler test. There is no HTTP fallback.
819+
899820
### File Output Pattern (Downloads)
900821

901822
For tools that return files, use `FileToolProcessor` to store files and return `UserFile` objects.
@@ -923,11 +844,11 @@ transformResponse: async (response, context) => {
923844
}
924845
```
925846

926-
#### In API Route (for complex file handling)
847+
#### In the operation handler (for complex file handling)
927848

928849
```typescript
929-
// Return file data that FileToolProcessor can handle
930-
return NextResponse.json({
850+
// Return file data that FileToolProcessor can handle. No API route is involved.
851+
return Response.json({
931852
success: true,
932853
output: {
933854
file: {

0 commit comments

Comments
 (0)