feat: add option for disabling file downloads (coder#5055)

jsjoeio · code-asher · TinLe · commit d04de3dca642 · 2022-04-23T17:47:21.000Z
* feat(cli): add disable-file-downloads to cli

* feat(e2e): add download test

* feat(e2e): add downloads disabled test

* refactor(e2e): explain how to debug unexpected close

* feat(patches): add disable file downloads

* wip: update diff

* Update src/node/cli.ts

Co-authored-by: Asher &lt;ash@coder.com&gt;

* fixup! add missing common/contextkeys file to patch

* fixup!: update patch

* fixup!: default disable-file-downloads undefined

* fixup!: combine e2e tests

* fixup!: use different test names

* feat: add CS_DISABLE_FILE_DOWNLOADS

* fixup!: make explicit and cleanup test

* fixup!: use beforeEach

Co-authored-by: Asher &lt;ash@coder.com&gt;
diff --git a/patches/disable-downloads.diff b/patches/disable-downloads.diff
@@ -0,0 +1,183 @@
+Add option to disable file downloads via CLI
+
+This patch adds support for a new CLI flag called `--disable-file-downloads`
+which allows a user to remove the "Download..." option that shows up when you
+right-click files in Code. The default value for this is `false`.
+
+To test this, start code-server with `--disable-file-downloads`, open editor,
+right-click on a file (not a folder) and you should **not** see the
+"Download..." option.
+
+Index: code-server/lib/vscode/src/vs/workbench/browser/web.api.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/workbench/browser/web.api.ts
++++ code-server/lib/vscode/src/vs/workbench/browser/web.api.ts
+@@ -210,6 +210,11 @@ export interface IWorkbenchConstructionO
+ 	 */
+ 	readonly userDataPath?: string
+ 
++	/**
++	 * Whether the "Download..." option is enabled for files.
++	 */
++	readonly isEnabledFileDownloads?: boolean
++
+ 	//#endregion
+ 
+ 
+Index: code-server/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts
++++ code-server/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts
+@@ -30,6 +30,11 @@ export interface IBrowserWorkbenchEnviro
+ 	 * Options used to configure the workbench.
+ 	 */
+ 	readonly options?: IWorkbenchConstructionOptions;
++
++	/**
++	 * Enable downloading files via menu actions.
++	 */
++	readonly isEnabledFileDownloads?: boolean;
+ }
+ 
+ export class BrowserWorkbenchEnvironmentService implements IBrowserWorkbenchEnvironmentService {
+@@ -61,6 +66,13 @@ export class BrowserWorkbenchEnvironment
+ 		return this.options.userDataPath;
+ 	}
+ 
++	get isEnabledFileDownloads(): boolean {
++		if (typeof this.options.isEnabledFileDownloads === "undefined") {
++			throw new Error('isEnabledFileDownloads was not provided to the browser');
++		}
++		return this.options.isEnabledFileDownloads;
++	}
++
+ 	@memoize
+ 	get settingsResource(): URI { return joinPath(this.userRoamingDataHome, 'settings.json'); }
+ 
+Index: code-server/lib/vscode/src/vs/server/node/serverEnvironmentService.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/server/node/serverEnvironmentService.ts
++++ code-server/lib/vscode/src/vs/server/node/serverEnvironmentService.ts
+@@ -15,6 +15,7 @@ export const serverOptions: OptionDescri
+ 	'disable-update-check': { type: 'boolean' },
+ 	'auth': { type: 'string' },
+ 	'locale': { type: 'string' },
++	'disable-file-downloads': { type: 'boolean' },
+ 
+ 	/* ----- server setup ----- */
+ 
+@@ -92,6 +93,7 @@ export interface ServerParsedArgs {
+ 	'disable-update-check'?: boolean;
+ 	'auth'?: string
+ 	'locale'?: string
++	'disable-file-downloads'?: boolean;
+ 
+ 	/* ----- server setup ----- */
+ 
+Index: code-server/lib/vscode/src/vs/server/node/webClientServer.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/server/node/webClientServer.ts
++++ code-server/lib/vscode/src/vs/server/node/webClientServer.ts
+@@ -290,6 +290,7 @@ export class WebClientServer {
+ 					logLevel: this._logService.getLevel(),
+ 				},
+ 				userDataPath: this._environmentService.userDataPath,
++				isEnabledFileDownloads: !this._environmentService.args['disable-file-downloads'],
+ 				settingsSyncOptions: !this._environmentService.isBuilt && this._environmentService.args['enable-sync'] ? { enabled: true } : undefined,
+ 				productConfiguration: <Partial<IProductConfiguration>>{
+ 					rootEndpoint: base,
+Index: code-server/lib/vscode/src/vs/workbench/browser/contextkeys.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/workbench/browser/contextkeys.ts
++++ code-server/lib/vscode/src/vs/workbench/browser/contextkeys.ts
+@@ -7,12 +7,11 @@ import { Event } from 'vs/base/common/ev
+ import { Disposable } from 'vs/base/common/lifecycle';
+ import { IContextKeyService, IContextKey } from 'vs/platform/contextkey/common/contextkey';
+ import { InputFocusedContext, IsMacContext, IsLinuxContext, IsWindowsContext, IsWebContext, IsMacNativeContext, IsDevelopmentContext, IsIOSContext } from 'vs/platform/contextkey/common/contextkeys';
+-import { SplitEditorsVertically, InEditorZenModeContext, ActiveEditorCanRevertContext, ActiveEditorGroupLockedContext, ActiveEditorCanSplitInGroupContext, SideBySideEditorActiveContext, AuxiliaryBarVisibleContext, SideBarVisibleContext, PanelAlignmentContext, PanelMaximizedContext, PanelVisibleContext, ActiveEditorContext, EditorsVisibleContext, TextCompareEditorVisibleContext, TextCompareEditorActiveContext, ActiveEditorGroupEmptyContext, MultipleEditorGroupsContext, EditorTabsVisibleContext, IsCenteredLayoutContext, ActiveEditorGroupIndexContext, ActiveEditorGroupLastContext, ActiveEditorReadonlyContext, EditorAreaVisibleContext, ActiveEditorAvailableEditorIdsContext, DirtyWorkingCopiesContext, EmptyWorkspaceSupportContext, EnterMultiRootWorkspaceSupportContext, HasWebFileSystemAccess, IsFullscreenContext, OpenFolderWorkspaceSupportContext, RemoteNameContext, VirtualWorkspaceContext, WorkbenchStateContext, WorkspaceFolderCountContext, PanelPositionContext } from 'vs/workbench/common/contextkeys';
++import { SplitEditorsVertically, InEditorZenModeContext, ActiveEditorCanRevertContext, ActiveEditorGroupLockedContext, ActiveEditorCanSplitInGroupContext, SideBySideEditorActiveContext, AuxiliaryBarVisibleContext, SideBarVisibleContext, PanelAlignmentContext, PanelMaximizedContext, PanelVisibleContext, ActiveEditorContext, EditorsVisibleContext, TextCompareEditorVisibleContext, TextCompareEditorActiveContext, ActiveEditorGroupEmptyContext, MultipleEditorGroupsContext, EditorTabsVisibleContext, IsCenteredLayoutContext, ActiveEditorGroupIndexContext, ActiveEditorGroupLastContext, ActiveEditorReadonlyContext, EditorAreaVisibleContext, ActiveEditorAvailableEditorIdsContext, DirtyWorkingCopiesContext, EmptyWorkspaceSupportContext, EnterMultiRootWorkspaceSupportContext, HasWebFileSystemAccess, IsFullscreenContext, OpenFolderWorkspaceSupportContext, RemoteNameContext, VirtualWorkspaceContext, WorkbenchStateContext, WorkspaceFolderCountContext, PanelPositionContext, IsEnabledFileDownloads } from 'vs/workbench/common/contextkeys';
+ import { TEXT_DIFF_EDITOR_ID, EditorInputCapabilities, SIDE_BY_SIDE_EDITOR_ID, DEFAULT_EDITOR_ASSOCIATION } from 'vs/workbench/common/editor';
+ import { trackFocus, addDisposableListener, EventType } from 'vs/base/browser/dom';
+ import { preferredSideBySideGroupDirection, GroupDirection, IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
+ import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
+-import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
+ import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
+ import { WorkbenchState, IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
+ import { IWorkbenchLayoutService, Parts, positionToString } from 'vs/workbench/services/layout/browser/layoutService';
+@@ -24,6 +23,7 @@ import { IEditorResolverService } from '
+ import { IPaneCompositePartService } from 'vs/workbench/services/panecomposite/browser/panecomposite';
+ import { Schemas } from 'vs/base/common/network';
+ import { WebFileSystemAccess } from 'vs/platform/files/browser/webFileSystemAccess';
++import { IBrowserWorkbenchEnvironmentService } from '../services/environment/browser/environmentService';
+ 
+ export class WorkbenchContextKeysHandler extends Disposable {
+ 	private inputFocusedContext: IContextKey<boolean>;
+@@ -75,7 +75,7 @@ export class WorkbenchContextKeysHandler
+ 		@IContextKeyService private readonly contextKeyService: IContextKeyService,
+ 		@IWorkspaceContextService private readonly contextService: IWorkspaceContextService,
+ 		@IConfigurationService private readonly configurationService: IConfigurationService,
+-		@IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService,
++		@IBrowserWorkbenchEnvironmentService private readonly environmentService: IBrowserWorkbenchEnvironmentService,
+ 		@IEditorService private readonly editorService: IEditorService,
+ 		@IEditorResolverService private readonly editorResolverService: IEditorResolverService,
+ 		@IEditorGroupsService private readonly editorGroupService: IEditorGroupsService,
+@@ -194,6 +194,9 @@ export class WorkbenchContextKeysHandler
+ 		this.auxiliaryBarVisibleContext = AuxiliaryBarVisibleContext.bindTo(this.contextKeyService);
+ 		this.auxiliaryBarVisibleContext.set(this.layoutService.isVisible(Parts.AUXILIARYBAR_PART));
+ 
++		// code-server
++		IsEnabledFileDownloads.bindTo(this.contextKeyService).set(this.environmentService.isEnabledFileDownloads ?? true)
++
+ 		this.registerListeners();
+ 	}
+ 
+Index: code-server/lib/vscode/src/vs/workbench/contrib/files/browser/fileActions.contribution.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/workbench/contrib/files/browser/fileActions.contribution.ts
++++ code-server/lib/vscode/src/vs/workbench/contrib/files/browser/fileActions.contribution.ts
+@@ -21,7 +21,7 @@ import { CLOSE_SAVED_EDITORS_COMMAND_ID,
+ import { AutoSaveAfterShortDelayContext } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService';
+ import { WorkbenchListDoubleSelection } from 'vs/platform/list/browser/listService';
+ import { Schemas } from 'vs/base/common/network';
+-import { DirtyWorkingCopiesContext, EmptyWorkspaceSupportContext, EnterMultiRootWorkspaceSupportContext, HasWebFileSystemAccess, WorkbenchStateContext, WorkspaceFolderCountContext, SidebarFocusContext, ActiveEditorCanRevertContext, ActiveEditorContext, ResourceContextKey } from 'vs/workbench/common/contextkeys';
++import { DirtyWorkingCopiesContext, EmptyWorkspaceSupportContext, EnterMultiRootWorkspaceSupportContext, HasWebFileSystemAccess, WorkbenchStateContext, WorkspaceFolderCountContext, SidebarFocusContext, ActiveEditorCanRevertContext, ActiveEditorContext, ResourceContextKey, IsEnabledFileDownloads } from 'vs/workbench/common/contextkeys';
+ import { IsWebContext } from 'vs/platform/contextkey/common/contextkeys';
+ import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
+ import { ThemeIcon } from 'vs/platform/theme/common/themeService';
+@@ -475,13 +475,16 @@ MenuRegistry.appendMenuItem(MenuId.Explo
+ 		id: DOWNLOAD_COMMAND_ID,
+ 		title: DOWNLOAD_LABEL
+ 	},
+-	when: ContextKeyExpr.or(
+-		// native: for any remote resource
+-		ContextKeyExpr.and(IsWebContext.toNegated(), ResourceContextKey.Scheme.notEqualsTo(Schemas.file)),
+-		// web: for any files
+-		ContextKeyExpr.and(IsWebContext, ExplorerFolderContext.toNegated(), ExplorerRootContext.toNegated()),
+-		// web: for any folders if file system API support is provided
+-		ContextKeyExpr.and(IsWebContext, HasWebFileSystemAccess)
++	when: ContextKeyExpr.and(
++		IsEnabledFileDownloads,
++		ContextKeyExpr.or(
++			// native: for any remote resource
++			ContextKeyExpr.and(IsWebContext.toNegated(), ResourceContextKey.Scheme.notEqualsTo(Schemas.file)),
++			// web: for any files
++			ContextKeyExpr.and(IsWebContext, ExplorerFolderContext.toNegated(), ExplorerRootContext.toNegated()),
++			// web: for any folders if file system API support is provided
++			ContextKeyExpr.and(IsWebContext, HasWebFileSystemAccess)
++		)
+ 	)
+ }));
+ 
+Index: code-server/lib/vscode/src/vs/workbench/common/contextkeys.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/workbench/common/contextkeys.ts
++++ code-server/lib/vscode/src/vs/workbench/common/contextkeys.ts
+@@ -30,6 +30,8 @@ export const IsFullscreenContext = new R
+ 
+ export const HasWebFileSystemAccess = new RawContextKey<boolean>('hasWebFileSystemAccess', false, true); // Support for FileSystemAccess web APIs (https://wicg.github.io/file-system-access)
+ 
++export const IsEnabledFileDownloads = new RawContextKey<boolean>('isEnabledFileDownloads', true, true);
++
+ //#endregion
+ 
+ 
diff --git a/patches/series b/patches/series
@@ -18,3 +18,4 @@ local-storage.diff
 service-worker.diff
 connection-type.diff
 sourcemaps.diff
+disable-downloads.diff
diff --git a/src/node/cli.ts b/src/node/cli.ts
@@ -49,6 +49,7 @@ export interface UserProvidedCodeArgs {
   category?: string
   "github-auth"?: string
   "disable-update-check"?: boolean
+  "disable-file-downloads"?: boolean
 }
 
 /**
@@ -157,6 +158,10 @@ export const options: Options<Required<UserProvidedArgs>> = {
       "Disable update check. Without this flag, code-server checks every 6 hours against the latest github release and \n" +
       "then notifies you once every week that a new release is available.",
   },
+  "disable-file-downloads": {
+    type: "boolean",
+    description: "Disable file downloads from Code.",
+  },
   // --enable can be used to enable experimental features. These features
   // provide no guarantees.
   enable: { type: "string[]" },
@@ -537,6 +542,10 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config
     args.password = process.env.PASSWORD
   }
 
+  if (process.env.CS_DISABLE_FILE_DOWNLOADS === "1") {
+    args["disable-file-downloads"] = true
+  }
+
   const usingEnvHashedPassword = !!process.env.HASHED_PASSWORD
   if (process.env.HASHED_PASSWORD) {
     args["hashed-password"] = process.env.HASHED_PASSWORD
diff --git a/test/e2e/downloads.test.ts b/test/e2e/downloads.test.ts
@@ -0,0 +1,48 @@
+import * as path from "path"
+import { promises as fs } from "fs"
+import { clean } from "../utils/helpers"
+import { describe, test, expect } from "./baseFixture"
+
+describe("Downloads (enabled)", true, [], {}, async () => {
+  const testName = "downloads-enabled"
+  test.beforeAll(async () => {
+    await clean(testName)
+  })
+
+  test("should see the 'Download...' option", async ({ codeServerPage }) => {
+    // Setup
+    const workspaceDir = await codeServerPage.workspaceDir
+    const tmpFilePath = path.join(workspaceDir, "unique-file.txt")
+    await fs.writeFile(tmpFilePath, "hello world")
+
+    // Action
+    const fileInExplorer = await codeServerPage.page.waitForSelector("text=unique-file.txt")
+    await fileInExplorer.click({
+      button: "right",
+    })
+
+    expect(await codeServerPage.page.isVisible("text=Download...")).toBe(true)
+  })
+})
+
+describe("Downloads (disabled)", true, ["--disable-file-downloads"], {}, async () => {
+  const testName = "downloads-disabled"
+  test.beforeAll(async () => {
+    await clean(testName)
+  })
+
+  test("should not see the 'Download...' option", async ({ codeServerPage }) => {
+    // Setup
+    const workspaceDir = await codeServerPage.workspaceDir
+    const tmpFilePath = path.join(workspaceDir, "unique-file.txt")
+    await fs.writeFile(tmpFilePath, "hello world")
+
+    // Action
+    const fileInExplorer = await codeServerPage.page.waitForSelector("text=unique-file.txt")
+    await fileInExplorer.click({
+      button: "right",
+    })
+
+    expect(await codeServerPage.page.isVisible("text=Download...")).toBe(false)
+  })
+})
diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts
@@ -134,7 +134,7 @@ export class CodeServer {
       })
 
       proc.on("close", (code) => {
-        const error = new Error("code-server closed unexpectedly")
+        const error = new Error("code-server closed unexpectedly. Try running with LOG_LEVEL=debug to see more info.")
         if (!this.closed) {
           this.logger.error(error.message, field("code", code))
         }
diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts
@@ -42,6 +42,7 @@ describe("parser", () => {
   beforeEach(() => {
     delete process.env.LOG_LEVEL
     delete process.env.PASSWORD
+    delete process.env.CS_DISABLE_FILE_DOWNLOADS
     console.log = jest.fn()
   })
 
@@ -92,6 +93,8 @@ describe("parser", () => {
 
           "--port=8081",
 
+          "--disable-file-downloads",
+
           ["--host", "0.0.0.0"],
           "4",
           "--",
@@ -108,6 +111,7 @@ describe("parser", () => {
       cert: {
         value: path.resolve("path/to/cert"),
       },
+      "disable-file-downloads": true,
       enable: ["feature1", "feature2"],
       help: true,
       host: "0.0.0.0",
@@ -346,6 +350,18 @@ describe("parser", () => {
     expect(process.env.GITHUB_TOKEN).toBe(undefined)
   })
 
+  it("should use env var CS_DISABLE_FILE_DOWNLOADS", async () => {
+    process.env.CS_DISABLE_FILE_DOWNLOADS = "1"
+    const args = parse([])
+    expect(args).toEqual({})
+
+    const defaultArgs = await setDefaults(args)
+    expect(defaultArgs).toEqual({
+      ...defaults,
+      "disable-file-downloads": true,
+    })
+  })
+
   it("should error if password passed in", () => {
     expect(() => parse(["--password", "supersecret123"])).toThrowError(
       "--password can only be set in the config file or passed in via $PASSWORD",
diff --git a/test/unit/node/plugin.test.ts b/test/unit/node/plugin.test.ts
@@ -38,6 +38,7 @@ describe("plugin", () => {
         "proxy-domain": [],
         config: "~/.config/code-server/config.yaml",
         verbose: false,
+        "disable-file-downloads": false,
         usingEnvPassword: false,
         usingEnvHashedPassword: false,
         "extensions-dir": "",

Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,7 @@ export class CodeServer {`
`134`	`134`	`})`
`135`	`135`
`136`	`136`	`proc.on("close", (code) => {`
`137`		`- const error = new Error("code-server closed unexpectedly")`
	`137`	`+ const error = new Error("code-server closed unexpectedly. Try running with LOG_LEVEL=debug to see more info.")`
`138`	`138`	`if (!this.closed) {`
`139`	`139`	`this.logger.error(error.message, field("code", code))`
`140`	`140`	`}`