🔑 fix: Decode Action OAuth Credentials After Decryption (#14698)

* 🔑 fix: Decode Action OAuth Credentials After Decryption

Action client credentials are URL-encoded before encryption, but the OAuth
token exchange decrypted them with `decryptV2` alone, sending values like
`abc%40domain.com` to the provider and causing 401s for secrets containing
`@`, `+`, `=`, `/` or `:`. Decoding falls back to the raw decrypted value so
legacy secrets stored before 299cabd6e (which contain an unescaped `%`) still work.

Fixes #14636

* 🔑 fix: Share Action Credential Codec Between Read Paths

The encode-before-encrypt convention had two readers with different
behaviour. `ActionService.decryptSensitiveValue` decoded unconditionally,
so a credential stored before encoding was introduced (299cabd6e, March
2025) containing a stray `%` threw `URIError` and failed every read of the
action, not just the token exchange.

Extract the pair into `packages/api/src/actions/crypto.ts` as the single
implementation and consume it from both `ActionService` and the OAuth token
exchange, so the two paths cannot drift again.

Tests cover the codec against real crypto: reserved characters and literal
percent escapes round-trip, pre-encoding values are returned unchanged, and
a pre-encoding value with a stray `%` no longer throws.

A version marker on newly encrypted values was prototyped and dropped: it
changes the stored format for every action credential while fixing nothing,
since values written by the encoding convention already decode correctly
and pre-encoding values stay unmarked either way.

---------

Co-authored-by: Danny Avila <danny@librechat.ai>

api/server/services/ActionService.js

@@ -2,7 +2,7 @@ const jwt = require('jsonwebtoken');
 const { nanoid } = require('nanoid');
 const { GraphEvents, sleep } = require('@librechat/agents');
 const { tool } = require('@librechat/agents/langchain/tools');
-const { logger, encryptV2, decryptV2 } = require('@librechat/data-schemas');
+const { logger, decryptV2 } = require('@librechat/data-schemas');
 const {
   sendEvent,
   logAxiosError,
@@ -10,6 +10,8 @@ const {
   refreshAccessToken,
   GenerationJobManager,
   createSSRFSafeAgents,
+  encryptSensitiveValue,
+  decryptSensitiveValue,
   validateActionOAuthMetadata,
 } = require('@librechat/api');
 const {
@@ -407,27 +409,6 @@ async function createActionTool({
   };
 }
 
-/**
- * Encrypts a sensitive value.
- * @param {string} value
- * @returns {Promise<string>}
- */
-async function encryptSensitiveValue(value) {
-  // Encode API key to handle special characters like ":"
-  const encodedValue = encodeURIComponent(value);
-  return await encryptV2(encodedValue);
-}
-
-/**
- * Decrypts a sensitive value.
- * @param {string} value
- * @returns {Promise<string>}
- */
-async function decryptSensitiveValue(value) {
-  const decryptedValue = await decryptV2(value);
-  return decodeURIComponent(decryptedValue);
-}
-
 /**
  * Encrypts sensitive metadata values for an action.
  *

packages/api/src/actions/crypto.spec.ts

@@ -0,0 +1,59 @@
+process.env.CREDS_KEY =
+  process.env.CREDS_KEY ?? '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef';
+
+// Loaded via dynamic import in beforeAll so encryption initializes after
+// CREDS_KEY is set above (encryptV2 reads the key at module load).
+let encryptSensitiveValue: typeof import('./crypto').encryptSensitiveValue;
+let decryptSensitiveValue: typeof import('./crypto').decryptSensitiveValue;
+let encryptV2: typeof import('@librechat/data-schemas').encryptV2;
+
+beforeAll(async () => {
+  ({ encryptSensitiveValue, decryptSensitiveValue } = await import('./crypto'));
+  ({ encryptV2 } = await import('@librechat/data-schemas'));
+});
+
+describe('action credential encryption', () => {
+  describe('round trip', () => {
+    it.each([
+      ['an email-style client id', 'client@id.com'],
+      ['reserved characters', 's+cret=/end'],
+      ['a colon', 'user:pass'],
+      ['a literal percent sign', '100%secret'],
+      ['a plus sign that must not become a space', 'a+b'],
+      ['a literal percent escape', 'secret%2Fvalue'],
+      ['unicode', 'sécret-π'],
+    ])('preserves %s', async (_label, value) => {
+      expect(await decryptSensitiveValue(await encryptSensitiveValue(value))).toBe(value);
+    });
+  });
+
+  describe('credentials stored before encoding was introduced', () => {
+    it('returns a pre-encoding value unchanged', async () => {
+      const stored = await encryptV2('plain-secret');
+
+      expect(await decryptSensitiveValue(stored)).toBe('plain-secret');
+    });
+
+    /**
+     * Regression: decoding unconditionally threw `URIError` here, which broke every read of the
+     * action rather than just mangling the credential.
+     */
+    it('does not throw on a pre-encoding value containing a stray percent sign', async () => {
+      const stored = await encryptV2('100%secret');
+
+      await expect(decryptSensitiveValue(stored)).resolves.toBe('100%secret');
+    });
+
+    /**
+     * Known, pre-existing limitation rather than desired behaviour: stored values carry no marker
+     * saying whether they were encoded, so a pre-encoding credential that happens to contain a
+     * valid escape is indistinguishable from an encoded one and is still decoded. Re-saving the
+     * action rewrites it in the encoded format and settles the ambiguity.
+     */
+    it('still rewrites a pre-encoding value that happens to contain a valid escape', async () => {
+      const stored = await encryptV2('secret%2Fvalue');
+
+      expect(await decryptSensitiveValue(stored)).toBe('secret/value');
+    });
+  });
+});

