pypa / flit

Simplified packaging of Python modules
https://flit.pypa.io/
BSD 3-Clause "New" or "Revised" License
2.14k stars 130 forks source link

Avoid Environment Variable Substitution by Using RawConfigParser #652

Closed Alyetama closed 11 months ago

Alyetama commented 11 months ago

The current error occurs due to ConfigParser performing environment variable substitution, causing issues when the password contains the '%' symbol. To fix this, I replaced ConfigParser with RawConfigParser, which avoids environment variable substitution.

Found 8 files tracked in git                                        I-flit.sdist
Built sdist: dist/resu-0.1.2.tar.gz                            I-flit_core.sdist
Copying package file(s) from /var/folders/zv/q47bvs0s6rgd8mxck25__yh40000gn/T/tmpjl6_47_1/resu-0.1.2/resu  I-flit_core.wheel
Writing metadata files                                         I-flit_core.wheel
Writing the record of files                                    I-flit_core.wheel
Built wheel: dist/resu-0.1.2-py2.py3-none-any.whl              I-flit_core.wheel
Traceback (most recent call last):
  File "/Users/Alyetama/miniforge3/bin/flit", line 8, in <module>
    sys.exit(main())
  File "/Users/Alyetama/miniforge3/lib/python3.10/site-packages/flit/__init__.py", line 200, in main
    main(args.ini_file, repository, args.pypirc, formats=set(args.format or []),
  File "/Users/Alyetama/miniforge3/lib/python3.10/site-packages/flit/upload.py", line 277, in main
    do_upload(built.wheel.file, built.wheel.builder.metadata, pypirc_path, repo_name)
  File "/Users/Alyetama/miniforge3/lib/python3.10/site-packages/flit/upload.py", line 251, in do_upload
    repo = get_repository(pypirc_path, repo_name)
  File "/Users/Alyetama/miniforge3/lib/python3.10/site-packages/flit/upload.py", line 90, in get_repository
    repos_cfg = get_repositories(pypirc_path)
  File "/Users/Alyetama/miniforge3/lib/python3.10/site-packages/flit/upload.py", line 57, in get_repositories
    'password': cp.get(name, 'password', fallback=None),
  File "/Users/Alyetama/miniforge3/lib/python3.10/configparser.py", line 800, in get
    return self._interpolation.before_get(self, section, option, value,
  File "/Users/Alyetama/miniforge3/lib/python3.10/configparser.py", line 395, in before_get
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "/Users/Alyetama/miniforge3/lib/python3.10/configparser.py", line 442, in _interpolate_some
    raise InterpolationSyntaxError(
configparser.InterpolationSyntaxError: '%' must be followed by '%' or '(', found: "REDACTED PASSWORD"

Changes: Updated the code where ConfigParser was used to use RawConfigParser instead. By using RawConfigParser, the password should not be subjected to any environment variable substitution, and the issue should be resolved.

takluyver commented 11 months ago

Thanks, that makes sense, and I can't see any reason interpolation would be needed in this file. I notice, though, that the docs on RawConfigParser suggest doing it differently:

Consider using ConfigParser instead which checks types of the values to be stored internally. If you don’t want interpolation, you can use ConfigParser(interpolation=None).

Do you think that makes sense here.

Note that you can also leave your password out of this file and store it with keyring instead, which may be more secure (for some types of threats, not all).

Alyetama commented 11 months ago

Thank you for the response. I agree with you. I have updated the file to replace RawConfigParseer with ConfigParser(interpolation=None) in alignment with the documentation's recommended practice. I am aware now that you can use keyring, but would still like to fix this bug since it's one of the password storing methods mentioned in the documentation.

takluyver commented 11 months ago

Thank you!