Skip to content

Security: Unsafe HTML iframe sandbox configuration allows same-origin access#1223

Merged
jaylfc merged 1 commit into
jaylfc:masterfrom
tomaioo:fix/security/unsafe-html-iframe-sandbox-configuration
Jun 20, 2026
Merged

Security: Unsafe HTML iframe sandbox configuration allows same-origin access#1223
jaylfc merged 1 commit into
jaylfc:masterfrom
tomaioo:fix/security/unsafe-html-iframe-sandbox-configuration

Conversation

@tomaioo

@tomaioo tomaioo commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Security: Unsafe HTML iframe sandbox configuration allows same-origin access

Problem

Severity: High | File: desktop/src/apps/BrowserApp/LiveBrowserView.tsx:L22

The LiveBrowserView component uses sandbox="allow-scripts allow-same-origin allow-forms" on an iframe. The combination of allow-scripts and allow-same-origin effectively removes most sandbox protections, allowing the embedded content to access the parent's cookies, storage, and DOM if same-origin. Additionally, allow-forms enables form submission. The stream token is passed via URL fragment, which while keeping it out of server logs, is still accessible to JavaScript in the iframe. If the nekoUrl is ever same-origin or becomes same-origin through DNS hijacking, this creates a significant attack surface.

Solution

Remove allow-same-origin from the sandbox attribute unless absolutely required. If cross-origin communication is needed, use postMessage with strict origin validation instead. Consider using allow-same-origin only with allow-scripts removed, or use a more restrictive sandbox. If allow-same-origin is required for functionality, document the security trade-off and ensure the nekoUrl is always on a separate, dedicated origin with no sensitive cookies.

Changes

  • desktop/src/apps/BrowserApp/LiveBrowserView.tsx (modified)

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced security of the embedded browser component by restricting cross-origin access permissions, reducing potential security vulnerabilities.

The LiveBrowserView component uses `sandbox="allow-scripts allow-same-origin allow-forms"` on an iframe. The combination of `allow-scripts` and `allow-same-origin` effectively removes most sandbox protections, allowing the embedded content to access the parent's cookies, storage, and DOM if same-origin. Additionally, `allow-forms` enables form submission. The stream token is passed via URL fragment, which while keeping it out of server logs, is still accessible to JavaScript in the iframe. If the nekoUrl is ever same-origin or becomes same-origin through DNS hijacking, this creates a significant attack surface.

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@github-actions

Copy link
Copy Markdown

👋 Thanks for the PR! This one targets master, which is our
stable branch (it's what live installs track). Please retarget it to
dev — click Edit next to the PR title and change the base
branch dropdown from master to dev. Your commits and any review
carry over, nothing is lost.

See CONTRIBUTING.md for the branch model.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 75fc3b65-e5ff-4e22-b6c0-b2ae413fef00

📥 Commits

Reviewing files that changed from the base of the PR and between 4c9778d and 734cabe.

📒 Files selected for processing (1)
  • desktop/src/apps/BrowserApp/LiveBrowserView.tsx

📝 Walkthrough

Walkthrough

The sandbox attribute on the iframe in LiveBrowserView has allow-same-origin removed, leaving only allow-scripts allow-forms. This is a single-line change with no other modifications.

Changes

iframe Sandbox Hardening

Layer / File(s) Summary
Remove allow-same-origin from iframe sandbox
desktop/src/apps/BrowserApp/LiveBrowserView.tsx
The iframe sandbox attribute is changed from allow-scripts allow-same-origin allow-forms to allow-scripts allow-forms, removing the allow-same-origin token.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐇 Hop hop, the sandbox gate is sealed,
No same-origin path revealed,
One token gone, the fence stands tall,
A single line to guard it all,
The rabbit nods — that's all it takes! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing allow-same-origin from iframe sandbox configuration to fix a security vulnerability.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gitar-bot

gitar-bot Bot commented Jun 20, 2026

Copy link
Copy Markdown

Note

Your trial team has used its Gitar budget, so automatic reviews are paused. Upgrade now to unlock full capacity. Comment "Gitar review" to trigger a review manually.
Learn more about usage limits

Code Review ✅ Approved

Removes the unsafe 'allow-same-origin' directive from the iframe sandbox configuration in LiveBrowserView to prevent unauthorized access to parent cookies and DOM. This hardens the browser component against potential cross-origin attacks.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Important

Your trial ends in 6 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

jaylfc added a commit that referenced this pull request Jun 20, 2026
…ifest/install-registry) + neko #1223 blocked on #71 Pi browser node
@jaylfc

jaylfc commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Thanks for this, good catch and a fair hardening. We brought up a live Neko session and traced the client: auth runs through URL query params into the WebSocket handshake, with no cookie or same-origin dependency on the connection path, so dropping allow-same-origin keeps login and the WebRTC stream working. The only effect is that a couple of non-essential UI preferences stop persisting, which is an acceptable trade for the tighter sandbox. Merging it. Appreciate the contribution.

@jaylfc jaylfc merged commit 01d4f04 into jaylfc:master Jun 20, 2026
3 of 4 checks passed
jaylfc added a commit that referenced this pull request Jun 20, 2026
…, tested, merged), white-screen diagnosed (not #1223, pre-existing render), next = browser redesign #66 + render fix #71
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