packages/api/src/actions/crypto.ts

@@ -0,0 +1,29 @@
+import { encryptV2, decryptV2 } from '@librechat/data-schemas';
+
+/**
+ * Encrypts an action credential, encoding it first so that reserved characters (`@`, `+`, `=`,
+ * `/`, `:`) survive storage.
+ */
+export async function encryptSensitiveValue(value: string): Promise<string> {
+  return encryptV2(encodeURIComponent(value));
+}
+
+/**
+ * Decrypts an action credential, reversing the encoding applied by {@link encryptSensitiveValue}.
+ *
+ * Encoding before encryption was introduced in 299cabd6e (March 2025). Credentials stored earlier
+ * were encrypted raw and carry no marker saying so, so a legacy value containing a stray `%` would
+ * make `decodeURIComponent` throw `URIError`; those are returned as decrypted.
+ *
+ * Every reader of these fields must use this helper. Decoding in one read path but not another
+ * sends different credentials to the provider depending on which path ran.
+ */
+export async function decryptSensitiveValue(encryptedValue: string): Promise<string> {
+  const decryptedValue = await decryptV2(encryptedValue);
+
+  try {
+    return decodeURIComponent(decryptedValue);
+  } catch {
+    return decryptedValue;
+  }
+}

packages/api/src/actions/index.ts

@@ -1,4 +1,5 @@
 export * from './credentials';
+export * from './crypto';
 export * from './protection';
 export * from './tools';
 export * from './update';

packages/api/src/oauth/tokens.spec.ts

@@ -22,6 +22,18 @@ jest.mock('@librechat/data-schemas', () => ({
     if (value === 'encrypted-refresh-token') {
       return 'refresh-token';
     }
+    if (value === 'encrypted-encoded-client-id') {
+      return 'client%40id.com';
+    }
+    if (value === 'encrypted-encoded-client-secret') {
+      return 's%2Bcret%3D%2Fend';
+    }
+    if (value === 'encrypted-legacy-client-id') {
+      return 'legacy%value';
+    }
+    if (value === 'encrypted-legacy-client-secret') {
+      return '100%secret';
+    }
     return value;
   }),
 }));
@@ -215,6 +227,132 @@ describe('action OAuth token exchange validation', () => {
     expect(accessConfig.httpsAgent).toBe(refreshConfig.httpsAgent);
   });
 
