fix(sap_concur): HMAC the token cache key and wire the sendback comment (#6794)

* fix(sap_concur): key the token cache with an HMAC and document rate casing

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.

* 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.

* 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.

Author: waleedlatif1

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,
@@ -226,11 +227,17 @@ 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.
+ * 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.
  */
 function tokenCacheKey(req: SapConcurAuth): string {
-  return createHash('sha256')
+  return createHmac('sha256', env.INTERNAL_API_SECRET)
     .update(
       JSON.stringify([
         req.datacenter,

apps/sim/blocks/blocks/sap_concur.ts

@@ -1007,6 +1007,18 @@ 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',
+      condition: {
+        field: 'operation',
+        value: 'sap_concur_move_travel_request',
+        and: { field: 'action', value: 'sendback' },
+      },
+      mode: 'advanced',
+    },
     {
       id: 'includeAllComments',
       title: 'Include All Comments',
@@ -1893,7 +1905,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 +1917,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.`,
@@ -2262,6 +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.action === 'sendback' ? params.sendbackComment || undefined : undefined,
             }
           case 'sap_concur_list_travel_request_comments':
             return { ...auth, requestUuid: params.requestUuid }
@@ -2532,6 +2548,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' },