From 05d4e90f915c8a24de831b4f3a796dbb4534d294 Mon Sep 17 00:00:00 2001 From: Ravi Kumar L Date: Tue, 12 May 2026 05:30:01 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=A9=EF=B8=8F=20feat:=20Strict=20CloudF?= =?UTF-8?q?ront=20signed=20cookie=20enforcement=20via=20`requireSignedAcce?= =?UTF-8?q?ss`=20(#13078)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cloudfront): add requireSignedAccess to enforce strict signed access Introduces cloudfront.requireSignedAccess (default false). When enabled, initializeCloudFront requires both CLOUDFRONT_KEY_PAIR_ID and CLOUDFRONT_PRIVATE_KEY, rejects the unimplemented imageSigning="url" mode, and initializeFileStorage throws to block startup on any CloudFront init failure. OSS path is unchanged: missing keys still log-and-continue when requireSignedAccess is false. Adds low-noise startup and cookie-issuance logs without leaking signed URLs, policies, signatures, private keys, or cookie values. * fix(cloudfront): reject requireSignedAccess unless imageSigning is "cookies" Previously requireSignedAccess=true was accepted with imageSigning="none" or "url", but setCloudFrontCookies() only runs for "cookies" — leaving strict mode toothless: CloudFront stayed publicly accessible, or image delivery broke on a distribution that actually requires signed access. Adds a Zod refinement plus a runtime guard in initializeCloudFront so the only currently-functional strict configuration is imageSigning "cookies". Signed URL mode can lift this restriction once implemented. * fix(cloudfront): resolve strict access type checks * chore(cloudfront): reduce strict startup log noise --------- Co-authored-by: Danny Avila --- packages/api/src/app/__tests__/cdn.test.ts | 48 ++++++++++++++- packages/api/src/app/cdn.ts | 5 ++ .../api/src/cdn/__tests__/cloudfront.test.ts | 59 +++++++++++++++++++ packages/api/src/cdn/cloudfront-cookies.ts | 4 ++ packages/api/src/cdn/cloudfront.ts | 22 ++++++- .../storage/cloudfront/__tests__/crud.test.ts | 1 + .../src/cloudfront-config.spec.ts | 39 ++++++++++++ packages/data-provider/src/config.ts | 6 ++ 8 files changed, 181 insertions(+), 3 deletions(-) diff --git a/packages/api/src/app/__tests__/cdn.test.ts b/packages/api/src/app/__tests__/cdn.test.ts index cc85fb594c..def5e92f79 100644 --- a/packages/api/src/app/__tests__/cdn.test.ts +++ b/packages/api/src/app/__tests__/cdn.test.ts @@ -21,6 +21,7 @@ jest.mock('~/cdn/cloudfront', () => ({ })); import type { AppConfig } from '@librechat/data-schemas'; +import type { CloudFrontConfig } from 'librechat-data-provider'; import { initializeFileStorage } from '../cdn'; import { initializeFirebase } from '~/cdn/firebase'; import { initializeAzureBlobService } from '~/cdn/azure'; @@ -33,6 +34,23 @@ const baseAppConfig: AppConfig = { imageOutputType: 'png', }; +type RequiredCloudFrontConfig = NonNullable; + +function makeCloudFrontConfig( + overrides: Partial = {}, +): RequiredCloudFrontConfig { + return { + domain: 'https://d123.cloudfront.net', + invalidateOnDelete: false, + imageSigning: 'none', + urlExpiry: 3600, + cookieExpiry: 1800, + includeRegionInPath: false, + requireSignedAccess: false, + ...overrides, + }; +} + describe('initializeFileStorage', () => { beforeEach(() => { jest.clearAllMocks(); @@ -88,7 +106,7 @@ describe('initializeFileStorage', () => { }); it('initializes CloudFront with config when fileStrategy is cloudfront', () => { - const cloudfrontConfig = { domain: 'https://d123.cloudfront.net' }; + const cloudfrontConfig = makeCloudFrontConfig(); const appConfig = { ...baseAppConfig, fileStrategy: FileSources.cloudfront, @@ -109,7 +127,7 @@ describe('initializeFileStorage', () => { }); it('initializes CloudFront from fileStrategies when fileStrategy is local, but avatar is configured for CloudFront', () => { - const cloudfrontConfig = { domain: 'https://d123.cloudfront.net' }; + const cloudfrontConfig = makeCloudFrontConfig(); const appConfig = { ...baseAppConfig, fileStrategy: FileSources.local, @@ -122,6 +140,32 @@ describe('initializeFileStorage', () => { expect(initializeS3).not.toHaveBeenCalled(); }); + it('throws when CloudFront init fails and requireSignedAccess is true', () => { + (initializeCloudFront as jest.Mock).mockReturnValue(false); + const cloudfrontConfig = makeCloudFrontConfig({ + imageSigning: 'cookies', + cookieDomain: '.example.com', + requireSignedAccess: true, + }); + const appConfig = { + ...baseAppConfig, + fileStrategy: FileSources.cloudfront, + cloudfront: cloudfrontConfig, + } as AppConfig; + expect(() => initializeFileStorage(appConfig)).toThrow(/requireSignedAccess=true/); + }); + + it('does not throw when CloudFront init fails and requireSignedAccess is false', () => { + (initializeCloudFront as jest.Mock).mockReturnValue(false); + const cloudfrontConfig = makeCloudFrontConfig(); + const appConfig = { + ...baseAppConfig, + fileStrategy: FileSources.cloudfront, + cloudfront: cloudfrontConfig, + } as AppConfig; + expect(() => initializeFileStorage(appConfig)).not.toThrow(); + }); + it('does not call any initializer when fileStrategy is local with no fileStrategies', () => { const appConfig = { ...baseAppConfig, fileStrategy: FileSources.local } as AppConfig; initializeFileStorage(appConfig); diff --git a/packages/api/src/app/cdn.ts b/packages/api/src/app/cdn.ts index 9feb606cfb..93607eddd8 100644 --- a/packages/api/src/app/cdn.ts +++ b/packages/api/src/app/cdn.ts @@ -25,6 +25,11 @@ function initializeStrategy(strategy: FileSources, appConfig: AppConfig): void { } const initialized = initializeCloudFront(cloudfrontConfig); if (!initialized) { + if (cloudfrontConfig.requireSignedAccess === true) { + throw new Error( + '[initializeFileStorage] CloudFront initialization failed and cloudfront.requireSignedAccess=true; refusing to start.', + ); + } logger.error( '[initializeFileStorage] CloudFront initialization failed. CloudFront operations will not work.', ); diff --git a/packages/api/src/cdn/__tests__/cloudfront.test.ts b/packages/api/src/cdn/__tests__/cloudfront.test.ts index 3e02d8738a..471a6d2d27 100644 --- a/packages/api/src/cdn/__tests__/cloudfront.test.ts +++ b/packages/api/src/cdn/__tests__/cloudfront.test.ts @@ -22,6 +22,7 @@ function makeConfig(overrides: Partial = {}): Required urlExpiry: 3600, cookieExpiry: 1800, includeRegionInPath: false, + requireSignedAccess: false, ...overrides, }; } @@ -136,6 +137,64 @@ describe('CloudFront CDN module', () => { ), ); }); + + describe('requireSignedAccess (strict mode)', () => { + it('logs strict-mode startup info when enabled and keys are present', async () => { + process.env.CLOUDFRONT_KEY_PAIR_ID = 'K123'; + process.env.CLOUDFRONT_PRIVATE_KEY = 'my-private-key'; + const { initializeCloudFront } = await load(); + expect( + initializeCloudFront(makeConfig({ imageSigning: 'cookies', requireSignedAccess: true })), + ).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + expect.stringContaining('Strict signed CloudFront access enabled at startup'), + ); + expect(mockLogger.info).not.toHaveBeenCalledWith( + expect.stringContaining('CloudFront cookie signing enabled'), + ); + }); + + it('returns false and logs strict failure when keys are missing', async () => { + const { initializeCloudFront } = await load(); + expect( + initializeCloudFront(makeConfig({ imageSigning: 'cookies', requireSignedAccess: true })), + ).toBe(false); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('Strict startup failure'), + ); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('CLOUDFRONT_KEY_PAIR_ID'), + ); + }); + + it('returns false when imageSigning is "none" even if keys are present', async () => { + process.env.CLOUDFRONT_KEY_PAIR_ID = 'K123'; + process.env.CLOUDFRONT_PRIVATE_KEY = 'my-private-key'; + const { initializeCloudFront } = await load(); + expect( + initializeCloudFront(makeConfig({ imageSigning: 'none', requireSignedAccess: true })), + ).toBe(false); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'cloudfront.requireSignedAccess=true requires cloudfront.imageSigning="cookies"', + ), + ); + }); + + it('returns false when imageSigning is "url" even if keys are present', async () => { + process.env.CLOUDFRONT_KEY_PAIR_ID = 'K123'; + process.env.CLOUDFRONT_PRIVATE_KEY = 'my-private-key'; + const { initializeCloudFront } = await load(); + expect( + initializeCloudFront(makeConfig({ imageSigning: 'url', requireSignedAccess: true })), + ).toBe(false); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'cloudfront.requireSignedAccess=true requires cloudfront.imageSigning="cookies"', + ), + ); + }); + }); }); describe('getCloudFrontConfig', () => { diff --git a/packages/api/src/cdn/cloudfront-cookies.ts b/packages/api/src/cdn/cloudfront-cookies.ts index 0c905df537..9bcfb8d58b 100644 --- a/packages/api/src/cdn/cloudfront-cookies.ts +++ b/packages/api/src/cdn/cloudfront-cookies.ts @@ -341,6 +341,10 @@ export function setCloudFrontCookies( path: '/', }); + logger.debug( + `[setCloudFrontCookies] Issued signed CloudFront cookies (paths=${signedCookieSets.length}, expiresInSec=${cookieExpiry}).`, + ); + return true; } catch (error) { logger.error('[setCloudFrontCookies] Failed to generate signed cookies:', error); diff --git a/packages/api/src/cdn/cloudfront.ts b/packages/api/src/cdn/cloudfront.ts index 4bf56de43c..43287aa844 100644 --- a/packages/api/src/cdn/cloudfront.ts +++ b/packages/api/src/cdn/cloudfront.ts @@ -29,6 +29,26 @@ export function initializeCloudFront(config: CloudFrontConfig): boolean { const keyPairId = process.env.CLOUDFRONT_KEY_PAIR_ID ?? null; const privateKey = process.env.CLOUDFRONT_PRIVATE_KEY ?? null; + const requireSignedAccess = config.requireSignedAccess === true; + + if (requireSignedAccess) { + logger.info('[initializeCloudFront] Strict signed CloudFront access enabled at startup.'); + + if (config.imageSigning !== 'cookies') { + logger.error( + `[initializeCloudFront] Strict startup failure: cloudfront.requireSignedAccess=true requires cloudfront.imageSigning="cookies" (got "${config.imageSigning ?? 'none'}"); signed URL mode is not yet implemented.`, + ); + return false; + } + + if (!keyPairId || !privateKey) { + logger.error( + '[initializeCloudFront] Strict startup failure: cloudfront.requireSignedAccess=true but CLOUDFRONT_KEY_PAIR_ID and/or CLOUDFRONT_PRIVATE_KEY are missing.', + ); + return false; + } + } + if (config.imageSigning === 'cookies' && (!keyPairId || !privateKey)) { logger.error( '[initializeCloudFront] imageSigning="cookies" requires CLOUDFRONT_KEY_PAIR_ID and CLOUDFRONT_PRIVATE_KEY env vars.', @@ -38,7 +58,7 @@ export function initializeCloudFront(config: CloudFrontConfig): boolean { cloudFrontConfig = { ...config, privateKey, keyPairId }; - if (config.imageSigning === 'cookies') { + if (config.imageSigning === 'cookies' && !requireSignedAccess) { logger.info( '[initializeCloudFront] CloudFront cookie signing enabled. Cookies will be set during auth.', ); diff --git a/packages/api/src/storage/cloudfront/__tests__/crud.test.ts b/packages/api/src/storage/cloudfront/__tests__/crud.test.ts index d3bf5bf975..5990cceac5 100644 --- a/packages/api/src/storage/cloudfront/__tests__/crud.test.ts +++ b/packages/api/src/storage/cloudfront/__tests__/crud.test.ts @@ -54,6 +54,7 @@ function makeConfig(overrides: Partial = {}): CloudFrontFu urlExpiry: 3600, cookieExpiry: 1800, includeRegionInPath: false, + requireSignedAccess: false, privateKey: null, keyPairId: null, ...overrides, diff --git a/packages/data-provider/src/cloudfront-config.spec.ts b/packages/data-provider/src/cloudfront-config.spec.ts index c3c0a00416..03e6158d5f 100644 --- a/packages/data-provider/src/cloudfront-config.spec.ts +++ b/packages/data-provider/src/cloudfront-config.spec.ts @@ -95,4 +95,43 @@ describe('cloudfrontConfigSchema cross-field refinements', () => { }); expect(result.success).toBe(true); }); + + it('rejects requireSignedAccess=true when imageSigning is "none"', () => { + const result = cloudfrontConfigSchema.safeParse({ + domain: 'https://cdn.example.com', + requireSignedAccess: true, + }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues[0].message).toContain( + 'cloudfront.requireSignedAccess=true requires cloudfront.imageSigning="cookies"', + ); + expect(result.error.issues[0].path).toEqual(['requireSignedAccess']); + } + }); + + it('rejects requireSignedAccess=true when imageSigning is "url"', () => { + const result = cloudfrontConfigSchema.safeParse({ + domain: 'https://cdn.example.com', + imageSigning: 'url', + requireSignedAccess: true, + }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues[0].message).toContain( + 'cloudfront.requireSignedAccess=true requires cloudfront.imageSigning="cookies"', + ); + expect(result.error.issues[0].path).toEqual(['requireSignedAccess']); + } + }); + + it('accepts requireSignedAccess=true with imageSigning="cookies" and cookieDomain', () => { + const result = cloudfrontConfigSchema.safeParse({ + domain: 'https://cdn.example.com', + imageSigning: 'cookies', + cookieDomain: '.example.com', + requireSignedAccess: true, + }); + expect(result.success).toBe(true); + }); }); diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index e7c0add133..5c4e1c6ce0 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -215,6 +215,7 @@ export const cloudfrontConfigSchema = z .optional(), storageRegion: z.string().min(1).optional(), includeRegionInPath: z.boolean().default(false), + requireSignedAccess: z.boolean().default(false), }) .refine((data) => !data.invalidateOnDelete || !!data.distributionId, { message: 'distributionId is required when invalidateOnDelete is true', @@ -225,6 +226,11 @@ export const cloudfrontConfigSchema = z 'cookieDomain is required when imageSigning is "cookies" (e.g., ".example.com" for API at api.example.com and CDN at cdn.example.com)', path: ['cookieDomain'], }) + .refine((data) => !data.requireSignedAccess || data.imageSigning === 'cookies', { + message: + 'cloudfront.requireSignedAccess=true requires cloudfront.imageSigning="cookies" (signed URL mode is not yet implemented)', + path: ['requireSignedAccess'], + }) .optional(); export type CloudFrontConfig = z.infer;