Skip to content

fix(hook): tokenized risky-akm-command assessor — eliminate false positives on prose mentions#67

Merged
itlackey merged 2 commits into
release/0.8.0from
fix/risky-command-tokenization
May 23, 2026
Merged

fix(hook): tokenized risky-akm-command assessor — eliminate false positives on prose mentions#67
itlackey merged 2 commits into
release/0.8.0from
fix/risky-command-tokenization

Conversation

@itlackey

Copy link
Copy Markdown
Owner

Summary

The Claude plugin's PreToolUse Bash hook used regex against the raw bash command string (/\bakm\s+vault\s+(create|set|unset|load)\b/ and similar). That regex fires on any occurrence of the phrase in argv — including commit messages, release notes, and heredoc bodies — because the hook input is the entire concatenated command. The OpenCode plugin, by contrast, already tokenized the command and matched argv positions; it never had false positives.

This PR ports the OpenCode approach into shared/ so both plugins use the same tokenized assessor.

Why this matters

The bug was caught in a real workflow: a gh release create --notes "...the vault-set legacy form is now hardened..." was blocked because the prose mention in the notes body tripped the regex. Hyphenating phrases as a workaround was the only escape hatch. That doesn't scale.

The proof point: even the commit message for this PR (which describes the bug using subcommand phrases verbatim) was initially blocked by the existing hook in the running session. After rephrasing, the commit went through — but the next contributor writing a CHANGELOG line about an akm subcommand would hit the same wall. The fix is structural, not cosmetic.

What changed

File Change
shared/risky-command.ts New. splitArguments() quote-aware tokenizer (promoted from opencode/index.ts), assessRiskyAkmCommand() tokenized assessor (covers accept/reject, push-publish, remove, all vault subcommands, gated config keys, mass-update, upgrade, trust-registration), blockedCommandMessage() formatter. Allowlists the documented eval-form vault loader pattern.
claude/shared/risky-command.ts New re-export shim — matches the existing pattern for claude/shared/{memory-candidates,memory-events,ref-extraction}.ts.
claude/hooks/akm-hook.ts assessRiskyClaudeCommand now delegates to the shared tokenized assessor. The content-aware akm remember secret scan stays plugin-side (it needs the redactor, not just a command pattern).
opencode/index.ts Deletes the local assessRiskyAkmCommand, blockedCommandMessage, RiskyCommandAssessment type, and splitArguments. Imports them from the shared module instead. Behaviour unchanged — the OpenCode plugin already did tokenized matching; this just deduplicates.
claude/README.md New ### Permission and approval policy section documenting the opt-in native permissions.ask block users can paste into ~/.claude/settings.json if they prefer Claude Code's built-in approve/deny dialog over the hook stderr. Notes which checks stay plugin-side because the native matcher can't express them (flag-conditional rules, content-aware secret scanning).

Path tried but not possible: ship default permissions in plugin.json

Confirmed via the claude-code-guide: Claude Code plugins cannot ship default permissions rules. There is no plugin-level mechanism. Users must configure permissions in their own settings.json. README docs are the supported path.

The Bash(pattern:*) matcher is prefix-anchored on the actual bash invocation; it does not match substring occurrences in argv. That is why README-documented native permissions are a viable complement to the hook — once users opt in, they get false-positive-free prompting for the simple prefix-matchable subcommands.

