From 3a9a0932a3021e4b340ad34e94aa8e09d3a588db Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 18 Aug 2026 17:15:39 +0000 Subject: [PATCH] :bug: Validate plugin UI URLs cannot target Penpot's own domain The plugin UI iframe combines allow-scripts and allow-same-origin in its sandbox. While necessary for plugins to use their own cookies and storage, this creates a risk if a plugin's UI URL resolves to Penpot's own origin, potentially allowing the iframe to escape sandbox isolation. Add validateUIUrl() that checks the resolved URL against Penpot's origin (from penpotPublicURI or location.origin) and throws if they match. Called in openModal() after prepareUrl() resolves the URL. Closes #11271 AI-assisted-by: qwen3.7-plus --- .../src/lib/plugin-manager.spec.ts | 39 +++++++++++ .../plugins-runtime/src/lib/plugin-manager.ts | 2 + .../src/lib/validate-url.spec.ts | 70 +++++++++++++++++++ .../plugins-runtime/src/lib/validate-url.ts | 22 ++++++ 4 files changed, 133 insertions(+) create mode 100644 plugins/libs/plugins-runtime/src/lib/validate-url.spec.ts create mode 100644 plugins/libs/plugins-runtime/src/lib/validate-url.ts diff --git a/plugins/libs/plugins-runtime/src/lib/plugin-manager.spec.ts b/plugins/libs/plugins-runtime/src/lib/plugin-manager.spec.ts index d53f4f5296..d8131f4224 100644 --- a/plugins/libs/plugins-runtime/src/lib/plugin-manager.spec.ts +++ b/plugins/libs/plugins-runtime/src/lib/plugin-manager.spec.ts @@ -3,6 +3,7 @@ import { createPluginManager } from './plugin-manager'; import { loadManifestCode, getValidUrl, prepareUrl } from './parse-manifest.js'; import { PluginModalElement } from './modal/plugin-modal.js'; import { openUIApi } from './api/openUI.api.js'; +import { validateUIUrl } from './validate-url.js'; import type { Context, Theme } from '@penpot/plugin-types'; import type { Manifest } from './models/manifest.model.js'; @@ -16,6 +17,10 @@ vi.mock('./api/openUI.api.js', () => ({ openUIApi: vi.fn(), })); +vi.mock('./validate-url.js', () => ({ + validateUIUrl: vi.fn(), +})); + describe('createPluginManager', () => { let mockContext: Context; let manifest: Manifest; @@ -294,4 +299,38 @@ describe('createPluginManager', () => { expect(mockContext.removeListener).toHaveBeenCalled(); expect(onCloseCallback).toHaveBeenCalled(); }); + + it('should validate the modal URL before opening', async () => { + const pluginManager = await createPluginManager( + mockContext, + manifest, + onCloseCallback, + onReloadModal, + ); + + pluginManager.openModal('Test Modal', '/test-url'); + + expect(validateUIUrl).toHaveBeenCalledWith( + 'https://example.com/plugin', + ); + }); + + it('should throw when URL validation fails', async () => { + vi.mocked(validateUIUrl).mockImplementation(() => { + throw new Error("Plugin UI URL must not point to Penpot's own domain"); + }); + + const pluginManager = await createPluginManager( + mockContext, + manifest, + onCloseCallback, + onReloadModal, + ); + + expect(() => pluginManager.openModal('Test Modal', '/test-url')).toThrow( + "Plugin UI URL must not point to Penpot's own domain", + ); + + expect(openUIApi).not.toHaveBeenCalled(); + }); }); diff --git a/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts b/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts index 8b811f55eb..5854e83c04 100644 --- a/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts +++ b/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts @@ -7,6 +7,7 @@ import { openUIApi } from './api/openUI.api.js'; import { OpenUIOptions } from './models/open-ui-options.model.js'; import { RegisterListener } from './models/plugin.model.js'; import { openUISchema } from './models/open-ui-options.schema.js'; +import { validateUIUrl } from './validate-url.js'; export async function createPluginManager( context: Context, @@ -94,6 +95,7 @@ export async function createPluginManager( const openModal = (name: string, url: string, options?: OpenUIOptions) => { const theme = context.theme as Theme; const modalUrl = prepareUrl(manifest, url, { theme }); + validateUIUrl(modalUrl); if (modal?.getAttribute('iframe-src') === modalUrl) { return; diff --git a/plugins/libs/plugins-runtime/src/lib/validate-url.spec.ts b/plugins/libs/plugins-runtime/src/lib/validate-url.spec.ts new file mode 100644 index 0000000000..9409d70166 --- /dev/null +++ b/plugins/libs/plugins-runtime/src/lib/validate-url.spec.ts @@ -0,0 +1,70 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { getPenpotOrigin, validateUIUrl } from './validate-url.js'; + +describe('validate-url', () => { + const originalLocation = globalThis.location; + const originalPenpotPublicURI = (globalThis as any).penpotPublicURI; + + beforeEach(() => { + delete (globalThis as any).penpotPublicURI; + }); + + afterEach(() => { + if (originalPenpotPublicURI !== undefined) { + (globalThis as any).penpotPublicURI = originalPenpotPublicURI; + } else { + delete (globalThis as any).penpotPublicURI; + } + }); + + describe('getPenpotOrigin', () => { + it('should return location.origin when penpotPublicURI is not set', () => { + expect(getPenpotOrigin()).toBe(originalLocation.origin); + }); + + it('should return origin from penpotPublicURI when set', () => { + (globalThis as any).penpotPublicURI = 'https://design.penpot.com/'; + expect(getPenpotOrigin()).toBe('https://design.penpot.com'); + }); + + it('should fall back to location.origin when penpotPublicURI is invalid', () => { + (globalThis as any).penpotPublicURI = 'not-a-valid-url'; + expect(getPenpotOrigin()).toBe(originalLocation.origin); + }); + }); + + describe('validateUIUrl', () => { + it('should throw when URL has same origin as location.origin', () => { + const penpotOrigin = originalLocation.origin; + expect(() => validateUIUrl(`${penpotOrigin}/some/path`)).toThrow( + "Plugin UI URL must not point to Penpot's own domain", + ); + }); + + it('should not throw when URL has different origin', () => { + expect(() => + validateUIUrl('https://example.com/plugin-ui'), + ).not.toThrow(); + }); + + it('should throw when URL matches penpotPublicURI origin', () => { + (globalThis as any).penpotPublicURI = 'https://design.penpot.com/'; + expect(() => + validateUIUrl('https://design.penpot.com/some/path'), + ).toThrow("Plugin UI URL must not point to Penpot's own domain"); + }); + + it('should not throw when URL has same hostname but different port', () => { + const url = new URL(originalLocation.origin); + const differentPort = `${url.protocol}//${url.hostname}:9999`; + expect(() => validateUIUrl(`${differentPort}/path`)).not.toThrow(); + }); + + it('should throw even when URL has different path on same origin', () => { + const penpotOrigin = originalLocation.origin; + expect(() => + validateUIUrl(`${penpotOrigin}/deeply/nested/path`), + ).toThrow(); + }); + }); +}); diff --git a/plugins/libs/plugins-runtime/src/lib/validate-url.ts b/plugins/libs/plugins-runtime/src/lib/validate-url.ts new file mode 100644 index 0000000000..012adef517 --- /dev/null +++ b/plugins/libs/plugins-runtime/src/lib/validate-url.ts @@ -0,0 +1,22 @@ +export function getPenpotOrigin(): string { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const publicUri = (globalThis as any).penpotPublicURI; + if (publicUri) { + try { + return new URL(publicUri).origin; + } catch { + // fall through to location.origin + } + } + return globalThis.location.origin; +} + +export function validateUIUrl(url: string): void { + const penpotOrigin = getPenpotOrigin(); + const parsed = new URL(url); + if (parsed.origin === penpotOrigin) { + throw new Error( + `Plugin UI URL must not point to Penpot's own domain: ${url}`, + ); + } +}