[Code] Check uri before executing the delete repository job (#47614) (#48302)

* [Code] Check uri before executing the delete repository job

* sanity check

* improve path check

x-pack/legacy/plugins/code/common/git_url_utils.test.ts

@@ -48,6 +48,11 @@ test('Git url validation', () => {
     validateGitUrl('https://github.com/elastic/elasticsearch.git', [], ['ssh']);
   }).toThrow('Git url protocol is not whitelisted.');
 
+  // An valid git url but contains invalid content
+  expect(() => {
+    validateGitUrl('https://github.com/elastic/../../elasticsearch.git', [], ['ssh']);
+  }).toThrow('Git url contains invalid content.');
+
   // An valid git url with both whitelisted host and protocol
   expect(
     validateGitUrl('https://github.com/elastic/elasticsearch.git', ['github.com'], ['https'])

x-pack/legacy/plugins/code/common/git_url_utils.ts

@@ -14,6 +14,14 @@ export function validateGitUrl(
   hostWhitelist?: string[],
   protocolWhitelist?: string[]
 ): boolean {
+  if (url.split('/').includes('..')) {
+    throw new Error(
+      i18n.translate('xpack.code.gitUrlUtil.urlContainInvalidContent', {
+        defaultMessage: 'Git url contains invalid content.',
+      })
+    );
+  }
+
   const repo = GitUrlParse(url);
 
   if (hostWhitelist && hostWhitelist.length > 0) {

x-pack/legacy/plugins/code/server/queue/delete_worker.test.ts

@@ -5,8 +5,10 @@
  */
 
 import sinon from 'sinon';
-import { EsClient, Esqueue } from '../lib/esqueue';
 
+import { WorkerReservedProgress } from '../../model';
+import { RepositoryIndexStatusReservedField } from '../indexer/schema';
+import { EsClient, Esqueue } from '../lib/esqueue';
 import { GitOperations } from '../git_operations';
 import { Logger } from '../log';
 import { LspService } from '../lsp/lsp_service';
@@ -55,12 +57,26 @@ test('Execute delete job.', async () => {
 
   // Setup EsClient
   const deleteSpy = sinon.fake.returns(Promise.resolve());
+  const getSpy = sinon.fake.returns(
+    Promise.resolve({
+      _source: {
+        [RepositoryIndexStatusReservedField]: {
+          uri: 'github.com/elastic/kibana',
+          progress: WorkerReservedProgress.COMPLETED,
+          timestamp: new Date(),
+          revision: 'abcdefg',
+        },
+      },
+    })
+  );
   const esClient = {
     indices: {
       delete: emptyAsyncFunc,
     },
+    get: emptyAsyncFunc,
   };
   esClient.indices.delete = deleteSpy;
+  esClient.get = getSpy;
 
   // Setup LspService
   const deleteWorkspaceSpy = sinon.fake.returns(Promise.resolve());
@@ -109,6 +125,195 @@ test('Execute delete job.', async () => {
   expect(deleteWorkspaceSpy.calledOnce).toBeTruthy();
 });
 
+test('On delete job uri does not exist.', async () => {
+  // Setup RepositoryService
+  const removeSpy = sinon.fake.returns(Promise.resolve());
+  const repoService = {
+    remove: emptyAsyncFunc,
+  };
+  repoService.remove = removeSpy;
+  const repoServiceFactory = {
+    newInstance: (): void => {
+      return;
+    },
+  };
+  const newInstanceSpy = sinon.fake.returns(repoService);
+  repoServiceFactory.newInstance = newInstanceSpy;
+
+  // Setup CancellationService
+  const cancelIndexJobSpy = sinon.spy();
+  const cancelCloneJobSpy = sinon.spy();
+  const cancelUpdateJobSpy = sinon.spy();
+  const cancellationService = {
+    cancelCloneJob: emptyAsyncFunc,
+    cancelUpdateJob: emptyAsyncFunc,
+    cancelIndexJob: emptyAsyncFunc,
+  };
+  cancellationService.cancelIndexJob = cancelIndexJobSpy;
+  cancellationService.cancelCloneJob = cancelCloneJobSpy;
+  cancellationService.cancelUpdateJob = cancelUpdateJobSpy;
+
+  // Setup EsClient
+  const deleteSpy = sinon.fake.returns(Promise.resolve());
+  const getSpy = sinon.fake.returns(Promise.reject(new Error('ES does not find the document')));
+  const esClient = {
+    indices: {
+      delete: emptyAsyncFunc,
+    },
+    get: emptyAsyncFunc,
+  };
+  esClient.indices.delete = deleteSpy;
+  esClient.get = getSpy;
+
+  // Setup LspService
+  const deleteWorkspaceSpy = sinon.fake.returns(Promise.resolve());
+  const lspService = {
+    deleteWorkspace: emptyAsyncFunc,
+  };
+  lspService.deleteWorkspace = deleteWorkspaceSpy;
+
+  // Setup GitOperations
+  const cleanRepoSpy = sinon.spy();
+  const gitOps = {
+    cleanRepo: cleanRepoSpy,
+  };
+
+  const deleteWorker = new DeleteWorker(
+    esQueue as Esqueue,
+    log,
+    esClient as EsClient,
+    {
+      security: {
+        enableGitCertCheck: true,
+      },
+    } as ServerOptions,
+    (gitOps as any) as GitOperations,
+    (cancellationService as any) as CancellationSerivce,
+    (lspService as any) as LspService,
+    (repoServiceFactory as any) as RepositoryServiceFactory
+  );
+
+  const result = await deleteWorker.executeJob({
+    payload: {
+      uri: 'repo/doesnot/exist',
+    },
+    options: {},
+    timestamp: 0,
+  });
+
+  expect(result.uri).toEqual('repo/doesnot/exist');
+  expect(result.res).toBeTruthy();
+
+  // Non of them should be called.
+  expect(cancelIndexJobSpy.notCalled).toBeTruthy();
+  expect(cancelCloneJobSpy.notCalled).toBeTruthy();
+  expect(cancelUpdateJobSpy.notCalled).toBeTruthy();
+  expect(newInstanceSpy.notCalled).toBeTruthy();
+  expect(removeSpy.notCalled).toBeTruthy();
+  expect(deleteSpy.notCalled).toBeTruthy();
+  expect(deleteWorkspaceSpy.notCalled).toBeTruthy();
+});
+
+test('On delete job uri contains ../', async () => {
+  // Setup RepositoryService
+  const removeSpy = sinon.fake.returns(Promise.resolve());
+  const repoService = {
+    remove: emptyAsyncFunc,
+  };
+  repoService.remove = removeSpy;
+  const repoServiceFactory = {
+    newInstance: (): void => {
+      return;
+    },
+  };
+  const newInstanceSpy = sinon.fake.returns(repoService);
+  repoServiceFactory.newInstance = newInstanceSpy;
+
+  // Setup CancellationService
+  const cancelIndexJobSpy = sinon.spy();
+  const cancelCloneJobSpy = sinon.spy();
+  const cancelUpdateJobSpy = sinon.spy();
+  const cancellationService = {
+    cancelCloneJob: emptyAsyncFunc,
+    cancelUpdateJob: emptyAsyncFunc,
+    cancelIndexJob: emptyAsyncFunc,
+  };
+  cancellationService.cancelIndexJob = cancelIndexJobSpy;
+  cancellationService.cancelCloneJob = cancelCloneJobSpy;
+  cancellationService.cancelUpdateJob = cancelUpdateJobSpy;
+
+  // Setup EsClient
+  const deleteSpy = sinon.fake.returns(Promise.resolve());
+  const getSpy = sinon.fake.returns(
+    Promise.resolve({
+      _source: {
+        [RepositoryIndexStatusReservedField]: {
+          uri: 'github.com/elastic/kibana/../../../../foo/bar',
+          progress: WorkerReservedProgress.COMPLETED,
+          timestamp: new Date(),
+          revision: 'abcdefg',
+        },
+      },
+    })
+  );
+  const esClient = {
+    indices: {
+      delete: emptyAsyncFunc,
+    },
+    get: emptyAsyncFunc,
+  };
+  esClient.indices.delete = deleteSpy;
+  esClient.get = getSpy;
+
+  // Setup LspService
+  const deleteWorkspaceSpy = sinon.fake.returns(Promise.resolve());
+  const lspService = {
+    deleteWorkspace: emptyAsyncFunc,
+  };
+  lspService.deleteWorkspace = deleteWorkspaceSpy;
+
+  // Setup GitOperations
+  const cleanRepoSpy = sinon.spy();
+  const gitOps = {
+    cleanRepo: cleanRepoSpy,
+  };
+
+  const deleteWorker = new DeleteWorker(
+    esQueue as Esqueue,
+    log,
+    esClient as EsClient,
+    {
+      security: {
+        enableGitCertCheck: true,
+      },
+    } as ServerOptions,
+    (gitOps as any) as GitOperations,
+    (cancellationService as any) as CancellationSerivce,
+    (lspService as any) as LspService,
+    (repoServiceFactory as any) as RepositoryServiceFactory
+  );
+
+  const result = await deleteWorker.executeJob({
+    payload: {
+      uri: 'github.com/elastic/kibana/../../../../foo/bar',
+    },
+    options: {},
+    timestamp: 0,
+  });
+
+  expect(result.uri).toEqual('github.com/elastic/kibana/../../../../foo/bar');
+  expect(result.res).toBeTruthy();
+
+  // Non of them should be called.
+  expect(cancelIndexJobSpy.notCalled).toBeTruthy();
+  expect(cancelCloneJobSpy.notCalled).toBeTruthy();
+  expect(cancelUpdateJobSpy.notCalled).toBeTruthy();
+  expect(newInstanceSpy.notCalled).toBeTruthy();
+  expect(removeSpy.notCalled).toBeTruthy();
+  expect(deleteSpy.notCalled).toBeTruthy();
+  expect(deleteWorkspaceSpy.notCalled).toBeTruthy();
+});
+
 test('On delete job enqueued.', async () => {
   // Setup EsClient
   const indexSpy = sinon.fake.returns(Promise.resolve());

x-pack/legacy/plugins/code/server/queue/delete_worker.ts

@@ -41,6 +41,24 @@ export class DeleteWorker extends AbstractWorker {
   public async executeJob(job: Job) {
     const { uri } = job.payload;
 
+    try {
+      // 1. Check if the uri exsits
+      await this.objectClient.getRepository(uri);
+      // 2. Double check if the uri contains ".." which could result in
+      // deleting incorrect files in the file systems.
+      if (uri.split('/').includes('..')) {
+        throw new Error('Repository URI should not contain "..".');
+      }
+    } catch (error) {
+      this.log.error(`Invalid repository uri ${uri}. Skip delete job.`);
+      this.log.error(error);
+      return {
+        uri,
+        // Because the repository does not exist, we can regard the delete job to be successful.
+        res: true,
+      };
+    }
+
     try {
       // 1. Cancel running clone and update workers
       await this.cancellationService.cancelCloneJob(uri, CancellationReason.REPOSITORY_DELETE);

x-pack/legacy/plugins/code/server/repository_service.ts

@@ -99,6 +99,9 @@ export class RepositoryService {
   public async remove(uri: string): Promise<DeleteWorkerResult> {
     const localPath = RepositoryUtils.repositoryLocalPath(this.repoVolPath, uri);
     try {
+      if (localPath.split(path.sep).includes('..')) {
+        throw new Error('Repository path should not contain "..".');
+      }
       // For now, just `rm -rf`
       await del([localPath], { force: true });
       this.log.info(`Remove local repository ${uri} done.`);