From 7cc79077d1833969b3460d2f881b212682e9a6db Mon Sep 17 00:00:00 2001 From: KernelDeimos <7225168+KernelDeimos@users.noreply.github.com> Date: Wed, 8 Oct 2025 16:03:21 -0400 Subject: [PATCH] fix(fs): better errors for unresolved selectors Adds an appropriate error at a couple different levels. The first is in FilesystemService. Callers of FilesystemService.node are expected to provide either an absolute path or an instance of a filesystem node selector, but there wasn't an explicit check for this so instead an arbitrary error (read property of undefined) would be thrown. The second is a higher-level APIError in the batch executor that's a bit more helpful than a 500 Internal Server Error. --- src/backend/src/api/APIError.js | 5 +++ .../src/filesystem/FilesystemService.js | 12 ++++++- src/backend/src/filesystem/node/selectors.js | 32 +++++++++++++++---- .../filesystem_api/batch/PathResolver.js | 3 ++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/backend/src/api/APIError.js b/src/backend/src/api/APIError.js index 3f23dc404..9c533a402 100644 --- a/src/backend/src/api/APIError.js +++ b/src/backend/src/api/APIError.js @@ -283,6 +283,11 @@ module.exports = class APIError { status: 400, message: 'Invalid file metadata.', }, + 'unresolved_relative_path': { + status: 400, + message: ({ path }) => `Unresolved relative path: ${quot(path)}. ` + + "You may need to specify a full path starting with '/'.", + }, // Open 'no_suitable_app': { diff --git a/src/backend/src/filesystem/FilesystemService.js b/src/backend/src/filesystem/FilesystemService.js index 44198930b..74c7ec17f 100644 --- a/src/backend/src/filesystem/FilesystemService.js +++ b/src/backend/src/filesystem/FilesystemService.js @@ -19,7 +19,7 @@ // TODO: database access can be a service const { RESOURCE_STATUS_PENDING_CREATE } = require('../modules/puterfs/ResourceService.js'); const { TraceService } = require('../services/TraceService.js'); -const { NodePathSelector, NodeUIDSelector, NodeInternalIDSelector } = require('./node/selectors.js'); +const { NodePathSelector, NodeUIDSelector, NodeInternalIDSelector, NodeSelector } = require('./node/selectors.js'); const FSNodeContext = require('./FSNodeContext.js'); const { Context } = require('../util/context.js'); const APIError = require('../api/APIError.js'); @@ -29,6 +29,8 @@ const { UserActorType } = require('../services/auth/Actor'); const { get_user } = require('../helpers'); const BaseService = require('../services/BaseService'); const { MANAGE_PERM_PREFIX } = require('../services/auth/permissionConts.mjs'); +const { PuterFSProvider } = require('../modules/puterfs/lib/PuterFSProvider.js'); +const { quot } = require('@heyputer/putility/src/libs/string.js'); class FilesystemService extends BaseService { static MODULES = { @@ -329,6 +331,14 @@ class FilesystemService extends BaseService { selector = new NodeInternalIDSelector('mysql', selector.mysql_id); } } + + if ( ! (selector instanceof NodeSelector) ) { + throw new Error( + 'FileSystemService could not resolve the specified node value ' + + quot(''+selector) + ` (type: ${typeof selector}) ` + + 'to a filesystem node selector', + ); + } system_dir_check: { if ( ! (selector instanceof NodePathSelector) ) break system_dir_check; diff --git a/src/backend/src/filesystem/node/selectors.js b/src/backend/src/filesystem/node/selectors.js index 717ecb17a..76e5200b1 100644 --- a/src/backend/src/filesystem/node/selectors.js +++ b/src/backend/src/filesystem/node/selectors.js @@ -19,8 +19,22 @@ const _path = require('path'); const { PuterPath } = require('../lib/PuterPath'); -class NodePathSelector { +/** + * The base class doesn't add any functionality, but it's useful for + * `instanceof` checks. + */ +class NodeSelector { + constructor () { + if ( this.constructor === NodeSelector ) { + throw new Error('cannot instantiate NodeSelector directly; ' + + 'that would be like using this: https://devmeme.puter.site/plug.webp'); + } + } +} + +class NodePathSelector extends NodeSelector { constructor (path) { + super(); this.value = path; } @@ -34,8 +48,9 @@ class NodePathSelector { } } -class NodeUIDSelector { +class NodeUIDSelector extends NodeSelector { constructor (uid) { + super(); this.value = uid; } @@ -58,8 +73,9 @@ class NodeUIDSelector { } } -class NodeInternalIDSelector { +class NodeInternalIDSelector extends NodeSelector { constructor (service, id, debugInfo) { + super(); this.service = service; this.id = id; this.debugInfo = debugInfo; @@ -81,8 +97,9 @@ class NodeInternalIDSelector { } } -class NodeChildSelector { +class NodeChildSelector extends NodeSelector { constructor (parent, name) { + super(); this.parent = parent; this.name = name; } @@ -101,7 +118,7 @@ class NodeChildSelector { } } -class RootNodeSelector { +class RootNodeSelector extends NodeSelector { static entry = { is_dir: true, is_root: true, @@ -114,6 +131,7 @@ class RootNodeSelector { node.uid = PuterPath.NULL_UUID; } constructor () { + super(); this.entry = this.constructor.entry; } @@ -122,8 +140,9 @@ class RootNodeSelector { } } -class NodeRawEntrySelector { +class NodeRawEntrySelector extends NodeSelector { constructor (entry) { + super(); // Fix entries from get_descendants if ( ! entry.uuid && entry.uid ) { entry.uuid = entry.uid; @@ -190,6 +209,7 @@ const relativeSelector = (parent, path) => { } module.exports = { + NodeSelector, NodePathSelector, NodeUIDSelector, NodeInternalIDSelector, diff --git a/src/backend/src/routers/filesystem_api/batch/PathResolver.js b/src/backend/src/routers/filesystem_api/batch/PathResolver.js index e7daf71b9..a4736f87a 100644 --- a/src/backend/src/routers/filesystem_api/batch/PathResolver.js +++ b/src/backend/src/routers/filesystem_api/batch/PathResolver.js @@ -96,6 +96,9 @@ module.exports = class PathResolver { if ( inputPath === '~' ) { return `/${username}`; } + if ( inputPath.startsWith('.') ) { + throw APIError.create('unresolved_relative_path', null, { path: inputPath }); + } const refName = this.getReferenceUsed(inputPath); if ( refName === null ) return inputPath;