Skip to content
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

[SocketDevScore] Add badges for socket.dev πŸ›‘οΈ #7727

Closed

Conversation

valeriangalliat
Copy link

@valeriangalliat valeriangalliat commented Mar 16, 2022

This adds a number of badges for the Socket (@SocketDev) JavaScript supply chain analysis service.

Screenshot

cc @feross you might like to take a look to this PR too!

πŸ“‹ Description

This badge is for the service Socket (socket.dev) that scans npm packages for supply chain attacks.

On their report for a given npm package, e.g. fetch-cookie they show a number of scores for supply chain security, quality, maintenance, vulnerability and license:

Screenshot

This PR allows to embed a badge version of those scores, e.g. socket score | 99%, socket score (quality) | 85% and so on.

πŸ”— Data

The data comes from the https://socket.dev/api/npm/package-info/issues?name=<package> endpoint, which appears to be publicly accessible without API key. I'm not aware of an API documentation for it but the endpoint was very straightforward to find and understand.

Now I'm writing this I realize I don't have explicit authorization to use this endpoint even though it's publicly accessible so it would be great if @feross or someone from @SocketDev could look at this and give an approval (or not). 😁

🎀 Motivation

I think the Socket scores are a valuable metric for npm packages and I would like to embed them next to my other shields.io badges in the readme of my packages!

@calebcartwright
Copy link
Member

Thanks for your interest and submitting a PR

However, given that this is out of the blue without an associated issue (which is perfectly fine!), it would be helpful if you could also provide all the relevant details we ask for as part of the issue template for new badge requests.

Relevant sections from the template included below for reference, where applicable, please address all points for each specific badge you are proposing.

πŸ“‹ Description

A clear and concise description of the new badge.

  • Which service is this badge for e.g: GitHub, Travis CI
  • What sort of information should this badge show?
    Provide an example in plain text e.g: "version | v1.01" or as a static badge
    (static badge generator can be found at https://shields.io)

πŸ”— Data

Where can we get the data from?

  • Is there a public API?
  • Does the API requires an API key?
  • Link to the API documentation.

🎀 Motivation

  • What is the specific use case?

@valeriangalliat
Copy link
Author

Thank you @calebcartwright, I just updated the PR description to follow the issue template as you pointed out :)

@calebcartwright
Copy link
Member

Thank you for the additional detail @valeriangalliat and yes we want to get some kind of documentation or official blessing before using any endpoint for reasons Chris has summarized well in #5506 (comment)

@chris48s
Copy link
Member

Agreed - we should get a confirmation that we're good to use this API.

As a tangent, I was reading up on socket.dev the other day. At the moment their GitHub app only supports detecting when you've accidentally added a typosquatted dependency as a dependency but their "coming soon" featureset for the GitHub app looks like it could be really useful in helping us flag package bump PRs that warrant additional review so I may talk about this more in another issue if those features materialize. I've installed it on another of my repos as a test for now.

@shields-cd shields-cd temporarily deployed to shields-staging-pr-7727 March 24, 2022 22:08 Inactive
@calebcartwright calebcartwright added service-badge Accepted and actionable changes, features, and bugs needs-upstream-help Not actionable without help from a service provider labels Mar 27, 2022
@calebcartwright
Copy link
Member

@valeriangalliat - we're going to be unable to move forward unless and until we get some kind of explicit acknowledgement from the service maintainers. That doesn't seem to be happening here, so perhaps you'd be interested in opening an issue on one of their repos, trying a chat channel, etc.?

@valeriangalliat
Copy link
Author

Thanks for following up @calebcartwright! I just asked them on Twitter and got an approval:

image

Is that a good enough authorization or should I ask them to post something directly in this thread? πŸ˜„

@calebcartwright
Copy link
Member

That works for me, thanks!

@calebcartwright calebcartwright removed the needs-upstream-help Not actionable without help from a service provider label Apr 7, 2022
@shields-cd shields-cd had a problem deploying to shields-staging-pr-7727 April 7, 2022 00:51 Failure
@shields-ci
Copy link

Messages
πŸ“– ✨ Thanks for your contribution to Shields, @valeriangalliat!

Generated by 🚫 dangerJS against 63f7fee

@calebcartwright
Copy link
Member

