Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gopass show still affected by the MIME effect #1738

Closed
fgarcia opened this issue Jan 20, 2021 · 12 comments
Closed

gopass show still affected by the MIME effect #1738

fgarcia opened this issue Jan 20, 2021 · 12 comments
Labels
documentation ux User experience / User Interface related wontfix

Comments

@fgarcia
Copy link

fgarcia commented Jan 20, 2021

I tried the example from #1594

somepasswd
---
Test / test.com
user:myuser
url: test.com/

I've seen that since release 1.11.0 the file is no longer modified every time I edit, but if I dump the file to screen with gopass show I still get the same modifications

somepasswd
url: test.com/
user: myuser
---
Test / test.com

Expected behavior

Show should just dump the file as stored (no modifications)

Environment

  • OS:Ubuntu 20.04
  • OS version: Linux yoga 5.8.0-38-generic Support for dmenu  #43~20.04.1-Ubuntu SMP Tue Jan 12 16:39:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
  • gopass Version: 1.11.0
  • Installation method: binary from github release page
@fgarcia
Copy link
Author

fgarcia commented Jan 20, 2021

I am not sure about how gopass interprets this file

somepasswd
---
user: myuser
foo:
  - bar
  - delta
url: test.com
---
some random note

Should I assume that the first line is always the password and the rest behaves just like markdown frontmatter??

@dominikschulz
Copy link
Member

This file would be parsed as YAML with an additional non-YAML body.

@AnomalRoil
Copy link
Member

If you want to disable all formatting and not benefit from key-values and YAML parsing, you need to set the option parsing to false in gopass config.

Your first example is probably parsed as a key-value entry because of the third line which is not valid YAML.
Your second example is probably as YAML since you used the --- separator, but the second --- separator is causing the YAML parsing to stop there and the random note is not displayed. (plus it's not valid YAML).

You can use also show -n to disable the parsing on a specific show operation.

@AnomalRoil
Copy link
Member

So it would look like this:

somepasswd
---
foo:
    - bar
    - delta
url: test.com
user: myuser

We don't really support having multiple YAML separators at the moment. Not sure if we want the extra hassle, since YAML might not be the most used of the formats for secrets anyway.

@AnomalRoil
Copy link
Member

Notice the expected behaviour of show and how it is doing the parsing when parsing is set to true is detailed here:
https://github.com/gopasspw/gopass/blob/master/docs/commands/show.md#parsing-and-secrets

@fgarcia
Copy link
Author

fgarcia commented Jan 20, 2021

Ops! my bad. I was confused with the formatting (parsing) being the default. Probably I am very spoiled since Yekyll / Ruby got me used to think that all note/text files are just markdowns files with front matter in YAML.

I've just disabled parsing because I always want show to match as close as possible my formatting. However that disables the feature to read key/values. Technically I want the parsing to extract key/values and just disable the formatting

Since I do not use key/value reading often parsing: false solves my issue

Please feel free to close this issue after considering these UX concerns. I am leaving those up to the maintainers.

  1. In my example the line some random note is lost/gone from the show output. Was that expected?

  2. Not sure if there is a standard for closing YAML separators, but users from static bloggers (at least Hugo and Jekyll ) might expect that to be the most natural way to type secrets. At least it was for me.

@AnomalRoil
Copy link
Member

I guess you could work around that using a alias to alias gopass show -n with gshow or some other command that you could then use to show you secrets but still use gopass name key when you want to get the value of a key.

Otherwise just run gopass -n entry whenever you want to display the content of an entry.

@dominikschulz Not sure if we want yet another config option to disable parsing upon show but still have parsing enabled when a key is set?
Or do we change the behaviour and ignore the parsing option when a key is specified?

Yeah, the YAML not rendering what is under it is a bug IMO and it should get added to the body instead of being hidden.

@dominikschulz
Copy link
Member

Maintaining config options is very painful. Please let's try to avoid that whenever possible.

Enabling parsing whenever a key is specified sounds good. Possibly with a warning.

The content below YAML should go into the body of the YAML secret. If it doesn't it's likely a bug.

@AnomalRoil
Copy link
Member

Yeah, we are only taking the data above the --- as part of the body.

func parseBody(r *bufio.Reader) (string, error) {
var sb strings.Builder
for {
nextLine, err := r.Peek(3)
if err != nil {
if err == io.EOF {
break
}
return "", err
}
if string(nextLine) == "---" {
debug.Log("Beginning of YAML section detected")
return sb.String(), nil
}
line, err := r.ReadString('\n')
if err != nil {
if err == io.EOF {
break
}
return "", err
}
sb.WriteString(line)
}
return "", fmt.Errorf("no YAML marker")
}

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 16, 2022
@AnomalRoil
Copy link
Member

@dominikschulz Is this "fixed" by the additional documentation around our YAML support not supporting the body below and only supporting one YAML block in #2244 ?

Currently we do support:

somepasswd
some random note
---
user: myuser
foo:
  - bar
  - delta
url: test.com

but NOT:

somepasswd
---
user: myuser
foo:
  - bar
  - delta
url: test.com
---
some random note

nor multiple documents/nodes in one entry:

somepasswd
---
user: myuser
foo:
  - bar
  - delta
url: test.com
...
---
extra: yaml block
with: more key-values
...

@dominikschulz
Copy link
Member

I think we can consider it fixed, yes.

@AnomalRoil AnomalRoil added documentation ux User experience / User Interface related labels May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation ux User experience / User Interface related wontfix
Projects
None yet
Development

No branches or pull requests

3 participants