Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

App production checks #24

Open
4 tasks
travi opened this issue Apr 23, 2018 · 9 comments
Open
4 tasks

App production checks #24

travi opened this issue Apr 23, 2018 · 9 comments

Comments

@travi
Copy link
Member

travi commented Apr 23, 2018

@lirantal
Copy link

I'm happy to help with these
Also consider lockfile-lint and the https://snyk.io/blog/why-npm-lockfiles-can-be-a-security-blindspot-for-injecting-malicious-modules/ as a reference reading resource

@travi
Copy link
Member Author

travi commented Nov 29, 2019

hi @lirantal 👋

thanks for checking in. i'm curious how you spotted this one :)

already have lockfile-lint covered but hoping to find a good way to get the others tied in as well. since these need to run against a served site, there is a little more complexity to including these in a normal build, but i'm not too far from enabling that more easily depending on the type of project that is generated.

@lirantal
Copy link

GitHub makes it easy with the Used by:

image

And cool, I actually didn't spot it there in those links :)

Any specific issues you ran into? seems to that "it just works" but happy to hear if you have any input to improve things.

@travi
Copy link
Member Author

travi commented Nov 30, 2019

things have been smooth with lockfile-lint so far, where i've gotten it added.

it took a bit of care to make sure i was using the flags that applied to my projects since most aren't enabled by default. i could maybe see it being more strict by default, but can understand that being a tough sell with the various configurations folks use.

i was a little worried that i'd see false-positives around https checking since that tends to thrash, but the npm team says they are always downloaded over https. i havent seen any warnings related to that yet, but maybe its related to most of the places i've added it being new projects using the latest npm cli.

@travi
Copy link
Member Author

travi commented Nov 30, 2019

also, kinda neat to see that 4 of the 8 badges in your used-by shot above are my projects :)

@travi
Copy link
Member Author

travi commented Dec 1, 2019

so, your nudge did motivate me to at least try out is-website-vulnerable on one of my sites. next step will be to figure out where to include it in the scaffolding process.

i did run into one issue when i pushed to travis that might be worth a more specific error. a quick search for the error suggested that the installed chrome was too old which was easily remedied by specifying a newer version, but i could see that tripping up some users.

@lirantal
Copy link

lirantal commented Dec 1, 2019

also, kinda neat to see that 4 of the 8 badges in your used-by shot above are my projects :)

you are my power user! 🎉
maybe I should considering getting some stickers and send them over :)

i did run into one issue when i pushed to travis that might be worth a more specific error.

oh this is really cool that you've pushed it as part of the linters!
that error is indeed probably related to chrome and lighthouse settings within a build job such as Travis CI. Thanks for fishing out the solution. Could you perhaps contribute that as a FAQ section in the readme for the is-website-vulnerable project?

also, if there's a more elegant way of handling this through some sharable configs I'd love to make it more apparent and easier to users.

@travi
Copy link
Member Author

travi commented Dec 5, 2019

you are my power user! 🎉
maybe I should considering getting some stickers and send them over :)

wiring into my scaffolder might be cheating a bit :) like many, i am a fan of stickers, but no need to go to extra effort on my account. i appreciate the effort you've been putting toward these tools.

oh this is really cool that you've pushed it as part of the linters!

getting this type of thing into the script that runs on every commit gives me the best chance to keep up. i opened a PR to mention local installation and use in an npm script as an option, but happy to tweak if you have feedback.

Could you perhaps contribute that as a FAQ section in the readme for the is-website-vulnerable project?

done. i started as a warning under the usage section rather than breaking into a new FAQ section, but again, happy to adjust if you have other thoughts.

also, if there's a more elegant way of handling this through some sharable configs I'd love to make it more apparent and easier to users.

this is always tricky to get to a good spot for a wide range of users. is-website-vulnerable seems pretty simple out of the box. lockfile-lint might have some opportunities, but that could easily be a skewed perspective since its easy to assume that my conventions are good community defaults. it does feel like the --validate-https flag could be enabled by default to encourage more strictness and instead enable opting-out. since i'm not a yarn user, i'm not sure if there is enough to go on, but it could be nice to automatically detect yarn/npm and then have some smart defaults for the related options. maybe this is only possible if a lockfile is present?

@lirantal
Copy link

lirantal commented Dec 6, 2019

Thanks for the input!

I love that comment about --validate-https enabled by default - maybe you could shoot a PR on the repo for that to make a change in the CLI part? I believe we do detect npm or yarn lockfiles automatically but if not then or something is missing then this would be an ideal PR too.

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

No branches or pull requests

2 participants