Security leak with github urls using credentials in environment variables

[cesarob]

• I have searched the issue tracker and believe that this is not a duplicate.

Steps to reproduce

I have the packages A, B and C with these dependencies: C -> B -> A. So C has A as a transitive dependency.
In the .toml file I have all the dependencies as an url like:

"A @ git+https://${GITHUB_PIP_TOKEN}@github.com/myorg/A.git@919b46ac5c7ba81fa794e1f89803b6df36dcc1fk"

Actual behavior

When generating the C pdm.lock file the $GITHUB_PIP_TOKEN value appear for the dependency A in the dependencies section of the B package.

Expected behavior

I expect the real GITHUP_PIP_TOKEN value nevers appear in a pdm.lock file.

Environment Information

# Paste the output of `pdm info && pdm info --env` below:

# pdm info && pdm info --env
PDM version:
  2.1.4
Python Interpreter:
  /usr/local/bin/python (3.9)
Project Root:
  /code
Project Packages:
  /code/__pypackages__/3.9

PDM 2.1.4 is installed, while 2.2.1 is available.
Please run `/root/.local/share/pdm/venv/bin/python -m pip install -U pdm` to upgrade.
Run `pdm config check_update false` to disable the check.
{
  "implementation_name": "cpython",
  "implementation_version": "3.9.7",
  "os_name": "posix",
  "platform_machine": "x86_64",
  "platform_release": "4.15.0-194-generic",
  "platform_system": "Linux",
  "platform_version": "#205-Ubuntu SMP Fri Sep 16 19:49:27 UTC 2022",
  "python_full_version": "3.9.7",
  "platform_python_implementation": "CPython",
  "python_version": "3.9",
  "sys_platform": "linux"
}

[frostming]

You have to list the git URL of A in the dependency list of C project. Otherwise, all transitive dependencies will be resolved by the backend and turn into static strings. We just can't keep them dynamic, because we can't assume which backend is being used by dependency packages.

[cesarob]

In fact we did and pdm with the same url detected them as different, so we removed the URL of A from C and then we spotted the reported issue. I will double check it today...

[cesarob]

I don't understand this "We just can't keep them dynamic, because we can't assume which backend is being used by dependency packages." The problem is not the usage in any backend it is that in the output we have the expanded version.

About "You have to list the git URL of A in the dependency list of C project. Otherwise, all transitive dependencies will be resolved by the backend and turn into static strings" I confirm that it doesn't work as the urls doesn't match. A expanded url is compared againts a non expanded one when in fact both refer to the same version.

I am going to paste a real example with the real outputs in my next comment.

[cesarob]

Here it goes:

***** PACKAGE A *****
Commit hash used in url: COMMIT_HASH_OF_A

***** PACKAGE B *****
Includes the irl of A in the dependencies section:

"A @ git+https://${GITHUB_PIP_TOKEN}@github.com/acme/A.git@COMMIT_HASH_OF_A",

Content of pdm.lock:

[[package]]
name = "A"
version = "0.1.2"
requires_python = ">=3.9"
git = "https://${GITHUB_PIP_TOKEN}@github.com/acme/A.git"
ref =      "COMMIT_HASH_OF_A"
revision = "COMMIT_HASH_OF_A"
summary = "..."
dependencies = [
    ...
]


***** PACKAGE C (OPTION 1) *****

Includes the previous packages with their commit hashes

dependencies = [
    "A @ git+https://${GITHUB_PIP_TOKEN}@github.com/acme/A.git@COMMIT_HASH_OF_A",
    "B @ git+https://${GITHUB_PIP_TOKEN}@github.com/acme/B.git@COMMIT_HASH_OF_B",
]

When generating the pdm.lock I get the following output:

pdm.termui:   Adding requirement common @ git+https://EXPANDED_TOKEN@github.com/acme/A.git@COMMIT_HASH_OF_A(from B 0.2.3)
pdm.termui: Conflicts detected:
  common @ git+https://${GITHUB_PIP_TOKEN}@github.com/acme/A.git@COMMIT_HASH_OF_A (from project)
  common @ git+https://EXPANDED_TOKEN@github.com/acme/A.git@COMMIT_HASH_OF_A (from <Candidate B 0.2.3 from None>)
pdm.termui: Candidate rejected: common 0.1.2
pdm.termui: Candidate rejected: python None
🔒 Lock failed
Unable to find a resolution for common
because of the following conflicts:
  common @ git+https://${GITHUB_PIP_TOKEN}@github.com/acme/A.git@COMMIT_HASH_OF_A (from project)
  common @ git+https://EXPANDED_TOKEN@github.com/acme/A.git@COMMIT_HASH_OF_A
(from <Candidate B 0.2.3 from None>)
To fix this, you could loosen the dependency version constraints in pyproject.toml. See
https://pdm.fming.dev/latest/usage/dependency//#solve-the-locking-failure for more details.
Traceback (most recent call last):
  File "/usr/local/bin/pdm", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.9/site-packages/pdm/core.py", line 254, in main
    return Core().main(args)
  File "/usr/local/lib/python3.9/site-packages/pdm/core.py", line 187, in main
    raise cast(Exception, err).with_traceback(traceback)
  File "/usr/local/lib/python3.9/site-packages/pdm/core.py", line 182, in main
    f(options.project, options)
  File "/usr/local/lib/python3.9/site-packages/pdm/cli/commands/lock.py", line 27, in handle
    actions.do_lock(
  File "/usr/local/lib/python3.9/site-packages/pdm/cli/actions.py", line 120, in do_lock
    raise ResolutionImpossible("Unable to find a resolution") from None
resolvelib.resolvers.ResolutionImpossible: Unable to find a resolution
Makefile:112: recipe for target 'pdm-lock' failed
make: *** [pdm-lock] Error 1


***** PACKAGE C (OPTION 2) *****

I remove A from the dependencies:

dependencies = [
    "B @ git+https://${GITHUB_PIP_TOKEN}@github.com/acme/B.git@COMMIT_HASH_OF_B",
]

The generation of the pdm.lock works but EXPANDED_TOKEN is leaked into the file: 

[[package]]
name = "A"
version = "0.1.2"
requires_python = ">=3.9"
git = "https://EXPANDED_TOKEN@github.com/acme/A.git"
ref = "COMMIT_HASH_OF_A"
revision = "COMMIT_HASH_OF_A"
summary = "..."
dependencies = [
    ...
]

[frostming]

don't understand this "We just can't keep them dynamic, because we can't assume which backend is being used by dependency packages." The problem is not the usage in any backend it is that in the output we have the expanded version.

The metadata in pyproject.toml, by design, is to be consumed by the backend to generate a [core metadata] for the package. And PDM's resolver will read the dependencies info from that, rather than pyprojec.toml. For a dependency, PDM doesn't need to know about the existence of project table in pyproject.toml. In fact, the variable expansion is rather tool-specific, for instance, setuptools doesn't expand any environment variables in the dependencies, and will keep them as-is, which will certainly generate metadata that isn't able to install. So of course, this problem is related to the usage of the backend.

While the resolution conflict looks like a bug, I will look into it later.

[cesarob]

Thanks for the quick fix!

[ferminho]

This is still quite a pain. We can't commit lockfiles with tokens on them for security reasons and because tokens are rotated now and then, breaking builds. We believe the options are:

• do not commit lockfiles, which defeats the purpose of lockfiles
• using an artifact repository instead of GitHub urls, which is not easy to implement
• list the B->A dep as optional

We'd love to hear about other alternatives. Thanks!