From 9a0c41499b2524a423130df182f3c0486979b2ef Mon Sep 17 00:00:00 2001 From: Santosh Yadav Date: Wed, 4 Nov 2026 07:01:59 +0100 Subject: [PATCH] fix(forms): quote page-derived fixture strings Encode paths and statuses as TypeScript string literals so page-reported values cannot inject fixture statements. Normalize comment metadata and reject unsupported report enums. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../devtools/src/__tests__/forms-cap.test.ts | 18 +++++++++++++++++ .../devtools/src/__tests__/forms-mcp.test.ts | 4 ++-- .../src/__tests__/forms-tools.test.ts | 11 ++++++++++ packages/devtools/src/rpc/forms-explain.ts | 6 +++--- packages/devtools/src/rpc/forms-tools.ts | 20 +++++++++++++++++++ 5 files changed, 54 insertions(+), 5 deletions(-) diff --git a/packages/devtools/src/__tests__/forms-cap.test.ts b/packages/devtools/src/__tests__/forms-cap.test.ts index 9abbf4fc..fc8a9f37 100644 --- a/packages/devtools/src/__tests__/forms-cap.test.ts +++ b/packages/devtools/src/__tests__/forms-cap.test.ts @@ -323,4 +323,22 @@ describe('answers that must stay parseable', () => { expect(code).toContain('items.0.a required, '); expect(code).toContain('and 1980 more not shown (call explain-invalid-forms for all)'); }); + + it('exports page-reported fixture strings as literals and single-line comments', () => { + const form = bigForm(0); + const path = `name'); throw new Error('injected');//`; + const status = `VALID\n'); throw new Error('injected');//`; + form.kind = `reactive\nthrow new Error('injected');//` as CollectedForm['kind']; + form.root.status = status as FormFieldNode['status']; + form.root.children = [control(path, { status: 'DISABLED' })]; + + const text = exportFormText(stateOf(form), { form: 'Orders', format: 'fixture' }); + const code = /```ts\n([\s\S]*)\n```/.exec(text)![1]; + + expect(code).toContain(`form.get(${JSON.stringify(path)})?.disable();`); + expect(code).toContain(`expect(form.status).toBe(${JSON.stringify(status)});`); + expect(code).toContain('(reactive throw new Error'); + expect(code).not.toContain("// status VALID'); throw"); + expect(code.split('\n').filter((line) => line.startsWith('//'))).toHaveLength(3); + }); }); diff --git a/packages/devtools/src/__tests__/forms-mcp.test.ts b/packages/devtools/src/__tests__/forms-mcp.test.ts index fe0f8c05..6954b9a6 100644 --- a/packages/devtools/src/__tests__/forms-mcp.test.ts +++ b/packages/devtools/src/__tests__/forms-mcp.test.ts @@ -459,8 +459,8 @@ describe('forms MCP tools', () => { const { call } = await withForms(); const fixture = await call('export-form', { form: 'Signup', format: 'fixture' }); expect(fixture).toContain('form.patchValue('); - expect(fixture).toContain("form.get('nickname')?.disable();"); - expect(fixture).toContain("expect(form.status).toBe('INVALID');"); + expect(fixture).toContain('form.get("nickname")?.disable();'); + expect(fixture).toContain('expect(form.status).toBe("INVALID");'); expect(fixture).toContain('[redacted]'); const snapshot = await call('export-form', { form: 'Profile' }); expect(snapshot).toContain('"kind": "signal"'); diff --git a/packages/devtools/src/__tests__/forms-tools.test.ts b/packages/devtools/src/__tests__/forms-tools.test.ts index b73f4907..078a4536 100644 --- a/packages/devtools/src/__tests__/forms-tools.test.ts +++ b/packages/devtools/src/__tests__/forms-tools.test.ts @@ -447,6 +447,7 @@ describe('isPageReport', () => { ...valid, forms: [form(control('', { type: 'group', children: [node as FormFieldNode] }))], }); + const firstForm = valid.forms[0]!; for (const report of [ null, [], @@ -457,6 +458,16 @@ describe('isPageReport', () => { { ...valid, forms: {} }, { ...valid, forms: [{ ...valid.forms[0], root: {} }] }, { ...valid, forms: [{ ...valid.forms[0], label: undefined }] }, + { ...valid, forms: [{ ...firstForm, kind: 'reactive\\nthrow new Error()' }] }, + { ...valid, forms: [{ ...firstForm, root: { ...firstForm.root, status: 'VALID\\nthrow' } }] }, + { + ...valid, + forms: [{ ...firstForm, root: { ...firstForm.root, type: 'group; throw' } }], + }, + { + ...valid, + events: [{ ...valid.events[0], type: 'value; throw' }], + }, child({ ...control('email'), errors: undefined }), child({ ...control('email'), errors: [{ kind: 'required' }] }), child({ ...control('email'), children: 'x' }), diff --git a/packages/devtools/src/rpc/forms-explain.ts b/packages/devtools/src/rpc/forms-explain.ts index e2a28a5b..b112ded4 100644 --- a/packages/devtools/src/rpc/forms-explain.ts +++ b/packages/devtools/src/rpc/forms-explain.ts @@ -656,7 +656,7 @@ export function exportFormText( const body = form.kind === 'signal' ? `const model = signal(${literal(p.raw)});\n// const f = form(model, schema);\n// expect(f().valid()).toBe(${form.root.status === 'VALID'});` - : `form.${p.excluded.length ? 'patchValue' : 'setValue'}(${literal(p.raw)});\n${p.excluded.map((path) => `form.get('${path}')?.disable();`).join('\n')}\nexpect(form.status).toBe('${form.root.status}');`; + : `form.${p.excluded.length ? 'patchValue' : 'setValue'}(${literal(p.raw)});\n${p.excluded.map((path) => `form.get(${literal(path)})?.disable();`).join('\n')}\nexpect(form.status).toBe(${literal(form.root.status)});`; const header = fixtureHeader(form, invalid, state.reportedAt); // Header and body share the budget; keep room for the truncation note. const room = Math.max(0, FORM_TOOL_MAX_CHARS - header.length - 1 - 200); @@ -682,8 +682,8 @@ function fixtureHeader(form: CollectedForm, invalid: FormFieldNode[], reportedAt ? ` and ${invalid.length - MAX_HEADER_ERRORS} more not shown (call explain-invalid-forms for all)` : ''; return [ - `// ${line(form.label)} (${form.kind}) captured ${new Date(reportedAt).toISOString()}`, - `// status ${form.root.status}${errors.length ? `; errors: ${errors.join(', ')}${more}` : ''}`, + `// ${line(form.label)} (${line(form.kind)}) captured ${new Date(reportedAt).toISOString()}`, + `// status ${line(form.root.status)}${errors.length ? `; errors: ${errors.join(', ')}${more}` : ''}`, '// Secret values are [redacted]; replace them before running.', ].join('\n'); } diff --git a/packages/devtools/src/rpc/forms-tools.ts b/packages/devtools/src/rpc/forms-tools.ts index ac495d02..ad9c655a 100644 --- a/packages/devtools/src/rpc/forms-tools.ts +++ b/packages/devtools/src/rpc/forms-tools.ts @@ -478,6 +478,22 @@ const OPTIONAL_TYPES: Record = { name: 'string', }; +const FORM_KINDS = new Set(['signal', 'reactive', 'template']); +const FIELD_TYPES = new Set(['group', 'array', 'control']); +const FIELD_STATUSES = new Set(['VALID', 'INVALID', 'PENDING', 'DISABLED']); +const FORM_EVENT_TYPES = new Set([ + 'value', + 'status', + 'touched', + 'dirty', + 'submit', + 'reset', + 'added', + 'removed', + 'moved', + 'validators', +]); + function optionalOk(node: Record): boolean { for (const [key, type] of Object.entries(OPTIONAL_TYPES)) { const value = node[key]; @@ -502,7 +518,9 @@ function isFieldNode(value: unknown, depth = 0): boolean { typeof node.key === 'string' && typeof node.path === 'string' && typeof node.type === 'string' && + FIELD_TYPES.has(node.type) && typeof node.status === 'string' && + FIELD_STATUSES.has(node.status) && typeof node.touched === 'boolean' && typeof node.dirty === 'boolean' && typeof node.bound === 'boolean' && @@ -522,6 +540,7 @@ function isCollectedForm(value: unknown): boolean { typeof form.id === 'string' && typeof form.label === 'string' && typeof form.kind === 'string' && + FORM_KINDS.has(form.kind) && (form.submit === undefined || isRecord(form.submit)) && (form.submitDom === undefined || (isRecord(form.submitDom) && @@ -547,6 +566,7 @@ function isFormEvent(value: unknown): boolean { typeof event.formId === 'string' && typeof event.path === 'string' && typeof event.type === 'string' && + FORM_EVENT_TYPES.has(event.type) && typeof event.timestamp === 'number' ); }