fix(request): proxy-agent breaks Studio (#580)

* fix(request): proxy-agent breaks Studio

* feat: do not load http-agent at all if not in cli

* test: support env in test-harness

src/__tests__/request.jest.test.ts

@@ -1,61 +1,82 @@
+import * as http from 'http';
+import * as url from 'url';
+import { DEFAULT_REQUEST_OPTIONS } from '../request';
 import { httpAndFileResolver } from '../resolvers/http-and-file';
 import { Spectral } from '../spectral';
+const ProxyAgent = require('proxy-agent');
+
+const PORT = 4001;
 
 describe('request', () => {
-  const oldProxyEnv = process.env.PROXY;
+  describe('when agent is set', () => {
+    let server: http.Server;
 
-  beforeEach(() => {
-    process.env = { PROXY: 'http://localhost:3000/' };
-  });
+    beforeAll(() => {
+      // nock cannot mock proxied requests
+      server = http
+        .createServer((req, res) => {
+          const { pathname } = url.parse(String(req.url));
+          if (pathname === '/custom-ruleset') {
+            res.writeHead(403);
+          } else if (pathname === '/ok.json') {
+            res.writeHead(200, { 'Content-Type': 'application/json' });
+            res.write(
+              JSON.stringify({
+                info: {
+                  title: '',
+                  description: 'Foo',
+                },
+              }),
+            );
+          } else {
+            res.writeHead(404);
+          }
 
-  afterEach(() => {
-    process.env.proxy = oldProxyEnv;
-  });
+          res.end();
+        })
+        .listen(PORT, '0.0.0.0');
+    });
+
+    afterAll(() => {
+      server.close();
+    });
 
-  describe('loading a ruleset', () => {
-    it('proxies the request', async () => {
-      const s = new Spectral();
+    beforeEach(() => {
+      DEFAULT_REQUEST_OPTIONS.agent = new ProxyAgent(`http://localhost:${PORT}`);
+    });
+
+    afterEach(() => {
+      delete DEFAULT_REQUEST_OPTIONS.agent;
+    });
+
+    describe('loading a ruleset', () => {
+      it('proxies the request', () => {
+        const s = new Spectral();
 
-      try {
-        await s.loadRuleset('http://localhost:4000/custom-ruleset');
-      } catch (e) {
-        expect(e.message).toBe(
-          'Could not parse http://localhost:4000/custom-ruleset: request to http://localhost:4000/custom-ruleset failed, reason: connect ECONNREFUSED 127.0.0.1:3000',
+        return expect(s.loadRuleset('http://localhost:4000/custom-ruleset')).rejects.toHaveProperty(
+          'message',
+          'Could not parse http://localhost:4000/custom-ruleset: Forbidden',
         );
-      }
+      });
     });
-  });
 
-  describe('loading a $ref', () => {
-    it('proxies the request', async () => {
-      const spec = {
-        openapi: '3.0.2',
-        paths: {
-          '/pets': {
-            get: {
-              responses: {
-                '200': {
-                  description: 'abc',
-                  content: {
-                    'application/json': {
-                      schema: {
-                        $ref: 'http://localhost:8089/ok.json',
-                      },
-                    },
-                  },
-                },
-              },
-            },
+    describe('loading a $ref', () => {
+      it('proxies the request', () => {
+        const doc = {
+          info: {
+            $ref: 'http://localhost:8089/ok.json#/info',
           },
-        },
-      };
+        };
 
-      const s = new Spectral({ resolver: httpAndFileResolver });
-      const results = await s.run(spec);
+        const s = new Spectral({ resolver: httpAndFileResolver });
 
-      expect(results[0].message).toBe(
-        'FetchError: request to http://localhost:8089/ok.json failed, reason: connect ECONNREFUSED 127.0.0.1:3000',
-      );
+        return expect(s.runWithResolved(doc)).resolves.toHaveProperty('resolved', {
+          info: {
+            title: '',
+            description: 'Foo',
+          },
+        });
+      });
     });
   });
 });

src/cli/index.ts

@@ -2,8 +2,14 @@
 
 import * as yargs from 'yargs';
 
+import { DEFAULT_REQUEST_OPTIONS } from '../request';
 import lintCommand from './commands/lint';
 
+if (process.env.PROXY) {
+  const ProxyAgent = require('proxy-agent');
+  DEFAULT_REQUEST_OPTIONS.agent = new ProxyAgent(process.env.PROXY);
+}
+
 export default yargs
   .scriptName('spectral')
   .parserConfiguration({

src/fs/reader.ts

@@ -8,7 +8,7 @@ export interface IReadOptions {
   timeout?: number;
 }
 
-export async function readFile(name: string, opts: IReadOptions) {
+export async function readFile(name: string, opts: IReadOptions): Promise<string> {
   if (isURL(name)) {
     let response;
     let timeout: NodeJS.Timeout | number | null = null;

src/request.ts

@@ -1,11 +1,7 @@
-import { RequestInit } from 'node-fetch';
-import * as ProxyAgent from 'proxy-agent';
+import fetch, { RequestInit } from 'node-fetch';
 
-const fetch = require('node-fetch');
+export const DEFAULT_REQUEST_OPTIONS: RequestInit = {};
 
-export default (uri: string, opts: RequestInit = {}) => {
-  const options =
-    typeof process === 'object' && process.env.PROXY ? { ...opts, agent: new ProxyAgent(process.env.PROXY) } : opts;
-
-  return fetch(uri, options);
+export default async (uri: string, opts: RequestInit = {}) => {
+  return fetch(uri, { ...opts, ...DEFAULT_REQUEST_OPTIONS });
 };

test-harness/helpers.ts

@@ -1,5 +1,5 @@
 export function parseScenarioFile(data: string) {
-  const regex = /====(test|document|command|status|stdout|stderr)====\r?\n/gi;
+  const regex = /====(test|document|command|status|stdout|stderr|env)====\r?\n/gi;
   const split = data.split(regex);
 
   const testIndex = split.findIndex(t => t === 'test');
@@ -8,6 +8,7 @@ export function parseScenarioFile(data: string) {
   const statusIndex = split.findIndex(t => t === 'status');
   const stdoutIndex = split.findIndex(t => t === 'stdout');
   const stderrIndex = split.findIndex(t => t === 'stderr');
+  const envIndex = split.findIndex(t => t === 'env');
 
   return {
     test: split[1 + testIndex],
@@ -16,5 +17,17 @@ export function parseScenarioFile(data: string) {
     status: split[1 + statusIndex],
     stdout: split[1 + stdoutIndex],
     stderr: split[1 + stderrIndex],
+    env: envIndex === -1 ? process.env : getEnv(split[1 + envIndex]),
   };
 }
+
+function getEnv(env: string): NodeJS.ProcessEnv {
+  return env.split(/\r?\n/).reduce(
+    (envs, line) => {
+      const [key, value = ''] = line.split('=');
+      envs[key] = value;
+      return envs;
+    },
+    { ...process.env },
+  );
+}

test-harness/index.ts

@@ -78,6 +78,7 @@ describe('cli acceptance tests', () => {
         shell: true,
         encoding: 'utf8',
         windowsVerbatimArguments: false,
+        env: scenario.env,
       });
 
       const expectedStatus = replaceVars(scenario.status.trim(), replacements);

test-harness/scenarios/proxy-agent.scenario

@@ -0,0 +1,15 @@
+====test====
+Requests for $refs are proxied when PROXY env variable is set
+====document====
+foo:
+  $ref: http://localhost:3002/foo.json#/ref
+====env====
+PROXY=http://localhost:3001
+====command====
+lint {document}
+====stdout====
+
+{document}
+ 2:9  error  invalid-ref  FetchError: request to http://localhost:3002/foo.json failed, reason: connect ECONNREFUSED 127.0.0.1:3001
+
+✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints)