From 3bbd8458b7bd00292cd79039396f01266d4b0a90 Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Thu, 23 Jul 2026 17:33:09 +0530 Subject: [PATCH 1/8] fix(flows): make native `oh:` tool results usable and fail loudly, document the real attachment chain Three defects found while proving out file attachment end to end against the live Composio catalog. 1. Native tool failures were silently swallowed. The `oh:` branch of `OpenHumanTools::invoke` serialized the `ToolResult` without ever inspecting `is_error`, so a tool that ran and FAILED (quota exceeded, file missing, no integration client) recorded the step, and therefore the run, as Success. A downstream node then bound a null value and the run still reported "completed". `reject_failed_native_tool_result` mirrors the contract `reject_unsuccessful_composio_response` already provides for Composio. 2. Native tool output was unbindable in practice. Serializing the whole `ToolResult` put the envelope on `item.json`, so reaching a field required `=nodes..item.json.content[0].data.`, an expression that evaluates but that no builder agent ever emits. `native_tool_payload` returns a lone `Json` block's `data` directly, so native nodes bind with the same `=nodes..item.json.` shape as everything else. This also makes prompt.md's existing claim (native `oh:` tool_calls carry no `data` wrapper and use the plain form) actually true. 3. Presigned storage links leaked into the approval UI and durable storage. `redact_args` did not treat file handoff params as sensitive, so a presigned URL, which is a bearer capability until it expires, appeared verbatim on the approval card and in the persisted approval record. Prompt guidance now documents the chain that actually works: produce the file, `oh:storage_upload_file`, `oh:storage_get_link` with a short TTL, then bind the returned `url` into the send action's file parameter. The file parameter is found by its `file_uploadable: true` marker in `get_tool_contract`, not by name (Gmail calls it `attachment`, Jira calls it `file_to_upload`). --- src/openhuman/approval/redact.rs | 44 ++++++ .../flows/agents/workflow_builder/prompt.md | 37 +++++ src/openhuman/tinyflows/caps.rs | 129 +++++++++++++++++- 3 files changed, 207 insertions(+), 3 deletions(-) diff --git a/src/openhuman/approval/redact.rs b/src/openhuman/approval/redact.rs index 2af2e41661..559e196441 100644 --- a/src/openhuman/approval/redact.rs +++ b/src/openhuman/approval/redact.rs @@ -69,6 +69,23 @@ const SENSITIVE_KEYS: &[&str] = &[ "authorization", "auth", "code", + // File-handoff params. A presigned storage link (`storage_get_link`) is a + // BEARER CAPABILITY: anyone holding the URL can fetch the file until it + // expires. These land in `tool_call` args whenever a flow hands a produced + // file to an externally-executed action (a Composio `file_uploadable` + // param such as Gmail's `attachment` or Jira's `file_to_upload`). Redacted + // args are both rendered on the approval card and persisted with the + // approval record, so leaving these clear would leak the capability into + // the UI and durable storage. + "attachment", + "attachments", + "file_to_upload", + "file_url", + "url", + "link", + "public_url", + "signed_url", + "presigned_url", ]; /// Produce a redacted clone of `args` suitable for persistence / @@ -432,6 +449,33 @@ mod tests { assert_eq!(summary.matches("").count(), 2); } + #[test] + fn file_handoff_links_are_redacted() { + // A presigned storage link is a bearer capability: anyone holding the + // URL can fetch the file until it expires. Redacted args are shown on + // the approval card AND persisted, so these must never appear clear. + let args = json!({ + "attachment": "https://files.example.test/f_1?sig=SECRETSIGNATURE", + "file_to_upload": "https://files.example.test/f_2?sig=ANOTHERSIG", + "url": "https://files.example.test/f_3?sig=THIRDSIG", + "public_url": "https://files.example.test/f_4?sig=FOURTHSIG", + }); + let red = redact_args(&args); + let blob = red.to_string(); + for leaked in [ + "SECRETSIGNATURE", + "ANOTHERSIG", + "THIRDSIG", + "FOURTHSIG", + "files.example.test", + ] { + assert!( + !blob.contains(leaked), + "presigned link leaked through redaction ({leaked}): {blob}" + ); + } + } + #[test] fn summarize_action_pulls_safe_fields() { let args = json!({ diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index 3dd5028101..adb3760ba3 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -551,6 +551,43 @@ And without `input_context`, don't reach for a jq expression woven into — that's prose, not jq, resolves to `null`, and both the `save_workflow` gate and `dry_run_workflow`'s `agent_prompt_nulls` will reject it. +### Attaching a produced file to an external action + +When the user wants a file **attached** (email attachment, Jira attachment, and +so on), the file must be handed over as a **URL the provider's servers can +fetch**. Composio actions execute on Composio's backend, so a local filesystem +path can never work: it fails at run time with `Error reading file at +/Users/... ENOENT`. Putting the content in the message body does **not** +satisfy an attachment request either. + +The working chain is three nodes: + +1. **Produce the file.** An `agent` node (usually `agent_ref: "code_executor"`) + or a `code` node writes it, then calls **`oh:storage_upload_file`** with the + local `path`. Bind its `file_id` downstream. +2. **Mint a short-lived link.** A `tool_call` on **`oh:storage_get_link`** with + `{ "file_id": "=nodes..item.json.file_id", "expires_in_seconds": 300 }` + returns `{ url, expires_at }`. Bind `=nodes..item.json.url`. + Prefer a short TTL: the provider fetches within seconds, and the URL is a + bearer capability for as long as it lives. + **Do not** upload with `visibility: "public"` to get a `public_url` instead. + That leaves a permanently world readable object; the presigned link expires. +3. **Send it.** A `tool_call` on the provider action, binding the link URL into + that action's file parameter. + +**Find the file parameter by its marker, never by guessing a name.** Call +`get_tool_contract` on the send action and look in `input_schema.properties` +for the property carrying **`"file_uploadable": true`** (it also shows +`"format": "path"`). The name differs per provider: Gmail's `GMAIL_SEND_EMAIL` +calls it `attachment`, Jira's `JIRA_ADD_ATTACHMENT` calls it `file_to_upload`. +`GMAIL_SEND_EMAIL` accepts a single value or a list, and Gmail caps total +message size at roughly 25 MB. + +**Do not invent a dedicated attachment action.** There is no +`GMAIL_SEND_EMAIL_WITH_ATTACHMENT`; `GMAIL_SEND_EMAIL` takes the attachment +directly. If `get_tool_contract` reports a slug is not a real action, that is a +hard stop: go back to `search_tool_catalog` rather than wiring it anyway. + ### Trigger kinds — which ones actually fire Set `config.trigger_kind` on the trigger node. **Only three fire automatically diff --git a/src/openhuman/tinyflows/caps.rs b/src/openhuman/tinyflows/caps.rs index 54c2e1ef68..de21228bb5 100644 --- a/src/openhuman/tinyflows/caps.rs +++ b/src/openhuman/tinyflows/caps.rs @@ -2631,6 +2631,63 @@ fn reject_unsuccessful_composio_response( ))) } +/// Native-tool analogue of [`reject_unsuccessful_composio_response`]. +/// +/// `execute_tool` returns `Ok(outcome)` for a tool that *ran* but *failed* — +/// the failure rides on [`ToolResult::is_error`] (quota exceeded, file missing, +/// no integration client configured). Nothing downstream inspected that flag, +/// so the tinyflows engine recorded the step — and therefore the run — as +/// `Success` even though the tool never did its job. Concretely: a file-upload +/// step could fail, the next node would bind a `null` URL, and the run still +/// reported "completed". +/// +/// Mirrors the Composio branch's contract so both paths turn a failed step into +/// `StepStatus::Error` (and, via `degrade_completed_status`, a failed run) +/// rather than a false "Completed". +fn reject_failed_native_tool_result( + slug: &str, + result: &crate::openhuman::skills::types::ToolResult, +) -> Result<()> { + if !result.is_error { + return Ok(()); + } + let rendered = result.output(); + let detail = match rendered.trim() { + "" => "no error detail returned by the tool", + d => d, + }; + tracing::warn!( + target: "flows", + %slug, + %detail, + "[flows] tool_call: native tool reported is_error — failing the step" + ); + Err(EngineError::Capability(format!( + "tool_call `{slug}` failed: {detail}" + ))) +} + +/// Unwraps a native (`oh:`) tool's [`ToolResult`] into the value a downstream +/// node actually binds against. +/// +/// Serializing the `ToolResult` verbatim (the previous behavior) placed the +/// whole envelope on `item.json`, so reaching a field required +/// `=nodes..item.json.content[0].data.`. That expression does +/// evaluate, but no builder agent ever emits it, which left native tools +/// effectively unbindable in practice. +/// +/// A lone `Json` block therefore returns its `data` directly, so a native node +/// binds with the same `=nodes..item.json.` shape used everywhere +/// else. Anything else (plain text, or mixed/multiple blocks) collapses to +/// `{ "text": }` so there is always a predictable field to bind. +fn native_tool_payload(result: &crate::openhuman::skills::types::ToolResult) -> Value { + use crate::openhuman::skills::types::ToolContent; + match result.content.as_slice() { + [ToolContent::Json { data }] => data.clone(), + _ => json!({ "text": result.output() }), + } +} + /// A [`ToolInvoker`] decorator that runs the host's Composio required-arg /// preflight before delegating to `inner`. /// @@ -2701,9 +2758,8 @@ impl ToolInvoker for OpenHumanTools { ) .await .map_err(EngineError::Capability)?; - return serde_json::to_value(&outcome.result).map_err(|e| { - EngineError::Capability(format!("could not serialize tool result: {e}")) - }); + return reject_failed_native_tool_result(slug, &outcome.result) + .map(|_| native_tool_payload(&outcome.result)); } // Autonomy-tier gate (Phase 2, made effect-aware): the node's @@ -3476,6 +3532,73 @@ mod tests { use super::*; use crate::openhuman::agent::prompts::types::IntegrationConnection; use crate::openhuman::composio::{ComposioExecuteResponse, ConnectedIntegration}; + use crate::openhuman::skills::types::{ToolContent, ToolResult}; + + // ── native `oh:` tool result handling ────────────────────────────────── + + #[test] + fn native_tool_payload_unwraps_a_single_json_block() { + // `storage_get_link` returns exactly one Json block. A downstream node + // must be able to bind `=nodes..item.json.url` — the same shape + // used everywhere else — not `...item.json.content[0].data.url`. + let result = ToolResult::json(json!({ + "url": "https://example.test/presigned", + "expires_at": "2026-01-01T00:00:00Z", + })); + let payload = native_tool_payload(&result); + assert_eq!(payload["url"], "https://example.test/presigned"); + assert_eq!(payload["expires_at"], "2026-01-01T00:00:00Z"); + assert!( + payload.get("content").is_none() && payload.get("is_error").is_none(), + "the ToolResult envelope must not leak into item.json: {payload}" + ); + } + + #[test] + fn native_tool_payload_collapses_text_to_a_bindable_field() { + let payload = native_tool_payload(&ToolResult::success("done")); + assert_eq!(payload["text"], "done"); + } + + #[test] + fn native_tool_payload_collapses_mixed_blocks_to_text() { + let result = ToolResult { + content: vec![ + ToolContent::Text { + text: "line".into(), + }, + ToolContent::Json { + data: json!({"k": 1}), + }, + ], + is_error: false, + markdown_formatted: None, + }; + let payload = native_tool_payload(&result); + let text = payload["text"].as_str().expect("text field"); + assert!(text.contains("line") && text.contains('k'), "got {text}"); + } + + #[test] + fn native_tool_failure_fails_the_step_instead_of_recording_success() { + // The bug this guards: `execute_tool` returns Ok for a tool that ran + // and FAILED (is_error), so the engine recorded the step — and the run + // — as Success while a downstream node bound a null value. + let result = ToolResult::error("storage quota exceeded"); + let err = reject_failed_native_tool_result("oh:storage_upload_file", &result) + .expect_err("an is_error ToolResult must fail the step"); + let msg = format!("{err:?}"); + assert!( + msg.contains("storage_upload_file") && msg.contains("storage quota exceeded"), + "error must name the tool and the provider detail: {msg}" + ); + } + + #[test] + fn native_tool_success_passes_through() { + let result = ToolResult::json(json!({"file_id": "f_1"})); + assert!(reject_failed_native_tool_result("oh:storage_upload_file", &result).is_ok()); + } // ── reject_unsuccessful_composio_response (B6) ────────────────────────── From 90bed55c13c5e9f197c01f22b6a025d1f688709b Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Thu, 23 Jul 2026 17:47:08 +0530 Subject: [PATCH 2/8] docs(flows): make the attachment guidance provider agnostic The first draft of this section leaked provider specifics into a rule whose whole point is "do not assume provider specifics": it named which toolkit calls the parameter what, and restated one provider's message size cap. That cap is already in the live contract's `description`, so repeating it in the prompt both duplicates `get_tool_contract` and undercuts the instruction to ground in it. Remembered limits also go stale silently. Now the rule stands on the `file_uploadable` marker alone, and explicitly sends the model to the contract for arity and limits. Provider names are gone from the section; the only named tools left are our own (`storage_upload_file`, `storage_get_link`), which are the actual API being described. --- .../flows/agents/workflow_builder/prompt.md | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index adb3760ba3..76b991e4bd 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -553,9 +553,9 @@ and `dry_run_workflow`'s `agent_prompt_nulls` will reject it. ### Attaching a produced file to an external action -When the user wants a file **attached** (email attachment, Jira attachment, and -so on), the file must be handed over as a **URL the provider's servers can -fetch**. Composio actions execute on Composio's backend, so a local filesystem +When the user wants a file **attached** to something you send or create through +a connected integration, the file must be handed over as a **URL the provider's +servers can fetch**. Composio actions execute on Composio's backend, so a local filesystem path can never work: it fails at run time with `Error reading file at /Users/... ENOENT`. Putting the content in the message body does **not** satisfy an attachment request either. @@ -578,15 +578,21 @@ The working chain is three nodes: **Find the file parameter by its marker, never by guessing a name.** Call `get_tool_contract` on the send action and look in `input_schema.properties` for the property carrying **`"file_uploadable": true`** (it also shows -`"format": "path"`). The name differs per provider: Gmail's `GMAIL_SEND_EMAIL` -calls it `attachment`, Jira's `JIRA_ADD_ATTACHMENT` calls it `file_to_upload`. -`GMAIL_SEND_EMAIL` accepts a single value or a list, and Gmail caps total -message size at roughly 25 MB. - -**Do not invent a dedicated attachment action.** There is no -`GMAIL_SEND_EMAIL_WITH_ATTACHMENT`; `GMAIL_SEND_EMAIL` takes the attachment -directly. If `get_tool_contract` reports a slug is not a real action, that is a -hard stop: go back to `search_tool_catalog` rather than wiring it anyway. +`"format": "path"`). That marker is the contract across toolkits; the property +name is not, so read it off the contract every time rather than assuming a +convention (one toolkit's is `attachment`, another's is `file_to_upload`). + +Everything else about that parameter also comes from the contract: whether it +accepts one value or a list, and any size or type limits the provider enforces, +are stated in its schema and `description`. Read them there. Do not rely on +remembered provider limits. + +**Do not invent a dedicated attachment action.** Send actions generally take +the file on the ordinary send, so search for an attachment-capable send before +assuming a separate one exists. If `get_tool_contract` reports a slug is not a +real action, that is a hard stop: go back to `search_tool_catalog` and pick a +real one rather than wiring it anyway. A slug that merely looks plausible by +naming convention is the single most expensive mistake you can make here. ### Trigger kinds — which ones actually fire From ff9d80094f01d4b5766537487b7f6152c033e852 Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Thu, 23 Jul 2026 17:52:54 +0530 Subject: [PATCH 3/8] docs(flows): keep the file handle out of agent structured output The first version of this chain had the producing agent call `storage_upload_file` and emit a `file_id` through `output_parser`. That routes the file handle through model-authored JSON, which is exactly the failure mode that kills these runs in practice (`missing required property file_path` / `file_id`, after the harness falls back to the `{text}` shape). Split the upload into its own `tool_call` node and have the producer write to a path chosen by the builder. The upload node then references that path as a literal, so `file_id` is a node output rather than model-authored JSON, and no `output_parser` sits anywhere in the file path. The producing agent's side effect on disk is what matters, not its output. --- .../flows/agents/workflow_builder/prompt.md | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index 76b991e4bd..96098d7a39 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -560,19 +560,30 @@ path can never work: it fails at run time with `Error reading file at /Users/... ENOENT`. Putting the content in the message body does **not** satisfy an attachment request either. -The working chain is three nodes: - -1. **Produce the file.** An `agent` node (usually `agent_ref: "code_executor"`) - or a `code` node writes it, then calls **`oh:storage_upload_file`** with the - local `path`. Bind its `file_id` downstream. -2. **Mint a short-lived link.** A `tool_call` on **`oh:storage_get_link`** with +The working chain is four nodes. **Keep the file handle out of any agent's +structured output.** Give the producer a fixed path you choose, then refer to +that same literal path from a separate upload node. An agent that has to emit a +`file_id` or `file_path` through `output_parser` is a schema mismatch waiting to +fail the run; the file only needs to exist on disk, so the agent's *side effect* +is what matters, not its output. + +1. **Produce the file at a path YOU chose.** An `agent` node (usually + `agent_ref: "code_executor"`) or a `code` node writes it. State the exact + absolute path in the prompt, e.g. "write the page to + `/tmp/openhuman-flow/report.html`". Do **not** give this node an + `output_parser` schema for the file, and do **not** bind anything off it. +2. **Upload it.** A `tool_call` on **`oh:storage_upload_file`** with that same + path as a **literal** string: `{ "path": "/tmp/openhuman-flow/report.html" }`. + Because this is a real node, its `file_id` is a node output rather than + model-authored JSON: bind `=nodes..item.json.file_id`. +3. **Mint a short-lived link.** A `tool_call` on **`oh:storage_get_link`** with `{ "file_id": "=nodes..item.json.file_id", "expires_in_seconds": 300 }` returns `{ url, expires_at }`. Bind `=nodes..item.json.url`. Prefer a short TTL: the provider fetches within seconds, and the URL is a bearer capability for as long as it lives. **Do not** upload with `visibility: "public"` to get a `public_url` instead. That leaves a permanently world readable object; the presigned link expires. -3. **Send it.** A `tool_call` on the provider action, binding the link URL into +4. **Send it.** A `tool_call` on the provider action, binding the link URL into that action's file parameter. **Find the file parameter by its marker, never by guessing a name.** Call From 16af573cfec35f94de42f4bb970c36ef54bf979b Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Thu, 23 Jul 2026 23:22:25 +0530 Subject: [PATCH 4/8] fix(flows): stop the author-gate rejecting a null bound to a native oh: upstream The null-arg author-gate (`validate_required_arg_resolvability`) and the dry-run `null_resolutions` diagnostic both delegate "is this null unverifiable vs a real wiring bug?" to one helper, which excluded native `oh:` tool_call upstreams. So a Composio send node binding `=nodes.get_link.item.json.url` from a native `oh:storage_get_link` node fell through to a HARD REJECT, because that url is null in the echo sandbox (native tools are opaque-echoed, exactly like Composio ones). This blocked the exact produce -> upload -> get_link -> send chain the attachment guidance prescribes: an independent judge traced a live "fix with agent" self-repair loop that built the correct chain, got gate-rejected four times, and halted on repeated tool failure. Native and Composio tool_call outputs are both opaque in the sandbox, so a null bound to either is unverifiable, not broken. Generalize `composio_tool_call_upstream_ref` -> `mock_opaque_tool_call_upstream_ref` and drop the `oh:` exclusion (keep excluding `=`-dynamic slugs). The dry-run suggestion now adapts to the upstream kind: a native `oh:` upstream binds FLAT at `.item.json.` (no `.data.` wrapper, no Composio `get_tool_contract`), so pointing it at the Composio `.data.` advice would send the agent chasing a path that never exists. The gate is NOT weakened: an agent/code/transform/trigger upstream null still hard-rejects (B18 intact, guarded by the unchanged reject test). Adds a direct unit for the helper, a downgrade test for the native-upstream case, and the drift check that was missing pre-merge (author the documented native chain, assert the gate passes it). --- src/openhuman/flows/builder_tools.rs | 62 +++++++++++----- src/openhuman/flows/ops.rs | 69 ++++++++++------- src/openhuman/flows/ops_tests.rs | 107 +++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 45 deletions(-) diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index 170c3c0bc0..2f756f5a0e 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -2587,32 +2587,52 @@ impl Tool for GetNodeKindContractTool { /// /// The common case reports `{ node_id, location, expression }` — a wiring /// mistake the agent should fix. But when the null-resolved expression binds to -/// the output of an upstream Composio `tool_call` node -/// ([`ops::composio_tool_call_upstream_ref`]), the entry is instead marked +/// the output of an upstream Composio-or-native `tool_call` node +/// ([`ops::mock_opaque_tool_call_upstream_ref`]), the entry is instead marked /// `unverifiable: true` and carries an honest `suggestion`: the echo sandbox -/// can NEVER produce a Composio tool's real output fields, so this particular -/// null is expected here and does NOT prove the binding wrong (WS6 — the -/// transcript audit where the agent re-wired an already-correct binding three -/// times chasing this exact false negative). The message points at -/// `get_tool_contract` / `get_tool_output_sample` as the real disambiguators. +/// can NEVER produce a tool's real output fields, so this particular null is +/// expected here and does NOT prove the binding wrong (WS6 — the transcript +/// audit where the agent re-wired an already-correct binding three times +/// chasing this exact false negative). The suggestion adapts to the upstream +/// kind: a Composio upstream points at `get_tool_contract` / +/// `get_tool_output_sample` and the `.item.json.data.` nesting; a native `oh:` +/// upstream points at the flat `.item.json.` shape instead. fn build_null_resolution_entry( node_id: &str, diag: &tinyflows::expr::NullResolution, graph: &WorkflowGraph, ) -> Value { - if let Some(upstream) = crate::openhuman::flows::ops::composio_tool_call_upstream_ref( + if let Some(upstream) = crate::openhuman::flows::ops::mock_opaque_tool_call_upstream_ref( &diag.expression, graph, node_id, ) { let field = diag.location.strip_prefix("args.").unwrap_or("args"); - return json!({ - "node_id": node_id, - "location": diag.location, - "expression": diag.expression, - "unverifiable": true, - "upstream_tool_call": upstream, - "suggestion": format!( + // The disambiguation advice differs by upstream kind: a native `oh:` + // tool's output binds FLAT (`.item.json.`) after + // `native_tool_payload`'s unwrap — it has no `.data.` wrapper and no + // Composio `get_tool_contract` — whereas a Composio action nests under + // `.item.json.data.`. Emitting the Composio advice for a native + // upstream would send the agent chasing a `.data.` path that will + // never exist. + let upstream_is_native = graph + .nodes + .iter() + .find(|n| n.id == upstream) + .and_then(|n| n.config.get("slug").and_then(Value::as_str)) + .is_some_and(|s| s.starts_with("oh:")); + let suggestion = if upstream_is_native { + format!( + "required arg `{field}` binds to the output of native tool_call node \ + `{upstream}` — the SANDBOX only echoes tool calls and can never produce \ + their real output fields, so this binding is UNVERIFIABLE here (not \ + necessarily wrong). A native `oh:` tool's real output binds FLAT at \ + `=nodes.{upstream}.item.json.` (no `.data.` wrapper). Confirm the \ + field name against that tool's own output shape. It is a real bug only if \ + the path doesn't match the tool's actual output." + ) + } else { + format!( "required arg `{field}` binds to the output of Composio tool_call node \ `{upstream}` — the SANDBOX only echoes tool calls and can never produce \ their real output fields, so this binding is UNVERIFIABLE here (not \ @@ -2621,7 +2641,15 @@ fn build_null_resolution_entry( `.item.json.data.`), or get_tool_output_sample {{ slug, args }} for the \ real shape. It is a real bug only if the path doesn't match the action's \ actual output." - ), + ) + }; + return json!({ + "node_id": node_id, + "location": diag.location, + "expression": diag.expression, + "unverifiable": true, + "upstream_tool_call": upstream, + "suggestion": suggestion, }); } json!({ @@ -2634,7 +2662,7 @@ fn build_null_resolution_entry( /// Every null-resolved `args.*` config expression that landed on a `tool_call` /// node, as `null_resolutions` diagnostic entries (see /// [`build_null_resolution_entry`] for the shape, including the WS6 -/// `unverifiable` Composio-upstream variant). Shared by the settled-run path +/// `unverifiable` Composio-or-native-upstream variant). Shared by the settled-run path /// (which fails the dry run on these) and the errored-run path (which surfaces /// only the `unverifiable` ones so a stop-policy preflight abort explains /// itself honestly instead of via the generic required-arg text). diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index f6ae6e6a40..8ab64974be 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -2371,19 +2371,21 @@ pub(crate) async fn validate_required_arg_resolvability(graph: &WorkflowGraph) - ); continue; } - // A null bound to the OUTPUT of an upstream Composio `tool_call` - // node is UNVERIFIABLE in this echo sandbox — the mock renders a - // Composio `tool_call` as `{tool, args, connection}` and can NEVER - // produce its real output fields (`.item.json.data.`), so a - // downstream binding to one resolves `null` here even when the - // wiring is perfectly correct. Hard-rejecting it (WS6) would block - // a possibly-correct graph from ever being proposed — the exact - // false-negative the transcript audit caught. Downgrade to a - // debug-logged skip; `dry_run_workflow` remains the surface that - // reports it (as an `unverifiable` diagnostic the agent can act on - // via get_tool_contract / get_tool_output_sample). + // A null bound to the OUTPUT of an upstream Composio-or-native + // `tool_call` node is UNVERIFIABLE in this echo sandbox — the mock + // renders BOTH a Composio and a native `oh:` `tool_call` as + // `{tool, args, connection}` and can NEVER produce their real output + // fields (`.item.json.data.` for Composio, `.item.json.` + // for a native tool), so a downstream binding to one resolves `null` + // here even when the wiring is perfectly correct. Hard-rejecting it + // (WS6) would block a possibly-correct graph from ever being proposed + // — the exact false-negative the transcript audit caught, and the one + // that made this gate reject #5148's own native-attachment chain. + // Downgrade to a debug-logged skip; `dry_run_workflow` remains the + // surface that reports it (as an `unverifiable` diagnostic the agent + // can act on via get_tool_contract / get_tool_output_sample). if let Some(upstream) = - composio_tool_call_upstream_ref(&diag.expression, graph, &step.node_id) + mock_opaque_tool_call_upstream_ref(&diag.expression, graph, &step.node_id) { tracing::debug!( target: "flows", @@ -2392,7 +2394,7 @@ pub(crate) async fn validate_required_arg_resolvability(graph: &WorkflowGraph) - %field, upstream = %upstream, expression = %diag.expression, - "[flows] required-arg resolvability check: arg binds to a Composio \ + "[flows] required-arg resolvability check: arg binds to a Composio-or-native \ tool_call's output — UNVERIFIABLE in the echo sandbox (the mock cannot \ produce real tool output fields), not rejecting; dry_run_workflow \ reports it instead" @@ -2515,18 +2517,25 @@ fn is_trigger_scoped_expression( } /// If a null-resolved config expression on `node_id` is bound to the OUTPUT of -/// an upstream **Composio `tool_call`** node (a `tool_call` whose `slug` is a -/// real Composio action — not `=`-derived, not native `oh:`), returns that -/// upstream node's id; otherwise `None`. +/// an upstream **`tool_call`** node whose sandbox output is an opaque echo — a +/// Composio curated action OR a native `oh:` tool (anything but a `=`-derived +/// dynamic slug) — returns that upstream node's id; otherwise `None`. /// -/// The dry-run / gate sandbox renders a Composio `tool_call` as a deterministic -/// echo (`{tool, args, connection}`) and can NEVER produce its real output -/// fields, so a downstream binding to `.item.json.data.` off such a node -/// resolves `null` in the sandbox **even when the wiring is correct** — the -/// binding is UNVERIFIABLE here, not necessarily broken. Callers use this to -/// tell that honest-uncertainty case apart from a genuinely broken binding -/// (one wired to an `agent` / `transform` / `code` / trigger upstream, whose -/// real output the sandbox DOES produce, so a null there IS a real bug). +/// The dry-run / gate sandbox renders BOTH a Composio `tool_call` and a native +/// `oh:` `tool_call` as a deterministic echo (`{tool, args, connection}`) and +/// can NEVER produce their real output fields, so a downstream binding off such +/// a node (`.item.json.data.` for Composio, or `.item.json.` for +/// a native tool after `native_tool_payload`'s unwrap) resolves `null` in the +/// sandbox **even when the wiring is correct** — the binding is UNVERIFIABLE +/// here, not necessarily broken. Callers use this to tell that honest- +/// uncertainty case apart from a genuinely broken binding (one wired to an +/// `agent` / `transform` / `code` / trigger upstream, whose real output the +/// sandbox DOES produce, so a null there IS a real bug). +/// +/// The native `oh:` case is why this exists beyond Composio: #5148's guidance +/// prescribes a `produce -> oh:storage_upload_file -> oh:storage_get_link -> +/// send` chain where the send binds `=nodes.get_link.item.json.url`; excluding +/// native upstreams here made the gate hard-reject that exact (correct) chain. /// /// Handles both addressing forms the engine can trace: /// - explicit `=nodes....` / `=.nodes[""]...` (parsed via @@ -2536,9 +2545,9 @@ fn is_trigger_scoped_expression( /// ambiguous fan-in is never mis-attributed to a single upstream node. /// /// Anything else (a `=run...` trigger reference, a jq expression not rooted at -/// one of the above, or a reference to a non-`tool_call` / native / dynamic -/// node) returns `None`. -pub(crate) fn composio_tool_call_upstream_ref<'a>( +/// one of the above, or a reference to a non-`tool_call` / `=`-dynamic node) +/// returns `None`. +pub(crate) fn mock_opaque_tool_call_upstream_ref<'a>( expr: &str, graph: &'a WorkflowGraph, node_id: &str, @@ -2574,7 +2583,11 @@ pub(crate) fn composio_tool_call_upstream_ref<'a>( return None; } let slug = node.config.get("slug").and_then(Value::as_str)?; - if slug.starts_with('=') || slug.starts_with("oh:") { + // A `=`-derived slug is a dynamic runtime slug we can't reason about. But a + // native `oh:` tool_call IS opaque-echoed by the mock exactly like a + // Composio one, so its downstream null is equally unverifiable, not broken — + // do NOT exclude it (that exclusion made the gate reject #5148's own chain). + if slug.starts_with('=') { return None; } Some(node.id.as_str()) diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 22eac2a248..3e6fe6cb06 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -3675,6 +3675,113 @@ async fn validate_required_arg_resolvability_ignores_native_and_dynamic_slugs() assert!(errors.is_empty(), "{errors:?}"); } +#[tokio::test] +async fn mock_opaque_tool_call_upstream_ref_matches_native_and_composio_upstreams() { + // Both a Composio curated action and a native `oh:` tool are opaque-echoed + // by the mock sandbox, so a null bound to EITHER is unverifiable (Some). + // An `agent` / `code` upstream's real output IS produced by the sandbox, and + // a `=`-dynamic slug is unknowable, so a null bound to those is genuine (None). + let g = graph(json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "code_up", "kind": "code", "name": "Code", + "config": { "language": "javascript", "source": "return {};" } }, + { "id": "agent_up", "kind": "agent", "name": "Agent", + "config": { "agent_ref": "researcher", "prompt": "x" } }, + { "id": "native_up", "kind": "tool_call", "name": "Link", + "config": { "slug": "oh:storage_get_link", "args": { "file_id": "f" } } }, + { "id": "composio_up", "kind": "tool_call", "name": "Profile", + "config": { "slug": "GMAIL_GET_PROFILE", "args": {} } }, + { "id": "dyn_up", "kind": "tool_call", "name": "Dyn", + "config": { "slug": "=item.slug", "args": {} } }, + { "id": "sink", "kind": "tool_call", "name": "Sink", + "config": { "slug": "GMAIL_SEND_EMAIL", "args": {} } } + ], + "edges": [] + })); + let up = |expr: &str| mock_opaque_tool_call_upstream_ref(expr, &g, "sink").map(str::to_string); + assert_eq!( + up("=nodes.native_up.item.json.url").as_deref(), + Some("native_up") + ); + assert_eq!( + up("=nodes.composio_up.item.json.data.emailAddress").as_deref(), + Some("composio_up") + ); + assert_eq!(up("=nodes.agent_up.item.json.field"), None); + assert_eq!(up("=nodes.code_up.item.json.field"), None); + assert_eq!(up("=nodes.dyn_up.item.json.x"), None); +} + +#[tokio::test] +async fn validate_required_arg_resolvability_downgrades_null_from_native_tool_call_upstream() { + // #5148's chain: a Composio `send` binds its `attachment` to a native + // `oh:storage_get_link` node's `url`. That `url` is null in the echo sandbox + // (native tools are opaque-echoed), but the wiring is correct, so the gate + // must NOT reject it. Before the native-upstream carve-out it did — the loop + // that halted the live "fix with agent" self-repair. + let g = graph(json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "prep", "kind": "code", "name": "Prep", + "config": { "language": "javascript", "source": "return {};" } }, + { "id": "get_link", "kind": "tool_call", "name": "Link", + "config": { "slug": "oh:storage_get_link", "args": { "file_id": "f_1" } } }, + { "id": "send", "kind": "tool_call", "name": "Send", + "config": { "slug": "GMAIL_SEND_EMAIL", + "args": { "recipient_email": "a@b.com", "subject": "hi", "body": "there", + "attachment": "=nodes.get_link.item.json.url" } } } + ], + "edges": [ + { "from_node": "t", "to_node": "prep" }, + { "from_node": "prep", "to_node": "get_link" }, + { "from_node": "get_link", "to_node": "send" } + ] + })); + let errors = validate_required_arg_resolvability(&g).await; + assert!( + errors.is_empty(), + "a native-upstream attachment null must be downgraded, got: {errors:?}" + ); +} + +#[tokio::test] +async fn native_file_attachment_chain_passes_required_arg_resolvability() { + // Drift check that was missing pre-merge: author #5148's OWN documented + // `produce -> oh:storage_upload_file -> oh:storage_get_link -> send` chain + // and assert the null-arg gate (the exact gate that rejected it in the live + // "fix with agent" loop) now passes it. Targets `validate_required_arg_ + // resolvability` directly (deterministic, no live catalog) rather than + // `run_builder_gates`, whose connection/contract gates need live Composio. + let g = graph(json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "make_page", "kind": "code", "name": "Write", + "config": { "language": "javascript", "source": "return {};" } }, + { "id": "upload", "kind": "tool_call", "name": "Upload", + "config": { "slug": "oh:storage_upload_file", "args": { "path": "report.html" } } }, + { "id": "get_link", "kind": "tool_call", "name": "Link", + "config": { "slug": "oh:storage_get_link", + "args": { "file_id": "=nodes.upload.item.json.file_id", "expires_in_seconds": 900 } } }, + { "id": "send", "kind": "tool_call", "name": "Send", + "config": { "slug": "GMAIL_SEND_EMAIL", + "args": { "recipient_email": "a@b.com", "subject": "AI trends", "body": "attached", + "attachment": "=nodes.get_link.item.json.url" } } } + ], + "edges": [ + { "from_node": "t", "to_node": "make_page" }, + { "from_node": "make_page", "to_node": "upload" }, + { "from_node": "upload", "to_node": "get_link" }, + { "from_node": "get_link", "to_node": "send" } + ] + })); + let errors = validate_required_arg_resolvability(&g).await; + assert!( + errors.is_empty(), + "the documented native attachment chain must pass the null-arg gate, got: {errors:?}" + ); +} + /// (Codex feedback on PR #4826) This gate sandbox-runs every graph against /// `json!({})` as the trigger payload, so a `tool_call` arg wired straight to /// the trigger's own data — `"to": "=item.email"` on a node whose only From f1b0ceb2dbb57fb0a53eee83901afae2964903be Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Thu, 23 Jul 2026 23:22:26 +0530 Subject: [PATCH 5/8] docs(flows): correct the attachment guidance path and link TTL Two bugs the judge found in the attachment section's worked example: - It wrote the file to an absolute `/tmp/openhuman-flow/report.html`, which is dead on arrival: `resolve_upload_path` confines uploads to the agent workspace and the file_write policy blocks writes outside it. Use a workspace-relative path (`report.html`), and say plainly that an absolute path outside the workspace is rejected. - It recommended `expires_in_seconds: 300`, but the outbound send is parked for human approval for up to ~10 minutes; a user who approves late hands the provider a dead URL. Raise the recommended TTL to 900 with a note that it must outlive the approval window (and not run far longer than needed, since the URL is a bearer capability while it lives). --- .../flows/agents/workflow_builder/prompt.md | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index 96098d7a39..f6cd5cdc7e 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -567,20 +567,25 @@ that same literal path from a separate upload node. An agent that has to emit a fail the run; the file only needs to exist on disk, so the agent's *side effect* is what matters, not its output. -1. **Produce the file at a path YOU chose.** An `agent` node (usually - `agent_ref: "code_executor"`) or a `code` node writes it. State the exact - absolute path in the prompt, e.g. "write the page to - `/tmp/openhuman-flow/report.html`". Do **not** give this node an +1. **Produce the file at a workspace-relative path YOU chose.** An `agent` node + (usually `agent_ref: "code_executor"`) or a `code` node writes it. State the + path in the prompt, e.g. "write the page to `report.html`". Use a path + **relative to the working directory**, never an absolute path like + `/tmp/...`: writes and uploads are confined to the agent workspace, and an + absolute path outside it is rejected. Do **not** give this node an `output_parser` schema for the file, and do **not** bind anything off it. 2. **Upload it.** A `tool_call` on **`oh:storage_upload_file`** with that same - path as a **literal** string: `{ "path": "/tmp/openhuman-flow/report.html" }`. - Because this is a real node, its `file_id` is a node output rather than - model-authored JSON: bind `=nodes..item.json.file_id`. -3. **Mint a short-lived link.** A `tool_call` on **`oh:storage_get_link`** with - `{ "file_id": "=nodes..item.json.file_id", "expires_in_seconds": 300 }` + path as a **literal** string: `{ "path": "report.html" }`. Because this is a + real node, its `file_id` is a node output rather than model-authored JSON: + bind `=nodes..item.json.file_id`. +3. **Mint a link that outlives approval.** A `tool_call` on + **`oh:storage_get_link`** with + `{ "file_id": "=nodes..item.json.file_id", "expires_in_seconds": 900 }` returns `{ url, expires_at }`. Bind `=nodes..item.json.url`. - Prefer a short TTL: the provider fetches within seconds, and the URL is a - bearer capability for as long as it lives. + The send is an outbound action, so it may be parked for human approval for up + to ~10 minutes before it fires; the link's TTL must comfortably outlive that + window or the provider will fetch a dead URL. The URL is a bearer capability + for as long as it lives, so do not set it far longer than needed either. **Do not** upload with `visibility: "public"` to get a `public_url` instead. That leaves a permanently world readable object; the presigned link expires. 4. **Send it.** A `tool_call` on the provider action, binding the link URL into From 4729b6f69d93a93947bc41fe1b50dc0202be5bea Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Fri, 24 Jul 2026 00:13:33 +0530 Subject: [PATCH 6/8] style: cargo fmt --all (pinned 1.96.1) after merging main The merge brought pre-existing formatting drift on `main` (provider.rs, caps.rs) that the CI toolchain (1.96.1) flags but a newer local rustfmt did not. Reformatted with the pinned toolchain so `cargo fmt --all -- --check` passes. No logic change. --- .../agent/harness/subagent_runner/ops/provider.rs | 12 ++++++------ src/openhuman/tinyflows/caps.rs | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/openhuman/agent/harness/subagent_runner/ops/provider.rs b/src/openhuman/agent/harness/subagent_runner/ops/provider.rs index b43b9dcbba..b55284331a 100644 --- a/src/openhuman/agent/harness/subagent_runner/ops/provider.rs +++ b/src/openhuman/agent/harness/subagent_runner/ops/provider.rs @@ -252,8 +252,9 @@ pub(crate) struct LazyToolkitResolver { /// for a slug, subsequent `resolve()` calls for the same slug reuse the /// cached instance — sharing its [`ContractGate`] state (#5119). #[allow(dead_code)] // used via pub(super) from tests - pub(super) resolved: - std::sync::Mutex>>, + pub(super) resolved: std::sync::Mutex< + std::collections::HashMap>, + >, } /// Minimum normalized-slug length before the prefix/superstring tier in @@ -302,14 +303,13 @@ impl LazyToolkitResolver { } let action = self.find_action(name)?; - let tool: std::sync::Arc = std::sync::Arc::new( - crate::openhuman::composio::ComposioActionTool::new( + let tool: std::sync::Arc = + std::sync::Arc::new(crate::openhuman::composio::ComposioActionTool::new( self.config.clone(), action.name.clone(), action.description.clone(), action.parameters.clone(), - ), - ); + )); // Store in cache for future lookups. { diff --git a/src/openhuman/tinyflows/caps.rs b/src/openhuman/tinyflows/caps.rs index 7d28524019..985ebad9fa 100644 --- a/src/openhuman/tinyflows/caps.rs +++ b/src/openhuman/tinyflows/caps.rs @@ -442,7 +442,10 @@ fn extract_fenced_json_block(text: &str) -> Option { let fence_start = text.find("```")?; let after_fence = text[fence_start + 3..].trim(); // Skip optional "json" after the opening fence - let content = after_fence.strip_prefix("json").unwrap_or(after_fence).trim(); + let content = after_fence + .strip_prefix("json") + .unwrap_or(after_fence) + .trim(); // Find the *last* closing ``` (preferring the outermost fence, which // matches how Markdown renderers treat nested fences — the last ``` is // the one that closes the block the LLM opened). @@ -5783,11 +5786,7 @@ mod tests { "schema": { "type": "array" } } }); - let result = build_agent_result( - "agent-1", - "Here is the list: [1, 2, 3]", - &request, - ); + let result = build_agent_result("agent-1", "Here is the list: [1, 2, 3]", &request); assert_eq!(result, json!([1, 2, 3])); } @@ -5822,7 +5821,8 @@ mod tests { "schema": { "type": "object" } } }); - let text = "Some text\n```json\n{\"from_fence\": true}\n```\nmore text { \"from_brace\": true }"; + let text = + "Some text\n```json\n{\"from_fence\": true}\n```\nmore text { \"from_brace\": true }"; let result = build_agent_result("agent-1", text, &request); assert_eq!(result, json!({ "from_fence": true })); } From 056bb4d367ee9f8fca78ea2687780c09e757c95c Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Fri, 24 Jul 2026 15:28:17 +0530 Subject: [PATCH 7/8] fix(flows): enforce a workspace-relative storage_upload_file path at author time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live testing showed the builder proposing an `oh:storage_upload_file` node with `path: /tmp/openhuman-flow/report.html` — an absolute path the runtime `resolve_upload_path` rejects (uploads are confined to the agent workspace), so the run dies at the upload step. The standing prompt already says to use a workspace-relative path, and the running binary carries that guidance, but the model ignores it and copies an absolute path from a prior flow's example. This is the recurring "prompt guidance does not reliably steer the builder" failure, so enforce it in code the same way the gate carve-out did. Adds a cheap sync author-gate `validate_upload_paths`: an `oh:storage_upload_file` node whose LITERAL `path` is absolute (`/tmp/...`, `/Users/...`) or escapes with `..` is rejected with an actionable message naming a relative example. A `=`-expression path is left to the runtime (its value is unknown at author time); an absent path is left to the required-arg gate. Wired into `run_builder_gates`. Scope note: this enforces the UPLOAD side. The producing node's write path lives in that agent's natural-language prompt and cannot be structurally gated; a mismatch there still surfaces loudly (file_write policy + upload "file not found") rather than silently. --- src/openhuman/flows/ops.rs | 59 ++++++++++++++++++++++++++++++++ src/openhuman/flows/ops_tests.rs | 42 +++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index c3006931e7..c11880c52e 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -442,6 +442,57 @@ pub(crate) fn to_flow_validation_error( /// Assumes `graph` is already structurally valid (run /// `validate_and_migrate_graph` / `validate_all` first) — these gates check /// resolvability/contracts on a compilable graph. +/// +/// Author-gate for `oh:storage_upload_file`: its literal `path` arg must be +/// workspace-relative. Uploads are confined to the agent workspace by the +/// runtime `resolve_upload_path` (a canonicalized path that escapes `action_dir` +/// is rejected), so an absolute path like `/tmp/report.html` or one climbing out +/// with `..` cannot work — it fails mid-run at the upload step. The prompt tells +/// the builder to use a relative path, but the model reliably ignores that and +/// copies an absolute path from a prior flow's example, so this enforces it in +/// code (a hard, actionable author-gate) rather than trusting the prose. +/// +/// Only LITERAL paths are checked: a `=`-expression resolves from upstream data +/// at runtime and is out of scope here (the runtime check still applies). An +/// absent `path` is left to the required-arg gate. +pub(crate) fn validate_upload_paths(graph: &WorkflowGraph) -> Vec { + const UPLOAD_SLUG: &str = "oh:storage_upload_file"; + let mut errors = Vec::new(); + for node in &graph.nodes { + if node.kind != NodeKind::ToolCall { + continue; + } + if node.config.get("slug").and_then(Value::as_str) != Some(UPLOAD_SLUG) { + continue; + } + let Some(raw) = node + .config + .get("args") + .and_then(|a| a.get("path")) + .and_then(Value::as_str) + else { + continue; + }; + let path = raw.trim(); + // Dynamic (resolved at runtime) or absent — not a literal we can check here. + if path.is_empty() || path.starts_with('=') { + continue; + } + let escapes_via_parent = path.split(['/', '\\']).any(|seg| seg == ".."); + if std::path::Path::new(path).is_absolute() || escapes_via_parent { + errors.push(format!( + "Node '{}': `oh:storage_upload_file` path `{path}` must be workspace-relative \ + (e.g. `report.html`). Uploads are confined to the agent workspace, so an \ + absolute path (`/tmp/...`, `/Users/...`) or one escaping with `..` is rejected \ + at run time. Use a relative path, and have the producing node write the file to \ + that same relative path.", + node.id + )); + } + } + errors +} + pub(crate) async fn run_builder_gates(config: &Config, graph: &WorkflowGraph) -> Vec { let compatibility_errors = config_aware_engine_compatibility_errors(config, graph); if !compatibility_errors.is_empty() { @@ -452,6 +503,14 @@ pub(crate) async fn run_builder_gates(config: &Config, graph: &WorkflowGraph) -> if !binding_errors.is_empty() { return binding_errors; } + // Cheap, sync: an `oh:storage_upload_file` literal `path` that is absolute or + // escapes the workspace. The runtime `resolve_upload_path` rejects it, but the + // model reliably ignores the prompt's "use a workspace-relative path" rule and + // copies an absolute `/tmp/...` path from prior flows, so enforce it in code. + let upload_path_errors = validate_upload_paths(graph); + if !upload_path_errors.is_empty() { + return upload_path_errors; + } // Cheap: an `agent` node's `agent_ref` that would hit the runtime's // `RegistryFallback` "unknown agent_ref" hard error mid-run. Almost always a // pure in-memory harness-registry lookup; only a ref that ISN'T a harness diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index c928bb5e79..1f00d6e5bd 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -3825,6 +3825,48 @@ async fn native_file_attachment_chain_passes_required_arg_resolvability() { ); } +fn upload_graph(path: Value) -> WorkflowGraph { + graph(json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "up", "kind": "tool_call", "name": "Upload", + "config": { "slug": "oh:storage_upload_file", "args": { "path": path } } } + ], + "edges": [ { "from_node": "t", "to_node": "up" } ] + })) +} + +#[test] +fn validate_upload_paths_rejects_an_absolute_path() { + // The live-observed bug: the model copies `/tmp/openhuman-flow/report.html` + // from a prior flow, which the runtime rejects (uploads are confined to the + // workspace). Catch it at author time with an actionable message. + let errors = validate_upload_paths(&upload_graph(json!("/tmp/openhuman-flow/report.html"))); + assert_eq!(errors.len(), 1, "{errors:?}"); + assert!(errors[0].contains("'up'"), "{}", errors[0]); + assert!(errors[0].contains("workspace-relative"), "{}", errors[0]); +} + +#[test] +fn validate_upload_paths_accepts_a_workspace_relative_path() { + assert!(validate_upload_paths(&upload_graph(json!("report.html"))).is_empty()); + assert!(validate_upload_paths(&upload_graph(json!("out/report.html"))).is_empty()); +} + +#[test] +fn validate_upload_paths_rejects_a_parent_escape() { + let errors = validate_upload_paths(&upload_graph(json!("../../etc/passwd"))); + assert_eq!(errors.len(), 1, "{errors:?}"); + assert!(errors[0].contains("escaping with `..`"), "{}", errors[0]); +} + +#[test] +fn validate_upload_paths_ignores_a_dynamic_path_expression() { + // A `=`-expression resolves at runtime; the author-gate can't know its value, + // so it must not reject it (the runtime check still applies). + assert!(validate_upload_paths(&upload_graph(json!("=nodes.prep.item.json.path"))).is_empty()); +} + /// (Codex feedback on PR #4826) This gate sandbox-runs every graph against /// `json!({})` as the trigger payload, so a `tool_call` arg wired straight to /// the trigger's own data — `"to": "=item.email"` on a node whose only From 40c14d00a523d7f6bceb05eb839f326ac3006c64 Mon Sep 17 00:00:00 2001 From: "cyrus@tinyhumans.ai" Date: Fri, 24 Jul 2026 18:39:19 +0530 Subject: [PATCH 8/8] fix(approval): don't over-redact bare url/link in approval args (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit + Greptile both flagged that adding bare `"url"` and `"link"` to `SENSITIVE_KEYS` over-redacts: `redact_args` is shared across native, Composio, and `http_request` approval paths, so it would blank out an `http_request` node's destination URL and any Composio action's benign `url`/`link` arg on the approval card — hiding exactly the information a human approver needs to judge an outbound action. The presigned-link threat is already covered by the specific file-handoff keys (`attachment`, `file_to_upload`, `file_url`, `public_url`, `signed_url`, `presigned_url`) — the link binds into a `file_uploadable` parameter like `attachment`, never a key literally named `url`. Drops `"url"` and `"link"`; the redaction test now also asserts a bare `url` (an http_request destination) stays VISIBLE. Also hyphenates "world-readable" in the prompt guidance (CodeRabbit nit). --- src/openhuman/approval/redact.rs | 12 +++++++++--- .../flows/agents/workflow_builder/prompt.md | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/openhuman/approval/redact.rs b/src/openhuman/approval/redact.rs index 8179e215a8..e3712f7293 100644 --- a/src/openhuman/approval/redact.rs +++ b/src/openhuman/approval/redact.rs @@ -81,8 +81,6 @@ const SENSITIVE_KEYS: &[&str] = &[ "attachments", "file_to_upload", "file_url", - "url", - "link", "public_url", "signed_url", "presigned_url", @@ -510,8 +508,12 @@ mod tests { let args = json!({ "attachment": "https://files.example.test/f_1?sig=SECRETSIGNATURE", "file_to_upload": "https://files.example.test/f_2?sig=ANOTHERSIG", - "url": "https://files.example.test/f_3?sig=THIRDSIG", + "file_url": "https://files.example.test/f_3?sig=THIRDSIG", "public_url": "https://files.example.test/f_4?sig=FOURTHSIG", + // A bare `url` (e.g. an `http_request` node's destination, or a + // Composio action's benign url arg) is NOT a file-handoff key and + // MUST stay visible so the human approver can judge the action. + "url": "https://webhook.site/VISIBLE-DESTINATION", }); let red = redact_args(&args); let blob = red.to_string(); @@ -527,6 +529,10 @@ mod tests { "presigned link leaked through redaction ({leaked}): {blob}" ); } + assert!( + blob.contains("webhook.site/VISIBLE-DESTINATION"), + "a bare `url` (e.g. an http_request destination) must NOT be redacted: {blob}" + ); } #[test] diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index f6cd5cdc7e..5f7c0e3daf 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -587,7 +587,7 @@ is what matters, not its output. window or the provider will fetch a dead URL. The URL is a bearer capability for as long as it lives, so do not set it far longer than needed either. **Do not** upload with `visibility: "public"` to get a `public_url` instead. - That leaves a permanently world readable object; the presigned link expires. + That leaves a permanently world-readable object; the presigned link expires. 4. **Send it.** A `tool_call` on the provider action, binding the link URL into that action's file parameter.