webpack: lockdown inlining #1101
Conversation
github-actions
bot
added
documentation
legobeat
force-pushed
the
naugtur/lockdown-inlining
branch
from
5119a84
to
0fc85d6
Compare
packages/webpack/README.md
Outdated
| - The plugin is attempting to add it as an asset to the compilation for the sake of Developer Experience. Feedback welcome. | ||
| - [SES lockdown][] must be added to the page without any bundling or transforming for any security guarantees to be sustained. | ||
| - The plugin is attempting to add it as an asset to the compilation for the sake of Developer Experience. `.js` extension is omitted to prevent minification. | ||
| - Optionally lockdown can be inlined into the bundle files. It's hard to determine which files are loaded when, so providing filenames for scripts that get to load as the first script on the page to apply lockdown only once is how one uses inlining. |
There was a problem hiding this comment.
This sentence is a little hard to understand
There was a problem hiding this comment.
It was even harder to write 😅
| await t.notThrowsAsync(async () => { | ||
| t.context.build = await scaffold(webpackConfig) | ||
| }, 'Expected the build to succeed') |
There was a problem hiding this comment.
the side-effect here is little malodorous to me, but I'm not sure what a better solution would look like.
There was a problem hiding this comment.
I'm not sure what you mean is a side effect here. scaffold is running the build and returning information it collects. If it fails, we report it as a testsuite failure.
Co-authored-by: Christopher Hiller <boneskull@boneskull.com>
| - The plugin is attempting to add it as an asset to the compilation for the sake of Developer Experience. Feedback welcome. | ||
| - [SES lockdown][] must be added to the page without any bundling or transforming for any security guarantees to be sustained. | ||
| - The plugin is attempting to add it as an asset to the compilation for the sake of Developer Experience. `.js` extension is omitted to prevent minification. | ||
| - Optionally lockdown can be inlined into the bundle files. It's hard to determine how entries in the config are deployed and used, so you need to list the scripts that get to load as the first script on theDo we have page to apply lockdown only once when inlined. When you have a single bundle, you just configure a list with one element. It gets more complex with builds for multiple pages. |
There was a problem hiding this comment.
| - Optionally lockdown can be inlined into the bundle files. It's hard to determine how entries in the config are deployed and used, so you need to list the scripts that get to load as the first script on theDo we have page to apply lockdown only once when inlined. When you have a single bundle, you just configure a list with one element. It gets more complex with builds for multiple pages. | |
| - Optionally, lockdown can be inlined into the bundle files. The user is expected to provide a list of the scripts allowed to load as the first script on the page. |
The rest of the text doesn't really seem relevant for the user (and the "single-item-can-be-provided-as-single-element-list" should be obvious)
There was a problem hiding this comment.
While the wording could be better (and I can see a copy-paste error now), I insist on explaining that we can't simply infer this from the webpack configuration and need the user to specify files to inline into.
There was a problem hiding this comment.
I think it's enough to make it clear that we need the user to specify the files - alluding to our perceived difficulty in automatically deriving it isn't really helpful and comes off at an attempt at an excuse at best and confusing at worst (:
There was a problem hiding this comment.
It is an excuse for the folks with a basic setup where there's only one resulting bundle file that I decided not to support for consistency. Wanna change that?
There was a problem hiding this comment.
I think the behavior is fine and I agree we should make it clear what configuration is required. But let's stick to the point and keep any discussion of implementation difficulties in developer docs as opposed to readme?
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com> Co-authored-by: Christopher Hiller <boneskull@boneskull.com>
Some use-cases (including snaps) need to inline SES lockdown into the bundle file. It's impossible to flawlessly guess which file will be the one to load first on a new page in more complex configurations, so I went with a static list you have to provide. separate file lockdown remains the default approach.
Updated the README with explanation of this and got it up to date with implementation,