T09 · Insecure Skill Coding Practices
- Location
src/core/file-manager.js:141- Finding
Unrestricted File Deletion Through Temporary-File Cleanup API
- Content
View full analysis
Vulnerability Details
File Location:
src/core/file-manager.js, lines 141–145
Vulnerability Type: Arbitrary file deletion / insufficient path validation
Risk Level: HighVulnerable Code
javascript async cleanupTempFile(filePath) { try { await fs.unlink(filePath); return true; } catch (error) { console.warn(`Failed to cleanup temp file ${filePath}: ${error.message}`); return false; } }Technical Analysis
cleanupTempFile()is documented as deleting a temporary file, but it passes the caller-controlledfilePathdirectly tofs.unlink()without verifying that the target is inside the configured temporary directory.No canonicalization, ownership tracking, filename allowlist, or path-boundary check is performed. Consequently, an absolute path such as
/home/service/config.json, or a traversal path escaping the temporary directory, can target any file writable by the Node.js process.The method is part of the exported
FileManagerpublic interface. Its documentation indocs/API.mdalso accepts a generalfilePath, so callers may invoke this primitive directly. Symbolic links and filesystem path aliases may further undermine a simple lexical prefix check unless the implementation resolves and validates canonical paths.Attack Path
-
An attacker obtains influence over the argument passed to
FileManager.cleanupTempFile(). This may occur when an integrating agent, API, or application exposes the cleanup operation to untrusted input. -
The attacker supplies the absolute path of a writable non-temporary file, for example:
javascript await manager.cleanupTempFile('/home/service/app-config.json'); -
The method passes that path directly to
fs.unlink(). -
Node.js deletes the target using the host process's filesystem privileges.
-
The attacker may repeat the operation against application data, configuration, logs, or other writable files.
The repository does not itself expo ...[truncated 805 chars]
-
- Remediation
View remediation
Remediation Suggestions
Restrict cleanup to files created and tracked by this
FileManagerinstance:- Resolve the configured temporary directory and candidate path to absolute canonical paths.
- Reject targets outside the temporary-directory boundary. Use
path.relative()rather than a raw string-prefix check. - Track paths returned by
createTempFile()in a private set and only permit deletion of tracked entries. - Use unpredictable, securely generated filenames, such as
crypto.randomUUID(). - Consider opening and creating temporary files with exclusive semantics and restrictive permissions.
- Remove a path from the tracking set only after successful deletion.
- Add tests covering absolute external paths,
../traversal, prefix-confusion paths, and symbolic links.
Example hardening pattern:
javascript import path from 'path'; import fs from 'fs/promises'; async cleanupTempFile(filePath) { try { const tempRoot = await fs.realpath(this.config.tempDir); const candidate = path.resolve(filePath); const parent = await fs.realpath(path.dirname(candidate)); const resolvedTarget = path.join(parent, path.basename(candidate)); const relative = path.relative(tempRoot, resolvedTarget); if ( relative === '' || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative) ) { throw new Error('Refusing to delete a file outside the temporary directory'); } if (!this.createdTempFiles?.has(resolvedTarget)) { throw new Error('Refusing to delete an untracked temporary file'); } await fs.unlink(resolvedTarget); this.createdTempFiles.delete(resolvedTarget); return true; } catch (error) { console.warn(`Temporary-file cleanup failed: ${error.message}`); return false; } }For stronger symbolic-link protection, create and manage a dedicated private temporary directory with restrictive permissions and avoid following attacker-controlled links. The integration la ...[truncated 71 chars]