363 Commits

  • fix(hooks): cover brace groups + yarn-run/bun-bare dev variants
    Two false-negatives surfaced in PR #1889 review:
    
    1. Brace-group bypass (Greptile).
       `{ npm run dev; }` evaluates the dev command in the *current*
       shell — semantically distinct from `( ... )` but with the same
       effect for this hook. `splitShellSegments` correctly cleaves the
       group at `;` into `["{ npm run dev", "}"]`, but the first segment's
       leading token under `readToken` is the bare `{`, which was not in
       `DEV_COMMAND_WORDS`, so the dev-pattern check was skipped.
    
       Fix: treat `{` and `}` as no-op tokens in `getLeadingCommandWord`
       so we keep walking to the real command word. Matches how shell
       itself parses brace groups (the braces are reserved words, not
       commands). Bash requires a space after `{` and a terminator before
       `}` for an actual group, so `{npm run dev}` correctly remains
       allowed (single token `{npm`, not in `DEV_COMMAND_WORDS`).
    
    2. Missing yarn-run / bun-bare variants (CodeRabbit).
       Both `yarn dev` *and* `yarn run dev` are valid (the latter is what
       `package.json` actually wires `dev` to under yarn 1.x). The same
       `(run )?` symmetry applies to bun. The previous `DEV_PATTERN` only
       matched `yarn\s+dev` and `bun\s+run\s+dev`, allowing the cross
       forms to pass through silently.
    
       Fix: `yarn(?:\s+run)?\s+dev` and `bun(?:\s+run)?\s+dev` — same
       shape `pnpm(?:\s+run)?\s+dev` was already using.
    
    Verified after this commit (every form now exits 2):
    
      { npm run dev; }
      { npm run dev ; }
      echo hi && { npm run dev; }
      ({ npm run dev; })
      $( { npm run dev; } )
      yarn run dev
      bun dev
    
    Verified still allowed (no regression):
    
      echo "{ npm run dev; }"   # literal inside double quotes
      {npm run dev}             # not a brace group per bash syntax
  • fix(lib): track quote state inside command-substitution depth counters
    Greptile flagged a bypass in PR #1889: `$(echo ")"; (npm run dev))`
    threaded the depth-counting loops in `extractCommandSubstitutions`
    and `extractSubshellGroups` to terminate early, because a literal `)`
    inside double quotes was treated as a real closing paren. The
    truncated body then ended in a dangling `"` that toggled `inDouble`
    in the outer scan, masking the subsequent `(npm run dev)` group from
    extraction.
    
    Reproduced (before this commit) by piping the synthetic PreToolUse
    payload `{"tool_input":{"command":"$(echo \")\"; (npm run dev))"}}`
    into `scripts/hooks/pre-bash-dev-server-block.js` and observing
    exit 0 (allow) where the dev pattern is clearly present.
    
    Fix: each `$(...)` and `(...)` body loop now tracks its own
    single/double quote state and only treats `(` / `)` as depth
    delimiters when outside quotes. The quoted `)` no longer closes
    the group early, the body now extends to the real closing paren,
    and the outer scan's quote state remains untouched.
    
    After this commit:
      $ echo '{"tool_input":{"command":"$(echo \")\"; (npm run dev))"}}' \
          | node scripts/hooks/pre-bash-dev-server-block.js; echo $?
      2
    
    The symmetric form `$(echo "(npm run dev)")` correctly remains
    allowed (bash does not honor `(...)` inside double quotes).
  • fix(hooks): close subshell bypass in pre-bash-dev-server-block
    Before this commit the dev-server-block hook ran the leading-command
    and dev-pattern check only against the top-level segments returned by
    `splitShellSegments`, which doesn't split on `$(...)`, backticks, or
    plain `(...)`. That left the policy bypassable by wrapping a dev
    command in any of those constructs:
    
      $(npm run dev)
      `npm run dev`
      echo $(npm run dev)
      (npm run dev)
    
    Each verified by piping a synthetic PreToolUse payload into the hook
    on this branch: every form above returned exit 0 (allow) where a plain
    `npm run dev` correctly returned exit 2 (block).
    
    Fix: expand the check space before running the leading-command rule.
    A small BFS walks the raw command, harvesting bodies from
    `extractCommandSubstitutions` (`$(...)` and backticks) and from
    `extractSubshellGroups` (plain `(...)`), then splits each harvested
    body through `splitShellSegments` and feeds the result into the
    existing `isBlockedDevSegment` check.
    
    This preserves every existing allow case (`tmux new-session -d -s dev
    "npm run dev"`, quoted-string mentions like `git commit -m "npm run
    dev fix"`, `echo hi`) because the leading-command rule is unchanged —
    only the set of segments it runs against grew.
    
    Known limitation, not fixed here: `eval "$(echo npm run dev)"` still
    slips through because the substitution body's leading command is
    `echo`, and statically modeling echo's output to recover the executed
    command is out of scope. The same class affects `gateguard-fact-force`
    (via `eval "$(echo rm -rf /)"` etc.) and is best addressed in both
    hooks together as a follow-up rather than as a one-off here.
  • feat(lib): add extractSubshellGroups for plain (...) subshells
    `extractCommandSubstitutions` only walks `$(...)` and backticks — the two
    shell constructs whose bodies are captured as strings. Bash also has
    plain `(...)` subshells (e.g. `(npm run dev)`), where the body executes
    in a child shell but is not value-captured. Our PreToolUse hooks need
    to peer inside those too, because a `(...)` group bypasses the
    top-level segment splitter just like `$(...)` does.
    
    This commit adds a sibling extractor with the same conventions as
    `extractCommandSubstitutions`:
    
    - single quotes literal — `'(npm run dev)'` is a string, ignored
    - double quotes literal for parens — `"(npm run dev)"` is a string
      (bash only honors `$(...)`, not bare `(...)`, inside double quotes)
    - skips `$(...)` and backtick spans so we don't double-extract
      bodies the other helper already handles
    - recurses into its own bodies for nested groups
    
    No consumer yet; the next commit wires both extractors into
    `scripts/hooks/pre-bash-dev-server-block.js` to close the subshell
    bypass surface.
  • feat(lib): extract shell command-substitution parser to shared lib
    Extract the `extractCommandSubstitutions` function originally
    introduced in scripts/hooks/gateguard-fact-force.js (PR #1853
    round 2) into scripts/lib/shell-substitution.js so other PreToolUse
    hooks can reuse the same single-quote-aware, double-quote-aware,
    nested-subshell-aware parser without duplicating it.
    
    No behavior change in this commit — the function body is copied
    verbatim and exposed via `module.exports`. The next commit wires it
    into scripts/hooks/pre-bash-dev-server-block.js to close that hook's
    own subshell-bypass holes.
    
    gateguard-fact-force.js still defines its own private copy of the
    function; consolidating both call sites onto this shared lib is a
    follow-up worth doing once this PR lands, but is intentionally out
    of scope here to keep the diff focused on the dev-server-block fix.
  • ci: gate observability on release safety evidence
    Add release-safety evidence coverage to observability readiness and refresh rc.1 publication gate docs.
  • docs: gate ECC progress sync readiness
    Make the ECC 2.0 GitHub/Linear/handoff/roadmap progress-sync model part of the local observability readiness gate instead of leaving it as roadmap prose only.
    
    - add `docs/architecture/progress-sync-contract.md` for GitHub, Linear, handoff, roadmap, and work-items sync
    - add a `Tracker Sync` check to `scripts/observability-readiness.js`
    - update observability tests with passing and missing-contract coverage
    - update observability and GA roadmap docs so the local readiness gate is now 18/18 and records #1848 supply-chain hardening evidence
    
    Validation:
    - node tests/scripts/observability-readiness.test.js (9 passed, 0 failed)
    - npm run observability:ready -- --format json (18/18, ready true)
    - npx markdownlint-cli 'docs/architecture/progress-sync-contract.md' 'docs/architecture/observability-readiness.md' 'docs/ECC-2.0-GA-ROADMAP.md'
    - git diff --check
    - node tests/docs/ecc2-release-surface.test.js (18 passed)
    - node tests/run-all.js (2378 passed, 0 failed)
    - GitHub CI for #1849 green across Ubuntu, Windows, and macOS
    
    No release, tag, npm publish, plugin tag, marketplace submission, or announcement was performed.
  • docs: add supply-chain incident response playbook
    Add a repo-level supply-chain incident response playbook for npm/GitHub Actions package-registry incidents, anchored on the May 2026 TanStack compromise and prior Shai-Hulud-style npm incidents.
    
    - add `docs/security/supply-chain-incident-response.md` with exposure checks, immediate response steps, workflow rules, publication rules, and escalation triggers
    - link the playbook from `SECURITY.md`
    - reject `pull_request_target` workflows that restore or save shared dependency caches
    - add a regression test for the new `pull_request_target + actions/cache` guardrail
    
    Validation:
    - node tests/ci/validate-workflow-security.test.js (12 passed, 0 failed)
    - node scripts/ci/validate-workflow-security.js (validated 7 workflow files)
    - npx markdownlint-cli 'SECURITY.md' 'docs/security/supply-chain-incident-response.md'
    - npx markdownlint-cli '**/*.md' --ignore node_modules
    - git diff --check
    - node tests/run-all.js (2377 passed, 0 failed)
    - GitHub CI for #1848 green across Ubuntu, Windows, and macOS
    
    No release, tag, npm publish, plugin tag, marketplace submission, or announcement was performed.
  • ci: require npm audit signature checks
    Require npm registry signature verification wherever workflow npm audit checks run.
    
    - add npm audit signatures to CI Security Scan and maintenance security audit jobs
    - teach the workflow security validator to reject npm audit without signature verification
    - keep the repair and Copilot prompt tests portable across Windows path/case and CRLF frontmatter behavior
    
    Validation:
    - node tests/run-all.js (2376 passed, 0 failed)
    - CI current-head matrix green on #1846
  • feat: add GitHub Copilot prompt support
    Adds GitHub Copilot VS Code instruction and prompt files for ECC workflows, with VS Code prompt frontmatter/settings aligned to current docs and tests covering the surface.
    
    Co-authored-by: Girish Kanjiyani <girish.kanjiyani5040@gmail.com>
    Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
  • fix: close block-no-verify bypass holes
    Backport Jamkris's fix for case-insensitive core.hooksPath overrides and the git commit -tn template-path false positive. Verified locally on current main with 25/25 block-no-verify tests and node tests/run-all.js passing 2369/2369.
  • ci: harden workflow install boundaries
    - run non-test workflow installs with npm ci --ignore-scripts where lifecycle scripts are not needed\n- reject plain npm ci in workflows with write permissions\n- reject actions/cache in id-token: write workflows to reduce OIDC publish cache-poisoning risk
  • docs: add data-backed harness adapter scorecard (#1785)
    * docs: add data-backed harness adapter scorecard
    
    * fix: normalize adapter matrix line endings
    
    * test: avoid doubled CRLF simulation
  • feat: add ECC statusline observability hooks
    Salvages the useful statusline/context monitor work from stale PR #1504 while preserving the current continuous-learning hook runner wiring.
    
    Adds the metrics bridge, context monitor, statusline script, shared cost/session bridge utilities, and tests. Fixes the reviewed false loop-detection hash collision for non-file tools, avoids default-session cost inflation, sanitizes statusline task lookup, and records hook payload session IDs in cost-tracker.
  • docs: salvage focused stale PR contributions
    - add Vite and Redis pattern skills from closed stale PRs
    
    - add frontend-slides support assets
    
    - port skill-comply runner fixes and LLM prompt/provider regressions
    
    - harden agent frontmatter validation and sync catalog counts
  • fix: port continuous-learning observer fixes
    Ports continuous-learning observer signal, storage, remote normalization, and v1 deprecation fixes onto current main.
  • fix: harden CI validators
    Ports personal-path validator hardening and quoted checkout detection onto current main.
  • fix: port hook session and dashboard safety fixes
    Ports suggest-compact session_id isolation and dashboard terminal/document launch safety onto current main.
  • fix: sync skill frontmatter and catalog counts
    Adds missing skill frontmatter, normalizes strict YAML metadata, syncs README catalog counts, and extends catalog validation for README/plugin/marketplace count drift.
  • fix(ci): flag SKILL.md frontmatter defects in validate-skills (#1669)
    * fix(ci): flag SKILL.md frontmatter defects in validate-skills
    
    Issue #1663 reported two SKILL.md frontmatter defects (missing `name:`
    on skill-stocktake; literal block-scalar `description: |-` on
    openclaw-persona-forge) that PR #1664 addresses at the data level.
    
    This change is complementary: it extends `scripts/ci/validate-skills.js`
    to catch the same class of defect statically going forward, so the
    frontmatter-vs-renderer problems do not silently reappear as new skills
    land.
    
    ## Checks added
    - Frontmatter must declare a `name:` field.
    - Frontmatter `description:` must not use a literal block scalar
      (`|` / `|-` / `|+`) — these preserve internal newlines and break
      flat-table renderers keyed off `description`. Folded (`>`) and inline
      strings are accepted.
    
    ## Behavior
    - Frontmatter findings default to WARN (exit 0) so this PR does not
      break CI while the two known offenders are still on main. Pass
      `--strict` or set `CI_STRICT_SKILLS=1` to promote them to ERROR
      (exit 1). Structural findings (missing / empty SKILL.md) remain
      errors as before.
    - Today against main, the validator reports exactly two warnings —
      the same two files called out in #1663 — and exits 0. When #1664
      lands, the validator reports zero warnings, at which point strict
      mode can be enabled in CI.
    
    ## Parser notes
    - Bespoke frontmatter parser mirrors the style of `validate-agents.js`
      (tolerant of UTF-8 BOM and CRLF; no new npm dependency).
    - Block-scalar continuation lines are skipped so keys inside a block
      scalar are not mistaken for top-level keys.
    - Hidden directories (`.something/`) under skills/ are now skipped.
    
    ## Tests
    Adds five focused tests to `tests/ci/validators.test.js`:
    - warns when frontmatter is missing `name` (default mode)
    - errors when frontmatter is missing `name` (--strict mode)
    - warns on literal block-scalar description (|-)
    - accepts folded (>) and inline descriptions under --strict
    - skips hidden directories under skills/
    
    ## Docs
    Adds two bullets to the `Skill Checklist` in CONTRIBUTING.md covering
    the two rules now surfaced by the validator.
    
    Refs #1663. Complements (does not compete with) #1664.
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    
    * fix(ci): harden SKILL.md frontmatter checks after bot review
    
    Address findings from CodeRabbit, Greptile, and cubic on #1669:
    
    - Guard empty or whitespace-only `name:` values. Previously
      `name:    ` silently passed because the presence check only
      tested key-set membership; now inspectFrontmatter captures
      trimmed values and validate flags an explicit 'name is empty'
      WARN/ERROR.
    - Broaden block-scalar detection to cover YAML 1.2 indent
      indicators (`|2`, `|-2`, `>2-`) and trailing comments
      (`|-  # note`). The old regex required a bare `|`/`>` with
      optional `+`/`-`, which let valid-but-disallowed forms slip
      through.
    - Update CONTRIBUTING.md checklist to list `|+` alongside `|`
      and `|-` for parity with the validator.
    - Extend runSkillsValidator to accept env overrides and add four
      regression tests: empty name, |+ description, |-2 + comment, and
      CI_STRICT_SKILLS=1.
    
    * fix(ci): address round-2 review on validate-skills frontmatter
    
    - Tighten extractFrontmatter closing delimiter to require a newline or
      end-of-file after the closing `---`, so body lines beginning with
      `---text` are not parsed as frontmatter (CodeRabbit).
    - Strip both trailing and comment-only values in inspectFrontmatter, so
      `name: # todo` is surfaced as empty rather than silently passing
      (cubic P2).
    - Extract validateSkillDir helper so the per-directory validation
      block moves out of validateSkills, keeping both functions under the
      50-line guideline (CodeRabbit nit).
    - Hoist runSkillsValidator to module scope in the test harness and
      share the spawnSync import with execFileSync so the helper stops
      re-requiring child_process on every invocation (CodeRabbit nit).
    - Add regression tests: comment-only `name:` values must fail strict
      mode; `---trailing` body lines must not be parsed as frontmatter.
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    
    * Update tests/ci/validators.test.js
    
    Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
    
    ---------
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
  • fix(hooks): resolve MCP health-check spawn ENOENT on Windows (#1456)
    * fix(hooks): resolve MCP health-check spawn ENOENT on Windows
    
    On Windows, commands like 'npx' are batch files (npx.cmd) that require
    shell expansion to resolve via PATH. Without shell: true, Node.js
    spawn() fails with ENOENT.
    
    However, absolute paths (e.g. C:\Program Files\nodejs\node.exe) must
    NOT use shell mode because cmd.exe misparses paths containing spaces.
    
    Fix: enable shell mode only for non-absolute commands on Windows, using
    path.isAbsolute() to distinguish. This matches how attemptReconnect()
    already handles the shell option.
    
    Fixes #1455
    
    * fix(hooks): harden Windows shell spawn — validate command for metacharacters
    
    Addresses bot review feedback on PR #1456:
    
    - Add UNSAFE_SHELL_CHARS regex to guard against shell injection when
      needsShell=true: cmd.exe operators (&, |, <, >, ^, %, !, (), ;,
      whitespace) are rejected before shell mode is enabled
    - Add typeof command === 'string' check so path.isAbsolute() cannot
      throw on malformed non-string command values
    - Rename test to 'via PATH resolution' (not Windows-only; runs all platforms)
    - Fix misleading test comment: 'node' resolves via PATH like npx.cmd but
      does not itself use .cmd; comment now accurately reflects the intent
    
    * fix(hooks): kill full process tree on Windows when shell mode is used
    
    When needsShell=true, the spawned child is cmd.exe. Calling child.kill()
    only terminates the shell, leaving the real server process orphaned.
    
    Use taskkill /PID <pid> /T /F on Windows+shell to kill the entire
    process tree rooted at cmd.exe. Fall back to SIGTERM+SIGKILL on all
    other platforms or when shell mode is not active.
    
    * fix(hooks): fall back to child.kill() when taskkill fails
    
    Windows taskkill can fail if it's not on PATH, the process already
    exited, or permissions are denied. Previously the failure was silently
    ignored and no kill signal reached the child.
    
    Now: capture the spawnSync result and fall back to child.kill('SIGKILL')
    on any taskkill error or non-zero status. This still may leak a
    detached server process but at least guarantees the cmd.exe shell is
    signaled.