Skip to content

fix(devcontainer): preserve remote user ownership - #1135

Draft
skevetter wants to merge 1 commit into
mainfrom
fix/devcontainer-ownership
Draft

fix(devcontainer): preserve remote user ownership#1135
skevetter wants to merge 1 commit into
mainfrom
fix/devcontainer-ownership

Conversation

@skevetter

Copy link
Copy Markdown
Contributor

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)

  1. Chown marker latch: chownWorkspace wrote its /var/devsy/chownWorkspace.marker before 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.
  2. Snapshot volumes lost ownership: the archiver hardcoded hdr.Uid = 0; hdr.Gid = 0 and extraction never applied header ownership, so restored volumes were entirely root-owned.
  3. Restore dropped remoteUser: snapshot manifests replayed only runArgs/containerEnv; an explicitly declared remoteUser was silently dropped, so restored image-sourced containers ran tools/IDE as root.
  4. Remote-user fallback gap: GetRemoteUser's docstring promised a Docker-inspect User fallback that was never implemented, degrading straight to root.

Fixes

  • Marker is written only after the chowns complete; workspace-root chown failures now fail setup, recursive per-entry failures are surfaced at warn level (still best-effort for read-only virtiofs entries).
  • Archiver records real uid/gid; extraction gains PreserveHeaderOwnership() (graceful degradation when unprivileged); volume restore opts in.
  • New sh.devsy.snapshot.remote-user manifest annotation, recorded at create time and replayed onto the synthesized config for both snapshot restore and up --from-snapshot.
  • GetRemoteUser now resolves per spec priority: remoteUsercontainerUserdevsy.user label → Docker-inspect User → root.

Validation

  • Unit tests: marker semantics (root-gated, run in-container), ownership round-trip incl. unprivileged degradation, remote-user resolution table.
  • Full unit suite green except 3 pre-existing pkg/git failures (fail on clean tree).
  • Live e2e against a local registry: up → mutate → snapshot create → delete → restore under the original id → files owned by vscode, no dubious ownership.
  • New regression spec 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.

@netlify

netlify Bot commented Aug 26, 2026

Copy link
Copy Markdown

Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit eafb8a4
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6a90f2b2376287000849566e

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Aug 26, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit eafb8a4
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a90f2b207447f0008d305e9

@github-actions

Copy link
Copy Markdown

⚠️ This PR contains unsigned commits. To get your PR merged, please sign those commits (git rebase --exec 'git commit -S --amend --no-edit -n' @{upstream}) and force push them to this branch (git push --force-with-lease).

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

  1. Generate a GPG key
  2. Add the GPG key to your GitHub account
  3. Configure git to use your GPG key for commit signing
Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

  1. Generate an SSH key and add it to ssh-agent
  2. Add the SSH key to your GitHub account
  3. Configure git to use your SSH key for commit signing
Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.

Learn how to use 1Password to sign your commits.

Watch the demo

@skevetter
skevetter force-pushed the fix/devcontainer-ownership branch 4 times, most recently from f983f7f to a806871 Compare August 28, 2026 02:13
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.
@skevetter
skevetter force-pushed the fix/devcontainer-ownership branch from a806871 to eafb8a4 Compare August 28, 2026 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant