Skip to content
Merged
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
9 changes: 7 additions & 2 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,6 @@ export class Commands {
if (!vscode.workspace.workspaceFolders?.length) {
newWindow = false;
}

if (!folderPath && useDefaultDirectory) {
folderPath = agent.expanded_directory;
}
Expand All @@ -1366,7 +1365,6 @@ export class Commands {
// reference a workspace without an agent name, which will be missed.
(opened) => opened.folderUri?.authority === remoteAuthority,
);

// openRecent will always use the most recent. Otherwise, if there are
// multiple we ask the user which to use.
if (opened.length === 1 || (opened.length > 1 && openRecent)) {
Expand Down Expand Up @@ -1399,6 +1397,13 @@ export class Commands {
this.logger.error(`IPC ping failed for ${remoteAuthority}`, err);
});

this.logger.info("Opening remote workspace", {
remoteAuthority,
workspace: `${workspace.owner_name}/${workspace.name}`,
agent: agent.name,
handoff: folderPath ? "folder" : "empty_window",
});

if (folderPath) {
await vscode.commands.executeCommand(
"vscode.openFolder",
Expand Down
15 changes: 12 additions & 3 deletions src/remote/remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,22 @@ export class Remote {
startupMode: StartupMode,
remoteSshExtensionId: string,
): Promise<RemoteDetails | undefined> {
const parts = parseRemoteAuthority(remoteAuthority);
let parts: AuthorityParts | null;
try {
parts = parseRemoteAuthority(remoteAuthority);
} catch (error) {
this.logger.warn("Failed to parse remote authority", {
remoteAuthority,
error: toError(error).message,
});
throw error;
}
if (!parts) {
// Not a Coder host.
return;
}

this.logger.debug("Setting up remote connection", {
this.logger.info("Setting up remote connection", {
remoteAuthority,
hostname: parts.safeHostname,
workspace: `${parts.username}/${parts.workspace}`,
agent: parts.agent || "(default)",
Expand Down
35 changes: 32 additions & 3 deletions src/uri/uriHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ interface UriRouteContext extends UriHandlerDeps {
}

type UriRouteHandler = (ctx: UriRouteContext) => Promise<void>;
type CoderUriRoute = "open" | "openDevContainer";

const routes: Readonly<Record<string, UriRouteHandler>> = {
"/open": handleOpen,
Expand All @@ -48,7 +49,10 @@ export function registerUriHandler(deps: UriHandlerDeps): vscode.Disposable {
});
} catch (error) {
const message = errToStr(error, "No error message was provided");
output.warn(`Failed to handle URI ${uri.toString()}: ${message}`);
output.warn("Failed to handle URI", {
...summarizeUri(uri),
error: message,
});
vscodeProposed.window.showErrorMessage("Failed to handle URI", {
detail: message,
modal: true,
Expand All @@ -67,6 +71,17 @@ function getRequiredParam(params: URLSearchParams, name: string): string {
return value;
}

function summarizeUri(uri: vscode.Uri): Record<string, string | boolean> {
const params = new URLSearchParams(uri.query);
return {
path: uri.path,
hasOwner: params.has("owner"),
hasWorkspace: params.has("workspace"),
hasAgent: params.has("agent"),
hasToken: params.has("token"),
};
}

async function handleOpen(ctx: UriRouteContext): Promise<void> {
const { params, serviceContainer, deploymentManager, commands } = ctx;

Expand All @@ -78,7 +93,7 @@ async function handleOpen(ctx: UriRouteContext): Promise<void> {
params.has("openRecent") &&
(!params.get("openRecent") || params.get("openRecent") === "true");

await setupDeployment(params, serviceContainer, deploymentManager);
await setupDeployment("open", params, serviceContainer, deploymentManager);

await commands.open({
workspaceOwner: owner,
Expand Down Expand Up @@ -108,7 +123,12 @@ async function handleOpenDevContainer(ctx: UriRouteContext): Promise<void> {
);
}

await setupDeployment(params, serviceContainer, deploymentManager);
await setupDeployment(
"openDevContainer",
params,
serviceContainer,
deploymentManager,
);

await commands.openDevContainer(
owner,
Expand All @@ -126,6 +146,7 @@ async function handleOpenDevContainer(ctx: UriRouteContext): Promise<void> {
* and token storage. Throws if user cancels URL input or login fails.
*/
async function setupDeployment(
route: CoderUriRoute,
params: URLSearchParams,
serviceContainer: ServiceContainer,
deploymentManager: Pick<DeploymentManager, "setDeployment">,
Expand Down Expand Up @@ -154,6 +175,14 @@ async function setupDeployment(
}

const safeHostname = toSafeHost(url);
const owner = params.get("owner") ?? "";
const workspace = params.get("workspace") ?? "";
serviceContainer.getLogger().info("Handling Coder URI", {
route,
safeHostname,
workspace: owner && workspace ? `${owner}/${workspace}` : "",
agent: params.get("agent") ?? "(unspecified)",
});

const token: string | undefined = params.get("token") ?? undefined;
const result = await authTelemetry.traceLogin("uri", () =>
Expand Down
4 changes: 2 additions & 2 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ export function findPort(text: string): number | null {
* SSH host names created by this extension match the format:
* coder-vscode.<safeHostname>--<username>--<workspace>(.<agent?>)
*
* If this is not a Coder host, return null.
* If this is not a Coder authority, return null.
*
* Throw an error if the host is invalid.
* Throw an error if a Coder authority is invalid.
*/
export function parseRemoteAuthority(authority: string): AuthorityParts | null {
const authorityParts = authority.split("+");
Expand Down
34 changes: 33 additions & 1 deletion test/unit/uri/uriHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,11 +363,43 @@ describe("uriHandler", () => {
await handleUri(createMockUri("/open", "workspace=w"));

expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining("Failed to handle URI"),
"Failed to handle URI",
expect.objectContaining({
error: expect.stringContaining("owner must be specified"),
}),
);
expect(showErrorMessage).toHaveBeenCalled();
});

it("does not log URI tokens on failure", async () => {
const { handleUri, commands, logger } = createTestContext();
commands.open.mockRejectedValue(new Error("Connection failed"));

await handleUri(
createMockUri(
"/open",
`owner=o&workspace=w&url=${encodeURIComponent(TEST_URL)}&token=secret-token`,
),
);

expect(logger.warn).toHaveBeenCalledWith(
"Failed to handle URI",
expect.objectContaining({
error: "Connection failed",
}),
);
const logged = [
...vi.mocked(logger.debug).mock.calls,
...vi.mocked(logger.info).mock.calls,
...vi.mocked(logger.warn).mock.calls,
]
.map((call) => JSON.stringify(call))
.join("\n");
expect(logged).not.toContain("secret-token");
expect(logged).not.toContain("token=");
expect(logged).not.toContain("vscode://coder.coder-remote");
});

it("propagates command errors", async () => {
const { handleUri, commands, showErrorMessage } = createTestContext();
commands.open.mockRejectedValue(new Error("Connection failed"));
Expand Down
27 changes: 26 additions & 1 deletion test/unit/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
openInBrowser,
parseRemoteAuthority,
resolveUiUrl,
toRemoteAuthority,
toSafeHost,
} from "@/util";

Expand Down Expand Up @@ -42,13 +43,17 @@ describe("parseRemoteAuthority", () => {

it.each([
{
label: "missing user and workspace",
label: "missing username and workspace",
sshHost: "coder-vscode.dev.coder.com",
},
{
label: "missing workspace",
sshHost: "coder-vscode.dev.coder.com--foo",
},
{
label: "manual host using Coder prefix",
sshHost: "coder-vscode.personal-host",
},
{
label: "empty username",
sshHost: "coder-vscode.dev.coder.com----bar",
Expand Down Expand Up @@ -88,6 +93,26 @@ describe("parseRemoteAuthority", () => {
username?: string;
}

it("round trips generated remote authorities", () => {
const authority = toRemoteAuthority(
"https://ほげ",
Comment thread
EhabY marked this conversation as resolved.
"alice",
"workspace",
"main",
);

expect(authority).toBe(
"ssh-remote+coder-vscode.xn--18j4d--alice--workspace.main",
);
expect(parseRemoteAuthority(authority)).toStrictEqual({
agent: "main",
sshHost: "coder-vscode.xn--18j4d--alice--workspace.main",
safeHostname: "xn--18j4d",
username: "alice",
workspace: "workspace",
} satisfies AuthorityParts);
});

it.each<ParseCase>([
{
label: "hostname without agent",
Expand Down