From bc3dbdea423538c5a0aa91ff2a4b73814e519750 Mon Sep 17 00:00:00 2001 From: sahilkhan09k Date: Wed, 22 Apr 2026 21:25:10 +0530 Subject: [PATCH] fix: improve environment validation in published docs (#5962) --- packages/hoppscotch-backend/src/errors.ts | 10 ++- .../published-docs.service.spec.ts | 30 ++++---- .../published-docs/published-docs.service.ts | 69 ++++++++++++------- 3 files changed, 68 insertions(+), 41 deletions(-) diff --git a/packages/hoppscotch-backend/src/errors.ts b/packages/hoppscotch-backend/src/errors.ts index c3a64aff..56bcbfbe 100644 --- a/packages/hoppscotch-backend/src/errors.ts +++ b/packages/hoppscotch-backend/src/errors.ts @@ -462,6 +462,12 @@ export const USER_ENVIRONMENT_IS_NOT_GLOBAL = export const USER_ENVIRONMENT_UPDATE_FAILED = 'user_environment/user_env_update_failed' as const; +/** + * User environment not found for the user + * (UserEnvironmentsService) + */ +export const USER_ENVIRONMENT_NOT_FOUND = 'user_environment/not_found' as const; + /** * User environment invalid environment name * (UserEnvironmentsService) @@ -977,8 +983,8 @@ export const PUBLISHED_DOCS_DELETION_FAILED = 'published_docs/deletion_failed'; * Published Docs invalid environment * (PublishedDocsService) */ -export const PUBLISHED_DOCS_INVALID_ENVIRONMENT = - 'published_docs/invalid_environment'; +export const PUBLISHED_DOCS_FORBIDDEN_ENVIRONMENT_ACCESS = + 'published_docs/forbidden_environment_access'; /** * Published Docs not found diff --git a/packages/hoppscotch-backend/src/published-docs/published-docs.service.spec.ts b/packages/hoppscotch-backend/src/published-docs/published-docs.service.spec.ts index 5d7769f4..c3b3aa0b 100644 --- a/packages/hoppscotch-backend/src/published-docs/published-docs.service.spec.ts +++ b/packages/hoppscotch-backend/src/published-docs/published-docs.service.spec.ts @@ -4,10 +4,12 @@ import { PUBLISHED_DOCS_CREATION_FAILED, PUBLISHED_DOCS_DELETION_FAILED, PUBLISHED_DOCS_INVALID_COLLECTION, - PUBLISHED_DOCS_INVALID_ENVIRONMENT, + PUBLISHED_DOCS_FORBIDDEN_ENVIRONMENT_ACCESS, PUBLISHED_DOCS_NOT_FOUND, PUBLISHED_DOCS_UPDATE_FAILED, + TEAM_ENVIRONMENT_NOT_FOUND, TEAM_INVALID_ID, + USER_ENVIRONMENT_NOT_FOUND, } from 'src/errors'; import * as E from 'fp-ts/Either'; import { PrismaService } from 'src/prisma/prisma.service'; @@ -1491,7 +1493,7 @@ describe('createPublishedDoc - environment support', () => { userUid: user.uid, } as any); mockPrisma.publishedDocs.findFirst.mockResolvedValueOnce(null); - mockPrisma.userEnvironment.findFirst.mockResolvedValueOnce(envData as any); + mockPrisma.userEnvironment.findUnique.mockResolvedValueOnce(envData as any); mockPrisma.publishedDocs.create.mockResolvedValueOnce({ ...userPublishedDoc, environmentID: 'env_1', @@ -1537,7 +1539,7 @@ describe('createPublishedDoc - environment support', () => { teamID: 'team_1', } as any); mockPrisma.publishedDocs.findFirst.mockResolvedValueOnce(null); - mockPrisma.teamEnvironment.findFirst.mockResolvedValueOnce(envData as any); + mockPrisma.teamEnvironment.findUnique.mockResolvedValueOnce(envData as any); mockPrisma.publishedDocs.create.mockResolvedValueOnce({ ...teamPublishedDoc, environmentID: 'team_env_1', @@ -1568,14 +1570,14 @@ describe('createPublishedDoc - environment support', () => { userUid: user.uid, } as any); mockPrisma.publishedDocs.findFirst.mockResolvedValueOnce(null); - mockPrisma.userEnvironment.findFirst.mockResolvedValueOnce(null); + mockPrisma.userEnvironment.findUnique.mockResolvedValueOnce(null); const result = await publishedDocsService.createPublishedDoc( { ...createArgs, environmentID: 'invalid_env' }, user, ); - expect(result).toEqualLeft(PUBLISHED_DOCS_INVALID_ENVIRONMENT); + expect(result).toEqualLeft(USER_ENVIRONMENT_NOT_FOUND); }); test('should return error when team environment ID is invalid', async () => { @@ -1593,14 +1595,14 @@ describe('createPublishedDoc - environment support', () => { teamID: 'team_1', } as any); mockPrisma.publishedDocs.findFirst.mockResolvedValueOnce(null); - mockPrisma.teamEnvironment.findFirst.mockResolvedValueOnce(null); + mockPrisma.teamEnvironment.findUnique.mockResolvedValueOnce(null); const result = await publishedDocsService.createPublishedDoc( teamArgs, user, ); - expect(result).toEqualLeft(PUBLISHED_DOCS_INVALID_ENVIRONMENT); + expect(result).toEqualLeft(TEAM_ENVIRONMENT_NOT_FOUND); }); test('should create published doc without environment when environmentID is not provided', async () => { @@ -1640,7 +1642,7 @@ describe('updatePublishedDoc - environment support', () => { }; mockPrisma.publishedDocs.findUnique.mockResolvedValueOnce(userPublishedDoc); - mockPrisma.userEnvironment.findFirst.mockResolvedValueOnce(envData as any); + mockPrisma.userEnvironment.findUnique.mockResolvedValueOnce(envData as any); mockPrisma.publishedDocs.update.mockResolvedValueOnce({ ...userPublishedDoc, environmentID: 'env_2', @@ -1701,7 +1703,7 @@ describe('updatePublishedDoc - environment support', () => { test('should return error when updating with invalid environment ID', async () => { mockPrisma.publishedDocs.findUnique.mockResolvedValueOnce(userPublishedDoc); - mockPrisma.userEnvironment.findFirst.mockResolvedValueOnce(null); + mockPrisma.userEnvironment.findUnique.mockResolvedValueOnce(null); const result = await publishedDocsService.updatePublishedDoc( userPublishedDoc.id, @@ -1709,7 +1711,7 @@ describe('updatePublishedDoc - environment support', () => { user, ); - expect(result).toEqualLeft(PUBLISHED_DOCS_INVALID_ENVIRONMENT); + expect(result).toEqualLeft(USER_ENVIRONMENT_NOT_FOUND); }); test('should not change environment when environmentID is not provided in update args', async () => { @@ -1743,7 +1745,7 @@ describe('updatePublishedDoc - environment support', () => { mockPrisma.publishedDocs.findUnique.mockResolvedValueOnce(teamPublishedDoc); mockPrisma.team.findFirst.mockResolvedValueOnce({ id: 'team_1' } as any); - mockPrisma.teamEnvironment.findFirst.mockResolvedValueOnce(envData as any); + mockPrisma.teamEnvironment.findUnique.mockResolvedValueOnce(envData as any); mockPrisma.publishedDocs.update.mockResolvedValueOnce({ ...teamPublishedDoc, environmentID: 'team_env_1', @@ -1796,7 +1798,7 @@ describe('getPublishedDocBySlugPublic - environment support', () => { mockUserCollectionService.exportUserCollectionToJSONObject.mockResolvedValueOnce( E.right(collectionData as any), ); - mockPrisma.userEnvironment.findFirst.mockResolvedValueOnce(envData as any); + mockPrisma.userEnvironment.findUnique.mockResolvedValueOnce(envData as any); const result = await publishedDocsService.getPublishedDocBySlugPublic( 'slug-collection-1', @@ -1894,14 +1896,14 @@ describe('getPublishedDocBySlugPublic - environment support', () => { E.right(collectionData as any), ); // Environment not found — fetchEnvironment returns Left - mockPrisma.userEnvironment.findFirst.mockResolvedValueOnce(null); + mockPrisma.userEnvironment.findUnique.mockResolvedValueOnce(null); const result = await publishedDocsService.getPublishedDocBySlugPublic( 'slug-collection-1', '1.0.0', ); - expect(result).toEqualLeft(PUBLISHED_DOCS_INVALID_ENVIRONMENT); + expect(result).toEqualLeft(USER_ENVIRONMENT_NOT_FOUND); }); test('should return null environment fields when no environment is associated', async () => { diff --git a/packages/hoppscotch-backend/src/published-docs/published-docs.service.ts b/packages/hoppscotch-backend/src/published-docs/published-docs.service.ts index 4b090c4d..d84e2399 100644 --- a/packages/hoppscotch-backend/src/published-docs/published-docs.service.ts +++ b/packages/hoppscotch-backend/src/published-docs/published-docs.service.ts @@ -13,12 +13,14 @@ import { PUBLISHED_DOCS_CREATION_FAILED, PUBLISHED_DOCS_DELETION_FAILED, PUBLISHED_DOCS_INVALID_COLLECTION, - PUBLISHED_DOCS_INVALID_ENVIRONMENT, + PUBLISHED_DOCS_FORBIDDEN_ENVIRONMENT_ACCESS, PUBLISHED_DOCS_NOT_FOUND, PUBLISHED_DOCS_UPDATE_FAILED, + TEAM_ENVIRONMENT_NOT_FOUND, TEAM_INVALID_COLL_ID, TEAM_INVALID_ID, USER_COLL_NOT_FOUND, + USER_ENVIRONMENT_NOT_FOUND, } from 'src/errors'; import * as E from 'fp-ts/Either'; import { PublishedDocs, PublishedDocsVersion } from './published-docs.model'; @@ -100,22 +102,43 @@ export class PublishedDocsService { environmentID: string, workspaceType: WorkspaceType, workspaceID: string, - ): Promise> { + ): Promise> { + // TEAM workspace environment if (workspaceType === WorkspaceType.TEAM) { - const env = await this.prisma.teamEnvironment.findFirst({ - where: { id: environmentID, teamID: workspaceID }, + const env = await this.prisma.teamEnvironment.findUnique({ + where: { id: environmentID }, }); - if (!env) return E.left(PUBLISHED_DOCS_INVALID_ENVIRONMENT); - return E.right({ name: env.name, variables: env.variables }); - } else if (workspaceType === WorkspaceType.USER) { - const env = await this.prisma.userEnvironment.findFirst({ - where: { id: environmentID, userUid: workspaceID }, + if (!env) return E.left(TEAM_ENVIRONMENT_NOT_FOUND); + + // Validate environment exists and belongs to the correct team + if (env.teamID !== workspaceID) + return E.left(PUBLISHED_DOCS_FORBIDDEN_ENVIRONMENT_ACCESS); + + return E.right({ + name: env.name, + variables: env.variables, }); - if (!env) return E.left(PUBLISHED_DOCS_INVALID_ENVIRONMENT); - return E.right({ name: env.name ?? '', variables: env.variables }); } - return E.left(PUBLISHED_DOCS_INVALID_ENVIRONMENT); + // USER workspace environment + if (workspaceType === WorkspaceType.USER) { + const env = await this.prisma.userEnvironment.findUnique({ + where: { id: environmentID }, + }); + if (!env) return E.left(USER_ENVIRONMENT_NOT_FOUND); + + // Validate environment exists and belongs to the correct user + if (env.userUid !== workspaceID) { + return E.left(PUBLISHED_DOCS_FORBIDDEN_ENVIRONMENT_ACCESS); + } + + return E.right({ + name: env.name ?? '', + variables: env.variables, + }); + } + + return E.left(PUBLISHED_DOCS_FORBIDDEN_ENVIRONMENT_ACCESS); } /** @@ -368,10 +391,8 @@ export class PublishedDocsService { ); if (E.isLeft(envResult)) return E.left(envResult.left); - if (E.isRight(envResult) && envResult.right) { - environmentName = envResult.right.name; - environmentVariables = envResult.right.variables; - } + environmentName = envResult.right.name; + environmentVariables = envResult.right.variables; } docToReturn = { @@ -577,10 +598,9 @@ export class PublishedDocsService { workspaceID, ); if (E.isLeft(envResult)) return E.left(envResult.left); - if (envResult.right) { - environmentName = envResult.right.name; - environmentVariables = envResult.right.variables; - } + + environmentName = envResult.right.name; + environmentVariables = envResult.right.variables; } // Attempt to create the published document @@ -697,11 +717,10 @@ export class PublishedDocsService { publishedDocs.workspaceID, ); if (E.isLeft(envResult)) return E.left(envResult.left); - if (envResult.right) { - environmentID = args.environmentID; - environmentName = envResult.right.name; - environmentVariables = envResult.right.variables; - } + + environmentID = args.environmentID; + environmentName = envResult.right.name; + environmentVariables = envResult.right.variables; } }