fix(approval): escape '+' in generated matchers

docs/plans/approval-system.md

@@ -241,6 +241,10 @@ SKILL.md 指南:拿到 `run` 的 `exit 7` → 把 `id` + 原命令 + 候选范
 
 1. **同 uid 自批准**(核心):见 §11。同 uid 下审批是「护栏 + 审计」,不是沙箱;唯一硬化办法 = 让 agent 跑独立 OS 用户/容器 + broker(可选 hard 模式)。密码学签名方案经红队否决(被改 `policy.yaml` / 直连 ssh 两扇侧门绕过)。
 2. **非锚定 / `\s` 正则缺陷 = 立即注入**:`Generalize` 一旦漏锚或用 `\s`,`ls\nrm -rf /` 即可绕过(引擎子串匹配 + `\s` 吃换行,均已实测)。缓解:代码内不变量自检(缺锚、含 `\s`/换行、含 `.*` 即测试失败)+ 注入语料表驱动测试 + fuzz。**最高优先级正确性风险**。
+2b. **转义放行表里的元字符 = 静默改写正则语义(2026-08-03 实测发生)**:`quoteBytesForRegexp` 曾把 `+` 当字面量放行,而 `+` 是量词。后果双向:批准 `echo a+b` 生成的 `\Aecho\x20a+b\z` **既不匹配自己的源串**(grant 静默失效 → 反复重批,一次远端部署里同一命令 12h 内被批 3 次),**又匹配 `echo ab`/`echo aab`**(授权操作者从未批准的命令)。串级不变量(锚定/无 `\s`/无 `.*`)全部通过,挡不住这类缺陷 —— 它们只看正则字符串,不看正则**含义**。
+   缓解(已实施):① 放行表提为 `literalSafePunct` 常量,并有测试逐字节断言 `regexp.QuoteMeta(b) == b`,再加回任何元字符即 CI 失败;② 构造闸门 `newMatcher` 断言**能编译 + 匹配自己的 `SourceCmd` + (exact 档)语法树恰为 `Concat[BeginText, Literal(SourceCmd), EndText]`** —— 最后一条是「只匹配这一条命令」的结构性证明,仅靠自匹配挡不住放宽;③ `FuzzExactMatcherSelfMatch` 用单字节增删改变异证明精确性。
+   同一批护栏另外抓出两个既有缺陷:**非法 UTF-8 命令**(Go 正则里 `\xF3` 表示 *rune* U+00F3 而非字节 0xF3,matcher 无法表达单个非法字节 → 现返回 `ErrInvalidUTF8Command`,与 NUL 同样按 hard-deny 处理)和**前导空格命令**(`splitASCIIWords` 丢前导空格,prefix 正则由 token 重建后匹配不上源串 → 改走 exact)。
+   存量修复:session grant 存了 `source_cmd`,读时按它重算(`sessionFileVersion` 2 + `normalizeGrants`),**严格收窄**且与今天重新审批的结果逐比特相同;`policy.yaml` 没有 `source_cmd`,故只用 `looksLegacyEscaped` 判定后**拒绝采信、不改写**(从可轮转的审计日志反推原文去改写操作者可见的 allow 规则不可靠),命令回落 exit 7 重批一次即可。
 3. **`prefix` 档(需显式开)的过度放宽**:会放宽写类多路子命令(`systemctl restart *`、`docker run *`、`git push *`)与读任意文件 leaf(`cat *`)。**默认 `safe-prefix` 不放宽这些**;开 `prefix` 即接受该取舍。边界仍由 解释器/特权/破坏性 denylist + 分离引擎兜。
 4. **审计 hash 回归**:新字段必须 `,omitempty` 且 `Record`/`canonicalRecord` 同步;用 golden 链 + 逐字段 tamper 测试守住。
 5. **resolution 重放 / 陈旧 pending**:`req_digest` 绑定 + `O_EXCL` + `id` 归属校验;pending/responses 按 TTL 清扫。无 HMAC,故不防同 uid **伪造**(同 §11),但防住**意外串号/重放**。
@@ -249,6 +253,8 @@ SKILL.md 指南:拿到 `run` 的 `exit 7` → 把 `id` + 原命令 + 候选范
 ## 14. 测试要点
 
 - `Generalize`:护栏 denylist、元字符/控制字符/Unicode 空白拒绝、锚定 + 无 `\s`/`.*` 不变量、prefix/exact 分流、字节级切词器;表驱动 + 注入语料(`\n \r \t \f \v` / Unicode 空白 / `$()` / 管道 / 反斜杠续行 / 引号 / glob)+ fuzz;**专测 `ls\nrm -rf /` 不被任何前缀 grant 命中**。
+- **matcher 语义(见 §13.2b)**:放行表逐字节 `QuoteMeta` 自检;每个可打印字节 `a<b>b` 既匹配源串、又不匹配去掉该字节的变体(量词探针);`+` 放宽用例(`echo a+b` 不得命中 `echo ab`);非法 UTF-8 与前导空格分流;`FuzzExactMatcherSelfMatch`(单字节增删改变异证明只匹配源串)+ `FuzzGeneralizeNoWiden`(不跨 `;`/换行/管道延伸)。
+- **存量修复**:v1 session 文件里的 legacy grant 按 `source_cmd` 重算且**修复被落盘**(不是每次读都重做);无法重算的丢弃;单条坏 grant 不得使整个 session 的匹配失败;legacy host 规则不再进 `hostMatchers`。
 - `Authorize`:**host_overrides 物理在前 + global 有显式 deny → 必返 HardDeny**;session/host grant 命中;实时重判(运行前新增 deny 使 grant 失效);once 在并发重跑下只被消费一次。
 - session store:flock 并发、TTL 过期、`session end` 清除、`host` 绑定。
 - request/resolution:`req_digest` 不符当未批;`O_EXCL` 防覆盖;`approval wait/status` 各退出码与缺失/过期/畸形分支。

internal/approval/adjudicate.go

