Skip to content

Forward login-shell env vars (bashrc/zshrc exports) to harness children - #202

Open
katulevskiy wants to merge 3 commits into
mirror/upstream-mainfrom
mirror/upstream-pr/330
Open

katulevskiy wants to merge 3 commits into
mirror/upstream-mainfrom
mirror/upstream-pr/330

Conversation

@katulevskiy

Copy link
Copy Markdown
Owner

Upstream mirror

Source: zeronsh#330
Original author: @labkita7
Original revision: 07f3add96a6ff3ff031a7c676e9d905f7f8eaece

This PR is an isolated Contribution Manager pilot mirror. Its reviews, checks and merge state are separate from upstream. An empty [skip ci] commit suppresses automatic bulk CI; the code tree matches the imported upstream revision.


Problem

When zeron runs as a headed app (Dock/Finder/launchd/systemd), the daemon never runs shell init, so harness children inherit a minimal environment. The login-shell snapshot (crates/harness/src/shell_env.rs) already fixes PATH resolution — but it kept only PATH and dropped every other export. Provider CLIs behind the harnesses (pi via pi-acp, and the rest) that read custom variables configured in ~/.bashrc / ~/.zshrc — API keys, base URLs, proxy settings — therefore failed even though the same CLIs work fine from the user's terminal.

Fix

  • The snapshot now keeps the full environment dumped between the markers (LoginShellEnv { path, vars }), with the same hostile-init defenses (last-begin-marker parse, PATH-presence validity gate, probe-marker drop).
  • compose_path — the single choke point every harness child spawn goes through (pi/ACP, Claude, Codex, Cursor, opencode, npm adapter installs, engine git/gh) — now also forwards snapshot variables the daemon's own environment doesn't already define. Explicit process env always wins; PWD/OLDPWD/SHLVL/_ and the probe's own ZERON_RESOLVING_ENVIRONMENT / TERM=dumb pairs are never forwarded.
  • ZERON_NO_LOGIN_SHELL_ENV=1 opts out of variable forwarding while keeping the PATH composition (ZERON_NO_LOGIN_SHELL=1 still disables the snapshot entirely).

Verification

  • cargo test -p zeron-harness --tests: 13 test binaries, 207 passed, 0 failed (10 ignored real-lifecycle tests by design).
  • New/extended tests:
    • unit: full-env parse (PATH excluded from vars), probe-pair dropping, duplicate keys, no-PATH dumps rejected, fake-shell end-to-end snapshot;
    • shell_env_resolution e2e: a var exported by the fake login shell reaches a spawned child command, an existing process var is not overridden, the probe marker never leaks.
  • cargo fmt --check clean for the touched files; remaining clippy/fmt findings in the crate are pre-existing on main (unrelated files, not touched here).

Open in Hoplite


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Hoplite and others added 2 commits September 11, 2026 22:42
GUI/service launches never run shell init, so the login-shell snapshot
is the daemon's only view of the user's environment — but it kept only
the PATH, dropping every other export. Provider CLIs (pi-acp and the
rest) that read custom variables configured in ~/.bashrc or ~/.zshrc
(API keys, base URLs, proxies) therefore failed under the headed app.

The snapshot now keeps the full environment between the markers and
compose_path forwards variables the daemon's own env doesn't already
define (explicit process env always wins; shell bookkeeping and the
probe's own markers are never forwarded). ZERON_NO_LOGIN_SHELL_ENV=1
opts out while keeping the PATH composition.

Co-authored-by: fajar sidik <jardik.oryza@gmail.com>
@contribution-manager-katulevskiy contribution-manager-katulevskiy Bot added the review:needs-triage Managed by Contribution Manager label Oct 6, 2026
@contribution-manager-katulevskiy

contribution-manager-katulevskiy Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Contribution Manager

needs triage · medium risk · untriaged

Open review workspace · Revision 7d0c4517

Next actions:

  • Direction decision needed
  • Scope, risk, and platform triage needed
  • Independent code review needed
  • Required check: core-tests (missing)

This summary is maintained by the review app. Decisions and test evidence are recorded in the workspace; the app does not merge PRs.

@contribution-manager-katulevskiy contribution-manager-katulevskiy Bot added cm:inbox Contribution Manager: waiting for triage and removed review:needs-triage Managed by Contribution Manager labels Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cm:inbox Contribution Manager: waiting for triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant