Skip to content

feat(security): add gated_commands TOTP gate for shell tool (phase 1, closes #3767)#5779

Open
DjaPy wants to merge 3 commits intozeroclaw-labs:masterfrom
DjaPy:feat/otp-gated-commands-issue-3767
Open

feat(security): add gated_commands TOTP gate for shell tool (phase 1, closes #3767)#5779
DjaPy wants to merge 3 commits intozeroclaw-labs:masterfrom
DjaPy:feat/otp-gated-commands-issue-3767

Conversation

@DjaPy
Copy link
Copy Markdown

@DjaPy DjaPy commented Apr 15, 2026

Summary

  • Base branch target: master
  • Problem: security.otp.gated_actions gates the entire shell tool, but there is no way to allow shell access in general while requiring TOTP only for specific destructive/admin commands (e.g. sudo *, rm -rf *, systemctl restart *).
  • Why it matters: Operators who trust their agent for everyday shell tasks (read, grep, cargo) still want a hard 2FA gate before elevated or irreversible operations — without disabling shell access entirely.
  • What changed: Added security.otp.gated_commands — a glob-style pattern list that requires a TOTP code via the new otp_code shell parameter before executing matching commands.
  • What did not change: gated_actions, gated_domains, estop, channel approval flow, Phase 2 (config immutability), Phase 3 (financial gating). security.otp.enabled = false (default) preserves all existing behaviour.

Label Snapshot (required)

  • Risk label (risk: low|medium|high): risk: medium
  • Size label (size: XS|S|M|L|XL, auto-managed/read-only): size: M
  • Scope labels: security, tool, config
  • Module labels: tool: shell, config: otp
  • Contributor tier label: auto-managed
  • If any auto-label is incorrect, note requested correction: N/A

Change Metadata

  • Change type: feature
  • Primary scope: security

Linked Issue

Supersede Attribution (required when Supersedes # is used)

N/A

Validation Evidence (required)

cargo fmt --all -- --check   → pass (fmt applied)
cargo clippy --all-targets -- -D warnings  → pass (0 errors, 0 warnings)
cargo build  → Finished dev profile in ~61s
cargo test --workspace  → all tests pass (0 failed)
cargo test -p zeroclaw-config -- policy::tests::gated  → 7/7 pass
cargo test -p zeroclaw-config -- policy::tests::with_gated  → 1/1 pass
cargo test -p zeroclaw-runtime -- shell  → 64/64 pass
  • Evidence provided: test run output above
  • If any command is intentionally skipped, explain why: N/A

Security Impact (required)

  • New permissions/capabilities? Yes — gated_commands allows expressing "allow but require 2FA" for shell commands
  • New external network calls? No
  • Secrets/tokens handling changed? No — OTP secret file path and encryption unchanged
  • File system access scope changed? No
  • Risk and mitigation: The new gate is additive. When gated_commands = [] (default) or security.otp.enabled = false, no behaviour changes. When configured, the gate is enforced before command execution at the tool level — not bypassable via approved=true. An attacker without the TOTP secret cannot satisfy the gate even with physical phone access.

Privacy and Data Hygiene (required)

  • Data-hygiene status: pass
  • Redaction/anonymization notes: No personal data in examples or tests
  • Neutral wording confirmation: Yes

Compatibility / Migration

  • Backward compatible? Yes — gated_commands defaults to empty list; existing configs unaffected
  • Config/env changes? Yes — new optional field security.otp.gated_commands = [...]
  • Migration needed? No

i18n Follow-Through (required when docs or user-facing wording changes)

  • i18n follow-through triggered? No — no user-facing string changes in docs
  • N/A

Human Verification (required)

  • Verified scenarios: empty gated_commands skips all gating; "sudo *" pattern blocks sudo reboot without OTP code and allows it with valid code; pipeline ls | sudo tee /etc/hosts correctly gates; security.otp.enabled = false disables gating entirely
  • Edge cases checked: bare "*" pattern; exact match without wildcard; multi-segment piped command; validator not configured but pattern set (returns config error)
  • What was not verified: live TOTP flow end-to-end on a running instance (requires physical authenticator)

Side Effects / Blast Radius (required)

  • Affected subsystems/workflows: ShellTool, SecurityPolicy, OtpConfig, orchestrator startup, gateway startup
  • Potential unintended effects: If gated_commands is set but security.otp.enabled = false, commands will fail with a clear error message instead of executing — operator must enable OTP to use the feature
  • Guardrails/monitoring for early detection: existing shell tool tests + new is_command_gated unit tests; CI will catch regressions

Agent Collaboration Notes (recommended)

  • Agent tools used: Claude Code (zeroclaw skill)
  • Workflow/plan summary: explored existing OTP/shell architecture → implemented schema → policy → shell → wiring → tests → PR
  • Verification focus: backward compat, no regressions in existing 561 policy tests and 64 shell tests
  • Confirmation: naming + architecture boundaries followed per AGENTS.md and CONTRIBUTING.md

Rollback Plan (required)

  • Fast rollback: security.otp.gated_commands = [] (empty list) disables the feature without redeploy; or set security.otp.enabled = false
  • Feature flags or config toggles: security.otp.enabled and security.otp.gated_commands
  • Observable failure symptoms: shell tool returns "Command requires TOTP verification" error for matched commands

Risks and Mitigations

  • Risk: Operator misconfigures gated_commands to match too broadly (e.g. "*") and gates all shell commands
    • Mitigation: Clear error message explains what to do; setting gated_commands = [] instantly disables; the "*" wildcard matching is documented in the config field docstring

DjaPy and others added 2 commits April 15, 2026 22:30
…loses zeroclaw-labs#3767)

Add `security.otp.gated_commands` — a list of glob-style shell command
patterns that require a valid TOTP code before execution, even when `shell`
is in `auto_approve`. A trailing `*` acts as a suffix glob (e.g. `"sudo *"`
gates any sudo subcommand); no wildcard means exact match.

Changes:
- `OtpConfig.gated_commands: Vec<String>` field with validation
- `SecurityPolicy.gated_commands` + `is_command_gated()` method + `with_gated_commands()` builder
- `matches_gated_pattern()` helper for word-boundary glob matching across pipe/chain segments
- `ShellTool.otp_validator` field + `otp_code` schema param; gated commands return an error
  prompting for the TOTP code, validated via `OtpValidator` on retry
- `all_tools()` builds `OtpValidator` from config and wires it into `ShellTool`
- Orchestrator and gateway propagate `gated_commands` into `SecurityPolicy`
- `prompt_summary()` surfaces gated patterns to the LLM system prompt
- 8 unit tests covering pattern matching, builder, and pipeline segment detection

`security.otp.enabled = false` (default) preserves existing behaviour.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@singlerider
Copy link
Copy Markdown
Collaborator

Hey! CI is failing on Security Audit due to RUSTSEC-2026-0098 and RUSTSEC-2026-0099, two rustls-webpki advisories published 2026-04-14. This is a repo-wide issue — not specific to your PR.

The fix landed in #5786 (merged today). Please merge upstream/master into your branch to pick up the updated .cargo/audit.toml and re-run CI:

git fetch upstream
git merge upstream/master
git push

Sorry for the noise!

Copy link
Copy Markdown
Collaborator

@singlerider singlerider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comprehension summary: Adds security.otp.gated_commands — a glob-style pattern list that requires a TOTP code via a new otp_code shell parameter before executing matching commands. The gate is enforced at the tool level before execution, not bypassable via approved=true. Adds is_command_gated() to SecurityPolicy, wires OtpValidator into ShellTool, and extends OtpConfig schema. Blast radius: shell tool execution path, security policy, config schema, and gateway/orchestrator startup wiring.

Security/performance assessment: The TOTP gate is enforced before execution, not after approval. gated_commands = [] (default) and security.otp.enabled = false (default) both preserve all existing behavior. No timing attack mitigation visible in the TOTP comparison, but this is inherited from the existing OTP infrastructure. Routing to @JordanTheJet for full security review as previously noted. See inline comment for a credential-in-logs concern.

[blocking] CI Required Gate and Security Required Gate are failing. Security Audit is failing on RUSTSEC-2026-0098 and RUSTSEC-2026-0099 (repo-wide rustls-webpki advisories, not PR-specific). The fix landed in master via #5786. Please merge upstream master and push to clear CI.

// Enforce gated_commands: commands matching configured patterns require
// a valid TOTP code supplied via the `otp_code` parameter.
if self.security.is_command_gated(command) {
let otp_code = args
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[blocking] otp_code is part of the args: serde_json::Value passed to execute(). Several logging paths operate on the raw args object before this extraction:

  • ApprovalManager::record_decision() calls summarize_args(&args) which serializes all argument keys and values into the audit log. The otp_code field will appear in the audit trail.
  • PR fix(observability): complete OTEL trace pipeline for all agent session types #5804 (pending) adds OTEL tool.arguments span attributes from raw tool args when ZEROCLAW_OTEL_TRACE_CONTENT=true. If that PR merges alongside this one, TOTP codes will appear in OTEL traces.
  • Any runtime JSONL trace that serializes tool arguments will include otp_code.

TOTP codes are time-limited (30s window) but should still not persist in logs or traces. The fix is to strip otp_code from args before passing to any logging/audit path, or to add otp_code to an explicit redaction list in summarize_args. This must be resolved before merge.

@github-project-automation github-project-automation bot moved this from Backlog to Needs Changes in ZeroClaw Project Board Apr 17, 2026
@singlerider singlerider added risk: high Auto risk: security/runtime/gateway/tools/workflows. size: M Auto size: 251-500 non-doc changed lines. security Auto scope: src/security/** changed. needs-author-action Author action required before merge labels Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-author-action Author action required before merge needs-maintainer-review risk: high Auto risk: security/runtime/gateway/tools/workflows. size: M Auto size: 251-500 non-doc changed lines.

Projects

Status: Needs Changes

Development

Successfully merging this pull request may close these issues.

[Feature]: Cross-channel TOTP gate for critical tool execution with admin command gating

2 participants