fix(executor): carry child provenance across the workflow agent tool result - #6619
Conversation
…result
A workflow invoked as an agent tool resolves `{{VAR}}` to the real decrypted
value in the child run, but the tool result handed back to the model vendor was
projected through a registry that had dropped those entries — so the plaintext
crossed to the vendor verbatim.
Mechanism. Model-facing projection runs `registry.forkForPropagatedEntries()`,
which keeps only entries a result explicitly carried. `EnvResolver` records a
resolution without `propagated`, unlike every other boundary that hands a value
onward. The child shares the caller's registry object, so `blockLogs`, trace
spans and error diagnostics still redact (they read the unforked registry) —
only the model fork loses them. The exposure is wider than the child's final
output: `mapChildOutputToParent` puts the full `childTraceSpans` — every child
block's inputs and outputs — into the returned object, and `postProcessToolOutput`
strips only `__`-prefixed keys.
The previous implementation ran the child over HTTP: the execute route emitted
`__resolvedSecretTraceProvenance`, the tool imported it as `propagated: true`,
and `transformResponse` curated the body so `childTraceSpans` never crossed. The
in-process branch returns before any of that.
Fix. `runWorkflowTool` exports committed provenance for the value it returns and
imports it back with `{ trusted: true }`, which marks those entries propagated —
the same crossing the custom-block branch already performs in `workflow-handler`.
Output-projection only: the returned result is unchanged, and the child executes
exactly as before. Redacted values render as `{{NAME}}`, matching the literal the
model saw before this regression.
Values shorter than `MIN_SUBSTITUTABLE_LITERAL_LENGTH` are still not redacted
anywhere — that floor governs detection as well as substitution, and is a
documented accepted cost. A test pins the behavior rather than leaving it silent.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview
Tests cover success and error paths, inert unused secrets, and the existing short-literal redaction floor. Reviewed by Cursor Bugbot for commit b5a099c. Configure here. |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | b5a099c | apps/sim/executor/handlers/workflow/workflow-tool-runner.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Greptile SummaryThis PR carries child-workflow secret provenance across the agent-tool result boundary so model-facing projection can redact resolved environment values from successful outputs, trace spans, and errors.
Confidence Score: 5/5The PR appears safe to merge and closes the child-workflow secret disclosure path without introducing an actionable regression. The returned bytes remain unchanged while provenance for secrets actually present in the result is promoted for model-facing projection on both success and failure, with incomplete provenance continuing to fail closed.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/handlers/workflow/workflow-tool-runner.ts | Adds a fail-closed provenance crossing on both completion paths, enabling the existing model projection to redact child-resolved secrets. |
| apps/sim/executor/handlers/workflow/workflow-tool-runner.test.ts | Exercises real registry, resolver, and projection behavior for successful results, trace spans, failures, unrelated secrets, and short literals. |
Sequence Diagram
sequenceDiagram
participant Agent as Workflow agent
participant Runner as runWorkflowTool
participant Child as Child workflow
participant Registry as Secret provenance registry
participant Projection as Model-facing projection
participant Vendor as Model vendor
Agent->>Runner: Invoke workflow tool
Runner->>Child: Execute with shared registry
Child->>Registry: Record resolved environment secret
Child-->>Runner: Output or error containing resolved value
Runner->>Registry: Export provenance for returned value
Runner->>Registry: Import as trusted propagated provenance
Runner-->>Projection: Raw tool result
Projection->>Registry: Fork propagated entries
Projection->>Projection: Replace plaintext with placeholder
Projection-->>Vendor: Redacted tool result
Reviews (1): Last reviewed commit: "fix(executor): carry child provenance ac..." | Re-trigger Greptile
Summary
A workflow invoked as an agent tool now resolves
{{VAR}}to the real decrypted value in the child run (#6611). The tool result handed back to the model vendor is projected through a registry that had dropped those entries, so the plaintext crosses to the vendor verbatim. This is onstagingonly — the release it belongs to has not been promoted — but it is a promotion blocker for staging → main.What reaches the vendor today on staging. For an agent whose tool is a workflow, the tool message serialized into the next model request contains the child's decrypted environment-variable values — in
output.result, and also insideoutput.childTraceSpans, which carries every child block's inputs and outputs. Reproduced with the real registry, the realEnvResolver, and the real model projection: a livesk-live-…style token appears in both.Mechanism.
projectToolResultForCopilot) forks withregistry.forkForPropagatedEntries(), which keeps only entries a result explicitly carried.EnvResolver(executor/variables/resolvers/env.ts) records the resolution withoutpropagated, unlike the other boundaries that hand a value onward (tools/index.ts,providers/runtime-context.ts).blockLogs, trace spans and error diagnostics still redact correctly — they read the unforked registry. Only the model fork loses the entries.mapChildOutputToParentputs the fullchildTraceSpansinto the returned object, andpostProcessToolOutputstrips only__-prefixed keys.Why
maindoes not have this. There,workflow_executorwas an HTTP tool: the execute route emitted__resolvedSecretTraceProvenance, the tool imported it aspropagated: true, andtransformResponsecurated the body sochildTraceSpansnever crossed. The in-process branch returns before any of that runs.The fix
runWorkflowToolexports committed provenance for the value it is about to return and imports it back with{ trusted: true }, which marks those entries propagated. This is the crossing the custom-block branch already performs inworkflow-handler.ts; the non-custom branch skipped it only because the two registries are the same object — exactly the assumption the model fork breaks.{{NAME}}— the same literal the model saw before the regression.Why not the alternative (marking
EnvResolver's recordspropagated):propagatedmeans "this result carries this secret across a boundary", not "a block consumed this secret".EnvResolverruns at input resolution for the executing block, in every workflow run, into the run-scoped registry — flipping it there would restate an output-crossing fact at an input site and change what crosses at every unrelated boundary in the product, for a defect that exists at exactly one. Smaller diff, much wider reach, and it does not become more correct than the precedent it replaces. Fixing the boundary that is actually broken keeps the canvas workflow block and the custom-block path untouched.On
childTraceSpans. Curation alone would not have fixedresult, so it was never a substitute. I also did not drop it here: the canvasworkflow_inputblock runs through the samerunWorkflowTool, andblock-executorliftschildTraceSpansoff that output into the block log for trace nesting (span-factoryreads it) before stripping it from block state. Dropping it in the runner would silently break child-run traces in the UI. Child block I/O reaching the vendor is still questionable on its own merits, but it needs a model-facing output projection hook, which does not exist today (tools haverequest.modelInput, no output equivalent) — worth a follow-up, and it is now redacted either way.Short-literal caveat, stated plainly.
MIN_SUBSTITUTABLE_LITERAL_LENGTH = 8governs detection as well as substitution, so an environment value of 7 characters or fewer is not covered by this fix — not in the model result, and not in logs or traces either. That floor is deliberate (it subsumed the per-value exception lists that fixed prior false-positive incidents) and this PR does not touch it. A test pins the behavior so it is documented rather than silent.Testing
workflow-tool-runner.test.tsdrive the realResolvedSecretTraceRegistry, the realEnvResolver, and the realprojectToolResultForCopilot. The two leak tests fail on plainstaging(secret present inresultand inchildTraceSpans; secret present in the projected error) and pass with the fix.executor/suite (1947),tools/index, resolved-secret registry / projection / refusal suites,providers/runtime-context.bun run type-checkclean; biome clean.Type of Change
Checklist