Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add allowedCharactersInPath whitelist #39

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open
11 changes: 11 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,16 @@
"packageManager": "[email protected]",
"engines": {
"node": ">=18"
},
"pnpm": {
"overrides": {
"braces@<3.0.3": ">=3.0.3",
"micromatch@<4.0.8": ">=4.0.8",
"vite@>=5.3.0 <5.3.6": ">=5.3.6",
"vite@>=5.3.0 <=5.3.5": ">=5.3.6",
"rollup@>=4.0.0 <4.22.4": ">=4.22.4",
"cross-spawn@>=7.0.0 <7.0.5": ">=7.0.5",
"nanoid@<3.3.8": ">=3.3.8"
}
}
}
2 changes: 1 addition & 1 deletion packages/validators/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@opengovsg/starter-kitty-validators",
"version": "1.2.11",
"version": "1.2.12",
"main": "./dist/index.js",
"types": "./dist/index.d.ts",
"exports": {
Expand Down
138 changes: 132 additions & 6 deletions packages/validators/src/__tests__/url.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { describe, expect, it } from 'vitest'
import { ZodError } from 'zod'

import { OptionsError } from '@/common/errors'
import { createUrlSchema, RelUrlValidator, UrlValidator } from '@/index'
import { UrlValidationError } from '@/url/errors'
import { defaultValidPathRegex } from '@/url/options'

describe('UrlValidator with default options', () => {
const validator = new UrlValidator()
Expand All @@ -12,6 +14,11 @@
expect(url).toBeInstanceOf(URL)
})

it('should parse a valid URL', () => {
const url = validator.parse('https://example.com/path?query=1#hash')
expect(url).toBeInstanceOf(URL)
})

it('should throw an error when the protocol is not http or https', () => {
expect(() => validator.parse('ftp://example.com')).toThrow(UrlValidationError)
})
Expand All @@ -26,14 +33,22 @@
expect(() => validator.parse('/path')).toThrow(UrlValidationError)
})

it('should not allow double slashes in pathnames', () => {
expect(() => validator.parse('https://example.com//test')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com\\\\test')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com/\\test')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com\\/test')).toThrow(UrlValidationError)
})

it('should not allow Next.js dynamic routes', () => {
expect(() => validator.parse('https://example.com/[[...slug]]')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com/[[slug]]')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com/[x]?x=1')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com/path/(.)part')).toThrow(UrlValidationError)
})

it('should prevent XSS attacks', () => {
expect(() => validator.parse('javascript&colonalert(/xss/)').protocol).toThrow(UrlValidationError)
expect(() => validator.parse('javascript&colonalert(/xss/)')).toThrow(UrlValidationError)
expect(() => validator.parse('javascript:alert(/xss/)')).toThrow(UrlValidationError)
})

Expand All @@ -47,19 +62,20 @@
describe('UrlValidator with custom protocol whitelist', () => {
const validator = new UrlValidator({
whitelist: {
protocols: ['http', 'https', 'mailto'],
protocols: ['http', 'https'],
validPathRegex: /.*/, // allow all characters for tests
},
})

it('should not throw an error when the protocol on the whitelist', () => {
expect(() => validator.parse('https://example.com')).not.toThrow()
expect(() => validator.parse('http://example.com')).not.toThrow()
expect(() => validator.parse('mailto:[email protected]')).not.toThrow()
})

it('should throw an error when the protocol is not on the whitelist', () => {
expect(() => validator.parse('ftp://example.com')).toThrow(UrlValidationError)
expect(() => validator.parse('javascript:alert()')).toThrow(UrlValidationError)
expect(() => validator.parse('mailto:[email protected]')).toThrow(UrlValidationError)
})
})

Expand All @@ -68,6 +84,7 @@
whitelist: {
protocols: ['http', 'https'],
hosts: ['example.com'],
validPathRegex: /.*/, // allow all characters for tests
},
})

Expand All @@ -85,13 +102,19 @@
whitelist: {
protocols: ['http', 'https'],
disallowHostnames: true,
validPathRegex: /.*/, // allow all characters for tests
},
})

it('should not throw an error with a proper domain', () => {
expect(() => validator.parse('https://example.com')).not.toThrow()
})

