fix: enforce session auth hostname invariant

src/core/secretsManager.ts

@@ -62,6 +62,22 @@ export class SecretsManager {
 		return `${prefix}${safeHostname}`;
 	}
 
+	private assertSessionAuthHostname(safeHostname: string, url: string): void {
+		let authHostname: string;
+		try {
+			authHostname = toSafeHost(url);
+		} catch {
+			throw new Error(
+				`Session auth hostname mismatch: expected "${safeHostname}", got an invalid URL "${url}"`,
+			);
+		}
+		if (authHostname !== safeHostname) {
+			throw new Error(
+				`Session auth hostname mismatch: expected "${safeHostname}", got "${authHostname}"`,
+			);
+		}
+	}
+
 	private async getSecret<T>(
 		prefix: SecretKeyPrefix,
 		safeHostname: string,
@@ -189,15 +205,31 @@ export class SecretsManager {
 			return undefined;
 		}
 		const result = SessionAuthSchema.safeParse(data);
-		return result.success ? result.data : undefined;
+		if (!result.success) {
+			return undefined;
+		}
+		try {
+			this.assertSessionAuthHostname(safeHostname, result.data.url);
+		} catch (error) {
+			this.logger.warn("Ignoring stored session auth:", error);
+			return undefined;
+		}
+		return result.data;
 	}
 
+	/**
+	 * Store session auth for a deployment.
+	 *
+	 * @throws If the auth URL is invalid or its hostname does not match the
+	 * deployment.
+	 */
 	public async setSessionAuth(
 		safeHostname: string,
 		auth: SessionAuth,
 	): Promise<void> {
 		// Parse through schema to strip any extra fields
 		const state = SessionAuthSchema.parse(auth);
+		this.assertSessionAuthHostname(safeHostname, state.url);
 		await this.setSecret(SESSION_KEY_PREFIX, safeHostname, state);
 	}
 

src/remote/remote.ts

@@ -823,10 +823,14 @@ export class Remote {
 
 		if (url.status === "fulfilled" && token.status === "fulfilled") {
 			this.logger.info("Migrating session auth from files for", safeHostname);
-			await this.secretsManager.setSessionAuth(safeHostname, {
-				url: url.value.trim(),
-				token: token.value.trim(),
-			});
+			try {
+				await this.secretsManager.setSessionAuth(safeHostname, {
+					url: url.value.trim(),
+					token: token.value.trim(),
+				});
+			} catch (error) {
+				this.logger.warn("Failed to migrate session auth from files:", error);
+			}
 		}
 	}
 

test/mocks/testHelpers.ts

@@ -487,6 +487,51 @@ export function createMockLogger(): Logger {
 	};
 }
 
+export interface LogEntry {
+	level: "trace" | "debug" | "info" | "warn" | "error";
+	message: string;
+	args: readonly unknown[];
+}
+
+/** Logger that records structured entries for tests of logging behavior. */
+export class LogCollector implements Logger {
+	private readonly _entries: LogEntry[] = [];
+
+	get entries(): readonly LogEntry[] {
+		return this._entries;
+	}
+
+	trace(message: string, ...args: unknown[]): void {
+		this.collect("trace", message, args);
+	}
+
+	debug(message: string, ...args: unknown[]): void {
+		this.collect("debug", message, args);
+	}
+
+	info(message: string, ...args: unknown[]): void {
+		this.collect("info", message, args);
+	}
+
+	warn(message: string, ...args: unknown[]): void {
+		this.collect("warn", message, args);
+	}
+
+	error(message: string, ...args: unknown[]): void {
+		this.collect("error", message, args);
+	}
+
+	show(): void {}
+
+	private collect(
+		level: LogEntry["level"],
+		message: string,
+		args: readonly unknown[],
+	): void {
+		this._entries.push({ level, message, args });
+	}
+}
+
 /** Resolve once pending microtasks and the macrotask queue have drained. */
 export async function flush(): Promise<void> {
 	await new Promise((resolve) => setImmediate(resolve));

test/unit/core/secretsManager.test.ts

@@ -9,25 +9,24 @@ import {
 import {
 	InMemoryMemento,
 	InMemorySecretStorage,
+	LogCollector,
 	createMockLogger,
 } from "../../mocks/testHelpers";
 
 describe("SecretsManager", () => {
 	let secretStorage: InMemorySecretStorage;
 	let memento: InMemoryMemento;
 	let mementoManager: MementoManager;
+	let logger: ReturnType<typeof createMockLogger>;
 	let secretsManager: SecretsManager;
 
 	beforeEach(() => {
 		vi.useRealTimers();
 		secretStorage = new InMemorySecretStorage();
 		memento = new InMemoryMemento();
 		mementoManager = new MementoManager(memento);
-		secretsManager = new SecretsManager(
-			secretStorage,
-			mementoManager,
-			createMockLogger(),
-		);
+		logger = createMockLogger();
+		secretsManager = new SecretsManager(secretStorage, mementoManager, logger);
 	});
 
 	describe("session auth", () => {
@@ -48,6 +47,102 @@ describe("SecretsManager", () => {
 			expect(newAuth?.token).toBe("new-token");
 		});
 
+		it("should accept a URL port for a matching hostname", async () => {
+			await secretsManager.setSessionAuth("example.com", {
+				url: "https://example.com:8443",
+				token: "test-token",
+			});
+
+			expect(await secretsManager.getSessionAuth("example.com")).toEqual({
+				url: "https://example.com:8443",
+				token: "test-token",
+			});
+		});
+
+		it.each([
+			{
+				name: "malformed URL",
+				url: "not a URL",
+				error:
+					'Session auth hostname mismatch: expected "example.com", got an invalid URL "not a URL"',
+			},
+			{
+				name: "mismatched hostname",
+				url: "https://other.example.com",
+				error:
+					'Session auth hostname mismatch: expected "example.com", got "other.example.com"',
+			},
+		])("should reject a write with a $name", async ({ url, error }) => {
+			const existingAuth = {
+				url: "https://example.com",
+				token: "existing-token",
+			};
+			await secretsManager.setSessionAuth("example.com", existingAuth);
+
+			await expect(
+				secretsManager.setSessionAuth("example.com", {
+					url,
+					token: "secret-token",
+				}),
+			).rejects.toThrow(error);
+
+			expect(await secretsManager.getSessionAuth("example.com")).toEqual(
+				existingAuth,
+			);
+		});
+
+		it.each([
+			{ name: "malformed URL", url: "not a URL" },
+			{
+				name: "mismatched hostname",
+				url: "https://other.example.com/private?token=secret",
+			},
+		])("should ignore stored auth with a $name", async ({ url }) => {
+			await secretStorage.store(
+				"coder.session.example.com",
+				JSON.stringify({ url, token: "secret-token" }),
+			);
+
+			expect(
+				await secretsManager.getSessionAuth("example.com"),
+			).toBeUndefined();
+		});
+
+		describe("logging", () => {
+			it.each([
+				{
+					name: "malformed URL",
+					url: "not a URL",
+					error:
+						'Session auth hostname mismatch: expected "example.com", got an invalid URL "not a URL"',
+				},
+				{
+					// A mismatched URL can carry credentials, so only its hostname is logged.
+					name: "mismatched hostname",
+					url: "https://other.example.com/private?token=secret",
+					error:
+						'Session auth hostname mismatch: expected "example.com", got "other.example.com"',
+				},
+			])("logs why a $name was ignored", async ({ url, error }) => {
+				const logs = new LogCollector();
+				const manager = new SecretsManager(secretStorage, mementoManager, logs);
+				await secretStorage.store(
+					"coder.session.example.com",
+					JSON.stringify({ url, token: "secret-token" }),
+				);
+
+				await manager.getSessionAuth("example.com");
+
+				expect(logs.entries).toEqual([
+					{
+						level: "warn",
+						message: "Ignoring stored session auth:",
+						args: [new Error(error)],
+					},
+				]);
+			});
+		});
+
 		it("should clear session auth", async () => {
 			await secretsManager.setSessionAuth("example.com", {
 				url: "https://example.com",
@@ -85,15 +180,15 @@ describe("SecretsManager", () => {
 				"example.com",
 			);
 
-			await secretsManager.setSessionAuth("other-com", {
+			await secretsManager.setSessionAuth("other.com", {
 				url: "https://other.com",
 				token: "other-token",
 			});
 			expect(await secretsManager.getKnownSafeHostnames()).toContain(
 				"example.com",
 			);
 			expect(await secretsManager.getKnownSafeHostnames()).toContain(
-				"other-com",
+				"other.com",
 			);
 		});
 
@@ -327,9 +422,9 @@ describe("SecretsManager", () => {
 					extraField: "should be stripped",
 				};
 
-				await secretsManager.setSessionAuth("example.com", authWithExtra);
+				await secretsManager.setSessionAuth("coder.example.com", authWithExtra);
 
-				const raw = await secretStorage.get("coder.session.example.com");
+				const raw = await secretStorage.get("coder.session.coder.example.com");
 				expect(JSON.parse(raw!)).toEqual({
 					url: "https://coder.example.com",
 					token: "test-token",
@@ -347,9 +442,9 @@ describe("SecretsManager", () => {
 					},
 				};
 
-				await secretsManager.setSessionAuth("example.com", authWithExtra);
+				await secretsManager.setSessionAuth("coder.example.com", authWithExtra);
 
-				const raw = await secretStorage.get("coder.session.example.com");
+				const raw = await secretStorage.get("coder.session.coder.example.com");
 				expect(JSON.parse(raw!)).toEqual({
 					url: "https://coder.example.com",
 					token: "test-token",
@@ -419,21 +514,18 @@ describe("SecretsManager", () => {
 		describe("backwards compatibility", () => {
 			interface BackwardsCompatTestCase {
 				name: string;
-				key: string;
 				data: Record<string, unknown>;
 				expected: unknown;
 			}
 
 			const sessionAuthCases: BackwardsCompatTestCase[] = [
 				{
 					name: "without optional oauth field",
-					key: "coder.session.example.com",
 					data: { url: "https://coder.example.com", token: "test-token" },
 					expected: { url: "https://coder.example.com", token: "test-token" },
 				},
 				{
 					name: "with OAuth without optional fields",
-					key: "coder.session.example.com",
 					data: {
 						url: "https://coder.example.com",
 						token: "test-token",
@@ -449,9 +541,13 @@ describe("SecretsManager", () => {
 
 			it.each(sessionAuthCases)(
 				"handles SessionAuth $name",
-				async ({ key, data, expected }) => {
-					await secretStorage.store(key, JSON.stringify(data));
-					const result = await secretsManager.getSessionAuth("example.com");
+				async ({ data, expected }) => {
+					await secretStorage.store(
+						"coder.session.coder.example.com",
+						JSON.stringify(data),
+					);
+					const result =
+						await secretsManager.getSessionAuth("coder.example.com");
 					expect(result).toEqual(expected);
 				},
 			);

test/unit/oauth/sessionManager.test.ts

@@ -213,22 +213,20 @@ describe("OAuthSessionManager", () => {
 	});
 
 	describe("getStoredTokens validation", () => {
-		it("returns undefined when URL mismatches", async () => {
+		it("returns undefined when the URL differs on the same hostname", async () => {
 			const { secretsManager, manager } = createTestContext();
 
-			// Manually set auth with different URL (can't use helper)
 			await secretsManager.setSessionAuth(TEST_HOSTNAME, {
-				url: "https://different-coder.example.com",
+				url: `${TEST_URL}:8443`,
 				token: "access-token",
 				oauth: {
 					refresh_token: "refresh-token",
 					expiry_timestamp: Date.now() + ONE_HOUR_MS,
-					scope: "",
+					scope: DEFAULT_OAUTH_SCOPES,
 				},
 			});
 
-			const result = await manager.isLoggedInWithOAuth();
-			expect(result).toBe(false);
+			expect(await manager.isLoggedInWithOAuth()).toBe(false);
 		});
 	});