From 419e5e2ff0f56087a2db8e703f9df6bc786aad41 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:25:38 +0530 Subject: [PATCH 1/2] Replace github.com/google/shlex with github.com/kballard/go-shellquote github.com/google/shlex has not seen any changes since 2019 and is unmaintained. It is used to split the SOPS_EDITOR/EDITOR environment variables for `sops edit`, and the SOPS_AGE_KEY_CMD and SOPS_AGE_SSH_PRIVATE_KEY_CMD environment variables for the age key source. Switch both call sites to github.com/kballard/go-shellquote, which follows POSIX shell word splitting rules. Besides getting rid of the unmaintained dependency, this fixes shlex dropping backslashes inside double-quoted strings: `"C:\Program Files\Editor\editor.exe" --wait` used to be split into `C:Program FilesEditoreditor.exe` and `--wait`, while it is now split into `C:\Program Files\Editor\editor.exe` and `--wait`. Inside double quotes, a backslash is only treated as an escape character if it is followed by `$`, `` ` ``, `"`, `\` or a newline, as in a POSIX shell. The only other difference is that go-shellquote does not strip `#` comments, which is not something that is useful in an editor or key command. github.com/google/shlex remains an indirect dependency, since github.com/ory/dockertest/v3 (only used by tests) still needs it, but it is no longer compiled into the sops binary. Fixes #1812. Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- age/keysource.go | 4 ++-- age/keysource_test.go | 20 ++++++++++++++++++++ cmd/sops/edit.go | 4 ++-- go.mod | 3 ++- go.sum | 2 ++ 5 files changed, 28 insertions(+), 5 deletions(-) diff --git a/age/keysource.go b/age/keysource.go index 4eb9a200ed..6140eb35fb 100644 --- a/age/keysource.go +++ b/age/keysource.go @@ -16,11 +16,11 @@ import ( "filippo.io/age/agessh" "filippo.io/age/armor" "filippo.io/age/plugin" + "github.com/kballard/go-shellquote" "github.com/sirupsen/logrus" "golang.org/x/crypto/ssh" "github.com/getsops/sops/v3/logging" - "github.com/google/shlex" ) const ( @@ -300,7 +300,7 @@ func (key *MasterKey) TypeToIdentifier() string { func getOutputFromCmd(cmdString string, envVars []string) ([]byte, error) { var out []byte - args, err := shlex.Split(cmdString) + args, err := shellquote.Split(cmdString) if err != nil { return nil, fmt.Errorf("failed to parse command %s: %w", cmdString, err) } diff --git a/age/keysource_test.go b/age/keysource_test.go index da315d1a40..4621f088ec 100644 --- a/age/keysource_test.go +++ b/age/keysource_test.go @@ -577,6 +577,26 @@ func TestMasterKey_loadIdentities(t *testing.T) { }) } +func TestGetOutputFromCmd(t *testing.T) { + t.Run("backslashes in double quotes", func(t *testing.T) { + out, err := getOutputFromCmd(`printf %s "C:\Users\sops\keys.txt"`, nil) + assert.NoError(t, err) + assert.Equal(t, `C:\Users\sops\keys.txt`, string(out)) + }) + + t.Run("env vars", func(t *testing.T) { + out, err := getOutputFromCmd(`sh -c 'printf %s "$FOO"'`, []string{"FOO=bar baz"}) + assert.NoError(t, err) + assert.Equal(t, "bar baz", string(out)) + }) + + t.Run("unterminated quote", func(t *testing.T) { + out, err := getOutputFromCmd(`printf %s 'foo`, nil) + assert.ErrorContains(t, err, "failed to parse command") + assert.Nil(t, out) + }) +} + // overwriteUserConfigDir sets the user config directory and the user home directory // based on the os.UserConfigDir logic. func overwriteUserConfigDir(t *testing.T, path string) { diff --git a/cmd/sops/edit.go b/cmd/sops/edit.go index bfc3a37b94..3d2bc05510 100644 --- a/cmd/sops/edit.go +++ b/cmd/sops/edit.go @@ -18,7 +18,7 @@ import ( "github.com/getsops/sops/v3/cmd/sops/common" "github.com/getsops/sops/v3/keyservice" "github.com/getsops/sops/v3/version" - "github.com/google/shlex" + "github.com/kballard/go-shellquote" exec "golang.org/x/sys/execabs" ) @@ -327,7 +327,7 @@ func runEditor(path string) error { } cmd = exec.Command(editor, path) } else { - parts, err := shlex.Split(editor) + parts, err := shellquote.Split(editor) if err != nil { return fmt.Errorf("invalid $%s: %s", envVar, editor) } diff --git a/go.mod b/go.mod index 811f12b2cd..eccb218878 100644 --- a/go.mod +++ b/go.mod @@ -22,11 +22,11 @@ require ( github.com/getsops/gopgagent v0.0.0-20241224165529-7044f28e491e github.com/go-viper/mapstructure/v2 v2.5.0 github.com/google/go-cmp v0.7.0 - github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/goware/prefixer v0.0.0-20160118172347-395022866408 github.com/hashicorp/go-cleanhttp v0.5.2 github.com/hashicorp/vault/api v1.23.0 github.com/huaweicloud/huaweicloud-sdk-go-v3 v0.1.213 + github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/lib/pq v1.12.3 github.com/mitchellh/go-homedir v1.1.0 github.com/mitchellh/go-wordwrap v1.0.1 @@ -103,6 +103,7 @@ require ( github.com/goccy/go-yaml v1.19.2 // indirect github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/google/s2a-go v0.1.9 // indirect + github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect github.com/google/uuid v1.6.0 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.20 // indirect github.com/googleapis/gax-go/v2 v2.24.0 // indirect diff --git a/go.sum b/go.sum index 24b7aeb953..355f92f6be 100644 --- a/go.sum +++ b/go.sum @@ -245,6 +245,8 @@ github.com/huaweicloud/huaweicloud-sdk-go-v3 v0.1.213 h1:uKt4C7kPCCDk47+VLTPLUY/ github.com/huaweicloud/huaweicloud-sdk-go-v3 v0.1.213/go.mod h1:M+yna96Fx9o5GbIUnF3OvVvQGjgfVSyeJbV9Yb1z/wI= github.com/json-iterator/go v1.1.13-0.20220915233716-71ac16282d12 h1:9Nu54bhS/H/Kgo2/7xNSUuC5G28VR8ljfrLKU2G4IjU= github.com/json-iterator/go v1.1.13-0.20220915233716-71ac16282d12/go.mod h1:TBzl5BIHNXfS9+C35ZyJaklL7mLDbgUkcgXzSLa8Tk0= +github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= +github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= From 98f03d9a2b2f690542164d9727807ab7cd30cb05 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:26:02 +0530 Subject: [PATCH 2/2] age: do not panic when SOPS_AGE_KEY_CMD is empty When SOPS_AGE_KEY_CMD or SOPS_AGE_SSH_PRIVATE_KEY_CMD is set, but is empty or only consists of whitespace, splitting the command results in no arguments at all, and sops panicked with panic: runtime error: slice bounds out of range [1:0] when trying to run the command. This can easily happen when the variable is exported without a value, for example by a CI template. Return an error instead, which is reported together with the other errors encountered while loading age identities. Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- age/keysource.go | 3 +++ age/keysource_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/age/keysource.go b/age/keysource.go index 6140eb35fb..c7f338ff81 100644 --- a/age/keysource.go +++ b/age/keysource.go @@ -304,6 +304,9 @@ func getOutputFromCmd(cmdString string, envVars []string) ([]byte, error) { if err != nil { return nil, fmt.Errorf("failed to parse command %s: %w", cmdString, err) } + if len(args) == 0 { + return nil, errors.New("failed to parse command: command is empty") + } cmd := exec.Command(args[0], args[1:]...) if envVars != nil { cmd.Env = append(os.Environ(), envVars[0:]...) diff --git a/age/keysource_test.go b/age/keysource_test.go index 4621f088ec..18d2adb988 100644 --- a/age/keysource_test.go +++ b/age/keysource_test.go @@ -575,6 +575,22 @@ func TestMasterKey_loadIdentities(t *testing.T) { assert.Nil(t, got) assert.Len(t, unusedLocations, 7) }) + + t.Run("empty cmd", func(t *testing.T) { + tmpDir := t.TempDir() + // Overwrite to ensure local config is not picked up by tests + overwriteUserConfigDir(t, tmpDir) + + t.Setenv(SopsAgeKeyCmdEnv, "") + + key := &MasterKey{} + got, unusedLocations, errs := key.loadIdentities() + assert.Len(t, errs, 1) + assert.Error(t, errs[0]) + assert.ErrorContains(t, errs[0], "command is empty") + assert.Nil(t, got) + assert.Len(t, unusedLocations, 7) + }) } func TestGetOutputFromCmd(t *testing.T) { @@ -595,6 +611,14 @@ func TestGetOutputFromCmd(t *testing.T) { assert.ErrorContains(t, err, "failed to parse command") assert.Nil(t, out) }) + + for _, cmdString := range []string{"", " "} { + t.Run(fmt.Sprintf("empty %q", cmdString), func(t *testing.T) { + out, err := getOutputFromCmd(cmdString, nil) + assert.ErrorContains(t, err, "command is empty") + assert.Nil(t, out) + }) + } } // overwriteUserConfigDir sets the user config directory and the user home directory