From cd70568f80e0307f085f25e559d2754f13e15434 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 16:28:38 -0700 Subject: [PATCH 1/3] fix(sap_concur): key the token cache with an HMAC and document rate casing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache key hashed a user-chosen password with a bare SHA-256. The key never leaves the process, but a password is low-entropy enough to brute-force out of a plain digest if one ever reached a heap dump or a debug log, which is what CodeQL flags. Keying the digest with a server-side secret makes it useless without that secret. A password-hashing KDF would be the wrong tool here: this runs on every token fetch, and the goal is collision-free partitioning rather than verification of a stored credential. The body wand prompt also claimed every payload family is camelCase. Exchange rate uploads are the exception — they take a snake_case currency_sets array of from_crn_code, to_crn_code, start_date and rate — and that operation is in BODY_OPS, so the blanket claim produced bodies Concur rejects. --- apps/sim/app/api/tools/sap_concur/shared.ts | 15 +++++++++++++-- apps/sim/blocks/blocks/sap_concur.ts | 4 +++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/tools/sap_concur/shared.ts b/apps/sim/app/api/tools/sap_concur/shared.ts index 8c7adf82665..414bf995220 100644 --- a/apps/sim/app/api/tools/sap_concur/shared.ts +++ b/apps/sim/app/api/tools/sap_concur/shared.ts @@ -1,10 +1,11 @@ -import { createHash } from 'node:crypto' +import { createHmac } from 'node:crypto' import { createLogger } from '@sim/logger' import { isPrivateIpHost } from '@sim/security/ssrf' import { getErrorMessage } from '@sim/utils/errors' import { truncate } from '@sim/utils/string' import { z } from 'zod' import { coalesceLocally } from '@/lib/concurrency/singleflight' +import { env } from '@/lib/core/config/env' import { MAX_JSON_API_RESPONSE_BYTES, secureFetchWithValidation, @@ -229,8 +230,18 @@ function readCachedToken(key: string): SapConcurToken | undefined { * and collide with a different tuple. The full sha256 digest is kept — truncating it * would lower the collision/forgery bar for no measurable gain. */ +/** + * Key the token cache by every credential that changes which token Concur mints. + * + * Keyed with a server-side secret rather than a bare digest. The inputs include a + * user-chosen password, which is low-entropy enough to brute-force from a plain + * SHA-256 if a key ever reached a heap dump or a debug log; an HMAC makes the key + * useless without the secret. A password-hashing KDF would be the wrong tool — this + * runs on every token fetch and the goal is collision-free partitioning, not + * verification of a stored credential. + */ function tokenCacheKey(req: SapConcurAuth): string { - return createHash('sha256') + return createHmac('sha256', env.INTERNAL_API_SECRET) .update( JSON.stringify([ req.datacenter, diff --git a/apps/sim/blocks/blocks/sap_concur.ts b/apps/sim/blocks/blocks/sap_concur.ts index 0641b80d334..1ac491c3363 100644 --- a/apps/sim/blocks/blocks/sap_concur.ts +++ b/apps/sim/blocks/blocks/sap_concur.ts @@ -1893,7 +1893,7 @@ Return ONLY the comma-separated travel config IDs - no explanations, no extra te enabled: true, prompt: `Generate the JSON request body for the selected SAP Concur operation from the user's request. -Match the payload to the resource being written. Every family below is camelCase. +Match the payload to the resource being written. Every family below is camelCase EXCEPT exchange rates, which is snake_case. Expense reports (v4): name, businessPurpose, comment, policyId, countryCode, countrySubDivisionCode, reportDate, startDate, endDate, and reportSource — reportSource is REQUIRED when updating a report and must be one of EA, MOB, OTHER, SE, TR, UI. @@ -1905,6 +1905,8 @@ SCIM users (Identity v4.1): create and update payloads use schemas, userName, na List items: listId, level, value, shortCode. Cash advances: amountRequested as { currency, amount }, name and userId (all required), plus optional accountCode, comment and purpose. +Exchange rates are the one snake_case family: currency_sets as an array of up to 100 entries, each { from_crn_code, to_crn_code, start_date as YYYY-MM-DD, rate }. + Omit fields the user did not describe rather than inventing identifiers. Return ONLY the JSON object - no explanations, no extra text.`, From c1c3e697127a142e198d94edda6bbb6dba640a37 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 16:38:39 -0700 Subject: [PATCH 2/3] fix(sap_concur): wire the travel request sendback comment through the block move_travel_request accepts a documented query comment that Concur applies to the sendback action, but the params branch never passed it and the block's only comment field is gated to create_report_comment, so the value was unreachable from the UI. Uses a dedicated sendbackComment subblock rather than widening the existing comment field: that one is required for create_report_comment while this is optional, so sharing an id would both clash on required-ness and let a value bleed between the two operations. --- apps/sim/blocks/blocks/sap_concur.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/apps/sim/blocks/blocks/sap_concur.ts b/apps/sim/blocks/blocks/sap_concur.ts index 1ac491c3363..eca2fab8198 100644 --- a/apps/sim/blocks/blocks/sap_concur.ts +++ b/apps/sim/blocks/blocks/sap_concur.ts @@ -1007,6 +1007,14 @@ Return ONLY the YYYY-MM-DD date - no explanations, no extra text.`, condition: { field: 'operation', value: 'sap_concur_create_report_comment' }, required: { field: 'operation', value: 'sap_concur_create_report_comment' }, }, + { + id: 'sendbackComment', + title: 'Sendback Comment', + type: 'long-input', + placeholder: 'Visible wherever Request comments are shown (sendback only)', + condition: { field: 'operation', value: 'sap_concur_move_travel_request' }, + mode: 'advanced', + }, { id: 'includeAllComments', title: 'Include All Comments', @@ -2264,6 +2272,7 @@ Return ONLY the JSON object - no explanations, no extra text.`, body: params.body || undefined, userId: params.travelRequestUserId || undefined, companyID: params.companyID || undefined, + comment: params.sendbackComment || undefined, } case 'sap_concur_list_travel_request_comments': return { ...auth, requestUuid: params.requestUuid } @@ -2534,6 +2543,11 @@ Return ONLY the JSON object - no explanations, no extra text.`, description: 'Optional company identifier for a travel request workflow action (documented as companyID, distinct from companyUuid)', }, + sendbackComment: { + type: 'string', + description: + 'Optional comment on a travel request workflow action — Concur applies it only to the sendback action, and it is visible wherever Request comments are shown', + }, travelRequestApprovedBefore: { type: 'string', description: 'Travel requests approved before' }, travelRequestApprovedAfter: { type: 'string', description: 'Travel requests approved after' }, travelRequestModifiedBefore: { type: 'string', description: 'Travel requests modified before' }, From a21f854daa12c29c411a2d07b84c4f4e5ee67844 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:01:00 -0700 Subject: [PATCH 3/3] fix(sap_concur): gate the sendback comment and merge duplicate TSDoc The sendbackComment field was conditioned only on the operation, so it rendered for submit, approve, cancel and every other workflow action even though Concur applies the comment to sendback alone. It is now gated on the action as well, and the params branch only forwards it for sendback so a value retained from an earlier sendback cannot ride along once the field is hidden. Also folds the two consecutive TSDoc blocks left above tokenCacheKey into one. Only the nearest block binds to the declaration, so the separator and collision reasoning in the earlier block was detached. --- apps/sim/app/api/tools/sap_concur/shared.ts | 16 ++++++---------- apps/sim/blocks/blocks/sap_concur.ts | 11 ++++++++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/apps/sim/app/api/tools/sap_concur/shared.ts b/apps/sim/app/api/tools/sap_concur/shared.ts index 414bf995220..28800c6256d 100644 --- a/apps/sim/app/api/tools/sap_concur/shared.ts +++ b/apps/sim/app/api/tools/sap_concur/shared.ts @@ -227,18 +227,14 @@ function readCachedToken(key: string): SapConcurToken | undefined { * * The whole tuple is JSON-encoded before hashing rather than concatenated with a * separator, so a free-form field (clientId, companyUuid) cannot span a field boundary - * and collide with a different tuple. The full sha256 digest is kept — truncating it - * would lower the collision/forgery bar for no measurable gain. - */ -/** - * Key the token cache by every credential that changes which token Concur mints. + * and collide with a different tuple. * * Keyed with a server-side secret rather than a bare digest. The inputs include a - * user-chosen password, which is low-entropy enough to brute-force from a plain - * SHA-256 if a key ever reached a heap dump or a debug log; an HMAC makes the key - * useless without the secret. A password-hashing KDF would be the wrong tool — this - * runs on every token fetch and the goal is collision-free partitioning, not - * verification of a stored credential. + * user-chosen password, which is low-entropy enough to brute-force from a plain SHA-256 + * if a key ever reached a heap dump or a debug log; an HMAC makes the key useless without + * the secret. A password-hashing KDF would be the wrong tool — this runs on every token + * fetch and the goal is collision-free partitioning, not verification of a stored + * credential. */ function tokenCacheKey(req: SapConcurAuth): string { return createHmac('sha256', env.INTERNAL_API_SECRET) diff --git a/apps/sim/blocks/blocks/sap_concur.ts b/apps/sim/blocks/blocks/sap_concur.ts index eca2fab8198..46809164f5c 100644 --- a/apps/sim/blocks/blocks/sap_concur.ts +++ b/apps/sim/blocks/blocks/sap_concur.ts @@ -1011,8 +1011,12 @@ Return ONLY the YYYY-MM-DD date - no explanations, no extra text.`, id: 'sendbackComment', title: 'Sendback Comment', type: 'long-input', - placeholder: 'Visible wherever Request comments are shown (sendback only)', - condition: { field: 'operation', value: 'sap_concur_move_travel_request' }, + placeholder: 'Visible wherever Request comments are shown', + condition: { + field: 'operation', + value: 'sap_concur_move_travel_request', + and: { field: 'action', value: 'sendback' }, + }, mode: 'advanced', }, { @@ -2272,7 +2276,8 @@ Return ONLY the JSON object - no explanations, no extra text.`, body: params.body || undefined, userId: params.travelRequestUserId || undefined, companyID: params.companyID || undefined, - comment: params.sendbackComment || undefined, + comment: + params.action === 'sendback' ? params.sendbackComment || undefined : undefined, } case 'sap_concur_list_travel_request_comments': return { ...auth, requestUuid: params.requestUuid }