From b458c5eb0a07e83932fd8486e70bdf4d9b301ef3 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:16:19 -0700 Subject: [PATCH] fix(ssh): count only directory traversals in glob expansion budget Plain files in a busy directory (e.g., many SSH keys in ~/.ssh) were counting against the traversal budget, unnecessarily limiting glob expansion. Refactor to use `withFileTypes: true` to distinguish directories from files, and move `globWithTraversalLimit` to the glob-readability module where directory budgets are already separated from entry budgets. --- .../ssh-config-include-completeness.test.ts | 13 +++++++++ src/main/ssh/ssh-config-include-expander.ts | 27 +++---------------- .../ssh-config-include-glob-readability.ts | 24 +++++++++++++++++ .../ssh/ssh-config-loader-regression.test.ts | 16 ++++++++--- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/main/ssh/ssh-config-include-completeness.test.ts b/src/main/ssh/ssh-config-include-completeness.test.ts index b5983cbb4e0..ad6261f2022 100644 --- a/src/main/ssh/ssh-config-include-completeness.test.ts +++ b/src/main/ssh/ssh-config-include-completeness.test.ts @@ -181,6 +181,19 @@ describe('SSH config Include completeness', () => { expect(expandSshConfigIncludes(configPath).fullyExpanded).toBe(true) }) + it('does not charge plain files in a busy directory against the traversal budget', () => { + const home = makeTemporaryHome() + const configPath = writeFile(home, '.ssh/config', 'Include **/50-*\n') + writeFile(home, '.ssh/zz/50-prod', 'Host prod\n HostName prod.example.com\n') + for (let index = 0; index < 1100; index += 1) { + writeFile(home, `.ssh/id_key_${index}`, '') + } + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + expect(expandSshConfigIncludes(configPath).content).toContain('prod.example.com') + expect(warn).not.toHaveBeenCalledWith(expect.stringContaining('walks more than')) + }) + it('stops a glob that walks too many directories and marks it incomplete', () => { const home = makeTemporaryHome() const configPath = writeFile(home, '.ssh/config', 'Include conf.d/*/config\n') diff --git a/src/main/ssh/ssh-config-include-expander.ts b/src/main/ssh/ssh-config-include-expander.ts index 19c42c15f3e..5c5e3e4822b 100644 --- a/src/main/ssh/ssh-config-include-expander.ts +++ b/src/main/ssh/ssh-config-include-expander.ts @@ -1,10 +1,12 @@ -import { globSync, readFileSync, realpathSync, statSync } from 'node:fs' +import { readFileSync, realpathSync, statSync } from 'node:fs' import { homedir, hostname } from 'node:os' import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { createGlobReadabilityProofs, findGlobExpansionUncertainty, + globWithTraversalLimit, hasGlobPattern, + MAX_INCLUDE_GLOB_TRAVERSAL, type GlobExpansionUncertainty, type GlobReadabilityProofs } from './ssh-config-include-glob-readability' @@ -43,8 +45,6 @@ type ResolvedIncludePaths = { } const MAX_INCLUDE_GLOB_MATCHES = 256 -// Caps the directories a single Include glob may walk on the main process before we stop collecting. -const MAX_INCLUDE_GLOB_TRAVERSAL = 1024 const MAX_INCLUDE_FILE_BYTES = 1024 * 1024 export function expandSshConfigIncludes(configPath: string): SshConfigExpansion { @@ -249,7 +249,7 @@ function resolveIncludePaths( const absolutePattern = resolveIncludePatternPath(withTokens, context) if (hasGlobPattern(absolutePattern)) { try { - const { matches, traversalLimited } = globWithTraversalLimit(absolutePattern) + const { matches, traversalLimited } = globWithTraversalLimit(absolutePattern, context.pathApi) matches.sort((left, right) => left.localeCompare(right)) if (traversalLimited) { console.warn( @@ -301,25 +301,6 @@ function resolveIncludePaths( } } -/** Stops descending once the walk exceeds its budget, keeping the matches already found. */ -function globWithTraversalLimit(pattern: string): { - matches: string[] - traversalLimited: boolean -} { - let remaining = MAX_INCLUDE_GLOB_TRAVERSAL - let traversalLimited = false - const matches = globSync(pattern, { - exclude: () => { - remaining -= 1 - if (remaining < 0) { - traversalLimited = true - } - return traversalLimited - } - }) - return { matches, traversalLimited } -} - function getCanonicalPath( filePath: string, context: IncludeExpansionContext, diff --git a/src/main/ssh/ssh-config-include-glob-readability.ts b/src/main/ssh/ssh-config-include-glob-readability.ts index 591e6a6f4c8..7182ccaa5c1 100644 --- a/src/main/ssh/ssh-config-include-glob-readability.ts +++ b/src/main/ssh/ssh-config-include-glob-readability.ts @@ -10,6 +10,8 @@ const MAX_GLOB_READABILITY_PATHS = 256 // larger allowance. A ~/.ssh holding hundreds of keys and control sockets is ordinary, and charging // its entries against the path budget reported a perfectly readable directory as unopenable. const MAX_GLOB_READABILITY_ENTRIES = 16_384 +// Caps the directories a single Include glob may walk on the main process before we stop collecting. +export const MAX_INCLUDE_GLOB_TRAVERSAL = 1024 const GLOB_READABILITY_LIMIT_REACHED = Symbol('glob-readability-limit-reached') /** @@ -174,3 +176,25 @@ function getGlobDirectoryPrefixes(pattern: string, pathApi: PathApi): string[] { } return prefixes } + +/** Stops descending once the walk exceeds its budget, keeping the matches already found. */ +export function globWithTraversalLimit( + pattern: string, + pathApi: PathApi +): { matches: string[]; traversalLimited: boolean } { + const walkedDirectories = new Set() + let traversalLimited = false + const matches = globSync(pattern, { + withFileTypes: true, + exclude: (entry) => { + // Only descents cost a readdir; counting plain files would let a busy ~/.ssh trip the budget. + if (!traversalLimited && (entry.isDirectory() || entry.isSymbolicLink())) { + walkedDirectories.add(pathApi.join(entry.parentPath, entry.name)) + traversalLimited = walkedDirectories.size > MAX_INCLUDE_GLOB_TRAVERSAL + } + return traversalLimited + } + }) + const paths = matches.map((entry) => pathApi.join(entry.parentPath, entry.name)) + return { matches: paths, traversalLimited } +} diff --git a/src/main/ssh/ssh-config-loader-regression.test.ts b/src/main/ssh/ssh-config-loader-regression.test.ts index 089ee9d99bb..5c452452932 100644 --- a/src/main/ssh/ssh-config-loader-regression.test.ts +++ b/src/main/ssh/ssh-config-loader-regression.test.ts @@ -1,6 +1,6 @@ import type * as FsModule from 'node:fs' import type * as OsModule from 'node:os' -import { win32 } from 'node:path' +import { posix, win32 } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' afterEach(() => { @@ -14,6 +14,14 @@ function normalizeWin(value: string): string { return win32.normalize(value.replaceAll('/', '\\')) } +/** Shapes paths like `globSync(..., { withFileTypes: true })` results. */ +function globEntries(paths: string[]) { + return paths.map((entry) => { + const pathApi = /^[a-zA-Z]:\\/.test(entry) ? win32 : posix + return { parentPath: pathApi.dirname(entry), name: pathApi.basename(entry) } + }) +} + function platformSshHome(): string { return process.platform === 'win32' ? 'C:\\Users\\testuser' : '/home/testuser' } @@ -79,10 +87,10 @@ describe('loadUserSshConfig regressions', () => { existsSync: (filePath: string) => files.has(normalizeWin(filePath)), globSync: (pattern: string) => normalizeWin(pattern) === normalizeWin('C:/Users/Test User/.ssh/conf.d/*.conf') - ? [ + ? globEntries([ normalizeWin('C:/Users/Test User/.ssh/conf.d/alpha.conf'), normalizeWin('C:/Users/Test User/.ssh/conf.d/zeta.conf') - ] + ]) : [], readFileSync: (filePath: string) => { const content = files.get(normalizeWin(filePath)) @@ -205,7 +213,7 @@ describe('loadUserSshConfig regressions', () => { ...actual, existsSync: (filePath: string) => filePath === configPath || includePaths.includes(filePath), - globSync: () => [...includePaths].toReversed(), + globSync: () => globEntries([...includePaths].toReversed()), readFileSync: (filePath: string) => { if (filePath === configPath) { return 'Include conf.d/*.conf\n'