mirrors/posthog
0
0
Fork 0
mirror of https://github.com/PostHog/posthog.git synced 2024-11-21 13:39:22 +01:00
Code Issues Releases Activity

chore(plugin-server): Validate fetch hostnames (#17183)

Browse Source 
* chore(plugin-server): Validate fetch hostnames

* Only apply Python host check on Cloud

* Update tests to use valid hook URLs

* Only apply plugin server host check in prod

* Update URLs in a couple more tests

* Only check hostnames on Cloud and remove port check

* Fix fetch mocking

* Roll out hostname guard per project

* Fix fetch call assertions

* Make `fetchHostnameGuardTeams` optional
This commit is contained in:
Michael Matloka 2023-09-18 14:38:02 +02:00 committed by GitHub
parent a87b247cd3
commit b7fe004d6b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 279 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

BIN
frontend/__snapshots__/scenes-app-recordings--recordings-play-list-no-pinned-recordings.png
View File

Binary file not shown.
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 110 KiB

After

Width:  |  Height:  |  Size: 75 KiB

8
plugin-server/jest.setup.fetch-mock.js
View File

@ -6,7 +6,11 @@ import fetch from 'node-fetch'
import { status } from './src/utils/status'
jest.mock('node-fetch')
jest.mock('node-fetch', () => ({
__esModule: true,
...jest.requireActual('node-fetch'), // Only mock fetch(), leave Request, Response, FetchError, etc. alone
default: jest.fn(),
}))
beforeEach(() => {
const responsesToUrls = {
@ -21,7 +25,7 @@ beforeEach(() => {
]),
}
fetch.mockImplementation(
jest.mocked(fetch).mockImplementation(
(url, options) =>
new Promise((resolve) =>
resolve({

1
plugin-server/package.json
View File

@ -127,6 +127,7 @@
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-promise": "^6.0.0",
"eslint-plugin-simple-import-sort": "^7.0.0",
"ipaddr.js": "^2.1.0",
"jest": "^28.1.1",
"nodemon": "^2.0.22",
"parse-prometheus-text-format": "^1.1.1",

8
plugin-server/pnpm-lock.yaml
View File

@ -279,6 +279,9 @@ devDependencies:
eslint-plugin-simple-import-sort:
specifier: ^7.0.0
version: 7.0.0(eslint@8.39.0)
ipaddr.js:
specifier: ^2.1.0
version: 2.1.0
jest:
specifier: ^28.1.1
version: 28.1.3(@types/node@16.18.25)(ts-node@10.9.1)
@ -7510,6 +7513,11 @@ packages:
resolution: {integrity: sha512-WKa+XuLG1A1R0UWhl2+1XQSi+fZWMsYKffMZTTYsiZaUD8k2yDAj5atimTUD2TZkyCkNEeYE5NhFZmupOGtjYQ==}
dev: false
/ipaddr.js@2.1.0:
resolution: {integrity: sha512-LlbxQ7xKzfBusov6UMi4MFpEg0m+mAm9xyNGEduwXMEDuf4WfzB/RZwMVYEd7IKGvh4IUkEXYxtAVu9T3OelJQ==}
engines: {node: '>= 10'}
dev: true
/is-arguments@1.1.1:
resolution: {integrity: sha512-8Q7EARjzEnKpt/PCD7e1cgUS0a6X8u5tdSiMqXhojOdoV9TsMsiO+9VLC5vAmO8N7/GmXn7yjR8qnA6bVAEzfA==}
engines: {node: '>= 0.4'}

3
plugin-server/src/config/config.ts
View File

@ -111,6 +111,7 @@ export function getDefaultConfig(): PluginsServerConfig {
CONVERSION_BUFFER_ENABLED_TEAMS: '',
CONVERSION_BUFFER_TOPIC_ENABLED_TEAMS: '',
BUFFER_CONVERSION_SECONDS: isDevEnv() ? 2 : 60, // KEEP IN SYNC WITH posthog/settings/ingestion.py
FETCH_HOSTNAME_GUARD_TEAMS: '',
PERSON_INFO_CACHE_TTL: 5 * 60, // 5 min
KAFKA_HEALTHCHECK_SECONDS: 20,
OBJECT_STORAGE_ENABLED: true,
@ -129,7 +130,7 @@ export function getDefaultConfig(): PluginsServerConfig {
APP_METRICS_GATHERED_FOR_ALL: isDevEnv() ? true : false,
MAX_TEAM_ID_TO_BUFFER_ANONYMOUS_EVENTS_FOR: 0,
USE_KAFKA_FOR_SCHEDULED_TASKS: true,
CLOUD_DEPLOYMENT: 'default', // Used as a Sentry tag
CLOUD_DEPLOYMENT: null,
STARTUP_PROFILE_DURATION_SECONDS: 300, // 5 minutes
STARTUP_PROFILE_CPU: false,

8
plugin-server/src/main/ingestion-queues/on-event-handler-consumer.ts
View File

@ -85,7 +85,13 @@ export const startAsyncWebhooksHandlerConsumer = async ({
const actionManager = new ActionManager(postgres)
await actionManager.prepare()
const actionMatcher = new ActionMatcher(postgres, actionManager, statsd)
const hookCannon = new HookCommander(postgres, teamManager, organizationManager, statsd)
const hookCannon = new HookCommander(
postgres,
teamManager,
organizationManager,
new Set(serverConfig.FETCH_HOSTNAME_GUARD_TEAMS.split(',').filter(String).map(Number)),
statsd
)
const concurrency = serverConfig.TASKS_PER_WORKER || 20
const pubSub = new PubSub(serverConfig, {

5
plugin-server/src/types.ts
View File

@ -181,6 +181,7 @@ export interface PluginsServerConfig {
CONVERSION_BUFFER_ENABLED_TEAMS: string
CONVERSION_BUFFER_TOPIC_ENABLED_TEAMS: string
BUFFER_CONVERSION_SECONDS: number
FETCH_HOSTNAME_GUARD_TEAMS: string
PERSON_INFO_CACHE_TTL: number
KAFKA_HEALTHCHECK_SECONDS: number
OBJECT_STORAGE_ENABLED: boolean // Disables or enables the use of object storage. It will become mandatory to use object storage
@ -201,7 +202,8 @@ export interface PluginsServerConfig {
USE_KAFKA_FOR_SCHEDULED_TASKS: boolean // distribute scheduled tasks across the scheduler workers
EVENT_OVERFLOW_BUCKET_CAPACITY: number
EVENT_OVERFLOW_BUCKET_REPLENISH_RATE: number
CLOUD_DEPLOYMENT: string
/** Label of the PostHog Cloud environment. Null if not running PostHog Cloud. @example 'US' */
CLOUD_DEPLOYMENT: string | null
// dump profiles to disk, covering the first N seconds of runtime
STARTUP_PROFILE_DURATION_SECONDS: number
@ -266,6 +268,7 @@ export interface Hub extends PluginsServerConfig {
lastActivityType: string
statelessVms: StatelessVmMap
conversionBufferEnabledTeams: Set<number>
fetchHostnameGuardTeams: Set<number>
// functions
enqueuePluginJob: (job: EnqueuedPluginJob) => Promise<void>
// ValueMatchers used for various opt-in/out features

4
plugin-server/src/utils/db/hub.ts
View File

@ -70,6 +70,9 @@ export async function createHub(
const conversionBufferEnabledTeams = new Set(
serverConfig.CONVERSION_BUFFER_ENABLED_TEAMS.split(',').filter(String).map(Number)
)
const fetchHostnameGuardTeams = new Set(
serverConfig.FETCH_HOSTNAME_GUARD_TEAMS.split(',').filter(String).map(Number)
)
const statsd: StatsD | undefined = createStatsdClient(serverConfig, threadId)
@ -181,6 +184,7 @@ export async function createHub(
rootAccessManager,
promiseManager,
conversionBufferEnabledTeams,
fetchHostnameGuardTeams,
pluginConfigsToSkipElementsParsing: buildIntegerMatcher(process.env.SKIP_ELEMENTS_PARSING_PLUGINS, true),
}

2
plugin-server/src/utils/env-utils.ts
View File

@ -40,6 +40,8 @@ export const isTestEnv = (): boolean => determineNodeEnv() === NodeEnv.Test
export const isDevEnv = (): boolean => determineNodeEnv() === NodeEnv.Development
export const isProdEnv = (): boolean => determineNodeEnv() === NodeEnv.Production
export const isCloud = (): boolean => !!process.env.CLOUD_DEPLOYMENT
export function isIngestionOverflowEnabled(): boolean {
const ingestionOverflowEnabled = process.env.INGESTION_OVERFLOW_ENABLED
return stringToBoolean(ingestionOverflowEnabled)

68
plugin-server/src/utils/fetch.ts
View File

@ -1,21 +1,73 @@
// This module wraps node-fetch with a sentry tracing-aware extension
import fetch, { FetchError, Request, Response } from 'node-fetch'
import { LookupAddress } from 'dns'
import dns from 'dns/promises'
import * as ipaddr from 'ipaddr.js'
import fetch, { type RequestInfo, type RequestInit, type Response, FetchError, Request } from 'node-fetch'
import { URL } from 'url'
import { runInSpan } from '../sentry'
function fetchWrapper(...args: Parameters<typeof fetch>): Promise<Response> {
const request = new Request(...args)
return runInSpan(
export async function trackedFetch(url: RequestInfo, init?: RequestInit): Promise<Response> {
const request = new Request(url, init)
return await runInSpan(
{
op: 'fetch',
description: `${request.method} ${request.url}`,
},
() => fetch(...args)
async () => await fetch(url, init)
)
}
fetchWrapper.isRedirect = fetch.isRedirect
fetchWrapper.FetchError = FetchError
trackedFetch.isRedirect = fetch.isRedirect
trackedFetch.FetchError = FetchError
export default fetchWrapper
export async function safeTrackedFetch(url: RequestInfo, init?: RequestInit): Promise<Response> {
const request = new Request(url, init)
return await runInSpan(
{
op: 'fetch',
description: `${request.method} ${request.url}`,
},
async () => {
await raiseIfUserProvidedUrlUnsafe(request.url)
return await fetch(url, init)
}
)
}
safeTrackedFetch.isRedirect = fetch.isRedirect
safeTrackedFetch.FetchError = FetchError
/**
* Raise if the provided URL seems unsafe, otherwise do nothing.
*
* Equivalent of Django raise_if_user_provided_url_unsafe.
*/
export async function raiseIfUserProvidedUrlUnsafe(url: string): Promise<void> {
// Raise if the provided URL seems unsafe, otherwise do nothing.
let parsedUrl: URL
try {
parsedUrl = new URL(url)
} catch (err) {
throw new FetchError('Invalid URL', 'posthog-host-guard')
}
if (!parsedUrl.hostname) {
throw new FetchError('No hostname', 'posthog-host-guard')
}
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
throw new FetchError('Scheme must be either HTTP or HTTPS', 'posthog-host-guard')
}
let addrinfo: LookupAddress[]
try {
addrinfo = await dns.lookup(parsedUrl.hostname, { all: true })
} catch (err) {
throw new FetchError('Invalid hostname', 'posthog-host-guard')
}
for (const { address } of addrinfo) {
// Prevent addressing internal services
if (ipaddr.parse(address).range() !== 'unicast') {
throw new FetchError('Internal hostname', 'posthog-host-guard')
}
}
}

13
plugin-server/src/worker/ingestion/hooks.ts
View File

@ -5,7 +5,8 @@ import { format } from 'util'
import { Action, Hook, PostIngestionEvent, Team } from '../../types'
import { PostgresRouter, PostgresUse } from '../../utils/db/postgres'
import fetch from '../../utils/fetch'
import { isCloud } from '../../utils/env-utils'
import { safeTrackedFetch, trackedFetch } from '../../utils/fetch'
import { status } from '../../utils/status'
import { getPropertyValueByPath, stringify } from '../../utils/utils'
import { OrganizationManager } from './organization-manager'
@ -256,6 +257,7 @@ export class HookCommander {
organizationManager: OrganizationManager
statsd: StatsD | undefined
siteUrl: string
fetchHostnameGuardTeams: Set<number>
/** Hook request timeout in ms. */
EXTERNAL_REQUEST_TIMEOUT = 10 * 1000
@ -264,11 +266,13 @@ export class HookCommander {
postgres: PostgresRouter,
teamManager: TeamManager,
organizationManager: OrganizationManager,
fetchHostnameGuardTeams?: Set<number>,
statsd?: StatsD
) {
this.postgres = postgres
this.teamManager = teamManager
this.organizationManager = organizationManager
this.fetchHostnameGuardTeams = fetchHostnameGuardTeams || new Set()
if (process.env.SITE_URL) {
this.siteUrl = process.env.SITE_URL
} else {
@ -358,9 +362,10 @@ export class HookCommander {
`⌛⌛⌛ Posting Webhook slow. Timeout warning after 5 sec! url=${webhookUrl} team_id=${team.id} event_id=${event.eventUuid}`
)
}, 5000)
const relevantFetch = isCloud() && this.fetchHostnameGuardTeams.has(team.id) ? safeTrackedFetch : trackedFetch
try {
await instrumentWebhookStep('fetch', async () => {
const request = await fetch(webhookUrl, {
const request = await relevantFetch(webhookUrl, {
method: 'POST',
body: JSON.stringify(message, undefined, 4),
headers: { 'Content-Type': 'application/json' },
@ -399,8 +404,10 @@ export class HookCommander {
`⌛⌛⌛ Posting RestHook slow. Timeout warning after 5 sec! url=${hook.target} team_id=${event.teamId} event_id=${event.eventUuid}`
)
}, 5000)
const relevantFetch =
isCloud() && this.fetchHostnameGuardTeams.has(hook.team_id) ? safeTrackedFetch : trackedFetch
try {
const request = await fetch(hook.target, {
const request = await relevantFetch(hook.target, {
method: 'POST',
body: JSON.stringify(payload, undefined, 4),
headers: { 'Content-Type': 'application/json' },

2
plugin-server/src/worker/plugins/mmdb.ts
View File

@ -1,5 +1,6 @@
import { Reader, ReaderModel } from '@maxmind/geoip2-node'
import { DateTime } from 'luxon'
import fetch from 'node-fetch'
import * as schedule from 'node-schedule'
import prettyBytes from 'pretty-bytes'
import { brotliDecompress } from 'zlib'
@ -12,7 +13,6 @@ import {
} from '../../config/mmdb-constants'
import { Hub, PluginAttachmentDB } from '../../types'
import { PostgresUse } from '../../utils/db/postgres'
import fetch from '../../utils/fetch'
import { status } from '../../utils/status'
import { delay } from '../../utils/utils'

52
plugin-server/src/worker/vm/imports.ts
View File

@ -12,33 +12,37 @@ import * as jsonwebtoken from 'jsonwebtoken'
import * as pg from 'pg'
import snowflake from 'snowflake-sdk'
import { PassThrough } from 'stream'
import { Hub } from 'types'
import * as url from 'url'
import * as zlib from 'zlib'
import fetch from '../../utils/fetch'
import { isCloud, isTestEnv } from '../../utils/env-utils'
import { safeTrackedFetch, trackedFetch } from '../../utils/fetch'
import { writeToFile } from './extensions/test-utils'
export const imports = {
...(process.env.NODE_ENV === 'test'
? {
'test-utils/write-to-file': writeToFile,
}
: {}),
'@google-cloud/bigquery': bigquery,
'@google-cloud/pubsub': pubsub,
'@google-cloud/storage': gcs,
'@posthog/plugin-contrib': contrib,
'@posthog/plugin-scaffold': scaffold,
'aws-sdk': AWS,
ethers: ethers,
'generic-pool': genericPool,
'node-fetch': fetch,
'snowflake-sdk': snowflake,
crypto: crypto,
jsonwebtoken: jsonwebtoken,
faker: faker,
pg: pg,
stream: { PassThrough },
url: url,
zlib: zlib,
export function determineImports(hub: Hub, teamId: number) {
return {
...(isTestEnv()
? {
'test-utils/write-to-file': writeToFile,
}
: {}),
'@google-cloud/bigquery': bigquery,
'@google-cloud/pubsub': pubsub,
'@google-cloud/storage': gcs,
'@posthog/plugin-contrib': contrib,
'@posthog/plugin-scaffold': scaffold,
'aws-sdk': AWS,
ethers: ethers,
'generic-pool': genericPool,
'node-fetch': isCloud() && hub.fetchHostnameGuardTeams.has(teamId) ? safeTrackedFetch : trackedFetch,
'snowflake-sdk': snowflake,
crypto: crypto,
jsonwebtoken: jsonwebtoken,
faker: faker,
pg: pg,
stream: { PassThrough },
url: url,
zlib: zlib,
}
}

4
plugin-server/src/worker/vm/vm.ts
View File

@ -11,7 +11,7 @@ import { createJobs } from './extensions/jobs'
import { createPosthog } from './extensions/posthog'
import { createStorage } from './extensions/storage'
import { createUtils } from './extensions/utilities'
import { imports } from './imports'
import { determineImports } from './imports'
import { transformCode } from './transforms'
import { upgradeExportEvents } from './upgrades/export-events'
import { addHistoricalEventsExportCapability } from './upgrades/historical-export/export-historical-events'
@ -34,6 +34,8 @@ export function createPluginConfigVM(
pluginConfig: PluginConfig, // NB! might have team_id = 0
indexJs: string
): PluginConfigVMResponse {
const imports = determineImports(hub, pluginConfig.team_id)
const timer = new Date()
const statsdTiming = (metric: string) => {

8
plugin-server/tests/main/db.test.ts
View File

@ -165,7 +165,7 @@ describe('DB', () => {
user_id: 1001,
resource_id: 69,
event: 'action_performed',
target: 'https://rest-hooks.example.com/',
target: 'https://example.com/',
created: new Date().toISOString(),
updated: new Date().toISOString(),
})
@ -188,7 +188,7 @@ describe('DB', () => {
team_id: 2,
resource_id: 69,
event: 'action_performed',
target: 'https://rest-hooks.example.com/',
target: 'https://example.com/',
},
],
bytecode: null,
@ -226,7 +226,7 @@ describe('DB', () => {
user_id: 1001,
resource_id: 69,
event: 'event_performed',
target: 'https://rest-hooks.example.com/',
target: 'https://example.com/',
created: new Date().toISOString(),
updated: new Date().toISOString(),
})
@ -236,7 +236,7 @@ describe('DB', () => {
user_id: 1001,
resource_id: 70,
event: 'event_performed',
target: 'https://rest-hooks.example.com/',
target: 'https://example.com/',
created: new Date().toISOString(),
updated: new Date().toISOString(),
})

40
plugin-server/tests/utils/fetch.test.ts Normal file
View File

@ -0,0 +1,40 @@
import { FetchError } from 'node-fetch'
import { raiseIfUserProvidedUrlUnsafe } from '../../src/utils/fetch'
test('raiseIfUserProvidedUrlUnsafe', async () => {
// Sync test cases with posthog/api/test/test_utils.py
await raiseIfUserProvidedUrlUnsafe('https://google.com?q=20') // Safe
await raiseIfUserProvidedUrlUnsafe('https://posthog.com') // Safe
await raiseIfUserProvidedUrlUnsafe('https://posthog.com/foo/bar') // Safe, with path
await raiseIfUserProvidedUrlUnsafe('https://posthog.com:443') // Safe, good port
await raiseIfUserProvidedUrlUnsafe('https://1.1.1.1') // Safe, public IP
await expect(raiseIfUserProvidedUrlUnsafe('')).rejects.toThrow(new FetchError('Invalid URL', 'posthog-host-guard'))
await expect(raiseIfUserProvidedUrlUnsafe('@@@')).rejects.toThrow(
new FetchError('Invalid URL', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('posthog.com')).rejects.toThrow(
new FetchError('Invalid URL', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('ftp://posthog.com')).rejects.toThrow(
new FetchError('Scheme must be either HTTP or HTTPS', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('http://localhost')).rejects.toThrow(
new FetchError('Internal hostname', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('http://192.168.0.5')).rejects.toThrow(
new FetchError('Internal hostname', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('http://0.0.0.0')).rejects.toThrow(
new FetchError('Internal hostname', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('http://10.0.0.24')).rejects.toThrow(
new FetchError('Internal hostname', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('http://172.20.0.21')).rejects.toThrow(
new FetchError('Internal hostname', 'posthog-host-guard')
)
await expect(raiseIfUserProvidedUrlUnsafe('http://fgtggggzzggggfd.com')).rejects.toThrow(
new FetchError('Invalid hostname', 'posthog-host-guard')
)
})

6
plugin-server/tests/worker/ingestion/event-pipeline/event-pipeline-integration.test.ts
View File

@ -176,7 +176,7 @@ describe('Event Pipeline integration test', () => {
user_id: commonUserId,
resource_id: 69,
event: 'action_performed',
target: 'https://rest-hooks.example.com/',
target: 'https://example.com/',
created: timestamp,
updated: timestamp,
} as Hook)
@ -200,7 +200,7 @@ describe('Event Pipeline integration test', () => {
hook: {
id: 'abc',
event: 'action_performed',
target: 'https://rest-hooks.example.com/',
target: 'https://example.com/',
},
data: {
event: 'xyz',
@ -224,7 +224,7 @@ describe('Event Pipeline integration test', () => {
// Using a more verbose way instead of toHaveBeenCalledWith because we need to parse request body
// and use expect.any for a few payload properties, which wouldn't be possible in a simpler way
expect(jest.mocked(fetch).mock.calls[0][0]).toBe('https://rest-hooks.example.com/')
expect(jest.mocked(fetch).mock.calls[0][0]).toBe('https://example.com/')
const secondArg = jest.mocked(fetch).mock.calls[0][1]
expect(JSON.parse(secondArg!.body as unknown as string)).toStrictEqual(expectedPayload)
expect(JSON.parse(secondArg!.body as unknown as string)).toStrictEqual(expectedPayload)

45
plugin-server/tests/worker/ingestion/hooks.test.ts
View File

@ -1,7 +1,8 @@
import { DateTime } from 'luxon'
import * as fetch from 'node-fetch'
import fetch, { FetchError } from 'node-fetch'
import { Action, PostIngestionEvent, Team } from '../../../src/types'
import { isCloud } from '../../../src/utils/env-utils'
import { UUIDT } from '../../../src/utils/utils'
import {
determineWebhookType,
@ -15,6 +16,8 @@ import {
} from '../../../src/worker/ingestion/hooks'
import { Hook } from './../../../src/types'
jest.mock('../../../src/utils/env-utils')
describe('hooks', () => {
describe('determineWebhookType', () => {
test('Slack', () => {
@ -471,29 +474,35 @@ describe('hooks', () => {
let hook: Hook
beforeEach(() => {
hookCommander = new HookCommander({} as any, {} as any, {} as any)
jest.mocked(isCloud).mockReturnValue(false) // Disable private IP guard
hook = {
id: 'id',
team_id: 2,
team_id: 1,
user_id: 1,
resource_id: 1,
event: 'foo',
target: 'foo.bar',
target: 'https://example.com/',
created: new Date().toISOString(),
updated: new Date().toISOString(),
}
hookCommander = new HookCommander(
{} as any,
{} as any,
{} as any,
new Set([hook.team_id]) // Hostname guard enabled
)
})
test('person = undefined', async () => {
await hookCommander.postRestHook(hook, { event: 'foo' } as any)
expect(fetch).toHaveBeenCalledWith('foo.bar', {
expect(fetch).toHaveBeenCalledWith('https://example.com/', {
body: JSON.stringify(
{
hook: {
id: 'id',
event: 'foo',
target: 'foo.bar',
target: 'https://example.com/',
},
data: {
event: 'foo',
@ -510,26 +519,28 @@ describe('hooks', () => {
})
test('person data from the event', async () => {
jest.mocked(isCloud).mockReturnValue(true) // Enable private IP guard, which example.com should pass
const now = new Date().toISOString()
const uuid = new UUIDT().toString()
await hookCommander.postRestHook(hook, {
event: 'foo',
teamId: 1,
teamId: hook.team_id,
person_id: uuid,
person_properties: { foo: 'bar' },
person_created_at: DateTime.fromISO(now).toUTC(),
} as any)
expect(fetch).toHaveBeenCalledWith('foo.bar', {
expect(fetch).toHaveBeenCalledWith('https://example.com/', {
body: JSON.stringify(
{
hook: {
id: 'id',
event: 'foo',
target: 'foo.bar',
target: 'https://example.com/',
},
data: {
event: 'foo',
teamId: 1,
teamId: hook.team_id,
person: {
uuid: uuid,
properties: { foo: 'bar' },
@ -545,5 +556,19 @@ describe('hooks', () => {
timeout: 10000,
})
})
test('private IP hook allowed on self-hosted', async () => {
await hookCommander.postRestHook({ ...hook, target: 'http://127.0.0.1' }, { event: 'foo' } as any)
expect(fetch).toHaveBeenCalledWith('http://127.0.0.1', expect.anything())
})
test('private IP hook forbidden on Cloud', async () => {
jest.mocked(isCloud).mockReturnValue(true)
await expect(
hookCommander.postRestHook({ ...hook, target: 'http://127.0.0.1' }, { event: 'foo' } as any)
).rejects.toThrow(new FetchError('Internal hostname', 'posthog-host-guard'))
})
})
})

2
plugin-server/tests/worker/plugins/mmdb.test.ts
View File

@ -1,7 +1,7 @@
import { ReaderModel } from '@maxmind/geoip2-node'
import { readFileSync } from 'fs'
import { DateTime } from 'luxon'
import * as fetch from 'node-fetch'
import fetch from 'node-fetch'
import { join } from 'path'
import { Hub, LogLevel } from '../../../src/types'

8
plugin-server/tests/worker/vm.extra-lazy.test.ts
View File

@ -1,4 +1,4 @@
import * as fetch from 'node-fetch'
import fetch from 'node-fetch'
import { Hub, PluginTaskType } from '../../src/types'
import { createHub } from '../../src/utils/db/hub'
@ -39,7 +39,7 @@ describe('VMs are extra lazy 💤', () => {
expect(lazyVm.ready).toEqual(true)
expect(lazyVm.setupPluginIfNeeded).not.toHaveBeenCalled()
expect(fetch).toHaveBeenCalledWith('https://onevent.com/')
expect(fetch).toHaveBeenCalledWith('https://onevent.com/', undefined)
})
test('VM with jobs gets setup immediately', async () => {
@ -64,7 +64,7 @@ describe('VMs are extra lazy 💤', () => {
expect(lazyVm.ready).toEqual(true)
expect(lazyVm.setupPluginIfNeeded).not.toHaveBeenCalled()
expect(fetch).toHaveBeenCalledWith('https://onevent.com/')
expect(fetch).toHaveBeenCalledWith('https://onevent.com/', undefined)
})
test('VM without tasks delays setup until necessary', async () => {
@ -91,7 +91,7 @@ describe('VMs are extra lazy 💤', () => {
await lazyVm.getOnEvent()
expect(lazyVm.ready).toEqual(true)
expect(lazyVm.setupPluginIfNeeded).toHaveBeenCalled()
expect(fetch).toHaveBeenCalledWith('https://onevent.com/')
expect(fetch).toHaveBeenCalledWith('https://onevent.com/', undefined)
})
test('getting methods and tasks returns null if plugin is in errored state', async () => {

32
plugin-server/tests/worker/vm.test.ts
View File

@ -1,5 +1,5 @@
import { PluginEvent, ProcessedPluginEvent } from '@posthog/plugin-scaffold'
import * as fetch from 'node-fetch'
import fetch from 'node-fetch'
import { KAFKA_EVENTS_PLUGIN_INGESTION, KAFKA_PLUGIN_LOG_ENTRIES } from '../../src/config/kafka-topics'
import { Hub, PluginLogEntrySource, PluginLogEntryType } from '../../src/types'
@ -122,7 +122,7 @@ describe('vm tests', () => {
})
expect(fetch).not.toHaveBeenCalled()
await vm.methods.teardownPlugin!()
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=hoho')
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=hoho', undefined)
})
test('processEvent', async () => {
@ -376,7 +376,7 @@ describe('vm tests', () => {
event: 'export',
}
await vm.methods.onEvent!(event)
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=export')
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=export', undefined)
})
test('export default', async () => {
@ -395,7 +395,7 @@ describe('vm tests', () => {
event: 'default export',
}
await vm.methods.onEvent!(event)
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=default export')
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=default export', undefined)
})
})
@ -723,7 +723,7 @@ describe('vm tests', () => {
}
await vm.methods.processEvent!(event)
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=fetched')
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=fetched', undefined)
expect(event.properties).toEqual({ count: 2, query: 'bla', results: [true, true] })
})
@ -745,7 +745,7 @@ describe('vm tests', () => {
}
await vm.methods.processEvent!(event)
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=fetched')
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=fetched', undefined)
expect(event.properties).toEqual({ count: 2, query: 'bla', results: [true, true] })
})
@ -766,7 +766,7 @@ describe('vm tests', () => {
}
await vm.methods.processEvent!(event)
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=fetched')
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=fetched', undefined)
expect(event.properties).toEqual({ count: 2, query: 'bla', results: [true, true] })
})
@ -1051,7 +1051,7 @@ describe('vm tests', () => {
event: 'onEvent',
}
await vm.methods.onEvent!(event)
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=onEvent')
expect(fetch).toHaveBeenCalledWith('https://google.com/results.json?query=onEvent', undefined)
})
describe('exportEvents', () => {
@ -1085,7 +1085,7 @@ describe('vm tests', () => {
await vm.methods.onEvent!({ ...defaultEvent, event: 'otherEvent2' })
await vm.methods.onEvent!({ ...defaultEvent, event: 'otherEvent3' })
await delay(1010)
expect(fetch).toHaveBeenCalledWith('https://export.com/results.json?query=otherEvent2&events=2')
expect(fetch).toHaveBeenCalledWith('https://export.com/results.json?query=otherEvent2&events=2', undefined)
expect(hub.appMetrics.queueMetric).toHaveBeenCalledWith({
teamId: pluginConfig39.team_id,
pluginConfigId: pluginConfig39.id,
@ -1136,8 +1136,8 @@ describe('vm tests', () => {
await vm.methods.onEvent!(event)
await delay(1010)
expect(fetch).toHaveBeenCalledTimes(4)
expect(fetch).toHaveBeenCalledWith('https://onevent.com/')
expect(fetch).toHaveBeenCalledWith('https://export.com/results.json?query=exported&events=2')
expect(fetch).toHaveBeenCalledWith('https://onevent.com/', undefined)
expect(fetch).toHaveBeenCalledWith('https://export.com/results.json?query=exported&events=2', undefined)
})
test('buffers bytes with exportEventsBufferBytes', async () => {
@ -1264,10 +1264,16 @@ describe('vm tests', () => {
indexJs
)
await vm.methods.onEvent!(defaultEvent)
expect(fetch).not.toHaveBeenCalledWith('https://export.com/results.json?query=default event&events=1')
expect(fetch).not.toHaveBeenCalledWith(
'https://export.com/results.json?query=default event&events=1',
undefined
)
await vm.methods.teardownPlugin!()
expect(fetch).toHaveBeenCalledWith('https://export.com/results.json?query=default event&events=1')
expect(fetch).toHaveBeenCalledWith(
'https://export.com/results.json?query=default event&events=1',
undefined
)
})
})

37
posthog/api/test/test_utils.py
View File

@ -147,20 +147,35 @@ class TestUtils(BaseTest):
self.assertEqual(safe_clickhouse_string("💜 \u1f49c\ 💜"), "💜 \u1f49c\ 💜")
def test_raise_if_user_provided_url_unsafe(self):
# Sync test cases with plugin-server/src/utils/fetch.test.ts
raise_if_user_provided_url_unsafe("https://google.com?q=20") # Safe
raise_if_user_provided_url_unsafe("https://posthog.com") # Safe
raise_if_user_provided_url_unsafe("https://posthog.com/foo/bar") # Safe, with path
raise_if_user_provided_url_unsafe("https://posthog.com:443") # Safe, good port
raise_if_user_provided_url_unsafe("https://1.1.1.1") # Safe, public IP
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("https://posthog.com:80")) # Bad port
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("ftp://posthog.com")) # Bad scheme
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("")) # Empty
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("posthog.com")) # No scheme
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("http://localhost")) # Internal
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("http://192.168.0.5")) # Internal
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("http://0.0.0.0")) # Internal
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("http://10.0.0.24")) # Internal
self.assertRaises(ValueError, lambda: raise_if_user_provided_url_unsafe("http://172.20.0.21")) # Internal
self.assertRaises(
ValueError, lambda: raise_if_user_provided_url_unsafe("http://fgtggggzzggggfd.com")
self.assertRaisesMessage(ValueError, "No hostname", lambda: raise_if_user_provided_url_unsafe(""))
self.assertRaisesMessage(ValueError, "No hostname", lambda: raise_if_user_provided_url_unsafe("@@@"))
self.assertRaisesMessage(ValueError, "No hostname", lambda: raise_if_user_provided_url_unsafe("posthog.com"))
self.assertRaisesMessage(
ValueError,
"Scheme must be either HTTP or HTTPS",
lambda: raise_if_user_provided_url_unsafe("ftp://posthog.com"),
)
self.assertRaisesMessage(
ValueError, "Internal hostname", lambda: raise_if_user_provided_url_unsafe("http://localhost")
)
self.assertRaisesMessage(
ValueError, "Internal hostname", lambda: raise_if_user_provided_url_unsafe("http://192.168.0.5")
)
self.assertRaisesMessage(
ValueError, "Internal hostname", lambda: raise_if_user_provided_url_unsafe("http://0.0.0.0")
)
self.assertRaisesMessage(
ValueError, "Internal hostname", lambda: raise_if_user_provided_url_unsafe("http://10.0.0.24")
)
self.assertRaisesMessage(
ValueError, "Internal hostname", lambda: raise_if_user_provided_url_unsafe("http://172.20.0.21")
)
self.assertRaisesMessage(
ValueError, "Invalid hostname", lambda: raise_if_user_provided_url_unsafe("http://fgtggggzzggggfd.com")
) # Non-existent

4
posthog/api/user.py
View File

@ -33,6 +33,7 @@ from posthog.api.organization import OrganizationSerializer
from posthog.api.shared import OrganizationBasicSerializer, TeamBasicSerializer
from posthog.api.utils import raise_if_user_provided_url_unsafe
from posthog.auth import authenticate_secondarily
from posthog.cloud_utils import is_cloud
from posthog.email import is_email_available
from posthog.event_usage import report_user_logged_in, report_user_updated, report_user_verified_email
from posthog.models import Team, User, UserScenePersonalisation, Dashboard
@ -450,7 +451,8 @@ def test_slack_webhook(request):
return JsonResponse({"error": "no webhook URL"})
message = {"text": "_Greetings_ from PostHog!"}
try:
raise_if_user_provided_url_unsafe(webhook)
if is_cloud(): # Protect against SSRF
raise_if_user_provided_url_unsafe(webhook)
response = requests.post(webhook, verify=False, json=message)
if response.ok:

19
posthog/api/utils.py
View File

@ -302,23 +302,20 @@ def parse_bool(value: Union[str, List[str]]) -> bool:
def raise_if_user_provided_url_unsafe(url: str):
"""Raise if the provided URL seems unsafe, otherwise do nothing."""
parsed_url: urllib.parse.ParseResult = urllib.parse.urlparse(url)
"""Raise if the provided URL seems unsafe, otherwise do nothing.
Equivalent of plugin server raiseIfUserProvidedUrlUnsafe.
"""
parsed_url: urllib.parse.ParseResult = urllib.parse.urlparse(url) # urlparse never raises errors
if not parsed_url.hostname:
raise ValueError("No hostname")
if parsed_url.scheme == "http":
port = 80
elif parsed_url.scheme == "https":
port = 443
else:
if parsed_url.scheme not in ("http", "https"):
raise ValueError("Scheme must be either HTTP or HTTPS")
if parsed_url.port is not None and parsed_url.port != port:
raise ValueError("Port does not match scheme")
# Disallow if hostname resolves to a private (internal) IP address
try:
addrinfo = socket.getaddrinfo(parsed_url.hostname, port)
addrinfo = socket.getaddrinfo(parsed_url.hostname, None)
except socket.gaierror:
raise ValueError("Invalid hostname")
for _, _, _, _, sockaddr in addrinfo:
if ip_address(sockaddr[0]).is_private: # Prevent addressing internal services
raise ValueError("Invalid hostname")
raise ValueError("Internal hostname")