feat: always set certain important variables automatically by evanspearman · Pull Request #1060 · gominimal/minimal

evanspearman · 2026-07-29T17:59:37Z

Summary

Sessions now come up with the environment variables a usable interactive shell expects — login identity, a styled prompt, and correct locale/timezone — set automatically instead of being left unset or wrong. This also fixes a correctness bug in how the daemon resolves inherited variables: they're now resolved against the user's environment, never the daemon's.

Motivation

A freshly-attached session shell was missing conventional environment a login shell would normally get from PAM//etc/passwd/sshd: no USER/LOGNAME/SHELL, a hardcoded PS1 forced by the launcher, and a locale (en_US.utf8 + LC_ALL) that isn't generated in the session rootfs — producing "cannot set locale" warnings. Worse, project-declared Inherit vars (e.g. LANG, TZ) were resolved daemon-side against the daemon's process environment, so a session would silently pick up the daemon's locale/timezone rather than the connecting user's.

What changed

Session defaults now live in the sandbox, as overridable defaults (crates/sandbox2/src/lib.rs)

Session sandboxes get a styled default PS1, plus USER/LOGNAME (from the configured username) and SHELL=/usr/bin/bash — mirroring what sshd/PAM would set from /etc/passwd.
Locale floor is now a safe, always-present C.UTF-8 set via LANG only (no LC_ALL), so a composition var or forwarded client locale can override it. Build/task sandboxes keep their fixed en_US.utf8 + LC_ALL for output stability.
All set as plain defaults, so composed env_vars win on key collision. The launcher's hardcoded BASELINE_VARS (PS1) is removed accordingly.

Client forwards locale/timezone; daemon accepts an allowlist (crates/minimal/src/lib.rs, crates/minimald/src/session.rs)

min forwards LANG, LC_*, and TZ into the session via ssh SendEnv.
The daemon applies an AcceptEnv-style allowlist (inherited_session_env) that keeps only those and drops control-plane vars (MINIMAL_SESSION_ID, TRACEPARENT, etc.) so they never leak into the shell.
ssh's ProxyCommand shell is pinned to /bin/sh, so a caller whose $SHELL is a bare name (fish) or an absent shell no longer breaks the transport.

Environment is layered by precedence at attach (crates/minimald/src/session_host.rs, session.rs)

New AttachEnv + layer_session_env: client-forwarded locale/TZ sit below the composition (defaults it may override); per-connection facts (TERM, from the PTY request) sit above it (authoritative, sshd-style). SSH_TTY/SSH_CONNECTION are deliberately not set — nothing in-session backs them.

Inherited vars resolve against the user's env, not the daemon's (crates/sessions/src/core/{compose,primitives}.rs, daemon/composer.rs)

New deferring_env() for the daemon composer: inherited vars are never resolved against the daemon's process environment.
ResolvedVar now preserves its pre-resolution spec, and contribution_to_pending ships that original spec (Inherit/InheritWithDefault) to the client, which performs the single authoritative resolution against the user's env and recomputes carries_user_data.

Supporting fixes

hardlink_dir_contents falls back to fs::copy on EXDEV (cache and destination on different filesystems, e.g. per-VM /state vs. rootfs /home), warning once per process instead of per file (crates/common/src/lib.rs).
.minimal/minimal.toml adds just to session packages; the example project's locked_commit pin is refreshed and claude-code dropped.

Tests

Allowlist keeps only LANG/LC_*/TZ and drops control-plane vars; LC_ is a prefix match, not substring.
layer_session_env precedence: inherited < composition < connection, with non-colliding keys from every layer surviving.
Daemon ships Inherit/InheritWithDefault specs and the client resolves them from the user's env (both hit and default-fallback paths), with carries_user_data set correctly.

Note

Set PS1, USER, LOGNAME, SHELL, and LANG automatically in session shells and forward locale/timezone from client

Session containers now receive a baseline environment including PS1, USER, LOGNAME, SHELL=/usr/bin/bash, and LANG=C.UTF-8 by default in sandbox2/src/lib.rs; LC_ALL is no longer set for sessions.
The SSH launcher in minimal/src/lib.rs now runs ProxyCommand under /bin/sh and forwards LANG, LC_*, and TZ via SendEnv.
The server-side attach handler filters client-provided channel env vars to an allowlist (LANG, LC_*, TZ, TERM) and layers them into the session with composition vars and connection facts taking precedence.
Daemon-side composition in sessions/src/daemon/composer.rs now uses a no-op env resolver so inherited vars are resolved against the client's environment rather than the daemon's.
hardlink_dir_contents in common/src/lib.rs now falls back to copying when source and destination are on different filesystems (EXDEV), emitting a one-time warning.
Behavioral Change: sessions that previously had no SHELL, USER, or PS1 will now see these set; composition-defined vars still override the defaults.

^{Macroscope summarized 7ad863b.}

Summary by CodeRabbit

New Features
- Added just and protobuf to the default session packages.
- Interactive shells now get consistent defaults (shell, prompt, and user/locale handling).
Bug Fixes
- SSH session attachment is more robust (uses a safe ProxyCommand shell and forwards locale/timezone settings).
- Network diagnostics now capture interfaces and routes using /proc/net when system tools are unavailable.
- Directory materialization continues across filesystem boundaries by copying when hardlinking isn’t possible.
- Inherited environment variables are now resolved correctly on the client side across the RPC boundary.

coderabbitai · 2026-07-29T18:00:05Z

📝 Walkthrough

Walkthrough

The change defers inherited session environment resolution to clients, propagates attach-time variables into sandbox launches, adds Linux procfs network fallbacks, handles cross-device hardlinking, and updates session and sandbox configuration behavior.

Changes

Session environment propagation

Layer / File(s)	Summary
Deferred environment resolution `crates/sessions/src/core/*`, `crates/sessions/src/daemon/composer.rs`	Inherited variable specifications are preserved across the wire and resolved against the client environment, with updated `carries_user_data` handling and regression tests.
Attach environment propagation `crates/minimal/src/lib.rs`, `crates/minimald/src/session.rs`, `crates/minimald/src/session_host.rs`	SSH forwards locale/timezone values, attach filtering retains `LANG`, `LC_*`, and `TZ`, and `AttachEnv` reaches session host launchers.
Sandbox environment layering `crates/minimald/src/session_host.rs`, `crates/sandbox2/src/lib.rs`, `.minimal/minimal.toml`, `crates/sessions/example_project/minimal.toml`	Launches merge environment layers by precedence; session defaults, locale settings, and package configuration are updated.

Diagnostics and network handling

Layer / File(s)	Summary
Linux procfs network capture `crates/diagnostics/src/net.rs`, `crates/minimald/src/diag.rs`	Interface and route collection falls back to procfs tables on Linux and uses shared table selectors.
Explicit SYN filtering predicate `crates/minimald/src/net/switch.rs`	The bare-SYN filtering condition is rewritten with explicit checks while retaining its behavior.

Cross-device materialization

Layer / File(s)	Summary
Hardlink copy fallback `crates/common/src/lib.rs`	Cross-filesystem hardlink failures copy files and emit a process-wide warning once; other failures remain errors.

Sandbox quota handling

Layer / File(s)	Summary
Namespace quota decision `crates/sandbox2/src/lib.rs`	Missing or zero namespace quotas now disable user namespaces through an equivalent predicate.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SSHClient
  participant Minimald
  participant SessionComposer
  participant SandboxLauncher
  SSHClient->>Minimald: attach with forwarded environment
  Minimald->>Minimald: filter inherited variables and add TERM
  Minimald->>SessionComposer: compose session variables
  SessionComposer-->>Minimald: preserve inherited specifications
  Minimald->>SandboxLauncher: launch with AttachEnv
  SandboxLauncher->>SandboxLauncher: layer environment variables

Possibly related PRs

gominimal/minimal#847: Overlaps with session composition and inherited-variable handling.
gominimal/minimal#864: Also concerns Linux /proc/net capture and diagnostics wiring.
gominimal/minimal#963: Targets the session package configuration used by this change.

Suggested reviewers: twitchyliquid64

Poem

A rabbit hops through shells at night,
Carrying locales tucked in tight.
Procfs charts the routes ahead,
Hardlinks copy where paths have fled.
“Compose!” it cheers, “and launch just right!”

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and clearly refers to the PR's environment-defaults change.
Description check	✅ Passed	The description covers summary, motivation, changes, and tests, though it omits the template's checklist and uses 'Tests' instead of 'Testing'.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

twitchyliquid64 · 2026-07-29T18:03:12Z

+                    // is a fixed filesystem-layout fact, not a per-file
+                    // condition.
+                    use std::sync::atomic::{AtomicBool, Ordering};
+                    static WARNED: AtomicBool = AtomicBool::new(false);


Statics make me sad but I guess theres no way to do this cleaner without it being more verbose than its worth

Yeah, I don't really like it either. I don't think we want the log spammed with every single file being copied though. There's probably a better way to do this, but we don't have a ton of time today.

twitchyliquid64 · 2026-07-29T18:05:42Z

-                    } else {
-                        Err(e)
                    }
+                    fs::copy(&path, &dst_path).map(|_| ())


Im also not sure how I feel about this, maybe we are far enough along that its better to be slow and fallback to a copy, but also my other thought is this might mask when your paths are wrong

That's a good point. This is really only required for nested sandboxes, and there may be a better way. We should probably change it to be a preference, but I'll cut an issue for it given timing.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/common/src/lib.rs`:
- Around line 224-242: Update the EXDEV fallback in the hard-linking flow to
avoid writing directly to the final destination, preserving an existing
destination if it appears after the hard_link attempt. Copy through an
exclusively created temporary file and rename it into place, or use an
equivalent create_new flow; treat AlreadyExists as successful completion,
matching the hard-link path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4abbb143-b1a5-4adc-9ae4-966f05ba9a78

📥 Commits

Reviewing files that changed from the base of the PR and between c4ff314 and 56006c6.

📒 Files selected for processing (13)

.minimal/minimal.toml
crates/common/src/lib.rs
crates/diagnostics/src/net.rs
crates/minimal/src/lib.rs
crates/minimald/src/diag.rs
crates/minimald/src/net/switch.rs
crates/minimald/src/session.rs
crates/minimald/src/session_host.rs
crates/sandbox2/src/lib.rs
crates/sessions/example_project/minimal.toml
crates/sessions/src/core/compose.rs
crates/sessions/src/core/primitives.rs
crates/sessions/src/daemon/composer.rs

coderabbitai · 2026-07-29T18:08:09Z

+                Err(e) if e.raw_os_error() == Some(libc::EXDEV) => {
+                    // Every file in a cross-device tree hits EXDEV, so warn only
+                    // on the first — a per-file log would flood with thousands
+                    // of identical lines. Once-per-process is enough: the cause
+                    // is a fixed filesystem-layout fact, not a per-file
+                    // condition.
+                    use std::sync::atomic::{AtomicBool, Ordering};
+                    static WARNED: AtomicBool = AtomicBool::new(false);
+                    if !WARNED.swap(true, Ordering::Relaxed) {
                        warn!(
-                            "Not linking {} => {}, already exists",
+                            "Copying instead of hardlinking: cache and \
+                             destination are on different filesystems; further \
+                             cross-device copies this run are silent (first: {} \
+                             => {})",
                            path.display(),
                            dst_path.display()
                        );
-                        Ok(())
-                    } else {
-                        Err(e)
                    }
+                    fs::copy(&path, &dst_path).map(|_| ())


🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash set -euo pipefail wc -l crates/common/src/lib.rs sed -n '180,280p' crates/common/src/lib.rs | cat -n

Repository: gominimal/minimal

Length of output: 5044

Preserve the existing-destination contract in the EXDEV fallback.
fs::copy(&path, &dst_path) writes to the final path directly, so a destination that appears after hard_link returns EXDEV can be overwritten. Use an exclusively created temp file or create_new/rename flow here, and treat AlreadyExists as success just like the hard-link path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/common/src/lib.rs` around lines 224 - 242, Update the EXDEV fallback in the hard-linking flow to avoid writing directly to the final destination, preserving an existing destination if it appears after the hard_link attempt. Copy through an exclusively created temporary file and rename it into place, or use an equivalent create_new flow; treat AlreadyExists as successful completion, matching the hard-link path.

The merge commit c0f9a4f resolved conflicts in crates/minimal/src/lib.rs by taking the pre-merge side, silently reverting two fixes that had landed on main: - #1053 — the `min init` refuse-to-overwrite guard: `InitArgs::force`, the bail on an existing `minimal.toml`, and the Updated/Created wording. Without it `min init` overwrites an existing `minimal.toml` with no backup and no prompt, which is the data loss #1032 was filed for. - #1060 — the `SHELL=/bin/sh` pin on the ssh ProxyCommand and the `SendEnv` forwarding of LANG/LC_*/TZ. Without the pin, attach dies at "banner exchange ... Broken pipe" for any user whose $SHELL is a bare name (fish) or absent from the ssh context. Neither revert was caught by the suite: there is no test for the init guard, and `interactive_attach_requires_a_tty_on_stdin` only asserts the error contains "not a TTY" and "--command", which holds either way. Rebuilt lib.rs from main and reapplied only the intended change, so the diff against main is now exactly the `hide = true` on `AttachArgs::command` plus the non-TTY error reword: +4/-3, was +9/-51. Verified: rustfmt clean; `cargo build -p minimal --locked` ok; `min session attach --help` no longer lists `-c`/`--command` while `--command` still parses; `min init --help` lists `--force` again. `cargo test -p minimal` cannot run on macOS (dev-deps pull minimald -> procfs/caps, Linux-only), so the suite is left to CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…surface (#1071) * chore(minimal): hide `min session attach --command` from help Add `hide = true` to the `#[arg(long, short)]` attribute on `AttachArgs::command` so `-c` / `--command` no longer appear in `min session attach --help`, `-h`, or shell completions. The flag stays functional for existing scripted callers; the option promises a general remote exec it cannot deliver (no PTY, only three daemon commands accepted), so it should not be advertised for discovery. The non-TTY attach error still points stuck callers at `--command`; reword it to frame the flag as a deliberate hidden escape hatch rather than drop the reference, keeping the advice actionable now that the flag is off the help surface. * fix(minimal): restore #1053 and #1060 clobbered by the merge commit The merge commit c0f9a4f resolved conflicts in crates/minimal/src/lib.rs by taking the pre-merge side, silently reverting two fixes that had landed on main: - #1053 — the `min init` refuse-to-overwrite guard: `InitArgs::force`, the bail on an existing `minimal.toml`, and the Updated/Created wording. Without it `min init` overwrites an existing `minimal.toml` with no backup and no prompt, which is the data loss #1032 was filed for. - #1060 — the `SHELL=/bin/sh` pin on the ssh ProxyCommand and the `SendEnv` forwarding of LANG/LC_*/TZ. Without the pin, attach dies at "banner exchange ... Broken pipe" for any user whose $SHELL is a bare name (fish) or absent from the ssh context. Neither revert was caught by the suite: there is no test for the init guard, and `interactive_attach_requires_a_tty_on_stdin` only asserts the error contains "not a TTY" and "--command", which holds either way. Rebuilt lib.rs from main and reapplied only the intended change, so the diff against main is now exactly the `hide = true` on `AttachArgs::command` plus the non-TTY error reword: +4/-3, was +9/-51. Verified: rustfmt clean; `cargo build -p minimal --locked` ok; `min session attach --help` no longer lists `-c`/`--command` while `--command` still parses; `min init --help` lists `--force` again. `cargo test -p minimal` cannot run on macOS (dev-deps pull minimald -> procfs/caps, Linux-only), so the suite is left to CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: gominimal-aw-bot[bot] <281738952+gominimal-aw-bot[bot]@users.noreply.github.com> Co-authored-by: Norrie Taylor <91171431+norrietaylor@users.noreply.github.com> Co-authored-by: Norrie Taylor <norrie@minimal.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

evanspearman added 2 commits July 29, 2026 02:37

feat: always set certain important variables automatically

4253bee

fix: fixed failing tests in diagnostics module

7ad863b

twitchyliquid64 approved these changes Jul 29, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 29, 2026

View reviewed changes

evanspearman force-pushed the evan/defaultvars branch from 56006c6 to 7ad863b Compare July 29, 2026 18:19

evanspearman merged commit 12c81c3 into main Jul 29, 2026
29 checks passed

evanspearman deleted the evan/defaultvars branch July 29, 2026 19:09

evanspearman mentioned this pull request Jul 29, 2026

Define a default set of env vars to always pass up (regardless of loadout/minimal.toml) #789

Closed

4 tasks

norrietaylor mentioned this pull request Jul 29, 2026

fix: Hide -c/--command on min session attach from the CLI help surface #1071

Merged

Navigation Menu

Uh oh!

feat: always set certain important variables automatically - #1060

feat: always set certain important variables automatically#1060
evanspearman merged 2 commits into
mainfrom
evan/defaultvars

evanspearman commented Jul 29, 2026 •

edited by macroscopeapp Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

twitchyliquid64 Jul 29, 2026

Uh oh!

evanspearman Jul 29, 2026

Uh oh!

twitchyliquid64 Jul 29, 2026

Uh oh!

evanspearman Jul 29, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

evanspearman commented Jul 29, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Set PS1, USER, LOGNAME, SHELL, and LANG automatically in session shells and forward locale/timezone from client

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

twitchyliquid64 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

evanspearman Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

twitchyliquid64 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

evanspearman Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

evanspearman commented Jul 29, 2026 •

edited by macroscopeapp Bot

Loading

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading