chore: make dev container setup work without npm registry access - #97
Open
ayeshurun wants to merge 5 commits into
Open
chore: make dev container setup work without npm registry access#97ayeshurun wants to merge 5 commits into
ayeshurun wants to merge 5 commits into
Conversation
The Node.js dev container feature failed to install on networks that block registry.npmjs.org, breaking container creation before setup could start. Node was only present to run `npm install -g changie`. changie ships as a standalone Go binary, so install it directly from its upstream GitHub release with checksum verification and drop the Node feature entirely. This removes a full JavaScript toolchain from an otherwise Python-only container. Also fix two latent issues in the setup script: - apt-get and the binary install now use sudo when not running as root. postCreateCommand runs as the remote user, where the previous bare apt-get exited 100 and aborted the script under `set -e`. - Support an optional git-ignored .devcontainer/local.env for environments that need an internal package mirror. pypi.org can be reachable while files.pythonhosted.org is blocked, which makes pip resolve and then fail on download. Verified end to end in the base image on amd64 and arm64, as both root and the non-root remote user. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c37de44-e0a0-4090-8f7e-299889108578
- Split apt-get update/install into separate statements. As the left operand of &&, a failing update did not trip set -e, so package installation was skipped silently and cmake was never installed. - Remove the base image's stale Yarn apt source. Its bundled RSA keyring predates Yarn's switch to an EdDSA signing key, so apt-get update fails verification and exits 100 on any network. - Parse .devcontainer/local.env as KEY=value data against an allowlist instead of sourcing it, so the file cannot run commands or clobber script variables. - Forward proxy variables explicitly through sudo, which resets the environment by default and previously discarded them. - Require exactly one matching checksum entry before verification. - Harden curl: https-only redirects, bounded retries and timeouts. - Resolve requirements files from the repo root rather than the caller's cwd. - Preflight sudo availability and non-interactive use. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c37de44-e0a0-4090-8f7e-299889108578
ayeshurun
force-pushed
the
dev/alonyeshurun/remove-devcontainer-node-feature
branch
from
August 26, 2026 08:25
6cfe12b to
c345bbd
Compare
Silence a ShellCheck SC2054 false positive by quoting the sudo --preserve-env argument, whose commas belong to sudo's option list rather than to the bash array. Use `env` as a no-op prefix on the root path instead of an empty array. Expanding an empty array under `set -u` is an error on bash < 4.4. Document that apt honours only lowercase proxy variables, so setting HTTP_PROXY alone leaves apt without a proxy while pip and curl work. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c37de44-e0a0-4090-8f7e-299889108578
… entry A tar member named `changie` could be a symlink or hard link. The previous `tar -xzf` materialised that entry, and the following `install` under sudo followed it, copying an arbitrary root-readable file into world-readable /usr/local/bin. Verified in mcr.microsoft.com/devcontainers/python:1-3.12-bullseye: a `changie -> /etc/shadow` member produced a 0755 root-owned copy of /etc/shadow readable by the unprivileged remote user. Extract the member's contents with `tar -O` instead. Link entries carry no content, so they yield zero bytes and the added non-empty check rejects them; the shell redirect always creates a regular file inside the 0700 mktemp directory, removing link semantics from the privileged step entirely. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c37de44-e0a0-4090-8f7e-299889108578
The comment claimed the install path "fails closed". That is only true for link entries. Verified in the target image that a directory member named `changie` streams its child's contents through `tar -O` (21 bytes, exit 0), passing the non-empty check; duplicate members concatenate similarly. Neither is exploitable: the bytes originate inside the checksum-gated archive, never from a host path, so the guard still prevents the privileged install from following a link into the filesystem. Reword to state that bound precisely rather than overclaiming. Comment-only change; bash -n and shellcheck --severity=style pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c37de44-e0a0-4090-8f7e-299889108578
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Dev container creation fails on networks that block
registry.npmjs.org. Theghcr.io/devcontainers/features/node:2feature aborts while installingpnpm:This happens during the image build, so the container never starts.
Investigating that failure surfaced a second, independent defect: even with Node removed,
apt-get updatefails in this base image, and the setup script was swallowing that failure and exiting 0 with none of the required build packages installed.Root causes
1. npm registry blocked by network policy. The request fails in ~20ms, and
nodejs.orgdownloads fine in the same layer, so general egress is healthy. The 73s in the log is npm's retry backoff. Node.js was in the container for exactly one reason:npm install -g changie. There is nopackage.jsonanywhere in the repo.2. Stale Yarn apt source in the base image.
mcr.microsoft.com/devcontainers/python:1-3.12-bullseyeships/etc/apt/sources.list.d/yarn.listplus an RSA keyring whose signing subkeys expired 2026-01-23. Yarn now signsInReleasewith EdDSA key62D54FD4003F6525, so apt reportsNO_PUBKEYandapt-get updateexits 100. This is not network-specific and is not caused by the npm block — it fails for every user of this image. TLS inspection was ruled out: the certificate chain is genuine Google Trust Services forCN=yarnpkg.com.3. The failure was silent. The script ran
apt-get update && apt-get install .... POSIX ignoreserrexitfor any command in an AND-OR list other than the last, so the failingupdatedid not tripset -e— it simply short-circuited theinstalland the script returned 0. Result:cmake,pkg-config,libcairo2-devandpython3-devwere never installed, with no error surfaced.Note: an earlier revision of this description claimed the non-root
apt-get update"aborted the script underset -e". That was wrong for the same&&reason, and is corrected above.Changes
Remove the Node.js feature.
changieis a standalone Go binary, so it is installed directly from its upstream GitHub release and verified against the publishedchecksums.txt. The version is pinned (overridable viaCHANGIE_VERSION). This drops an entire JavaScript toolchain from a Python-only container.Fix the apt failure.
apt-get updateandapt-get installare now separate statements, and the stale Yarn source is removed beforeupdateruns. Without both changes the split alone would only make the existing failure visible, not fix it. Scope note:apt-get updateexits non-zero for signature failures like this one, so it now aborts underset -e, but it still exits 0 when a source is merely unreachable.installremains the real gate there — an unavailable package fails loudly rather than being silently skipped, which was the actual bug.APT::Update::Error-Mode=anywould makeupdatestrict, but was deliberately not adopted: it would turn a transient mirror blip into a hard container-build failure on exactly the restricted networks this PR targets.Use sudo when not running as root.
postCreateCommandruns as the remote user, where bareapt-getcannot write to/var/lib/apt. The script detects its own uid, preflights thatsudoexists and works non-interactively, and behaves identically as either user. Proxy variables are forwarded explicitly via--preserve-env, since sudo resets the environment by default.pip3is deliberately not run under sudo: Debian's pip falls back to--userand sudo would stripPIP_INDEX_URL.Install the verified binary without materialising archive entries. The
changiemember is extracted to stdout (tar -xzOf) and redirected into a fresh regular file inside a0700temp dir, which is then non-empty-checked beforeinstall. Extracting normally and installing the resulting path would follow a link entry: an archive whosechangiemember is a symlink to/etc/shadowcausedsudo install -m 0755to read it as root and write a world-readable copy to/usr/local/bin/changie.tar -Oemits the member's contents, and link entries carry none, so both symlinks and hard links yield 0 bytes and are rejected. This is defence in depth, not a fix for the underlying trust model — see below.Support an optional git-ignored
.devcontainer/local.env. For environments needing an internal package mirror. It is parsed asKEY=valuedata against a fixed allowlist, never sourced, so it cannot execute commands or clobber script variables. Worth noting:pypi.orgcan be reachable whilefiles.pythonhosted.orgis blocked, so pip resolves dependencies and only then fails on download..devcontainer/local.env.exampledocuments the pattern; the real file is git-ignored.Verification
Run in
mcr.microsoft.com/devcontainers/python:1-3.12-bullseye. Assertions check installed outcomes, not just exit codes — an exit-code-only check is what let root cause 3 hide.linux/amd64cmake/pkg-config/libcairo2-dev/python3-devpresent; deps importable;changie version v1.26.0linux/arm64/usr/local/bin/changie, mode 0755apt-get updateexits 100 and the script now aborts instead of skipping installschangiemember is a symlink to/etc/shadowor/etc/passwd/etc/shadowto a 0755 file)changiemember is a hard link, or an empty regular filechangiemember is a directory with a child file../evil,/changie, or../changietarexits 2 (Not found in archive), script aborts beforeinstall; nothing written anywhere on diskCHANGIE_VERSIONcurl: (22) 404, exit 22local.envcontaining shell metacharacters, unlisted keys, CRLFsudo_cmdandPATHunclobbered; unlisted keys ignored--preserve-envpreserves 3/3 proxy vars and does not leakPIP_INDEX_URLNotes and trade-offs
checksums.txtis fetched from the same release as the archive, so an actor who can replace the asset can replace its checksum entry. It detects corruption, truncation and CDN inconsistency; it is not source authentication and must not be read as an end-to-end supply-chain boundary. After the link-entry fix, a substituted release yields user-level executable compromise — the same exposure already inherent in this container's unpinnedrequirements-dev.txtand in CI'snpm i -g changie. Pinning per-architecture digests in this repo would narrow that, but only for this one path, and it conflicts with the documentedCHANGIE_VERSIONoverride unless a version→digest map is introduced. Deliberately deferred to a repo-wide supply-chain decision, not an oversight. "Pinned version" here should not be read as "reproducible build": the base image tag, feature major tag and most ofrequirements-dev.txtremain mutable..github/workflows/changelog-existence.ymlstill usesnpm i -g changie. GitHub-hosted runners reach npmjs fine, so it is left alone. The dev container pins 1.26.0 while CI floats to latest; latest is currently 1.26.0, so this is future drift risk rather than a present mismatch.fab-build.ymlpath-filters tosrc/**,tests/**,pyproject.toml,tox.tomlandrequirements*.txt, so.devcontainer/**,scripts/**and.gitignorereceive zero jobs. Confirmed on this PR:gh pr checks 97returns only the changelog and title jobs;fab-builddoes not run at all. That is plausibly why root cause 3 went unnoticed, and two independent reviewers flagged it. Tracked separately so this network bugfix does not grow a CI surface.tar -Omay emit several payloads joined together; this does not reliably yield a broken binary, since ELF tolerates trailing bytes (verified:/bin/trueplus a trailing member produced a 39,698-byte file that executed successfully). A directory member namedchangielikewise streams its children's contents (verified: achangie/directory holding a 21-byte child emitted exactly those 21 bytes, exit 0). Neither is a new exposure — both require the same substituted-release trust failure that could simply ship a malicious single member — and neither can cause host-file disclosure or a privileged link-follow, because the bytes always originate inside the checksum-gated archive rather than from a host path. No additional guard added.CONTRIBUTING.mdsays changie is "pre-installed in the development container", which remains accurate.changiepackage and the GitHub release are byte-identical builds.blackandmypyare referenced in contributor docs but absent fromrequirements-dev.txt, and pip console scripts land in~/.local/bin, which is not onPATHfor the non-root user.