support absolute root path for serveStatic

[miyamonz]

Currently, serveStatic's options.root doesn't support absolute paths. It appears that the getFilePath utility function isn't designed for this case. I've created a small wrapper to address this issue.

[yusukebe]

HI @miyamonz !

Thank you for the PR. This is a good feature. I've leaved some comments. Please check them!

[lilnasy]

@miyamonz @yusukebe Spent a solid 30 minutes on this bug. ~~Do you mind if I continue this PR?~~ Seems like only the only comment is for a function name. Would love to see this land soon.

[yusukebe]

I've reconsidered this feature. Allowing absolute paths could potentially lead to a traversal attack vulnerability.

If we decide to proceed, we need to address this risk seriously. I would like to hear opinions about this potential security issue.

[lilnasy]

I'm not sure I see the concern. It's common enough to offer absolute path as root, and it's done by disallowing "/../" in the path.

For example, this is what send uses:

https://github.com/pillarjs/send/blob/b69cbb3dc4c09c37917d08a4c13fcd1bac97ade5/index.js#L63

Could you elaborate where a threat could come from?

[yusukebe]

In Hono, we also do not allow /../ in this line. So, as you mentioned, we might not need to consider it as a risk.

https://github.com/honojs/hono/blob/main/src/utils/filepath.ts#L9

[lilnasy]

That's reassuring. What do you think is next to progress this PR?

[yuyinws]

Any progress on this PR? It's very useful in some cases.

[schettn]

Any progress on this PR? It's very useful in some cases.

Workaround: https://github.com/honojs/hono/pull/3108#issuecomment-2225636805

[fumieval]

I've just stumbled upon this issue. I think one way to move this forward is to add a field like allowAbsoluteRoot: true to ServeStaticOptions, so that existing users of serveStatic are not affected by potential security issues.

[schettn]

@yusukebe

Why not just introduce another param?

serveStatic({ root: './' })) serveStatic({ absoluteRoot: '/' }))

• Add some checks to prevent both using both. (Typescript unions / runtime checks)

And then merge them at some point when hono has a major.

[yusukebe]

@fumieval @schettn

I like allowAbsoluteRoot: true option. Letting users choose the option is a good idea.

[schettn]

Should this be implemented in import { getFilePath } from 'hono/utils/filepath'?

Then just pass allowAbsoluteRoot to getFilePath instead of implementing getFilePathforAbsRoot here.

[yusukebe]

Hey! I'll work on this matter from now.