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

Avoid Environment Variable Substitution by Using RawConfigParser #652

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

Alyetama
Copy link
Contributor

@Alyetama Alyetama commented Aug 2, 2023

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.

Replace ConfigParser with RawConfigParser to avoid environment variable substitution.
@takluyver
Copy link
Member

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).

Switch to configparser.ConfigParser(interpolation=None) instead of legacy RawConfigParser.
@Alyetama
Copy link
Contributor Author

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 takluyver added the bug label Aug 15, 2023
@takluyver takluyver added this to the 3.10 milestone Aug 15, 2023
@takluyver
Copy link
Member

Thank you!

@takluyver takluyver merged commit d8cc776 into pypa:main Aug 15, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants