Correctly handle new multiline secrets

internal/action/generate.go

@@ -436,6 +436,7 @@ func (s *Action) generateReplaceExisting(ctx context.Context, name, key, passwor
 
 func setMetadata(sec gopass.Secret, kvps map[string]string) {
 	for k, v := range kvps {
+		debug.Log("setting %s to %s", k, v)
 		_ = sec.Set(k, v)
 	}
 }

internal/action/insert.go

@@ -185,6 +185,7 @@ func (s *Action) insertGetSecret(ctx context.Context, name, pw string) (gopass.S
 
 // insertYAML will overwrite existing keys.
 func (s *Action) insertYAML(ctx context.Context, name, key string, content []byte, kvps map[string]string) error {
+	debug.Log("insertYAML: %s - %s -> %s", name, key, content)
 	if ctxutil.IsInteractive(ctx) {
 		pw, err := termio.AskForString(ctx, name+":"+key, "")
 		if err != nil {
@@ -200,12 +201,15 @@ func (s *Action) insertYAML(ctx context.Context, name, key string, content []byt
 		if err != nil {
 			return exit.Error(exit.Encrypt, err, "failed to set key %q of %q: %s", key, name, err)
 		}
+		debug.Log("using existing secret %s", name)
 	} else {
 		sec = secrets.New()
+		debug.Log("creating new secret %s", name)
 	}
 
 	setMetadata(sec, kvps)
 
+	debug.Log("setting %s to %s", key, string(content))
 	if err := sec.Set(key, string(content)); err != nil {
 		return exit.Error(exit.Usage, err, "failed set key %q of %q: %q", key, name, err)
 	}

pkg/debug/debug.go

@@ -44,6 +44,10 @@ func initDebug() bool {
 		return false
 	}
 
+	// we need to explicitly set logSecrets to false in case tests run under an environment
+	// where GOPASS_DEBUG_LOG_SECRETS is true. Otherwise setting it to false in the test
+	// context won't have any effect.
+	logSecrets = false
 	if sv := os.Getenv("GOPASS_DEBUG_LOG_SECRETS"); sv != "" && sv != "false" {
 		logSecrets = true
 	}

pkg/debug/debug_test.go

@@ -51,6 +51,7 @@ func TestDebug(t *testing.T) {
 
 	fn := filepath.Join(td, "gopass.log")
 	t.Setenv("GOPASS_DEBUG_LOG", fn)
+	t.Setenv("GOPASS_DEBUG_LOG_SECRETS", "false")
 
 	// it's been already initialized, need to re-init
 	assert.True(t, initDebug())
@@ -64,6 +65,7 @@ func TestDebug(t *testing.T) {
 
 	logStr := string(buf)
 	assert.Contains(t, logStr, "foo")
+	assert.NotEqual(t, "true", os.Getenv("GOPASS_DEBUG_LOG_SECRETS"))
 	assert.NotContains(t, logStr, "secret")
 	assert.NotContains(t, logStr, "toolong")
 	assert.Contains(t, logStr, "shorter")

pkg/fsutil/fsutil_test.go

@@ -28,6 +28,7 @@ func TestCleanFilename(t *testing.T) {
 
 func TestCleanPath(t *testing.T) {
 	tempdir := t.TempDir()
+	t.Setenv("GOPASS_HOMEDIR", "")
 
 	m := map[string]string{
 		".":                                 "",

pkg/gitconfig/config_test.go

@@ -3,8 +3,8 @@ package gitconfig
 import (
 	"bytes"
 	"fmt"
-	"os"
 	"math/rand"
+	"os"
 	"path/filepath"
 	"strconv"
 	"strings"

pkg/gopass/secrets/akv.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"bytes"
 	"fmt"
+	"io"
 	"strings"
 
 	"github.com/gopasspw/gopass/internal/set"
@@ -28,7 +29,6 @@ func NewAKV() *AKV {
 		kvp: make(map[string][]string),
 		raw: strings.Builder{},
 	}
-	a.raw.WriteString("\n")
 
 	return a
 }
@@ -92,6 +92,9 @@ func (a *AKV) Set(key string, value any) error {
 	// if the key does exist we must make sure to update only
 	// the first instance and leave all others intact.
 
+	if a.raw.Len() == 0 {
+		a.raw.WriteString("\n")
+	}
 	s := bufio.NewScanner(strings.NewReader(a.raw.String()))
 	a.raw = strings.Builder{}
 
@@ -143,6 +146,10 @@ func (a *AKV) Add(key string, value any) error {
 	sv := fmt.Sprintf("%s", value)
 	a.kvp[key] = append(a.kvp[key], sv)
 
+	// we must not accidentially write the KVP into the password field
+	if a.raw.Len() == 0 {
+		a.raw.WriteString("\n")
+	}
 	a.raw.WriteString(fmt.Sprintf("%s: %s\n", key, sv))
 
 	return nil
@@ -298,7 +305,21 @@ func (a *AKV) Body() string {
 
 // Write appends the buffer to the secret's body.
 func (a *AKV) Write(buf []byte) (int, error) {
-	return a.raw.Write(buf)
+	var w io.Writer
+	w = &a.raw
+
+	// If the body is empty before writing we must treat the first line
+	// of the newly written content as the password or multi-line insert
+	// and similar operations (like insert from stdin) might fail.
+	// For regular command line usage this is actually a non-issue since
+	// the gopass process will exit after writing and when reading the secret
+	// it will be (correctly) parsed. But for tests, library and maybe REPL
+	// usage this is an issue.
+	if a.raw.Len() == 0 {
+		w = &pwWriter{w: &a.raw, cb: func(pw string) { a.password = pw }}
+	}
+
+	return w.Write(buf)
 }
 
 // FromMime returns whether this secret was converted from a Mime secret of not.
@@ -310,3 +331,33 @@ func (a *AKV) FromMime() bool {
 func (a *AKV) SafeStr() string {
 	return "(elided)"
 }
+
+// pwWriter is a io.Writer that will extract the first line of the input stream and
+// then write it to the password field of the provided callback. The first line can stretch
+// multiple chunks but once the first line has been completed any writes to this
+// writer will be silently discarded.
+type pwWriter struct {
+	w   io.Writer
+	cb  func(string)
+	buf strings.Builder
+}
+
+func (p *pwWriter) Write(buf []byte) (int, error) {
+	n, err := p.w.Write(buf)
+	if p.cb == nil {
+		return n, err
+	}
+
+	i := bytes.Index(buf, []byte("\n"))
+	if i > 0 {
+		p.buf.Write(buf[0:i])
+
+		p.cb(p.buf.String())
+		p.cb = nil
+
+		return n, err
+	}
+	p.buf.Write(buf)
+
+	return n, err
+}

pkg/gopass/secrets/akv_test.go

@@ -84,6 +84,37 @@ zab: 123
 
 	sec := ParseAKV([]byte(in))
 	assert.Equal(t, in, string(sec.Bytes()))
+	assert.Equal(t, "passw0rd", sec.Password())
+}
+
+func TestMultilineInsertAKV(t *testing.T) {
+	t.Parallel()
+
+	in := `passw0rd
+foo: baz
+foo: bar
+zab: 123
+`
+
+	sec := NewAKV()
+	_, err := sec.Write([]byte(in))
+	assert.NoError(t, err)
+	assert.Equal(t, in, string(sec.Bytes()))
+	assert.Equal(t, "passw0rd", sec.Password())
+
+	_, err = sec.Write([]byte("more text"))
+	assert.NoError(t, err)
+	assert.Equal(t, "passw0rd", sec.Password())
+}
+
+func TestSetKeyValuePairToEmptyAKV(t *testing.T) {
+	t.Parallel()
+
+	sec := NewAKV()
+	assert.NoError(t, sec.Set("foo", "bar"))
+	v, found := sec.Get("foo")
+	assert.True(t, found)
+	assert.Equal(t, "bar", v)
 }
 
 func TestParseAKV(t *testing.T) {
@@ -310,3 +341,34 @@ func FuzzParseAKV(f *testing.F) {
 		ParseAKV(in)
 	})
 }
+
+func TestPwWriter(t *testing.T) {
+	a := NewAKV()
+	p := pwWriter{w: &a.raw, cb: func(pw string) { a.password = pw }}
+
+	// multi-chunk passwords are supported
+	_, err := p.Write([]byte("foo"))
+	assert.NoError(t, err)
+
+	_, err = p.Write([]byte("bar\n"))
+	assert.NoError(t, err)
+
+	// but anything after the first line is discarded
+	_, err = p.Write([]byte("baz\n"))
+	assert.NoError(t, err)
+
+	assert.Equal(t, "foobar", a.Password())
+	assert.Equal(t, "baz\n", a.Body())
+}
+
+func TestInvalidPwWriter(t *testing.T) {
+	defer func() {
+		r := recover()
+		assert.NotNil(t, r)
+	}()
+	p := pwWriter{}
+
+	// will panic because the writer is nil
+	_, err := p.Write([]byte("foo"))
+	assert.Error(t, err)
+}