[danpalmer]

Having had a quick look through this workflow it seems to miss most opportunities to ensure a safe build.

- Downloads binaries for use in build with no hash/signing verification.

- Doesn't pin shared actions.

- Uses Yarn to install dependencies (which can involve downloading/executing arbitrary code from anywhere)

- Doesn't sign the final binary.

None of this is necessarily wrong, all would make maintenance harder in the long run, but it means this project is really about removing MS branding and some telemetry, and that there is a security trade-off to get those benefits.

[benatkin]

None of these are a big deal.

> - Downloads binaries for use in build with no hash/signing verification.

It downloads them using TLS.

> - Doesn't pin shared actions.

The shared actions are just @actions/checkout and @actions/setup-node. They're official. I wouldn't pin them - YAGNI.

> - Uses Yarn to install dependencies (which can involve downloading/executing arbitrary code from anywhere)

It downloads/executes code based on the carefully chosen dependencies

> - Doesn't sign the final binary.

That's platform dependent I think. For Mac OS X it does.

Seems like FUD, which you might be able to recognize because you say "None of this is necessarily wrong". Especially the part about pinning first party GitHub Actions. There would be nothing wrong with that but it is much more useful to pin third party GitHub Actions, and IMHO suboptimal to pin first party actions.

[vladvasiliu]

Isn't the whole point of this comment thread "vulnerabilities to supply chain attacks"?

>> - Downloads binaries for use in build with no hash/signing verification.

> It downloads them using TLS.

If the binary is updated to a shady version, sure, no one will be able to tamper with the download, they're certain to have received the correct shady stuff.

[danpalmer]

Ah yes it does sign on macOS.

I don't think it's quite FUD, but I do agree none of these are strictly necessary, all can be rationalised as unnecessary and for many users this project probably provides a perfectly reasonable security posture. However the fact that there's so little explicit acknowledgement of the security concerns, and that 2 minutes looking at the repo turned these things up, suggests that security is not a priority of the project. Again, not the wrong thing to do, but maybe not the trade-offs all users will want.

Pinning actions is so low effort/high reward that even the low risk makes it worth it for a project like this in my opinion. Official actions are certainly much safer, but ultimately it's still just human review and PRs being merged.

Downloading over TLS negates some impact of hash/signing verification, but it would be a nice extra layer. You're otherwise putting a lot of trust in the combination of DNS+CDN+Hosting. I've seen hijacked sites due to IPs being re-used on cloud providers for example. Unlikely, but again easy to do and high impact in the rare situation that is is taken advantage of.

Yarn dependencies may be carefully chosen, I'm not familiar with the VSCode practices. I bet that official binaries however are not built like this – I'd bet that there are allowances for specific network connectivity and binary execution, and that everything else is locked down. To my knowledge GitHub Actions have open internet access. I wouldn't even say this is low risk either, the NPM ecosystem is so deeply nested that I'm sure malicious code could be snuck in somewhere. This is a lot harder to solve for this project, and certainly the most debatable aspect as to whether it's worth it or not.

[lozenge]

The yarn.lock includes checksums, if yarn is not checking checksums properly then that affects every project in Node.js, not just this one.

Malicious code with the correct checksum? VSCode team is not auto updating dependencies but I also doubt they are reviewing the source code of every package they update. I've never worked anywhere that does. So yeah, "gulp-vinyl-zip" (or any other package used at build time) could add some code that secretly triggers when run in the VSCode repository and makes some malicious source code changes. But, it's still going to be the same code in VSCode and VSCodium. Unless the attacker decides to use specific logic to target one or the other.

[danpalmer]

It's possible, and common, for NPM packages to download additional binaries at install and/or run time. These would not be included in the hashes, and are essentially untrusted, often hosted on random S3 buckets. I'd hope there are checks in the official project to ensure these aren't being used, but there aren't any checks in this project.

[patrakov]

How would you write such a check?

[lozenge]

My employer uses artifactory as a mirror of npm and one step of adding a package to the mirror is installing it on a VM/container with no network access. If the package has a "postinstall" script requiring internet access then it will fail. There's also code scanning that will find at least casual attacks like "download and run this crypto ransomware".

I think the idea MS has deployed this is over optimistic. Look at this with Google. https://giraffesecurity.dev/posts/google-remote-code-executi...

Also, it depends how determined your attacker is. If they write code to detect whether they're being installed in the vscode project, have access to commercially available security scanning tools to ensure they evade detection, etc...

[benatkin]

Using socket.dev is one way!

As for writing such a check manually, you would just need to check "bin" in */package.json after installing everything, and verify each script.

Trusting a big company seems to be another suggestion I see in this thread too. I don't agree with that one.

[benatkin]

> low effort/high reward

Are there any shared actions that aren't actions/name-of-project? If not, that's zero reward.

[danpalmer]

I'm not sure I follow why the "official" actions under the actions org are materially different to others? I agree they're likely to have better processes around contributions, but they are fundamentally an open contribution model with human review, and human review is fallible. Unless they had extra sandboxing, pinning requirements, materially better testing, etc, I'm not sure there's a huge difference.

[benatkin]

No, PR review isn't the only thing that prevents these from being updated. In the yml it's set to a release branch. So it isn't especially fallible.

https://github.com/actions/setup-node/tree/releases/v2

[benatkin]

However, the Linux one grabs two from third party, to GitHub, repos. These seem relatively safe, from official systems level projects — snapcraft and docker. I'm going to suggest to Codium they add a SHA to pin those, just like I would suggest they unpin GitHub Actions if they'd had them pinned. Sorry I was looking at the Mac one because I have a Mac even though the link up the thread was for Linux.