fix(devcontainer): preserve remote user ownership - #1135
Conversation
✅ Deploy Preview for images-devsy-sh canceled.
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for devsydev canceled.
|
|
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
f983f7f to
a806871
Compare
Restored and recreated workspaces could leave the workspace folder root-owned, so git inside the devcontainer failed with "detected dubious ownership in repository". Root causes: - chownWorkspace wrote its marker before attempting the chown and swallowed chown errors, so a failed or interrupted first run was permanently skipped on every later run. - Snapshot volumes lost ownership: the archiver hardcoded uid/gid 0 and extraction never applied header ownership, so restored volumes were entirely root-owned. - Snapshot restore replayed only runArgs/containerEnv, silently dropping an explicitly declared remoteUser. - GetRemoteUser never implemented its documented Docker-inspect User fallback, degrading straight to root. Fixes: - Marker is written only after the chown completes; workspace-root chown failures now fail setup, recursive per-entry failures are surfaced at warn level (still best-effort for read-only virtiofs entries). chownWorkspace also skips (without erroring or latching the marker) when the workspace folder does not exist, since compose services are never guaranteed a devsy-synthesized workspaceMount. - Archiver records real uid/gid; extraction gains PreserveHeaderOwnership() with graceful degradation when unprivileged; volume restore opts in. - New sh.devsy.snapshot.remote-user manifest annotation, recorded at create time and replayed for both snapshot restore and up --from-snapshot. - GetRemoteUser now resolves per spec priority: remoteUser -> containerUser -> devsy.user label -> Docker-inspect User -> root.
a806871 to
eafb8a4
Compare
Problem
Restored and recreated workspaces could leave the workspace folder root-owned, so git inside the devcontainer failed with
detected dubious ownership in repository— the devcontainer user should own the workspace.Root causes (four, same family)
chownWorkspacewrote its/var/devsy/chownWorkspace.markerbefore attempting the chown and swallowed all chown errors (log.Warn/log.Debug, returning nil). Any failed or interrupted first run was permanently skipped on every later run.hdr.Uid = 0; hdr.Gid = 0and extraction never applied header ownership, so restored volumes were entirely root-owned.runArgs/containerEnv; an explicitly declaredremoteUserwas silently dropped, so restored image-sourced containers ran tools/IDE as root.GetRemoteUser's docstring promised a Docker-inspectUserfallback that was never implemented, degrading straight toroot.Fixes
PreserveHeaderOwnership()(graceful degradation when unprivileged); volume restore opts in.sh.devsy.snapshot.remote-usermanifest annotation, recorded at create time and replayed onto the synthesized config for bothsnapshot restoreandup --from-snapshot.GetRemoteUsernow resolves per spec priority:remoteUser→containerUser→devsy.userlabel → Docker-inspectUser→ root.Validation
pkg/gitfailures (fail on clean tree).vscode, no dubious ownership.restores files owned by the remote user when reusing the original id+ non-root testdata fixture (requires the suite's usual registry prerequisites to run in CI).This PR was authored with GPG-signed commits.