Upgraded got package from v9.6.0 to v11.8.6 (#16261)

Refs TryGhost/Team#2459
-upgraded got from v9.6.0 to v11.8.6 to support following redirects (and
other fixes)
-got v12+ requires ESM, so we do not want to upgrade further at this
time
-required changes to a few libraries that use externalRequests
-mention discovery service tests updated to test for follow redirects

ghost/core/core/server/lib/request-external.js

@@ -20,22 +20,32 @@ function isPrivateIp(addr) {
 async function errorIfHostnameResolvesToPrivateIp(options) {
     // allow requests through to local Ghost instance
     const siteUrl = new URL(config.get('url'));
-    const requestUrl = new URL(options.href);
+    const requestUrl = new URL(options.url.href);
     if (requestUrl.host === siteUrl.host) {
         return Promise.resolve();
     }
 
-    const result = await dnsPromises.lookup(options.hostname);
+    const result = await dnsPromises.lookup(options.url.hostname);
 
     if (isPrivateIp(result.address)) {
         return Promise.reject(new errors.InternalServerError({
             message: 'URL resolves to a non-permitted private IP block',
             code: 'URL_PRIVATE_INVALID',
-            context: options.href
+            context: options.url.href
         }));
     }
 }
 
+async function errorIfInvalidUrl(options) {
+    if (!options.url.hostname || !validator.isURL(options.url.hostname)) {
+        throw new errors.InternalServerError({
+            message: 'URL invalid.',
+            code: 'URL_MISSING_INVALID',
+            context: options.url.href
+        });
+    }
+}
+
 // same as our normal request lib but if any request in a redirect chain resolves
 // to a private IP address it will be blocked before the request is made.
 const externalRequest = got.extend({
@@ -44,16 +54,7 @@ const externalRequest = got.extend({
     },
     timeout: 10000, // default is no timeout
     hooks: {
-        init: [(options) => {
+        beforeRequest: [errorIfInvalidUrl,errorIfHostnameResolvesToPrivateIp],
-            if (!options.hostname || !validator.isURL(options.hostname)) {
-                throw new errors.InternalServerError({
-                    message: 'URL empty or invalid.',
-                    code: 'URL_MISSING_INVALID',
-                    context: options.href
-                });
-            }
-        }],
-        beforeRequest: [errorIfHostnameResolvesToPrivateIp],
         beforeRedirect: [errorIfHostnameResolvesToPrivateIp]
     }
 });

ghost/core/core/server/services/oembed/nft-oembed.js

@@ -42,9 +42,8 @@ class NFTOEmbedProvider {
             headers['X-API-KEY'] = this.dependencies.config.apiKey;
         }
         const result = await externalRequest(`https://api.opensea.io/api/v1/asset/${transaction}/${asset}/?format=json`, {
-            json: true,
             headers
-        });
+        }).json();
         return {
             version: '1.0',
             type: 'nft',

ghost/core/package.json

@@ -170,7 +170,7 @@
     "fs-extra": "11.1.0",
     "ghost-storage-base": "1.0.0",
     "glob": "8.1.0",
-    "got": "9.6.0",
+    "got": "11.8.6",
     "gscan": "4.36.0",
     "human-number": "2.0.1",
     "image-size": "1.0.2",

ghost/core/test/e2e-api/admin/oembed.test.js

@@ -128,6 +128,12 @@ describe('Oembed API', function () {
         });
 
         it('errors when fetched url is an IP address', async function () {
+            // in order to follow the 302, we need to stub differently; externalRequest will block the internal IP
+            dnsPromises.lookup.restore();
+            let dnsStub = sinon.stub(dnsPromises, 'lookup');
+            dnsStub.onCall(0).returns(Promise.resolve({address: '123.123.123.123'}));
+            dnsStub.onCall(1).returns(Promise.resolve({address: '0.0.0.0'}));
+
             const redirectMock = nock('http://test.com/')
                 .get('/')
                 .reply(302, undefined, {Location: 'http://0.0.0.0:8080'});
@@ -147,7 +153,7 @@ describe('Oembed API', function () {
                 .expect('Cache-Control', testUtils.cacheRules.private)
                 .expect(422);
 
-            pageMock.isDone().should.be.true();
+            pageMock.isDone().should.be.false(); // we shouldn't hit this; blocked by externalRequest
             should.exist(res.body.errors);
         });
 
@@ -376,6 +382,15 @@ describe('Oembed API', function () {
         });
 
         it('skips fetching IPv4 addresses', async function () {
+            dnsPromises.lookup.restore();
+            sinon.stub(dnsPromises, 'lookup').callsFake(function (hostname) {
+                if (hostname === '192.168.0.1') {
+                    return Promise.resolve({address: '192.168.0.1'});
+                } else {
+                    return Promise.resolve({address: '123.123.123.123'});
+                }
+            });
+
             const pageMock = nock('http://test.com')
                 .get('/')
                 .reply(200, '<html><head><link rel="alternate" type="application/json+oembed" href="http://192.168.0.1/oembed"></head></html>');
@@ -399,6 +414,15 @@ describe('Oembed API', function () {
         });
 
         it('skips fetching IPv6 addresses', async function () {
+            dnsPromises.lookup.restore();
+            sinon.stub(dnsPromises, 'lookup').callsFake(function (hostname) {
+                if (hostname === '[2607:f0d0:1002:51::4]') {
+                    return Promise.resolve({address: '192.168.0.1'});
+                } else {
+                    return Promise.resolve({address: '123.123.123.123'});
+                }
+            });
+            
             const pageMock = nock('http://test.com')
                 .get('/')
                 .reply(200, '<html><head><link rel="alternate" type="application/json+oembed" href="http://[2607:f0d0:1002:51::4]:9999/oembed"></head></html>');
@@ -422,6 +446,14 @@ describe('Oembed API', function () {
         });
 
         it('skips fetching localhost', async function () {
+            dnsPromises.lookup.restore();
+            sinon.stub(dnsPromises, 'lookup').callsFake(function (hostname) {
+                if (hostname === 'localhost') {
+                    return Promise.resolve({address: '127.0.0.1'});
+                } else {
+                    return Promise.resolve({address: '123.123.123.123'});
+                }
+            });
             const pageMock = nock('http://test.com')
                 .get('/')
                 .reply(200, '<html><head><link rel="alternate" type="application/json+oembed" href="http://localhost:9999/oembed"></head></html>');

ghost/core/test/unit/server/lib/request-external.test.js

@@ -263,7 +263,7 @@ describe('External Request', function () {
                 throw new Error('Request should have rejected with invalid url message');
             }, (err) => {
                 should.exist(err);
-                err.message.should.be.equal('URL empty or invalid.');
+                err.code.should.be.equal('ERR_INVALID_URL');
             });
         });
 
@@ -279,7 +279,8 @@ describe('External Request', function () {
                 throw new Error('Request should have rejected with invalid url message');
             }, (err) => {
                 should.exist(err);
-                err.message.should.be.equal('URL empty or invalid.');
+                // got v11+ throws an error instead of the external requests lib
+                err.message.should.be.equal('No URL protocol specified');
             });
         });
 
@@ -300,7 +301,7 @@ describe('External Request', function () {
             }, (err) => {
                 requestMock.isDone().should.be.true();
                 should.exist(err);
-                err.statusMessage.should.be.equal('Not Found');
+                err.response.statusMessage.should.be.equal('Not Found');
             });
         });
 
@@ -322,9 +323,7 @@ describe('External Request', function () {
             }, (err) => {
                 requestMock.isDone().should.be.true();
                 should.exist(err);
-                err.statusMessage.should.be.equal('Internal Server Error');
+                err.response.statusMessage.should.be.equal(`Internal Server Error`);
-                err.body.should.match(/something awful happened/);
-                err.body.should.match(/AWFUL_ERROR/);
             });
         });
     });

