From 5048a40307cb8511e40cfacfbf6c1f13fefde1da Mon Sep 17 00:00:00 2001
From: Dominik Schulz <dominik.schulz@gauner.org>
Date: Sat, 22 Aug 2020 17:36:00 +0200
Subject: [PATCH] Fix safecontent obstruction (#1541)

RELEASE_NOTES=n/a

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
---
 cmd/gopass-jsonapi/internal/jsonapi/api_test.go | 6 +++---
 internal/action/show.go                         | 6 +++---
 internal/action/show_test.go                    | 6 ++++--
 internal/secrets/kv.go                          | 3 ++-
 internal/secrets/kv_test.go                     | 2 --
 internal/secrets/yaml.go                        | 3 ++-
 internal/secrets/yaml_test.go                   | 2 +-
 7 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/cmd/gopass-jsonapi/internal/jsonapi/api_test.go b/cmd/gopass-jsonapi/internal/jsonapi/api_test.go
index 09e11be52f..a15d0e0e29 100644
--- a/cmd/gopass-jsonapi/internal/jsonapi/api_test.go
+++ b/cmd/gopass-jsonapi/internal/jsonapi/api_test.go
@@ -177,16 +177,16 @@ sub:
 
 	runRespondMessage(t,
 		`{"type":"getData","entry":"foo"}`,
-		`{"hallo":"welt"}`,
+		`{"hallo":"welt","password":"20"}`,
 		"", secrets)
 
 	runRespondMessage(t,
 		`{"type":"getData","entry":"bar"}`,
-		`{"login":"muh"}`,
+		`{"login":"muh","password":"20"}`,
 		"", secrets)
 	runRespondMessage(t,
 		`{"type":"getData","entry":"complex"}`,
-		`{"login":"hallo","number":"42","sub":"map.subentry:123."}`,
+		`{"login":"hallo","number":"42","password":"20","sub":"map.subentry:123."}`,
 		"", secrets)
 
 	runRespondMessage(t,
diff --git a/internal/action/show.go b/internal/action/show.go
index 83ecad39d8..d60b73aa12 100644
--- a/internal/action/show.go
+++ b/internal/action/show.go
@@ -180,18 +180,18 @@ func (s *Action) showGetContent(ctx context.Context, sec gopass.Secret) (string,
 	if ctxutil.IsShowSafeContent(ctx) && !ctxutil.IsForce(ctx) {
 		var sb strings.Builder
 		for _, k := range sec.Keys() {
-			if k == "Password" {
-				continue
-			}
 			sb.WriteString(k)
 			sb.WriteString(": ")
 			// check is this key should be obstructed
 			if isUnsafeKey(k, sec) {
+				debug.Log("obstructing unsafe key %s", k)
 				sb.WriteString(randAsterisk())
 			} else {
 				sb.WriteString(sec.Get(k))
 			}
+			sb.WriteString("\n")
 		}
+		sb.WriteString("\n")
 		sb.WriteString(sec.GetBody())
 		if IsAlsoClip(ctx) {
 			return sec.Get("password"), sb.String()
diff --git a/internal/action/show_test.go b/internal/action/show_test.go
index cd6fb4ade6..0ce418daa1 100644
--- a/internal/action/show_test.go
+++ b/internal/action/show_test.go
@@ -71,7 +71,8 @@ func TestShowMulti(t *testing.T) {
 		c := gptest.CliCtx(ctx, t, "bar/baz")
 
 		assert.NoError(t, act.Show(c))
-		assert.Equal(t, "Bar: zab", buf.String())
+		assert.Contains(t, buf.String(), "Bar: zab")
+		assert.Contains(t, buf.String(), "Password: ***")
 		buf.Reset()
 	})
 
@@ -122,7 +123,8 @@ func TestShowMulti(t *testing.T) {
 		c := gptest.CliCtx(ctx, t, "bar/baz")
 
 		assert.NoError(t, act.Show(c))
-		assert.Equal(t, "Bar: zab", buf.String())
+		assert.Contains(t, buf.String(), "Bar: zab")
+		assert.Contains(t, buf.String(), "Password: ***")
 		buf.Reset()
 	})
 }
diff --git a/internal/secrets/kv.go b/internal/secrets/kv.go
index 485b48c33e..40f12c1f4e 100644
--- a/internal/secrets/kv.go
+++ b/internal/secrets/kv.go
@@ -56,10 +56,11 @@ func (k *KV) Bytes() []byte {
 
 // Keys returns all keys
 func (k *KV) Keys() []string {
-	keys := make([]string, 0, len(k.data))
+	keys := make([]string, 0, len(k.data)+1)
 	for key := range k.data {
 		keys = append(keys, key)
 	}
+	keys = append(keys, "password")
 	sort.Strings(keys)
 	return keys
 }
diff --git a/internal/secrets/kv_test.go b/internal/secrets/kv_test.go
index 1338d44601..4fd8279cbc 100644
--- a/internal/secrets/kv_test.go
+++ b/internal/secrets/kv_test.go
@@ -12,7 +12,6 @@ func TestKV(t *testing.T) {
 	mlValue := `somepasswd
 Test / test.com
 username: myuser@test.com
-password: somepasswd
 url: http://www.test.com/
 `
 	s, err := ParseKV([]byte(mlValue))
@@ -24,7 +23,6 @@ url: http://www.test.com/
 	t.Logf("Secret:\n%+v\n%s\n", s, string(s.Bytes()))
 
 	mlOut := `somepasswd
-password: somepasswd
 url: http://www.test.com/
 username: myuser@test.com
 Test / test.com
diff --git a/internal/secrets/yaml.go b/internal/secrets/yaml.go
index 5b3bde89ee..84e175c909 100644
--- a/internal/secrets/yaml.go
+++ b/internal/secrets/yaml.go
@@ -27,10 +27,11 @@ type YAML struct {
 
 // Keys returns all keys
 func (y *YAML) Keys() []string {
-	keys := make([]string, 0, len(y.data))
+	keys := make([]string, 0, len(y.data)+1)
 	for key := range y.data {
 		keys = append(keys, key)
 	}
+	keys = append(keys, "password")
 	sort.Strings(keys)
 	return keys
 }
diff --git a/internal/secrets/yaml_test.go b/internal/secrets/yaml_test.go
index fcfb55d668..b37356af86 100644
--- a/internal/secrets/yaml_test.go
+++ b/internal/secrets/yaml_test.go
@@ -201,7 +201,7 @@ sub:
 	assert.Equal(t, "hallo", s.Get("login"))
 	assert.Equal(t, "42", s.Get("number"))
 	assert.Equal(t, "map[subentry:123]", s.Get("sub"))
-	assert.Equal(t, []string{"login", "number", "sub"}, s.Keys())
+	assert.Equal(t, []string{"login", "number", "password", "sub"}, s.Keys())
 }
 
 func TestYAMLMIME(t *testing.T) {