Comment by dboreham | Hacker News Reader

Comment by dboreham | Hacker News Reader

One thing I wonder about when I see this is:

Back in the day the source code control systems I used (primarily CVS and Subversion but also others) would send a commit/checkin email that contained the _entire_ set of diffs.

As a development lead and manager I made it part of my daily routine to _read_ the checkin emails. Of course I couldn't read every last line of code, but I could certainly spot common mistakes such as checking in extra unrelated files inadvertently; changing tabbing/formatting in code that shouldn't have changed; and often I'd spot bugs in passing. This wasn't the formal code review process, but it kept me up to date on what was going into the repo and it allowed me to catch some basic and obvious errors.

Now, back to the future and GitHub and siblings don't do this. I suspect the reason is security : the desire to not have source code floating around in potentially insecure mailboxes.

However, for an OSS project the security concern is moot.

Reading this history of this attack I feel that had someone been reading commit emails that contained the actual code changed/added, it would have been easy to detect. As it is, the typical GitHub commit notification email tells you almost nothing. You have to click through a link to see the changes (which I'm guessing nobody did in this case).

The issue is that the dependency change will show up in the repo as:

- "event-stream": "1.8.4"

+ "event-stream": "1.8.5"

Looks completely innocuous, a patch version bump, but it can technically be a completely different package, it may not even have any relation to any open sourced code, people can push anything to npm.

People don't check in their dependencies. event-stream is hosted on npm and downloaded by people who download packages depending on it automatically. Given this situation there is no diff of it to view.

yeah - thinking it'd be helpful for npm to have a diff command to check source changes when doing upgrades.

Unfortunately, this is made difficult to "bootstrap" due to npm not supporting/promoting repeatable builds - so you'd just see a diff of minified messes with no ability to track back to source code changes. You'd want to do this alongside a rollout of a larger ecosystem around cryptographically secure repeatable build tracking, which would take time and require "drag-along" of the entire ecosystem of abandoned packages.

Very few packages actually contain minified sources - it's not really needed with modern build tools. I think it was an exception in this case, and that by itself could be suspicious. Current version of npm now contains a package-lock.json file which is made to make builds pretty close to deterministic. It contains the entire dependency tree of dependency names, versions, and hashes. I say pretty close because it's still possible to break "npm install" by unpublishing a package.

If something looks innocuous a diligent engineer will review it twice as hard. I used to fail reviews on one-liners during my professional years with people around me asking me what's taking so long—it's just one line of code.

Well there's definitely a conflict between trying to get things done quickly and reviewing things thoroughly in most development jobs.

You have to weigh the risk of going out of business because you didn't deliver value fast enough vs. the risk of having a security incident.

In the Node ecosystem there are SAAS products (https://greenkeeper.io/) that will automatically update your dependencies, run your tests, and merge in the updates (that line change I showed is an example of what it would look like) if the tests pass. That shows you how much thought Node/JS developers put into upgrading their dependencies.

The event-stream update would be done automatically in this instance because the code still worked, although it was compromised.

These kind of attacks can easily just not publish commits to GitHub though. The attack vector is npm, not GH.

You can still get patch emails with Git repositories. In fact, you can easily create them from your own copy of the repo.

This hack has nothing to do with the scm but everything with the carelessness of npm.

I must be misunderstanding the attack: didn't it involve checking in malicious code to a repo used to build a package published in npm?

Or are you suggesting that all code in github should be considered untrusted, but somehow able to be verified by npm?

Reply on news.ycombinator.com