Replies: 2 comments
|
Excellent report, and the canary methodology on part 2 is the right way to prove it. Your PowerShell finding is the runtime half of a wider class in that bridge, "a config that loads, runs, and enforces nothing". I measured the config half against the same source and there are two more surfaces, one of which is fully silent. Silent. Events outside the supported seven are never read.
const CLAUDE_EVENTS = [
'SessionStart', 'UserPromptSubmit', 'PreToolUse', 'PostToolUse',
'Stop', 'SubagentStart', 'SubagentStop',
] as const
for (const event of CLAUDE_EVENTS) {
const rawGroups = hooksMap[event]
...
}So a Not silent, credit where due. Non-command hook types do warn, Silent in a different way. A matcher can be valid and still never select anything.
if (mode === 'claude-code' && CLAUDE_LITERAL.test(pattern)) return undefined
return compileRegex(pattern) === undefined ? `invalid ${mode} regex matcher ...` : undefined
Taken together with your finding, there are four ways to end up with an enforcing hook that enforces nothing, and only one of them warns:
On your suggestion, documenting the trailing Disclosure, I ran into these building a Claude Code to DSH migration tool, so it now names dead hooks before you move them. The measured notes live at https://github.com/sjh9714/dsh-movein/blob/main/docs/compat.md if useful. Your Windows canary is the piece I did not have, thanks for posting it. |
|
第 2 条的 canary 方法论确实是证明这类问题的正确做法(@sjh9714 说得对)。我想把这一条的诉求形状往前推一步——因为按现在的提法修完,同一个洞还会以别的形式回来。 "补一个
|
Uh oh!
There was an error while loading. Please reload this page.
Two Windows issues found while evaluating
dshon Windows 11, both reproduced on47f94385with Node 24.19.0. Posting here sinceCONTRIBUTING.mdasks for Discussions rather than PRs.1.
dsh plugin addsplits a package path that contains a spaceapps/cli/src/plugin.tscallsspawnSync('pnpm', args, { shell: process.platform === 'win32' }). Withshell: trueand CMD, Node setswindowsVerbatimArguments, so the argv is concatenated into one unquoted command line and cmd re-tokenizes it.installs two dependencies,
Repoand a brokenlink:Clones\lhh010\dsh-minigames, then prints the misleadingRepo declares no dsh.bundlewarning. Staging the package at a space-free path works around it. Relative specifiers also resolve against the caller's cwd rather than the profile directory, which surprised me separately.One fix that passes here removes
shell: trueand spawns the command processor directly, so Node applies its own quoting:with
runPlugincallingspawnSync(command, pnpmArgs, { cwd: dir, stdio: 'inherit' })and noshelloption. A four-case regression test on that builder passes locally; happy to paste it if useful.2. Hook bridges fail open on Windows, silently, for any hook that invokes an interpreter
packages/hooks/hooks-claude-coderuns a hook'scommandthrough the host shell. On Windows that is PowerShell, which does not adopt a native child's exit code as its own process exit code:Since a non-2 exit is classified as a non-blocking error, a
PreToolUsedeny hook is skipped and nothing reports it. Verified by canary in the headless profile: the identical hook blocks with the suffix and does not block without it, so a user can have a hook config that loads, runs, and enforces nothing.A bash-idiom hook fails harder, because PowerShell parses the whole command before running any of it. With
echo MARKER > file; echo x >&2; exit 2, the marker file was never written:>&2is not valid PowerShell, so the leading write never executed either.Suggestion: either run hook commands through a shell that propagates child exit codes, or document the trailing
; exit $LASTEXITCODErequirement in both bridge READMEs. The Codex bridge README already listscommandWindowsas ignored, which is a related trap for anyone bringing an existing config over.Happy to provide full patches or the canary scripts if that would help.
All reactions