From fa3ca498c2609d381ca0b66fb8b9e9ee376c94df Mon Sep 17 00:00:00 2001 From: Vladislav Meshkorudnyj Date: Sat, 4 Jul 2026 23:38:05 +0200 Subject: [PATCH] fix(secure-file): re-harden on coarse-ctime filesystems Store POSIX mode bits in secure-file hardening cache entries so permission drift is detected even when ctime granularity is coarse. Also clears inherited HISTFILE in the local PTY test harness for hermetic WSL history assertions, and adds a deterministic coarse-ctime regression test for directory and credential-file mode drift. --- src/main/providers/local-pty-provider.test.ts | 9 ++ src/shared/secure-file-coarse-ctime.test.ts | 123 ++++++++++++++++++ src/shared/secure-file.ts | 11 ++ 3 files changed, 143 insertions(+) create mode 100644 src/shared/secure-file-coarse-ctime.test.ts 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