it('should not throw an error with a proper domain', () => {
const url = validator.parse('https://example.com/path?query=1#hash')
expect(url).toBeInstanceOf(URL)
})

it('should throw an error with a hostname', () => {
expect(() => validator.parse('https://tld')).toThrow(UrlValidationError)
expect(() => validator.parse('https://.tld')).toThrow(UrlValidationError)
Expand All @@ -110,13 +133,19 @@
protocols: ['http', 'https'],
hosts: ['example.com', 'localhost'],
disallowHostnames: true,
validPathRegex: /.*/, // allow all characters for tests
},
})

it('should not throw an error when the host is on the whitelist', () => {
expect(() => validator.parse('https://example.com')).not.toThrow()
})

it('should not throw an error when the host is on the whitelist', () => {
const url = validator.parse('https://example.com/path?query=1#hash')
expect(url).toBeInstanceOf(URL)
})

it('should ignore the disallowHostnames option', () => {
expect(() => validator.parse('https://localhost')).not.toThrow()
})
Expand All @@ -125,6 +154,10 @@
describe('UrlValidator with base URL', () => {
const validator = new UrlValidator({
baseOrigin: 'https://example.com',
whitelist: {
protocols: ['http', 'https'], // default
validPathRegex: /.*/, // allow all characters for tests
},
})

it('should parse a valid relative URL', () => {
Expand Down Expand Up @@ -161,6 +194,82 @@
UrlValidationError,
)
})

it('should not allow Next.js dynamic routes', () => {
expect(() => validator.parse('/[[x]]http:example.com/(.)[y]/?x&y')).toThrow(UrlValidationError)
})
})

describe('UrlValidator with valid path regex', () => {
const validator = new UrlValidator({
whitelist: {
protocols: ['http', 'https'],
validPathRegex: /^[abc123/]*$/,
},
})
it('should parse a valid URL', () => {
const url = validator.parse('https://example.com/abc123')
expect(url).toBeInstanceOf(URL)
})

it('should parse a valid URL with query string and hash', () => {
const url = validator.parse('https://example.com/abc123?q=1#hash')
expect(url).toBeInstanceOf(URL)
})

it('should throw an error when the path does not conform to path regex', () => {
expect(() => validator.parse('https://example.com/abc1234')).toThrow(UrlValidationError)
})
})

describe('UrlValidator with the default valid path regex', () => {
const validator = new UrlValidator({})

it('should parse a valid URL', () => {
const url = validator.parse('https://example.com')
expect(url).toBeInstanceOf(URL)
})

it('should parse a valid URL', () => {
const url = validator.parse('https://example.com/path?query=1#hash')
expect(url).toBeInstanceOf(URL)
})

it('should throw an error when the path does not conform to path regex', () => {
expect(() => validator.parse('https://example.com/1@23')).toThrow(UrlValidationError)
})

it('should throw an error when the protocol is not http or https', () => {
expect(() => validator.parse('ftp://example.com')).toThrow(UrlValidationError)
})

it('should allow any host when no host whitelist is provided', () => {
expect(() => validator.parse('https://open.gov.sg')).not.toThrow()
})

// https://url.spec.whatwg.org/#missing-scheme-non-relative-url
it('should throw an error when missing a scheme and no base URL or base URL', () => {
expect(() => validator.parse('example.com')).toThrow(UrlValidationError)
expect(() => validator.parse('/path')).toThrow(UrlValidationError)
})

it('should not allow Next.js dynamic routes', () => {
expect(() => validator.parse('https://example.com/[[...slug]]')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com/[[slug]]')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com/[x]?x=1')).toThrow(UrlValidationError)
expect(() => validator.parse('https://example.com/path/(.)part')).toThrow(UrlValidationError)
})

it('should prevent XSS attacks', () => {
expect(() => validator.parse('javascript&colonalert(/xss/)')).toThrow(UrlValidationError)
expect(() => validator.parse('javascript:alert(/xss/)')).toThrow(UrlValidationError)
})

it('should throw an error when given an invalid type', () => {
expect(() => validator.parse(123)).toThrow(UrlValidationError)
expect(() => validator.parse(undefined)).toThrow(UrlValidationError)
expect(() => validator.parse(['1', '2'])).toThrow(UrlValidationError)
})
})

describe('UrlValidator with invalid options', () => {
Expand All @@ -185,6 +294,11 @@
expect(url).toBeInstanceOf(URL)
})

it('should parse a valid absolute URL', () => {
const url = validator.parse('https://a.com/path?query=1#hash')
expect(url).toBeInstanceOf(URL)
})

it('should throw an error on invalid URL', () => {
expect(() => validator.parse('https://b.com/hello')).toThrow(UrlValidationError)
})
Expand Down Expand Up @@ -251,6 +365,7 @@

it('should throw an error when the path is a NextJS dynamic path', () => {
expect(() => validator.parsePathname('https://a.com/hello/[id]?id=3')).toThrow(UrlValidationError)
expect(() => validator.parsePathname('https://a.com/hello/(.)bye')).toThrow(UrlValidationError)
})

it('should fallback to fallbackUrl if it is provided', () => {
Expand All @@ -271,7 +386,17 @@
protocols: ['http', 'https', 'mailto'],
},
})
expect(() => schema.parse('mailto:[email protected]')).not.toThrow()
expect(() => schema.parse('https://example.com')).not.toThrow()
})

it('should create a schema with custom options', () => {
const schema = createUrlSchema({
whitelist: {
protocols: ['http', 'https', 'mailto'],
validPathRegex: /^[a-zA-Z0-9.-_/@]*$/,
Fixed Show fixed Hide fixed
},
})
expect(() => schema.parse('mailto:[email protected]')).not.toThrow()
})

it('should throw an error when the options are invalid', () => {
Expand All @@ -287,13 +412,14 @@
protocols: ['http', 'https'],
hosts: ['example.com'],
disallowHostnames: true,
validPathRegex: defaultValidPathRegex,
},
}),
).not.toThrow()
})

it('should reject relaative URLs when the base URL is not provided', () => {
it('should reject relative URLs when the base URL is not provided', () => {
const schema = createUrlSchema()
expect(() => schema.parse('/path')).toThrow(UrlValidationError)
expect(() => schema.parse('/path')).toThrow(ZodError)
})
})
3 changes: 2 additions & 1 deletion packages/validators/src/url/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { fromError } from 'zod-validation-error'

import { OptionsError } from '@/common/errors'
import { UrlValidationError } from '@/url/errors'
import { defaultOptions, optionsSchema, UrlValidatorOptions } from '@/url/options'
import { defaultOptions, defaultValidPathRegex, optionsSchema, UrlValidatorOptions } from '@/url/options'
import { toSchema } from '@/url/schema'

/**
Expand Down Expand Up @@ -128,6 +128,7 @@ export class RelUrlValidator extends UrlValidator {
whitelist: {
protocols: ['http', 'https'],
hosts: [urlObject.host],
validPathRegex: defaultValidPathRegex,
},
})
}
Expand Down
15 changes: 13 additions & 2 deletions packages/validators/src/url/options.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { z } from 'zod'

export const defaultValidPathRegex = /^[a-zA-Z0-9._\-/]*$/

export const defaultOptions = {
whitelist: {
protocols: ['http', 'https'],
disallowHostnames: false,
validPathRegex: defaultValidPathRegex,
},
}

Expand All @@ -27,7 +30,15 @@ export const whitelistSchema = z.object({
*
* @defaultValue false
*/
disallowHostnames: z.boolean().optional(),
disallowHostnames: z.boolean().default(false),
/**
* Regex to match against the URL path.
* Use / .* / to allow all characters
* If your protocol is mailto, you will need to include "\@" here.
*
* @defaultValue defaultValidPathRgex
*/
validPathRegex: z.instanceof(RegExp).default(defaultValidPathRegex),
})

/**
Expand Down Expand Up @@ -61,7 +72,7 @@ export interface UrlValidatorOptions {
*
* @public
*/
export type UrlValidatorWhitelist = z.infer<typeof whitelistSchema>
export type UrlValidatorWhitelist = z.input<typeof whitelistSchema>

export const optionsSchema = z.object({
baseOrigin: z
Expand Down
Loading
Loading