-
Notifications
You must be signed in to change notification settings - Fork 0
Fix socket connection issues #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||
| import { spawn } from "node:child_process"; | ||||||
| import { access, appendFile, mkdir, readFile, realpath } from "node:fs/promises"; | ||||||
| import { access, appendFile, lstat, mkdir, readFile, readlink, realpath, rm, symlink } from "node:fs/promises"; | ||||||
| import { accessSync, constants as fsConstants } from "node:fs"; | ||||||
| import { createServer } from "node:net"; | ||||||
| import os from "node:os"; | ||||||
|
|
@@ -77,6 +77,12 @@ export interface PortAvailability { | |||||
| pids: string[]; | ||||||
| } | ||||||
|
|
||||||
| export type ManagedContainerSshMountCompatibility = | ||||||
| | "not-applicable" | ||||||
| | "unchanged" | ||||||
| | "created-symlink" | ||||||
| | "updated-symlink"; | ||||||
|
|
||||||
| export function isExecutableAvailable(command: string): boolean { | ||||||
| return findExecutableOnPath(command) !== null; | ||||||
| } | ||||||
|
|
@@ -604,6 +610,61 @@ export async function removeContainers(containerIds: string[]): Promise<void> { | |||||
| }); | ||||||
| } | ||||||
|
|
||||||
| export async function ensureManagedContainerSshMountCompatibility( | ||||||
| container: DockerInspect, | ||||||
| sshAuthSockSource: string | null, | ||||||
| ): Promise<ManagedContainerSshMountCompatibility> { | ||||||
| const trimmedSshAuthSockSource = sshAuthSockSource?.trim() || null; | ||||||
| if (!trimmedSshAuthSockSource || trimmedSshAuthSockSource === DOCKER_DESKTOP_SSH_AUTH_SOCK_SOURCE) { | ||||||
| return "not-applicable"; | ||||||
| } | ||||||
|
|
||||||
| const containerSshAuthSock = getContainerSshAuthSockPath(trimmedSshAuthSockSource); | ||||||
| if (!containerSshAuthSock) { | ||||||
| return "not-applicable"; | ||||||
| } | ||||||
|
|
||||||
| const mountedSource = getContainerBindMountSource(container, containerSshAuthSock); | ||||||
| if (!mountedSource) { | ||||||
| return "unchanged"; | ||||||
| } | ||||||
|
|
||||||
| const resolvedMountedSource = path.resolve(mountedSource); | ||||||
| const resolvedCurrentSshAuthSock = path.resolve(trimmedSshAuthSockSource); | ||||||
| if (resolvedMountedSource === resolvedCurrentSshAuthSock) { | ||||||
| return "unchanged"; | ||||||
| } | ||||||
|
|
||||||
| let mountedSourceStat: Awaited<ReturnType<typeof lstat>> | null = null; | ||||||
| try { | ||||||
| mountedSourceStat = await lstat(resolvedMountedSource); | ||||||
| } catch (error) { | ||||||
| if (!isMissingPathError(error)) { | ||||||
| throw error; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (!mountedSourceStat) { | ||||||
| await mkdir(path.dirname(resolvedMountedSource), { recursive: true }); | ||||||
| await symlink(resolvedCurrentSshAuthSock, resolvedMountedSource); | ||||||
| return "created-symlink"; | ||||||
| } | ||||||
|
|
||||||
| if (!mountedSourceStat.isSymbolicLink()) { | ||||||
| return "unchanged"; | ||||||
| } | ||||||
|
|
||||||
| const existingTarget = await readlink(resolvedMountedSource); | ||||||
| const resolvedExistingTarget = path.resolve(path.dirname(resolvedMountedSource), existingTarget); | ||||||
| if (resolvedExistingTarget === resolvedCurrentSshAuthSock) { | ||||||
| return "unchanged"; | ||||||
| } | ||||||
|
|
||||||
| await rm(resolvedMountedSource); | ||||||
|
||||||
| await rm(resolvedMountedSource); | |
| await rm(resolvedMountedSource, { force: true }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { mkdtemp, mkdir, readFile, realpath, rm, writeFile } from "node:fs/promises"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { mkdtemp, mkdir, readFile, readlink, realpath, rm, symlink, writeFile } from "node:fs/promises"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os from "node:os"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import path from "node:path"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { afterEach, describe, expect, test } from "bun:test"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -29,6 +29,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolveSshPublicKey, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolveShellContainerId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolveGhCliToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ensureManagedContainerSshMountCompatibility, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requiresSshAuthSockPermissionFix, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolveSshAuthSockSource, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "../src/runtime"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -110,6 +111,107 @@ describe("resolveSshAuthSockSource", () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe("ensureManagedContainerSshMountCompatibility", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test("creates a compatibility symlink when the previous ssh agent mount source is missing", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tempDir = await mkdtemp(path.join(os.tmpdir(), "devbox-test-")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tempPaths.push(tempDir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const currentSocketPath = path.join(tempDir, "current", "agent.sock"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const previousSocketPath = path.join(tempDir, "old", "agent.sock"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await mkdir(path.dirname(currentSocketPath), { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await writeFile(currentSocketPath, "socket"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const outcome = await ensureManagedContainerSshMountCompatibility( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Id: "container-1", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Mounts: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Type: "bind", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Source: previousSocketPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Destination: "/run/devbox-ssh-auth.sock", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentSocketPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(outcome).toBe("created-symlink"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(await readlink(previousSocketPath)).toBe(currentSocketPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test("updates an existing stale compatibility symlink to the current ssh agent socket", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tempDir = await mkdtemp(path.join(os.tmpdir(), "devbox-test-")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tempPaths.push(tempDir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const currentSocketPath = path.join(tempDir, "current", "agent.sock"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const staleSocketPath = path.join(tempDir, "stale", "agent.sock"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const compatibilityPath = path.join(tempDir, "old", "agent.sock"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await mkdir(path.dirname(currentSocketPath), { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await mkdir(path.dirname(staleSocketPath), { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await mkdir(path.dirname(compatibilityPath), { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await writeFile(currentSocketPath, "current"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await writeFile(staleSocketPath, "stale"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await symlink(staleSocketPath, compatibilityPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const outcome = await ensureManagedContainerSshMountCompatibility( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Id: "container-1", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Mounts: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Type: "bind", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Source: compatibilityPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Destination: "/run/devbox-ssh-auth.sock", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentSocketPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(outcome).toBe("updated-symlink"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(await readlink(compatibilityPath)).toBe(currentSocketPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test("does nothing when the existing mount already points at the current host ssh agent socket", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tempDir = await mkdtemp(path.join(os.tmpdir(), "devbox-test-")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tempPaths.push(tempDir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const currentSocketPath = path.join(tempDir, "current", "agent.sock"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await mkdir(path.dirname(currentSocketPath), { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await writeFile(currentSocketPath, "current"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ensureManagedContainerSshMountCompatibility( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Id: "container-1", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Mounts: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Type: "bind", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Source: currentSocketPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Destination: "/run/devbox-ssh-auth.sock", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentSocketPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).resolves.toBe("unchanged"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test("does nothing when the existing mount source is a symlink that already resolves to the current host ssh agent socket", async () => { | |
| const tempDir = await mkdtemp(path.join(os.tmpdir(), "devbox-test-")); | |
| tempPaths.push(tempDir); | |
| const currentSocketPath = path.join(tempDir, "current", "agent.sock"); | |
| const compatibilitySocketPath = path.join(tempDir, "compat", "agent.sock"); | |
| await mkdir(path.dirname(currentSocketPath), { recursive: true }); | |
| await mkdir(path.dirname(compatibilitySocketPath), { recursive: true }); | |
| await writeFile(currentSocketPath, "current"); | |
| await symlink(currentSocketPath, compatibilitySocketPath); | |
| await expect( | |
| ensureManagedContainerSshMountCompatibility( | |
| { | |
| Id: "container-1", | |
| Mounts: [ | |
| { | |
| Type: "bind", | |
| Source: compatibilitySocketPath, | |
| Destination: "/run/devbox-ssh-auth.sock", | |
| }, | |
| ], | |
| }, | |
| currentSocketPath, | |
| ), | |
| ).resolves.toBe("unchanged"); | |
| expect(await readlink(compatibilitySocketPath)).toBe(currentSocketPath); | |
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Path comparisons here use
path.resolve, which does not canonicalize symlinks. On macOS (e.g./tmpvs/private/tmp) or whenSSH_AUTH_SOCKitself is a symlink, this can cause an unnecessaryupdated-symlinkoutcome even though both paths refer to the same underlying socket. Consider comparing canonicalized paths (e.g. via the existingresolveComparablePathhelper orrealpathwhere available) before deciding to update the symlink.