Skip to content

facts: fix requires_command shell operator precedence with compound commands#1579

Open
wowi42 wants to merge 1 commit intopyinfra-dev:3.xfrom
KalvadTech:fix/requires-command-operator-precedence
Open

facts: fix requires_command shell operator precedence with compound commands#1579
wowi42 wants to merge 1 commit intopyinfra-dev:3.xfrom
KalvadTech:fix/requires-command-operator-precedence

Conversation

@wowi42
Copy link
Contributor

@wowi42 wowi42 commented Mar 5, 2026

When a fact uses requires_command and its command contains && chains (e.g. runit.RunitStatus), the generated shell command has incorrect operator precedence:

! command -v sv >/dev/null || export SVDIR=... && cd "$SVDIR" && ...

Shell parses || and && left-to-right, so when sv is missing the || short-circuits but subsequent && clauses still run and fail, causing the fact to error instead of returning the default value.

Wrap the fact command in a subshell so || applies to the entire chain:

! command -v sv >/dev/null || ( export SVDIR=... && cd "$SVDIR" && ... )

  • Pull request is based on the default branch (3.x at this time)
  • Pull request includes tests for any new/updated operations/facts
  • Pull request includes documentation for any new/updated operations/facts
  • Tests pass (see scripts/dev-test.sh)
  • Type checking & code style passes (see scripts/dev-lint.sh)

…ommands

When a fact uses `requires_command` and its command contains `&&` chains
(e.g. runit.RunitStatus), the generated shell command has incorrect
operator precedence:

  ! command -v sv >/dev/null || export SVDIR=... && cd "$SVDIR" && ...

Shell parses `||` and `&&` left-to-right, so when `sv` is missing the
`||` short-circuits but subsequent `&&` clauses still run and fail,
causing the fact to error instead of returning the default value.

Wrap the fact command in a subshell so `||` applies to the entire chain:

  ! command -v sv >/dev/null || ( export SVDIR=... && cd "$SVDIR" && ... )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant