fix: [review-r1] seed loop sudo invocation bypasses sudoers env_reset (#929)
All checks were successful
All checks were successful
`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) <noreply@anthropic.com>
This commit is contained in:
parent
5e83ecc2ef
commit
f214080280
2 changed files with 24 additions and 1 deletions
|
|
@ -928,6 +928,13 @@ _disinto_init_nomad() {
|
||||||
# hasn't sourced it yet; sibling vault-* scripts (engines/policies/
|
# hasn't sourced it yet; sibling vault-* scripts (engines/policies/
|
||||||
# auth/import) default VAULT_ADDR internally via _hvault_default_env,
|
# auth/import) default VAULT_ADDR internally via _hvault_default_env,
|
||||||
# but vault-seed-forgejo.sh requires the caller to set it.
|
# 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
|
if [ -n "$with_services" ]; then
|
||||||
local vault_addr="${VAULT_ADDR:-http://127.0.0.1:8200}"
|
local vault_addr="${VAULT_ADDR:-http://127.0.0.1:8200}"
|
||||||
local IFS=','
|
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
|
echo "Error: vault-seed-${svc}.sh must run as root and sudo is not installed" >&2
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
sudo -n "VAULT_ADDR=$vault_addr" -- "$seed_script" || exit $?
|
sudo -n -- env "VAULT_ADDR=$vault_addr" "$seed_script" || exit $?
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
|
|
|
||||||
|
|
@ -177,6 +177,22 @@ setup_file() {
|
||||||
[ "$seed_line" -lt "$deploy_line" ]
|
[ "$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" {
|
@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
|
run "$DISINTO_BIN" init placeholder/repo --backend=nomad --with forgejo,forgejo --dry-run
|
||||||
[ "$status" -eq 0 ]
|
[ "$status" -eq 0 ]
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue