diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index bc236e9f9bd..d37faa0ac57 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -98,6 +98,7 @@ describe('LocalPtyProvider', () => { let exitCb: ((info: { exitCode: number }) => void) | undefined let origShell: string | undefined let origPowerlevelWizardDisable: string | undefined + let origHistFile: string | undefined let origPlatform: PropertyDescriptor | undefined beforeEach(() => { @@ -105,8 +106,11 @@ describe('LocalPtyProvider', () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) origShell = process.env.SHELL origPowerlevelWizardDisable = process.env.POWERLEVEL9K_DISABLE_CONFIGURATION_WIZARD + origHistFile = process.env.HISTFILE process.env.SHELL = '/bin/zsh' delete process.env.POWERLEVEL9K_DISABLE_CONFIGURATION_WIZARD + // injectHistoryEnv preserves an inherited HISTFILE, so clear it for hermetic history assertions. + delete process.env.HISTFILE existsSyncMock.mockReturnValue(true) statSyncMock.mockReturnValue({ isDirectory: () => true, mode: 0o755 }) @@ -158,6 +162,11 @@ describe('LocalPtyProvider', () => { } else { process.env.POWERLEVEL9K_DISABLE_CONFIGURATION_WIZARD = origPowerlevelWizardDisable } + if (origHistFile === undefined) { + delete process.env.HISTFILE + } else { + process.env.HISTFILE = origHistFile + } }) describe('spawn', () => { diff --git a/src/shared/secure-file-coarse-ctime.test.ts b/src/shared/secure-file-coarse-ctime.test.ts new file mode 100644 index 00000000000..64c33ba36f1 --- /dev/null +++ b/src/shared/secure-file-coarse-ctime.test.ts @@ -0,0 +1,123 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { chmodSyncMock, existsSyncMock, paths, statSyncMock, state } = vi.hoisted(() => ({ + chmodSyncMock: vi.fn(), + existsSyncMock: vi.fn(), + paths: { + dir: '/secure', + file: '/secure/secret.json' + }, + statSyncMock: vi.fn(), + state: { + dirMode: 0o755, + fileMode: 0o600 + } +})) + +vi.mock('fs', () => ({ + chmodSync: chmodSyncMock, + existsSync: existsSyncMock, + mkdirSync: vi.fn(), + renameSync: vi.fn(), + rmSync: vi.fn(), + statSync: statSyncMock, + writeFileSync: vi.fn() +})) + +vi.mock('child_process', () => ({ + execFile: vi.fn(), + execFileSync: vi.fn() +})) + +import { + __resetSecureFileHardenedPathsForTests, + __resetSecureFileWindowsUserSidForTests, + hardenExistingSecureFile +} from './secure-file' + +describe('secure-file coarse-ctime mode drift', () => { + const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + + beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) + state.dirMode = 0o755 + state.fileMode = 0o600 + existsSyncMock.mockReturnValue(true) + statSyncMock.mockImplementation((targetPath: string) => { + const path = String(targetPath) + if (path === paths.dir) { + return fakeStats(true, state.dirMode) + } + if (path === paths.file) { + return fakeStats(false, state.fileMode) + } + throw new Error(`unexpected stat path ${path}`) + }) + chmodSyncMock.mockImplementation((targetPath: string, mode: number) => { + const path = String(targetPath) + if (path === paths.dir) { + state.dirMode = mode & 0o777 + } else if (path === paths.file) { + state.fileMode = mode & 0o777 + } + }) + __resetSecureFileWindowsUserSidForTests() + __resetSecureFileHardenedPathsForTests() + }) + + afterEach(() => { + chmodSyncMock.mockReset() + existsSyncMock.mockReset() + statSyncMock.mockReset() + __resetSecureFileWindowsUserSidForTests() + __resetSecureFileHardenedPathsForTests() + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } + }) + + it('re-hardens a directory when only POSIX mode changes under stable timestamps', () => { + hardenExistingSecureFile(paths.file) + expect(chmodSyncMock.mock.calls).toEqual([ + [paths.dir, 0o700], + [paths.file, 0o600] + ]) + + chmodSyncMock.mockClear() + // Coarse-ctime filesystems can report identical timestamps after chmod; only mode changes. + state.dirMode = 0o755 + + hardenExistingSecureFile(paths.file) + + expect(chmodSyncMock.mock.calls).toEqual([[paths.dir, 0o700]]) + }) + + it('re-hardens a file when only POSIX mode changes under stable timestamps', () => { + hardenExistingSecureFile(paths.file) + expect(chmodSyncMock.mock.calls).toEqual([ + [paths.dir, 0o700], + [paths.file, 0o600] + ]) + + chmodSyncMock.mockClear() + // Coarse-ctime filesystems can report identical timestamps after chmod; only mode changes. + state.fileMode = 0o644 + + hardenExistingSecureFile(paths.file) + + expect(chmodSyncMock.mock.calls).toEqual([[paths.file, 0o600]]) + }) +}) + +function fakeStats(isDirectory: boolean, mode: number) { + return { + birthtimeMs: 10, + ctimeMs: 20, + dev: 30, + ino: isDirectory ? 40 : 41, + isDirectory: () => isDirectory, + mode, + mtimeMs: 50, + size: isDirectory ? 0 : 2 + } +} diff --git a/src/shared/secure-file.ts b/src/shared/secure-file.ts index 146c62b09e0..1ed451b5633 100644 --- a/src/shared/secure-file.ts +++ b/src/shared/secure-file.ts @@ -18,6 +18,7 @@ type HardenedPathCacheEntry = { dev: number ino: number size: number + mode: number ctimeMs: number mtimeMs: number birthtimeMs: number @@ -130,6 +131,7 @@ export function hardenExistingSecureFile(targetPath: string): void { } } +/** Applies the platform-appropriate permission restriction to a path once, bypassing the cache. */ export function hardenSecurePath( targetPath: string, options: { @@ -146,6 +148,7 @@ export function hardenSecurePath( ) } +/** Applies hardening; async Windows calls only report that best-effort ACL work was accepted. */ function applySecurePathRestriction( targetPath: string, isDirectory: boolean, @@ -170,6 +173,7 @@ function applySecurePathRestriction( return true } +/** Caches the current metadata snapshot for a just-hardened path, or clears it if the path is gone. */ function rememberHardenedPath(targetPath: string, isDirectory: boolean): void { const entry = getHardenedPathCacheEntry(targetPath, isDirectory) if (entry) { @@ -179,6 +183,10 @@ function rememberHardenedPath(targetPath: string, isDirectory: boolean): void { } } +/** + * Snapshots a path's identity, mode, and timestamps so later drift is detectable. + * Mode is tracked directly so a chmod is caught even where coarse ctime granularity hides it. + */ function getHardenedPathCacheEntry( targetPath: string, isDirectory: boolean @@ -193,6 +201,7 @@ function getHardenedPathCacheEntry( dev: stats.dev, ino: stats.ino, size: stats.size, + mode: stats.mode & 0o777, ctimeMs: stats.ctimeMs, mtimeMs: stats.mtimeMs, birthtimeMs: stats.birthtimeMs @@ -202,6 +211,7 @@ function getHardenedPathCacheEntry( } } +/** True when two snapshots describe the same unchanged path (identity, mode, timestamps). */ function hardenedPathCacheEntriesMatch( a: HardenedPathCacheEntry, b: HardenedPathCacheEntry @@ -211,6 +221,7 @@ function hardenedPathCacheEntriesMatch( a.dev === b.dev && a.ino === b.ino && a.size === b.size && + a.mode === b.mode && a.ctimeMs === b.ctimeMs && a.mtimeMs === b.mtimeMs && a.birthtimeMs === b.birthtimeMs