fix(devbase): accept a single-file history bind, and stop mounting the host home - #12
Conversation
…inking it
Frames are moving from binding the whole host home at /WSL_USER to binding the
host's ~/.zsh_history as a single file at /WSL_USER/.zsh_history, because
mounting a home directory to reach one file in it put every container within
read-write reach of the host's SSH keys and credentials — and, in the workspace
repository, made the readonly flag on its GitHub token directory decorative
(RSRMID-3052).
The Feature needs no change to *accept* that: the history step only ever reads
/WSL_USER/.zsh_history, so a single-file bind satisfies every branch, and the
"host mount exists" test has always built exactly that shape. What it does need
is a branch for the narrow bind's own way of going wrong.
A file bind whose source does not exist on the host is created by Docker as a
*directory*, on the host, where it outlives the container. /WSL_USER is then
neither absent nor empty, so nothing upstream catches it, and the old code
reported SUCCESS:
- `touch` on an existing directory succeeds — it sets the mtime — so the
bootstrap check short-circuited instead of failing,
- `ln -sf` linked ~/.zsh_history to that directory,
- and the verification used -e, which a symlink to a directory satisfies.
So the container logged "History linked" while zsh could not write a single
line of history to it. Exactly the defect class this suite exists to prevent,
and it would have shipped fleet-wide with the mount change.
Two fixes, deliberately at different widths. The directory check names the
cause the reader can act on: the frame is missing the initializeCommand touch
that guarantees the host file exists before the bind. And the verification moves
from -e to -f, which closes the class rather than the instance — the only thing
worth logging here is a link to a file that can be written.
Verified by exercising the function against a relocated mount path for all five
shapes (narrow bind, source-is-a-directory, mount absent, mount empty, and the
legacy whole-home mount, which still bootstraps and links so unmigrated
repositories keep working), and by re-running the source-is-a-directory case
with the guard stripped out to confirm the new assertion goes red.
Both frames bound ${localEnv:HOME} at /WSL_USER to reach one file in it,
~/.zsh_history, which is all devbase reads there. That handed the container
read-write reach over the host's SSH keys, shell profiles and every other
repository on the machine. In the workspace repository the same pattern made
the readonly flag on its GitHub token directory decorative — the same inodes
were writable by the second path — which is what RSRMID-3052 is closing across
the fleet.
Bind the single file at the exact path the Feature reads, and touch it in
initializeCommand alongside ~/.gitconfig, for the reason already documented
there: a missing bind source becomes a root-owned directory on the host, and
without the touch the preceding commit's new check is what a rebuild would hit.
~/.gitconfig also becomes readonly. devbase writes credential and signing
config with `git config --local`, never `--global`, precisely so the host's
file stays untouched, so nothing here loses an ability it was using.
~/.zsh_history and ~/.claude stay read-write because both are written.
Narrowing the mount retires the ${localEnv:HOME}${localEnv:USERPROFILE} idiom
along with the line that carried it. It had no conditional to work with and
relied on exactly one of the two being empty; a host defining both yielded a
nonexistent path, Docker created it, and history silently stopped surviving
rebuilds. The other mounts here already used bare ${localEnv:HOME}.
This does not rely on an unreleased guarantee: the published 1.7.0 both locks
pin already links a single-file bind correctly — its history step reads only
/WSL_USER/.zsh_history and that function is unchanged since 1.7.0. The new
check from the preceding commit improves the diagnostics when the touch is
missing; it is not what makes this work.
|
`node_project`'s "installs Node dependencies from a real package.json" check
has been failing on main since pnpm 12 reached CI through the node feature's
`pnpmVersion: latest`. Not caused by the mount work in this branch — verified
by dispatching this workflow on main unchanged (run 34218813164), which fails
identically.
devbase tries `pnpm install --frozen-lockfile` first, and that is a headless
install, so it needs a lockfile. The probe shipped a package.json alone and
still reached the success branch, because pnpm 11 quietly created the missing
lockfile rather than refusing. pnpm 12 made it strict:
$ # pnpm 11.25.0, no lockfile
$ pnpm install --frozen-lockfile --silent ; echo $?
0 # lockfile created for us
$ # pnpm 12.3.4, same input
$ pnpm install --frozen-lockfile --silent
Error: ERR_PNPM_NO_LOCKFILE
╰─▶ Headless installation requires a pnpm-lock.yaml file, but none was
found. Run `pnpm install` without --frozen-lockfile to create one.
So the frozen attempt now fails, the `--no-frozen-lockfile` retry takes over
and succeeds, and it reports through log_detail — "Installed without the frozen
lockfile" — rather than log_success "Node dependencies installed", which is
what this check greps for.
Worth being clear that the container behaviour was never broken. The retry did
exactly the job it was added for and node_modules is installed; what broke is
an assertion that had been passing on pnpm's leniency.
Two changes:
- The probe commits a lockfile, so it exercises the branch it names. Hand
written rather than generated: an empty importer set is all a manifest with
no dependencies needs, it reaches no network, and it satisfies
--frozen-lockfile on pnpm 11 and 12 alike — which is the point, because
`latest` floats and whichever major CI lands on next has to pass unchanged.
- A check for the no-lockfile case, which only pnpm 12 made distinct from the
above. It accepts either log line for the same version-agnostic reason, and
it guards a specific temptation: reading the frozen attempt as a hard
requirement and making a missing lockfile fatal would stop a container over a
file the retry can produce.
Both failing greps now dump the log on failure. The original reported only
"check failed", which is why the cause took a control run and two local pnpm
versions to establish rather than being readable from CI.
Changed course: the fix is now in this PRAbove I said I was deliberately keeping this out. I have folded it in as one self-contained commit,
It is one commit touching one file, so splitting it back out is trivial if you would rather it went separately. What the fix doesThe probe now commits a lockfile, so it exercises the frozen branch it names. Written by hand rather than generated — an empty importer set is all a dependency-less manifest needs, it reaches no network, and it satisfies $ printf "lockfileVersion: '9.0'\n\nimporters:\n\n .: {}\n" > pnpm-lock.yaml
$ pnpm install --frozen-lockfile --silent # 11.25.0 -> rc=0, node_modules present
$ pnpm dlx pnpm@12.3.4 install --frozen-lockfile --silent # 12.3.4 -> rc=0, node_modules presentThat cross-major property is the point rather than a detail: the node feature installs pnpm Second, a check for the no-lockfile case, which only pnpm 12 made distinct — it is refused now instead of silently created, so devbase reaches the refreshed retry and the container still comes up. It accepts either log line, for the same version-agnostic reason, and it guards a specific temptation: reading the frozen attempt as a hard requirement and making a missing lockfile fatal would stop a container over a file the retry can produce. Both failing greps now Still worth a ticketThe test was the thing that broke, but the underlying hazard is that |
|
🎉 This PR is included in version 1.7.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Part of RSRMID-3052 — every devcontainer in the fleet binds the whole host home read-write at
/WSL_USERto reach one file in it,~/.zsh_history, which is alldevbasereads there.Two commits, deliberately separate
fix(devbase)— the Feature needs no change to accept the narrow single-file bind: its history step only ever reads/WSL_USER/.zsh_history, and that function is unchanged since 1.7.0, which is the version every consumer's lock pins. Theshell history is linked when the host mount existstest has always built exactly that shape.What it did need is a branch for the narrow bind's own way of going wrong, and this turned out to be worse than expected. A file bind whose source does not exist on the host is created by Docker as a directory, on the host.
/WSL_USERis then neither absent nor empty, so nothing upstream catches it, and the old code reported SUCCESS:touchon an existing directory succeeds (it sets the mtime), so the bootstrap check short-circuited instead of failingln -sflinked~/.zsh_historyto that directory-e, which a symlink to a directory satisfiesSo the container logged
History linkedwhile zsh could not write a single line of history. That is the defect class this suite exists to prevent, and it would have shipped fleet-wide alongside the mount change.Fixed at two widths: the directory check names the cause a reader can act on (the frame is missing its
initializeCommandtouch), and the success verification moves from-eto-f, which closes the class rather than the instance.ci(devcontainer)— this repository's own two frames stop mounting the host home.~/.gitconfigalso goes readonly: devbase writes credential and signing config withgit config --local, never--global, precisely so the host's file stays untouched, so nothing loses an ability it was using. Narrowing the mount retires the${localEnv:HOME}${localEnv:USERPROFILE}idiom along with the line that carried it.Per the dogfooding rule, this does not rely on an unreleased guarantee — the published 1.7.0 both locks pin already links a single-file bind correctly. The new check improves diagnostics when the touch is missing; it is not what makes the frames work.
Verification
pnpm features:lintandpnpm prettierare clean. The container suite runs here in CI (features/**is touched).The function was also exercised directly against a relocated mount path for all five shapes:
is a directory, namesinitializeCommand, no symlinkis empty, unchangedAnd per the "check a new assertion against a broken implementation" rule, the source-is-a-directory case was re-run with the guard stripped out: it reports SUCCESS and the new assertion goes red, so the check cannot silently pass.
Release
fix(devbase)— a patch. Consumers on:1with no lock pick it up on their next rebuild; the eight repositories that commit a lock keep 1.7.0 until someone re-resolves, which is fine because 1.7.0 already accepts the narrow bind.