OverviewFilesCommitsIssuesDocsReleasesPeers

hyper-note commits / 275b199

security: guard getNote + media resolve against path traversal
Both functions concatenate a caller-supplied identifier into a drive path (/notes/<noteId>.json, /media/<identifier>) and are reachable from PUBLIC routes that don't pre-validate: web.js:306 passes noteId raw to getNote, and the public media routes pass identifier raw to MediaService.resolve. A value like '../notes/x' or 'a/../../y' could read outside the intended subtree. Validate at the source (defense in depth, covers every caller): reject any identifier containing '/', '\\', '..', NUL, or empty/non-string → return null (not found) before touching storage. Test: both guards reject a battery of traversal inputs and provably short- circuit before any storage access (stubbed managers throw if reached). 2/2 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
275b1995c03e230c0d8f7a5a896adaa0df935c38
by plankatron <fleeky@gmail.com> · 2026-06-23T20:18:06.000Z

Changed files (3) +42 -0

lib/note-manager.js +8 -0
……@@ -656,6 +656,14 @@ export class NoteManager {
656656 }
657657
658658 async getNote (notebook, noteId) {
659 // Path-traversal guard: noteId addresses /notes/<id>.json — it is never a
660 // path. Public routes (web.js) pass it raw, so validate at the source.
661 if (typeof noteId !== 'string' || !noteId ||
662 noteId.includes('/') || noteId.includes('\\') ||
663 noteId.includes('..') || noteId.includes('\0')) {
664 return null
665 }
666
659667 // SharedNotebook path (multi-writer)
660668 const snb = this._getSharedNotebook(notebook)
661669 if (snb) {
lib/services/media-service.js +8 -0
……@@ -92,6 +92,14 @@ export class MediaService {
9292 * - data: Buffer (for drive/shared fallbacks)
9393 */
9494 async resolve (notebook, identifier) {
95 // Path-traversal guard: identifier is a media hash/filename used to build a
96 // drive path (/media/<identifier>) — it is never a path itself.
97 if (typeof identifier !== 'string' || !identifier ||
98 identifier.includes('/') || identifier.includes('\\') ||
99 identifier.includes('..') || identifier.includes('\0')) {
100 return null
101 }
102
95103 // 1. MediaStore (content-addressed)
96104 const info = await this.mediaStore.getInfo(identifier)
97105 if (info) {
test/unit/path-traversal.test.js +26 -0
……@@ -0,0 +1,26 @@
1import test from 'brittle'
2import { NoteManager } from '../../lib/note-manager.js'
3import { MediaService } from '../../lib/services/media-service.js'
4
5const TRAVERSALS = ['../notes/secret', 'a/../../x', 'foo/bar', 'x\\y', '..', 'a\0b', '']
6
7test('getNote rejects path-traversal noteIds before any storage access', async (t) => {
8 const trap = () => { throw new Error('storage touched — traversal guard failed') }
9 const nm = new NoteManager({ getDrive: trap })
10 nm._getSharedNotebook = trap // must not be reached for malicious input
11
12 for (const bad of TRAVERSALS) {
13 t.is(await nm.getNote('nb', bad), null, `rejected noteId ${JSON.stringify(bad)}`)
14 }
15
16 clearInterval(nm._keyCacheCleanupInterval) // release the key-cache timer
17})
18
19test('MediaService.resolve rejects path-traversal identifiers before any storage access', async (t) => {
20 const trap = new Proxy({}, { get () { throw new Error('storage touched — traversal guard failed') } })
21 const ms = new MediaService({ mediaStore: trap, driveManager: trap, noteManager: trap })
22
23 for (const bad of TRAVERSALS) {
24 t.is(await ms.resolve('nb', bad), null, `rejected identifier ${JSON.stringify(bad)}`)
25 }
26})