diff --git a/api/server/services/ActionService.js b/api/server/services/ActionService.js index 2c877c515e..85b3d45cdc 100644 --- a/api/server/services/ActionService.js +++ b/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} - */ -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} - */ -async function decryptSensitiveValue(value) { - const decryptedValue = await decryptV2(value); - return decodeURIComponent(decryptedValue); -} - /** * Encrypts sensitive metadata values for an action. * diff --git a/packages/api/src/actions/crypto.spec.ts b/packages/api/src/actions/crypto.spec.ts new file mode 100644 index 0000000000..af1a0693ba --- /dev/null +++ b/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'); + }); + }); +}); diff --git a/packages/api/src/actions/crypto.ts b/packages/api/src/actions/crypto.ts new file mode 100644 index 0000000000..85f1121113 --- /dev/null +++ b/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 { + 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 { + const decryptedValue = await decryptV2(encryptedValue); + + try { + return decodeURIComponent(decryptedValue); + } catch { + return decryptedValue; + } +} diff --git a/packages/api/src/actions/index.ts b/packages/api/src/actions/index.ts index d0ecba2ce2..1204d803f8 100644 --- a/packages/api/src/actions/index.ts +++ b/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'; diff --git a/packages/api/src/oauth/tokens.spec.ts b/packages/api/src/oauth/tokens.spec.ts index f3e5b4a29e..3d47840e79 100644 --- a/packages/api/src/oauth/tokens.spec.ts +++ b/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; + 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; + 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; + 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( { diff --git a/packages/api/src/oauth/tokens.ts b/packages/api/src/oauth/tokens.ts index 02fb80e287..2a44af334d 100644 --- a/packages/api/src/oauth/tokens.ts +++ b/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 = { '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 = { 'Content-Type': 'application/x-www-form-urlencoded',