From dddf3500ec604f851536b0e460db334133f10064 Mon Sep 17 00:00:00 2001 From: marknguyen1302 Date: Fri, 12 Jul 2024 00:15:44 +0700 Subject: [PATCH] fix: validate path in fs (#3152) * fix: validate path in fs * fix other fs issue * fix test * fix test * fix test * fix: do not check file exist on model status validation * chore: bump version * remove copyFileSync method --------- Co-authored-by: Louis --- core/src/browser/fs.ts | 5 --- core/src/node/api/processors/download.ts | 3 +- core/src/node/api/processors/fs.ts | 39 ++++++++++++++++++------ core/src/node/api/processors/fsExt.ts | 9 ++++-- core/src/node/helper/config.ts | 3 +- core/src/node/helper/path.ts | 11 ++++++- core/src/types/api/index.ts | 1 - extensions/model-extension/package.json | 2 +- extensions/model-extension/src/index.ts | 7 ++++- 9 files changed, 57 insertions(+), 23 deletions(-) diff --git a/core/src/browser/fs.ts b/core/src/browser/fs.ts index 164e3b6479..3d4b948e95 100644 --- a/core/src/browser/fs.ts +++ b/core/src/browser/fs.ts @@ -64,10 +64,6 @@ const appendFileSync = (...args: any[]) => globalThis.core.api?.appendFileSync(. const syncFile: (src: string, dest: string) => Promise = (src, dest) => globalThis.core.api?.syncFile(src, dest) -/** - * Copy file sync. - */ -const copyFileSync = (...args: any[]) => globalThis.core.api?.copyFileSync(...args) const copyFile: (src: string, dest: string) => Promise = (src, dest) => globalThis.core.api?.copyFile(src, dest) @@ -95,7 +91,6 @@ export const fs = { rm, unlinkSync, appendFileSync, - copyFileSync, copyFile, syncFile, fileStat, diff --git a/core/src/node/api/processors/download.ts b/core/src/node/api/processors/download.ts index a4af474002..07486bdf88 100644 --- a/core/src/node/api/processors/download.ts +++ b/core/src/node/api/processors/download.ts @@ -1,6 +1,6 @@ import { resolve, sep } from 'path' import { DownloadEvent } from '../../../types/api' -import { normalizeFilePath } from '../../helper/path' +import { normalizeFilePath, validatePath } from '../../helper/path' import { getJanDataFolderPath } from '../../helper' import { DownloadManager } from '../../helper/download' import { createWriteStream, renameSync } from 'fs' @@ -37,6 +37,7 @@ export class Downloader implements Processor { const modelId = array.pop() ?? '' const destination = resolve(getJanDataFolderPath(), normalizedPath) + validatePath(destination) const rq = request({ url, strictSSL, proxy }) // Put request to download manager instance diff --git a/core/src/node/api/processors/fs.ts b/core/src/node/api/processors/fs.ts index a66f5a0e95..0557d21875 100644 --- a/core/src/node/api/processors/fs.ts +++ b/core/src/node/api/processors/fs.ts @@ -1,5 +1,5 @@ -import { join } from 'path' -import { normalizeFilePath } from '../../helper/path' +import { join, resolve } from 'path' +import { normalizeFilePath, validatePath } from '../../helper/path' import { getJanDataFolderPath } from '../../helper' import { Processor } from './Processor' import fs from 'fs' @@ -15,17 +15,29 @@ export class FileSystem implements Processor { process(route: string, ...args: any): any { const instance = this as any const func = instance[route] - if (func) { return func(...args) } else { return import(FileSystem.moduleName).then((mdl) => mdl[route]( - ...args.map((arg: any) => { - return typeof arg === 'string' && - (arg.startsWith(`file:/`) || arg.startsWith(`file:\\`)) - ? join(getJanDataFolderPath(), normalizeFilePath(arg)) - : arg + ...args.map((arg: any, index: number) => { + if(index !== 0) { + return arg + } + if (index === 0 && typeof arg !== 'string') { + throw new Error(`Invalid argument ${JSON.stringify(args)}`) + } + const path = + (arg.startsWith(`file:/`) || arg.startsWith(`file:\\`)) + ? join(getJanDataFolderPath(), normalizeFilePath(arg)) + : arg + + if(path.startsWith(`http://`) || path.startsWith(`https://`)) { + return path + } + const absolutePath = resolve(path) + validatePath(absolutePath) + return absolutePath }) ) ) @@ -42,8 +54,11 @@ export class FileSystem implements Processor { path = join(getJanDataFolderPath(), normalizeFilePath(path)) } + const absolutePath = resolve(path) + validatePath(absolutePath) + return new Promise((resolve, reject) => { - fs.rm(path, { recursive: true, force: true }, (err) => { + fs.rm(absolutePath, { recursive: true, force: true }, (err) => { if (err) { reject(err) } else { @@ -63,8 +78,11 @@ export class FileSystem implements Processor { path = join(getJanDataFolderPath(), normalizeFilePath(path)) } + const absolutePath = resolve(path) + validatePath(absolutePath) + return new Promise((resolve, reject) => { - fs.mkdir(path, { recursive: true }, (err) => { + fs.mkdir(absolutePath, { recursive: true }, (err) => { if (err) { reject(err) } else { @@ -73,4 +91,5 @@ export class FileSystem implements Processor { }) }) } + } diff --git a/core/src/node/api/processors/fsExt.ts b/core/src/node/api/processors/fsExt.ts index 4787da65b3..041586b5a6 100644 --- a/core/src/node/api/processors/fsExt.ts +++ b/core/src/node/api/processors/fsExt.ts @@ -1,6 +1,6 @@ import { join } from 'path' import fs from 'fs' -import { appResourcePath, normalizeFilePath } from '../../helper/path' +import { appResourcePath, normalizeFilePath, validatePath } from '../../helper/path' import { getJanDataFolderPath, getJanDataFolderPath as getPath } from '../../helper' import { Processor } from './Processor' import { FileStat } from '../../../types' @@ -21,6 +21,7 @@ export class FSExt implements Processor { // Handles the 'syncFile' IPC event. This event is triggered to synchronize a file from a source path to a destination path. syncFile(src: string, dest: string) { const reflect = require('@alumna/reflect') + validatePath(dest) return reflect({ src, dest, @@ -70,14 +71,18 @@ export class FSExt implements Processor { writeBlob(path: string, data: any) { try { const normalizedPath = normalizeFilePath(path) + const dataBuffer = Buffer.from(data, 'base64') - fs.writeFileSync(join(getJanDataFolderPath(), normalizedPath), dataBuffer) + const writePath = join(getJanDataFolderPath(), normalizedPath) + validatePath(writePath) + fs.writeFileSync(writePath, dataBuffer) } catch (err) { console.error(`writeFile ${path} result: ${err}`) } } copyFile(src: string, dest: string): Promise { + validatePath(dest) return new Promise((resolve, reject) => { fs.copyFile(src, dest, (err) => { if (err) { diff --git a/core/src/node/helper/config.ts b/core/src/node/helper/config.ts index ee9a1f8566..1a341a6252 100644 --- a/core/src/node/helper/config.ts +++ b/core/src/node/helper/config.ts @@ -7,7 +7,8 @@ import childProcess from 'child_process' const configurationFileName = 'settings.json' // TODO: do no specify app name in framework module -const defaultJanDataFolder = join(os.homedir(), 'jan') +// TODO: do not default the os.homedir +const defaultJanDataFolder = join(os?.homedir() || '', 'jan') const defaultAppConfig: AppConfiguration = { data_folder: defaultJanDataFolder, quick_ask: false, diff --git a/core/src/node/helper/path.ts b/core/src/node/helper/path.ts index c20889f4c9..a2d57ed3e7 100644 --- a/core/src/node/helper/path.ts +++ b/core/src/node/helper/path.ts @@ -1,4 +1,5 @@ -import { join } from 'path' +import { join, resolve } from 'path' +import { getJanDataFolderPath } from './config' /** * Normalize file path @@ -33,3 +34,11 @@ export async function appResourcePath(): Promise { // server return join(global.core.appPath(), '../../..') } + +export function validatePath(path: string) { + const janDataFolderPath = getJanDataFolderPath() + const absolutePath = resolve(__dirname, path) + if (!absolutePath.startsWith(janDataFolderPath)) { + throw new Error(`Invalid path: ${absolutePath}`) + } +} diff --git a/core/src/types/api/index.ts b/core/src/types/api/index.ts index a9d0341bda..45f265b5ac 100644 --- a/core/src/types/api/index.ts +++ b/core/src/types/api/index.ts @@ -90,7 +90,6 @@ export enum ExtensionRoute { } export enum FileSystemRoute { appendFileSync = 'appendFileSync', - copyFileSync = 'copyFileSync', unlinkSync = 'unlinkSync', existsSync = 'existsSync', readdirSync = 'readdirSync', diff --git a/extensions/model-extension/package.json b/extensions/model-extension/package.json index c0ca949bd1..6bd8bbe5e0 100644 --- a/extensions/model-extension/package.json +++ b/extensions/model-extension/package.json @@ -1,7 +1,7 @@ { "name": "@janhq/model-extension", "productName": "Model Management", - "version": "1.0.32", + "version": "1.0.33", "description": "Model Management Extension provides model exploration and seamless downloads", "main": "dist/index.js", "node": "dist/node/index.cjs.js", diff --git a/extensions/model-extension/src/index.ts b/extensions/model-extension/src/index.ts index aa8f6603bf..7561ee6edf 100644 --- a/extensions/model-extension/src/index.ts +++ b/extensions/model-extension/src/index.ts @@ -383,7 +383,12 @@ export default class JanModelExtension extends ModelExtension { // model binaries (sources) are absolute path & exist const existFiles = await Promise.all( - model.sources.map((source) => fs.existsSync(source.url)) + model.sources.map( + (source) => + // Supposed to be a local file url + !source.url.startsWith(`http://`) && + !source.url.startsWith(`https://`) + ) ) if (existFiles.every((exist) => exist)) return true