fix(ssh): separate glob entry and path budgets, memoize traversals

Entries were previously charged against the path budget, causing a readable
~/.ssh holding many keys and control sockets to be incorrectly reported as
unopenable, which permanently disabled endpoint restoration via uncertain
alias claims.

Separate budget allocation allows large readable directories to prove
complete. Add per-expansion memoization so sibling Includes share traversal
work, and distinguish between "unreadable" (I/O fault) and
"unproven-within-budget" (too large to scan) uncertainty. Redact
${VAR}-substituted paths from logs to prevent leaking secrets.
This commit is contained in:
Jinjing
2026-09-28 15:35:29 -07:00
parent 3a75ba8c29
commit 12119eaecd
4 changed files with 310 additions and 63 deletions
@@ -30,6 +30,7 @@ const lockedPaths = new Set<string>()
const canRestrictFileAccess = process.platform !== 'win32' && (process.getuid?.() ?? 0) !== 0
afterEach(() => {
vi.restoreAllMocks()
vi.useRealTimers()
vi.unstubAllEnvs()
invalidateSshConfigAliasClaimCache()
@@ -151,6 +152,68 @@ describe('SSH config Include completeness', () => {
expect(expandSshConfigIncludes(configPath).fullyExpanded).toBe(true)
})
/**
* A ~/.ssh holding hundreds of keys and control sockets is ordinary. Reporting it as unopenable
* turned the alias claim permanently uncertain, which silently disabled endpoint restoration.
*/
it('proves a glob complete under a readable directory with hundreds of entries', () => {
const home = makeTemporaryHome()
// The glob's literal parent is ~/.ssh itself, so its entries are what the scan has to walk.
const configPath = writeFile(home, '.ssh/config', 'Include 50-*\n')
writeFile(home, '.ssh/50-wildcard', 'Host *\n ForwardAgent yes\n')
for (let index = 0; index < 302; index += 1) {
writeFile(home, `.ssh/id_key_${index}`, '')
}
expect(expandSshConfigIncludes(configPath).fullyExpanded).toBe(true)
expect(sshConfigMayClaimAlias('prod', loadUserSshConfigAliasClaims())).toBe(false)
})
it('proves a nested glob complete when its literal parent holds hundreds of entries', () => {
const home = makeTemporaryHome()
const configPath = writeFile(home, '.ssh/config', 'Include sub*/config\n')
writeFile(home, '.ssh/sub-work/config', 'Host *\n ForwardAgent yes\n')
for (let index = 0; index < 302; index += 1) {
writeFile(home, `.ssh/id_key_${index}`, '')
}
expect(expandSshConfigIncludes(configPath).fullyExpanded).toBe(true)
})
})
describe('SSH config Include warnings', () => {
const SECRET_SEGMENT = 's3cr3t-vault'
it('names the pattern, not the substituted directory, when ${VAR} resolved a path', () => {
const home = makeTemporaryHome()
const configPath = writeFile(home, '.ssh/config', 'Include ${ORCA_TEST_SSH_VAULT}/50-prod\n')
mkdirSync(join(home, '.ssh', SECRET_SEGMENT, '50-prod'), {
recursive: true
})
vi.stubEnv('ORCA_TEST_SSH_VAULT', join(home, '.ssh', SECRET_SEGMENT))
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
expandSshConfigIncludes(configPath)
expect(warn).toHaveBeenCalled()
expect(warn.mock.calls.flat().join('\n')).not.toContain(SECRET_SEGMENT)
})
it.runIf(canRestrictFileAccess)(
'keeps the substituted directory out of a glob completeness warning',
() => {
const home = makeTemporaryHome()
const configPath = writeFile(home, '.ssh/config', 'Include ${ORCA_TEST_SSH_VAULT}/*/config\n')
writeFile(home, `.ssh/${SECRET_SEGMENT}/personal/config`, 'Host personal\n')
lockPath(join(home, '.ssh', SECRET_SEGMENT, 'personal'))
vi.stubEnv('ORCA_TEST_SSH_VAULT', join(home, '.ssh', SECRET_SEGMENT))
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
expect(expandSshConfigIncludes(configPath).fullyExpanded).toBe(false)
expect(warn.mock.calls.flat().join('\n')).not.toContain(SECRET_SEGMENT)
}
)
})
describe('SSH config alias claim completeness', () => {
+109 -33
View File
@@ -1,7 +1,13 @@
import { globSync, readFileSync, realpathSync, statSync } from 'node:fs'
import { homedir, hostname } from 'node:os'
import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence'
import { findGlobExpansionUncertainty, hasGlobPattern } from './ssh-config-include-glob-readability'
import {
createGlobReadabilityProofs,
findGlobExpansionUncertainty,
hasGlobPattern,
type GlobExpansionUncertainty,
type GlobReadabilityProofs
} from './ssh-config-include-glob-readability'
import {
expandEnvironmentVariables,
expandIncludeTokens,
@@ -21,6 +27,19 @@ type SshConfigExpansion = {
type IncludeExpansionContext = IncludePathContext & {
cache: Map<string, string>
fullyExpanded: boolean
/** Shared across every Include so one ~/.ssh is enumerated once per expansion, not once per glob. */
globProofs: GlobReadabilityProofs
}
type ResolvedIncludePaths = {
paths: string[]
/**
* What to log in place of a resolved path, or `null` when the resolved path is safe to log.
*
* `${VAR}` expansion can substitute a directory the user keeps secret, and these warnings land in
* local logs. The pattern as written identifies the offending Include line just as well.
*/
redactedLabel: string | null
}
const MAX_INCLUDE_GLOB_MATCHES = 256
@@ -34,6 +53,7 @@ export function expandSshConfigIncludes(configPath: string): SshConfigExpansion
const context: IncludeExpansionContext = {
cache: new Map(),
fullyExpanded: true,
globProofs: createGlobReadabilityProofs(),
home,
pathApi,
rootDir: pathApi.dirname(configPath),
@@ -46,22 +66,41 @@ export function expandSshConfigIncludes(configPath: string): SshConfigExpansion
return { content: lines.join('\n'), fullyExpanded: context.fullyExpanded }
}
function markIncomplete(context: IncludeExpansionContext, target: string): void {
function markIncomplete(context: IncludeExpansionContext, target: string, detail?: string): void {
context.fullyExpanded = false
console.warn(`[ssh] Could not expand SSH config Include "${target}"; hosts may be missing`)
const because = detail ? ` (${detail})` : ''
console.warn(
`[ssh] Could not expand SSH config Include "${target}"${because}; hosts may be missing`
)
}
/** An unreadable directory is the user's to fix; an unproven one is only bigger than our budget. */
function describeGlobUncertainty(
uncertainty: GlobExpansionUncertainty,
redactedLabel: string | null
): string {
const where = redactedLabel === null ? `"${uncertainty.target}"` : 'a directory it matches'
return uncertainty.reason === 'unreadable'
? `could not enumerate ${where}`
: `${where} is too large to scan for completeness`
}
/**
* `logTarget` is what warnings about this file name; it differs from `filePath` only when the path
* came out of `${VAR}` expansion and so must not reach a log.
*/
function expandSshConfigFile(
filePath: string,
context: IncludeExpansionContext,
activeStack: string[]
activeStack: string[],
logTarget: string = filePath
): string[] {
const canonicalPath = getCanonicalPath(filePath, context)
const canonicalPath = getCanonicalPath(filePath, context, logTarget)
if (!canonicalPath || activeStack.includes(canonicalPath)) {
return []
}
const rawContent = readCachedFile(canonicalPath, context)
const rawContent = readCachedFile(canonicalPath, context, logTarget)
if (rawContent === null) {
return []
}
@@ -77,8 +116,17 @@ function expandSshConfigFile(
}
for (const includeArg of includeArgs) {
for (const matchedPath of resolveIncludePaths(includeArg, context)) {
appendExpandedLines(expandedLines, expandSshConfigFile(matchedPath, context, nextStack))
const resolved = resolveIncludePaths(includeArg, context)
for (const matchedPath of resolved.paths) {
appendExpandedLines(
expandedLines,
expandSshConfigFile(
matchedPath,
context,
nextStack,
resolved.redactedLabel ?? matchedPath
)
)
}
}
}
@@ -94,13 +142,17 @@ function appendExpandedLines(target: string[], lines: readonly string[]): void {
}
}
function readCachedFile(filePath: string, context: IncludeExpansionContext): string | null {
function readCachedFile(
filePath: string,
context: IncludeExpansionContext,
logTarget: string
): string | null {
const cached = context.cache.get(filePath)
if (cached !== undefined) {
return cached
}
if (!isReadableRegularFile(filePath, context)) {
if (!isReadableRegularFile(filePath, context, logTarget)) {
return null
}
@@ -110,7 +162,7 @@ function readCachedFile(filePath: string, context: IncludeExpansionContext): str
return content
} catch (error) {
if (!isDefinitiveAbsence(error)) {
markIncomplete(context, filePath)
markIncomplete(context, logTarget)
}
return null
}
@@ -172,17 +224,24 @@ function splitQuotedArguments(input: string): string[] {
return args
}
function resolveIncludePaths(pattern: string, context: IncludeExpansionContext): string[] {
function resolveIncludePaths(
pattern: string,
context: IncludeExpansionContext
): ResolvedIncludePaths {
const withEnv = expandEnvironmentVariables(pattern)
if (withEnv === null) {
markIncomplete(context, pattern)
return []
markIncomplete(context, pattern, 'unset environment variable')
return { paths: [], redactedLabel: pattern }
}
// Only a substitution that actually fired can carry a secret into a resolved path.
const redactedLabel = withEnv === pattern ? null : pattern
const logTarget = (resolved: string): string => redactedLabel ?? resolved
const withTokens = expandIncludeTokens(withEnv, context)
if (withTokens === null) {
markIncomplete(context, pattern)
return []
markIncomplete(context, pattern, 'token needs a connection target')
return { paths: [], redactedLabel }
}
const absolutePattern = resolveIncludePatternPath(withTokens, context)
@@ -191,22 +250,31 @@ function resolveIncludePaths(pattern: string, context: IncludeExpansionContext):
const matches = globSync(absolutePattern).sort((left, right) => left.localeCompare(right))
if (matches.length > MAX_INCLUDE_GLOB_MATCHES) {
console.warn(
`[ssh] Include pattern "${absolutePattern}" matched ${matches.length} files; processing first ${MAX_INCLUDE_GLOB_MATCHES}`
`[ssh] Include pattern "${logTarget(absolutePattern)}" matched ${matches.length} files; processing first ${MAX_INCLUDE_GLOB_MATCHES}`
)
context.fullyExpanded = false
return matches.slice(0, MAX_INCLUDE_GLOB_MATCHES)
return {
paths: matches.slice(0, MAX_INCLUDE_GLOB_MATCHES),
redactedLabel
}
}
// Unconditional, not only on an empty result: a partial expansion is exactly as unproven, and
// it is the half that goes on to feed a confident alias claim.
const uncertainTarget = findGlobExpansionUncertainty(absolutePattern, context.pathApi)
if (uncertainTarget) {
markIncomplete(context, uncertainTarget)
const uncertainty = findGlobExpansionUncertainty(absolutePattern, context.pathApi, {
proofs: context.globProofs
})
if (uncertainty) {
markIncomplete(
context,
logTarget(absolutePattern),
describeGlobUncertainty(uncertainty, redactedLabel)
)
}
return matches
return { paths: matches, redactedLabel }
} catch {
// A glob that threw walked a directory it could not read; it never proved the set is empty.
markIncomplete(context, absolutePattern)
return []
markIncomplete(context, logTarget(absolutePattern), 'glob traversal failed')
return { paths: [], redactedLabel }
}
}
@@ -214,36 +282,44 @@ function resolveIncludePaths(pattern: string, context: IncludeExpansionContext):
// Include living behind an unreadable parent directory as if the user had never written it.
try {
statSync(absolutePattern)
return [absolutePattern]
return { paths: [absolutePattern], redactedLabel }
} catch (error) {
if (!isDefinitiveAbsence(error)) {
markIncomplete(context, absolutePattern)
markIncomplete(context, logTarget(absolutePattern))
}
return []
return { paths: [], redactedLabel }
}
}
function getCanonicalPath(filePath: string, context: IncludeExpansionContext): string | null {
function getCanonicalPath(
filePath: string,
context: IncludeExpansionContext,
logTarget: string
): string | null {
try {
return realpathSync.native(filePath)
} catch (error) {
if (!isDefinitiveAbsence(error)) {
markIncomplete(context, filePath)
markIncomplete(context, logTarget)
}
return null
}
}
function isReadableRegularFile(filePath: string, context: IncludeExpansionContext): boolean {
function isReadableRegularFile(
filePath: string,
context: IncludeExpansionContext,
logTarget: string
): boolean {
try {
const stats = statSync(filePath)
if (!stats.isFile()) {
console.warn(`[ssh] Skipping SSH config include "${filePath}": not a regular file`)
console.warn(`[ssh] Skipping SSH config include "${logTarget}": not a regular file`)
return false
}
if (stats.size > MAX_INCLUDE_FILE_BYTES) {
console.warn(
`[ssh] Skipping SSH config include "${filePath}": size ${stats.size} exceeds ${MAX_INCLUDE_FILE_BYTES} bytes`
`[ssh] Skipping SSH config include "${logTarget}": size ${stats.size} exceeds ${MAX_INCLUDE_FILE_BYTES} bytes`
)
context.fullyExpanded = false
return false
@@ -251,7 +327,7 @@ function isReadableRegularFile(filePath: string, context: IncludeExpansionContex
return true
} catch (error) {
if (!isDefinitiveAbsence(error)) {
markIncomplete(context, filePath)
markIncomplete(context, logTarget)
}
return false
}
@@ -3,6 +3,7 @@ import { tmpdir } from 'node:os'
import { join, posix, win32 } from 'node:path'
import { afterEach, describe, expect, it, vi } from 'vitest'
import {
createGlobReadabilityProofs,
findGlobExpansionUncertainty,
getLiteralGlobParent
} from './ssh-config-include-glob-readability'
@@ -58,18 +59,42 @@ describe('findGlobExpansionUncertainty', () => {
}
const pattern = join(root, '*', 'config')
// Large enough for the literal parent's three entries, so the glob traversal is what runs out.
expect(findGlobExpansionUncertainty(pattern, posix, 4)).toBe(pattern)
// Only the literal parent's own open fits, so the glob traversal is what runs out.
expect(findGlobExpansionUncertainty(pattern, posix, { maxPaths: 2 })).toEqual({
reason: 'unproven-within-budget',
target: pattern
})
})
it('charges directory entries to the same budget', () => {
/**
* Entries get their own budget because they are batched reads inside an already-open directory.
* Charging them against the path budget reported a readable ~/.ssh holding a few hundred keys and
* control sockets as unopenable, which permanently disabled the alias claim it feeds.
*/
it('proves a directory holding far more entries than the path budget complete', () => {
const root = mkdtempSync(join(tmpdir(), 'orca-ssh-glob-wide-'))
temporaryDirectories.push(root)
const matched = join(root, 'sub')
mkdirSync(matched)
writeFileSync(join(matched, 'config'), '')
for (let index = 0; index < 302; index += 1) {
writeFileSync(join(root, `id_key_${index}`), '')
}
expect(findGlobExpansionUncertainty(join(root, 'sub*', 'config'), posix)).toBeNull()
})
it('reports a directory past the entry budget as unproven rather than unreadable', () => {
const root = mkdtempSync(join(tmpdir(), 'orca-ssh-glob-entries-'))
temporaryDirectories.push(root)
for (const name of ['one', 'two', 'three']) {
writeFileSync(join(root, name), '')
}
expect(findGlobExpansionUncertainty(join(root, '*'), posix, 2)).toBe(root)
expect(findGlobExpansionUncertainty(join(root, '*'), posix, { maxEntries: 2 })).toEqual({
reason: 'unproven-within-budget',
target: root
})
})
it('proves a bounded readable glob complete', () => {
@@ -79,7 +104,11 @@ describe('findGlobExpansionUncertainty', () => {
mkdirSync(directory)
writeFileSync(join(directory, 'config'), '')
expect(findGlobExpansionUncertainty(join(root, '*', 'config'), posix, 32)).toBeNull()
expect(
findGlobExpansionUncertainty(join(root, '*', 'config'), posix, {
maxPaths: 32
})
).toBeNull()
})
it('keeps enumeration failures uncertain after the directory opens', () => {
@@ -89,6 +118,26 @@ describe('findGlobExpansionUncertainty', () => {
throw Object.assign(new Error('enumeration failed'), { code: 'EIO' })
})
expect(findGlobExpansionUncertainty(join(root, '*'), posix)).toBe(root)
expect(findGlobExpansionUncertainty(join(root, '*'), posix)).toEqual({
reason: 'unreadable',
target: root
})
})
it('walks a directory once per expansion, so sibling Includes share the proof', () => {
const root = mkdtempSync(join(tmpdir(), 'orca-ssh-glob-proofs-'))
temporaryDirectories.push(root)
writeFileSync(join(root, 'a-config'), '')
writeFileSync(join(root, 'b-config'), '')
const proofs = createGlobReadabilityProofs()
expect(findGlobExpansionUncertainty(join(root, 'a*'), posix, { proofs })).toBeNull()
// Any re-enumeration now fails, so a second null can only have come from the memo.
vi.spyOn(Dir.prototype, 'readSync').mockImplementation(() => {
throw Object.assign(new Error('enumeration failed'), { code: 'EIO' })
})
expect(findGlobExpansionUncertainty(join(root, 'b*'), posix, { proofs })).toBeNull()
expect(findGlobExpansionUncertainty(join(root, 'b*'), posix)).not.toBeNull()
})
})
@@ -4,9 +4,52 @@ import type { PathApi } from './ssh-config-include-path-resolution'
const GLOB_METACHARACTER = /[*?[]/
// Alias checks run on the main process, so uncertainty is safer than an unbounded proof scan.
// Paths are the expensive axis: each one is an open on a filesystem that may be a network mount.
const MAX_GLOB_READABILITY_PATHS = 256
// Entries are batched reads inside a directory that is already open, so they get their own, far
// 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
const GLOB_READABILITY_LIMIT_REACHED = Symbol('glob-readability-limit-reached')
/**
* Why a glob could not be proven complete. Both block a confident alias claim, but they are not the
* same problem and must not be reported as each other: one is a permission or I/O fault the user can
* fix, the other is a readable tree that is simply too big to walk on the main process.
*/
export type GlobUncertaintyReason = 'unreadable' | 'unproven-within-budget'
export type GlobExpansionUncertainty = {
/** The directory that could not be walked, or the pattern when its own traversal ran out. */
target: string
reason: GlobUncertaintyReason
}
/**
* Per-expansion memo of directories already walked, keyed by path, so sibling Includes under one
* parent (`Include conf.d/a*` plus `Include conf.d/b*`) enumerate that parent once.
*
* Only definitive verdicts are stored. Budget exhaustion is not one: it depends on what the calling
* scan had left, so another scan may still prove the same directory.
*/
export type GlobReadabilityProofs = Map<string, GlobExpansionUncertainty | null>
export type GlobReadabilityScanOptions = {
maxEntries?: number
maxPaths?: number
proofs?: GlobReadabilityProofs
}
type ScanBudget = {
entries: number
paths: number
proofs: GlobReadabilityProofs
}
export function createGlobReadabilityProofs(): GlobReadabilityProofs {
return new Map()
}
export function hasGlobPattern(input: string): boolean {
return GLOB_METACHARACTER.test(input)
}
@@ -20,8 +63,7 @@ export function getLiteralGlobParent(pattern: string, pathApi: PathApi): string
}
/**
* The path that prevents proving a glob complete, or `null` when every traversed directory was
* enumerated. A proof that exceeds its path budget returns the pattern itself as the uncertainty.
* What prevents proving a glob complete, or `null` when every traversed directory was enumerated.
*
* `globSync` reports what it could see and never reports what it could not: an unreadable directory
* yields fewer matches, not an error. Walk each globbed directory level so partial matches are not
@@ -30,22 +72,26 @@ export function getLiteralGlobParent(pattern: string, pathApi: PathApi): string
export function findGlobExpansionUncertainty(
pattern: string,
pathApi: PathApi,
maxPaths = MAX_GLOB_READABILITY_PATHS
): string | null {
const budget = { remaining: maxPaths }
const unopenableParent = findUnopenableDirectory(getLiteralGlobParent(pattern, pathApi), budget)
if (unopenableParent) {
return unopenableParent
options: GlobReadabilityScanOptions = {}
): GlobExpansionUncertainty | null {
const budget: ScanBudget = {
entries: options.maxEntries ?? MAX_GLOB_READABILITY_ENTRIES,
paths: options.maxPaths ?? MAX_GLOB_READABILITY_PATHS,
proofs: options.proofs ?? createGlobReadabilityProofs()
}
const unwalkableParent = findUnwalkableDirectory(getLiteralGlobParent(pattern, pathApi), budget)
if (unwalkableParent) {
return unwalkableParent
}
for (const prefix of getGlobDirectoryPrefixes(pattern, pathApi)) {
const directories = globWithinReadabilityLimit(prefix, budget)
if (directories === null) {
return pattern
return { reason: 'unproven-within-budget', target: pattern }
}
for (const directory of directories) {
const unopenable = findUnopenableDirectory(directory, budget)
if (unopenable) {
return unopenable
const unwalkable = findUnwalkableDirectory(directory, budget)
if (unwalkable) {
return unwalkable
}
}
}
@@ -53,38 +99,51 @@ export function findGlobExpansionUncertainty(
}
/** Missing directories and paths below regular files are definitive empty matches. */
function findUnopenableDirectory(directory: string, budget: { remaining: number }): string | null {
function findUnwalkableDirectory(
directory: string,
budget: ScanBudget
): GlobExpansionUncertainty | null {
if (budget.proofs.has(directory)) {
return budget.proofs.get(directory) ?? null
}
if (budget.paths <= 0) {
return { reason: 'unproven-within-budget', target: directory }
}
budget.paths -= 1
try {
// Read every entry: some network filesystems open successfully but fail during enumeration.
const handle = opendirSync(directory)
try {
while (handle.readSync() !== null) {
// Exhaust the directory so a late enumeration error cannot look like a complete glob.
budget.remaining -= 1
if (budget.remaining < 0) {
// Entries share the scan budget: an unbounded directory stays unproven, not blocking.
return directory
budget.entries -= 1
if (budget.entries < 0) {
// Readable so far, just bigger than one scan may walk: unproven, and not memoisable.
return { reason: 'unproven-within-budget', target: directory }
}
}
} finally {
handle.closeSync()
}
budget.proofs.set(directory, null)
return null
} catch (error) {
return isDefinitiveAbsence(error) ? null : directory
const verdict: GlobExpansionUncertainty | null = isDefinitiveAbsence(error)
? null
: { reason: 'unreadable', target: directory }
budget.proofs.set(directory, verdict)
return verdict
}
}
/** Stop a proof scan before a broad glob can synchronously walk an unbounded tree. */
function globWithinReadabilityLimit(
pattern: string,
budget: { remaining: number }
): string[] | null {
function globWithinReadabilityLimit(pattern: string, budget: ScanBudget): string[] | null {
try {
return globSync(pattern, {
exclude: () => {
budget.remaining -= 1
if (budget.remaining < 0) {
budget.paths -= 1
if (budget.paths < 0) {
throw GLOB_READABILITY_LIMIT_REACHED
}
return false