+  describe('URL-encoded stored credentials', () => {
+    const encodedFields = {
+      ...baseFields,
+      encrypted_oauth_client_id: 'encrypted-encoded-client-id',
+      encrypted_oauth_client_secret: 'encrypted-encoded-client-secret',
+    };
+
+    const legacyFields = {
+      ...baseFields,
+      encrypted_oauth_client_id: 'encrypted-legacy-client-id',
+      encrypted_oauth_client_secret: 'encrypted-legacy-client-secret',
+    };
+
+    it('decodes credentials in the authorization-code exchange body', async () => {
+      await getAccessToken(
+        {
+          ...encodedFields,
+          code: 'authorization-code',
+          redirect_uri: 'https://chat.example.com/api/actions/action-1/oauth/callback',
+          token_exchange_method: TokenExchangeMethodEnum.DefaultPost,
+        },
+        createTokenMethods(),
+      );
+
+      const params = new URLSearchParams(getAxiosConfig().data as string);
+
+      expect(params.get('client_id')).toBe('client@id.com');
+      expect(params.get('client_secret')).toBe('s+cret=/end');
+    });
+
+    it('decodes credentials in the authorization-code Basic auth header', async () => {
+      await getAccessToken(
+        {
+          ...encodedFields,
+          code: 'authorization-code',
+          redirect_uri: 'https://chat.example.com/api/actions/action-1/oauth/callback',
+          token_exchange_method: TokenExchangeMethodEnum.BasicAuthHeader,
+        },
+        createTokenMethods(),
+      );
+
+      const headers = getAxiosConfig().headers as Record<string, string>;
+      const credentials = Buffer.from(
+        headers.Authorization.replace('Basic ', ''),
+        'base64',
+      ).toString('utf8');
+
+      expect(credentials).toBe('client@id.com:s+cret=/end');
+    });
+
+    it('decodes credentials in the refresh-token exchange body', async () => {
+      await refreshAccessToken(
+        {
+          ...encodedFields,
+          refresh_token: 'refresh-token',
+          token_exchange_method: TokenExchangeMethodEnum.DefaultPost,
+        },
+        createTokenMethods(),
+      );
+
+      const params = new URLSearchParams(getAxiosConfig().data as string);
+
+      expect(params.get('client_id')).toBe('client@id.com');
+      expect(params.get('client_secret')).toBe('s+cret=/end');
+    });
+
+    it('decodes credentials in the refresh-token Basic auth header', async () => {
+      await refreshAccessToken(
+        {
+          ...encodedFields,
+          refresh_token: 'refresh-token',
+          token_exchange_method: TokenExchangeMethodEnum.BasicAuthHeader,
+        },
+        createTokenMethods(),
+      );
+
+      const headers = getAxiosConfig().headers as Record<string, string>;
+      const credentials = Buffer.from(
+        headers.Authorization.replace('Basic ', ''),
+        'base64',
+      ).toString('utf8');
+
+      expect(credentials).toBe('client@id.com:s+cret=/end');
+    });
+
+    it('passes through legacy unencoded credentials containing a stray percent sign', async () => {
+      await expect(
+        getAccessToken(
+          {
+            ...legacyFields,
+            code: 'authorization-code',
+            redirect_uri: 'https://chat.example.com/api/actions/action-1/oauth/callback',
+            token_exchange_method: TokenExchangeMethodEnum.DefaultPost,
+          },
+          createTokenMethods(),
+        ),
+      ).resolves.toEqual(tokenResponse);
+
+      const params = new URLSearchParams(getAxiosConfig().data as string);
+
+      expect(params.get('client_id')).toBe('legacy%value');
+      expect(params.get('client_secret')).toBe('100%secret');
+    });
+
+    it('passes through legacy unencoded credentials when refreshing', async () => {
+      await expect(
+        refreshAccessToken(
+          {
+            ...legacyFields,
+            refresh_token: 'refresh-token',
+            token_exchange_method: TokenExchangeMethodEnum.BasicAuthHeader,
+          },
+          createTokenMethods(),
+        ),
+      ).resolves.toEqual(tokenResponse);
+
+      const headers = getAxiosConfig().headers as Record<string, string>;
+      const credentials = Buffer.from(
+        headers.Authorization.replace('Basic ', ''),
+        'base64',
+      ).toString('utf8');
+
+      expect(credentials).toBe('legacy%value:100%secret');
+    });
+  });
+
   it('allows explicitly exempted private token endpoints', async () => {
     await getAccessToken(
       {

packages/api/src/oauth/tokens.ts

@@ -1,10 +1,11 @@
 import axios from 'axios';
+import { logger, encryptV2 } from '@librechat/data-schemas';
 import { TokenExchangeMethodEnum } from 'librechat-data-provider';
-import { logger, encryptV2, decryptV2 } from '@librechat/data-schemas';
 import type { IToken, TokenMethods } from '@librechat/data-schemas';
 import type { AxiosError } from 'axios';
 import { DEFAULT_OAUTH_TOKEN_TTL_SECONDS, normalizeExpiresIn } from './expiry';
 import { validateActionOAuthEndpoint } from './validation';
+import { decryptSensitiveValue } from '~/actions/crypto';
 import { createSSRFSafeAgents } from '~/auth';
 import { logAxiosError } from '~/utils';
 
@@ -189,8 +190,8 @@ export async function refreshAccessToken(
   await validateActionOAuthEndpoint(client_url, 'client_url', allowedAddresses);
 
   try {
-    const oauth_client_id = await decryptV2(encrypted_oauth_client_id);
-    const oauth_client_secret = await decryptV2(encrypted_oauth_client_secret);
+    const oauth_client_id = await decryptSensitiveValue(encrypted_oauth_client_id);
+    const oauth_client_secret = await decryptSensitiveValue(encrypted_oauth_client_secret);
 
     const headers: Record<string, string> = {
       'Content-Type': 'application/x-www-form-urlencoded',
@@ -294,8 +295,8 @@ export async function getAccessToken(
 }> {
   await validateActionOAuthEndpoint(client_url, 'client_url', allowedAddresses);
 
-  const oauth_client_id = await decryptV2(encrypted_oauth_client_id);
-  const oauth_client_secret = await decryptV2(encrypted_oauth_client_secret);
+  const oauth_client_id = await decryptSensitiveValue(encrypted_oauth_client_id);
+  const oauth_client_secret = await decryptSensitiveValue(encrypted_oauth_client_secret);
 
   const headers: Record<string, string> = {
     'Content-Type': 'application/x-www-form-urlencoded',