From 84c59f45dae74a6193424f23902ac181334e1670 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Thu, 18 Dec 2025 06:30:58 -0800 Subject: [PATCH] Chore: harden SafeUrlPipe --- src-ui/src/app/pipes/safeurl.pipe.spec.ts | 39 +++++++++++++++++++---- src-ui/src/app/pipes/safeurl.pipe.ts | 21 +++++++++--- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src-ui/src/app/pipes/safeurl.pipe.spec.ts b/src-ui/src/app/pipes/safeurl.pipe.spec.ts index b1fc16885..90cca3878 100644 --- a/src-ui/src/app/pipes/safeurl.pipe.spec.ts +++ b/src-ui/src/app/pipes/safeurl.pipe.spec.ts @@ -13,20 +13,45 @@ describe('SafeUrlPipe', () => { pipe = TestBed.inject(SafeUrlPipe) }) - it('should bypass security and trust the url', () => { - const url = 'https://example.com' + it('should trust only same-origin http/https urls', () => { + const origin = window.location.origin + const url = `${origin}/some/path` const domSanitizer = TestBed.inject(DomSanitizer) const sanitizerSpy = jest.spyOn( domSanitizer, 'bypassSecurityTrustResourceUrl' ) - let safeResourceUrl = pipe.transform(url) + const safeResourceUrl = pipe.transform(url) expect(safeResourceUrl).not.toBeNull() - expect(sanitizerSpy).toHaveBeenCalled() + expect(sanitizerSpy).toHaveBeenCalledWith(url) + }) - safeResourceUrl = pipe.transform(null) - expect(safeResourceUrl).not.toBeNull() - expect(sanitizerSpy).toHaveBeenCalled() + it('should return null for null or unsafe urls', () => { + const sanitizerSpy = jest.spyOn( + TestBed.inject(DomSanitizer), + 'bypassSecurityTrustResourceUrl' + ) + + expect(pipe.transform(null)).toBeTruthy() + expect(sanitizerSpy).toHaveBeenCalledWith('') + expect(pipe.transform('javascript:alert(1)')).toBeTruthy() + expect(sanitizerSpy).toHaveBeenCalledWith('') + const otherOrigin = + window.location.origin === 'https://example.com' + ? 'https://evil.com' + : 'https://example.com' + expect(pipe.transform(`${otherOrigin}/file`)).toBeTruthy() + expect(sanitizerSpy).toHaveBeenCalledWith('') + }) + + it('should return null for malformed urls', () => { + const sanitizerSpy = jest.spyOn( + TestBed.inject(DomSanitizer), + 'bypassSecurityTrustResourceUrl' + ) + + expect(pipe.transform('http://[invalid-url')).toBeTruthy() + expect(sanitizerSpy).toHaveBeenCalledWith('') }) }) diff --git a/src-ui/src/app/pipes/safeurl.pipe.ts b/src-ui/src/app/pipes/safeurl.pipe.ts index 37fdd743b..7a0c65a2c 100644 --- a/src-ui/src/app/pipes/safeurl.pipe.ts +++ b/src-ui/src/app/pipes/safeurl.pipe.ts @@ -1,5 +1,6 @@ import { Pipe, PipeTransform, inject } from '@angular/core' import { DomSanitizer } from '@angular/platform-browser' +import { environment } from 'src/environments/environment' @Pipe({ name: 'safeUrl', @@ -7,11 +8,23 @@ import { DomSanitizer } from '@angular/platform-browser' export class SafeUrlPipe implements PipeTransform { private sanitizer = inject(DomSanitizer) - transform(url) { - if (url == null) { + transform(url: string | null) { + if (!url) return this.sanitizer.bypassSecurityTrustResourceUrl('') + try { + const parsed = new URL(url, window.location.origin) + const allowedOrigins = new Set([ + window.location.origin, + new URL(environment.apiBaseUrl).origin, + ]) + const isHttp = ['http:', 'https:'].includes(parsed.protocol) + const originAllowed = allowedOrigins.has(parsed.origin) + + if (!isHttp || !originAllowed) { + return this.sanitizer.bypassSecurityTrustResourceUrl('') + } + return this.sanitizer.bypassSecurityTrustResourceUrl(parsed.toString()) + } catch { return this.sanitizer.bypassSecurityTrustResourceUrl('') - } else { - return this.sanitizer.bypassSecurityTrustResourceUrl(url) } } }