From ee2a0c42a92d33059a39fd15fbbd5dd3d5ab6440 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Wed, 17 May 2023 15:58:24 -0400 Subject: [PATCH] Disables Git access in Restricted Mode (untrusted) --- src/env/node/git/git.ts | 8 +++- src/env/node/git/localGitProvider.ts | 2 +- src/git/errors.ts | 9 ++++ src/git/gitProviderService.ts | 13 ++++++ src/webviews/apps/home/home.html | 11 +++++ src/webviews/apps/home/home.ts | 64 ++++++++++++++++------------ src/webviews/home/homeWebview.ts | 32 +++++++++++--- src/webviews/home/protocol.ts | 1 + 8 files changed, 105 insertions(+), 35 deletions(-) diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index a964fef92593f..243914101920d 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -9,7 +9,7 @@ import type { CoreConfiguration } from '../../../constants'; import { GlyphChars } from '../../../constants'; import type { GitCommandOptions, GitSpawnOptions } from '../../../git/commandOptions'; import { GitErrorHandling } from '../../../git/commandOptions'; -import { StashPushError, StashPushErrorReason } from '../../../git/errors'; +import { StashPushError, StashPushErrorReason, WorkspaceUntrustedError } from '../../../git/errors'; import type { GitDiffFilter } from '../../../git/models/diff'; import { isUncommitted, isUncommittedStaged, shortenRevision } from '../../../git/models/reference'; import type { GitUser } from '../../../git/models/user'; @@ -123,6 +123,8 @@ export class Git { async git(options: ExitCodeOnlyGitCommandOptions, ...args: any[]): Promise; async git(options: GitCommandOptions, ...args: any[]): Promise; async git(options: GitCommandOptions, ...args: any[]): Promise { + if (!workspace.isTrusted) throw new WorkspaceUntrustedError(); + const start = hrtime(); const { configs, correlationKey, errors: errorHandling, encoding, ...opts } = options; @@ -224,6 +226,8 @@ export class Git { } async gitSpawn(options: GitSpawnOptions, ...args: any[]): Promise { + if (!workspace.isTrusted) throw new WorkspaceUntrustedError(); + const start = hrtime(); const { cancellation, configs, stdin, stdinEncoding, ...opts } = options; @@ -1645,6 +1649,8 @@ export class Git { ? (emptyArray as []) : [true, normalizePath(data.trimStart().replace(/[\r|\n]+$/, ''))]; } catch (ex) { + if (ex instanceof WorkspaceUntrustedError) return emptyArray as []; + const unsafeMatch = /^fatal: detected dubious ownership in repository at '([^']+)'[\s\S]*git config --global --add safe\.directory '?([^'\n]+)'?$/m.exec( ex.stderr, diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 2208be4ec6b3d..a3d0a1dde8171 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1088,7 +1088,7 @@ export class LocalGitProvider implements GitProvider, Disposable { [safe, repoPath] = await this.git.rev_parse__show_toplevel(uri.fsPath); if (safe) { this.unsafePaths.delete(uri.fsPath); - } else { + } else if (safe === false) { this.unsafePaths.add(uri.fsPath); } if (!repoPath) return undefined; diff --git a/src/git/errors.ts b/src/git/errors.ts index a3b814906ec64..e9baa1e0a9ffb 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -81,6 +81,15 @@ export class StashPushError extends Error { Error.captureStackTrace?.(this, StashApplyError); } } + +export class WorkspaceUntrustedError extends Error { + constructor() { + super('Unable to perform Git operations because the current workspace is untrusted'); + + Error.captureStackTrace?.(this, WorkspaceUntrustedError); + } +} + export const enum WorktreeCreateErrorReason { AlreadyCheckedOut = 1, AlreadyExists = 2, diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index a3531bf0b1465..ade35f2a96a5f 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -82,6 +82,12 @@ import type { RichRemoteProvider } from './remotes/richRemoteProvider'; import type { GitSearch, SearchQuery } from './search'; const emptyArray = Object.freeze([]) as unknown as any[]; +const emptyDisposable = Object.freeze({ + dispose: () => { + /* noop */ + }, +}); + const maxDefaultBranchWeight = 100; const weightedDefaultBranches = new Map([ ['master', maxDefaultBranchWeight], @@ -216,6 +222,13 @@ export class GitProviderService implements Disposable { this.resetCaches('providers'); this.updateContext(); }), + !workspace.isTrusted + ? workspace.onDidGrantWorkspaceTrust(() => { + if (workspace.isTrusted && workspace.workspaceFolders?.length) { + void this.discoverRepositories(workspace.workspaceFolders, { force: true }); + } + }) + : emptyDisposable, ...this.registerCommands(), ); diff --git a/src/webviews/apps/home/home.html b/src/webviews/apps/home/home.html index 5d983327163e2..01a02f30db163 100644 --- a/src/webviews/apps/home/home.html +++ b/src/webviews/apps/home/home.html @@ -96,6 +96,17 @@

Unsafe repository

+
diff --git a/src/webviews/apps/home/home.ts b/src/webviews/apps/home/home.ts index 541388258355f..f3ddae314c0e9 100644 --- a/src/webviews/apps/home/home.ts +++ b/src/webviews/apps/home/home.ts @@ -247,36 +247,26 @@ export class HomeApp extends App { } private updateNoRepo() { - const { repositories } = this.state; - const hasRepos = repositories.openCount > 0; - const value = hasRepos ? 'true' : 'false'; - - let $el = document.getElementById('no-repo'); - $el?.setAttribute('aria-hidden', value); - if (hasRepos) { - $el?.setAttribute('hidden', value); - } else { - $el?.removeAttribute('hidden'); - } + const { + repositories: { openCount, hasUnsafe, trusted }, + } = this.state; - $el = document.getElementById('no-repo-alert'); - const showUnsafe = repositories.hasUnsafe && !hasRepos; - const $unsafeEl = document.getElementById('unsafe-repo-alert'); - if (showUnsafe) { - $el?.setAttribute('aria-hidden', 'true'); - $el?.setAttribute('hidden', 'true'); - $unsafeEl?.setAttribute('aria-hidden', 'false'); - $unsafeEl?.removeAttribute('hidden'); - } else { - $unsafeEl?.setAttribute('aria-hidden', 'true'); - $unsafeEl?.setAttribute('hidden', 'true'); - $el?.setAttribute('aria-hidden', value); - if (hasRepos) { - $el?.setAttribute('hidden', value); - } else { - $el?.removeAttribute('hidden'); - } + if (!trusted) { + setElementVisibility('untrusted-alert', true); + setElementVisibility('no-repo', false); + setElementVisibility('no-repo-alert', false); + setElementVisibility('unsafe-repo-alert', false); + + return; } + + setElementVisibility('untrusted-alert', false); + + const noRepos = openCount === 0; + + setElementVisibility('no-repo', noRepos); + setElementVisibility('no-repo-alert', noRepos && !hasUnsafe); + setElementVisibility('unsafe-repo-alert', hasUnsafe); } private updateLayout() { @@ -371,6 +361,24 @@ export class HomeApp extends App { } } +function setElementVisibility(elementOrId: string | HTMLElement | null | undefined, visible: boolean) { + let el; + if (typeof elementOrId === 'string') { + el = document.getElementById(elementOrId); + } else { + el = elementOrId; + } + if (el == null) return; + + if (visible) { + el.setAttribute('aria-hidden', 'false'); + el.removeAttribute('hidden'); + } else { + el.setAttribute('aria-hidden', 'true'); + el?.setAttribute('hidden', 'true'); + } +} + function toggleArrayItem(list: string[] = [], item: string, add = true) { const hasStep = list.includes(item); if (!hasStep && add) { diff --git a/src/webviews/home/homeWebview.ts b/src/webviews/home/homeWebview.ts index 337184ecccb5b..01b87b34c5fb8 100644 --- a/src/webviews/home/homeWebview.ts +++ b/src/webviews/home/homeWebview.ts @@ -1,5 +1,5 @@ import type { ConfigurationChangeEvent } from 'vscode'; -import { Disposable, window } from 'vscode'; +import { Disposable, window, workspace } from 'vscode'; import { getAvatarUriFromGravatarEmail } from '../../avatars'; import { ViewsLayout } from '../../commands/setViewsLayout'; import type { Container } from '../../container'; @@ -10,11 +10,18 @@ import { executeCoreCommand, registerCommand } from '../../system/command'; import { configuration } from '../../system/configuration'; import type { Deferrable } from '../../system/function'; import { debounce } from '../../system/function'; +import { getSettledValue } from '../../system/promise'; import type { StorageChangeEvent } from '../../system/storage'; import type { IpcMessage } from '../protocol'; import { onIpc } from '../protocol'; import type { WebviewController, WebviewProvider } from '../webviewController'; -import type { CompleteStepParams, DismissBannerParams, DismissSectionParams, State } from './protocol'; +import type { + CompleteStepParams, + DidChangeRepositoriesParams, + DismissBannerParams, + DismissSectionParams, + State, +} from './protocol'; import { CompletedActions, CompleteStepCommandType, @@ -27,6 +34,12 @@ import { DismissStatusCommandType, } from './protocol'; +const emptyDisposable = Object.freeze({ + dispose: () => { + /* noop */ + }, +}); + export class HomeWebviewProvider implements WebviewProvider { private readonly _disposable: Disposable; @@ -36,6 +49,9 @@ export class HomeWebviewProvider implements WebviewProvider { this.container.git.onDidChangeRepositories(this.onRepositoriesChanged, this), configuration.onDidChange(this.onConfigurationChanged, this), this.container.storage.onDidChange(this.onStorageChanged, this), + !workspace.isTrusted + ? workspace.onDidGrantWorkspaceTrust(this.notifyDidChangeRepositories, this) + : emptyDisposable, ); } @@ -221,7 +237,12 @@ export class HomeWebviewProvider implements WebviewProvider { } private async getState(subscription?: Subscription): Promise { - const sub = await this.getSubscription(subscription); + const [visibilityResult, subscriptionResult] = await Promise.allSettled([ + this.getRepoVisibility(), + this.getSubscription(subscription), + ]); + + const sub = getSettledValue(subscriptionResult)!; const steps = this.container.storage.get('home:steps:completed', []); const sections = this.container.storage.get('home:sections:dismissed', []); const dismissedBanners = this.container.storage.get('home:banners:dismissed', []); @@ -233,7 +254,7 @@ export class HomeWebviewProvider implements WebviewProvider { subscription: sub.subscription, completedActions: sub.completedActions, plusEnabled: this.getPlusEnabled(), - visibility: await this.getRepoVisibility(), + visibility: getSettledValue(visibilityResult)!, completedSteps: steps, dismissedSections: sections, avatar: sub.avatar, @@ -255,11 +276,12 @@ export class HomeWebviewProvider implements WebviewProvider { }); } - private getRepositoriesState() { + private getRepositoriesState(): DidChangeRepositoriesParams { return { count: this.container.git.repositoryCount, openCount: this.container.git.openRepositoryCount, hasUnsafe: this.container.git.hasUnsafeRepositories(), + trusted: workspace.isTrusted, }; } diff --git a/src/webviews/home/protocol.ts b/src/webviews/home/protocol.ts index 823eeb9cb9f5f..050fbf67c0c46 100644 --- a/src/webviews/home/protocol.ts +++ b/src/webviews/home/protocol.ts @@ -57,6 +57,7 @@ export interface DidChangeRepositoriesParams { count: number; openCount: number; hasUnsafe: boolean; + trusted: boolean; } export const DidChangeRepositoriesType = new IpcNotificationType('repositories/didChange');