@@ -50,7 +50,13 @@ func ApplyDecision(opts ApplyOptions, id string, verdict Verdict, scope Scope) (
 			if !req.Candidate.Promotable {
 				return ApplyResult{}, fmt.Errorf("approval %s cannot be promoted to host scope", req.ID)
 			}
-			if err := validateMatcherInvariant(req.Candidate.Regex); err != nil {
+			// Validate the stored candidate BEFORE anything durable is written.
+			// applyHostGrant persists req.Candidate.Regex verbatim, so a request
+			// minted by an older binary can carry a matcher that does not match
+			// its own command. Rejecting here — ahead of Resolve and the audit
+			// append — keeps a rejection from burning the resolution and leaving
+			// an approval that can never be re-decided.
+			if err := validateHostCandidate(req); err != nil {
 				return ApplyResult{}, err
 			}
 		} else if _, err := Exact(req.Cmd); err != nil {
@@ -116,7 +122,7 @@ func applyHostGrant(opts ApplyOptions, req PendingRequest) (string, error) {
 	if !req.Candidate.Promotable {
 		return "", fmt.Errorf("approval %s cannot be promoted to host scope", req.ID)
 	}
-	if err := validateMatcherInvariant(req.Candidate.Regex); err != nil {
+	if err := validateHostCandidate(req); err != nil {
 		return "", err
 	}
 	next := opts.Bundle
@@ -160,6 +166,25 @@ func applyHostGrant(opts ApplyOptions, req PendingRequest) (string, error) {
 	return ruleName, nil
 }
 
+// validateHostCandidate proves a pending request's stored matcher is safe to
+// persist as a host rule: it must satisfy the string invariant, compile, and
+// actually match the command the operator reviewed. The last check is what
+// stops a request minted before the '+' escaping fix from being promoted into
+// a permanent rule that authorizes commands nobody approved.
+func validateHostCandidate(req PendingRequest) error {
+	expr, err := compileMatcher(req.Candidate.Regex)
+	if err != nil {
+		return err
+	}
+	if !expr.MatchString(req.Cmd) {
+		return fmt.Errorf("approval %s carries a matcher that does not match its own command; have the agent re-submit the request", req.ID)
+	}
+	if looksLegacyEscaped(req.Candidate.Regex) {
+		return fmt.Errorf("approval %s was created by an older AgentSSH with an unsafe matcher; have the agent re-submit the request", req.ID)
+	}
+	return nil
+}
+
 func appendApprovalAudit(opts ApplyOptions, req PendingRequest, verdict Verdict, scope Scope) error {
 	if opts.Audit.Path == "" {
 		return nil

internal/approval/authorize.go

@@ -126,7 +126,7 @@ func authorize(cfg policy.Config, inv inventory.Inventory, sessionStore SessionS
 		}
 	}
 	matcher, err := Generalize(command, runtime.HostGrantMode)
-	if errors.Is(err, ErrNULCommand) {
+	if errors.Is(err, ErrNULCommand) || errors.Is(err, ErrInvalidUTF8Command) {
 		return Authorization{Status: AuthHardDeny, Decision: decision}, nil
 	}
 	if err != nil {
@@ -162,6 +162,16 @@ func splitPolicy(cfg policy.Config, host string) (policy.Config, []Matcher) {
 				continue
 			}
 			if key == policy.HostRulesKey(host) {
+				// A rule minted before '+' was escaped authorizes commands the
+				// operator never approved, and policy.yaml stores no source
+				// command to repair it from. Refuse to honor it: the command
+				// falls back to needs-approval, and approving once writes a
+				// correct rule alongside. The stale rule stays in the file as
+				// inert text — rewriting an operator-owned allow rule from an
+				// inferred source would be worse than one re-approval.
+				if err := CheckHostGrantRule(rule); err != nil {
+					continue
+				}
 				hostMatchers = append(hostMatchers, Matcher{
 					Kind:       MatcherExact,
 					Regex:      rule.Match.CmdRegex,
@@ -200,9 +210,19 @@ func copyPolicyConfig(cfg policy.Config) policy.Config {
 	return next
 }
 
+// CheckHostGrantRule reports whether a persisted approval rule is still safe to
+// honor. Beyond the string invariant it rejects patterns generated before '+'
+// was escaped: those act as quantifiers and authorize commands the operator
+// never approved.
 func CheckHostGrantRule(rule policy.Rule) error {
 	if rule.Group != policy.ApprovalGroup {
 		return fmt.Errorf("rule is not an approval host rule")
 	}
-	return validateMatcherInvariant(rule.Match.CmdRegex)
+	if _, err := compileMatcher(rule.Match.CmdRegex); err != nil {
+		return err
+	}
+	if looksLegacyEscaped(rule.Match.CmdRegex) {
+		return fmt.Errorf("approval rule %s was generated by an older AgentSSH and may match commands that were never approved; re-approve the command and remove this rule", rule.Name)
+	}
+	return nil
 }

internal/approval/generalize.go

@@ -18,8 +18,28 @@ const (
 
 var ErrNULCommand = errors.New("approval command contains NUL")
 
+// ErrInvalidUTF8Command rejects commands that are not valid UTF-8. Go's regexp
+// has no way to express a single invalid byte: `\xF3` in a pattern denotes the
+// *rune* U+00F3 (encoded C3 B3), not the byte 0xF3. A matcher generated from
+// such a command would therefore fail to match the approved command while
+// matching a different one, so these commands are rejected outright rather
+// than approved with a matcher that cannot mean what it says.
+var ErrInvalidUTF8Command = errors.New("approval command is not valid UTF-8")
+
 const tailTokenClass = `[A-Za-z0-9@%+=:,./_-]+`
 
+// literalSafePunct lists the punctuation bytes quoteBytesForRegexp may emit
+// verbatim into a regex. Every byte here MUST be a regex non-metacharacter:
+// the output is spliced into a pattern outside any character class, so a
+// quantifier ('+', '*', '?') would silently rewrite the pattern's meaning.
+// '+' used to live here and did exactly that — an approved "a+b" produced
+// \Aa+b\z, which failed to match its own source yet matched "ab" and "aab",
+// authorizing commands the operator never approved.
+// TestLiteralAllowlistContainsNoRegexMetachar enforces this invariant.
+// Note: tailTokenClass above is a character class, where '+' is literal and
+// the trailing '+' is a deliberate quantifier — that one is correct as-is.
+const literalSafePunct = `@%=:,/_-`
+
 var interpreterOrEscapable = map[string]struct{}{
 	"sh": {}, "bash": {}, "dash": {}, "zsh": {}, "ksh": {}, "env": {}, "find": {}, "xargs": {},
 	"awk": {}, "gawk": {}, "sed": {}, "perl": {}, "ruby": {}, "node": {}, "php": {},
@@ -57,13 +77,16 @@ func Generalize(command string, mode HostGrantMode) (Matcher, error) {
 	if strings.Contains(command, "\x00") {
 		return Matcher{}, ErrNULCommand
 	}
+	if !utf8.ValidString(command) {
+		return Matcher{}, ErrInvalidUTF8Command
+	}
 	if mode == "" {
 		mode = HostGrantSafePrefix
 	}
 	forceExact := scanForExactOnly(command)
 	tokens := splitASCIIWords(command)
 	if len(tokens) == 0 {
-		return exactMatcher(command, true), nil
+		return exactMatcher(command, true)
 	}
 	head := commandBase(tokens[0])
 	promotable := true
@@ -74,40 +97,50 @@ func Generalize(command string, mode HostGrantMode) (Matcher, error) {
 	if isInterpreterOrEscapable(head) || isDestructiveLeaf(head) || hasEnvPrefix(command) {
 		forceExact = true
 	}
+	// splitASCIIWords drops leading spaces, and prefixMatcher rebuilds the
+	// pattern from tokens — so a leading-space command would produce a prefix
+	// matcher that cannot match the command it came from. The exact matcher
+	// preserves bytes, and is the narrower choice regardless.
+	if strings.HasPrefix(command, " ") {
+		forceExact = true
+	}
 	if forceExact || mode == HostGrantExact {
-		return exactMatcher(command, promotable), nil
+		return exactMatcher(command, promotable)
 	}
 
 	prefix := prefixForMode(tokens, head, mode)
 	tail := tokens[len(prefix):]
 	if len(prefix) == 0 || !tailTokensSafe(tail) || (mode == HostGrantSafePrefix && !safePrefixTailTokensSafe(head, tail)) {
-		return exactMatcher(command, promotable), nil
+		return exactMatcher(command, promotable)
 	}
-	return prefixMatcher(command, prefix, promotable), nil
+	return prefixMatcher(command, prefix, promotable)
 }
 
 func Exact(command string) (Matcher, error) {
 	if strings.Contains(command, "\x00") {
 		return Matcher{}, ErrNULCommand
 	}
-	return exactMatcher(command, true), nil
+	if !utf8.ValidString(command) {
+		return Matcher{}, ErrInvalidUTF8Command
+	}
+	return exactMatcher(command, true)
 }
 
-func exactMatcher(command string, promotable bool) Matcher {
-	return mustValidateMatcher(Matcher{
+func exactMatcher(command string, promotable bool) (Matcher, error) {
+	return newMatcher(Matcher{
 		Kind:       MatcherExact,
 		Regex:      `\A` + quoteBytesForRegexp(command) + `\z`,
 		Promotable: promotable,
 		SourceCmd:  command,
 	})
 }
 
-func prefixMatcher(command string, prefix []string, promotable bool) Matcher {
+func prefixMatcher(command string, prefix []string, promotable bool) (Matcher, error) {
 	parts := make([]string, 0, len(prefix))
 	for _, token := range prefix {
 		parts = append(parts, quoteBytesForRegexp(token))
 	}
-	return mustValidateMatcher(Matcher{
+	return newMatcher(Matcher{
 		Kind:       MatcherPrefix,
 		Regex:      `\A` + strings.Join(parts, `[ \t]+`) + `(?:[ \t]+` + tailTokenClass + `)*[ \t]*\z`,
 		Prefix:     append([]string(nil), prefix...),
@@ -299,7 +332,7 @@ func quoteBytesForRegexp(value string) string {
 	for i := 0; i < len(value); {
 		b := value[i]
 		if (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z') || (b >= '0' && b <= '9') ||
-			b == '@' || b == '%' || b == '+' || b == '=' || b == ':' || b == ',' || b == '/' || b == '_' || b == '-' {
+			strings.IndexByte(literalSafePunct, b) >= 0 {
 			builder.WriteByte(b)
 			i++
 			continue

internal/approval/generalize_fuzz_test.go

@@ -0,0 +1,107 @@
+package approval
+
+import (
+	"regexp"
+	"strings"
+	"testing"
+	"unicode/utf8"
+)
+
+// FuzzExactMatcherSelfMatch asserts the two properties an exact matcher must
+// hold for any command: it matches the command it was generated from, and it
+// matches nothing else. The '+' defect violated both at once — the generated
+// pattern failed against its own source while matching shorter variants — and
+// went unnoticed because the hand-written corpus happened to contain no '+'.
+func FuzzExactMatcherSelfMatch(f *testing.F) {
+	seeds := []string{
+		"echo a+b",
+		"x++",
+		"a.+b",
+		"deploy --tag v1+build",
+		`docker exec c python -c "g=lambda p:u.urlopen(b+p,timeout=30)"`,
+		"systemctl status nginx",
+		"ls -la /var",
+		"日志 --tail 20",
+		"",
+		" ",
+	}
+	// Every byte quoteBytesForRegexp may emit verbatim, so a future edit to the
+	// allowlist is exercised here as well as in the table test.
+	for i := 0; i < len(literalSafePunct); i++ {
+		seeds = append(seeds, "a"+string(rune(literalSafePunct[i]))+"b")
+	}
+	for _, seed := range seeds {
+		f.Add(seed)
+	}
+
+	f.Fuzz(func(t *testing.T, command string) {
+		// NUL and invalid UTF-8 are rejected by contract (a matcher cannot
+		// express a lone invalid byte), and long inputs only slow the mutation
+		// sweep below without reaching new generator paths.
+		if strings.ContainsRune(command, 0) || !utf8.ValidString(command) || len(command) > 256 {
+			t.Skip()
+		}
+		matcher, err := Exact(command)
+		if err != nil {
+			t.Fatalf("Exact(%q): %v", command, err)
+		}
+		expr, err := regexp.Compile(matcher.Regex)
+		if err != nil {
+			t.Fatalf("generated regex does not compile: %v regex=%s", err, matcher.Regex)
+		}
+		if !expr.MatchString(command) {
+			t.Fatalf("matcher does not match its source command %q: regex=%s", command, matcher.Regex)
+		}
+		// Exactness: no single-byte edit of the command may still match. Each
+		// mutation changes the length by exactly one, so a mutant can never
+		// coincide with the original and there are no false failures.
+		limit := min(len(command), 64)
+		for i := 0; i < limit; i++ {
+			mutations := []string{
+				command[:i] + command[i+1:],                // delete byte i
+				command[:i] + command[i:i+1] + command[i:], // duplicate byte i
+				command[:i] + "X" + command[i:],            // insert at i
+			}
+			for _, mutation := range mutations {
+				if expr.MatchString(mutation) {
+					t.Fatalf("matcher for %q also matched %q: regex=%s", command, mutation, matcher.Regex)
+				}
+			}
+		}
+	})
+}
+
+// FuzzGeneralizeNoWiden covers the prefix path, where a matcher legitimately
+// matches more than its source: it must still never extend across a command
+// separator into an unapproved command.
+func FuzzGeneralizeNoWiden(f *testing.F) {
+	for _, seed := range []string{
+		"systemctl status nginx",
+		"ls -la /var",
+		"git diff HEAD",
+		"kubectl get pods",
+		"echo a+b",
+	} {
+		f.Add(seed)
+	}
+	f.Fuzz(func(t *testing.T, command string) {
+		if strings.ContainsRune(command, 0) || !utf8.ValidString(command) || len(command) > 256 {
+			t.Skip()
+		}
+		for _, mode := range []HostGrantMode{HostGrantExact, HostGrantSafePrefix, HostGrantPrefix} {
+			matcher, err := Generalize(command, mode)
+			if err != nil {
+				t.Fatalf("Generalize(%q, %s): %v", command, mode, err)
+			}
+			matched, err := matcher.Match(command)
+			if err != nil || !matched {
+				t.Fatalf("matcher does not match its source command %q (mode %s): matched=%v err=%v regex=%s", command, mode, matched, err, matcher.Regex)
+			}
+			for _, suffix := range []string{"; rm -rf /", "\nid", " | id", " && id", "`id`"} {
+				if matched, err := matcher.Match(command + suffix); err != nil || matched {
+					t.Fatalf("matcher for %q (mode %s) reached across %q: matched=%v err=%v regex=%s", command, mode, suffix, matched, err, matcher.Regex)
+				}
+			}
+		}
+	})
+}