diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 564103a0c8..616c6368b9 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -1,34 +1,11 @@ const express = require('express') -const validator = require('validator') const { startDownUpload } = require('../helpers/upload') const { prepareStream } = require('../helpers/utils') +const { validateURL } = require('../helpers/request') const { getURLMeta, getProtectedGot } = require('../helpers/request') const logger = require('../logger') -/** - * Validates that the download URL is secure - * - * @param {string} url the url to validate - * @param {boolean} ignoreTld whether to allow local addresses - */ -const validateURL = (url, ignoreTld) => { - if (!url) { - return false - } - - const validURLOpts = { - protocols: ['http', 'https'], - require_protocol: true, - require_tld: !ignoreTld, - } - if (!validator.isURL(url, validURLOpts)) { - return false - } - - return true -} - /** * @callback downloadCallback * @param {Error} err @@ -45,6 +22,8 @@ const validateURL = (url, ignoreTld) => { * @returns {Promise} */ const downloadURL = async (url, blockLocalIPs, traceId) => { + // TODO in next major, rename all blockLocalIPs to allowLocalUrls and invert the bool, to make it consistent + // see discussion https://github.com/transloadit/uppy/pull/4554/files#r1268677162 try { const protectedGot = getProtectedGot({ url, blockLocalIPs }) const stream = protectedGot.stream.get(url, { responseType: 'json' }) diff --git a/packages/@uppy/companion/src/server/helpers/request.js b/packages/@uppy/companion/src/server/helpers/request.js index 0fe52cefca..74fa1bfadb 100644 --- a/packages/@uppy/companion/src/server/helpers/request.js +++ b/packages/@uppy/companion/src/server/helpers/request.js @@ -7,6 +7,7 @@ const ipaddr = require('ipaddr.js') const got = require('got').default const path = require('node:path') const contentDisposition = require('content-disposition') +const validator = require('validator') const logger = require('../logger') @@ -46,7 +47,32 @@ module.exports.getRedirectEvaluator = (rawRequestURL, isEnabled) => { } /** - * Returns http Agent that will prevent requests to private IPs (to preven SSRF) + * Validates that the download URL is secure + * + * @param {string} url the url to validate + * @param {boolean} allowLocalUrls whether to allow local addresses + */ +const validateURL = (url, allowLocalUrls) => { + if (!url) { + return false + } + + const validURLOpts = { + protocols: ['http', 'https'], + require_protocol: true, + require_tld: !allowLocalUrls, + } + if (!validator.isURL(url, validURLOpts)) { + return false + } + + return true +} + +module.exports.validateURL = validateURL + +/** + * Returns http Agent that will prevent requests to private IPs (to prevent SSRF) */ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => { function dnsLookup (hostname, options, callback) { @@ -95,6 +121,8 @@ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => { return protocol.startsWith('https') ? HttpsAgent : HttpAgent } +module.exports.getProtectedHttpAgent = getProtectedHttpAgent + function getProtectedGot ({ url, blockLocalIPs }) { const HttpAgent = getProtectedHttpAgent({ protocol: 'http', blockLocalIPs }) const HttpsAgent = getProtectedHttpAgent({ protocol: 'https', blockLocalIPs }) diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js index 2268b0aec4..074d18d156 100644 --- a/packages/@uppy/companion/src/server/provider/Provider.js +++ b/packages/@uppy/companion/src/server/provider/Provider.js @@ -4,11 +4,12 @@ class Provider { /** * - * @param {object} options + * @param {{providerName: string, allowLocalUrls: boolean}} options */ - constructor (options) { // eslint-disable-line no-unused-vars + constructor ({ allowLocalUrls }) { // Some providers might need cookie auth for the thumbnails fetched via companion this.needsCookieAuth = false + this.allowLocalUrls = allowLocalUrls return this } diff --git a/packages/@uppy/companion/src/server/provider/index.js b/packages/@uppy/companion/src/server/provider/index.js index 0e06f5f459..e49aff660d 100644 --- a/packages/@uppy/companion/src/server/provider/index.js +++ b/packages/@uppy/companion/src/server/provider/index.js @@ -53,7 +53,8 @@ module.exports.getProviderMiddleware = (providers) => { const middleware = (req, res, next, providerName) => { const ProviderClass = providers[providerName] if (ProviderClass && validOptions(req.companion.options)) { - req.companion.provider = new ProviderClass({ providerName }) + const { allowLocalUrls } = req.companion.options + req.companion.provider = new ProviderClass({ providerName, allowLocalUrls }) req.companion.providerClass = ProviderClass if (isOAuthProvider(ProviderClass.authProvider)) {