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

feature: disable root path check when serve is false #467

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -435,9 +435,34 @@ Assume this structure with the compressed asset as a sibling of the un-compresse
└── index.html
```

#### Disable serving
#### Disable serving and CWD behavior

If you would just like to use the reply decorator and not serve whole directories automatically, you can simply pass the option `{ serve: false }`. This will prevent the plugin from serving everything under `root`.
If you want to use only the reply decorator without automatically serving whole directories, pass the option `{ serve: false }`. This prevents the plugin from serving everything under `root`.

When `serve: false` is used:

- If no `root` is provided, the plugin will use the current working directory (CWD) as the default root.
- The `sendFile` method will send files relative to the CWD or the specified `root`.

Example usage:

```js
const fastify = require('fastify')()
const path = require('node:path')

fastify.register(require('@fastify/static'), {
serve: false,
// root is optional when serve is false, will default to CWD if not provided
root: path.join(__dirname, 'public')
})

fastify.get('/file', (req, reply) => {
// This will serve the file from the CWD if no root was provided
reply.sendFile('myFile.html')
})
```

This configuration allows you to use the `sendFile` decorator without automatically serving an entire directory.

#### Disabling reply decorator

Expand Down
45 changes: 28 additions & 17 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ const supportedEncodings = ['br', 'gzip', 'deflate']
send.mime.default_type = 'application/octet-stream'

async function fastifyStatic (fastify, opts) {
if (opts.serve === false && opts.root === undefined) {
opts.root = process.cwd()
fastify.log.warn('No root path provided. Defaulting to current working directory. This may pose security risks if not intended.')
}

opts.root = normalizeRoot(opts.root)
checkRootPathForErrors(fastify, opts.root)
checkRootPathForErrors(fastify, opts.root, opts.serve === false)

const setHeaders = opts.setHeaders
if (setHeaders !== undefined && typeof setHeaders !== 'function') {
Expand Down Expand Up @@ -408,7 +413,7 @@ function normalizeRoot (root) {
return root
}

function checkRootPathForErrors (fastify, rootPath) {
function checkRootPathForErrors (fastify, rootPath, skipExistenceCheck) {
Copy link
Member

Choose a reason for hiding this comment

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

I am think is it necessary to skip the check of root path?
You should never use a invalid root path, defaulting to cwd is a trade-off for security reason.

All of your served file should be contained within to the root and prevent path escape.

Copy link
Author

@nimesh0505 nimesh0505 Sep 12, 2024

Choose a reason for hiding this comment

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

@climba03003 Thank you for your feedback on the security implications. Before implementing changes, I would like to confirm the approach that we remove the skipExistenceCheck parameter from the checkPath and checkRootPathForErrors functions since we need perform full validation of the path.

Copy link
Member

Choose a reason for hiding this comment

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

You can skip for cwd, but you must provide a valid root.
No matter other setting.

if (rootPath === undefined) {
throw new Error('"root" option is required')
}
Expand All @@ -425,40 +430,46 @@ function checkRootPathForErrors (fastify, rootPath) {
}

// check each path and fail at first invalid
rootPath.map((path) => checkPath(fastify, path))
rootPath.map((path) => checkPath(fastify, path, skipExistenceCheck))
return
}

if (typeof rootPath === 'string') {
return checkPath(fastify, rootPath)
return checkPath(fastify, rootPath, skipExistenceCheck)
}

throw new Error('"root" option must be a string or array of strings')
}

function checkPath (fastify, rootPath) {
function checkPath (fastify, rootPath, skipExistenceCheck) {
// skip all checks if rootPath is the CWD
if (rootPath === process.cwd()) {
return
}
if (typeof rootPath !== 'string') {
throw new Error('"root" option must be a string')
}
if (path.isAbsolute(rootPath) === false) {
throw new Error('"root" option must be an absolute path')
}

let pathStat
if (!skipExistenceCheck) {
let pathStat

try {
pathStat = statSync(rootPath)
} catch (e) {
if (e.code === 'ENOENT') {
fastify.log.warn(`"root" path "${rootPath}" must exist`)
return
}
try {
pathStat = statSync(rootPath)
} catch (e) {
if (e.code === 'ENOENT') {
fastify.log.warn(`"root" path "${rootPath}" must exist`)
return
}

throw e
}
throw e
}

if (pathStat.isDirectory() === false) {
throw new Error('"root" option must point to a directory')
if (pathStat.isDirectory() === false) {
throw new Error('"root" option must point to a directory')
}
}
}

Expand Down
73 changes: 73 additions & 0 deletions test/static.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4023,3 +4023,76 @@ t.test('content-length in head route should not return zero when using wildcard'
})
})
})

t.test('serve: false disables root path check', t => {
t.plan(4)

t.test('should default to CWD when no root is provided', async (t) => {
t.plan(3)
const fastify = Fastify()

let warningLogged = false
fastify.log.warn = (message) => {
if (message.includes('No root path provided')) {
warningLogged = true
}
}

await fastify.register(fastifyStatic, { serve: false })

t.ok(warningLogged, 'should log a warning about defaulting to CWD')

fastify.get('/test', (req, reply) => {
reply.sendFile('test/static/index.html')
})

const response = await fastify.inject('/test')
t.equal(response.statusCode, 200, 'should serve file from CWD')

const fs = require('fs')
const expectedContent = fs.readFileSync('test/static/index.html', 'utf8')
t.equal(response.payload, expectedContent, 'should serve correct file content')
})

t.test('should not throw when root is non-existent directory and serve is false', async (t) => {
t.plan(1)
const fastify = Fastify()
const nonExistentPath = path.join(__dirname, 'definitely-non-existent-directory')

await fastify.register(fastifyStatic, {
serve: false,
root: nonExistentPath
})
t.pass('should not throw for non-existent directory')
})

t.test('should still validate root is a string or array of strings', async (t) => {
t.plan(1)
const fastify = Fastify()

try {
await fastify.register(fastifyStatic, {
serve: false,
root: 123
})
t.fail('Should have thrown an error for non-string root')
} catch (error) {
t.match(error.message, /"root" option must be a string or array of strings/, 'Should throw for non-string root')
}
})

t.test('should still validate root is an absolute path', async (t) => {
t.plan(1)
const fastify = Fastify()

try {
await fastify.register(fastifyStatic, {
serve: false,
root: 'relative/path'
})
t.fail('Should have thrown an error for relative path')
} catch (error) {
t.match(error.message, /"root" option must be an absolute path/, 'Should throw for relative path')
}
})
})