diff --git a/scripts/elway b/scripts/elway index 7555fe1..0ac8a2d 100755 --- a/scripts/elway +++ b/scripts/elway @@ -570,14 +570,34 @@ def run_upload_step(ctx: SSHContext, step: Step, prefix: str) -> int: # ─── Skip + change evaluators (tier 1 + tier 2) ──────────────────────────── -def _quiet_rc(ctx: SSHContext, remote_cmd: str) -> int: - """Run a shell expression on the remote under bash -c, discarding output.""" +def _quiet_rc(ctx: SSHContext, remote_cmd: str, sudo: bool = False) -> int: + """Run a shell expression on the remote under bash -c, discarding output. + + ⚠ `sudo` MUST track the step's own sudo setting. Conditions used to run + unprivileged no matter what the step said, and that fails in the dangerous + direction: a root-requiring `when:` (say `pvesh get ...`, which exits 255 + for a non-root user) returns non-zero, elway reports the step `skipped`, + and a playbook that never performed its change reports overall OK. Measured + 2026-09-19 on esh-pve — three runs of an exclusion playbook reported + success while changing nothing, and only the verify phase caught it. + + Sudo-less steps are unaffected: their conditions still evaluate as the + login user, which is what they mean. + """ # Wrapping in bash -c gives `!`, `[[`, pipes, etc. consistent semantics # across hosts whose default login shell might be dash (Debian default) or # something else weird. - wrapped = "bash -c " + shlex.quote(remote_cmd) + if sudo: + ensure_sudo(ctx) + wrapped = "sudo -S -p '' bash -c " + shlex.quote(remote_cmd) + stdin_data = (ctx.sudo_password or "") + "\n" + else: + wrapped = "bash -c " + shlex.quote(remote_cmd) + stdin_data = None return subprocess.run( ctx.ssh_cmd(wrapped), + input=stdin_data, + text=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, ).returncode @@ -586,15 +606,15 @@ def _quiet_rc(ctx: SSHContext, remote_cmd: str) -> int: def evaluate_skip(ctx: SSHContext, step: Step) -> Optional[str]: """If any pre-condition says skip, return a human-readable reason. Else None.""" if step.when is not None: - rc = _quiet_rc(ctx, step.when) + rc = _quiet_rc(ctx, step.when, step.sudo) if rc != 0: return f"when: expr exited {rc} (needed 0 to run)" if step.creates is not None: - rc = _quiet_rc(ctx, f"test -e {shlex.quote(step.creates)}") + rc = _quiet_rc(ctx, f"test -e {shlex.quote(step.creates)}", step.sudo) if rc == 0: return f"creates: {step.creates} already exists" if step.removes is not None: - rc = _quiet_rc(ctx, f"test -e {shlex.quote(step.removes)}") + rc = _quiet_rc(ctx, f"test -e {shlex.quote(step.removes)}", step.sudo) if rc != 0: return f"removes: {step.removes} is already absent" return None @@ -604,7 +624,7 @@ def evaluate_changed(ctx: SSHContext, step: Step) -> bool: """Post-step: should this count as `changed`? Default True (Ansible shell default).""" if step.changed_when is None: return True - rc = _quiet_rc(ctx, step.changed_when) + rc = _quiet_rc(ctx, step.changed_when, step.sudo) # exit 0 = the change-detector expression "fired" = step counts as changed. return rc == 0