These are chat archives for assetgraph/assetgraph

11th
Oct 2016
Peter Müller
@Munter
Oct 11 2016 07:48
I updated assetgraph in the hyperlink package. That means it now populates fetch('...'). When I ran it as a test on a product in mobilelife hyperlink hung on accessing their API. Apparently they use fetch :)
Andreas Lind
@papandreou
Oct 11 2016 07:50
Hah :)
Andreas Lind
@papandreou
Oct 11 2016 07:57
@Munter Just released ag 5.0.0-28 / ag-b 5.0.0-21 with a proper fix for that inline script serialization bug that broke your blog with buildProduction --sourcemaps --contentsecuritypolicy along with that </script> problem I noticed.
Peter Müller
@Munter
Oct 11 2016 07:57
Nice
I think I'm about ready to move my blog over to netlify permanently
I just hope they move on that cache header bug soon. I can't override their defaults, so my static assets are not cached optimal
Andreas Lind
@papandreou
Oct 11 2016 07:59
Yeah, that's annoying. They said they were going to look into that?
Cool that you got that header manifest file to work for CSP.
Peter Müller
@Munter
Oct 11 2016 08:04
They said they were looking into it the day after I reported it. The latest i heard a week or two ago is that the path is bundled up with other changes, which are stuck in review
Apparently they are not fans of many small deploys. Ironic
Andreas Lind
@papandreou
Oct 11 2016 08:05
Hahahahaha :)
Do as I say, not as I do!
Peter Müller
@Munter
Oct 11 2016 09:52
@papandreou The wrong sha-calculation doesn't seem to work: view-source:https://mntr.netlify.com/
It looks like that script tag is html prettified?
When I didn't have source maps there were no newlines in it.
Andreas Lind
@papandreou
Oct 11 2016 10:15
Oh no :/
Will look after lunch
Peter Müller
@Munter
Oct 11 2016 10:42
I moved my blog permanently now. What is there is already far better than on gh-pages
Andreas Lind
@papandreou
Oct 11 2016 10:56
Regarding that CSP hash error -- the analytics script on https://mntr.netlify.com/ still has that trailing space character. I thought I fixed that :/
Yup, it's that.
Peter Müller
@Munter
Oct 11 2016 11:01
I wonder if netlifys npm cache played a trick on me there
I did bump ag-b to the latest version, so it should invalidate their npm cache for the project
Andreas Lind
@papandreou
Oct 11 2016 11:22
I'll try to see if I can reproduce it. I need to figure out how to install jekyll.
Peter Müller
@Munter
Oct 11 2016 11:23
oh dear. Don't bother
or, you need ruby's bundler. Then bundle install should work
It never has for me
Always crap
I can send you a tarball when I get home to my machine that does have jekyll
Andreas Lind
@papandreou
Oct 11 2016 11:28
@Munter sudo apt-get install jekyll did the trick :)
And I can reproduce the bug.
Peter Müller
@Munter
Oct 11 2016 11:32
Maybe I actually cut down on enough of the jekyll plugins that bundle install is not needed. Interesting
I've done a bit to remove all sorts of cruft with my upgrade to the latest jekyll
Should just migrte this to harp or wintersmith :P
Andreas Lind
@papandreou
Oct 11 2016 11:39
hehe
Hmm, weird, when I create a new buildProduction test case with build/index.html in its entirety, it doesn't happen.
Andreas Lind
@papandreou
Oct 11 2016 11:46
A-ha! Now I went back and added all the options provided to the buildProduction transform from the actual buildProduction command run by npm run build. That reproduces it. Turns out what breaks it is this seemingly innocent option:
    .buildProduction({
        // ...
        javaScriptSerializationOptions: { newline: '\n' }
    })
Peter Müller
@Munter
Oct 11 2016 11:52
I was wondering about those newlines. Didn't look minified enough to me :)
Andreas Lind
@papandreou
Oct 11 2016 11:55
Apparently I've misunderstood what escodegen.generate(ast, {format: { newline: '\n' } }); means. Will fix.
Peter Müller
@Munter
Oct 11 2016 11:58
From that statement I wouldn't be able to guess it
Andreas Lind
@papandreou
Oct 11 2016 11:59
Yeah, it seems like you're just passing the default option, right? :)
Peter Müller
@Munter
Oct 11 2016 11:59
I'm not passing anything formatting related
So how come the formatting is changed between the hashing and writing to disc? are we not marking an asset as dirty somewhere so it hashes on a different version?
Andreas Lind
@papandreou
Oct 11 2016 12:01
Apparently Chrome strips trailing whitespace from the script before computing the hash.
So we need to consistently do the same before computing ours. Safest bet seems to be always strip trailing whitespace from inline scripts, so that's my fix.
^ is why you get that behavior without explicitly requesting it.
Peter Müller
@Munter
Oct 11 2016 12:02
Interesting. That's going to bite some people that try to do manual hashing
Andreas Lind
@papandreou
Oct 11 2016 12:09
Yeah, I bet.
Try ag-b 5.0.0-22
Peter Müller
@Munter
Oct 11 2016 12:10
Done. Awaiting build
Andreas Lind
@papandreou
Oct 11 2016 12:11
Works for me locally.
(also when I remove tools/netlify-headers.js from npm run build :)
Peter Müller
@Munter
Oct 11 2016 12:14
And it's live :D
Andreas Lind
@papandreou
Oct 11 2016 12:14
@Munter Awesome :)
Peter Müller
@Munter
Oct 11 2016 12:15
For demo purposes I should publish the unbuilt code somewhere as well
Andreas Lind
@papandreou
Oct 11 2016 12:15
https://csp-evaluator.withgoogle.com/ is pretty happy, except that it wants you to add object-src 'none'
Peter Müller
@Munter
Oct 11 2016 12:15
So I can point to it s a use case for assetgraph
Andreas Lind
@papandreou
Oct 11 2016 12:16
Also, it suggests adding script-src 'unsafe-inline' to be compatible with older browsers (like I did yesterday as well)
The directive is ignored when there's a 'nonce-...' or '<algorithm>-<hash>' source for the same directive.
So script-src 'unsafe-inline' 'nonce-developmentonly'; style-src 'unsafe-inline' 'nonce-developmentonly' is a good combo.
Peter Müller
@Munter
Oct 11 2016 12:18
It's ignored when there is a nonce or a hash, but not if there is nothing? So in effect I just whitelisted anything inline
Andreas Lind
@papandreou
Oct 11 2016 12:19
I was hoping you'd ask that :). reviewContentSecurityPolicy avoids removing all nonces, thereby only leaving 'unsafe-inline'. I made it add the sha256 hash of the empty script in that edge case :)
So as long as you have 'nonce-developmentonly' alongside 'unsafe-inline' in dev, you'll be good.
An alternative would be removing 'unsafe-inline' as well if all nonces were removed and there are no inline scripts, but that would be slightly more invasive. For style-src, 'unsafe-inline' also governs whether the style=... attribute is allowed, and we currently don't include that in our CSP processing. I'm thinking that'll have to wait until CSP 3 and https://w3c.github.io/webappsec-csp/#unsafe-hashed-attributes-usage becomes available.
Andreas Lind
@papandreou
Oct 11 2016 13:15
Gah... assetgraph/assetgraph#635
Stupid Safari
Andreas Lind
@papandreou
Oct 11 2016 13:37
@Munter Interestingly, clicking your "Tweet This" button triggers a CSP error. It happens after landing on twitter.com, so that's probably a bug of theirs.
Peter Müller
@Munter
Oct 11 2016 13:39
That's twitters own csp though
Andreas Lind
@papandreou
Oct 11 2016 13:40
Yeah
Just a funny coincidence. Maybe they have a newer snippet that doesn't trigger it.
Peter Müller
@Munter
Oct 11 2016 13:44
I'm not using their js library anymore. Killed it an just use the url
Can seespee also potentially detect CSP violations?
Andreas Lind
@papandreou
Oct 11 2016 13:47
Yeah, we could add a validation mode.
Peter Müller
@Munter
Oct 11 2016 13:48
I guess that would potentially also require a js execution of things on each html page to see the full extent
Andreas Lind
@papandreou
Oct 11 2016 13:48
Right now its only mode is "whitelist stuff until it works"
Not if we still limit the scope to what's statically decidable.
Peter Müller
@Munter
Oct 11 2016 13:48
Right now I'm just having a hard time working with this concept of anything I add might break the csp for me somehow. Really would like a validator on a CI
Andreas Lind
@papandreou
Oct 11 2016 13:48
But you're right, pairing it up with jsdom and fetch+execute external resources.
The good news is that you should be getting CSP violations in dev for the things you've forgotten.
But yeah, I think you're right that there's a use case.
Peter Müller
@Munter
Oct 11 2016 13:56
The ideal CSP tool would be one that lets me not worry about policy during development, essentially running with no CSP defined. Then the production build figures things out and just adds what's neeed
Andreas Lind
@papandreou
Oct 11 2016 13:58
I'm afraid that's almost impossible -- you'd have to guarantee that you've exercised all code paths and views to be sure you've detected everything. There's also the "policy" part of it where we should encourage conscious decisions about what to whitelist.
I found having the CSP active in dev to be the best compromise.
Peter Müller
@Munter
Oct 11 2016 14:09
Is it possible to run CSP in dual mode? Have a more lax block policy and a stricter warning report policy so you can get real world feedback in production?
Andreas Lind
@papandreou
Oct 11 2016 14:10
@Munter Yeah, you can specify multiple CSPs and leave one of them in "report only" mode (Content-Security-Policy-Report-Only)
reviewContentSecurityPolicy will update each policy.
Peter Müller
@Munter
Oct 11 2016 14:11
Actually I think what I would like is the ability to define policies in direct relation to the additions that need them
So when I include the twitter widget I define the CSP on that script tag. Then it magically gets aggregated
I'm already starting to lose track of what things needed what parts of the policy
Andreas Lind
@papandreou
Oct 11 2016 14:12
Isn't that accomplished by changing the CSP as part of the commit that introduces the addition?
Fragmenting the CSP like that would be nice, and it's certainly possible for us to support that, but it would mean that the CSP can't be active without a build step.
Peter Müller
@Munter
Oct 11 2016 14:15
Noone I know goes into a full git history trawl when they need to detect single line changes over time. They try to grok it up front, then make a mistake, which then gets discovered later. I know because I am one of these developers
Would be nice to define the policy with yaml or json so it would at least be possible to track additions as separate lines instead of a substring modification in the same line
Andreas Lind
@papandreou
Oct 11 2016 14:18
How about:
        <meta http-equiv="Content-Security-Policy" content="
default-src 'self';
script-src 'self' 'nonce-developmentonly' https://www.google-analytics.com https://mntrdk.disqus.com/embed.js;
style-src 'self' 'unsafe-inline' https://fonts.googleapis.com/css a.disquscdn.com;
font-src https://fonts.gstatic.com;
frame-src https://disqus.com/;
img-src 'self' https://www.google-analytics.com a.disquscdn.com referrer.disqus.com https://img.youtube.com https://i.vimeocdn.com;
object-src 'none'
">
I'm pretty sure that gets re-compacted during buildProduction.
... But I know what you mean.
Peter Müller
@Munter
Oct 11 2016 14:21
<meta http-equiv="Content-Security-Policy" content="
default-src
    'self'
;

script-src
  'self'
  'nonce-developmentonly'
  https://www.google-analytics.com https://mntrdk.disqus.com/embed.js
;

style-src
  'self'
  'unsafe-inline' https://fonts.googleapis.com/css a.disquscdn.com
;

font-src
  https://fonts.gstatic.com
;

frame-src
  https://disqus.com/
;

img-src
  'self'
  https://www.google-analytics.com
  a.disquscdn.com
  referrer.disqus.com
  https://img.youtube.com
  https://i.vimeocdn.com
;

object-src
  'none'
;
">
;)
Andreas Lind
@papandreou
Oct 11 2016 14:22
Sure, why not :)
Peter Müller
@Munter
Oct 11 2016 14:22
Still decoupled from their context though
Andreas Lind
@papandreou
Oct 11 2016 14:22
Try it and let me know if it helps
Peter Müller
@Munter
Oct 11 2016 14:25
It would be nice to have an additive mode, so I can add another set of rules further down that extend the previous ones. Then put that right next to the thing that triggers those rules
Then we are in 'needs a build-system'-territory again
Andreas Lind
@papandreou
Oct 11 2016 14:26
Yeah, that would be a nice feature. The semantics of having multiple CSPs is that each CSP in effect has to thumbs up each thing.
... which is almost exactly the opposite fo what you want.
Peter Müller
@Munter
Oct 11 2016 14:26
Yeah I want just one of them to green light
Obviously that can have downsides as well
Andreas Lind
@papandreou
Oct 11 2016 14:37
CSP is obviously not a zero-cost thing to deploy in its current state. It scores "high difficulty" here: https://wiki.mozilla.org/Security/Guidelines/Web_Security
I hope we're making it just a little bit easier.
Peter Müller
@Munter
Oct 11 2016 14:39
Auto generating hashes of inline scripts certainly helps
Andreas Lind
@papandreou
Oct 11 2016 21:34
@Munter Any opinion on assetgraph/assetgraph#635 ?