diff --git a/internal/action/generate.go b/internal/action/generate.go index 5b5fb99c5d..4a0439e061 100644 --- a/internal/action/generate.go +++ b/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) } } diff --git a/internal/action/insert.go b/internal/action/insert.go index 1a2eafa5b7..0dbe579865 100644 --- a/internal/action/insert.go +++ b/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) } diff --git a/pkg/debug/debug.go b/pkg/debug/debug.go index 968cf48d3a..de73051606 100644 --- a/pkg/debug/debug.go +++ b/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 } diff --git a/pkg/debug/debug_test.go b/pkg/debug/debug_test.go index 6b8cd0996a..10856c74d2 100644 --- a/pkg/debug/debug_test.go +++ b/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") diff --git a/pkg/fsutil/fsutil_test.go b/pkg/fsutil/fsutil_test.go index d57ecc93d0..ff113598ab 100644 --- a/pkg/fsutil/fsutil_test.go +++ b/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{ ".": "", diff --git a/pkg/gitconfig/config_test.go b/pkg/gitconfig/config_test.go index 8608eb5ee7..f3656a1f9a 100644 --- a/pkg/gitconfig/config_test.go +++ b/pkg/gitconfig/config_test.go @@ -3,8 +3,8 @@ package gitconfig import ( "bytes" "fmt" - "os" "math/rand" + "os" "path/filepath" "strconv" "strings" diff --git a/pkg/gopass/secrets/akv.go b/pkg/gopass/secrets/akv.go index 0b2294d6da..8fcfb302a2 100644 --- a/pkg/gopass/secrets/akv.go +++ b/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 +} diff --git a/pkg/gopass/secrets/akv_test.go b/pkg/gopass/secrets/akv_test.go index ca0cfaad32..c663703b7f 100644 --- a/pkg/gopass/secrets/akv_test.go +++ b/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) +} diff --git a/pkg/pwgen/pwrules/pwrules_gen.go b/pkg/pwgen/pwrules/pwrules_gen.go index 96bc5c3104..3c52285fc2 100644 --- a/pkg/pwgen/pwrules/pwrules_gen.go +++ b/pkg/pwgen/pwrules/pwrules_gen.go @@ -8,7 +8,6 @@ // https://raw.githubusercontent.com/apple/password-manager-resources/main/quirks/change-password-URLs.json // // https://raw.githubusercontent.com/apple/password-manager-resources/main/quirks/password-rules.json -// package pwrules var genAliases = map[string][]string{