fix(dev-infra): harden worktree hook ownership

This commit is contained in:
Tianyi Cui
2026-07-27 22:54:44 +08:00
parent 81b56a0d97
commit f7729b6f53
9 changed files with 342 additions and 47 deletions

View File

@@ -7,12 +7,15 @@ import { dirname, isAbsolute, join, resolve } from 'node:path'
const MINIMUM_GIT = [2, 26, 0]
const HOOKS_DIRECTORY = 'dsh-hooks'
const OWNERSHIP_MARKER = '.dsh-lefthook-owned'
const OWNERSHIP_MARKER_CONTENT = 'deepseek-harness worktree-local lefthook hooks\n'
const LEGACY_OWNERSHIP_MARKER_CONTENT = 'deepseek-harness worktree-local lefthook hooks\n'
const OWNERSHIP_MARKER_VERSION = 1
const OWNERSHIP_MARKER_OWNER = 'deepseek-harness worktree-local lefthook hooks'
const INSTALL_LOCK = 'dsh-lefthook-install.lock'
const INSTALL_LOCK_TIMEOUT_MS = 30_000
const INSTALL_LOCK_POLL_MS = 50
const ALLOW_HOOKS_PATH_OVERRIDE = 'DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE'
const CONDITIONAL_INCLUDE_PATTERN = '^includeif\\..*\\.path$'
const REPOSITORY_EXTENSION_PATTERN = '^extensions\\.'
function errorCode(error) {
return typeof error === 'object' && error !== null && 'code' in error
@@ -177,17 +180,37 @@ function registeredWorktreeConfigPaths(commonDirectory) {
return paths
}
function assertDormantWorktreeConfigs(root, commonDirectory, commonConfigPath, currentConfigPath) {
if (worktreeConfigExtensionEnabled(root, commonConfigPath)) return
function lstatIfPresent(path) {
try {
return lstatSync(path)
} catch (error) {
if (errorCode(error) === 'ENOENT') return undefined
throw error
}
}
function assertCommonConfigFile(commonConfigPath) {
const configStat = lstatIfPresent(commonConfigPath)
if (configStat === undefined || !configStat.isFile() || configStat.isSymbolicLink()) {
throw new Error(
`refusing common repository config ${JSON.stringify(commonConfigPath)} because it is not a regular file`,
)
}
}
function assertWorktreeConfigFiles(root, commonDirectory, commonConfigPath, currentConfigPath) {
const extensionEnabled = worktreeConfigExtensionEnabled(root, commonConfigPath)
for (const configPath of registeredWorktreeConfigPaths(commonDirectory)) {
if (!existsSync(configPath)) continue
const configStat = lstatSync(configPath)
const configStat = lstatIfPresent(configPath)
if (configStat === undefined) continue
if (!configStat.isFile() || configStat.isSymbolicLink()) {
const state = extensionEnabled ? 'active' : 'dormant'
throw new Error(
`cannot enable extensions.worktreeConfig while dormant worktree config ${JSON.stringify(configPath)} `
+ 'is not a regular file; inspect it and enable the extension explicitly, or remove it, before retrying',
`refusing ${state} worktree config ${JSON.stringify(configPath)} because it is not a regular file; `
+ 'replace it with a regular worktree config or remove it before retrying',
)
}
if (extensionEnabled) continue
if (!hasDirectConfigEntries(root, configPath)) continue
const isCurrent = normalizedPath(configPath) === normalizedPath(currentConfigPath)
const owner = isCurrent ? 'current' : 'sibling'
@@ -259,7 +282,13 @@ function conditionalIncludeRisk(root, entry, inspect) {
return inspectConditionalConfig(root, target, inspect)
}
function migrationConfigSubject(root, configPath) {
function migrationConfigSubject(root, configPath, rejectRepositoryExtensions) {
if (rejectRepositoryExtensions) {
const extensionEntry = fileConfigMatchingEntries(root, configPath, REPOSITORY_EXTENSION_PATTERN)[0]
if (extensionEntry !== undefined) {
return `${extensionEntry.name} (${configSource(extensionEntry)})`
}
}
const worktreeEntry = fileConfigEntries(root, configPath, 'core.worktree')[0]
if (worktreeEntry !== undefined) return `core.worktree (${configSource(worktreeEntry)})`
const trueBareEntry = fileConfigEntries(root, configPath, 'core.bare')
@@ -272,7 +301,7 @@ function hooksPathConfigSubject(root, configPath) {
return entry === undefined ? undefined : `core.hooksPath (${configSource(entry)})`
}
function ensureWorktreeConfig(root, commonConfigPath) {
function planWorktreeConfigMigration(root, commonConfigPath) {
const versions = fileConfigValues(root, commonConfigPath, 'core.repositoryFormatVersion')
const versionText = assertSingle(versions, 'core.repositoryFormatVersion')
const version = Number(versionText)
@@ -280,6 +309,21 @@ function ensureWorktreeConfig(root, commonConfigPath) {
throw new Error(`unsupported core.repositoryFormatVersion: ${JSON.stringify(versionText)}`)
}
if (version === 0) {
const extensionEntry = fileConfigMatchingEntries(
root,
commonConfigPath,
REPOSITORY_EXTENSION_PATTERN,
)[0]
if (extensionEntry !== undefined) {
throw new Error(
`cannot upgrade core.repositoryFormatVersion from 0 while dormant repository extension `
+ `${extensionEntry.name} is configured (${configSource(extensionEntry)}); `
+ 'audit and migrate it, then set repository format 1 explicitly before retrying',
)
}
}
const extensionEnabled = worktreeConfigExtensionEnabled(root, commonConfigPath)
if (!extensionEnabled) {
@@ -287,7 +331,7 @@ function ensureWorktreeConfig(root, commonConfigPath) {
const risk = conditionalIncludeRisk(
root,
entry,
configPath => migrationConfigSubject(root, configPath),
configPath => migrationConfigSubject(root, configPath, version === 0),
)
if (risk !== undefined) {
const reason = risk.subject ?? risk.detail
@@ -318,6 +362,11 @@ function ensureWorktreeConfig(root, commonConfigPath) {
const directBareText = assertSingle(fileConfigValues(root, commonConfigPath, 'core.bare'), 'core.bare')
const directBare = directBareText === undefined ? undefined : parseGitBoolean(directBareText, 'core.bare')
return { directBare, extensionEnabled, version }
}
function applyWorktreeConfigMigration(root, commonConfigPath, migration) {
const { directBare, extensionEnabled, version } = migration
if (version === 0) {
git(['config', '--file', commonConfigPath, 'core.repositoryFormatVersion', '1'], root)
}
@@ -430,13 +479,38 @@ async function acquireInstallLock(commonDirectory) {
}
}
function ensureOwnedHooksDirectory(hooksPath) {
const markerPath = join(hooksPath, OWNERSHIP_MARKER)
if (!existsSync(hooksPath)) {
mkdirSync(hooksPath, { mode: 0o700 })
writeFileSync(markerPath, OWNERSHIP_MARKER_CONTENT, { flag: 'wx', mode: 0o600 })
return
function ownershipMarkerContent(hooksPath) {
return `${JSON.stringify({
version: OWNERSHIP_MARKER_VERSION,
owner: OWNERSHIP_MARKER_OWNER,
hooksPath,
})}\n`
}
function parseOwnershipMarker(content, hooksPath) {
if (content === LEGACY_OWNERSHIP_MARKER_CONTENT) return { hooksPath, legacy: true }
let parsed
try {
parsed = JSON.parse(content)
} catch {
return undefined
}
if (
typeof parsed !== 'object'
|| parsed === null
|| parsed.version !== OWNERSHIP_MARKER_VERSION
|| parsed.owner !== OWNERSHIP_MARKER_OWNER
|| typeof parsed.hooksPath !== 'string'
|| !isAbsolute(parsed.hooksPath)
) {
return undefined
}
return { hooksPath: parsed.hooksPath, legacy: false }
}
function inspectOwnedHooksDirectory(hooksPath) {
const markerPath = join(hooksPath, OWNERSHIP_MARKER)
if (!existsSync(hooksPath)) return undefined
const hooksStat = lstatSync(hooksPath)
if (!hooksStat.isDirectory() || hooksStat.isSymbolicLink()) {
throw new Error(`refusing to use non-directory or symlinked hooks path ${hooksPath}`)
@@ -445,9 +519,36 @@ function ensureOwnedHooksDirectory(hooksPath) {
throw new Error(`refusing to overwrite unowned hooks directory ${hooksPath}`)
}
const markerStat = lstatSync(markerPath)
if (!markerStat.isFile() || markerStat.isSymbolicLink() || readFileSync(markerPath, 'utf8') !== OWNERSHIP_MARKER_CONTENT) {
const marker = markerStat.isFile() && !markerStat.isSymbolicLink() && markerStat.nlink === 1
? parseOwnershipMarker(readFileSync(markerPath, 'utf8'), hooksPath)
: undefined
if (marker === undefined) {
throw new Error(`refusing to overwrite hooks directory with an invalid ownership marker: ${hooksPath}`)
}
for (const name of readdirSync(hooksPath)) {
if (name === OWNERSHIP_MARKER) continue
const entryPath = join(hooksPath, name)
const entryStat = lstatSync(entryPath)
if (!entryStat.isFile() || entryStat.isSymbolicLink() || entryStat.nlink !== 1) {
throw new Error(
`refusing to overwrite non-regular or multiply linked hook entry ${JSON.stringify(entryPath)}`,
)
}
}
return { markerPath, ...marker }
}
function ensureOwnedHooksDirectory(hooksPath) {
const inspected = inspectOwnedHooksDirectory(hooksPath)
if (inspected !== undefined) return inspected
mkdirSync(hooksPath, { mode: 0o700 })
const markerPath = join(hooksPath, OWNERSHIP_MARKER)
writeFileSync(markerPath, ownershipMarkerContent(hooksPath), { flag: 'wx', mode: 0o600 })
return { markerPath, hooksPath, legacy: false }
}
function updateOwnershipMarker(markerPath, hooksPath) {
writeFileSync(markerPath, ownershipMarkerContent(hooksPath), { mode: 0o600 })
}
function environmentWithoutCommandGitConfig() {
@@ -588,6 +689,13 @@ async function main() {
let installationError
try {
assertCommonConfigFile(commonConfigPath)
assertWorktreeConfigFiles(
root,
commonDirectory,
commonConfigPath,
worktreeConfigPath,
)
const worktreeEntries = fileConfigEntries(root, worktreeConfigPath, 'core.hooksPath')
const includedWorktreeEntry = worktreeEntries.find(
entry => !originIsFile(entry.origin, root, worktreeConfigPath),
@@ -599,14 +707,20 @@ async function main() {
worktreeEntries.map(entry => entry.value),
'worktree core.hooksPath',
)
let ownedHooksDirectory
if (worktreePath !== undefined && worktreePath !== hooksPath) {
refuseScopedHooksPath({ origin: `file:${worktreeConfigPath}`, scope: 'worktree', value: worktreePath })
ownedHooksDirectory = inspectOwnedHooksDirectory(hooksPath)
if (ownedHooksDirectory === undefined || ownedHooksDirectory.hooksPath !== worktreePath) {
refuseScopedHooksPath({ origin: `file:${worktreeConfigPath}`, scope: 'worktree', value: worktreePath })
}
}
const directWorktreePathIsOwned = worktreePath !== undefined
&& (worktreePath === hooksPath || ownedHooksDirectory?.hooksPath === worktreePath)
const effectiveEntry = effectiveConfigEntry(root, 'core.hooksPath')
if (effectiveEntry !== undefined) {
const effectivePathIsOwned = effectiveEntry.scope === 'worktree'
&& effectiveEntry.value === hooksPath
&& worktreePath === hooksPath
&& effectiveEntry.value === worktreePath
&& directWorktreePathIsOwned
&& originIsFile(effectiveEntry.origin, root, worktreeConfigPath)
if (!effectivePathIsOwned) {
if (effectiveEntry.scope === 'command' || effectiveEntry.scope === 'worktree') {
@@ -622,19 +736,21 @@ async function main() {
}
assertConditionalHooksPaths(root, worktreeConfigPath)
assertDormantWorktreeConfigs(
root,
commonDirectory,
commonConfigPath,
worktreeConfigPath,
)
ensureOwnedHooksDirectory(hooksPath)
ensureWorktreeConfig(root, commonConfigPath)
const migration = planWorktreeConfigMigration(root, commonConfigPath)
ownedHooksDirectory = ensureOwnedHooksDirectory(hooksPath)
if (
worktreePath !== undefined
&& worktreePath !== hooksPath
&& ownedHooksDirectory.hooksPath !== worktreePath
) {
throw new Error(`hooks directory ownership changed while relocating ${JSON.stringify(worktreePath)}`)
}
applyWorktreeConfigMigration(root, commonConfigPath, migration)
let pathChanged = false
try {
git(['config', '--worktree', 'core.hooksPath', hooksPath], root)
pathChanged = worktreePath === undefined
pathChanged = worktreePath !== hooksPath
const installedEntry = effectiveConfigEntry(root, 'core.hooksPath')
if (
installedEntry === undefined
@@ -645,9 +761,22 @@ async function main() {
throw new Error('new worktree-local core.hooksPath did not become the effective direct worktree value')
}
runLefthook(root, lefthook)
updateOwnershipMarker(ownedHooksDirectory.markerPath, hooksPath)
} catch (error) {
if (pathChanged) {
git(['config', '--worktree', '--unset-all', 'core.hooksPath'], root)
try {
if (worktreePath === undefined) {
git(['config', '--worktree', '--unset-all', 'core.hooksPath'], root)
} else {
git(['config', '--worktree', 'core.hooksPath', worktreePath], root)
}
} catch (rollbackError) {
throw new AggregateError(
[error, rollbackError],
`Lefthook installation failed: ${String(error)}; `
+ `worktree hook rollback also failed: ${String(rollbackError)}`,
)
}
}
throw error
}

View File

@@ -2,10 +2,14 @@ import { spawn, spawnSync } from 'node:child_process'
import {
chmodSync,
existsSync,
linkSync,
mkdirSync,
mkdtempSync,
lstatSync,
readFileSync,
renameSync,
rmSync,
symlinkSync,
writeFileSync,
} from 'node:fs'
import { tmpdir } from 'node:os'
@@ -91,6 +95,10 @@ if (!shouldFail) {
for (const name of ['pre-commit', 'pre-push']) writeFileSync(join(hooksPath, name), hook, { mode: 0o755 })
}
if (existsSync(running)) unlinkSync(running)
if (process.env.DSH_TEST_LEFTHOOK_BREAK_WORKTREE_CONFIG === '1') {
const configPath = execFileSync('git', ['rev-parse', '--git-path', 'config.worktree'], { encoding: 'utf8' }).trim()
writeFileSync(configPath, '[invalid\\n')
}
if (shouldFail) process.exit(77)
`
}
@@ -275,6 +283,125 @@ describe('worktree-local Lefthook installer', () => {
expect(existsSync(join(hooksPath(fixture, fixture.main), '.fake-lefthook-running'))).toBe(false)
})
it('repairs its owned absolute hook path after the checkout moves', async () => {
const fixture = createFixture()
const oldRoot = fixture.main
const first = await runInstaller(fixture, oldRoot)
expect(first.status, first.stderr).toBe(0)
const oldHooks = hooksPath(fixture, oldRoot)
const movedRoot = join(fixture.container, 'moved-main')
renameSync(oldRoot, movedRoot)
const moved = await runInstaller(fixture, movedRoot)
expect(moved.status, moved.stderr).toBe(0)
const movedHooks = hooksPath(fixture, movedRoot)
expect(movedHooks).not.toBe(oldHooks)
expect(git(fixture, movedRoot, ['config', '--worktree', '--get', 'core.hooksPath'])).toBe(movedHooks)
const canonicalMoved = git(fixture, movedRoot, ['rev-parse', '--show-toplevel'])
expect(readFileSync(join(movedHooks, 'pre-commit'), 'utf8')).toContain(`# root=${canonicalMoved}`)
expect(readFileSync(join(movedHooks, '.dsh-lefthook-owned'), 'utf8')).toContain(
JSON.stringify(movedHooks),
)
})
it.skipIf(process.platform === 'win32')('refuses a multiply linked ownership marker before relocation rewrites it', async () => {
const fixture = createFixture()
const oldRoot = fixture.main
const first = await runInstaller(fixture, oldRoot)
expect(first.status, first.stderr).toBe(0)
const oldHooks = hooksPath(fixture, oldRoot)
const markerName = '.dsh-lefthook-owned'
const externalMarker = join(fixture.container, 'external-marker')
linkSync(join(oldHooks, markerName), externalMarker)
const externalContent = readFileSync(externalMarker, 'utf8')
const movedRoot = join(fixture.container, 'moved-main')
renameSync(oldRoot, movedRoot)
const result = await runInstaller(fixture, movedRoot)
expect(result.status).toBe(1)
expect(result.stderr).toContain('invalid ownership marker')
expect(readFileSync(externalMarker, 'utf8')).toBe(externalContent)
})
it.skipIf(process.platform === 'win32')('refuses aliased generated hooks before Lefthook can overwrite their targets', async () => {
for (const kind of ['symlink', 'hardlink'] as const) {
const fixture = createFixture()
const first = await runInstaller(fixture, fixture.main)
expect(first.status, first.stderr).toBe(0)
const hook = join(hooksPath(fixture, fixture.main), 'pre-commit')
const externalHook = join(fixture.container, `${kind}-external-hook`)
rmSync(hook)
write(externalHook, `external ${kind} target\n`)
if (kind === 'symlink') symlinkSync(externalHook, hook)
else linkSync(externalHook, hook)
const externalContent = readFileSync(externalHook, 'utf8')
const result = await runInstaller(fixture, fixture.main)
expect(result.status).toBe(1)
expect(result.stderr).toContain('non-regular or multiply linked hook entry')
expect(readFileSync(externalHook, 'utf8')).toBe(externalContent)
}
})
it('restores the marker-backed stale hook path when relocation reinstall fails', async () => {
const fixture = createFixture()
const oldRoot = fixture.main
const first = await runInstaller(fixture, oldRoot)
expect(first.status, first.stderr).toBe(0)
const oldHooks = hooksPath(fixture, oldRoot)
const markerName = '.dsh-lefthook-owned'
const previousMarker = readFileSync(join(oldHooks, markerName), 'utf8')
const movedRoot = join(fixture.container, 'moved-main')
renameSync(oldRoot, movedRoot)
const failed = await runInstaller(fixture, movedRoot, { DSH_TEST_LEFTHOOK_FAIL: '1' })
expect(failed.status).toBe(1)
expect(failed.stderr).toContain('exit status 77')
const movedHooks = hooksPath(fixture, movedRoot)
expect(git(fixture, movedRoot, ['config', '--worktree', '--get', 'core.hooksPath'])).toBe(oldHooks)
expect(readFileSync(join(movedHooks, markerName), 'utf8')).toBe(previousMarker)
})
it('refuses dormant repository extensions before upgrading the repository format', async () => {
const fixture = createFixture()
const commonConfig = join(commonDirectory(fixture), 'config')
git(fixture, fixture.main, ['config', 'extensions.dshUnknown', 'true'])
expect(gitResult(fixture, fixture.main, ['status', '--porcelain']).status).toBe(0)
const result = await runInstaller(fixture, fixture.main)
expect(result.status).toBe(1)
expect(result.stderr).toContain('dormant repository extension extensions.dshunknown')
expect(git(fixture, fixture.main, [
'config', '--file', commonConfig, '--get', 'core.repositoryFormatVersion',
])).toBe('0')
expect(gitResult(fixture, fixture.main, ['config', '--get', 'extensions.worktreeConfig']).status).toBe(1)
expect(gitResult(fixture, fixture.main, ['status', '--porcelain']).status).toBe(0)
expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false)
})
it.skipIf(process.platform === 'win32')('refuses a symlinked common repository config before writing through it', async () => {
const fixture = createFixture()
const commonConfig = join(commonDirectory(fixture), 'config')
const externalConfig = join(fixture.container, 'external-common.gitconfig')
renameSync(commonConfig, externalConfig)
symlinkSync(externalConfig, commonConfig)
const externalContent = readFileSync(externalConfig, 'utf8')
const result = await runInstaller(fixture, fixture.main)
expect(result.status).toBe(1)
expect(result.stderr).toContain('common repository config')
expect(result.stderr).toContain('not a regular file')
expect(lstatSync(commonConfig).isSymbolicLink()).toBe(true)
expect(readFileSync(externalConfig, 'utf8')).toBe(externalContent)
expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false)
})
it('leaves stale installer locks for explicit recovery', async () => {
const fixture = createFixture()
const lockPath = installLockPath(fixture)
@@ -387,9 +514,33 @@ describe('worktree-local Lefthook installer', () => {
expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false)
})
it.skipIf(process.platform === 'win32')('refuses an active symlinked worktree config before writing through it', async () => {
const fixture = createFixture()
const commonConfig = join(commonDirectory(fixture), 'config')
const worktreeConfig = join(gitDirectory(fixture, fixture.main), 'config.worktree')
const externalConfig = join(fixture.container, 'external.gitconfig')
const externalContent = '[user]\n\tname = External owner\n'
write(externalConfig, externalContent)
git(fixture, fixture.main, ['config', '--file', commonConfig, 'core.repositoryFormatVersion', '1'])
git(fixture, fixture.main, ['config', '--file', commonConfig, 'extensions.worktreeConfig', 'true'])
symlinkSync(externalConfig, worktreeConfig)
const result = await runInstaller(fixture, fixture.main)
expect(result.status).toBe(1)
expect(result.stderr).toContain('active worktree config')
expect(result.stderr).toContain('not a regular file')
expect(lstatSync(worktreeConfig).isSymbolicLink()).toBe(true)
expect(readFileSync(externalConfig, 'utf8')).toBe(externalContent)
expect(gitResult(fixture, fixture.main, [
'config', '--file', externalConfig, '--get', 'core.hooksPath',
]).status).toBe(1)
expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false)
})
it('refuses migration keys loaded through active or conditional common-config includes', async () => {
for (const includeKey of ['include.path', 'includeIf.onbranch:conditional.path']) {
for (const key of ['core.worktree', 'core.bare']) {
for (const key of ['core.worktree', 'core.bare', 'extensions.dshunknown']) {
const fixture = createFixture()
const commonConfig = join(commonDirectory(fixture), 'config')
const includedConfig = join(fixture.container, `${includeKey.split('.')[0]}-${key.replace('.', '-')}.gitconfig`)
@@ -597,6 +748,21 @@ describe('worktree-local Lefthook installer', () => {
expect(readFileSync(legacyHook, 'utf8')).toBe('#!/bin/sh\n# legacy pre-push\n')
})
it('reports installation and hook-path rollback failures together', async () => {
const fixture = createFixture()
const result = await runInstaller(fixture, fixture.main, {
DSH_TEST_LEFTHOOK_BREAK_WORKTREE_CONFIG: '1',
DSH_TEST_LEFTHOOK_FAIL: '1',
})
expect(result.status).toBe(1)
expect(result.stderr).toContain('Lefthook installation failed')
expect(result.stderr).toContain('exit status 77')
expect(result.stderr).toContain('worktree hook rollback also failed')
expect(result.stderr).toContain('git config --worktree --unset-all core.hooksPath failed')
})
it('refuses an unowned directory at the reserved worktree hook path', async () => {
const fixture = createFixture()
const reservedHook = join(hooksPath(fixture, fixture.main), 'pre-commit')

File diff suppressed because one or more lines are too long