diff --git a/apps/backend/src/config/migrations.ts b/apps/backend/src/config/migrations.ts index c65ce84e..3b5c0eff 100644 --- a/apps/backend/src/config/migrations.ts +++ b/apps/backend/src/config/migrations.ts @@ -33,6 +33,7 @@ import { UpdateOrderEntity1769990652833 } from '../migrations/1769990652833-Upda import { DonationItemFoodTypeNotNull1771524930613 } from '../migrations/1771524930613-DonationItemFoodTypeNotNull'; import { MoveRequestFieldsToOrders1770571145350 } from '../migrations/1770571145350-MoveRequestFieldsToOrders'; import { RenameDonationMatchingStatus1771260403657 } from '../migrations/1771260403657-RenameDonationMatchingStatus'; +import { FixTrackingLinks1773041840374 } from '../migrations/1773041840374-FixTrackingLinks'; import { CleanupRequestsAndAllocations1771821377918 } from '../migrations/1771821377918-CleanupRequestsAndAllocations'; const schemaMigrations = [ @@ -71,6 +72,7 @@ const schemaMigrations = [ DonationItemFoodTypeNotNull1771524930613, MoveRequestFieldsToOrders1770571145350, RenameDonationMatchingStatus1771260403657, + FixTrackingLinks1773041840374, CleanupRequestsAndAllocations1771821377918, ]; diff --git a/apps/backend/src/foodRequests/request.service.spec.ts b/apps/backend/src/foodRequests/request.service.spec.ts index d404d62a..99432e50 100644 --- a/apps/backend/src/foodRequests/request.service.spec.ts +++ b/apps/backend/src/foodRequests/request.service.spec.ts @@ -120,7 +120,7 @@ describe('RequestsService', () => { orderId: 1, status: OrderStatus.DELIVERED, foodManufacturerName: 'FoodCorp Industries', - trackingLink: 'www.samplelink/samplelink', + trackingLink: 'https://www.samplelink.com/samplelink', items: expectedItems, }); }); diff --git a/apps/backend/src/migrations/1773041840374-FixTrackingLinks.ts b/apps/backend/src/migrations/1773041840374-FixTrackingLinks.ts new file mode 100644 index 00000000..8449da3e --- /dev/null +++ b/apps/backend/src/migrations/1773041840374-FixTrackingLinks.ts @@ -0,0 +1,19 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +export class FixTrackingLinks1773041840374 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(` + UPDATE orders + SET tracking_link = 'https://www.samplelink.com/samplelink' + WHERE tracking_link = 'www.samplelink/samplelink'; + `); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(` + UPDATE orders + SET tracking_link = 'www.samplelink/samplelink' + WHERE tracking_link = 'https://www.samplelink.com/samplelink'; + `); + } +} diff --git a/apps/backend/src/orders/dtos/tracking-cost.dto.ts b/apps/backend/src/orders/dtos/tracking-cost.dto.ts index 1c29ce6e..27327384 100644 --- a/apps/backend/src/orders/dtos/tracking-cost.dto.ts +++ b/apps/backend/src/orders/dtos/tracking-cost.dto.ts @@ -1,7 +1,12 @@ import { IsUrl, IsNumber, Min, IsOptional } from 'class-validator'; export class TrackingCostDto { - @IsUrl({}, { message: 'Tracking link must be a valid URL' }) + @IsUrl( + { + protocols: ['http', 'https'], + }, + { message: 'Tracking link must be a valid HTTP/HTTPS URL' }, + ) @IsOptional() trackingLink?: string; diff --git a/apps/backend/src/orders/order.service.spec.ts b/apps/backend/src/orders/order.service.spec.ts index b97f4827..78dc5a7b 100644 --- a/apps/backend/src/orders/order.service.spec.ts +++ b/apps/backend/src/orders/order.service.spec.ts @@ -156,7 +156,7 @@ describe('OrdersService', () => { orderId: 1, status: OrderStatus.DELIVERED, foodManufacturerName: 'FoodCorp Industries', - trackingLink: 'www.samplelink/samplelink', + trackingLink: 'https://www.samplelink.com/samplelink', items: [ { id: 1, @@ -358,7 +358,7 @@ describe('OrdersService', () => { describe('updateTrackingCostInfo', () => { it('throws when order is non-existent', async () => { const trackingCostDto: TrackingCostDto = { - trackingLink: 'test', + trackingLink: 'www.test.com', shippingCost: 5.99, }; @@ -375,7 +375,7 @@ describe('OrdersService', () => { ); }); - it('updates tracking link for shipped order', async () => { + it('sanitizes and updates tracking link for shipped order', async () => { const trackingCostDto: TrackingCostDto = { trackingLink: 'samplelink.com', }; @@ -384,7 +384,7 @@ describe('OrdersService', () => { const order = await service.findOne(3); expect(order.trackingLink).toBeDefined(); - expect(order.trackingLink).toEqual('samplelink.com'); + expect(order.trackingLink).toEqual('https://samplelink.com/'); }); it('updates shipping cost for shipped order', async () => { @@ -399,7 +399,7 @@ describe('OrdersService', () => { expect(order.shippingCost).toEqual('12.99'); }); - it('updates both shipping cost and tracking link', async () => { + it('updates both shipping cost and tracking link (sanitized)', async () => { const trackingCostDto: TrackingCostDto = { trackingLink: 'testtracking.com', shippingCost: 7.5, @@ -408,7 +408,7 @@ describe('OrdersService', () => { await service.updateTrackingCostInfo(3, trackingCostDto); const order = await service.findOne(3); - expect(order.trackingLink).toEqual('testtracking.com'); + expect(order.trackingLink).toEqual('https://testtracking.com/'); expect(order.shippingCost).toEqual('7.50'); }); @@ -452,6 +452,21 @@ describe('OrdersService', () => { ); }); + it('throws when tracking link is invalid', async () => { + const trackingCostDto: TrackingCostDto = { + trackingLink: `javascript:alert("you've been hacked!")`, + shippingCost: 7.5, + }; + + await expect( + service.updateTrackingCostInfo(3, trackingCostDto), + ).rejects.toThrow( + new BadRequestException( + 'Invalid tracking link. Only valid HTTP/HTTPS URLs are accepted.', + ), + ); + }); + it('sets status to shipped when both fields provided and previous status pending', async () => { const trackingCostDto: TrackingCostDto = { trackingLink: 'testtracking.com', diff --git a/apps/backend/src/orders/order.service.ts b/apps/backend/src/orders/order.service.ts index 329576c7..8c381499 100644 --- a/apps/backend/src/orders/order.service.ts +++ b/apps/backend/src/orders/order.service.ts @@ -8,7 +8,7 @@ import { Repository, In } from 'typeorm'; import { Order } from './order.entity'; import { Pantry } from '../pantries/pantries.entity'; import { FoodManufacturer } from '../foodManufacturers/manufacturers.entity'; -import { validateId } from '../utils/validation.utils'; +import { sanitizeUrl, validateId } from '../utils/validation.utils'; import { OrderStatus } from './types'; import { TrackingCostDto } from './dtos/tracking-cost.dto'; import { OrderDetailsDto } from './dtos/order-details.dto'; @@ -283,6 +283,16 @@ export class OrdersService { ); } + if (dto.trackingLink) { + const sanitized = sanitizeUrl(dto.trackingLink); + if (!sanitized) { + throw new BadRequestException( + 'Invalid tracking link. Only valid HTTP/HTTPS URLs are accepted.', + ); + } + dto.trackingLink = sanitized; + } + const order = await this.repo.findOneBy({ orderId }); if (!order) { throw new NotFoundException(`Order ${orderId} not found`); diff --git a/apps/backend/src/utils/validation.utils.spec.ts b/apps/backend/src/utils/validation.utils.spec.ts index 01c1f790..2d2905d0 100644 --- a/apps/backend/src/utils/validation.utils.spec.ts +++ b/apps/backend/src/utils/validation.utils.spec.ts @@ -2,7 +2,7 @@ import { BadRequestException, InternalServerErrorException, } from '@nestjs/common'; -import { validateEnv, validateId } from './validation.utils'; +import { sanitizeUrl, validateEnv, validateId } from './validation.utils'; describe('validateId', () => { it('should not throw an error for a valid ID', () => { @@ -39,3 +39,38 @@ describe('validateEnv', () => { ); }); }); + +describe('sanitizeUrl', () => { + it('should return null for malicious protocols', () => { + const maliciousProtocols = ['javascript:', 'data:', 'file:', 'vbscript:']; + + for (const protocol of maliciousProtocols) { + expect(sanitizeUrl(protocol + 'test')).toBeNull(); + } + }); + + it('should return null for empty or invalid URLs', () => { + expect(sanitizeUrl('')).toBeNull(); + expect(sanitizeUrl('https://')).toBeNull(); + expect(sanitizeUrl('https://foo')).toBeNull(); + }); + + it('should accept valid http/https URLs', () => { + const validHttpUrl = 'http://www.tracking.com/test'; + const validHttpsUrl = 'https://www.tracking.com/test'; + expect(sanitizeUrl(validHttpUrl)).toBe(validHttpUrl); + expect(sanitizeUrl(validHttpsUrl)).toBe(validHttpsUrl); + }); + + it('adds https:// to URL without protocol', () => { + expect(sanitizeUrl('www.tracking.com/test')).toBe( + 'https://www.tracking.com/test', + ); + }); + + it('trims whitespace from URL', () => { + expect(sanitizeUrl(' https://www.tracking.com/test ')).toBe( + 'https://www.tracking.com/test', + ); + }); +}); diff --git a/apps/backend/src/utils/validation.utils.ts b/apps/backend/src/utils/validation.utils.ts index be7f141a..700f757e 100644 --- a/apps/backend/src/utils/validation.utils.ts +++ b/apps/backend/src/utils/validation.utils.ts @@ -18,3 +18,24 @@ export function validateEnv(name: string): string { return v; } + +export function sanitizeUrl(url: string): string | null { + try { + const trimmed = url.trim(); + if (!trimmed) return null; + + let fullUrl = trimmed; + if (!/^https?:\/\//i.test(trimmed)) { + fullUrl = 'https://' + trimmed; + } + + const urlObj = new URL(fullUrl); + + if (urlObj.protocol !== 'http:' && urlObj.protocol !== 'https:') + return null; + if (!urlObj.hostname || !urlObj.hostname.includes('.')) return null; + return urlObj.href; + } catch { + return null; + } +}