hook-utils: shared helpers degrade silently to permissive answers (fail-open defaults in the SSOT lib)

[kyle-sexton]

Problem

hook-utils.sh helpers degrade silently to a permissive answer when their prerequisite is missing. That default is fine for the advisory/formatting hooks the helpers were written for, and wrong for the security guards that now consume them: a guard that cannot determine the truth must not proceed as if it had.

This is fleet-wide. hook-utils.sh is SSOT-synced by scripts/sync-hook-utils.sh from lib/hook-utils.sh, so every carrying plugin inherits the behavior.

Two instances, found independently by two different agents on two different PRs. That is the reason to file the pattern rather than either instance.

Instance 1 — path helpers fall back to the unresolved input

hook::physical_path returns its input unchanged when neither realpath nor readlink -f exists:

hook::physical_path() {
  local resolved
  if resolved=$(realpath -- "$1" 2>/dev/null) || resolved=$(readlink -f -- "$1" 2>/dev/null); then
    if [[ -n "$resolved" ]]; then printf '%s' "$resolved"; return; fi
  fi
  printf '%s' "$1"        # <- lexical path, silently
}

hook::repo_root does the same, falling back to the hint when git rev-parse --show-toplevel cannot answer.

Consequence found on #1097: a guard leaning on hook::physical_path for a containment check compared the lexical path where no canonicalizer existed, so an escaping symlink read as in-repository and was pinned by its lexical path.

Consequence on #1085 (this PR): block-noncanonical-commit needed "which repository will this command run in", where an unknown answer must block. Because both helpers report success-with-a-guess, the guard cannot distinguish "resolved" from "fell back", so neither helper was usable and the guard grew its own resolution with visible failure. Declining the shared helpers was the right call, but it means the lib is now bypassed by exactly the consumers with the strictest needs.

Instance 2 — the tokenizer does not honor # as a shell comment

hook::bash_parse_segments has no # handling, so words a shell would discard survive into the parsed argv.

Concrete chain, traced on #1085 with alias.b = 'commit --allow-empty -m bypass' persisted in the current repo:

$ git -c alias.a='!git b #' a -C <other-repo>

