🐛 Fix deep-harden of host plugin context on load (#11521)

* 🐛 Fix deep-harden of host plugin context on load

ses.harden(context) in loadPlugin deep-freezes every host-owned object
and function reachable through the context. The host keeps needing to
modify those across page navigation (listener wrappers, proxies), so a
later property augmentation (e.g. assigning toString) throws
'TypeError: Cannot assign to read only property toString' and kills the
MCP session (penpot/penpot#11001).

Pass the host context through untouched; sandbox isolation stays at the
compartment boundary (hardened sandbox-owned globals + ses.safeReturn).

Regression test: load-plugin-context.spec.ts (no ses mock).

AI-assisted-by: muse-spark-1.3
Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>

* 🐛 Add real SES bootstrap to host-context regression test

The previous load-plugin-context.spec.ts had no SES bootstrap, so it
failed on the original code with 'ReferenceError: harden is not defined'
instead of the intended freeze assertion, and passed on the fixed code
merely by avoiding ses.harden.

Now the spec bootstraps real SES (repairIntrinsics + hardenIntrinsics),
adds a control test proving real ses.harden deep-freezes host-owned
functions (Object.isFrozen === true, later toString assignment throws
TypeError - the #11001 crash signature), and keeps the regression test
asserting loadPlugin leaves host functions unfrozen and patchable.

AI-assisted-by: muse-spark-1.3
Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>

* 🐛 Add production-order hardening contrast evidence

Proves the initialization-ordering hazard behind #11001 (cf. #8636):
in production, index.ts runs repairIntrinsics only at module load while
hardenIntrinsics runs later in createSandbox. The original loadPlugin
called ses.harden(context) between those steps, freezing the shared
Function.prototype with plain data properties so later override taming
is skipped and any subsequent fn.toString assignment throws TypeError.

Kept in a separate spec file so the full SES bootstrap in
load-plugin-context.spec.ts cannot mask the ordering effect.

AI-assisted-by: muse-spark-1.3
Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>

* 🐛 Apply approved lint fix and CHANGELOG entry

Restores the two approved deliverables missing from the previous push:
the prefer-rest-params fix in load-plugin-harden-order.spec.ts
(replacing the deprecated arguments usage) and the plugins-runtime
CHANGELOG entry for the host-context harden fix (#11001).

AI-assisted-by: muse-spark-1.3

Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>

* 🐛 Remove deep-hardening of host plugin context on load

Signed-off-by: makesomethingshit <junsoo1172@gmail.com>
Co-authored-by: multica-agent <github@multica.ai>
Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>

* 🐛 Align CHANGELOG and context comment with reviewed evidence

AI-assisted-by: multica-agent
Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>
Co-authored-by: multica-agent <github@multica.ai>

* 🔥 Remove SES semantic tests from plugin regression coverage

Drop the tests that only verify SES library semantics rather than Penpot
application behavior:

- Delete load-plugin-harden-order.spec.ts (pure SES initialization-order
  evidence, never calls loadPlugin).
- Remove the ses.harden control test and its SES bootstrap setup from
  load-plugin-context.spec.ts.
- Remove the #8636 hardening-order contrast test and the now-unused ses
  import from load-plugin-real-path.spec.ts.

Keep the application-level regression coverage: the real loadPlugin
initialization path, permission enforcement, host-context isolation and
safeReturn protection. No production code changes.

Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>
AI-assisted-by: Omen Alpha

---------

Signed-off-by: Junsoo Choi <junsoo1172@gmail.com>
Signed-off-by: makesomethingshit <junsoo1172@gmail.com>
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: Andrey Antukh <niwi@niwi.nz>

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)
 

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<ReturnType<typeof createPlugin>>);
+  });
+
+  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();
+  });
+});

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<symbol, string>();
+  // 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<string, unknown> {
+  const plugins = getPlugins();
+  const last = plugins[plugins.length - 1] as unknown as {
+    compartment: { compartment: { globalThis: Record<string, unknown> } };
+  };
+  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);
+  });
+});

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<ReturnType<typeof createPlugin>>[] = [];
 
@@ -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);