mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-20 18:08:55 +00:00
fix(plugins): load plugin iframe via srcdoc to keep opaque-origin isolation on desktop
Dropping allow-same-origin (so the plugin UI iframe runs on an opaque origin and cannot reach window.parent.ea) blanks the UI on packaged file:// builds: a blob: URL inherits the host origin and an opaque-origin iframe cannot fetch it. srcdoc is parsed inline (no cross-origin fetch), so the iframe renders under its opaque origin everywhere. - buildPluginIframeHtml() returns the document string (was createPluginIframeUrl returning a blob: URL); component binds it via [srcdoc] with bypassSecurityTrustHtml (srcdoc is a SecurityContext.HTML sink, raw strings would be sanitized). - remove now-unused blob URL tracking/cleanup. - sandbox stays a static attribute (Angular NG0910). (cherry picked from commit 5efab5d5ac2da5e6e1babf25e7e37d0d965b7559)
This commit is contained in:
parent
b361f86a91
commit
7e6b2df1ad
5 changed files with 60 additions and 73 deletions
|
|
@ -1,5 +1,4 @@
|
|||
import { Injectable } from '@angular/core';
|
||||
import { cleanupAllPluginIframeUrls } from './util/plugin-iframe.util';
|
||||
|
||||
/**
|
||||
* Simplified cleanup service following KISS principles.
|
||||
|
|
@ -34,8 +33,5 @@ export class PluginCleanupService {
|
|||
cleanupAll(): void {
|
||||
// Just clear all references - let Angular manage iframe DOM lifecycle
|
||||
this._pluginIframes.clear();
|
||||
|
||||
// Clean up all blob URLs to prevent memory leaks
|
||||
cleanupAllPluginIframeUrls();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -59,16 +59,19 @@
|
|||
}
|
||||
|
||||
<!-- Plugin iframe -->
|
||||
@if (iframeSrc() && !error()) {
|
||||
@if (iframeSrcdoc() && !error()) {
|
||||
<div
|
||||
class="iframe-container"
|
||||
[class.is-resizing]="isResizing()"
|
||||
>
|
||||
<!-- sandbox must stay a static attribute and mirror PLUGIN_IFRAME_SANDBOX:
|
||||
Angular forbids binding security-sensitive iframe attributes (throws NG0910). -->
|
||||
Angular forbids binding security-sensitive iframe attributes (throws NG0910).
|
||||
No allow-same-origin: the iframe runs on an opaque origin so it cannot reach
|
||||
window.parent.ea. Loaded via srcdoc (not a blob: URL) so it still renders on
|
||||
packaged file:// builds, where an opaque-origin blob: fetch would be blocked. -->
|
||||
<iframe
|
||||
#iframe
|
||||
[src]="iframeSrc()"
|
||||
[srcdoc]="iframeSrcdoc()"
|
||||
data-plugin-iframe
|
||||
[attr.data-plugin-id]="pluginId()"
|
||||
sandbox="allow-scripts allow-forms allow-popups allow-modals"
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import {
|
|||
ViewChild,
|
||||
} from '@angular/core';
|
||||
import { ActivatedRoute, Router } from '@angular/router';
|
||||
import { DomSanitizer, SafeResourceUrl } from '@angular/platform-browser';
|
||||
import { DomSanitizer, SafeHtml } from '@angular/platform-browser';
|
||||
import { Subscription } from 'rxjs';
|
||||
import { trigger, transition, style, animate } from '@angular/animations';
|
||||
import { PluginService } from '../../plugin.service';
|
||||
|
|
@ -18,9 +18,8 @@ import { PluginBridgeService } from '../../plugin-bridge.service';
|
|||
import { PluginCleanupService } from '../../plugin-cleanup.service';
|
||||
import {
|
||||
PluginIframeConfig,
|
||||
createPluginIframeUrl,
|
||||
buildPluginIframeHtml,
|
||||
handlePluginMessage,
|
||||
cleanupPluginIframeUrl,
|
||||
} from '../../util/plugin-iframe.util';
|
||||
import {
|
||||
MatCard,
|
||||
|
|
@ -93,12 +92,11 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
|
|||
readonly pluginId = signal<string>('');
|
||||
readonly isLoading = signal<boolean>(true);
|
||||
readonly error = signal<string | null>(null);
|
||||
readonly iframeSrc = signal<SafeResourceUrl | null>(null);
|
||||
readonly iframeSrcdoc = signal<SafeHtml | null>(null);
|
||||
readonly isResizing = this._layoutService.isPanelResizing;
|
||||
|
||||
private _messageListener?: EventListener;
|
||||
private _routeSubscription?: Subscription;
|
||||
private _currentIframeUrl: string | null = null;
|
||||
|
||||
async ngOnInit(): Promise<void> {
|
||||
// If directPluginId is provided, load that plugin directly
|
||||
|
|
@ -235,11 +233,8 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
|
|||
boundMethods: this._pluginBridge.createBoundMethods(pluginId, plugin.manifest),
|
||||
};
|
||||
|
||||
// Create iframe URL using blob URL
|
||||
const iframeUrl = createPluginIframeUrl(config);
|
||||
|
||||
// Store the URL for cleanup
|
||||
this._currentIframeUrl = iframeUrl;
|
||||
// Build the iframe document (loaded via srcdoc, not a blob: URL)
|
||||
const iframeHtml = buildPluginIframeHtml(config);
|
||||
|
||||
// Store message handler for cleanup
|
||||
// Filter by event.source so messages from other plugin iframes (side panel,
|
||||
|
|
@ -265,17 +260,12 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
|
|||
// Set up message communication
|
||||
window.addEventListener('message', this._messageListener);
|
||||
|
||||
// Create safe URL and set iframe source
|
||||
const safeUrl = this._sanitizer.bypassSecurityTrustResourceUrl(iframeUrl);
|
||||
PluginLog.log(
|
||||
`Setting iframe src for plugin ${pluginId}:`,
|
||||
iframeUrl.startsWith('blob:')
|
||||
? `blob:${iframeUrl.split(':')[1].substring(0, 20)}...`
|
||||
: iframeUrl.substring(0, 100) + '...',
|
||||
);
|
||||
this.iframeSrc.set(safeUrl);
|
||||
// Set the iframe document. `srcdoc` is a SecurityContext.HTML sink, so the
|
||||
// host-built document (which contains the inline API bridge <script>) must
|
||||
// be marked trusted or Angular's HTML sanitizer would strip the script.
|
||||
this.iframeSrcdoc.set(this._sanitizer.bypassSecurityTrustHtml(iframeHtml));
|
||||
this.isLoading.set(false);
|
||||
PluginLog.log(`Plugin ${pluginId} iframe src set, loading complete`);
|
||||
PluginLog.log(`Plugin ${pluginId} iframe srcdoc set, loading complete`);
|
||||
}
|
||||
|
||||
private _createBridgeToken(): string {
|
||||
|
|
@ -295,13 +285,6 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
|
|||
PluginLog.log(`Removed message listener for plugin: ${currentPluginId}`);
|
||||
}
|
||||
|
||||
// Cleanup blob URL if it exists
|
||||
if (this._currentIframeUrl) {
|
||||
cleanupPluginIframeUrl(this._currentIframeUrl);
|
||||
console.log(`Cleaned up blob URL for plugin: ${currentPluginId}`);
|
||||
this._currentIframeUrl = null;
|
||||
}
|
||||
|
||||
// Clear iframe reference from cleanup service (but don't remove from DOM).
|
||||
// Skip when embedding (work-view, side panel) so the plugin keeps its hook
|
||||
// and button registrations across embed mount/unmount cycles.
|
||||
|
|
@ -310,13 +293,11 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
|
|||
PluginLog.log(`Cleaned up plugin references for: ${currentPluginId}`);
|
||||
}
|
||||
|
||||
// Set iframe to empty data URL to stop execution but keep iframe in DOM
|
||||
this.iframeSrc.set(
|
||||
this._sanitizer.bypassSecurityTrustResourceUrl(
|
||||
'data:text/html,<html><body></body></html>',
|
||||
),
|
||||
// Reset iframe to an empty document to stop execution but keep it in DOM
|
||||
this.iframeSrcdoc.set(
|
||||
this._sanitizer.bypassSecurityTrustHtml('<html><body></body></html>'),
|
||||
);
|
||||
PluginLog.log(`Set iframe to empty data URL for plugin: ${currentPluginId}`);
|
||||
PluginLog.log(`Reset iframe to empty document for plugin: ${currentPluginId}`);
|
||||
}
|
||||
|
||||
onIframeLoad(): void {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import {
|
|||
} from '@super-productivity/plugin-api';
|
||||
import { PluginBridgeService } from '../plugin-bridge.service';
|
||||
import {
|
||||
buildPluginIframeHtml,
|
||||
createPluginApiScript,
|
||||
handlePluginMessage,
|
||||
PluginIframeConfig,
|
||||
|
|
@ -310,6 +311,34 @@ describe('handlePluginMessage()', () => {
|
|||
expect(PLUGIN_IFRAME_SANDBOX).not.toContain('allow-same-origin');
|
||||
});
|
||||
|
||||
describe('buildPluginIframeHtml()', () => {
|
||||
const pluginBridge = {
|
||||
createBoundMethods: () => ({}),
|
||||
} as unknown as PluginBridgeService;
|
||||
|
||||
it('returns an inline HTML document (not a blob: URL) for srcdoc', () => {
|
||||
const html = buildPluginIframeHtml({
|
||||
...createConfig(pluginBridge),
|
||||
indexHtml: '<html><head></head><body><div id="app"></div></body></html>',
|
||||
});
|
||||
expect(typeof html).toBe('string');
|
||||
expect(html.startsWith('blob:')).toBe(false);
|
||||
// plugin content is preserved
|
||||
expect(html).toContain('<div id="app"></div>');
|
||||
});
|
||||
|
||||
it('injects the API bridge script (with the bridge token) before </body>', () => {
|
||||
const html = buildPluginIframeHtml({
|
||||
...createConfig(pluginBridge),
|
||||
indexHtml: '<html><head></head><body></body></html>',
|
||||
});
|
||||
expect(html).toContain('const bridgeToken = "test-bridge-token"');
|
||||
expect(html).toContain('window.PluginAPI');
|
||||
// script is injected inside the body, before its closing tag
|
||||
expect(html.indexOf('window.PluginAPI')).toBeLessThan(html.indexOf('</body>'));
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects raw iframe calls to bridge methods outside the iframe API allowlist', async () => {
|
||||
const sourceWindow = jasmine.createSpyObj<{ postMessage: jasmine.Spy }>(
|
||||
'sourceWindow',
|
||||
|
|
|
|||
|
|
@ -533,13 +533,18 @@ export const createPluginApiScript = (config: PluginIframeConfig): string => {
|
|||
`;
|
||||
};
|
||||
|
||||
// Store blob URLs for cleanup
|
||||
const activeBlobUrls = new Set<string>();
|
||||
|
||||
/**
|
||||
* Create iframe URL with injected API using blob URLs instead of data URLs
|
||||
* Build the full HTML document for a plugin UI iframe: the plugin's index.html
|
||||
* with the host CSS and the postMessage API bridge injected.
|
||||
*
|
||||
* Returned as a string for `iframe.srcdoc`, NOT a `blob:` URL. srcdoc content is
|
||||
* parsed inline, so the iframe loads under its sandbox-assigned opaque origin
|
||||
* even on packaged `file://` builds. A `blob:` URL created by the host inherits
|
||||
* the host origin, and an opaque-origin iframe (no `allow-same-origin`) cannot
|
||||
* fetch it — which blanks the plugin UI on desktop. srcdoc avoids that
|
||||
* cross-origin fetch entirely. See PLUGIN_IFRAME_SANDBOX.
|
||||
*/
|
||||
export const createPluginIframeUrl = (config: PluginIframeConfig): string => {
|
||||
export const buildPluginIframeHtml = (config: PluginIframeConfig): string => {
|
||||
const apiScript = createPluginApiScript(config);
|
||||
const cssInjection = createPluginCssInjection();
|
||||
const fullCssInjection =
|
||||
|
|
@ -566,34 +571,7 @@ export const createPluginIframeUrl = (config: PluginIframeConfig): string => {
|
|||
html = html + apiScript;
|
||||
}
|
||||
|
||||
// Create blob URL instead of data URL
|
||||
const blob = new Blob([html], { type: 'text/html' });
|
||||
const blobUrl = URL.createObjectURL(blob);
|
||||
|
||||
// Track the blob URL for cleanup
|
||||
activeBlobUrls.add(blobUrl);
|
||||
|
||||
return blobUrl;
|
||||
};
|
||||
|
||||
/**
|
||||
* Clean up a blob URL when iframe is no longer needed
|
||||
*/
|
||||
export const cleanupPluginIframeUrl = (url: string): void => {
|
||||
if (url.startsWith('blob:') && activeBlobUrls.has(url)) {
|
||||
URL.revokeObjectURL(url);
|
||||
activeBlobUrls.delete(url);
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Clean up all active blob URLs (useful for cleanup on destroy)
|
||||
*/
|
||||
export const cleanupAllPluginIframeUrls = (): void => {
|
||||
activeBlobUrls.forEach((url) => {
|
||||
URL.revokeObjectURL(url);
|
||||
});
|
||||
activeBlobUrls.clear();
|
||||
return html;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue