diff --git a/CLAUDE.md b/CLAUDE.md index edbb4a8..4861e3a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,7 +22,7 @@ See [README §Configuration](README.md#configuration) and [`ash.toml.example`](a ### Error codes touching this - `path_denied` — verb path arg fell outside the active jail policy. -- `not_implemented` — reserved for future ops a backend genuinely cannot perform. No live verb returns this today. +- `not_implemented` — a backend cannot perform an op. `git --op blame` returns it under `[git].backend = "shellout"` (blame requires the go-git backend). ## When to prefer ash over bash diff --git a/README.md b/README.md index 560ece9..a4a0574 100644 --- a/README.md +++ b/README.md @@ -77,9 +77,9 @@ This project is two experiments at once: the shell itself, and a deliberate stud - **`ash help [--verb ]`** is the authoritative per-verb arg schema — never duplicated in markdown. - **[docs/vocab/inventory.md](docs/vocab/inventory.md)** is the authoritative inventory of every stable string ash emits — error codes, status enums, pretty-form headers, labels — with cl100k token costs. Regenerated by `make vocab`; CI lint (`make vocab-check`) fails on drift. -Agents in repos that ran `ash init` see only the hook's deny messages today; no CLAUDE.md is propagated to target repos yet (tracked as a follow-up). The design rationale and maintenance ritual for these surfaces live in [docs/agent-guidance.md](docs/agent-guidance.md). +Agents in repos that ran `ash init` get a CLAUDE.md section with the switch criteria and gotchas, alongside the hook's deny messages. The design rationale and maintenance ritual for these surfaces live in [docs/agent-guidance.md](docs/agent-guidance.md). -**Native adoption via MCP.** The companion binary `ashmcp` exposes every read-side verb as a typed Model Context Protocol tool (`ash_read`, `ash_grep`, `ash_lang_def`, …) over stdio. MCP-aware harnesses see them alongside their built-ins from session start, so adoption no longer depends on the hook's block-and-nudge loop. Copy-paste snippets and verification recipes for Claude Code and Claude Desktop, plus the migration path off the hook, live in [docs/adoption/](docs/adoption/). +**Native adoption via MCP.** The companion binary `ashmcp` exposes the read- and write-side verbs as typed Model Context Protocol tools (`ash_read`, `ash_grep`, `ash_edit`, …) over stdio. MCP-aware harnesses see them alongside their built-ins from session start, so adoption no longer depends on the hook's block-and-nudge loop. Copy-paste snippets and verification recipes for Claude Code and Claude Desktop, plus the migration path off the hook, live in [docs/adoption/](docs/adoption/). **Layering vocabulary.** [docs/architecture/layers.md](docs/architecture/layers.md) is the one-pager naming the four tiers (protocol / verb library / dispatch / clients) and showing which tier a given change belongs in. Start there when proposing structural work. @@ -243,14 +243,14 @@ Instrumentation was wired in from the first verb, not retrofitted. A tool that c ## Benchmarks -`ash bench` runs 19 canonical cases covering every measurable verb — `read`, `write`, `edit`, `diff`, `find`, `grep`, `git`, `stat` — and compares ash against the bash equivalent the agent would otherwise have used. Both sides tokenize with the same `cl100k_base` encoder. Every run persists to `.ash/ledger.db`, so regressions show up in the diff, not the incident report. +`ash bench` runs 21 canonical cases covering every measurable verb — `read`, `write`, `edit`, `diff`, `find`, `grep`, `git`, `stat` — and compares ash against the bash equivalent the agent would otherwise have used. Both sides tokenize with the same `cl100k_base` encoder. Every run persists to `.ash/ledger.db`, so regressions show up in the diff, not the incident report. -**Current baseline (2026-05-10):** **−54.8% tokens** overall (31,196 ash vs 69,013 bash across 19 cases). Per-case breakdown: [bench/baseline.md](bench/baseline.md). +**Current baseline (2026-05-18):** **−63.8% tokens** overall (41,767 ash vs 115,410 bash across 21 cases). Per-case breakdown: [bench/baseline.md](bench/baseline.md). ### Running ```sh -ash bench # one-shot, all 19 cases +ash bench # one-shot, all 21 cases make bench # repeat=5, warmup=2, writes bench/latest.json (gitignored) make bench-baseline # stable run + update bench/baseline.json + bench/baseline.md ``` @@ -266,7 +266,7 @@ ash bench --baseline 7d --regress-tokens 0.05 # tighter 5% threshold ### Regression contract -`bench/baseline.json` is the checked-in token budget for all 19 cases. `--compare baseline,latest` diffs your working state against it — zero regressions on a no-op change is the bar. Latency is informational only (machine-dependent) and lives separately in `bench/latency-snapshot.json`. +`bench/baseline.json` is the checked-in token budget for all 21 cases. `--compare baseline,latest` diffs your working state against it — zero regressions on a no-op change is the bar. Latency is informational only (machine-dependent) and lives separately in `bench/latency-snapshot.json`. Design rationale: [docs/bench.md](docs/bench.md). Implementation: [docs/bench-2.md](docs/bench-2.md). @@ -307,9 +307,9 @@ Go daemon, UDS transport, MessagePack with versioned schema dictionary, three re ### Phase 2 — coding-agent core — current (ship 14) -Live: `write`, `edit`, `stat`, `diff`, `git` (status/log/diff/show), `test`, `bench`, `metrics`, `report`, `hook`, `help`, `init`, `uninit`, `stop`. Plus: configuration substrate (`ash.toml`), jail policy, ledger retention, in-process go-git backend, cross-repo report aggregation, PreToolUse hook with per-verb exclusions. +Live: `write`, `edit`, `stat`, `diff`, `git` (status/log/diff/show/blame), `build`, `test`, `bench`, `metrics`, `report`, `hook`, `help`, `init`, `uninit`, `stop`. Plus: configuration substrate (`ash.toml`), jail policy, ledger retention, in-process go-git backend, cross-repo report aggregation, PreToolUse hook with per-verb exclusions. -Upcoming for phase 2: `build`, `fmt`, `run`, `proc`, `obj`, persistent session/object store, job ledger for async operations, reference Go client library. +Upcoming for phase 2: `fmt`, `run`, `proc`, `obj`, persistent session/object store, job ledger for async operations, reference Go client library. ### Phase 3 — semantic layer diff --git a/cmd/ash/main.go b/cmd/ash/main.go index d87f2de..75840ee 100644 --- a/cmd/ash/main.go +++ b/cmd/ash/main.go @@ -40,17 +40,17 @@ func main() { verb := os.Args[1] + if verb == "--version" || verb == "-V" { + fmt.Printf("ash %s — use 'ash help' for the verb list\n", version) + return + } + // `ash hook` is a client-only fast path for the Claude Code PreToolUse // hook. It reads the harness payload from stdin, computes the decision // in-process, writes the Claude-format response to stdout, and best- // effort fires a normal ash request to the daemon for ledger // instrumentation. It never auto-starts the daemon — hook latency is // on the agent's critical path. - if verb == "--version" || verb == "-V" { - fmt.Printf("ash %s — use 'ash help' for the verb list\n", version) - return - } - if verb == "hook" { runHook() return diff --git a/cmd/ashd/main.go b/cmd/ashd/main.go index c44e802..b1f3a91 100644 --- a/cmd/ashd/main.go +++ b/cmd/ashd/main.go @@ -37,10 +37,10 @@ import ( // one-shot startup config (ledger cleanup, daemon concurrency cap) // stay restart-required — see CLAUDE.md gotcha #1. // -// jail.SetPolicy is a package-global pointer swap and is goroutine-safe; -// concurrent handler goroutines either see the old or the new policy -// for any given verb call, matching the "next request sees new config" -// semantic the hot-reload contract promises. +// jail.SetPolicy swaps a package-global policy pointer under an RWMutex +// and is goroutine-safe; concurrent handler goroutines see either the +// old or the new policy for any given verb call, matching the "next +// request sees new config" semantic the hot-reload contract promises. func applyEnforcementConfig(rootFlag string, cfg *config.Config) { jail.SetPolicy(jail.FromConfig(cfg.Jail.Enabled, rootFlag, cfg.Jail.AllowPaths, cfg.Jail.DenyPaths)) } @@ -169,7 +169,13 @@ func main() { log.Fatalf("ashd: %v", err) } _ = os.Remove(sockFlag) + // Create the socket 0600 from the start: net.Listen otherwise + // applies the process umask, leaving a brief window where the + // socket is world-connectable before the Chmod below. Umask is + // process-global but daemon startup here is single-threaded. + oldMask := syscall.Umask(0o177) listener, err := net.Listen("unix", sockFlag) + syscall.Umask(oldMask) if err != nil { log.Fatalf("ashd: listen %s: %v", sockFlag, err) } @@ -497,7 +503,7 @@ func handle(conn net.Conn, led *ledger.Ledger, runners map[string]verbs.Runner, RequestID: req.ID, Timestamp: time.Now(), Verb: req.Verb, - ArgsMsgpack: argsBlob(reqBuf), + ArgsMsgpack: argsBlob(req), OK: rsp.OK, ErrCode: errCode, ErrMsg: errMsg, @@ -649,10 +655,9 @@ const argsMaxStrBytes = 1024 // String values longer than argsMaxStrBytes are replaced with "". // Env is intentionally excluded — secrets in req.Env (forwarded to test // subprocesses, ASH-132) must not leak into args_msgpack. -// Returns nil when the request has no args or cannot be decoded. -func argsBlob(reqBuf []byte) []byte { - req, err := proto.DecodeRequest(reqBuf) - if err != nil || len(req.Args) == 0 { +// Returns nil when the request has no args. +func argsBlob(req *proto.Request) []byte { + if req == nil || len(req.Args) == 0 { return nil } sanitized := make(map[string]any, len(req.Args)) diff --git a/docs/configuration.md b/docs/configuration.md index 7329367..ba8cbfe 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -223,7 +223,7 @@ max_concurrent_handlers = 32 backend = "go-git" EOF bin/ash stop && bin/ash git --op status -# git --op status returns not_implemented because go-git path is stubbed. +# git --op status returns a structured StatusResult; go-git is the default backend. # daemon log line shows config=ash.toml. rm ash.toml hosts-link # restore clean state diff --git a/docs/install.md b/docs/install.md deleted file mode 100644 index a54d054..0000000 --- a/docs/install.md +++ /dev/null @@ -1,186 +0,0 @@ -# Plan — installing ash into target repos for ledger capture - -> **Status:** Shipped Phase 2. `ash init`/`ash uninit` (ASH-47, commit 490dcf0; refactored under ASH-70 in commit 4982819), `make install`/`make uninstall`, and `ash report --root`/`--all` (the original `--all-roots` shipped as `--all`) are all live. This doc is the original plan record; authoritative current behavior is `bin/ash help --verb init` / `uninit` / `report`. - -## Context - -`ash` is in Phase 2; the recursive-development premise says agents working on -*any* repo should drive `ash` so we collect ledger data and feed the next -phase's design. Today the only repo that benefits is ash itself: there is -no install workflow, the hook config in `.claude/settings.json` hardcodes -`$CLAUDE_PROJECT_DIR/bin/ash`, and re-copying binaries on every rebuild -would dominate the friction. - -We want a one-shot `make install` that puts ash on `$PATH` once, plus an -`ash init` verb that bootstraps any target repo (hook config, gitignore, -registry entry) idempotently. Cross-repo report aggregation rounds out -the install→capture→analyze loop so a session in the ash repo can analyze -ledgers captured in other repos. - -User decisions (confirmed): **symlink-on-`$PATH`** install model; -**include** cross-repo ledger analysis in this plan. - -## Scope - -Three pieces, in order of dependency: - -1. `make install` / `make uninstall` — symlink binaries onto `$PATH`. -2. `ash init` verb — wire up a target repo's `.claude/settings.json`, - `.gitignore`, and append to a global registry of installed roots. -3. `ash report --root

` and `ash report --all-roots` — read/aggregate - foreign ledgers using the registry. - -Out of scope: the friction note in -[docs/encoding-results.md](encoding-results.md) -about the hook denying writes outside project root — tracked separately. - -## 1. `make install` / `make uninstall` - -Edit [Makefile](../Makefile). Add two targets: - -```make -PREFIX ?= $(HOME)/.local/bin - -install: all - mkdir -p $(PREFIX) - ln -sf $(CURDIR)/bin/ash $(PREFIX)/ash - ln -sf $(CURDIR)/bin/ashd $(PREFIX)/ashd - @echo "installed: $(PREFIX)/{ash,ashd}" - -uninstall: - rm -f $(PREFIX)/ash $(PREFIX)/ashd -``` - -Symlinks (not copies) so a rebuild of ash auto-updates every target. The -existing `killStaleIfNeeded` logic in -[cmd/ash/main.go:399](../cmd/ash/main.go#L399) -already compares the ashd binary mtime against the socket and restarts -the daemon when stale — `os.Stat` follows symlinks, so this Just Works -across all target repos on the next call after a rebuild. - -PATH check on install — emit a warning if `$(PREFIX)` isn't on `$PATH`. - -## 2. `ash init` verb - -New verb, server-handled (so the call instruments itself in the target -repo's ledger). Flags: - -- `--path

` (default `.`) — target repo root. -- `--force` — overwrite an existing differing PreToolUse entry. -- `--no_registry` — skip writing to the global installed-repos registry - (escape hatch for ephemeral repos). - -Behavior: - -1. Resolve `

` to an absolute path; validate it's a directory. -2. Read `

/.claude/settings.json` if present, else start with `{}`. -3. Merge a PreToolUse entry whose command is exactly `"ash hook"` (PATH - form). Detection rule: any entry whose `hooks[].command` contains the - substring `ash hook` and whose matcher is a superset of - `Grep|Glob|Bash|Edit|Write|Read` is treated as already-installed. - - Already installed → no-op, exit OK. - - Different ash command (e.g. `$CLAUDE_PROJECT_DIR/bin/ash hook`) → - leave untouched and print a warning unless `--force`. - - Absent → append a new entry. -4. Write `.claude/settings.json` back (`json.MarshalIndent`, 2 spaces). - Create `.claude/` if missing. -5. If `

/.gitignore` exists and lacks a `.ash/` line, append it (with - a leading newline if the file doesn't end in one). Skip silently if - no `.gitignore`. -6. Unless `--no_registry`, append the absolute root to - `$XDG_CONFIG_HOME/ash/installed-repos.txt` (fallback `~/.config/ash/`), - deduplicated. Create the file if missing. -7. Result envelope: - - `path` (absolute root) - - `settings_written` (bool) - - `gitignore_updated` (bool) - - `registry_updated` (bool) - - `already_installed` (bool, set when the merge was a no-op) - - `warnings []string` - -Mirror an `ash uninit --path

` verb that removes the hook entry, the -gitignore line, and the registry row. Useful for clean teardown. - -### Files to modify / create - -- New: `internal/verbs/init/init.go` — verb implementation. -- New: `internal/verbs/init/init_test.go` — unit tests covering - fresh repo, already-installed repo, conflicting hook entry, - no-gitignore repo. -- Modify: the verb registry (now [internal/verbs/verbs.go](../internal/verbs/verbs.go); the originally-planned `internal/server/server.go` was never created — the daemon dispatch lives in `cmd/ashd/main.go` instead) — register `init` and `uninit`. -- Modify: [cmd/ash/main.go](../cmd/ash/main.go) — verb argument schema and - pretty renderer. -- Modify: [internal/proto/proto.go](../internal/proto/proto.go) — - add `InitArgs` / `InitResult` types. -- Modify: [internal/proto/pretty.go](../internal/proto/pretty.go) — - pretty form for the result. - -### Reuse - -- `session.Root(cwd)` from - [internal/session/paths.go](../internal/session/paths.go) - for resolving the target root if `--path .`. -- The temp-file + rename pattern already used in - [internal/verbs/write/](../internal/verbs/write/) - for atomic settings.json write. - -## 3. `ash report --root

` and `--all-roots` - -Extend the existing `report` verb (no new verb). Flags: - -- `--root

` — open `

/.ash/ledger.db` instead of the daemon's own - ledger. Mutually exclusive with `--all-roots`. -- `--all-roots` — read the installed-repos registry, open each ledger, - aggregate per-verb counts/percentiles across all of them; pretty form - shows a per-repo breakdown column. - -Implementation note: the daemon's `report` handler currently opens its -own ledger via the connected `ledger.DB`. Refactor to accept an explicit -ledger path, defaulting to the daemon's. Foreign ledgers open read-only -(`?mode=ro` in the SQLite DSN) so a running daemon in the foreign repo -isn't disturbed. - -Files to modify: - -- [internal/verbs/report/report.go](../internal/verbs/report/report.go) - — add flags, foreign-ledger open path, aggregation. -- [cmd/ash/main.go](../cmd/ash/main.go) — argument schema entries. -- New: `internal/registry/registry.go` (or fold into `internal/session/`) - — read/write `installed-repos.txt`. Shared by `init`, `uninit`, `report`. - -## Documentation - -- Update [README.md](../README.md) — add an "Installing into a target - repo" section walking through `make install` → - `cd ../target && ash init`. -- Update [CLAUDE.md](../CLAUDE.md) — bash whitelist note, and a - one-liner under "How to invoke ash" pointing at the install verbs. -- The `init` and `uninit` verbs go into the live-verbs section. - -## Verification - -End-to-end, in order: - -1. `make install` from the ash repo. `which ash && which ashd` resolves - inside `$PREFIX`. `ash help` works from any cwd. -2. `cd ~/some-other-repo && ash init`. Inspect: - - `.claude/settings.json` contains the PreToolUse hook with command - `"ash hook"`. - - `.gitignore` ends with `.ash/`. - - `~/.config/ash/installed-repos.txt` contains the absolute root. -3. From the same other repo: `ash find --path . --max_depth 1`. Confirm - `.ash/ledger.db` is created and `ash metrics --last 5` shows the - call. The hook fires for harness Read/Bash and denies them. -4. Re-run `ash init` — exits with `already_installed=true`, no file - changes. -5. From the ash repo: `ash report --root ~/some-other-repo` returns the - foreign repo's per-verb summary; `ash report --all-roots` returns an - aggregate across both ash and the target. -6. `ash uninit --path ~/some-other-repo` removes the hook entry, - gitignore line, and registry row. Re-running `ash init` reinstalls - cleanly. -7. Rebuild ash (`make all`); next `ash` call from any target repo - restarts that repo's daemon (verify by checking session id in the - ledger changes). No manual update step in any target. -8. Unit tests: `ash test --packages internal/verbs/init,internal/verbs/report` - pass, including the conflicting-hook and no-gitignore branches. diff --git a/docs/vocab/inventory.json b/docs/vocab/inventory.json index 28d39b5..1654072 100644 --- a/docs/vocab/inventory.json +++ b/docs/vocab/inventory.json @@ -1045,7 +1045,7 @@ "sites": [ { "file": "internal/verbs/git/git.go", - "line": 206 + "line": 231 } ], "hints": [ @@ -1199,6 +1199,10 @@ "file": "internal/verbs/git/blame.go", "line": 89 }, + { + "file": "internal/verbs/git/git.go", + "line": 181 + }, { "file": "internal/verbs/git/gogit_blame.go", "line": 32 @@ -1361,6 +1365,10 @@ } ], "hints": [ + { + "text": "a git revision, date, author, or pathspec never starts with '-'", + "cl100k_tokens": 15 + }, { "text": "valid ops: outline, def, refs, callers, impl", "cl100k_tokens": 12 diff --git a/docs/vocab/inventory.md b/docs/vocab/inventory.md index ef795ca..28c981f 100644 --- a/docs/vocab/inventory.md +++ b/docs/vocab/inventory.md @@ -258,7 +258,7 @@ This file is regenerated by `make vocab`; CI runs `make vocab-check` to fail on | `too_large` | 2 | 1 | | | | `unknown_op` | 2 | 1 | ✓ | | | `ambiguous` | 1 | 1 | ✓ | | -| `args` | 1 | 71 | ✓ | | +| `args` | 1 | 72 | ✓ | | | `config` | 1 | 9 | | | | `exists` | 1 | 1 | ✓ | | | `io` | 1 | 7 | | | diff --git a/internal/jail/relpath.go b/internal/jail/relpath.go index 51d4ef0..563da2e 100644 --- a/internal/jail/relpath.go +++ b/internal/jail/relpath.go @@ -3,7 +3,6 @@ package jail import ( "fmt" "path/filepath" - "sort" "strings" ) @@ -103,12 +102,9 @@ func PrettyPath(p string) string { return "." } } - // PathPrefixes is already longest-first; iterate in order and stop at - // the first hit so we strip the longest matching prefix. - sorted := make([]string, len(prefixes)) - copy(sorted, prefixes) - sort.Slice(sorted, func(i, j int) bool { return len(sorted[i]) > len(sorted[j]) }) - for _, pref := range sorted { + // PathPrefixes is already longest-first (its contract), so the first + // prefix that matches is the longest — strip it and return. + for _, pref := range prefixes { if strings.HasPrefix(p, pref+"/") { return p[len(pref)+1:] } diff --git a/internal/ledger/ledger.go b/internal/ledger/ledger.go index 09ea7b7..9179fb8 100644 --- a/internal/ledger/ledger.go +++ b/internal/ledger/ledger.go @@ -157,6 +157,9 @@ func Open(path, projectRoot, clientInfo string) (*Ledger, error) { db.Close() return nil, fmt.Errorf("ledger: schema: %w", err) } + // The ledger holds file paths and result fragments — keep it + // owner-only, not the 0644 SQLite creates by default. + _ = os.Chmod(path, 0o600) if _, err := db.Exec(`INSERT OR IGNORE INTO meta (key, value) VALUES ('schema_version', ?)`, schemaVersion); err != nil { db.Close() return nil, fmt.Errorf("ledger: schema version: %w", err) diff --git a/internal/ledger/tokens.go b/internal/ledger/tokens.go index 66301e9..85c1ad3 100644 --- a/internal/ledger/tokens.go +++ b/internal/ledger/tokens.go @@ -36,7 +36,10 @@ func (c *Counter) Count(s string) int { if c == nil || c.enc == nil { return 0 } - return len(c.enc.Encode(s, nil, nil)) + // EncodeOrdinary skips the special-token scan — ash responses never + // contain <|endoftext|>-style tokens, so counts are identical and + // the path is cheaper (one fewer full []rune pass). + return len(c.enc.EncodeOrdinary(s)) } // StripPrefixes returns s with every occurrence of "/" replaced diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go index 2b0a63a..0df4b7d 100644 --- a/internal/lsp/cache/cache.go +++ b/internal/lsp/cache/cache.go @@ -118,9 +118,13 @@ func Open(opts Options) (*Cache, error) { return &Cache{db: db, ttl: opts.TTL}, nil } -// Close releases the database handle. Safe to call multiple times; only -// the first call performs work. +// Close releases the database handle. Safe to call multiple times, and +// on a nil receiver — ashd leaves langCache nil when [lsp] is disabled +// (the default) yet still defers Close. func (c *Cache) Close() error { + if c == nil { + return nil + } c.mu.Lock() defer c.mu.Unlock() if c.db == nil { diff --git a/internal/lsp/lsp.go b/internal/lsp/lsp.go index eacc2e6..ccc1ba3 100644 --- a/internal/lsp/lsp.go +++ b/internal/lsp/lsp.go @@ -435,6 +435,9 @@ func (b *Broker) start(ctx context.Context) error { return &Error{Code: "lsp_spawn", Msg: err.Error()} } if err := cmd.Start(); err != nil { + stdin.Close() + stdout.Close() + stderr.Close() return &Error{Code: "lsp_spawn", Msg: err.Error()} } diff --git a/internal/verbs/git/git.go b/internal/verbs/git/git.go index 347b9bf..6e83839 100644 --- a/internal/verbs/git/git.go +++ b/internal/verbs/git/git.go @@ -115,8 +115,9 @@ func ParseArgs(in map[string]any) (*Args, *proto.Error) { if a.Ignored, perr = argutil.OptionalBool(in, "ignored", false); perr != nil { return nil, perr } - // log-op flags. Strings pass through to git unmodified; git itself is - // the validator for date formats, refspecs, and pathspecs. + // log-op flags. String values pass through to git, which validates + // their content (date formats, refspecs, pathspecs); ParseArgs + // rejects only a leading '-' — see the ASH-211 guard below. if a.Limit, perr = argutil.OptionalPosInt(in, "limit", LogDefaultLimit, LogMaxLimit); perr != nil { return nil, perr } @@ -160,6 +161,30 @@ func ParseArgs(in map[string]any) (*Args, *proto.Error) { if a.LimitBytes, perr = argutil.OptionalPosInt(in, "bytes", DiffDefaultLimitBytes, DiffMaxLimitBytes); perr != nil { return nil, perr } + // Security (ASH-211): reject revision/pathspec args whose value + // begins with "-". The shellout backend splices these straight into + // the git argv, and git would read e.g. --range '--output=/path' as + // an option — `git log --output=FILE` writes attacker-chosen files. + // A legitimate ref, range, date, author, or pathspec never starts + // with "-", so rejection breaks no real call and closes the hole + // for both the shellout and go-git backends. + for _, f := range []struct{ name, val string }{ + {"range", a.Range}, + {"author", a.Author}, + {"since", a.Since}, + {"until", a.Until}, + {"pathspec", a.Pathspec}, + {"ref", a.Ref}, + {"rev", a.Rev}, + } { + if strings.HasPrefix(f.val, "-") { + return nil, &proto.Error{ + Code: "args", + Msg: f.name + " may not begin with '-'", + Hint: "a git revision, date, author, or pathspec never starts with '-'", + } + } + } if perr := jail.CheckPaths(map[string]string{ "path": a.Path, }); perr != nil { diff --git a/internal/verbs/git/git_test.go b/internal/verbs/git/git_test.go index 8d00b5f..207a464 100644 --- a/internal/verbs/git/git_test.go +++ b/internal/verbs/git/git_test.go @@ -247,6 +247,22 @@ func TestParseArgs_WireShape(t *testing.T) { } } +// TestParseArgs_RejectsDashPrefixedRevArgs covers the ASH-211 guard: a +// rev/pathspec value beginning with "-" is rejected before it can reach +// the git argv. git would otherwise read --range '--output=X' as an +// option, and `git log --output=FILE` writes attacker-chosen files. +func TestParseArgs_RejectsDashPrefixedRevArgs(t *testing.T) { + for _, arg := range []string{"range", "author", "since", "until", "pathspec", "ref", "rev"} { + _, perr := ParseArgs(map[string]any{"op": "log", arg: "--output=/tmp/pwned"}) + if perr == nil || perr.Code != "args" { + t.Errorf("%s='--output=...': expected args error, got %+v", arg, perr) + } + } + if _, perr := ParseArgs(map[string]any{"op": "log", "range": "HEAD~3..HEAD"}); perr != nil { + t.Fatalf("legit range rejected: %+v", perr) + } +} + // -- integration smoke test ---------------------------------------------- // // Builds a real repo via `git init`, exercises Run, and asserts the diff --git a/internal/verbs/hook/hook.go b/internal/verbs/hook/hook.go index 8ffda3a..0e57234 100644 --- a/internal/verbs/hook/hook.go +++ b/internal/verbs/hook/hook.go @@ -69,7 +69,7 @@ type Result struct { Reason string `msgpack:"reason,omitempty"` // human-readable deny reason (matches what Claude shows) } -const nudgeTail = `See CLAUDE.md "When to prefer ash over bash". If ash genuinely falls short, run it anyway and write a session note in docs/session-notes/.` +const nudgeTail = `See CLAUDE.md "When to prefer ash over bash". If ash genuinely falls short, run it anyway — that is a finding worth recording (see CLAUDE.md "Session feedback ritual").` // Read is denied for source-text files but allowed for image/PDF/notebook // formats that ash read can't render meaningfully. diff --git a/internal/verbs/hook/hook_test.go b/internal/verbs/hook/hook_test.go index daa3f86..8b3bec8 100644 --- a/internal/verbs/hook/hook_test.go +++ b/internal/verbs/hook/hook_test.go @@ -360,7 +360,7 @@ func TestDecide_bash(t *testing.T) { if tc.wantSugg != "" && !strings.Contains(r.Suggested, tc.wantSugg) { t.Errorf("suggested: want substring %q, got %q", tc.wantSugg, r.Suggested) } - if r.Decision == "deny" && !strings.Contains(r.Reason, "session-notes") { + if r.Decision == "deny" && !strings.Contains(r.Reason, nudgeTail) { t.Errorf("deny reason should include nudge tail: %q", r.Reason) } // ASH-69 regression: a redirection operator must never end up as @@ -506,7 +506,7 @@ func TestEncodeClaudeDecision(t *testing.T) { t.Errorf("permissionDecision: %v", hso["permissionDecision"]) } reason, _ := hso["permissionDecisionReason"].(string) - if !strings.Contains(reason, "ash grep") || !strings.Contains(reason, "session-notes") { + if !strings.Contains(reason, "ash grep") || !strings.Contains(reason, nudgeTail) { t.Errorf("reason: %q", reason) } } diff --git a/internal/verbs/initverb/template.md b/internal/verbs/initverb/template.md index 24252a6..58eb078 100644 --- a/internal/verbs/initverb/template.md +++ b/internal/verbs/initverb/template.md @@ -71,5 +71,4 @@ a fresh daemon. Don't `pkill ashd` — it bypasses graceful shutdown. - `ash help` — authoritative verb list. - `ash help --verb ` — per-verb arg schema. - `.ash/ledger.db` — SQLite ledger of every call (one row per invocation). -- The upstream ash project's `CLAUDE.md` and `docs/` for design depth and - the per-ship session notes that drove these gotchas. +- The upstream ash project's `CLAUDE.md` and `docs/` for design depth.