From cf620a1c388615c18ddb4d11403a83d0e28c6951 Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Fri, 10 Jul 2026 10:42:45 +0700 Subject: [PATCH 1/5] Fix cross-workspace blob/thumbnail and metadata disclosure The preview pod's thumbnail and metadata routes did not validate the caller's token against the workspace taken from the URL. Reported-by: Ugur Ozer, Aeon AI Risk Management Signed-off-by: Artyom Savchenko --- pods/preview/src/__tests__/middleware.test.ts | 144 ++++++++++++++++++ pods/preview/src/config.ts | 2 + pods/preview/src/middleware.ts | 49 ++++++ pods/preview/src/server.ts | 6 +- 4 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 pods/preview/src/__tests__/middleware.test.ts diff --git a/pods/preview/src/__tests__/middleware.test.ts b/pods/preview/src/__tests__/middleware.test.ts new file mode 100644 index 00000000000..8dccd30e956 --- /dev/null +++ b/pods/preview/src/__tests__/middleware.test.ts @@ -0,0 +1,144 @@ +// +// Copyright © 2025 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { type AccountUuid, systemAccountUuid, type WorkspaceUuid } from '@hcengineering/core' +import { extractToken } from '@hcengineering/server-client' +import { type Token } from '@hcengineering/server-token' +import { type NextFunction, type Response } from 'express' + +import { HttpError } from '../error' +import { type RequestWithAuth, withBlob, withOptionalAuth } from '../middleware' + +jest.mock('@hcengineering/server-client', () => ({ + extractToken: jest.fn() +})) + +const extractTokenMock = extractToken as jest.MockedFunction + +const workspaceA = '00000000-0000-4000-8000-00000000000a' as WorkspaceUuid +const workspaceB = '00000000-0000-4000-8000-00000000000b' as WorkspaceUuid +const account = '00000000-0000-4000-8000-0000000000ac' as AccountUuid + +function makeToken (token: Partial): Token { + const result: Token = { account, workspace: workspaceA, extra: {} } + return { ...result, ...token } +} + +function makeRequest (workspace: string, name: string, token?: Token): RequestWithAuth { + return { headers: {}, params: { workspace, name }, token } as unknown as RequestWithAuth +} + +const res = {} as unknown as Response + +describe('withOptionalAuth', () => { + beforeEach(() => { + extractTokenMock.mockReset() + }) + + it('rejects requests without a token in secure mode', () => { + extractTokenMock.mockReturnValue(undefined) + const next = jest.fn() as unknown as NextFunction + + withOptionalAuth(true)(makeRequest(workspaceA, 'blob'), res, next) + + expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 401 })) + }) + + it('rejects guest and readonly tokens in secure mode', () => { + for (const extra of [{ guest: 'true' }, { readonly: 'true' }]) { + extractTokenMock.mockReturnValue(makeToken({ extra })) + const next = jest.fn() as unknown as NextFunction + + withOptionalAuth(true)(makeRequest(workspaceA, 'blob'), res, next) + + expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 401 })) + } + }) + + it('attaches the token in secure mode', () => { + const token = makeToken({}) + extractTokenMock.mockReturnValue(token) + const req = makeRequest(workspaceA, 'blob') + const next = jest.fn() as unknown as NextFunction + + withOptionalAuth(true)(req, res, next) + + expect(req.token).toBe(token) + expect(next).toHaveBeenCalledWith() + }) + + it('attaches the token in insecure mode without requiring it', () => { + extractTokenMock.mockReturnValue(undefined) + const req = makeRequest(workspaceA, 'blob') + const next = jest.fn() as unknown as NextFunction + + withOptionalAuth(false)(req, res, next) + + expect(req.token).toBeUndefined() + expect(next).toHaveBeenCalledWith() + }) +}) + +describe('withBlob', () => { + it('rejects a missing or malformed workspace', () => { + for (const workspace of ['', 'not-a-uuid', '../../etc']) { + const next = jest.fn() as unknown as NextFunction + withBlob(makeRequest(workspace, 'blob'), res, next) + expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 400 })) + } + }) + + it('rejects a missing blob name', () => { + const next = jest.fn() as unknown as NextFunction + withBlob(makeRequest(workspaceA, ''), res, next) + expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 400 })) + }) + + it('rejects a token scoped to another workspace', () => { + const next = jest.fn() as unknown as NextFunction + + withBlob(makeRequest(workspaceB, 'blob', makeToken({ workspace: workspaceA })), res, next) + + expect(next).toHaveBeenCalledWith(expect.any(HttpError)) + expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 401 })) + }) + + it('allows a token scoped to the requested workspace', () => { + const next = jest.fn() as unknown as NextFunction + + withBlob(makeRequest(workspaceA, 'blob', makeToken({ workspace: workspaceA })), res, next) + + expect(next).toHaveBeenCalledWith() + }) + + it('allows the system account and admins to access any workspace', () => { + for (const token of [ + makeToken({ account: systemAccountUuid, workspace: workspaceA }), + makeToken({ workspace: workspaceA, extra: { admin: 'true' } }) + ]) { + const next = jest.fn() as unknown as NextFunction + withBlob(makeRequest(workspaceB, 'blob', token), res, next) + expect(next).toHaveBeenCalledWith() + } + }) + + it('allows any workspace when no token is attached', () => { + const next = jest.fn() as unknown as NextFunction + + withBlob(makeRequest(workspaceB, 'blob'), res, next) + + expect(next).toHaveBeenCalledWith() + }) +}) diff --git a/pods/preview/src/config.ts b/pods/preview/src/config.ts index b79cdc62c6e..2e2dd78d80e 100644 --- a/pods/preview/src/config.ts +++ b/pods/preview/src/config.ts @@ -27,6 +27,7 @@ export interface Config { Port: number Secret: string ServiceID: string + Secure: boolean Cache: CacheConfig } @@ -35,6 +36,7 @@ const config: Config = (() => { Port: parseInt(process.env.PORT ?? '4040'), Secret: process.env.SECRET, ServiceID: process.env.SERVICE_ID ?? 'preview', + Secure: process.env.SECURE === 'true', Cache: { enabled: process.env.CACHE_ENABLED !== 'false', cachePath: process.env.CACHE_PATH, diff --git a/pods/preview/src/middleware.ts b/pods/preview/src/middleware.ts index 0d8b0e01870..470358af14d 100644 --- a/pods/preview/src/middleware.ts +++ b/pods/preview/src/middleware.ts @@ -66,6 +66,55 @@ export const withAuthorization = (req: RequestWithAuth, res: Response, next: Nex } } +/** + * Attaches the token when present. In secure mode a valid token is required. + */ +export const withOptionalAuth = (secure: boolean): RequestHandler => { + return secure + ? withAuthorization + : (req: RequestWithAuth, res: Response, next: NextFunction) => { + req.token = extractToken(req.headers) + next() + } +} + +const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + +const isUuid = (value: string): boolean => uuidRegex.test(value) + +/** + * Validates blob route params and ensures the caller's token grants access to + * the workspace taken from the URL. Mirrors the Datalake `withBlob` middleware. + */ +export const withBlob = (req: RequestWithAuth, res: Response, next: NextFunction): void => { + try { + const workspace = req.params.workspace + const name = req.params.name + + if (workspace === undefined || workspace === '' || !isUuid(workspace)) { + throw new HttpError(400, 'Missing workspace') + } + if (name === undefined || name === '') { + throw new HttpError(400, 'Missing blob name') + } + + // If authorization is not enforced allow any workspace + if (req.token != null) { + const hasWorkspaceAccess = + (req.token.workspace as string) === workspace || + req.token.account === systemAccountUuid || + req.token.extra?.admin === 'true' + if (!hasWorkspaceAccess) { + throw new HttpError(401, 'Unauthorized') + } + } + + next() + } catch (err: any) { + next(err) + } +} + export interface ErrorHandlerOptions { ctx: MeasureContext } diff --git a/pods/preview/src/server.ts b/pods/preview/src/server.ts index 18026a42f1f..3434dc8c98c 100644 --- a/pods/preview/src/server.ts +++ b/pods/preview/src/server.ts @@ -29,7 +29,7 @@ import { pipeline } from 'stream/promises' import { createCache } from './cache' import { type Config } from './config' -import { type RequestWithAuth, errorHandler, keepAlive } from './middleware' +import { type RequestWithAuth, errorHandler, keepAlive, withBlob, withOptionalAuth } from './middleware' import { createPreviewService, ThumbnailParams } from './service' import { TemporaryDir } from './tempdir' @@ -174,6 +174,8 @@ export async function createServer (ctx: MeasureContext, config: Config): Promis app.get( '/metadata/:workspace/:name', + withOptionalAuth(config.Secure), + withBlob, wrapRequest(ctx, 'getMetadata', async (ctx, req, res) => { const workspace = req.params.workspace as WorkspaceUuid const name = req.params.name @@ -185,6 +187,8 @@ export async function createServer (ctx: MeasureContext, config: Config): Promis app.get( '/image/:transform/:workspace/:name', + withOptionalAuth(config.Secure), + withBlob, wrapRequest(ctx, 'getThumbnail', async (ctx, req, res) => { const workspace = req.params.workspace as WorkspaceUuid const name = req.params.name From ebeefa1ec90827a4ab767973f8dc44f051771d8c Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Fri, 10 Jul 2026 10:50:50 +0700 Subject: [PATCH 2/5] Update copyright year Signed-off-by: Artyom Savchenko --- pods/preview/src/__tests__/middleware.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pods/preview/src/__tests__/middleware.test.ts b/pods/preview/src/__tests__/middleware.test.ts index 8dccd30e956..3bd189df886 100644 --- a/pods/preview/src/__tests__/middleware.test.ts +++ b/pods/preview/src/__tests__/middleware.test.ts @@ -1,5 +1,5 @@ // -// Copyright © 2025 Hardcore Engineering Inc. +// Copyright © 2026 Hardcore Engineering Inc. // // Licensed under the Eclipse Public License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. You may From f3b09a66c32db81f33b7755896510bcdc9041410 Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Fri, 10 Jul 2026 17:28:34 +0700 Subject: [PATCH 3/5] Use secure mode Signed-off-by: Artyom Savchenko --- pods/preview/src/__tests__/middleware.test.ts | 33 ++++++----------- pods/preview/src/config.ts | 2 -- pods/preview/src/middleware.ts | 35 +++++++------------ pods/preview/src/server.ts | 6 ++-- 4 files changed, 27 insertions(+), 49 deletions(-) diff --git a/pods/preview/src/__tests__/middleware.test.ts b/pods/preview/src/__tests__/middleware.test.ts index 3bd189df886..81d5acf4ac6 100644 --- a/pods/preview/src/__tests__/middleware.test.ts +++ b/pods/preview/src/__tests__/middleware.test.ts @@ -19,7 +19,7 @@ import { type Token } from '@hcengineering/server-token' import { type NextFunction, type Response } from 'express' import { HttpError } from '../error' -import { type RequestWithAuth, withBlob, withOptionalAuth } from '../middleware' +import { type RequestWithAuth, withAuthorization, withBlob } from '../middleware' jest.mock('@hcengineering/server-client', () => ({ extractToken: jest.fn() @@ -42,53 +42,42 @@ function makeRequest (workspace: string, name: string, token?: Token): RequestWi const res = {} as unknown as Response -describe('withOptionalAuth', () => { +describe('withAuthorization', () => { beforeEach(() => { extractTokenMock.mockReset() }) - it('rejects requests without a token in secure mode', () => { + it('rejects requests without a token', () => { extractTokenMock.mockReturnValue(undefined) const next = jest.fn() as unknown as NextFunction - withOptionalAuth(true)(makeRequest(workspaceA, 'blob'), res, next) + withAuthorization(makeRequest(workspaceA, 'blob'), res, next) expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 401 })) }) - it('rejects guest and readonly tokens in secure mode', () => { + it('rejects guest and readonly tokens', () => { for (const extra of [{ guest: 'true' }, { readonly: 'true' }]) { extractTokenMock.mockReturnValue(makeToken({ extra })) const next = jest.fn() as unknown as NextFunction - withOptionalAuth(true)(makeRequest(workspaceA, 'blob'), res, next) + withAuthorization(makeRequest(workspaceA, 'blob'), res, next) expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 401 })) } }) - it('attaches the token in secure mode', () => { + it('attaches a valid token to the request', () => { const token = makeToken({}) extractTokenMock.mockReturnValue(token) const req = makeRequest(workspaceA, 'blob') const next = jest.fn() as unknown as NextFunction - withOptionalAuth(true)(req, res, next) + withAuthorization(req, res, next) expect(req.token).toBe(token) expect(next).toHaveBeenCalledWith() }) - - it('attaches the token in insecure mode without requiring it', () => { - extractTokenMock.mockReturnValue(undefined) - const req = makeRequest(workspaceA, 'blob') - const next = jest.fn() as unknown as NextFunction - - withOptionalAuth(false)(req, res, next) - - expect(req.token).toBeUndefined() - expect(next).toHaveBeenCalledWith() - }) }) describe('withBlob', () => { @@ -134,11 +123,11 @@ describe('withBlob', () => { } }) - it('allows any workspace when no token is attached', () => { + it('rejects requests with no token attached', () => { const next = jest.fn() as unknown as NextFunction - withBlob(makeRequest(workspaceB, 'blob'), res, next) + withBlob(makeRequest(workspaceA, 'blob'), res, next) - expect(next).toHaveBeenCalledWith() + expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 401 })) }) }) diff --git a/pods/preview/src/config.ts b/pods/preview/src/config.ts index 2e2dd78d80e..b79cdc62c6e 100644 --- a/pods/preview/src/config.ts +++ b/pods/preview/src/config.ts @@ -27,7 +27,6 @@ export interface Config { Port: number Secret: string ServiceID: string - Secure: boolean Cache: CacheConfig } @@ -36,7 +35,6 @@ const config: Config = (() => { Port: parseInt(process.env.PORT ?? '4040'), Secret: process.env.SECRET, ServiceID: process.env.SERVICE_ID ?? 'preview', - Secure: process.env.SECURE === 'true', Cache: { enabled: process.env.CACHE_ENABLED !== 'false', cachePath: process.env.CACHE_PATH, diff --git a/pods/preview/src/middleware.ts b/pods/preview/src/middleware.ts index 470358af14d..8d47e95b55d 100644 --- a/pods/preview/src/middleware.ts +++ b/pods/preview/src/middleware.ts @@ -66,25 +66,14 @@ export const withAuthorization = (req: RequestWithAuth, res: Response, next: Nex } } -/** - * Attaches the token when present. In secure mode a valid token is required. - */ -export const withOptionalAuth = (secure: boolean): RequestHandler => { - return secure - ? withAuthorization - : (req: RequestWithAuth, res: Response, next: NextFunction) => { - req.token = extractToken(req.headers) - next() - } -} - const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i const isUuid = (value: string): boolean => uuidRegex.test(value) /** * Validates blob route params and ensures the caller's token grants access to - * the workspace taken from the URL. Mirrors the Datalake `withBlob` middleware. + * the workspace taken from the URL. Must run after `withAuthorization`, which + * guarantees a token is present. */ export const withBlob = (req: RequestWithAuth, res: Response, next: NextFunction): void => { try { @@ -98,15 +87,17 @@ export const withBlob = (req: RequestWithAuth, res: Response, next: NextFunction throw new HttpError(400, 'Missing blob name') } - // If authorization is not enforced allow any workspace - if (req.token != null) { - const hasWorkspaceAccess = - (req.token.workspace as string) === workspace || - req.token.account === systemAccountUuid || - req.token.extra?.admin === 'true' - if (!hasWorkspaceAccess) { - throw new HttpError(401, 'Unauthorized') - } + const token = req.token + if (token == null) { + throw new HttpError(401, 'Unauthorized') + } + + const hasWorkspaceAccess = + (token.workspace as string) === workspace || + token.account === systemAccountUuid || + token.extra?.admin === 'true' + if (!hasWorkspaceAccess) { + throw new HttpError(401, 'Unauthorized') } next() diff --git a/pods/preview/src/server.ts b/pods/preview/src/server.ts index 3434dc8c98c..32fa7d67f0a 100644 --- a/pods/preview/src/server.ts +++ b/pods/preview/src/server.ts @@ -29,7 +29,7 @@ import { pipeline } from 'stream/promises' import { createCache } from './cache' import { type Config } from './config' -import { type RequestWithAuth, errorHandler, keepAlive, withBlob, withOptionalAuth } from './middleware' +import { type RequestWithAuth, errorHandler, keepAlive, withAuthorization, withBlob } from './middleware' import { createPreviewService, ThumbnailParams } from './service' import { TemporaryDir } from './tempdir' @@ -174,7 +174,7 @@ export async function createServer (ctx: MeasureContext, config: Config): Promis app.get( '/metadata/:workspace/:name', - withOptionalAuth(config.Secure), + withAuthorization, withBlob, wrapRequest(ctx, 'getMetadata', async (ctx, req, res) => { const workspace = req.params.workspace as WorkspaceUuid @@ -187,7 +187,7 @@ export async function createServer (ctx: MeasureContext, config: Config): Promis app.get( '/image/:transform/:workspace/:name', - withOptionalAuth(config.Secure), + withAuthorization, withBlob, wrapRequest(ctx, 'getThumbnail', async (ctx, req, res) => { const workspace = req.params.workspace as WorkspaceUuid From 85c07febdeab284d02aa81984d6b542b45c2c22a Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Fri, 10 Jul 2026 17:31:00 +0700 Subject: [PATCH 4/5] Fix formatting Signed-off-by: Artyom Savchenko --- pods/preview/src/middleware.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pods/preview/src/middleware.ts b/pods/preview/src/middleware.ts index 8d47e95b55d..d87a111be26 100644 --- a/pods/preview/src/middleware.ts +++ b/pods/preview/src/middleware.ts @@ -93,9 +93,7 @@ export const withBlob = (req: RequestWithAuth, res: Response, next: NextFunction } const hasWorkspaceAccess = - (token.workspace as string) === workspace || - token.account === systemAccountUuid || - token.extra?.admin === 'true' + (token.workspace as string) === workspace || token.account === systemAccountUuid || token.extra?.admin === 'true' if (!hasWorkspaceAccess) { throw new HttpError(401, 'Unauthorized') } From f94465e0acada62d027bfe3ffa3b71537e472664 Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Tue, 14 Jul 2026 17:42:05 +0700 Subject: [PATCH 5/5] Remove uuid check Signed-off-by: Artyom Savchenko --- pods/preview/src/__tests__/middleware.test.ts | 19 +++++++++++++------ pods/preview/src/middleware.ts | 6 +----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pods/preview/src/__tests__/middleware.test.ts b/pods/preview/src/__tests__/middleware.test.ts index 81d5acf4ac6..e8687da2f18 100644 --- a/pods/preview/src/__tests__/middleware.test.ts +++ b/pods/preview/src/__tests__/middleware.test.ts @@ -81,12 +81,19 @@ describe('withAuthorization', () => { }) describe('withBlob', () => { - it('rejects a missing or malformed workspace', () => { - for (const workspace of ['', 'not-a-uuid', '../../etc']) { - const next = jest.fn() as unknown as NextFunction - withBlob(makeRequest(workspace, 'blob'), res, next) - expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 400 })) - } + it('rejects a missing workspace', () => { + const next = jest.fn() as unknown as NextFunction + withBlob(makeRequest('', 'blob'), res, next) + expect(next).toHaveBeenCalledWith(expect.objectContaining({ code: 400 })) + }) + + it('accepts a non-uuid workspace id when the token matches it', () => { + const workspace = 'not-a-uuid' + const next = jest.fn() as unknown as NextFunction + + withBlob(makeRequest(workspace, 'blob', makeToken({ workspace: workspace as WorkspaceUuid })), res, next) + + expect(next).toHaveBeenCalledWith() }) it('rejects a missing blob name', () => { diff --git a/pods/preview/src/middleware.ts b/pods/preview/src/middleware.ts index d87a111be26..017a053a1c2 100644 --- a/pods/preview/src/middleware.ts +++ b/pods/preview/src/middleware.ts @@ -66,10 +66,6 @@ export const withAuthorization = (req: RequestWithAuth, res: Response, next: Nex } } -const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i - -const isUuid = (value: string): boolean => uuidRegex.test(value) - /** * Validates blob route params and ensures the caller's token grants access to * the workspace taken from the URL. Must run after `withAuthorization`, which @@ -80,7 +76,7 @@ export const withBlob = (req: RequestWithAuth, res: Response, next: NextFunction const workspace = req.params.workspace const name = req.params.name - if (workspace === undefined || workspace === '' || !isUuid(workspace)) { + if (workspace === undefined || workspace === '') { throw new HttpError(400, 'Missing workspace') } if (name === undefined || name === '') {