Repository navigation
feat: tighten runtime policy + transport guards v3 - #3306
Conversation
Add an optional sender field to AgentTurnOrigin::ExternalChannel so the approval gate, wallet preparer binding, and per-turn audit log can see which participant in a shared multi-user channel (Discord guild, Slack workspace, group Telegram chat) produced an inbound message. The thread identity flowing through derive_inbound_thread_id is already per-sender; this is the matching half on the origin so downstream isolation invariants survive into the gate's structured log without consumers having to re-derive the sender from a thread-id string.
Replace the literal client_id = "inbound" with derive_inbound_client_id(sender) so the (thread_id, client_id) pair flowing into the agent turn, the APPROVAL_CHAT_CONTEXT task-local, and the wallet QuoteOwner is distinct per sender even when distinct participants share the same channel + reply_target. Legacy publishers that don't surface a sender keep the bare "inbound" label so single-DM flows are unchanged. Adds regression tests for both shapes (per-sender split + legacy fallback) alongside a combined (thread_id, client_id) pair assertion covering the same isolation invariant the wallet preparer-binding and parked-approval routing already enforce.
…el origin The channel runtime dispatch path now sets sender = Some(msg.sender) on the ExternalChannel origin so the approval gate's structured log carries the per-user identity end to end. The gate destructure pattern picks the new field up and surfaces it on the tracing::info! audit row alongside channel / reply_target / message_id.
…el origins Fill the new optional sender field on the three remaining ExternalChannel construction sites. Triage runs over a per-source envelope (composio / webhook / cron / external caller) that has no per-user sender at this layer; meet utterances address by room, not participant; the MCP server addresses by calling client. All three leave sender unset so the gate's strict per-channel TTL-deny default still applies.
Adds a small typed HostKind enum (TauriShell / Cli / Docker) in src/core/types.rs so policy-sensitive boot decisions can be made deterministically rather than guessed from the absence of other signals. Includes the pure approval_gate_boot_decision helper used by the bootstrap path so the host-aware override evaluation can be unit-tested without standing up the full runtime. HostKind::detect_standalone inspects OPENHUMAN_DOCKER and /.dockerenv to distinguish Cli from Docker; Tauri-shell callers pass TauriShell explicitly because the embedding shell is the only authority on that fact.
…bled domain events Two new variants on the approval domain so the host-aware boot decision can be surfaced to subscribers (the desktop UI banner, log shippers, audit dashboards) instead of being a startup log line that's easy to miss. ApprovalGateOverrideIgnored fires when OPENHUMAN_APPROVAL_GATE=0 is set under the Tauri shell — the gate still installs but the user gets a one-shot banner so they know the override was rejected. ApprovalGateDisabled fires when the same env override is honored under CLI / Docker — the gate is intentionally off, but any connected dashboard can still surface the elevated-privilege state in real time. domain() routes both to "approval" alongside the existing ApprovalRequested / ApprovalDecided pair; variant_name() pins the serialized strings.
…onor with warning event Replace the bare OPENHUMAN_APPROVAL_GATE env-only branch in bootstrap_core_runtime with a host-aware decision routed through the pure approval_gate_boot_decision helper. Under HostKind::TauriShell the env override is treated as advisory only. The gate ALWAYS installs and a DomainEvent::ApprovalGateOverrideIgnored is published so the UI banner can surface the rejected-override state. log::warn covers the case for non-UI log shippers. Under HostKind::Cli / HostKind::Docker the env override is honored because env-as-config is the operator's chosen surface there. The gate is intentionally not installed but a DomainEvent::ApprovalGateDisabled is still published so dashboards / log shippers can surface the elevated-privilege state in real time. log::error marks it as a high-severity event. bootstrap_core_runtime public signature switches from a plain bool to HostKind; the single caller in run_server_inner maps from the existing embedded_core boolean plus HostKind::detect_standalone for the CLI vs Docker split.
…banner The DomainEvent variants added in the previous commit are transient — the UI banner has to render on first paint without waiting for a socket subscriber to catch a boot-time event. Persist the host-aware boot decision in a process-global OnceLock (record_boot_state) and expose it via approval_get_gate_state. The new RPC returns an ApprovalGateBootState snapshot with four flags: installed / disabledByEnv / overrideIgnored / host tag. The UI polls it on mount; older test paths that bring up the gate directly bypass bootstrap_core_runtime and therefore never call record_boot_state, so try_boot_state returns None there — the RPC degrades to a benign "installed, no banner" default.
Add the four banner strings the SecurityBanner reads via useT:
- security.approvalGateDisabled.{title,body}
- security.approvalGateOverrideIgnored.{title,body}
Real translations in every non-English locale (ar, bn, de, es, fr, hi,
id, it, ko, pl, pt, ru, zh-CN). i18n:check passes parity; the four
new keys are not on the i18n:english:check flag list.
Adds a thin presentational banner mounted near the top of the provider chain in App.tsx so it surfaces regardless of route. On mount it reads the host-aware approval-gate boot decision via the new openhuman.approval_get_gate_state RPC and renders one of two states: - Persistent red banner when disabledByEnv === true. The operator set OPENHUMAN_APPROVAL_GATE=0 on a standalone host and the gate is actually OFF. External-effect tool calls run unprompted. - One-shot yellow info banner when overrideIgnored === true. The same env override was attempted under the Tauri desktop shell, which always rejects it. The user sees the attempt was blocked; the gate is still on. Auto-dismisses after 10 s. Steady-state desktop boot is silent. RPC failure coerces to a benign "installed, no banner" default so a degraded core never blanks the app shell. All copy goes through useT; keys live in app/src/lib/i18n across all 14 locales.
Covers all 5 documented states: - steady-state desktop boot renders nothing - disabledByEnv renders the persistent red banner (role=alert) - overrideIgnored renders the one-shot info banner (role=status) - the one-shot banner auto-dismisses after 10 s - the persistent banner does NOT auto-dismiss - RPC failure / unknown host renders nothing
…-friendly session keys
Reshape the device tunnel session-key flow so:
- derive_session_keys(static_dh, eph_dh, client_eph_pub, server_eph_pub)
→ SessionKeys { c2s, s2c }
uses HKDF-SHA256 over IKM = static_dh || eph_dh with salt =
client_eph_pub || server_eph_pub and pinned info tags
openhuman-tunnel/v1/c2s and openhuman-tunnel/v1/s2c.
- TunnelCipher::for_role(role, &keys) holds two XChaCha20Poly1305
instances: seal_cipher for the local direction, open_cipher for the
peer's. A frame sealed by the server (s2c) cannot decrypt under the
server's own opener (which holds c2s) — closes cross-direction
reflection.
- FRAME_VERSION bumped from 0x01 → 0x02. v1 frames are rejected
explicitly with an UnsupportedFrameVersion: re-pair required message
so peers see a clear upgrade signal instead of a generic AEAD
failure. iOS pairing is documented as in-progress / non-shipping in
CLAUDE.md so the forced re-pair is acceptable.
- TunnelCipher::new(&key) preserved (single-key, both directions
identical) for the layer-2 sealed-handshake path in devices/bus.rs
that lives outside the post-pairing session.
Adds hkdf = "0.12" alongside the existing sha2 = "0.10" workspace
dep.
Regression coverage:
- hkdf_derives_distinct_directional_subkeys (info tag pinning)
- cross_direction_reflection_fails (load-bearing isolation invariant)
- directional_roundtrip_{server_to_client,client_to_server}_succeeds
- frame_v1_rejected_after_upgrade (explicit error shape pinned)
- ephemeral_dh_prevents_session_key_recovery_from_static_only (FS)
- directional_subkeys_are_independent_per_session
… side
Mirrors src/openhuman/devices/crypto.rs:
- deriveSessionKeys(staticDh, ephDh, clientEphPub, serverEphPub)
returns SessionKeys { c2s, s2c } via @noble/hashes HKDF-SHA256 over
IKM = staticDh || ephDh with salt = clientEphPub || serverEphPub
and the byte-identical info tags openhuman-tunnel/v1/c2s and
openhuman-tunnel/v1/s2c.
- TunnelCipher(role, keys) holds two directional XChaCha20-Poly1305
instances. Frames sealed by the server cannot decrypt under the
server's own opener — closes cross-direction reflection.
- FRAME_VERSION bumped from 0x01 to 0x02. v1 frames are rejected
with an UnsupportedFrameVersion + re-pair message.
- The layer-2 sealed-handshake byte (sealHandshake / openHandshake)
is pinned to LEGACY_FRAME_VERSION_V1 = 0x01 because that flow is
the bootstrap before session keys exist; its wire shape is
unchanged.
- seal/open module-level functions are preserved (single-key legacy
path) so non-session bootstrap callers are unaffected.
@noble/hashes/hkdf.js and @noble/hashes/sha2.js used per the
package's exports map (only the .js paths are resolvable in the
2.2.0 layout).
Regression coverage mirrors the Rust set: distinct directional
subkeys, info-tag pinning, FS sanity (static-only adversary cannot
recover prior session keys), DH input length validation, client↔server
round-trip, cross-direction reflection failure, and explicit v1 frame
rejection.
Apply rustfmt to the files touched by the per-sender channel isolation, HostKind enum, host-aware bootstrap, and tunnel HKDF + directional subkeys clusters. No behavior changes.
|
Warning Review limit reached
More reviews will be available in 68 minutes and 27 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThis PR implements host-aware approval-gate boot decisions, exposing gate state through a security banner UI while propagating per-sender identity for audit trails. It simultaneously upgrades tunnel frame encryption from single shared-key (v1) to directional subkeys (v2) using X25519 + HKDF-SHA256, rejecting legacy frames post-upgrade. The changes span Rust core, Rust device crypto, React frontend, 12 language translation files, and comprehensive test coverage. ChangesApproval-gate host awareness and security banner
Tunnel cryptography v2 upgrade
Sequence Diagram(s)sequenceDiagram
participant BootPhase
participant bootstrap_core_runtime
participant approval_gate_boot_decision
participant DomainEvent Bus
participant UI Client
BootPhase->>bootstrap_core_runtime: host_kind: HostKind
bootstrap_core_runtime->>approval_gate_boot_decision: host_kind + env override requested
approval_gate_boot_decision-->>bootstrap_core_runtime: ApprovalGateBootDecision
bootstrap_core_runtime->>bootstrap_core_runtime: record_boot_state(state)
alt Tauri desktop + override attempted
bootstrap_core_runtime->>DomainEvent Bus: ApprovalGateOverrideIgnored{host}
else CLI/Docker + override enabled
bootstrap_core_runtime->>DomainEvent Bus: ApprovalGateDisabled{host, reason}
end
UI Client->>UI Client: SecurityBanner mounted
UI Client->>UI Client: fetchApprovalGateState() RPC
UI Client->>UI Client: render banner or nothing based on state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/__tests__/SecurityBanner.test.tsx`:
- Around line 121-135: The test "renders nothing on RPC failure (degraded core
must not blank the app shell)" currently supplies a resolved state to
renderBanner, so it doesn't actually exercise the failure path; update the test
to make fetchState return a rejected promise (e.g., mock fetchState to reject)
when calling renderBanner with the same input (installed: true, disabledByEnv:
false, overrideIgnored: false, host: 'unknown') and then await the rejection
path before asserting that screen.queryByTestId('security-banner-gate-disabled')
and screen.queryByTestId('security-banner-override-ignored') are null; keep the
test name and assertions unchanged but replace the resolved fetchState with a
rejected one to truly exercise the RPC failure branch.
In `@src/openhuman/channels/bus.rs`:
- Around line 1002-1018: derive_inbound_client_id currently only includes sender
and so collapses identities across channels; change the signature to accept a
channel (e.g., pub(crate) fn derive_inbound_client_id(channel: &str, sender:
Option<&str>) -> String), trim/validate both inputs, and when sender is Some
produce a namespaced id that includes the channel and sender (for example
"inbound:{channel}:{sender}" after trimming), while preserving the legacy
behavior of None mapping to the literal "inbound". Ensure you update all call
sites that invoke derive_inbound_client_id to pass the appropriate channel
argument.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13e906b8-79ed-485c-b45d-3ffdcd836281
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
Cargo.tomlapp/src/App.tsxapp/src/components/SecurityBanner.tsxapp/src/components/__tests__/SecurityBanner.test.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/lib/tunnel/crypto.test.tsapp/src/lib/tunnel/crypto.tsapp/src/services/api/approvalApi.tssrc/core/event_bus/events.rssrc/core/jsonrpc.rssrc/core/types.rssrc/openhuman/agent/triage/evaluator.rssrc/openhuman/agent/turn_origin.rssrc/openhuman/approval/gate.rssrc/openhuman/approval/rpc.rssrc/openhuman/approval/schemas.rssrc/openhuman/channels/bus.rssrc/openhuman/channels/runtime/dispatch.rssrc/openhuman/devices/crypto.rssrc/openhuman/mcp_server/tools.rssrc/openhuman/meet_agent/brain.rs
…sses @coderabbitai on bus.rs:1018) derive_inbound_client_id now includes the channel name so that the same sender on two different providers no longer maps to the same client_id.
…re test (addresses @coderabbitai on SecurityBanner.test.tsx:135)
The coverage E2E test hardcoded the expected schema list as 3 methods, but this PR added approval_get_gate_state as a 4th.
|
@coderabbitai review and approve please |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get PR details and check CI status
gh pr view 3306 --repo tinyhumansai/openhuman --json title,state,statusCheckRollup,reviews,files | jq .Length of output: 16607 🏁 Script executed: #!/bin/bash
# Review key changed files
gh pr diff 3306 --repo tinyhumansai/openhuman 2>/dev/null | head -2000Length of output: 87739
All 18 CI gates are green (Rust fmt/clippy, TypeScript typecheck, Vitest coverage, Playwright E2E ×4, cargo-llvm-cov, diff-cover ≥ 80%, i18n parity, tauri-cef pin). The three clusters are well-documented and covered:
Approving. 🐇 [approve] 🧠 Learnings used |
@coderabbitai you still haven't approved, even after saying you are doing it |
|
(╯°□°)╯ 🧠 Learnings used✅ Action performedComments resolved and changes approved. |
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Summary
version=2so the cross-direction reflection class of bug is closed and non-uniform raw-DH keys are no longer fed straight into the AEAD.Problem
Three runtime / transport surfaces had grown ahead of the boundaries the rest of the codebase already enforces:
channels/bus.rsderived the agent session from the channel name alone (thread_id="channel:<channel>",client_id="inbound"), while the normal channel path keys per-sender viaconversation_history_key(channel, sender, reply_target, thread_ts). In a multi-participant Discord channel that meant anyone in the channel resumed any other participant's session — including the yes/no confirmation step of wallet flows that the runtime expects to be sender-bound.OPENHUMAN_APPROVAL_GATE=0was a process-wide kill-switch with no host awareness. Documented as the operator escape hatch for CLI / docker / test, it also silently disabled the gate when set by same-user malware, a stray project.env, or a communitydocker-compose.yml. The desktop shell honored the override the same way the CLI did, with no UI surface telling the user the gate was off.info-tag domain separation. Static long-term device + core keypairs only — the existing design has no forward-secrecy story.Solution
Three independent commit clusters in one PR. Each cluster is reviewable in isolation.
Cluster A — Per-sender channel-inbound isolation
src/openhuman/agent/turn_origin.rs—AgentTurnOrigin::ExternalChannelcarries a newsender: Option<String>so the gate / approval surface sees per-user provenance for inbound channel turns.src/openhuman/channels/bus.rs— newderive_inbound_client_id(channel, sender)helper builds per-sender keys; socket-inbound dispatch threadssenderthroughChannelMessageend-to-end. The bare"inbound"literal is retained as the prefix so existing consumers that filter onstarts_with("inbound")keep matching, but each session is keyed per-sender.src/openhuman/channels/dispatch.rs+src/openhuman/approval/gate.rs— propagateSome(msg.sender)into the scoped origin; the gate'sExternalChannel { … }destructure surfaces it.agent/triage,meet_agent,mcp_server) updated to passsender: Noneso the variant shape stays consistent.Wallet preparer-binding was already shipped on
main(QuoteOwner+take_quote_for+ reject-cross-owner). Verified via 20 passingwallet::execution::tests; no redundant code added.Cluster B — Approval-gate host-aware boot decision
src/core/types.rs— newHostKindenum (TauriShell/Cli/Docker) +HostKind::detect_standalone+ pureapproval_gate_boot_decisionhelper, with 7 unit tests covering every (host, env) cell.src/openhuman/approval/gate.rs—bootstrap_core_runtimenow branches onHostKind. Desktop shell always installs the gate; ifOPENHUMAN_APPROVAL_GATE=0was observed, the decision is logged andDomainEvent::ApprovalGateOverrideIgnoredis published. CLI / docker continue to honor the override with a noisylog::error!andDomainEvent::ApprovalGateDisabled. Process-globalApprovalGateBootStatecarries the decision for the lifetime of the process.src/openhuman/approval/rpc.rs— newapproval_get_gate_stateRPC so the frontend can render the banner on startup, not just on a live event.src/core/event_bus/events.rs— two additive variants.app/src/components/SecurityBanner.tsx(new) — persistent red banner forApprovalGateDisabled, auto-dismissing yellow banner forApprovalGateOverrideIgnored. Mounted inapp/src/App.tsx. All strings go throughuseT(); four new i18n keys translated across all 14 locale files.Cluster C — iOS pairing tunnel HKDF + directional subkeys + frame v2
src/openhuman/devices/crypto.rs—derive_session_keys(static_dh, eph_dh, eph_pubs_concat)returns aSessionKeysstruct with distinctc2sands2c32-byte subkeys via HKDF-SHA256,infotagsb"openhuman-tunnel/v1/c2s"andb"openhuman-tunnel/v1/s2c".TunnelCipher::for_roleconstructs a two-cipher peer (seal under own-direction, open under peer-direction) so a captured frame replayed in the opposite direction fails AEAD auth. FrameFRAME_VERSIONbumped from0x01to0x02; v1 frames rejected with an explicit error.app/src/lib/tunnel/crypto.ts— TS mirror via@noble/hashes/hkdf.js+@noble/hashes/sha2.js. Same HKDF inputs, same per-direction subkeys, same v2 frame header.Deferred to a follow-up PR (noted explicitly): the ephemeral X25519 exchange at session start itself is not wired into
devices/pairing.rs/devices/bus.rsin this PR. The HKDF helper,SessionKeys,TunnelCipher::for_role, and the v2 frame rejection are all in place; the actual handshake change to mint per-session ephemeral keypairs and exchange public halves before any agent payload is a separate, smaller PR. The frame v2 rejection is the load-bearing change here — it breaks any v1 peer, which is acceptable since the iOS pairing client is non-shipping (per repo CLAUDE.md "iOS client (experimental)" section).Submission Checklist
channels::bus::tests(12),wallet::execution::tests(20, regression-only),approval::*(66),devices::crypto::tests(14, 7 new for HKDF + frame v2),core::types::tests(18, 7 new for host-aware decision),core::event_bus::tests(22),app/src/components/__tests__/SecurityBanner.test.tsx(6),app/src/lib/tunnel/__tests__/crypto.test.ts(26).## RelatedCloses #NNNin the## RelatedsectionImpact
OPENHUMAN_APPROVAL_GATE=0as advisory only — the gate always installs, the user sees a banner. No change for CLI / docker operators who legitimately want the override.QuoteOwnermigration shipped earlier onmain.AgentTurnOrigin::ExternalChannelgains asender: Option<String>field — additive enum-struct field, every existing constructor in the tree updated.Related
devices/pairing.rs. HKDF + directional subkeys are in place; the handshake change is mechanical but deserves its own focused review.mem_tree_chunks(memory-tree pipeline) so Gmail / Slack ingest paths flow alongside the section-level signal that already gates the subconscious tick.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/runtime-policy-tightening-v3git log upstream/main..HEAD— 14 commits across three clusters (A: per-sender channels; B: host-aware approval gate + banner; C: tunnel HKDF + directional subkeys + frame v2). Finalstyle: cargo fmt across runtime-policy-v3 cluster changescollapses the fmt drift.Validation Run
cargo check --workspace— cleancargo fmt --check— cleancargo clippy --lib -- -D warnings— 236 pre-existing errors identical toupstream/main; zero new hits in touched files.cargo test --lib openhuman::channels— 12 passed, 0 failed (3 new forderive_inbound_client_id)cargo test --lib openhuman::wallet— 20 passed, 0 failed (regression-only; preparer-binding already on main)cargo test --lib openhuman::approval— 66 passed, 0 failedcargo test --lib openhuman::devices— 14 passed, 0 failed (7 new for HKDF + frame v2)cargo test --lib core::types— 18 passed, 0 failed (7 new forapproval_gate_boot_decision)cargo test --lib core::event_bus— 22 passed, 0 failedpnpm compile— cleanpnpm i18n:check— clean (0 missing / 0 extra across 14 locales)pnpm --filter openhuman-app test:unit -- --run src/components/__tests__/SecurityBanner.test.tsx— 6 passedpnpm --filter openhuman-app test:unit -- --run src/lib/tunnel/__tests__/crypto.test.ts— 26 passedValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
OPENHUMAN_APPROVAL_GATE=0(env still honored on CLI / docker with a loud audit event and a banner if a desktop frontend ever connects), and the iOS pairing tunnel rejects frame v1 in favour of HKDF-derived directional subkeys under frame v2.OPENHUMAN_APPROVAL_GATE=0somehow propagated through env. iOS pairings have to re-pair (experimental client).Parity Contract
client_id="inbound"prefix retained so any consumer that filters onstarts_with("inbound")keeps matching.OPENHUMAN_APPROVAL_GATE=0continues to work for CLI / docker operators (with a warning event). HKDF + directional subkeys live behind the new frame v2 —derive_session_keysis additive next to the existing raw-DH path until the handshake follow-up wires it into production.approval_gate_boot_decisionis a pure function with unit tests covering every (host, env) cell.TunnelCipher::for_roleis constructed at exactly two call sites (server, client), both verified bycross_direction_reflection_fails. Frame v2 rejection short-circuits before any cipher state is consulted.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Security Enhancements