From 0be00d75dfecd05f52ed3412c1ffb82f5a743439 Mon Sep 17 00:00:00 2001 From: abd-ulbasit Date: Sat, 17 Oct 2026 22:25:03 +0500 Subject: [PATCH] Allow passing several files to 'sops exec-env'. All arguments except the last one are now files to decrypt; the last one is still the command to run. 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, which is the same result as nesting one exec-env call per file. Only that last value is passed on, so which file wins does not depend on how the started process (or execve with --same-process) handles duplicate keys. All files are decrypted before the command starts, so if one of them fails, the command does not run. With more than one file, an error about one of the files starts with its name. Errors loading the config file are not prefixed, since the same config applies to all files. Calls with a single file give the same output, error messages and exit codes as before. The only difference is the order of the checks: the flags are now checked before the config file is loaded, so if both are invalid, the error about the flags is reported. Signed-off-by: abd-ulbasit --- cmd/sops/exec_env_test.go | 95 +++++++++++++++++++ cmd/sops/main.go | 124 +++++++++++++++++-------- cmd/sops/subcommand/exec/exec.go | 25 +++++ cmd/sops/subcommand/exec/exec_test.go | 72 +++++++++++++++ functional-tests/src/lib.rs | 127 ++++++++++++++++++++++++++ 5 files changed, 403 insertions(+), 40 deletions(-) create mode 100644 cmd/sops/exec_env_test.go create mode 100644 cmd/sops/subcommand/exec/exec_test.go 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(