test: verify link preview safety checks with mutations

.github/workflows/tests.yml

@@ -146,5 +146,8 @@ jobs:
         if: steps.build-cache.outputs.cache-hit != 'true'
         run: bun run build
 
+      - name: GIF Security Mutations
+        run: composer test:mutations
+
       - name: Tests
         run: composer test:unit

composer.json

@@ -116,8 +116,10 @@
             "@test:lint",
             "@test:types",
             "@test:type-coverage",
-            "@test:unit"
-        ]
+            "@test:unit",
+            "@test:mutations"
+        ],
+        "test:mutations": "bun run test:mutations"
     },
     "extra": {
         "laravel": {

package.json

@@ -8,7 +8,8 @@
         "lint": "NODE_OPTIONS='--experimental-strip-types' vp fmt resources/ && NODE_OPTIONS='--experimental-strip-types' vp lint --fix",
         "test:lint": "NODE_OPTIONS='--experimental-strip-types' vp fmt --check resources/ && NODE_OPTIONS='--experimental-strip-types' vp lint",
         "test:types": "tsc --noEmit && tsc --noEmit --project tests/tsconfig.json",
-        "test:unit": "php artisan wayfinder:generate --with-form && node --experimental-strip-types --test tests/Unit/*.test.ts"
+        "test:unit": "php artisan wayfinder:generate --with-form && node --experimental-strip-types --test tests/Unit/*.test.ts",
+        "test:mutations": "node --experimental-strip-types --test tests/Mutation/*.test.ts"
     },
     "dependencies": {
         "@daypicker/react": "^10.0.2",

tests/Mutation/GifSecurity.test.ts

@@ -0,0 +1,145 @@
+import assert from 'node:assert/strict';
+import { spawnSync } from 'node:child_process';
+import type { SpawnSyncReturns } from 'node:child_process';
+import {
+    mkdtempSync,
+    mkdirSync,
+    readFileSync,
+    rmSync,
+    writeFileSync,
+} from 'node:fs';
+import { tmpdir } from 'node:os';
+import { dirname, join } from 'node:path';
+import { test } from 'node:test';
+import { fileURLToPath } from 'node:url';
+import { phpGifMutations } from './php-gif-mutations.ts';
+import type { GifMutation } from './php-gif-mutations.ts';
+import { playbackMutations } from './playback-mutations.ts';
+
+const root = fileURLToPath(new URL('../..', import.meta.url));
+const temporary = mkdtempSync(join(tmpdir(), 'devnull-gif-mutations-'));
+const mutationTarget = 'PEST_MUTATION_TESTING';
+const mutationSource = 'PEST_MUTATION_FILE';
+const timeoutMilliseconds = 60_000;
+const controls = new Set<string>();
+
+function runPhp(
+    mutation: GifMutation,
+    source: string,
+): SpawnSyncReturns<string> {
+    const modifiedFile = join(temporary, 'mutated.php');
+    writeFileSync(modifiedFile, source);
+
+    return spawnSync(
+        'php',
+        [
+            'vendor/bin/pest',
+            '--ci',
+            '--compact',
+            '--colors=never',
+            mutation.test,
+        ],
+        {
+            cwd: root,
+            encoding: 'utf8',
+            timeout: timeoutMilliseconds,
+            env: {
+                ...process.env,
+                [mutationTarget]: join(root, mutation.source),
+                [mutationSource]: modifiedFile,
+            },
+        },
+    );
+}
+
+function runPlayback(
+    mutation: GifMutation,
+    source: string,
+): SpawnSyncReturns<string> {
+    const modifiedFile = join(temporary, mutation.source);
+    const testFile = join(temporary, mutation.test);
+    mkdirSync(dirname(modifiedFile), { recursive: true });
+    mkdirSync(dirname(testFile), { recursive: true });
+    writeFileSync(join(temporary, 'package.json'), '{"type":"module"}');
+    writeFileSync(modifiedFile, source);
+    writeFileSync(testFile, readFileSync(join(root, mutation.test)));
+    const environment = { ...process.env };
+    delete environment.NODE_TEST_CONTEXT;
+
+    return spawnSync(
+        process.execPath,
+        ['--experimental-strip-types', '--test', testFile],
+        {
+            encoding: 'utf8',
+            timeout: timeoutMilliseconds,
+            env: environment,
+        },
+    );
+}
+
+function output(result: SpawnSyncReturns<string>): string {
+    assert.ifError(result.error);
+    assert.notEqual(result.status, null, 'Mutation process did not finish');
+
+    return result.stdout + result.stderr;
+}
+
+type Runner = (
+    mutation: GifMutation,
+    source: string,
+) => SpawnSyncReturns<string>;
+
+function verifyControl(
+    mutation: GifMutation,
+    source: string,
+    run: Runner,
+): void {
+    if (controls.has(mutation.source)) {
+        return;
+    }
+
+    const control = run(mutation, source);
+    const report = output(control);
+    assert.equal(control.status, 0, report);
+    assert.match(
+        report,
+        run === runPhp ? /Tests:\s+.*\bpassed\b/ : /\bpass [1-9]\d*/,
+        report,
+    );
+    controls.add(mutation.source);
+}
+
+function checkMutation(mutation: GifMutation): void {
+    const original = readFileSync(join(root, mutation.source), 'utf8');
+    const run = mutation.source.endsWith('.php') ? runPhp : runPlayback;
+    verifyControl(mutation, original, run);
+    assert.ok(
+        original.includes(mutation.before),
+        `Mutation no longer matches ${mutation.source}`,
+    );
+    const result = run(
+        mutation,
+        original.replace(mutation.before, mutation.after),
+    );
+    const report = output(result);
+    assert.notEqual(
+        result.status,
+        0,
+        `Mutation survived: ${mutation.name}\n${report}`,
+    );
+    assert.match(
+        report,
+        run === runPhp ? /Tests:\s+.*\bfailed\b/ : /ERR_ASSERTION/,
+        report,
+    );
+}
+
+await test('GIF security mutation gate', async (context) => {
+    context.after(() => rmSync(temporary, { recursive: true, force: true }));
+
+    for (const mutation of [...phpGifMutations, ...playbackMutations]) {
+        await context.test(`detects mutation: ${mutation.name}`, () =>
+            checkMutation(mutation),
+        );
+    }
+});

tests/Mutation/php-gif-mutations.ts

@@ -0,0 +1,199 @@
+export type GifMutation = {
+    name: string;
+    source: string;
+    test: string;
+    before: string;
+    after: string;
+};
+
+export const phpGifMutations: GifMutation[] = [
+    {
+        name: 'concurrent workers must serialize an unfurl',
+        source: 'app/Jobs/UnfurlLink.php',
+        test: 'tests/Unit/Jobs/UnfurlLinkConcurrencyTest.php',
+        before: '->lock(self::METADATA_LOCK)',
+        after: '',
+    },
+    {
+        name: 'unfurl workers must allow foreign-key references',
+        source: 'app/Jobs/UnfurlLink.php',
+        test: 'tests/Unit/Jobs/UnfurlLinkConcurrencyTest.php',
+        before: '->lock(self::METADATA_LOCK)',
+        after: '->lockForUpdate()',
+    },
+    {
+        name: 'publication retries stay bounded',
+        source: 'app/Models/LinkPreview.php',
+        test: 'tests/Unit/Actions/CreateCommitTest.php',
+        before: "            ->where('publication_unfurl_retries', '<', self::MAX_PUBLICATION_UNFURL_RETRIES)\n",
+        after: '',
+    },
+    {
+        name: 'definitive unfurl outcomes remain cached',
+        source: 'app/Models/LinkPreview.php',
+        test: 'tests/Unit/Actions/CreateCommitTest.php',
+        before: "            ->whereNotNull('unfurl_failed_at')\n",
+        after: '',
+    },
+    {
+        name: 'redirect limit cannot increase',
+        source: 'app/Actions/UpdateLinkPreview.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'MAX_REDIRECTS = 3',
+        after: 'MAX_REDIRECTS = 4',
+    },
+    {
+        name: 'redirect limit cannot decrease',
+        source: 'app/Actions/UpdateLinkPreview.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'MAX_REDIRECTS = 3',
+        after: 'MAX_REDIRECTS = 2',
+    },
+    {
+        name: 'redirect cycles stop before refetching',
+        source: 'app/Actions/UpdateLinkPreview.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'if (isset($visited[$source]))',
+        after: 'if (false)',
+    },
+    {
+        name: 'non-default ports are rejected',
+        source: 'app/Services/LinkPreviews/LinkPreviewAddress.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'if ($url->getPort() !== null)',
+        after: 'if (false)',
+    },
+    {
+        name: 'non-global addresses are rejected',
+        source: 'app/Services/LinkPreviews/LinkPreviewAddress.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'return filter_var($address, FILTER_VALIDATE_IP, FILTER_FLAG_GLOBAL_RANGE) !== false',
+        after: 'return true',
+    },
+    {
+        name: 'Symfony special-use subnets stay blocked',
+        source: 'app/Services/LinkPreviews/LinkPreviewAddress.php',
+        test: 'tests/Unit/Services/LinkPreviews/LinkPreviewAddressTest.php',
+        before: '...IpUtils::PRIVATE_SUBNETS,',
+        after: '',
+    },
+    {
+        name: 'additional relay ranges stay blocked',
+        source: 'app/Services/LinkPreviews/LinkPreviewAddress.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: "'192.88.99.0/24',",
+        after: '',
+    },
+    {
+        name: 'all DNS answers must be public',
+        source: 'app/Services/LinkPreviews/LinkPreviewAddress.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'array_all($addresses, $this->isPublic(...))',
+        after: 'true',
+    },
+    {
+        name: 'NAT64 translations stay blocked',
+        source: 'app/Services/LinkPreviews/LinkPreviewAddress.php',
+        test: 'tests/Unit/Services/LinkPreviews/LinkPreviewAddressTest.php',
+        before: "'64:ff9b::/32',",
+        after: '',
+    },
+    {
+        name: 'response body is capped',
+        source: 'app/Services/LinkPreviews/LimitedResponseBody.php',
+        test: 'tests/Unit/Services/LinkPreviews/LimitedResponseBodyTest.php',
+        before: '$remaining = max(0, $this->maxBytes - $this->tell());',
+        after: '$remaining = PHP_INT_MAX;',
+    },
+    {
+        name: 'seeking cannot bypass the byte cap',
+        source: 'app/Services/LinkPreviews/LimitedResponseBody.php',
+        test: 'tests/Unit/Services/LinkPreviews/LimitedResponseBodyTest.php',
+        before: 'max(0, $this->maxBytes - $this->tell())',
+        after: 'max(-1, $this->maxBytes - $this->tell())',
+    },
+    {
+        name: 'overall deadline stays at five seconds',
+        source: 'app/Services/LinkPreviews/LinkPreviewBudget.php',
+        test: 'tests/Unit/Services/LinkPreviews/LinkPreviewBudgetTest.php',
+        before: 'TIMEOUT_SECONDS = 5',
+        after: 'TIMEOUT_SECONDS = 6',
+    },
+    {
+        name: 'remaining timeout never rounds upward',
+        source: 'app/Services/LinkPreviews/LinkPreviewBudget.php',
+        test: 'tests/Unit/Services/LinkPreviews/LinkPreviewBudgetTest.php',
+        before: 'floor(($deadline - $this->seconds())',
+        after: 'round(($deadline - $this->seconds())',
+    },
+    {
+        name: 'automatic HTTP redirects stay disabled',
+        source: 'app/Services/LinkPreviews/LinkPreviewHttpClient.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: '->withoutRedirecting()',
+        after: '->withOptions([RequestOptions::ALLOW_REDIRECTS => true])',
+    },
+    {
+        name: 'automatic decompression stays disabled',
+        source: 'app/Services/LinkPreviews/LinkPreviewHttpClient.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'RequestOptions::DECODE_CONTENT => false',
+        after: 'RequestOptions::DECODE_CONTENT => true',
+    },
+    {
+        name: 'requests pin the checked address',
+        source: 'app/Services/LinkPreviews/LinkPreviewHttpClient.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'CURLOPT_CONNECT_TO => [$connection]',
+        after: 'CURLOPT_CONNECT_TO => []',
+    },
+    {
+        name: 'content type is checked',
+        source: 'app/Services/PreviewLinkProviders/GifPreviewLink.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'if ($mediaType !== self::CONTENT_TYPE)',
+        after: 'if (false)',
+    },
+    {
+        name: 'both supported GIF signatures remain valid',
+        source: 'app/Services/PreviewLinkProviders/GifPreviewLink.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: "SIGNATURES = ['GIF87a', 'GIF89a']",
+        after: "SIGNATURES = ['GIF89a']",
+    },
+    {
+        name: 'truncated GIF descriptors are rejected',
+        source: 'app/Services/PreviewLinkProviders/GifPreviewLink.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'MIN_HEADER_BYTES = 13',
+        after: 'MIN_HEADER_BYTES = 12',
+    },
+    {
+        name: 'GIF dimensions must be nonzero',
+        source: 'app/Services/PreviewLinkProviders/GifPreviewLink.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'return $width !== self::ZERO_DIMENSION && $height !== self::ZERO_DIMENSION;',
+        after: 'return true;',
+    },
+    {
+        name: 'oversized URLs stay rejected',
+        source: 'app/Services/PreviewLinkProviders/GifPreviewLink.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'mb_strlen($sourceUrl) > Urls::MAX_LENGTH',
+        after: 'mb_strlen($sourceUrl) > Urls::MAX_LENGTH + 1',
+    },
+    {
+        name: 'maximum-length URLs remain supported',
+        source: 'app/Services/PreviewLinkProviders/GifPreviewLink.php',
+        test: 'tests/Unit/Actions/UpdateLinkPreviewTest.php',
+        before: 'mb_strlen($sourceUrl) > Urls::MAX_LENGTH',
+        after: 'mb_strlen($sourceUrl) > Urls::MAX_LENGTH - 1',
+    },
+    {
+        name: 'Giphy hosts cannot be spoofed',
+        source: 'app/Services/EmbedProviders/Giphy.php',
+        test: 'tests/Unit/Services/EmbedProviders/GiphyTest.php',
+        before: 'if (! in_array($url->getAsciiHost(), self::SUPPORTED_HOSTS, true))',
+        after: 'if (false)',
+    },
+];