Skip to content

fix(skills): keep inline shell in the routed profile env - #113975

Open
poijygfdyy wants to merge 2 commits into
NousResearch:mainfrom
poijygfdyy:fix/skill-inline-shell-profile-env
Open

poijygfdyy wants to merge 2 commits into
NousResearch:mainfrom
poijygfdyy:fix/skill-inline-shell-profile-env

Conversation

@poijygfdyy

@poijygfdyy poijygfdyy commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Invariant

A SKILL.md inline-shell expansion executed for a routed profile must spawn under that profile's child-process environment. It must not inherit the gateway/Desktop process-launch profile's HERMES_HOME, credentials, or profile-specific .env settings.

Current-main RED

Audited against upstream main at bbaf7af5c83546d19f8060f4097d3bb25cd1a3c3; a final refresh after implementation still showed the same main SHA.

agent.skill_preprocessing.run_inline_shell() currently calls subprocess.run(..., env=delegated_child_subprocess_env()). Outside a delegation/Kanban context, delegated_child_subprocess_env() returns None, so subprocess.run() inherits the process-wide os.environ. In a multiplexed gateway/Desktop backend, that environment belongs to the launch profile even while the caller is scoped to another served profile.

The first commit adds a focused regression for the real run_inline_shell() spawn seam. It binds process HERMES_HOME to a launch home, adds a launch-profile-only .env marker, enters a different context-local served-home override, and captures the environment passed to subprocess.run(). The contract requires an explicit child env with the served HERMES_HOME and without launch-profile-only residue. Current main passes env=None, so the regression is deterministically RED from the current production mechanism.

Merged #111617 independently identified this exact agent/skill_preprocessing.py spawn as an unfixed residual in its full subprocess/profile audit; it did not modify this path.

Fix

Route the inline shell through the repository's existing served-profile child-env builder:

env=served_profile_child_env(inherit_credentials=True)

inherit_credentials=True is deliberate here. Inline shell is an explicitly enabled user skill feature and historically inherited the process environment; the fix preserves access to the active profile's intended credentials/settings while stripping launch-profile residue and pinning HERMES_HOME to the routed profile. The builder also finalizes through delegated_child_subprocess_env(...), so existing delegation/Kanban environment fencing is preserved.

Command execution, cwd, timeout, stdout/stderr handling, Windows creation flags, and inline-shell error semantics are otherwise unchanged.

Collision audit

Refreshed both before implementation and immediately before publication across open/recent/historical PRs, issues, commits, exact function names, root-cause terms, and the affected file.

A final PR search after the fix found no second implementation of the profile-env correction.

Final diff

Two commits, preserving RED regression → production fix, and two files only:

  • agent/skill_preprocessing.py: 2 additions / 2 deletions; replace the nullable delegation-only env with the served-profile child-env builder.
  • tests/agent/test_skill_inline_shell_profile_env.py: 38-line focused regression for launch-home residue vs routed-profile ownership.

The branch is 2 commits ahead and 0 behind the audited main SHA. No unrelated runtime behavior is included.

Validation

  • Current-main source audit (bbaf7af5c83546d19f8060f4097d3bb25cd1a3c3)
  • Deterministic current-main RED established from the real spawn seam (env=None outside delegation)
  • Test-only regression committed before production changes
  • Existing served-profile child-env/delegation composition audited
  • Pre-implementation mechanism collision search
  • Fresh pre-publication PR/issue/commit/function/file collision search
  • Final two-file diff audited
  • Exact-head upstream workflows created
  • Repository pytest / Ruff / full suite executed in this environment
  • Hosted CI executed

Repository pytest/Ruff are not claimed: this execution environment does not have a repository checkout available for canonical test execution. For exact head a6882652fd2abaccd7cfc4ad59f4141ad4d4440e, upstream CI, Nix flake check, and Docker Build, Test, and Publish all currently conclude action_required; no jobs executed behind the external-contributor approval gate, so no hosted-CI pass or failure is claimed.

@poijygfdyy
poijygfdyy marked this pull request as ready for review September 17, 2026 08:11
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) area/profiles Multi-profile isolation, HERMES_HOME scoping labels Sep 17, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Swaps agent/skill_preprocessing.py::run_inline_shell's child env from delegated_child_subprocess_env() (which on the non-delegated path passes env=None, i.e. full launch-environ inheritance) to served_profile_child_env(inherit_credentials=True), so a served profile's SKILL.md inline shell gets the routed profile's own secrets instead of the launch profile's environ. The direction is right and closes a real cross-profile credential leak under multiplex; the test pins HERMES_HOME routing and launch-residue stripping.

  1. New exception path contradicts run_inline_shell's never-raise contract — non-blocking — agent/skill_preprocessing.py (env= expression inside the try whose only handler is except subprocess.TimeoutExpired). Per tools/environments/local.py::served_profile_child_env's documented contract on main, with inherit_credentials=True, no target home override, and no bound profile secret scope while multiplexing is active, the call raises UnscopedSecretError (fail-closed: "the child would inherit the launch profile's credentials"). That exception propagates out of run_inline_shell, whose own docstring promises "Failures return an [inline-shell ...] marker instead of raising, so one bad snippet can't wreck the whole skill message" — under that state a single inline snippet now wrecks the whole skill message. Narrow window (served profiles normally set the override; plain CLI has multiplex off), so suggest confirming whether an unbound-scope multiplex surface can reach skill preprocessing; if so, either catch Exception around the env build and fall back to the marker, or keep the old env builder for the unbound case.

This branch has not been deployed

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants