From 84a25372f8f66d5b5faeb577bfcb44c86e9df6af Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 2 Apr 2024 03:22:59 +0500 Subject: [PATCH 01/28] Rename proxy from index.ts to proxy.ts --- proxy/{index.ts => proxy.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename proxy/{index.ts => proxy.ts} (100%) diff --git a/proxy/index.ts b/proxy/proxy.ts similarity index 100% rename from proxy/index.ts rename to proxy/proxy.ts From b82151ee06fe81e82d610691805b32d3f1f30554 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 2 Apr 2024 03:23:24 +0500 Subject: [PATCH 02/28] Rename proxy from index.ts to proxy.ts --- proxy/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/package.json b/proxy/package.json index 821921e5..e39cb9ea 100644 --- a/proxy/package.json +++ b/proxy/package.json @@ -3,7 +3,7 @@ "private": true, "type": "module", "scripts": { - "start": "tsx ./index.ts" + "start": "tsx ./proxy.ts" }, "dependencies": { "tsx": "4.7.2" From b0ddaeebdd97d8ced035355427d0e4d91235ca76 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 2 Apr 2024 04:18:05 +0500 Subject: [PATCH 03/28] Update README.md file with usage example --- proxy/README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/proxy/README.md b/proxy/README.md index 87830029..ec0f6373 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -3,3 +3,18 @@ HTTP-server to proxy all RSS fetching request from web client. User could use it to bypass censorship or to try web client before they install upcoming extension (to bypass CORS limit of web app). + +## Usage + +### Start proxy + +```shell +pnpm start +# // Proxy server running on port 5284 +``` + +### Make a request + +```shell +curl localhost:5284/https://hplush.dev +``` From 1a1a703e7c8be810b0d196c4c70c40487c89339d Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 2 Apr 2024 04:22:35 +0500 Subject: [PATCH 04/28] Update README.md file with issue link --- proxy/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/proxy/README.md b/proxy/README.md index ec0f6373..f863cd7b 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -18,3 +18,9 @@ pnpm start ```shell curl localhost:5284/https://hplush.dev ``` + +## Additional info: + +### Discussing security and protection from span + +- https://github.com/hplush/slowreader/issues/100 From 3c600722fc11ee84b298b3d96cc7e15e6b024fc4 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 2 Apr 2024 04:24:23 +0500 Subject: [PATCH 05/28] Implement basic security measures in proxy.ts - Only GET - Remove cookie and host headers - Ban requests to obvious local ips - Ban requests not from PRODUCTION in PRODUCTION mode --- proxy/proxy.ts | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 9abbe832..7fbf68b5 100755 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -1,11 +1,47 @@ import { createServer } from 'node:http' import { styleText } from 'node:util' +const PORT = 5284 +const IS_PRODUCTION = process.env.mode === 'production' +const PRODUCTION_DOMAIN_SUFFIX = '.slowreader.app' + const server = createServer(async (req, res) => { + // Todo (@toplenboren) what about other protocols? We do not want ssh for example + // Todo (@toplenboren) what about adding a rate limiter? let url = decodeURIComponent(req.url!.slice(1)) let sent = false try { + // Other requests are used to modify the data, and we do not need them to load RSS + if (req.method !== 'GET') { + throw new Error('Only GET requests are allowed.') + } + + // In production mode we only allow request from production domain + if (IS_PRODUCTION) { + const origin = req.headers.origin + if (!origin || !origin.endsWith(PRODUCTION_DOMAIN_SUFFIX)) { + throw new Error('Unauthorized origin.') + } + } + + // Todo (@toplenboren) what to do with ipv6? + // Todo (@toplenboren) what about those: https://en.wikipedia.org/wiki/Reserved_IP_addresses + // Do not allow requests to addresses that are reserved: + // 127.* + // 192.168.*,* + const localhostRegex = + /^(127\.\d{1,3}\.\d{1,3}\.\d{1,3}|192\.168\.\d{1,3}\.\d{1,3})$/ + const requestUrl = new URL(url) + if (localhostRegex.test(requestUrl.hostname)) { + throw new Error('Requests to localhost IP addresses are not allowed.') + } + + // Remove all cookie headers and host header from request + delete req.headers.cookie + delete req.headers['set-cookie'] + delete req.headers.host + let proxy = await fetch(url, { headers: { ...(req.headers as HeadersInit), @@ -36,8 +72,8 @@ const server = createServer(async (req, res) => { } }) -server.listen(5284, () => { +server.listen(PORT, () => { process.stderr.write( - styleText('green', 'Proxy server running on port 5284\n') + styleText('green', `Proxy server running on port ${PORT}`) ) }) From 2819f15867a1f5e5946d2cdd449a1c32d6e3592c Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 2 Apr 2024 04:25:13 +0500 Subject: [PATCH 06/28] Add ExceptionCaughtLocallyJS since it is a pattern in proxy.js --- proxy/proxy.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 7fbf68b5..3adb84c6 100755 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -1,3 +1,5 @@ +// noinspection ExceptionCaughtLocallyJS + import { createServer } from 'node:http' import { styleText } from 'node:util' From 0aec2c0a0382988c87bc1f177eb3b33f766f7155 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Mon, 8 Apr 2024 18:24:48 +0500 Subject: [PATCH 07/28] rename proxy.js back to index.js --- proxy/{proxy.ts => index.ts} | 0 proxy/package.json | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename proxy/{proxy.ts => index.ts} (100%) diff --git a/proxy/proxy.ts b/proxy/index.ts similarity index 100% rename from proxy/proxy.ts rename to proxy/index.ts diff --git a/proxy/package.json b/proxy/package.json index e39cb9ea..821921e5 100644 --- a/proxy/package.json +++ b/proxy/package.json @@ -3,7 +3,7 @@ "private": true, "type": "module", "scripts": { - "start": "tsx ./proxy.ts" + "start": "tsx ./index.ts" }, "dependencies": { "tsx": "4.7.2" From bb960b8f27a94416003e83016990e9586b962de3 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 00:44:04 +0500 Subject: [PATCH 08/28] add tests to proxy module --- package.json | 3 +- pnpm-lock.yaml | 7 +++ proxy/index.ts | 77 ++----------------------------- proxy/package.json | 9 +++- proxy/proxy.ts | 74 ++++++++++++++++++++++++++++++ proxy/test/proxy.test.ts | 99 ++++++++++++++++++++++++++++++++++++++++ proxy/test/utils.ts | 73 +++++++++++++++++++++++++++++ 7 files changed, 266 insertions(+), 76 deletions(-) create mode 100644 proxy/proxy.ts create mode 100644 proxy/test/proxy.test.ts create mode 100644 proxy/test/utils.ts diff --git a/package.json b/package.json index e6c01984..9055c8f2 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,8 @@ "core", "server", "web", - "loader-tests" + "loader-tests", + "proxy" ], "devDependencies": { "@logux/eslint-config": "53.0.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 77e129ac..c8638fd2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -167,9 +167,16 @@ importers: proxy: dependencies: + node-fetch: + specifier: 3.3.2 + version: 3.3.2 tsx: specifier: 4.7.2 version: 4.7.2 + devDependencies: + c8: + specifier: 9.1.0 + version: 9.1.0 server: dependencies: diff --git a/proxy/index.ts b/proxy/index.ts index 3adb84c6..669eb93b 100755 --- a/proxy/index.ts +++ b/proxy/index.ts @@ -1,81 +1,10 @@ -// noinspection ExceptionCaughtLocallyJS - -import { createServer } from 'node:http' import { styleText } from 'node:util' +import proxy from './proxy.js' const PORT = 5284 -const IS_PRODUCTION = process.env.mode === 'production' -const PRODUCTION_DOMAIN_SUFFIX = '.slowreader.app' - -const server = createServer(async (req, res) => { - // Todo (@toplenboren) what about other protocols? We do not want ssh for example - // Todo (@toplenboren) what about adding a rate limiter? - let url = decodeURIComponent(req.url!.slice(1)) - let sent = false - - try { - // Other requests are used to modify the data, and we do not need them to load RSS - if (req.method !== 'GET') { - throw new Error('Only GET requests are allowed.') - } - - // In production mode we only allow request from production domain - if (IS_PRODUCTION) { - const origin = req.headers.origin - if (!origin || !origin.endsWith(PRODUCTION_DOMAIN_SUFFIX)) { - throw new Error('Unauthorized origin.') - } - } - - // Todo (@toplenboren) what to do with ipv6? - // Todo (@toplenboren) what about those: https://en.wikipedia.org/wiki/Reserved_IP_addresses - // Do not allow requests to addresses that are reserved: - // 127.* - // 192.168.*,* - const localhostRegex = - /^(127\.\d{1,3}\.\d{1,3}\.\d{1,3}|192\.168\.\d{1,3}\.\d{1,3})$/ - const requestUrl = new URL(url) - if (localhostRegex.test(requestUrl.hostname)) { - throw new Error('Requests to localhost IP addresses are not allowed.') - } - - // Remove all cookie headers and host header from request - delete req.headers.cookie - delete req.headers['set-cookie'] - delete req.headers.host - - let proxy = await fetch(url, { - headers: { - ...(req.headers as HeadersInit), - host: new URL(url).host - }, - method: req.method - }) - - res.writeHead(proxy.status, { - 'Access-Control-Allow-Headers': '*', - 'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE', - 'Access-Control-Allow-Origin': '*', - 'Content-Type': proxy.headers.get('content-type') ?? 'text/plain' - }) - sent = true - res.write(await proxy.text()) - res.end() - } catch (e) { - if (e instanceof Error) { - process.stderr.write(styleText('red', e.stack ?? e.message) + '\n') - if (!sent) { - res.writeHead(500, { 'Content-Type': 'text/plain' }) - res.end('Internal Server Error') - } - } else if (typeof e === 'string') { - process.stderr.write(styleText('red', e) + '\n') - } - } -}) -server.listen(PORT, () => { - process.stderr.write( +proxy.listen(PORT, () => { + process.stdout.write( styleText('green', `Proxy server running on port ${PORT}`) ) }) diff --git a/proxy/package.json b/proxy/package.json index 821921e5..5bf38ec8 100644 --- a/proxy/package.json +++ b/proxy/package.json @@ -3,9 +3,16 @@ "private": true, "type": "module", "scripts": { - "start": "tsx ./index.ts" + "start": "tsx ./index.ts", + "test": "FORCE_COLOR=1 pnpm run /^test:/", + "test:coverage": "c8 pnpm bnt", + "clean:coverage": "rm -rf coverage" }, "dependencies": { + "node-fetch": "3.3.2", "tsx": "4.7.2" + }, + "devDependencies": { + "c8": "9.1.0" } } diff --git a/proxy/proxy.ts b/proxy/proxy.ts new file mode 100644 index 00000000..3eae2a65 --- /dev/null +++ b/proxy/proxy.ts @@ -0,0 +1,74 @@ +import { createServer } from 'node:http' +import { styleText } from 'node:util' + +const IS_PRODUCTION = process.env.mode === 'production' +const PRODUCTION_DOMAIN_SUFFIX = '.slowreader.app' + +const proxy = createServer(async (req, res) => { + // Todo (@toplenboren) what about other protocols? We do not want ssh for example + // Todo (@toplenboren) what about adding a rate limiter? + let url = decodeURIComponent(req.url!.slice(1)) + let sent = false + + try { + // Other requests are typically used to modify the data, and we do not typically need them to load RSS + if (req.method !== 'GET') { + throw new Error('Only GET requests are allowed.') + } + + // In production mode we only allow request from production domain + if (IS_PRODUCTION) { + const origin = req.headers.origin + if (!origin || !origin.endsWith(PRODUCTION_DOMAIN_SUFFIX)) { + throw new Error('Unauthorized origin.') + } + } + + // Todo (@toplenboren) what to do with ipv6? + // Todo (@toplenboren) what about those: https://en.wikipedia.org/wiki/Reserved_IP_addresses + // Do not allow requests to addresses that are reserved: + // 127.* + // 192.168.*,* + const localhostRegex = + /^(127\.\d{1,3}\.\d{1,3}\.\d{1,3}|192\.168\.\d{1,3}\.\d{1,3})$/ + const requestUrl = new URL(url) + if (localhostRegex.test(requestUrl.hostname)) { + throw new Error('Requests to localhost IP addresses are not allowed.') + } + + // Remove all cookie headers and host header from request + delete req.headers.cookie + delete req.headers['set-cookie'] + delete req.headers.host + + let proxy = await fetch(url, { + headers: { + ...(req.headers as HeadersInit), + host: new URL(url).host + }, + method: req.method + }) + + res.writeHead(proxy.status, { + 'Access-Control-Allow-Headers': '*', + 'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE', + 'Access-Control-Allow-Origin': '*', + 'Content-Type': proxy.headers.get('content-type') ?? 'text/plain' + }) + sent = true + res.write(await proxy.text()) + res.end() + } catch (e) { + if (e instanceof Error) { + process.stderr.write(styleText('red', e.stack ?? e.message) + '\n') + if (!sent) { + res.writeHead(500, { 'Content-Type': 'text/plain' }) + res.end('Internal Server Error') + } + } else if (typeof e === 'string') { + process.stderr.write(styleText('red', e) + '\n') + } + } +}) + +export default proxy diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts new file mode 100644 index 00000000..3f85854a --- /dev/null +++ b/proxy/test/proxy.test.ts @@ -0,0 +1,99 @@ +import { before, test, describe } from 'node:test' +import { equal } from 'node:assert' +import proxy from '../proxy.js' +import fetch from 'node-fetch' +import { createServer } from 'node:http' +import { initTestHttpServer, getTestHttpServer } from './utils.js' + +/** + * Proxy is tested with a service that outputs request and response data. + * + * -> -> + */ +const targetServer = createServer((req, res) => { + res.writeHead(200, { + 'Content-Type': 'text/json' + }) + + res.end( + JSON.stringify({ + response: 'ok', + request: { headers: req.headers, method: req.method, url: req.url } + }) + ) +}) + +describe('proxy tests', async () => { + await initTestHttpServer('proxy', proxy, { port: 3999 }) + await initTestHttpServer('target', targetServer, { port: 4000 }) + + let proxyServerUrl = '' + let targetServerUrl = '' + + before(() => { + const proxy = getTestHttpServer('proxy') + if (proxy) { + proxyServerUrl = proxy.baseUrl + } + + const target = getTestHttpServer('target') + if (target) { + targetServerUrl = target.baseUrl + } + + if (!target || !proxy) { + throw new Error( + "Couldn't set up target server or proxy server. Something is misconfigured. Please check out 'proxy.test.js'" + ) + } + + console.log(`Proxy server running on ${proxyServerUrl}`) + console.log(`Target test server running on ${targetServerUrl}`) + }) + + await test('proxy works', async () => { + const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`) + const parsedResponse = await response.json() + // @ts-ignore + equal(parsedResponse?.response, 'ok') + }) + + await describe('security', async () => { + await test('can use only GET ', async () => { + const methods = ['DELETE', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'TRACE'] + + for (const method of methods) { + await test(`can not use ${method}`, async () => { + const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { + method + }) + equal(response.status, 500) + }) + } + }) + + await describe('cookies', async () => { + await test('proxy clears set-cookie header', async () => { + const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { + headers: { 'set-cookie': 'accessToken=1234abc; userId=1234' } + }) + const parsedResponse = await response.json() + // @ts-ignore + equal(parsedResponse?.['set-cookie'], undefined) + // @ts-ignore + equal(parsedResponse?.['cookie'], undefined) + }) + + await test('proxy clears cookie header', async () => { + const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { + headers: { cookie: 'accessToken=1234abc; userId=1234' } + }) + const parsedResponse = await response.json() + // @ts-ignore + equal(parsedResponse?.['set-cookie'], undefined) + // @ts-ignore + equal(parsedResponse?.cookie, undefined) + }) + }) + }) +}) diff --git a/proxy/test/utils.ts b/proxy/test/utils.ts new file mode 100644 index 00000000..8aa37f5c --- /dev/null +++ b/proxy/test/utils.ts @@ -0,0 +1,73 @@ +import http from 'http' +import { after, before } from 'node:test' + +const __testServers: { + [key: string]: { + server: http.Server + address: string + port: number + baseUrl: string + } +} = {} + +/** + * Use this function if you need to init local http server for testing. Express is supported. Server will be closed automatically after tests finish + * + * @example + * Usage: in your *.test.ts file + * + * await describe('test', async () => { + * await initTestHttpServer('test', ...) + * + * await test('test', async () => { + * fetch(getTestHttpServer('test')) + * }) + * }) + * + * @param name + * @param server + * @param opts + */ +export async function initTestHttpServer( + name: string, + server: any, + opts?: { protocol?: string; port?: number } +) { + await before(async () => { + const port = opts?.port || 0 + const protocol = opts?.protocol || 'http' + + // port=0 is + const testServerInstance = await server.listen(port) + // @ts-ignore + const addressInfo: { address: string; family: string; port: number } = + testServerInstance.address() + + const testServerAddress = + addressInfo?.address === '::' ? 'localhost' : addressInfo.address + const testServerPort = addressInfo?.port + + __testServers[name] = { + server: testServerInstance, + address: testServerAddress, + port: testServerPort, + baseUrl: `${protocol}://${testServerAddress}:${testServerPort}` + } + }) + + await after(async () => { + if (__testServers[name]) { + // @ts-ignore + __testServers[name].server.close() + delete __testServers[name] + } + }) +} + +/** + * Use it to get server initialized by initTestHttpServer function + * @param name + */ +export function getTestHttpServer(name: string) { + return __testServers[name] +} From 7873d0dcc7b60c5c311914d61d6199816bdd0d87 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 02:13:54 +0500 Subject: [PATCH 09/28] add query params parsing, proxy server configuration, martian ip block --- proxy/index.ts | 10 ++- proxy/package.json | 2 +- proxy/proxy.ts | 165 +++++++++++++++++++++++++-------------- proxy/test/proxy.test.ts | 71 +++++++++++++++-- 4 files changed, 180 insertions(+), 68 deletions(-) diff --git a/proxy/index.ts b/proxy/index.ts index 669eb93b..7aaf25c4 100755 --- a/proxy/index.ts +++ b/proxy/index.ts @@ -1,8 +1,16 @@ import { styleText } from 'node:util' -import proxy from './proxy.js' +import createProxyServer from './proxy.js' const PORT = 5284 +const IS_PRODUCTION = process.env.mode === 'production' +const PRODUCTION_DOMAIN_SUFFIX = '.slowreader.app' + +const proxy = createProxyServer({ + isProduction: IS_PRODUCTION, + productionDomainSuffix: PRODUCTION_DOMAIN_SUFFIX +}) + proxy.listen(PORT, () => { process.stdout.write( styleText('green', `Proxy server running on port ${PORT}`) diff --git a/proxy/package.json b/proxy/package.json index 5bf38ec8..055a4416 100644 --- a/proxy/package.json +++ b/proxy/package.json @@ -9,7 +9,7 @@ "clean:coverage": "rm -rf coverage" }, "dependencies": { - "node-fetch": "3.3.2", + "martian-cidr": "2.0.3", "tsx": "4.7.2" }, "devDependencies": { diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 3eae2a65..983887ec 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -1,74 +1,119 @@ import { createServer } from 'node:http' import { styleText } from 'node:util' +// @ts-ignore +import isMartianIP from 'martian-cidr' -const IS_PRODUCTION = process.env.mode === 'production' -const PRODUCTION_DOMAIN_SUFFIX = '.slowreader.app' +/** + * Creates proxy server + * + * isProduction - main toggle for production features: + * - will allow only request that match productionDomainSuffix + * - will debounce requests with + * + * silentMode - will silent the output. Useful for testing + * + * hostnameWhitelist - Sometimes you need to allow request to certain ips like localhost for testing purposes, like localhost + * + * productionDomainSuffix - if isProduction, then only request from origins that match this param are allowed + * + * @example + * const p = createProxyServer() + * p.listen(5555).then(() => console.log('running on 5555')) + * + * @param { isProduction, silentMode, hostnameWhitelist, productionDomainSuffix } config + */ +const createProxyServer = ( + config: { + isProduction?: boolean + silentMode?: boolean + hostnameWhitelist?: Array + productionDomainSuffix?: string + } = {} +) => { + const isProduction = config.isProduction || false + const silentMode = config.silentMode || false + const hostnameWhitelist = config.hostnameWhitelist || [] + const productionDomainSuffix = config.productionDomainSuffix || '' -const proxy = createServer(async (req, res) => { - // Todo (@toplenboren) what about other protocols? We do not want ssh for example - // Todo (@toplenboren) what about adding a rate limiter? - let url = decodeURIComponent(req.url!.slice(1)) - let sent = false + return createServer(async (req, res) => { + // Todo (@toplenboren) what about adding a rate limiter? + let url = decodeURIComponent(req.url!.slice(1)) + let sent = false - try { - // Other requests are typically used to modify the data, and we do not typically need them to load RSS - if (req.method !== 'GET') { - throw new Error('Only GET requests are allowed.') - } + try { + // Only http or https protocols are allowed + if (!url.startsWith('http://') && !url.startsWith('https://')) { + throw new Error('Only http or https requests are allowed.') + } - // In production mode we only allow request from production domain - if (IS_PRODUCTION) { - const origin = req.headers.origin - if (!origin || !origin.endsWith(PRODUCTION_DOMAIN_SUFFIX)) { - throw new Error('Unauthorized origin.') + // Other requests are typically used to modify the data, and we do not typically need them to load RSS + if (req.method !== 'GET') { + throw new Error('Only GET requests are allowed.') } - } - // Todo (@toplenboren) what to do with ipv6? - // Todo (@toplenboren) what about those: https://en.wikipedia.org/wiki/Reserved_IP_addresses - // Do not allow requests to addresses that are reserved: - // 127.* - // 192.168.*,* - const localhostRegex = - /^(127\.\d{1,3}\.\d{1,3}\.\d{1,3}|192\.168\.\d{1,3}\.\d{1,3})$/ - const requestUrl = new URL(url) - if (localhostRegex.test(requestUrl.hostname)) { - throw new Error('Requests to localhost IP addresses are not allowed.') - } + // In production mode we only allow request from production domain + if (isProduction) { + const origin = req.headers.origin + if (!origin || !origin.endsWith(productionDomainSuffix)) { + throw new Error('Unauthorized origin.') + } + } + + const requestUrl = new URL(url) + if (!hostnameWhitelist.includes(requestUrl.hostname)) { + // Do not allow requests to addresses that are reserved: + // 127.* + // 192.168.*,* + // https://en.wikipedia.org/wiki/Reserved_IP_addresses + if (isMartianIP(requestUrl.hostname)) { + throw new Error( + 'Requests to IPs from local or reserved subnets are not allowed.' + ) + } + } - // Remove all cookie headers and host header from request - delete req.headers.cookie - delete req.headers['set-cookie'] - delete req.headers.host + // Remove all cookie headers and host header from request + delete req.headers.cookie + delete req.headers['set-cookie'] + delete req.headers.host - let proxy = await fetch(url, { - headers: { - ...(req.headers as HeadersInit), - host: new URL(url).host - }, - method: req.method - }) + let proxy = await fetch(url, { + headers: { + ...(req.headers as HeadersInit), + host: new URL(url).host + }, + method: req.method + }) - res.writeHead(proxy.status, { - 'Access-Control-Allow-Headers': '*', - 'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE', - 'Access-Control-Allow-Origin': '*', - 'Content-Type': proxy.headers.get('content-type') ?? 'text/plain' - }) - sent = true - res.write(await proxy.text()) - res.end() - } catch (e) { - if (e instanceof Error) { - process.stderr.write(styleText('red', e.stack ?? e.message) + '\n') - if (!sent) { - res.writeHead(500, { 'Content-Type': 'text/plain' }) - res.end('Internal Server Error') + res.writeHead(proxy.status, { + 'Access-Control-Allow-Headers': '*', + 'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE', + 'Access-Control-Allow-Origin': '*', + 'Content-Type': proxy.headers.get('content-type') ?? 'text/plain' + }) + sent = true + res.write(await proxy.text()) + res.end() + } catch (e) { + if (e instanceof Error) { + if (!silentMode) { + process.stderr.write(styleText('red', e.stack ?? e.message) + '\n') + } + if (!sent) { + res.writeHead(500, { 'Content-Type': 'text/plain' }) + res.end('Internal Server Error') + } + } else if (typeof e === 'string') { + if (!silentMode) { + process.stderr.write(styleText('red', e) + '\n') + } + if (!sent) { + res.writeHead(500, { 'Content-Type': 'text/plain' }) + res.end('Internal Server Error') + } } - } else if (typeof e === 'string') { - process.stderr.write(styleText('red', e) + '\n') } - } -}) + }) +} -export default proxy +export default createProxyServer diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts index 3f85854a..099746a3 100644 --- a/proxy/test/proxy.test.ts +++ b/proxy/test/proxy.test.ts @@ -1,9 +1,9 @@ import { before, test, describe } from 'node:test' import { equal } from 'node:assert' -import proxy from '../proxy.js' -import fetch from 'node-fetch' import { createServer } from 'node:http' import { initTestHttpServer, getTestHttpServer } from './utils.js' +import createProxyServer from '../proxy.js' +import { URL } from 'url' /** * Proxy is tested with a service that outputs request and response data. @@ -11,6 +11,11 @@ import { initTestHttpServer, getTestHttpServer } from './utils.js' * -> -> */ const targetServer = createServer((req, res) => { + const parsedUrl = new URL(req.url || '', `http://${req.headers.host}`) + + const queryParams = Object.fromEntries(parsedUrl.searchParams.entries()) + const requestPath = parsedUrl.pathname + res.writeHead(200, { 'Content-Type': 'text/json' }) @@ -18,13 +23,23 @@ const targetServer = createServer((req, res) => { res.end( JSON.stringify({ response: 'ok', - request: { headers: req.headers, method: req.method, url: req.url } + request: { + headers: req.headers, + method: req.method, + url: req.url, + queryParams, + requestPath + } }) ) }) describe('proxy tests', async () => { - await initTestHttpServer('proxy', proxy, { port: 3999 }) + await initTestHttpServer( + 'proxy', + createProxyServer({ silentMode: true, hostnameWhitelist: ['localhost'] }), + { port: 3999 } + ) await initTestHttpServer('target', targetServer, { port: 4000 }) let proxyServerUrl = '' @@ -55,14 +70,28 @@ describe('proxy tests', async () => { const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`) const parsedResponse = await response.json() // @ts-ignore + equal(response.status, 200) + equal(parsedResponse?.response, 'ok') + }) + + await test('proxy transfers query params and path', async () => { + const response = await fetch( + `${proxyServerUrl}/${targetServerUrl}/foo/bar?foo=bar&bar=foo` + ) + const parsedResponse = await response.json() + // @ts-ignore + equal(response.status, 200) equal(parsedResponse?.response, 'ok') + equal(parsedResponse?.request?.requestPath, '/foo/bar') + equal(parsedResponse?.request?.queryParams?.foo, 'bar') + equal(parsedResponse?.request?.queryParams?.bar, 'foo') }) await describe('security', async () => { await test('can use only GET ', async () => { - const methods = ['DELETE', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'TRACE'] + const forbiddenMethods = ['DELETE', 'HEAD', 'OPTIONS', 'POST', 'PUT'] - for (const method of methods) { + for (const method of forbiddenMethods) { await test(`can not use ${method}`, async () => { const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { method @@ -72,11 +101,39 @@ describe('proxy tests', async () => { } }) + await test('can use only http or https protocols', async () => { + const forbiddenProtocols = [ + 'ssh', + 'ftp', + 'sftp', + 'rdp', + 'arp', + 'smtp', + 'pop3', + 'imap', + 'dns', + 'dhcp', + 'snmp' + ] + + for (const protocol of forbiddenProtocols) { + await test(`can not use ${protocol}`, async () => { + const response = await fetch( + `${proxyServerUrl}/${targetServerUrl.replace('http', protocol)}`, + {} + ) + equal(response.status, 500) + }) + } + }) + await describe('cookies', async () => { await test('proxy clears set-cookie header', async () => { const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { headers: { 'set-cookie': 'accessToken=1234abc; userId=1234' } }) + + equal(response.status, 200) const parsedResponse = await response.json() // @ts-ignore equal(parsedResponse?.['set-cookie'], undefined) @@ -88,6 +145,8 @@ describe('proxy tests', async () => { const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { headers: { cookie: 'accessToken=1234abc; userId=1234' } }) + + equal(response.status, 200) const parsedResponse = await response.json() // @ts-ignore equal(parsedResponse?.['set-cookie'], undefined) From 16704da0177766be002485dfc44f491f6e8e6adc Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 02:14:05 +0500 Subject: [PATCH 10/28] fixes on review: do not document api --- proxy/README.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/proxy/README.md b/proxy/README.md index f863cd7b..4b6d0d49 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -13,12 +13,6 @@ pnpm start # // Proxy server running on port 5284 ``` -### Make a request - -```shell -curl localhost:5284/https://hplush.dev -``` - ## Additional info: ### Discussing security and protection from span From dcf9c3cc7011bc6c0e29d464cef004d4a4e74ac6 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 02:49:46 +0500 Subject: [PATCH 11/28] add failing local addr querying test --- proxy/proxy.ts | 11 +++++------ proxy/test/proxy.test.ts | 16 ++++++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 983887ec..675e4e6f 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -8,7 +8,6 @@ import isMartianIP from 'martian-cidr' * * isProduction - main toggle for production features: * - will allow only request that match productionDomainSuffix - * - will debounce requests with * * silentMode - will silent the output. Useful for testing * @@ -36,7 +35,6 @@ const createProxyServer = ( const productionDomainSuffix = config.productionDomainSuffix || '' return createServer(async (req, res) => { - // Todo (@toplenboren) what about adding a rate limiter? let url = decodeURIComponent(req.url!.slice(1)) let sent = false @@ -77,7 +75,7 @@ const createProxyServer = ( delete req.headers['set-cookie'] delete req.headers.host - let proxy = await fetch(url, { + let targetResponse = await fetch(url, { headers: { ...(req.headers as HeadersInit), host: new URL(url).host @@ -85,14 +83,15 @@ const createProxyServer = ( method: req.method }) - res.writeHead(proxy.status, { + res.writeHead(targetResponse.status, { 'Access-Control-Allow-Headers': '*', 'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE', 'Access-Control-Allow-Origin': '*', - 'Content-Type': proxy.headers.get('content-type') ?? 'text/plain' + 'Content-Type': + targetResponse.headers.get('content-type') ?? 'text/plain' }) sent = true - res.write(await proxy.text()) + res.write(await targetResponse.text()) res.end() } catch (e) { if (e instanceof Error) { diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts index 099746a3..e507a10b 100644 --- a/proxy/test/proxy.test.ts +++ b/proxy/test/proxy.test.ts @@ -37,10 +37,9 @@ const targetServer = createServer((req, res) => { describe('proxy tests', async () => { await initTestHttpServer( 'proxy', - createProxyServer({ silentMode: true, hostnameWhitelist: ['localhost'] }), - { port: 3999 } + createProxyServer({ silentMode: true, hostnameWhitelist: ['localhost'] }) ) - await initTestHttpServer('target', targetServer, { port: 4000 }) + await initTestHttpServer('target', targetServer) let proxyServerUrl = '' let targetServerUrl = '' @@ -61,9 +60,6 @@ describe('proxy tests', async () => { "Couldn't set up target server or proxy server. Something is misconfigured. Please check out 'proxy.test.js'" ) } - - console.log(`Proxy server running on ${proxyServerUrl}`) - console.log(`Target test server running on ${targetServerUrl}`) }) await test('proxy works', async () => { @@ -127,6 +123,14 @@ describe('proxy tests', async () => { } }) + await test('can not use proxy to query local address', async () => { + const response = await fetch( + `${proxyServerUrl}/${targetServerUrl.replace('localhost', '127.0.0.1')}`, + {} + ) + equal(response.status, 500) + }) + await describe('cookies', async () => { await test('proxy clears set-cookie header', async () => { const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { From df1f14ba3d7afff19f182b8e81da8cdeb065b039 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 02:51:30 +0500 Subject: [PATCH 12/28] fixes on ci: markdown capitalization fixes --- proxy/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proxy/README.md b/proxy/README.md index 4b6d0d49..5cc47f81 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -6,15 +6,15 @@ User could use it to bypass censorship or to try web client before they install ## Usage -### Start proxy +### Start Proxy ```shell pnpm start # // Proxy server running on port 5284 ``` -## Additional info: +## Additional Info: -### Discussing security and protection from span +### Discussing Security and Protection from Spam - https://github.com/hplush/slowreader/issues/100 From 9bd40f4eeb27011c74cc24c64beddd9f5095e0b9 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 02:52:25 +0500 Subject: [PATCH 13/28] fixes on review: shell -> sh --- proxy/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/README.md b/proxy/README.md index 5cc47f81..54446716 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -8,7 +8,7 @@ User could use it to bypass censorship or to try web client before they install ### Start Proxy -```shell +```sh pnpm start # // Proxy server running on port 5284 ``` From 14da548c4cbb47d314f22757794d02fa2a6006ae Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 03:05:21 +0500 Subject: [PATCH 14/28] fixes on review: from -> From --- proxy/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/README.md b/proxy/README.md index 54446716..0e4570e9 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -15,6 +15,6 @@ pnpm start ## Additional Info: -### Discussing Security and Protection from Spam +### Discussing Security and Protection From Spam - https://github.com/hplush/slowreader/issues/100 From d9e56be4d134a04ec5764c2470ce3bbdf6a8162f Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 03:06:36 +0500 Subject: [PATCH 15/28] fixes on review: fix documentation typo --- proxy/proxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 675e4e6f..60c0c436 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -11,7 +11,7 @@ import isMartianIP from 'martian-cidr' * * silentMode - will silent the output. Useful for testing * - * hostnameWhitelist - Sometimes you need to allow request to certain ips like localhost for testing purposes, like localhost + * hostnameWhitelist - Sometimes you need to allow request to certain ips like localhost for testing purposes. * * productionDomainSuffix - if isProduction, then only request from origins that match this param are allowed * From 49e3721f3646c77d8131b30a201bbe4f1e97f4ec Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 03:14:02 +0500 Subject: [PATCH 16/28] fixes on review: lint code --- proxy/index.ts | 1 + proxy/proxy.ts | 22 +++++++-------- proxy/test/proxy.test.ts | 59 ++++++++++++++++++++-------------------- proxy/test/utils.ts | 26 +++++++++--------- 4 files changed, 55 insertions(+), 53 deletions(-) diff --git a/proxy/index.ts b/proxy/index.ts index 7aaf25c4..3a9f2fa5 100755 --- a/proxy/index.ts +++ b/proxy/index.ts @@ -1,4 +1,5 @@ import { styleText } from 'node:util' + import createProxyServer from './proxy.js' const PORT = 5284 diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 60c0c436..fc4c324a 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -1,7 +1,7 @@ -import { createServer } from 'node:http' -import { styleText } from 'node:util' // @ts-ignore import isMartianIP from 'martian-cidr' +import { createServer } from 'node:http' +import { styleText } from 'node:util' /** * Creates proxy server @@ -23,16 +23,16 @@ import isMartianIP from 'martian-cidr' */ const createProxyServer = ( config: { + hostnameWhitelist?: string[] isProduction?: boolean - silentMode?: boolean - hostnameWhitelist?: Array productionDomainSuffix?: string + silentMode?: boolean } = {} ) => { - const isProduction = config.isProduction || false - const silentMode = config.silentMode || false - const hostnameWhitelist = config.hostnameWhitelist || [] - const productionDomainSuffix = config.productionDomainSuffix || '' + let isProduction = config.isProduction || false + let silentMode = config.silentMode || false + let hostnameWhitelist = config.hostnameWhitelist || [] + let productionDomainSuffix = config.productionDomainSuffix || '' return createServer(async (req, res) => { let url = decodeURIComponent(req.url!.slice(1)) @@ -51,13 +51,13 @@ const createProxyServer = ( // In production mode we only allow request from production domain if (isProduction) { - const origin = req.headers.origin - if (!origin || !origin.endsWith(productionDomainSuffix)) { + let origin = req.headers.origin + if (!origin?.endsWith(productionDomainSuffix)) { throw new Error('Unauthorized origin.') } } - const requestUrl = new URL(url) + let requestUrl = new URL(url) if (!hostnameWhitelist.includes(requestUrl.hostname)) { // Do not allow requests to addresses that are reserved: // 127.* diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts index e507a10b..11ae38ae 100644 --- a/proxy/test/proxy.test.ts +++ b/proxy/test/proxy.test.ts @@ -1,9 +1,10 @@ -import { before, test, describe } from 'node:test' import { equal } from 'node:assert' import { createServer } from 'node:http' -import { initTestHttpServer, getTestHttpServer } from './utils.js' +import { before, describe, test } from 'node:test' +import { URL } from 'node:url' + import createProxyServer from '../proxy.js' -import { URL } from 'url' +import { getTestHttpServer, initTestHttpServer } from './utils.js' /** * Proxy is tested with a service that outputs request and response data. @@ -11,10 +12,10 @@ import { URL } from 'url' * -> -> */ const targetServer = createServer((req, res) => { - const parsedUrl = new URL(req.url || '', `http://${req.headers.host}`) + let parsedUrl = new URL(req.url || '', `http://${req.headers.host}`) - const queryParams = Object.fromEntries(parsedUrl.searchParams.entries()) - const requestPath = parsedUrl.pathname + let queryParams = Object.fromEntries(parsedUrl.searchParams.entries()) + let requestPath = parsedUrl.pathname res.writeHead(200, { 'Content-Type': 'text/json' @@ -22,14 +23,14 @@ const targetServer = createServer((req, res) => { res.end( JSON.stringify({ - response: 'ok', request: { headers: req.headers, method: req.method, - url: req.url, queryParams, - requestPath - } + requestPath, + url: req.url + }, + response: 'ok' }) ) }) @@ -37,7 +38,7 @@ const targetServer = createServer((req, res) => { describe('proxy tests', async () => { await initTestHttpServer( 'proxy', - createProxyServer({ silentMode: true, hostnameWhitelist: ['localhost'] }) + createProxyServer({ hostnameWhitelist: ['localhost'], silentMode: true }) ) await initTestHttpServer('target', targetServer) @@ -45,12 +46,12 @@ describe('proxy tests', async () => { let targetServerUrl = '' before(() => { - const proxy = getTestHttpServer('proxy') + let proxy = getTestHttpServer('proxy') if (proxy) { proxyServerUrl = proxy.baseUrl } - const target = getTestHttpServer('target') + let target = getTestHttpServer('target') if (target) { targetServerUrl = target.baseUrl } @@ -63,18 +64,18 @@ describe('proxy tests', async () => { }) await test('proxy works', async () => { - const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`) - const parsedResponse = await response.json() + let response = await fetch(`${proxyServerUrl}/${targetServerUrl}`) + let parsedResponse = await response.json() // @ts-ignore equal(response.status, 200) equal(parsedResponse?.response, 'ok') }) await test('proxy transfers query params and path', async () => { - const response = await fetch( + let response = await fetch( `${proxyServerUrl}/${targetServerUrl}/foo/bar?foo=bar&bar=foo` ) - const parsedResponse = await response.json() + let parsedResponse = await response.json() // @ts-ignore equal(response.status, 200) equal(parsedResponse?.response, 'ok') @@ -85,11 +86,11 @@ describe('proxy tests', async () => { await describe('security', async () => { await test('can use only GET ', async () => { - const forbiddenMethods = ['DELETE', 'HEAD', 'OPTIONS', 'POST', 'PUT'] + let forbiddenMethods = ['DELETE', 'HEAD', 'OPTIONS', 'POST', 'PUT'] - for (const method of forbiddenMethods) { + for (let method of forbiddenMethods) { await test(`can not use ${method}`, async () => { - const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { + let response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { method }) equal(response.status, 500) @@ -98,7 +99,7 @@ describe('proxy tests', async () => { }) await test('can use only http or https protocols', async () => { - const forbiddenProtocols = [ + let forbiddenProtocols = [ 'ssh', 'ftp', 'sftp', @@ -112,9 +113,9 @@ describe('proxy tests', async () => { 'snmp' ] - for (const protocol of forbiddenProtocols) { + for (let protocol of forbiddenProtocols) { await test(`can not use ${protocol}`, async () => { - const response = await fetch( + let response = await fetch( `${proxyServerUrl}/${targetServerUrl.replace('http', protocol)}`, {} ) @@ -124,7 +125,7 @@ describe('proxy tests', async () => { }) await test('can not use proxy to query local address', async () => { - const response = await fetch( + let response = await fetch( `${proxyServerUrl}/${targetServerUrl.replace('localhost', '127.0.0.1')}`, {} ) @@ -133,25 +134,25 @@ describe('proxy tests', async () => { await describe('cookies', async () => { await test('proxy clears set-cookie header', async () => { - const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { + let response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { headers: { 'set-cookie': 'accessToken=1234abc; userId=1234' } }) equal(response.status, 200) - const parsedResponse = await response.json() + let parsedResponse = await response.json() // @ts-ignore equal(parsedResponse?.['set-cookie'], undefined) // @ts-ignore - equal(parsedResponse?.['cookie'], undefined) + equal(parsedResponse?.cookie, undefined) }) await test('proxy clears cookie header', async () => { - const response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { + let response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { headers: { cookie: 'accessToken=1234abc; userId=1234' } }) equal(response.status, 200) - const parsedResponse = await response.json() + let parsedResponse = await response.json() // @ts-ignore equal(parsedResponse?.['set-cookie'], undefined) // @ts-ignore diff --git a/proxy/test/utils.ts b/proxy/test/utils.ts index 8aa37f5c..914c0658 100644 --- a/proxy/test/utils.ts +++ b/proxy/test/utils.ts @@ -1,12 +1,12 @@ -import http from 'http' +import type http from 'node:http' import { after, before } from 'node:test' const __testServers: { [key: string]: { - server: http.Server address: string - port: number baseUrl: string + port: number + server: http.Server } } = {} @@ -31,27 +31,27 @@ const __testServers: { export async function initTestHttpServer( name: string, server: any, - opts?: { protocol?: string; port?: number } + opts?: { port?: number; protocol?: string } ) { await before(async () => { - const port = opts?.port || 0 - const protocol = opts?.protocol || 'http' + let port = opts?.port || 0 + let protocol = opts?.protocol || 'http' // port=0 is - const testServerInstance = await server.listen(port) + let testServerInstance = await server.listen(port) // @ts-ignore - const addressInfo: { address: string; family: string; port: number } = + let addressInfo: { address: string; family: string; port: number } = testServerInstance.address() - const testServerAddress = - addressInfo?.address === '::' ? 'localhost' : addressInfo.address - const testServerPort = addressInfo?.port + let testServerAddress = + addressInfo.address === '::' ? 'localhost' : addressInfo.address + let testServerPort = addressInfo.port __testServers[name] = { - server: testServerInstance, address: testServerAddress, + baseUrl: `${protocol}://${testServerAddress}:${testServerPort}`, port: testServerPort, - baseUrl: `${protocol}://${testServerAddress}:${testServerPort}` + server: testServerInstance } }) From 3c3cc73b36212ae1c60779345840a63fc627e33f Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 03:21:47 +0500 Subject: [PATCH 17/28] fixes on review: add return types to getTestHttpServer and initTestHttpServer functions --- proxy/test/utils.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/proxy/test/utils.ts b/proxy/test/utils.ts index 914c0658..e5f7491a 100644 --- a/proxy/test/utils.ts +++ b/proxy/test/utils.ts @@ -1,13 +1,15 @@ import type http from 'node:http' import { after, before } from 'node:test' +interface TestServer { + address: string + baseUrl: string + port: number + server: http.Server +} + const __testServers: { - [key: string]: { - address: string - baseUrl: string - port: number - server: http.Server - } + [key: string]: TestServer } = {} /** @@ -32,8 +34,9 @@ export async function initTestHttpServer( name: string, server: any, opts?: { port?: number; protocol?: string } -) { - await before(async () => { +): Promise { + // + before(async () => { let port = opts?.port || 0 let protocol = opts?.protocol || 'http' @@ -55,7 +58,7 @@ export async function initTestHttpServer( } }) - await after(async () => { + after(async () => { if (__testServers[name]) { // @ts-ignore __testServers[name].server.close() @@ -68,6 +71,6 @@ export async function initTestHttpServer( * Use it to get server initialized by initTestHttpServer function * @param name */ -export function getTestHttpServer(name: string) { +export function getTestHttpServer(name: string): TestServer | undefined { return __testServers[name] } From 890f50bef9404810ff0e345eb2191c18225320c4 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Tue, 9 Apr 2024 03:22:06 +0500 Subject: [PATCH 18/28] fixes on review: add return type to createProxyServer --- proxy/proxy.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index fc4c324a..799c0064 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -1,5 +1,6 @@ // @ts-ignore import isMartianIP from 'martian-cidr' +import type http from 'node:http' import { createServer } from 'node:http' import { styleText } from 'node:util' @@ -28,7 +29,7 @@ const createProxyServer = ( productionDomainSuffix?: string silentMode?: boolean } = {} -) => { +): http.Server => { let isProduction = config.isProduction || false let silentMode = config.silentMode || false let hostnameWhitelist = config.hostnameWhitelist || [] From 54f263bd2d2d8d62614961e5b920c6897571b79c Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 01:30:11 +0500 Subject: [PATCH 19/28] fixes on review: update pnpm lockfile --- pnpm-lock.yaml | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c8638fd2..b503a981 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -167,9 +167,9 @@ importers: proxy: dependencies: - node-fetch: - specifier: 3.3.2 - version: 3.3.2 + martian-cidr: + specifier: 2.0.3 + version: 2.0.3 tsx: specifier: 4.7.2 version: 4.7.2 @@ -3460,6 +3460,10 @@ packages: resolution: {integrity: sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==} engines: {node: '>= 0.10'} + ipaddr.js@2.1.0: + resolution: {integrity: sha512-LlbxQ7xKzfBusov6UMi4MFpEg0m+mAm9xyNGEduwXMEDuf4WfzB/RZwMVYEd7IKGvh4IUkEXYxtAVu9T3OelJQ==} + engines: {node: '>= 10'} + is-absolute-url@2.1.0: resolution: {integrity: sha512-vOx7VprsKyllwjSkLV79NIhpyLfr3jAp7VaTCMXOJHu4m0Ew1CZ2fcjASwmV1jI3BWuWHB013M48eyeldk9gYg==} engines: {node: '>=0.10.0'} @@ -3914,6 +3918,10 @@ packages: resolution: {integrity: sha512-o5vL7aDWatOTX8LzaS1WMoaoxIiLRQJuIKKe2wAw6IeULDHaqbiqiggmx+pKvZDb1Sj+pE46Sn1T7lCqfFtg1Q==} engines: {node: '>=16'} + martian-cidr@2.0.3: + resolution: {integrity: sha512-SWEgvwk2RpvSmcqHrOnnvz0fMneETn/lYINBICUFmwQEBH/ZN2YjuI0GKSI8oCWDkvEC7p7vzghzdMmiNePgEw==} + engines: {node: '>=14'} + mathml-tag-names@2.1.3: resolution: {integrity: sha512-APMBEanjybaPzUrfqU0IMU5I0AswKMH7k8OTLs0vvV4KZpExkTkY87nR/zpbuTPj+gARop7aGUbl11pnDfW6xg==} @@ -9577,6 +9585,8 @@ snapshots: ipaddr.js@1.9.1: {} + ipaddr.js@2.1.0: {} + is-absolute-url@2.1.0: {} is-alphabetical@2.0.1: {} @@ -9994,6 +10004,10 @@ snapshots: markdown-extensions@2.0.0: {} + martian-cidr@2.0.3: + dependencies: + ipaddr.js: 2.1.0 + mathml-tag-names@2.1.3: {} mdast-util-from-markdown@2.0.0: From 635dc6f13f85ccdd4d174c8e4bd41218b69c7992 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 01:35:30 +0500 Subject: [PATCH 20/28] fixes on review: use named export instead of default one --- proxy/index.ts | 2 +- proxy/proxy.ts | 4 +--- proxy/test/proxy.test.ts | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/proxy/index.ts b/proxy/index.ts index 3a9f2fa5..f4260424 100755 --- a/proxy/index.ts +++ b/proxy/index.ts @@ -1,6 +1,6 @@ import { styleText } from 'node:util' -import createProxyServer from './proxy.js' +import { createProxyServer } from './proxy.js' const PORT = 5284 diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 799c0064..a34933bb 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -22,7 +22,7 @@ import { styleText } from 'node:util' * * @param { isProduction, silentMode, hostnameWhitelist, productionDomainSuffix } config */ -const createProxyServer = ( +export const createProxyServer = ( config: { hostnameWhitelist?: string[] isProduction?: boolean @@ -115,5 +115,3 @@ const createProxyServer = ( } }) } - -export default createProxyServer diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts index 11ae38ae..e03f110f 100644 --- a/proxy/test/proxy.test.ts +++ b/proxy/test/proxy.test.ts @@ -3,7 +3,7 @@ import { createServer } from 'node:http' import { before, describe, test } from 'node:test' import { URL } from 'node:url' -import createProxyServer from '../proxy.js' +import { createProxyServer } from '../proxy.js' import { getTestHttpServer, initTestHttpServer } from './utils.js' /** From a1a72e2495fcb99e61edcd4236a43e5c2f24f47e Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 02:34:22 +0500 Subject: [PATCH 21/28] fixes on review: fix utils.ts file --- proxy/test/utils.ts | 62 ++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/proxy/test/utils.ts b/proxy/test/utils.ts index e5f7491a..fb2867ae 100644 --- a/proxy/test/utils.ts +++ b/proxy/test/utils.ts @@ -1,3 +1,15 @@ +// Use this function if you need to init local http server for testing. Express is supported. Server will be closed automatically after tests finish +// +// Usage: in your *.test.ts file +// +// await describe('test', async () => { +// await initTestHttpServer('test', ...) +// +// await test('test', async () => { +// fetch(getTestHttpServer('test')) +// })}) +// + import type http from 'node:http' import { after, before } from 'node:test' @@ -8,49 +20,34 @@ interface TestServer { server: http.Server } -const __testServers: { +const testServers: { [key: string]: TestServer } = {} -/** - * Use this function if you need to init local http server for testing. Express is supported. Server will be closed automatically after tests finish - * - * @example - * Usage: in your *.test.ts file - * - * await describe('test', async () => { - * await initTestHttpServer('test', ...) - * - * await test('test', async () => { - * fetch(getTestHttpServer('test')) - * }) - * }) - * - * @param name - * @param server - * @param opts - */ export async function initTestHttpServer( name: string, - server: any, + server: http.Server, opts?: { port?: number; protocol?: string } ): Promise { - // before(async () => { + // port=0 is let port = opts?.port || 0 let protocol = opts?.protocol || 'http' - // port=0 is - let testServerInstance = await server.listen(port) - // @ts-ignore + let testServerInstance = server.listen(port) + let addressInfo: { address: string; family: string; port: number } = - testServerInstance.address() + testServerInstance.address() as { + address: string + family: string + port: number + } let testServerAddress = addressInfo.address === '::' ? 'localhost' : addressInfo.address let testServerPort = addressInfo.port - __testServers[name] = { + testServers[name] = { address: testServerAddress, baseUrl: `${protocol}://${testServerAddress}:${testServerPort}`, port: testServerPort, @@ -59,18 +56,13 @@ export async function initTestHttpServer( }) after(async () => { - if (__testServers[name]) { - // @ts-ignore - __testServers[name].server.close() - delete __testServers[name] + if (testServers[name]) { + testServers[name]?.server.close() + delete testServers[name] } }) } -/** - * Use it to get server initialized by initTestHttpServer function - * @param name - */ export function getTestHttpServer(name: string): TestServer | undefined { - return __testServers[name] + return testServers[name] } From 4ed2e6f897f88f9a9e02458de1fbd26f1901c614 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 02:58:55 +0500 Subject: [PATCH 22/28] fixes on review: fix README.md --- proxy/README.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/proxy/README.md b/proxy/README.md index 0e4570e9..cd315b6d 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -4,7 +4,7 @@ HTTP-server to proxy all RSS fetching request from web client. User could use it to bypass censorship or to try web client before they install upcoming extension (to bypass CORS limit of web app). -## Usage +## Scripts ### Start Proxy @@ -13,8 +13,13 @@ pnpm start # // Proxy server running on port 5284 ``` -## Additional Info: +## Abuse Protection -### Discussing Security and Protection From Spam +- Proxy allows only GET requests +- Proxy do not allow requests to reserved ip addresses like `127.0.0.1` +- Proxy allows only http or https +- Proxy removes cookie headers -- https://github.com/hplush/slowreader/issues/100 +## Test Strategy + +Proxy is tested using e2e testing. To write tests `initTestHttpServer` should be used From 12833a95794a7b83cb475b5ab969cb0ac70af836 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 03:45:29 +0500 Subject: [PATCH 23/28] fixes on review: return 400 if request is bad --- proxy/proxy.ts | 62 ++++++++++++++++++---------------------- proxy/test/proxy.test.ts | 19 +++--------- 2 files changed, 32 insertions(+), 49 deletions(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index a34933bb..fe5c23e2 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -3,25 +3,8 @@ import isMartianIP from 'martian-cidr' import type http from 'node:http' import { createServer } from 'node:http' import { styleText } from 'node:util' +import { isIP } from 'net' -/** - * Creates proxy server - * - * isProduction - main toggle for production features: - * - will allow only request that match productionDomainSuffix - * - * silentMode - will silent the output. Useful for testing - * - * hostnameWhitelist - Sometimes you need to allow request to certain ips like localhost for testing purposes. - * - * productionDomainSuffix - if isProduction, then only request from origins that match this param are allowed - * - * @example - * const p = createProxyServer() - * p.listen(5555).then(() => console.log('running on 5555')) - * - * @param { isProduction, silentMode, hostnameWhitelist, productionDomainSuffix } config - */ export const createProxyServer = ( config: { hostnameWhitelist?: string[] @@ -30,9 +13,13 @@ export const createProxyServer = ( silentMode?: boolean } = {} ): http.Server => { + // Main toggle for production features let isProduction = config.isProduction || false + // Silence the output. Used for testing let silentMode = config.silentMode || false + // Allow request to certain ips like 'localhost'. Used for testing purposes let hostnameWhitelist = config.hostnameWhitelist || [] + // if isProduction, then only request from origins that match this param are allowed let productionDomainSuffix = config.productionDomainSuffix || '' return createServer(async (req, res) => { @@ -42,19 +29,25 @@ export const createProxyServer = ( try { // Only http or https protocols are allowed if (!url.startsWith('http://') && !url.startsWith('https://')) { - throw new Error('Only http or https requests are allowed.') + res.writeHead(400, { 'Content-Type': 'text/plain' }) + res.end('Bad Request: Only HTTP or HTTPS are supported') + return } // Other requests are typically used to modify the data, and we do not typically need them to load RSS if (req.method !== 'GET') { - throw new Error('Only GET requests are allowed.') + res.writeHead(400, { 'Content-Type': 'text/plain' }) + res.end('Bad Request: Only GET requests are allowed') + return } // In production mode we only allow request from production domain if (isProduction) { let origin = req.headers.origin if (!origin?.endsWith(productionDomainSuffix)) { - throw new Error('Unauthorized origin.') + res.writeHead(400, { 'Content-Type': 'text/plain' }) + res.end('Bad Request: Unauthorized origin') + return } } @@ -64,10 +57,14 @@ export const createProxyServer = ( // 127.* // 192.168.*,* // https://en.wikipedia.org/wiki/Reserved_IP_addresses - if (isMartianIP(requestUrl.hostname)) { - throw new Error( - 'Requests to IPs from local or reserved subnets are not allowed.' - ) + if ( + (isIP(requestUrl.hostname) === 4 || + isIP(requestUrl.hostname) === 6) && + isMartianIP(requestUrl.hostname) + ) { + res.writeHead(400, { 'Content-Type': 'text/plain' }) + res.end('Bad Request: Requests to reserved IPs are not allowed') + return } } @@ -87,7 +84,7 @@ export const createProxyServer = ( res.writeHead(targetResponse.status, { 'Access-Control-Allow-Headers': '*', 'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE', - 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Origin': req.headers['Origin'] || '*', 'Content-Type': targetResponse.headers.get('content-type') ?? 'text/plain' }) @@ -99,18 +96,15 @@ export const createProxyServer = ( if (!silentMode) { process.stderr.write(styleText('red', e.stack ?? e.message) + '\n') } - if (!sent) { - res.writeHead(500, { 'Content-Type': 'text/plain' }) - res.end('Internal Server Error') - } } else if (typeof e === 'string') { if (!silentMode) { process.stderr.write(styleText('red', e) + '\n') } - if (!sent) { - res.writeHead(500, { 'Content-Type': 'text/plain' }) - res.end('Internal Server Error') - } + } + + if (!sent) { + res.writeHead(500, { 'Content-Type': 'text/plain' }) + res.end('Internal Server Error') } } }) diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts index e03f110f..b4bac464 100644 --- a/proxy/test/proxy.test.ts +++ b/proxy/test/proxy.test.ts @@ -6,11 +6,6 @@ import { URL } from 'node:url' import { createProxyServer } from '../proxy.js' import { getTestHttpServer, initTestHttpServer } from './utils.js' -/** - * Proxy is tested with a service that outputs request and response data. - * - * -> -> - */ const targetServer = createServer((req, res) => { let parsedUrl = new URL(req.url || '', `http://${req.headers.host}`) @@ -58,7 +53,7 @@ describe('proxy tests', async () => { if (!target || !proxy) { throw new Error( - "Couldn't set up target server or proxy server. Something is misconfigured. Please check out 'proxy.test.js'" + "Couldn't set up target server or proxy server. Please check out 'proxy.test.js'" ) } }) @@ -66,7 +61,6 @@ describe('proxy tests', async () => { await test('proxy works', async () => { let response = await fetch(`${proxyServerUrl}/${targetServerUrl}`) let parsedResponse = await response.json() - // @ts-ignore equal(response.status, 200) equal(parsedResponse?.response, 'ok') }) @@ -76,7 +70,6 @@ describe('proxy tests', async () => { `${proxyServerUrl}/${targetServerUrl}/foo/bar?foo=bar&bar=foo` ) let parsedResponse = await response.json() - // @ts-ignore equal(response.status, 200) equal(parsedResponse?.response, 'ok') equal(parsedResponse?.request?.requestPath, '/foo/bar') @@ -93,7 +86,7 @@ describe('proxy tests', async () => { let response = await fetch(`${proxyServerUrl}/${targetServerUrl}`, { method }) - equal(response.status, 500) + equal(response.status, 400) }) } }) @@ -119,7 +112,7 @@ describe('proxy tests', async () => { `${proxyServerUrl}/${targetServerUrl.replace('http', protocol)}`, {} ) - equal(response.status, 500) + equal(response.status, 400) }) } }) @@ -129,7 +122,7 @@ describe('proxy tests', async () => { `${proxyServerUrl}/${targetServerUrl.replace('localhost', '127.0.0.1')}`, {} ) - equal(response.status, 500) + equal(response.status, 400) }) await describe('cookies', async () => { @@ -140,9 +133,7 @@ describe('proxy tests', async () => { equal(response.status, 200) let parsedResponse = await response.json() - // @ts-ignore equal(parsedResponse?.['set-cookie'], undefined) - // @ts-ignore equal(parsedResponse?.cookie, undefined) }) @@ -153,9 +144,7 @@ describe('proxy tests', async () => { equal(response.status, 200) let parsedResponse = await response.json() - // @ts-ignore equal(parsedResponse?.['set-cookie'], undefined) - // @ts-ignore equal(parsedResponse?.cookie, undefined) }) }) From 30bcc4fb5467b2de5260c35bd27e7ad68496ff6f Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 03:46:13 +0500 Subject: [PATCH 24/28] fixes on review: fix eslint errors --- proxy/proxy.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index fe5c23e2..a64f0d39 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -2,8 +2,8 @@ import isMartianIP from 'martian-cidr' import type http from 'node:http' import { createServer } from 'node:http' +import { isIP } from 'node:net' import { styleText } from 'node:util' -import { isIP } from 'net' export const createProxyServer = ( config: { @@ -84,7 +84,7 @@ export const createProxyServer = ( res.writeHead(targetResponse.status, { 'Access-Control-Allow-Headers': '*', 'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE', - 'Access-Control-Allow-Origin': req.headers['Origin'] || '*', + 'Access-Control-Allow-Origin': req.headers.Origin || '*', 'Content-Type': targetResponse.headers.get('content-type') ?? 'text/plain' }) From 2826a759d769c769d6eb2bbc87fcae685ce7f206 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 18:04:15 +0500 Subject: [PATCH 25/28] fixes on review: change env var name --- proxy/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/index.ts b/proxy/index.ts index f4260424..7cd33b5b 100755 --- a/proxy/index.ts +++ b/proxy/index.ts @@ -4,7 +4,7 @@ import { createProxyServer } from './proxy.js' const PORT = 5284 -const IS_PRODUCTION = process.env.mode === 'production' +const IS_PRODUCTION = process.env.NODE_ENV === 'production' const PRODUCTION_DOMAIN_SUFFIX = '.slowreader.app' const proxy = createProxyServer({ From c378114d8a29b3477630d2585c2f525b8ec4be2e Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 18:23:24 +0500 Subject: [PATCH 26/28] fixes on review: add timeout tests + upgrade error handling to use custom classes --- proxy/proxy.ts | 51 ++++++++++++++++++++++++++-------------- proxy/test/proxy.test.ts | 24 +++++++++++++++++-- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index a64f0d39..72e6a84e 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -5,12 +5,19 @@ import { createServer } from 'node:http' import { isIP } from 'node:net' import { styleText } from 'node:util' +class BadRequestError extends Error { + constructor(message: string) { + super(message) + } +} + export const createProxyServer = ( config: { hostnameWhitelist?: string[] isProduction?: boolean productionDomainSuffix?: string silentMode?: boolean + fetchTimeout?: number } = {} ): http.Server => { // Main toggle for production features @@ -21,6 +28,7 @@ export const createProxyServer = ( let hostnameWhitelist = config.hostnameWhitelist || [] // if isProduction, then only request from origins that match this param are allowed let productionDomainSuffix = config.productionDomainSuffix || '' + let fetchTimeout = config.fetchTimeout || 2500 return createServer(async (req, res) => { let url = decodeURIComponent(req.url!.slice(1)) @@ -29,25 +37,19 @@ export const createProxyServer = ( try { // Only http or https protocols are allowed if (!url.startsWith('http://') && !url.startsWith('https://')) { - res.writeHead(400, { 'Content-Type': 'text/plain' }) - res.end('Bad Request: Only HTTP or HTTPS are supported') - return + throw new BadRequestError('Only HTTP or HTTPS are supported') } // Other requests are typically used to modify the data, and we do not typically need them to load RSS if (req.method !== 'GET') { - res.writeHead(400, { 'Content-Type': 'text/plain' }) - res.end('Bad Request: Only GET requests are allowed') - return + throw new BadRequestError('Only GET requests are allowed') } // In production mode we only allow request from production domain if (isProduction) { let origin = req.headers.origin if (!origin?.endsWith(productionDomainSuffix)) { - res.writeHead(400, { 'Content-Type': 'text/plain' }) - res.end('Bad Request: Unauthorized origin') - return + throw new BadRequestError('Unauthorized Origin') } } @@ -62,9 +64,7 @@ export const createProxyServer = ( isIP(requestUrl.hostname) === 6) && isMartianIP(requestUrl.hostname) ) { - res.writeHead(400, { 'Content-Type': 'text/plain' }) - res.end('Bad Request: Requests to reserved IPs are not allowed') - return + throw new BadRequestError('Requests to reserved ips are not allowed') } } @@ -78,7 +78,8 @@ export const createProxyServer = ( ...(req.headers as HeadersInit), host: new URL(url).host }, - method: req.method + method: req.method, + signal: AbortSignal.timeout(fetchTimeout) }) res.writeHead(targetResponse.status, { @@ -92,12 +93,25 @@ export const createProxyServer = ( res.write(await targetResponse.text()) res.end() } catch (e) { - if (e instanceof Error) { - if (!silentMode) { + // Known errors: + if (e instanceof Error && e.name === 'TimeoutError') { + res.writeHead(400, { 'Content-Type': 'text/plain' }) + res.end('Bad Request: Request was aborted due to timeout') + return + } + + if (e instanceof BadRequestError) { + res.writeHead(400, { 'Content-Type': 'text/plain' }) + res.end(`Bad Request: ${e.message}`) + return + } + + // Unknown or internal errors: + + if (!silentMode) { + if (e instanceof Error) { process.stderr.write(styleText('red', e.stack ?? e.message) + '\n') - } - } else if (typeof e === 'string') { - if (!silentMode) { + } else if (typeof e === 'string') { process.stderr.write(styleText('red', e) + '\n') } } @@ -105,6 +119,7 @@ export const createProxyServer = ( if (!sent) { res.writeHead(500, { 'Content-Type': 'text/plain' }) res.end('Internal Server Error') + return } } }) diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts index b4bac464..7aa1216e 100644 --- a/proxy/test/proxy.test.ts +++ b/proxy/test/proxy.test.ts @@ -6,10 +6,17 @@ import { URL } from 'node:url' import { createProxyServer } from '../proxy.js' import { getTestHttpServer, initTestHttpServer } from './utils.js' -const targetServer = createServer((req, res) => { +const targetServer = createServer(async (req, res) => { let parsedUrl = new URL(req.url || '', `http://${req.headers.host}`) let queryParams = Object.fromEntries(parsedUrl.searchParams.entries()) + + if (queryParams.sleep) { + await new Promise(resolve => + setTimeout(resolve, parseInt(queryParams.sleep || '0')) + ) + } + let requestPath = parsedUrl.pathname res.writeHead(200, { @@ -33,8 +40,13 @@ const targetServer = createServer((req, res) => { describe('proxy tests', async () => { await initTestHttpServer( 'proxy', - createProxyServer({ hostnameWhitelist: ['localhost'], silentMode: true }) + createProxyServer({ + hostnameWhitelist: ['localhost'], + silentMode: true, + fetchTimeout: 100 + }) ) + await initTestHttpServer('target', targetServer) let proxyServerUrl = '' @@ -65,6 +77,14 @@ describe('proxy tests', async () => { equal(parsedResponse?.response, 'ok') }) + await test('proxy timeout works', async () => { + let response = await fetch( + `${proxyServerUrl}/${targetServerUrl}?sleep=500`, + {} + ) + equal(response.status, 400) + }) + await test('proxy transfers query params and path', async () => { let response = await fetch( `${proxyServerUrl}/${targetServerUrl}/foo/bar?foo=bar&bar=foo` From 6201c12b5ce491b086810e10b12d6cdaec338798 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Sat, 20 Apr 2024 18:31:01 +0500 Subject: [PATCH 27/28] fixes on review: fix eslint related errors --- proxy/proxy.ts | 4 ++-- proxy/test/proxy.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 72e6a84e..7b48fc68 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -8,16 +8,17 @@ import { styleText } from 'node:util' class BadRequestError extends Error { constructor(message: string) { super(message) + this.name = 'BadRequestError' } } export const createProxyServer = ( config: { + fetchTimeout?: number hostnameWhitelist?: string[] isProduction?: boolean productionDomainSuffix?: string silentMode?: boolean - fetchTimeout?: number } = {} ): http.Server => { // Main toggle for production features @@ -119,7 +120,6 @@ export const createProxyServer = ( if (!sent) { res.writeHead(500, { 'Content-Type': 'text/plain' }) res.end('Internal Server Error') - return } } }) diff --git a/proxy/test/proxy.test.ts b/proxy/test/proxy.test.ts index 7aa1216e..a320bc6f 100644 --- a/proxy/test/proxy.test.ts +++ b/proxy/test/proxy.test.ts @@ -41,9 +41,9 @@ describe('proxy tests', async () => { await initTestHttpServer( 'proxy', createProxyServer({ + fetchTimeout: 100, hostnameWhitelist: ['localhost'], - silentMode: true, - fetchTimeout: 100 + silentMode: true }) ) From 12ac8baa84bdbe5cbd91ef3f48a563eb62cb4eec Mon Sep 17 00:00:00 2001 From: Mikhail Gorbunov Date: Mon, 22 Apr 2024 02:11:41 +0500 Subject: [PATCH 28/28] fixes on review: remove documentation and introduce custom typings for isMartianIp package --- proxy/README.md | 6 +++--- proxy/global.d.ts | 3 +++ proxy/proxy.ts | 15 +++++---------- proxy/test/utils.ts | 12 ------------ 4 files changed, 11 insertions(+), 25 deletions(-) create mode 100644 proxy/global.d.ts diff --git a/proxy/README.md b/proxy/README.md index cd315b6d..43c479a5 100644 --- a/proxy/README.md +++ b/proxy/README.md @@ -16,10 +16,10 @@ pnpm start ## Abuse Protection - Proxy allows only GET requests -- Proxy do not allow requests to reserved ip addresses like `127.0.0.1` -- Proxy allows only http or https +- Proxy do not allow requests to in-cloud ip addresses like `127.0.0.1` +- Proxy allows only HTTP or HTTPS protocols - Proxy removes cookie headers ## Test Strategy -Proxy is tested using e2e testing. To write tests `initTestHttpServer` should be used +To test proxy we emulate the real HTTP servers (end-to-end testing). diff --git a/proxy/global.d.ts b/proxy/global.d.ts new file mode 100644 index 00000000..5161b7a1 --- /dev/null +++ b/proxy/global.d.ts @@ -0,0 +1,3 @@ +declare module 'martian-cidr' { + export default function isMartianIP(ip: string): boolean +} diff --git a/proxy/proxy.ts b/proxy/proxy.ts index 7b48fc68..8056859e 100644 --- a/proxy/proxy.ts +++ b/proxy/proxy.ts @@ -1,4 +1,3 @@ -// @ts-ignore import isMartianIP from 'martian-cidr' import type http from 'node:http' import { createServer } from 'node:http' @@ -12,7 +11,7 @@ class BadRequestError extends Error { } } -export const createProxyServer = ( +export function createProxyServer( config: { fetchTimeout?: number hostnameWhitelist?: string[] @@ -20,14 +19,10 @@ export const createProxyServer = ( productionDomainSuffix?: string silentMode?: boolean } = {} -): http.Server => { - // Main toggle for production features +): http.Server { let isProduction = config.isProduction || false - // Silence the output. Used for testing let silentMode = config.silentMode || false - // Allow request to certain ips like 'localhost'. Used for testing purposes let hostnameWhitelist = config.hostnameWhitelist || [] - // if isProduction, then only request from origins that match this param are allowed let productionDomainSuffix = config.productionDomainSuffix || '' let fetchTimeout = config.fetchTimeout || 2500 @@ -56,7 +51,7 @@ export const createProxyServer = ( let requestUrl = new URL(url) if (!hostnameWhitelist.includes(requestUrl.hostname)) { - // Do not allow requests to addresses that are reserved: + // Do not allow requests to addresses inside our cloud: // 127.* // 192.168.*,* // https://en.wikipedia.org/wiki/Reserved_IP_addresses @@ -65,11 +60,11 @@ export const createProxyServer = ( isIP(requestUrl.hostname) === 6) && isMartianIP(requestUrl.hostname) ) { - throw new BadRequestError('Requests to reserved ips are not allowed') + throw new BadRequestError('Requests to in-cloud ips are not allowed') } } - // Remove all cookie headers and host header from request + // Remove all cookie headers so they will not be set on proxy domain delete req.headers.cookie delete req.headers['set-cookie'] delete req.headers.host diff --git a/proxy/test/utils.ts b/proxy/test/utils.ts index fb2867ae..938d153b 100644 --- a/proxy/test/utils.ts +++ b/proxy/test/utils.ts @@ -1,15 +1,3 @@ -// Use this function if you need to init local http server for testing. Express is supported. Server will be closed automatically after tests finish -// -// Usage: in your *.test.ts file -// -// await describe('test', async () => { -// await initTestHttpServer('test', ...) -// -// await test('test', async () => { -// fetch(getTestHttpServer('test')) -// })}) -// - import type http from 'node:http' import { after, before } from 'node:test'