Uh oh!
There was an error while loading. Please reload this page.
feat(deployed-form): added deployed form input - #2679
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
emir-karabegforce-pushed
the
feat/deploy-form
branch
from
January 10, 2026 06:02
bb4c470 to
dc2523cCompareemir-karabeg
marked this pull request as ready for review
January 10, 2026 07:07
emir-karabeg
commented
Jan 10, 2026
Collaborator
emir-karabegforce-pushed
the
feat/deploy-form
branch
from
January 10, 2026 07:11
d414838 to
9fbef1fCompareContributor
Greptile OverviewGreptile SummaryOverviewThis PR introduces a deployed forms feature allowing workflows to be shared as public/password-protected/email-gated embeddable forms. The implementation mirrors the chat deployment pattern with form creation/management APIs, public form UI, authentication options, and field customization. Critical Issues Found (6 Major Bugs)🔴 Priority 1: File Uploads Completely BrokenLocation: Form fields support file uploads ( Chat deployment handles this correctly by converting files to base64 and processing them server-side with 🔴 Priority 1: No Required Field ValidationLocation: Users can submit forms without filling required fields. The UI shows red asterisks (*) for required fields but performs zero validation on submit. Empty/undefined values for required fields are accepted. 🔴 Priority 1: Prototype Pollution RiskLocation: Spreading user-submitted Chat deployment structures input explicitly without spreading user data. 🟡 Priority 2: Missing Email Auth UILocation: Email authentication is fully supported in the backend ( Password auth has a 🟡 Priority 2: Object/Array Fields Have Type IssuesLocation: Object and array fields store values as strings (users manually type JSON), but there's no validation that the JSON is valid before submission. This creates type mismatches and allows invalid JSON to be sent to workflows. 🟡 Priority 2: Number Fields Allow Invalid InputLocation: Number field converts input to Architecture NotesWhat Works Well
Gaps and Inconsistencies
Security Considerations
Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User as User Browser
participant FormUI as Form Page
participant FormAPI as Form API Endpoint
participant AuthUtil as Auth Validation
participant DB as Database
participant Engine as Workflow Engine
Note over User,Engine: Initial Form Load (GET)
User->>FormUI: Visit form URL
FormUI->>FormAPI: GET with identifier
FormAPI->>DB: Query form config
DB-->>FormAPI: Return form details
FormAPI->>AuthUtil: Validate authentication
alt Authentication Required
AuthUtil-->>FormAPI: Auth needed
FormAPI-->>FormUI: 401 with auth type
FormUI-->>User: Show auth screen
User->>FormUI: Submit credentials
FormUI->>FormAPI: POST with credentials
AuthUtil-->>FormAPI: Validation passes
FormAPI-->>FormUI: Return form schema
else Public Access
AuthUtil-->>FormAPI: Access granted
FormAPI-->>FormUI: Return form schema
end
FormUI-->>User: Display form fields
Note over User,Engine: Form Submission (POST)
User->>FormUI: Fill and submit form
Note right of FormUI: Issue: No validation<br/>Issue: JSON stringify breaks files
FormUI->>FormAPI: POST form data
FormAPI->>AuthUtil: Validate access
AuthUtil-->>FormAPI: Access granted
FormAPI->>DB: Check form is active
alt Form Inactive
FormAPI-->>FormUI: 403 Unavailable
end
FormAPI->>FormAPI: Preprocess execution
Note right of FormAPI: Rate limiting checks<br/>Deployment validation
FormAPI->>FormAPI: Build workflow input
Note right of FormAPI: Issue: Spreads formData<br/>Issue: Wrong trigger type
FormAPI->>Engine: Execute workflow
Engine-->>FormAPI: Stream results
FormAPI->>FormAPI: Consume stream
FormAPI-->>FormUI: Success response
FormUI-->>User: Show thank you screen
|
| const response = await fetch(`/api/form/${identifier}`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ formData }), |
Contributor
There was a problem hiding this comment.
File objects cannot be serialized to JSON. When a form field has type files, the formData object contains File objects which will cause JSON.stringify() to fail or convert them to empty objects {}.
This breaks file upload functionality completely. You need to either:
- Use
FormDatainstead of JSON for file uploads - Convert files to base64 strings before JSON serialization
- Upload files separately and pass file references in the form data
Suggested change
| body: JSON.stringify({ formData }), | |
| // For forms with files, use FormData instead of JSON | |
| consthasFiles=Object.values(formData).some(v=>vinstanceofFile||(Array.isArray(v)&&v.some(item=>iteminstanceofFile))) | |
| letrequestBody | |
| letheaders={} | |
| if(hasFiles){ | |
| constformDataObj=newFormData() | |
| formDataObj.append('formData',JSON.stringify(formData))// Note: files still need special handling | |
| requestBody=formDataObj | |
| }else{ | |
| headers={'Content-Type': 'application/json'} | |
| requestBody=JSON.stringify({ formData }) | |
| } | |
| constresponse=awaitfetch(`/api/form/${identifier}`,{ | |
| method: 'POST', | |
| headers, | |
| body: requestBody, | |
| }) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/form.tsx
Line: 170:170
Comment:
File objects cannot be serialized to JSON. When a form field has type `files`, the `formData` object contains File objects which will cause `JSON.stringify()` to fail or convert them to empty objects `{}`.
This breaks file upload functionality completely. You need to either:
1. Use `FormData` instead of JSON for file uploads
2. Convert files to base64 strings before JSON serialization
3. Upload files separately and pass file references in the form data
```suggestion // For forms with files, use FormData instead of JSON const hasFiles = Object.values(formData).some(v => v instanceof File || (Array.isArray(v) && v.some(item => item instanceof File))) let requestBody let headers = {} if (hasFiles) { const formDataObj = new FormData() formDataObj.append('formData', JSON.stringify(formData)) // Note: files still need special handling requestBody = formDataObj } else { headers = { 'Content-Type': 'application/json' } requestBody = JSON.stringify({ formData }) } const response = await fetch(`/api/form/${identifier}`, { method: 'POST', headers, body: requestBody, })```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+222
to
+225
| const workflowInput = { | ||
| input: formData, | ||
| ...formData, // Spread form fields at top level for convenience | ||
| } |
Contributor
There was a problem hiding this comment.
Prototype pollution vulnerability: Using the spread operator ...formData without filtering dangerous keys allows attackers to inject properties like __proto__, constructor, or prototype.
An attacker could submit:
{
"formData": {
"__proto__": { "isAdmin": true },
"name": "John"
}
}This pollutes the prototype chain of all objects, potentially leading to authentication bypasses or privilege escalation.
Suggested change
| constworkflowInput={ | |
| input: formData, | |
| ...formData,// Spread form fields at top level for convenience | |
| } | |
| // Pass form data as the workflow input - SAFELY filter dangerous keys | |
| constsafeFormData=Object.fromEntries( | |
| Object.entries(formData).filter(([key])=> | |
| !['__proto__','constructor','prototype'].includes(key) | |
| ) | |
| ) | |
| constworkflowInput={ | |
| input: safeFormData, | |
| ...safeFormData, | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/form/[identifier]/route.ts
Line: 222:225
Comment:
Prototype pollution vulnerability: Using the spread operator `...formData` without filtering dangerous keys allows attackers to inject properties like `__proto__`, `constructor`, or `prototype`.
An attacker could submit:
```json
{
"formData": {
"__proto__": { "isAdmin": true },
"name": "John"
}
}
```
This pollutes the prototype chain of all objects, potentially leading to authentication bypasses or privilege escalation.
```suggestion // Pass form data as the workflow input - SAFELY filter dangerous keys const safeFormData = Object.fromEntries( Object.entries(formData).filter(([key]) => !['__proto__', 'constructor', 'prototype'].includes(key) ) ) const workflowInput = { input: safeFormData, ...safeFormData, }```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+166
to
+170
| if ((password || email) && !formData) { | ||
| const response = addCorsHeaders(createSuccessResponse({ authenticated: true }), request) | ||
| setFormAuthCookie(response, deployment.id, deployment.authType, deployment.password) | ||
| return response | ||
| } |
Contributor
There was a problem hiding this comment.
Authentication cookie is only set when credentials are provided WITHOUT form data, but NOT when form data is submitted successfully. This means users must re-authenticate on every single form submission, breaking the user experience for password/email-protected forms.
Suggested change
| if((password||email)&&!formData){ | |
| constresponse=addCorsHeaders(createSuccessResponse({authenticated: true}),request) | |
| setFormAuthCookie(response,deployment.id,deployment.authType,deployment.password) | |
| returnresponse | |
| } | |
| // If only authentication credentials provided (no form data), just return authenticated | |
| if((password||email)&&!formData){ | |
| constresponse=addCorsHeaders(createSuccessResponse({authenticated: true}),request) | |
| setFormAuthCookie(response,deployment.id,deployment.authType,deployment.password) | |
| returnresponse | |
| } | |
| // Set auth cookie for authenticated users before processing form | |
| constshouldSetCookie=deployment.authType!=='public'&&(password||email) |
Then after line 283 (in the success response), add:
constresponse=addCorsHeaders(createSuccessResponse({success: true,
executionId,thankYouTitle: customizations?.thankYouTitle||'Thank you!',thankYouMessage:
customizations?.thankYouMessage||'Your response has been submitted successfully.',}),request)// Set auth cookie so user stays authenticatedif(deployment.authType!=='public'){setFormAuthCookie(response,deployment.id,deployment.authType,deployment.password)}returnresponsePrompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/form/[identifier]/route.ts
Line: 166:170
Comment:
Authentication cookie is only set when credentials are provided WITHOUT form data, but NOT when form data is submitted successfully. This means users must re-authenticate on every single form submission, breaking the user experience for password/email-protected forms.
```suggestion // If only authentication credentials provided (no form data), just return authenticated if ((password || email) && !formData) { const response = addCorsHeaders(createSuccessResponse({ authenticated: true }), request) setFormAuthCookie(response, deployment.id, deployment.authType, deployment.password) return response } // Set auth cookie for authenticated users before processing form const shouldSetCookie = deployment.authType !== 'public' && (password || email)```
Then after line 283 (in the success response), add:
```typescriptconst response =addCorsHeaders(
createSuccessResponse({
success: true,
executionId,
thankYouTitle: customizations?.thankYouTitle||'Thank you!',
thankYouMessage:
customizations?.thankYouMessage||'Your response has been submitted successfully.',
}),
request
)
// Set auth cookie so user stays authenticatedif (deployment.authType!=='public') {
setFormAuthCookie(response, deployment.id, deployment.authType, deployment.password)
}
returnresponse```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+119
to
+130
| case 'object': | ||
| case 'array': | ||
| return ( | ||
| <Textarea | ||
| value={(value as string) ?? ''} | ||
| onChange={(e: React.ChangeEvent<HTMLTextAreaElement>) => onChange(e.target.value)} | ||
| placeholder={ | ||
| placeholder || (field.type === 'array' ? '["item1", "item2"]' : '{"key": "value"}') | ||
| } | ||
| className='min-h-[100px] rounded-[10px] font-mono text-[13px] shadow-sm transition-colors focus:border-gray-400 focus:ring-2 focus:ring-gray-100' | ||
| /> | ||
| ) |
Contributor
There was a problem hiding this comment.
Object and array fields accept raw string input without JSON validation. Users can type invalid JSON which will be submitted to the workflow, causing runtime errors during execution.
The field should validate JSON syntax and show an error indicator when invalid JSON is entered.
Suggested change
| case'object': | |
| case'array': | |
| return( | |
| <Textarea | |
| value={(valueasstring)??''} | |
| onChange={(e: React.ChangeEvent<HTMLTextAreaElement>)=>onChange(e.target.value)} | |
| placeholder={ | |
| placeholder||(field.type==='array' ? '["item1", "item2"]' : '{"key": "value"}') | |
| } | |
| className='min-h-[100px] rounded-[10px] font-mono text-[13px] shadow-sm transition-colors focus:border-gray-400 focus:ring-2 focus:ring-gray-100' | |
| /> | |
| ) | |
| case'object': | |
| case'array': { | |
| const[jsonError,setJsonError]=useState<string|null>(null) | |
| consthandleJsonChange=(text: string)=>{ | |
| try{ | |
| if(text.trim()===''){ | |
| onChange(field.type==='array' ? [] : {}) | |
| setJsonError(null) | |
| return | |
| } | |
| constparsed=JSON.parse(text) | |
| onChange(parsed) | |
| setJsonError(null) | |
| }catch{ | |
| setJsonError('Invalid JSON') | |
| // Store the text so user doesn't lose their input | |
| onChange(text) | |
| } | |
| } | |
| return( | |
| <div> | |
| <Textarea | |
| value={typeofvalue==='string' ? value : JSON.stringify(value,null,2)} | |
| onChange={(e: React.ChangeEvent<HTMLTextAreaElement>)=>handleJsonChange(e.target.value)} | |
| placeholder={ | |
| placeholder||(field.type==='array' ? '["item1", "item2"]' : '{"key": "value"}') | |
| } | |
| className={cn( | |
| 'min-h-[100px] rounded-[10px] font-mono text-[13px] shadow-sm transition-colors focus:border-gray-400 focus:ring-2 focus:ring-gray-100', | |
| jsonError&&'border-red-500' | |
| )} | |
| /> | |
| {jsonError&&( | |
| <pclassName="mt-1 text-xs text-red-500">{jsonError}</p> | |
| )} | |
| </div> | |
| ) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/components/form-field.tsx
Line: 119:130
Comment:
Object and array fields accept raw string input without JSON validation. Users can type invalid JSON which will be submitted to the workflow, causing runtime errors during execution.
The field should validate JSON syntax and show an error indicator when invalid JSON is entered.
```suggestion case 'object': case 'array': { const [jsonError, setJsonError] = useState<string | null>(null) const handleJsonChange = (text: string) => { try { if (text.trim() === '') { onChange(field.type === 'array' ? [] : {}) setJsonError(null) return } const parsed = JSON.parse(text) onChange(parsed) setJsonError(null) } catch { setJsonError('Invalid JSON') // Store the text so user doesn't lose their input onChange(text) } } return ( <div> <Textarea value={typeof value === 'string' ? value : JSON.stringify(value, null, 2)} onChange={(e: React.ChangeEvent<HTMLTextAreaElement>) => handleJsonChange(e.target.value)} placeholder={ placeholder || (field.type === 'array' ? '["item1", "item2"]' : '{"key": "value"}') } className={cn( 'min-h-[100px] rounded-[10px] font-mono text-[13px] shadow-sm transition-colors focus:border-gray-400 focus:ring-2 focus:ring-gray-100', jsonError && 'border-red-500' )} /> {jsonError && ( <p className="mt-1 text-xs text-red-500">{jsonError}</p> )} </div> ) }```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+158
to
+162
| const handleSubmit = useCallback( | ||
| async (e: React.FormEvent) => { | ||
| e.preventDefault() | ||
| if (!formConfig) return | ||
Contributor
There was a problem hiding this comment.
No validation for required fields before form submission. The form displays required field indicators (*) but doesn't enforce them, allowing users to submit incomplete forms.
Suggested change
| consthandleSubmit=useCallback( | |
| async(e: React.FormEvent)=>{ | |
| e.preventDefault() | |
| if(!formConfig)return | |
| consthandleSubmit=useCallback( | |
| async(e: React.FormEvent)=>{ | |
| e.preventDefault() | |
| if(!formConfig)return | |
| // Validate required fields | |
| constfields=formConfig.inputSchema||[] | |
| constfieldConfigMap=newMap( | |
| (formConfig.customizations?.fieldConfigs||[]).map((fc)=>[fc.name,fc]) | |
| ) | |
| for(constfieldoffields){ | |
| constconfig=fieldConfigMap.get(field.name) | |
| constisRequired=config?.required??field.required | |
| constvalue=formData[field.name] | |
| if(isRequired){ | |
| // Check if value is empty | |
| constisEmpty=value===undefined||value===null||value===''|| | |
| (Array.isArray(value)&&value.length===0) | |
| if(isEmpty){ | |
| constlabel=config?.label||field.name | |
| setError(`${label} is required`) | |
| return | |
| } | |
| } | |
| } | |
| try{ |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/form.tsx
Line: 158:162
Comment:
No validation for required fields before form submission. The form displays required field indicators (`*`) but doesn't enforce them, allowing users to submit incomplete forms.
```suggestion const handleSubmit = useCallback( async (e: React.FormEvent) => { e.preventDefault() if (!formConfig) return // Validate required fields const fields = formConfig.inputSchema || [] const fieldConfigMap = new Map( (formConfig.customizations?.fieldConfigs || []).map((fc) => [fc.name, fc]) ) for (const field of fields) { const config = fieldConfigMap.get(field.name) const isRequired = config?.required ?? field.required const value = formData[field.name] if (isRequired) { // Check if value is empty const isEmpty = value === undefined || value === null || value === '' || (Array.isArray(value) && value.length === 0) if (isEmpty) { const label = config?.label || field.name setError(`${label} is required`) return } } } try {```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+86
to
+96
| export function addCorsHeaders(response: NextResponse, request: NextRequest): NextResponse { | ||
| const origin = request.headers.get('origin') || '' | ||
| if (origin) { | ||
| response.headers.set('Access-Control-Allow-Origin', origin) | ||
| response.headers.set('Access-Control-Allow-Credentials', 'true') | ||
| response.headers.set('Access-Control-Allow-Methods', 'GET, POST, OPTIONS') | ||
| response.headers.set('Access-Control-Allow-Headers', 'Content-Type, X-Requested-With') | ||
| } | ||
| return response |
Contributor
There was a problem hiding this comment.
Open CORS policy allows ANY origin to make credentialed requests. Line 90 reflects the request origin without validation, creating a security vulnerability where malicious sites can embed forms and steal user data or authentication cookies.
For embedded forms, you should either:
- Maintain an allowlist of trusted domains
- Require form owners to specify allowed origins
- At minimum, validate the origin format
Suggested change
| exportfunctionaddCorsHeaders(response: NextResponse,request: NextRequest): NextResponse{ | |
| constorigin=request.headers.get('origin')||'' | |
| if(origin){ | |
| response.headers.set('Access-Control-Allow-Origin',origin) | |
| response.headers.set('Access-Control-Allow-Credentials','true') | |
| response.headers.set('Access-Control-Allow-Methods','GET, POST, OPTIONS') | |
| response.headers.set('Access-Control-Allow-Headers','Content-Type, X-Requested-With') | |
| } | |
| returnresponse | |
| exportfunctionaddCorsHeaders(response: NextResponse,request: NextRequest): NextResponse{ | |
| constorigin=request.headers.get('origin')||'' | |
| // Only allow CORS for valid HTTP(S) origins, not arbitrary values | |
| if(origin&&/^https?:\/\/.+/.test(origin)){ | |
| // TODO: Implement origin allowlist per form deployment | |
| // For now, allow all valid HTTP origins but log them for monitoring | |
| response.headers.set('Access-Control-Allow-Origin',origin) | |
| response.headers.set('Access-Control-Allow-Credentials','true') | |
| response.headers.set('Access-Control-Allow-Methods','GET, POST, OPTIONS') | |
| response.headers.set('Access-Control-Allow-Headers','Content-Type, X-Requested-With') | |
| } | |
| returnresponse | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/core/security/deployment.ts
Line: 86:96
Comment:
Open CORS policy allows ANY origin to make credentialed requests. Line 90 reflects the request origin without validation, creating a security vulnerability where malicious sites can embed forms and steal user data or authentication cookies.
For embedded forms, you should either:
1. Maintain an allowlist of trusted domains
2. Require form owners to specify allowed origins
3. At minimum, validate the origin format
```suggestionexport function addCorsHeaders(response: NextResponse, request: NextRequest): NextResponse { const origin = request.headers.get('origin') || '' // Only allow CORS for valid HTTP(S) origins, not arbitrary values if (origin && /^https?:\/\/.+/.test(origin)) { // TODO: Implement origin allowlist per form deployment // For now, allow all valid HTTP origins but log them for monitoring response.headers.set('Access-Control-Allow-Origin', origin) response.headers.set('Access-Control-Allow-Credentials', 'true') response.headers.set('Access-Control-Allow-Methods', 'GET, POST, OPTIONS') response.headers.set('Access-Control-Allow-Headers', 'Content-Type, X-Requested-With') } return response}```
How can I resolve this? If you propose a fix, please make it concise.emir-karabegforce-pushed
the
feat/deploy-form
branch
from
January 10, 2026 07:21
9fbef1f to
1a41a88Compareemir-karabegforce-pushed
the
feat/deploy-form
branch
from
January 10, 2026 07:30
cbe8060 to
49c3815Compareemir-karabegforce-pushed
the
feat/deploy-form
branch
from
January 10, 2026 07:37
49c3815 to
a342c2eCompareemir-karabeg
commented
Jan 10, 2026
Collaborator
Uh oh!
There was an error while loading. Please reload this page.
| const response = await fetch(`/api/form/${identifier}`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ formData }), |
Contributor
There was a problem hiding this comment.
File uploads are completely broken. The FormField component allows users to select files (type='files' on line 132 of form-field.tsx), but JSON.stringify({ formData }) cannot serialize File objects. This will either throw an error or send empty objects {} for file fields.
Chat deployment handles this correctly by:
- Converting files to base64 in the client (via
chatFileSchemavalidation) - Processing files server-side with
ChatFiles.processChatFiles() - Uploading to storage and generating presigned URLs
Fix: Either disable file upload fields entirely for forms, or implement proper file handling similar to chat:
- Convert File objects to base64 data URLs client-side before submission
- Process files server-side like chat does (lines 207-245 in
app/api/chat/[identifier]/route.ts) - Update
formDatatype handling to support files
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/form.tsx
Line: 170:170
Comment:
File uploads are completely broken. The `FormField` component allows users to select files (type='files' on line 132 of form-field.tsx), but `JSON.stringify({ formData })` cannot serialize `File` objects. This will either throw an error or send empty objects `{}` for file fields.
Chat deployment handles this correctly by:
1. Converting files to base64 in the client (via `chatFileSchema` validation)
2. Processing files server-side with `ChatFiles.processChatFiles()`3. Uploading to storage and generating presigned URLs
**Fix**: Either disable file upload fields entirely for forms, or implement proper file handling similar to chat:
- Convert File objects to base64 data URLs client-side before submission
- Process files server-side like chat does (lines 207-245 in `app/api/chat/[identifier]/route.ts`)
- Update `formData` type handling to support files
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+158
to
+195
| const handleSubmit = useCallback( | ||
| async (e: React.FormEvent) => { | ||
| e.preventDefault() | ||
| if (!formConfig) return | ||
| try { | ||
| setIsSubmitting(true) | ||
| setError(null) | ||
| const response = await fetch(`/api/form/${identifier}`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ formData }), | ||
| }) | ||
| const data = await response.json() | ||
| if (!response.ok) { | ||
| throw new Error(data.error || 'Failed to submit form') | ||
| } | ||
| setThankYouData({ | ||
| title: data.thankYouTitle || 'Thank you!', | ||
| message: | ||
| data.thankYouMessage || | ||
| formConfig.customizations.thankYouMessage || | ||
| 'Your response has been submitted successfully.', | ||
| }) | ||
| setIsSubmitted(true) | ||
| } catch (err: unknown) { | ||
| logger.error('Error submitting form:', err) | ||
| setError(err instanceof Error ? err.message : 'Failed to submit form') | ||
| } finally { | ||
| setIsSubmitting(false) | ||
| } | ||
| }, | ||
| [identifier, formConfig, formData] | ||
| ) |
Contributor
There was a problem hiding this comment.
Missing required field validation. Users can submit forms without filling in required fields (marked with required: true in field configs).
The form displays a red asterisk (*) for required fields (line 222 in form-field.tsx) but performs no validation on submit. Empty or undefined values for required fields are accepted.
Fix: Add validation before submission:
consthandleSubmit=useCallback(async(e: React.FormEvent)=>{e.preventDefault()if(!formConfig)return// Validate required fieldsconstfields=formConfig.inputSchema||[]constfieldConfigMap=newMap((formConfig.customizations?.fieldConfigs||[]).map((fc)=>[fc.name,fc]))constmissingFields: string[]=[]for(constfieldoffields){constconfig=fieldConfigMap.get(field.name)constisRequired=config?.required??field.requiredconstvalue=formData[field.name]if(isRequired&&(value===undefined||value===''||value===null)){missingFields.push(config?.label||field.name)}}if(missingFields.length>0){setError(`Please fill in required fields: ${missingFields.join(', ')}`)return}// ... rest of submission logic},[identifier,formConfig,formData])Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/form.tsx
Line: 158:195
Comment:
Missing required field validation. Users can submit forms without filling in required fields (marked with `required: true` in field configs).
The form displays a red asterisk (*) for required fields (line 222 in form-field.tsx) but performs no validation on submit. Empty or undefined values for required fields are accepted.
**Fix**: Add validation before submission:
```typescriptconst handleSubmit =useCallback(
async (e:React.FormEvent) => {
e.preventDefault()
if (!formConfig) return// Validate required fieldsconst fields =formConfig.inputSchema|| []
const fieldConfigMap =newMap(
(formConfig.customizations?.fieldConfigs|| []).map((fc) => [fc.name, fc])
)
const missingFields:string[] = []
for (const field offields) {
const config =fieldConfigMap.get(field.name)
const isRequired =config?.required??field.requiredconst value =formData[field.name]
if (isRequired&& (value===undefined||value===''||value===null)) {
missingFields.push(config?.label||field.name)
}
}
if (missingFields.length>0) {
setError(`Please fill in required fields: ${missingFields.join(', ')}`)
return
}
// ... rest of submission logic
},
[identifier, formConfig, formData]
)
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+222
to
+225
| const workflowInput = { | ||
| input: formData, | ||
| ...formData, // Spread form fields at top level for convenience | ||
| } |
Contributor
There was a problem hiding this comment.
Potential prototype pollution vulnerability. Spreading user-submitted formData directly into workflowInput allows attackers to inject properties like __proto__, constructor, or prototype.
While JavaScript's object spread is generally safe against prototype pollution (it doesn't copy inherited properties), this pattern is risky because:
- It trusts user input completely
formDatacould containinputkey, overwriting line 223- Special keys could interfere with workflow execution
Comparison: Chat deployment doesn't spread user input - it carefully structures the input object (lines 206-223 in app/api/chat/[identifier]/route.ts):
constworkflowInput: any={ input, conversationId }if(files&&Array.isArray(files)&&files.length>0){// ... process filesworkflowInput.files=uploadedFiles}Fix: Don't spread formData. Structure the input explicitly:
constworkflowInput={input: formData,// Only add specific known fields if needed}Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/form/[identifier]/route.ts
Line: 222:225
Comment:
Potential prototype pollution vulnerability. Spreading user-submitted `formData` directly into `workflowInput` allows attackers to inject properties like `__proto__`, `constructor`, or `prototype`.
While JavaScript's object spread is generally safe against prototype pollution (it doesn't copy inherited properties), this pattern is risky because:
1. It trusts user input completely
2.`formData` could contain `input` key, overwriting line 223
3. Special keys could interfere with workflow execution
**Comparison**: Chat deployment doesn't spread user input - it carefully structures the input object (lines 206-223 in `app/api/chat/[identifier]/route.ts`):
```typescriptconst workflowInput:any= { input, conversationId }
if (files&&Array.isArray(files) &&files.length>0) {
// ... process filesworkflowInput.files=uploadedFiles
}
```**Fix**: Don't spread formData. Structure the input explicitly:
```typescriptconst workflowInput = {
input: formData,
// Only add specific known fields if needed
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+80
to
+98
| const authError = data.error | ||
| if (authError === 'auth_required_password') { | ||
| setAuthRequired('password') | ||
| setFormConfig({ | ||
| id: '', | ||
| title: data.title || 'Form', | ||
| customizations: data.customizations || {}, | ||
| }) | ||
| return | ||
| } | ||
| if (authError === 'auth_required_email') { | ||
| setAuthRequired('email') | ||
| setFormConfig({ | ||
| id: '', | ||
| title: data.title || 'Form', | ||
| customizations: data.customizations || {}, | ||
| }) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Email authentication type is supported in the backend (line 149 of app/api/form/utils.ts) but there's no UI component for email auth, unlike password auth which has PasswordAuth component.
When authType === 'email', users see the error state "auth_required_email" but have no way to enter their email to authenticate.
Fix: Create an EmailAuth component similar to PasswordAuth (lines 235-237), or reuse the chat email auth component from app/chat/components/auth/email/email-auth.tsx.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/form.tsx
Line: 80:98
Comment:
Email authentication type is supported in the backend (line 149 of `app/api/form/utils.ts`) but there's no UI component for email auth, unlike password auth which has `PasswordAuth` component.
When `authType === 'email'`, users see the error state "auth_required_email" but have no way to enter their email to authenticate.
**Fix**: Create an `EmailAuth` component similar to `PasswordAuth` (lines 235-237), or reuse the chat email auth component from `app/chat/components/auth/email/email-auth.tsx`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+119
to
+130
| case 'object': | ||
| case 'array': | ||
| return ( | ||
| <Textarea | ||
| value={(value as string) ?? ''} | ||
| onChange={(e: React.ChangeEvent<HTMLTextAreaElement>) => onChange(e.target.value)} | ||
| placeholder={ | ||
| placeholder || (field.type === 'array' ? '["item1", "item2"]' : '{"key": "value"}') | ||
| } | ||
| className='min-h-[100px] rounded-[10px] font-mono text-[13px] shadow-sm transition-colors focus:border-gray-400 focus:ring-2 focus:ring-gray-100' | ||
| /> | ||
| ) |
Contributor
There was a problem hiding this comment.
Object and array fields store their values as strings but the type system expects actual objects/arrays. Users must manually type JSON in the textarea, and there's no validation that the JSON is valid before submission.
This creates issues:
- Type mismatch: Line 123 casts to
string, but form initialization (lines 125-126 in form.tsx) sets these as{}or[] - No JSON validation: Invalid JSON like
{foo bar}will be sent to the workflow - Poor UX: Users must know JSON syntax
Fix: Either:
- Parse and validate JSON on blur/change:
onChange={(e)=>{try{constparsed=JSON.parse(e.target.value)onChange(parsed)}catch{// Show validation error}}}
- Or store as string consistently and document that workflows receive JSON strings for these fields
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/components/form-field.tsx
Line: 119:130
Comment:
Object and array fields store their values as strings but the type system expects actual objects/arrays. Users must manually type JSON in the textarea, and there's no validation that the JSON is valid before submission.
This creates issues:
1.**Type mismatch**: Line 123 casts to `string`, but form initialization (lines 125-126 in form.tsx) sets these as `{}` or `[]`2.**No JSON validation**: Invalid JSON like `{foo bar}` will be sent to the workflow
3.**Poor UX**: Users must know JSON syntax
**Fix**: Either:
- Parse and validate JSON on blur/change:
```typescriptonChange={(e) => {
try {
const parsed =JSON.parse(e.target.value)
onChange(parsed)
} catch {
// Show validation error
}
}}
```- Or store as string consistently and document that workflows receive JSON strings for these fields
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+105
to
+117
| case 'number': | ||
| return ( | ||
| <Input | ||
| type='number' | ||
| value={(value as string) ?? ''} | ||
| onChange={(e) => { | ||
| const val = e.target.value | ||
| onChange(val === '' ? '' : Number(val)) | ||
| }} | ||
| placeholder={placeholder || 'Enter a number'} | ||
| className='rounded-[10px] shadow-sm transition-colors focus:border-gray-400 focus:ring-2 focus:ring-gray-100' | ||
| /> | ||
| ) |
Contributor
There was a problem hiding this comment.
Number field allows invalid inputs. Line 112 converts input to Number(val), which produces NaN for invalid inputs like "abc" or "12.5.6". However, NaN is still submitted to the workflow since there's no validation.
Additionally:
- Empty string '' is kept as string, creating type inconsistency (sometimes number, sometimes string)
- No min/max validation
- No decimal vs integer distinction
Fix: Add validation and handle edge cases:
onChange={(e)=>{constval=e.target.valueif(val===''){onChange('')return}constnum=Number(val)if(isNaN(num)){// Optionally show error or prevent invalid inputreturn}onChange(num)}}Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/form/[identifier]/components/form-field.tsx
Line: 105:117
Comment:
Number field allows invalid inputs. Line 112 converts input to `Number(val)`, which produces `NaN` for invalid inputs like "abc" or "12.5.6". However, `NaN` is still submitted to the workflow since there's no validation.
Additionally:
- Empty string '' is kept as string, creating type inconsistency (sometimes number, sometimes string)
- No min/max validation
- No decimal vs integer distinction
**Fix**: Add validation and handle edge cases:
```typescriptonChange={(e) => {
const val =e.target.valueif (val==='') {
onChange('')
return
}
const num =Number(val)
if (isNaN(num)) {
// Optionally show error or prevent invalid inputreturn
}
onChange(num)
}}
```
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Type of Change
Testing
Tested manually
Checklist