Use safe expansion for SSH_CONNECTION and SHLVL variable

[larsgerber]

Description

Resolves #1722

Replaced direct variable expansion with ${VAR:-} to avoid “parameter not set” errors when running the Mac App Store command-line interface.

Checklist

• Formatted with cargo fmt
• Built with nix build
• Ran flake checks with nix flake check
• Added or updated relevant tests (leave unchecked if not applicable)
• Added or updated relevant documentation (leave unchecked if not applicable)
• Linked to related issues (leave unchecked if not applicable)

Summary by CodeRabbit

• Bug Fixes
  • macOS remote-building setup now robustly handles unset SSH/session variables in non-interactive SSH sessions. This prevents errors when those variables are absent and ensures the Nix profile sourcing behavior remains consistent across different shell environments.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR fixes an issue where unset environment variables (SSH_CONNECTION and SHLVL) were causing "parameter not set" errors in shell scripts. Parameter expansions now use default values to ensure safe evaluation in all shell contexts without altering control flow.

Changes

Cohort / File(s)	Summary
SSH parameter expansion defaults src/action/macos/configure_remote_building.rs, tests/fixtures/macos/macos.json	Updated shell snippet to use safe parameter expansion: ${SSH_CONNECTION:-} (empty default) and ${SHLVL:-0} (zero default) to prevent "parameter not set" errors in non-interactive SSH sessions. Test fixture updated to match.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

• Straightforward parameter expansion defaults applied consistently across implementation and test fixture
• No logic changes or control flow modifications
• Simple, homogeneous pattern applied to two related files

Poem

🐰 A shell was crying, "Variables not set!"
So we gave them defaults—no need to fret.
With :- and :-0, the problem's done,
SSH connections now flow like fun!
🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: adding safe variable expansion for SSH_CONNECTION and SHLVL to prevent parameter not set errors.
Linked Issues check	✅ Passed	The PR fully addresses issue #1722 by implementing the exact fix proposed: using ${SSH_CONNECTION:-} and ${SHLVL:-0} safe expansions in the shell conditional.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the SSH_CONNECTION and SHLVL variable expansion issue; no out-of-scope modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac037fb and db1eeef.

📒 Files selected for processing (2)

• src/action/macos/configure_remote_building.rs (1 hunks)
• tests/fixtures/macos/macos.json (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/fixtures/macos/macos.json

🔇 Additional comments (1)

src/action/macos/configure_remote_building.rs (1)

22-31: Safe parameter expansion change looks correct and preserves behavior

The updated condition

[ -n "${SSH_CONNECTION:-}" ] && [ "${SHLVL:-0}" -eq 1 ]

is generated correctly via

"${{SSH_CONNECTION:-}}"  // -> "${SSH_CONNECTION:-}"
"${{SHLVL:-0}}"         // -> "${SHLVL:-0}"

and does what you want:

• When SSH_CONNECTION/SHLVL are set, behavior matches the previous version.
• When either is unset under set -u/NOUNSET, the test now safely evaluates to false instead of erroring, so mas (and other tools) won’t see /etc/zshenv: SSH_CONNECTION: parameter not set while still skipping Nix profile sourcing.

No issues spotted with quoting or the integer comparison; this cleanly fixes the linked bug without changing the positive-path semantics.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[grahamc]

I lightly tested this by running this to get the final string:

fn main() {
    const PROFILE_NIX_FILE_SHELL: &str =
        "/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh";

    let st = format!(
        r#"
# Set up Nix only on SSH connections
# See: https://github.com/DeterminateSystems/nix-installer/pull/714
if [ -e '{PROFILE_NIX_FILE_SHELL}' ] && [ -n "${{SSH_CONNECTION}}" ] && [ "${{SHLVL}}" -eq 1 ]; then
if [ -e '{PROFILE_NIX_FILE_SHELL}' ] && [ -n "${{SSH_CONNECTION:-}}" ] && [ "${{SHLVL:-0}}" -eq 1 ]; then
    . '{PROFILE_NIX_FILE_SHELL}'
fi
# End Nix
"#
    );

    println!("{}", st);
}

and edited my /etc/zshenv:

grahamc@Mac nix-src % cat /etc/zshenv

# Set up Nix only on SSH connections
# See: https://github.com/DeterminateSystems/nix-installer/pull/714
if [ -e '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh' ] && [ -n "${SSH_CONNECTION:-}" ] && [ "${SHLVL:-0}" -eq 1 ]; then
    . '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh'
fi
# End Nix

and everything seems fine from here. I think this is good, but I haven't actually tested the mas cli.

Reading the patch, it looks correct on the face of it, and shellcheck in sh mode doesn't raise any new concerns.