From cd80e845b84f85543b80bb1e9d067e95e44ec4da Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen <opensource@fthiessen.de> Date: Thu, 11 Jul 2024 19:33:45 +0200 Subject: [PATCH] feat(filename): Improve filename validation to support Nextcloud 30 capabilities With Nextcloud 30 there are more options to restrict filenames, so we need to adjust our filename validation. This now also provides `validateFilename` which will throw an error if invalid, to allow better user feedback on what is wrong. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de> --- __tests__/utils/filename-validation.spec.ts | 197 ++++++++++++++++++++ __tests__/utils/filename.spec.ts | 41 +--- lib/index.ts | 3 +- lib/utils/filename-validation.ts | 130 +++++++++++++ lib/utils/filename.ts | 21 --- package-lock.json | 13 ++ package.json | 1 + 7 files changed, 344 insertions(+), 62 deletions(-) create mode 100644 __tests__/utils/filename-validation.spec.ts create mode 100644 lib/utils/filename-validation.ts diff --git a/__tests__/utils/filename-validation.spec.ts b/__tests__/utils/filename-validation.spec.ts new file mode 100644 index 00000000..76ae6bdb --- /dev/null +++ b/__tests__/utils/filename-validation.spec.ts @@ -0,0 +1,197 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later or LGPL-3.0-or-later + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { InvalidFilenameError, InvalidFilenameErrorReason, isFilenameValid, validateFilename } from '../../lib/index' + +const nextcloudCapabilities = vi.hoisted(() => ({ getCapabilities: vi.fn(() => ({ files: {} })) })) +vi.mock('@nextcloud/capabilities', () => nextcloudCapabilities) + +describe('isFilenameValid', () => { + beforeEach(() => { + vi.restoreAllMocks() + delete window._oc_config + }) + + it('works for valid filenames', async () => { + expect(isFilenameValid('foo.bar')).toBe(true) + }) + + it('works for invalid filenames', async () => { + expect(isFilenameValid('foo\\bar')).toBe(false) + }) + + it('does not catch any interal exceptions', async () => { + // invalid capability just to get an exception here + nextcloudCapabilities.getCapabilities.mockImplementationOnce(() => ({ files: { forbidden_filename_extensions: 3 } })) + expect(() => isFilenameValid('hello')).toThrowError(TypeError) + }) +}) + +describe('validateFilename', () => { + + beforeEach(() => { + vi.restoreAllMocks() + delete window._oc_config + }) + + it('works for valid filenames', async () => { + expect(() => validateFilename('foo.bar')).not.toThrow() + }) + + it('has fallback invalid characters', async () => { + expect(() => validateFilename('foo\\bar')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('foo/bar')).toThrowError(InvalidFilenameError) + }) + + it('has fallback invalid names', async () => { + expect(() => validateFilename('.htaccess')).toThrowError(InvalidFilenameError) + }) + + it('has fallback invalid extension', async () => { + expect(() => validateFilename('file.txt.part')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('file.txt.filepart')).toThrowError(InvalidFilenameError) + }) + + // Nextcloud 29 + it('fallback fetching forbidden characters from oc config', async () => { + window._oc_config = { forbidden_filenames_characters: ['=', '?'] } + expect(() => validateFilename('foo.bar')).not.toThrow() + expect(() => validateFilename('foo=bar')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('foo?bar')).toThrowError(InvalidFilenameError) + }) + + // Nextcloud 30+ + it('fetches forbidden characters from capabilities', async () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filename_characters: ['=', '?'] } })) + expect(() => validateFilename('foo')).not.toThrow() + expect(() => validateFilename('foo?')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('foo=bar')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('?foo')).toThrowError(InvalidFilenameError) + }) + + it('fetches forbidden extensions from capabilities', async () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filename_extensions: ['.txt', '.tar.gz'] } })) + expect(() => validateFilename('foo.md')).not.toThrow() + expect(() => validateFilename('foo.txt')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('foo.tar.gz')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('foo.tar.zstd')).not.toThrow() + }) + + it('fetches forbidden filenames from capabilities', async () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filenames: ['thumbs.db'] } })) + expect(() => validateFilename('thumbs.png')).not.toThrow() + expect(() => validateFilename('thumbs.db')).toThrowError(InvalidFilenameError) + }) + + it('fetches forbidden filename basenames from capabilities', async () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filename_basenames: ['com0'] } })) + expect(() => validateFilename('com1.txt')).not.toThrow() + expect(() => validateFilename('com0.txt')).toThrowError(InvalidFilenameError) + }) + + it('handles forbidden filenames case-insensitive', () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filenames: ['thumbs.db'] } })) + expect(() => validateFilename('thumbS.db')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('thumbs.DB')).toThrowError(InvalidFilenameError) + }) + + it('handles forbidden filename basenames case-insensitive', () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filename_basenames: ['com0'] } })) + expect(() => validateFilename('COM0')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('com0')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('com0.namespace')).toThrowError(InvalidFilenameError) + }) + + it('handles forbidden filename extensions case-insensitive', () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filename_extensions: ['.txt'] } })) + expect(() => validateFilename('file.TXT')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('FILE.txt')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('FiLe.TxT')).toThrowError(InvalidFilenameError) + }) + + it('handles hidden files correctly', () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filename_basenames: ['.hidden'], forbidden_filename_extensions: ['.txt'] } })) + // forbidden basename '.hidden' + expect(() => validateFilename('.hidden')).toThrowError(InvalidFilenameError) + expect(() => validateFilename('.hidden.png')).toThrowError(InvalidFilenameError) + // basename is .txt so not forbidden + expect(() => validateFilename('.txt')).not.toThrowError(InvalidFilenameError) + expect(() => validateFilename('.txt.png')).not.toThrowError(InvalidFilenameError) + // forbidden extension + expect(() => validateFilename('.other-hidden.txt')).toThrowError(InvalidFilenameError) + }) + + it('sets error properties correctly on invalid filename', async () => { + try { + validateFilename('.htaccess') + expect(true, 'should not be reached').toBeFalsy() + } catch (error) { + expect(error).toBeInstanceOf(InvalidFilenameError) + expect((error as InvalidFilenameError).reason).toBe(InvalidFilenameErrorReason.ReservedName) + expect((error as InvalidFilenameError).segment).toBe('.htaccess') + expect((error as InvalidFilenameError).filename).toBe('.htaccess') + } + }) + + it('sets error properties correctly on invalid extension', async () => { + try { + validateFilename('file.part') + expect(true, 'should not be reached').toBeFalsy() + } catch (error) { + expect(error).toBeInstanceOf(InvalidFilenameError) + expect((error as InvalidFilenameError).reason).toBe(InvalidFilenameErrorReason.Extension) + expect((error as InvalidFilenameError).segment).toBe('.part') + expect((error as InvalidFilenameError).filename).toBe('file.part') + } + }) + + it('sets error properties correctly on invalid character', async () => { + try { + validateFilename('file\\name') + expect(true, 'should not be reached').toBeFalsy() + } catch (error) { + expect(error).toBeInstanceOf(InvalidFilenameError) + expect((error as InvalidFilenameError).reason).toBe(InvalidFilenameErrorReason.Character) + expect((error as InvalidFilenameError).segment).toBe('\\') + expect((error as InvalidFilenameError).filename).toBe('file\\name') + } + }) + + it('sets error properties correctly on invalid basename', async () => { + nextcloudCapabilities.getCapabilities.mockImplementation(() => ({ files: { forbidden_filename_basenames: ['com0'] } })) + try { + validateFilename('com0.namespace') + expect(true, 'should not be reached').toBeFalsy() + } catch (error) { + expect(error).toBeInstanceOf(InvalidFilenameError) + expect((error as InvalidFilenameError).reason).toBe(InvalidFilenameErrorReason.ReservedName) + expect((error as InvalidFilenameError).segment).toBe('com0') + expect((error as InvalidFilenameError).filename).toBe('com0.namespace') + } + }) +}) + +describe('InvalidFilenameError', () => { + + it('sets the filename', () => { + const error = new InvalidFilenameError({ filename: 'file', segment: 'fi', reason: InvalidFilenameErrorReason.Extension }) + expect(error.filename).toBe('file') + }) + + it('sets the segment', () => { + const error = new InvalidFilenameError({ filename: 'file', segment: 'fi', reason: InvalidFilenameErrorReason.Extension }) + expect(error.segment).toBe('fi') + }) + + it('sets the reason', () => { + const error = new InvalidFilenameError({ filename: 'file', segment: 'fi', reason: InvalidFilenameErrorReason.Extension }) + expect(error.reason).toBe(InvalidFilenameErrorReason.Extension) + }) + + it('sets the message', () => { + const error = new InvalidFilenameError({ filename: 'file', segment: 'fi', reason: InvalidFilenameErrorReason.Extension }) + expect(error.message).toMatchInlineSnapshot('"Invalid extension \'fi\' in filename \'file\'"') + }) +}) diff --git a/__tests__/utils/filename.spec.ts b/__tests__/utils/filename.spec.ts index aae9e4db..b7f78b2e 100644 --- a/__tests__/utils/filename.spec.ts +++ b/__tests__/utils/filename.spec.ts @@ -2,48 +2,9 @@ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later or LGPL-3.0-or-later */ -import { describe, it, expect, beforeEach, vi } from 'vitest' +import { describe, it, expect, vi } from 'vitest' import { getUniqueName } from '../../lib/index' -describe('isFilenameValid', () => { - beforeEach(() => { - delete window._oc_config - vi.resetModules() - }) - - it('works for valid filenames', async () => { - const { isFilenameValid } = await import('../../lib/index') - - expect(isFilenameValid('foo.bar')).toBe(true) - }) - - it('has fallback invalid characters', async () => { - const { isFilenameValid } = await import('../../lib/index') - - expect(isFilenameValid('foo\\bar')).toBe(false) - expect(isFilenameValid('foo/bar')).toBe(false) - }) - - it('reads invalid characters from oc config', async () => { - window._oc_config = { forbidden_filenames_characters: ['=', '?'] } - const { isFilenameValid } = await import('../../lib/index') - - expect(isFilenameValid('foo.bar')).toBe(true) - expect(isFilenameValid('foo=bar')).toBe(false) - expect(isFilenameValid('foo?bar')).toBe(false) - }) - - it('supports invalid filename regex', async () => { - window._oc_config = { forbidden_filenames_characters: ['/'], blacklist_files_regex: '\\.(part|filepart)$' } - const { isFilenameValid } = await import('../../lib/index') - - expect(isFilenameValid('foo.bar')).toBe(true) - expect(isFilenameValid('foo.part')).toBe(false) - expect(isFilenameValid('foo.filepart')).toBe(false) - expect(isFilenameValid('.filepart')).toBe(false) - }) -}) - describe('getUniqueName', () => { it('returns the same name if unique', () => { const name = 'file.txt' diff --git a/lib/index.ts b/lib/index.ts index 273c12d1..5f8d45e4 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -20,7 +20,8 @@ export { File, type IFile } from './files/file' export { Folder, type IFolder } from './files/folder' export { Node, NodeStatus, type INode } from './files/node' -export { isFilenameValid, getUniqueName } from './utils/filename' +export * from './utils/filename-validation' +export { getUniqueName } from './utils/filename' export { formatFileSize, parseFileSize } from './utils/fileSize' export { orderBy } from './utils/sorting' export { sortNodes, FilesSortingMode, type FilesSortingOptions } from './utils/fileSorting' diff --git a/lib/utils/filename-validation.ts b/lib/utils/filename-validation.ts new file mode 100644 index 00000000..ea33ecb1 --- /dev/null +++ b/lib/utils/filename-validation.ts @@ -0,0 +1,130 @@ +/** + * SPDX-FileCopyrightText: Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later or LGPL-3.0-or-later + */ + +import { getCapabilities } from '@nextcloud/capabilities' + +interface NextcloudCapabilities extends Record<string, unknown> { + files: { + 'bigfilechunking': boolean + // those are new in Nextcloud 30 + 'forbidden_filenames'?: string[] + 'forbidden_filename_basenames'?: string[] + 'forbidden_filename_characters'?: string[] + 'forbidden_filename_extensions'?: string[] + } +} + +export enum InvalidFilenameErrorReason { + ReservedName = 'reserved name', + Character = 'character', + Extension = 'extension', +} + +interface InvalidFilenameErrorOptions { + /** + * The filename that was validated + */ + filename: string + + /** + * Reason why the validation failed + */ + reason: InvalidFilenameErrorReason + + /** + * Part of the filename that caused this error + */ + segment: string +} + +export class InvalidFilenameError extends Error { + + public constructor(options: InvalidFilenameErrorOptions) { + super(`Invalid ${options.reason} '${options.segment}' in filename '${options.filename}'`, { cause: options }) + } + + /** + * The filename that was validated + */ + public get filename() { + return (this.cause as InvalidFilenameErrorOptions).filename + } + + /** + * Reason why the validation failed + */ + public get reason() { + return (this.cause as InvalidFilenameErrorOptions).reason + } + + /** + * Part of the filename that caused this error + */ + public get segment() { + return (this.cause as InvalidFilenameErrorOptions).segment + } + +} + +/** + * Validate a given filename + * @param filename The filename to check + * @throws {InvalidFilenameError} + */ +export function validateFilename(filename: string): void { + const capabilities = (getCapabilities() as NextcloudCapabilities).files + + // Handle forbidden characters + // This needs to be done first as the other checks are case insensitive! + const forbiddenCharacters = capabilities.forbidden_filename_characters ?? window._oc_config?.forbidden_filenames_characters ?? ['/', '\\'] + for (const character of forbiddenCharacters) { + if (filename.includes(character)) { + throw new InvalidFilenameError({ segment: character, reason: InvalidFilenameErrorReason.Character, filename }) + } + } + + // everything else is case insensitive (the capabilities are returned lowercase) + filename = filename.toLocaleLowerCase() + + // Handle forbidden filenames, on older Nextcloud versions without this capability it was hardcoded in the backend to '.htaccess' + const forbiddenFilenames = capabilities.forbidden_filenames ?? ['.htaccess'] + if (forbiddenFilenames.includes(filename)) { + throw new InvalidFilenameError({ filename, segment: filename, reason: InvalidFilenameErrorReason.ReservedName }) + } + + // Handle forbidden basenames + const endOfBasename = filename.indexOf('.', 1) + const basename = filename.substring(0, endOfBasename === -1 ? undefined : endOfBasename) + const forbiddenFilenameBasenames = capabilities.forbidden_filename_basenames ?? [] + if (forbiddenFilenameBasenames.includes(basename)) { + throw new InvalidFilenameError({ filename, segment: basename, reason: InvalidFilenameErrorReason.ReservedName }) + } + + // The legacy 'blacklist_files_regex' was hardcoded to the extension '.part' and '.filepart' + // So if the new (Nextcloud 30) capability is not awailable then we fallback to that + const forbiddenFilenameExtensions = capabilities.forbidden_filename_extensions ?? ['.part', '.filepart'] + for (const extension of forbiddenFilenameExtensions) { + if (filename.length > extension.length && filename.endsWith(extension)) { + throw new InvalidFilenameError({ segment: extension, reason: InvalidFilenameErrorReason.Extension, filename }) + } + } +} + +/** + * Check the validity of a filename + * This is a convinient wrapper for `checkFilenameValidity` to only return a boolean for the valid + * @param filename Filename to check validity + */ +export function isFilenameValid(filename: string): boolean { + try { + validateFilename(filename) + return true + } catch (error) { + if (error instanceof InvalidFilenameError) { + return false + } + throw error + } +} diff --git a/lib/utils/filename.ts b/lib/utils/filename.ts index 7bd88392..67969c4c 100644 --- a/lib/utils/filename.ts +++ b/lib/utils/filename.ts @@ -5,27 +5,6 @@ import { basename, extname } from 'path' -const forbiddenCharacters = window._oc_config?.forbidden_filenames_characters ?? ['/', '\\'] -const forbiddenFilenameRegex = window._oc_config?.blacklist_files_regex ? new RegExp(window._oc_config.blacklist_files_regex) : null - -/** - * Check the validity of a filename - * This is a convinient wrapper for `checkFilenameValidity` to only return a boolean for the valid - * @param filename Filename to check validity - */ -export function isFilenameValid(filename: string): boolean { - // Check forbidden characters (available with Nextcloud 29) - if (forbiddenCharacters.some((character) => filename.includes(character))) { - return false - } - // Check forbidden filename regex - if (forbiddenFilenameRegex !== null && filename.match(forbiddenFilenameRegex)) { - return false - } - // in Nextcloud 30 also check forbidden file extensions and file prefixes - return true -} - interface UniqueNameOptions { /** * A function that takes an index and returns a suffix to add to the file name, defaults to '(index)' diff --git a/package-lock.json b/package-lock.json index 1c498c7d..15a581be 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "license": "AGPL-3.0-or-later", "dependencies": { "@nextcloud/auth": "^2.3.0", + "@nextcloud/capabilities": "^1.2.0", "@nextcloud/l10n": "^3.1.0", "@nextcloud/logger": "^3.0.2", "@nextcloud/paths": "^2.1.0", @@ -1205,6 +1206,18 @@ "npm": "^10.0.0" } }, + "node_modules/@nextcloud/capabilities": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/@nextcloud/capabilities/-/capabilities-1.2.0.tgz", + "integrity": "sha512-L1NQtOfHWzkfj0Ple1MEJt6HmOHWAi3y4qs+OnwSWexqJT0DtXTVPyRxi7ADyITwRxS5H9R/HMl6USAj4Nr1nQ==", + "dependencies": { + "@nextcloud/initial-state": "^2.1.0" + }, + "engines": { + "node": "^20.0.0", + "npm": "^10.0.0" + } + }, "node_modules/@nextcloud/eslint-config": { "version": "8.4.1", "resolved": "https://registry.npmjs.org/@nextcloud/eslint-config/-/eslint-config-8.4.1.tgz", diff --git a/package.json b/package.json index 9dfc46e3..d563a103 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ }, "dependencies": { "@nextcloud/auth": "^2.3.0", + "@nextcloud/capabilities": "^1.2.0", "@nextcloud/l10n": "^3.1.0", "@nextcloud/logger": "^3.0.2", "@nextcloud/paths": "^2.1.0",