fix(agiloft): make the redaction pipeline impossible to compose wrongly

mzxchandra · mzxchandra · commit 242ef7ce16ce · 2026-08-12T21:42:29.000-07:00
Review finding. The create path ran describeAgiloftError on the raw body and
redacted the result. That helper strips tags and collapses whitespace, so a
credential containing bracket or repeated-space characters was reshaped before
redaction looked for it, and a fragment could still reach the caller.

That is the fifth time these three steps have been ordered wrongly in this
branch, in both directions, each time in a different file. The steps are now
one function: callers hand over the raw body and get back a string that is
safe to relay, and there is no correct way to compose them by hand.

Tests cover both reshaping transforms - a password carrying angle brackets and
one carrying a double space - and assert the helper still reduces a body to
its typed exception message.
diff --git a/apps/sim/app/api/tools/agiloft/create_record/route.ts b/apps/sim/app/api/tools/agiloft/create_record/route.ts
@@ -1,6 +1,5 @@
 import { createLogger } from '@sim/logger'
 import { getErrorMessage, toError } from '@sim/utils/errors'
-import { truncate } from '@sim/utils/string'
 import { type NextRequest, NextResponse } from 'next/server'
 import { agiloftCreateRecordContract } from '@/lib/api/contracts/tools/agiloft'
 import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
@@ -12,7 +11,7 @@ import type { AgiloftRecordResponse } from '@/tools/agiloft/types'
 import {
   buildCreateRecordBody,
   buildCreateRecordUrl,
-  describeAgiloftError,
+  describeAgiloftFailure,
   redactAgiloftSecrets,
 } from '@/tools/agiloft/utils'
 import { executeEwRequest, resolveAgiloftInstance } from '@/tools/agiloft/utils.server'
@@ -143,7 +142,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
            * and the credentials are in the submitted form body, so an error page
            * that echoes request parameters would carry them back.
            */
-          const described = truncate(redactAgiloftSecrets(describeAgiloftError(text), params), 300)
+          const described = describeAgiloftFailure(text, params)
 
           /**
            * Every failure below is one Agiloft already decided on, so each is
diff --git a/apps/sim/tools/agiloft/utils.test.ts b/apps/sim/tools/agiloft/utils.test.ts
@@ -18,6 +18,7 @@ import {
   buildSelectRecordsUrl,
   buildUpsertRecordBody,
   describeAgiloftError,
+  describeAgiloftFailure,
   ewCredentialBody,
   parseFieldList,
   redactAgiloftSecrets,
@@ -420,3 +421,37 @@ describe('credential redaction', () => {
     )
   })
 })
+
+describe('describeAgiloftFailure', () => {
+  /**
+   * describeAgiloftError strips tags and collapses whitespace, so redacting
+   * after it runs can miss a credential that those transforms reshaped. The
+   * pipeline exists so the order cannot be composed wrongly at a call site.
+   */
+  it('redacts a credential the HTML strip would otherwise reshape', () => {
+    const creds = { login: 'svc.user', password: 'a<b>c' }
+    const body = `<html><body>EWWrongDataException has occurred: submitted $password=${creds.password}</body></html>`
+
+    const described = describeAgiloftFailure(body, creds)
+
+    expect(described).not.toContain(creds.password)
+    expect(described).toContain('[redacted]')
+  })
+
+  it('redacts a credential whitespace collapsing would otherwise reshape', () => {
+    const creds = { login: 'svc.user', password: 'two  spaces' }
+    const described = describeAgiloftFailure(`<html>echo ${creds.password}</html>`, creds)
+
+    expect(described).not.toContain(creds.password)
+    expect(described).toContain('[redacted]')
+  })
+
+  it('still reduces the body to its typed exception message', () => {
+    const described = describeAgiloftFailure(
+      '<html><body>EWWrongDataException has occurred: [task-1] Wrong format pointed to start_date</body></html>',
+      { login: 'svc.user', password: 'not-a-real-password' }
+    )
+
+    expect(described).toBe('EWWrongDataException: Wrong format pointed to start_date')
+  })
+})
diff --git a/apps/sim/tools/agiloft/utils.ts b/apps/sim/tools/agiloft/utils.ts
@@ -1,3 +1,4 @@
+import { truncate } from '@sim/utils/string'
 import type {
   AgiloftAsyncStatusParams,
   AgiloftAttachmentInfoParams,
@@ -85,6 +86,26 @@ export function redactAgiloftSecrets(
   return safe
 }
 
+/**
+ * Turns a raw Agiloft response body into the one-line detail relayed to a
+ * caller or written to a log.
+ *
+ * The ordering is the entire point of this function existing. Redaction has to
+ * run while the text is still exactly what the server sent: `describeAgiloftError`
+ * strips tags and collapses whitespace, and `truncate` clips, and either can
+ * reshape a credential so it no longer matches what is being replaced - leaving
+ * a fragment of it in the output. Composing the three by hand has gone wrong
+ * repeatedly, in both directions, so callers hand over the raw body and get back
+ * a string that is safe to relay.
+ */
+export function describeAgiloftFailure(
+  rawBody: string,
+  credentials: { login: string; password: string },
+  maxLength = 300
+): string {
+  return truncate(describeAgiloftError(redactAgiloftSecrets(rawBody, credentials)), maxLength)
+}
+
 /** Language sent on every Agiloft call; EWLogin rejects the request without it. */
 export const AGILOFT_LANG = 'en'
 