Test plan

  • bun test tests/claude-plugin.test.ts52 pass / 0 fail
  • bun test tests/opencode-plugin.test.ts145 pass / 0 fail
  • One pre-existing failure in tests/opencode-eval-harness.test.ts confirmed unrelated by running against baseline (fails identically without this branch's changes)
  • Live behavior: blocked-message text now includes the formatted reason + approval guidance (existing test for "explicit user approval" phrase still passes)
  • Manual smoke after merge: confirm Claude plugin no longer trips on a gh release create --notes containing risky-subcommand phrases in the body
  • Manual smoke: confirm a real vault-mutation invocation still blocks as expected

🤖 Generated with Claude Code

itlackey and others added 2 commits May 23, 2026 02:16
Replace the Claude plugin's regex-based assessRiskyClaudeCommand
(which scanned the raw bash command string and false-positived on any
prose mention of risky akm subcommands appearing inside commit
messages, release notes, or heredoc bodies) with a tokenized assessor
shared with the OpenCode plugin.

The new shared/risky-command.ts:
  - splitArguments(): quote-aware shell tokenizer (already existed in
    opencode/index.ts; promoted to shared)
  - assessRiskyAkmCommand(): finds the actual akm argv token, then
    checks subcommand positions. Only fires on real shell invocations
  - blockedCommandMessage(): common formatter
  - allowlists the documented eval-form vault loader pattern

Both plugins now import from shared/risky-command.ts. The OpenCode
plugin behaviour is unchanged (it already did tokenized matching). The
Claude plugin no longer fires on prose mentions in argv.

Tests: Claude 52/52, OpenCode 145/145. One pre-existing failure in
opencode-eval-harness is unrelated to this change (verified by running
against the baseline).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude Code plugins cannot ship default permission rules in
plugin.json (the field is not supported). Users who prefer Claude
Code's built-in approve/deny dialog over the plugin hook's stderr
block can opt in by pasting a permissions.ask block into their own
~/.claude/settings.json or .claude/settings.json.

Document the recommended block (covering accept/reject, remove,
upgrade, vault subcommands), explain the matcher's prefix-anchored
semantics, and note which checks remain plugin-side because they
cannot be expressed as a Bash() prefix pattern (content-aware
remember secret scan; flag-conditional rules like save with --push).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@itlackey itlackey merged commit 190a9bb into release/0.8.0 May 23, 2026
1 of 2 checks passed
@itlackey itlackey deleted the fix/risky-command-tokenization branch May 23, 2026 07:24
itlackey added a commit that referenced this pull request May 23, 2026
Merges the ref-extraction & live-stash validation work from main's
b9bc8b6 + 0ce71cf into release/0.8.0, replacing the prior minimal
`extractAkmRefsFromString` re-export with a real per-branch resolver
copy and a sidecar-driven validation pipeline.

Background
----------
The post-tool hook used to embed `- ref: <type>:<slug>` lines directly
into the per-session buffer for every ref-shaped substring it found in
a Bash command body. When the buffer was flushed into a durable session
memory by `akm remember`, the literal strings inside heredocs, grep
patterns, jq queries, and JSON payloads were indistinguishable from
real refs, and `akm lint` flagged them all as `missing-ref`. The next
session capture regenerated the same flags - a permanent lint
treadmill.

Architecture
------------
- Producer (recordPostTool) is now permissive: it accumulates ref
  candidates in a per-session `<sid>.refs.jsonl` sidecar without
  injecting `- ref:` lines into the body.
- Consumer (captureMemory) reads the sidecar, validates each candidate
  by checking whether the referenced asset resolves in the local stash
  (mirroring akm-core's `refExistsInAnyStash` so the hook avoids
  spawning a subprocess per ref), sorts/dedupes the survivors, and
  emits them as a `refs:` YAML array inside the session-checkpoint
  inner frontmatter.
- Anything that fails to resolve is silently dropped, so literal
  strings can no longer pollute the durable memory's ref list. The
  paired akm-core change teaches `akm lint` to treat the `refs:`
  frontmatter array as authoritative for captured memories.

Files changed
-------------
- `shared/ref-extraction.ts` - full resolver (extractAllRefs +
  validateRefCandidates + validateLiveRefs). Preserves the legacy
  `extractAkmRefsFromString` API for the strict whitespace-tokenized
  callers in `claude/hooks/akm-hook.ts` (pretoolNonBash) and
  `opencode/index.ts`. Includes the CONTRACT header pinning behavior
  against the akm-core sister copy.
- `claude/shared/ref-extraction.ts` - runtime-shipped twin (was a
  re-export stub on release/0.8.0). Now a real copy so the
  contract test can drive both implementations through identical
  fixtures, and so the published Claude plugin doesn't break by
  losing the runtime file.
- `claude/hooks/akm-hook.ts` -
  - imports `extractAllRefs, validateRefCandidates`
  - adds `appendSessionRefCandidates`, `readSessionRefCandidates`,
    `buildRefsFrontmatterBlock`, `resolveStashRoots`
  - `extractPostToolFields` now passes raw text to `extractAllRefs`
    (no sanitize() pre-filtering) so candidates are collected from any
    sub-string position
  - `recordPostTool` writes one buffer entry per call (command only)
    and appends candidates to the sidecar
  - `captureMemory` reads the sidecar, validates against the live
    stash, and emits a `refs:` block in the YAML frontmatter only
    when at least one candidate survives validation. Sidecar is
    cleaned up on session-end and on early-exit paths alongside the
    existing buffer cleanup.
- `tests/ref-extraction.test.ts` - new, ports b9bc8b6's ref-extraction
  unit suite (10 cases covering extractAllRefs, validateRefCandidates,
  validateLiveRefs).
- `tests/ref-resolver-contract.test.ts` - new, ports 0ce71cf's
  21-case contract fixture that drives BOTH in-repo resolver copies
  (shared/ and claude/shared/) through identical inputs. Mirrors the
  akm-core repo's `tests/contracts/ref-resolver-contract.test.ts`.
- `tests/claude-plugin.test.ts` - adds three integration tests:
  validated-refs-frontmatter, omit-refs-when-empty, and
  no-refs-in-buffer-body.

What was kept from release/0.8.0
--------------------------------
- The richer captureMemory pipeline (checkpoint mode, evidence
  aggregation, `formatEvidenceSummary`, related events/candidates,
  source paths, `extractCandidatesFromText` with targetRefHints,
  auto-signal feedback) is preserved verbatim - the b9bc8b6 changes
  layer on top of it instead of replacing it.
- The tokenized `assessRiskyAkmCommand` from
  `shared/risky-command.ts` (PR #67) remains the canonical risky-command
  assessor; main's e58ad5d regex anchor fix is out of scope here.
- The strict whitespace-tokenized `extractAkmRefsFromString` API
  remains exported for `pretoolNonBash` and the OpenCode plugin.

Migration: captures written under the old code keep their `missing-ref`
flags - a follow-up sweep can backfill them. Captures written by the
updated hook will be clean from the next session onward.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant