fix(agiloft): refuse an object nested in a multi-value field

The guard that rejects unencodable field values only ran on the top-level
value, so an object inside an array fell through to String() and wrote
"[object Object]" into the record while reporting success. Extracting the
render step means array entries get the same refusal a bare value does.

Pre-existing in the upsert body builder, but it now sits on the create path
too, which is the operation this branch is making trustworthy.

Also fills the coverage gaps a diff audit turned up: invalid and non-object
JSON in the data param, an empty field list for natural language search, a
search that matched nothing, separator characters in an encoded value, and
the block wiring that makes page and limit reachable for natural language
search.

Author: mzxchandra

apps/sim/app/api/tools/agiloft/create_record/route.test.ts

@@ -238,6 +238,39 @@ describe('EWCreate', () => {
     expect(data.error).toContain('the record may exist')
     expect(data.error).toContain('retrying creates a second record')
   })
+
+  it('rejects a data parameter that is not JSON without calling Agiloft', async () => {
+    const response = await POST(createMockRequest('POST', { ...baseBody, data: 'title = X' }))
+    const data = (await response.json()) as { success: boolean; error?: string }
+
+    expect(data.success).toBe(false)
+    expect(data.error).toContain('must be a JSON object of field names to values')
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  /**
+   * A JSON array parses fine but has no field names, so it would post a body
+   * carrying nothing but credentials and create an empty record.
+   */
+  it('rejects valid JSON that is not an object of field names', async () => {
+    const response = await POST(createMockRequest('POST', { ...baseBody, data: '["a","b"]' }))
+    const data = (await response.json()) as { success: boolean; error?: string }
+
+    expect(data.success).toBe(false)
+    expect(data.error).toContain('must be a JSON object of field names to values')
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('refuses an object field value rather than writing [object Object]', async () => {
+    const response = await POST(
+      createMockRequest('POST', { ...baseBody, data: '{"nested":{"a":1}}' })
+    )
+    const data = (await response.json()) as { success: boolean; error?: string }
+
+    expect(data.success).toBe(false)
+    expect(data.error).toContain('has no encoding for')
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
 })
 
 describe('field projection', () => {

apps/sim/app/api/tools/agiloft/nlp_search/route.test.ts

@@ -161,6 +161,21 @@ describe('EWNLPSearch response', () => {
     expect(data.error).toContain('One has to specify $login, $password')
   })
 
+  it('reports a search that matched nothing as an empty success, not a failure', async () => {
+    inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValueOnce(
+      res({ json: { success: true, message: '', result: [] } })
+    )
+
+    const response = await POST(createMockRequest('POST', baseBody))
+    const data = (await response.json()) as {
+      success: boolean
+      output: { records: unknown[]; totalCount: number; truncated: boolean }
+    }
+
+    expect(data.success).toBe(true)
+    expect(data.output).toEqual({ records: [], totalCount: 0, truncated: false })
+  })
+
   it('caps the records it returns and reports the result as truncated', async () => {
     inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValueOnce(
       res({

apps/sim/blocks/blocks/agiloft.test.ts

@@ -0,0 +1,40 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { AgiloftBlock } from '@/blocks/blocks/agiloft'
+
+function conditionFor(subBlockId: string) {
+  const subBlock = AgiloftBlock.subBlocks.find((entry) => entry.id === subBlockId)
+  expect(subBlock, `subBlock ${subBlockId} is missing`).toBeDefined()
+  return subBlock?.condition
+}
+
+describe('AgiloftBlock', () => {
+  /**
+   * Natural Language Search runs across the whole knowledge base with no table
+   * to narrow it, so pagination is the caller's only bound on the result size.
+   * A condition naming only search_records hides both fields from the operation
+   * that most needs them.
+   */
+  it('offers page and limit to natural language search, not only to search records', () => {
+    for (const field of ['page', 'limit']) {
+      expect(conditionFor(field)).toEqual({
+        field: 'operation',
+        value: ['search_records', 'nlp_search'],
+      })
+    }
+  })
+
+  /**
+   * The NLP search route returns records, totalCount, and truncated — never a
+   * limit — so advertising limit for that operation promises an output that
+   * never arrives.
+   */
+  it('does not advertise a limit output for natural language search', () => {
+    expect(AgiloftBlock.outputs.limit.condition).toEqual({
+      field: 'operation',
+      value: 'search_records',
+    })
+  })
+})

apps/sim/tools/agiloft/utils.test.ts

@@ -17,6 +17,7 @@ import {
   buildRetrieveAttachmentUrl,
   buildSavedSearchUrl,
   buildSelectRecordsUrl,
+  buildUpsertRecordBody,
   describeAgiloftError,
   ewCredentialBody,
   parseFieldList,
@@ -248,6 +249,20 @@ describe('EWCreate', () => {
   it('refuses an object value rather than writing [object Object] into the record', () => {
     expect(() => buildCreateRecordBody(baseParams, { nested: { a: 1 } })).toThrow(TypeError)
   })
+
+  /**
+   * A raw `&` or `=` inside a value would otherwise split the body into extra
+   * pairs, writing attacker-chosen fields onto the record.
+   */
+  it('escapes separators in a value so it cannot open a second field', () => {
+    const sent = new URLSearchParams(
+      buildCreateRecordBody(baseParams, { note: 'a&$table=other=b' })
+    )
+
+    expect(sent.get('note')).toBe('a&$table=other=b')
+    expect(sent.get('$table')).toBe('contract')
+    expect(sent.getAll('$table')).toHaveLength(1)
+  })
 })
 
 describe('EWNLPSearch', () => {
@@ -282,6 +297,18 @@ describe('EWNLPSearch', () => {
     expect(sent.getAll('field')).toEqual(['id', 'contract_title1'])
   })
 
+  /**
+   * The contract only requires a non-empty string, so a list of separators
+   * reaches here. Sending an empty `field` pair would ask Agiloft for a field
+   * with no name.
+   */
+  it('sends no field pair when the list holds nothing but separators', () => {
+    const sent = new URLSearchParams(buildNlpSearchBody({ ...nlpParams, fields: ' , , ' }))
+
+    expect(sent.getAll('field')).toEqual([])
+    expect(sent.get('nlp_query')).toBe('Active NDAs submitted last month')
+  })
+
   it('omits pagination when it was not requested', () => {
     const sent = new URLSearchParams(buildNlpSearchBody(nlpParams))
     expect(sent.has('page')).toBe(false)
@@ -294,3 +321,26 @@ describe('EWNLPSearch', () => {
     expect(sent.get('limit')).toBe('25')
   })
 })
+
+describe('multi-value field encoding', () => {
+  /**
+   * An object nested in a multi-value field is as unencodable as a bare one.
+   * String()-ing it would write "[object Object]" into the record and report
+   * success, which is worse than refusing the call.
+   */
+  it('refuses an object inside an array rather than writing [object Object]', () => {
+    expect(() => buildCreateRecordBody(baseParams, { contactMethod: ['phone', { a: 1 }] })).toThrow(
+      TypeError
+    )
+    expect(() =>
+      buildUpsertRecordBody({ ...baseParams, match: 'id' }, { contactMethod: ['phone', { a: 1 }] })
+    ).toThrow(TypeError)
+  })
+
+  it('keeps encoding scalar array entries after the guard', () => {
+    const sent = new URLSearchParams(
+      buildCreateRecordBody(baseParams, { contactMethod: ['phone', 42, true] })
+    )
+    expect(sent.getAll('contactMethod')).toEqual(['phone', '42', 'true'])
+  })
+})

apps/sim/tools/agiloft/utils.ts

@@ -239,25 +239,35 @@ export function encodeEwFormBody(fields: Array<[string, string]>): string {
  * string. Objects have no documented encoding at all, and String()-ing one
  * silently writes "[object Object]" into the record.
  */
+/**
+ * Renders one field value, refusing anything Agiloft has no encoding for.
+ *
+ * Applied to array entries as well as bare values: an object nested in a
+ * multi-value field is just as unencodable as a bare one, and `String()` would
+ * quietly write "[object Object]" into the record instead of failing.
+ */
+function encodeFieldValue(field: string, value: unknown): string {
+  if (typeof value === 'object') {
+    throw new TypeError(
+      `Field "${field}" is an object, which Agiloft has no encoding for. Use a string, a number, or an array of values.`
+    )
+  }
+  return String(value)
+}
+
 function pushRecordFields(fields: Array<[string, string]>, data: Record<string, unknown>): void {
   for (const [field, value] of Object.entries(data)) {
     if (value === undefined || value === null) continue
 
     if (Array.isArray(value)) {
       for (const entry of value) {
         if (entry === undefined || entry === null) continue
-        fields.push([field, String(entry)])
+        fields.push([field, encodeFieldValue(field, entry)])
       }
       continue
     }
 
-    if (typeof value === 'object') {
-      throw new TypeError(
-        `Field "${field}" is an object, which Agiloft has no encoding for. Use a string, a number, or an array of values.`
-      )
-    }
-
-    fields.push([field, String(value)])
+    fields.push([field, encodeFieldValue(field, value)])
   }
 }