security: harden loopback OAuth callback and require token for API listeners

docs/releases/unreleased.md

@@ -42,3 +42,7 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l
 - (#2147) Added context-menu actions for recording task completion today, on the scheduled date, on the due date, or on a chosen date. The actions can be grouped in a submenu from Appearance settings. See [Completing Tasks](https://tasknotes.dev/features/task-management/#completing-tasks).
   - Rescheduling a recurring task can reactivate affected completed or skipped instances after confirmation. See [Recurring Tasks](https://tasknotes.dev/features/recurring-tasks/).
   - Thanks to @renatomen for the contribution.
+
+## Security
+
+- Require an API token before starting local API/MCP listeners, and validate one-use OAuth callbacks on an OS-assigned loopback port without reflecting callback text into HTML.

src/api/httpTypes.ts

@@ -16,6 +16,7 @@ export interface HTTPResponseLike {
 }
 
 export interface HTTPServerLike {
+	address?(): { port: number } | string | null;
 	listening?: boolean;
 	listen(port: number, callback?: () => void): void;
 	listen(port: number, hostname: string, callback?: () => void): void;

src/services/HTTPAPIService.ts

@@ -188,9 +188,9 @@ export class HTTPAPIService implements IWebhookNotifier {
 	private authenticate(req: HTTPRequestLike): boolean {
 		const authToken = this.plugin.settings.apiAuthToken;
 
-		// Skip auth if no token is configured
+		// A missing credential never opens an unauthenticated listener.
 		if (!authToken) {
-			return true;
+			return false;
 		}
 
 		const authHeader = req.headers.authorization;
@@ -292,6 +292,17 @@ export class HTTPAPIService implements IWebhookNotifier {
 	}
 
 	async start(): Promise<void> {
+		if (!Platform.isDesktop || !Platform.isDesktopApp)
+			throw new Error("The HTTP API is only available in the desktop app.");
+		if (!this.plugin.settings.apiAuthToken) {
+			this.plugin.settings.apiAuthToken = btoa(
+				String.fromCharCode(...crypto.getRandomValues(new Uint8Array(32)))
+			)
+				.replace(/\+/g, "-")
+				.replace(/\//g, "_")
+				.replace(/=+$/g, "");
+			await this.plugin.saveSettings();
+		}
 		return new Promise((resolve, reject) => {
 			if (!Platform.isDesktop || !Platform.isDesktopApp) {
 				reject(new Error("The HTTP API is only available in the desktop app."));

src/services/OAuthService.ts

@@ -157,12 +157,7 @@ export class OAuthService {
 			const codeChallenge = await this.generateCodeChallenge(codeVerifier);
 			const state = this.generateState();
 
-			// Find available port
-			const port = await this.findAvailablePort(
-				OAUTH_CONSTANTS.CALLBACK_PORT_START,
-				OAUTH_CONSTANTS.CALLBACK_PORT_END
-			);
-			await this.startCallbackServer(port);
+			const port = await this.startCallbackServer(0);
 
 			// Update redirect URI for this session
 			const originalRedirectUri = config.redirectUri;
@@ -185,11 +180,11 @@ export class OAuthService {
 					`Opening browser for ${provider} authorization...`
 				);
 
-				// Open browser to authorization URL
+				// Register the resolver before the browser can return a fast callback.
+				const callback = this.waitForCallback(state, 300000);
+				void callback.catch(() => undefined);
 				await this.openAuthorizationUrl(authUrl);
-
-				// Wait for callback with timeout
-				const code = await this.waitForCallback(state, 300000); // 5 minute timeout
+				const code = await callback;
 
 				// Exchange code for tokens
 				const tokens = await this.exchangeCodeForTokens(config, code, codeVerifier);
@@ -202,6 +197,10 @@ export class OAuthService {
 					`Successfully connected to ${provider} Calendar!`
 				);
 			} finally {
+				// Clear a pending resolver even if opening the browser failed.
+				const pending = this.pendingOAuthState.get(state);
+				this.pendingOAuthState.delete(state);
+				pending?.reject(new Error("OAuth flow ended before authorization"));
 				// Restore original redirect URI
 				config.redirectUri = originalRedirectUri;
 			}
@@ -231,43 +230,19 @@ export class OAuthService {
 				return;
 			}
 		} catch (error) {
-			tasknotesLogger.warn("Failed to open OAuth URL in system browser; falling back to window.open.", {
-				category: "provider",
-				operation: "oauth-open-external",
-				error,
-			});
+			tasknotesLogger.warn(
+				"Failed to open OAuth URL in system browser; falling back to window.open.",
+				{
+					category: "provider",
+					operation: "oauth-open-external",
+					error,
+				}
+			);
 		}
 
 		window.open(authUrl, "_blank");
 	}
 
-	/**
-	 * Finds an available port in the given range
-	 */
-	private async findAvailablePort(startPort: number, endPort: number): Promise<number> {
-		const http = ensureHttpModule();
-
-		for (let port = startPort; port <= endPort; port++) {
-			try {
-				await new Promise<void>((resolve, reject) => {
-					const server = http.createServer();
-					server.once("error", reject);
-					server.once("listening", () => {
-						server.close();
-						resolve();
-					});
-					server.listen(port, "127.0.0.1");
-				});
-				return port;
-			} catch {
-				// Port in use, try next one
-				continue;
-			}
-		}
-
-		throw new Error(`No available ports found between ${startPort} and ${endPort}`);
-	}
-
 	/**
 	 * Generates a random code verifier for PKCE
 	 */
@@ -330,10 +305,10 @@ export class OAuthService {
 	/**
 	 * Starts a temporary HTTP server to receive the OAuth callback
 	 */
-	private async startCallbackServer(port: number): Promise<void> {
+	private async startCallbackServer(port: number): Promise<number> {
 		return new Promise((resolve, reject) => {
 			if (this.callbackServer) {
-				resolve(); // Already running
+				reject(new Error("An OAuth callback is already pending"));
 				return;
 			}
 
@@ -363,7 +338,12 @@ export class OAuthService {
 			});
 
 			this.callbackServer.listen(port, "127.0.0.1", () => {
-				resolve();
+				const address = this.callbackServer?.address?.();
+				if (!address || typeof address === "string") {
+					reject(new Error("OAuth listener did not expose its bound port"));
+					return;
+				}
+				resolve(address.port);
 			});
 		});
 	}
@@ -389,70 +369,48 @@ export class OAuthService {
 	 * Handles incoming HTTP requests to the callback server
 	 */
 	private handleCallback(req: HTTPRequestLike, res: HTTPResponseLike): void {
-		const hostHeader = req.headers.host;
-		const host = Array.isArray(hostHeader) ? hostHeader[0] : (hostHeader ?? "localhost");
-		const url = new URL(req.url || "", `http://${host}`);
-		const code = url.searchParams.get("code");
-		const state = url.searchParams.get("state");
-		const error = url.searchParams.get("error");
-
-		// Send response to browser
-		res.writeHead(200, { "Content-Type": "text/html" });
-
-		if (error) {
-			res.end(`
-				<!DOCTYPE html>
-				<html>
-					<head><title>OAuth Error</title></head>
-					<body>
-						<h1>Authorization Failed</h1>
-						<p>Error: ${error}</p>
-						<p>You can close this window.</p>
-					</body>
-				</html>
-			`);
-
-			const pending = state ? this.pendingOAuthState.get(state) : null;
-			if (pending && state) {
-				pending.reject(new Error(`OAuth error: ${error}`));
-				this.pendingOAuthState.delete(state);
-			}
+		const headers = {
+			"Content-Type": "text/html; charset=utf-8",
+			"Content-Security-Policy":
+				"default-src 'none'; frame-ancestors 'none'; base-uri 'none'",
+			"X-Content-Type-Options": "nosniff",
+			"Cache-Control": "no-store",
+		};
+		let url: URL;
+		try {
+			url = new URL(req.url || "/", "http://127.0.0.1");
+		} catch {
+			res.writeHead(400, headers);
+			res.end("<!doctype html><title>Invalid callback</title>");
 			return;
 		}
-
-		if (!code || !state) {
-			res.end(`
-				<!DOCTYPE html>
-				<html>
-					<head><title>OAuth Error</title></head>
-					<body>
-						<h1>Invalid Callback</h1>
-						<p>Missing required parameters.</p>
-						<p>You can close this window.</p>
-					</body>
-				</html>
-			`);
+		const state = url.searchParams.get("state");
+		const pending = state ? this.pendingOAuthState.get(state) : undefined;
+		const code = url.searchParams.get("code");
+		const error = url.searchParams.get("error");
+		if (
+			req.method !== "GET" ||
+			url.pathname !== "/" ||
+			!state ||
+			!pending ||
+			(!code && !error)
+		) {
+			res.writeHead(400, headers);
+			res.end(
+				"<!doctype html><title>Invalid callback</title><p>No pending authorization matches this callback.</p>"
+			);
 			return;
 		}
-
-		res.end(`
-			<!DOCTYPE html>
-			<html>
-				<head><title>OAuth Success</title></head>
-				<body>
-					<h1>Authorization Successful!</h1>
-					<p>You can close this window and return to Obsidian.</p>
-					<script>window.close();</script>
-				</body>
-			</html>
-		`);
-
-		// Resolve the pending promise
-		const pending = this.pendingOAuthState.get(state);
-		if (pending) {
-			pending.resolve(code);
-			this.pendingOAuthState.delete(state);
-		}
+		// Consume before responding: a replay cannot resolve the pending flow.
+		this.pendingOAuthState.delete(state);
+		res.writeHead(error ? 400 : 200, headers);
+		res.end(
+			error
+				? "<!doctype html><title>Authorization failed</title><p>Authorization was not completed. Return to Obsidian.</p>"
+				: "<!doctype html><title>Authorization complete</title><p>You can close this window and return to Obsidian.</p>"
+		);
+		if (error) pending.reject(new Error("OAuth authorization failed"));
+		else pending.resolve(code as string);
 	}
 
 	/**

tests/services/OAuthService.callback.test.ts

@@ -0,0 +1,35 @@
+import { OAuthService } from "../../src/services/OAuthService";
+import { OAuthSecretStore } from "../../src/services/OAuthSecretStore";
+
+describe("OAuth callback validation", () => {
+	it("validates and consumes OAuth state before writing a static response", () => {
+		const store = new OAuthSecretStore({ getSecret: () => null, setSecret: () => {} });
+		const service: any = new OAuthService({} as any, store);
+		const response: any = { writeHead: jest.fn(), end: jest.fn() };
+		service.handleCallback(
+			{ method: "GET", url: "/?error=%3Cscript%3E&state=unknown", headers: {} },
+			response
+		);
+		expect(response.writeHead.mock.calls[0][0]).toBe(400);
+		expect(response.end.mock.calls[0][0]).not.toContain("<script>");
+		const resolve = jest.fn();
+		service.pendingOAuthState.set("known", { resolve, reject: jest.fn() });
+		response.writeHead = jest.fn(() =>
+			expect(service.pendingOAuthState.has("known")).toBe(false)
+		);
+		service.handleCallback(
+			{ method: "GET", url: "/?code=fixture-code&state=known", headers: {} },
+			response
+		);
+		expect(resolve).toHaveBeenCalledWith("fixture-code");
+		expect(response.writeHead.mock.calls[0][1]["Content-Security-Policy"]).toContain(
+			"frame-ancestors 'none'"
+		);
+		expect(response.writeHead.mock.calls[0][1]["Cache-Control"]).toBe("no-store");
+		service.handleCallback(
+			{ method: "GET", url: "/?code=replay&state=known", headers: {} },
+			response
+		);
+		expect(resolve).toHaveBeenCalledTimes(1);
+	});
+});

tests/unit/issues/issue-1923-http-api-loopback-cors.test.ts

@@ -48,7 +48,7 @@ function createPlugin(): TaskNotesPlugin {
 			...DEFAULT_SETTINGS,
 			enableAPI: true,
 			apiPort: 9191,
-			apiAuthToken: "",
+			apiAuthToken: "test-auth-token",
 		},
 		app: {
 			vault: {
@@ -83,7 +83,7 @@ function createRequest(origin?: string): HTTPRequestLike {
 	return {
 		method: "GET",
 		url: "/api/health",
-		headers: origin ? { origin } : {},
+		headers: { ...(origin ? { origin } : {}), authorization: "Bearer test-auth-token" },
 		on: jest.fn(),
 	};
 }
@@ -193,4 +193,15 @@ describe("Issue #1923: HTTP API loopback binding and CORS", () => {
 		expect(resolveLocalCORSOrigin("http://192.168.1.20:5173", "http://127.0.0.1:9191"))
 			.toBeUndefined();
 	});
+
+	it("does not enable unauthenticated HTTP API access when the token is empty", () => {
+		const service: any = Object.create(HTTPAPIService.prototype);
+		service.plugin = { settings: { apiAuthToken: "" } };
+		expect(service.authenticate({ headers: {} })).toBe(false);
+		service.plugin.settings.apiAuthToken = "fixture-token";
+		expect(service.authenticate({ headers: { authorization: "Bearer wrong" } })).toBe(false);
+		expect(service.authenticate({ headers: { authorization: "Bearer fixture-token" } })).toBe(
+			true
+		);
+	});
 });