diff --git a/src/test/suite/views/data-browsing-app/messageHandler.test.tsx b/src/test/suite/views/data-browsing-app/messageHandler.test.tsx index 5e5bc6035..6fd50106e 100644 --- a/src/test/suite/views/data-browsing-app/messageHandler.test.tsx +++ b/src/test/suite/views/data-browsing-app/messageHandler.test.tsx @@ -1,8 +1,12 @@ import { expect } from 'chai'; import sinon from 'sinon'; -import { PreviewMessageType } from '../../../../views/data-browsing-app/extension-app-message-constants'; +import { + PreviewMessageType, + isMessageFromExtension, +} from '../../../../views/data-browsing-app/extension-app-message-constants'; import { createStore } from '../../../../views/data-browsing-app/store'; +import type { AppDispatch } from '../../../../views/data-browsing-app/store'; import { handleExtensionMessage, setupMessageHandler, @@ -69,16 +73,15 @@ describe('messageHandler test suite', function () { ); }); - it('should handle undefined documents', function () { - const store = createStore(); - handleExtensionMessage(store.dispatch, { - command: PreviewMessageType.loadPage, - isInitialLoad: true, - } as any); - - expect(store.getState().documentQuery.displayedDocuments).to.deep.equal( - [], - ); + it('should reject a loadPage with undefined documents at the boundary', function () { + // handleExtensionMessage trusts its input; the listener is what keeps + // a payload like this from reaching it. + expect( + isMessageFromExtension({ + command: PreviewMessageType.loadPage, + isInitialLoad: true, + }), + ).to.be.false; }); it('should clear getDocuments error on successful load', function () { @@ -327,5 +330,121 @@ describe('messageHandler test suite', function () { sinon.match.func, ); }); + + describe('untrusted messages', function () { + // Drive the registered listener itself with events a real window sends. + const getListener = (dispatch: AppDispatch): EventListener => { + const addEventListenerSpy = sinon.spy(window, 'addEventListener'); + setupMessageHandler(dispatch); + return addEventListenerSpy.firstCall.args[1] as EventListener; + }; + + const fromHost = (data: unknown): MessageEvent => + new MessageEvent('message', { data, origin: window.location.origin }); + + it('dispatches a well-formed message from the host frame', function () { + const store = createStore(); + const listener = getListener(store.dispatch); + + listener( + fromHost({ + command: PreviewMessageType.loadPage, + documents: [{ _id: '1' }], + }), + ); + + expect(store.getState().documentQuery.displayedDocuments).to.deep.equal( + [{ _id: '1' }], + ); + }); + + it('ignores a message from another origin', function () { + const store = createStore(); + const listener = getListener(store.dispatch); + + listener( + new MessageEvent('message', { + data: { + command: PreviewMessageType.loadPage, + documents: [{ _id: 'injected' }], + }, + origin: 'vscode-webview://some-other-panel', + }), + ); + + expect(store.getState().documentQuery.displayedDocuments).to.deep.equal( + [], + ); + }); + + it('ignores a message with no origin', function () { + const store = createStore(); + const listener = getListener(store.dispatch); + + listener( + new MessageEvent('message', { + data: { + command: PreviewMessageType.loadPage, + documents: [{ _id: 'injected' }], + }, + }), + ); + + expect(store.getState().documentQuery.displayedDocuments).to.deep.equal( + [], + ); + }); + + it('ignores a loadPage whose documents are not an array', function () { + const store = createStore(); + const listener = getListener(store.dispatch); + + listener( + fromHost({ + command: PreviewMessageType.loadPage, + documents: 'not-an-array', + }), + ); + + expect(store.getState().documentQuery.isLoading).to.be.true; + }); + + it('ignores an updateTotalCount whose count is not a number', function () { + const store = createStore(); + const listener = getListener(store.dispatch); + + listener( + fromHost({ + command: PreviewMessageType.updateTotalCount, + totalCount: '100', + }), + ); + + expect(store.getState().documentQuery.hasReceivedCount).to.be.false; + }); + + it('ignores an unknown command', function () { + const store = createStore(); + const listener = getListener(store.dispatch); + + listener(fromHost({ command: 'NOT_A_REAL_COMMAND' })); + + expect(store.getState().documentQuery.displayedDocuments).to.deep.equal( + [], + ); + }); + + it('ignores a non-object payload', function () { + const store = createStore(); + const listener = getListener(store.dispatch); + + listener(fromHost('LOAD_PAGE')); + listener(fromHost(null)); + + expect(store.getState().documentQuery.displayedDocuments).to.deep.equal( + [], + ); + }); + }); }); }); diff --git a/src/test/suite/views/data-browsing-app/preview-page.test.tsx b/src/test/suite/views/data-browsing-app/preview-page.test.tsx index 6d57ffb05..9390376bf 100644 --- a/src/test/suite/views/data-browsing-app/preview-page.test.tsx +++ b/src/test/suite/views/data-browsing-app/preview-page.test.tsx @@ -113,6 +113,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [{ _id: '123', name: 'Test' }], @@ -172,6 +173,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.requestCancelled, }, @@ -193,6 +195,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [{ _id: '123', name: 'Test' }], @@ -211,6 +214,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [], @@ -235,6 +239,7 @@ describe('PreviewApp test suite', function () { // Send loadPage message window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [{ _id: '1', name: 'Doc1' }], @@ -244,6 +249,7 @@ describe('PreviewApp test suite', function () { // Send updateTotalCount message (now sent separately) window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.updateTotalCount, totalCount: 1, @@ -301,6 +307,7 @@ describe('PreviewApp test suite', function () { // Send loadPage message window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents, @@ -310,6 +317,7 @@ describe('PreviewApp test suite', function () { // Send updateTotalCount message (now sent separately) window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.updateTotalCount, totalCount, @@ -393,6 +401,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents, @@ -401,6 +410,7 @@ describe('PreviewApp test suite', function () { ); window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.updateTotalCount, totalCount: null, @@ -427,6 +437,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents, @@ -435,6 +446,7 @@ describe('PreviewApp test suite', function () { ); window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.updateTotalCountError, error: errorMessage, @@ -486,6 +498,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [{ _id: '123', name: 'TestDocument', value: 42 }], @@ -509,6 +522,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [ @@ -542,6 +556,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [testDocument], @@ -581,6 +596,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [{ _id: '1', name: 'Doc1' }], @@ -789,6 +805,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [{ _id: '1', name: 'FirstPage' }], @@ -805,6 +822,7 @@ describe('PreviewApp test suite', function () { act(() => { window.dispatchEvent( new MessageEvent('message', { + origin: window.location.origin, data: { command: PreviewMessageType.loadPage, documents: [{ _id: '11', name: 'SecondPage' }], diff --git a/src/views/data-browsing-app/extension-app-message-constants.ts b/src/views/data-browsing-app/extension-app-message-constants.ts index e19443b1b..98850de7c 100644 --- a/src/views/data-browsing-app/extension-app-message-constants.ts +++ b/src/views/data-browsing-app/extension-app-message-constants.ts @@ -159,3 +159,42 @@ export type MessageFromExtensionToWebview = | UpdateTotalCountErrorMessage | UpdateThemeColorsMessage | DocumentDeletedMessage; + +const isOptionalString = (value: unknown): boolean => + value === undefined || typeof value === 'string'; + +/** + * Checks a payload against the command it claims to be, so the webview never + * acts on a message it cannot attribute. Unknown commands are rejected too. + */ +export function isMessageFromExtension( + data: unknown, +): data is MessageFromExtensionToWebview { + if (data === null || typeof data !== 'object') { + return false; + } + + const message = data as Record; + + switch (message.command) { + case PreviewMessageType.loadPage: + return Array.isArray(message.documents); + case PreviewMessageType.getDocumentError: + case PreviewMessageType.updateTotalCountError: + return isOptionalString(message.error); + case PreviewMessageType.requestCancelled: + case PreviewMessageType.documentDeleted: + return true; + case PreviewMessageType.updateTotalCount: + return ( + message.totalCount === null || typeof message.totalCount === 'number' + ); + case PreviewMessageType.updateThemeColors: + return ( + typeof message.themeColors === 'object' && + typeof message.themeKind === 'string' + ); + default: + return false; + } +} diff --git a/src/views/data-browsing-app/store/messageHandler.ts b/src/views/data-browsing-app/store/messageHandler.ts index a0f61a2d0..4d2adc800 100644 --- a/src/views/data-browsing-app/store/messageHandler.ts +++ b/src/views/data-browsing-app/store/messageHandler.ts @@ -1,5 +1,8 @@ import type { MessageFromExtensionToWebview } from '../extension-app-message-constants'; -import { PreviewMessageType } from '../extension-app-message-constants'; +import { + PreviewMessageType, + isMessageFromExtension, +} from '../extension-app-message-constants'; import type { AppDispatch } from './index'; import type { PreviewDocument } from './documentQuerySlice'; import { @@ -18,11 +21,7 @@ export const handleExtensionMessage = ( ): void => { switch (message.command) { case PreviewMessageType.loadPage: - dispatch( - documentsReceived( - message.documents ? (message.documents as PreviewDocument[]) : [], - ), - ); + dispatch(documentsReceived(message.documents as PreviewDocument[])); break; case PreviewMessageType.getDocumentError: { const errorMessage = message.error || 'Failed to fetch documents'; @@ -55,10 +54,24 @@ export const handleExtensionMessage = ( } }; +/** + * Each panel gets its own unguessable `vscode-webview://` origin, which + * is the origin VS Code posts extension messages from. + * + * Checking `event.source === window.parent` instead rejects every real message: + * this frame is sandboxed, so `window.parent` is the frame itself while real + * messages carry the unreachable host frame as their source. + */ +const isFromExtensionHost = (event: MessageEvent): boolean => + event.origin === window.location.origin; + export const setupMessageHandler = (dispatch: AppDispatch): (() => void) => { const handleMessage = (event: MessageEvent): void => { - const message: MessageFromExtensionToWebview = event.data; - handleExtensionMessage(dispatch, message); + if (!isFromExtensionHost(event) || !isMessageFromExtension(event.data)) { + return; + } + + handleExtensionMessage(dispatch, event.data); }; window.addEventListener('message', handleMessage);