feat: container registry

[kanadgupta]

🚥 Fixes RM-6538

🧰 Changes

The moment we've been waiting for y'all: enhancing our semantic-release workflow to automatically publish docker images to the GitHub Container Registry!

There are a bunch of moving parts, but if you look at the Linear ticket, I've added comments with links to a couple internal videos that I created

🧬 QA & Testing

I did a couple of test pushes to the Container Registry and confirmed that a working, containerized version of rdme is pushed to the registry!

• You can see the release workflow here: https://github.com/readmeio/rdme/actions/runs/4350052453/jobs/7600379201
• And the corresponding image in the container registry here: https://github.com/readmeio/rdme/pkgs/container/rdme/75372142

And the performance gains are pretty incredible. Here's the before:

And here's the after:

There are a few aspects of this process that we can only test once we merge into next, but that's why we're doing pre-releases now amirite?

[RyanGWU82]

I don't feel like I have a good grasp of the big picture of everything that happens during a release here, but if you've tested it and you're comfortable with it, then that's a good sign. Is there documentation for the build/release process and what's happening under the hood?

Can't approve it because of the unsanitized input to shell commands, but once that's fixed I'm 👍🏼.

[RyanGWU82]

child_process.exec() takes a string that it passes to the shell. However that's not particularly safe, and we're not doing anything to sanitize the input when building the shell commands in run().

Never pass unsanitized user input to this function. Any input containing shell metacharacters may be used to trigger arbitrary command execution.

Source: https://nodejs.org/api/child_process.html#child_processexeccommand-options-callback

Can we switch these commands to child_process.execFile and pass an array of arguments? https://nodejs.org/api/child_process.html#child_processexecfilefile-args-options-callback

[kanadgupta]

So I might be misunderstanding but I'm not sure arbitrary command execution is much of an issue here — I already do some sanitization checks here to ensure that the env variable input is valid (and this is really the only "user" input):

rdme/bin/docker.js

Lines 16 to 21 in a9f14e4

	if (!Object.keys(metadata.labels || {})?.length) {
	throw new Error('Invalid shape (missing labels data)');
	}
	if (!metadata?.tags?.length) {
	throw new Error('Invalid shape (missing tags data)');
	}

And beyond that, everything is mostly hardcoded strings to construct the docker CLI commands and call the runDockerCmd (f.k.a. runCmd) function (and I've ran through the commands locally a bunch to confirm that they're valid).

In any case, I refactored this in 1b07bb7 to use execFile, TIL! It's much more readable now too so thanks for the suggestion 🎊

[RyanGWU82]

Looks good! My thinking was the tags, which come from git branch names and those could be anything. It looks like git does have some sanitization (I tried a semicolon and got fatal: 'foo; reboot' is not a valid branch name) but I could imagine there are ways around this if someone was motivated to try hard enough.

[erunion]

:nice-rgb: