fix: refuse a bypassed run whose required field is still unanswered

2026-09-06 08:01:35 +00:00 · 2026-09-03 19:10:06 +02:00
parent 42a3488194
commit 03d935090b
2 changed files with 146 additions and 11 deletions
@@ -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 () => {
@@ -5537,6 +5537,44 @@ type FormRunSpec = {
 	startJob: (submitted: Record<string, any>) => Promise<string>
 }

+/** 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<string, any>, proposed: Record<string, any>): 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<string, any>)
+		}
+		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<string> {
@@ -5587,7 +5617,13 @@ async function runThroughForm(spec: FormRunSpec, ctx: WriteDraftCtx): Promise<st
 	// a form answered a tick after it mounts flashes its fields at a user who was never going
 	// to fill them in.
 	const needsUser = formNeedsUser(schema, proposed, strippedKeys)
-	const autoAccepted = postureAnswers && (!toolCallbacks.requestRunArgs || !needsUser)
+	// The posture cannot answer a question it was never asked. A host with no form has nowhere
+	// to put one, so a run still carrying an unanswered field would start missing an argument —
+	// including the secret just stripped out of the model's own proposal.
+	if (!toolCallbacks.requestRunArgs && needsUser) {
+		return 'This chat cannot show a run form, so a script cannot be run from here.'
+	}
+	const autoAccepted = postureAnswers && !needsUser
 	const form: RunFormDisplay = {
 		path: spec.path,
 		summary: spec.summary || undefined,