feat(shellscan): AST-based shell pre-flight classifier for the pre-tool hook

[peycheff-com]

Summary

Replaces the evadable substring needle list in core/cmd/helm-ai-kernel/hook_cmd.go with a structural bash AST classifier (new core/pkg/shellscan) as advisory input to the existing signed decision path. Verdict authority is unchanged — workstation.Decide still produces the permit/receipt verdict, fail-closed; the classifier only decides whether a shell command reaches that path.

Evidence base: research/opencode-study/03-core-tools.md (opencode's tree-sitter shell scan + arity dictionary) and research/opencode-study/25-helm-map-agent-side.md (A1: needle list trivially evadable — rm -r -f, xargs rm, find -delete, variable indirection; A2: arity table; guidance to keep the needle list as a fallback layer).

What shipped

• AST parse of every command via mvdan.cc/sh/v3 (pure Go, zero cgo; chosen over tree-sitter Go bindings to keep the TCB small and the build cgo-free; only new transitive deps are golang.org/x/sys/golang.org/x/term, already in the module graph). Walks all command nodes including pipelines, subshells, and command substitutions.
• Arity-aware command prefixes (git checkout, npm run dev, docker compose up) via a Go port of opencode's arity dictionary — groundwork for permit scoping (A2).
• Evasion closure: split/reordered/long flags (rm -r -f, rm --recursive --force, flags after operands), sudo/doas/env/nice/nohup/stdbuf/setsid/busybox/xargs wrapper unwrapping, recursive sh -c/bash -c/eval payload classification (depth-capped), base64/xxd/openssl decode-to-shell pipelines, path obfuscation (/bin/./rm), find -delete / find -exec rm -rf, and write-redirects to sensitive targets (>> .env) that bypassed Write-tool path checks.
• Fail-closed on the opaque: unparseable input, dynamic command words ($(echo rm) -rf /), dynamic flags/operands on destructive families, and dynamic eval/sh -c payloads all route to the decision path.
• Zero regression by construction: the legacy needle list is kept verbatim inside shellscan as a fallback OR-layer; AST detection is strictly additive.

Test evidence

• go test ./pkg/shellscan/ -count=1 → ok — 52 decide cases (12 needle-parity + 40 evasion/fail-closed, ≥20 evasion required), 26 benign allowlist-regression cases, arity prefix table tests, signal/Via-chain tests.
• go test ./cmd/helm-ai-kernel/ -run 'TestHookPreTool' -count=1 → ok — all pre-existing hook golden tests pass unchanged; new TestHookPreToolDeniesEvasiveBashViaASTClassifier (10 evasive commands → deny + one signed receipt each) and TestHookPreToolStillAllowsBenignBashAfterASTClassifier (6 benign commands → silent passthrough, no receipts).
• go test ./pkg/shellscan/ ./cmd/helm-ai-kernel/ -count=1 → ok (full CLI package suite incl. golden tests).
• go build ./... and go vet ./pkg/shellscan/ ./cmd/helm-ai-kernel/ → clean; gofmt -l → clean.

Risk notes

• Deliberate tightening: previously-passing opaque commands (rm $f, bash -c $PAYLOAD, unparseable input) now reach the decision path and are denied under the default profile (OPERATE_PERMISSIONS_EMPTY). This is the fail-closed intent, but operators with existing permits may see new decision receipts.
• PowerShell is not parsed (opencode scans both); bash/POSIX only. bash script.sh (static script file) remains passthrough-with-signal; script contents are opaque — noted for follow-up.
• Classifier is classification input to policy, not a sandbox boundary (research V3); sandbox runner stays the load-bearing control.
• No new network calls, no credential handling changes, no permit/receipt semantic changes.

Preflight (self-applied per .claude/skills/helm-pr-preflight)

• Changed files enumerated: 4 modified + 3 new, all under core/ (hook + new shellscan package + go.mod/go.sum). No unrelated dirty state.
• Gates run: go vet ✅, gofmt ✅, focused go tests ✅. Docs/contracts gates N/A (no docs/proto changes). Skill mirrors N/A.
• Specialist routing recommendation: kernel-guardian review advised (workstation hook classification path) — not executed in this runtime.

Gates

This PR still requires the repo's normal source-owned deterministic gates, distinct-provider 2-of-2 permit, and exact-head approval-only App interlock. It must not be merged on the strength of this PR body, reviews, or labels.

Linear: proposed key HELM-W1 (proposal only; Linear MCP unavailable in this runtime).

[strix-security]

Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here.

[Copilot]

Pull request overview

Adds a new AST-based shell command classifier (core/pkg/shellscan) and wires it into the pre-tool hook so Bash commands are structurally scanned (mvdan shell parser) before deciding whether to route them through the existing signed workstation decision (permit/receipt) path.

Changes:

• Introduces shellscan.Classify to parse shell into an AST, unwrap common wrappers, detect destructive patterns, and emit advisory reasons/signals.
• Adds an arity-aware prefix dictionary to classify command “meaning” beyond raw string needles.
• Updates the pre-tool hook to use the classifier for Bash, and adds regression + evasion tests for both shellscan and the hook.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
core/pkg/shellscan/shellscan.go	New AST-based classifier (wrappers, destructive matchers, signals, legacy-needle fallback).
core/pkg/shellscan/arity.go	Arity dictionary + prefix derivation for human-meaningful command prefixes.
core/pkg/shellscan/shellscan_test.go	Unit tests for decision/pass cases, signals, wrapper chains, and prefix behavior.
core/cmd/helm-ai-kernel/hook_cmd.go	Hooks Bash pre-tool classification into shellscan.Classify and carries classifier reason into decision path.
core/cmd/helm-ai-kernel/hook_cmd_test.go	Adds hook-level tests for AST-closed evasions and benign passthrough regression.
core/go.mod	Adds mvdan shell parser dependency.
core/go.sum	Updates sums for new dependencies.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

core/pkg/shellscan/shellscan.go:78

• sensitiveRedirectTargets is described as mirroring the hook’s sensitive-write target list, but it currently omits the Windows-style .git\\ entry that isSensitiveWriteTarget matches (hook_cmd.go:295-303). That allows a bash redirect target containing .git\\ to bypass the sensitive redirect detection layer.

	"id_rsa",
	"id_ed25519",
	".git/",
}

core/pkg/shellscan/shellscan.go:155

• classifyString uses collector-wide sawPipe/sawShell/sawEval/sawDecode state to decide "encoded payload decoded into a shell or eval". Because this state is shared across recursive classifyString calls (e.g. via bash -c, eval, env -S), a nested snippet that happens to contain a pipe can incorrectly influence the outer snippet’s encoded-wrapper decision even when no decode-to-shell pipeline exists in the outer command.

func (c *collector) classifyString(src, via string, depth int) {
	if depth > maxWrapperDepth {
		c.decide("wrapper nesting too deep to classify statically")
		return
	}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

core/pkg/shellscan/shellscan.go:78

• sensitiveRedirectTargets is meant to mirror isSensitiveWriteTarget in the hook, but it’s missing the Windows-style .git\\ entry (hook has both .git/ and .git\\). That allows redirects like >> .git\\config to bypass the same sensitive-target protection in the shell classifier.

// sensitiveRedirectTargets mirrors the sensitive-write list in the hook so a
// shell redirect cannot bypass the Write-tool path protection.
var sensitiveRedirectTargets = []string{
	".env",
	".pem",
	".key",
	"id_rsa",
	"id_ed25519",
	".git/",
}

core/pkg/shellscan/shellscan.go:189

• The SignalEncodedWrapper decision check uses global booleans (sawDecode, sawPipe, sawShell, sawEval) aggregated across the entire parsed snippet. That can produce false positives when decode and shell/eval occur in different statements or unrelated pipelines (e.g. echo x | base64 -d > out; sh -c 'echo hi'), routing benign commands into the decision path (regression risk).

	if c.sawDecode && (c.sawPipe && (c.sawShell || c.sawEval)) {
		c.signal(SignalEncodedWrapper)
		c.decide("encoded payload decoded into a shell or eval")
	}

core/pkg/shellscan/arity.go:168

• Prefix claims that "tokens that look like flags stop prefix growth", but the implementation can currently include flags inside the returned prefix (e.g. tokens ["git","-C","/repo","reset"] returns "git -C" because arity["git"]=2). That makes prefixes unstable/less human-meaningful and undermines the arity-table intent for future scoping.

// Prefix returns the arity-derived, human-meaningful command prefix for a
// tokenized command (e.g. ["git","checkout","main"] -> "git checkout",
// ["npm","run","dev"] -> "npm run dev"). Longest matching prefix wins;
// unknown commands fall back to the first token. Tokens that look like flags
// stop prefix growth unless a longer dictionary entry matched first.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (4)

core/pkg/shellscan/shellscan.go:189

• SignalEncodedWrapper detection is based on collector-wide booleans (sawDecode, sawPipe, sawShell, sawEval) and triggers when all are seen anywhere in the parsed snippet. This can produce false positives across independent statements/pipelines (e.g. echo x | base64 -d; echo ok | sh would be flagged even though the decode output is not piped into the shell), changing behavior from the PR’s stated “decode-to-shell pipeline” intent.

	if c.sawDecode && (c.sawPipe && (c.sawShell || c.sawEval)) {
		c.signal(SignalEncodedWrapper)
		c.decide("encoded payload decoded into a shell or eval")
	}

core/pkg/shellscan/shellscan.go:821

• In matchGit (clean), the loop continues when it sees the end-of-options marker -- instead of stopping, so pathspec operands like -- -fd can be misinterpreted as flags and incorrectly classified as destructive. Break out of flag scanning when -- is reached.

		for _, tok := range rest {
			if tok.dynamic {
				c.decide("git clean with unresolvable flags (fail-closed)")
				return
			}

core/pkg/shellscan/arity.go:181

• Prefix can return prefixes that include flag tokens (e.g. tokens like ["git","-C","/repo",...] yield prefix "git -C" because arity["git"]=2). This contradicts the comment that “flags never count as tokens” and will produce misleading prefixes for common CLI patterns with global flags.

	if n, ok := arity[tokens[0]]; ok && n <= len(tokens) {
		return joinTokens(tokens[:n])
	}

core/pkg/shellscan/shellscan.go:75

• sensitiveRedirectTargets claims to mirror the hook’s sensitive-write list, but it’s missing the Windows-path needle .git\\ that isSensitiveWriteTarget checks for. This creates an inconsistency where echo x > .git\\config could bypass the redirect-based protection layer.

var sensitiveRedirectTargets = []string{
	".env",
	".pem",
	".key",
	"id_rsa",

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

core/pkg/shellscan/shellscan.go:78

• sensitiveRedirectTargets is stated to mirror the hook's sensitive write targets, but it currently omits the Windows-style .git\\ needle that isSensitiveWriteTarget checks for (core/cmd/helm-ai-kernel/hook_cmd.go:295-303). This allows a redirect like > .git\\config to bypass the sensitive-redirect detection.

	".git/",
}

core/pkg/shellscan/shellscan.go:189

• Encoded-wrapper detection is based on collector-wide booleans (sawDecode, sawPipe, sawShell, sawEval) that can be set by unrelated statements in the same input. For example, echo a | base64 -d | cat; sh -c 'echo hi' would satisfy the condition even though the decoded bytes are not piped into a shell/eval. This can introduce false positives (extra receipts/denies) and an inaccurate reason string.

	if c.sawDecode && (c.sawPipe && (c.sawShell || c.sawEval)) {
		c.signal(SignalEncodedWrapper)
		c.decide("encoded payload decoded into a shell or eval")
	}

core/pkg/shellscan/arity.go:170

• Prefix's docstring says "Tokens that look like flags stop prefix growth", but the implementation can return prefixes that include flags (e.g. []string{"git","-C","/repo","status"} returns "git -C" because arity["git"]=2). This makes prefixes less human-meaningful and contradicts the documented behavior.

// unknown commands fall back to the first token. Tokens that look like flags
// stop prefix growth unless a longer dictionary entry matched first.
func Prefix(tokens []string) string {
	if len(tokens) == 0 {

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

core/pkg/shellscan/shellscan.go:78

• sensitiveRedirectTargets is intended to mirror the hook’s isSensitiveWriteTarget list, but it’s missing the Windows-style ".git\" needle (hook_cmd.go includes both ".git/" and ".git\"). This can let a redirect like echo x >> .git\\config bypass the sensitive redirect detection on Windows paths.

// sensitiveRedirectTargets mirrors the sensitive-write list in the hook so a
// shell redirect cannot bypass the Write-tool path protection.
var sensitiveRedirectTargets = []string{
	".env",
	".pem",
	".key",
	"id_rsa",
	"id_ed25519",
	".git/",
}

core/pkg/shellscan/shellscan.go:189

• Encoded-wrapper detection uses collector-wide booleans (sawDecode/sawPipe/sawShell/sawEval) aggregated across the entire parsed snippet. This can incorrectly label unrelated statements as an "encoded payload decoded into a shell or eval" (e.g. a base64 -d in one pipeline and a separate bash reading stdin elsewhere in the command string). This produces misleading receipt reasons/signals and could spuriously route benign multi-statement commands to the decision path.

// classifyString parses a shell snippet and walks every command node,
// including those inside pipelines, substitutions and subshells.
func (c *collector) classifyString(src, via string, depth int) {
	if depth > maxWrapperDepth {
		c.decide("wrapper nesting too deep to classify statically")
		return
	}
	parser := syntax.NewParser()
	file, err := parser.Parse(strings.NewReader(src), "")
	if err != nil {
		c.signal(SignalParseError)
		c.parseOK = false
		c.decide("unparseable shell command (fail-closed)")
		return
	}
	if len(file.Stmts) > 1 {
		c.signal(SignalChaining)
	}
	syntax.Walk(file, func(node syntax.Node) bool {
		switch n := node.(type) {
		case *syntax.BinaryCmd:
			switch n.Op {
			case syntax.Pipe:
				c.sawPipe = true
				c.signal(SignalChaining)
			case syntax.AndStmt, syntax.OrStmt:
				c.signal(SignalChaining)
			}
		case *syntax.CmdSubst, *syntax.ProcSubst:
			c.signal(SignalCommandSubstitution)
		case *syntax.Redirect:
			c.classifyRedirect(n)
		case *syntax.CallExpr:
			c.classifyCall(n, via, depth)
		}
		return true
	})
	if c.sawDecode && (c.sawPipe && (c.sawShell || c.sawEval)) {
		c.signal(SignalEncodedWrapper)
		c.decide("encoded payload decoded into a shell or eval")
	}

core/pkg/shellscan/shellscan.go:1206

• In git clean flag scanning, -- should terminate option parsing. The current condition if !HasPrefix("-") || tok.text == "--" { continue } continues scanning after --, so path operands after -- that start with - can be misinterpreted as flags and spuriously trigger a destructive classification.

		force, dirs := false, false
		for _, tok := range rest {
			if tok.dynamic {
				c.decide("git clean with unresolvable flags (fail-closed)")
				return
			}
			if !strings.HasPrefix(tok.text, "-") || tok.text == "--" {
				continue
			}

core/pkg/shellscan/shellscan.go:1261

• In docker rm flag scanning, -- should terminate option parsing. Without handling it, a container name after -- (or any operand starting with '-') can be treated as a flag and spuriously trigger a destructive classification.

	for _, tok := range rest {
		if tok.dynamic {
			c.decide("docker rm with unresolvable flags (fail-closed)")
			return
		}
		if tok.text == "--force" {
			c.decide("docker rm --force")
			return
		}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

core/pkg/shellscan/shellscan.go:189

• Encoded-wrapper detection is based on global booleans (sawDecode, sawPipe, sawShell/sawEval) across the entire parse. This can incorrectly mark unrelated statements as an "encoded payload decoded into a shell" (e.g., a benign pipe earlier + a separate base64 -d file + a separate bash script.sh). Consider tying this detection to the specific pipeline structure instead of cross-statement aggregation (or scoping/resetting these flags per pipeline).

			case syntax.Pipe:
				c.sawPipe = true
				c.signal(SignalChaining)
			case syntax.AndStmt, syntax.OrStmt:
				c.signal(SignalChaining)
			}
		case *syntax.CmdSubst, *syntax.ProcSubst:
			c.signal(SignalCommandSubstitution)
		case *syntax.Redirect:
			c.classifyRedirect(n)
		case *syntax.CallExpr:
			c.classifyCall(n, via, depth)
		}
		return true
	})
	if c.sawDecode && (c.sawPipe && (c.sawShell || c.sawEval)) {
		c.signal(SignalEncodedWrapper)
		c.decide("encoded payload decoded into a shell or eval")
	}

core/pkg/shellscan/arity.go:182

• Prefix's docstring says flags stop prefix growth, but the implementation can return prefixes that include flags (e.g., tokens starting with git -C ... will yield git -C). This makes the recorded Command.Prefix misleading for common “global-flag then subcommand” patterns and diverges from the comment above the function.

// Prefix returns the arity-derived, human-meaningful command prefix for a
// tokenized command (e.g. ["git","checkout","main"] -> "git checkout",
// ["npm","run","dev"] -> "npm run dev"). Longest matching prefix wins;
// unknown commands fall back to the first token. Tokens that look like flags
// stop prefix growth unless a longer dictionary entry matched first.
func Prefix(tokens []string) string {
	if len(tokens) == 0 {
		return ""
	}
	for length := len(tokens); length > 1; length-- {
		key := joinTokens(tokens[:length])
		if n, ok := arity[key]; ok && n <= len(tokens) {
			return joinTokens(tokens[:n])
		}
	}
	if n, ok := arity[tokens[0]]; ok && n <= len(tokens) {
		return joinTokens(tokens[:n])
	}
	return tokens[0]

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

core/pkg/shellscan/shellscan.go:189

• encoded payload detection currently uses global flags (sawDecode, sawPipe, sawShell/sawEval) aggregated across the whole parsed snippet. That can incorrectly flip a benign bash -c '...' (or benign eval "...") to Decide=true just because some other pipeline in the same snippet used | and some other command used base64 -d/xxd -r. This becomes an unintended deny/receipt for otherwise-allowed commands.

A safer approach is to only consider stdin-reading shell invocations (i.e., actual ... | sh / ... | bash -s style) as sawShell, and drop sawEval from the heuristic (eval doesn’t consume pipeline stdin as code).

	if c.sawDecode && (c.sawPipe && (c.sawShell || c.sawEval)) {
		c.signal(SignalEncodedWrapper)
		c.decide("encoded payload decoded into a shell or eval")
	}

core/pkg/shellscan/shellscan.go:980

• c.sawShell is set for all shell invocations (including bash -c '...'), but it’s only used to drive the decode-to-shell heuristic. As a result, any snippet that contains both (a) a decode command like base64 -d somewhere and (b) any pipe anywhere can end up being treated as an encoded payload decoded into a shell, even when the shell is executing a static -c script and is not consuming decoded stdin as code.

To avoid false-positive denies, only set sawShell for stdin-reading shells (the stdin branch).

		case shellNames[name]:
			c.signal(SignalShellInvocation)
			c.sawShell = true
			rest := args[1:]

core/pkg/shellscan/shellscan.go:78

• sensitiveRedirectTargets claims to mirror the hook’s sensitive write target list, but it’s missing the ".git\\" variant that the hook blocks (isSensitiveWriteTarget includes both .git/ and .git\\). On Windows/MSYS-style paths, a redirect like > .git\config could bypass the intended protection in this pre-flight layer.

// sensitiveRedirectTargets mirrors the sensitive-write list in the hook so a
// shell redirect cannot bypass the Write-tool path protection.
var sensitiveRedirectTargets = []string{
	".env",
	".pem",
	".key",
	"id_rsa",
	"id_ed25519",
	".git/",
}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

core/pkg/shellscan/shellscan.go:189

• Encoded-wrapper detection uses global sawDecode/sawPipe/sawShell/sawEval booleans and can route unrelated commands to the decision path (e.g., bash script.sh; echo x | base64 -d > out would satisfy the condition even though the decoded bytes are not executed). This can create false denies and misleading reasons. Prefer either (a) structural detection tied to a specific pipeline edge, or (b) keeping this as an audit signal only (no decision) since shell reads stdin already fail-closes the dangerous ... | sh/bash cases.

	if c.sawDecode && (c.sawPipe && (c.sawShell || c.sawEval)) {
		c.signal(SignalEncodedWrapper)
		c.decide("encoded payload decoded into a shell or eval")
	}

core/pkg/shellscan/arity.go:169

• Prefix() currently includes flags/flag-values in the computed prefix (e.g. tokens ["git","-C","/repo","checkout","main"] will yield "git -C"), which contradicts the comment about stopping on flags and makes prefixes unstable/unhelpful for permit scoping and receipts. Prefix derivation should stop once a flag-like token is encountered (at minimum) so recorded prefixes don’t contain flags.

// Prefix returns the arity-derived, human-meaningful command prefix for a
// tokenized command (e.g. ["git","checkout","main"] -> "git checkout",
// ["npm","run","dev"] -> "npm run dev"). Longest matching prefix wins;
// unknown commands fall back to the first token. Tokens that look like flags
// stop prefix growth unless a longer dictionary entry matched first.
func Prefix(tokens []string) string {

core/pkg/shellscan/shellscan.go:835

• command is treated as a generic executor wrapper and passed through dropWrapperFlags(), but command’s common query modes (command -v/-V foo) do not execute foo. Today those flags are unmodeled, so command -v git will route to the decision path (likely deny by default), which is a high-probability regression for benign “is tool installed?” probes. Handle command separately: treat -v/-V as non-executing (record and return), allow -p/--, and fail-closed on unrecognized flags.

		case name == "nice" || name == "nohup" || name == "time" || name == "command" || name == "builtin" || name == "stdbuf" || name == "setsid" || name == "exec":
			c.signal(SignalEnvWrapper)
			args = dropWrapperFlags(name, args[1:])
			via = joinVia(via, name)