mirror of
https://github.com/penpot/penpot.git
synced 2026-09-10 14:09:17 +00:00
🐛 Validate plugin UI URLs cannot target Penpot's own domain (#11273)
* 🐛 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 * 💄 Fix prettier formatting in plugin-manager.spec.ts Apply prettier formatting to fix format:check failure. AI-assisted-by: qwen3.7-plus * 🐛 Fix problem with penpot origin plugins --------- Co-authored-by: alonso.torres <alonso.torres@kaleidos.net>
This commit is contained in:
parent
ff63668c1e
commit
5f1e151e84
@ -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,39 @@ 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',
|
||||
manifest.host,
|
||||
);
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@ -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, manifest.host);
|
||||
|
||||
if (modal?.getAttribute('iframe-src') === modalUrl) {
|
||||
return;
|
||||
|
||||
117
plugins/libs/plugins-runtime/src/lib/validate-url.spec.ts
Normal file
117
plugins/libs/plugins-runtime/src/lib/validate-url.spec.ts
Normal file
@ -0,0 +1,117 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import {
|
||||
getPenpotOrigin,
|
||||
isPenpotOrigin,
|
||||
validateUIUrl,
|
||||
} from './validate-url.js';
|
||||
|
||||
describe('validate-url', () => {
|
||||
const originalLocation = globalThis.location;
|
||||
const originalPenpotPublicURI = (globalThis as any).penpotPublicURI;
|
||||
const externalHost = 'https://example.com';
|
||||
|
||||
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('isPenpotOrigin', () => {
|
||||
it('should be true for a URL on Penpot origin', () => {
|
||||
expect(
|
||||
isPenpotOrigin(`${originalLocation.origin}/plugin/manifest.json`),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should be false for a URL on another origin', () => {
|
||||
expect(isPenpotOrigin(`${externalHost}/manifest.json`)).toBe(false);
|
||||
});
|
||||
|
||||
it('should be false for an unparseable URL', () => {
|
||||
expect(isPenpotOrigin('not-a-valid-url')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateUIUrl', () => {
|
||||
it('should throw when URL has same origin as location.origin', () => {
|
||||
const penpotOrigin = originalLocation.origin;
|
||||
expect(() =>
|
||||
validateUIUrl(`${penpotOrigin}/some/path`, externalHost),
|
||||
).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', externalHost),
|
||||
).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', externalHost),
|
||||
).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`, externalHost),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should throw even when URL has different path on same origin', () => {
|
||||
const penpotOrigin = originalLocation.origin;
|
||||
expect(() =>
|
||||
validateUIUrl(`${penpotOrigin}/deeply/nested/path`, externalHost),
|
||||
).toThrow();
|
||||
});
|
||||
|
||||
it('should not throw when the manifest is served from Penpot origin', () => {
|
||||
const penpotOrigin = originalLocation.origin;
|
||||
expect(() =>
|
||||
validateUIUrl(`${penpotOrigin}/some/path`, `${penpotOrigin}/plugin`),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should not throw when the manifest is served from penpotPublicURI origin', () => {
|
||||
(globalThis as any).penpotPublicURI = 'https://design.penpot.com/';
|
||||
expect(() =>
|
||||
validateUIUrl(
|
||||
'https://design.penpot.com/some/path',
|
||||
'https://design.penpot.com/plugin',
|
||||
),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should still throw when the manifest host is unparseable', () => {
|
||||
const penpotOrigin = originalLocation.origin;
|
||||
expect(() => validateUIUrl(`${penpotOrigin}/path`, '')).toThrow(
|
||||
"Plugin UI URL must not point to Penpot's own domain",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
44
plugins/libs/plugins-runtime/src/lib/validate-url.ts
Normal file
44
plugins/libs/plugins-runtime/src/lib/validate-url.ts
Normal file
@ -0,0 +1,44 @@
|
||||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the given URL is served from Penpot's own origin. Unparseable URLs
|
||||
* are considered external.
|
||||
*/
|
||||
export function isPenpotOrigin(url: string): boolean {
|
||||
try {
|
||||
return new URL(url).origin === getPenpotOrigin();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Rejects UI URLs that resolve to Penpot's own origin, which would let the
|
||||
* plugin iframe escape its sandbox isolation.
|
||||
*
|
||||
* Plugins whose manifest is itself served from Penpot's origin are part of the
|
||||
* instance and are exempt from the check.
|
||||
*/
|
||||
export function validateUIUrl(url: string, manifestHost: string): void {
|
||||
if (isPenpotOrigin(manifestHost)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const parsed = new URL(url);
|
||||
if (parsed.origin === getPenpotOrigin()) {
|
||||
throw new Error(
|
||||
`Plugin UI URL must not point to Penpot's own domain: ${url}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user