fix(integrations): confine the Splunk XML tolerance and the CrowdStrike commit list

Splitting the XML tolerance out of readSplunkJson into readSplunkDispatchJson
puts it only on the three dispatching and job-control tools that need it. The
results path can no longer read any non-JSON body as an empty envelope, so a 2xx
HTML interstitial surfaces instead of reporting a search that matched nothing.
The dispatch reader anchors on the one documented `<response>` root, so an
interstitial fails there too.

A batched delete now records the IDs Falcon echoed in `resources` rather than
the IDs that were requested. A batch can answer 200 while reporting per-ID
failures, and naming those as deleted told the caller to drop still-live
indicators from the retry.

Author: waleedlatif1

apps/sim/app/api/tools/crowdstrike/query/operations.test.ts

@@ -1072,6 +1072,47 @@ describe('CrowdStrike extended operations', () => {
       expect(data.error).toContain(deletedByFirstBatch[0])
     })
 
+    /**
+     * A batch can answer 200 while reporting per-ID failures in `errors`. Recording
+     * the requested chunk would name indicators that are still live and tell the
+     * caller to drop them from the retry, so only the IDs Falcon echoed in
+     * `resources` count as committed.
+     */
+    it('reports only the IDs Falcon confirmed, not every ID in a partly-failed batch', async () => {
+      const indicatorIds = longIds(300, 'ioc')
+      let deleteCall = 0
+      let skipped = ''
+      const confirmed: string[] = []
+
+      fetchMock.mockImplementation((rawUrl: string) => {
+        deleteCall += 1
+        if (deleteCall === 2) {
+          return Promise.resolve(
+            jsonResponse({ errors: [{ code: 429, message: 'Rate limit exceeded' }] }, 429)
+          )
+        }
+        const ids = idsFromUrl(rawUrl)
+        skipped = ids[0]
+        confirmed.push(...ids.slice(1))
+        return Promise.resolve(
+          jsonResponse({
+            resources: ids.slice(1),
+            errors: [{ code: 404, id: ids[0], message: 'Indicator not found' }],
+          })
+        )
+      })
+
+      const response = await POST(
+        requestFor({ operation: 'crowdstrike_delete_indicators', indicatorIds })
+      )
+      const data = await response.json()
+
+      expect(response.status).toBe(429)
+      expect(confirmed.length).toBeGreaterThan(0)
+      expect(data.error).toContain(`${confirmed.length} ID(s) were already deleted`)
+      expect(data.error).not.toContain(skipped)
+    })
+
     it('leaves a first-batch delete failure unannotated — nothing was committed', async () => {
       const indicatorIds = longIds(300, 'ioc')
 

apps/sim/app/api/tools/crowdstrike/query/operations.ts

@@ -137,13 +137,16 @@ const MAX_COMMITTED_IDS_IN_MESSAGE = 400
  * Rewrites a failed batch's envelope so the reported error names the deletions the
  * earlier batches already committed.
  *
- * Batches run sequentially and each one that answered `ok` really did delete its
- * indicators — Falcon has no way to roll them back. Short-circuiting on a later
- * batch would therefore report a bare failure over work that already happened, and
- * a blind retry would target IDs that no longer exist. Only the message survives to
- * the caller ({@link fail} keeps `status` and the message, not `data`), so the
- * committed prefix is written onto `errors[0].message`, which is the first thing
- * {@link getFalconErrorMessage} reads.
+ * Batches run sequentially and Falcon has no way to roll back a deletion it already
+ * performed. Short-circuiting on a later batch would therefore report a bare failure
+ * over work that already happened, and a blind retry would target IDs that no longer
+ * exist. Only the message survives to the caller ({@link fail} keeps `status` and the
+ * message, not `data`), so the committed list is written onto `errors[0].message`,
+ * which is the first thing {@link getFalconErrorMessage} reads.
+ *
+ * `committed` holds the IDs Falcon echoed in `resources`, never the IDs that were
+ * requested, so an ID that failed inside an otherwise-200 batch is not reported as
+ * deleted.
  */
 function withCommittedIds(
   result: CrowdStrikeCallResult,
@@ -227,7 +230,15 @@ async function callCrowdStrikeByIds(
       return options.method === 'DELETE' ? withCommittedIds(result, committed) : result
     }
 
-    committed.push(...chunk)
+    /**
+     * Only the IDs Falcon echoed in `resources` were actually deleted. A batch can
+     * answer 200 while reporting per-ID failures in `errors`, so recording the
+     * requested chunk would name indicators that are still live and tell the
+     * caller to drop them from the retry.
+     */
+    if (options.method === 'DELETE') {
+      committed.push(...getStringResources(result.data))
+    }
 
     if (index === 0) {
       status = result.status

apps/sim/tools/splunk/cancel_search_job.ts

@@ -7,7 +7,7 @@ import {
   buildSplunkFormHeaders,
   buildSplunkUrl,
   mapSplunkMessages,
-  readSplunkJson,
+  readSplunkDispatchJson,
   SPLUNK_CONNECTION_PARAMS,
   SPLUNK_MESSAGES_OUTPUT,
   splunkPathSegment,
@@ -43,7 +43,7 @@ export const cancelSearchJobTool: ToolConfig<
   },
 
   transformResponse: async (response: Response, params) => {
-    const data = await readSplunkJson(response)
+    const data = await readSplunkDispatchJson(response)
     return {
       success: true,
       output: {

apps/sim/tools/splunk/create_search_job.ts

@@ -7,7 +7,7 @@ import {
   buildSplunkFormHeaders,
   buildSplunkUrl,
   normalizeSearchQuery,
-  readSplunkJson,
+  readSplunkDispatchJson,
   requireSplunkSid,
   SPLUNK_CONNECTION_PARAMS,
 } from '@/tools/splunk/utils'
@@ -151,7 +151,7 @@ export const createSearchJobTool: ToolConfig<
   },
 
   transformResponse: async (response: Response) => {
-    const data = await readSplunkJson(response)
+    const data = await readSplunkDispatchJson(response)
     return { success: true, output: { sid: requireSplunkSid(data) } }
   },
 

apps/sim/tools/splunk/dispatch_saved_search.ts

@@ -6,7 +6,7 @@ import {
   buildSplunkFormBody,
   buildSplunkFormHeaders,
   buildSplunkUrl,
-  readSplunkJson,
+  readSplunkDispatchJson,
   requireSplunkSid,
   SPLUNK_CONNECTION_PARAMS,
   splunkPathSegment,
@@ -107,7 +107,7 @@ export const dispatchSavedSearchTool: ToolConfig<
   },
 
   transformResponse: async (response: Response) => {
-    const data = await readSplunkJson(response)
+    const data = await readSplunkDispatchJson(response)
     return { success: true, output: { sid: requireSplunkSid(data) } }
   },
 

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

@@ -8,6 +8,7 @@ import {
   buildSplunkUrl,
   getSplunkPaging,
   normalizeSearchQuery,
+  readSplunkDispatchJson,
   readSplunkJson,
   requireSplunkSid,
   savedSearchFieldQuery,
@@ -85,30 +86,80 @@ describe('readSplunkJson', () => {
     await expect(readSplunkJson(new Response('{"results":[]}'))).resolves.toEqual({ results: [] })
   })
 
+  /**
+   * The results path must never turn an unparseable body into an empty result set:
+   * that reports a lost result set as a search that legitimately matched nothing.
+   * The XML dispatch tolerance is deliberately not reachable from here.
+   */
+  it('throws on a truncated JSON body rather than reporting an empty result set', async () => {
+    await expect(readSplunkJson(new Response('{"results":[{"_raw":"partial'))).rejects.toThrow()
+  })
+
+  it('throws on a non-JSON body', async () => {
+    await expect(readSplunkJson(new Response('upstream connect error'))).rejects.toThrow()
+  })
+
+  it('throws on a 2xx HTML interstitial instead of reporting zero events', async () => {
+    await expect(
+      readSplunkJson(new Response('<!DOCTYPE html><html><body>Sign in</body></html>'))
+    ).rejects.toThrow()
+  })
+
+  it('throws even on the dispatch XML envelope — results are always JSON', async () => {
+    await expect(
+      readSplunkJson(new Response('<response><sid>1457683115.100</sid></response>'))
+    ).rejects.toThrow()
+  })
+})
+
+describe('readSplunkDispatchJson', () => {
   /**
    * `output_mode` is not in the documented parameter table for the dispatching and
    * job-control endpoints, and the only response the reference documents for them
    * is XML. Throwing here reported a cancel that succeeded server-side as a
    * failure, and pre-empted the error the caller raises for a missing value.
    */
-  it('returns an empty envelope for an XML body instead of throwing', async () => {
+  it('returns an empty envelope for the documented XML response', async () => {
     await expect(
-      readSplunkJson(new Response('<response><sid>1457683115.100</sid></response>'))
+      readSplunkDispatchJson(new Response('<response><sid>1457683115.100</sid></response>'))
+    ).resolves.toEqual({})
+  })
+
+  it('accepts the XML declaration and the self-closing form', async () => {
+    await expect(
+      readSplunkDispatchJson(
+        new Response('<?xml version="1.0"?><response><sid>1.1</sid></response>')
+      )
     ).resolves.toEqual({})
+    await expect(readSplunkDispatchJson(new Response('<response/>'))).resolves.toEqual({})
+  })
+
+  it('still parses a JSON body', async () => {
+    await expect(readSplunkDispatchJson(new Response('{"sid":"1.1"}'))).resolves.toEqual({
+      sid: '1.1',
+    })
+  })
+
+  it('returns an empty envelope for 204 and for an empty body', async () => {
+    await expect(readSplunkDispatchJson(new Response(null, { status: 204 }))).resolves.toEqual({})
+    await expect(readSplunkDispatchJson(new Response('  '))).resolves.toEqual({})
   })
 
   /**
-   * The XML tolerance must not extend to a body that was meant to be JSON.
-   * Swallowing a truncated result payload would hand `get_search_results` an empty
-   * envelope, reporting a lost result set as a search that legitimately matched
-   * nothing.
+   * The tolerance is anchored to the one documented root element. A proxy error
+   * page or an SSO interstitial served with a 2xx is not a successful dispatch, and
+   * reading it as an empty envelope would hide it behind the missing-sid message.
    */
-  it('throws on a truncated JSON body rather than reporting an empty result set', async () => {
-    await expect(readSplunkJson(new Response('{"results":[{"_raw":"partial'))).rejects.toThrow()
+  it('throws on a 2xx HTML interstitial rather than reading it as an empty envelope', async () => {
+    await expect(
+      readSplunkDispatchJson(new Response('<!DOCTYPE html><html><body>Sign in</body></html>'))
+    ).rejects.toThrow()
   })
 
-  it('throws on a non-JSON, non-XML body', async () => {
-    await expect(readSplunkJson(new Response('upstream connect error'))).rejects.toThrow()
+  it('throws on some other XML document', async () => {
+    await expect(
+      readSplunkDispatchJson(new Response('<error><message>Gateway timeout</message></error>'))
+    ).rejects.toThrow()
   })
 })
 

apps/sim/tools/splunk/utils.ts

@@ -152,35 +152,60 @@ export function buildSplunkFormBody(
 }
 
 /**
- * Read a Splunk JSON body, tolerating a body that is empty or is not JSON at all.
+ * Read a Splunk JSON body, tolerating only an empty one.
  *
  * The results endpoint answers `204 No Content` with no body while a job has not
  * produced results yet, and the job control endpoint documents "Returned values:
  * None". `Response.json()` throws on an empty body, so a caller that polled a
  * still-running job would surface `Unexpected end of JSON input` instead of an
  * empty result set.
  *
- * An XML body is tolerated the same way because `output_mode` does not appear in
- * the documented parameter table for the dispatching and job-control endpoints,
- * and the only response the reference documents for them is the XML
- * `<response><sid>...</sid></response>`. Throwing on that XML would report a
- * request that succeeded server-side as a failure — cancelling a job really does
- * cancel it — and would pre-empt the specific error the caller raises when the
- * envelope carries no usable value. Returning `{}` lets that error surface
- * instead, and a JSON body (which these endpoints do return in practice when
- * `output_mode=json` is honored) still parses normally.
+ * Nothing else is tolerated. Every request pins `output_mode=json`, so a
+ * non-empty body that will not parse is corrupt, truncated, or not from Splunk at
+ * all — a proxy's HTML error page or an SSO interstitial served with a 2xx.
+ * Swallowing any of those would hand `get_search_results` an empty envelope and
+ * report a lost result set as a search that legitimately matched nothing.
  *
- * The tolerance stops at XML. A body that is neither empty nor XML is meant to be
- * JSON, so a parse failure there means the payload is corrupt or truncated —
- * swallowing it would hand `get_search_results` an empty envelope and report a
- * lost result set as a successful search with zero events.
+ * Callers that must accept the documented XML dispatch envelope use
+ * {@link readSplunkDispatchJson} instead; the tolerance is deliberately not
+ * available on this path.
  */
 export async function readSplunkJson(response: Response): Promise<unknown> {
   if (response.status === 204) return {}
-  const text = await response.text()
-  const body = text.trim()
+  const body = (await response.text()).trim()
   if (!body) return {}
-  if (body.startsWith('<')) return {}
+  return JSON.parse(body)
+}
+
+/**
+ * The one XML body the reference documents: `<response><sid>...</sid></response>`,
+ * optionally preceded by an XML declaration. Anchored to that root element so an
+ * HTML interstitial (`<!DOCTYPE html>`, `<html>`) or any other XML document still
+ * fails to parse rather than being read as an empty envelope.
+ */
+const SPLUNK_XML_RESPONSE_ROOT = /^(?:<\?xml[^>]*\?>\s*)?<response[\s/>]/i
+
+/**
+ * Read a body from the dispatching and job-control endpoints, which may answer in
+ * XML rather than JSON.
+ *
+ * `output_mode` does not appear in the documented parameter table for these
+ * endpoints, so the XML `<response><sid>...</sid></response>` is the only response
+ * the reference actually promises. Throwing on it would report a request that
+ * succeeded server-side as a failure — cancelling a job really does cancel it —
+ * and would pre-empt the specific error the caller raises when the envelope
+ * carries no usable value. Returning `{}` lets {@link requireSplunkSid} name the
+ * problem instead, and a JSON body (which these endpoints do return in practice
+ * when `output_mode=json` is honored) still parses normally.
+ *
+ * Only that exact root element is accepted. Anything else — including an HTML
+ * error page served with a 2xx — falls through to `JSON.parse` and throws.
+ */
+export async function readSplunkDispatchJson(response: Response): Promise<unknown> {
+  if (response.status === 204) return {}
+  const body = (await response.text()).trim()
+  if (!body) return {}
+  if (SPLUNK_XML_RESPONSE_ROOT.test(body)) return {}
   return JSON.parse(body)
 }