CSP compliance

[jr01]

Describe the bug

Hi!

I'm working on content-security-policy (CSP) compliance in our product and getting a console error in Angular Slickgrid:

custom-angular-slickgrid.component.html:3 Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self' 'nonce-randomNonceGoesHere'". Either the 'unsafe-inline' keyword, a hash ('sha256-XXXXXX='), or a nonce ('nonce-...') is required to enable inline execution. Note that hashes do not apply to event handlers, style attributes and javascript: navigations unless the 'unsafe-hashes' keyword is present.

This is because of the style= in

Angular-Slickgrid/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.html

Line 3 in e8b3792

<div attr.id='{{gridId}}' class="slickgrid-container" style="width: 100%">

The grid is working and displaying fine however, so it's not a blocker...

Reproduction

Sorry no minimal repro, I can create it later if desired, but a summart of what I did in my app.

As described here https://angular.io/guide/security#content-security-policy I changed my index.html to <app-root ngCspNonce="randomNonceGoesHere"></app-root>.

For ng serve support I updated angular.json to set the CSP header on response:

"serve": {
  "builder": "@angular-devkit/build-angular:dev-server",
  "options": {
    "headers": {
      "Content-Security-Policy": "default-src 'self';script-src 'self' 'nonce-randomNonceGoesHere';style-src 'self' 'nonce-randomNonceGoesHere'"
    }
  }

As described here https://ghiscoding.gitbook.io/slickgrid-universal/developer-guides/csp-compliance I did something like the following in my component:

constructor(
  @Inject(CSP_NONCE) private _cspNonce: string
  )
  
  ...
  
	if (this._cspNonce) {
	  this._gridOptions.enableHtmlRendering = false;
	  this._gridOptions.nonce = this._cspNonce;
	  this._gridOptions.dataView.useCSPSafeFilter = true;
	}

And lastly I updated all my formatters to return HtmlResult.

Expectation

A simple fix would be to change to [ngStyle]="{ width: '100%' }". I already tested that by subclassing the AngularSlickgridComponent on my end.

But perhaps :

1. The width: 100% style can be moved to .slickgrid-container or removed completely since it didn't do anything for me... I will investigate/test of course, but perhaps you remember why it was put there?
2. Could the @Inject(CSP_NONCE)... and setting the grid options be moved to relevant places in Angular Slickgrid? Then CSP would just work out-of-the-box :) I haven't yet investigated what solution other components are using.
3. There could be more CSP issues with Angular Slickgrid that pop-up. Should CSP be enabled in the Angular Slickgrid demo site and cypress tests? That would probably reveal all issues now and with future changes.

Environment Info

Angular 17.1.3
Angular-Slickgrid 7.4.1

Validations

• Follow our Code of Conduct
• Read the HOWTO - Quick Start.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion.
• The provided reproduction is a minimal reproducible example of the bug.

[ghiscoding]

• The width: 100% style can be moved to .slickgrid-container or removed completely since it didn't do anything for me... I will investigate/test of course, but perhaps you remember why it was put there?

Yeah I guess we could remove it but it might be better to add this back in the stylesheet, probably better to do that in Slickgrid-Universal since I don't have any CSS in Angular-Slickgrid anymore.

• Could the @Inject(CSP_NONCE)... and setting the grid options be moved to relevant places in Angular Slickgrid? Then CSP would just work out-of-the-box :) I haven't yet investigated what solution other components are using.

There's already a nonce grid option which then adds it the SlickGrid dynamic stylesheet. CSP is not a fun exercise in Angular, we opted to do it on the server side instead (we are not using Angular-Slickgrid in that project though, in fact I don't have any recent project using my libs sadly).

• There could be more CSP issues with Angular Slickgrid that pop-up. Should CSP be enabled in the Angular Slickgrid demo site and cypress tests? That would probably reveal all issues now and with future changes.

I've done a lot of work with another user's help to make SlickGrid CSP compliant, you can see all the PRs we've done in 6pac/slickgrid (and the issue that started it), which as you can imagine were all replicated to Slickgrid-Universal as well. I enabled some CSP in Slickgrid-Universal in the index.html, I do have the script-src and style-src as unsafe but it's mainly because I use plain JS in there without a framework so my plain code for the demo itself is for sure not CSP compliant. Still we shouldn't need them in other frameworks like Angular but I did not have the time to try it.

Also note that in the SlickGrid project, with help from the other user, we decided to make certain CSP options as separate options because it had an impact on perf. I wrote a quick Wiki on SlickGrid project and you can see this SlickGrid Example. From the Wiki you'll see this SlickDataView({ useCSPSafeFilter: true }), which in my lib can be enabled via the grid options like this

this.gridOptions = {
  dataView: { useCSPSafeFilter: true, }
}

I should also eventually replicate this Wiki to my own docs, the approach would be similar.

