diff --git a/pkg/yqlib/doc/operators/system-operators.md b/pkg/yqlib/doc/operators/system-operators.md index a29acadc..f9c0346c 100644 --- a/pkg/yqlib/doc/operators/system-operators.md +++ b/pkg/yqlib/doc/operators/system-operators.md @@ -63,7 +63,7 @@ a: hello ``` then ```bash -yq '.a = system("/bin/echo")' sample.yml +yq '.a = system("/usr/bin/echo")' sample.yml ``` will output ```yaml diff --git a/pkg/yqlib/operator_system.go b/pkg/yqlib/operator_system.go index f7173939..f2e8c9b7 100644 --- a/pkg/yqlib/operator_system.go +++ b/pkg/yqlib/operator_system.go @@ -8,6 +8,20 @@ import ( "strings" ) +func resolveSystemArgs(argsNode *CandidateNode) []string { + if argsNode.Kind == SequenceNode { + args := make([]string, 0, len(argsNode.Content)) + for _, child := range argsNode.Content { + args = append(args, child.Value) + } + return args + } + if argsNode.Tag != "!!null" { + return []string{argsNode.Value} + } + return nil +} + func systemOperator(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode) (Context, error) { if !ConfiguredSecurityPreferences.EnableSystemOps { log.Warning("system operator is disabled, use --enable-system-operator flag to enable") @@ -19,55 +33,46 @@ func systemOperator(d *dataTreeNavigator, context Context, expressionNode *Expre return context.ChildContext(results), nil } - var command string - var argsExpression *ExpressionNode - - // check if it's a block operator (command; args) or just (command) - if expressionNode.RHS.Operation.OperationType == blockOpType { - block := expressionNode.RHS - commandNodes, err := d.GetMatchingNodes(context.ReadOnlyClone(), block.LHS) - if err != nil { - return Context{}, err - } - if commandNodes.MatchingNodes.Front() == nil { - return Context{}, fmt.Errorf("system operator: command expression returned no results") - } - command = commandNodes.MatchingNodes.Front().Value.(*CandidateNode).Value - argsExpression = block.RHS - } else { - commandNodes, err := d.GetMatchingNodes(context.ReadOnlyClone(), expressionNode.RHS) - if err != nil { - return Context{}, err - } - if commandNodes.MatchingNodes.Front() == nil { - return Context{}, fmt.Errorf("system operator: command expression returned no results") - } - command = commandNodes.MatchingNodes.Front().Value.(*CandidateNode).Value - } - - // evaluate args if present - var args []string - if argsExpression != nil { - argsNodes, err := d.GetMatchingNodes(context.ReadOnlyClone(), argsExpression) - if err != nil { - return Context{}, err - } - if argsNodes.MatchingNodes.Front() != nil { - argsNode := argsNodes.MatchingNodes.Front().Value.(*CandidateNode) - if argsNode.Kind == SequenceNode { - for _, child := range argsNode.Content { - args = append(args, child.Value) - } - } else if argsNode.Tag != "!!null" { - args = []string{argsNode.Value} - } - } - } + // determine at parse time whether we have (command; args) or just (command) + hasArgs := expressionNode.RHS.Operation.OperationType == blockOpType var results = list.New() for el := context.MatchingNodes.Front(); el != nil; el = el.Next() { candidate := el.Value.(*CandidateNode) + nodeContext := context.SingleReadonlyChildContext(candidate) + + var command string + var args []string + + if hasArgs { + block := expressionNode.RHS + commandNodes, err := d.GetMatchingNodes(nodeContext, block.LHS) + if err != nil { + return Context{}, err + } + if commandNodes.MatchingNodes.Front() == nil { + return Context{}, fmt.Errorf("system operator: command expression returned no results") + } + command = commandNodes.MatchingNodes.Front().Value.(*CandidateNode).Value + + argsNodes, err := d.GetMatchingNodes(nodeContext, block.RHS) + if err != nil { + return Context{}, err + } + if argsNodes.MatchingNodes.Front() != nil { + args = resolveSystemArgs(argsNodes.MatchingNodes.Front().Value.(*CandidateNode)) + } + } else { + commandNodes, err := d.GetMatchingNodes(nodeContext, expressionNode.RHS) + if err != nil { + return Context{}, err + } + if commandNodes.MatchingNodes.Front() == nil { + return Context{}, fmt.Errorf("system operator: command expression returned no results") + } + command = commandNodes.MatchingNodes.Front().Value.(*CandidateNode).Value + } var stdin bytes.Buffer if candidate.Tag != "!!null" { diff --git a/pkg/yqlib/operator_system_test.go b/pkg/yqlib/operator_system_test.go index 64f1a3f3..388f4446 100644 --- a/pkg/yqlib/operator_system_test.go +++ b/pkg/yqlib/operator_system_test.go @@ -1,9 +1,19 @@ package yqlib import ( + "os/exec" "testing" ) +func findExec(t *testing.T, name string) string { + t.Helper() + path, err := exec.LookPath(name) + if err != nil { + t.Skipf("skipping: %v not found: %v", name, err) + } + return path +} + var systemOperatorDisabledScenarios = []expressionScenario{ { description: "system operator returns null when disabled", @@ -16,46 +26,7 @@ var systemOperatorDisabledScenarios = []expressionScenario{ }, } -var systemOperatorEnabledScenarios = []expressionScenario{ - { - description: "Run a command with an argument", - subdescription: "Use `--enable-system-operator` to enable the system operator.", - document: "country: Australia", - expression: `.country = system("/usr/bin/echo"; "test")`, - expected: []string{ - "D0, P[], (!!map)::country: test\n", - }, - }, - { - description: "Run a command without arguments", - subdescription: "Omit the semicolon and args to run the command with no extra arguments.", - document: "a: hello", - expression: `.a = system("/bin/echo")`, - expected: []string{ - "D0, P[], (!!map)::a: \"\"\n", - }, - }, - { - description: "Run a command with multiple arguments", - subdescription: "Pass an array of arguments.", - skipDoc: true, - document: "a: hello", - expression: `.a = system("/bin/echo"; ["foo", "bar"])`, - expected: []string{ - "D0, P[], (!!map)::a: foo bar\n", - }, - }, - { - description: "Command failure returns error", - skipDoc: true, - document: "a: hello", - expression: `.a = system("/bin/false")`, - expectedError: "system command '/bin/false' failed: exit status 1", - }, -} - func TestSystemOperatorDisabledScenarios(t *testing.T) { - // ensure system operator is disabled originalEnableSystemOps := ConfiguredSecurityPreferences.EnableSystemOps defer func() { ConfiguredSecurityPreferences.EnableSystemOps = originalEnableSystemOps @@ -70,6 +41,9 @@ func TestSystemOperatorDisabledScenarios(t *testing.T) { } func TestSystemOperatorEnabledScenarios(t *testing.T) { + echoPath := findExec(t, "echo") + falsePath := findExec(t, "false") + originalEnableSystemOps := ConfiguredSecurityPreferences.EnableSystemOps defer func() { ConfiguredSecurityPreferences.EnableSystemOps = originalEnableSystemOps @@ -77,8 +51,55 @@ func TestSystemOperatorEnabledScenarios(t *testing.T) { ConfiguredSecurityPreferences.EnableSystemOps = true - for _, tt := range systemOperatorEnabledScenarios { + scenarios := []expressionScenario{ + { + description: "Run a command with an argument", + subdescription: "Use `--enable-system-operator` to enable the system operator.", + document: "country: Australia", + expression: `.country = system("` + echoPath + `"; "test")`, + expected: []string{ + "D0, P[], (!!map)::country: test\n", + }, + }, + { + description: "Run a command without arguments", + subdescription: "Omit the semicolon and args to run the command with no extra arguments.", + document: "a: hello", + expression: `.a = system("` + echoPath + `")`, + expected: []string{ + "D0, P[], (!!map)::a: \"\"\n", + }, + }, + { + description: "Run a command with multiple arguments", + subdescription: "Pass an array of arguments.", + skipDoc: true, + document: "a: hello", + expression: `.a = system("` + echoPath + `"; ["foo", "bar"])`, + expected: []string{ + "D0, P[], (!!map)::a: foo bar\n", + }, + }, + { + description: "Command and args are evaluated per matched node", + skipDoc: true, + document: "cmd: " + echoPath + "\narg: hello", + expression: `.result = system(.cmd; .arg)`, + expected: []string{ + "D0, P[], (!!map)::cmd: " + echoPath + "\narg: hello\nresult: hello\n", + }, + }, + { + description: "Command failure returns error", + skipDoc: true, + document: "a: hello", + expression: `.a = system("` + falsePath + `")`, + expectedError: "system command '" + falsePath + "' failed: exit status 1", + }, + } + + for _, tt := range scenarios { testScenario(t, &tt) } - appendOperatorDocumentScenario(t, "system-operators", systemOperatorEnabledScenarios) + appendOperatorDocumentScenario(t, "system-operators", scenarios) }