Skip to content

feat: expose extra_env in LocalInteractiveSession and tool instance in tool_execute extensions#1377

Open
Krashnicov wants to merge 1 commit intoagent0ai:developmentfrom
Krashnicov:feat/tool-execute-extra-env
Open

feat: expose extra_env in LocalInteractiveSession and tool instance in tool_execute extensions#1377
Krashnicov wants to merge 1 commit intoagent0ai:developmentfrom
Krashnicov:feat/tool-execute-extra-env

Conversation

@Krashnicov
Copy link
Copy Markdown
Contributor

@Krashnicov Krashnicov commented Mar 29, 2026

feat: extra_env support for LocalInteractiveSession and SSHInteractiveSession

Problem

Code execution subprocesses had no way to receive caller-injected environment variables (e.g. project-specific env, secrets resolved by a plugin at runtime) without either modifying the subprocess command or exposing the full agent environment to the subprocess.

Solution

Adds extra_env: dict | None = None to both LocalInteractiveSession and SSHInteractiveSession, letting callers pass per-session environment variables that are available from the start of the shell session.

Security

The LocalInteractiveSession implementation uses an explicit whitelist rather than merging os.environ in full. Only the following keys are forwarded from the host environment:

PATH, HOME, USER, SHELL, TERM, LANG, LC_ALL, TMPDIR, PWD

API keys, tokens, passwords, and all other framework environment variables are not forwarded. The extra_env dict is then merged on top — callers receive exactly what they pass, nothing more.

SSHInteractiveSession injects extra vars via export KEY='value' in the session's initial_command block. Values are shlex.quote()-escaped to prevent injection.

Backward Compatibility

Fully backward compatible. extra_env defaults to None. Existing call sites passing only cwd= are unaffected — when extra_env=None, the env argument passed to TTYSession is None and the original os.environ.copy() fallback in TTYSession.__init__ applies.

Files Changed

  • plugins/_code_execution/helpers/shell_local.py_SAFE_ENV_KEYS whitelist + extra_env param
  • plugins/_code_execution/helpers/shell_ssh.pyextra_env param + export-prefix injection

@frdel
Copy link
Copy Markdown
Collaborator

frdel commented Apr 1, 2026

I don't like the idea of all env vars being passed automatically when you specify extra_envs, this way all framework env vars like api keys are exposed to the code exec runtime.
Also why would it work different in local TTY and SSH?

@Krashnicov
Copy link
Copy Markdown
Contributor Author

Krashnicov commented Apr 2, 2026

@frdel — both concerns addressed:

1. os.environ leak — fixed via _SAFE_ENV_KEYS whitelist

Replaced the full merge with a whitelist approach:

_SAFE_ENV_KEYS = {"PATH", "HOME", "USER", "SHELL", "TERM", "LANG", "LC_ALL", "TMPDIR", "PWD"}

env = (
    {k: v for k, v in os.environ.items() if k in _SAFE_ENV_KEYS} | self.extra_env
) if self.extra_env else None

Only the minimal set of keys needed for a functional shell is forwarded from the parent environment — no API keys, tokens, or framework secrets can leak. When extra_env=None (the default), env=None is passed to TTYSession which falls back to os.environ.copy() — existing behaviour is completely unchanged.

2. SSHInteractiveSession parity — fixed

Added extra_env: dict | None = None to SSHInteractiveSession.__init__ matching LocalInteractiveSession's signature exactly. Applied via export KEY=VALUE prefix in the initial_command block (using shlex.quote() on values for injection safety). Used the export-prefix approach rather than invoke_shell(environment=...) since the latter depends on server-side AcceptEnv configuration and is not reliable for interactive shells.

Both classes now have identical extra_env: dict | None = None signatures with zero call-site changes required.

@Krashnicov Krashnicov force-pushed the feat/tool-execute-extra-env branch from 4c9f95c to eac8a8a Compare April 2, 2026 10:30
@Krashnicov
Copy link
Copy Markdown
Contributor Author

\u26a0\ufe0f ANSI Regex Regression Risk in shell_ssh.py

While preparing a standalone fix for the ANSI escape regex bug, I discovered that upstream/development (d357c24) already carries the fix \u2014 introduced via PR #1344 (ws-rework, merged 2026-03-30). However, this PR will silently reintroduce the broken regex if merged as-is.

Background

The clean_string() ANSI regex was missing an \x1b anchor, causing the character class [@-Z] (ASCII 0x40\u20130x5A) to strip all uppercase A\u2013Z and underscore from every line of code_execution_tool output. The fix was included in the ws-rework as:

# upstream/development HEAD \u2014 fixed
ansi_escape = re.compile(r"\x1b(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")

What this PR does to that line

This PR explicitly modifies the clean_string() block (diff line 226\u2013239), and the replacement reverts to the broken form:

# This PR at line 232 \u2014 broken (no \x1b anchor)
ansi_escape = re.compile(r"(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")

Merge simulation confirms regression

git merge-tree upstream/development HEAD produces a clean merge with no conflicts, but the resulting shell_ssh.py blob contains the broken regex \u2014 because git correctly applies this PR's explicit rewrite of that section as the authoritative change.

Verified via hex inspection of the merge-tree result blob (a2377953):

  • Merged line 232 hex: ...7222283f3a5b402d5a... = r"(?:[@-Z... \u2192 broken
  • \x1b literal (5c783142) is absent in the merged result

Fix

Please rebase feat/tool-execute-extra-env onto current upstream/development and ensure the ansi_escape regex in your rewrite retains the \x1b anchor:

ansi_escape = re.compile(r"\x1b(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")

The extra_env feature additions here are solid \u2014 just needs the \x1b guard preserved in the clean_string() rewrite. Happy to help with the rebase if useful.

…in SSHInteractiveSession; preserve \x1b anchor in clean_string()
@Krashnicov Krashnicov force-pushed the feat/tool-execute-extra-env branch from eac8a8a to 5ed937a Compare April 5, 2026 07:15
@Krashnicov
Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/development (d357c24) — branch is 1 commit ahead of upstream HEAD.

The \x1b anchor is now present in clean_string() — regression avoided.

# clean_string() ansi_escape — after fix (shell_ssh.py:232)
ansi_escape = re.compile(r"\x1b(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")

Verified with Python functional test — strips ANSI sequences without garbling bare uppercase letters.

All extra_env functionality preserved in both shell_local.py and shell_ssh.py.

Force-pushed to fork as commit 5ed937a4. Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants