Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion mobile/src/components/RenameWorkspaceModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ export function RenameWorkspaceModal({
variant="caption"
style={{ marginTop: spacing.sm, color: theme.colors.foregroundMuted }}
>
Only lowercase letters, digits, underscore, and hyphen (1-64 characters)
Only lowercase letters, digits, underscore, hyphen, and forward slash (1-64
characters, no leading/trailing/consecutive slashes)
</ThemedText>
)}
</View>
Expand Down
43 changes: 41 additions & 2 deletions mobile/src/utils/workspaceValidation.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from "bun:test";
import { validateWorkspaceName } from "./workspaceValidation";
import { validateWorkspaceName, sanitizeWorkspaceNameForPath } from "./workspaceValidation";

describe("validateWorkspaceName", () => {
describe("empty names", () => {
Expand Down Expand Up @@ -55,6 +55,12 @@ describe("validateWorkspaceName", () => {
expect(validateWorkspaceName("test-workspace-name")).toEqual({ valid: true });
});

it("accepts forward slashes in branch-style names", () => {
expect(validateWorkspaceName("feature/my-branch")).toEqual({ valid: true });
expect(validateWorkspaceName("fix/issue-123")).toEqual({ valid: true });
expect(validateWorkspaceName("user/feature/deep")).toEqual({ valid: true });
});

it("accepts mixed valid characters", () => {
expect(validateWorkspaceName("feature-branch_123")).toEqual({ valid: true });
expect(validateWorkspaceName("fix-001")).toEqual({ valid: true });
Expand All @@ -73,6 +79,18 @@ describe("validateWorkspaceName", () => {
expect(result.error).toContain("lowercase letters");
});

it("rejects leading slash", () => {
expect(validateWorkspaceName("/feature").valid).toBe(false);
});

it("rejects trailing slash", () => {
expect(validateWorkspaceName("feature/").valid).toBe(false);
});

it("rejects consecutive slashes", () => {
expect(validateWorkspaceName("feature//branch").valid).toBe(false);
});

it("rejects special characters", () => {
const invalidNames = [
"test!",
Expand All @@ -92,7 +110,6 @@ describe("validateWorkspaceName", () => {
"test}",
"test|",
"test\\",
"test/",
"test?",
"test<",
"test>",
Expand Down Expand Up @@ -126,3 +143,25 @@ describe("validateWorkspaceName", () => {
});
});
});

describe("sanitizeWorkspaceNameForPath", () => {
it("returns name unchanged when no slashes", () => {
expect(sanitizeWorkspaceNameForPath("my-branch")).toBe("my-branch");
});

it("replaces single slash with hyphen", () => {
expect(sanitizeWorkspaceNameForPath("feature/my-branch")).toBe("feature-my-branch");
});

it("replaces multiple slashes in deep paths", () => {
expect(sanitizeWorkspaceNameForPath("user/feature/deep")).toBe("user-feature-deep");
});

it("preserves existing consecutive hyphens", () => {
expect(sanitizeWorkspaceNameForPath("feature--branch")).toBe("feature--branch");
});

it("does not collapse hyphens adjacent to replaced slash", () => {
expect(sanitizeWorkspaceNameForPath("feature/-branch")).toBe("feature--branch");
});
});
18 changes: 14 additions & 4 deletions mobile/src/utils/workspaceValidation.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/**
* Validates workspace name format
* - Must be 1-64 characters long
* - Can only contain: lowercase letters, digits, underscore, hyphen
* - Pattern: [a-z0-9_-]{1,64}
* - Can only contain: lowercase letters, digits, underscore, hyphen, forward slash
* - No leading, trailing, or consecutive slashes
* - Pattern: [a-z0-9_-]+(?:\/[a-z0-9_-]+)* (1-64 characters)
*/
export function validateWorkspaceName(name: string): { valid: boolean; error?: string } {
if (!name || name.length === 0) {
Expand All @@ -13,13 +14,22 @@ export function validateWorkspaceName(name: string): { valid: boolean; error?: s
return { valid: false, error: "Workspace name cannot exceed 64 characters" };
}

const validPattern = /^[a-z0-9_-]+$/;
const validPattern = /^[a-z0-9_-]+(?:\/[a-z0-9_-]+)*$/;
if (!validPattern.test(name)) {
return {
valid: false,
error: "Workspace name can only contain lowercase letters, digits, underscore, and hyphen",
error:
"Workspace names can only contain lowercase letters, numbers, hyphens, underscores, and forward slashes (no leading, trailing, or consecutive slashes)",
};
}

return { valid: true };
}

/**
* Convert a workspace name to a filesystem-safe path component by replacing
* forward slashes with hyphens.
*/
export function sanitizeWorkspaceNameForPath(name: string): string {
return name.replace(/\//g, "-");
}
46 changes: 42 additions & 4 deletions src/common/utils/validation/workspaceValidation.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { validateWorkspaceName } from "./workspaceValidation";
import { validateWorkspaceName, sanitizeWorkspaceNameForPath } from "./workspaceValidation";

describe("validateWorkspaceName", () => {
describe("valid names", () => {
Expand All @@ -22,6 +22,12 @@ describe("validateWorkspaceName", () => {
expect(validateWorkspaceName("feature-test-123").valid).toBe(true);
});

test("accepts forward slashes in branch-style names", () => {
expect(validateWorkspaceName("feature/my-branch").valid).toBe(true);
expect(validateWorkspaceName("fix/issue-123").valid).toBe(true);
expect(validateWorkspaceName("user/feature/deep").valid).toBe(true);
});

test("accepts combinations", () => {
expect(validateWorkspaceName("feature-branch_123").valid).toBe(true);
expect(validateWorkspaceName("a1-b2_c3").valid).toBe(true);
Expand Down Expand Up @@ -72,13 +78,45 @@ describe("validateWorkspaceName", () => {
expect(validateWorkspaceName("branch%123").valid).toBe(false);
expect(validateWorkspaceName("branch!123").valid).toBe(false);
expect(validateWorkspaceName("branch.123").valid).toBe(false);
expect(validateWorkspaceName("branch/123").valid).toBe(false);
expect(validateWorkspaceName("branch\\123").valid).toBe(false);
});

test("rejects names with slashes", () => {
expect(validateWorkspaceName("feature/branch").valid).toBe(false);
test("rejects leading slash", () => {
expect(validateWorkspaceName("/feature").valid).toBe(false);
});

test("rejects trailing slash", () => {
expect(validateWorkspaceName("feature/").valid).toBe(false);
});

test("rejects consecutive slashes", () => {
expect(validateWorkspaceName("feature//branch").valid).toBe(false);
});

test("rejects backslashes", () => {
expect(validateWorkspaceName("path\\to\\branch").valid).toBe(false);
});
});
});

describe("sanitizeWorkspaceNameForPath", () => {
test("returns name unchanged when no slashes", () => {
expect(sanitizeWorkspaceNameForPath("my-branch")).toBe("my-branch");
});

test("replaces single slash with hyphen", () => {
expect(sanitizeWorkspaceNameForPath("feature/my-branch")).toBe("feature-my-branch");
});

test("replaces multiple slashes in deep paths", () => {
expect(sanitizeWorkspaceNameForPath("user/feature/deep")).toBe("user-feature-deep");
});

test("preserves existing consecutive hyphens", () => {
expect(sanitizeWorkspaceNameForPath("feature--branch")).toBe("feature--branch");
});

test("does not collapse hyphens adjacent to replaced slash", () => {
expect(sanitizeWorkspaceNameForPath("feature/-branch")).toBe("feature--branch");
});
});
22 changes: 16 additions & 6 deletions src/common/utils/validation/workspaceValidation.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/**
* Validates workspace name format
* - Must be 1-64 characters long
* - Can only contain: lowercase letters, digits, underscore, hyphen
* - Pattern: [a-z0-9_-]{1,64}
* - Can only contain: lowercase letters, digits, underscore, hyphen, forward slash
* - No leading, trailing, or consecutive slashes
* - Pattern: [a-z0-9_-]+(?:\/[a-z0-9_-]+)* (1-64 characters)
*/
export function validateWorkspaceName(name: string): { valid: boolean; error?: string } {
if (!name || name.length === 0) {
Expand All @@ -13,16 +14,25 @@ export function validateWorkspaceName(name: string): { valid: boolean; error?: s
return { valid: false, error: "Workspace name cannot exceed 64 characters" };
}

const validPattern = /^[a-z0-9_-]+$/;
const validPattern = /^[a-z0-9_-]+(?:\/[a-z0-9_-]+)*$/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard plan-file migration before allowing slash names

Allowing / in workspace names here makes plan paths become nested (getPlanFilePath builds .../{workspaceName}.md in src/common/utils/planStorage.ts:25-31), but movePlanFile still runs a bare mv and neither creates the destination parent directory nor checks execBuffered exit status (src/node/utils/runtime/helpers.ts:246-253). Renaming a workspace with an existing plan from something like foo to feature/foo will therefore silently fail to move the file, and later reads target only the new path, so the plan appears to disappear after rename.

Useful? React with 👍 / 👎.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add sanitized-name collision checks to fork entrypoints

By permitting / in validateWorkspaceName, this change introduces aliasing (feature/a and feature-a) but only create()/rename() received sanitized collision guards; fork() still only validates format (see workspaceService.fork around the validateWorkspaceName(resolvedName) check) and can persist a conflicting name in runtimes that don't enforce distinct directories (notably LocalRuntime.forkWorkspace). Since plan paths now sanitize / to - (getPlanFilePath), those two workspaces resolve to the same plan file and can overwrite each other’s plan data.

Useful? React with 👍 / 👎.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve plan file when renaming to slash-delimited names

Allowing / in workspace names here introduces a silent data-loss path for plan files: movePlanFile (in src/node/utils/runtime/helpers.ts) still does mv oldPath newPath without creating the new parent directory and swallows failures, so renaming a workspace like foo to feature/foo leaves the old plan at .../foo.md and the renamed workspace appears to have no plan. This did not occur before slash names were accepted because both old/new plan paths stayed in the same directory depth.

Useful? React with 👍 / 👎.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep Coder workspace creation working with slash names

Allowing / here means names like feature/my-branch now pass validation, but Coder creation still fails later because CoderSSHRuntime.finalizeConfig() derives mux-${finalBranchName}, toCoderCompatibleName() only rewrites underscores/hyphens, and CODER_NAME_REGEX then rejects the remaining slash (src/node/runtime/CoderSSHRuntime.ts, around toCoderCompatibleName and finalizeConfig). In practice, users on Coder runtimes hit a hard error when creating/forking exactly the branch-style names this change now accepts, so this introduces a runtime-specific regression unless slash is converted (or rejected earlier for Coder).

Useful? React with 👍 / 👎.

if (!validPattern.test(name)) {
return {
valid: false,
// Workspace names become folder names, git branches, and session directories,
// so they need to be filesystem-safe across platforms.
error:
"Workspace names can only contain lowercase letters, numbers, hyphens, and underscores",
"Workspace names can only contain lowercase letters, numbers, hyphens, underscores, and forward slashes (no leading, trailing, or consecutive slashes)",
};
}

return { valid: true };
}

/**
* Convert a workspace name to a filesystem-safe path component by replacing
* forward slashes with hyphens.
*
* This allows git-style branch names like "feature/my-branch" to be used as
* workspace names while remaining safe for directory names and session paths.
*/
export function sanitizeWorkspaceNameForPath(name: string): string {
return name.replace(/\//g, "-");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make workspace path sanitization collision-safe

sanitizeWorkspaceNameForPath() currently rewrites every / to -, so distinct workspace names like feature/a and feature-a map to the same filesystem path. Because WorktreeManager.getWorkspacePath() and SSHRuntime.getWorkspacePath() now depend on this mapping, creating/renaming/deleting one of these workspaces can target the other’s directory, and create retries may fall back to a suffixed branch instead of the requested branch. Please use a reversible encoding (or add a deterministic disambiguator) so each valid workspace name has a unique path.

Useful? React with 👍 / 👎.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip sanitizing absolute workspace names

sanitizeWorkspaceNameForPath() rewrites every / to -, which also transforms in-place workspace names that are absolute paths (e.g. AgentSession.ensureMetadata stores workspaceName = normalizedWorkspacePath for direct workspaces). The new collision checks in WorkspaceService.create/rename/fork compare these sanitized values, so an existing in-place workspace like /home/user/project can incorrectly block a valid name such as -home-user-project with a false “same directory” error, even though runtime path resolution explicitly treats absolute names as unsanitized paths.

Useful? React with 👍 / 👎.

}
3 changes: 2 additions & 1 deletion src/node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { isValidModelFormat, normalizeGatewayModel } from "@/common/utils/ai/mod
import { ensurePrivateDirSync } from "@/node/utils/fs";
import { stripTrailingSlashes } from "@/node/utils/pathUtils";
import { getContainerName as getDockerContainerName } from "@/node/runtime/DockerRuntime";
import { sanitizeWorkspaceNameForPath } from "@/common/utils/validation/workspaceValidation";

// Re-export project/provider types from dedicated schema/types files (for preload usage)
export type { Workspace, ProjectConfig, ProjectsConfig, ProviderConfig, CanonicalProvidersConfig };
Expand Down Expand Up @@ -1060,7 +1061,7 @@ export class Config {
// otherwise fall back to worktree-style path for legacy compatibility
const projectName = this.getProjectName(projectPath);
const workspacePath =
metadata.namedWorkspacePath ?? path.join(this.srcDir, projectName, metadata.name);
metadata.namedWorkspacePath ?? path.join(this.srcDir, projectName, path.isAbsolute(metadata.name) ? metadata.name : sanitizeWorkspaceNameForPath(metadata.name));
const workspaceEntry: Workspace = {
path: workspacePath,
id: metadata.id,
Expand Down
1 change: 1 addition & 0 deletions src/node/runtime/CoderSSHRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const CODER_NAME_REGEX = /^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$/;
*/
function toCoderCompatibleName(name: string): string {
return name
.replace(/\//g, "-") // Replace slashes with hyphens (branch-style names)
.replace(/_/g, "-") // Replace underscores with hyphens
.replace(/^-+|-+$/g, "") // Remove leading/trailing hyphens
.replace(/-{2,}/g, "-"); // Collapse multiple hyphens
Expand Down
30 changes: 29 additions & 1 deletion src/node/runtime/SSHRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { getOriginUrlForBundle } from "./gitBundleSync";
import { gitNoHooksPrefix } from "@/node/utils/gitNoHooksEnv";
import type { PtyHandle, PtySessionParams, SSHTransport } from "./transports";
import { streamToString, shescape } from "./streamUtils";
import { sanitizeWorkspaceNameForPath } from "@/common/utils/validation/workspaceValidation";

/** Name of the shared bare repo directory under each project on the remote. */
const BASE_REPO_DIR = ".mux-base.git";
Expand Down Expand Up @@ -338,7 +339,13 @@ export class SSHRuntime extends RemoteRuntime {

getWorkspacePath(projectPath: string, workspaceName: string): string {
const projectName = getProjectName(projectPath);
return path.posix.join(this.config.srcBaseDir, projectName, workspaceName);
// In-place workspaces use absolute paths as workspace names — do not sanitize
// those since path.posix.join already handles them correctly and sanitizing
// would turn e.g. "/home/user/project" into "-home-user-project".
const safeName = path.posix.isAbsolute(workspaceName)
? workspaceName
: sanitizeWorkspaceNameForPath(workspaceName);
return path.posix.join(this.config.srcBaseDir, projectName, safeName);
}

/**
Expand Down Expand Up @@ -1147,6 +1154,27 @@ export class SSHRuntime extends RemoteRuntime {
const oldPath = this.getWorkspacePath(projectPath, oldName);
const newPath = this.getWorkspacePath(projectPath, newName);

// Short-circuit: if the sanitized paths are identical (e.g. renaming
// "feature-a" <-> "feature/a"), skip the directory move — only the
// persisted name changes, not the physical location.
if (oldPath === newPath) {
// Still rename the git branch to maintain the workspace/branch name sync.
// Best-effort: branch rename can fail if the workspace is in detached HEAD
// state or if the branch has drifted, matching the normal rename path.
try {
const expandedOldPath = expandTildeForSSH(oldPath);
const branchStream = await this.exec(
`git -C ${shescape.quote(expandedOldPath)} branch -m ${shescape.quote(oldName)} ${shescape.quote(newName)}`,
{ cwd: this.config.srcBaseDir, timeout: 10, abortSignal }
);
await branchStream.stdin.abort();
await branchStream.exitCode;
} catch {
// Best-effort: branch rename can fail (detached HEAD, branch drift, etc.)
}
return { success: true, oldPath, newPath };
}

try {
const expandedOldPath = expandTildeForSSH(oldPath);
const expandedNewPath = expandTildeForSSH(newPath);
Expand Down
Loading
Loading