diff --git a/cmd/mavend/actions_act.go b/cmd/mavend/actions_act.go index 2b96855..f3395dc 100644 --- a/cmd/mavend/actions_act.go +++ b/cmd/mavend/actions_act.go @@ -51,6 +51,12 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st phrase := actPhrase(dec.Slots.Fn, dec.Slots.Args) h.park(dec.Slots.Fn, dec.Slots.Args, phrase) return "выполнить «" + phrase + "»? скажи «да» или «нет»." + case errors.Is(err, tool.ErrNeedsAuthedSurface): + // Irreversible (internal/tool/risk.go). A confirm turn would not + // help: everything that proposed this act — the STT, the router, + // the fuzzy allowlist match — is a guess, and a spoken "да" checks + // none of it. She names the gap instead. + return "это я из голоса не выполню — после него ничего не вернуть. запусти сам, если правда надо." case errors.Is(err, tool.ErrNotEnabled): return h.proposeGap(ctx, dec) case errors.Is(err, tool.ErrNotConnected), errors.Is(err, mcp.ErrNotConnected), errors.Is(err, mcp.ErrNoServer): diff --git a/docs/design.md b/docs/design.md index 4d204ab..f29a2ad 100644 --- a/docs/design.md +++ b/docs/design.md @@ -293,6 +293,40 @@ don't improvise.** Destructive ones still gate behind confirm. Misroute correction is append-only and grows the router's examples with use — same shape as `nudges.outcome` tuning cooldowns, no retrain. +#### Risk tiers, not one boolean + +`Destructive` on a tool row is one bit set by whoever ticked the checkbox on +`/tools`. It is a mechanism, and it never said which acts are destructive, +whether a confirmed act stays confirmed, or what a new tool domain inherits. +`internal/tool/risk.go` is the policy (Vikunja #449). The tier is DERIVED from +the row, not stored, so it can be argued with in one place instead of being +whatever the last person to enable the tool believed. + +| Tier | What it is | What it costs | +|---|---|---| +| `safe` | a read, or a change he can undo by saying the opposite | runs on first hearing | +| `destructive` | it changes something real and undoing it takes work | one confirm turn, every time | +| `irreversible` | the thing does not come back: a wipe, a format, a delete with no bin | voice may not authorise it at all | + +Three rules fall out, and they are the part that was missing: + +- **Which acts are destructive is not only the checkbox.** A house row always + is, because there is no read-only way to turn the heating off. A row whose + argv names one of the irreversible verbs always is, whatever the row says. +- **A confirmed act never stays confirmed.** At any tier. A confirmation binds + one capability, one target and one argument list, and it dies with the parked + turn (90s). "The same act again" is a new act. A sticky confirm is a standing + grant and nothing on the voice path may hold one. +- **A new domain inherits `destructive`, not `safe`.** A dispatch shape the + policy does not recognise gets the confirm turn. A domain argues its way down + to running freely; it never has to argue its way up to being gated. + +The irreversible tier is refused rather than asked about, because a confirm +turn would be theatre: everything that proposed the act — an STT guess, a +router guess, a fuzzy allowlist match — is a guess, and a spoken "да" checks +none of it. She names the gap and he runs it himself. The row stays enabled; +refusing to run it from voice is not the same as taking it off the allowlist. + --- ## Voice pipeline (STT / TTS) diff --git a/internal/tool/risk.go b/internal/tool/risk.go new file mode 100644 index 0000000..cb1ad38 --- /dev/null +++ b/internal/tool/risk.go @@ -0,0 +1,145 @@ +package tool + +import ( + "strings" + + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/mcp" + "github.com/kami/maven/internal/smarthome" +) + +// Risk tiers (Vikunja #449). +// +// What existed before this file was a mechanism and no policy: one +// `Destructive` boolean per row, set by whoever ticked the checkbox on /tools. +// Nothing said which acts are destructive, whether a confirmed act stays +// confirmed, or what a new tool domain inherits — so every domain answered +// those questions for itself, and two of them answered differently. +// +// The tiers below are the policy. They are derived from the row, not stored: +// a derivation can be argued with and corrected in one place, while a column +// is whatever the last person to enable the tool believed. +// +// The three questions, answered once: +// +// - WHICH ACTS ARE DESTRUCTIVE. A house row always is, because there is no +// read-only way to turn the heating off. A row whose argv names one of the +// irreversible verbs always is, whatever the checkbox says. Everything else +// is what the row was enabled as. +// - DOES A CONFIRMED ACT STAY CONFIRMED. No. Never, at any tier. A +// confirmation binds one capability, one target and one argument list, and +// it expires with the parked turn (confirmTTL, 90s). "Same act again" is a +// new act and costs a new turn. A sticky confirm is a standing grant, and +// nothing on the voice path may hold one. +// - WHAT A NEW DOMAIN INHERITS. The default is TierDestructive, not +// TierSafe. A dispatch shape this file does not recognise gets the confirm +// turn — a new domain must argue its way DOWN to running freely, never up +// to needing a confirm. +type Risk string + +const ( + // TierSafe — a read, or a mutation the owner can undo by saying the + // opposite. Runs on first hearing. + TierSafe Risk = "safe" + // TierDestructive — it changes something real and undoing it takes work. + // One confirm turn, every time, never remembered. + TierDestructive Risk = "destructive" + // TierIrreversible — the thing it acts on does not come back: a wipe, a + // format, a delete with no bin behind it. A confirm turn is not enough, + // because the whole chain that proposed it — an STT guess, a router guess, + // a fuzzy allowlist match — has a spoken "да" as its only check. She names + // the gap and he runs it himself. + TierIrreversible Risk = "irreversible" +) + +// Policy — what a tier requires of the act path. +// +// There is deliberately no "sticky for" field. Non-stickiness is the policy, +// and a knob that could turn it off would be the thing to argue with instead +// of the rule. +type Policy struct { + // Confirm — the act does not run on first hearing. + Confirm bool + // VoiceMayRun — a spoken confirmation is enough authority to run it. + VoiceMayRun bool +} + +// PolicyFor returns the requirements of a tier. An unknown tier is treated as +// destructive, for the same reason the default derivation is. +func PolicyFor(r Risk) Policy { + switch r { + case TierSafe: + return Policy{Confirm: false, VoiceMayRun: true} + case TierIrreversible: + return Policy{Confirm: true, VoiceMayRun: false} + default: + return Policy{Confirm: true, VoiceMayRun: true} + } +} + +// irreversibleVerbs — argv heads and subcommands that destroy the thing they +// name. Matched as whole argv elements, never as substrings: "rm" must not +// fire on "/usr/bin/rmdir-report" and "drop" must not fire on "dropbox". +// +// The list is short on purpose. It is not a sandbox and it does not try to be +// one — an enabled row can already run anything the daemon's user can run. +// What it is, is the set of words that mean "and then it is gone", so that the +// one act nobody can walk back is the one act a spoken "да" cannot authorise. +var irreversibleVerbs = map[string]bool{ + "rm": true, "rmdir": true, "shred": true, "srm": true, + "mkfs": true, "fdisk": true, "parted": true, "wipefs": true, + "dd": true, "format": true, + "drop": true, "drop-database": true, "destroy": true, "purge": true, + "prune": true, "truncate": true, +} + +// RiskOf derives the tier of an enabled tool row. +func RiskOf(t ipc.Tool) Risk { + if isIrreversible(t.Cmd) { + return TierIrreversible + } + // A house row is a physical change to the flat, and the confirm turn on it + // is structural rather than a column: /tools writes the checkbox straight + // through on enable, so unticking it once turned an unlock into a row that + // ran on first hearing. Nothing any surface writes removes the second turn + // from a physical device. + if _, _, ok := smarthome.ParseCmd(t.Cmd); ok { + return TierDestructive + } + // An MCP row is a call to somebody else's server. It is enabled with a + // fingerprint of what it declared at approval time (Vikunja #251), and the + // tier tracks the same flag every other row uses — the point of this branch + // is that it is NOT special-cased into running freely. + if _, _, ok := mcp.ParseCmd(t.Cmd); ok { + if t.Destructive { + return TierDestructive + } + return TierSafe + } + if t.Destructive { + return TierDestructive + } + if len(t.Cmd) == 0 { + // Not a shape this file knows how to read. The default is the confirm + // turn: a new domain argues its way down, not up. + return TierDestructive + } + return TierSafe +} + +// isIrreversible reports whether any argv element is one of the verbs that +// destroys what it names. Every element, not just the head: "sudo rm" and +// "docker volume prune" both hide the verb behind a wrapper. +func isIrreversible(cmd []string) bool { + for _, arg := range cmd { + word := strings.ToLower(strings.TrimSpace(arg)) + // Take the last path element, so /bin/rm reads as rm. + if i := strings.LastIndex(word, "/"); i >= 0 { + word = word[i+1:] + } + if irreversibleVerbs[word] { + return true + } + } + return false +} diff --git a/internal/tool/tool.go b/internal/tool/tool.go index d948d58..6b27924 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -67,6 +67,12 @@ var ( // proposal, and drafting a new proposal for a tool that already exists and // is enabled is a lie about what is wrong. ErrNotConnected = errors.New("tool is enabled but its backend is not connected") + // ErrNeedsAuthedSurface — the row is enabled and the act is understood, + // and its tier is one a spoken "да" may not authorise (risk.go, + // TierIrreversible). Held apart from ErrNeedsConfirm because there is no + // confirm turn that would help: asking again would imply the second answer + // changes the outcome. + ErrNeedsAuthedSurface = errors.New("tool is irreversible and voice may not authorise it") ) // MCPCaller is the seam for an act that is an MCP tool call rather than a @@ -121,7 +127,12 @@ func (e *Executor) WithHome(h HomeCaller) *Executor { // Exec looks up name in the store and runs Cmd+args as argv (no shell). // confirmed=true is the second turn of a destructive act (the user said "да"); // it bypasses the ErrNeedsConfirm gate. Non-enabled ⇒ ErrNotEnabled; a -// destructive tool with confirmed=false ⇒ ErrNeedsConfirm. +// destructive tool with confirmed=false ⇒ ErrNeedsConfirm; an irreversible one +// ⇒ ErrNeedsAuthedSurface, confirmed or not. +// +// Exec IS the voice path. Nothing else calls it, which is why the tier check +// needs no surface argument: the authority it can offer a tool is a spoken +// "да", and TierIrreversible says that is not enough. func (e *Executor) Exec(ctx context.Context, name string, args []string, confirmed bool) (string, error) { t, err := e.api.LookupTool(ctx, name) if errors.Is(err, ipc.ErrToolNotFound) { @@ -133,7 +144,15 @@ func (e *Executor) Exec(ctx context.Context, name string, args []string, confirm if t.Status != "enabled" { return "", ErrNotEnabled } - if t.Destructive && !confirmed { + // The tier decides, not the column (Vikunja #449). RiskOf reads the row and + // answers the three questions the boolean never did: which acts are + // destructive, whether a confirm sticks (it never does), and what an + // unrecognised shape inherits (the confirm turn). + policy := PolicyFor(RiskOf(t)) + if !policy.VoiceMayRun { + return "", ErrNeedsAuthedSurface + } + if policy.Confirm && !confirmed { return "", ErrNeedsConfirm } // An MCP row is a call to a configured server, not a process. Everything