From 03d935090bc916f32f5ab7cfba5a6cab1bfdc2ba Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:10:06 +0200 Subject: [PATCH] fix: refuse a bypassed run whose required field is still unanswered --- .../copilot/chat/global/core.test.ts | 101 +++++++++++++++++- .../components/copilot/chat/global/core.ts | 56 ++++++++-- 2 files changed, 146 insertions(+), 11 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index 8fc22a60e3..6a58f894e8 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -4501,7 +4501,11 @@ describe('global AI tools', () => { } as any) const secret: any[] = [] await withCompletedTestJob(() => - callGlobalTool('run_script', { path: 'f/scripts/secret', args: { token: 'hunter2' } }, yolo(secret)) + callGlobalTool( + 'run_script', + { path: 'f/scripts/secret', args: { token: 'hunter2' } }, + yolo(secret) + ) ) expect(secret.find((x) => x.runForm)?.runForm.submitted).toBeUndefined() @@ -4517,6 +4521,75 @@ describe('global AI tools', () => { expect(ready.find((x) => x.runForm)?.runForm.submitted).toBe(true) }) + // What the mounted form would have refused to submit, the bypass must not start: ArgInput + // marks a required empty scalar invalid and disables Run, and a nested required field is a + // question the form would have shown. Neither is a value the posture can answer for. + it('opens a run form under yolo for an empty or nested-missing required field', async () => { + const yolo = (statuses: any[]) => ({ + ...toolCallbacks, + setToolStatus: (_toolId: string, status: any) => statuses.push(status), + shouldAutoAcceptToolConfirmations: () => true, + requestRunArgs: async (_toolId: string, form: any) => form.args + }) + + // Required, and the model sent the empty string the form refuses to submit. + vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce({ + path: 'f/scripts/blank', + schema: { properties: { name: { type: 'string' } }, required: ['name'] } + } as any) + const blank: any[] = [] + await withCompletedTestJob(() => + callGlobalTool('run_script', { path: 'f/scripts/blank', args: { name: '' } }, yolo(blank)) + ) + expect(blank.find((x) => x.runForm)?.runForm.submitted).toBeUndefined() + + // Required below the top level, where the declaration says exactly which fields the + // form would have rendered. + vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce({ + path: 'f/scripts/nested', + schema: { + properties: { + config: { + type: 'object', + properties: { api_key: { type: 'string' } }, + required: ['api_key'] + } + }, + required: ['config'] + } + } as any) + const nested: any[] = [] + await withCompletedTestJob(() => + callGlobalTool('run_script', { path: 'f/scripts/nested', args: { config: {} } }, yolo(nested)) + ) + expect(nested.find((x) => x.runForm)?.runForm.submitted).toBeUndefined() + + // Answered at both levels: nothing is outstanding, so the posture still answers and no + // field is mounted. Without this the guard above could pass by never bypassing at all. + vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce({ + path: 'f/scripts/filled', + schema: { + properties: { + config: { + type: 'object', + properties: { api_key: { type: 'string' } }, + required: ['api_key'] + } + }, + required: ['config'] + } + } as any) + const filled: any[] = [] + await withCompletedTestJob(() => + callGlobalTool( + 'run_script', + { path: 'f/scripts/filled', args: { config: { api_key: 'k' } } }, + yolo(filled) + ) + ) + expect(filled.find((x) => x.runForm)?.runForm.submitted).toBe(true) + }) + // The bypass is the user's standing answer, not a licence for the host to skip asking: // a chat with nowhere to put a form still refuses the run under any other posture. it('run_script refuses a host with no form unless the posture answers for it', async () => { @@ -4535,6 +4608,32 @@ describe('global AI tools', () => { expect(refused).toContain('cannot show a run form') }) + // The posture answers for a host that has a form; it cannot answer for one that has none. + // A secret is stripped from the proposal whatever the posture, so bypassing here would run + // the script missing the very argument the model tried to supply. + it('run_script refuses a formless host under yolo when a field is left unanswered', async () => { + vi.mocked(ScriptService.getScriptByPath).mockResolvedValue({ + path: 'f/scripts/noform-secret', + schema: { + properties: { token: { type: 'string', password: true } }, + required: ['token'] + } + } as any) + + const refused = await callGlobalTool( + 'run_script', + { path: 'f/scripts/noform-secret', args: { token: 'hunter2' } }, + { + ...toolCallbacks, + requestRunArgs: undefined, + shouldAutoAcceptToolConfirmations: () => true + } + ) + + expect(JobService.runScriptByPath).not.toHaveBeenCalled() + expect(refused).toContain('cannot show a run form') + }) + // The transcript is re-cloned into IndexedDB on every save, and a form takes as much text // as the user pastes. What the card stores is bounded; what the job runs is not. it('run_script stores a marker for oversized arguments but runs them in full', async () => { diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index c40ecb6e9e..8aef0a8fd7 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -5537,6 +5537,44 @@ type FormRunSpec = { startJob: (submitted: Record) => Promise } +/** Whether a required field carries no answer. `''` counts as unanswered because the mounted + * form says so — ArgInput marks a required empty scalar invalid and disables Run — so treating + * it as filled would let the bypass start a run the form itself would have refused. Objects are + * exempt for the same reason, inverted: ArgInput skips them in that check, so an empty one is + * the form's business and not a missing answer. */ +function requiredValueMissing(value: unknown): boolean { + if (value === undefined || value === null) return true + return value === '' +} + +/** Whether a required key is unanswered anywhere the mounted form would have shown a field for + * it. Descends only into a declaration that names its own `properties` and `required`: there the + * shape is unambiguous and the form would render those fields. `oneOf` and free-form objects are + * left alone — resolving which branch is open is guesswork, and guessing "needs the user" parks + * the test-and-iterate loop the bypass posture exists to serve. */ +function requiredUnanswered(schema: Record, proposed: Record): boolean { + const required = schema?.required + if (!Array.isArray(required)) return false + const properties = schema?.properties ?? {} + return required.some((key) => { + if (typeof key !== 'string') return false + const declared = Object.hasOwn(properties, key) ? properties[key] : undefined + const value = proposed?.[key] + if (requiredValueMissing(value)) return declared?.default === undefined + if ( + declared && + !Array.isArray(declared.oneOf) && + declared.properties && + Array.isArray(declared.required) && + typeof value === 'object' && + !Array.isArray(value) + ) { + return requiredUnanswered(declared, value as Record) + } + return false + }) +} + /** Whether the form holds something the model could not have supplied, so the bypass posture * has nothing to answer with. Either it was stripped for being the user's to give — a secret, * a file — or the schema requires it and neither the proposal nor a default carries a value. */ @@ -5546,15 +5584,7 @@ function formNeedsUser( strippedKeys: string[] ): boolean { if (strippedKeys.length > 0) return true - const required = schema?.required - if (!Array.isArray(required)) return false - const properties = schema?.properties ?? {} - return required.some((key) => { - if (typeof key !== 'string') return false - if (proposed[key] !== undefined && proposed[key] !== null) return false - const declared = Object.hasOwn(properties, key) ? properties[key] : undefined - return declared?.default === undefined - }) + return requiredUnanswered(schema, proposed) } async function runThroughForm(spec: FormRunSpec, ctx: WriteDraftCtx): Promise { @@ -5587,7 +5617,13 @@ async function runThroughForm(spec: FormRunSpec, ctx: WriteDraftCtx): Promise