mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-15 16:03:02 +00:00
fix: redact an adopted run against the schema its page ran
Redaction only visits the leaves the schema it is given declares, and the page runs the deployed version as it stands when Run is pressed. A deploy between this call reading the schema and that press can add a file argument ours never had, and its bytes then passed unredacted into the persisted card and the model's result, under the size limit that would have caught a large one. `adopt` hands over the schema it ran against alongside the job and arguments, and both redactions use it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ef870a52e4
commit
5ea041b4c2
@@ -2137,14 +2137,20 @@ export class AIChatManager implements ChatViewHost {
|
||||
|
||||
/** The reader ran the item on its own page; the tool follows that job rather than
|
||||
* starting one. False when the call is no longer waiting. */
|
||||
handleRunFormJob = (toolId: string, jobId: string, args: Record<string, any>): boolean => {
|
||||
handleRunFormJob = (
|
||||
toolId: string,
|
||||
jobId: string,
|
||||
args: Record<string, any>,
|
||||
schema: Record<string, any>
|
||||
): boolean => {
|
||||
if (!this.isRunFormPending(toolId)) return false
|
||||
// Snapshotted for the same reason the draft is: these are `$state` proxies off the
|
||||
// page's own form, which keeps editing them after the run.
|
||||
this.#settleRunForm(toolId, {
|
||||
kind: 'job',
|
||||
jobId,
|
||||
args: ($state.snapshot(args) ?? {}) as Record<string, any>
|
||||
args: ($state.snapshot(args) ?? {}) as Record<string, any>,
|
||||
schema: ($state.snapshot(schema) ?? {}) as Record<string, any>
|
||||
})
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -5461,7 +5461,8 @@ describe('global AI tools', () => {
|
||||
requestRunArgs: async () => ({
|
||||
kind: 'job',
|
||||
jobId: 'job-123',
|
||||
args: { name: 'Grace' }
|
||||
args: { name: 'Grace' },
|
||||
schema: { properties: { name: { type: 'string' } } }
|
||||
})
|
||||
}
|
||||
)
|
||||
@@ -5474,6 +5475,46 @@ describe('global AI tools', () => {
|
||||
expect(result).not.toContain('Ada')
|
||||
})
|
||||
|
||||
// Redaction only visits the leaves the schema it is given declares, and the page runs the
|
||||
// deployed version as it is now — which a deploy can have given an argument this call's
|
||||
// schema never had. Redacting with ours would put the bytes in the persisted card and in
|
||||
// the model's context.
|
||||
it('run_script redacts an adopted job against the schema that page ran', async () => {
|
||||
vi.mocked(ScriptService.getScriptByPath).mockResolvedValue({
|
||||
path: 'f/scripts/redeployed',
|
||||
schema: { properties: { name: { type: 'string' } } }
|
||||
} as any)
|
||||
|
||||
const statuses: any[] = []
|
||||
const result = await withCompletedTestJob(() =>
|
||||
callGlobalTool(
|
||||
'run_script',
|
||||
{ path: 'f/scripts/redeployed', args: { name: 'Ada' } },
|
||||
{
|
||||
...toolCallbacks,
|
||||
setToolStatus: (_id: string, s: any) => statuses.push(s),
|
||||
requestRunArgs: async () => ({
|
||||
kind: 'job',
|
||||
jobId: 'job-123',
|
||||
args: { name: 'Grace', attachment: 'QUJDREVG' },
|
||||
// The deployed version the page loaded declares a file this call never saw.
|
||||
schema: {
|
||||
properties: {
|
||||
name: { type: 'string' },
|
||||
attachment: { type: 'string', contentEncoding: 'base64' }
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
const parameters = statuses.filter((s) => s.parameters).at(-1)?.parameters
|
||||
expect(parameters?.name).toBe('Grace')
|
||||
expect(parameters?.attachment).not.toBe('QUJDREVG')
|
||||
expect(result).not.toContain('QUJDREVG')
|
||||
})
|
||||
|
||||
// The posture answers wherever it is set, form or no form: what it answers is consent, and a
|
||||
// host without one has nothing left to ask. A secret still becomes a reference first, which
|
||||
// is the only thing the missing form would have done.
|
||||
|
||||
@@ -6397,7 +6397,12 @@ async function runThroughForm(spec: FormRunSpec, ctx: WriteDraftCtx): Promise<st
|
||||
// The card's details pane must show what ran, not what was proposed. Bytes are marked by
|
||||
// size because the card is persisted; everything else stands as the run page shows it for
|
||||
// the same job.
|
||||
const forCard = redactFileArgs(toRun, schema as any)
|
||||
// The page's own, for an adopted run: a deploy between this call reading the schema and the
|
||||
// reader pressing Run can have added arguments, and redaction only visits the leaves the
|
||||
// schema it is given declares — so a file argument missing from ours would keep its bytes
|
||||
// through both the persisted card and the model's result.
|
||||
const ranAgainst = (adopted?.schema ?? schema) as any
|
||||
const forCard = redactFileArgs(toRun, ranAgainst)
|
||||
// The transcript is re-cloned into IndexedDB on every save and a form carries whatever was
|
||||
// pasted into it, so past what the pane would render the card reads the arguments off the
|
||||
// job instead. Only once there is a job to read them from: substituting the marker any
|
||||
@@ -6466,7 +6471,7 @@ async function runThroughForm(spec: FormRunSpec, ctx: WriteDraftCtx): Promise<st
|
||||
: ''
|
||||
// Redacted for the model alone: what it proposed is already in its own tool call, but a
|
||||
// secret the user typed into the form would be entering its context here.
|
||||
const redacted = redactFileArgs(redactSecretArgs(toRun, schema as any), schema as any)
|
||||
const redacted = redactFileArgs(redactSecretArgs(toRun, ranAgainst), ranAgainst)
|
||||
const submittedJson = JSON.stringify(redacted)
|
||||
const shown =
|
||||
submittedJson.length > MAX_SUBMITTED_ARGS_LENGTH
|
||||
|
||||
@@ -615,8 +615,10 @@ export type RunFormDisplay = {
|
||||
export type RunFormAnswer =
|
||||
| { kind: 'args'; args: Record<string, any> }
|
||||
/** A job another surface already started. Its `args` are what that surface sent, already
|
||||
* coerced and minted by its own form, so they are reported rather than processed. */
|
||||
| { kind: 'job'; jobId: string; args: Record<string, any> }
|
||||
* coerced and minted by its own form, so they are reported rather than processed, and
|
||||
* `schema` is the one it ran against — which redaction needs, since it only visits the
|
||||
* leaves a schema declares. */
|
||||
| { kind: 'job'; jobId: string; args: Record<string, any>; schema: Record<string, any> }
|
||||
|
||||
export type RunFormDraft = {
|
||||
args: Record<string, any>
|
||||
|
||||
@@ -315,7 +315,8 @@
|
||||
// The call asked for this run; it follows the job rather than starting one, and
|
||||
// shows it in place of this page — so navigating here as well would leave two
|
||||
// surfaces open on the same job.
|
||||
if (!call?.adopt(run, args)) await onNavigate('/run/' + run + '?workspace=' + workspace)
|
||||
if (!call?.adopt(run, args, flow?.schema ?? {}))
|
||||
await onNavigate('/run/' + run + '?workspace=' + workspace)
|
||||
} catch (e) {
|
||||
throw e
|
||||
} finally {
|
||||
|
||||
@@ -417,7 +417,8 @@
|
||||
// The call asked for this run; it follows the job rather than starting one, and
|
||||
// shows it in place of this page — so navigating here as well would leave two
|
||||
// surfaces open on the same job.
|
||||
if (!call?.adopt(run, args)) await onNavigate('/run/' + run + '?workspace=' + workspace)
|
||||
if (!call?.adopt(run, args, script?.schema ?? {}))
|
||||
await onNavigate('/run/' + run + '?workspace=' + workspace)
|
||||
} catch (err) {
|
||||
sendUserToast(`Could not create job: ${err.body}`, true)
|
||||
} finally {
|
||||
|
||||
@@ -15,10 +15,13 @@ export type PendingRun = {
|
||||
* is the live draft of the card's own form rather than a copy to keep in step. */
|
||||
readonly proposedArgs: Record<string, any>
|
||||
/** The job the reader started here answers the call, with the arguments it was started
|
||||
* with — the card has no other way to show what ran. True when the call took it: the chat
|
||||
* with — the card has no other way to show what ran — and the schema it ran against, which
|
||||
* a deploy can have moved on from the one the call fetched. Both are needed: redaction
|
||||
* only visits the leaves a schema declares, so a file argument this page has and that one
|
||||
* does not would reach the transcript as bytes. True when the call took the job: the chat
|
||||
* then shows that run in place of this page, so the page must not navigate to it as well.
|
||||
* False when the call settled while the job was starting, and the run is the reader's
|
||||
* alone. */
|
||||
adopt: (jobId: string, args: Record<string, any>) => boolean
|
||||
adopt: (jobId: string, args: Record<string, any>, schema: Record<string, any>) => boolean
|
||||
decline: () => void
|
||||
}
|
||||
|
||||
@@ -612,7 +612,7 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
get proposedArgs() {
|
||||
return manager.pendingRunFormArgs(toolCallId) ?? {}
|
||||
},
|
||||
adopt: (jobId, args) => manager.handleRunFormJob(toolCallId, jobId, args),
|
||||
adopt: (jobId, args, schema) => manager.handleRunFormJob(toolCallId, jobId, args, schema),
|
||||
decline: () => manager.handleRunFormCancel(toolCallId)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user