The CI failures seem to be non-spurious. Are you able to get the tests passing locally? (npm run test:services -- --only=socket should do the trick)

@valeriangalliat
Copy link
Author

@calebcartwright tests pass on my side:

0407220918 Server is starting up: http://localhost:1111/
  SocketDevScore
    [live] should show final score
      βœ” 
	[ GET /supplyChainRisk-score/npm/vue.json ] (523ms)
    [mocked] should show color
      βœ” 
	[ GET /supplyChainRisk-score/npm/mock-for-package-score.json ]
    [live] should show final score with scope
      βœ” 
	[ GET /supplyChainRisk-score/npm/@vue/cli.json ] (280ms)
    [live] should show quality
      βœ” 
	[ GET /quality-score/npm/vue.json ] (311ms)
    [live] should show maintenance
      βœ” 
	[ GET /maintenance-score/npm/vue.json ] (208ms)
    [live] should show vulnerability
      βœ” 
	[ GET /vulnerability-score/npm/vue.json ] (252ms)
    [live] should show license
      βœ” 
	[ GET /license-score/npm/vue.json ] (218ms)
    [live] unknown package
      βœ” 
	[ GET /supplyChainRisk-score/npm/npm-api-does-not-have-this-package.json ] (268ms)


  8 passing (3s)

But it looks like the CI and CD both worked after you merged master into this branch yesterday?

@calebcartwright
Copy link
Member

Hmm so they are, perhaps it was transient after all!

@calebcartwright
Copy link
Member

Apologies for the delay in review follow up. I've been poking around and one concerning item I've seen is that the upstream API is quite slow to respond. Do you know if it's attempting to perform a real-time analysis when it receives a score request?

It does seem to have some kind of caching mechanism, but the first score request for a badge often takes more than 5 seconds.

Shields has a rather tight time window in which to complete the entire request/response flow of the badge back to our users, otherwise the common places users put our badges, like GitHub, will refuse to render the badge.

@chris48s
Copy link
Member

I think one aspect of this is that they have not analysed the entire NPM registry, so if you request a package that has already been analysed before then it will be quicker, but if you request a package they have never analysed then it will be slow because it has to go off and do the analysis on-the-fly. I'd guess that they must also sometimes invalidate the results (can't present the same stale data forever). I'd guess they probably do that each time a new version of the upstream package is published, but it could be on a schedule or there could also be some secondary cache at play too..

I think we do have some other badges that work like this e.g: Mozilla Observatory. The first request in a 24 hour period will never load, but once it has cached the result subsequent requests will be fast. Its not ideal, but its not unprecedented.

It might be useful to understand it a bit more. IMO we could certainly live with one slow request per package version

@calebcartwright
Copy link
Member

I've been seeing the same thing all day with the packages I've been testing with, including many of the ones included in the proposed changes as examples and test targets.

I could live with something like the Mozilla Observatory experience wherein there's maybe once per day for a single person when your badge would fail to load. However, what I'm seeing so far seems to be quite different in terms of frequency. If your experience is different and closer to M.O. that's great, but mine is quite different: markedly slower, and definitely measured in terms of low single digit hours; not days.

@chris48s
Copy link
Member

Yeah I think we do need more info ideally. That's what I'm getting at when I say

but it could be on a schedule or there could also be some secondary cache at play too..

Interestingly, I've just tried a bunch of packages today. Some that I tried yesterday, some that I didn't. All of them returned a result quickly .e.g:

Then I tried https://socket.dev/api/npm/package-info/score?name=@vue/cli and it took aaaaaaaages (and the latest release for that package was ~a month ago). I wonder if there is something like the size of the codebase or size of dependency tree that makes the result for some packages take a super long time to calculate.

@valeriangalliat
Copy link
Author

Great observations, thanks for the thorough review, I didn't consider this aspect.

My understanding (from this podcast) is that they analyze packages on demand and cache the results, e.g. if we request scores for express they lookup what's the latest express version, and if they already analyzed that version, return the cached results, otherwise analyze then return results (which is quite slower).

They can also change the rules for determining scores in which case, parts or all of the analysis might need to be recomputed with the new rules to come up with the score, which is also going to be slower.

