From d0c6ab6a6ceec091f3af7722c60ef769b7314b7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Andr=C3=A9?= Date: Wed, 30 Oct 2024 20:24:54 +0100 Subject: [PATCH 1/4] [AssetMapper] Add Integrity Hashes to ImportMap This is a basic implementation to support integrity hashes within import maps: - Computes a base64-encoded SHA-384 digest in the factory. - Renders the integrity attribute for JavaScript files in the import map. **TODO** - [ ] Make the integrity hash optional (e.g., through a constructor argument in the factory) - [ ] Compute hashes only for certain assets / types / paths ? - [ ] Expose configuration settings - [ ] Adapt the FrameworkBundle / DI - [ ] Determine handling approach for CSS files **Sources** - [Subresource Integrity (SRI) Goals - W3C](https://www.w3.org/TR/SRI/#goals) - [JSPM: JS Integrity with Import Maps](https://jspm.org/js-integrity-with-import-maps) _PS: I'm a bit short on time lately... so if anyone wants to help or take over, please feel free!_ --- .../Factory/MappedAssetFactory.php | 13 +++++ .../ImportMap/ImportMapGenerator.php | 7 ++- .../ImportMap/ImportMapRenderer.php | 7 ++- .../Component/AssetMapper/MappedAsset.php | 6 ++ .../Tests/Factory/MappedAssetFactoryTest.php | 18 +++++- .../ImportMap/ImportMapGeneratorTest.php | 7 ++- .../Tests/ImportMap/ImportMapRendererTest.php | 58 +++++++++++++++++++ 7 files changed, 110 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/AssetMapper/Factory/MappedAssetFactory.php b/src/Symfony/Component/AssetMapper/Factory/MappedAssetFactory.php index 597a9ae429624..a621e3ce2848f 100644 --- a/src/Symfony/Component/AssetMapper/Factory/MappedAssetFactory.php +++ b/src/Symfony/Component/AssetMapper/Factory/MappedAssetFactory.php @@ -63,6 +63,7 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map $asset->getDependencies(), $asset->getFileDependencies(), $asset->getJavaScriptImports(), + $this->getIntegrity($asset, $content), ); $this->assetsCache[$logicalPath] = $asset; @@ -73,6 +74,18 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map return $this->assetsCache[$logicalPath]; } + /** + * Returns an SRI integrity hash for the given asset. + */ + private function getIntegrity(MappedAsset $asset, ?string $content): string + { + if (null !== $content) { + return 'sha384-'.base64_encode(hash('sha384', $content, true)); + } + + return 'sha384-'.base64_encode(hash_file('sha384', $asset->sourcePath, true)); + } + /** * Returns an array of "string digest" and "bool predigested". * diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php index 89579fb313ed2..f40e7d3b56de2 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php @@ -50,7 +50,7 @@ public function getEntrypointNames(): array /** * @param string[] $entrypointNames * - * @return array + * @return array * * @internal */ @@ -83,7 +83,7 @@ public function getImportMapData(array $entrypointNames): array /** * @internal * - * @return array + * @return array */ public function getRawImportMapData(): array { @@ -106,6 +106,9 @@ public function getRawImportMapData(): array $path = $asset->publicPath; $data = ['path' => $path, 'type' => $entry->type->value]; + if ($asset->integrity) { + $data['integrity'] = $asset->integrity; + } $rawImportMapData[$entry->importName] = $data; } diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php index 48c869b00711c..abb555d8be9a9 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php @@ -47,6 +47,7 @@ public function render(string|array $entryPoint, array $attributes = []): string $importMapData = $this->importMapGenerator->getImportMapData($entryPoint); $importMap = []; + $integrityMap = []; $modulePreloads = []; $cssLinks = []; $polyfillPath = null; @@ -70,8 +71,12 @@ public function render(string|array $entryPoint, array $attributes = []): string } $preload = $data['preload'] ?? false; + $integrity = $data['integrity'] ?? null; if ('css' !== $data['type']) { $importMap[$importName] = $path; + if ($integrity) { + $integrityMap[$path] = $integrity; + } if ($preload) { $modulePreloads[] = $path; } @@ -96,7 +101,7 @@ public function render(string|array $entryPoint, array $attributes = []): string } $scriptAttributes = $attributes || $this->scriptAttributes ? ' '.$this->createAttributesString($attributes) : ''; - $importMapJson = json_encode(['imports' => $importMap], \JSON_THROW_ON_ERROR | \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES | \JSON_HEX_TAG); + $importMapJson = json_encode(['imports' => $importMap, 'integrity' => $integrityMap], \JSON_THROW_ON_ERROR | \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES | \JSON_HEX_TAG); $output .= << diff --git a/src/Symfony/Component/AssetMapper/MappedAsset.php b/src/Symfony/Component/AssetMapper/MappedAsset.php index 6bb828d61cae2..3c9cc07751137 100644 --- a/src/Symfony/Component/AssetMapper/MappedAsset.php +++ b/src/Symfony/Component/AssetMapper/MappedAsset.php @@ -52,6 +52,7 @@ public function __construct( private array $dependencies = [], private array $fileDependencies = [], private array $javaScriptImports = [], + public readonly ?string $integrity = null, ) { if (null !== $sourcePath) { $this->sourcePath = $sourcePath; @@ -72,6 +73,11 @@ public function __construct( } } + public function getIntegrity(): ?string + { + return $this->integrity; + } + /** * Assets that the content of this asset depends on - for internal caching. * diff --git a/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php b/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php index a7939c88ffa83..e86cee6b919d4 100644 --- a/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php @@ -36,6 +36,7 @@ public function testCreateMappedAsset() $this->assertSame('file2.js', $asset->logicalPath); $this->assertMatchesRegularExpression('/^\/final-assets\/file2-[a-zA-Z0-9]{7,128}\.js$/', $asset->publicPath); $this->assertSame('/final-assets/file2.js', $asset->publicPathWithoutDigest); + $this->assertSame('sha384-ZDljYTViYzY0NTgyZjA4ZTBmMjgwODY1NDNlMmRhMTY2NTVlOTNhYTlkZjMwZGY5YzU0NjdlNDExMThjY2RjNGFmNWZkNDhmZDg0ODIzMmVmMjkyNmIwNGE2NGJkMjdi', $asset->integrity); } public function testCreateMappedAssetRespectsPreDigestedPaths() @@ -46,11 +47,12 @@ public function testCreateMappedAssetRespectsPreDigestedPaths() $this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css', $asset->publicPath); // for pre-digested files, the digest *is* part of the public path $this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css', $asset->publicPathWithoutDigest); + $this->assertSame('sha384-YThlMTY4MzI3MGY3ZjFlNTk0M2VhMDQ0MzMyYjEwYjRkNGQ2NjU4YzZlMDZjYjA3YTgwNDUzNjUwOTQyOGI4NjQ1YmFiMmIyMzg4ZWZhOGRiMGQ5MjU4MjJjNThlOTkz', $asset->integrity); } public function testCreateMappedAssetWithContentThatChanged() { - $file1Compiler = new class implements AssetCompilerInterface { + $file1Compiler = new class () implements AssetCompilerInterface { public function supports(MappedAsset $asset): bool { return true; @@ -81,6 +83,17 @@ public function testCreateMappedAssetWithContentThatDoesNotChange() $this->assertNull($asset->content); } + public function testCreateMappedAssetComputeIntegrity() + { + $assetMapper = $this->createFactory(); + + $asset = $assetMapper->createMappedAsset('file1.css', __DIR__.'/../Fixtures/dir1/file1.css'); + $this->assertSame('sha384-XgVLYsLqVK+VujG5zkQyFuRtBc98ql0YRAQYP8CT8paQgxTtAUAdgcvTO9TxlUXp', $asset->integrity); + + $asset = $assetMapper->createMappedAsset('file2.js', __DIR__.'/../Fixtures/dir1/file2.js'); + $this->assertSame('sha384-2cpbxkWC8I4PKAhlQ+LaFmVek6qd8w35xUZ+QRGMzcSvX9SP2EgjLvKSawSmS9J7', $asset->integrity); + } + public function testCreateMappedAssetWithContentErrorsOnCircularReferences() { $factory = $this->createFactory(); @@ -92,7 +105,7 @@ public function testCreateMappedAssetWithContentErrorsOnCircularReferences() public function testCreateMappedAssetWithDigest() { - $file6Compiler = new class implements AssetCompilerInterface { + $file6Compiler = new class () implements AssetCompilerInterface { public function supports(MappedAsset $asset): bool { return true; @@ -119,6 +132,7 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac $factory = $this->createFactory($file6Compiler); $asset = $factory->createMappedAsset('subdir/file6.js', __DIR__.'/../Fixtures/dir2/subdir/file6.js'); $this->assertSame('7e4f24ebddd4ab2a3bcf0d89270b9f30', $asset->digest); + $this->assertSame('sha384-1GNXqZ7KhQjqjTLnfTOAln3lJn8wex5coUWjjZ7DT40GKkCb5XK+P6kNTHjdnXs/', $asset->integrity); } public function testCreateMappedAssetWithPredigested() diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php index bdc8bc36c1ed7..909c4d7defebb 100644 --- a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php @@ -348,6 +348,7 @@ public static function getRawImportMapDataTests(): iterable '/path/to/simple.js', publicPathWithoutDigest: '/assets/simple.js', publicPath: '/assets/simple-d1g3st.js', + integrity: 'simple-integrity', ); yield 'it adds dependency to the importmap' => [ [ @@ -360,7 +361,7 @@ public static function getRawImportMapDataTests(): iterable new MappedAsset( 'app.js', publicPath: '/assets/app-d1g3st.js', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false, addImplicitlyToImportMap: true)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false, addImplicitlyToImportMap: true)], ), $simpleAsset, ], @@ -372,6 +373,7 @@ public static function getRawImportMapDataTests(): iterable '/assets/simple.js' => [ 'path' => '/assets/simple-d1g3st.js', 'type' => 'js', + 'integrity' => 'simple-integrity', ], ], ]; @@ -401,6 +403,7 @@ public static function getRawImportMapDataTests(): iterable '/assets/simple.js' => [ 'path' => '/assets/simple-d1g3st.js', 'type' => 'js', + 'integrity' => 'simple-integrity', ], ], ]; @@ -440,6 +443,7 @@ public static function getRawImportMapDataTests(): iterable '/assets/simple.js' => [ 'path' => '/assets/simple-d1g3st.js', 'type' => 'js', + 'integrity' => 'simple-integrity', ], ], ]; @@ -472,6 +476,7 @@ public static function getRawImportMapDataTests(): iterable '/assets/simple.js' => [ 'path' => '/assets/simple-d1g3st.js', 'type' => 'js', + 'integrity' => 'simple-integrity', ], 'imports_simple' => [ 'path' => '/assets/imports_simple-d1g3st.js', diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php index a4770635c4e6d..2f110291e2931 100644 --- a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php @@ -211,4 +211,62 @@ public function testItAddsPreloadLinks() $this->assertSame(['as' => 'style'], $linkProvider->getLinks()[0]->getAttributes()); $this->assertSame('/assets/styles/app-preload-d1g35t.css', $linkProvider->getLinks()[0]->getHref()); } + + public function testIntegrityMap(): void + { + $importMapGenerator = $this->createMock(ImportMapGenerator::class); + $importMapGenerator->expects($this->once()) + ->method('getImportMapData') + ->with(['app']) + ->willReturn([ + 'app_js_preload' => [ + 'path' => '/assets/app-preload-d1g35t.js', + 'type' => 'js', + 'integrity' => 'sha384-abc123-js', + ], + 'app_js_no_preload' => [ + 'path' => '/assets/app-no-preload-d1g35t.js', + 'type' => 'js', + 'integrity' => 'sha384-abc123-js-no', + ], + 'app_css_preload' => [ + 'path' => '/assets/styles/app-preload-d1g35t.css', + 'type' => 'css', + 'integrity' => 'sha384-abc123-css', + ], + 'app_css_no_preload' => [ + 'path' => '/assets/styles/app-nopreload-d1g35t.css', + 'type' => 'css', + 'integrity' => 'sha384-abc123-css-no' + ], + ]); + + $assetPackages = $this->createMock(Packages::class); + $assetPackages->expects($this->any()) + ->method('getUrl') + ->willReturnCallback(function ($path) { + // try to imitate the behavior of the real service + if (str_starts_with($path, 'http') || str_starts_with($path, '/')) { + return $path; + } + + return '/subdirectory/'.$path; + }); + + $renderer = new ImportMapRenderer($importMapGenerator, $assetPackages, polyfillImportName: false); + $html = $renderer->render(['app']); + + $this->assertStringContainsString(<<assertStringContainsString(<< Date: Wed, 30 Oct 2024 20:32:47 +0100 Subject: [PATCH 2/4] fabbot --- .../AssetMapper/Tests/Factory/MappedAssetFactoryTest.php | 4 ++-- .../AssetMapper/Tests/ImportMap/ImportMapRendererTest.php | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php b/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php index e86cee6b919d4..c957a37c86e79 100644 --- a/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php @@ -52,7 +52,7 @@ public function testCreateMappedAssetRespectsPreDigestedPaths() public function testCreateMappedAssetWithContentThatChanged() { - $file1Compiler = new class () implements AssetCompilerInterface { + $file1Compiler = new class implements AssetCompilerInterface { public function supports(MappedAsset $asset): bool { return true; @@ -105,7 +105,7 @@ public function testCreateMappedAssetWithContentErrorsOnCircularReferences() public function testCreateMappedAssetWithDigest() { - $file6Compiler = new class () implements AssetCompilerInterface { + $file6Compiler = new class implements AssetCompilerInterface { public function supports(MappedAsset $asset): bool { return true; diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php index 2f110291e2931..eea9f23bc1c96 100644 --- a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php @@ -212,7 +212,7 @@ public function testItAddsPreloadLinks() $this->assertSame('/assets/styles/app-preload-d1g35t.css', $linkProvider->getLinks()[0]->getHref()); } - public function testIntegrityMap(): void + public function testIntegrityMap() { $importMapGenerator = $this->createMock(ImportMapGenerator::class); $importMapGenerator->expects($this->once()) @@ -237,7 +237,7 @@ public function testIntegrityMap(): void 'app_css_no_preload' => [ 'path' => '/assets/styles/app-nopreload-d1g35t.css', 'type' => 'css', - 'integrity' => 'sha384-abc123-css-no' + 'integrity' => 'sha384-abc123-css-no', ], ]); @@ -267,6 +267,5 @@ public function testIntegrityMap(): void "/subdirectory/assets/app-no-preload-d1g35t.js": "sha384-abc123-js-no" } EOTXT, $html); - } } From e8c0c50d37ed4496c6385fcf632c3f4cfe633642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Andr=C3=A9?= Date: Thu, 31 Oct 2024 00:51:01 +0100 Subject: [PATCH 3/4] fix tests --- .../AssetMapper/Tests/Factory/MappedAssetFactoryTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php b/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php index c957a37c86e79..49eabd0f06e0e 100644 --- a/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php @@ -36,7 +36,7 @@ public function testCreateMappedAsset() $this->assertSame('file2.js', $asset->logicalPath); $this->assertMatchesRegularExpression('/^\/final-assets\/file2-[a-zA-Z0-9]{7,128}\.js$/', $asset->publicPath); $this->assertSame('/final-assets/file2.js', $asset->publicPathWithoutDigest); - $this->assertSame('sha384-ZDljYTViYzY0NTgyZjA4ZTBmMjgwODY1NDNlMmRhMTY2NTVlOTNhYTlkZjMwZGY5YzU0NjdlNDExMThjY2RjNGFmNWZkNDhmZDg0ODIzMmVmMjkyNmIwNGE2NGJkMjdi', $asset->integrity); + $this->assertSame('sha384-2cpbxkWC8I4PKAhlQ+LaFmVek6qd8w35xUZ+QRGMzcSvX9SP2EgjLvKSawSmS9J7', $asset->integrity); } public function testCreateMappedAssetRespectsPreDigestedPaths() @@ -47,7 +47,7 @@ public function testCreateMappedAssetRespectsPreDigestedPaths() $this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css', $asset->publicPath); // for pre-digested files, the digest *is* part of the public path $this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css', $asset->publicPathWithoutDigest); - $this->assertSame('sha384-YThlMTY4MzI3MGY3ZjFlNTk0M2VhMDQ0MzMyYjEwYjRkNGQ2NjU4YzZlMDZjYjA3YTgwNDUzNjUwOTQyOGI4NjQ1YmFiMmIyMzg4ZWZhOGRiMGQ5MjU4MjJjNThlOTkz', $asset->integrity); + $this->assertSame('sha384-qOFoMnD38eWUPqBEMysQtNTWZYxuBssHqARTZQlCi4ZFurKyOI76jbDZJYIsWOmT', $asset->integrity); } public function testCreateMappedAssetWithContentThatChanged() From 82b44daac183be9b7dd2e7e1f513f08045f56422 Mon Sep 17 00:00:00 2001 From: pierreboissinot Date: Thu, 31 Oct 2024 00:07:22 +0100 Subject: [PATCH 4/4] [AssetMapper] Add integrity hash to modulepreload --- .../AssetMapper/ImportMap/ImportMapRenderer.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php index abb555d8be9a9..4a08c04d3257e 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php @@ -78,7 +78,7 @@ public function render(string|array $entryPoint, array $attributes = []): string $integrityMap[$path] = $integrity; } if ($preload) { - $modulePreloads[] = $path; + $modulePreloads[] = ['path' => $path, 'integrity' => $integrity]; } } elseif ($preload) { $cssLinks[] = $path; @@ -142,10 +142,13 @@ public function render(string|array $entryPoint, array $attributes = []): string HTML; } - foreach ($modulePreloads as $url) { - $url = $this->escapeAttributeValue($url); + foreach ($modulePreloads as $modulePreload) { + $url = $this->escapeAttributeValue($modulePreload['path']); + if ($integrity = $modulePreload['integrity']) { + $integrity = " integrity=\"$integrity\""; + } - $output .= "\n"; + $output .= "\n"; } if (\count($entryPoint) > 0) {