Validate URL schemes and double extensions on file import

[pballart]

Task/Issue URL: https://app.asana.com/0/0/1210890453697052/f/

Description

Hardens the third-party file/zip import path (security-audit follow-up):

• Drops bookmarks whose URL uses a javascript:/data: scheme — activating such a bookmark would execute embedded content instead of navigating.
• Rejects files and zip entries with a disguising double extension (e.g. foo.swift.html).

Both rules live once in BrowserServicesKit (String.hasUnsafeBookmarkImportScheme() / String.hasMultipleFileExtensions) and are invoked from each platform's import paths — the scheme check at the shared bookmark-save chokepoint (iOS BookmarkOrFolder.isInvalidBookmark, macOS LocalBookmarkStore), and the double-extension check at both the archive readers (ImportArchiveReader) and the direct-file entry points (iOS DataImportManager.importFile, macOS SafariArchiveImporter.readContentsForSelectedFile). Archives themselves are exempt — their entries are validated individually.

Testing Steps

1. Import a bookmarks HTML file with a normal https: bookmark plus javascript:/data: bookmarks (iOS + macOS) → the https: one imports; the unsafe ones are dropped (counted as failed, not saved).
2. Import a zip / Safari archive containing bookmarks.html + passwords.csv plus a decoy like evil.swift.html → normal files import; the double-extension entry is skipped.
3. Import a standalone file named with a double extension (e.g. bookmarks.evil.html) directly → it's rejected (nothing imported).
4. Regression: import a normal Safari/Chrome export and confirm all valid bookmarks still import.

Impact and Risks

Low–Medium — changes which imported bookmarks are persisted (privacy/security hardening; no change to existing stored data).

What could go wrong?

A legitimate item could be dropped. Mitigations: the scheme check is limited to javascript:/data: and is bookmarks-only (passwords/cards unaffected). The double-extension rule is intentionally strict (any filename with >1 dot) — an accepted trade-off agreed in the kick-off — so a legitimately double-dotted archive entry is skipped, and a directly-selected file with that shape is rejected outright (the import fails). The strictness was a deliberate decision over false-positive risk.

Quality Considerations

• Validation centralized in BSK and reused at every per-platform import sink, so a future import source inherits both checks.
• Unit tests added in BSK (StringFileExtensionsTests, BookmarkImportSchemeValidationTests), iOS (ImportArchiveReaderTests, DataImportManagerTests), and macOS (LocalBookmarkStoreTests, SafariArchiveImporterTests). iOS tests verified passing locally; macOS + BSK suites run on CI.

---

Internal references:

Definition of Done | Engineering Expectations | Tech Design Template

---

Note

Medium Risk
Import behavior changes which bookmarks and files are accepted; scheme filtering is narrow but the double-extension rule may skip legitimate multi-dot filenames (e.g. report.tar.gz).

Overview
Hardens third-party data import (security-audit follow-up) by blocking two abuse paths: executable bookmark targets and disguised filenames.

Unsafe bookmark URLs: Shared helpers in BrowserServicesKit (hasUnsafeBookmarkImportScheme() for javascript: / data:) extend BookmarkOrFolder.isInvalidBookmark, so iOS import via BookmarkCoreDataImporter drops those entries. macOS adds the same filter in LocalBookmarkStore when persisting imports (folders with stray unsafe URLs are kept so children still import).

Double extensions: hasMultipleFileExtensions flags last path components with more than one dot (e.g. foo.swift.html). Direct file picks are rejected on iOS (DataImportManager.importFile returns nil) and macOS standalone Safari imports (SafariArchiveImporter yields empty contents / import failure). Zip entries with that pattern are skipped in both platforms’ ImportArchiveReader while normal .html/.csv/.json entries still load.

Unit tests cover BSK helpers and platform import/archive paths.

^{Reviewed by Cursor Bugbot for commit d6b7f2a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 4125bf5. Configure here.}