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>
|
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