feat: http filtering (finos#415)

* feat: filter non-git HTTP requests being proxied

Fixes finos#410

* feat: refactor to one validation function

- Accept is only set for certain git actions so refactored into
  one validation function
- pass routes correctly to express (fix TypeError on startup)

* feat: separate url stripping into its own function

- separate out the HTTP git filtering logic from the URL parsing, which
  includes stripping the GitHub repo owner/name.

src/proxy/index.js

@@ -1,7 +1,7 @@
 /* eslint-disable max-len */
 const proxyApp = require('express')();
 const bodyParser = require('body-parser');
-const routes = require('./routes').router;
+const router = require('./routes').router;
 const config = require('../config');
 const db = require('../db');
 const { GIT_PROXY_SERVER_PORT: proxyHttpPort } = require('../config/env').Vars;
@@ -14,7 +14,7 @@ const options = {
 
 // Setup the proxy middleware
 proxyApp.use(bodyParser.raw(options));
-proxyApp.use('/', routes);
+proxyApp.use('/', router);
 
 const start = async () => {
   // Check to see if the default repos are in the repo list

src/proxy/routes/index.js

@@ -5,13 +5,65 @@ const proxy = require('express-http-proxy');
 const router = express.Router();
 const chain = require('../chain');
 
+/**
+ * For a given Git HTTP request destined for a GitHub repo,
+ * remove the GitHub specific components of the URL.
+ * @param {string} url URL path of the request
+ * @return {string} Modified path which removes the {owner}/{repo} parts
+ */
+const stripGitHubFromGitPath = (url) => {
+  const parts = url.split('/');
+  // url = '/{owner}/{repo}.git/{git-path}'
+  // url.split('/') = ['', '{owner}', '{repo}.git', '{git-path}']
+  if (parts.length !== 4 && parts.length !== 5) {
+    console.error('unexpected url received: ', url);
+    return undefined;
+  }
+  parts.splice(1, 2); // remove the {owner} and {repo} from the array
+  return parts.join('/');
+};
+
+/**
+ * Check whether an HTTP request has the expected properties of a
+ * Git HTTP request. The URL is expected to be "sanitized", stripped of
+ * specific paths such as the GitHub {owner}/{repo}.git parts.
+ * @param {string} url Sanitized URL which only includes the path specific to git
+ * @param {*} headers Request headers (TODO: Fix JSDoc linting and refer to node:http.IncomingHttpHeaders)
+ * @return {boolean} If true, this is a valid and expected git request. Otherwise, false.
+ */
+const validGitRequest = (url, headers) => {
+  const { 'user-agent': agent, accept } = headers;
+  if (
+    [
+      '/info/refs?service=git-upload-pack',
+      '/info/refs?service=git-receive-pack',
+    ].includes(url)
+  ) {
+    // https://www.git-scm.com/docs/http-protocol#_discovering_references
+    // We can only filter based on User-Agent since the Accept header is not
+    // sent in this request
+    return agent.startsWith('git/');
+  }
+  if (['/git-upload-pack', '/git-receive-pack'].includes(url)) {
+    // https://www.git-scm.com/docs/http-protocol#_uploading_data
+    return agent.startsWith('git/') && accept.startsWith('application/x-git-');
+  }
+  return false;
+};
+
 router.use(
   '/',
   proxy('https://github.com', {
     filter: async function (req, res) {
       try {
-        console.log(req.url);
-        console.log('recieved');
+        console.log('request url: ', req.url);
+        console.log('host: ', req.headers.host);
+        console.log('user-agent: ', req.headers['user-agent']);
+        const gitPath = stripGitHubFromGitPath(req.url);
+        if (gitPath === undefined || !validGitRequest(gitPath, req.headers)) {
+          res.status(400).send('Invalid request received');
+          return false;
+        }
         if (req.body && req.body.length) {
           req.rawBody = req.body.toString('utf8');
         }
@@ -73,4 +125,9 @@ const handleMessage = async (message) => {
   return packetMessage;
 };
 
-module.exports = { router, handleMessage };
+module.exports = {
+  router,
+  handleMessage,
+  validGitRequest,
+  stripGitHubFromGitPath,
+};

test/testRouteFilter.js

@@ -0,0 +1,88 @@
+/* eslint-disable max-len */
+const chai = require('chai');
+const validGitRequest = require('../src/proxy/routes').validGitRequest;
+const stripGitHubFromGitPath =
+  require('../src/proxy/routes').stripGitHubFromGitPath;
+
+chai.should();
+
+const expect = chai.expect;
+
+describe('url filters for proxying ', function () {
+  it('stripGitHubFromGitPath should return the sanitized URL with owner & repo removed', function () {
+    expect(
+      stripGitHubFromGitPath(
+        '/octocat/hello-world.git/info/refs?service=git-upload-pack',
+      ),
+    ).eq('/info/refs?service=git-upload-pack');
+  });
+
+  it('stripGitHubFromGitPath should return undefined if the url', function () {
+    expect(stripGitHubFromGitPath('/octocat/hello-world')).undefined;
+  });
+
+  it('validGitRequest should return true for safe requests on expected URLs', function () {
+    [
+      '/info/refs?service=git-upload-pack',
+      '/info/refs?service=git-receive-pack',
+      '/git-upload-pack',
+      '/git-receive-pack',
+    ].forEach((url) => {
+      expect(
+        validGitRequest(url, {
+          'user-agent': 'git/2.30.0',
+          accept: 'application/x-git-upload-pack-request',
+        }),
+      ).true;
+    });
+  });
+
+  it('validGitRequest should return false for unsafe URLs', function () {
+    ['/', '/foo'].forEach((url) => {
+      expect(
+        validGitRequest(url, {
+          'user-agent': 'git/2.30.0',
+          accept: 'application/x-git-upload-pack-request',
+        }),
+      ).false;
+    });
+  });
+
+  it('validGitRequest should return false for a browser request', function () {
+    expect(
+      validGitRequest('/', {
+        'user-agent': 'Mozilla/5.0',
+        accept: '*/*',
+      }),
+    ).false;
+  });
+
+  it('validGitRequest should return false for unexpected combinations of headers & URLs', function () {
+    // expected Accept=application/x-git-upload-pack
+    expect(
+      validGitRequest('/git-upload-pack', {
+        'user-agent': 'git/2.30.0',
+        accept: '*/*',
+      }),
+    ).false;
+
+    // expected User-Agent=git/*
+    expect(
+      validGitRequest('/info/refs?service=git-upload-pack', {
+        'user-agent': 'Mozilla/5.0',
+        accept: '*/*',
+      }),
+    ).false;
+  });
+
+  it('validGitRequest should return false for unexpected content-type on certain URLs', function () {
+    ['application/json', 'text/html', '*/*'].map((accept) => {
+      expect(
+        validGitRequest('/git-upload-pack', {
+          'user-agent': 'git/2.30.0',
+          accept: accept,
+        }),
+      ).false;
+    });
+  });
+});