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: URL validator #1

Merged
merged 16 commits into from
Jul 17, 2024
Merged
30 changes: 30 additions & 0 deletions .github/workflows/linting.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
name: Linting

on:
push:
branches-ignore:
- develop

jobs:
lint:
runs-on: ubuntu-latest
steps:
- name: Checkout 🛎️
uses: actions/checkout@v3
with:
persist-credentials: false

- name: Install pnpm
uses: pnpm/action-setup@v4

- name: Setup Node LTS ✨
uses: actions/setup-node@v3
with:
cache: pnpm
node-version: lts/*

- name: Installing dependencies 📦️
run: pnpm install

- name: Lint 🛀
run: pnpm -r run lint
51 changes: 51 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: Tests

on:
pull_request:
branches: [develop]
push:
branches: [develop]

jobs:
tests:
runs-on: ubuntu-latest
name: Run tests
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Install pnpm
uses: pnpm/action-setup@v4

- uses: actions/setup-node@v3
with:
cache: pnpm
node-version-file: ".nvmrc"

- name: Install dependencies
run: pnpm install

- name: Run tests
run: |
pnpm -r run test

build:
runs-on: ubuntu-latest
steps:
- name: Checkout source code
uses: actions/checkout@v3

- name: Install pnpm
uses: pnpm/action-setup@v4

- uses: actions/setup-node@v3
with:
cache: pnpm
node-version-file: ".nvmrc"

- name: Install
run: pnpm install

- name: Compile
run: |
pnpm -r run build
144 changes: 27 additions & 117 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,130 +1,40 @@
# Logs
logs
*.log
npm-debug.log*
yarn-debug.log*
yarn-error.log*
lerna-debug.log*
.pnpm-debug.log*

# Diagnostic reports (https://nodejs.org/api/report.html)
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json

# Runtime data
pids
*.pid
*.seed
*.pid.lock

# Directory for instrumented libs generated by jscoverage/JSCover
lib-cov

# Coverage directory used by tools like istanbul
coverage
*.lcov

# nyc test coverage
.nyc_output

# Grunt intermediate storage (https://gruntjs.com/creating-plugins#storing-task-files)
.grunt

# Bower dependency directory (https://bower.io/)
bower_components

# node-waf configuration
.lock-wscript

# Compiled binary addons (https://nodejs.org/api/addons.html)
build/Release

# Dependency directories
node_modules/
jspm_packages/

# Snowpack dependency directory (https://snowpack.dev/)
web_modules/

# TypeScript cache
*.tsbuildinfo

# Optional npm cache directory
.npm

# Optional eslint cache
.eslintcache
# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.

# Optional stylelint cache
.stylelintcache
# Dependencies
node_modules
.pnp
.pnp.js

# Microbundle cache
.rpt2_cache/
.rts2_cache_cjs/
.rts2_cache_es/
.rts2_cache_umd/

# Optional REPL history
.node_repl_history

# Output of 'npm pack'
*.tgz

# Yarn Integrity file
.yarn-integrity

# dotenv environment variable files
# Local env files
.env
.env.local
.env.development.local
.env.test.local
.env.production.local
.env.local

# parcel-bundler cache (https://parceljs.org/)
.cache
.parcel-cache

# Next.js build output
.next
out

# Nuxt.js build / generate output
.nuxt
dist

# Gatsby files
.cache/
# Comment in the public line in if your project uses Gatsby and not Next.js
# https://nextjs.org/blog/next-9-1#public-directory-support
# public

# vuepress build output
.vuepress/dist

# vuepress v2.x temp and cache directory
.temp
.cache

# Docusaurus cache and generated files
.docusaurus
# Testing
coverage

# Serverless directories
.serverless/
# Turbo
.turbo

# FuseBox cache
.fusebox/
# Vercel
.vercel

# DynamoDB Local files
.dynamodb/
# Build Outputs
.next/
out/
build
dist

# TernJS port file
.tern-port

# Stores VSCode versions used for testing VSCode extensions
.vscode-test
# Debug
npm-debug.log*
yarn-debug.log*
yarn-error.log*

# yarn v2
.yarn/cache
.yarn/unplugged
.yarn/build-state.yml
.yarn/install-state.gz
.pnp.*
# Misc
.DS_Store
*.pem
.vscode/
.eslintcache
Empty file added .npmrc
Empty file.
1 change: 1 addition & 0 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
v20.10.0
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
# starter-kitty
# Starter Kitty

Common app components that are safe-by-default.

## Packages

- [`@opengovsg/starter-kitty-validators`](./packages/validators/): Common input validators.
19 changes: 19 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "starter-kitty",
"private": true,
"scripts": {
"build": "turbo build",
"dev": "turbo dev",
"lint": "turbo lint",
"format": "prettier --write \"**/*.{ts,tsx,md}\""
},
"devDependencies": {
"prettier": "^3.2.5",
"turbo": "^2.0.6",
"typescript": "^5.4.5"
},
"packageManager": "[email protected]",
"engines": {
"node": ">=18"
}
}
24 changes: 24 additions & 0 deletions packages/validators/.eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"extends": ["opengovsg"],
"ignorePatterns": ["dist/**/*", "vitest.config.ts"],
"plugins": ["import", "eslint-plugin-tsdoc"],
"rules": {
"import/no-unresolved": "error",
"tsdoc/syntax": "error"
},
"parser": "@typescript-eslint/parser",
"parserOptions": {
"project": "**/tsconfig.json"
},
"settings": {
"import/parsers": {
"@typescript-eslint/parser": [".ts", ".tsx"]
},
"import/resolver": {
"typescript": {
"alwaysTryTypes": true,
"project": "**/tsconfig.json"
}
}
}
}
1 change: 1 addition & 0 deletions packages/validators/.prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tsconfig.json
6 changes: 6 additions & 0 deletions packages/validators/.prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"trailingComma": "all",
"tabWidth": 2,
"semi": false,
"singleQuote": true
}
60 changes: 60 additions & 0 deletions packages/validators/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Validators

This package provides a set of validators that provide sensible defaults to prevent common security vulnerabilities.

## Class: `UrlValidator`

Validates URLs against a whitelist of allowed protocols and hostnames, preventing open redirects, XSS, SSRF, and other security vulnerabilities.

### new UrlValidator(options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this as TSDoc comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks boss ur the best


`options?`: `<Object>`

- `baseOrigin`: `<string>` - The base origin to use for relative URLs. If no base origin is provided, relative URLs will be considered invalid.

An origin does not include the path or query parameters. For example, a valid base origin is `https://example.com` or `http://localhost:3000`.

- `whitelist`: `<Object>`
- `protocols`: `<string[]>` - A list of allowed protocols. If no protocols are provided, the validator will use the default protocols: `['http', 'https']`.

**Caution: allowing `javascript` or `data` protocols can lead to XSS vulnerabilities.**
- `hostnames`: `<string[]>` - A list of allowed hostnames. If no hostnames are provided, the validator will allow any hostname.

**It is recommended to provide a list of allowed hostnames to prevent open redirects.**

If no options are provided, the validator will use the default options:

```javascript
{
whitelist: {
protocols: ['http', 'https'],
},
}
```

### UrlValidator.parse(url)

- `url`: `<string>` - The URL to parse.
- Returns: `<URL>` - The parsed URL object.
- Throws: `<UrlValidationError>` - If the URL is invalid or unsafe.

### Example

```javascript
const validator = new UrlValidator({
whitelist: {
protocols: ['http', 'https', 'mailto'],
hosts: ['open.gov.sg'],
},
})
```

Validating a post-login redirect URL provided in a query parameter:

```javascript
try {
router.push(validator.parse(redirectUrl))
} catch (error) {
router.push('/home')
}
Comment on lines +55 to +59
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most validators use more of an if ... else pattern based on a boolean return value; is there a strong reason to use the try catch pattern instead? I guess one advantage is more meaningful error messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I also wanted to return a parsed version of the URL, which might be safer than saying OK to the string URL and letting them re-use it in e.g. router.push() - in the Isomer case, it was something like if isRelativeUrl(url) then router.push(url), but the Next.js weirdness could potentially have been avoided if parse returns a sane URL and it was router.push(parse(url)).

If we want to make it a parser + validator in that it returns the parsed result, I don't really like the "returning both the result and status" pattern because it doesn't force devs to handle errors, but throwing exceptions do. Maybe one option would be to do something like zod's parse and safeParse and let people choose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! I think stick to a single implementation i.e. the try catch pattern.

```
Loading
Loading