fix: the placeholder agent socket gets a directory of its own

[JSmithRobotics]

The bug

When dl launches a workspace on a host with no ssh agent, it synthesises a placeholder unix socket and hands its path to devpod as SSH_AUTH_SOCK, so the devcontainer's bind mount has something real to point at instead of an empty string.

devpod's ChownAgentSock (pkg/devcontainer/setup/setup.go) runs unconditionally after every devpod up, whether or not agent forwarding actually succeeded:

agentSockFile := os.Getenv("SSH_AUTH_SOCK")
copy2.ChownR(filepath.Dir(agentSockFile), user)

ChownR (pkg/copy/copy.go) is a recursive filepath.WalkDir that Lchowns every entry it finds under that directory. A live agent never triggers a problem here because devpod builds its own dedicated relay directory holding exactly one socket. But Host::absent_agent_socket put the placeholder directly in devlaunch's cache root, so filepath.Dir(SSH_AUTH_SOCK) resolved to the entire cache directory: every cloned repo, every ssh control socket, everything dl stores. If any entry anywhere under that root is not owned by the container's remote user, or sits on a read-only mount, the recursive chown fails and the whole launch dies, with no indication that agent forwarding (which is working exactly as designed) had anything to do with it.

Observed on a real launch, where the walk reached a read-only bind:

chown ssh agent sock file: lchown /home/vscode/.ssh/known_hosts: read-only file system

The fix

Give the placeholder socket a directory of its own, so the recursive walk has exactly one trivial thing to walk. Both candidate paths get this: the cache-dir path, and the /tmp fallback used when the cache path is too deep to fit in a unix socket address (the 104-byte sockaddr_un limit). The fallback's directory name is now uid-keyed (devlaunch-<uid>-agent-sock), which is the uniqueness the placeholder's filename used to carry on its own; the filename itself is now the fixed no-agent.sock, with the directory doing the per-user keying instead.

bind_placeholder's existing symlink-safety guarantee (never trust a path via metadata, which follows symlinks, always symlink_metadata plus an owner check) is extended one level up via a new ensure_our_dir helper, so a symlinked directory at the placeholder's parent is refused rather than followed into a tree this process does not own.

Testing

Extended the existing placeholder-socket unit tests to assert the directory holding the placeholder contains nothing else and is never the cache root or /tmp itself, and added a test that a symlinked directory at the placeholder's parent path is refused. All pass; cargo clippy and cargo fmt --check clean; no public API or CLI flag changes.

Summary by Sourcery

Isolate placeholder SSH agent sockets in exclusively owned directories so agent setup cannot recursively chown unrelated files.

Bug Fixes:

• Prevent placeholder SSH agent socket setup from causing devpod to recursively chown the entire cache or temporary directory during launches without an SSH agent.
• Reject symlinked or non-owned placeholder parent directories to avoid traversing and modifying paths outside the launcher's control.

Enhancements:

• Place placeholder agent sockets in dedicated per-user directories for both cache and temporary-path fallbacks while preserving Unix socket path-length constraints.

Tests:

• Extend placeholder socket tests to verify dedicated directories contain only the socket and add coverage for refusing symlinked parent directories, including existing sockets.

[sourcery-ai]

Reviewer's Guide

The PR confines absent-agent placeholder sockets to dedicated, uid-scoped directories for both cache and /tmp fallback paths, preventing devpod's unconditional recursive chown from affecting unrelated files. It also validates parent-directory ownership without following symlinks and adds regression tests for isolation, safety, and socket-path length limits.

Sequence diagram for safe absent-agent socket setup

sequenceDiagram
    participant Host as Host
    participant HostAgent as HostAgent
    participant Filesystem as Filesystem
    participant Devpod as Devpod

    Host->>HostAgent: absent_agent_socket()
    HostAgent->>HostAgent: choose cache or /tmp uid-scoped candidate
    HostAgent->>Filesystem: bind_placeholder(path)
    Filesystem->>Filesystem: ensure_our_dir(parent)
    alt parent is owned directory
        Filesystem->>Filesystem: symlink_metadata(socket)
        Filesystem-->>HostAgent: socket path
    else parent is symlink or not owned
        Filesystem-->>HostAgent: None
    end
    HostAgent-->>Host: SSH_AUTH_SOCK path
    Host->>Devpod: launch workspace
    Devpod->>Filesystem: ChownR(filepath.Dir(SSH_AUTH_SOCK), user)
    Filesystem-->>Devpod: chown dedicated directory only

