From f21408028006182a9c66d4df6b251c02c3d5a308 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 22:14:05 +0000 Subject: [PATCH] fix: [review-r1] seed loop sudo invocation bypasses sudoers env_reset (#929) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sudo -n "VAULT_ADDR=$vault_addr" -- "$seed_script"` passed VAULT_ADDR as a sudoers env-assignment argument. With the default `env_reset=on` policy (almost all distros), sudo silently discards env assignments unless the variable is in `env_keep` — and VAULT_ADDR is not. The seeder then hit its own precondition check at vault-seed-forgejo.sh:109 and died with "VAULT_ADDR unset", breaking the fresh-LXC non-root acceptance path the PR was written to close. Fix: run `env` as the command under sudo — `sudo -n -- env "VAULT_ADDR=$vault_addr" "$seed_script"` — so VAULT_ADDR is set in the child process directly, unaffected by sudoers env handling. The root (non-sudo) branch already used shell-level env assignment and was correct. Adds a grep-level regression guard that pins the `env VAR=val` invocation and negative-asserts the unsafe bare-argument form. Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/disinto | 9 ++++++++- tests/disinto-init-nomad.bats | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/bin/disinto b/bin/disinto index 0a78db6..5f57927 100755 --- a/bin/disinto +++ b/bin/disinto @@ -928,6 +928,13 @@ _disinto_init_nomad() { # hasn't sourced it yet; sibling vault-* scripts (engines/policies/ # auth/import) default VAULT_ADDR internally via _hvault_default_env, # but vault-seed-forgejo.sh requires the caller to set it. + # + # The non-root branch invokes the seeder as `sudo -n -- env VAR=val + # script` rather than `sudo -n VAR=val -- script`: sudo treats bare + # `VAR=val` args as sudoers env-assignments, which the default + # `env_reset=on` policy silently discards unless the variable is in + # `env_keep` (VAULT_ADDR is not). Using `env` as the actual command + # sets VAULT_ADDR in the child process regardless of sudoers policy. if [ -n "$with_services" ]; then local vault_addr="${VAULT_ADDR:-http://127.0.0.1:8200}" local IFS=',' @@ -944,7 +951,7 @@ _disinto_init_nomad() { echo "Error: vault-seed-${svc}.sh must run as root and sudo is not installed" >&2 exit 1 fi - sudo -n "VAULT_ADDR=$vault_addr" -- "$seed_script" || exit $? + sudo -n -- env "VAULT_ADDR=$vault_addr" "$seed_script" || exit $? fi fi done diff --git a/tests/disinto-init-nomad.bats b/tests/disinto-init-nomad.bats index 8467ebb..21f4303 100644 --- a/tests/disinto-init-nomad.bats +++ b/tests/disinto-init-nomad.bats @@ -177,6 +177,22 @@ setup_file() { [ "$seed_line" -lt "$deploy_line" ] } +# Regression guard (PR #929 review): `sudo -n VAR=val -- cmd` is subject +# to sudoers env_reset policy and silently drops VAULT_ADDR unless it's +# in env_keep (it isn't in default configs). vault-seed-forgejo.sh +# requires VAULT_ADDR and dies at its own precondition check if unset, +# so the non-root branch MUST invoke `sudo -n -- env VAR=val cmd` so +# that `env` sets the variable in the child process regardless of +# sudoers policy. This grep-level guard catches a revert to the unsafe +# form that silently broke non-root seed runs on a fresh LXC. +@test "seed loop invokes sudo via 'env VAR=val' (bypasses sudoers env_reset)" { + run grep -F 'sudo -n -- env "VAULT_ADDR=' "$DISINTO_BIN" + [ "$status" -eq 0 ] + # Negative: no bare `sudo -n "VAR=val" --` form anywhere in the file. + run grep -F 'sudo -n "VAULT_ADDR=' "$DISINTO_BIN" + [ "$status" -ne 0 ] +} + @test "disinto init --backend=nomad --with forgejo,forgejo --dry-run handles comma-separated services" { run "$DISINTO_BIN" init placeholder/repo --backend=nomad --with forgejo,forgejo --dry-run [ "$status" -eq 0 ]