What about putting a 4 seconds timeout on the request to this API and return status: pending otherwise? I think GitHub doesn't cache images long enough for this to be a big issue. Unless it happens very often that it responds in over 4 seconds...

Then, would an endpoint that:

  • always returns the latest cached response even if stale,
  • returns status: pending right away the very first time it encounters a package,

be better to back a badge?

(I'm not aware of such an endpoint nor am I in a position to create one on Socket, just asking out of curiosity)


Either way I think having Socket badges is kind of a niche use case at the moment so I wouldn't put too much effort into this until there's more demand for it. Feel free to close this PR for now, and we can always reopen it in the future if more people want this and if the Socket team is interested in implementing a compatible endpoint.

Cheers!

@calebcartwright
Copy link
Member

Either way I think having Socket badges is kind of a niche use case at the moment so I wouldn't put too much effort into this until there's more demand for it. Feel free to close this PR for now, and we can always reopen it in the future if more people want this and if the Socket team is interested in implementing a compatible endpoint.

I won't speak for Chris nor any of the other maintainers, but I'm rather intrigued by the Socket service itself and quite bullish on being able to provide badges for it as I think it's a great fit for the types of badges we like to be able to provide our community.

I think there's two related but arguably distinct tracks/considerations here.

  1. Given that the Socket service and API contract appears to be in beta/active development, is there an opportunity for collaboration or at least for us to share some feedback that the Socket.dev team can consider when designing and building out their API surface
  2. Could/should we try to proceed with our badge implementation based on the current state of the API

I personally would prefer a "yes" to the first, provided we've got a good way of connecting with the team (my apologies @valeriangalliat as I thought you were associated with/a member of the Socket.dev team, but realized I'm confusing a couple PRs/new badges we have open).

For the second, I'm still on the fence, but would want to dig into more than some of the anecdotal experiences we've had so far

What about putting a 4 seconds timeout on the request to this API and return status: pending otherwise? I think GitHub doesn't cache images long enough for this to be a big issue. Unless it happens very often that it responds in over 4 seconds...

Two separate things here, the first being a Shields side break-glass like mechanism with the service class to account for behavior, the second being the specifics and overall nature. Remember that there's a rather sequential flow to the entire badge request/response loop; the API call our badge servers make to an upstream provider is just one piece of that overall flow, and it's the entire flow that needs to complete in ~3-3.5 seconds. I also don't want to minimize the impact of GitHub and camo's caching as it's a very significant factor in how users experience badges.

Then, would an endpoint that .... always returns the latest cached response even if stale,

I think this would be the best, at least for our purposes. The second approach strikes me as more of a pivot to an asynchronous flow which isn't something we'd be able to utilize directly, and doesn't give us anything we couldn't already handle ourselves by limiting the amount of time we'd wait for the synchronous response and just returning pending from our end.

@feross
Copy link

feross commented May 6, 2022

I personally would prefer a "yes" to the first, provided we've got a good way of connecting with the team

I'm on the Socket team and I'm happy to share this feedback with the team and help make our API work for the shields use case. Let me know how I can help. If you ever need to reach me directly, my email is @socket.dev.

always returns the latest cached response even if stale

All data in our analysis pipeline has a TTL. Some work can be cached forever. Other work (like checking the number of npm downloads a package has) cannot be cached and has a TTL set to ~1 week. If you request a package score and data is available, but stale, we'll return it for up to 1 week. This is similar to the HTTP Header "stale while revalidate". We'll return stale data that is up to 1 week old whileΒ simultaneously kicking off a new analysis in the background to update the score.

Let me just double-check with the team that the public API is using the same stale-while-revalidate logic as the https://socket.dev website itself. Will get back to you!

Let me know if I can be helpful in any other way :)

@jhiesey
Copy link

jhiesey commented May 9, 2022

I wrote the stale while revalidate logic for socket.dev and can confirm it works like @feross said above.

To clarify, we always return data on that endpoint even if it's up to a week stale. In the the Cache-Control HTTP header, we set max-age=<time until stale>,stale-while-revalidate=<time until stale + 1 week>. So if max-age=0, that means you're getting stale data. Whenever we return stale data, we will kick off a recalculation on our backend, so hopefully next time you'll get more recent data.

One other reason that endpoint will be slow to respond is if we change the logic to compute the scores (which we do somewhat frequently as we fix bugs or enhance the analysis). We don't return stale data if the formula changed, so we can't guarantee the endpoint will be fast in general.

@calebcartwright
Copy link
Member

Thank you both very much, and apologies for the delayed response! The behavior as described certainly seems reasonable and feasible for our needs. The concern was that our experiences, albeit limited and anecdotal, didn't seem to be lining up with that description.

I'm a bit stretched on bandwidth at the moment and believe most of the other maintainers are as well, but one or more of this should get back around to testing this more thoroughly in the not too distant future.

@chris48s
Copy link
Member

chris48s commented May 17, 2022

Something for us to think about: If we're going to take this on, I think my biggest concern is that we have this badge that intermittently doesn't display a value for reasons that aren't obvious to an end user and becomes a source of questions or support requests we can't really do anything about.

So one question is: Will this API respond quickly 100% of the time, and it seems like we can deal with the answer to that being 'no'.

Then another question is: Can we present or explain that behaviour (in badge form) to end users in a way that doesn't generate burden?

@calebcartwright
Copy link
Member

As an update, so far the response times seem much snappier, even for the packages we were seeing issues with before ❀️

The net new scans can still take a minute (figuratively, more like ~30-40 seconds literally) but that's not unexpected nor concerning.

One thing I did notice, as a rare side effect at being prone to typos, is that an unknown/bad package name is returning a 500 response whereas I'd have expected something more like a 404. Is the 500 intentional?

e.g.

$ curl -I  https://socket.dev/api/npm/package-info/score?name=gatbsy
HTTP/2 500 
date: Mon, 30 May 2022 02:00:20 GMT
content-type: application/json; charset=utf-8
content-security-policy: default-src 'self' ; connect-src 'self' https://socketusercontent.com *.hubspot.com ; frame-src 'self' *.hubspot.com ; img-src * data: ; object-src 'none' ; script-src 'self' *.hs-scripts.com *.hscollectedforms.net *.hs-banner.com *.hs-analytics.net *.usemessages.com ; style-src 'self' 'unsafe-inline' ; base-uri 'none' ; frame-ancestors 'self' ; form-action 'self' https://github.com ;
cross-origin-opener-policy: same-origin
document-policy: document-write=?0
etag: W/"2b-bmZMytH5Joz1u1QlKYUq9dYqiwE"
origin-agent-cluster: ?1
permissions-policy: geolocation=(), camera=(), microphone=(), sync-xhr=()
referrer-policy: strict-origin-when-cross-origin
strict-transport-security: max-age=31536000; includeSubDomains; preload
vary: Accept-Encoding
x-content-type-options: nosniff
cf-cache-status: DYNAMIC
expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
report-to: {"endpoints":[{"url":"https:\/\/a.nel.cloudflare.com\/report\/v3?s=DqUfXhS5Oza4qRYp70dpmn2ziexSSHLgDZOe%2FNgw7zAqs1Z1L8Z%2BOyhhwx%2BbKpceca6CzF1BCAN6EyiQOxraqHxXam1v%2FZ3eBsO8MmtU3lZ3G6aQY9PH6YA%2Feu3ZP97avMvJakx%2BQpw%3D"}],"group":"cf-nel","max_age":604800}
nel: {"success_fraction":0,"report_to":"cf-nel","max_age":604800}
server: cloudflare
cf-ray: 7133e023aaecb09f-ATL
alt-svc: h3=":443"; ma=86400, h3-29=":443"; ma=86400

@feross
Copy link

feross commented Jun 17, 2022

One thing I did notice, as a rare side effect at being prone to typos, is that an unknown/bad package name is returning a 500 response whereas I'd have expected something more like a 404. Is the 500 intentional?

Not intentional. We're working on fixing this.

@calebcartwright
Copy link
Member

I'm going to close given the age, but certainly feel free to reopen or follow up with us when this is ready to be picked back up!

@chris48s
Copy link
Member

Hello. After some discussion in #9174 we've settled on a pattern for handling services that work in this way (based on some of the conversation in this PR) and implemented it in #9233

I appreciate this has take a while, but would you be up for re-visiting this PR using the approach shown in code example in #9233 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants