Skip to content

Commit

Permalink
Improves Git security with untrusted workspaces
Browse files Browse the repository at this point in the history
  • Loading branch information
Eric Amodio committed Jul 13, 2021
1 parent 5b8ce76 commit 5ef5837
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 37 deletions.
44 changes: 23 additions & 21 deletions extensions/git/src/git.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ function parseVersion(raw: string): string {
return raw.replace(/^git version /, '');
}

function findSpecificGit(path: string, onLookup: (path: string) => void): Promise<IGit> {
function findSpecificGit(path: string, onValidate: (path: string) => boolean): Promise<IGit> {
return new Promise<IGit>((c, e) => {
onLookup(path);
if (!onValidate(path)) {
return e('git not found');
}

const buffers: Buffer[] = [];
const child = cp.spawn(path, ['--version']);
Expand All @@ -75,7 +77,7 @@ function findSpecificGit(path: string, onLookup: (path: string) => void): Promis
});
}

function findGitDarwin(onLookup: (path: string) => void): Promise<IGit> {
function findGitDarwin(onValidate: (path: string) => boolean): Promise<IGit> {
return new Promise<IGit>((c, e) => {
cp.exec('which git', (err, gitPathBuffer) => {
if (err) {
Expand All @@ -85,7 +87,9 @@ function findGitDarwin(onLookup: (path: string) => void): Promise<IGit> {
const path = gitPathBuffer.toString().replace(/^\s+|\s+$/g, '');

function getVersion(path: string) {
onLookup(path);
if (!onValidate(path)) {
return e('git not found');
}

// make sure git executes
cp.exec('git --version', (err, stdout) => {
Expand Down Expand Up @@ -117,43 +121,41 @@ function findGitDarwin(onLookup: (path: string) => void): Promise<IGit> {
});
}

function findSystemGitWin32(base: string, onLookup: (path: string) => void): Promise<IGit> {
function findSystemGitWin32(base: string, onValidate: (path: string) => boolean): Promise<IGit> {
if (!base) {
return Promise.reject<IGit>('Not found');
}

return findSpecificGit(path.join(base, 'Git', 'cmd', 'git.exe'), onLookup);
return findSpecificGit(path.join(base, 'Git', 'cmd', 'git.exe'), onValidate);
}

function findGitWin32InPath(onLookup: (path: string) => void): Promise<IGit> {
function findGitWin32InPath(onValidate: (path: string) => boolean): Promise<IGit> {
const whichPromise = new Promise<string>((c, e) => which('git.exe', (err, path) => err ? e(err) : c(path)));
return whichPromise.then(path => findSpecificGit(path, onLookup));
return whichPromise.then(path => findSpecificGit(path, onValidate));
}

function findGitWin32(onLookup: (path: string) => void): Promise<IGit> {
return findSystemGitWin32(process.env['ProgramW6432'] as string, onLookup)
.then(undefined, () => findSystemGitWin32(process.env['ProgramFiles(x86)'] as string, onLookup))
.then(undefined, () => findSystemGitWin32(process.env['ProgramFiles'] as string, onLookup))
.then(undefined, () => findSystemGitWin32(path.join(process.env['LocalAppData'] as string, 'Programs'), onLookup))
.then(undefined, () => findGitWin32InPath(onLookup));
function findGitWin32(onValidate: (path: string) => boolean): Promise<IGit> {
return findSystemGitWin32(process.env['ProgramW6432'] as string, onValidate)
.then(undefined, () => findSystemGitWin32(process.env['ProgramFiles(x86)'] as string, onValidate))
.then(undefined, () => findSystemGitWin32(process.env['ProgramFiles'] as string, onValidate))
.then(undefined, () => findSystemGitWin32(path.join(process.env['LocalAppData'] as string, 'Programs'), onValidate))
.then(undefined, () => findGitWin32InPath(onValidate));
}

export async function findGit(hint: string | string[] | undefined, onLookup: (path: string) => void): Promise<IGit> {
const hints = Array.isArray(hint) ? hint : hint ? [hint] : [];

export async function findGit(hints: string[], onValidate: (path: string) => boolean): Promise<IGit> {
for (const hint of hints) {
try {
return await findSpecificGit(hint, onLookup);
return await findSpecificGit(hint, onValidate);
} catch {
// noop
}
}

try {
switch (process.platform) {
case 'darwin': return await findGitDarwin(onLookup);
case 'win32': return await findGitWin32(onLookup);
default: return await findSpecificGit('git', onLookup);
case 'darwin': return await findGitDarwin(onValidate);
case 'win32': return await findGitWin32(onValidate);
default: return await findSpecificGit('git', onValidate);
}
} catch {
// noop
Expand Down
26 changes: 24 additions & 2 deletions extensions/git/src/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,30 @@ export async function deactivate(): Promise<any> {
}

async function createModel(context: ExtensionContext, outputChannel: OutputChannel, telemetryReporter: TelemetryReporter, disposables: Disposable[]): Promise<Model> {
const pathHint = workspace.getConfiguration('git').get<string | string[]>('path');
const info = await findGit(pathHint, path => outputChannel.appendLine(localize('looking', "Looking for git in: {0}", path)));
const pathValue = workspace.getConfiguration('git').get<string | string[]>('path');
let pathHints = Array.isArray(pathValue) ? pathValue : pathValue ? [pathValue] : [];

const { isTrusted, workspaceFolders = [] } = workspace;
const excludes = isTrusted ? [] : workspaceFolders.map(f => path.normalize(f.uri.fsPath).replace(/[\r\n]+$/, ''));

if (!isTrusted && pathHints.length !== 0) {
// Filter out any non-absolute paths
pathHints = pathHints.filter(p => path.isAbsolute(p));
}

const info = await findGit(pathHints, gitPath => {
outputChannel.appendLine(localize('validating', "Validating found git in: {0}", gitPath));
if (excludes.length === 0) {
return true;
}

const normalized = path.normalize(gitPath).replace(/[\r\n]+$/, '');
const skip = excludes.some(e => normalized.startsWith(e));
if (skip) {
outputChannel.appendLine(localize('skipped', "Skipped found git in: {0}", gitPath));
}
return !skip;
});

const askpass = await Askpass.create(outputChannel, context.storagePath);
disposables.push(askpass);
Expand Down
45 changes: 31 additions & 14 deletions extensions/git/src/model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,23 +147,23 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe
await Promise.all((workspace.workspaceFolders || []).map(async folder => {
const root = folder.uri.fsPath;
const children = await new Promise<string[]>((c, e) => fs.readdir(root, (err, r) => err ? e(err) : c(r)));
const promises = children
.filter(child => child !== '.git')
.map(child => this.openRepository(path.join(root, child)));
const subfolders = new Set(children.filter(child => child !== '.git').map(child => path.join(root, child)));

const folderConfig = workspace.getConfiguration('git', folder.uri);
const paths = folderConfig.get<string[]>('scanRepositories') || [];
const scanPaths = (workspace.isTrusted ? workspace.getConfiguration('git', folder.uri) : config).get<string[]>('scanRepositories') || [];
for (const scanPath of scanPaths) {
if (scanPath !== '.git') {
continue;
}

for (const possibleRepositoryPath of paths) {
if (path.isAbsolute(possibleRepositoryPath)) {
if (path.isAbsolute(scanPath)) {
console.warn(localize('not supported', "Absolute paths not supported in 'git.scanRepositories' setting."));
continue;
}

promises.push(this.openRepository(path.join(root, possibleRepositoryPath)));
subfolders.add(path.join(root, scanPath));
}

await Promise.all(promises);
await Promise.all([...subfolders].map(f => this.openRepository(f)));
}));
}

Expand Down Expand Up @@ -226,6 +226,10 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe
}

private async onDidChangeVisibleTextEditors(editors: readonly TextEditor[]): Promise<void> {
if (!workspace.isTrusted) {
return;
}

const config = workspace.getConfiguration('git');
const autoRepositoryDetection = config.get<boolean | 'subFolders' | 'openEditors'>('autoRepositoryDetection');

Expand All @@ -251,20 +255,33 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe
}

@sequentialize
async openRepository(path: string): Promise<void> {
if (this.getRepository(path)) {
async openRepository(repoPath: string): Promise<void> {
if (this.getRepository(repoPath)) {
return;
}

const config = workspace.getConfiguration('git', Uri.file(path));
const config = workspace.getConfiguration('git', Uri.file(repoPath));
const enabled = config.get<boolean>('enabled') === true;

if (!enabled) {
return;
}

if (!workspace.isTrusted) {
// Check if the folder is a bare repo: if it has a file named HEAD && `rev-parse --show -cdup` is empty
try {
fs.accessSync(path.join(repoPath, 'HEAD'), fs.constants.F_OK);
const result = await this.git.exec(repoPath, ['-C', repoPath, 'rev-parse', '--show-cdup'], { log: false });
if (result.stderr.trim() === '' && result.stdout.trim() === '') {
return;
}
} catch {
// If this throw, we should be good to open the repo (e.g. HEAD doesn't exist)
}
}

try {
const rawRoot = await this.git.getRepositoryRoot(path);
const rawRoot = await this.git.getRepositoryRoot(repoPath);

// This can happen whenever `path` has the wrong case sensitivity in
// case insensitive file systems
Expand All @@ -286,7 +303,7 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe
await repository.status();
} catch (ex) {
// noop
this.outputChannel.appendLine(`Opening repository for path='${path}' failed; ex=${ex}`);
this.outputChannel.appendLine(`Opening repository for path='${repoPath}' failed; ex=${ex}`);
}
}

Expand Down

0 comments on commit 5ef5837

Please sign in to comment.