ghost/members-api/lib/services/geolocation.js

@@ -7,9 +7,8 @@ module.exports = class GeolocationService {
         if (!ipAddress || (!IPV4_REGEX.test(ipAddress) && !IPV6_REGEX.test(ipAddress))) {
             return;
         }
-
         const geojsUrl = `https://get.geojs.io/v1/ip/geo/${encodeURIComponent(ipAddress)}.json`;
-        const response = await got(geojsUrl, {json: true, timeout: 500});
+        const response = await got(geojsUrl, {timeout: 500}).json();
-        return response.body;
+        return response;
     }
 };

ghost/members-api/package.json

@@ -41,7 +41,7 @@
     "body-parser": "1.20.1",
     "bson-objectid": "2.0.4",
     "express": "4.18.2",
-    "got": "9.6.0",
+    "got": "11.8.6",
     "jsonwebtoken": "8.5.1",
     "lodash": "4.17.21",
     "moment": "2.29.4",

ghost/oembed-service/lib/oembed-service.js

@@ -4,7 +4,6 @@ const logging = require('@tryghost/logging');
 const {extract, hasProvider} = require('oembed-parser');
 const cheerio = require('cheerio');
 const _ = require('lodash');
-const {CookieJar} = require('tough-cookie');
 const charset = require('charset');
 const iconv = require('iconv-lite');
 
