fix(claude-ops): resolve default marketplace across version skew; generalize CRLF gotcha; correct install_new render doc (F1-F3)

[kyle-sexton]

Closes #1176.

Consumer-audit fixes for the claude-ops plugins sync skill (handoff-inbox item 20260723-155027), re-verified against current source (0.19.0). Version 0.19.0 → 0.19.1.

Changes

• F1 (bug). fleet-state.sh's resolve_default_marketplace exact-matched the running plugin root against the version-pinned installPath, so any skew between the session's loaded version and the installed one — a marketplace autoUpdate after session start, or sync's own Step-3 self-update of claude-ops — broke the primary no---marketplace path with "could not resolve the default marketplace". Added a version-agnostic fallback matching the version-stripped …/cache/<marketplace>/<plugin> prefix (exact match still tried first; the marketplace stays distinguishable via the path), plus a clearer error that prints the searched root and names the skew cause.
• F2a (doc). Generalized context/gotchas.md's CRLF entry from "this host's jq build" to any captured Windows value (python print, PowerShell interop, git config, CRLF file read): a trailing \r on a constructed claude plugin id makes the CLI report Plugin "<name>" not found with the full id passed (marketplace suffix corrupted), colliding with the bare-name symptom — documented and cross-referenced both ways.
• F3a (doc). plugins/SKILL.md now reads an unset ${user_config.install_new} — which renders the literal placeholder, since the manifest default is not substituted for an unset key (verified vs CC 2.1.218) — as the expected unset state → default ask, not an invalid value; only an explicitly-set unsupported value is flagged.

Verification

• bash plugins/claude-ops/skills/plugins/scripts/fleet-state.test.sh → 28 cases, 0 failed (new version-skew case green).
• shellcheck -x -S warning fleet-state.sh → clean.
• scripts/check-changed-skills.sh origin/main → PASS for the plugins skill (runs the fleet-state test).

Deferred (noted on #1176, not in this PR)

• F2b: ship a sync-sweep.sh sibling owning the Step 2+3 update loop (CR-stripping, fail-loud, structured per-id JSON) so sync.md shrinks to "run this, report from its output". Bigger net-new surface with its own test.
• F3b: verify upstream whether a declared userConfig default is intended to substitute for an unset key; file a CC docs issue if docs and behavior disagree. F3a is correct regardless.

Related

• Handoff-inbox item 20260723-155027-claude-ops-plugins-sync-audit (producer: KyleSexton workstation, CC 2.1.218).
• Prior inbox items landed this session: docs(bash-format): document project scope + setup gotchas; add opt-in/shfmt-fallback test coverage (F1, F2, F6) #1171 (bash-format), docs(hook-observability): refresh statusMessage adoption (30/31) + define dim-9; cover buffer_stdin timeout & EPOCHREALTIME skip (F4, F3, F2-shared) #1172 (hook-observability + hook-utils).

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 2m 17s —— View job

---

Security Review — PR #1177

• Load task context
• Fetch PR diff
• Read changed shell scripts (fleet-state.sh, fleet-state.test.sh)
• Read changed docs/manifests
• Analyze for injection surfaces, path traversal, secret handling, and logic flaws
• Post findings

---

Summary

No security issues found in the changes introduced by this PR.

Reviewed all six changed files with focus on the shell code changes in fleet-state.sh and the new test case in fleet-state.test.sh. Details of what was examined:

---

fleet-state.sh — resolve_default_marketplace() refactor (F1)

The primary code change extracts norm_root computation from inside the function into the caller and adds a two-stage matching path. Security properties hold:

• Stage 1 & 2 jq queries both pass norm_root/norm_root_parent via --arg, never via string interpolation. All external values in jq invocations across this function (and the wider script) are properly parameterized with --arg/--argjson. No injection surface here.

• sed 's/.*@//' processes jq's output of .key (a marketplace plugin ID from CC-internal installed_plugins.json). This strips through the last @ to yield the marketplace name. The result is later passed to jq as --arg n "$name" — no shell evaluation, no injection.

• Catalog fixture path: "$FLEET_STATE_CATALOG_DIR/$name.json" concatenates a marketplace name into a file path. The marketplace name originates from CC-internal files (installed_plugins.json) or the --marketplace CLI argument. This is a pre-existing pattern unchanged by this PR; and FLEET_STATE_CATALOG_DIR is explicitly documented as test-only.

• norm_root_parent computation uses pure bash parameter expansion (${norm_root%/*}) on a value derived from hook::normalize_path "$(hook::physical_path "$plugin_root")" where plugin_root is CLAUDE_PLUGIN_ROOT (a CC-controlled env var). No external process, no injection.

• Stage 2 jq sub("/[^/]*$";"") operation runs entirely within jq on installPath values from CC-internal files — no shell execution path.

• The existing jq() { command jq "$@" | tr -d '\r'; } CRLF-sanitizing wrapper continues to cover all jq call sites in Stage 2 the same as Stage 1.

Broader security patterns verified (unchanged but confirmed still in place)

• SCRIPT_DIR and HOOK_UTILS resolved via ${BASH_SOURCE[0]%/*} + builtin cd/pwd, not dirname — documented defense against PATH-hijacked dirname binaries (the comment correctly scopes this defense's limits at BASH_FUNC_builtin%% and BASH_ENV).
• source "$HOOK_UTILS" via builtin source — bypasses an inherited exported source function.
• All jq invocations use --arg/--argjson for external data; none embed shell variables directly in jq filter strings.

fleet-state.test.sh — new version-skew test case

Test constructs deterministic file paths under a temp directory, uses jq -cn --arg root "$native_installed" (properly parameterized) to build the fixture JSON, and captures script output for assertion. No eval, no injection surface, no unguarded variable interpolation in the new block.

Documentation changes (SKILL.md, gotchas.md, CHANGELOG.md, plugin.json)

No executable code. No secrets, credentials, or sensitive values introduced.

---

Confidence: HIGH — the changed code paths are short and straightforward; all data flows were traced from source to jq invocation. No CRITICAL or IMPORTANT findings.

[claude]

Claude finished @kyle-sexton's task in 3m 37s —— View job

---

PR Review — fix(claude-ops): resolve default marketplace across version skew; generalize CRLF gotcha; correct install_new render doc (F1-F3)

• Gather context
• Read changed files
• Review fleet-state.sh (F1 — version-agnostic fallback)
• Review fleet-state.test.sh (new test cases)
• Review context/gotchas.md (F2a — CRLF generalization)
• Review skills/plugins/SKILL.md (F3a — install_new render doc)
• Review plugin.json and CHANGELOG.md
• Synthesize findings and post review

---

Overview

Solid, well-scoped patch. All three fixes are correct, the refactor is clean, and the new test case exercises the exact failure path being fixed. A few minor observations below.

---

F1 — fleet-state.sh version-agnostic fallback (fleet-state.sh:218–241)

Correctness: OK.

The two-stage design is sound:

• Stage 1 preserves the prior exact match as the fast path.
• Stage 2 strips only the trailing /<version> segment (via sub("/[^/]*$";"")) — the remaining …/cache/<marketplace>/<plugin> prefix is unique per plugin-within-marketplace, so the match can't cross-contaminate two plugins from the same marketplace or two marketplaces shipping the same plugin name.

One pattern worth noting:

# line 234
[[ -n "$norm_root_parent" && "$norm_root_parent" != "$norm_root" ]] || return 0

The guard bails with return 0 (empty stdout) when norm_root_parent is empty or unchanged (e.g., if the plugin root is / or a bare filename). The caller checks [[ -z "$TARGET" ]], so this correctly surfaces as "could not resolve." No bug, but the return 0 is semantically "could not resolve" — a return 1 would make the intent more explicit. That said, the caller's pattern is the right one for the job; either exit code works given how TARGET is checked.

The refactor of moving CLAUDE_PLUGIN_ROOT resolution out of the function and into the caller is the right call — it lets the caller reuse $norm_root in the error message without threading it back through a subshell. No other callers exist in the file.

Stage 2's jq sub behavior on paths with no /: If installPath has no slash, sub("/[^/]*$";"") returns the string unchanged. This means it could false-match if norm_root_parent also happened to equal that raw value. In practice installPath is always an absolute path, so this is only a theoretical concern on severely malformed state — the existing schema validation would already have bailed. Not a bug.

---

F1 — fleet-state.test.sh new case (fleet-state.test.sh:457–475)

Correctness: OK.

The test uses real directories so hook::physical_path (i.e., realpath) resolves both paths consistently. The installed version path (0.18.3) and session path (0.19.0) are distinct sibling directories under a shared parent, which exactly models the mid-session autoUpdate scenario. The run_state function merges 2>&1, so both stdout and stderr feed into the assertions correctly.

The cygpath fallback for non-Windows is correct — the POSIX path is passed through unchanged, which is what installed_plugins.json would contain on Linux CI.

One gap: the test doesn't assert that Stage 1 was bypassed (it can't — Stage 1 is an internal detail). But the fixture structure makes Stage 1's failure axiomatic (installed = 0.18.3, running = 0.19.0 → exact match cannot succeed), so the observable assertion on marketplace.name is the right contract test.

Also good: the pre-existing assert_contains "default marketplace: error names the searched root" "$out" "nowhere-installed" assertion (added to the prior unresolvable-root case) directly validates the improved error message. This is a new assertion added in this PR to an existing test case, which is easy to miss in a diff scan.

---

F2a — context/gotchas.md (gotchas.md:17–21)

Correctness: OK.

The cross-reference anchor:

[Captured values on Windows carry `\r`](#captured-values-on-windows-carry-r--strip-it-before-embedding-in-any-command-or-json)

The heading text ## Captured values on Windows carry \\r` — strip it before embedding in any command or JSONgenerates the anchor#captured-values-on-windows-carry-r--strip-it-before-embedding-in-any-command-or-jsonunder GitHub's rules (backticks stripped, backslash stripped, em dash stripped, double space from em-dash removal →--`). The link is correct.

The reframing from "this host's jq build" to "any captured value" is accurate and the concrete 57/58-failures example is a strong motivator for the generalization. The two failure modes (jq --argjson and claude plugin id corruption) are clearly differentiated.

---

F3a — skills/plugins/SKILL.md (SKILL.md:128–144)

Correctness: OK.

The fix is accurate and the verification note ("verified 2026-07-23 against CC 2.1.218") is the right thing to include so a future reader knows this is empirically grounded, not assumed. The behavior gap (manifest default not substituted for an unset key) is confirmed in-session and properly deferred to F3b for an upstream docs inquiry.

The instruction to the model — "read the literal placeholder as the expected unset state → use the default ask, and do NOT report it as an invalid value" — is precise and directly actionable. This is the right kind of skill-doc language.

---

plugin.json and CHANGELOG.md

Version bump 0.19.0 → 0.19.1: correct for a bug-fix-plus-doc patch release.

CHANGELOG: follows the existing project convention of omitting release dates from version headers (consistent with ## [0.19.0] and earlier entries). The Fixed/Changed split accurately reflects the change types.

---

Summary

No blocking issues. The PR is ready to merge.

The one optional suggestion: in resolve_default_marketplace, consider changing return 0 to return 1 on the guard line (L234) to make "could not resolve" semantically explicit — but this is a nit, not a correctness concern given the caller's [[ -z "$TARGET" ]] check.