Skip to content

feat: support manifest-driven plugin iframe embeds#4033

Open
ai-ag2026 wants to merge 2 commits into
nesquena:masterfrom
ai-ag2026:pr/plugin-manifest-iframe-embed
Open

feat: support manifest-driven plugin iframe embeds#4033
ai-ag2026 wants to merge 2 commits into
nesquena:masterfrom
ai-ag2026:pr/plugin-manifest-iframe-embed

Conversation

@ai-ag2026

Copy link
Copy Markdown
Contributor

Summary

  • let dashboard plugin manifests opt into iframe bootstrap payload injection via webui.inline_payload
  • let manifests opt into self-contained iframe HTML by inlining declared dist CSS/JS via webui.inline_dist_assets
  • rebase default style.css / index.js references to authenticated dashboard-plugin asset routes without special-casing any plugin name

Contract Routing

Task type: dashboard plugin serving enhancement
Touched areas: plugin iframe page route (/dashboard-plugins/<name>/page) and plugin panel static guards
Scope boundaries: manifest-driven, read-only embedding for sandboxed plugin pages; no Project Cockpit/TARS-specific endpoint or core Insights UI changes included.

Test plan

  • python -m pytest tests/test_plugins_panel.py -q -o addopts= → 33 passed
  • python -m py_compile api/routes.py
  • git diff --check origin/master...HEAD
  • added-line private/secret marker scan → 0

@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds manifest-driven iframe embedding for dashboard plugins, allowing plugin manifests to opt into server-side bootstrap payload injection (webui.inline_payload) and self-contained HTML by inlining declared CSS/JS assets (webui.inline_dist_assets). It also rebases default style.css/index.js references to authenticated dashboard-plugin asset routes without any plugin-name special-casing.

  • _plugin_inline_payload reads a JSON file declared in the manifest and emits a <script> block assigning it to a validated JS global; the payload deduplication check was improved from a naive substring search to a regex that only matches inside <script> tags.
  • _inline_declared_dist_assets replaces link/script tags in existing plugin HTML with inlined CSS/JS content, with path-containment guards to prevent traversal; _inject_plugin_payload orchestrates both steps and falls through to href/src rebasing when inlining is not requested.
  • A new behavioral integration test exercises the full symlink + inlining path end-to-end, complementing a source-inspection test that asserts no hard-coded plugin names remain.

Confidence Score: 4/5

Safe to merge after addressing the HTML-injection issue in _inject_plugin_payload where name_escaped_for_path uses only quote-stripping instead of full HTML escaping.

The route rewrites href and src attributes in plugin-served HTML using _name.replace('"', '') for the plugin name, while the fallback HTML-shell path just below correctly calls html.escape(name). A plugin name containing > breaks out of the attribute context, injecting raw HTML into the served page. The plugin iframe runs in a null-origin sandbox which limits what injected script can do, but the injection itself is real and present in the changed code.

api/routes.py — specifically the name_escaped_for_path variable in _inject_plugin_payload.

Security Review

  • HTML injection in plugin iframe page (api/routes.py, _inject_plugin_payload): name_escaped_for_path is sanitized with only _name.replace('\"', ''). A plugin name containing > escapes the attribute context in the rewritten href and src strings, injecting arbitrary HTML into the sandboxed plugin page. The fallback HTML-shell path already uses html.escape(name) correctly.

Important Files Changed

Filename Overview
api/routes.py Adds manifest-driven iframe inlining helpers. The name_escaped_for_path variable uses only replace('"','') instead of html.escape() for URL path construction inside HTML attributes, creating an HTML-injection vector in the plugin iframe page.
tests/test_plugins_panel.py Adds a behavioral integration test exercising the full inline-assets-and-payload path including a symlink scenario, and a source-inspection test asserting no hard-coded plugin names. Both tests are correctly structured.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GET /plugin-tab-path] --> B{dashboard_dir exists?}
    B -- No --> Z[Return False / 404]
    B -- Yes --> C[Resolve dashboard_path]
    C --> D{dist/index.html exists?}
    D -- Yes --> E[read_text to _inject_plugin_payload]
    D -- No --> F{static/index.html exists?}
    F -- Yes --> E
    F -- No --> G{dist/index.js exists?}
    G -- Yes --> H[Generate HTML shell]
    G -- No --> Z
    E --> I[_inline_declared_dist_assets]
    I --> J{inline_dist_assets == True?}
    J -- No --> K[Return html unchanged]
    J -- Yes --> L[Inline CSS]
    L --> M[Inline JS]
    M --> N[Return inlined HTML]
    E --> O[Rebase href/src if not inlining]
    O --> P{payload_script and not already injected?}
    N --> P
    P -- Yes --> Q[Insert payload before first script tag]
    P -- No --> R[Return html_text]
    Q --> R
    R --> S[Encode UTF-8 and send 200]
    H --> S
Loading

Reviews (10): Last reviewed commit: "ci: retrigger flaky gateway sync shard" | Re-trigger Greptile

Comment thread api/routes.py Outdated
Comment thread api/routes.py Outdated
Comment thread tests/test_plugins_panel.py
@ai-ag2026 ai-ag2026 force-pushed the pr/plugin-manifest-iframe-embed branch 5 times, most recently from 38424e8 to c6f805e Compare June 14, 2026 16:00
Comment thread api/routes.py Outdated
@ai-ag2026 ai-ag2026 force-pushed the pr/plugin-manifest-iframe-embed branch 2 times, most recently from 670ee31 to 1b29211 Compare June 14, 2026 16:13
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Review — manifest-driven embed is well-guarded; a couple of notes on the inline escaping

Reading the new plugin-page block at api/routes.py:7272-7412 on the PR head against origin/master (where the page shell just did index_html.read_bytes() with no injection), the design is clean and the security posture holds up. This is genuinely manifest-driven — no plugin name is special-cased, and test_plugin_page_embed_is_manifest_driven_not_name_special_cased pins that (window.__LOCAL_PLUGIN_OVERVIEW__/name == "local-plugin" are asserted absent).

Path containment is correct

All three inline helpers resolve candidate files and then re-anchor them under the plugin root before reading. For the payload (routes.py:7298-7301):

payload_path = (_dashboard_path / rel).resolve()
try:
    payload_path.relative_to(_dashboard_path)
except ValueError:
    continue

Same .resolve() + relative_to pattern guards css/entry in _inline_declared_dist_assets. Since _dashboard_path = dashboard_dir.resolve() and _PLUGIN_STATIC_ROOTS[name] is itself stored resolved (api/plugins.py:97), the comparison operates on fully-resolved real paths on both sides — so a symlinked plugin root still contains correctly. The added symlink branch in test_plugin_page_embed_inlines_declared_assets_and_payload_behaviorally exercises exactly that, which is the right thing to test here.

CSP is preserved

The sandbox allow-scripts allow-forms allow-popups + nosniff headers (routes.py:7257-7258) are untouched — inlining the bundle into the shell doesn't loosen the iframe sandbox, so the inlined script still runs origin-less. Good: that's the property that makes embedding declared dist assets safe.

Script/style breakout escaping — two observations

The payload and JS inlining use the standard </<\/ mitigation, which is correct for JS string and script contexts:

encoded = json.dumps(payload, ensure_ascii=False).replace("</", "<\\/")
...
script_text = script_path.read_text(...).replace("</script", "<\\/script")

Two small things worth a look (neither blocking):

  1. CSS close-tag escaping is HTML-correct but CSS-lossy. style_text.replace("</style", "<\\/style") prevents an embedded </style> from closing the raw-text element early (good), but <\/style isn't meaningful CSS, so a legitimate </style literal inside a CSS content: value would be corrupted rather than rendered. Rare in practice; just flagging that the JS path round-trips cleanly while the CSS path doesn't.

  2. Payload </<\/ is broad but safe. Because the value is always json.dumps(...), </ can only appear inside JSON string values, and <\/ in a JS string literal decodes back to </ — so this is lossless for the payload, unlike the CSS case. No action needed, just confirming the asymmetry is fine.

Re-injection guard

_inject_plugin_payload guards against double-injection with a regex that matches an existing <script ...>__GLOBAL__= (routes.py), and the test deliberately plants window.__DEMO_PAYLOAD__= inside an HTML comment to prove a comment doesn't suppress injection. That's the correct edge to cover — the regex keys on <script so the commented occurrence is ignored.

Verdict

No blockers. Containment, CSP, and the manifest-driven contract are all sound, and the behavioral test covers the real serving path rather than asserting on string fragments alone. If you want to tighten the CSS path, escaping only the literal </style> sequence (or HTML-escaping just the < of a close tag) would avoid the lossy-CSS edge — but it's cosmetic given how unlikely </style is inside a real stylesheet.

@ai-ag2026 ai-ag2026 force-pushed the pr/plugin-manifest-iframe-embed branch 2 times, most recently from 173e0cb to 6e46b9d Compare June 14, 2026 19:10
@ai-ag2026 ai-ag2026 force-pushed the pr/plugin-manifest-iframe-embed branch from 6e46b9d to ac72630 Compare June 15, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants