Skip to content

Commit e134201

Browse files
waleedlatif1icecrasher321
authored andcommitted
fix(mothership): close sharing, line-window and snapshot gaps in content delivery
Chat may share or see links only to clean files whatever the surface, text is redacted before a line window cuts it, accounting follows a decision, cached compiles and knowledge metadata bind to the versions they show, row-search matches classify at their matched revision, and table counts follow admitted rows.
1 parent a5ccba8 commit e134201

42 files changed

Lines changed: 841 additions & 116 deletions

Some content is hidden

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

‎.claude/rules/sim-content-delivery.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ the workflow executor see byte-identical results.
3838
redaction reaches: binary, base64, archives, images, public links).
3939
- Gates, installed by the surface: the **registry gate** (Chat tools, CLI, mounts) imports entries
4040
into the registry egress projects with; the **session gate** (run_code sandbox proxy) records
41-
them into the workbench history. Neither ever admits `unknown`, and one refused item never
42-
latches the shared registry.
41+
them into the workbench history. Neither admits `unknown`, with one exception: the CLI's direct
42+
single-file download (`files get`) admits it under `unknown: 'taint'`, which latches the call's
43+
registry so the whole output is withheld and the workbench copy stays unknown. It needs a
44+
registry to latch; without one it refuses. One refused item never latches the shared registry.
4345
- `observeContentOutsideModel` reads for Chat without classifying, for content that never reaches
4446
the model (the user's own edit preview; a derived file that carries its sources' provenance).
4547
Name why at each use.
@@ -73,7 +75,7 @@ own unless noted.
7375
| Bulk download zip | Left out | Left out (opaque) | Included |
7476
| `files search` | Results dropped | Results dropped (a match is an oracle) | Returned |
7577
| File edit (`PATCH` content) | 403 before reading | Diagnostics redacted | Unchanged |
76-
| Chat public share activation | 403 | 403 (a link is opaque) | Shares |
78+
| Chat public share activation, and seeing an existing link (any surface, run_code included) | 403 | 403 (a link leaves every gate) | Shares |
7779
| Rendered view, streamed image, inline attachments | Refused | Refused (opaque) | Delivered |
7880
| Doc compile | Compile refused | Error text redacted | Compiles |
7981
| Run file download | 403 | Redacted | Delivered |
@@ -82,5 +84,5 @@ own unless noted.
8284
| KB export, table export download | Not available to Chat | | |
8385
| Table rows, query, read, `table_selection` | Row dropped (single read: 403) | Redacted | Delivered |
8486
| Row search | Match dropped | Match in a secret column dropped | Returned |
85-
| Table import from a file | Rows stored unknown | Per-cell entries | Exact-empty, as before |
87+
| Table import from a file | Rows stored unknown | Per-cell entries; a row whose coercion rewrote a secret, or whose raw cells cannot be checked, is stored unknown | Exact-empty, as before |
8688
| `tools execute` for file, knowledge, table, memory, function, custom, MCP tools | Not available to Chat | | |

‎.cursor/rules/sim-content-delivery.mdc‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ the workflow executor see byte-identical results.
2929
redaction reaches: binary, base64, archives, images, public links).
3030
- Gates, installed by the surface: the **registry gate** (Chat tools, CLI, mounts) imports entries
3131
into the registry egress projects with; the **session gate** (run_code sandbox proxy) records
32-
them into the workbench history. Neither ever admits `unknown`, and one refused item never
33-
latches the shared registry.
32+
them into the workbench history. Neither admits `unknown`, with one exception: the CLI's direct
33+
single-file download (`files get`) admits it under `unknown: 'taint'`, which latches the call's
34+
registry so the whole output is withheld and the workbench copy stays unknown. It needs a
35+
registry to latch; without one it refuses. One refused item never latches the shared registry.
3436
- `observeContentOutsideModel` reads for Chat without classifying, for content that never reaches
3537
the model (the user's own edit preview; a derived file that carries its sources' provenance).
3638
Name why at each use.
@@ -64,7 +66,7 @@ own unless noted.
6466
| Bulk download zip | Left out | Left out (opaque) | Included |
6567
| `files search` | Results dropped | Results dropped (a match is an oracle) | Returned |
6668
| File edit (`PATCH` content) | 403 before reading | Diagnostics redacted | Unchanged |
67-
| Chat public share activation | 403 | 403 (a link is opaque) | Shares |
69+
| Chat public share activation, and seeing an existing link (any surface, run_code included) | 403 | 403 (a link leaves every gate) | Shares |
6870
| Rendered view, streamed image, inline attachments | Refused | Refused (opaque) | Delivered |
6971
| Doc compile | Compile refused | Error text redacted | Compiles |
7072
| Run file download | 403 | Redacted | Delivered |
@@ -73,5 +75,5 @@ own unless noted.
7375
| KB export, table export download | Not available to Chat | | |
7476
| Table rows, query, read, `table_selection` | Row dropped (single read: 403) | Redacted | Delivered |
7577
| Row search | Match dropped | Match in a secret column dropped | Returned |
76-
| Table import from a file | Rows stored unknown | Per-cell entries | Exact-empty, as before |
78+
| Table import from a file | Rows stored unknown | Per-cell entries; a row whose coercion rewrote a secret, or whose raw cells cannot be checked, is stored unknown | Exact-empty, as before |
7779
| `tools execute` for file, knowledge, table, memory, function, custom, MCP tools | Not available to Chat | | |

‎apps/sim/lib/core/application/content-delivery.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,21 @@ describe('runDeclaredContentOperation', () => {
246246
expect(record.gateFailure).toBe(failure)
247247
})
248248

249+
it('does not account for a result whose admission failed and was caught', async () => {
250+
const failing: ContentDeliveryGate = {
251+
async admit() {
252+
throw new Error('gate down')
253+
},
254+
}
255+
const outcome = await observeContentDelivery(failing, () =>
256+
runDeclaredContentOperation(SOURCED, CHAT, async () => {
257+
await admitContent(CHAT, [source('read')]).catch(() => undefined)
258+
return 'content returned anyway'
259+
}).catch((error: unknown) => error)
260+
)
261+
expect(outcome).toBeInstanceOf(ContentDeliveryUnaccountedError)
262+
})
263+
249264
it('does not let a nested use case account for what its enclosing operation returns', async () => {
250265
const admitAll: ContentDeliveryGate = {
251266
async admit(sources) {

‎apps/sim/lib/core/application/content-delivery.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ export interface ContentSource {
4040
*/
4141
export interface ContentDeliveryGate {
4242
admit(sources: readonly ContentSource[]): Promise<readonly boolean[]>
43+
/**
44+
* The text as this surface's egress will show it, or `undefined` when it cannot be shown. A gate
45+
* whose surface redacts (Chat's registry) implements it; see {@link redactBeforeCutting}.
46+
*/
47+
project?(text: string): string | undefined
4348
}
4449

4550
/**
@@ -135,6 +140,24 @@ export function observeContentDelivery<T>(gate: ContentDeliveryGate, execute: ()
135140
return gates.run(gate, execute)
136141
}
137142

143+
/**
144+
* Text a use case is about to cut (a line window, a clip), redacted first by the surface's gate.
145+
* Egress redacts whole secrets only, so a cut through one would leave a fragment it cannot
146+
* recognize. Other callers, and surfaces that do not redact, get the text unchanged.
147+
*/
148+
export function redactBeforeCutting(
149+
principal: Principal | undefined,
150+
text: string,
151+
kind: ContentSourceKind
152+
): string {
153+
if (principal && !isCopilotPrincipal(principal)) return text
154+
const project = gates.getStore()?.project
155+
if (!project) return text
156+
const projected = project(text)
157+
if (projected === undefined) throw new ContentRefusedError(kind)
158+
return projected
159+
}
160+
138161
/** Admits every item: see {@link observeContentOutsideModel}. */
139162
const OUTSIDE_MODEL_GATE: ContentDeliveryGate = {
140163
async admit(sources) {
@@ -222,8 +245,9 @@ export async function admitContent(
222245
) {
223246
throw new ContentDeliveryUndeclaredError()
224247
}
225-
markAccounted()
248+
/** Accounting follows a decision: a caller that catches a failure has classified nothing. */
226249
if (sources.length === 0 || !mustClassifyContent(principal)) {
250+
markAccounted()
227251
return new ContentAdmission(
228252
sources,
229253
sources.map(() => true)
@@ -242,6 +266,7 @@ export async function admitContent(
242266
}
243267
throw error
244268
}
269+
markAccounted()
245270
if (ledger) {
246271
sources.forEach((source, index) => {
247272
if (verdicts[index]) {
@@ -274,6 +299,8 @@ export async function admitWrittenContent(
274299
*/
275300
const [first] = sources
276301
if (!ledgers.getStore() || !first) throw error
302+
/** The result is decided after all: withheld whole, as saved-not-shown. */
303+
markAccounted()
277304
withholdAfterWrite(first.kind)
278305
return new ContentAdmission(
279306
sources,

‎apps/sim/lib/knowledge/application/admit-knowledge-content.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { admitEach, type ContentSourceKind } from '@/lib/core/application/conten
33
import {
44
classifyKnowledgeChunks,
55
classifyKnowledgeDocumentMetadata,
6+
type KnowledgeDocumentSnapshot,
7+
knowledgeDocumentSnapshot,
68
} from '@/lib/knowledge/secret-provenance'
79

810
interface KnowledgeAdmissionOptions {
@@ -16,18 +18,28 @@ const writtenAs = (options: KnowledgeAdmissionOptions, kind: ContentSourceKind)
1618
/**
1719
* Admits each document whose rendered metadata a result returns, on its own, and returns the ids
1820
* admitted. `value` is what the result shows of the document, so only secrets it contains are
19-
* imported. A caller no gate observes gets every id back without a provenance read.
21+
* imported; `snapshot` names the stored fields it shows, when they are not its own keys. A caller
22+
* no gate observes gets every id back without a provenance read.
2023
*/
2124
export async function admitKnowledgeDocuments(
2225
principal: Principal,
23-
documents: readonly { id: string; value: unknown }[],
26+
documents: readonly { id: string; value: unknown; snapshot?: KnowledgeDocumentSnapshot }[],
2427
options: KnowledgeAdmissionOptions = {}
2528
): Promise<Set<string>> {
2629
const admitted = await admitEach(
2730
principal,
2831
documents,
2932
async (items) => {
30-
const provenance = await classifyKnowledgeDocumentMetadata(items.map((item) => item.id))
33+
/** Each classification is bound to the metadata the item shows; see the classifier. */
34+
const snapshots = new Map<string, KnowledgeDocumentSnapshot[]>()
35+
for (const item of items) {
36+
const snapshot = item.snapshot ?? knowledgeDocumentSnapshot(item.value)
37+
if (snapshot) snapshots.set(item.id, [...(snapshots.get(item.id) ?? []), snapshot])
38+
}
39+
const provenance = await classifyKnowledgeDocumentMetadata(
40+
items.map((item) => item.id),
41+
snapshots
42+
)
3143
return items.map((item) => ({
3244
kind: 'knowledge-document' as const,
3345
id: item.id,

‎apps/sim/lib/knowledge/application/chunks.test.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ import { admitContent } from '@/lib/core/application/content-delivery'
4949
import { ForbiddenOperationError } from '@/lib/core/application/forbidden'
5050
import { WORKSPACE_ACCESS_SCOPE } from '@/lib/knowledge/access/scope'
5151
import { KnowledgeDocumentNotReadyError } from '@/lib/knowledge/application/chunk-errors'
52-
import { bulkUpdateKnowledgeChunks, listKnowledgeChunks } from '@/lib/knowledge/application/chunks'
52+
import {
53+
bulkUpdateKnowledgeChunks,
54+
createKnowledgeChunk,
55+
listKnowledgeChunks,
56+
} from '@/lib/knowledge/application/chunks'
5357

5458
describe('knowledge chunk application use cases', () => {
5559
beforeEach(() => {
@@ -75,6 +79,37 @@ describe('knowledge chunk application use cases', () => {
7579
})
7680
}
7781

82+
it('admits a created chunk with the document name its response shows', async () => {
83+
workspaceAuthzMockFns.mockResolveEffectiveWorkspacePermission.mockResolvedValue('write')
84+
knowledgeContextsMockFns.mockResolveCanonicalActiveKnowledgeDocumentContext.mockResolvedValue({
85+
workspaceId: 'workspace-1',
86+
workspaceOrganizationId: null,
87+
allowPersonalApiKeys: true,
88+
billedAccountUserId: 'billing-owner-1',
89+
access: { get: async () => WORKSPACE_ACCESS_SCOPE },
90+
knowledgeBaseId: 'knowledge-1',
91+
knowledgeBase: { id: 'knowledge-1', embeddingModel: 'text-embedding-3-small' },
92+
documentId: 'document-1',
93+
document: { id: 'document-1', processingStatus: 'completed', filename: 'named.txt' },
94+
})
95+
mocks.createChunk.mockResolvedValue({ id: 'chunk-new', content: 'body', tokenCount: 1 })
96+
admitOnly(['chunk-new'])
97+
98+
const result = await createKnowledgeChunk.execute({
99+
principal: createSessionPrincipal(),
100+
input: {
101+
knowledgeBaseId: 'knowledge-1',
102+
documentId: 'document-1',
103+
content: 'body',
104+
resolveContentProvenance: () => undefined,
105+
},
106+
})
107+
108+
expect(result.chunk.documentName).toBe('named.txt')
109+
const [, [item]] = mocks.admitKnowledgeChunks.mock.calls[0]
110+
expect(item.value).toMatchObject({ documentId: 'document-1', documentName: 'named.txt' })
111+
})
112+
78113
describe('a searched chunk listing', () => {
79114
const page = {
80115
chunks: [

‎apps/sim/lib/knowledge/application/chunks.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,14 @@ async function requireAdmittedChunk(
7575
principal: Principal,
7676
documentId: string,
7777
chunk: { id: string; content: string },
78-
options: { written?: boolean } = {}
78+
options: { written?: boolean; shown?: Record<string, unknown> } = {}
7979
): Promise<void> {
80+
/** `shown` is every other field the response adds beside the chunk, such as its document name. */
81+
const value = options.shown ? { ...chunk, ...options.shown } : chunk
8082
const admitted = await admitKnowledgeChunks(
8183
principal,
82-
[{ id: chunk.id, documentId, content: chunk.content, value: chunk }],
83-
options
84+
[{ id: chunk.id, documentId, content: chunk.content, value }],
85+
{ written: options.written }
8486
)
8587
if (!options.written && !admitted.has(chunk.id)) throw new ContentRefusedError('knowledge-chunk')
8688
}
@@ -239,7 +241,10 @@ export const createKnowledgeChunk = defineAuthorizedKnowledgeUseCase({
239241
} catch (error) {
240242
logger.warn('Failed to calculate cost for chunk upload', { error })
241243
}
242-
await requireAdmittedChunk(principal, context.documentId, chunk, { written: true })
244+
await requireAdmittedChunk(principal, context.documentId, chunk, {
245+
written: true,
246+
shown: { documentId: context.documentId, documentName: context.document.filename },
247+
})
243248
return {
244249
chunk: {
245250
...chunk,

‎apps/sim/lib/knowledge/application/tags.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,31 @@ describe('knowledge tag application use cases', () => {
396396
)
397397
})
398398

399+
it('answers Chat an empty usage for a base with no tag definitions', async () => {
400+
mocks.readDetailedUsage.mockResolvedValue([])
401+
await expect(
402+
readDetailedKnowledgeTagUsage.execute({
403+
principal: chat,
404+
input: { knowledgeBaseId: 'knowledge-b' },
405+
})
406+
).resolves.toMatchObject({ usage: [] })
407+
expect(admission.admitKnowledgeDocuments).not.toHaveBeenCalled()
408+
})
409+
410+
it('binds each occurrence to the stored tag value it shows', async () => {
411+
await readDetailedKnowledgeTagUsage.execute({
412+
principal: chat,
413+
input: { knowledgeBaseId: 'knowledge-b' },
414+
})
415+
const [, documents] = admission.admitKnowledgeDocuments.mock.calls[1]
416+
expect(documents).toEqual([
417+
expect.objectContaining({
418+
id: 'doc-shared',
419+
snapshot: { filename: 'shared.csv', tag2: 'ops' },
420+
}),
421+
])
422+
})
423+
399424
it('decides each (definition, document) occurrence on its own', async () => {
400425
const result = await readDetailedKnowledgeTagUsage.execute({
401426
principal: chat,

‎apps/sim/lib/knowledge/application/tags.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,17 @@ async function admitTagUsage(
363363
principal: Principal,
364364
usage: Awaited<ReturnType<typeof getTagUsage>>
365365
): Promise<Awaited<ReturnType<typeof getTagUsage>>> {
366+
/** A base with no tag definitions returns nothing, which is still an accounting. */
367+
if (usage.length === 0) await admitContent(principal, [])
366368
const admitted = []
367369
for (const definition of usage) {
368370
const ids = await admitKnowledgeDocuments(
369371
principal,
370-
definition.documents.map((doc) => ({ id: doc.id, value: doc }))
372+
definition.documents.map((doc) => ({
373+
id: doc.id,
374+
value: doc,
375+
snapshot: { filename: doc.name, [definition.tagSlot]: doc.tagValue },
376+
}))
371377
)
372378
const documents = definition.documents.filter((doc) => ids.has(doc.id))
373379
admitted.push(

‎apps/sim/lib/knowledge/secret-provenance.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
55
import type { DbTransaction } from '@/lib/db/types'
66
import type { DurableSecretProvenance } from '@/lib/execution/durable-secret-provenance'
77
import {
8+
classifyKnowledgeDocumentMetadata,
89
createKnowledgeDocumentSourceValue,
910
importKnowledgePersistedResponseSecretProvenance,
1011
importKnowledgeSearchResultSecretProvenance,
12+
knowledgeDocumentSnapshot,
1113
loadKnowledgeDocumentSecretRegistry,
1214
replaceKnowledgeDocumentSecretProvenanceInTx,
1315
} from '@/lib/knowledge/secret-provenance'
@@ -167,3 +169,50 @@ describe('knowledge durable provenance enforcement', () => {
167169
expect(registry.isPermanentlyIncomplete()).toBe(true)
168170
})
169171
})
172+
173+
describe('document metadata classification bound to the shown snapshot', () => {
174+
const NUMBERED_ROW = { ...DOCUMENT_ROW, tag1: 'emea', number1: 42 }
175+
176+
beforeEach(() => {
177+
resetDbChainMock()
178+
})
179+
180+
it('keeps the stored classification while the shown metadata is what is stored', async () => {
181+
queueTableRows(document, [NUMBERED_ROW])
182+
const result = await classifyKnowledgeDocumentMetadata(
183+
[NUMBERED_ROW.id],
184+
new Map([[NUMBERED_ROW.id, [{ filename: 'source.pdf', tag1: 'emea', number1: '42' }]]])
185+
)
186+
expect(result.get(NUMBERED_ROW.id)?.status).toBe('exact')
187+
})
188+
189+
it.each([
190+
['a renamed document', { filename: 'previous-name.pdf' }],
191+
['an edited tag', { tag1: 'previous value' }],
192+
['a tag cleared since', { tag2: 'previous value' }],
193+
['an edited number', { number1: '7' }],
194+
])(
195+
'reads %s as unknown, so stale values are not paired with new provenance',
196+
async (_, shown) => {
197+
queueTableRows(document, [NUMBERED_ROW])
198+
const result = await classifyKnowledgeDocumentMetadata(
199+
[NUMBERED_ROW.id],
200+
new Map([[NUMBERED_ROW.id, [shown]]])
201+
)
202+
expect(result.get(NUMBERED_ROW.id)).toEqual({ status: 'unknown' })
203+
}
204+
)
205+
206+
it('picks only the editable metadata a returned value shows', () => {
207+
expect(
208+
knowledgeDocumentSnapshot({
209+
id: 'd',
210+
filename: 'f.pdf',
211+
tag3: 'x',
212+
fileUrl: '/u',
213+
date1: 'd',
214+
})
215+
).toEqual({ filename: 'f.pdf', tag3: 'x' })
216+
expect(knowledgeDocumentSnapshot({ id: 'd', fileUrl: '/u' })).toBeUndefined()
217+
})
218+
})

0 commit comments

Comments
 (0)