Loading

File-Level Changes

Change	Details	Files
Isolate the placeholder socket in a dedicated directory so devpod's recursive ownership walk cannot traverse the cache or temporary directory.	Add fixed cache-relative directory and socket names. Create uid-keyed directories for both temporary fallback candidates. Extend tests to verify the parent directory is isolated and contains only the placeholder.	rust/devlaunch-core/src/flows/launch.rs
Harden placeholder path creation against symlinked or unowned parent directories.	Add owner-checked, non-following directory validation with symlink_metadata. Refuse to create or bind the placeholder when its parent directory is unsafe.	rust/devlaunch-core/src/flows/launch.rs
Expand regression coverage for fallback paths and path-safety behavior.	Test rejection of symlinked placeholder directories without writing through the alias. Verify cache and /tmp are never used directly as the recursive chown root. Check the uid-keyed fallback remains within the Unix socket path-length limit.	rust/devlaunch-core/src/flows/launch.rs

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="rust/devlaunch-core/src/flows/launch.rs" line_range="2177-2182" />
<code_context>
     if is_our_socket(path) {
         return Some(path.to_owned());
     }
-    std::fs::create_dir_all(path.parent()?).ok()?;
+    if !ensure_our_dir(path.parent()?) {
+        return None;
+    }
     // SAFETY: as above.
</code_context>
<issue_to_address>
**🚨 issue (security):** An existing socket is accepted before `ensure_our_dir` checks its parent, so a socket reached through a symlinked parent directory is reused and devpod recursively chowns the directory tree behind that symlink.

**Triggers:** When the placeholder socket already exists and its parent directory has been replaced with a symlink.

**Suggested fix:** Call `ensure_our_dir(path.parent()?)` before the `is_our_socket` reuse return, or revalidate the parent immediately before returning the existing socket.

```suggestion
    if !ensure_our_dir(path.parent()?) {
        return None;
    }
    if is_our_socket(path) {
        return Some(path.to_owned());
    }
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

[codecov]

Codecov Report

❌ Patch coverage is 92.68293% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.41%. Comparing base (11d160d) to head (1fc0c87).

Files with missing lines	Patch %	Lines
rust/devlaunch-core/src/flows/launch.rs	92.68%	6 Missing ⚠️

Additional details and impacted files

Flag	Coverage Δ
python	42.98% <ø> (ø)
rust	94.63% <92.68%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
shipped code (rust)	94.63% <92.68%> (-0.02%)	⬇️
harness and tooling (python)	42.98% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JSmithRobotics]

Fixed in 1fc0c87, and the finding was correct.

bind_placeholder validated the parent on the path that creates a socket and not on the path that reuses one. The doc above the function already claimed both — "the same is true one level up" — so the code was contradicting its own stated invariant rather than merely missing a case.

Why it is reachable: is_our_socket decides with symlink_metadata, which refuses to follow a symlink at the final component and follows every component above it. A parent replaced by a symlink therefore leaves a genuine socket of this user's reachable through it, passing every test the reuse makes. devpod Lchowns the whole of filepath.Dir(SSH_AUTH_SOCK), so that is the tree it would then walk — which is the entire thing this PR exists to prevent.

The fix is the suggested ordering: ensure_our_dir first, both paths behind it.

The new test states the trap exactly — a real socket bound by the test process, inside a directory reached through a symlink. It asserts is_our_socket passes on purpose, because the socket being fine and the directory not being fine is the whole case. I verified it is load-bearing by putting the old order back: it fails with a socket under a directory this process does not own was handed to devpod.

Also rebased onto 0.59.2.