Skip to content

Add new output callback function #44

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

Closed
wants to merge 1 commit into from

Conversation

fcsonline
Copy link

Summary

Adding a new output callback function passing the computed CSP rules to execute custom logic.

Our team is interested to add CSP policy, but we want to add it as HTTP header instead of meta tag. This enables the capability to use frame-ancestors, report-uri, or sandbox not available with meta tag.

Right now, the rule is automatically added as meta tag. To be able to add it as HTTP header, we need to compute the rule by csp-html-webpack-plugin and pass it to a custom function to use it for example by Nginx.

Requirements (place an x in each [ ])

@CLAassistant
Copy link

CLAassistant commented Aug 9, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@fcsonline fcsonline force-pushed the feature/output-function branch from b41857a to 2a245bf Compare August 9, 2019 11:13
@codecov
Copy link

codecov bot commented Aug 9, 2019

Codecov Report

Merging #44 into master will decrease coverage by 16.08%.
The diff coverage is 37.5%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #44       +/-   ##
===========================================
- Coverage   93.85%   77.77%   -16.09%     
===========================================
  Files           2        2               
  Lines         114      117        +3     
  Branches       22       23        +1     
===========================================
- Hits          107       91       -16     
- Misses          6       23       +17     
- Partials        1        3        +2
Impacted Files Coverage Δ
plugin.js 72.63% <37.5%> (-19.76%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95d9f8b...2a245bf. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 9, 2019

Codecov Report

Merging #44 into master will decrease coverage by 16.63%.
The diff coverage is 46.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #44       +/-   ##
===========================================
- Coverage   93.85%   77.22%   -16.64%     
===========================================
  Files           2        2               
  Lines         114      180       +66     
  Branches       22       42       +20     
===========================================
+ Hits          107      139       +32     
- Misses          6       37       +31     
- Partials        1        4        +3     
Impacted Files Coverage Δ
plugin.js 74.05% <46.66%> (-18.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95d9f8b...2a245bf. Read the comment docs.

@AnujRNair
Copy link
Contributor

Thanks for your submission! Unfortunately, this is not currently a feature we're looking to add, so I'm going to close this PR.

@AnujRNair AnujRNair closed this Mar 17, 2020
@bcanseco
Copy link

bcanseco commented Apr 7, 2020

@AnujRNair Would you guys consider a PR for a hook that would let other plugins make use of this.buildPolicy()'s response? This webpack plugin is awesome, but I'm in the same situation as @fcsonline. Without frame-ancestors support, I'm looking into other ways to leverage the fine work you've done here.

@AnujRNair
Copy link
Contributor

@bcanseco sure, let's chat about it!

There are a few routes we could go down here, I'm interested in your use case. Are you looking to:

  • Use the CSP in another html-webpack-plugin? If so, we could look to attach some representation of the policy onto the pluginData variable so it can be accessed elsewhere
  • Use the CSP outside of webpack? If so, we can pass a callback function as originally requested so the developer can store the policy however they want, in whatever format they want

Let me know, and we can discuss some implementation details!

@AnujRNair AnujRNair reopened this Apr 7, 2020
@bcanseco
Copy link

bcanseco commented Apr 7, 2020

@AnujRNair I think my ultimate goal is the latter: to use the policy outside of webpack. Specifically inside an nginx configuration file (here's what I'm currently doing with @fcsonline's branch)

There's probably more elegant ways to go about it, but I think some kind of callback would work the best. Whether that be a new option like proposed in this PR or perhaps some way to get access to the policy from the enabled option when used as a function.

new CspHtmlWebpackPlugin(defaultPolicy, {
  enabled: (htmlPluginData, buildPolicy) => {
    const policy = buildPolicy();

    /* do stuff with the policy */

    return false;
  };
});

or...

new CspHtmlWebpackPlugin(defaultPolicy, {
  enabled: (htmlPluginData) => {
    const {policy} = htmlPluginData;

    /* do stuff with the policy */

    return false;
  };
});

@fcsonline
Copy link
Author

fcsonline commented Apr 7, 2020

Our team is doing something similar. We output the generated CSP rule to a plaintext file and we add a dynamic header in Nginx with a small Lua script. With this, we achieve all the benefits of CSP without the requirement of a full dynamic server.

The original pull request was designed to be opt-in, so developers with this requirement can tweak the webpack configuration to achieve it.

@AnujRNair
Copy link
Contributor

@bcanseco @fcsonline I created a pull request with the ability to overwrite the default processing function in #58

Would appreciate your reviews and thought there!

@AnujRNair AnujRNair closed this Apr 8, 2020
@fcsonline
Copy link
Author

Reviewed! It looks good! 👌

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.

4 participants