python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.72k stars 2.27k forks source link

Keyring errors during non-publishing operations #1917

Closed wahuneke closed 1 year ago

wahuneke commented 4 years ago

I gather that keyring integration was added as a convenience feature so that you can publish packages using keys stored in your keyring. This ticket describes that feature:

https://github.com/python-poetry/poetry/issues/210

But it appears that poetry tries to access the keyring even for install operations. In one of my first times using poetry - I, as a security paranoid person, decided to abort installation of a py package I wanted because the installation process seemed to be wanting to access my keyring.

I think people should be careful about granting programs access to things they should not need to access. This behavior is an example of a problem that makes that hard to achieve.

For me, I worked around the problem by pip uninstalling the 'keyring' package from that virt env.

In the future, I think it would be beneficial to poetry's adoption and to its users, if users are not prompted for keyring unless keys should actually be needed (for a 'publish' operation).

Thanks! I'm happy to be trying out poetry.

ralbertazzi commented 1 year ago

Just wanted to mention that there is at least one case for which a keyring is needed for non-publish operations, i.e. when authenticating towards Google Cloud Artifact Registry (auth is managed by a Google-provided keyring lib): https://github.com/GoogleCloudPlatform/artifact-registry-python-tools/issues/17#issuecomment-1040309388

remram44 commented 1 year ago

Either way, there is nothing stopping Poetry for getting credentials once it hits HTTP 401, rather than right away.

ralbertazzi commented 1 year ago

We just merged a small improvement that prevents calling the keyring when credentials are passed through config and/or environment variables (or a mixture of them). See https://github.com/python-poetry/poetry/issues/1917#issuecomment-608034129 and https://github.com/python-poetry/poetry/issues/1917#issuecomment-616171383 that now should be solved. It should appear in the next Poetry release

raxod502 commented 1 year ago

That's great! Just to be clear, though, that's a separate bug and doesn't solve for the fundamental issue that Poetry attempts to invoke the keyring when performing an operation that does not require it (such as installing from an unauthenticated registry), right?

SLLau commented 1 year ago

Ok I solved it for my poetry inside windows machine without any GUI.

python -m pip install keyring
python -m keyring --disable

I tried to use PYTHON_KEYRING_BACKEND but it does not work for me because I had no installed keyring. Poetry need to avoid to use keyring. This is a huge problem for Windows and Linux especially when you want to use it inside Docker/Qemu

Enjoy!

thanks @GH0st3rs for https://github.com/python-poetry/poetry/issues/1917#issuecomment-1503657969 tried and works in linux (Raspbian Debian GNU/Linux 11 (bullseye) Linux 5.15.84-v8+ aarch64 with Python 3.9.16)
poetry --version == 1.4.2 python -m pip list | grep pip == 23.1.2

pyenv local 3.9.16
poetry init
poetry env use $(pyenv which python)
poetry add --group dev black -vvv # ❌  stalls with keyring,backend ☹️
poetry run python -m pip install keyring
poetry run python -m keyring --disable
poetry add --group dev black # ✅ INSTALLs with no ISSUE 😁
stur86 commented 1 year ago

Either way, there is nothing stopping Poetry for getting credentials once it hits HTTP 401, rather than right away.

Or trying to get the credentials, catching any error, then trying to connect with or without credentials depending on whether it worked, and then only and only if it got an HTTP 401 and had no keyring raising an error which complains about it.

13steinj commented 1 year ago

Hey, maybe I'm just a bit off in the head here but this appears to be a 2 year old issue on something that can occur on a fair number of configurations... can we at least link this issue directly in the (verbose?) output? It's fairly non-descript, and this kind of thing is what pushes people away from otherwise great software (see various linked issues / comments / commits in this thread). Current verbose output end:

   1  .local/share/pypoetry/venv/lib/python3.11/site-packages/keyring/backends/SecretService.py:112 in get_credential
       110│         scheme = self.schemes[self.scheme]
       111│         query = self._query(service, username)
     → 112│         collection = self.get_preferred_collection()
       113│ 
       114│         with closing(collection.connection):

  KeyringLocked

  Failed to unlock the collection!

  at .local/share/pypoetry/venv/lib/python3.11/site-packages/keyring/backends/SecretService.py:67 in get_preferred_collection
       63│             raise InitError("Failed to create the collection: %s." % e)
       64│         if collection.is_locked():
       65│             collection.unlock()
       66│             if collection.is_locked():  # User dismissed the prompt
    →  67│                 raise KeyringLocked("Failed to unlock the collection!")
       68│         return collection
       69│ 
       70│     def unlock(self, item):
       71│         if hasattr(item, 'unlock'):

The mere fact that this is accessing the keyring, with this verbose/debug output on a poetry self update would make some people (myself included) believe that they accidentally downloaded some malware that was trying to access the keyring. Now, maybe I'm just not that bright; but I'm sure there are people at organizations somewhere posting this output in their slack channels and overzealous "security" professionals jump to assuming this was malware as well and would take swift, overzealous action.

Personally this seems to have happened to me because my organization decided to set up a box with a default password that I was to change later but it is non-trivial to replicate such password changes to the login keyring auto-magically. And as the top comment says, it occurs even on a poetry self update).

stur86 commented 1 year ago

Agreed this is a big pain and close to an app killer. It's also generally an obstacle to adoption. In my org we shifted to using Poetry but now I hesitate recommending it to people for new projects solely because of this and I have now to remind everyone to do the whole export song and dance every time this pops up, which is way too often. It confuses and scares away anyone who's not a big more of an experienced user, and it wastes everyone's time. Seriously, decide something and fix the problem already somehow, but do it quickly; this is the kind of stuff that kills a software's momentum.

viteski commented 1 year ago

you can use this in your dev environment to work around the issue on Debian/Ubuntu machines

function unlock_keyring() {
    export "$(echo -n "$(pass)" | gnome-keyring-daemon --replace --unlock)"
    unset "$(pass)"
}

you'll have to apt install the gnome-keyring package gnome-keyring-pkcs11/jammy-updates,now 40.0-3ubuntu3 amd64 [installed,automatic] gnome-keyring/jammy-updates,now 40.0-3ubuntu3 amd64 [installed,automatic] libpam-gnome-keyring/jammy-updates,now 40.0-3ubuntu3 amd64 [installed,automatic]

But, it is troublesome.

bgyarfas commented 1 year ago

Another workaround in the case you don't need the keyring at all

python3 -m keyring --disable

If you are also running pyenv you'll need to run this with the system python

pyenv shell system
python3 -m keyring --disable
vipcxj commented 1 year ago

The same issue for the latest version. A big bong bug not fixed for 3 years?

ericriff commented 1 year ago

I'm just dealing with this myself, on the latest version (1.6.1). This workaround worked for me

python3 -m keyring --disable

but it is not ideal. I handle the environment of many devs on a company, I can't ask them all to run this command.

13steinj commented 1 year ago

The same issue for the latest version. A big bong bug not fixed for 3 years?

Because

In fairness to them, it doesn't happen in most desktop environments. However companies in various fashion have moved many devs to "use an ssh terminal and be stuck with it." or something of the like. Which is sometimes a necessity, but isn't what open source devs "target" in terms of usability testing (if they do usability testing at all).

Similarly in fairness, after doing some evil hack to avoid this issue once, it's not a recurring problem.

remram44 commented 1 year ago

"this is open source software" is a terrible excuse, the community has provided many patches that have all been ignored.

13steinj commented 1 year ago

is a terrible excuse, the community has provided many patches that have all been ignored.

I don't disagree, it's just the excuse always given to me by every project I've made a similar comment on :upside_down_face:

It may be possible that the maintainers don't have enough time to go through the issues / prs at this point (there are quite a few, after all). It may be possible that they've stopped caring. It may be possible that they don't accept the patches made in various fashion.

In fact, yours, explicitly was asked for tests and it doesn't appear as if you've written them; so, bit odd to complain about ignored patches. And in that there is a reference (without response from the author) about #6612 which has been superseded (and relatively recent) by #8078

I can understand the frustration, but it's not as if things aren't moving and it's not as if the fault is purely on maintainers for not merging any one person's PR.

dimbleby commented 1 year ago

for what it's worth there have been a couple of abandoned tries at this (I know of #6612, #7037 - neither of which were ignored) - but #8227 looks to me as though it's pretty close. So with any luck that'll go in soon

moberst commented 1 year ago

Just came across this multi-year-old issue for the first time only to find that it was just merged into the master branch. Hurray!

For anyone else in a similar position who comes across this issue and is looking for the fix, you can install the current master branch via the following (taken from the installation docs here). Looking forward to this being in the stable version.

curl -sSL https://install.python-poetry.org | python3 - --git https://github.com/python-poetry/poetry.git@master

simaotwx commented 1 year ago

For now I'm using the workaround mentioned before

python3 -m keyring --disable

Bad timing that I just got to use poetry for the first time and got hit by a flood of error messages.

Looks like it has been fixed so I hope this will not be an issue in the future. IMO it should not hard-fail since the keyring is not necessary to install packages.

arturtoshev commented 1 year ago

Installing the latest version from the master branch as suggested above did the job yesterday, but I now ran into poetry install getting stuck halfway through.

What I found to really solve the problem for me was clearing the cache:

curl -sSL https://install.python-poetry.org | python3 - --uninstall  # uninstall poetry
rm -r $HOME/.cache/pypoetry  # clear cache
curl -sSL https://install.python-poetry.org | python3 -  # install latest release

Could someone add clearing the cache as an option during uninstall or at least mention it in the documentation?

ilyagr commented 1 year ago

I installed poetry 1.7.0, and I now have poetry install and poetry lock hang forever.

Removing the cache as suggested by the previous comment did not help.

The workaround of export PYTHON_KEYRING_BACKEND=keyring.backends.fail.Keyring still works and solves the problem.

See some more details in https://gist.github.com/ilyagr/1bebabebc93e6662d139009d19ed8c1b; I'll probably file a proper bug later. Update: Filed https://github.com/python-poetry/poetry/issues/8623.

github-actions[bot] commented 8 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.