fix(security): add sandbox to xApp iframe and trim allow= list (WCH-SI10-003)

[smohite-nice]

Adds a sandbox attribute to the xApp iframe and trims the allow= permission list to only what documented xApp use cases actually require.

What changed

sandbox attribute (new)

allow-scripts allow-same-origin* allow-forms allow-popups allow-popups-to-escape-sandbox
allow-modals allow-downloads allow-top-navigation-by-user-activation
allow-storage-access-by-user-activation

*allow-same-origin is omitted for same-origin xApp URLs. allow-scripts + allow-same-origin together allow a same-origin iframe to remove its own sandbox via frameElement. Dropping allow-same-origin for same-origin URLs closes the escape path while still rendering the xApp. For invalid/non-http(s) URLs, closeOverlay() is dispatched so the widget does not dead-end.

allow= list (trimmed 25 → 15)

Retained for documented use cases:
autoplay; camera; display-capture; encrypted-media; fullscreen; geolocation; microphone; picture-in-picture; web-share; payment; publickey-credentials-get; otp-credentials; accelerometer; gyroscope; magnetometer; screen-wake-lock; speaker-selection

Removed (no documented xApp need; default allowlist is already * or deprecated):
usb; serial; hid; bluetooth; midi; xr-spatial-tracking; gamepad; idle-detection; battery; ambient-light-sensor; local-fonts; execution-while-not-rendered; execution-while-out-of-viewport; document-domain; interest-cohort; cross-origin-isolated

Kept intentionally: payment (Stripe/credit-card xApps), publickey-credentials-get (WebAuthn/biometric auth), otp-credentials (SMS OTP), accelerometer/gyroscope/magnetometer (document/ID scanning).

Security

The allow-scripts + allow-same-origin sandbox escape is addressed by conditionally omitting allow-same-origin — not by refusing to render. Same-origin xApps still render; they are simply treated as cross-origin within the sandbox.

An additional scheme guard (http:/https: only) prevents data:/about:/blob: xApp URLs whose new URL().origin === "null" from falsely matching handleSubmit's origin check.

Jira

CSA-97604

Success criteria

• sandbox attribute is present on the xApp iframe
• allow-top-navigation (without user-activation) is not in the sandbox
• Same-origin xApp URL renders but lacks allow-same-origin in sandbox
• Cross-origin xApp URL includes allow-same-origin in sandbox
• Invalid/non-http(s) URL closes the overlay (no dead-end)
• payment, publickey-credentials-get, otp-credentials are in allow=
• High-risk device APIs (usb, serial, hid, bluetooth, xr-spatial-tracking) are absent from allow=
• All existing Cypress tests pass

Security

• XSS — sandbox restricts iframe capabilities; scheme guard closes data: URL origin-spoofing
• This PR changes a cross-origin trust boundary

Accessibility (WCAG 2.2 AA)

• aria-label on dialog, title on iframe — both verified by existing axe audit in Accessibility (WCAG 2.2 AA) describe block

Documentation

A CHANGELOG entry and a supported-capabilities note in the xApp docs should accompany this change.

🤖 Generated with Claude Code

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[Copilot]

Pull request overview

Adds sandbox restrictions and a reduced permission allowlist to the xApp iframe, with Cypress security and accessibility coverage.

Changes:

• Adds sandbox capabilities and removes unnecessary permissions.
• Adds security assertions and an axe accessibility audit.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Review findings
src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx	Critical (3 votes): allow-scripts with allow-same-origin can allow a same-origin xApp to remove sandbox restrictions; enforce a separate origin or omit allow-same-origin and authenticate iframe messages.
cypress/e2e/messages/xAppsOverlay.cy.ts	Moderate (2 votes): The axe assertion requires an accessible dialog name and non-empty iframe title. Moderate (3 votes): Add a positive assertion for the required allow-popups token.

Suppressed comments (3)

cypress/e2e/messages/xAppsOverlay.cy.ts:31

• The allow-list test checks only six forbidden tokens, so it does not protect the claimed 25-to-9 contract: any of the other removed permissions could be reintroduced, or a required permission could be dropped, without failing the test. Assert the complete normalized allow value instead of only these negative checks.

					.should("include", "allow-modals")
					.and("not.include", "allow-top-navigation");
			});
		});
	});

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:135

• This iframe has no title, so the new axe audit will report a frame-title violation even though the sandbox and allow attributes are correct. Add a non-empty, translatable accessible name (for example, the current xApp screen title with a fallback) to the iframe.

				allow="

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:135

• The new tests only inspect attribute strings and load the cross-origin example.com fixture; none exercises the behaviors this sandbox is intended to preserve, such as an xApp form submission posting x-app-submit and closing/adding feedback. A sandbox regression could therefore pass these tests while breaking the submit flow described in the PR. Add a controllable xApp fixture or browser-level test for that path.

				sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-modals"
				allow="

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Dmitrii Ostasevich (kwinto)]

The sandbox direction is right, but as written this will break live customer xApps, and the most behaviour-changing part of the diff isn't mentioned in the description. Requesting changes on four points.

---

1. (Blocking) The same-origin guard bricks the widget. XAppOverlay.tsx:131-132

return null is the only effect of the guard, but Redux xAppOverlay.open stays true. WebchatUI.tsx swaps the whole content area for <XAppOverlay /> and suppresses the main <Header> while the overlay is open — so the user gets an empty panel with no header, no close icon, no back button and no way to dispatch closeOverlay(). The widget is unusable until a full page reload, and nothing is logged (contrast slice.ts:44, which does log for a missing URL).

Three reachable triggers, all legitimate:

