From ef4722890e8680ab467e8892224bd80de7582751 Mon Sep 17 00:00:00 2001 From: Chris Wendt Date: Mon, 17 Sep 2018 17:54:31 -0700 Subject: [PATCH] fix: prevent transitive importing of entrypoints --- src/config/link.entry.js | 1 + src/config/options.entry.js | 1 + src/extension/envAssertion.ts | 11 +++ src/extension/scripts/background.tsx | 78 +++++++------------ src/extension/scripts/inject.tsx | 3 + src/extension/scripts/link.tsx | 3 + src/extension/scripts/options.tsx | 3 + src/shared/backend/PortMessageTransports.ts | 2 +- src/shared/backend/extensions.ts | 5 +- .../options/BrowserSettingsEditor.tsx} | 4 +- .../components/options/ExtensionRegistry.tsx | 2 +- src/shared/messaging.ts | 18 +++++ src/types/globals/index.d.ts | 2 +- webpack/base.config.ts | 7 +- 14 files changed, 80 insertions(+), 60 deletions(-) create mode 100644 src/config/link.entry.js create mode 100644 src/config/options.entry.js create mode 100644 src/extension/envAssertion.ts rename src/{extension/scripts/extensions.tsx => shared/components/options/BrowserSettingsEditor.tsx} (96%) create mode 100644 src/shared/messaging.ts diff --git a/src/config/link.entry.js b/src/config/link.entry.js new file mode 100644 index 00000000..65d30602 --- /dev/null +++ b/src/config/link.entry.js @@ -0,0 +1 @@ +window.EXTENSION_ENV = 'LINK' diff --git a/src/config/options.entry.js b/src/config/options.entry.js new file mode 100644 index 00000000..4d50376d --- /dev/null +++ b/src/config/options.entry.js @@ -0,0 +1 @@ +window.EXTENSION_ENV = 'OPTIONS' diff --git a/src/extension/envAssertion.ts b/src/extension/envAssertion.ts new file mode 100644 index 00000000..b83d8673 --- /dev/null +++ b/src/extension/envAssertion.ts @@ -0,0 +1,11 @@ +export function assertEnv(env: typeof window['EXTENSION_ENV']): void { + if (window.EXTENSION_ENV !== env) { + throw new Error( + 'Detected transitive import of an entrypoint! ' + + window.EXTENSION_ENV + + ' attempted to import a file that is only intended to be imported by ' + + env + + '.' + ) + } +} diff --git a/src/extension/scripts/background.tsx b/src/extension/scripts/background.tsx index 6488263d..0318ced3 100644 --- a/src/extension/scripts/background.tsx +++ b/src/extension/scripts/background.tsx @@ -15,8 +15,11 @@ import storage, { defaultStorageItems } from '../../browser/storage' import * as tabs from '../../browser/tabs' import initializeCli from '../../libs/cli' import { resolveClientConfiguration } from '../../shared/backend/server' -import { isBackground } from '../../shared/context' +import { ExtensionConnectionInfo, onFirstMessage } from '../../shared/messaging' import { DEFAULT_SOURCEGRAPH_URL, setSourcegraphUrl } from '../../shared/util/context' +import { assertEnv } from '../envAssertion' + +assertEnv('BACKGROUND') let customServerOrigins: string[] = [] @@ -439,52 +442,29 @@ const spawnAndConnect = ({ connectPortAndWorker(port, worker) }) -/** - * The information necessary to connect to a Sourcegraph extension. - */ -export interface ExtensionConnectionInfo { - extensionID: string - jsBundleURL: string -} - -/** - * Executes the callback only on the first message that's received on the port. - */ -export const onFirstMessage = (port: chrome.runtime.Port, callback: (message: any) => void) => { - const cb = message => { - port.onMessage.removeListener(cb) - callback(message) - } - port.onMessage.addListener(cb) -} - -// This must not execute anywhere but the background script, otherwise messages -// will get duplicated and cause all kinds of unintuitive behavior. -if (isBackground) { - // This is the bridge between content scripts (that want to connect to Sourcegraph extensions) and the background - // script (that spawns JS bundles or connects to WebSocket endpoints).: - chrome.runtime.onConnect.addListener(port => { - // When a content script wants to create a connection to a Sourcegraph extension, it first connects to the - // background script on a random port and sends a message containing the platform information for that - // Sourcegraph extension (e.g. a JS bundle at localhost:1234/index.js). - onFirstMessage(port, (connectionInfo: ExtensionConnectionInfo) => { - // The background script receives the message and attempts to spawn the - // extension: - spawnAndConnect({ connectionInfo, port }).then( - // If spawning succeeds, the background script sends {} (so the content script knows it succeeded) and - // the port communicates using the internal Sourcegraph extension RPC API after that. - () => { - // Success is represented by the absence of an error - port.postMessage({}) - }, - // If spawning fails, the background script sends { error } (so the content script knows it failed) and - // the port is immediately disconnected. There is always a 1-1 correspondence between ports and content - // scripts, so this won't disrupt any other connections. - error => { - port.postMessage({ error }) - port.disconnect() - } - ) - }) +// This is the bridge between content scripts (that want to connect to Sourcegraph extensions) and the background +// script (that spawns JS bundles or connects to WebSocket endpoints).: +chrome.runtime.onConnect.addListener(port => { + // When a content script wants to create a connection to a Sourcegraph extension, it first connects to the + // background script on a random port and sends a message containing the platform information for that + // Sourcegraph extension (e.g. a JS bundle at localhost:1234/index.js). + onFirstMessage(port, (connectionInfo: ExtensionConnectionInfo) => { + // The background script receives the message and attempts to spawn the + // extension: + spawnAndConnect({ connectionInfo, port }).then( + // If spawning succeeds, the background script sends {} (so the content script knows it succeeded) and + // the port communicates using the internal Sourcegraph extension RPC API after that. + () => { + // Success is represented by the absence of an error + port.postMessage({}) + }, + // If spawning fails, the background script sends { error } (so the content script knows it failed) and + // the port is immediately disconnected. There is always a 1-1 correspondence between ports and content + // scripts, so this won't disrupt any other connections. + error => { + port.postMessage({ error }) + port.disconnect() + } + ) }) -} +}) diff --git a/src/extension/scripts/inject.tsx b/src/extension/scripts/inject.tsx index a3efbb5a..bcdf4a25 100644 --- a/src/extension/scripts/inject.tsx +++ b/src/extension/scripts/inject.tsx @@ -20,6 +20,9 @@ import { injectBitbucketServer } from '../../libs/bitbucket/inject' import { injectGitHubApplication } from '../../libs/github/inject' import { injectPhabricatorApplication } from '../../libs/phabricator/app' import { injectSourcegraphApp } from '../../libs/sourcegraph/inject' +import { assertEnv } from '../envAssertion' + +assertEnv('CONTENT') /** * Main entry point into browser extension. diff --git a/src/extension/scripts/link.tsx b/src/extension/scripts/link.tsx index 319418dc..aeda4693 100644 --- a/src/extension/scripts/link.tsx +++ b/src/extension/scripts/link.tsx @@ -1,4 +1,7 @@ import storage from '../../browser/storage' +import { assertEnv } from '../envAssertion' + +assertEnv('LINK') const searchParams = new URLSearchParams(window.location.search) diff --git a/src/extension/scripts/options.tsx b/src/extension/scripts/options.tsx index 0adc3b2e..cef80472 100644 --- a/src/extension/scripts/options.tsx +++ b/src/extension/scripts/options.tsx @@ -6,6 +6,9 @@ import * as React from 'react' import { render } from 'react-dom' import storage from '../../browser/storage' import { OptionsDashboard } from '../../shared/components/options/OptionsDashboard' +import { assertEnv } from '../envAssertion' + +assertEnv('OPTIONS') const inject = () => { const injectDOM = document.createElement('div') diff --git a/src/shared/backend/PortMessageTransports.ts b/src/shared/backend/PortMessageTransports.ts index b8850765..85c6b756 100644 --- a/src/shared/backend/PortMessageTransports.ts +++ b/src/shared/backend/PortMessageTransports.ts @@ -7,7 +7,7 @@ import { MessageReader, MessageWriter, } from 'sourcegraph/module/protocol/jsonrpc2/transport' -import { ExtensionConnectionInfo } from '../../extension/scripts/background' +import { ExtensionConnectionInfo } from '../messaging' class PortMessageReader extends AbstractMessageReader implements MessageReader { private pending: Message[] = [] diff --git a/src/shared/backend/extensions.ts b/src/shared/backend/extensions.ts index 5f4daf7d..878ca2f1 100644 --- a/src/shared/backend/extensions.ts +++ b/src/shared/backend/extensions.ts @@ -11,8 +11,8 @@ import { Settings, } from '@sourcegraph/extensions-client-common/lib/settings' import { LoadingSpinner } from '@sourcegraph/react-loading-spinner' -import * as JSONC from '@sqs/jsonc-parser' import { applyEdits } from '@sqs/jsonc-parser' +import * as JSONC from '@sqs/jsonc-parser' import { removeProperty, setProperty } from '@sqs/jsonc-parser/lib/edit' import { isEqual } from 'lodash' import Alert from 'mdi-react/AlertIcon' @@ -25,8 +25,7 @@ import { TextDocumentDecoration } from 'sourcegraph/module/protocol/plainTypes' import uuid from 'uuid' import { Disposable } from 'vscode-languageserver' import storage, { StorageItems } from '../../browser/storage' -import { onFirstMessage } from '../../extension/scripts/background' -import { ExtensionConnectionInfo } from '../../extension/scripts/background' +import { ExtensionConnectionInfo, onFirstMessage } from '../messaging' import { getContext } from './context' import { createAggregateError, isErrorLike } from './errors' import { queryGraphQL } from './graphql' diff --git a/src/extension/scripts/extensions.tsx b/src/shared/components/options/BrowserSettingsEditor.tsx similarity index 96% rename from src/extension/scripts/extensions.tsx rename to src/shared/components/options/BrowserSettingsEditor.tsx index 842ec7c0..bc9e7e2e 100644 --- a/src/extension/scripts/extensions.tsx +++ b/src/shared/components/options/BrowserSettingsEditor.tsx @@ -1,10 +1,10 @@ // We want to polyfill first. -import '../../config/polyfill' +import '../../../config/polyfill' import * as React from 'react' import { Button, FormGroup, Input, Label } from 'reactstrap' import { Subscription } from 'rxjs' -import storage from '../../browser/storage' +import storage from '../../../browser/storage' interface State { clientSettings: string diff --git a/src/shared/components/options/ExtensionRegistry.tsx b/src/shared/components/options/ExtensionRegistry.tsx index 45f17a73..0003b1e1 100644 --- a/src/shared/components/options/ExtensionRegistry.tsx +++ b/src/shared/components/options/ExtensionRegistry.tsx @@ -7,10 +7,10 @@ import { import * as React from 'react' import { RouteComponentProps } from 'react-router-dom' import { Subscription } from 'rxjs' -import { BrowserSettingsEditor } from '../../../extension/scripts/extensions' import { createExtensionsContextController } from '../../../shared/backend/extensions' import { GQL } from '../../../types/gqlschema' import { sourcegraphUrl } from '../../util/context' +import { BrowserSettingsEditor } from './BrowserSettingsEditor' interface OptionsPageProps extends RouteComponentProps<{}> {} interface OptionsPageState extends ConfigurationCascadeProps {} diff --git a/src/shared/messaging.ts b/src/shared/messaging.ts new file mode 100644 index 00000000..0090aeea --- /dev/null +++ b/src/shared/messaging.ts @@ -0,0 +1,18 @@ +/** + * The information necessary to connect to a Sourcegraph extension. + */ +export interface ExtensionConnectionInfo { + extensionID: string + jsBundleURL: string +} + +/** + * Executes the callback only on the first message that's received on the port. + */ +export const onFirstMessage = (port: chrome.runtime.Port, callback: (message: any) => void) => { + const cb = message => { + port.onMessage.removeListener(cb) + callback(message) + } + port.onMessage.addListener(cb) +} diff --git a/src/types/globals/index.d.ts b/src/types/globals/index.d.ts index 3147b459..a8fa5d49 100644 --- a/src/types/globals/index.d.ts +++ b/src/types/globals/index.d.ts @@ -8,7 +8,7 @@ interface Window { | undefined SOURCEGRAPH_PHABRICATOR_EXTENSION: boolean | undefined SG_ENV: 'EXTENSION' | 'PAGE' - EXTENSION_ENV: 'CONTENT' | 'BACKGROUND' | null + EXTENSION_ENV: 'CONTENT' | 'BACKGROUND' | 'OPTIONS' | 'LINK' | null SOURCEGRAPH_BUNDLE_URL: string | undefined // Bundle Sourcegraph URL is set from the Phabricator extension. safariMessager?: { send: (message: { type: string; payload: any }, cb?: (res?: any) => void) => void diff --git a/webpack/base.config.ts b/webpack/base.config.ts index b0657a0a..cce5bd2e 100644 --- a/webpack/base.config.ts +++ b/webpack/base.config.ts @@ -7,6 +7,8 @@ const buildEntry = (...files) => files.map(file => path.join(__dirname, file)) const contentEntry = '../src/config/content.entry.js' const backgroundEntry = '../src/config/background.entry.js' +const linkEntry = '../src/config/link.entry.js' +const optionsEntry = '../src/config/options.entry.js' const pageEntry = '../src/config/page.entry.js' const extEntry = '../src/config/extension.entry.js' @@ -20,9 +22,8 @@ const babelLoader: webpack.RuleSetUseItem = { export default { entry: { background: buildEntry(extEntry, backgroundEntry, '../src/extension/scripts/background.tsx'), - link: buildEntry(extEntry, contentEntry, '../src/extension/scripts/link.tsx'), - options: buildEntry(extEntry, backgroundEntry, '../src/extension/scripts/options.tsx'), - extensions: buildEntry(extEntry, backgroundEntry, '../src/extension/scripts/extensions.tsx'), + link: buildEntry(extEntry, linkEntry, '../src/extension/scripts/link.tsx'), + options: buildEntry(extEntry, optionsEntry, '../src/extension/scripts/options.tsx'), inject: buildEntry(extEntry, contentEntry, '../src/extension/scripts/inject.tsx'), phabricator: buildEntry(pageEntry, '../src/libs/phabricator/extension.tsx'),