Skip to content

Navigation Menu

Sign in
Sign up

feat(shell): one persistent shell session per task so cwd and env survive #3400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kovtcharov-amd wants to merge 4 commits into main
base: main
Choose a base branch
Loading
from kalin/shell-session-3380-clean
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions .security-suppressions.json
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,19 @@
{"path": "src/gaia/hub/native_launcher.py", "rule": "S603", "justification": "native binary path is an installed hub artifact (operator-controlled); args are constructed, not shell"},
{"path": "src/gaia/mcp/mcp_bridge.py", "rule": "B104", "justification": "binds 0.0.0.0 only on explicit caller opt-in (documented flag), never by default"},
{
"path": "src/gaia/agents/tools/shell_tools.py",
"rule": "B602",
"justification": "Sandboxed shell tool. Every command (and each pipeline segment) is validated against a whitelist via _validate_command before execution; shell=True is enabled ONLY on Windows so cmd.exe can resolve built-ins (dir/cd/type) and pipes that Git-for-Windows tools rely on. Converting to args-list would break piped/whitelisted commands the tool exists to run."
"path": "src/gaia/agents/tools/shell_session.py",
"rule": "B404",
"justification": "Running shell commands is this module's purpose. The whitelist and path policy that decide what may run stay in shell_tools.py, and apply per pipeline segment before anything reaches here."
},
{
"path": "src/gaia/agents/tools/shell_session.py",
"rule": "B603",
"justification": "argv is always a constructed list, never shell-interpreted by Python. It names either the generated script this module wrote (which sources a command validated per segment by shell_tools.py) or a skill-granted binary, which is run as argv precisely so its arguments cannot act."
},
{
"path": "src/gaia/agents/tools/shell_session.py",
"rule": "B607",
"justification": "taskkill is a Windows system binary invoked by name with a pid from this module's own child. The alternative is orphaning the process tree a timed-out command started."
}
]
}
7 changes: 7 additions & 0 deletions docs/sdk/mixins/tool-mixins.mdx
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,15 @@ class MyAgent(Agent, ShellToolsMixin):
self.register_shell_tools()
# Registers:
# - run_shell_command(command, working_directory=None, timeout=30)
# - get_shell_state()
# - set_shell_variable(name, value)
# - reset_shell_session()
```

Commands share one shell session per agent, so a `cd` or a variable set in one
call is still in effect for the next. `get_shell_state` reads that state back,
and `reset_shell_session` returns it to where it started.

---

## 9.4 File Search Mixin
Expand Down
55 changes: 51 additions & 4 deletions docs/spec/shell-tools-mixin.mdx
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ ShellToolsMixin provides secure shell command execution with comprehensive rate

**Key Features:**
- Whitelist-based command security (only safe, read-only commands)
- One persistent shell session per agent — cwd and environment survive between calls
- Dual-tier rate limiting (burst + sustained)
- Path traversal prevention with argument validation
- Git read-only operations support
Expand All @@ -41,21 +42,28 @@ ShellToolsMixin provides secure shell command execution with comprehensive rate
- Blacklist dangerous commands (rm, chmod, sudo, etc.)
- Git read-only subcommands only
- Working directory support
- Timeout protection (default: 30s)
- Timeout protection (default: 30s), killing the command's whole process tree

2. **Rate Limiting**
2. **Persistent Session State**
- One session per agent; `cd` and exported variables carry to the next call
- `get_shell_state` reports the directory and the changed environment
- `set_shell_variable` sets a variable for the rest of the session
- `reset_shell_session` returns the session to its starting state
- Serialised access; deterministic teardown with no surviving child

3. **Rate Limiting**
- Max 10 commands per minute (sustained)
- Max 3 commands per 10 seconds (burst)
- Clear wait time messaging
- Timestamp tracking with deque

3. **Path Security**
4. **Path Security**
- Validate working directory access
- Check arguments for path traversal (.., separators)
- Resolve relative paths to absolute
- PathValidator integration

4. **Output Management**
5. **Output Management**
- Capture stdout and stderr separately
- Truncate large outputs (max 10,000 chars)
- Return execution duration
Expand Down Expand Up @@ -106,6 +114,9 @@ class ShellToolsMixin:

Tools provided:
- run_shell_command: Execute safe shell commands with security checks
- get_shell_state: Report the session's directory and changed environment
- set_shell_variable: Set a variable for the rest of the session
- reset_shell_session: Return the session to its starting state
"""

def __init__(self, *args, **kwargs):
Expand Down Expand Up @@ -255,6 +266,42 @@ if cmd_base == "git":
}
```

### Persistent Session

Every command runs in the agent's one `ShellSession`
([`shell_session.py`](https://github.com/amd/gaia/blob/main/src/gaia/agents/tools/shell_session.py)),
a port of the C++ toolbelt's `gaia::ShellSession`. What persists is the session
*state*, not a resident child process: each command runs a generated script that
restores the session's directory and variables, sources the command from its own
file, then reports the resulting `pwd` and environment to a side file the session
absorbs. Nothing outlives the call, so there is no child to orphan.

- **Serialised.** One lock per session, taken for the whole command. A call that
cannot take it within its own timeout returns a "session busy" error rather
than blocking — a command that outran its tool timeout is still inside the
subprocess, and waiting on it forever would wedge the agent.
- **Timeouts kill the tree.** POSIX kills the process group; Windows uses
`taskkill /T`. A timed-out build does not leave its compilers running.
- **`cd` obeys the path policy.** A directory the agent may not access is not
absorbed, and the result carries a warning saying so. Otherwise persistence
would be a way to reach a forbidden directory and then read files by bare
name, which the per-argument check never sees.
- **Granted binaries still run as argv.** A skill-granted CLI is never handed to
a shell as a string; it gets the session's directory and environment, but its
arguments cannot act.
- **`working_directory` is unchanged.** An explicit per-call directory scopes
that call only; the session stays where it was.

On Windows commands run as a `cmd.exe` batch script, so `%VAR%` expands as it
would at a prompt and a `for` variable is written `%%i`. Point `GAIA_SHELL` at a
POSIX shell (Git Bash, MSYS, WSL) to run shell scripts instead.

Shell state that is not a directory or a variable — aliases, functions, shell
options — is not carried. The read-only whitelist has no `export`, so
`set_shell_variable` is how a variable gets set; it refuses `PATH`,
`PYTHONPATH`, `LD_*` and the rest of the names that decide which binary or which
code a later command runs.

### Path Traversal Prevention

```python
Expand Down
10 changes: 7 additions & 3 deletions hub/agents/chat/python/gaia_agent_chat/agent.py
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2346,9 +2346,9 @@ def save_current_session(self) -> bool:
def __del__(self):
"""Cleanup when agent is destroyed.

Releases watchdog observers, HTTP session, and the two SQLite
connections owned by this agent. ``__del__`` is best-effort — Python
doesn't guarantee it fires on interpreter shutdown — but explicit
Releases watchdog observers, HTTP session, the shell session, and the
two SQLite connections owned by this agent. ``__del__`` is best-effort —
Python doesn't guarantee it fires on interpreter shutdown — but explicit
close() makes tests deterministic (WAL journals released, file handles
closed) and avoids leaking Session/connection objects in long-running
services like the Agent UI backend.
Expand All @@ -2372,3 +2372,7 @@ def __del__(self):
self._scratchpad.close_db()
except Exception as e:
logger.error(f"Error closing scratchpad during cleanup: {e}")
try:
self.close_shell_session()
except Exception as e:
logger.error(f"Error closing the shell session during cleanup: {e}")
15 changes: 13 additions & 2 deletions hub/agents/chat/python/gaia_agent_chat/tool_bundles.py
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,16 @@
),
ToolBundle(
name="shell",
members=frozenset({"run_shell_command", "get_system_info"}),
description="Run shell commands and query the system.",
members=frozenset(
{
"run_shell_command",
"get_shell_state",
"set_shell_variable",
"reset_shell_session",
"get_system_info",
}
),
description="Run shell commands in a persistent session and query the system.",
),
ToolBundle(
name="clipboard",
Expand Down Expand Up @@ -351,6 +359,9 @@
members=frozenset(
{
"run_shell_command",
"get_shell_state",
"set_shell_variable",
"reset_shell_session",
"execute_python_file",
"get_system_info",
}
Expand Down
Loading
Loading

AltStyle によって変換されたページ (->オリジナル) /