diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index 9cd186baf0..63163e2aca 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -10,6 +10,7 @@ - **plugins-runtime**: `Library.createComponent()` now rejects invalid input (an empty shape list, or a shape inside a component copy) with a validation error instead of returning a component proxy pointing at nothing. - **plugins-runtime**: Setting an individual padding/margin side (`leftPadding`, `topMargin`, …) now re-derives the padding/margin type, switching to `multiple` when the four sides stop being symmetric (so the value is actually painted) and back to `simple` once top/bottom and left/right are mirrored again. +- **plugins-runtime**: Removed the premature deep-hardening of the host plugin context, which froze shared host functions (including `Function.prototype`) before SES override taming, causing `TypeError: Cannot assign to read only property 'toString'` on later host-side function extension. Related to #11001. ## 1.5.0 (2026-07-08) diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin-context.spec.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin-context.spec.ts new file mode 100644 index 0000000000..52a0267db0 --- /dev/null +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin-context.spec.ts @@ -0,0 +1,82 @@ +import { describe, it, vi, expect, beforeEach } from 'vitest'; +import { loadPlugin, setContextBuilder, getPlugins } from './load-plugin'; +import { createPlugin } from './create-plugin'; +import { ses } from './ses.js'; +import type { Context } from '@penpot/plugin-types'; +import type { Manifest } from './models/manifest.model.js'; + +vi.mock('./create-plugin', () => ({ + createPlugin: vi.fn(), +})); + +// NOTE: `./ses.js` is intentionally NOT mocked here: the test spies on the +// real `ses.harden` to assert that `loadPlugin` never hardens the host +// context. + +describe('loadPlugin host context boundary (regression for #11001)', () => { + let manifest: Manifest; + + beforeEach(() => { + manifest = { + pluginId: 'test-plugin', + name: 'Test Plugin', + host: '', + code: '', + permissions: ['content:read'], + }; + + vi.mocked(createPlugin).mockResolvedValue({ + plugin: { + close: vi.fn(), + sendMessage: vi.fn(), + }, + } as unknown as Awaited>); + }); + + it('does not freeze host-owned functions reachable through the context', async () => { + const hostListener = function hostListener() { + return 'host-value'; + }; + const nestedHostObject = { + nestedFn() { + return 'nested'; + }, + }; + const hostContext = { + addListener: hostListener, + nested: nestedHostObject, + } as unknown as Context; + + setContextBuilder(() => hostContext); + + const hardenSpy = vi.spyOn(ses, 'harden'); + + await loadPlugin(manifest); + + // The host context itself must be passed through untouched so the host + // can keep modifying its own runtime objects (e.g. on page navigation). + expect(createPlugin).toHaveBeenCalledWith( + hostContext, + manifest, + expect.any(Function), + undefined, + ); + + // Host-owned functions must remain extensible: page navigation and + // runtime code may patch/augment them (e.g. assigning `toString` on a + // wrapped listener). A deep `ses.harden(context)` here would freeze + // them and turn such later assignments into + // `TypeError: Cannot assign to read only property 'toString'`. + expect(Object.isFrozen(hostListener)).toBe(false); + expect(Object.isExtensible(hostListener)).toBe(true); + expect(Object.isFrozen(nestedHostObject)).toBe(false); + expect(() => { + hostListener.toString = () => 'patched-by-host'; + }).not.toThrow(); + + expect(hardenSpy).not.toHaveBeenCalled(); + expect(getPlugins()).toHaveLength(1); + + hardenSpy.mockRestore(); + }); +}); diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin-real-path.spec.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin-real-path.spec.ts new file mode 100644 index 0000000000..06ef7b20b7 --- /dev/null +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin-real-path.spec.ts @@ -0,0 +1,169 @@ +import { describe, it, vi, expect, beforeAll } from 'vitest'; +import 'ses'; +import { loadPlugin, setContextBuilder, getPlugins } from './load-plugin'; +import type { Context } from '@penpot/plugin-types'; +import type { Manifest } from './models/manifest.model.js'; + +// Real initialization-path regression tests for #11001. +// +// NOTE: `./create-plugin`, `./plugin-manager` and +// `./create-sandbox` are intentionally NOT mocked here. This spec exercises +// the real `loadPlugin → createPlugin → createPluginManager → createSandbox` +// path with the real SES implementation, mirroring the production +// initialization order from `plugins-runtime/src/index.ts`: +// repairIntrinsics (module load) → loadPlugin → createSandbox/hardenIntrinsics +// +// `hardenIntrinsics()` is deliberately NOT called up front: the first test +// must run in the production window where only `repairIntrinsics` has run. +// Tests run in declaration order; the later tests build on the locked-down +// state the first real `loadPlugin` leaves behind (via `createSandbox`). +// +// Note on SES isolation: this suite depends on Vitest's default +// file-level isolation. Each test file runs in a separate worker +// process, so SES intrinsics frozen here do not leak into other +// spec files. + +const REPAIR_OPTIONS = { + evalTaming: 'unsafeEval', + stackFiltering: 'verbose', + errorTaming: 'unsafe', + consoleTaming: 'unsafe', + errorTrapping: 'none', + unhandledRejectionTrapping: 'none', +}; + +function makeManifest( + code: string, + permissions: Manifest['permissions'], +): Manifest { + return { + pluginId: 'test-plugin', + name: 'Test Plugin', + host: '', + code, + permissions, + }; +} + +function makeHostFixture() { + const listenerTypes: string[] = []; + const listeners = new Map(); + // Inline code (empty host + non-URL code) resolves without network, so no + // fetch mock is needed. UI/modal APIs are never touched by the probe code. + const createRectangle = vi.fn(() => ({ type: 'rectangle-marker' })); + const selection: object[] = [{ id: 'shape-1' }]; + const context = { + addListener: (type: string, _callback: (...args: unknown[]) => unknown) => { + const id = Symbol(type); + listeners.set(id, type); + listenerTypes.push(type); + return id; + }, + removeListener: (id: symbol) => { + listeners.delete(id); + }, + theme: 'dark', + createRectangle, + selection, + // Host-only member: present on the raw context but NOT part of the + // public penpot API. Plugin code must never see it (see B-2 below). + __internalSecret: 'host-internal', + } as unknown as Context; + return { context, listenerTypes, createRectangle, selection }; +} + +function lastCompartmentGlobalThis(): Record { + const plugins = getPlugins(); + const last = plugins[plugins.length - 1] as unknown as { + compartment: { compartment: { globalThis: Record } }; + }; + return last.compartment.compartment.globalThis; +} + +describe('loadPlugin real initialization path (regression for #11001)', () => { + beforeAll(() => { + // Production module-load step only: repairs intrinsics WITHOUT + // installing override taming, exactly like `index.ts` at import time. + ( + globalThis as unknown as { repairIntrinsics(opts: object): void } + ).repairIntrinsics({ ...REPAIR_OPTIONS }); + }); + + it('loads through the real path and keeps host function augmentation working', async () => { + const fixture = makeHostFixture(); + setContextBuilder(() => fixture.context); + + await loadPlugin( + makeManifest('penpot.on("finish", function () {});', ['content:read']), + ); + + // The plugin code really ran inside the sandbox: the manager registers + // `themechange` + `finish`, and the plugin code adds its own `finish` + // listener through the public API. + expect(fixture.listenerTypes).toEqual(['themechange', 'finish', 'finish']); + expect(getPlugins()).toHaveLength(1); + + // The user-facing behavior from #11001: host-side augmentation of a + // fresh function (e.g. assigning `toString` during page navigation) + // succeeds after a real plugin load. + const freshWrapper = function freshWrapper() { + return 'navigation-wrapper'; + }; + expect(() => { + freshWrapper.toString = () => 'patched-by-runtime'; + }).not.toThrow(); + expect(fixture.listenerTypes.length).toBe(3); + }); + + it('denies the write API without permission and leaves the host untouched', async () => { + const fixture = makeHostFixture(); + setContextBuilder(() => fixture.context); + + await expect( + loadPlugin(makeManifest('penpot.createRectangle();', ['content:read'])), + ).rejects.toThrow(/content:write/); + expect(fixture.createRectangle).not.toHaveBeenCalled(); + }); + + it('allows the same write API with permission', async () => { + const fixture = makeHostFixture(); + setContextBuilder(() => fixture.context); + + await loadPlugin( + makeManifest('penpot.createRectangle();', [ + 'content:read', + 'content:write', + ]), + ); + expect(fixture.createRectangle).toHaveBeenCalledTimes(1); + }); + + it('does not expose raw host-only context members to plugin code', async () => { + const fixture = makeHostFixture(); + setContextBuilder(() => fixture.context); + + await loadPlugin( + makeManifest('globalThis.__probe = typeof penpot.__internalSecret;', [ + 'content:read', + ]), + ); + // The public `penpot` object is a boundary proxy over a curated API, not + // the raw host context, so host-only members are invisible inside. + expect(lastCompartmentGlobalThis()['__probe']).toBe('undefined'); + }); + + it('keeps safeReturn protection on returned values without blocking allowed edits', async () => { + const fixture = makeHostFixture(); + setContextBuilder(() => fixture.context); + + await loadPlugin( + makeManifest( + 'penpot.createRectangle(); ' + + 'globalThis.__selectionFrozen = Object.isFrozen(penpot.selection);', + ['content:read', 'content:write'], + ), + ); + expect(fixture.createRectangle).toHaveBeenCalledTimes(1); + expect(lastCompartmentGlobalThis()['__selectionFrozen']).toBe(true); + }); +}); diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts index d05178f1f7..dfd44f4971 100644 --- a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts @@ -3,7 +3,6 @@ import type { Context } from '@penpot/plugin-types'; import { loadManifest } from './parse-manifest.js'; import { Manifest } from './models/manifest.model.js'; import { createPlugin } from './create-plugin.js'; -import { ses } from './ses.js'; let plugins: Awaited>[] = []; @@ -54,8 +53,23 @@ export const loadPlugin = async function ( closeAllPlugins(); + // The host context is not deeply frozen at this load stage. + // + // The context still contains host-internal function objects and shared + // prototypes that the host may legitimately extend after plugin load + // (for example, by assigning custom properties). Deep-freezing here + // would freeze those prototypes before SES override taming completes, + // preventing later host-side mutations with a "Cannot assign to read + // only property" TypeError. + // + // Responsibility boundary: this function forwards the context to the + // sandbox layer without deep-freezing it. The public API that plugins + // consume is constructed by the API module (`api/index.ts`), and + // `createSandbox`'s proxy handler applies `ses.safeReturn` to values + // crossing into the sandbox. Compartment isolation and intrinsics + // hardening are performed by createSandbox, not here. const plugin = await createPlugin( - ses.harden(context) as Context, + context, manifest, () => { plugins = plugins.filter((api) => api !== plugin);