H-6544: Load Petrinaut in HASH in an iFrame, and add the AI assistant#8790
H-6544: Load Petrinaut in HASH in an iFrame, and add the AI assistant#8790CiaranMn wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
github-actions
Bot
added
area/deps
PR SummaryHigh Risk Overview Security and observability: Middleware applies a dedicated embed CSP ( AI assistant: New streaming App Router UX/perf: The process route mounts the iframe before Reviewed by Cursor Bugbot for commit e6fca59. Bugbot is set up for automated code reviews on this repo. Configure here. |
![semgrep-code-hashintel[bot]](https://avatars.githubusercontent.com/u/22116112?s=60&v=4){kind=small}
| }, []); | ||
|
|
||
| const send = useCallback<IframeBridge["send"]>((message) => { | ||
| window.parent.postMessage(message, "*"); |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Using "*" as the postMessage target origin allows any webpage to intercept messages sent from this iframe, risking information disclosure of sensitive data like user actions and document state.
More details about this
The window.parent.postMessage() calls are using "*" as the target origin, which means any webpage can receive the messages sent from this iframe.
Exploit scenario:
- An attacker hosts a malicious webpage and tricks a user into visiting it while they have a tab open to your application containing this iframe.
- The attacker's webpage listens for messages with
window.addEventListener("message", ...)to capture any postMessage events from iframes. - When the iframe executes
window.parent.postMessage({ kind: "ready" }, "*"), the message broadcasts to any origin, including the attacker's malicious site. - Similarly, when
send()is called withwindow.parent.postMessage(message, "*"), the attacker intercepts sensitive data likesaveResult,revisionsList, orsetReadonlycallbacks that may contain user data, document content, or application state. - The attacker can extract this information and use it to steal user data or manipulate the application state.
Even though the code comment notes that "the message is delivered to the parent window only," using "*" removes the browser's origin-check enforcement, allowing any frame in any tab to listen and capture the messages.
To resolve this comment:
✨ Commit fix suggestion
| window.parent.postMessage(message, "*"); | |
| // Use a validated, explicit parent origin from trusted configuration. | |
| // Ensure `hostOrigin` is derived from trusted config (for example, a query param or injected config), | |
| // normalized via `new URL(...).origin`, and checked against an exact allowlist before use. | |
| window.parent.postMessage(message, hostOrigin); |
View step-by-step instructions
- Replace the wildcard target origin in both
window.parent.postMessage(..., "*")calls with a specific allowed origin value, for examplewindow.parent.postMessage(message, hostOrigin). - Add a way to get the parent application's origin from trusted configuration instead of trying to read it from the iframe at runtime. For example, pass it into the iframe through a query parameter such as
?parentOrigin=https://example.comor an injected config value, then store it in a variable likeconst hostOrigin = .... - Validate that the configured origin is exactly one expected origin before using it. For example, parse it with
new URL(parentOrigin).originand compare it against an allowlist such asconst allowedOrigins = ["https://example.com"]. - Update the initial ready message to use the validated origin:
window.parent.postMessage({ kind: "ready" }, hostOrigin). - Update the
sendcallback to use the same validated origin:window.parent.postMessage(message, hostOrigin). - Reject incoming
messageevents unlessevent.originmatches the same expected origin before processingdata. This keeps the send and receive sides consistent and prevents untrusted windows from driving the iframe logic.
Alternatively, if the parent origin can vary by environment, define an environment-specific allowlist such as ["https://app.example.com", "https://staging.example.com"] and only use the origin when it matches one of those exact values.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by wildcard-postmessage-configuration.
You can view more details about this finding in the Semgrep AppSec Platform.
| window.parent.postMessage( | ||
| { kind: "ready" } satisfies IframeToHostMessage, | ||
| "*", | ||
| ); |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
The postMessage API is called with wildcard origin "*", allowing any website to potentially intercept sensitive messages sent between the iframe and parent window.
More details about this
The window.parent.postMessage() call uses "*" as the target origin, which means any website can potentially receive the message containing the { kind: "ready" } object.
Exploit scenario: An attacker could create a malicious website and trick a user into visiting it while they have your iframe-based application open in another tab. The attacker's page could listen for postMessage events via window.addEventListener("message", (event) => { console.log(event.data); }) and intercept the { kind: "ready" } message. While this specific message may seem low-risk, if the application later sends sensitive data through window.parent.postMessage(message, "*") (as seen in the send function), an attacker could harvest that information. Additionally, attackers could send malicious messages back to the iframe that get processed by the onMessage handler, potentially manipulating the iframe's state or triggering unintended actions like current.onSaveResult(data) with forged data.
The vulnerability exists because postMessage with "*" bypasses origin verification—the browser will deliver the message to window.parent but the message itself is accessible to any page that listens for it, not just the intended parent window.
To resolve this comment:
✨ Commit fix suggestion
| window.parent.postMessage( | |
| { kind: "ready" } satisfies IframeToHostMessage, | |
| "*", | |
| ); | |
| // SECURITY: Replace this placeholder with the exact trusted host origin | |
| // configured by the application (for example, from a validated iframe | |
| // query parameter or environment-specific config). Do not use "*". | |
| const PARENT_ORIGIN = "https://example.com"; | |
| window.parent.postMessage( | |
| { kind: "ready" } satisfies IframeToHostMessage, | |
| PARENT_ORIGIN, | |
| ); |
View step-by-step instructions
- Replace the wildcard target origin in
window.parent.postMessage(..., "*")with a specific trusted origin string, such aswindow.parent.postMessage(message, "https://example.com"). - Pass the host origin into the iframe from a trusted source instead of trying to read it from
window.parent. For example, provide it in the iframe URL like...?parentOrigin=https%3A%2F%2Fexample.comor in an initialization message that you validate before storing it. - Store that origin in a variable and reuse it for every message to the parent, including both the
{ kind: "ready" }message and the genericsendcallback, for exampleconst parentOrigin = ...; window.parent.postMessage({ kind: "ready" }, parentOrigin);andwindow.parent.postMessage(message, parentOrigin);. - Validate the origin value before using it so only expected schemes and hosts are allowed. For example, parse it with
new URL(parentOrigin)and only allow known origins such ashttps://example.com. - Reject incoming
messageevents unlessevent.originexactly matches the same trusted parent origin before calling any handler, for exampleif (event.origin !== parentOrigin) return;. - Alternatively, if the parent origin is fixed for each environment, define it in configuration with a value like
const PARENT_ORIGIN = "https://example.com"and use that constant everywhere instead of"*". This works becausepostMessageonly delivers the data when the target window's origin matches the value you provide.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by wildcard-postmessage-configuration.
You can view more details about this finding in the Semgrep AppSec Platform.
| * Target origin is "*" — see `use-iframe-bridge.ts` for why a stricter | ||
| * value isn't readable from a null-origin sandbox. | ||
| */ | ||
| window.parent.postMessage(message, "*"); |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Iframe is sending error data including stack traces to any origin via postMessage with wildcard target, exposing sensitive debugging information to attackers.
More details about this
The iframe is sending error information to its parent window with a wildcard origin ("*") in window.parent.postMessage(). This means any malicious script running on any origin can intercept the message object, which contains error details including the stack trace via the serializeError() function.
Attack scenario:
- An attacker controls a website and tricks a user into visiting it while the iframe from your application is embedded elsewhere (e.g.,
/processes/<id>/embed). - The attacker's site frames or embeds content that triggers an error in your iframe (e.g., by loading a malicious script that throws an exception).
- When
installIframeErrorReporter()registers theerrorevent listener, the error is caught andpost("window-error", event.error ?? event.message)is called. - The
serializeError(raw)function extracts sensitive data:name,message, andstackproperties from the error object. - This error object is packaged into the
messagevariable along withsource: "window-error",mode: activeMode, and sent viawindow.parent.postMessage(message, "*"). - Because the target origin is
"*", any script on any page that can manipulate the frame hierarchy can receive this message and extract the error stack, which may contain file paths, variable names, or other sensitive information useful for further attacks.
To resolve this comment:
✨ Commit fix suggestion
| window.parent.postMessage(message, "*"); | |
| const parentOriginParam = new URLSearchParams(window.location.search).get("parentOrigin"); | |
| if (!parentOriginParam) { | |
| return; | |
| } | |
| let parentOrigin: string; | |
| try { | |
| parentOrigin = new URL(parentOriginParam).origin; | |
| } catch { | |
| return; | |
| } | |
| // Update this allowlist to the exact parent origins that are permitted to host this iframe. | |
| const ALLOWED_PARENT_ORIGINS = ["https://example.com"]; | |
| if (!ALLOWED_PARENT_ORIGINS.includes(parentOrigin)) { | |
| return; | |
| } | |
| window.parent.postMessage(message, parentOrigin); |
View step-by-step instructions
-
Replace the wildcard target origin with a specific allowed origin instead of
*.
For example, changewindow.parent.postMessage(message, "*")towindow.parent.postMessage(message, parentOrigin). -
Add a trusted parent origin value that the iframe can read at runtime.
A simple option is to pass it in the iframe URL and validate it before use, for exampleconst parentOrigin = new URLSearchParams(window.location.search).get("parentOrigin");. -
Validate that the origin is present and well-formed before sending the message.
For example, parse it withnew URL(parentOrigin)and only useurl.origin; if parsing fails or the value is empty, return without callingpostMessage(...). -
Restrict the value to the exact parent origins your app expects.
For example, checkif (!ALLOWED_PARENTS.includes(url.origin)) return;beforewindow.parent.postMessage(message, url.origin). This prevents an attacker from supplying their own origin string. -
Update the iframe host code to pass the expected parent origin when it creates the iframe URL.
For example, build the iframesrcwith a query parameter such ashttps://example.com/embed?...&parentOrigin=${encodeURIComponent(window.location.origin)}. -
If this iframe can run in a sandboxed or
null-origin context, keep using the parent-provided origin value rather than trying to derive it from the iframe’s own origin.
postMessageonly needs the receiver’s origin, so an opaque iframe origin does not require using*.
Alternatively, if the parent origin is fixed for this deployment, hardcode the exact origin in configuration and use that value directly, such as const parentOrigin = "https://example.com";, instead of accepting it from the URL.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by wildcard-postmessage-configuration.
You can view more details about this finding in the Semgrep AppSec Platform.
🤖 Augment PR SummarySummary: This PR isolates Petrinaut’s execution by moving it into a sandboxed iframe, and adds a streamed AI assistant backed by an authenticated OpenAI proxy. Changes:
Technical Notes: Adds CORS headers for self-hosted fonts to work from a null-origin iframe, and conditionally disables Sentry init inside the embed document in favor of postMessage-based error reporting. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| "font-src": ["'self'", "data:"], | ||
|
|
||
| "connect-src": ["'self'"], |
There was a problem hiding this comment.
apps/hash-frontend/src/lib/csp.ts:137 — The embed CSP comment says connect-src is 'none', but the directive currently allows 'self', which seems to permit network requests from the sandboxed iframe to same-host endpoints. Consider aligning the directive (or the comment) with the intended “no network from iframe” boundary to avoid accidental reachability.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| setPendingSaveRequestId((prev) => | ||
| prev === payload.requestId ? null : prev, | ||
| ); | ||
| if (payload.result.ok) { |
There was a problem hiding this comment.
apps/hash-frontend/src/pages/processes/[uuid]/embed.page/embed-content.tsx:161 — onSaveResult updates mode/savedSnapshot for any successful payload even when payload.requestId doesn’t match the current pendingSaveRequestId, so a late save response from a previous net/navigation could overwrite the active editor state. It seems safer to ignore saveResult messages that don’t correspond to the currently pending request.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| * `iframeRef.current` identity isn't reactive on its own, so consumers | ||
| * remount the `<iframe>` via `key` and we observe the load event. | ||
| */ | ||
| useEffect(() => { |
There was a problem hiding this comment.
apps/hash-frontend/src/pages/processes/shared/use-host-bridge.ts:136 — This load listener only attaches to the iframe element that exists when the effect runs; if the iframe is ever re-mounted/swapped (as the comment describes), iframeRef.current changes but this effect won’t re-run to attach to the new element. That could leave isReady stale and allow the host to send messages before the new iframe has posted ready.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const postToHost = (message: IframeToHostMessage) => { | ||
| // Target origin "*" — see `use-iframe-bridge.ts` for why a stricter value | ||
| // isn't readable from a null-origin sandbox. Delivered to the parent only. | ||
| window.parent.postMessage(message, "*"); |
| window.addEventListener("message", (event) => { | ||
| if (event.source !== window.parent) { | ||
| return; | ||
| } | ||
| const data = event.data as unknown; | ||
| if ( | ||
| typeof data !== "object" || | ||
| data === null || | ||
| typeof (data as { kind?: unknown }).kind !== "string" | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const message = data as HostToIframeMessage; | ||
| if ( | ||
| message.kind !== "aiChatResponseStart" && | ||
| message.kind !== "aiChatChunk" && | ||
| message.kind !== "aiChatEnd" && | ||
| message.kind !== "aiChatError" | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const pending = pendingRequests.get(message.requestId); | ||
| if (!pending) { | ||
| return; | ||
| } | ||
|
|
||
| switch (message.kind) { | ||
| case "aiChatResponseStart": { | ||
| pending.responded = true; | ||
| pending.resolveResponse( | ||
| new Response(pending.stream, { | ||
| status: message.status, | ||
| statusText: message.statusText, | ||
| }), | ||
| ); | ||
| break; | ||
| } | ||
| case "aiChatChunk": { | ||
| try { | ||
| pending.controller.enqueue(message.bytes); | ||
| } catch { | ||
| // Stream already closed/errored (e.g. consumer aborted) — drop. | ||
| } | ||
| break; | ||
| } | ||
| case "aiChatEnd": { | ||
| try { | ||
| pending.controller.close(); | ||
| } catch { | ||
| // Already closed. | ||
| } | ||
| pendingRequests.delete(message.requestId); | ||
| break; | ||
| } | ||
| case "aiChatError": { | ||
| const error = new Error(message.message); | ||
| if (pending.responded) { | ||
| try { | ||
| pending.controller.error(error); | ||
| } catch { | ||
| // Already settled. | ||
| } | ||
| } else { | ||
| pending.rejectResponse(error); | ||
| } | ||
| pendingRequests.delete(message.requestId); | ||
| break; | ||
| } | ||
| } | ||
| }); |
| * Target origin is "*" — see `use-iframe-bridge.ts` for why a stricter | ||
| * value isn't readable from a null-origin sandbox. | ||
| */ | ||
| window.parent.postMessage(message, "*"); |
| * The data is delivered to this specific iframe's window, not | ||
| * broadcast, so a third party can't intercept it. | ||
| */ | ||
| iframeWindow.postMessage(message, "*"); |
| window.parent.postMessage( | ||
| { kind: "ready" } satisfies IframeToHostMessage, | ||
| "*", | ||
| ); |
| }, []); | ||
|
|
||
| const send = useCallback<IframeBridge["send"]>((message) => { | ||
| window.parent.postMessage(message, "*"); |
| context: Record<string, unknown> = {}, | ||
| ) => { | ||
| // eslint-disable-next-line no-console | ||
| console.error(`[Petrinaut AI] ${reason}`, context); |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e6fca59. Configure here.
| [iframeRef], | ||
| ); | ||
|
|
||
| return { isReady, send }; |
There was a problem hiding this comment.
Unmemoized bridge object causes excessive effect re-runs
Medium Severity
useHostBridge returns a plain { isReady, send } object literal without useMemo, creating a new reference on every render. Since process-editor.tsx lists bridge in the dependency arrays of multiple useEffect hooks (e.g. the revisionsList and setReadonly effects), every host re-render fires those effects and sends redundant postMessages to the iframe. Each redundant message triggers a state update and re-render inside the iframe. The same issue exists in useIframeBridge's return value.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit e6fca59. Configure here.
| title, | ||
| decisionTime: result.decisionTime, | ||
| }, | ||
| revisions: buildRevisionSummaries(revisions), |
There was a problem hiding this comment.
Stale revisions closure in save result callback
Low Severity
The onRequestSave handler's .then callback references revisions from the closure captured at handler-invocation time. Since persistDefinition internally awaits refetchRevisions() (which updates Apollo's cache), the closure's revisions is stale by the time saveResult is sent — it won't include the newly-saved revision. The iframe's version picker briefly shows the old revision count until the separate revisionsList effect corrects it.
Reviewed by Cursor Bugbot for commit e6fca59. Configure here.
2 participants
🌟 What is the purpose of this PR?
Two changes to Petrinaut in HASH:
Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🛡 What tests cover this?
❓ How to test this?
📹 Demo