-
Notifications
You must be signed in to change notification settings - Fork 22
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
Package Security Check #72
Comments
That's a great idea, it could be added as another source here, by batching the calls if possible, it would be similar to how the downloads are fetched. Do you want to work on this @mitchellsimoens? |
@mitchellsimoens @Haroenv I really like this idea. Maybe someone from @nodesecurity would be willing to let us know whether they are on board and possibly review final interface and/or implementation? |
👍 we previously considered showing an "insecure" badge in the search results at https://www.jsdelivr.com and having this information available directly in the index would definitely make it easier. |
You can request the data from 200 packages at the same time, I have a small demo I can push where other people can help |
@Haroenv if you already started something I would love to help on that branch, maybe you can split up work and assign me little tasks I can attempt to address in single commits? |
https://github.com/algolia/npm-search/tree/feat/nsp, but it's pretty late here and there's some bug that needs fixing where the |
Going to take a look at the bug and see if I can get the key to appear in the correct spot. Is there anything extra I need to know about first time dev setup not in readme? |
You need an Algolia app with api key and index. If you tell me which appid I can give you more records and operations so you don’t need to pay @rdela |
Nice. Signed in with GH earlier and went through onboarding, it was stellar and docs look like they are same caliber. My first round of questions: Ok to use this appid made during tour? If so, ok to post that here in issue or should I email/DM it to you somewhere? Tried to create new app and got…
I guess the default appId ( Looks like I can just Do I need to worry about Assuming readme Usage > Development production warning…
will not apply because I will not have a “production” index yet? |
Yes that should be good. The appId is not exactly secret so you can send it here if you need more usage
oh yeah that's right, I think if you pick a (free) plan you can create new apps, just not in trial.
Yes, that's the appid used in production
yep, both are equivalent for this usage.
you can put the index name in config.js (where it has a default of
it will create or append to that index
This is for people at Algolia with access to the production index. |
Cool, appid: Trial seems pretty generous and I am only testing this for now. If you want to wait to uncap til I hit limits or trial expires in 13 days, fine by me, can mention you here then and include key again. Any reason to get rid of Like the Actors example and way it demos searchable attributes and custom ranking formula, might refer to it later on, but could always create another account and leave in free plan for that. So would you mind elaborating on:
…the one we Want to make sure I know how to tell if/when it is fixed before I dive in though. |
I think I did something wrong when mapping in the weekend, the data wasn't present in the index, I haven't taken a look at what the cause was yet. It'll be fixed if the records have a You can remove or keep the getting started index, it won't impact your testing here. Let us know if you want extended records/operations anytime |
Great news all around, think I am headed in the right direction, will try to PR in next day or so, thanks Haroen! |
@Haroenv thanks for stopping by the Gatsby issue, this is still on my radar and I was not trying to bring this discussion over there, only to ask you about Gatsby plugin index design in progress in case you had ideas from all of your experience implementing and helping others implement search. I am not sure whether we will end up displaying security info in the initial Gatsby index yet even though I personally am heavily in favor. Part of the reason I mentioned you was the idea to show some sort of indication that a package is not 1.0 yet is interesting, and may be worth me opening a separate issue on the Yarn site repo about. But we can circle back to it once this is merged. Will let you know how I get on with the code tomorrow and over the weekend, have some time set aside for this so should be able to have something by Monday. Will you be the one implementing the Yarn interface? Is it worth trying to get it displaying in their templates in some form once I get data matched correctly or we would need to work that out later? |
I or someone else can do it on yarnpkg/website. It should be pretty straightforward to simply display it a little like the deprecated view and some link back to nsp @rdela |
Cool thanks @Haroenv will PR this before I mess with Yarn site |
@Haroenv Ok so I am even with you now on my fork, first run fails with: Is this a me still being in trial issue? We are pulling |
@Haroenv added both of your emails I saw glancing through commit logs as collaborators on my trial app so you can see firsthand if you like |
@rdela, yes there will be some records that are too big for a regular index, you can filter those out. Is it the nsp data which is too big? Maybe we should try to find a more data conservative way of writing it 🤔 |
Is this being worked on? |
Feel free to work on this @jimaek |
Yes, exactly what Haroen said, feel free to work on this. I have not touched it since comments above from Dec. 2017 but am willing to try to help anyone make progress if they want to attempt to bring it up to date. That said, my time is very limited currently so I may not be of much assistance. The feat/nsp branch Haroen created is still available on my fork even though it has been deleted from this repo. NSP/^Lift were swallowed by npm in April 2018. The nsp package was deprecated 7 May 2018 and the Although the use case is different since it wants a |
Ok, we will check it out since we want to use this at jsDelivr |
@jimaek do you still need it? |
In our case Snyk sent a PR directly jsdelivr/www.jsdelivr.com#330 |
I think it'd be quite helpful to check the package and it's dependencies for security issues like nsp and display it in the package details page and may even be a great idea to put in the search result list.
The text was updated successfully, but these errors were encountered: