Skip to content

Commit 04ff76b

Browse files
committed
fix(storage): an upload link works only for the user it was minted for, signed in (#30892)
Both upload-link doors now read the request's own session first. No session answers 401 AUTH_REQUIRED, as every other storage door does, so the link is not an anonymous door (ADR-0138 D2) and the anonymous sweep's everything-else class holds. A session naming another user answers the one unusable-link 404 and spends nothing. The page's PUT carries the session cookie. The MCP tool, the served skill, the docs and the changeset say the link is opened signed in. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WV4cyqd3A7fbCWrqM3KCz
1 parent bc5ad15 commit 04ff76b

13 files changed

Lines changed: 183 additions & 63 deletions

‎.changeset/30892-mcp-read-file-upload-link.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66

77
feat(mcp): an agent can read a record's files and attach a file to a record — `read_file` and `create_upload_link` over `/api/v1/mcp`, and the single-use upload link they hand out (#30892)
88

9-
Clause-②: yes (widening) — two MCP tools join the HTTP surface where the storage service is installed (`read_file` under `data:read`, `create_upload_link` under `data:write`), and two public routes join the storage family: `GET` and `PUT /api/v1/storage/upload-links/:token`. Nothing that answered before answers differently.
9+
Clause-②: yes (widening) — two MCP tools join the HTTP surface where the storage service is installed (`read_file` under `data:read`, `create_upload_link` under `data:write`), and two signed-in routes join the storage family: `GET` and `PUT /api/v1/storage/upload-links/:token`. Nothing that answered before answers differently.
1010
Metadata-protocol: none
1111

1212
- **`read_file`** takes `objectName`, `recordId` and a file field (with `fileId` when the field holds several files) or an `attachmentId`. With neither, it lists the record's files. It answers the file's facts (id, name, type, size, the read cap that applies), a short-lived download link, and the content: a PDF's or Word document's text (extracted on the server, capped in pages and characters), an image as an MCP image block, a text file as text. A file over the cap (10 MiB) answers its facts and the download link, without content. Access follows the record: a record the caller cannot read answers `404 RECORD_NOT_FOUND`, as `get_record` does, and a file the download door would refuse answers that door's `403`.
13-
- **`create_upload_link`** takes a record the caller may edit plus a file field or `attachments: true`. It answers a link that accepts exactly one upload for ten minutes, bound to that record and that field (or the record's attachments). The upload lands as the caller would land it: the file field is written through the record's own update door, so the field's `accept` and `maxSize`, the record's edit access and the upload size limit (`storage.max_upload_mb`, `413` before the link is spent) all apply. It writes the same `sys_file` row, size recorded, and the same `storage.upload` as any other upload, so whatever a host counts toward quota counts it. A used, expired or unknown link answers the same `404`.
13+
- **`create_upload_link`** takes a record the caller may edit plus a file field or `attachments: true`. It answers a link that accepts exactly one upload for ten minutes, bound to that record and that field (or the record's attachments), and usable only by the same user, signed in: a request without a session answers `401 AUTH_REQUIRED`, like every other storage door. The upload lands as the caller would land it: the file field is written through the record's own update door, so the field's `accept` and `maxSize`, the record's edit access and the upload size limit (`storage.max_upload_mb`, `413` before the link is spent) all apply. It writes the same `sys_file` row, size recorded, and the same `storage.upload` as any other upload, so whatever a host counts toward quota counts it. A used, expired or unknown link, or one made for another user, answers the same `404`.
1414
- **`@objectstack/mcp`** loads `unpdf` (pinned 1.9.1) by dynamic import on the first PDF read, with pdf.js eval off; a Word document is read with `node:zlib` under a bounded inflate.

‎content/docs/ai/connect-mcp.mdx‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,12 @@ record means no file**, refused with the same `RECORD_NOT_FOUND`.
281281
- **`create_upload_link({ objectName, recordId, field? | attachments: true })`**
282282
needs a record the caller may edit (and, for a field, a field they may
283283
write). It answers a link and its expiry (ten minutes). The user opens the
284-
link in a browser, on a phone too, picks one file, and it lands on exactly
285-
that record and field, or as an attachment of that record. The link works
286-
once: a second use, an expired link and an unknown link all get the same
287-
answer. The upload is stored as the user who asked for the link, with their
284+
link in a browser where they are signed in to this deployment, picks one
285+
file, and it lands on exactly that record and field, or as an attachment of
286+
that record. On a phone, they sign in there first. The link works only for
287+
the user it was made for, and only once: without a session it answers
288+
`401`, and a second use, an expired link, an unknown link and another user's
289+
link all get the same `404`. The upload is stored as that user, with their
288290
access checked again when the file arrives, and it meets the storage
289291
service's maximum upload size before the link is spent. Sending a file's
290292
bytes through the MCP protocol itself is not supported yet.

‎content/docs/api/plugin-endpoints.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,8 @@ other adapter.
161161
| GET | `/storage/upload/chunked/:uploadId/progress` | `storage.resumeUpload` | Read session progress (first step of a resume) |
162162
| GET | `/storage/files/:fileId/url` | `storage.getDownloadUrl` | Resolve a short-lived signed download URL |
163163
| GET | `/storage/files/:fileId` | — | Stable browser URL; 302s to the same signed URL (what file fields carry) |
164-
| GET | `/storage/upload-links/:token` | — | The page a single-use upload link opens (minted by the MCP `create_upload_link` tool): a file picker |
165-
| PUT | `/storage/upload-links/:token` | — | Spend the link on the request body; the file lands on the record and field the link is bound to |
164+
| GET | `/storage/upload-links/:token` | — | The page a single-use upload link opens (minted by the MCP `create_upload_link` tool) for the signed-in user it was made for: a file picker |
165+
| PUT | `/storage/upload-links/:token` | — | Spend the link on the request body, as that signed-in user; the file lands on the record and field the link is bound to |
166166

167167
`storage.upload(file)` runs steps 1–3 for you, including the direct-to-storage
168168
`PUT` in step 2. Step 1 refuses an empty `mimeType` with `400 INVALID_REQUEST`,

‎packages/mcp/src/mcp-http-tools.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1931,9 +1931,10 @@ export function registerFileTools(
19311931
{
19321932
description:
19331933
'Create a link through which the user uploads one file onto a record: into a file field (field) or ' +
1934-
'as an attachment (attachments: true). Give the link to the user to open; it works once, for this ' +
1935-
'record and field only, and expires (see expiresAt). The upload is stored as the user, so they must be ' +
1936-
'able to edit the record (and the field).',
1934+
'as an attachment (attachments: true). Give the link to the user to open in a browser where they are ' +
1935+
'signed in to this deployment as themselves; it works only for them, once, for this record and field ' +
1936+
'only, and expires (see expiresAt). The upload is stored as the user, so they must be able to edit the ' +
1937+
'record (and the field).',
19371938
inputSchema: strictToolInput(
19381939
{
19391940
surface: 'this create_upload_link call',
@@ -1974,7 +1975,8 @@ export function registerFileTools(
19741975
recordId,
19751976
...(field !== undefined ? { field } : { attachments: true }),
19761977
instructions:
1977-
'Give this link to the user to open in a browser and pick the file. It works once, until expiresAt.',
1978+
'Give this link to the user to open in a browser where they are signed in to this deployment, and ' +
1979+
'pick the file. It works only for them, once, until expiresAt.',
19781980
});
19791981
} catch (err) {
19801982
return errorResultWithCode(err);

‎packages/mcp/src/skill-md.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,10 @@ const READ_FILE_TOOL_LINES = `- **read_file({ objectName, recordId, field?, file
187187
/** [#30892] `create_upload_link`'s entry, rendered where `registerFileTools` registers it (`data:write`). */
188188
const UPLOAD_LINK_TOOL_LINES = `- **create_upload_link({ objectName, recordId, field? | attachments: true })** —
189189
a link through which the user uploads one file onto a record: into a file
190-
field, or as an attachment. Give the link to the user; it works once, for
191-
that record and field only, and expires at \`expiresAt\`. The upload is
192-
stored as the user, so they must be able to edit the record.
190+
field, or as an attachment. Give the link to the user to open where they
191+
are signed in to this deployment; it works only for them, once, for that
192+
record and field only, and expires at \`expiresAt\`. The upload is stored
193+
as the user, so they must be able to edit the record.
193194
`;
194195

195196
/** The file pair's entries for a host whose bridge carries them, each for the grant that registers it. */

‎packages/qa/dogfood/test/showcase-mcp-record-files.dogfood.test.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
// bearer reaches `/api/v1/mcp`, the bridge reads the record through
1515
// `get_record`'s door under the showcase's own owner RLS and
1616
// `controlled_by_parent` derivation, the storage plugin answers, and the link
17-
// is spent through the real `PUT /api/v1/storage/upload-links/:token`.
17+
// is spent through the real `PUT /api/v1/storage/upload-links/:token` by the
18+
// user it was minted for, signed in (the link is not an anonymous door: no
19+
// session answers 401, as every other storage door does).
1820
//
1921
// The records: `showcase_invoice` (owner RLS on `created_by`, `enable.files`,
2022
// so it holds attachments) and `showcase_invoice_line` (`controlled_by_parent`,
@@ -42,7 +44,8 @@ const MIB = 1024 * 1024;
4244
const READ_CAP = 10 * MIB;
4345
/** `UPLOAD_LINK_TTL_SECONDS` in `@objectstack/service-storage`. */
4446
const LINK_TTL_MS = 600_000;
45-
const UNUSABLE = 'This upload link cannot be used: it was used already, it has expired, or it does not exist. Ask for a new link.';
47+
const UNUSABLE =
48+
'This upload link cannot be used: it was used already, it has expired, it was made for someone else, or it does not exist. Ask for a new link.';
4649

4750
// The member's set: CRUD on the invoice graph, so a request reaches the RLS
4851
// layer, plus an OWNER policy on the master invoice. The line carries no RLS of
@@ -141,9 +144,17 @@ describe('[#30892] showcase: an agent reads and attaches a record\'s files over
141144
return { path: toPath(json.url), answer: json };
142145
}
143146

144-
/** Spend a link: an anonymous PUT of `body`, as a browser page or `curl -T` sends it. */
145-
const put = (path: string, body: Buffer, headers: Record<string, string> = {}) =>
146-
stack.raw(path, { method: 'PUT', headers: { 'content-type': 'application/pdf', 'x-file-name': 'receipt.pdf', ...headers }, body });
147+
/** The credential a link request carries: the member's unless named, none for `null`. */
148+
const authOf = (token: string | null): Record<string, string> => (token === null ? {} : { Authorization: `Bearer ${token}` });
149+
/** Open a link's page, signed in as `token`'s user. */
150+
const open = (path: string, token: string | null = memberToken) => stack.raw(path, { headers: authOf(token) });
151+
/** Spend a link: a PUT of `body`, signed in as `token`'s user, as the page's script or `curl -T` sends it. */
152+
const put = (path: string, body: Buffer, headers: Record<string, string> = {}, token: string | null = memberToken) =>
153+
stack.raw(path, {
154+
method: 'PUT',
155+
headers: { 'content-type': 'application/pdf', 'x-file-name': 'receipt.pdf', ...headers, ...authOf(token) },
156+
body,
157+
});
147158

148159
const line = (id: string) => ql.findOne('showcase_invoice_line', { where: { id }, ...SYS });
149160
/** The file id a line's `receipt` holds; a read expands a file reference to its file. */
@@ -217,7 +228,7 @@ describe('[#30892] showcase: an agent reads and attaches a record\'s files over
217228

218229
// The page a person opens: no cache, no referrer, a CSP, and nothing
219230
// about the record or the user on it.
220-
const page = await stack.raw(path);
231+
const page = await open(path);
221232
expect(page.status).toBe(200);
222233
expect(page.headers.get('cache-control')).toContain('no-store');
223234
expect(page.headers.get('referrer-policy')).toBe('no-referrer');
@@ -255,21 +266,40 @@ describe('[#30892] showcase: an agent reads and attaches a record\'s files over
255266
const again = await put(path, minimalPdf('Replaced'));
256267
expect(again.status).toBe(404);
257268
expect(((await again.json()) as any).error).toMatchObject({ code: 'RESOURCE_NOT_FOUND', message: UNUSABLE });
258-
expect((await stack.raw(path)).status).toBe(404);
269+
expect((await open(path)).status).toBe(404);
259270
// An unknown token answers exactly the same.
260271
const unknown = await put('/api/v1/storage/upload-links/00000000-0000-4000-8000-000000000000.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA', pdf);
261272
expect(unknown.status).toBe(404);
262273
expect(((await unknown.json()) as any).error).toMatchObject({ code: 'RESOURCE_NOT_FOUND', message: UNUSABLE });
263274
fileId = (await receiptOf(memberLine))!;
264275
});
265276

277+
it('works only for the member, signed in: anonymous is 401, another signed-in user the same 404, and neither spends it', async () => {
278+
const { path } = await mint({ objectName: 'showcase_invoice_line', recordId: memberLine, field: 'receipt' });
279+
const held = await receiptOf(memberLine);
280+
for (const res of [await open(path, null), await put(path, minimalPdf('Anonymous'), {}, null)]) {
281+
expect(res.status).toBe(401);
282+
expect(((await res.json()) as any).error.code).toBe('AUTH_REQUIRED');
283+
}
284+
expect((await open(path, adminToken)).status).toBe(404);
285+
const stranger = await put(path, minimalPdf('Not yours'), {}, adminToken);
286+
expect(stranger.status).toBe(404);
287+
expect(((await stranger.json()) as any).error).toMatchObject({ code: 'RESOURCE_NOT_FOUND', message: UNUSABLE });
288+
expect(await receiptOf(memberLine)).toBe(held);
289+
// Still open: the member spends it.
290+
const own = await put(path, pdf);
291+
expect(own.status).toBe(200);
292+
fileId = ((await own.json()) as any).data.fileId;
293+
expect(await receiptOf(memberLine)).toBe(fileId);
294+
});
295+
266296
it('expires: past ten minutes the page and the upload answer the same 404, and nothing lands', async () => {
267297
const { path } = await mint({ objectName: 'showcase_invoice_line', recordId: memberLine, field: 'receipt' });
268298
const held = await receiptOf(memberLine);
269299
const now = Date.now();
270300
const spy = vi.spyOn(Date, 'now').mockReturnValue(now + LINK_TTL_MS + 1_000);
271301
try {
272-
expect((await stack.raw(path)).status).toBe(404);
302+
expect((await open(path)).status).toBe(404);
273303
const late = await put(path, minimalPdf('Late'));
274304
expect(late.status).toBe(404);
275305
expect(((await late.json()) as any).error).toMatchObject({ code: 'RESOURCE_NOT_FOUND', message: UNUSABLE });
@@ -330,7 +360,7 @@ describe('[#30892] showcase: an agent reads and attaches a record\'s files over
330360

331361
it('access follows the record: the admin\'s invoice is refused, and its attachment cannot be read through the member\'s own invoice', async () => {
332362
const { path } = await mint({ objectName: 'showcase_invoice', recordId: adminInvoice, attachments: true }, adminToken);
333-
const res = await put(path, Buffer.from('admin only'), { 'content-type': 'text/plain', 'x-file-name': 'admin.txt' });
363+
const res = await put(path, Buffer.from('admin only'), { 'content-type': 'text/plain', 'x-file-name': 'admin.txt' }, adminToken);
334364
expect(res.status).toBe(200);
335365
adminAttachmentId = ((await res.json()) as any).data.attachmentId;
336366

‎packages/services/service-storage/src/file-attachment-service.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,12 +297,12 @@ export class FileAttachmentService
297297
return this.recordFilesOrRefuse().createUploadLinkAs(object, recordId, target, context);
298298
}
299299

300-
async inspectUploadLink(token: string): Promise<UploadLinkView> {
301-
return this.recordFilesOrRefuse().inspectUploadLink(token);
300+
async inspectUploadLink(token: string, callerUserId: string): Promise<UploadLinkView> {
301+
return this.recordFilesOrRefuse().inspectUploadLink(token, callerUserId);
302302
}
303303

304-
async consumeUploadLink(token: string, upload: UploadLinkUpload): Promise<UploadLinkOutcome> {
305-
return this.recordFilesOrRefuse().consumeUploadLink(token, upload);
304+
async consumeUploadLink(token: string, upload: UploadLinkUpload, callerUserId: string): Promise<UploadLinkOutcome> {
305+
return this.recordFilesOrRefuse().consumeUploadLink(token, upload, callerUserId);
306306
}
307307

308308
async attachFile(input: AttachFileInput): Promise<AttachedFile> {

0 commit comments

Comments
 (0)