fix(sap_concur): align the integration with SAP Concur's documented API (#6790)

* fix(sap_concur): align the integration with SAP Concur's documented API

Validated all 70 tools, the block, and both proxy routes against SAP's
published API docs.

Auth:
- add the password and companyUuid to the token cache key so a request
  with the wrong password can no longer be served a cached token minted
  from someone else's
- wire the documented company-level flow (username = company UUID,
  credtype = authtoken) so companyUuid actually scopes a token
- expand the datacenter allowlist to the documented set (adds glz, apj1,
  usg, the impl hosts, and the www- twins) and drop the undocumented cn
  host; validate the returned geolocation by shape instead of membership
- coalesce concurrent token fetches so a fan-out mints one token
- forward Retry-After so 429 retries pace off Concur's own hint
- handle the errorMessageList, SCIM detail, and legacy Error.Message
  shapes instead of falling through to a generic HTTP message
- pin redirects and cap the response body

Block:
- collapse six contextType subBlocks that disagreed on their default, so
  a new block no longer seeds MANAGER for every operation
- clamp contextType to each operation's documented set
- stop requiring a userId and contextType that the default operation's
  tool does not accept, and scope the receipt fields to the upload ops
- reach six params that had no subBlock, and pass userId on travel
  request updates so a stale value cannot impersonate

Tools:
- correct response shapes that resolved to undefined: budget headers,
  budget categories, allocations, receipts, SCIM nextCursor, and the
  delete endpoints that return a bare boolean
- use the Travel Request Amount schema (currency, not currencyCode)
- narrow the four XML-only travel tools to a documented string payload
  and request application/xml
- surface real errors instead of a JSON parse failure when the proxy
  returns a non-JSON body
- cap receipt uploads at the documented sizes before downloading

Adds 106 tests covering the token cache, geolocation validation, path
traversal, and error extraction.

* fix(sap_concur): drop the removed forwardId subblock via a migration

Removing the `forwardId` subblock without a migration entry breaks
deployed workflows that still carry a value under that key.

It fed a `concur-forwardid` request header that is documented nowhere in
Concur's Receipts v4 or Image v1 references, so it was never honored.
There is no replacement subblock and the value is an opaque
caller-chosen string rather than a secret, so it is dropped outright.

* fix(sap_concur): stop swallowing upload response-read failures

The upload route caught every error from the bounded response read and
continued down the success path, so a size-limit breach or a stream
failure surfaced as an upstream success with a null or header-only body.

Concur returns Content-Length: 0 on a successful image-only upload, and
readResponseTextWithLimit already returns an empty string for that
without throwing, so dropping the catch keeps the legitimate empty-body
case working while letting real read failures reach the route's handler.

* fix(sap_concur): unblock company auth and correct the body wand prompt

The password grant marked username required, so the company-level flow —
which sends the company UUID as the token username and has no user login
— could not be configured at all, even though the request schema and
token fetch already accept companyUuid without a username. Username is
now optional for that grant and the server-side check reports which of
the two is missing. Relabels the password and companyUuid fields to say
what they carry in the company flow.

The shared body wand prompt also still described several payloads the
way they looked before this branch: quick expenses in PascalCase rather
than v4 camelCase, travel requests and expected expenses using
currencyCode where the Request v4 Amount schema uses currency, the
standard SCIM SearchRequest URN instead of Concur's, startIndex as a
search parameter when it is unsupported, and a cash advance shape that
does not match the documented request. A wand-generated body was
therefore rejected for most of the create operations it covers.

* fix(sap_concur): keep Concur's status when an error body fails to read

Removing the blanket catch from the upload read fixed one failure mode
and introduced its inverse: a cap breach or stream error while reading a
non-success body threw before the route reached the branch that
preserves Concur's status, so an upstream 4xx surfaced as a Sim 500 and
could trigger a retry the caller should not make.

Both routes now split the two cases. On a success status the body is the
result, so a read failure still propagates. On an error status the body
only supplies the message, so a read failure resolves empty and the
upstream status survives, with the message falling back to the generic
HTTP-status form.

Adds 21 tests covering both helpers over success, error, empty-body and
boundary statuses; inverting the status check turns 14 of them red.

Author: waleedlatif1

apps/docs/content/docs/en/integrations/sap_concur.mdx

@@ -1,15 +1,20 @@
 import { createLogger } from '@sim/logger'
-import { toError } from '@sim/utils/errors'
+import { generateId } from '@sim/utils/id'
 import { type NextRequest, NextResponse } from 'next/server'
 import { getValidationErrorMessage, isZodError } from '@/lib/api/server'
 import { checkInternalAuth } from '@/lib/auth/hybrid'
-import { secureFetchWithValidation } from '@/lib/core/security/input-validation.server'
+import {
+  MAX_JSON_API_RESPONSE_BYTES,
+  secureFetchWithValidation,
+} from '@/lib/core/security/input-validation.server'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import {
   assertSafeExternalUrl,
+  describeSapConcurFetchError,
   extractSapConcurError,
   fetchSapConcurAccessToken,
+  forwardedSapConcurHeaders,
   SAP_CONCUR_OUTBOUND_FETCH_TIMEOUT_MS,
   type SapConcurProxyRequest,
   SapConcurProxyRequestSchema,
@@ -39,10 +44,52 @@ function buildApiUrl(geolocation: string, req: ProxyRequest): string {
   return url.includes('?') ? `${url}&${queryString}` : `${url}?${queryString}`
 }
 
+/**
+ * Map a non-2xx Concur status that cannot be re-emitted as an error status onto 502.
+ *
+ * With `maxRedirects: 0` a 3xx carrying a `Location` never reaches here — it rejects with
+ * "Too many redirects" and is handled in the outer catch. What does reach here is a 3xx
+ * *without* a `Location`, and a 304, which is excluded from the redirect handling
+ * upstream. Neither is a usable error status to return to the caller.
+ */
+function clampErrorStatus(status: number): number {
+  return status >= 400 ? status : 502
+}
+
 interface Invocation {
   status: number
   body: unknown
   raw: string
+  /** Concur response headers forwarded onto this route's response. */
+  headers: Record<string, string>
+}
+
+/**
+ * Invoke a Concur API endpoint with the bearer token.
+ *
+ * `concur-correlationid` is a support/tracing header expected to be a fresh RFC 4122
+ * UUID per request; it does not scope a request to a company. Redirects are refused so
+ * the Authorization header is never forwarded to another origin.
+ *
+ * `stripAuthOnRedirect` is unreachable while `maxRedirects` is 0 — no redirect is ever
+ * followed for it to act on. It is kept as defense-in-depth so raising `maxRedirects`
+ * later cannot silently start forwarding the bearer token; do not remove it as dead code.
+ */
+/**
+ * Read a Concur response body, keeping the upstream status meaningful.
+ *
+ * On a success status the body is the result, so a stream failure is a real
+ * error and must propagate. On an error status the body only supplies the
+ * message, and throwing would turn Concur's 4xx into a Sim 500 — the status is
+ * preserved instead and the message falls back to the generic HTTP-status form.
+ */
+export async function readConcurProxyBody(response: {
+  status: number
+  text: () => Promise<string>
+}): Promise<string> {
+  const read = response.text()
+  if (response.status >= 200 && response.status < 300) return read
+  return read.catch(() => '')
 }
 
 async function callConcur(
@@ -54,10 +101,10 @@ async function callConcur(
   const hasBody = req.body !== undefined && req.body !== null
   const headers: Record<string, string> = {
     Authorization: `Bearer ${accessToken}`,
-    Accept: 'application/json',
+    Accept: req.accept ?? 'application/json',
   }
   if (hasBody) headers['Content-Type'] = req.contentType ?? 'application/json'
-  if (req.companyUuid) headers['concur-correlationid'] = req.companyUuid
+  headers['concur-correlationid'] = generateId()
 
   const response = await secureFetchWithValidation(
     url,
@@ -70,11 +117,14 @@ async function callConcur(
           : JSON.stringify(req.body)
         : undefined,
       timeout: SAP_CONCUR_OUTBOUND_FETCH_TIMEOUT_MS,
+      maxRedirects: 0,
+      stripAuthOnRedirect: true,
+      maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
     },
     'apiUrl'
   )
 
-  const raw = await response.text()
+  const raw = await readConcurProxyBody(response)
   let parsed: unknown = null
   if (raw.length > 0) {
     try {
@@ -83,7 +133,12 @@ async function callConcur(
       parsed = raw
     }
   }
-  return { status: response.status, body: parsed, raw }
+  return {
+    status: response.status,
+    body: parsed,
+    raw,
+    headers: forwardedSapConcurHeaders(response.headers),
+  }
 }
 
 export const POST = withRouteHandler(async (request: NextRequest) => {
@@ -108,7 +163,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
     if (invocation.status >= 200 && invocation.status < 300) {
       const data = invocation.status === 204 ? null : invocation.body
-      return NextResponse.json({ success: true, output: { status: invocation.status, data } })
+      return NextResponse.json(
+        { success: true, output: { status: invocation.status, data } },
+        { headers: invocation.headers }
+      )
     }
 
     const message = extractSapConcurError(invocation.body, invocation.status)
@@ -117,7 +175,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
     )
     return NextResponse.json(
       { success: false, error: message, status: invocation.status },
-      { status: invocation.status }
+      { status: clampErrorStatus(invocation.status), headers: invocation.headers }
     )
   } catch (error) {
     if (isZodError(error)) {
@@ -128,6 +186,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       )
     }
     logger.error(`[${requestId}] Unexpected Concur proxy error:`, error)
-    return NextResponse.json({ success: false, error: toError(error).message }, { status: 500 })
+    return NextResponse.json(
+      { success: false, error: describeSapConcurFetchError(error) },
+      { status: 500 }
+    )
   }
 })

apps/sim/app/api/tools/sap_concur/proxy/route.ts

@@ -0,0 +1,130 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockReadResponseTextWithLimit, mockSecureFetch, MOCK_MAX_JSON_BYTES } = vi.hoisted(() => ({
+  mockReadResponseTextWithLimit: vi.fn(),
+  mockSecureFetch: vi.fn(),
+  MOCK_MAX_JSON_BYTES: 10 * 1024 * 1024,
+}))
+
+vi.mock('@/lib/core/utils/stream-limits', () => {
+  class PayloadSizeLimitError extends Error {
+    observedBytes?: number
+    constructor(message: string, observedBytes?: number) {
+      super(message)
+      this.name = 'PayloadSizeLimitError'
+      this.observedBytes = observedBytes
+    }
+  }
+  return {
+    PayloadSizeLimitError,
+    readResponseTextWithLimit: mockReadResponseTextWithLimit,
+  }
+})
+
+vi.mock('@/lib/core/security/input-validation.server', () => ({
+  secureFetchWithValidation: mockSecureFetch,
+  MAX_JSON_API_RESPONSE_BYTES: MOCK_MAX_JSON_BYTES,
+}))
+
+import { readConcurProxyBody } from '@/app/api/tools/sap_concur/proxy/route'
+import { readConcurUploadBody } from '@/app/api/tools/sap_concur/upload/route'
+
+/** Minimal response shape both helpers accept. */
+function uploadResponse(status: number): Parameters<typeof readConcurUploadBody>[0] {
+  return {
+    status,
+    headers: new Headers(),
+    body: null,
+  }
+}
+
+function proxyResponse(
+  status: number,
+  text: () => Promise<string>
+): Parameters<typeof readConcurProxyBody>[0] {
+  return { status, text }
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  mockReadResponseTextWithLimit.mockReset()
+})
+
+/**
+ * Both helpers make the same success/error split, so the cases are declared once and run
+ * against each helper. `readConcurUploadBody` reads through the mocked
+ * `readResponseTextWithLimit`; `readConcurProxyBody` reads through `response.text()`.
+ */
+const helpers = [
+  {
+    name: 'readConcurUploadBody',
+    read: (status: number, result: Promise<string>) => {
+      mockReadResponseTextWithLimit.mockReturnValue(result)
+      return readConcurUploadBody(uploadResponse(status))
+    },
+  },
+  {
+    name: 'readConcurProxyBody',
+    read: (status: number, result: Promise<string>) =>
+      readConcurProxyBody(proxyResponse(status, () => result)),
+  },
+] as const
+
+describe.each(helpers)('$name response body reads', ({ read }) => {
+  it('resolves with the body text on a success status', async () => {
+    await expect(read(200, Promise.resolve('{"id":"exp-1"}'))).resolves.toBe('{"id":"exp-1"}')
+  })
+
+  it('resolves with an empty string for an empty success body', async () => {
+    await expect(read(200, Promise.resolve(''))).resolves.toBe('')
+  })
+
+  it('propagates a read failure on a success status', async () => {
+    const failure = new Error('Concur upload response exceeded 10485760 bytes')
+    await expect(read(201, Promise.reject(failure))).rejects.toBe(failure)
+  })
+
+  it('resolves with the body text on an error status', async () => {
+    await expect(read(400, Promise.resolve('{"message":"Invalid userId"}'))).resolves.toBe(
+      '{"message":"Invalid userId"}'
+    )
+  })
+
+  it('swallows a read failure on a 4xx status', async () => {
+    await expect(read(403, Promise.reject(new Error('stream aborted')))).resolves.toBe('')
+  })
+
+  it('swallows a read failure on a 5xx status', async () => {
+    await expect(read(503, Promise.reject(new Error('stream aborted')))).resolves.toBe('')
+  })
+
+  /**
+   * The source compares `status >= 200 && status < 300`, so 200 and 299 take the strict
+   * path and 199 and 300 take the tolerant one.
+   */
+  it.each([200, 299])('treats %i as a success status', async (status) => {
+    const failure = new Error('read failed')
+    await expect(read(status, Promise.reject(failure))).rejects.toBe(failure)
+  })
+
+  it.each([199, 300])('treats %i as a non-success status', async (status) => {
+    await expect(read(status, Promise.reject(new Error('read failed')))).resolves.toBe('')
+  })
+})
+
+describe('readConcurUploadBody byte cap wiring', () => {
+  it('reads under the shared JSON response byte cap', async () => {
+    mockReadResponseTextWithLimit.mockReturnValue(Promise.resolve('{}'))
+    const response = uploadResponse(200)
+
+    await expect(readConcurUploadBody(response)).resolves.toBe('{}')
+
+    expect(mockReadResponseTextWithLimit).toHaveBeenCalledWith(response, {
+      maxBytes: MOCK_MAX_JSON_BYTES,
+      label: 'Concur upload response',
+    })
+  })
+})