Skip to content

Commit 952cdff

Browse files
committed
fix(agiloft): redact credentials before the shared reader truncates the body
Review round 3. The create path redacts before truncating, but natural language search reads its response through readAlrestJson, which embeds its own truncated slice of the body in the error it throws. The route redacted that message afterwards, by which point a credential clipped at the 300-character boundary had already been reduced to a prefix that a full-value replace can never match. readAlrestJson now redacts while the text is whole, for the callers that send credentials on the request itself. The rest authenticate with a bearer token and cannot echo one back, so they pass nothing and are unchanged. Login had the same shape and no redaction at all, which is worse than the reported case: EWLogin posts the credentials in its form body, and all three of its failure paths relayed the response text. It now redacts before any of them run. The regression test asserts no prefix of the password survives, not just the whole value, since a prefix is what the boundary produces.
1 parent 172e475 commit 952cdff

3 files changed

Lines changed: 60 additions & 7 deletions

File tree

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,3 +222,30 @@ describe('EWNLPSearch response', () => {
222222
expect(data.output.truncated).toBe(true)
223223
})
224224
})
225+
226+
describe('EWNLPSearch credential handling at the truncation boundary', () => {
227+
/**
228+
* The shared alrest reader embeds a truncated slice of the body in its error.
229+
* Clipping before redacting can cut through a credential and leave a prefix
230+
* the route's later replace can no longer match, so the redaction has to
231+
* happen while the text is still whole.
232+
*/
233+
it('redacts a credential that would otherwise be clipped by truncation', async () => {
234+
const filler = 'x'.repeat(290)
235+
inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValueOnce(
236+
res({
237+
ok: false,
238+
status: 500,
239+
text: `${filler}$password=${PLACEHOLDER_PASSWORD}`,
240+
})
241+
)
242+
243+
const response = await POST(createMockRequest('POST', baseBody))
244+
const data = (await response.json()) as { error?: string }
245+
246+
expect(data.error).not.toContain(PLACEHOLDER_PASSWORD)
247+
for (let cut = 6; cut < PLACEHOLDER_PASSWORD.length; cut++) {
248+
expect(data.error).not.toContain(PLACEHOLDER_PASSWORD.slice(0, cut))
249+
}
250+
})
251+
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7373
* the two halves deliberately use different conventions. Do not
7474
* "correct" this to `parseEwRest` to match create.
7575
*/
76-
const payload = await readAlrestJson<Record<string, unknown>[]>(response)
76+
const payload = await readAlrestJson<Record<string, unknown>[]>(response, params)
7777

7878
/**
7979
* `result` is documented as an array, but a single-record or empty-object

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

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import {
77
validateUrlWithDNS,
88
} from '@/lib/core/security/input-validation.server'
99
import type { AgiloftBaseParams, AgiloftCredentials } from '@/tools/agiloft/types'
10-
import { AGILOFT_LANG, agiloftAlrestBase, describeAgiloftError } from '@/tools/agiloft/utils'
10+
import {
11+
AGILOFT_LANG,
12+
agiloftAlrestBase,
13+
describeAgiloftError,
14+
redactAgiloftSecrets,
15+
} from '@/tools/agiloft/utils'
1116
import type { HttpMethod, ToolResponse } from '@/tools/types'
1217

1318
const logger = createLogger('AgiloftAuthServer')
@@ -104,7 +109,12 @@ export async function agiloftLoginPinned(
104109
),
105110
})
106111

107-
const text = await response.text()
112+
/**
113+
* Login posts the credentials in its form body, so an error page echoing the
114+
* submitted parameters echoes them. Redacted while the text is still whole,
115+
* before any of the messages below truncate it.
116+
*/
117+
const text = redactAgiloftSecrets(await response.text(), params)
108118

109119
if (!response.ok) {
110120
throw new Error(`Agiloft login failed (${response.status}): ${describeAgiloftError(text)}`)
@@ -237,12 +247,26 @@ export function isAgiloftRefusal(error: unknown): error is AgiloftAlrestError {
237247
* — whether it failed by status code, by `success: false`, or by returning
238248
* something that is not JSON at all.
239249
*/
240-
export async function readAlrestJson<T>(response: SecureFetchResponse): Promise<T | undefined> {
241-
const text = await response.text()
250+
export async function readAlrestJson<T>(
251+
response: SecureFetchResponse,
252+
credentials?: { login: string; password: string }
253+
): Promise<T | undefined> {
254+
const rawText = await response.text()
255+
256+
/**
257+
* Redacted here rather than by the caller, because the messages below embed a
258+
* truncated slice of the body. Clipping first can cut through a credential and
259+
* leave a prefix that a later full-value replace can no longer match, so the
260+
* redaction has to happen while the text is still whole.
261+
*
262+
* `credentials` is passed by the operations that send them on the request
263+
* itself; the rest authenticate with a bearer token and cannot echo one back.
264+
*/
265+
const text = credentials ? redactAgiloftSecrets(rawText, credentials) : rawText
242266

243267
let envelope: AlrestEnvelope<T>
244268
try {
245-
envelope = JSON.parse(text)
269+
envelope = JSON.parse(rawText)
246270
} catch {
247271
throw new AgiloftAlrestError(
248272
`Agiloft returned a non-JSON response (${response.status}): ${truncate(text, 300)}`
@@ -257,7 +281,9 @@ export async function readAlrestJson<T>(response: SecureFetchResponse): Promise<
257281
.join('; ') ||
258282
envelope.message ||
259283
describeAgiloftError(truncate(text, 300))
260-
throw new AgiloftAlrestError(`Agiloft error: ${detail}`)
284+
throw new AgiloftAlrestError(
285+
`Agiloft error: ${credentials ? redactAgiloftSecrets(detail, credentials) : detail}`
286+
)
261287
}
262288

263289
return envelope.result

0 commit comments

Comments
 (0)