feat(agent): agentic coding runtime — gated OS capabilities (filesystem, shell, install) via deterministic permission tiers + chat approvals

[sanil-23]

Summary

• Adds a deterministic, fail-closed OS-access permission gate for the agent: every shell/file/network/install action is classified in Rust (classify_command → gate_decision) and routed through an ApprovalGate before execute().
• Three access tiers surfaced in Settings → Agent OS access (read-only / ask-before-edit / full) + a "confine to workspace" toggle and per-folder trusted roots; credential & system dirs are an unconditional, trusted-root-proof block.
• Approvals are chat-only: interactive turns park and prompt (Approve/Deny card + typed yes/no); background/triage/cron turns are never gated.
• Recognizable hard-reject markers ([policy-blocked] / [policy-denied]) so the agent stops reiterating a permanently-refused call instead of grinding to max_iterations; backed by a shared circuit breaker across both agent loops.
• Cross-platform path hardening, an escalate-only LLM command category, a default ~/OpenHuman/projects read-write root, and the install chokepoint (install_tool).

Problem

The coding agent ran shell/file/network actions with no enforced boundary and no human checkpoint, and on a refusal it would re-issue the same doomed call until the iteration cap. There was no deterministic, OS-level notion of "what is this agent allowed to touch on this machine," and enforcement could not live in the system prompt (issue #1339).

Solution

• Enforcement is all in Rust (security/policy.rs: classify_command, gate_decision, check_gated_command, is_path_string_allowed/validate_path, is_always_forbidden) — never the prompt. Unknown command ⇒ Write (fail-closed), not Read.
• The harness routes external_effect_with_args() == true tools through ApprovalGate (flag-gated by OPENHUMAN_APPROVAL_GATE); the gate parks only when a tokio task-local chat context is present, publishes DomainEvent::ApprovalRequested, and resumes via approval_decide.
• Hard rejects carry stable markers; RepeatFailureGuard halts on the first verbatim repeat of a marked reject (vs the generic 3-strike path), in both run_tool_call_loop and run_inner_loop.
• UI: AgentAccessPanel (tiers + workspace toggle + granted folders) and the in-composer ApprovalRequestCard.

Submission Checklist

• Tests added or updated (happy path + failure/edge) — policy matrix + path rules (policy_tests), gate park/approve/deny/TTL + chat-only invariant (gate.rs), RepeatFailureGuard + marker detection, a harness↔gate seam test (park→approve runs / deny blocks / no-context not gated), a table test asserting every acting tool's read-only block carries the marker.
• Diff coverage ≥ 80% — local cargo-llvm-cov/diff-cover not run over this large upstream-merge diff; deferring to the coverage CI gate on this PR.
• Coverage matrix updated — N/A pending: will reconcile docs/TEST-COVERAGE-MATRIX.md rows if CI flags missing feature IDs.
• No new external network dependencies introduced (no mock-backed paths added; gate/policy/guard tests are pure).
• Manual smoke checklist — N/A: no release-cut surface changed beyond the settings panel.
• Linked issue closed via Closes #NNN — N/A: no open tracking issue (the related Keep triggers read-only until users approve writes #1339 is already closed); generalizes its read-only-until-approved idea to the whole agent.

Impact

• Desktop/CLI: agent actions are now gated; with the flag off, behavior matches pre-gate (Prompt-class runs unprompted) — read-only blocking, path hardening, and classification are always live.
• Security: credential/system dirs unconditionally blocked; full-access tier is documented full-trust (not sandboxed). No migration; [autonomy] config is additive with serde defaults (rate limit defaults to unlimited, honored end-to-end).
• This branch was merged up to current upstream/main; it reconciles an overlapping stub autonomy-settings implementation that had landed upstream (kept the full tier model, dropped the stub).

Related

• Related: Keep triggers read-only until users approve writes #1339 (closed — "keep triggers read-only until users approve writes"; this PR generalizes that idea to all agent OS actions). No open issue to auto-close.
• Follow-up PR(s)/TODOs: thread SecurityPolicy into run_tests/run_linter; optional app-level WDIO smoke for the approval card; pre-create ~/OpenHuman/projects at boot (currently created on first agent write).

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: sanil-23:feat/agentic-runtime
• Commit SHA: see PR head

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • In-chat human approval flow with an approval card (approve/deny), per-conversation pending approvals, and opt-out interactive prompts
  • Agent OS access settings UI: access tiers, workspace confinement, granted folders, and immediate-apply UX
  • New host tools: detect installed developer tools and an installer tool (package install)

• Bug Fixes

  • Circuit breaker to halt repeated failing tool actions
  • Consistent “[policy-blocked]” error markers and improved socket/approval reconciliation and session invalidation when agent-access changes

• Documentation

  • Expanded docs for agent access modes, approval workflow, and projects-home behavior

[sanil-23]

Review batch addressed (commits 944e851 / cd56a9f / b857f75)

Swept all 18 actionable items from the latest review (15 inline + 3 outside-diff). 16 fixed, 2 pushed back.

Fixed — approval-gate flow + security (944e8514)

• 🔴 jsonrpc.rs — stop logging token-derived session_id (it's the raw OPENHUMAN_CORE_TOKEN); redact to the ephemeral UUID only. + case-insensitive =false parsing.
• 🟠 gate.rs:334-365 — timeout-race approve falls through to clear_thread (no stale thread→request mapping).
• 🟠 gate.rs:234-249 — persist-failure denial carries POLICY_DENIED_MARKER.
• 🟠 gate.rs:201 — unscoped intercept_audited tests now scope APPROVAL_CHAT_CONTEXT inside their spawned tasks. This was the real cause of the Linux Rust Core Tests + Quality timeouts — the no-context→Allow branch made those tests hang on list_pending; the Windows job runs a different subset so it passed. Verified: gate suite now 10/10 in ~2s.
• 🟠 web.rs:627 — gate.decide Ok(None) no longer ACKed as success; falls through to a fresh turn.
• 🟠 tool_loop.rs:1208 — emit TurnCompleted before the circuit-breaker early return.

Fixed — tool/config hardening (cd56a9f6)

• 🟠 install_tool.rs:120 [policy-denied] marker · 🟠 :27 cap stdout/stderr at 1 MiB · 🟡 detect_tools.rs:55 Unix exec-bit check · 🟡 config/schemas.rs accept u64 (matches TypeSchema::U64), clamp to internal u32.

Fixed — frontend approval UX (b857f755)

• 🟠 ApprovalRequestCard.tsx:48 localized fallback + namespaced debug (no raw error to UI) · 🟠 AgentAccessPanel.tsx:114 last-write-wins save guard · 🟡 ChatRuntimeProvider.tsx + chatRuntimeSlice.ts clear pending approval on disconnect/hydrate · 🟡 en.ts platform-neutral placeholder.

Pushed back (2)

• 🔴 shell.rs:195 (fail-open) — replied above: no direct caller exists; the gate is enforced at the single harness boundary; the suggested guard would over-block background/Full/gate-off turns; the deterministic floor (check_gated_command) holds unconditionally regardless.
• 🟡 ApprovalRequestCard.test.tsx:32 (shared test-utils) — the shared renderWithProviders store omits the chatRuntime slice these tests assert against; expanding shared infra for one component test isn't warranted.

[M3gA-Mind]

PR #2631 — feat(agent): agentic coding runtime — gated OS capabilities

Walkthrough

This PR lands a deterministic, fail-closed OS-permission gate for the coding agent across 93 files and 5 481 additions. The Rust core gains SecurityPolicy (classify_command → gate_decision → check_gated_command) that classifies every shell/file/network/install action into a coarse CommandClass and routes Prompt-class calls through a new ApprovalGate before execute() runs. Three access tiers (read-only / supervised / full) plus a workspace-confinement toggle and per-folder trusted roots are surfaced in a new Settings → Agent OS access panel (AgentAccessPanel). An in-composer ApprovalRequestCard parks the turn and surfaces Approve / Deny to the user; background/cron turns bypass the gate. A RepeatFailureGuard circuit breaker with stable [policy-blocked] / [policy-denied] markers stops the agent from grinding the same doomed call to max_iterations. The approach is well-architected and the test coverage is substantial.

Changes

File	Summary
src/openhuman/security/policy.rs	New SecurityPolicy — classify_command, gate_decision, check_gated_command, validate_path, is_always_forbidden, ActionTracker
src/openhuman/security/live_policy.rs	Process-global, hot-swappable SecurityPolicy singleton
src/openhuman/approval/gate.rs	ApprovalGate — parks tool calls on a oneshot, resolves via RPC/chat-reply, 10-min TTL
src/openhuman/agent/harness/tool_loop.rs	Wires RepeatFailureGuard, ApprovalGate::intercept_audited, per-tool gate routing
src/openhuman/tools/impl/system/install_tool.rs	New install_tool — triple-gated OS package install
src/openhuman/tools/impl/system/detect_tools.rs	New detect_tools — PATH-based tool detection
app/src/components/settings/panels/AgentAccessPanel.tsx	Tier/workspace-only/trusted-roots settings panel with auto-save
app/src/components/chat/ApprovalRequestCard.tsx	In-composer approval card wired to openhuman.approval_decide
app/src/store/chatRuntimeSlice.ts	pendingApprovalByThread slice + clearPendingApprovalForThread
app/src/providers/ChatRuntimeProvider.tsx	Routes approval_request socket event into Redux
src/openhuman/channels/providers/web.rs	ApprovalSurfaceSubscriber bridges gate events → socket; chat-reply routing
src/openhuman/security/policy_tests.rs	600-line test suite for policy matrix + path rules
src/openhuman/config/schema/autonomy.rs	Adds trusted_roots, allow_tool_install to AutonomyConfig
app/src/lib/i18n/en.ts + 24 chunk files	43 new i18n keys; all 12 non-English locales filled
src/openhuman/tools/impl/filesystem/file_write.rs	Adds external_effect_with_args for ask-before-edit gate routing

Actionable comments (3)

⚠️ Major

1. src/openhuman/tools/impl/system/install_tool.rs:71-77 — Package name validator allows path-traversal sequences (../)

is_valid_package_name permits . and / in the character set (needed for npm @scope/pkg and PyPI extras), but this also passes ../../evil, ../requirements.txt, and @scope/../etc/passwd. These are handed verbatim as a subprocess argv argument — no shell injection — but file-based package managers execute them as filesystem paths:

• pip install ../../evil_requirements.txt installs from a file outside the workspace.
• cargo install .. installs from a local crate in the parent directory.

The approval gate shows the package name to the user, so a careful user can deny. But the underlying validator should not accept .. path components.

Suggested change:

// before
fn is_valid_package_name(pkg: &str) -> bool {
    !pkg.is_empty()
        && pkg.len() <= 200
        && pkg.chars().all(|c| {
            c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '+' | '-' | '@' | '/' | ':')
        })
}

// after
fn is_valid_package_name(pkg: &str) -> bool {
    if pkg.is_empty() || pkg.len() > 200 {
        return false;
    }
    // Reject path traversal: absolute paths, or any ".." / "." component.
    if pkg.starts_with('/') || pkg.split('/').any(|c| c == ".." || c == ".") {
        return false;
    }
    pkg.chars().all(|c| {
        c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '+' | '-' | '@' | '/' | ':')
    })
}

---

2. app/src/components/settings/panels/AgentAccessPanel.tsx:18 — ALLOW_TOOL_INSTALL = true silently overrides a user's allow_tool_install = false config

The constant is hardcoded to true and passed on every persist() call. A user who explicitly set allow_tool_install: false in their TOML config will have that preference silently overridden the first time they open this panel and change any setting. The panel does not load or display the current allow_tool_install value, so users have no visibility into the change.

If the intent is "install is always available but still gated by the approval prompt," the fix is to read the value from the backend on load and round-trip it unchanged:

Suggested change:

// Add to state:
const [allowToolInstall, setAllowToolInstall] = useState(false);

// In the load effect:
setAllowToolInstall(resp.result.allow_tool_install ?? false);

// In persist():
await openhumanUpdateAutonomySettings({
  level: next.level,
  workspace_only: next.workspaceOnly,
  trusted_roots: next.trustedRoots,
  allow_tool_install: allowToolInstall, // round-trip actual value, not hardcoded
});

---

3. src/openhuman/config/schema/autonomy.rs:29-30 — auto_approve / always_ask config fields are silently dead

AutonomyConfig carries auto_approve: Vec<String> (defaulting to ["file_read", "memory_search", …]) and always_ask: Vec<String>. Neither field is transferred to SecurityPolicy in SecurityPolicy::from_config() and neither exists on the SecurityPolicy struct. A user who sets auto_approve = ["shell"] in their TOML config gets no error and no effect — the approval gate ignores it completely.

Either implement these fields (the non-empty defaults suggest they were intended) or remove them and their defaults to avoid misleading users and a confusing gap in the schema docs.

---

💡 Refactor / suggestion

4. src/openhuman/tools/impl/filesystem/file_write.rs:62-75 — Blocking target.exists() in a sync trait method called from an async executor

external_effect_with_args calls target.exists() — a synchronous filesystem stat — inline in the async tool-loop. For a single stat() this is unlikely to stall an executor thread in practice, but it is technically blocking I/O in async code. The comment acknowledges it is a best-effort TOCTOU probe. If the Tool trait ever gains an async external_effect_with_args, this can move there; until then, add a code comment noting the intentional sync call.

5. src/openhuman/security/policy.rs:925-931 — has_hidden_execution backtick check is not quote-aware (intentionally conservative but inconsistent)

command.contains('') catches every backtick, including inside double-quoted strings (echo "use `code` here"). This is intentionally fail-closed for Supervised mode. But contains_unquoted_background_ampersand(for&`) IS quote-aware. The inconsistency means a human-approved command with a literal quoted backtick silently fails the post-approval in-tool guard with no visible error to the user. Add a comment documenting the intentional coarseness so future maintainers don't inadvertently change it.

---

Nitpicks (4)

• app/src/components/settings/panels/AgentAccessPanel.tsx:181-205 — Tier buttons lack role="radiogroup" / role="radio" / aria-checked. Screen readers see three independent buttons instead of a radio group.
• app/src/components/settings/panels/AgentAccessPanel.tsx:300 — savedNote is never cleared after a delay; "✓ Saved" persists indefinitely. A brief setTimeout(() => setSavedNote(null), 3000) matches typical settings-panel UX.
• src/openhuman/security/policy.rs:829 — pacman -s <pattern> (lowercase search) is a read-only operation that will be over-classified as Install because "-s".starts_with("-s") matches. This is the safe/conservative direction (over-prompting), but worth noting in a comment.
• src/openhuman/security/live_policy.rs:75 — state.workspace_dir.read().map(|g| g.clone()).unwrap_or_default() silently swallows a PoisonError. Prefer .read().ok().map(|g| g.clone()).unwrap_or_default() to make the fallback intent explicit.

Questions for the author (1)

• src/openhuman/tools/impl/system/install_tool.rs:113 — external_effect() returns true (no external_effect_with_args override), and gate_decision(Install) returns Prompt in all acting tiers — Full included. Is the intent that OS installs always prompt even in Full? If so, a doc comment on external_effect() stating "always-prompt intent" would prevent future maintainers from optimizing it away.

Verified / looks good

• The fail-closed CommandClass Ord-based max() composition is correct: an unrecognized command resolves to Write, never Read.
• is_always_forbidden is unconditional and trusted-root-proof: credential paths and OS directories cannot be reached even with an explicit trusted root grant.
• ApprovalGate registers the oneshot waiter before persisting the DB row — correctly handles a fast approval_decide that races the insert.
• The TTL-race case (Approve committed at exactly timeout) reads store::get_decision to honor the persisted result rather than blindly denying — the subtle race described in the #2367 comment is addressed.
• RepeatFailureGuard uses HARD_REJECT_REPEAT_THRESHOLD = 2 for policy markers (halt on first verbatim repeat) and the broader REPEAT_FAILURE_THRESHOLD = 3 for generic failures — semantics are correct.
• Symlink-escape defense runs at both the string level (try_canonicalize_under_workspace) and the I/O level (validate_parent_path), with a dedicated test.
• i18n coverage: all 43 new keys added to en.ts and mirrored across all 12 locale chunk files — CI gate will pass.
• Gate bypass for background/cron turns (no ApprovalChatContext) is tested in gate.rs::no_chat_context_is_allowed_not_gated.

[M3gA-Mind]

Major — path traversal via .. in package name validator

is_valid_package_name allows . and / characters (needed for npm @scope/pkg), but this means ../../evil, ../requirements.txt, and @scope/../etc/passwd all pass validation. These strings are handed verbatim to the subprocess argv — no shell injection — but file-based package managers (pip install ../../foo.txt, cargo install ..) treat them as filesystem paths, enabling installs outside the workspace.

The approval gate shows the package name, so a vigilant user can deny. But the validator should not accept .. components.

fn is_valid_package_name(pkg: &str) -> bool {
    if pkg.is_empty() || pkg.len() > 200 {
        return false;
    }
    // Block path traversal: absolute path prefix or any ".." / "." component.
    if pkg.starts_with('/') || pkg.split('/').any(|c| c == ".." || c == ".") {
        return false;
    }
    pkg.chars().all(|c| {
        c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '+' | '-' | '@' | '/' | ':')
    })
}

[sanil-23]

Fixed differently than suggested (1635a32). You're right that this is a real hole, but the path-traversal validator is the wrong lens: install_tool mutates the host, not the workspace, and a valid-looking malicious package (pip install evil-pkg) evades any name check. The real control for a host-mutating tool is human approval. So I added Gate 0: install_tool now fails closed unless run in an interactive approval turn (APPROVAL_CHAT_CONTEXT, set on the web-chat path) — background/triage/cron turns refuse with [policy-denied]. Every actual install is now human-Approved via the gate; reverted the .. validator.

[M3gA-Mind]

Major — hardcoded ALLOW_TOOL_INSTALL = true silently overrides user config

This constant is sent on every persist() call. If a user has allow_tool_install: false in their TOML config, opening this panel and changing the tier or workspace toggle will silently set it to true — without showing the current value or warning. The user loses a deliberately-set preference with no feedback.

Round-trip the value from the backend instead:

// State
const [allowToolInstall, setAllowToolInstall] = useState(false);

// In load effect, after setLevel / setWorkspaceOnly:
setAllowToolInstall(resp.result.allow_tool_install ?? false);

// In persist():
await openhumanUpdateAutonomySettings({
  level: next.level,
  workspace_only: next.workspaceOnly,
  trusted_roots: next.trustedRoots,
  allow_tool_install: allowToolInstall,   // actual value, not hardcoded
});

If the product decision is truly "always enable, always gate via approval," then the allow_tool_install config field is misleading — remove it from the schema rather than silently forcing it true from the UI.

[sanil-23]

Pushing back here. allow_tool_install: true doesn't silently enable unconsented installs — install_tool is always approval-gated: every install parks an ApprovalCard and the user Approves/Denies it (now also enforced fail-closed in autonomous turns via the new Gate 0). So the flag means "install tool available, gated by approval," not "installs happen without asking." Consent is captured per-install by the gate, not by a static config knob — so the panel intentionally forces it true rather than exposing a redundant toggle. Kept as-is + clarified the comment.

[M3gA-Mind]

Major — auto_approve and always_ask config fields are silently ignored at runtime

These fields have non-empty defaults (auto_approve defaults to ["file_read", "memory_search", "memory_list", "get_time", "list_dir"]) suggesting they are part of the intended API. However, neither field is transferred to SecurityPolicy in SecurityPolicy::from_config() and neither exists on the struct. A user who sets auto_approve = ["shell"] in their TOML config gets no error and no effect — the approval gate ignores it.

If this is intentional deferral, remove the fields (and the defaults) to avoid confusion. If they are planned, track the gap:

// in SecurityPolicy::from_config — document the gap until implemented:
// TODO: auto_approve / always_ask are loaded but not yet wired into the
// ApprovalGate always-allowlist or the tool-loop skip-list. Filed as #NNNN.

[sanil-23]

Documented (1635a32). These fields are pre-existing (on main before this PR), and you're right they're loaded-but-unused. Added a NOTE in SecurityPolicy::from_config that auto_approve/always_ask aren't wired into the gate's allow/ask lists yet (approval is tier + CommandClass driven); wiring them is a follow-up rather than scope for this PR since they predate it.

[M3gA-Mind]

Suggestion — target.exists() is a blocking filesystem call in an async context

std::path::Path::exists() issues a synchronous stat syscall. This runs inline in the async tool-loop, blocking a tokio executor thread for the duration of the syscall. In practice a stat() completes in microseconds so it will not cause observable timeouts, but it violates async hygiene and will become a problem if this method is ever called on a slow/remote filesystem.

Since the Tool trait cannot easily make external_effect_with_args async, at minimum add a comment documenting this as an intentional blocking call:

// Sync stat: intentionally blocking since the Tool trait is sync here.
// Fast for local paths; would need block_in_place if remote fs is ever supported.
target.exists()

Alternatively, pre-compute the existence check in the gate routing (where async is available) and pass it in via a separate check_external_effect path.

[sanil-23]

Added the comment (1635a32) — documented that the exists() probe is an intentional sync/blocking stat (fast for local paths; would need block_in_place only if a remote fs is ever supported). Thanks.

[M3gA-Mind]

Nitpick — has_hidden_execution backtick check is not quote-aware (document the intent)

command.contains('')catches every backtick, including inside a double-quoted string (e.g.echo "use `code` syntax"). This is intentionally fail-closed. However, contains_unquoted_background_ampersand(for&`) IS quote-aware, so the inconsistency is surprising to future maintainers.

A command like echo "I used \ls` here"` — entirely benign — would be silently blocked by the in-tool guard after the human already approved it in the gate. The user sees no error; the shell call just fails.

Add a comment documenting the intentional coarseness:

fn has_hidden_execution(command: &str) -> bool {
    // Backtick check is deliberately NOT quote-aware: any backtick in the
    // command string is blocked in Supervised mode, even inside double-quoted
    // literals. Over-blocking is the safe direction here. See also the
    // quote-aware contains_unquoted_background_ampersand for the & case.
    command.contains('`')
        || command.contains("$(")
        || command.contains("<(")
        || command.contains(">(")
        || contains_unquoted_background_ampersand(command)
}

[sanil-23]

Added the comment (1635a32) documenting that the backtick check is intentionally NOT quote-aware (over-blocking is the safe direction), and noting the contrast with the quote-aware & check (which must allow benign 2>&1 fd-dups). Thanks for flagging the inconsistency.

[M3gA-Mind]

Architecture is sound — fail-closed classification, Rust-only enforcement, always-forbidden credential paths, solid test coverage. Two items to track post-merge: (1) the ALLOW_TOOL_INSTALL = true hardcoding in AgentAccessPanel.tsx silently overwrites any manual allow_tool_install = false in config — either drop the backend field or expose the knob; (2) confirm validate_path canonicalizes before forbidden-list checks to close a potential traversal bypass, and add a test. Everything else is minor. Approving.