fix(links): open only web pages in the in-app browser

app/Devnull/SiteLink.php

@@ -70,24 +70,41 @@ public static function screen(?string $url): ?string
         return null;
     }
 
+    /**
+     * Whether a URL from devnull is a web page the browser may open: http or
+     * https, with a host, and no user or password before it. Nothing else a
+     * link might carry, like javascript:, file: or another app's scheme, can
+     * take the phone somewhere unexpected, and the in-app browser only opens
+     * web pages anyway.
+     *
+     * @phpstan-assert-if-true string $url
+     */
+    public static function isWebUrl(?string $url): bool
+    {
+        $parts = $url === null ? false : parse_url($url);
+
+        if ($parts === false || isset($parts['user']) || isset($parts['pass'])) {
+            return false;
+        }
+
+        return in_array(strtolower($parts['scheme'] ?? ''), ['http', 'https'], true) && ($parts['host'] ?? '') !== '';
+    }
+
     /**
      * The path of an http or https URL on one of dev/null's hosts. A URL with
      * a user or password before its host is never one, as browsers and PHP
      * can disagree on where its host starts.
      */
     private static function pathOnDevnull(?string $url): ?string
     {
-        $parts = $url === null ? false : parse_url($url);
-
-        if ($parts === false || isset($parts['user']) || isset($parts['pass'])) {
+        if (! self::isWebUrl($url)) {
             return null;
         }
 
-        $scheme = strtolower($parts['scheme'] ?? '');
-        $host = strtolower($parts['host'] ?? '');
+        $parts = (array) parse_url((string) $url);
         $hosts = [...self::Hosts, strtolower((string) parse_url((string) config('devnull.url'), PHP_URL_HOST))];
 
-        if (! in_array($scheme, ['http', 'https'], true) || $host === '' || ! in_array($host, $hosts, true)) {
+        if (! in_array(strtolower($parts['host'] ?? ''), $hosts, true)) {
             return null;
         }
 

app/NativeComponents/Concerns/DraftsIssues.php

@@ -7,6 +7,7 @@
 use App\Devnull\DraftImages;
 use App\Devnull\Exceptions\ApiException;
 use App\Devnull\IssueCommand;
+use App\Devnull\SiteLink;
 use App\Devnull\Stash;
 use App\Devnull\Toasts;
 use Illuminate\Support\Facades\Cache;
@@ -169,7 +170,7 @@ public function openIssueFromSheet(): void
      */
     public function viewOpenedIssue(): void
     {
-        if ($this->openedIssueUrl !== null) {
+        if (SiteLink::isWebUrl($this->openedIssueUrl)) {
             Browser::inApp($this->openedIssueUrl);
         }
     }

app/NativeComponents/Concerns/ShowsCommits.php

@@ -272,7 +272,7 @@ public function openImage(string $id, int $index = 0): void
         $images = $this->shownCommit($id)?->images ?? [];
         $image = $images[$index] ?? null;
 
-        if ($image === null || ! $this->isWebUrl($image->url)) {
+        if ($image === null || ! SiteLink::isWebUrl($image->url)) {
             return;
         }
 
@@ -588,20 +588,11 @@ private function openUrl(?string $url): void
 
     private function openInBrowser(?string $url): void
     {
-        if ($this->isWebUrl($url)) {
+        if (SiteLink::isWebUrl($url)) {
             Browser::inApp($url);
         }
     }
 
-    /**
-     * Only an http or https address is opened, in the browser or the viewer, so
-     * nothing else a row might carry can take the phone somewhere unexpected.
-     */
-    private function isWebUrl(?string $url): bool
-    {
-        return $url !== null && in_array(parse_url($url, PHP_URL_SCHEME), ['http', 'https'], true);
-    }
-
     /**
      * Puts the viewer's image back at rest: fresh shared values, which the
      * phone seeds at one and none, so what the last image's gestures left
@@ -623,7 +614,7 @@ private function showInViewedImage(int $index): void
     {
         $image = $this->viewerImages[$index] ?? null;
 
-        if (! $this->viewerOpen || $image === null || ! $this->isWebUrl($image->url)) {
+        if (! $this->viewerOpen || $image === null || ! SiteLink::isWebUrl($image->url)) {
             return;
         }
 

app/NativeComponents/Concerns/SyncsOutbox.php

@@ -12,6 +12,7 @@
 use App\Devnull\Outbox;
 use App\Devnull\ResponseCache;
 use App\Devnull\SignedRequest;
+use App\Devnull\SiteLink;
 use App\Devnull\Stash;
 use App\Devnull\Toasts;
 use App\Models\PendingWrite;
@@ -375,7 +376,7 @@ private function viewIssueFromToast(array $data): void
     {
         $url = $data['url'] ?? null;
 
-        if (is_string($url) && in_array(parse_url($url, PHP_URL_SCHEME), ['http', 'https'], true)) {
+        if (is_string($url) && SiteLink::isWebUrl($url)) {
             Browser::inApp($url);
         }
     }

app/NativeComponents/Conversation.php

@@ -514,7 +514,7 @@ public function openLink(int $id): void
             return;
         }
 
-        if ($url !== null) {
+        if (SiteLink::isWebUrl($url)) {
             Browser::inApp($url);
         }
     }

app/NativeComponents/Profile.php

@@ -12,6 +12,7 @@
 use App\Devnull\Exceptions\ApiException;
 use App\Devnull\RowCache;
 use App\Devnull\ShownTabs;
+use App\Devnull\SiteLink;
 use App\Devnull\Stash;
 use App\Devnull\Timeline;
 use App\Icons\Lucide;
@@ -564,8 +565,10 @@ public function openGitHubMention(string $username): void
 
     public function openGitHubProfile(): void
     {
-        if ($this->profile !== null) {
-            Browser::inApp($this->profile->user->gitHubProfileUrl());
+        $url = $this->profile?->user->gitHubProfileUrl();
+
+        if (SiteLink::isWebUrl($url)) {
+            Browser::inApp($url);
         }
     }
 

tests/Feature/Devnull/SiteLinkTest.php

@@ -55,3 +55,25 @@
     'a username with two hyphens in a row' => ['https://dev-null.sh/@a--da'],
     'a username longer than GitHub allows' => ['https://dev-null.sh/@'.str_repeat('a', 40)],
 ]);
+
+it('takes an http or https address with a host for a web page', function (string $url) {
+    expect(SiteLink::isWebUrl($url))->toBeTrue();
+})->with([
+    'https' => ['https://nativephp.com/docs?page=1#top'],
+    'http' => ['http://example.com'],
+    'a scheme in capitals' => ['HTTPS://NATIVEPHP.COM'],
+]);
+
+it('takes nothing else for a web page', function (?string $url) {
+    expect(SiteLink::isWebUrl($url))->toBeFalse();
+})->with([
+    'nothing' => [null],
+    'javascript' => ['javascript:alert(1)'],
+    'a file on the phone' => ['file:///etc/passwd'],
+    "an app's own scheme" => ['devnull://auth/callback'],
+    'a phone number' => ['tel:+33100000000'],
+    'an email address' => ['mailto:ada@example.com'],
+    'no host' => ['https://'],
+    'no scheme' => ['nativephp.com'],
+    'a user and password before the host' => ['https://nativephp.com@evil.com/'],
+]);

tests/Feature/Screens/ConversationTest.php

@@ -460,6 +460,17 @@ function serveRemoteThread(array $thread, ?array $poll = null, int $pushStatus =
         ->assertNativeCalledTimes('Browser.OpenInApp', 1);
 });
 
+it('opens no link preview that is not a web page', function () {
+    serveRemoteThread(threadJson([
+        remoteMessageJson(['id' => 1, 'body' => 'Tap me', 'link_preview' => ['url' => 'javascript:alert(1)', 'title' => 'Tap me']]),
+    ], ['read_through_id' => 1]));
+
+    finishRefreshes(Native::visit('/remotes/conversations/ada'))
+        ->tap('link-1')
+        ->assertNoNavigation()
+        ->assertNativeNotCalled('Browser.OpenInApp');
+});
+
 it('pads the end of the thread for the composer bar alone, and for the reply bar while replying', function () {
     serveRemoteThread(threadJson([remoteMessageJson(['id' => 1])], ['read_through_id' => 1]));
 

tests/Feature/Screens/IssueCommandsTest.php

@@ -206,6 +206,14 @@ function hasRef(TestableComponent $screen, string $ref): bool
         ->and(hasRef($screen, 'sheet-commit'))->toBeTrue();
 });
 
+it('opens no issue link that is not a web page', function () {
+    $screen = issueSheetOnLog([['number' => 123, 'url' => 'javascript:alert(1)'], 201])->input('sheet-body', '/bug Broken.')->tap('sheet-open-issue');
+
+    $screen->tap('view-issue')
+        ->emitNative(ToastActionPressed::class, ['toastId' => 'toast-1', 'actionId' => 'view-issue', 'data' => ['url' => 'file:///etc/passwd']])
+        ->assertNativeNotCalled('Browser.OpenInApp');
+});
+
 it('says so in a toast with View issue when the sheet was swiped away before the issue opened', function () {
     holdRefreshes();
     $screen = issueSheetOnLog()->input('sheet-body', '/feature Dark mode')->tap('sheet-open-issue')->dismissSheet('sheet');

tests/Feature/Screens/ProfileTest.php

@@ -111,6 +111,17 @@ function adaCommitJson(array $overrides = []): array
         ->assertNativeCalled('Browser.OpenInApp', fn (array $params): bool => $params['url'] === 'https://ada.example');
 });
 
+it('opens no GitHub profile link that is not a web page', function () {
+    fakeApi([
+        'GET /api/v1/users/ada' => [profileJson(user: ['github_url' => 'javascript:alert(1)'])],
+        'GET /api/v1/users/ada/commits' => [pageJson([])],
+    ]);
+
+    Native::visit('/log/users/ada')
+        ->tap('profile-github')
+        ->assertNativeNotCalled('Browser.OpenInApp');
+});
+
 it('opens a website on dev/null here rather than in the browser', function () {
     fakeApi([
         'GET /api/v1/users/ada' => [profileJson(user: ['website' => 'https://dev-null.sh/@grace'])],