Skip to content

Commit

Permalink
fix: validate path in fs (#3152)
Browse files Browse the repository at this point in the history
* 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 <louis@jan.ai>
  • Loading branch information
marknguyen1302 and louis-jan committed Jul 12, 2024
1 parent b28a976 commit dddf350
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 23 deletions.
5 changes: 0 additions & 5 deletions core/src/browser/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ const appendFileSync = (...args: any[]) => globalThis.core.api?.appendFileSync(.
const syncFile: (src: string, dest: string) => Promise<any> = (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<void> = (src, dest) =>
globalThis.core.api?.copyFile(src, dest)
Expand Down Expand Up @@ -95,7 +91,6 @@ export const fs = {
rm,
unlinkSync,
appendFileSync,
copyFileSync,
copyFile,
syncFile,
fileStat,
Expand Down
3 changes: 2 additions & 1 deletion core/src/node/api/processors/download.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
Expand Down
39 changes: 29 additions & 10 deletions core/src/node/api/processors/fs.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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
})
)
)
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -73,4 +91,5 @@ export class FileSystem implements Processor {
})
})
}

}
9 changes: 7 additions & 2 deletions core/src/node/api/processors/fsExt.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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,
Expand Down Expand Up @@ -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<void> {
validatePath(dest)
return new Promise((resolve, reject) => {
fs.copyFile(src, dest, (err) => {
if (err) {
Expand Down
3 changes: 2 additions & 1 deletion core/src/node/helper/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 10 additions & 1 deletion core/src/node/helper/path.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { join } from 'path'
import { join, resolve } from 'path'
import { getJanDataFolderPath } from './config'

/**
* Normalize file path
Expand Down Expand Up @@ -33,3 +34,11 @@ export async function appResourcePath(): Promise<string> {
// 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}`)
}
}
1 change: 0 additions & 1 deletion core/src/types/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ export enum ExtensionRoute {
}
export enum FileSystemRoute {
appendFileSync = 'appendFileSync',
copyFileSync = 'copyFileSync',
unlinkSync = 'unlinkSync',
existsSync = 'existsSync',
readdirSync = 'readdirSync',
Expand Down
2 changes: 1 addition & 1 deletion extensions/model-extension/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
7 changes: 6 additions & 1 deletion extensions/model-extension/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit dddf350

Please sign in to comment.