reparse handed to the tokenizer:  git b # -C <other-repo>
tokenized argv:                   [git, b, #, -C, <other-repo>]     <- '#' kept as a word
after alias splice:               git commit --allow-empty -m bypass '#' -C <other-repo>
commit-argument scan:             sees `-C`  ->  reads it as --reuse-message  ->  exempt=1
verdict:                          exit 0 (allowed)

real git: hands the body to a shell, which drops everything from the '#',
          so git runs `git b` only and the non-canonical commit LANDS.

git appends the invocation's trailing words to a ! body, and the shell then discards them at the #. The guard keeps them, and a leftover -C grants a bogus --reuse-message exemption. Note the exemption is the fail-open: the alias is correctly resolved and the commit -m is reached, then the verdict is thrown away.

No guard-local workaround is appropriate — deciding whether a given # is quoted is the tokenizer's job, and approximating shell comment semantics inside one guard is the modelling-instead-of-asking mistake that produced several earlier bypasses in this same file.

Why one issue

Both instances are the same design decision: a shared helper answering "I could not determine this" as a usable value instead of a distinguishable failure. Fixing them one at a time invites the next consumer to hit the third instance. What is worth deciding is the lib-wide contract.

Fix direction

• Make undeterminable distinguishable. Give the affected helpers a failure signal a caller can branch on — a non-zero return, or an out-param plus empty result — so a guard can fail closed while an advisory hook keeps its current permissive behavior by ignoring it. Preserving today's behavior for existing callers matters: these are consumed fleet-wide.
• Document the contract per helper, so a future consumer does not have to read the implementation to learn that success does not mean resolved.
• hook::bash_parse_segments: honor # as a comment where a shell would, with the quoting rules the tokenizer already implements for ', ", and $'…'.
• Sequencing note: any change here forces a version bump across every carrying plugin via sync-hook-utils.sh --check-bump, so it wants to be a deliberate lib change rather than a rider on a feature PR.

Related

• guardrails: block-dangerous-git resolves lease hash-width for the wrong repository across a ! alias hop #1469 — block-dangerous-git's repo_oid_width(), another "the guard cannot see which repository this runs in" instance.
• fix(guardrails): close git-alias-chain re-expansion bypass of both git guards #1085 and fix(security): patch six Claude Security scan findings (F1–F6) #1097 — the two PRs where these were found; both declined or worked around the helpers rather than using them.

[kyle-sexton]

Independent corroboration of this pattern from #1097, reached without knowledge of this issue — which is itself a signal that the fail-open default is easy to build on unknowingly.

hook::physical_path silently returns the lexical path

hook::physical_path() {
  local resolved
  if resolved=$(realpath -- "$1" 2>/dev/null) || resolved=$(readlink -f -- "$1" 2>/dev/null); then
    if [[ -n "$resolved" ]]; then
      printf '%s' "$resolved"
      return
    fi
  fi
  printf '%s' "$1"   # <-- degrades to the input, indistinguishably
}

The caller cannot tell "canonicalized" from "gave up". There is no exit-code signal and no sentinel.

How that produced a fail-open in a security predicate

#1097 added a guard to markdown-format.sh refusing approval when a repository module reference resolves OUTSIDE the repository — a symlink aimed out of the tree, or a ../ escape — because no signature over repository content can cover it. That guard is built directly on hook::physical_path:

resolved="$(hook::physical_path "$candidate")"
case "$resolved" in
  "$CONFIG_ROOT"/*) ;;    # inside the repo — pin it
  *) RISK_UNPINNABLE=1; return 1 ;;   # escapes — refuse approval
esac

On a host with no canonicalizer, hook::physical_path returns the lexical path. An escaping symlink at $CONFIG_ROOT/rules/escaping.cjs therefore resolves to a path inside $CONFIG_ROOT, takes the first branch, and gets pinned by its lexical path — so the external target is free to change under a live approval while the signature stays byte-identical. The guard fails open on exactly the hosts where it matters most, and no test on a runner that has realpath would ever show it.

The workaround, and why it argues for fixing the lib

The fix shipped in #1097 detects the degradation at the call site rather than from the helper:

# A symlink never canonicalizes to itself, so an unchanged physical path is the
# observable signature of failed canonicalization.
if [[ -L "$candidate" && "$resolved" == "$candidate" ]]; then
  RISK_UNPINNABLE=1
  return 1
fi

That works, but note what it costs: every caller that needs a trustworthy answer has to re-derive this inference independently, and has to know to. markdown-format.sh now does it in two places (this guard and the pre-existing membership scope at lines 92–108, which had reached the same conclusion earlier for the same reason). Two call sites in one file re-implementing the same degradation probe is the argument for the signal belonging in the helper.

Suggested shape, for whatever this issue converges on

A distinguishable failure — non-zero exit, or an explicit PHYSICAL_PATH_UNRESOLVED sentinel — so a caller must choose a posture rather than inherit permissiveness by default. Security callers then fail closed explicitly; cosmetic callers (path display, telemetry subjects) can keep using the lexical fallback deliberately, which is a legitimate use and should stay available.

Worth checking hook::repo_root and hook::normalize_path for the same shape while in there, and whether any current caller of hook::physical_path makes a trust decision on its result without the -L-plus-unchanged probe.

Cross-refs: #1491 (no gate covers shell portability) is a sibling class — both are defects that are invisible on the platform CI runs and only appear where the userland differs.

[kyle-sexton]

This was generated by AI during triage.

Triage: verified — human-gated

Both instances confirmed real by reading lib/hook-utils.sh directly (not just the issue's excerpts):

• hook::physical_path (lines ~176-185): confirmed it returns "$1" unchanged, with a normal (zero) return, when neither realpath nor readlink -f resolves. No signal distinguishes "canonicalized" from "echoed input back."
• hook::repo_root (lines ~219-229): confirmed the same shape — falls back to the hint (minus a trailing /.claude) when git rev-parse --show-toplevel fails, again with a normal return.
• hook::bash_parse_segments: grepped the whole tokenizer for any #/comment handling — none exists. Every quoting branch (', ", $'…', backslash) is implemented; # is simply absent, so it survives into parsed argv exactly as the issue describes.
• Confirmed scripts/sync-hook-utils.sh exists and is the SSOT-sync mechanism the issue describes — the fleet-wide framing is accurate, not exaggerated.

Dependent-guard census (the specific ask: how many/which guards actually depend on this).

hook::bash_parse_segments — the #-comment gap — is consumed by four live security guards in plugins/guardrails/hooks/: block-noncanonical-commit.sh, block-no-verify.sh, block-dangerous-git.sh, block-convention-violation.sh. All four parse untrusted Bash command strings through this one tokenizer to produce an allow/block verdict. The issue's own repro demonstrates the bypass through block-noncanonical-commit.sh, but the missing # handling is upstream of all four — this is not a single-guard exposure, it is a currently-shipped gap in the whole guard family.

hook::physical_path / hook::repo_root are consumed far more broadly (14 carrying plugins sync the lib). Sorting the actual call sites by whether the permissive fallback is a security problem:

• Security-relevant: plugins/claude-ops/hooks/claude-ops-paths.sh's claude_ops::resolve_project_relative_dir relies on hook::physical_path resolving symlinks to stop a user-configured store destination from escaping the physical project root via a symlinked ancestor. If neither resolver is present, this containment check degrades to a lexical comparison — the same failure class as the #1097 finding this issue already cites, just in a different consumer.
• Not security-relevant (matches the issue's own framing): every formatting/advisory hook that calls hook::repo_root for REPO_ROOT (bash-format, biome-format, go-format, ruff-format, typos-format, powershell-format, markdown-format, eol-normalizer, actionlint, desktop-notification) uses it to locate config, not to gate a security decision — markdown-format.sh even carries its own comment acknowledging the degrade is intentional there. Within guardrails, skill-reference-verify.sh, cli-flag-verify.sh, and flag-commit-pr-skill-bypass.sh use hook::repo_root to locate the plugins directory / convention-pattern resolver, not as a containment gate.

Why human-gated, not autonomous-eligible. The proposed fix direction (give the two path helpers a distinguishable failure signal; teach the tokenizer #) is sound, but the concrete signal shape — non-zero return vs. empty-output-plus-return vs. an out-param — is a cross-repo contract decision for a lib synced into 14 plugins via scripts/sync-hook-utils.sh --check-bump. Getting it wrong, or silently changing behavior an advisory caller depends on, is a fleet-wide regression, not a single-file fix. That plus the live, already-shipped exposure across four security guards makes this worth a maintainer's eyes on the contract shape before an agent touches the SSOT lib.

Priority: priority: high (above the assessed default priority: medium) — significant impact: a live, exploitable gap reachable through four shipped security guards via the shared tokenizer, plus a real (if narrower) symlink-escape gap in claude-ops. Not critical: no active/observed exploit, and the guards still catch the large majority of cases the #-comment gap doesn't touch.

Role-label note: .work-item-tracker.json has no config.role_labels entry, so the canonical human-gated role resolves to its documented default (needs-human).

Agent Brief

Type: Bug
Summary: hook-utils.sh's path helpers and command tokenizer answer "I could not determine this" as if it were a resolved value, so a security-guard caller cannot fail closed on an undetermined answer.

Current behavior:

• hook::physical_path returns its input path unchanged, with a success return, when no path canonicalizer (realpath/readlink -f) is available — indistinguishable from an actual resolution.
• hook::repo_root returns its hint unchanged, with a success return, when git rev-parse --show-toplevel cannot resolve a repository root — indistinguishable from an actual resolution.
• hook::bash_parse_segments does not treat an unquoted # as a shell comment terminator, so words a real shell would discard survive into the parsed argv passed to every consumer, including four guardrails security guards.

Desired behavior:

• A caller of hook::physical_path or hook::repo_root can distinguish "successfully resolved" from "fell back to the unresolved input" without re-implementing the resolution logic itself, so a security guard can choose to fail closed on the fallback case while an advisory/formatting caller keeps ignoring the distinction and retains today's permissive behavior.
• hook::bash_parse_segments drops an unquoted # and everything after it within the current segment, honoring the same quoting rules the tokenizer already applies to ', ", and $'…'.

Key interfaces:

• hook::physical_path — needs a documented, caller-visible signal for "no resolver available / fell back," on top of its existing input/output shape.
• hook::repo_root — same distinguishability requirement for the git-toplevel-vs-hint-fallback branch.
• hook::bash_parse_segments — must treat unquoted # as a comment terminator for the rest of the current segment.

Acceptance criteria:

• A caller can tell, without string-inspecting the return value, whether hook::physical_path actually canonicalized its input.
• A caller can tell, without string-inspecting the return value, whether hook::repo_root actually resolved via git.
• Every currently-passing advisory/formatting-hook test (bash-format, markdown-format, etc.) still passes unchanged when the caller ignores the new failure signal.
• hook::bash_parse_segments given an input containing git b # -C <other-repo> produces argv that drops # and everything after it in that segment, matching real-shell comment semantics.
• scripts/sync-hook-utils.sh --check-bump is run and every carrying plugin's synced copy of hook-utils.sh is updated together with a version bump in the same change.

Out of scope:

• Migrating block-noncanonical-commit.sh / block-dangerous-git.sh to actually consume hook::physical_path / hook::repo_root for directory resolution — that's a follow-up once the lib exposes a usable fail-closed signal, not part of this fix.
• Broader shell-comment edge cases beyond a bare unquoted # (e.g., $#, arithmetic contexts), unless the existing tokenizer structure makes handling them free.

Related

• guardrails: block-dangerous-git resolves lease hash-width for the wrong repository across a ! alias hop #1469 — block-dangerous-git's wrong-repository resolution, same "shared code cannot see which repository it's really answering for" theme, verified and human-gated earlier this session.
• guardrails: block-noncanonical-commit analyzes the wrong repository when a ! alias body relocates the process (cd) #1486 — block-noncanonical-commit's !-alias-relocation gap; also human-gated this session; a different specific mechanism but the same underlying trust-boundary shape.

Outcome: labels normalized off priority: needs-triage; applying priority: high + needs-human. area: security retained. Type was already native Bug.

[kyle-sexton]

Claimed by cursor/fix-1487-hook-utils-fail-closed-2ae1 — PR incoming for distinguishable failure signals on path helpers and # comment handling in bash_parse_segments.

[kyle-sexton]

Frontier disposition (drain session)

Ruling: Classified for drain orchestration.

• hook-utils: shared helpers degrade silently to permissive answers (fail-open defaults in the SSOT lib) #1487 → IMPLEMENT_NOW (in flight as PR fix(hook-utils): distinguish unresolved helpers and honor # comments #2549) — dual frontier AGREED
• guardrails: block-noncanonical-commit resolves the repository at the outer invocation when the nested one determines it #1500 / guardrails: alias_launch_dir misses a nested alias when show-prefix is empty under locating globals (live bypass on main) #1553 → CLASS_C design-carrier cluster (path-composition vs alias-lookup chokepoint); host/design owner required before autonomous patch

Human gate cleared for classification; implementation authority follows the ruling.