fix: harden survey webview — TLS, script injection, URL scheme, inspectable (ENG-1810/1811/1812/1813) - #51
Conversation
WalkthroughSurveyWebView’s coordinator no longer creates URL credentials from server trust during TLS authentication challenges. It now always returns 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
- ENG-1811: gate WKWebView.isInspectable behind #if DEBUG so the survey WebView is not inspectable in release builds. - ENG-1812: restrict external URLs opened from survey content to http/https; refuse tel/sms/custom-app/file/javascript schemes. - ENG-1813: base64-encode the survey payload before embedding it in the WebView HTML instead of splicing it into a JS template literal, removing the script-injection surface. Also drops the quote-mangling workaround that corrupted survey text containing double quotes. Adds regression tests for ENG-1812 and ENG-1813 and updates the existing WEBVIEW_DATA test to decode the base64 payload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pandeymangg
changed the title
The challenge parameter is unused now that the handler always performs default TLS handling; name it _ to satisfy SonarQube. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Automated review — summaryCore fixes look correct: TLS One real gap — ENG-1812 (URL allowlist) is incomplete:
Worth adding (P2):
Minor (P3): |
…-1812) Review found the scheme allowlist was only checked in the onOpenExternalURL JS-bridge path. Direct navigation from survey markup (<a href="tel:...">, window.location, meta refresh, form POST) bypassed it and reached WKWebView's native scheme handling. Add decidePolicyForNavigationAction to the Coordinator: only the in-memory survey document loads in-frame; every other navigation is cancelled and routed through the shared openExternalURL helper (http/https opened, everything else blocked). The allowlist is now enforced on both the bridge and direct navigation. Tests: - navigation policy (about:blank/nil load in-frame; web + non-web schemes do not) - non-serverTrust auth challenges also use .performDefaultHandling - quotes and angle brackets in survey content survive the base64 round-trip (guards the removed \"->' mangling) - dedupe the atob(...) payload extraction into one helper Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The webView parameter is unused; name it _ to satisfy SonarCloud (same as the auth-challenge handler). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Upstream 2.1.0 (PR formbricks#51) carries the four WebView hardening fixes this commit originally introduced — TLS chain validation, DEBUG-gated isInspectable, an external-URL scheme allowlist and a base64-encoded payload — so only the fork documentation remains here. Rewritten for the 2.1.0 base in a later commit.
Upstream 2.1.0 (PR formbricks#51) absorbed the four WebView security fixes the fork was created for, so the security section shrinks to the one delta that remains: https-only external links, where upstream still allows http. Also records how the branch is maintained (main mirrors upstream, SJ work rebased on top, the tag is the version), and notes how the awaitable attribute sync interacts with 2.1.0's new interaction-based segment refresh.
2 participants
Summary
Security hardening of the survey WebView, from the SJ app-team report (patched in their iOS fork). Four related fixes, all confined to
SurveyWebView.swiftandFormbricksViewModel.swift.isInspectableenabled in release buildsENG-1810 — Restore default TLS validation
The
WKNavigationDelegateauth-challenge handler answered every challenge withURLCredential(trust:), accepting any server certificate and disabling chain validation for the whole survey WebView — trivial man-in-the-middle interception of payload, responses, and config.Fix: respond with
completionHandler(.performDefaultHandling, nil)so the OS validates the certificate chain.ENG-1813 — Payload no longer injected as executable JS
The survey payload was find-replaced into a backtick template literal (
const json = `{{WEBVIEW_DATA}}`). Survey content containing a backtick — or${...}, which template literals evaluate eagerly — could break out and run arbitrary JS in the WebView.Fix: base64-encode the payload in Swift and decode it at runtime (
atob→TextDecoder→JSON.parse). Base64 output is[A-Za-z0-9+/=]only, so it cannot contain a breakout character. Also removed a\"→'workaround that was corrupting survey text with double quotes.ENG-1812 — Restrict external URL schemes
Links from survey content were passed straight to
UIApplication.shared.open, allowing any registered scheme (tel:,sms:, custom app deep links, etc.).Fix: allowlist
http/httpsviaJsMessageHandler.isAllowedExternalURL(_:); everything else is refused and logged.ENG-1811 — Web Inspector debug-only
WKWebView.isInspectablewas settrueunconditionally, leaving the survey WebView inspectable in production.Fix: wrap in
#if DEBUG.Tests
testWebViewAuthChallengeUsesDefaultHandlingAndDoesNotForceTrust— asserts.performDefaultHandling+ nil credential. Verified it fails when the force-trust bug is reintroduced.testExternalURLSchemeAllowlist— http/https allowed; tel/sms/mailto/facetime/file/javascript/custom-app blocked.testWebViewPayloadIsBase64EncodedNotRawTemplateLiteral— asserts the raw template literal is gone and the embedded blob is pure base64 that decodes to valid JSON. ExistingtestWebViewDataUsesSurveyOverwritesupdated to decode the base64 payload.#if DEBUGis a compile-time flag and the test bundle builds in DEBUG, so the release path isn't observable at runtime. Verified by inspection.Full suite green on the iOS Simulator; no new warnings.
Scope
iOS SDK only. The report also asks to audit the same patterns in Android (
onReceivedSslError/handler.proceed()), Flutter, and React Native wrappers — tracked separately.Acceptance
#if DEBUG