fix: search could list files that shouldn't be

Massimo Melina committed Jan 18, 2025 at 20:28 UTC 9624e08801b9fd05eb3143d51f152b25445520bd
6 files changed +163 -148
src/api.vfs.ts
+6 -5
@@ -7,7 +7,7 @@ import { mkdir, stat } from 'fs/promises'
7 import { ApiError, ApiHandlers } from './apiMiddleware'
8 import { dirname, extname, join, resolve } from 'path'
9 import {
10 - dirStream, enforceFinal, enforceStarting, isDirectory, isValidFileName, isWindowsDrive, makeMatcher, PERM_KEYS,
10 + enforceFinal, enforceStarting, isDirectory, isValidFileName, isWindowsDrive, makeMatcher, PERM_KEYS,
11 VfsNodeAdminSend
12 } from './misc'
13 import {
@@ -17,6 +17,7 @@ import {
17 import { getDiskSpace, getDiskSpaces, getDrives, reg } from './util-os'
18 import { getBaseUrlOrDefault, getServerStatus } from './listen'
19 import { SendListReadable } from './SendList'
20 +import { walkDir } from './walkDir'
21
22 // to manipulate the tree we need the original node
23 async function urlToNodeOriginal(uri: string) {
@@ -202,14 +203,14 @@ const apis: ApiHandlers = {
203 try {
204 const matching = makeMatcher(fileMask)
205 path = isWindowsDrive(path) ? path + '\\' : resolve(path || '/')
205 - for await (const entry of dirStream(path)) {
206 + await walkDir(path, {}, async entry => {
207 if (ctx.isAborted())
207 - return
208 + return null
209 const {path:name} = entry
210 const isDir = entry.isDirectory()
211 if (!isDir)
212 if (!files || fileMask && !matching(name))
212 - continue
213 + return
214 try {
215 const stats = entry.stats || await stat(join(path, name))
216 list.add({
@@ -220,7 +221,7 @@ const apis: ApiHandlers = {
221 k: isDir ? 'd' : undefined,
222 })
223 } catch {} // just ignore entries we can't stat
223 - }
224 + })
225 await sendPropsAsap.catch(() => {})
226 list.close()
227 } catch (e: any) {
src/util-files.ts
-11
@@ -7,7 +7,6 @@ import { basename, dirname } from 'path'
7 import glob from 'fast-glob'
8 import { IS_WINDOWS } from './const'
9 import { once, Readable } from 'stream'
10 -import { createDirStream, DirStreamEntry } from './dirStream'
10 // @ts-ignore
11 import unzipper from 'unzip-stream'
12
@@ -73,16 +72,6 @@ export function adjustStaticPathForGlob(path: string) {
72 return glob.escapePath(path.replace(/\\/g, '/'))
73 }
74
76 -export async function* dirStream(path: string, { depth=0, onlyFiles=false, onlyFolders = false, hidden=true }={}) {
77 - if (!await isDirectory(path))
78 - throw Error('ENOTDIR')
79 - for await (const entry of createDirStream(path, { depth, hidden })) {
80 - const dirent = entry as DirStreamEntry
81 - if (dirent.isDirectory() ? onlyFiles : (onlyFolders || !dirent.isFile())) continue
82 - yield dirent
83 - }
84 -}
85 -
75 export async function unzip(stream: Readable, cb: (path: string) => Promisable<false | string>) {
76 let pending: Promise<any> = Promise.resolve()
77 return new Promise((resolve, reject) =>
src/vfs.ts
+113 -80
@@ -3,7 +3,7 @@
3 import fs from 'fs/promises'
4 import { basename, dirname, join, resolve } from 'path'
5 import {
6 - dirStream, makeMatcher, setHidden, onlyTruthy, isValidFileName, throw_, VfsPerms, Who,
6 + makeMatcher, setHidden, onlyTruthy, isValidFileName, throw_, VfsPerms, Who,
7 isWhoObject, WHO_ANY_ACCOUNT, defaultPerms, PERM_KEYS, removeStarting, HTTP_SERVER_ERROR, try_
8 } from './misc'
9 import Koa from 'koa'
@@ -16,6 +16,8 @@ import { getCurrentUsername } from './auth'
16 import { Stats } from 'node:fs'
17 import fswin from 'fswin'
18 import { DESCRIPT_ION, descriptIon } from './comments'
19 +import { walkDir } from './walkDir'
20 +import { Readable } from 'node:stream'
21
22 const showHiddenFiles = defineConfig('show_hidden_files', false)
23
@@ -116,7 +118,7 @@ export async function urlToNode(url: string, ctx?: Koa.Context, parent: VfsNode=
118 try {
119 if (!showHiddenFiles.get() && await isHiddenFile(ret.source))
120 throw 'hiddenFile'
119 - ret.isFolder = (await nodeStats(ret))!.isDirectory() // throws if doesn't exist on disk
121 + ret.isFolder = (await nodeStats(ret))!.isDirectory() // throws if it doesn't exist on disk
122 }
123 catch {
124 if (!getRest)
@@ -266,6 +268,14 @@ export function statusCodeForMissingPerm(node: VfsNode, perm: keyof VfsPerms, ct
268 }
269 }
270
271 +interface WalkNodeOptions {
272 + ctx?: Koa.Context,
273 + depth?: number,
274 + prefixPath?: string,
275 + requiredPerm?: undefined | keyof VfsPerms,
276 + onlyFolders?: boolean,
277 + onlyFiles?: boolean
278 +}
279 // it's responsibility of the caller to verify you have list permission on parent, as callers have different needs.
280 export async function* walkNode(parent: VfsNode, {
281 ctx,
@@ -274,89 +284,112 @@ export async function* walkNode(parent: VfsNode, {
284 requiredPerm,
285 onlyFolders = false,
286 onlyFiles = false,
277 -}: { ctx?: Koa.Context,depth?: number, prefixPath?: string, requiredPerm?: undefined | keyof VfsPerms, onlyFolders?: boolean, onlyFiles?: boolean } = {}): AsyncIterableIterator<VfsNode> {
278 - const { children, source } = parent
279 - const taken = prefixPath ? undefined : new Set()
280 - const maskApplier = parentMaskApplier(parent)
281 - const parentsCache = new Map() // we use this only if depth > 0
282 - const visitLater: any = []
283 - if (children)
284 - for (const child of children) {
285 - if (await nodeIsDirectory(child) ? onlyFiles : onlyFolders) continue
286 - const nodeName = getNodeName(child)
287 - const name = prefixPath + nodeName
288 - taken?.add(normalizeFilename(name))
289 - const item = { ...child, name }
290 - if (!await canSee(item)) continue
291 - if (item.source) // real items must be accessible
292 - try { await fs.access(item.source) }
293 - catch { continue }
294 - yield item
295 - if (!depth || !await nodeIsDirectory(child).catch(() => false)) continue
296 - parentsCache.set(name, item)
297 - inheritMasks(item, parent, nodeName)
298 - if (!ctx || hasPermission(item, 'can_list', ctx)) // check perm before recursion
287 +}: WalkNodeOptions = {}) {
288 + let started = false
289 + const stream = new Readable({
290 + objectMode: true,
291 + async read() {
292 + if (started) return // for simplicity, we care about starting, and never suspend
293 + started = true
294 + const { children, source } = parent
295 + const taken = prefixPath ? undefined : new Set()
296 + const maskApplier = parentMaskApplier(parent)
297 + const parentsCache = new Map() // we use this only if depth > 0
298 + const visitLater: any = []
299 + if (children) for (const child of children) {
300 + const nodeName = getNodeName(child)
301 + const name = prefixPath + nodeName
302 + taken?.add(normalizeFilename(name))
303 + const item = { ...child, name }
304 + if (await cantSee(item)) continue
305 + if (item.source) // real items must be accessible
306 + try { await fs.access(item.source) }
307 + catch { continue }
308 + const isFolder = await nodeIsDirectory(child)
309 + if (onlyFiles ? !isFolder : (!onlyFolders || isFolder))
310 + stream.push(item)
311 + if (!depth || !isFolder || cantRecur(item)) continue
312 + parentsCache.set(name, item)
313 visitLater.push([item, name]) // prioritize siblings
300 - }
301 - try {
302 -
303 - if (!source)
304 - return
305 - if (requiredPerm && ctx // no permission, no reason to continue (at least for dynamic elements)
306 - && !hasPermission(parent, requiredPerm, ctx)
307 - && !masksCouldGivePermission(parent.masks, requiredPerm))
308 - return
314 + }
315
310 - try {
311 - let lastDir = prefixPath.slice(0, -1) || '.'
312 - parentsCache.set(lastDir, parent)
313 - for await (const entry of dirStream(source, { depth, onlyFolders, hidden: showHiddenFiles.get() })) {
314 - if (ctx?.isAborted()) break
315 - const {path} = entry
316 - const isFolder = entry.isDirectory()
317 - const name = prefixPath + (parent.rename?.[path] || path)
318 - if (descriptIon.get() && basename(name) === DESCRIPT_ION)
319 - continue
320 - if (taken?.has(normalizeFilename(name))) continue
321 - if (depth) {
322 - const dir = dirname(name)
323 - if (dir !== lastDir)
324 - parent = parentsCache.get(lastDir = dir)
316 + try {
317 + if (!source)
318 + return
319 + if (requiredPerm && ctx // no permission, no reason to continue (at least for dynamic elements)
320 + && !hasPermission(parent, requiredPerm, ctx)
321 + && !masksCouldGivePermission(parent.masks, requiredPerm))
322 + return
323 +
324 + try {
325 + let lastDir = prefixPath.slice(0, -1) || '.'
326 + parentsCache.set(lastDir, parent)
327 + await walkDir(source, { depth, hidden: showHiddenFiles.get() }, async entry => {
328 + if (ctx?.isAborted()) {
329 + stream.push(null)
330 + return null
331 + }
332 + const {path} = entry
333 + const isFolder = entry.isDirectory()
334 + const name = prefixPath + (parent.rename?.[path] || path)
335 + if (descriptIon.get() && basename(name) === DESCRIPT_ION)
336 + return
337 + if (taken?.has(normalizeFilename(name))) // taken by vfs node above
338 + return false // false just in case it's a folder
339 + if (depth) {
340 + const dir = dirname(name)
341 + if (dir !== lastDir)
342 + parent = parentsCache.get(lastDir = dir)
343 + }
344 +
345 + const item: VfsNode = {
346 + name,
347 + isFolder,
348 + source: join(source, path),
349 + rename: renameUnderPath(parent.rename, path),
350 + }
351 + if (await cantSee(item)) // can't see: don't produce and don't recur
352 + return false
353 + if (onlyFiles ? !isFolder : (!onlyFolders || isFolder))
354 + stream.push(item)
355 + if (cantRecur(item))
356 + return false
357 + if (isFolder)
358 + parentsCache.set(name, item)
359 + entry.closingBranch?.then(p =>
360 + parentsCache.delete(p || '.'))
361 + })
362 }
326 -
327 - const item: VfsNode = {
328 - name,
329 - isFolder,
330 - source: join(source, path),
331 - rename: renameUnderPath(parent.rename, path),
363 + catch(e) {
364 + console.debug('walkNode', source, e) // ENOTDIR, or lacking permissions
365 }
333 - if (isFolder) // store it even if we can't see it (masks), as its children can be produced by dirStream
334 - parentsCache.set(name, item)
335 - if (!(onlyFiles && isFolder) && await canSee(item))
336 - yield item
337 - entry.closingBranch?.then(p =>
338 - parentsCache.delete(p || '.'))
366 + parentsCache.clear() // hoping for faster GC
367 + }
368 + finally {
369 + for (const [item, name] of visitLater)
370 + for await (const x of walkNode(item, { ctx, depth: depth - 1, prefixPath: name + '/', requiredPerm, onlyFolders }))
371 + stream.push(x)
372 + stream.push(null)
373 + }
374 +
375 + function cantRecur(item: VfsNode) {
376 + if (ctx && !hasPermission(item, 'can_list', ctx)) return true
377 + inheritMasks(item, parent)
378 + }
379 +
380 + // item will be changed, so be sure to pass a temp node
381 + async function cantSee(item: VfsNode) {
382 + await maskApplier(item)
383 + inheritFromParent(parent, item)
384 + if (ctx && !hasPermission(item, 'can_see', ctx)) return true
385 + item.isTemp = true
386 }
387 }
341 - catch(e) {
342 - console.debug('walkNode', source, e) // ENOTDIR, or lacking permissions
343 - }
344 - parentsCache.clear() // hoping for faster GC
345 - }
346 - finally {
347 - for (const [item, name] of visitLater)
348 - yield* walkNode(item, { ctx, depth: depth - 1, prefixPath: name + '/', requiredPerm, onlyFolders })
349 - }
388 + })
389
351 - // item will be changed, so be sure to pass a temp node
352 - async function canSee(item: VfsNode) {
353 - // we basename for depth>0 where we already have the rest of the path in the parent's url, and would be duplicated
354 - await maskApplier(item, basename(getNodeName(item)))
355 - inheritFromParent(parent, item)
356 - if (ctx && !hasPermission(item, 'can_see', ctx)) return
357 - item.isTemp = true
358 - return item
359 - }
390 + // must use a stream to be able to work with the callback-based mechanism of walkDir, but Readable is not typed so we wrap it with a generator
391 + for await (const item of stream)
392 + yield item as VfsNode
393 }
394
395 export function masksCouldGivePermission(masks: Masks | undefined, perm: keyof VfsPerms): boolean {
@@ -378,7 +411,7 @@ export function parentMaskApplier(parent: VfsNode) {
411 k = k.startsWith('**/') ? k.slice(3) : !k.includes('/') ? k : '' // ** globstar matches also zero subfolders, so this mask must be applied here too
412 return k && { mods, matcher: makeMatcher(k), mustBeFolder }
413 }))
381 - return async (item: VfsNode, virtualBasename=getNodeName(item)) => {
414 + return async (item: VfsNode, virtualBasename=basename(getNodeName(item))) => { // we basename for depth>0
415 let isFolder: boolean | undefined = undefined
416 for (const { matcher, mods, mustBeFolder } of matchers) {
417 if (mustBeFolder !== undefined) {
@@ -392,7 +425,7 @@ export function parentMaskApplier(parent: VfsNode) {
425 }
426 }
427
395 -function inheritMasks(item: VfsNode, parent: VfsNode, virtualBasename:string) {
428 +function inheritMasks(item: VfsNode, parent: VfsNode, virtualBasename=getNodeName(item)) {
429 const { masks } = parent
430 if (!masks) return
431 const o: Masks = {}
src/walkDir.ts renamed
+39 -51
@@ -2,10 +2,10 @@ import { makeQ } from './makeQ'
2 import { stat, readdir } from 'fs/promises'
3 import { IS_WINDOWS } from './const'
4 import { join } from 'path'
5 -import { Readable } from 'stream'
6 -import { pendingPromise } from './cross'
5 +import { pendingPromise, Promisable } from './cross'
6 import { Stats, Dirent } from 'node:fs'
7 import fswin from 'fswin'
8 +import { isDirectory } from './util-files'
9
10 export interface DirStreamEntry extends Dirent {
11 closingBranch?: Promise<string>
@@ -14,39 +14,26 @@ export interface DirStreamEntry extends Dirent {
14
15 const dirQ = makeQ(3)
16
17 -export function createDirStream(startPath: string, { depth=0, hidden=true }) {
17 +// cb returns void = just go on, null = stop, false = go on but don't recur (in case of depth)
18 +export function walkDir(path: string, { depth = 0, hidden = true }: {
19 + depth?: number,
20 + hidden?: boolean
21 +}, cb: (e: DirStreamEntry) => Promisable<void | null | false>) {
22 let stopped = false
19 - let started = false
23 const closingQ: string[] = []
21 - const stream = new Readable({
22 - objectMode: true,
23 - read() {
24 - if (started) return
25 - started = true
26 - dirQ.add(() => readDir('', depth)
27 - .then(res => { // don't make the job await for it, but use it to close the stream
28 - Promise.resolve(res?.branchDone).then(() => {
29 - stream.push(null)
30 - })
31 - }, e => {
32 - stream.emit('error', e)
33 - return stream.push(null)
34 - })
35 - )
36 - }
37 - })
38 - stream.on('close', () => stopped = true)
39 - return Object.assign(stream, {
40 - stop() {
41 - stopped = true
42 - if (!dirQ.isWorking())
43 - stream.push(null)
44 - },
24 + return new Promise(async (resolve, reject) => {
25 + if (!await isDirectory(path))
26 + throw Error('ENOTDIR')
27 + dirQ.add(() => readDir('', depth)
28 + .then(res => { // don't make the job await for it, but use it to know it's over
29 + Promise.resolve(res?.branchDone).then(resolve)
30 + }, reject)
31 + )
32 })
33
47 - async function readDir(path: string, depth: number) {
34 + async function readDir(relativePath: string, depth: number) {
35 if (stopped) return
49 - const base = join(startPath, path)
36 + const base = join(path, relativePath)
37 const subDirsDone: Promise<any>[] = []
38 let n = 0
39 let last: DirStreamEntry | undefined
@@ -62,7 +49,7 @@ export function createDirStream(startPath: string, { depth=0, hidden=true }) {
49 for (const f of entries) {
50 if (stopped) break
51 if (!hidden && f.IS_HIDDEN) continue
65 - work(Object.assign(Object.create(methods), {
52 + await work(Object.assign(Object.create(methods), {
53 isDir: f.IS_DIRECTORY,
54 name: f.LONG_NAME,
55 stats: { size: f.SIZE, birthtime: f.CREATION_TIME, mtime: f.LAST_WRITE_TIME } as Stats
@@ -80,36 +67,37 @@ export function createDirStream(startPath: string, { depth=0, hidden=true }) {
67 const expanded: DirStreamEntry = entry
68 if (stats)
69 expanded.stats = stats
83 - work(expanded)
70 + await work(expanded)
71 }
72
86 - function work(entry: DirStreamEntry) {
87 - entry.path = (path && path + '/') + entry.name
73 + async function work(entry: DirStreamEntry) {
74 + entry.path = (relativePath && relativePath + '/') + entry.name
75 if (last && closingQ.length) // pending entries
76 last.closingBranch = Promise.resolve(closingQ.shift()!)
77 last = entry
91 - if (depth > 0 && entry.isDirectory()) {
92 - const branchDone = pendingPromise() // per-job
93 - const job = () =>
94 - readDir(entry.path, depth - 1) // recur
95 - .then(x => x, () => {}) // mute errors
96 - .then(res => { // don't await, as readDir must resolve without branch being done
97 - if (!res?.n)
98 - closingQ.push(entry.path) // no children to tell i'm done
99 - Promise.resolve(res?.branchDone).then(() =>
100 - branchDone.resolve())
101 - })
102 - dirQ.add(job) // this won't start until next tick
103 - subDirsDone.push(branchDone)
104 - }
105 - stream.push(entry)
78 + const res = await cb(entry)
79 + if (res === null) return stopped = true
80 + if (res === false) return
81 n++
82 + if (!depth || !entry.isDirectory()) return
83 + const branchDone = pendingPromise() // per-job
84 + const job = () =>
85 + readDir(entry.path, depth - 1) // recur
86 + .then(x => x, () => {}) // mute errors
87 + .then(res => { // don't await, as readDir must resolve without branch being done
88 + if (!res?.n)
89 + closingQ.push(entry.path) // no children to tell i'm done
90 + Promise.resolve(res?.branchDone).then(() =>
91 + branchDone.resolve())
92 + })
93 + dirQ.add(job) // this won't start until next tick
94 + subDirsDone.push(branchDone)
95 }
96 const branchDone = Promise.allSettled(subDirsDone).then(() => {})
97 if (last) // using streams, we don't know when the entries are received, so we need to notify on last item
110 - last.closingBranch = branchDone.then(() => path)
98 + last.closingBranch = branchDone.then(() => relativePath)
99 else
112 - closingQ.push(path) // ok, we'll ask next one to carry this info
100 + closingQ.push(relativePath) // ok, we'll ask next one to carry this info
101 // don't return the promise directly, as this job ends here, but communicate to caller the promise for the whole branch
102 return { branchDone, n }
103 }
tests/config.yaml
+4
@@ -6,6 +6,8 @@ vfs:
6 mime: text/plain
7 protectFromAbove/child/*.txt:
8 can_read: false
9 + cantSearchForMasks/page:
10 + can_list: false
11 children:
12 - source: tests/alfa.txt
13 name: x%#x
@@ -57,6 +59,8 @@ vfs:
59 masks:
60 "*":
61 can_list: true
62 + - name: cantSearchForMasks
63 + source: tests
64 - name: cantReadBut
65 source: tests
66 can_read: false
tests/test.ts
+1 -1
@@ -61,7 +61,7 @@ describe('basics', () => {
61 it('cantListBut.zip', req('/cantListBut/?get=zip', 403))
62 it('cantListBut.parent', reqList('/', { permInList: { 'cantListBut/': 'l' } }))
63 it('cantListBut.child masked', reqList('/cantListBut/page', 200))
64 -
64 + it('cantSearchForMasks', reqList('/', { outList: ['cantSearchForMasks/page/gpl.png'] }, { search: 'gpl' }))
65 it('cantReadBut', reqList('/cantReadBut/', 403))
66 it('cantReadBut.can', req('/cantReadBut/alfa.txt', 200))
67 it('cantReadBut.parent', reqList('/', { permInList: { 'cantReadBut/': '!r' } }))