Desktop: Restrict Web Clipper pairing to extension origins (#16276)

.gitignore

@@ -1324,6 +1324,7 @@ packages/lib/BaseModel.test.js
 packages/lib/BaseModel.js
 packages/lib/BaseSyncTarget.js
 packages/lib/Cache.js
+packages/lib/ClipperServer.test.js
 packages/lib/ClipperServer.js
 packages/lib/CssUtils.js
 packages/lib/EventDispatcher.test.js

.ignore.eslint

@@ -1349,6 +1349,7 @@ packages/lib/BaseModel.test.js
 packages/lib/BaseModel.js
 packages/lib/BaseSyncTarget.js
 packages/lib/Cache.js
+packages/lib/ClipperServer.test.js
 packages/lib/ClipperServer.js
 packages/lib/CssUtils.js
 packages/lib/EventDispatcher.test.js

packages/lib/ClipperServer.test.ts

@@ -0,0 +1,41 @@
+import { isAllowedPairingOrigin, isPairingPath } from './ClipperServer';
+
+describe('ClipperServer', () => {
+
+	test.each([
+		// The router strips all leading slashes, so the gate must too, otherwise
+		// e.g. //auth would skip the check but still reach the auth handler.
+		['/auth', true],
+		['/auth/check', true],
+		['//auth', true],
+		['///auth/check', true],
+		['auth', true],
+		['/notes', false],
+		['/auth_other', false],
+		['', false],
+	])('should identify pairing path %s -> %s', (pathname, expected) => {
+		expect(isPairingPath(pathname)).toBe(expected);
+	});
+
+	test.each([
+		// Web origins must be rejected: these are the CSRF vector.
+		['http://attacker.example', false],
+		['https://attacker.example', false],
+		['http://127.0.0.1:41184', false],
+		['https://localhost', false],
+
+		// Extension origins are the legitimate caller.
+		['chrome-extension://some-extension-id', true],
+		['moz-extension://11111111-2222-3333-4444-555555555555', true],
+		['safari-web-extension://ABCDEF', true],
+
+		// No Origin (native clients, curl) is not a browser CSRF vector.
+		['', true],
+		[undefined as unknown as string, true],
+
+		// Malformed origins are rejected.
+		['not a url', false],
+	])('should decide pairing origin %s -> %s', (origin, expected) => {
+		expect(isAllowedPairingOrigin(origin)).toBe(expected);
+	});
+});

packages/lib/ClipperServer.ts

@@ -2,6 +2,7 @@ import Setting from './models/Setting';
 import Logger from '@joplin/utils/Logger';
 import Api, { RequestFile } from './services/rest/Api';
 import ApiResponse from './services/rest/ApiResponse';
+import { ltrimSlashes } from './path-utils';
 import * as urlParser from 'url';
 const { randomClipperPort, startPort } = require('./randomClipperPort');
 const enableServerDestroy = require('server-destroy');
@@ -15,6 +16,30 @@ export enum StartState {
 
 type ClipperDispatch = (action: { type: string; [key: string]: unknown })=> void;
 
+// The pairing endpoints hand out the permanent API token without requiring one,
+// so they must not be callable from a website. The extension calls them from an
+// extension origin; a web page carries an http(s) origin. No-Origin requests
+// (native clients) aren't a browser CSRF vector, so they're allowed.
+//
+// Stored without leading slashes and compared via ltrimSlashes, matching how the
+// router normalises paths - otherwise //auth would skip this check but still
+// reach the auth handler.
+const pairingPaths = ['auth', 'auth/check'];
+
+export const isPairingPath = (pathname: string) => {
+	return pairingPaths.includes(ltrimSlashes(pathname || ''));
+};
+
+export const isAllowedPairingOrigin = (origin: string) => {
+	if (!origin) return true;
+	try {
+		const protocol = new URL(origin).protocol;
+		return protocol !== 'http:' && protocol !== 'https:';
+	} catch (error) {
+		return false;
+	}
+};
+
 export default class ClipperServer {
 
 	private logger_: Logger;
@@ -191,6 +216,12 @@ export default class ClipperServer {
 
 			const url = urlParser.parse(request.url, true);
 
+			if (request.method !== 'OPTIONS' && isPairingPath(url.pathname) && !isAllowedPairingOrigin(request.headers.origin)) {
+				this.logger().warn(`Rejected pairing request from disallowed origin: ${request.headers.origin}`);
+				writeResponse(403, { error: 'This endpoint cannot be called from a web page' });
+				return;
+			}
+
 			const execRequest = async (request: import('http').IncomingMessage, body = '', files: RequestFile[] = []) => {
 				try {
 					const response = await this.api_.route(request.method as Parameters<Api['route']>[0], url.pathname, url.query, body, files);