@@ -68,16 +67,7 @@ class OEmbed {
         this.config = config;
 
         /** @type {IExternalRequest} */
-        this.externalRequest = async (url, requestConfig) => {
+        this.externalRequest = externalRequest;
-            if (this.isIpOrLocalhost(url)) {
-                return this.unknownProvider(url);
-            }
-            const response = await externalRequest(url, requestConfig);
-            if (this.isIpOrLocalhost(response.url)) {
-                return this.unknownProvider(url);
-            }
-            return response;
-        };
 
         /** @type {ICustomProvider[]} */
         this.customProviders = [];
@@ -117,16 +107,13 @@ class OEmbed {
      * @param {string} url
      * @param {Object} options
      *
-     * @returns {Promise<{url: string, body: any, headers: any}>}
+     * @returns {GotPromise<any>}
      */
-    async fetchPage(url, options) {
+    fetchPage(url, options) {
-        const cookieJar = new CookieJar();
         return this.externalRequest(
             url,
             {
-                cookieJar,
+                timeout: 2000,
-                method: 'GET',
-                timeout: 2 * 1000,
                 followRedirect: true,
                 ...options
             });
@@ -140,7 +127,7 @@ class OEmbed {
     async fetchPageHtml(url) {
         // Fetch url and get response as binary buffer to
         // avoid implicit cast
-        const {headers, body, url: responseUrl} = await this.fetchPage(
+        let {headers, body, url: responseUrl} = await this.fetchPage(
             url,
             {
                 encoding: 'binary',
@@ -162,7 +149,7 @@ class OEmbed {
             }
 
             const decodedBody = iconv.decode(
-                Buffer.from(body, 'binary'), encoding);
+                body, encoding);
 
             return {
                 body: decodedBody,
@@ -184,12 +171,9 @@ class OEmbed {
      * @returns {Promise<{url: string, body: Object}>}
      */
     async fetchPageJson(url) {
-        const {body, url: pageUrl} = await this.fetchPage(
+        const res = await this.fetchPage(url, {responseType: 'json'});
-            url,
+        const body = res.body;
-            {
+        const pageUrl = res.url;
-                json: true
-            });
-
         return {
             body,
             url: pageUrl
@@ -247,34 +231,6 @@ class OEmbed {
         };
     }
 
-    /**
-     * @param {string} url
-     * @returns {boolean}
-     */
-    isIpOrLocalhost(url) {
-        try {
-            const IPV4_REGEX = /^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/;
-            const IPV6_REGEX = /:/; // fqdns will not have colons
-            const HTTP_REGEX = /^https?:/i;
-
-            const siteUrl = new URL(this.config.get('url'));
-            const {protocol, hostname, host} = new URL(url);
-
-            // allow requests to Ghost's own url through
-            if (siteUrl.host === host) {
-                return false;
-            }
-
-            if (!HTTP_REGEX.test(protocol) || hostname === 'localhost' || IPV4_REGEX.test(hostname) || IPV6_REGEX.test(hostname)) {
-                return true;
-            }
-
-            return false;
-        } catch (e) {
-            return true;
-        }
-    }
-
     /**
      * @param {string} url
      * @param {string} html
@@ -300,7 +256,6 @@ class OEmbed {
 
             // fetch oembed response from embedded rel="alternate" url
             const oembedResponse = await this.fetchPageJson(oembedUrl);
-
             // validate the fetched json against the oembed spec to avoid
             // leaking non-oembed responses
             const body = oembedResponse.body;

ghost/webmentions/lib/MentionDiscoveryService.js

@@ -17,7 +17,7 @@ module.exports = class MentionDiscoveryService {
         try {
             const response = await this.#externalRequest(url.href, {
                 throwHttpErrors: false,
-                followRedirects: true,
+                followRedirect: true,
                 maxRedirects: 10
             });
             if (response.statusCode === 404) {

ghost/webmentions/lib/MentionSendingService.js

@@ -79,22 +79,24 @@ module.exports = class MentionSendingService {
 
     async send({source, target, endpoint}) {
         logging.info('[Webmention] Sending webmention from ' + source.href + ' to ' + target.href + ' via ' + endpoint.href);
+        
+        // default content type is application/x-www-form-encoded which is what we need for the webmentions spec
         const response = await this.#externalRequest.post(endpoint.href, {
-            body: {
+            form: {
                 source: source.href,
                 target: target.href,
                 source_is_ghost: true
             },
-            form: true,
             throwHttpErrors: false,
             maxRedirects: 10,
             followRedirect: true,
-            methodRewriting: false, // WARNING! this setting has a different meaning in got v12!
             timeout: 10000
         });
+
         if (response.statusCode >= 200 && response.statusCode < 300) {
             return;
         }
+        
         throw new errors.BadRequestError({
             message: 'Webmention sending failed with status code ' + response.statusCode,
             statusCode: response.statusCode

ghost/webmentions/test/MentionDiscoveryService.test.js

@@ -37,22 +37,20 @@ describe('MentionDiscoveryService', function () {
         assert.equal(endpoint, null);
     });
 
-    // TODO: need to support redirects
+    it('Follows redirects', async function () {
-    // it('Follows redirects', async function () {
+        let url = new URL('http://redirector.io/');
+        let nextUrl = new URL('http://testpage.com/');
 
-    //     let url = new URL('http://redirector.io/');
+        nock(url.href)
-    //     let nextUrl = new URL('http://testpage.com/');
+            .intercept('/', 'HEAD')
+            .reply(301, undefined, {location: nextUrl.href})
+            .get('/')
+            .reply(200, '<link rel="webmention" href="http://valid.site.org" />Very cool site', {'content-type': 'text/html'});
 
-    //     let mockRedirect = nock(url.href)
+        let endpoint = await service.getEndpoint(url);
-    //         .intercept("/", "HEAD")
-    //         .reply(301, undefined, { location: nextUrl.href })
-    //         .get('/')
-    //         .reply(200, '<link rel="webmention" href="http://valid.site.org" />Very cool site', { 'content-type': 'text/html' });
 
-    //     let endpoint = await service.getEndpoint(url);
+        assert(endpoint instanceof URL);
-
+    });
-    //     assert(endpoint instanceof URL);
-    // });
 
     describe('Can parse headers', function () {
         it('Returns null for a valid non-html site', async function () {

ghost/webmentions/test/MentionSendingService.test.js

@@ -380,6 +380,9 @@ describe('MentionSendingService', function () {
 
     describe('send', function () {
         it('Can handle 202 accepted responses', async function () {
+            const source = new URL('https://example.com/source');
+            const target = new URL('https://target.com/target');
+            const endpoint = new URL('https://example.org/webmentions-test');
             const scope = nock('https://example.org')
                 .persist()
                 .post('/webmentions-test', `source=${encodeURIComponent('https://example.com/source')}&target=${encodeURIComponent('https://target.com/target')}&source_is_ghost=true`)
@@ -387,14 +390,17 @@ describe('MentionSendingService', function () {
 
             const service = new MentionSendingService({externalRequest});
             await service.send({
-                source: new URL('https://example.com/source'),
+                source: source,
-                target: new URL('https://target.com/target'),
+                target: target,
-                endpoint: new URL('https://example.org/webmentions-test')
+                endpoint: endpoint
             });
             assert(scope.isDone());
         });
 
         it('Can handle 201 created responses', async function () {
+            const source = new URL('https://example.com/source');
+            const target = new URL('https://target.com/target');
+            const endpoint = new URL('https://example.org/webmentions-test');
             const scope = nock('https://example.org')
                 .persist()
                 .post('/webmentions-test', `source=${encodeURIComponent('https://example.com/source')}&target=${encodeURIComponent('https://target.com/target')}&source_is_ghost=true`)
@@ -402,9 +408,9 @@ describe('MentionSendingService', function () {
 
             const service = new MentionSendingService({externalRequest});
             await service.send({
-                source: new URL('https://example.com/source'),
+                source: source,
-                target: new URL('https://target.com/target'),
+                target: target,
-                endpoint: new URL('https://example.org/webmentions-test')
+                endpoint: endpoint
             });
             assert(scope.isDone());
         });
@@ -439,6 +445,28 @@ describe('MentionSendingService', function () {
             assert(scope.isDone());
         });
 
+        it('Can handle redirect responses', async function () {
+            const scope = nock('https://example.org')
+                .persist()
+                .post('/webmentions-test')
+                .reply(302, '', {
+                    Location: 'https://example.org/webmentions-test-2'
+                });
+            const scope2 = nock('https://example.org')
+                .persist()
+                .post('/webmentions-test-2')
+                .reply(201);
+        
+            const service = new MentionSendingService({externalRequest});
+            await service.send({
+                source: new URL('https://example.com'),
+                target: new URL('https://example.com'),
+                endpoint: new URL('https://example.org/webmentions-test')
+            });
+            assert(scope.isDone());
+            assert(scope2.isDone());
+        });
+
         it('Can handle network errors', async function () {
             const scope = nock('https://example.org')
                 .persist()
@@ -454,41 +482,6 @@ describe('MentionSendingService', function () {
             assert(scope.isDone());
         });
 
-        // Redirects are currently not supported by got for POST requests!
-        //it('Can handle redirect responses', async function () {
-        //    const scope = nock('https://example.org')
-        //        .persist()
-        //        .post('/webmentions-test')
-        //        .reply(302, '', {
-        //            headers: {
-        //                Location: 'https://example.org/webmentions-test-2'
-        //            }
-        //        });
-        //    const scope2 = nock('https://example.org')
-        //        .persist()
-        //        .post('/webmentions-test-2')
-        //        .reply(201);
-        //
-        //    const service = new MentionSendingService({externalRequest});
-        //    await service.send({
-        //        source: new URL('https://example.com'),
-        //        target: new URL('https://example.com'),
-        //        endpoint: new URL('https://example.org/webmentions-test')
-        //    });
-        //    assert(scope.isDone());
-        //    assert(scope2.isDone());
-        //});
-        // TODO: also check if we don't follow private IPs after redirects
-
-        it('Does not send to private IP', async function () {
-            const service = new MentionSendingService({externalRequest});
-            await assert.rejects(service.send({
-                source: new URL('https://example.com/source'),
-                target: new URL('https://target.com/target'),
-                endpoint: new URL('http://localhost/webmentions')
-            }), /non-permitted private IP/);
-        });
-
         it('Does not send to private IP behind DNS', async function () {
             // Test that we don't make a request when a domain resolves to a private IP
             // domaincontrol.com -> 127.0.0.1