GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(executor): copy the env map at the workflow-tool boundary by waleedlatif1 · Pull Request #6618 · simstudioai/sim

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up hardening to #6611, which began forwarding the invoking run's environment variables into a workflow executed as an Agent tool.

A full runtime audit of #6611 confirmed that nothing in the child execution path writes through ctx.environmentVariables today — so this is not a live defect. It closes the one structural gap that audit surfaced.

Why

tools/index.ts was the only consumer that handed the parent's live env map across an execution boundary by reference, and it hands it to the longest-lived consumer there is — the child holds that object for its entire run, across WorkflowBlockHandler.executeCore's prologue.

Every other consumer already copies before handing the map on:

Consumer Copies via
agent-handler.ts:2496, :2576 normalizeStringRecord(ctx.environmentVariables)
function-handler.ts:87 normalizeStringRecord(...)
condition-handler.ts:52 normalizeStringRecord(...)
providers/utils.ts:1629 normalizeStringRecord(...)
tools/index.ts (workflow tool) nothing — the outlier this PR fixes

A future write through the child's reference would corrupt the parent's env and every later sibling tool call in the same agent turn — a cross-run bug with no local symptom, and exactly the kind of thing that is invisible in review.

Why a shallow spread is exact

The value is typed Record<string, string>, and the sub-Executor already re-copies it through normalizeStringRecord at executor.ts:73, so the child receives a byte-identical map either way. The spread also subsumes the previous ?? {} — spreading undefined yields {}.

Type of Change

  • Bug fix (hardening)

Testing

New test mutates the forwarded map and asserts the parent context is unchanged; it fails without the spread. Full executor/ + tools/index.test.ts suite: 2096 passed. Type-check and Biome clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
greptile-apps[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@waleedlatif1
fix(executor): copy the env map at the workflow-tool boundary
a320f0b 
Follow-up hardening to #6611, which began forwarding the invoking run's
environment variables into a workflow run as an agent tool.

A runtime audit of that change confirmed nothing today writes through
`ctx.environmentVariables`, so this is not a live defect. But `tools/index.ts`
was the only consumer handing the map across an execution boundary by
reference, and it hands it to the longest-lived consumer there is: the child
holds it for its entire run. `agent-handler`, `function-handler`,
`condition-handler` and `providers/utils` all copy via `normalizeStringRecord`
before handing the map anywhere. A future write through the child's reference
would corrupt the parent's env and every later sibling tool call in the same
agent turn — a cross-run bug with no local symptom.

A shallow spread is exact here: the value is typed `Record<string, string>`,
and the sub-Executor already re-copies it through `normalizeStringRecord`
(`executor.ts:73`), so the child receives a byte-identical map either way. The
spread also subsumes the previous `?? {}`, since spreading `undefined` yields
`{}`.

The test mutates the forwarded map and asserts the parent context is unchanged;
it fails without the spread.
@vercel

vercel Bot commented Aug 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 12, 2026 8:57am

Request Review
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vercel
vercel Bot temporarily deployed to Preview August 12, 2026 08:57 Inactive
@cursor

cursor Bot commented Aug 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

PR Summary

Low Risk
Defensive copy of a string env map with a regression test; audit found no live write-through bug today.

Overview
Hardens the workflow-as-tool path so the parent's environmentVariables are shallow-copied into the child run instead of shared by reference.

Previously tools/index.ts was the only consumer that forwarded the live parent map across this long-lived boundary. A child write could have corrupted the parent env and later sibling tool calls. Adds a regression test that mutates the forwarded map and asserts the parent context stays unchanged.

Reviewed by Cursor Bugbot for commit a320f0b. Configure here.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens the workflow-tool execution boundary by shallow-copying the invoking run’s environment-variable map before forwarding it to a child workflow.

  • Prevents child execution from retaining an alias to the parent environment map.
  • Adds a regression test proving forwarded values remain intact while mutations cannot affect the parent context.

Confidence Score: 5/5

The PR appears safe to merge because the copy preserves the established environment-variable contract while removing cross-run reference aliasing.

The production environment map is normalized to a plain string record, the child accepts the same record shape without relying on object identity, and the regression test covers the intended isolation behavior.

Important Files Changed
Filename Overview
apps/sim/tools/index.ts Replaces direct environment-map forwarding with a shallow copy, preserving the string-record contents while isolating parent and child references.
apps/sim/tools/index.test.ts Adds focused coverage confirming value forwarding, distinct object identity, and parent isolation from child-side mutations.

Reviews (1): Last reviewed commit: "fix(executor): copy the env map at the w..." | Re-trigger Greptile
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@waleedlatif1
waleedlatif1 merged commit 082deb2 into staging Aug 12, 2026
24 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@waleedlatif1
waleedlatif1 deleted the fix/b3-agent-tool-env-map-copy branch August 12, 2026 09:01
@waleedlatif1 waleedlatif1 mentioned this pull request Aug 12, 2026
v0.8.0: desktop app, workflow UI, v2 endpoints #6606
Merged

This branch was previously deployed

1 inactive deployment
Preview — a320f0bf Deployed Aug 12, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@waleedlatif1
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.