fix(agiloft): parse the raw login body and keep the username out of logs

mzxchandra · mzxchandra · commit b769a154a76e · 2026-08-12T18:16:26.000-07:00
Review round 4.

The previous round redacted the login response before parsing it. A token is
opaque base64, so a short credential can appear inside one by coincidence -
a three-character password is near certain to - and redacting first rewrote
the token, breaking bearer auth for every alrest operation. Parsing now stays
on the raw text and only the failure messages use the redacted copy, which is
the split the shared alrest reader already had.

The post-transmit create handler recorded the Agiloft username in structured
logs. It was added so the "check the table" instruction had something to
search on, but the login is half of a credential pair and whoever reads that
log already knows which account the block is configured with. It is gone, and
the logged error message now goes through the same redaction as the one
returned to the caller, since it can carry echoed upstream text.

The regression test builds a token with the password inside it and asserts the
token comes back byte for byte.
diff --git a/apps/sim/app/api/tools/agiloft/create_record/route.ts b/apps/sim/app/api/tools/agiloft/create_record/route.ts
@@ -168,7 +168,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
             const declined = AGILOFT_EXCEPTION.test(described)
             logger.error(`[${requestId}] Agiloft create returned no record ID`, {
               table: params.table,
-              login: params.login,
               fields: Object.keys(fieldValues),
               declined,
             })
@@ -219,10 +218,16 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
        * 500 is what would have the caller retry and duplicate the record, which
        * is the failure this operation exists to prevent.
        */
+      /**
+       * The message goes through the same redaction the caller-facing error
+       * does: it can carry upstream text, and the credentials travel in this
+       * request's body. The login is not recorded at all - whoever reads this
+       * already knows which account the block is configured with, and it is
+       * half of a credential pair.
+       */
       logger.error(`[${requestId}] Agiloft create failed after the request was sent`, {
-        error,
+        error: redactAgiloftSecrets(toError(error).message, params),
         table: params.table,
-        login: params.login,
       })
 
       return NextResponse.json({
diff --git a/apps/sim/tools/agiloft/utils.server.test.ts b/apps/sim/tools/agiloft/utils.server.test.ts
@@ -16,7 +16,7 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({
   secureFetchWithPinnedIP: mockSecureFetch,
 }))
 
-import { executeAgiloftRequest } from '@/tools/agiloft/utils.server'
+import { agiloftLoginPinned, executeAgiloftRequest } from '@/tools/agiloft/utils.server'
 
 const baseParams = {
   instanceUrl: 'https://example.agiloft.com',
@@ -166,3 +166,36 @@ describe('executeAgiloftRequest', () => {
     expect(mockSecureFetch).not.toHaveBeenCalled()
   })
 })
+
+describe('agiloftLoginPinned token integrity', () => {
+  /**
+   * A token is opaque base64, so a short credential can appear inside it by
+   * coincidence. Redacting the body before parsing would rewrite the token and
+   * break every request that carries it, so parsing stays on the raw text.
+   */
+  it('returns the token intact when the password occurs inside it', async () => {
+    const password = 'abc'
+    const token = `eyJhbGciOiJIUzI1NiJ9.${password}payload.signature`
+
+    mockSecureFetch.mockResolvedValueOnce(
+      mockResponse({ json: { access_token: token, authentication_scheme: 'Bearer ' } })
+    )
+
+    const session = await agiloftLoginPinned({ ...baseParams, password }, '93.184.216.34')
+
+    expect(session.token).toBe(token)
+    expect(session.authorization).toBe(`Bearer ${token}`)
+  })
+
+  it('redacts the credentials from a login failure message', async () => {
+    mockSecureFetch.mockResolvedValueOnce(
+      mockResponse({
+        ok: false,
+        status: 500,
+        text: `<html><body>Error: $login=admin&$password=${PLACEHOLDER_PASSWORD}</body></html>`,
+      })
+    )
+
+    await expect(agiloftLoginPinned(baseParams, '93.184.216.34')).rejects.toThrow(/\[redacted\]/)
+  })
+})
diff --git a/apps/sim/tools/agiloft/utils.server.ts b/apps/sim/tools/agiloft/utils.server.ts
@@ -109,26 +109,33 @@ export async function agiloftLoginPinned(
     ),
   })
 
+  const rawText = await response.text()
+
   /**
    * Login posts the credentials in its form body, so an error page echoing the
    * submitted parameters echoes them. Redacted while the text is still whole,
    * before any of the messages below truncate it.
+   *
+   * Only the messages use this. Parsing stays on `rawText`: a token is opaque
+   * base64, so a short credential can appear inside it by coincidence, and
+   * redacting first would rewrite the token and break every request that
+   * carries it.
    */
-  const text = redactAgiloftSecrets(await response.text(), params)
+  const safeText = redactAgiloftSecrets(rawText, params)
 
   if (!response.ok) {
-    throw new Error(`Agiloft login failed (${response.status}): ${describeAgiloftError(text)}`)
+    throw new Error(`Agiloft login failed (${response.status}): ${describeAgiloftError(safeText)}`)
   }
 
   let data: { access_token?: string; authentication_scheme?: string }
   try {
-    data = JSON.parse(text)
+    data = JSON.parse(rawText)
   } catch {
-    throw new Error(`Agiloft login returned a non-JSON response: ${truncate(text, 200)}`)
+    throw new Error(`Agiloft login returned a non-JSON response: ${truncate(safeText, 200)}`)
   }
 
   if (!data.access_token) {
-    throw new Error(`Agiloft login did not return an access token: ${truncate(text, 200)}`)
+    throw new Error(`Agiloft login did not return an access token: ${truncate(safeText, 200)}`)
   }
 
   const scheme = (data.authentication_scheme || 'Bearer').trim() || 'Bearer'
