diff --git a/cmd/sops/exec_env_test.go b/cmd/sops/exec_env_test.go new file mode 100644 index 0000000000..29f5409723 --- /dev/null +++ b/cmd/sops/exec_env_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/urfave/cli" + + "github.com/getsops/sops/v3" + "github.com/getsops/sops/v3/cmd/sops/codes" +) + +func treeWithItems(items ...sops.TreeItem) *sops.Tree { + return &sops.Tree{Branches: sops.TreeBranches{sops.TreeBranch(items)}} +} + +func TestEnvFromTree(t *testing.T) { + env, err := envFromTree(treeWithItems( + sops.TreeItem{Key: "A", Value: "1"}, + sops.TreeItem{Key: sops.Comment{Value: "ignored"}, Value: nil}, + sops.TreeItem{Key: "N", Value: 42}, + sops.TreeItem{Key: "B", Value: true}, + sops.TreeItem{Key: "M", Value: "x\ny=z"}, + )) + require.NoError(t, err) + assert.Equal(t, []string{"A=1", "N=42", "B=true", "M=x\ny=z"}, env) +} + +func TestEnvFromTreeErrors(t *testing.T) { + tests := []struct { + name string + item sops.TreeItem + expected string + }{ + { + name: "list value", + item: sops.TreeItem{Key: "L", Value: []interface{}{"a", "b"}}, + expected: "cannot use complex value in environment; offending key L", + }, + { + name: "map value", + item: sops.TreeItem{Key: "M", Value: sops.TreeBranch{{Key: "a", Value: "b"}}}, + expected: "cannot use complex value in environment; offending key M", + }, + { + name: "non-string key", + item: sops.TreeItem{Key: 7, Value: "x"}, + expected: "cannot use non-string keys in environment, got int", + }, + { + name: "'=' in key", + item: sops.TreeItem{Key: "A=B", Value: "x"}, + expected: "cannot use keys with '=' in environment: A=B", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := envFromTree(treeWithItems(tt.item)) + assert.EqualError(t, err, tt.expected) + }) + } +} + +func TestExitErrorWithFileName(t *testing.T) { + tests := []struct { + name string + err error + expectedMsg string + expectedCode int + }{ + { + name: "plain error", + err: errors.New("cannot use keys with '=' in environment: A=B"), + expectedMsg: "b.yaml: cannot use keys with '=' in environment: A=B", + expectedCode: codes.ErrorGeneric, + }, + { + name: "exit error keeps its exit code", + err: cli.NewExitError("Error reading file: some reason", codes.CouldNotReadInputFile), + expectedMsg: "b.yaml: Error reading file: some reason", + expectedCode: codes.CouldNotReadInputFile, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := exitErrorWithFileName("b.yaml", tt.err) + assert.EqualError(t, err, tt.expectedMsg) + var exitCoder cli.ExitCoder + require.ErrorAs(t, err, &exitCoder) + assert.Equal(t, tt.expectedCode, exitCoder.ExitCode()) + }) + } +} diff --git a/cmd/sops/main.go b/cmd/sops/main.go index 8092a3f400..829467bfe8 100644 --- a/cmd/sops/main.go +++ b/cmd/sops/main.go @@ -185,7 +185,12 @@ func main() { { Name: "exec-env", Usage: "execute a command with decrypted values inserted into the environment", - ArgsUsage: "[file to decrypt] [command to run]", + ArgsUsage: "[file to decrypt]... [command to run]", + Description: `If more than one file is given, the values of all files are inserted into + the environment. If a key appears in several files, the value from the + last of these files is used. Flags must come before the first file: a + boolean flag placed between two files moves the file after it in front of + the others.`, Flags: append([]cli.Flag{ cli.BoolFlag{ Name: "background", @@ -210,17 +215,14 @@ func main() { }, }, keyserviceFlags...), Action: func(c *cli.Context) error { - if c.NArg() != 2 { + if c.NArg() < 2 { return common.NewExitError(fmt.Errorf("error: missing file to decrypt"), codes.ErrorGeneric) } - fileName := c.Args()[0] - command := c.Args()[1] - - inputStore, err := inputStore(c, fileName) - if err != nil { - return toExitError(err) - } + // All arguments but the last one are files to decrypt, + // the last one is the command to run. + fileNames := c.Args()[:c.NArg()-1] + command := c.Args()[c.NArg()-1] svcs := keyservices(c) @@ -228,15 +230,6 @@ func main() { if err != nil { return toExitError(err) } - opts := decryptOpts{ - OutputStore: &dotenv.Store{}, - InputStore: inputStore, - InputPath: fileName, - Cipher: aes.NewCipher(), - KeyServices: svcs, - DecryptionOrder: order, - IgnoreMAC: c.Bool("ignore-mac"), - } if c.Bool("background") { log.Warn("exec-env's --background option is deprecated and will be removed in a future version of sops") @@ -246,31 +239,27 @@ func main() { } } - tree, err := decryptTree(opts) - if err != nil { - return toExitError(err) - } - + // Decrypt all files before running the command, so that it never + // runs with only part of the environment. A key that appears in + // several files gets the value from the last of them, as if + // exec-env had been nested once per file. var env []string - for _, item := range tree.Branches[0] { - if stores.IsComplexValue(item.Value) { - return cli.NewExitError(fmt.Errorf("cannot use complex value in environment; offending key %s", item.Key), codes.ErrorGeneric) - } - if _, ok := item.Key.(sops.Comment); ok { - continue - } - key, ok := item.Key.(string) - if !ok { - return cli.NewExitError(fmt.Errorf("cannot use non-string keys in environment, got %T", item.Key), codes.ErrorGeneric) - } - if strings.Contains(key, "=") { - return cli.NewExitError(fmt.Errorf("cannot use keys with '=' in environment: %s", key), codes.ErrorGeneric) + for _, fileName := range fileNames { + store, err := inputStore(c, fileName) + if err != nil { + // The config is the same for all files, so this error + // is not about this file and keeps its message. + return toExitError(err) } - value, ok := item.Value.(string) - if !ok { - value = stores.ValToString(item.Value) + fileEnv, err := decryptFileToEnv(c, fileName, store, svcs, order) + if err != nil { + if len(fileNames) > 1 { + // Not every error mentions the file it belongs to. + return exitErrorWithFileName(fileName, err) + } + return toExitError(err) } - env = append(env, fmt.Sprintf("%s=%s", key, value)) + env = exec.MergeEnv(env, fileEnv) } if err := exec.ExecWithEnv(exec.ExecOpts{ @@ -2308,6 +2297,61 @@ func getRotateOpts(c *cli.Context, fileName string, inputStore common.Store, out }, nil } +// decryptFileToEnv decrypts a file for exec-env and returns its top-level +// keys and values as KEY=VALUE pairs. +func decryptFileToEnv(c *cli.Context, fileName string, store common.Store, svcs []keyservice.KeyServiceClient, order []string) ([]string, error) { + tree, err := decryptTree(decryptOpts{ + OutputStore: &dotenv.Store{}, + InputStore: store, + InputPath: fileName, + Cipher: aes.NewCipher(), + KeyServices: svcs, + DecryptionOrder: order, + IgnoreMAC: c.Bool("ignore-mac"), + }) + if err != nil { + return nil, err + } + return envFromTree(tree) +} + +// envFromTree converts the top-level keys and values of a decrypted tree +// to KEY=VALUE pairs for use as environment variables. +func envFromTree(tree *sops.Tree) ([]string, error) { + var env []string + for _, item := range tree.Branches[0] { + if stores.IsComplexValue(item.Value) { + return nil, fmt.Errorf("cannot use complex value in environment; offending key %s", item.Key) + } + if _, ok := item.Key.(sops.Comment); ok { + continue + } + key, ok := item.Key.(string) + if !ok { + return nil, fmt.Errorf("cannot use non-string keys in environment, got %T", item.Key) + } + if strings.Contains(key, "=") { + return nil, fmt.Errorf("cannot use keys with '=' in environment: %s", key) + } + value, ok := item.Value.(string) + if !ok { + value = stores.ValToString(item.Value) + } + env = append(env, fmt.Sprintf("%s=%s", key, value)) + } + return env, nil +} + +// exitErrorWithFileName converts err to an exit error whose message starts +// with fileName, keeping the exit code that toExitError assigns to err. +func exitErrorWithFileName(fileName string, err error) error { + exitCode := codes.ErrorGeneric + if exitCoder, ok := toExitError(err).(cli.ExitCoder); ok { + exitCode = exitCoder.ExitCode() + } + return cli.NewExitError(fmt.Sprintf("%s: %s", fileName, err), exitCode) +} + func toExitError(err error) error { if cliErr, ok := err.(*cli.ExitError); ok && cliErr != nil { return cliErr diff --git a/cmd/sops/subcommand/exec/exec.go b/cmd/sops/subcommand/exec/exec.go index 0abb16c19b..7bdb3c403d 100644 --- a/cmd/sops/subcommand/exec/exec.go +++ b/cmd/sops/subcommand/exec/exec.go @@ -180,3 +180,28 @@ func ExecWithEnv(opts ExecOpts) error { return cmd.Run() } + +// MergeEnv returns the KEY=VALUE pairs from base followed by the ones from +// overrides, leaving out every entry of base whose key also appears in +// overrides. This way the last value for a key wins without relying on how +// the started process handles duplicate keys. Keys are compared as is, +// that is, case-sensitively. +func MergeEnv(base, overrides []string) []string { + overridden := make(map[string]struct{}, len(overrides)) + for _, entry := range overrides { + overridden[envKey(entry)] = struct{}{} + } + merged := make([]string, 0, len(base)+len(overrides)) + for _, entry := range base { + if _, ok := overridden[envKey(entry)]; !ok { + merged = append(merged, entry) + } + } + return append(merged, overrides...) +} + +// envKey returns the key of a KEY=VALUE pair. +func envKey(entry string) string { + key, _, _ := strings.Cut(entry, "=") + return key +} diff --git a/cmd/sops/subcommand/exec/exec_test.go b/cmd/sops/subcommand/exec/exec_test.go new file mode 100644 index 0000000000..09ec1178e3 --- /dev/null +++ b/cmd/sops/subcommand/exec/exec_test.go @@ -0,0 +1,72 @@ +package exec + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeEnv(t *testing.T) { + tests := []struct { + name string + base []string + overrides []string + expected []string + }{ + { + name: "empty base", + overrides: []string{"A=1", "B=2"}, + expected: []string{"A=1", "B=2"}, + }, + { + name: "no overrides", + base: []string{"A=1", "B=2"}, + expected: []string{"A=1", "B=2"}, + }, + { + name: "disjoint keys keep their order", + base: []string{"A=1", "B=2"}, + overrides: []string{"C=3"}, + expected: []string{"A=1", "B=2", "C=3"}, + }, + { + name: "override replaces earlier entry", + base: []string{"A=1", "B=1"}, + overrides: []string{"A=2"}, + expected: []string{"B=1", "A=2"}, + }, + { + name: "only the key decides, values may contain '=' and newlines", + base: []string{"A=x=y", "B=1"}, + overrides: []string{"A=1=2\n3"}, + expected: []string{"B=1", "A=1=2\n3"}, + }, + { + name: "keys are case-sensitive", + base: []string{"a=1"}, + overrides: []string{"A=2"}, + expected: []string{"a=1", "A=2"}, + }, + { + name: "empty value overrides", + base: []string{"A=1"}, + overrides: []string{"A="}, + expected: []string{"A="}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, MergeEnv(tt.base, tt.overrides)) + }) + } +} + +func TestMergeEnvDoesNotModifyArguments(t *testing.T) { + base := []string{"A=1", "B=1"} + overrides := []string{"A=2"} + + MergeEnv(base, overrides) + + assert.Equal(t, []string{"A=1", "B=1"}, base) + assert.Equal(t, []string{"A=2"}, overrides) +} diff --git a/functional-tests/src/lib.rs b/functional-tests/src/lib.rs index 698d310516..7bd700a0a7 100644 --- a/functional-tests/src/lib.rs +++ b/functional-tests/src/lib.rs @@ -1562,6 +1562,133 @@ echo -E "${bar}" assert_eq!(String::from_utf8_lossy(&output.stdout), "baz\nbam\n"); } + #[test] + fn exec_env_multiple_files() { + let first_file_path = prepare_temp_file( + "test_exec_env_multiple_files_1.yaml", + r#"foo: from first +shared: from first +"# + .as_bytes(), + ); + let second_file_path = prepare_temp_file( + "test_exec_env_multiple_files_2.yaml", + r#"bar: from second +shared: from second +"# + .as_bytes(), + ); + for file_path in [&first_file_path, &second_file_path] { + assert!( + Command::new(SOPS_BINARY_PATH) + .arg("encrypt") + .arg("-i") + .arg(file_path) + .output() + .expect("Error running sops") + .status + .success(), + "sops didn't exit successfully" + ); + } + let print_vars = prepare_temp_file( + "print_vars_multiple_files.sh", + r#"#!/bin/bash +echo -E "${foo}|${bar}|${shared}" +"# + .as_bytes(), + ); + + // Values from all files are present; on a key collision the later file wins. + let output = Command::new(SOPS_BINARY_PATH) + .arg("exec-env") + .arg(first_file_path.clone()) + .arg(second_file_path.clone()) + .arg(format!("/bin/bash {}", print_vars)) + .output() + .expect("Error running sops"); + println!( + "stdout: {}, stderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.status.success(), "sops didn't exit successfully"); + assert_eq!( + String::from_utf8_lossy(&output.stdout), + "from first|from second|from second\n" + ); + + // The order of the files decides which value wins. + let output = Command::new(SOPS_BINARY_PATH) + .arg("exec-env") + .arg(second_file_path.clone()) + .arg(first_file_path.clone()) + .arg(format!("/bin/bash {}", print_vars)) + .output() + .expect("Error running sops"); + println!( + "stdout: {}, stderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.status.success(), "sops didn't exit successfully"); + assert_eq!( + String::from_utf8_lossy(&output.stdout), + "from first|from second|from first\n" + ); + + // If a file cannot be used, the command does not run. With several + // files, the error starts with the name of the file; with one file, + // the message is unchanged. The config applies to all files, so an + // error in it does not get a file name. + let plain_file_path = prepare_temp_file( + "test_exec_env_multiple_files_plain.yaml", + "baz: not encrypted\n".as_bytes(), + ); + let bad_config_path = prepare_temp_file( + "test_exec_env_multiple_files_config.yaml", + "creation_rules: [\n".as_bytes(), + ); + for (args, expected_stderr) in [ + ( + vec!["exec-env", &first_file_path, &plain_file_path], + format!("{}: sops metadata not found\n", plain_file_path), + ), + ( + vec!["exec-env", &plain_file_path], + "sops metadata not found\n".to_string(), + ), + ( + vec![ + "--config", + &bad_config_path, + "exec-env", + &first_file_path, + &second_file_path, + ], + "error loading config: ".to_string(), + ), + ] { + let output = Command::new(SOPS_BINARY_PATH) + .args(&args) + .arg(format!("/bin/bash {}", print_vars)) + .output() + .expect("Error running sops"); + println!( + "stdout: {}, stderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(!output.status.success(), "sops succeeded: {:?}", args); + assert_eq!(String::from_utf8_lossy(&output.stdout), ""); + assert!( + String::from_utf8_lossy(&output.stderr).starts_with(&expected_stderr), + "stderr does not start with {:?}", + expected_stderr + ); + } + } + #[test] fn exec_file() { let file_path = prepare_temp_file(