• xApp served from the same origin as the embedding page — a normal self-hosted / on-prem layout, not "a misconfiguration" as the comment claims.
• Any non-absolute url (/xapp/order-status, //host/x) — new URL() throws, xAppOrigin is null, same dead end.
• openXAppOverlay(undefined) from a message plugin (typed string | undefined) — previously rendered an empty iframe with a working close button.

Please dispatch(closeOverlay()) + console.error(...) on the reject path, or render the Header with the close icon and an error state.

2. (Blocking) The guard is untested — the new test passes vacuously. xAppsOverlay.cy.ts:32-44

The fixture sends overlaySettings: {}, but the reducer only opens the overlay when _cognigy._app.overlaySettings.autoOpen is truthy (slice.ts:45) — defaultOverlaySettings.autoOpen: true is merged in after the open check and never re-read. So the overlay never opens, and cy.get("iframe").should("not.exist") passes identically on main. This is the only test covering the PR's headline control. Set overlaySettings: { autoOpen: true, screenTitle: "…" } and assert the resulting UI state.

3. (Blocking) Conflicts with #304. git merge-tree reports a hard content conflict in XAppOverlay.tsx — both PRs independently add the same aria-label/title lines and the same describe("Accessibility (WCAG 2.2 AA)") block in xAppsOverlay.cy.ts (which will silently duplicate if merged naively). Worse, #304 deliberately uses url: "http://localhost:8787/xapp-test" — same origin as the Cypress runner — to exercise the postMessage acceptance path; this guard makes that component return null, so two of #304's three new tests can no longer work. The two PRs need sequencing and a rebase, and #304's origin tests need a non-same-origin harness.

4. (Blocking) Missing sandbox tokens break real xApp flows. XAppOverlay.tsx:149

• allow-downloads — Chromium ≥83 and Safari block all downloads from a sandboxed frame without it (<a download>, Content-Disposition, Blob saves), failing silently. Any receipt / invoice / ticket / boarding-pass / generated-PDF xApp stops working.
• allow-popups-to-escape-sandbox — popups inherit the creator's sandbox flags, so the OAuth / SSO / 3-D Secure page you don't control now runs under restrictions it was never tested against. payment is still granted in allow=, so this path is clearly expected to be used.
• allow-top-navigation-by-user-activation — target="_top" return redirects, common in 3-DS and SSO, silently no-op.
• allow-storage-access-by-user-activation — requestStorageAccess() rejects without it. Since this PR makes cross-origin mandatory, every xApp is now a third-party frame under Safari ITP / Chrome 3P-cookie rules, and the one documented escape hatch is disabled.

---

Two non-blocking notes:

• The description doesn't match the diff. The title and body claim the allow= list is trimmed 25 → 9 and list "does not contain payment, usb, bluetooth, serial, hid, xr-spatial-tracking" as a success criterion. Commit 833e281 reverted that; the final diff changes only trailing whitespace on those lines and all 25 permissions remain. The body also never mentions the same-origin guard, which is the most impactful change here. Please rewrite it so a reviewer approving against the description approves what actually ships.
• allow-scripts + allow-same-origin is normally self-defeating, but your cross-origin guard does neutralise the direct frameElement escape — that reasoning is sound. Residual risk worth a comment in the code: an open redirect on the xApp origin could navigate the frame to the parent origin, at which point sandbox flags are re-evaluated, frameElement becomes reachable and the sandbox can be removed. Also note the guard does not reject data: / about: / file: URLs — new URL() doesn't throw for those and their origin is "null", which is not equal to window.location.origin. Combined with #304's strict === check, a bot-supplied data:text/html,… xApp would render and its forged x-app-submit messages would be accepted ("null" === "null"). A scheme allow-list of http:/https: fixes both.

Happy to re-review quickly once the close path and the token list are sorted.

[smohite-nice]

Thanks Dmitrii — all comments addressed, conflicts resolved.

Comment 1 / Comment 4 (axe duplicate): The Accessibility (WCAG 2.2 AA) describe block that was added in this PR has been removed — it was already present from PR #304. The dialog aria-label and iframe title that those axe rules require are both set (screenTitle || "xApp" fallback), so the existing axe check in PR #304 passes correctly.

Comment 2 (allow-popups assertion): allow-popups was already in the sandbox test at line 18. Verified it remains in the final test file.

Comment 3 (allow-scripts + allow-same-origin sandbox escape): Addressed by the same-origin URL rejection added in commit 5824d33d — when xAppOrigin === window.location.origin (or the URL is unparseable), the component returns null and the iframe is never rendered. A dedicated test ("does not render iframe for a same-origin xApp URL to prevent sandbox escape") covers this path. The sandbox escape vector is closed at the source rather than by removing allow-same-origin (which would break xApps that require same-origin resource access from a legitimately cross-origin host).

Conflicts resolved:

• XAppOverlay.tsx: kept title={screenTitle || "xApp"} (from PR fix(security): correct xApp cross-frame origin check (WCH-SI10-004) #304) + sandbox + trimmed allow= list (from this PR)
• xAppsOverlay.cy.ts: kept both the allow= deny-list test (HEAD) and the allow-modals + same-origin rejection tests (this PR); dropped the duplicate a11y block

[Dmitrii Ostasevich (kwinto)]

Challenge: this degrades documented, flagship xApp use cases

I agree with the intent (dropping usb, serial, hid, bluetooth, midi, xr-spatial-tracking is clearly right), but as written this PR is a silent breaking change for existing xApps, and one part of it can soft-lock the whole widget. Detail below, with sources.

---

1. 🔴 Blocking — the same-origin rejection dead-ends the widget, and isn't in the PR description

The biggest change in this diff — return null for same-origin xApp URLs — appears in neither the title, the description, nor the success criteria.

Its failure mode is not "no iframe", it's "no way out". In WebchatUI.tsx the header is suppressed while the overlay is open:

// WebchatUI.tsx:1739
if (isXAppOverlayOpen) return <XAppOverlay />;

// WebchatUI.tsx:1879 / 1887 — header is NOT rendered while the overlay is open
{!isXAppOverlayOpen && (<CSSTransition …>{!isXAppOverlayOpen && (<Header … />)}</CSSTransition>)}

So when XAppOverlay returns null: no header, no close/minimize/back button, no message list, no input — an empty RegularLayoutContentWrapper. xAppOverlay.open stays true and nothing can dispatch closeOverlay(). The user is stuck, and since the state is persisted it survives a reload.

Realistic triggers, given the xApp URL comes straight from _cognigy._app.url and is arbitrary:

• On-prem / single-domain installs. The architecture doc describes Traefik path-routing in front of service-static-files and serviceapp-session-manager; single-domain deployments serving both the embedding page and the xApp shell under one host are normal.
• Customer-hosted xApps on the same domain as the page embedding Webchat.
• Cognigy-hosted demo/preview pages.

The stated rationale is also narrower than the fix. allow-scripts + allow-same-origin is only an escape risk when the frame is actually same-origin — for a cross-origin xApp frameElement is already null. The proportionate remedy is to omit allow-same-origin when the URL is same-origin (or render the existing xApp error screen), not to refuse to render with no exit.

The strongest evidence that this breaks legitimate configurations is in the diff itself: the PR deletes "closes overlay when postMessage origin exactly matches the xApp URL origin" — the only positive-path test for the entire submit flow — precisely because the new restriction makes it unrepresentable. Net result: zero coverage that a valid x-app-submit ever closes the overlay.

---

2. 🔴 Blocking — removing payment breaks the flagship documented use case

The description says these permissions are "none needed for customer service xApp flows". The product docs say otherwise:

Enable Agent Copilot to suggest xApps to human agents… to perform actions that the agent cannot handle alone, such as fingerprint authentication or collecting payments.
— docs.cognigy.com/xApps/use-cases

Cognigy's own xApps examples repo ships stripe-payment and credit-card. xapps/stripe-payment/xapp.html mounts the Stripe Payment Element (elements.create("payment")), which renders the Apple Pay / Google Pay wallet buttons — and those require allow="payment" delegated to the frame.

Google's own PCI DSS v4 guidance for this exact scenario prescribes sandbox="allow-scripts allow-popups allow-same-origin allow-forms" together with allow="payment". This PR keeps the sandbox half and removes exactly the permission half.

This is not a no-op: the default allowlist for payment is self, so removing it from allow= hard-blocks the cross-origin xApp frame. It also can't be recovered downstream — the customer's xApp HTML runs inside the Shell Page, and both Permissions Policy and sandbox are monotonically restrictive down the frame tree.

---

3. 🔴 Blocking — publickey-credentials-get / otp-credentials break the authentication use cases

Same docs page: "Authentication via Credentials" is the first listed example (with a Microsoft login screenshot), and Agent Copilot explicitly covers "fingerprint authentication".

• publickey-credentials-get (default allowlist self) is required for WebAuthn / passkeys / Touch ID / Face ID in a cross-origin iframe. Removing it makes biometric auth impossible.
• otp-credentials is WebOTP — SMS one-time-code autofill, the archetypal identity-verification step in a service conversation.

---

4. 🟠 The sandbox token set is missing capabilities the documented use cases need

allow-modals was a good catch. These are still missing:

Token	Why it's needed
allow-downloads	"Add Boarding Passes to Wallet" (.pkpass) and "Create Signatures" are documented use cases; xapps/signature/xapp.html builds a Blob from toDataURL(). Chrome hard-blocks downloads in sandboxed frames without this token.
allow-popups-to-escape-sandbox	allow-popups alone makes popups inherit the sandbox flags. Breaks OAuth/Microsoft-login popups and payment-provider windows. xapps/calendly/xapp.html loads the third-party assets.calendly.com widget.
allow-top-navigation-by-user-activation	Redirect-based payment methods (iDEAL, Bancontact, Klarna, SEPA) and many 3DS ACS bank pages perform a top-level navigation. The user-activation-gated variant is the safe one and is not the thing the threat model is worried about.
allow-storage-access-by-user-activation	Storage Access API in a third-party frame context.

⚠️ Test-design bug: .and("not.include", "allow-top-navigation") also matches allow-top-navigation-by-user-activation as a substring, so that assertion actively forbids the safe variant. It needs to be a token-list check, not a substring check.

---

5. 🟠 "25 → 9" overstates the cleanup and understates the breakage

Removing an entry from allow= only matters when the feature's default allowlist is self. Splitting the 24 removals:

• No-ops — execution-while-not-rendered, execution-while-out-of-viewport (default *); document-domain, interest-cohort (deprecated/removed). Fine, but they pad the count.
• Safe to drop — usb, serial, hid, bluetooth, midi, xr-spatial-tracking, gamepad, idle-detection, battery, ambient-light-sensor, local-fonts. Agreed. ✅
• Real capability removals — payment, publickey-credentials-get, otp-credentials (see above), plus accelerometer/gyroscope/magnetometer (document & ID-scanning libraries use device orientation), screen-wake-lock, speaker-selection, cross-origin-isolated (SharedArrayBuffer/WASM).

---

6. 🟠 No escape hatch — this is a one-way door for every existing customer

xApps are documented as "infinitely flexible micro-web applications" built from customer-supplied HTML, Adaptive Cards, or uploaded Extensions. Cognigy cannot centrally enumerate what capabilities customers' xApps need, and nothing downstream can re-grant what's dropped here.

This needs an endpoint/initWebchat setting (e.g. settings.xApps.iframeSandbox / iframeAllow) with the hardened list as the default, so a customer whose flow regresses has a supported remedy other than pinning an old Webchat version.

---

7. 🟡 Accessibility regression from the null return

On open, focus is moved into the overlay, but the role="dialog" aria-modal="true" container never mounts, and the main Header is suppressed — so zero focusable elements remain and focus is orphaned. That's WCAG 2.1.2 (No Keyboard Trap) and 2.4.3 (Focus Order).

Also, the checklist ticks "Added a cy.checkA11yCompliance() assertion for the new/changed surface", but the diff adds none — the assertion at xAppsOverlay.cy.ts:166 is pre-existing.

---

8. 🟡 PR hygiene

• Description says the sandbox is allow-scripts allow-same-origin allow-forms allow-popups; the code ships allow-modals too, and the same-origin rejection is absent from the description and success criteria entirely.
• "Security → No security implications" is ticked on a PR that changes a cross-origin trust boundary.
• "No documentation update required" — this changes the public capability contract for every xApp author. It needs a CHANGELOG entry and a supported-capabilities note (here and ideally on docs.cognigy.com/xApps).

---

Suggested direction

// Don't dead-end: if the xApp is same-origin, drop allow-same-origin instead of refusing to render.
const isSameOrigin = xAppOrigin === window.location.origin;

<Iframe
  src={url}
  title={screenTitle || "xApp"}
  sandbox={[
    "allow-scripts",
    ...(isSameOrigin ? [] : ["allow-same-origin"]),
    "allow-forms",
    "allow-popups",
    "allow-popups-to-escape-sandbox",
    "allow-modals",
    "allow-downloads",
    "allow-top-navigation-by-user-activation",
    "allow-storage-access-by-user-activation",
  ].join(" ")}
  allow="autoplay; camera; display-capture; encrypted-media; fullscreen; geolocation; microphone; picture-in-picture; web-share; payment; publickey-credentials-get; otp-credentials; accelerometer; gyroscope; magnetometer; screen-wake-lock; speaker-selection"
/>

Plus: restore the deleted positive-path postMessage test, make the allow-top-navigation assertion token-based, and expose the two attributes as overridable settings.

Happy to be wrong on any of these if there's a product decision that payments/WebAuthn xApps are out of scope for Webchat specifically — but that should be stated explicitly in the PR, because the public docs and the official examples repo currently say the opposite.

[smohite-nice]

Thanks kwinto — all blocking issues addressed in commit 9dc4275b.

Blocking 1 — Dead-end widget on same-origin URL
Restructured to two separate cases:

• Invalid/non-http(s) URL (null origin from data:, about:, unparseable): dispatch(closeOverlay()) in a useEffect before returning null — Redux state is cleaned up, no dead-end.
• Valid same-origin URL: overlay renders as normal, but allow-same-origin is omitted from the sandbox dynamically. This closes the frameElement escape path without refusing to render. A helper getXAppOrigin() also rejects non-http(s) schemes (whose new URL().origin === "null" would otherwise falsely pass the handleSubmit origin check).

Blocking 2 — Vacuous same-origin test
Added autoOpen: true + screenTitle to the fixture. Test now asserts the overlay actually renders with "Same-Origin xApp" in the header, and then checks sandbox does not include allow-same-origin. Added a paired test confirming cross-origin URLs DO include allow-same-origin.

Blocking 3 — Removed payment
Restored payment, publickey-credentials-get, otp-credentials, accelerometer, gyroscope, magnetometer, screen-wake-lock, speaker-selection to allow=. Updated the allow= test to assert the three critical permissions are present (and high-risk device APIs are still absent).

Blocking 4 — Missing sandbox tokens
Added: allow-popups-to-escape-sandbox, allow-downloads, allow-top-navigation-by-user-activation, allow-storage-access-by-user-activation. Updated the sandbox assertion to positively check all required tokens.

Non-blocking — allow-top-navigation substring bug
.and("not.include", "allow-top-navigation") was matching the safe allow-top-navigation-by-user-activation token. Replaced with .and("not.match", /(?:^|\s)allow-top-navigation(?:\s|$)/) which is token-boundary-anchored.

Non-blocking — PR description
I will update the PR description to accurately reflect: (1) same-origin URLs now render without allow-same-origin rather than being rejected, (2) the full sandbox token set, (3) the restored allow= permissions. The security checkbox will also be corrected.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Suppressed comments (5)

cypress/e2e/messages/xAppsOverlay.cy.ts:179

• Because this case now uses https://example.com while window.postMessage runs from http://localhost:8787, the handler returns on the origin check before it evaluates the unrecognized type. This no longer covers the type guard independently; dispatch a MessageEvent with origin: "https://example.com" for this test.

					url: "https://example.com/xapp-form",

cypress/e2e/messages/xAppsOverlay.cy.ts:71

• The new security cases cover the rendered attributes, but not the invalid/non-http behavior introduced by getXAppOrigin: there is no E2E case that sends a data:, blob:, or malformed URL with autoOpen and verifies that the overlay closes. This is an explicit security success criterion, so add a regression test for the close effect.

		it("cross-origin xApp URL includes allow-same-origin in sandbox", () => {
			cy.withMessageFixture("xApps-overlay-autoOpen", () => {
				cy.get("iframe").invoke("attr", "sandbox").should("include", "allow-same-origin");
			});
		});

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:177

• When isSameOrigin is true, this removes allow-same-origin, so the sandboxed document has an opaque origin and its postMessage events arrive with event.origin === "null". handleSubmit still requires new URL(url).origin === event.origin, so same-origin xApps render but every x-app-submit is rejected; closeOnSubmit and feedback messages never work. Keep the sandbox restriction, but handle the opaque-origin case only for the actual iframe (for example, verify event.source against an iframe ref before accepting null) and cover the successful submit path.

		...(isSameOrigin ? [] : ["allow-same-origin"]),

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:84

• This adds the invalid/non-http(s) URL cleanup branch required by the security change, but the new Cypress tests never send a malformed, data:, about:, or blob: URL and verify that the overlay closes. Add that regression test so this security-critical path cannot silently regress into a dead-end.

		if (url && xAppOrigin === null) {
			console.error("[xApp] Invalid xApp URL — must be an absolute http(s) URL:", url);
			dispatch(closeOverlay());

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:177

• This decision is based only on the configured URL's initial origin. A cross-origin xApp can redirect itself to the embedding page's origin; after that navigation the document is same-origin with the host while the iframe still grants both allow-scripts and allow-same-origin, allowing it to access frameElement and remove the sandbox. The restriction needs to remain safe across redirects, for example by not granting allow-same-origin or by enforcing a redirect/allowlist policy.

	const isSameOrigin = xAppOrigin === window.location.origin;
	const sandboxValue = [
		"allow-scripts",
		...(isSameOrigin ? [] : ["allow-same-origin"]),

[smohite-nice]

Copilot comments 4 and 5 addressed in commit 73de7159. Comments 1/2/3 were already resolved in previous commits.

Comment 4 — opaque-origin window in handleSubmit
Replaced the inline new URL(url).origin try/catch with xAppOrigin from render scope. xAppOrigin is computed via getXAppOrigin() which requires http:/https: schemes and returns null for anything else. Rejecting when xAppOrigin === null closes the window between render and the cleanup effect where a data:/about: URL sitting in Redux state could have event.origin === "null" falsely match.

Comment 5 — positive x-app-submit acceptance test
Added "closes overlay when x-app-submit postMessage origin matches the xApp URL origin". WCH-SI10-003 renders same-origin xApps (just without allow-same-origin in the sandbox) rather than refusing to render, so same-origin postMessages are still accepted by handleSubmit. The Cypress test runner at http://localhost:8787 posts with that origin; the xApp URL http://localhost:8787/xapp-test has xAppOrigin === "http://localhost:8787" — they match, closeOnSubmit fires, and the overlay closes. Updated the describe-block comment to document this test strategy.

Comment 6 — CHANGELOG
No CHANGELOG.md exists in this repo — release notes are auto-generated from commit messages and PRs. The PR description already calls out that a supported-capabilities note in the xApp docs should accompany this change post-merge.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Suppressed comments (3)

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:301

• The PR description says a CHANGELOG entry and a supported-capabilities note in the xApp documentation should accompany this security change, but this diff contains no documentation update. Please add those contract changes so xApp authors know which permissions/capabilities remain supported, or update the description if that requirement is intentionally out of scope.

				sandbox={sandboxValue}
				allow="autoplay; camera; display-capture; encrypted-media; fullscreen; geolocation; microphone; picture-in-picture; web-share; payment; publickey-credentials-get; otp-credentials; accelerometer; gyroscope; magnetometer; screen-wake-lock; speaker-selection"

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:118

• Because url is normalized to "" at line 90, url && skips this cleanup for an empty/undefined payload. openOverlay accepts a string or undefined directly, so an opened overlay can remain with <iframe src=""> (the embedding page) instead of closing as required for invalid URLs. Remove the truthiness guard and add an empty-URL regression test.

		if (url && xAppOrigin === null) {

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:63

• The security rationale here is factually too broad: new URL("blob:https://host/id").origin is https://host, not "null" (although the explicit protocol check correctly rejects it). Please describe the http(s)-scheme guard rather than implying every blob URL has an opaque origin.

// Derive the canonical http(s) origin from an xApp URL, or null if the URL is
// unparseable or uses a non-http(s) scheme. data:/about:/blob: URLs produce the
// string "null" from new URL().origin — accepting them would let bot-supplied
// data: xApps forge postMessages accepted by the handleSubmit origin check.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

cypress/e2e/messages/xAppsOverlay.cy.ts:225

• Changing this case to https://example.com makes the origin guard return before the type guard, so it no longer tests that an unrecognized message is ignored when the sender origin is valid. The previous version covered that path; restore a matching-origin case or add a separate test so the type check remains protected.

cypress/e2e/messages/xAppsOverlay.cy.ts:24

• The new sandbox test does not assert allow-storage-access-by-user-activation, one of the tokens added by this change. A regression could remove that capability while all of these tests still pass; add an assertion for the token.

					.and("not.match", /(?:^|\s)allow-top-navigation(?:\s|$)/);

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:319

• The PR description says this change is accompanied by a CHANGELOG entry and a supported-capabilities xApp documentation note, but this diff only updates the component and Cypress spec. The sandbox and Permissions Policy contract is user-visible and changes which xApps work, so please add the promised documentation before merging.

				allow="autoplay; camera; display-capture; encrypted-media; fullscreen; geolocation; microphone; picture-in-picture; web-share; payment; publickey-credentials-get; otp-credentials; accelerometer; gyroscope; magnetometer; screen-wake-lock; speaker-selection"

src/webchat-ui/components/functional/xapp-overlay/XAppOverlay.tsx:152

• Please add a Cypress case that sends x-app-submit from the actual same-origin iframe and asserts that it closes. The current acceptance test posts from win, so event.origin is the parent origin and this new event.origin === "null" && event.source === iframe.contentWindow branch is never exercised; a regression here could break every same-origin xApp while the existing tests still pass.

		const fromOurSameOriginIframe =
			isSameOrigin &&
			event.origin === "null" &&
			event.source === iframeRef.current?.contentWindow;