fix: key every per-deployment store by origin

CHANGELOG.md

@@ -20,6 +20,11 @@
 
 - A session saved for one URL is no longer used for a URL on the same host with
   a different scheme or port.
+- Each deployment origin (scheme, host, and port) now has its own stored
+  session, CLI credentials, SSH hosts, and CLI binary cache, so two origins on
+  one host no longer overwrite each other or connect a window to the wrong
+  deployment. HTTPS on the default port is unchanged; other origins need one
+  new login and a fresh CLI download.
 - The session token and `coder.headerCommand` headers are now only sent to the
   deployment, not to other servers such as a `coder.binarySource` host.
 - Sessions for `http://` deployments are saved to the OS keyring only with Coder

src/common/url.ts

@@ -1,16 +1,34 @@
 import url from "node:url";
 import * as vscode from "vscode";
 
-import { parseDeploymentUrl, validateFilenameHostname } from "./validation";
+import {
+	InputValidationError,
+	parseDeploymentUrl,
+	validateFilenameHostname,
+} from "./validation";
 
 /**
- * Given a URL, return the host in a format that is safe to write.
+ * The URL's origin as a file- and SSH-safe label that keys per-deployment
+ * storage: `host` for HTTPS on 443, else `host_<scheme><port>` without `https`
+ * or a default port (`host_8443`, `host_http`). A hostname with `_` always gets
+ * a suffix (`my_host_https`), so labels never collide.
  */
 export function toSafeHost(rawUrl: string): string {
 	const u = parseDeploymentUrl(rawUrl);
+	if (u.protocol !== "https:" && u.protocol !== "http:") {
+		throw new InputValidationError(
+			"Invalid URL: the deployment URL must use http or https.",
+			u.protocol,
+		);
+	}
 	// If the host is invalid, an empty string is returned.  Although, `new URL`
 	// should already have thrown in that case.
-	return validateFilenameHostname(url.domainToASCII(u.hostname) || u.hostname);
+	const hostname = url.domainToASCII(u.hostname) || u.hostname;
+	let suffix = (u.protocol === "http:" ? "http" : "") + u.port;
+	if (!suffix && hostname.includes("_")) {
+		suffix = "https";
+	}
+	return validateFilenameHostname(suffix ? `${hostname}_${suffix}` : hostname);
 }
 
 export function removeTrailingSlashes(value: string): string {

src/common/validation.ts

@@ -82,15 +82,16 @@ export function validateFilenameHostname(value: string): string {
 	return value;
 }
 
+/**
+ * A bracketed IPv6 host, optionally with a `toSafeHost` origin suffix
+ * (`_https`, `_http`, `_http8080`, `_8443`).
+ */
+const BRACKETED_IPV6_LABEL = /^\[([^\]%]+)\](?:_(?:https|http\d*|\d+))?$/;
+
 /** Shell-safe, literal SSH host characters; IP structure uses Node's parser. */
 export function isShellSafeHostname(value: string): boolean {
-	return (
-		/^[\w.-]+$/.test(value) ||
-		(value.startsWith("[") &&
-			value.endsWith("]") &&
-			!value.includes("%") &&
-			isIP(value.slice(1, -1)) === 6)
-	);
+	const ipv6 = BRACKETED_IPV6_LABEL.exec(value);
+	return /^[\w.-]+$/.test(value) || (ipv6 !== null && isIP(ipv6[1]) === 6);
 }
 
 export function validateShellSafeHostname(value: string): string {

src/deployment/types.ts

@@ -5,7 +5,7 @@ import type { User } from "coder/site/src/api/typesGenerated";
 /**
  * Represents a Coder deployment with its URL and hostname.
  * The safeHostname is used as a unique identifier for storing credentials and configuration.
- * It is derived from the URL hostname (via toSafeHost) or from SSH host parsing.
+ * It is the URL's origin label (via toSafeHost) or comes from SSH host parsing.
  */
 export const DeploymentSchema = z.object({
 	url: z.string(),

src/login/loginCoordinator.ts

@@ -5,7 +5,7 @@ import * as vscode from "vscode";
 import { CoderApi } from "../api/coderApi";
 import { needToken } from "../api/httpAgent";
 import { HttpStatusCode } from "../api/httpStatusCode";
-import { isSameOrigin, openInBrowser } from "../common/url";
+import { openInBrowser, toSafeHost } from "../common/url";
 import { CertificateError } from "../error/certificateError";
 import { CrossOriginRedirectError } from "../error/crossOriginRedirectError";
 import { OAuthAuthorizer } from "../oauth/authorizer";
@@ -76,10 +76,8 @@ interface LoginAttemptContext {
 	client: CoderApi;
 	deployment: Deployment;
 	isAutoLogin: boolean;
-	/** Stored session for the hostname, if any. */
+	/** Stored session for the deployment's origin, if any. */
 	auth: SessionAuth | undefined;
-	/** `auth`, but only when it matches the deployment's exact origin. */
-	sameOriginAuth: SessionAuth | undefined;
 }
 
 /**
@@ -170,6 +168,13 @@ export class LoginCoordinator implements vscode.Disposable {
 						if (!newUrl) {
 							return { success: false, reason: "no_url_provided" };
 						}
+						// Another origin's session belongs under another label.
+						if (toSafeHost(newUrl) !== safeHostname) {
+							vscode.window.showErrorMessage(
+								`${newUrl} is a different deployment from ${safeHostname}. Sign in from the Coder sidebar to use it.`,
+							);
+							return { success: false, reason: "no_url_provided" };
+						}
 
 						return this.attemptLogin(
 							{ url: newUrl, safeHostname },
@@ -200,16 +205,13 @@ export class LoginCoordinator implements vscode.Disposable {
 		registration: OAuth2ClientRegistrationResponse | undefined,
 	): Promise<void> {
 		const { safeHostname, url } = deployment;
-		const previous = await this.secretsManager.getSessionAuth(safeHostname);
+		const previous = await this.secretsManager.getSessionAuthForUrl(url);
 		const replaced = previous?.token === result.token ? undefined : previous;
 		// A refresh replaced the stored session during this login; keep it.
 		if (result.method === "stored_token" && replaced) {
 			return;
 		}
-		const reusedOAuth =
-			!replaced && previous && isSameOrigin(previous.url, url)
-				? previous.oauth
-				: undefined;
+		const reusedOAuth = replaced ? undefined : previous?.oauth;
 
 		// Empty token is valid for mTLS
 		await this.secretsManager.setSessionAuth(safeHostname, {
@@ -339,17 +341,12 @@ export class LoginCoordinator implements vscode.Disposable {
 			);
 		}
 
-		const auth = await this.secretsManager.getSessionAuth(
-			deployment.safeHostname,
-		);
+		const auth = await this.secretsManager.getSessionAuthForUrl(deployment.url);
 		const ctx: LoginAttemptContext = {
 			client,
 			deployment,
 			isAutoLogin,
 			auth,
-			// Only sent to its own origin, so a lookalike hostname cannot get it.
-			sameOriginAuth:
-				auth && isSameOrigin(auth.url, deployment.url) ? auth : undefined,
 		};
 
 		if (providedToken) {
@@ -380,7 +377,7 @@ export class LoginCoordinator implements vscode.Disposable {
 		providedToken: string,
 		tokenSignInConfirmed: boolean,
 	): Promise<LoginResult> {
-		const { client, deployment, isAutoLogin, auth, sameOriginAuth } = ctx;
+		const { client, deployment, isAutoLogin, auth } = ctx;
 		this.logger.debug("Trying provided token");
 		const result = await this.tryTokenAuth(client, providedToken, isAutoLogin);
 		if (result === "unauthorized") {
@@ -398,13 +395,13 @@ export class LoginCoordinator implements vscode.Disposable {
 			// Silent when the link repeats the same user on the same origin, or an
 			// earlier prompt confirmed a sign-in that replaces nothing.
 			const silent =
-				sameOriginAuth?.username === result.user.username ||
+				auth?.username === result.user.username ||
 				(auth === undefined && tokenSignInConfirmed);
 			if (!silent) {
 				// Checked silently, only to word the prompt.
 				const expired =
-					sameOriginAuth?.token !== undefined &&
-					(await this.tryTokenAuth(client, sameOriginAuth.token, true)) ===
+					auth?.token !== undefined &&
+					(await this.tryTokenAuth(client, auth.token, true)) ===
 						"unauthorized";
 				const confirmed = await this.confirmSignIn(
 					deployment.url,
@@ -422,20 +419,16 @@ export class LoginCoordinator implements vscode.Disposable {
 		return withLoginMethod("provided_token", result);
 	}
 
-	/** Stored session for the deployment's exact origin, if it still works. */
+	/** Stored session for the deployment's origin, if it still works. */
 	private async tryStoredSession(
 		ctx: LoginAttemptContext,
 	): Promise<LoginResult | undefined> {
-		const { client, isAutoLogin, sameOriginAuth } = ctx;
-		if (!sameOriginAuth?.token) {
+		const { client, isAutoLogin, auth } = ctx;
+		if (!auth?.token) {
 			return undefined;
 		}
 		this.logger.debug("Trying stored session token");
-		const result = await this.tryTokenAuth(
-			client,
-			sameOriginAuth.token,
-			isAutoLogin,
-		);
+		const result = await this.tryTokenAuth(client, auth.token, isAutoLogin);
 		if (result === "unauthorized") {
 			return undefined;
 		}
@@ -446,7 +439,7 @@ export class LoginCoordinator implements vscode.Disposable {
 	private async tryCliCredentials(
 		ctx: LoginAttemptContext,
 	): Promise<LoginResult | undefined> {
-		const { client, deployment, isAutoLogin, auth, sameOriginAuth } = ctx;
+		const { client, deployment, isAutoLogin, auth } = ctx;
 		const configs = vscode.workspace.getConfiguration();
 		if (
 			!(await this.cliCredentialManager.hasCliStore(deployment.url, configs))
@@ -482,8 +475,8 @@ export class LoginCoordinator implements vscode.Disposable {
 					title: "Sign in with the Coder CLI's session?",
 					detail: `The Coder CLI's session signs you in as "${result.user.username}"`,
 				},
-				// A same-origin session reached this point only because it failed.
-				{ username: auth.username, expired: sameOriginAuth !== undefined },
+				// The stored session reached this point only because it failed.
+				{ username: auth.username, expired: true },
 			);
 			if (!confirmed) {
 				return undefined;

src/storage/secretsManager.ts

@@ -1,6 +1,6 @@
 import { z } from "zod";
 
-import { isSameOrigin, toSafeHost } from "../common/url";
+import { toSafeHost } from "../common/url";
 import { isValidDeploymentUrl } from "../common/validation";
 import { DeploymentSchema, type Deployment } from "../deployment/types";
 
@@ -153,11 +153,15 @@ export class SecretsManager {
 			const deployment = result.success ? result.data.deployment : null;
 			// Treat an invalid URL stored by an older version as signed out, so the
 			// next sign-in replaces it.
-			if (deployment && !isValidDeploymentUrl(deployment.url)) {
+			if (!deployment) {
+				return null;
+			}
+			if (!isValidDeploymentUrl(deployment.url)) {
 				this.logger.warn("Ignoring current deployment with an invalid URL");
 				return null;
 			}
-			return deployment;
+			// Older versions stored the bare hostname for every origin.
+			return { ...deployment, safeHostname: toSafeHost(deployment.url) };
 		} catch {
 			return null;
 		}
@@ -208,6 +212,7 @@ export class SecretsManager {
 		});
 	}
 
+	/** By origin label; callers with a URL use `getSessionAuthForUrl`. */
 	public async getSessionAuth(
 		safeHostname: string,
 	): Promise<SessionAuth | undefined> {
@@ -240,8 +245,8 @@ export class SecretsManager {
 	}
 
 	/**
-	 * The stored session auth for `url`, only when recorded for the same
-	 * origin. Fails closed on an unparsable URL.
+	 * The stored session auth for `url`'s origin. Fails closed on an
+	 * unparsable URL.
 	 */
 	public async getSessionAuthForUrl(
 		url: string,
@@ -252,14 +257,7 @@ export class SecretsManager {
 		} catch {
 			return undefined;
 		}
-		const auth = await this.getSessionAuth(safeHostname);
-		if (auth && !isSameOrigin(auth.url, url)) {
-			this.logger.warn(
-				`Ignoring stored session auth for ${safeHostname}: recorded for ${new URL(auth.url).origin}, not ${new URL(url).origin}`,
-			);
-			return undefined;
-		}
-		return auth;
+		return this.getSessionAuth(safeHostname);
 	}
 
 	/**

test/unit/api/authInterceptor.test.ts

@@ -332,7 +332,7 @@ describe("AuthInterceptor", () => {
 		it("does not retry with a token stored for another origin", async () => {
 			const { secretsManager, axiosInstance, createInterceptor } =
 				createTestContext();
-			await secretsManager.setSessionAuth(TEST_HOSTNAME, {
+			await secretsManager.setSessionAuth(`${TEST_HOSTNAME}_8443`, {
 				url: `${TEST_URL}:8443`,
 				token: "other-origin-token",
 			});

test/unit/common/url.test.ts

@@ -24,24 +24,57 @@ describe("toSafeHost", () => {
 
 	it("preserves the IPv6 deployment key on POSIX but rejects it on Windows", () => {
 		vi.spyOn(os, "platform").mockReturnValue("linux");
-		expect(toSafeHost("https://[::1]:8080")).toBe("[::1]");
+		expect(toSafeHost("https://[::1]:8080")).toBe("[::1]_8080");
 		vi.spyOn(os, "platform").mockReturnValue("win32");
 		expect(() => toSafeHost("https://[::1]:8080")).toThrow(
 			/Cannot connect on Windows/,
 		);
 	});
 
 	it.each([
-		["https://foobar:8080", "foobar"],
+		["https://foobar", "foobar"],
+		["https://foobar:443", "foobar"],
+		["https://foobar:8080", "foobar_8080"],
+		["http://foobar", "foobar_http"],
+		["http://foobar:80", "foobar_http"],
+		["http://foobar:8080", "foobar_http8080"],
+		["https://foobar:80", "foobar_80"],
+		["http://foobar:443", "foobar_http443"],
+		["https://my_host", "my_host_https"],
+		["https://my_host:8443", "my_host_8443"],
 		["https://ほげ", "xn--18j4d"],
 		["https://عربي", "xn--ngbrx4e"],
 		["https://test.😉.invalid", "test.xn--n28h.invalid"],
 		["https://dev.😉-coder.com", "dev.xn---coder-vx74e.com"],
-		["http://ignore-port.com:8080", "ignore-port.com"],
 	])("returns %s for %s", (input, expected) => {
 		expect(toSafeHost(input)).toBe(expected);
 	});
 
+	it("gives two URLs the same label exactly when they share an origin", () => {
+		const hosts = ["host", "HOST", "host.com", "my_host", "xn--n3h.com"];
+		// Hostnames that look like another origin's label.
+		hosts.push("host_80", "host_http", "host_https", "host_http8080");
+		const urls = hosts.flatMap((host) =>
+			["http", "https"].flatMap((scheme) =>
+				["", ":80", ":443", ":8080"].map(
+					(port) => `${scheme}://${host}${port}`,
+				),
+			),
+		);
+
+		for (const a of urls) {
+			for (const b of urls) {
+				expect(toSafeHost(a) === toSafeHost(b), `${a} vs ${b}`).toBe(
+					new URL(a).origin === new URL(b).origin,
+				);
+			}
+		}
+	});
+
+	it("rejects schemes other than http and https", () => {
+		expect(() => toSafeHost("ftp://host")).toThrow("must use http or https");
+	});
+
 	it("throws for invalid URLs", () => {
 		expect(() => toSafeHost("invalid url")).toThrow("Invalid URL");
 	});