Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an enableIntegrityHashes() method to the public API #522

Merged
merged 1 commit into from Mar 29, 2019

Conversation

Lyrkan
Copy link
Collaborator

@Lyrkan Lyrkan commented Feb 13, 2019

This PR adds an Encore.enableIntegrityHashes() method that allows to compute integrity hashes of all the files referenced in the entrypoints.json (closes #418).

For instance:

// Enable it for all builds with the
// default hash algorithm (sha384)
Encore.enableIntegrityHashes();

// Enable it only in production with
// a custom hash algorithm
Encore.enableIntegrityHashes(
    Encore.isProduction(),
    'sha512'
);

And here is the resulting entrypoints.json file:

{
  "entrypoints": {
    "main": {
      "css": [
        "/main.3d1dcb7e.css"
      ],
      "js": [
        "/main.7c6b7c81.js"
      ]
    }
  },
  "integrity": {
    "/main.3d1dcb7e.css": "sha384-ce7d1nV3CFoSIfinwm53befb9CMHNAAlPEb61deOf3zBvpXK9lct44/U2ieSOKt4",
    "/main.7c6b7c81.js": "sha384-kHFhNTJlbSuDijSTimOHZGTxKzlLYCWc03AmmRSAE3b173SMlGiQG6uasAzl29+0"
  }
}

@Lyrkan Lyrkan force-pushed the integrity-hashes branch 3 times, most recently from 8584814 to 8ab3959 Compare February 14, 2019 13:09
@Lyrkan
Copy link
Collaborator Author

Lyrkan commented Feb 14, 2019

Slightly changed how the integrity part of the entrypoints.json looks, from:

{
    "integrity": {
        "file1" : { "algorithm": "<alg>", "hash": "<hash>" },
        "file2" : { "algorithm": "<alg>", "hash": "<hash>" }
    }
}

to:

{
    "integrity": {
        "algorithm": "<alg>",
        "hashes": {
            "file1" : "<hash>",
            "file2" : "<hash>"
        }
    }
}

I thought the first version could be useful (for instance if we added a way to use a different algorithm for each file) but after thinking about it for a while I'm not sure that's really needed.

Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comments - this is going to be really, really nice. Thanks for the work so far!


if (webpackConfig.integrityAlgorithm) {
for (const file of assets[asset][fileType]) {
if (!(file in integrity.hashes)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is file sometimes already in integrity.hashes? Like, maybe a shared file that's used by multiple entries?

Also, we can flip this to `if (file in integrity.hashes) { continue; }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is file sometimes already in integrity.hashes? Like, maybe a shared file that's used by multiple entries?

Yep, the runtime.js file for instance :)

Also, we can flip this to `if (file in integrity.hashes) { continue; }

Will be part of the next changeset!

}, null, 2);
return JSON.stringify({
entrypoints: assets,
integrity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include the integrity key at all if the feature is disabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to be able to detect if Encore supported that feature to make the bundle display a relevant error if someone tried to use it with an old version of Encore.

But since I also agree about not making it optional in the bundle anymore (by detecting if integrity is present in the manifest) that's not needed anymore :)

@Lyrkan
Copy link
Collaborator Author

Lyrkan commented Mar 1, 2019

Ok so, changed the content again to something like:

{
    "integrity": {
        "file1" : "<alg>-<hash1>",
        "file2" : "<alg>-<hash2>"
    }
}

The reason being that it now allows to generate multiple hashes for each file (which is a feature supported by the integrity attribute):

{
    "integrity": {
        "file1" : "<alg1>-<hash1> <alg2>-<hash2>",
        "file2" : "<alg1>-<hash3> <alg2>-<hash4>"
    }
}

Which also means that the Encore.enableIntegrityHashes() can now take an array of strings as its second parameter, for instance:

Encore.enableIntegrityHashes(
    true,
    ['sha256', 'sha384', 'sha512']
);

@weaverryan
Copy link
Member

Needs a rebase now :)

@weaverryan
Copy link
Member

This is going to be awesome! Thank you @Lyrkan!

@weaverryan weaverryan merged commit fa915aa into symfony:master Mar 29, 2019
weaverryan added a commit that referenced this pull request Mar 29, 2019
…(Lyrkan)

This PR was merged into the master branch.

Discussion
----------

Add an enableIntegrityHashes() method to the public API

This PR adds an `Encore.enableIntegrityHashes()` method that allows to compute integrity hashes of all the files referenced in the `entrypoints.json` (closes #418).

For instance:

```js
// Enable it for all builds with the
// default hash algorithm (sha384)
Encore.enableIntegrityHashes();

// Enable it only in production with
// a custom hash algorithm
Encore.enableIntegrityHashes(
    Encore.isProduction(),
    'sha512'
);
```

And here is the resulting `entrypoints.json` file:
```json
{
  "entrypoints": {
    "main": {
      "css": [
        "/main.3d1dcb7e.css"
      ],
      "js": [
        "/main.7c6b7c81.js"
      ]
    }
  },
  "integrity": {
    "/main.3d1dcb7e.css": "sha384-ce7d1nV3CFoSIfinwm53befb9CMHNAAlPEb61deOf3zBvpXK9lct44/U2ieSOKt4",
    "/main.7c6b7c81.js": "sha384-kHFhNTJlbSuDijSTimOHZGTxKzlLYCWc03AmmRSAE3b173SMlGiQG6uasAzl29+0"
  }
}
```

Commits
-------

fa915aa Add an enableIntegrityHashes() method to the public API
@Phobetor
Copy link

Epic! This will be really useful. Thanks @Lyrkan!

weaverryan added a commit to symfony/webpack-encore-bundle that referenced this pull request Mar 29, 2019
This PR was squashed before being merged into the master branch (closes #42).

Discussion
----------

Add support for integrity hashes

This PR allows to automatically add `integrity` attributes on `<script>` and `<link>` tags based on the content of the `entrypoints.json` file (related to the following PR on Encore: symfony/webpack-encore#522).

It requires the following configuration:

```js
// webpack.config.js
// Enable it for all builds with the
// default hash algorithm (sha384)
Encore.enableIntegrityHashes();

// Or enable it only in production
// with a custom hash algorithm
Encore.enableIntegrityHashes(
    Encore.isProduction(),
    'sha384'
);

// Or with multiple hash algorithms
Encore.enableIntegrityHashes(
    Encore.isProduction(),
    ['sha384','sha512']
);
```

Then, calling `yarn encore` then generates an entrypoints.json that contains hashes for all the files it references:

```js
{
  "entrypoints": {
    // (...)
  },
  "integrity": {
    "/build/runtime.fa8f03f5.js": "sha384-5WSgDNxkAY6j6/bzAcp3v//+PCXLgXCU3u5QgRXWiRfMnN4Ic/a/EF6HJnbRXik8",
    "/build/0.b70b772e.js": "sha384-FA3+8ecenjmV1Y751s0fKxGBNtyLBA8hDY4sqFoqvsCPOamLlA5ckhRBttBg1esp",
    // (...)
  }
}
```

And these hashes are automatically added when calling `encore_entry_script_tags` and `encore_entry_link_tags`:

```html
<html lang="en">
  <head>
    <!-- ... -->
    <link rel="stylesheet" href="/build/css/app.2235bc2d.css" integrity="sha384-Jmd35HF93DFCXjisVeMi6U3lniH/mOdAF6wLtOMqhYMh2ZiBRUdtF7jXB55IAKfm">
    <!-- ... -->
  </head>
  <body id="homepage">
    <!-- ... -->
    <script src="/build/runtime.fa8f03f5.js" integrity="sha384-5WSgDNxkAY6j6/bzAcp3v//+PCXLgXCU3u5QgRXWiRfMnN4Ic/a/EF6HJnbRXik8"></script>
    <script src="/build/0.b70b772e.js" integrity="sha384-FA3+8ecenjmV1Y751s0fKxGBNtyLBA8hDY4sqFoqvsCPOamLlA5ckhRBttBg1esp"></script>
    <!-- ... -->
  </body>
</html>
```

An example using Symfony Demo can be found here: Lyrkan/symfony-demo@91a06cd

Commits
-------

84c41ed Add support for integrity hashes
@@ -750,6 +752,25 @@ class WebpackConfig {
this.loaderConfigurationCallbacks[name] = callback;
}

enableIntegrityHashes(enabled = true, algorithms = ['sha384']) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also forbid passing enabled = true with an empty array as second argument ? That would actually disable it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include JavaScript file hashes in entrypoints.json for hash & nonce attributes (CSP)
4 participants