feat: three-mode safety policy (strict / balanced / autonomous) with native shell classification - #3835
Conversation
…native shell classification
Tool approval today is a coarse two-option choice: approve a call once,
or approve everything for the rest of the session. Prompt fatigue
pushes people to the blanket option without them realising it also
covers destructive commands. There is no middle ground, and no obvious
way back. The shell classifier (safer: true) was also entangled with
the approval verdict, which made its judgement impossible to reuse.
Introduce three explicit safety modes and split the classifier into a
pure labeller so the mode is what actually gates the call:
safe destructive unknown
strict ask ask ask
balanced ALLOW ask ask
autonomous ALLOW ALLOW ALLOW
Every call gets a safe / destructive / unknown label from a new
dependency-free pkg/safety package (shell commands via the pattern
taxonomy, other tools via their MCP annotations), and the pipeline
becomes: preempt hooks -> custom rules -> mode x label -> default
hooks -> ask user. Custom rules always win over the mode; session
ask rules always prompt while agent-author ask rules yield to a
user-chosen auto-approving mode.
Escalation is guided and reversible: confirmation prompts offer the
next rung, a new --safety flag and the PATCH safety-policy endpoint
set or undo the mode at any time, and downgrading genuinely revokes
the blanket approval (the yolo toggle restores the pre-escalation
mode, surviving branch and fork).
Compatibility: legacy policy values (unsafe / safer / safe-auto) and
resume verbs are accepted and normalized; safer maps to balanced,
which is deliberately narrower (unknown commands now prompt). Sessions
that never choose a mode keep the historical default. The shell
toolset's safer: flag stays parseable but is ignored; the safer_shell
builtin degrades to a metadata-only labeller and warns at config build
time. Confirmation events and the on_tool_approval_decision hook now
carry safety_label. Resuming a session stored as autonomous keeps
autonomous without re-passing --yolo; an explicit --safety on resume
overrides.
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The three-mode safety policy implementation (strict / balanced / autonomous) is well-structured. The new pkg/safety package provides a clean, dependency-free classifier with thread-safe pattern compilation via sync.OnceValues. Legacy resume verbs (approve-session, approve-safe, approve-safer) are properly normalized through NormalizeResumeType before the policy switch, so backward compatibility is maintained. The safer_shell deprecation path is intentional per its documentation. No bugs introduced by this PR were confirmed.
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The three-mode safety policy (strict / balanced / autonomous) is well-implemented. The (mode × label) decision table in pkg/runtime/toolexec/permissions.go is correct and exhaustively tested. The classifier in pkg/safety/ is conservative by design (unknown = ClassUnknown, compound shell commands blocked, deny-flag list). Session state transitions are mutex-guarded. Legacy wire compatibility (unsafe/safer/safe-auto normalization) is correctly implemented throughout the pipeline.
The permission pipeline change — where team-tier ForceAsk rules yield to a user-chosen Autonomous mode while session-tier ForceAsk always prompts — is consistent with the pre-PR behavior (old yolo bypassed all ForceAsk), and the PR actually improves it by preserving session-tier ForceAsk even under Autonomous. This design decision is correctly documented in code comments.
|
Don't merge yet there are still a few bugs. I am currently correcting it |
Why
Tool approval today is a coarse two-option choice. When a call needs confirmation, the user can approve it once — or approve everything for the rest of the session. Prompt fatigue pushes people to the blanket option without them realising it also covers destructive commands (
rm -rf,docker volume rm, …). There is no middle ground, and no obvious way back.The shell classifier (
safer: true) was also entangled with the approval verdict: it read the session policy and decided whether to ask, which made its judgement impossible to reuse and every new approval behavior an edit to the classifier.What this PR does
Introduce three explicit safety modes and split the classifier into a pure labeller so the mode is what actually gates the call:
--yolo: everything runs.Every call gets a
safe/destructive/unknownlabel from a new dependency-freepkg/safetypackage (shell commands via the pattern taxonomy, other tools via their MCP annotations), and the pipeline becomes:Custom rules always win over the mode — a
deny:rule blocks even autonomous, anallow:rule silences even strict. One deliberate asymmetry: session-scopedask:rules (direct user intent) always prompt, while agent-authorask:rules yield to a user-chosen auto-approving mode.Escalation is guided and reversible: confirmation prompts offer the next rung ([B]alanced on safe calls, approve-all on risky ones), a new
--safetyflag and the existingPATCH /sessions/:id/safety-policyendpoint set or undo the mode at any time, and downgrading genuinely revokes the blanket approval.Compatibility
unsafe/safer/safe-auto) and resume verbs (approve-session,approve-safe,approve-safer) are accepted and normalized, so existing API consumers keep working. One deliberate nuance:safermaps tobalanced, which is narrower — oldsaferwaved unrecognised commands through silently,balancedprompts for them. The mapping errs toward asking; it never widens.strictis stricter than that on purpose.safer:flag stays parseable but is ignored — classification now always runs. YAMLs pinning thesafer_shellbuiltin keep working; it degrades to a metadata-only labeller.on_tool_approval_decisionhook now carrysafety_label, so host applications can render risk tiers without re-implementing the taxonomy.autonomouskeeps it autonomous without re-passing--yolo. On main, blanket approval had to be re-requested on every run. An explicit--safetyon resume still overrides the stored mode.Supersedes #3806 (same goal; this version keeps the decision table authoritative — no hidden read-only fast path under explicit strict — moves the classifier out of the hook lane into a runtime component, and preserves wire compatibility for configs, resume verbs, and persisted sessions).