From 4ef5b6a5b3a6b304e7a780c63fe62edc4cd3c154 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:20:31 -0500 Subject: [PATCH] Revert storing `indexes` and `find_links` in lockfile headers due to security (#16575) Indexes might have passwords embedded in them. We document that you should use env var interpolation for that: https://github.com/pantsbuild/pants/blob/f425d233315cf9e2b9be420c7b2652bb123d35f2/docs/markdown/Python/python/python-third-party-dependencies.md?plain=1#L504-L525 But, by the time `lockfile_metadata.py` sees the index, interpolation already happens, so we will write the secrets to the file. https://github.com/pantsbuild/pants/issues/16576 tracks restoring this support in a more secure way. In the meantime, it is not a huge deal to take away this tracking. The main risk is that users don't realize they need to regenerate their lockfile when they should. [ci skip-rust] [ci skip-build-wheels] --- .../pants/backend/python/goals/lockfile.py | 2 -- .../backend/python/goals/lockfile_test.py | 5 ---- .../python/util_rules/lockfile_metadata.py | 28 ------------------- .../util_rules/lockfile_metadata_test.py | 22 --------------- .../python/util_rules/pex_requirements.py | 17 ----------- .../util_rules/pex_requirements_test.py | 18 ++---------- .../backend/python/util_rules/pex_test.py | 2 -- .../core/goals/update_build_files_test.py | 2 -- 8 files changed, 3 insertions(+), 93 deletions(-) diff --git a/src/python/pants/backend/python/goals/lockfile.py b/src/python/pants/backend/python/goals/lockfile.py index eaf323d94df..d15fef2d6fd 100644 --- a/src/python/pants/backend/python/goals/lockfile.py +++ b/src/python/pants/backend/python/goals/lockfile.py @@ -309,8 +309,6 @@ async def generate_lockfile( ) for i in req.requirements }, - indexes=set(pip_args_setup.resolve_config.indexes), - find_links=set(pip_args_setup.resolve_config.find_links), manylinux=pip_args_setup.resolve_config.manylinux, requirement_constraints=( set(pip_args_setup.resolve_config.constraints_file.constraints) diff --git a/src/python/pants/backend/python/goals/lockfile_test.py b/src/python/pants/backend/python/goals/lockfile_test.py index 55f9fa1d918..ad59c6e42cb 100644 --- a/src/python/pants/backend/python/goals/lockfile_test.py +++ b/src/python/pants/backend/python/goals/lockfile_test.py @@ -14,7 +14,6 @@ ) from pants.backend.python.goals.lockfile import rules as lockfile_rules from pants.backend.python.goals.lockfile import setup_user_lockfile_requests -from pants.backend.python.subsystems.repos import PythonRepos from pants.backend.python.subsystems.setup import RESOLVE_OPTION_KEY__DEFAULT, PythonSetup from pants.backend.python.target_types import PythonRequirementTarget from pants.backend.python.util_rules import pex @@ -80,10 +79,6 @@ def _generate( // "generated_with_requirements": [ // "ansicolors{ansicolors_version}" // ], - // "indexes": [ - // "{PythonRepos.pypi_index}" - // ], - // "find_links": [], // "manylinux": "manylinux2014", """ ) diff --git a/src/python/pants/backend/python/util_rules/lockfile_metadata.py b/src/python/pants/backend/python/util_rules/lockfile_metadata.py index fd5b793025e..f614b12dd8d 100644 --- a/src/python/pants/backend/python/util_rules/lockfile_metadata.py +++ b/src/python/pants/backend/python/util_rules/lockfile_metadata.py @@ -24,8 +24,6 @@ class InvalidPythonLockfileReason(Enum): INVALIDATION_DIGEST_MISMATCH = "invalidation_digest_mismatch" INTERPRETER_CONSTRAINTS_MISMATCH = "interpreter_constraints_mismatch" REQUIREMENTS_MISMATCH = "requirements_mismatch" - INDEXES_MISMATCH = "indexes_mismatch" - FIND_LINKS_MISMATCH = "find_links_mismatch" MANYLINUX_MISMATCH = "manylinux_mismatch" CONSTRAINTS_FILE_MISMATCH = "constraints_file_mismatch" ONLY_BINARY_MISMATCH = "only_binary_mismatch" @@ -44,8 +42,6 @@ def new( *, valid_for_interpreter_constraints: InterpreterConstraints, requirements: set[PipRequirement], - indexes: set[str], - find_links: set[str], manylinux: str | None, requirement_constraints: set[PipRequirement], only_binary: set[PipRequirement], @@ -62,8 +58,6 @@ def new( return PythonLockfileMetadataV3( valid_for_interpreter_constraints, requirements, - indexes=indexes, - find_links=find_links, manylinux=manylinux, requirement_constraints=requirement_constraints, only_binary=only_binary, @@ -87,8 +81,6 @@ def is_valid_for( user_interpreter_constraints: InterpreterConstraints, interpreter_universe: Iterable[str], user_requirements: Iterable[PipRequirement], - indexes: Iterable[str], - find_links: Iterable[str], manylinux: str | None, requirement_constraints: Iterable[PipRequirement], only_binary: Iterable[PipRequirement], @@ -136,8 +128,6 @@ def is_valid_for( interpreter_universe: Iterable[str], # Everything below is not used by v1. user_requirements: Iterable[PipRequirement], - indexes: Iterable[str], - find_links: Iterable[str], manylinux: str | None, requirement_constraints: Iterable[PipRequirement], only_binary: Iterable[PipRequirement], @@ -208,8 +198,6 @@ def is_valid_for( interpreter_universe: Iterable[str], user_requirements: Iterable[PipRequirement], # Everything below is not used by V2. - indexes: Iterable[str], - find_links: Iterable[str], manylinux: str | None, requirement_constraints: Iterable[PipRequirement], only_binary: Iterable[PipRequirement], @@ -238,8 +226,6 @@ def is_valid_for( class PythonLockfileMetadataV3(PythonLockfileMetadataV2): """Lockfile version that considers constraints files.""" - indexes: set[str] - find_links: set[str] manylinux: str | None requirement_constraints: set[PipRequirement] only_binary: set[PipRequirement] @@ -254,8 +240,6 @@ def _from_json_dict( ) -> PythonLockfileMetadataV3: v2_metadata = super()._from_json_dict(json_dict, lockfile_description, error_suffix) metadata = _get_metadata(json_dict, lockfile_description, error_suffix) - indexes = metadata("indexes", Set[str], lambda l: set(l)) - find_links = metadata("find_links", Set[str], lambda l: set(l)) manylinux = metadata("manylinux", str, lambda l: l) # type: ignore[no-any-return] requirement_constraints = metadata( "requirement_constraints", @@ -282,8 +266,6 @@ def _from_json_dict( return PythonLockfileMetadataV3( valid_for_interpreter_constraints=v2_metadata.valid_for_interpreter_constraints, requirements=v2_metadata.requirements, - indexes=indexes, - find_links=find_links, manylinux=manylinux, requirement_constraints=requirement_constraints, only_binary=only_binary, @@ -294,8 +276,6 @@ def _from_json_dict( def additional_header_attrs(cls, instance: LockfileMetadata) -> dict[Any, Any]: instance = cast(PythonLockfileMetadataV3, instance) return { - "indexes": sorted(instance.indexes), - "find_links": sorted(instance.find_links), "manylinux": instance.manylinux, "requirement_constraints": sorted(str(i) for i in instance.requirement_constraints), "only_binary": sorted(str(i) for i in instance.only_binary), @@ -310,8 +290,6 @@ def is_valid_for( user_interpreter_constraints: InterpreterConstraints, interpreter_universe: Iterable[str], user_requirements: Iterable[PipRequirement], - indexes: Iterable[str], - find_links: Iterable[str], manylinux: str | None, requirement_constraints: Iterable[PipRequirement], only_binary: Iterable[PipRequirement], @@ -325,8 +303,6 @@ def is_valid_for( user_interpreter_constraints=user_interpreter_constraints, interpreter_universe=interpreter_universe, user_requirements=user_requirements, - indexes=indexes, - find_links=find_links, manylinux=manylinux, requirement_constraints=requirement_constraints, only_binary=only_binary, @@ -335,10 +311,6 @@ def is_valid_for( .failure_reasons ) - if self.indexes != set(indexes): - failure_reasons.add(InvalidPythonLockfileReason.INDEXES_MISMATCH) - if self.find_links != set(find_links): - failure_reasons.add(InvalidPythonLockfileReason.FIND_LINKS_MISMATCH) if self.manylinux != manylinux: failure_reasons.add(InvalidPythonLockfileReason.MANYLINUX_MISMATCH) if self.requirement_constraints != set(requirement_constraints): diff --git a/src/python/pants/backend/python/util_rules/lockfile_metadata_test.py b/src/python/pants/backend/python/util_rules/lockfile_metadata_test.py index 28bb3163cdb..c2a8ce5e753 100644 --- a/src/python/pants/backend/python/util_rules/lockfile_metadata_test.py +++ b/src/python/pants/backend/python/util_rules/lockfile_metadata_test.py @@ -32,8 +32,6 @@ def test_metadata_header_round_trip() -> None: ["CPython==2.7.*", "PyPy", "CPython>=3.6,<4,!=3.7.*"] ), requirements=reqset("ansicolors==0.1.0"), - indexes={"index"}, - find_links={"find-links"}, manylinux="manylinux2014", requirement_constraints={PipRequirement.parse("constraint")}, only_binary={PipRequirement.parse("bdist")}, @@ -67,12 +65,6 @@ def test_add_header_to_lockfile() -> None: # "generated_with_requirements": [ # "ansicolors==0.1.0" # ], -# "indexes": [ -# "index" -# ], -# "find_links": [ -# "find-links" -# ], # "manylinux": null, # "requirement_constraints": [ # "constraint" @@ -95,8 +87,6 @@ def line_by_line(b: bytes) -> list[bytes]: metadata = PythonLockfileMetadata.new( valid_for_interpreter_constraints=InterpreterConstraints([">=3.7"]), requirements=reqset("ansicolors==0.1.0"), - indexes={"index"}, - find_links={"find-links"}, manylinux=None, requirement_constraints={PipRequirement.parse("constraint")}, only_binary={PipRequirement.parse("bdist")}, @@ -183,8 +173,6 @@ def test_is_valid_for_v1(user_digest, expected_digest, user_ic, expected_ic, mat user_interpreter_constraints=InterpreterConstraints(user_ic), interpreter_universe=INTERPRETER_UNIVERSE, user_requirements=set(), - indexes=set(), - find_links=set(), manylinux=None, requirement_constraints=set(), only_binary=set(), @@ -264,8 +252,6 @@ def test_is_valid_for_interpreter_constraints_and_requirements( PythonLockfileMetadataV3( InterpreterConstraints(lock_ics), reqset(*lock_reqs), - indexes=set(), - find_links=set(), manylinux=None, requirement_constraints=set(), only_binary=set(), @@ -278,8 +264,6 @@ def test_is_valid_for_interpreter_constraints_and_requirements( user_interpreter_constraints=InterpreterConstraints(user_ics), interpreter_universe=INTERPRETER_UNIVERSE, user_requirements=reqset(*user_reqs), - indexes=set(), - find_links=set(), manylinux=None, requirement_constraints=set(), only_binary=set(), @@ -294,8 +278,6 @@ def test_is_valid_for_v3_metadata(is_tool: bool) -> None: InterpreterConstraints([]), reqset(), # Everything below is new to v3+. - indexes={"index"}, - find_links={"find-links"}, manylinux=None, requirement_constraints={PipRequirement.parse("c1")}, only_binary={PipRequirement.parse("bdist")}, @@ -306,8 +288,6 @@ def test_is_valid_for_v3_metadata(is_tool: bool) -> None: user_interpreter_constraints=InterpreterConstraints([]), interpreter_universe=INTERPRETER_UNIVERSE, user_requirements=reqset(), - indexes={"different-index"}, - find_links={"different-find-links"}, manylinux="manylinux2014", requirement_constraints={PipRequirement.parse("c2")}, only_binary={PipRequirement.parse("not-bdist")}, @@ -317,7 +297,5 @@ def test_is_valid_for_v3_metadata(is_tool: bool) -> None: InvalidPythonLockfileReason.CONSTRAINTS_FILE_MISMATCH, InvalidPythonLockfileReason.ONLY_BINARY_MISMATCH, InvalidPythonLockfileReason.NO_BINARY_MISMATCH, - InvalidPythonLockfileReason.INDEXES_MISMATCH, - InvalidPythonLockfileReason.FIND_LINKS_MISMATCH, InvalidPythonLockfileReason.MANYLINUX_MISMATCH, } diff --git a/src/python/pants/backend/python/util_rules/pex_requirements.py b/src/python/pants/backend/python/util_rules/pex_requirements.py index da152cdb1ed..5a5a88ff814 100644 --- a/src/python/pants/backend/python/util_rules/pex_requirements.py +++ b/src/python/pants/backend/python/util_rules/pex_requirements.py @@ -449,8 +449,6 @@ def validate_metadata( user_interpreter_constraints=interpreter_constraints, interpreter_universe=python_setup.interpreter_versions_universe, user_requirements=user_requirements, - indexes=resolve_config.indexes, - find_links=resolve_config.find_links, manylinux=resolve_config.manylinux, requirement_constraints=( resolve_config.constraints_file.constraints @@ -513,21 +511,6 @@ def _common_failure_reasons( `[python].no_binary`) """ ) - if InvalidPythonLockfileReason.INDEXES_MISMATCH in failure_reasons: - yield softwrap( - """ - - The `indexes` arguments have changed from when the lockfile was generated. - (Indexes are set via the option `[python-repos].indexes`) - """ - ) - if InvalidPythonLockfileReason.FIND_LINKS_MISMATCH in failure_reasons: - yield softwrap( - """ - - The `find_links` arguments have changed from when the lockfile was generated. - (Find links is set via the option `[python-repos].find_links` or the deprecated - `[python-repos].repos`) - """ - ) if InvalidPythonLockfileReason.MANYLINUX_MISMATCH in failure_reasons: yield softwrap( """ diff --git a/src/python/pants/backend/python/util_rules/pex_requirements_test.py b/src/python/pants/backend/python/util_rules/pex_requirements_test.py index dda633d3e2a..36e60301ee9 100644 --- a/src/python/pants/backend/python/util_rules/pex_requirements_test.py +++ b/src/python/pants/backend/python/util_rules/pex_requirements_test.py @@ -31,8 +31,6 @@ METADATA = PythonLockfileMetadataV3( InterpreterConstraints(["==3.8.*"]), {PipRequirement.parse("ansicolors"), PipRequirement.parse("requests")}, - indexes={"index"}, - find_links={"find-links"}, manylinux=None, requirement_constraints={PipRequirement.parse("abc")}, only_binary={PipRequirement.parse("bdist")}, @@ -197,7 +195,7 @@ def contains(msg: str, if_: bool) -> None: @pytest.mark.parametrize( ( "invalid_reqs,invalid_interpreter_constraints,invalid_constraints_file,invalid_only_binary," - + "invalid_no_binary,invalid_indexes,invalid_find_links,invalid_manylinux" + + "invalid_no_binary,invalid_manylinux" ), [ ( @@ -206,8 +204,6 @@ def contains(msg: str, if_: bool) -> None: invalid_constraints_file, invalid_only_binary, invalid_no_binary, - invalid_indexes, - invalid_find_links, invalid_manylinux, ) for invalid_reqs in (True, False) @@ -215,8 +211,6 @@ def contains(msg: str, if_: bool) -> None: for invalid_constraints_file in (True, False) for invalid_only_binary in (True, False) for invalid_no_binary in (True, False) - for invalid_indexes in (True, False) - for invalid_find_links in (True, False) for invalid_manylinux in (True, False) if ( invalid_reqs @@ -224,8 +218,6 @@ def contains(msg: str, if_: bool) -> None: or invalid_constraints_file or invalid_only_binary or invalid_no_binary - or invalid_indexes - or invalid_find_links or invalid_manylinux ) ], @@ -236,8 +228,6 @@ def test_validate_user_lockfiles( invalid_constraints_file: bool, invalid_only_binary: bool, invalid_no_binary: bool, - invalid_indexes: bool, - invalid_find_links: bool, invalid_manylinux: bool, caplog, ) -> None: @@ -267,8 +257,8 @@ def test_validate_user_lockfiles( req_strings, create_python_setup(InvalidLockfileBehavior.warn), ResolvePexConfig( - indexes=("bad-index" if invalid_indexes else "index",), - find_links=("bad-find-links" if invalid_find_links else "find-links",), + indexes=(), + find_links=(), manylinux="not-manylinux" if invalid_manylinux else None, constraints_file=ResolvePexConstraintsFile( EMPTY_DIGEST, @@ -300,8 +290,6 @@ def contains(msg: str, if_: bool = True) -> None: contains("The constraints file at c.txt has changed", if_=invalid_constraints_file) contains("The `only_binary` arguments have changed", if_=invalid_only_binary) contains("The `no_binary` arguments have changed", if_=invalid_no_binary) - contains("The `indexes` arguments have changed", if_=invalid_indexes) - contains("The `find_links` arguments have changed", if_=invalid_find_links) contains("The `manylinux` argument has changed", if_=invalid_manylinux) contains("./pants generate-lockfiles --resolve=a`") diff --git a/src/python/pants/backend/python/util_rules/pex_test.py b/src/python/pants/backend/python/util_rules/pex_test.py index edd0cba6257..e12d67479fc 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -754,8 +754,6 @@ def test_lockfile_validation(rule_runner: RuleRunner) -> None: requirement_constraints=set(), only_binary=set(), no_binary=set(), - indexes=set(), - find_links=set(), manylinux=None, ).add_header_to_lockfile(b"", regenerate_command="regen", delimeter="#") rule_runner.write_files({"lock.txt": lock_content.decode()}) diff --git a/src/python/pants/core/goals/update_build_files_test.py b/src/python/pants/core/goals/update_build_files_test.py index 9a34b03035b..7599ff224a9 100644 --- a/src/python/pants/core/goals/update_build_files_test.py +++ b/src/python/pants/core/goals/update_build_files_test.py @@ -148,8 +148,6 @@ def test_find_python_interpreter_constraints_from_lockfile() -> None: requirement_constraints=set(), only_binary=set(), no_binary=set(), - indexes=set(), - find_links=set(), manylinux=None, )