I certainly welcome any contributions in that regards, I'm not sure it would work with all examples though

[jr01]

• The width: 100% style can be moved to .slickgrid-container or removed completely since it didn't do anything for me... I will investigate/test of course, but perhaps you remember why it was put there?

Yeah I guess we could remove it but it might be better to add this back in the stylesheet, probably better to do that in Slickgrid-Universal since I don't have any CSS in Angular-Slickgrid anymore.

Did some testing. Removing the width: 100% changed nothing in the demo site (fixed width, variable width grid etc). Only thing I can think of is some user that has applied padding/margin to .slickgrid-container or to .gridPane then things might change. I also noticed Slickgrid Aurelia has the same, so, as you mention, perhaps it's best to just move this last piece of styling to Slickgrid-Universal css.

• Could the @Inject(CSP_NONCE)... and setting the grid options be moved to relevant places in Angular Slickgrid? Then CSP would just work out-of-the-box :) I haven't yet investigated what solution other components are using.

There's already a nonce grid option which then adds it the SlickGrid dynamic stylesheet. CSP is not a fun exercise in Angular, we opted to do it on the server side instead (we are not using Angular-Slickgrid in that project though, in fact I don't have any recent project using my libs sadly).

• There could be more CSP issues with Angular Slickgrid that pop-up. Should CSP be enabled in the Angular Slickgrid demo site and cypress tests? That would probably reveal all issues now and with future changes.

I've done a lot of work with another user's help to make SlickGrid CSP compliant, you can see all the PRs we've done in 6pac/slickgrid (and the issue that started it), which as you can imagine were all replicated to Slickgrid-Universal as well. I enabled some CSP in Slickgrid-Universal in the index.html, I do have the script-src and style-src as unsafe but it's mainly because I use plain JS in there without a framework so my plain code for the demo itself is for sure not CSP compliant. Still we shouldn't need them in other frameworks like Angular but I did not have the time to try it.

Also note that in the SlickGrid project, with help from the other user, we decided to make certain CSP options as separate options because it had an impact on perf. I wrote a quick Wki on SlickGrid project and you can see this SlickGrid Example. From the Wiki you'll see this SlickDataView({ useCSPSafeFilter: true }), which in my lib can be enabled via the grid options like this

this.gridOptions = {
  dataView: { useCSPSafeFilter: true, }
}

So I took a swing at converting the demo site to be CSP compliant, but while doing so I realized some examples would become a bit complicated for the average user to understand. E.g. the column formatters can't return a string with html like <span>...</span> anymore, but would have to be written like const elem = document.createElement('span'); ...; return elem....

Then I could bring @Inject(CSP_NONCE)... part into AngularSlickgridComponent and set all relevant options in mergeGridOptions method, but I don't think there is much value to it, since a user (like myself) has a good enough documentation about CSP from the slickgrid universal site and would need to do some work anyway to make their application/site CSP compliant.

So I'm also throwing both idea's in the fireplace :)

So there is just 1 thing left and that is to move the width: 100% to Slickgrid-Universal and remove it from this lib, Aurelia etc. But I'm not exactly sure what I should do to make all that happen and I think you are 100x faster than me in that regard :) So if you could make that happen perhaps 😍 ?

[ghiscoding]

E.g. the column formatters can't return a string with html like <span>...</span> anymore, but would have to be written like const elem = document.createElement('span'); ...; return elem....

Yes and No, one of the thing I've changed in latest major to be more CSP Compliant is to have Formatters accept not only strings but also native HTML Element and also internally since I added DOMPurify I changed the default option to return TrustedHTML. So both approach should still pass CSP, however 1 thing to note is that a Formatter with native HTML Element will have better perf since it doesn't have to convert HTML string to HTML elements for the DOM, so it's faster (but you probably won't see much of a difference since SlickGrid uses virtual scroll)

Then I could bring @Inject(CSP_NONCE)... part into AngularSlickgridComponent and set all relevant options in mergeGridOptions method, but I don't think there is much value to it, since a user (like myself) has a good enough documentation about CSP from the slickgrid universal site and would need to do some work anyway to make their application/site CSP compliant.

Yeah I don't want to make it more complicate for regular non-CSP users and very often enabling CSP also mean perf drops, so it's better to stay raw in some cases. Oh and I forgot that I added CSP Docs only on the new GitBook docs website, so I did write about it :)

So there is just 1 thing left and that is to move the width: 100% to Slickgrid-Universal and remove it from this lib, Aurelia etc. But I'm not exactly sure what I should do to make all that happen and I think you are 100x faster than me in that regard :)

Yeah of course, I will do that in a bit. Like I said earlier, I'm working on a new feature that mainly touches the styling, so I'll add it to my TODOs ;)

[ghiscoding]

@jr01 this should be fixed now, hope you also enjoy the brand new Dark Mode 😉 Cheers

[jr01]

Wow. Thanks @ghiscoding !