fix: add isPathInside functionality to check path relationships (#8590)

* feat(ipc): add isPathInside functionality to check path relationships

- Introduced a new IPC channel for checking if a path is inside another path, enhancing path validation capabilities.
- Implemented the isPathInside function in the file utility, which accurately determines parent-child path relationships, handling edge cases.
- Updated relevant components to utilize the new isPathInside function for validating app data and backup paths, ensuring better user experience and error handling.
- Added comprehensive tests for isPathInside to cover various scenarios, including edge cases and error handling.

* format code
This commit is contained in:
beyondkmp 2025-07-28 15:45:52 +08:00 committed by GitHub
parent 5bafb3f1b7
commit c4182a950f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 211 additions and 6 deletions

View File

@ -21,6 +21,7 @@ export enum IpcChannel {
App_Select = 'app:select',
App_HasWritePermission = 'app:has-write-permission',
App_ResolvePath = 'app:resolve-path',
App_IsPathInside = 'app:is-path-inside',
App_Copy = 'app:copy',
App_SetStopQuitApp = 'app:set-stop-quit-app',
App_SetAppDataPath = 'app:set-app-data-path',

View File

@ -55,7 +55,7 @@ import { setOpenLinkExternal } from './services/WebviewService'
import { windowService } from './services/WindowService'
import { calculateDirectorySize, getResourcePath } from './utils'
import { decrypt, encrypt } from './utils/aes'
import { getCacheDir, getConfigDir, getFilesDir, hasWritePermission, untildify } from './utils/file'
import { getCacheDir, getConfigDir, getFilesDir, hasWritePermission, isPathInside, untildify } from './utils/file'
import { updateAppDataConfig } from './utils/init'
import { compress, decompress } from './utils/zip'
@ -294,6 +294,11 @@ export function registerIpc(mainWindow: BrowserWindow, app: Electron.App) {
return path.resolve(untildify(filePath))
})
// Check if a path is inside another path (proper parent-child relationship)
ipcMain.handle(IpcChannel.App_IsPathInside, async (_, childPath: string, parentPath: string) => {
return isPathInside(childPath, parentPath)
})
// Set app data path
ipcMain.handle(IpcChannel.App_SetAppDataPath, async (_, filePath: string) => {
updateAppDataConfig(filePath)

View File

@ -9,7 +9,16 @@ import { detectAll as detectEncodingAll } from 'jschardet'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { readTextFileWithAutoEncoding } from '../file'
import { getAllFiles, getAppConfigDir, getConfigDir, getFilesDir, getFileType, getTempDir, untildify } from '../file'
import {
getAllFiles,
getAppConfigDir,
getConfigDir,
getFilesDir,
getFileType,
getTempDir,
isPathInside,
untildify
} from '../file'
// Mock dependencies
vi.mock('node:fs')
@ -343,4 +352,154 @@ describe('file', () => {
expect(untildify('~/folder_with_underscores')).toBe('/mock/home/folder_with_underscores')
})
})
describe('isPathInside', () => {
beforeEach(() => {
// Mock path.resolve to simulate path resolution
vi.mocked(path.resolve).mockImplementation((...args) => {
const joined = args.join('/')
return joined.startsWith('/') ? joined : `/${joined}`
})
// Mock path.normalize to simulate path normalization
vi.mocked(path.normalize).mockImplementation((p) => p.replace(/\/+/g, '/'))
// Mock path.relative to calculate relative paths
vi.mocked(path.relative).mockImplementation((from, to) => {
// Simple mock implementation for testing
const fromParts = from.split('/').filter((p) => p)
const toParts = to.split('/').filter((p) => p)
// Find common prefix
let i = 0
while (i < fromParts.length && i < toParts.length && fromParts[i] === toParts[i]) {
i++
}
// Calculate relative path
const upLevels = fromParts.length - i
const downPath = toParts.slice(i)
if (upLevels === 0 && downPath.length === 0) {
return ''
}
const result = ['..'.repeat(upLevels), ...downPath].filter((p) => p).join('/')
return result || '.'
})
// Mock path.isAbsolute
vi.mocked(path.isAbsolute).mockImplementation((p) => p.startsWith('/'))
})
describe('basic parent-child relationships', () => {
it('should return true when child is inside parent', () => {
expect(isPathInside('/root/test/child', '/root/test')).toBe(true)
expect(isPathInside('/root/test/deep/child', '/root/test')).toBe(true)
expect(isPathInside('child/deep', 'child')).toBe(true)
})
it('should return false when child is not inside parent', () => {
expect(isPathInside('/root/test', '/root/test/child')).toBe(false)
expect(isPathInside('/root/other', '/root/test')).toBe(false)
expect(isPathInside('/different/path', '/root/test')).toBe(false)
expect(isPathInside('child', 'child/deep')).toBe(false)
})
it('should return true when paths are the same', () => {
expect(isPathInside('/root/test', '/root/test')).toBe(true)
expect(isPathInside('child', 'child')).toBe(true)
})
})
describe('edge cases that startsWith cannot handle', () => {
it('should correctly distinguish similar path names', () => {
// The problematic case mentioned by user
expect(isPathInside('/root/test aaa', '/root/test')).toBe(false)
expect(isPathInside('/root/test', '/root/test aaa')).toBe(false)
// More similar cases
expect(isPathInside('/home/user-data', '/home/user')).toBe(false)
expect(isPathInside('/home/user', '/home/user-data')).toBe(false)
expect(isPathInside('/var/log-backup', '/var/log')).toBe(false)
})
it('should handle paths with spaces correctly', () => {
expect(isPathInside('/path with spaces/child', '/path with spaces')).toBe(true)
expect(isPathInside('/path with spaces', '/path with spaces/child')).toBe(false)
})
it('should handle Windows-style paths', () => {
// Mock for Windows paths
vi.mocked(path.resolve).mockImplementation((...args) => {
const joined = args.join('\\').replace(/\//g, '\\')
return joined.match(/^[A-Z]:/) ? joined : `C:${joined}`
})
vi.mocked(path.normalize).mockImplementation((p) => p.replace(/\\+/g, '\\'))
// Mock path.relative for Windows paths
vi.mocked(path.relative).mockImplementation((from, to) => {
const fromParts = from.split('\\').filter((p) => p && p !== 'C:')
const toParts = to.split('\\').filter((p) => p && p !== 'C:')
// Find common prefix
let i = 0
while (i < fromParts.length && i < toParts.length && fromParts[i] === toParts[i]) {
i++
}
// Calculate relative path
const upLevels = fromParts.length - i
const downPath = toParts.slice(i)
if (upLevels === 0 && downPath.length === 0) {
return ''
}
const upPath = Array(upLevels).fill('..').join('\\')
const result = [upPath, ...downPath].filter((p) => p).join('\\')
return result || '.'
})
expect(isPathInside('C:\\Users\\test\\child', 'C:\\Users\\test')).toBe(true)
expect(isPathInside('C:\\Users\\test aaa', 'C:\\Users\\test')).toBe(false)
})
})
describe('error handling', () => {
it('should return false when path operations throw errors', () => {
vi.mocked(path.resolve).mockImplementation(() => {
throw new Error('Path resolution failed')
})
expect(isPathInside('/any/path', '/any/parent')).toBe(false)
})
})
describe('comparison with startsWith behavior', () => {
const testCases: [string, string, boolean, boolean][] = [
['/root/test aaa', '/root/test', false, true], // isPathInside vs startsWith
['/root/test', '/root/test aaa', false, false],
['/root/test/child', '/root/test', true, true],
['/home/user-data', '/home/user', false, true]
]
it.each(testCases)(
'should correctly handle %s vs %s',
(child: string, parent: string, expectedIsPathInside: boolean, expectedStartsWith: boolean) => {
const isPathInsideResult = isPathInside(child, parent)
const startsWithResult = child.startsWith(parent)
expect(isPathInsideResult).toBe(expectedIsPathInside)
expect(startsWithResult).toBe(expectedStartsWith)
// Verify that isPathInside gives different (correct) result in problematic cases
if (expectedIsPathInside !== expectedStartsWith) {
expect(isPathInsideResult).not.toBe(startsWithResult)
}
}
)
})
})
})

View File

@ -46,6 +46,42 @@ export async function hasWritePermission(dir: string) {
}
}
/**
* Check if a path is inside another path (proper parent-child relationship)
* This function correctly handles edge cases that string.startsWith() cannot handle,
* such as distinguishing between '/root/test' and '/root/test aaa'
*
* @param childPath - The path that might be inside the parent path
* @param parentPath - The path that might contain the child path
* @returns true if childPath is inside parentPath, false otherwise
*/
export function isPathInside(childPath: string, parentPath: string): boolean {
try {
const resolvedChild = path.resolve(childPath)
const resolvedParent = path.resolve(parentPath)
// Normalize paths to handle different separators
const normalizedChild = path.normalize(resolvedChild)
const normalizedParent = path.normalize(resolvedParent)
// Check if they are the same path
if (normalizedChild === normalizedParent) {
return true
}
// Get relative path from parent to child
const relativePath = path.relative(normalizedParent, normalizedChild)
// If relative path is empty, they are the same
// If relative path starts with '..', child is not inside parent
// If relative path is absolute, child is not inside parent
return relativePath !== '' && !relativePath.startsWith('..') && !path.isAbsolute(relativePath)
} catch (error) {
logger.error('Failed to check path relationship:', error as Error)
return false
}
}
export function getFileType(ext: string): FileTypes {
ext = ext.toLowerCase()
return fileTypeMap.get(ext) || FileTypes.OTHER

View File

@ -60,6 +60,8 @@ const api = {
select: (options: Electron.OpenDialogOptions) => ipcRenderer.invoke(IpcChannel.App_Select, options),
hasWritePermission: (path: string) => ipcRenderer.invoke(IpcChannel.App_HasWritePermission, path),
resolvePath: (path: string) => ipcRenderer.invoke(IpcChannel.App_ResolvePath, path),
isPathInside: (childPath: string, parentPath: string) =>
ipcRenderer.invoke(IpcChannel.App_IsPathInside, childPath, parentPath),
setAppDataPath: (path: string) => ipcRenderer.invoke(IpcChannel.App_SetAppDataPath, path),
getDataPathFromArgs: () => ipcRenderer.invoke(IpcChannel.App_GetDataPathFromArgs),
copy: (oldPath: string, newPath: string, occupiedDirs: string[] = []) =>

View File

@ -210,13 +210,15 @@ const DataSettings: FC = () => {
}
// check new app data path is not in old app data path
if (newAppDataPath.startsWith(appInfo.appDataPath)) {
const isInOldPath = await window.api.isPathInside(newAppDataPath, appInfo.appDataPath)
if (isInOldPath) {
window.message.error(t('settings.data.app_data.select_error_same_path'))
return
}
// check new app data path is not in app install path
if (newAppDataPath.startsWith(appInfo.installPath)) {
const isInInstallPath = await window.api.isPathInside(newAppDataPath, appInfo.installPath)
if (isInInstallPath) {
window.message.error(t('settings.data.app_data.select_error_in_app_path'))
return
}

View File

@ -75,14 +75,14 @@ const LocalBackupSettings: React.FC = () => {
// check new local backup dir is not in app data path
// if is in app data path, show error
if (resolvedDir.startsWith(appInfo!.appDataPath)) {
if (await window.api.isPathInside(resolvedDir, appInfo!.appDataPath)) {
window.message.error(t('settings.data.local.directory.select_error_app_data_path'))
return false
}
// check new local backup dir is not in app install path
// if is in app install path, show error
if (resolvedDir.startsWith(appInfo!.installPath)) {
if (await window.api.isPathInside(resolvedDir, appInfo!.installPath)) {
window.message.error(t('settings.data.local.directory.select_error_in_app_install_path'))
return false
}