Security hardening for browser boundary, SSRF, FS bridge, profile isolation, exports, and tokens

[Zurgli]

This PR bundles six security hardening changes discovered during a review of wmux.

I did not find a CONTRIBUTING file or other contributor guidance in the repository, so I was not sure whether you prefer one larger security hardening PR or several smaller ones. I grouped the changes here so you can review them together and decide what you want to take.

Original issues identified

• Raw CDP passthrough bypassed higher-level browser safety checks. An authenticated client could issue arbitrary DevTools commands and bypass navigation restrictions.
• SSRF protections only validated URL strings and did not enforce post-resolution checks on the actual target addresses.
• The renderer filesystem bridge relied on path resolution without canonicalizing symlinks, which allowed denylist bypasses.
• Browser profile isolation was not real in the renderer because all browser surfaces shared a single persistent partition.
• Browser/MCP export helpers accepted arbitrary output paths and could write outside a controlled directory.
• Local auth token handling was inconsistent on Windows: one token path was ACL-hardened, another relied on Unix-style mode bits that Windows ignores.

What this PR changes

1. Browser boundary hardening

• Removes the public raw Browser.cdp.send passthrough from the exposed RPC surface.
• Replaces the needed navigation behavior with a reviewed Browser.goBack path.
• Narrows �rowser.cdp.info to the metadata Playwright still needs.

2. SSRF enforcement

• Adds resolved-address validation in the main process at the actual navigation boundary.
• Rejects private, link-local, loopback, and null targets after hostname resolution instead of trusting only the input string.

3. Filesystem bridge hardening

• Canonicalizes paths with
  ealpath before policy checks.
• Applies sensitive-path checks to the real target, preventing symlink bypasses.
• Extends protected paths to cover the daemon token as well.

4. Browser profile isolation

• Carries actual browser partition data through the browser surface model.
• Uses the real per-surface partition in the renderer instead of a shared hardcoded partition.

5. Export path restrictions

• Restricts browser PDF/trace exports to a controlled export root.
• Rejects absolute paths and traversal outside that root.

6. Token hardening

• Introduces a shared secure token writer.
• Applies the same Windows ACL hardening to both the daemon token and the MCP token.

Development history in my fork

These changes were developed and reviewed as six separate PRs in my fork before being bundled here:

• 1 Harden browser RPC boundary
• 2 Harden browser navigation SSRF checks
• 3 Harden renderer filesystem bridge
• 4 Wire real browser profile partitions
• 5 Restrict browser export output paths
• 6 Harden local auth token permissions

If you prefer, I can also split this back out into smaller upstream PRs that match those six threads.

[openwong2kim]

PR #1 Review — Security Hardening

Thanks for the thorough security review, @Zurgli. The six hardening areas are well-identified and the fixes are structurally sound. I've done a code-level review of every changed file and have feedback below, organized into blocking items (must fix before merge) and non-blocking suggestions.

---

Blocking

1. Scope: daemon persistence/recovery work should be a separate PR

This PR adds ~870 lines to src/daemon/ — StateWriter.ts (202 lines, new), recoverSessions() (~150 lines), RingBuffer expansion, buildState() wiring, and buffer snapshot logic in daemon/index.ts. These are feature additions (session persistence and crash recovery), not security hardening.

Mixing feature work into a security PR creates two problems:

• It makes the security changes harder to audit in isolation.
• If we need to cherry-pick or backport the security fixes, the daemon feature comes along for the ride.

Ask: Please split the daemon persistence/recovery changes into a separate PR. The security-scoped changes in DaemonPipeServer.ts (switching to secureWriteTokenFile) can stay here — that's a 7-line diff and clearly security-related.

2. IPv6-mapped IPv4 bypass in SSRF validation

validateIpv6Address in navigationPolicy.ts checks for fc00::/7 (private) and fe80::/10 (link-local) but does not handle IPv6-mapped IPv4 addresses. An attacker can bypass the check with:

::ffff:169.254.169.254   → passes IPv6 validator (not fc00 or fe80)
::ffff:10.0.0.1          → same bypass
::ffff:192.168.1.1       → same bypass

Fix: After expanding the IPv6 address, check if it's in ::ffff:0:0/96 (groups 0–4 are zero, group 5 is ffff). If so, extract the last two groups as an IPv4 address and run it through validateIpv4Address.

// After expanding, before other checks:
if (expanded.slice(0, 5).join(':') === '0000:0000:0000:0000:ffff') {
  const hi = parseInt(expanded[6], 16);
  const lo = parseInt(expanded[7], 16);
  const ipv4 = `${hi >> 8}.${hi & 0xff}.${lo >> 8}.${lo & 0xff}`;
  return validateIpv4Address(ipv4);
}

Please also add a test case for this in navigationPolicy.test.ts.

3. Token ACL failure should throw, not warn

In src/shared/security.ts, secureWriteTokenFile catches icacls failures and logs console.warn, then returns normally:

} catch (aclErr) {
  console.warn('[secureWriteTokenFile] Could not set file ACL:', aclErr);
}

This means on Windows, if ACL hardening fails, the token file remains readable by other users on the machine — silently. For a security-critical auth token, this should be a hard failure.

Fix: Re-throw after logging, or at minimum delete the token file and throw so the caller knows the token was not securely written.

} catch (aclErr) {
  // Token file exists with default ACLs — not safe to leave it.
  try { fs.unlinkSync(filePath); } catch { /* ignore */ }
  throw new Error(`Failed to set secure ACL on ${filePath}: ${aclErr}`);
}

Update the test in security.test.ts to verify this failure behavior.

---

Non-blocking (suggestions for follow-up)

4. DNS rebinding window (SSRF)

The current flow is: lookup() → validateResolvedAddress() → loadURL(). Between validation and the actual connection, DNS could return a different IP (classic DNS rebinding). This is acknowledged in the original code's comment and your PR improves it significantly, but the TOCTOU window still exists.

For a future iteration, consider pinning the resolved IP and passing it directly to the navigation layer, or using a custom DNS resolver that caches the validated result for the duration of the request. Not blocking this PR since the improvement is already substantial.

5. Filesystem bridge TOCTOU

resolveAccessiblePath calls realpath() then the caller separately does readdir()/readFile(). Between these two operations, a symlink target could change. This is a narrow window and hard to exploit in practice, but worth noting.

Future improvement: open with O_NOFOLLOW or use fd-based operations where possible. Not blocking.

6. BrowserPanel remount on partition change

In Pane.tsx, the key changed to ${surface.id}:${surface.browserPartition}. This means when a session profile changes, the entire <webview> is destroyed and recreated — losing scroll position, form input state, in-progress downloads, etc.

This is the correct security behavior (you can't change a webview's partition after creation), but it's a UX regression worth documenting. Consider showing a brief notification to the user when this happens so they understand why their browser state reset.

7. browser.goBack — 300ms sleep

In navigation.ts, after calling browser.goBack there's a hardcoded setTimeout(resolve, 300) before reading the URL. This is fragile — on slow pages it may read the old URL, on fast pages it's unnecessary latency.

Consider using Electron's did-navigate / did-navigate-in-page event or a Playwright waitForNavigation pattern instead.

8. CI workflow

The added .github/workflows/ci.yml is a welcome addition. Two small suggestions:

• Add npm audit or a dependency check step.
• Consider caching node_modules beyond just the npm cache for faster CI runs.

9. Inspector context enhancements in BrowserPanel

The buildContext(), getOpenTag(), and getSiblingsSummary() changes in BrowserPanel.tsx are nice quality-of-life improvements for the inspector, but they're not security-related. If you do split this PR, these could go with the feature work or a separate small PR.

---

Summary

| Item | Verdict

-- | -- | --
1 | Split daemon persistence/recovery into separate PR | Blocking
2 | Handle ::ffff: mapped IPv4 in SSRF validator | Blocking
3 | Throw on Windows ACL failure instead of warn | Blocking
4 | DNS rebinding TOCTOU window | Non-blocking (future)
5 | FS bridge TOCTOU | Non-blocking (future)
6 | Document webview remount UX impact | Non-blocking
7 | Replace 300ms sleep with navigation event | Non-blocking
8 | CI workflow enhancements | Non-blocking
9 | Split inspector context changes | Non-blocking

Once items 1–3 are addressed, I'm happy to merge. This is high-quality work — the test coverage across all six areas is especially appreciated.

[Zurgli]

Thank you for the thorough review, @openwong2kim! Updated this PR to address the blocking review items.

Changes made:

• Rebased the PR onto upstream main and kept the scope to the intended security hardening changes only.
• Fixed the SSRF validator to treat IPv6-mapped IPv4 addresses like ::ffff:x.x.x.x as IPv4 for policy enforcement.
• Added coverage for the mapped-IPv4 SSRF case in navigationPolicy.test.ts.
• Changed secureWriteTokenFile to fail closed on Windows ACL hardening errors by removing the token file and throwing.
• Added test coverage for the ACL failure path in security.test.ts.

[openwong2kim]

Merged! Thank you so much for this contribution, @Zurgli — this is genuinely excellent work.

The six hardening areas were clearly identified, well-scoped, and the fixes are structurally sound. I especially appreciate:

• The thoroughness of the SSRF validation, including the IPv6-mapped IPv4 fix in the follow-up commit
• • The fail-closed approach on the Windows ACL hardening — that's exactly the right call for security-critical token files
• • The test coverage across all six areas, which made reviewing this much easier and gives us confidence going forward
    The fact that you developed these as six separate PRs in your fork before bundling them here shows real discipline in how you approached the work. And your responsiveness to the blocking feedback was fast and precise.

This is a strong first contribution. Looking forward to seeing more from you — welcome aboard!

[openwong2kim]

Quick heads-up @Zurgli — your commits aren't linking to your GitHub profile on the contributor graph yet. This usually means the email in your local Git config doesn't match any email registered on your GitHub account.

You can fix it by adding that email in GitHub → Settings → Emails, or updating your local config:

git config --global user.email "your-github-email@example.com"

Just wanted to make sure you get proper credit for this work!

P.S. I've added a CONTRIBUTING.md based on your feedback about missing contributor guidance. Should make things smoother for future contributions.

[Zurgli]

@openwong2kim oh no, I had updated that on my local, but the worktree for the requested changes pulled the wrong account! Is there any chance you can help resolve this? I have amended the commits and can update the PR with the new ones if that's possible.

[openwong2kim]

Hey @Zurgli! Since the PR is already merged, the easiest fix is to add the email address used in those commits to your GitHub account under Settings > Emails. GitHub will automatically attribute the commits to your profile once the email is linked. No need to re-push anything!

If that's not possible for some reason, just let me know — I can revert the merge so you can re-submit with the correct account.

[Zurgli]

So sorry for the trouble, that one's for work, and I'm keeping these two accounts separate. Really appreciate you being flexible 🙏 On Wed, Apr 1, 2026 at 8:56 AM, ***@***.***> wrote: openwong2kim left a comment (openwong2kim/wmux#1) Hey @Zurgli! Since the PR is already merged, the easiest fix is to add the email address used in those commits to your GitHub account under Settings > Emails. GitHub will automatically attribute the commits to your profile once the email is linked. No need to re-push anything! If that's not possible for some reason, just let me know — I can revert the merge so you can re-submit with the correct account. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[openwong2kim]

Hey @Zurgli! No worries at all, totally understand keeping work and personal accounts separate. I've reverted the merge so you can
re-submit the PR with your correct account whenever you're ready. The codebase hasn't changed in the areas you touched, so it should be a
clean re-apply. Thanks again for the great security work!