fix(agiloft): verify the record a projected read returns, and fail an ID-less create

Two findings from review.

A projected read interpolates the record ID into a search predicate. A value
carrying query operators could change which records the predicate selects, and
the route then accepted the first row without checking it. Agiloft record IDs
are integers, so a non-numeric ID is now rejected outright rather than escaped,
and the row is matched on ID instead of taken positionally.

A create that returned no ID still reported success with a null ID. Callers
chain on that ID, so it now fails instead.

Author: waleedlatif1

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

@@ -230,3 +230,42 @@ describe('search result ceiling', () => {
     expect(data.output.totalCount).toBe(200)
   })
 })
+
+describe('review round 1 fixes', () => {
+  it('fails a create that comes back without a record ID', async () => {
+    arrange(res({ json: { success: true, result: {} } }))
+
+    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('did not return an ID')
+  })
+
+  it('refuses a non-numeric record ID on a projected read rather than interpolating it', async () => {
+    const response = await READ(
+      createMockRequest('POST', {
+        ...baseBody,
+        recordId: "1' || priority='High",
+        fields: 'contract_title1',
+      })
+    )
+    const data = (await response.json()) as { success: boolean; error?: string }
+
+    expect(data.success).toBe(false)
+    expect(data.error).toContain('must be numeric')
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('does not return an unrelated record when the search matches something else', async () => {
+    arrange(res({ json: { success: true, result: [{ id: 99, contract_title1: 'Other' }] } }))
+
+    const response = await READ(
+      createMockRequest('POST', { ...baseBody, recordId: '6342', fields: 'contract_title1' })
+    )
+    const data = (await response.json()) as { success: boolean; error?: string }
+
+    expect(data.success).toBe(false)
+    expect(data.error).toContain('no record for ID 6342')
+  })
+})

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

@@ -76,12 +76,22 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
         const record = await readAlrestJson<Record<string, unknown>>(response)
         const id = record?.id
 
+        /**
+         * A create that reports no ID did not create anything usable — callers
+         * chain on this ID, so surface it as a failure rather than handing back
+         * a successful-looking null.
+         */
+        if (id == null) {
+          return {
+            success: false,
+            output: { id: null, fields: record ?? {} },
+            error: 'Agiloft did not return an ID for the created record',
+          }
+        }
+
         return {
           success: true,
-          output: {
-            id: id == null ? null : String(id),
-            fields: record ?? {},
-          },
+          output: { id: String(id), fields: record ?? {} },
         }
       }
     )

apps/sim/app/api/tools/agiloft/read_record/route.ts

@@ -61,6 +61,21 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
      * fetch is cheaper.
      */
     const requestedFields = parseFieldList(params.fields)
+    const recordId = params.recordId.trim()
+
+    /**
+     * The projected read puts the ID inside a search predicate, so anything
+     * other than a plain number could change which records the query selects.
+     * Agiloft record IDs are integers, so reject everything else rather than
+     * trying to escape it.
+     */
+    if (requestedFields && !/^\d+$/.test(recordId)) {
+      return NextResponse.json({
+        success: false,
+        output: { id: null, fields: {} },
+        error: `Record ID must be numeric to read specific fields, got "${recordId}"`,
+      })
+    }
 
     const result = await executeAlrestRequest<AgiloftRecordResponse>(
       params,
@@ -79,7 +94,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
           headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
           body: JSON.stringify({
             field: requestedFields.includes('id') ? requestedFields : ['id', ...requestedFields],
-            query: `id=${params.recordId.trim()}`,
+            query: `id=${recordId}`,
           }),
         }
       },
@@ -88,13 +103,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
           response
         )
 
-        const record = Array.isArray(payload) ? payload[0] : payload
+        /**
+         * Match on ID rather than taking the first row: a search answers with a
+         * result set, and returning an unrelated record as a successful read
+         * would be worse than failing.
+         */
+        const record = Array.isArray(payload)
+          ? payload.find((row) => String(row?.id ?? '') === recordId)
+          : payload
 
         if (!record) {
           return {
             success: false,
             output: { id: null, fields: {} },
-            error: `Agiloft returned no record for ID ${params.recordId.trim()}`,
+            error: `Agiloft returned no record for ID ${recordId}`,
           }
         }