feat(privacy): S2 — egress descriptor + disclosure event (#4436)#4812
Conversation
…inyhumansai#4436) Introduce the egress spine for the privacy epic: a uniform EgressDescriptor ({provider_slug, service, is_external, reason, data_kinds}) plus a DomainEvent::ExternalTransferPending that carries it, emitted before any external transfer. The descriptor already holds the S5 identification-risk fields (risk_level + risk_categories, default-empty) so later slices attach risk without reshaping the contract. emit_external_transfer drops local-only transfers and attaches ambient chat routing. Hooks marked for S3/S4/S7.
…ansai#4436) Add EgressSurfaceSubscriber, mirroring the ApprovalRequested -> approval_request pattern: bridges DomainEvent::ExternalTransferPending to the external_transfer_pending web-channel event (args mirror the serialized descriptor) when the transfer carries chat routing; background/CLI/cron transfers stay on the domain bus only. Registered unconditionally at startup (channels runtime + jsonrpc serve boot).
tinyhumansai#4436) Emit an EgressDescriptor at the single inference chokepoint in create_chat_provider_from_string (concrete provider resolved; local runtimes disclosed as non-external and never fire the event) and at the cloud embedding request. Reuses S1's is_local_provider_string for the external/local split.
tinyhumansai#4436) Disclose Composio tool calls (both execute_tool entry points, one emit per logical call) and OpenHuman managed-backend round-trips (IntegrationClient post/get; query string stripped so only the endpoint is disclosed).
…sai#4436) Disclose the destination host of agent-driven http_request / web_fetch calls after URL validation, before the round-trip (http_request also records that a request body accompanies the transfer).
…sai#4436) The S2 egress emit was wired only into the legacy Provider path (create_chat_provider_from_string). Post-tinyhumansai#4784 real agent turns build via the crate-native ChatModel/turn path (create_chat_model_with_model_id and the create_turn_chat_model* family), whose managed / local-runtime / BYOK-cloud sub-paths return early WITHOUT touching the Provider path — so the default managed chat turn disclosed no egress (confirmed by an inference-probe smoke on staging firing zero events). Emit at the three universal construction chokepoints shared by every ChatModel and turn entry, guaranteeing exactly one descriptor per construction: - managed backend -> resolve_managed_backend (external); the sole funnel for the Provider AND ChatModel/turn managed builds. emit_inference_egress now skips PROVIDER_OPENHUMAN so the Provider path doesn't double-emit. - local runtime -> try_create_local_runtime_chat_model_from_string, past the non-local return + gates (disclosed as NON-external, no pending event). - BYOK cloud slug -> try_create_cloud_slug_chat_model_from_string_with_native_tools, past the managed/bespoke/OpenhumanJwt returns + gates (external). The Provider path's top-level emit still covers its direct callers and the bespoke fallthrough; the two paths are mutually exclusive per construction.
…humansai#4436) Drive create_chat_model_with_model_id (the production turn path) directly: - managed/external role emits EXACTLY ONE ExternalTransferPending (unique default_model marker guards against miss and double on the process-wide bus). - local runtime emits NONE (non-external suppression holds). Complements the coordinator's Provider-path real-path test. Also reorders an egress emit_tests import to satisfy rustfmt.
📝 WalkthroughWalkthroughAdds a unified egress descriptor and ChangesEgress descriptor and event contract
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EgressSource
participant EgressEmitter
participant GlobalEventBus
participant EgressSurfaceSubscriber
participant WebUI
EgressSource->>EgressEmitter: emit_external_transfer(descriptor)
EgressEmitter->>GlobalEventBus: publish ExternalTransferPending
GlobalEventBus->>EgressSurfaceSubscriber: deliver chat-scoped event
EgressSurfaceSubscriber->>WebUI: emit external_transfer_pending with descriptor args
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
feature
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82952f6a13
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| crate::openhuman::security::egress::emit_external_transfer( | ||
| crate::openhuman::security::egress::EgressDescriptor::inference("openhuman", &model, true), | ||
| ); |
There was a problem hiding this comment.
In the crate-native turn path I checked (tinyagents::build_turn_models_crate), a single managed chat turn constructs the primary model, every WORKLOAD_ROUTE_TIERS route, and a summarizer before any request is made; each construction reaches resolve_managed_backend and now publishes ExternalTransferPending here. With chat context the web bridge surfaces every one of those as external_transfer_pending, so one user prompt can produce multiple/audit false transfer disclosures even when only one model request is actually sent. Move this emission to the actual request dispatch path, or otherwise dedupe/scope it to real outbound calls.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 2828a62 + 4a0dd88 — the managed-turn build (primary + route tiers + summarizer) now runs under a per-turn dedup ledger (dedup_turn_scope), so a single prompt discloses each distinct destination once instead of once per model construction. Emitting at construction (not dispatch) is documented as a bounded residual.
| if body.is_some() { | ||
| desc = desc | ||
| .with_data_kind(crate::openhuman::security::egress::DataKind::ToolArguments); | ||
| } |
There was a problem hiding this comment.
When http_request is called with custom headers but no body (the tool schema even gives Authorization as an example), those header values are sent to the external host, but this descriptor still reports only DataKind::Url because extra data is added only when body.is_some(). That under-reports what leaves the device in the privacy disclosure/audit for header-only API calls; treat non-empty headers as metadata/tool arguments before emitting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 1de41d0 — non-empty custom headers now add DataKind::Metadata to the descriptor, so a header-only request (e.g. an Authorization token, no body) is no longer under-reported as URL-only. Logic extracted to a unit-tested network_egress_descriptor helper.
…scriptor # Conflicts: # src/openhuman/embeddings/cloud.rs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/openhuman/channels/providers/web/event_bus.rs (1)
136-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPer-event diagnostic logged at
info, notdebug/trace.This log fires on every
ExternalTransferPendingwith chat context (i.e., every external LLM/Composio/network call in a chat turn), unlike the one-time registration message at Lines 77-79. As per coding guidelines,src/openhuman/**/*.rs: "Uselogortracingatdebug/tracelevels for Rust diagnostics." Atinfo, this will be emitted at the default log level for every disclosed transfer.📝 Proposed fix
- log::info!( + log::debug!( "[web-channel] egress-surface emitting external_transfer_pending provider={} service={} reason={:?} thread_id={thread_id} client_id={client_id}", descriptor.provider_slug, descriptor.service, descriptor.reason, );🤖 Prompt for 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. In `@src/openhuman/channels/providers/web/event_bus.rs` around lines 136 - 141, Change the per-event log in the ExternalTransferPending egress path from info to debug or trace, keeping the existing message and fields intact. Leave the one-time registration log unchanged.Source: Coding guidelines
src/openhuman/security/egress/types.rs (1)
71-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider deriving
Ord/PartialOrdonIdentificationRisk.The doc says "Identification-risk level for the payload" and is "Ordered least → most identifying." But the enum only derives
PartialEq, Eq, notOrd/PartialOrd, so no comparison operators exist yet despite the documented ordering intent. Once S5 lands and needs threshold checks (e.g.risk_level >= Medium), this will need adding anyway — worth doing now while the enum is small and unstable.♻️ Suggested derive
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum IdentificationRisk {🤖 Prompt for 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. In `@src/openhuman/security/egress/types.rs` around lines 71 - 85, Update the IdentificationRisk enum derives to include PartialOrd and Ord, preserving its declared variant order from Unknown through High so threshold comparisons such as risk_level >= Medium work as documented.src/openhuman/inference/provider/factory.rs (1)
669-687: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSkip branches in
emit_inference_egresshave no diagnostics.The empty/
"cloud"sentinel skip and thePROVIDER_OPENHUMANskip both return silently — nolog::trace!/log::debug!— unlike this file's own convention (e.g.enforce_local_only_inferencelogs on every branch). Grep-friendly logging on these branches would help debug missed/duplicate egress emissions later.
As per coding guidelines: "New or changed flows must include verbose, grep-friendly diagnostics for entry/exit, branches, external calls, retries/timeouts, state transitions, and errors; never log secrets or full PII."🤖 Prompt for 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. In `@src/openhuman/inference/provider/factory.rs` around lines 669 - 687, The skip branches in emit_inference_egress currently return without diagnostics. Add verbose, grep-friendly trace or debug logging before returning for empty/"cloud" sentinels and for PROVIDER_OPENHUMAN, identifying the branch and role without logging secrets or full PII; preserve the existing skip behavior.Source: Coding guidelines
🤖 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 `@src/openhuman/inference/provider/factory.rs`:
- Around line 775-781: The emit_inference_egress call currently runs before
provider validation, allowing pending events for requests that fail locally.
Move emit_inference_egress(role, p) below verify_session_active(config)? and all
branch-specific empty-model/slug checks, preserving the existing ordering for
the cloud-slug path and emitting only after validation succeeds.
---
Nitpick comments:
In `@src/openhuman/channels/providers/web/event_bus.rs`:
- Around line 136-141: Change the per-event log in the ExternalTransferPending
egress path from info to debug or trace, keeping the existing message and fields
intact. Leave the one-time registration log unchanged.
In `@src/openhuman/inference/provider/factory.rs`:
- Around line 669-687: The skip branches in emit_inference_egress currently
return without diagnostics. Add verbose, grep-friendly trace or debug logging
before returning for empty/"cloud" sentinels and for PROVIDER_OPENHUMAN,
identifying the branch and role without logging secrets or full PII; preserve
the existing skip behavior.
In `@src/openhuman/security/egress/types.rs`:
- Around line 71-85: Update the IdentificationRisk enum derives to include
PartialOrd and Ord, preserving its declared variant order from Unknown through
High so threshold comparisons such as risk_level >= Medium work as documented.
🪄 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: f078ada1-aa40-45a4-a300-63145efd904a
📒 Files selected for processing (18)
src/core/event_bus/events.rssrc/core/jsonrpc.rssrc/openhuman/channels/providers/web/event_bus.rssrc/openhuman/channels/providers/web/mod.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/composio/client.rssrc/openhuman/embeddings/cloud.rssrc/openhuman/inference/provider/factory.rssrc/openhuman/inference/provider/factory_tests.rssrc/openhuman/integrations/client.rssrc/openhuman/security/egress/emit.rssrc/openhuman/security/egress/emit_tests.rssrc/openhuman/security/egress/mod.rssrc/openhuman/security/egress/types.rssrc/openhuman/security/egress/types_tests.rssrc/openhuman/security/mod.rssrc/openhuman/tools/impl/network/http_request.rssrc/openhuman/tools/impl/network/web_fetch.rs
) A single managed chat turn resolves the primary model, each workload route tier, and the summarizer through the same egress chokepoint, so one prompt published multiple identical ExternalTransferPending events. Add a task-local dedup ledger (dedup_turn_scope) that collapses same destination/reason disclosures within a turn; absent outside the scope so single egress sites and CLI/cron paths still emit each transfer.
…re per destination (tinyhumansai#4436) Wrap the primary/route-tier/summarizer construction in build_turn_models_crate with dedup_turn_scope so a managed turn discloses each distinct destination once instead of once per model construction. Distinct tier models still disclose; the construction-time vs dispatch-time residual is documented.
…ses (tinyhumansai#4436) The top-level Provider-path disclosure fired before verify_session_active, so an external provider that failed the session gate published a transfer that never left the device. Move the emit below the gate (the managed OpenHuman path returns above it and self-discloses via resolve_managed_backend).
…nsai#4436) A header-only http_request (e.g. an Authorization token, no body) sent header values to the external host but the descriptor reported only the URL. Add DataKind::Metadata when headers are present via a pure, unit-tested network_egress_descriptor helper.
2 participants
Summary
EgressDescriptor { provider_slug, service, is_external, reason, data_kinds, risk_level, risk_categories }produced at every external-transfer point, so each transfer carries what leaves, to where, and why.DomainEvent::ExternalTransferPendingbefore external transfer, bridged to the frontendexternal_transfer_pendingweb-socket event (reuses the existingApprovalRequested → socketpattern).Providerand the post-feat(inference): Phase 3 P3-B — crate-native turn path + delete compatible*.rs [WIP] #4784 crate-nativeChatModel/turn path), Composio tool execution, backend integrations, embeddings, and network tools.risk_level/risk_categories(both#[serde(default)]) so downstream slices wire in without a breaking reshape.Problem
OpenHuman's local-first model is a differentiator, but professional users need to know, at the moment of use, what leaves the machine and to which service. Today egress is scattered across many call sites with no unified "what/where/why" notion. #4256 tracks the full capability; S1 (#4435, merged) shipped the Privacy Mode switch. This PR lands the spine every later slice (S3 disclosure UI, S4 approval, S7 enforcement) keys off — without it, coverage would be inconsistent per surface (fails the epic's AC5).
Solution
security/egress/module: theEgressDescriptortype with semantic constructors (inference/composio/integration/embedding/network_fetch) +with_riskbuilder, andemit_external_transfer()— the one call every egress point makes right before the transfer leaves (drops local-only, attaches ambient chat routing, publishes the domain event).DomainEvent::ExternalTransferPending+ anEgressSurfaceSubscriberthat bridges it to the web-channel socket, registered at startup.create_chat_provider_from_string(Provider) path; a live staging smoke revealed the default managed-backend chat turn builds via the crate-nativecreate_chat_model_with_model_id(ChatModel) path and disclosed nothing. Fixed by emitting at the three universal chokepoints all ChatModel/turn builds funnel through —resolve_managed_backend(managed = external),try_create_local_runtime_chat_model_*(local = non-external),try_create_cloud_slug_chat_model_*(BYOK = external) — and makingemit_inference_egressskipPROVIDER_OPENHUMANso a construction fires exactly one descriptor on either path.tools/impl/network/curl.rs,polymarket*, integrationspatch/delete/upload_multipart/get_bytes) are left for S7 (feat(privacy): S7 — Local-only enforcement across integrations + tools (#4256) #4441), which must gate those same sites for enforcement — tracked there rather than widened here to keep the spine reviewable.Submission Checklist
create_chat_provider_from_stringandcreate_chat_model_with_model_idasserting exactly one emit on external construction and none on local runtime.cargo-llvm-cov+diff-covervsupstream/main, single-threaded): 89% on changed lines (factory.rs96.2%). The remaining uncovered lines are startup-registration + network-tool emit sites that only execute at app runtime (startup registration confirmed running via the live boot smoke below).## Related— N/A (no matrix feature row for this infra slice).dev:appon staging +inference-probe --mode raw: the default managed-backend construction now logs[privacy][egress] ExternalTransferPending provider=openhuman service=reasoning-v1 reason=Inference(was zero before the ChatModel-path fix), and the socket-bridge subscriber registers at boot.Closes #NNNin the## RelatedsectionImpact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— N/A (no frontend changes; backend-only slice)pnpm typecheck— N/A (no TS changes)cargo test --lib openhuman::security::egress+ factory real-path egress tests (all green); liveinference-probe --mode rawon stagingcargo fmt --check+cargo checkcleanpnpm rust:check(src-tauri) clean (pre-push hook)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
ExternalTransferPendingdisclosure event; local-only transfers are disclosed as non-external and publish nothing. No transfer is blocked or altered.external_transfer_pendingnow.Summary by CodeRabbit
New Features
Bug Fixes