gopasspw / gopassbridge

A web extension for Firefox and Chrome to insert login credentials from gopass
MIT License
264 stars 24 forks source link

Version check fails [after upgrade to gopass 1.10] #171

Closed andresterba closed 4 years ago

andresterba commented 4 years ago

I get the following message from the bridge after updating to gopass 1.10.1

2020-08-26-115459_grim 2020-08-26-115242_grim

Despite the message everything works just fine.

The code over here looks fine, so maybe the response from my local gopass is wrong? Can you give me any hints how to debug this as I've never done anything with browser extensions?

vdrandom commented 4 years ago

This also blocks automatic auto fill suggestions based on the uri. Having to search for it as a result is very inconvenient. Maybe it still should warn user about outdated versions without disabling this functionality?

ghost commented 4 years ago

Same, however I'm not actually using an outdated version. Not sure what happened. Maybe there was a regression in the v1.10.1 release?

kristiankostadinov commented 4 years ago

The version I am getting via the gopass json api seems to be wrong (You can use query-jsonapi.sh to try it out).

echo '{"type":"getVersion"}' | ./test-client | gopass-jsonapi listen
{"version":"0.0.0","major":0,"minor":0,"patch":0}
Pharb commented 4 years ago

Thanks a lot for the reports and suggestions.

Yes, that was an issue with the Makefile in gopass using "v" as a prefix to the version, which could not be parsed as a valid semantic version string. Therefore the fallback "0.0.0" was used, which does not pass the gopassbridge version check.

The fix is here: https://github.com/gopasspw/gopass/pull/1568/files

The official binaries from Github releases should be unaffected by this issue: https://github.com/gopasspw/gopass/releases/tag/v1.10.1

In case you compile gopass-jsonapi yourself, you can do so again from the latest master to resolve this.

ghost commented 4 years ago

I'd like to see if we can get this fixed for now in a minor version bump release and move additional changes to 1.10.3? Arch has an official package for gopass, and there were already issues with 1.10.0 that required a bump. Modifying the PKGBUILD and Makefile to compile gopass-jsonapi from master is a bit tedious and I don't think the current packager will be willing to do it, and until then anyone using this is likely to consider alternatives due to the ongoing issues.

vdrandom commented 4 years ago

I think it's up to the archlinux package maintainer to update the package. Especially considering that the official 1.10.1 is unaffected by this issue unlike the version packaged by the maintainer.

Or you can do it yourself, adding a patch to the pkgbuild script is a trivial thing to do. (It also doesn't have to be master, you or the package mantainer can just use the patch from the pull request mentioned above.)

Airblader commented 4 years ago

I'm affected by this and a bit confused at the moment.

The fix is here: https://github.com/gopasspw/gopass/pull/1568/files

So it's a bug directly in gopass(?)

The official binaries from Github releases should be unaffected by this issue:

Why wouldn't it be? Doesn't gopass build its binaries from its own build system…?

I think it's up to the archlinux package maintainer to update the package.

The PKGBUILD directly pulls the release from GitHub. I neither understand how that is not "the official 1.10.1" release nor why it would be up to the package maintainer rather than gopass putting out a patch release.

I also don't see anything in the PKGBUILD that would introduce this bug, so I don't understand how the packaging process is breaking supposedly non-broken software despite the fact that the PR to fix the problem was against that software and not the package. Clearly I must be misunderstanding something here and would be happy if someone could shed light on it :-)

vdrandom commented 4 years ago

I might have misunderstood. Never looked into that too much, just saw the mention of the problem not affecting the current version. My point anyway was not about blaming a maintainer or anything like that.

But I do think that if the upstream is not in a hurry to release a supposedly important fix, it is up to maintainer to apply the patch if it is available. The issue, in my opinion, is irritating enough to not wait for some milestone or an upstream release.

blinry commented 4 years ago

Pinging @Foxboron and @shibumi, the maintainers of Arch's gopass package! <3

tl;dr: gopass 1.10.1 needs this patch to work with the gopassbridge browser plugin.

Foxboron commented 4 years ago

Why wouldn't it be? Doesn't gopass build its binaries from its own build system…?

gopass has had issues where some commits are not part of the release tag, but included in the built binary. I have hilighted the issue before but the release engineering hasn't improved.

shibumi commented 4 years ago

@blinry interesting.. I used the gopass bridge with current gopass the last days and it worked (I could insert a pw via browser plugin). The only thing that does not work is automatically selecting the correct website.

vdrandom commented 4 years ago

The only thing that does not work is automatically selecting the correct website.

That is exactly the issue here. It is irritating, and I would consider it a bug worth fixing with an updated package even if the upstream hasn't released it yet.

martinhoefling commented 4 years ago

So the point is that on gopass-bridge side, we do check the version on activation. That replaces a potential result matching the host from the api. I would prefer to keep it that way for simplicity.

Foxboron commented 4 years ago

gopass-1.10.1-2 has the patch in Arch, but please submit on our tracker in the future and make a case as to why we should patch something upstream broke. I can't keep up with being hilighted in different issues.

Pharb commented 4 years ago

Thanks for the investigations and updates to the Arch package.

Doesn't gopass build its binaries from its own build system…?

My (limited) understanding is, that there are two ways to compile the gopass binaries, via goreleaser or using the Makefile.

The root cause for this "bug" is basically an inconsistency in how these two approaches determine the main.version parameter used for the compilation. The Makefile just reads it from the file (cat VERSION), while goreleaser is more advanced with parsing the version from the git tag (and stripping the v prefix as documented).

If somebody wants to improve this situation further, I suggest to open a proposal for changes to the gopass build process and research the different tradeoffs and reasons for maintaining these different build systems.

Airblader commented 3 years ago

I'm experiencing this again after the update to gopass 1.12.x. :-( I first had to install gopass-jsonapi separately via go (since it seems to have been split off) and re-run it, but now the version check fails once again.

I have to say (and I know this is the gopassbridge project) the gopass experience has been slightly frustrating, every update so far has caused serious breakage and there's some 4–5 repositories involved now and I don't even know which one to go to.

Airblader commented 3 years ago

@Pharb Should I open a new issue? Is there any information I can provide other than this?

$ gopass --version
gopass 1.12.1 (2021-03-02 07:19:25) go1.16 linux amd64

# gopass-jsonapi built via go get github.com/gopasspw/gopass-jsonapi
# gopassbridge 0.8.0
Pharb commented 3 years ago

@Airblader Sorry for the frustration. gopassbridge is currently lacking a bit of maintenance support, any help is greatly appreciated. Focus is on gopass core development to make sure that gopass itself can be maintained. Sideeffect is more maintenance effort necessary for goapssbridge/jsonapi, due to decoupling gopass-jsonapi as a standalone tool.

Could you please try again with the latest main branch, which should fix this version reporting again: https://github.com/gopasspw/gopass-jsonapi/pull/3

Airblader commented 3 years ago

@Pharb That PR was merged over two weeks ago, I just installed it via go get yesterday.

Pharb commented 3 years ago

@Airblader What is the output from gopass-jsonapi --version?

Airblader commented 3 years ago
$ ./gopass-jsonapi --version
Incorrect Usage. flag provided but not defined: -version

NAME:
   gopass-jsonapi - Setup and run gopass-jsonapi as native messaging hosts, e.g. for browser plugins

USAGE:
   gopass-jsonapi [global options] command [command options] [arguments...]

COMMANDS:
   configure  Setup gopass-jsonapi native messaging manifest for selected browser
   help, h    Shows a list of commands or help for one command

GLOBAL OPTIONS:
   --help, -h  show help (default: false)
2021/03/03 13:09:34 flag provided but not defined: -version

Other variants like -version, -v etc. don't change that. This is after a fresh run of

$ sudo rm -rf go/
$ go get github.com/gopasspw/gopass-jsonapi
Pharb commented 3 years ago

@Airblader Thanks for trying, I can also reproduce the issue and opened https://github.com/gopasspw/gopass-jsonapi/issues/14

I also updated the README to suggest using the Makefile to build the binary instead: https://github.com/gopasspw/gopass-jsonapi/commit/e60e6aa8e73bb8f5011d68615a5d571aab49cfc3

git clone https://github.com/gopasspw/gopass-jsonapi.git
cd gopass-jsonapi
make build
./gopass-jsonapi help

Generally it should be fine to just download the compiled binary from the latest release: https://github.com/gopasspw/gopass-jsonapi/releases/tag/v1.11.1

Airblader commented 3 years ago

@Pharb Thanks, downloading the binary from there worked!

prologic commented 2 years ago

I'm also running into this exact same problem as well. I have the very latest gopass and gopass-jsonapi installed via go install ...@latest:

$ gopass version
gopass 1.14.9-git+HEAD go1.19.2 darwin arm64
<root>     -  gpg 2.3.8 - gitfs 2.37.0
Available Crypto Backends: age, gpgcli, plain
Available Storage Backends: fossilfs, fs, gitfs

But like @Airblader pointed out there is no -v/--version flag for gopass-jsonapi so that can't be verified.

Is this a fix for this? I'm 99.9% sure I'm on the latest version of both binaries, but I suspect there's a bug still with the way the version is being checked/detected? 🤔

prologic commented 2 years ago

I think the work-around is to not install via go install as the version code checking doesn't consider additional parts of the version string like -git+HEAD I think? 🤔

So for me on macOS using Homebrew:

$ rm $GOBIN/gopass*
$ brew install gopass gopass-jsonapi