Skip to content

Commit 62add72

Browse files
committed
fix(permission): aliases in Evaluate, base rules, fail-closed defaults (#543-#549)
- bash↔shell and list↔glob aliases applied in Evaluate/Ask - Path/command Ask patterns derived from tool definitions - SetBaseRules called in TUI/serve/ACP (matching run mode) - Session deny→reject normalization; 400 on unknown action - Path pattern parsing; todowrite under deny-edit rules - Always = session-only scope (documented; warn if no store) - Empty Patterns fail-closed; doom_loop/guardrail docs honest
1 parent d25b40c commit 62add72

19 files changed

Lines changed: 425 additions & 73 deletions

‎cmd/tinycode/acp.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ func runACP(args []string) {
7171
}
7272

7373
toolReg, permSvc, toolCtx := initTooling(b, dir, cfg)
74+
applyConfigPermissions(permSvc, toolReg, cfg)
7475

7576
lspMgr := initLSP(dir, cfg, toolReg)
7677
defer lspMgr.Close()

‎cmd/tinycode/config.go‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,7 @@ func initAgentRegistry(cfg *config.Info, directory string) *agent.Registry {
199199
}
200200
var userPerms permission.Ruleset
201201
if cfg.Permission != nil {
202-
for _, a := range cfg.Permission.Allow {
203-
userPerms = append(userPerms, permission.Rule{Permission: a, Pattern: "*", Action: permission.ActionAllow})
204-
}
205-
for _, d := range cfg.Permission.Deny {
206-
userPerms = append(userPerms, permission.Rule{Permission: d, Pattern: "*", Action: permission.ActionDeny})
207-
}
202+
userPerms = permission.FromConfig(cfg.Permission.Allow, cfg.Permission.Deny)
208203
}
209204

210205
reg := agent.NewRegistry()
@@ -254,6 +249,18 @@ func initTooling(b *bus.Bus, directory string, cfg ...*config.Info) (*tool.Regis
254249
return toolReg, permSvc, toolCtx
255250
}
256251

252+
// applyConfigPermissions wires config allow/deny into the permission service
253+
// and disables tools that are globally denied. Shared by run, tui, serve, and acp.
254+
func applyConfigPermissions(permSvc *permission.Service, toolReg *tool.Registry, cfg *config.Info) {
255+
if cfg == nil || cfg.Permission == nil {
256+
return
257+
}
258+
configRules := permission.FromConfig(cfg.Permission.Allow, cfg.Permission.Deny)
259+
permSvc.SetBaseRules(configRules)
260+
disabled := permission.Disabled(toolReg.List(), configRules)
261+
toolReg.SetDisabled(disabled)
262+
}
263+
257264
func initLSP(dir string, cfg *config.Info, toolReg *tool.Registry) *lsp.Manager {
258265
var lspCfg *lsp.Config
259266
if cfg.LSP != nil {

‎cmd/tinycode/run.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,7 @@ func runRun() {
110110
lspMgr := initLSP(dir, cfg, toolReg)
111111
defer lspMgr.Close()
112112

113-
if cfg.Permission != nil {
114-
configRules := permission.FromConfig(cfg.Permission.Allow, cfg.Permission.Deny)
115-
permSvc.SetBaseRules(configRules)
116-
disabled := permission.Disabled(toolReg.List(), configRules)
117-
toolReg.SetDisabled(disabled)
118-
}
113+
applyConfigPermissions(permSvc, toolReg, cfg)
119114

120115
permReplyCh := setupRunPermissions(b, permSvc, *skipPermsFlag, *interactiveFlag, *permsFlag)
121116

‎cmd/tinycode/serve.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ func runServe() {
4444
agentReg := initAgentRegistry(cfg, dir)
4545

4646
toolReg, permSvc, toolCtx := initTooling(b, dir, cfg)
47+
applyConfigPermissions(permSvc, toolReg, cfg)
4748

4849
lspMgr := initLSP(dir, cfg, toolReg)
4950
defer lspMgr.Close()
@@ -129,6 +130,7 @@ func runWeb() {
129130
agentReg := initAgentRegistry(cfg, dir)
130131

131132
toolReg, permSvc, toolCtx := initTooling(b, dir, cfg)
133+
applyConfigPermissions(permSvc, toolReg, cfg)
132134

133135
lspMgr := initLSP(dir, cfg, toolReg)
134136
defer lspMgr.Close()

‎cmd/tinycode/tui.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func runTUI(args []string) {
5858
}
5959

6060
toolReg, permSvc, toolCtx := initTooling(b, dir, cfg)
61+
applyConfigPermissions(permSvc, toolReg, cfg)
6162

6263
lspMgr := initLSP(dir, cfg, toolReg)
6364
defer lspMgr.Close()

‎docs/spec/12-permissions.md‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ type Ruleset []Rule
3232

3333
### Edit Tool Normalization
3434

35-
Tools `edit`, `write`, and `apply_patch` all map to the `"edit"` permission when checking disabled status via `Disabled()`. This ensures a single `deny edit *` rule blocks all file-writing tools.
35+
Tools `edit`, `write`, `apply_patch`, and `todowrite` all map to the `"edit"` permission when checking disabled status via `Disabled()`. This ensures a single `deny edit *` rule blocks all file-writing tools.
36+
37+
### Permission Aliases
38+
39+
`bash`↔`shell` and `list`↔`glob` are aliases. `Evaluate` and `Disabled` expand both sides so a rule written as `allow bash *` matches an Ask with permission `shell` (and vice versa).
3640

3741
## 12.2 Default Rules
3842

@@ -41,13 +45,13 @@ var DefaultRules = Ruleset{
4145
{Permission: "read", Pattern: "*", Action: ActionAllow},
4246
{Permission: "read", Pattern: ".env*", Action: ActionAsk},
4347
{Permission: "webfetch", Pattern: "*", Action: ActionAsk},
44-
{Permission: "doom_loop", Pattern: "*", Action: ActionAsk},
45-
{Permission: "guardrail", Pattern: "*", Action: ActionAsk},
4648
{Permission: "external_directory", Pattern: "*", Action: ActionAsk},
4749
}
4850
```
4951

50-
DefaultRules are automatically prepended during evaluation. They establish the baseline: file reads are allowed, but `.env*` files, web fetches, doom loop overrides, guardrail bypasses, and external directory access all require user approval.
52+
DefaultRules are automatically prepended during evaluation. They establish the baseline: file reads are allowed, but `.env*` files, web fetches, and external directory access all require user approval.
53+
54+
Doom-loop detection is a hard-stop in the session processor (`checkDoomLoop`), not a permission Ask. Destructive shell commands use the `destructive-shell` permission from the shell/monitor tools.
5155

5256
## 12.3 Evaluation
5357

@@ -87,7 +91,7 @@ Concatenates multiple rulesets into a single flat slice. Order is preserved -- e
8791
func Disabled(tools []string, ruleset Ruleset) map[string]bool
8892
```
8993

90-
Returns the set of tool names that are globally denied (pattern `"*"`, action `"deny"`). For tools in the `editTools` map (`edit`, `write`, `apply_patch`), the permission is normalized to `"edit"` before checking.
94+
Returns the set of tool names that are globally denied (pattern `"*"`, action `"deny"`). For tools in the `editTools` map (`edit`, `write`, `apply_patch`, `todowrite`), the permission is normalized to `"edit"` before checking.
9195

9296
## 12.4 Wildcard Matching
9397

@@ -199,23 +203,26 @@ type Service struct {
199203
```
200204
1. Lock mutex
201205
2. If service is closed -> return ErrClosed
202-
3. For each pattern in input.Patterns:
206+
3. If Patterns is empty -> unlock, return ErrDenied (fail closed)
207+
4. For each pattern in input.Patterns:
203208
a. Evaluate(permission, pattern, baseRules, input.Ruleset, approved)
204209
b. If any rule -> deny: unlock, return DeniedError (with matching rules)
205210
c. If any rule -> ask: mark needsAsk = true
206-
4. If all rules -> allow: unlock, return nil
207-
5. Generate request ID (ascending sortable, "permission" prefix)
208-
6. Create pending entry with buffered reply channel (cap 1)
209-
7. Store in pending map
210-
8. Unlock mutex
211-
9. Publish "permission.asked" event on bus
212-
10. Select:
211+
5. If all rules -> allow: unlock, return nil
212+
6. Generate request ID (ascending sortable, "permission" prefix)
213+
7. Create pending entry with buffered reply channel (cap 1)
214+
8. Store in pending map
215+
9. Unlock mutex
216+
10. Publish "permission.asked" event on bus
217+
11. Select:
213218
- Reply received on channel -> return result.err
214219
- Context cancelled -> clean up pending entry, return ctx.Err()
215220
```
216221

217222
The ask is a blocking call -- the goroutine waits on a channel until either the user replies or the context is cancelled.
218223

224+
Tool execution passes path or command strings as Patterns (via `askPatterns` in `internal/tool`), not just the tool name, so rules like `read .env*` match correctly.
225+
219226
## 12.7 Reply Flow
220227

221228
`Service.RespondToAsk(input)` handles user replies.
@@ -258,10 +265,14 @@ type RuleStore interface {
258265
### Lifecycle
259266

260267
- `SetStore(store, projectID)` configures persistence and loads any previously saved rules into the `approved` list
261-
- "Always" approvals are saved on each `ReplyAlways` response
268+
- "Always" approvals are saved on each `ReplyAlways` response when a store is configured
262269
- Rules are scoped to a project (identified by `projectID`)
263270
- Persisted rules survive process restarts
264271

272+
### Session-only "always" (current default)
273+
274+
No `RuleStore` is wired in TUI/serve/ACP/run by default. In that case, `"always"` approvals live in the in-memory `approved` list for the lifetime of the process only — they do not survive restarts. `RespondToAsk` logs a warning when persisting without a store so this is not silent.
275+
265276
## 12.9 Config Integration
266277

267278
```go

‎docs/spec/15-security.md‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,22 +184,20 @@ See [12-permissions.md](12-permissions.md) for the full permission system specif
184184

185185
### Default Rules
186186

187-
6 rules applied as the base layer for every session:
187+
4 rules applied as the base layer for every session:
188188

189189
| Permission | Pattern | Action | Security Purpose |
190190
|------------|---------|--------|-----------------|
191191
| `read` | `*` | allow | Broad read access for tools |
192192
| `read` | `.env*` | ask | Protects secret files from silent reads |
193193
| `webfetch` | `*` | ask | Gates all external HTTP requests |
194-
| `doom_loop` | `*` | ask | User confirmation on loop detection |
195-
| `guardrail` | `*` | ask | Safety guardrail confirmation |
196194
| `external_directory` | `*` | ask | Protects paths outside project directory |
197195

198196
### Doom Loop Detection
199197

200-
Source: `internal/session/processor_validation.go`
198+
Source: `internal/session/processor_validation.go`, `internal/session/processor_loop.go`
201199

202-
`isDoomLoop()` detects when the last N tool calls have identical signatures (same tool name and arguments). Default threshold: 3 consecutive identical calls (`defaultDoomThreshold`). Triggers `permission.Ask()` with `"doom_loop"` permission.
200+
`isDoomLoop()` detects when the last N tool calls have identical signatures (same tool name and arguments). Default threshold: 3 consecutive identical calls (`defaultDoomThreshold`). This is a hard-stop in the processor (`checkDoomLoop` returns an error) — it does not go through `permission.Ask`.
203201

204202
### Consecutive Failure Tracking
205203

‎internal/agent/agent.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,9 +639,10 @@ func permissionFromConfigMap(m map[string]any) permission.Ruleset {
639639
for key, val := range m {
640640
switch v := val.(type) {
641641
case string:
642+
perm, pattern := splitPermissionKey(key)
642643
rules = append(rules, permission.Rule{
643-
Permission: key,
644-
Pattern: "*",
644+
Permission: perm,
645+
Pattern: pattern,
645646
Action: permission.Action(v),
646647
})
647648
case map[string]any:
@@ -660,3 +661,13 @@ func permissionFromConfigMap(m map[string]any) permission.Ruleset {
660661
}
661662
return rules
662663
}
664+
665+
// splitPermissionKey splits "permission pattern" on the first space.
666+
// When no space is present, the pattern defaults to "*".
667+
func splitPermissionKey(s string) (permission, pattern string) {
668+
idx := strings.IndexByte(s, ' ')
669+
if idx < 0 {
670+
return s, "*"
671+
}
672+
return s[:idx], s[idx+1:]
673+
}

‎internal/agent/agent_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,4 +490,41 @@ func TestNativeAgentPermissions(t *testing.T) {
490490
if rule.Action != permission.ActionDeny {
491491
t.Errorf("explore should deny edit via * deny, got %s", rule.Action)
492492
}
493+
494+
// Explore allow bash * should match shell Ask (alias).
495+
rule = permission.Evaluate("shell", "ls", explore.Permission)
496+
if rule.Action != permission.ActionAllow {
497+
t.Errorf("explore should allow shell via bash alias, got %s", rule.Action)
498+
}
499+
}
500+
501+
func TestPermissionFromConfigMap_PathPatterns(t *testing.T) {
502+
rules := permissionFromConfigMap(map[string]any{
503+
"read .env*": "ask",
504+
"edit": "deny",
505+
"bash": map[string]any{
506+
"rm *": "deny",
507+
},
508+
})
509+
510+
var foundEnv, foundEdit, foundBash bool
511+
for _, r := range rules {
512+
switch {
513+
case r.Permission == "read" && r.Pattern == ".env*" && r.Action == permission.ActionAsk:
514+
foundEnv = true
515+
case r.Permission == "edit" && r.Pattern == "*" && r.Action == permission.ActionDeny:
516+
foundEdit = true
517+
case r.Permission == "bash" && r.Pattern == "rm *" && r.Action == permission.ActionDeny:
518+
foundBash = true
519+
}
520+
}
521+
if !foundEnv {
522+
t.Error("expected read .env* ask from space-separated key")
523+
}
524+
if !foundEdit {
525+
t.Error("expected edit * deny")
526+
}
527+
if !foundBash {
528+
t.Error("expected bash rm * deny from nested map")
529+
}
493530
}

‎internal/permission/defaults.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ package permission
22

33
// DefaultRules provides a base ruleset applied before user or agent rules.
44
// It allows read operations, and requires user approval for sensitive
5-
// patterns and safety-related permissions.
5+
// patterns and external directory access.
6+
//
7+
// Note: doom_loop is enforced as a hard-stop in the session processor
8+
// (checkDoomLoop), not via permission.Ask. Destructive shell commands use
9+
// the "destructive-shell" permission from the shell/monitor tools.
610
var DefaultRules = Ruleset{
711
{Permission: "read", Pattern: "*", Action: ActionAllow},
812
{Permission: "read", Pattern: ".env*", Action: ActionAsk},
913
{Permission: "webfetch", Pattern: "*", Action: ActionAsk},
10-
{Permission: "doom_loop", Pattern: "*", Action: ActionAsk},
11-
{Permission: "guardrail", Pattern: "*", Action: ActionAsk},
1214
{Permission: "external_directory", Pattern: "*", Action: ActionAsk},
1315
}

0 commit comments

Comments
 (0)