Skip to content

Commit

Permalink
Revert storing indexes and find_links in lockfile headers due to …
Browse files Browse the repository at this point in the history
…security (pantsbuild#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. 

pantsbuild#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]
  • Loading branch information
Eric-Arellano authored Aug 19, 2022
1 parent 9a6e8d3 commit 4ef5b6a
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 93 deletions.
2 changes: 0 additions & 2 deletions src/python/pants/backend/python/goals/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions src/python/pants/backend/python/goals/lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -80,10 +79,6 @@ def _generate(
// "generated_with_requirements": [
// "ansicolors{ansicolors_version}"
// ],
// "indexes": [
// "{PythonRepos.pypi_index}"
// ],
// "find_links": [],
// "manylinux": "manylinux2014",
"""
)
Expand Down
28 changes: 0 additions & 28 deletions src/python/pants/backend/python/util_rules/lockfile_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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],
Expand All @@ -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,
Expand All @@ -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],
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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]
Expand All @@ -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",
Expand All @@ -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,
Expand All @@ -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),
Expand All @@ -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],
Expand All @@ -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,
Expand All @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
Expand Down Expand Up @@ -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"
Expand All @@ -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")},
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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")},
Expand All @@ -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")},
Expand All @@ -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,
}
17 changes: 0 additions & 17 deletions src/python/pants/backend/python/util_rules/pex_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
Expand Down Expand Up @@ -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"
),
[
(
Expand All @@ -206,26 +204,20 @@ 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)
for invalid_interpreter_constraints in (True, False)
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
or invalid_interpreter_constraints
or invalid_constraints_file
or invalid_only_binary
or invalid_no_binary
or invalid_indexes
or invalid_find_links
or invalid_manylinux
)
],
Expand All @@ -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:
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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`")
Expand Down
2 changes: 0 additions & 2 deletions src/python/pants/backend/python/util_rules/pex_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()})
Expand Down
2 changes: 0 additions & 2 deletions src/python/pants/core/goals/update_build_files_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand Down

0 comments on commit 4ef5b6a

Please sign in to comment.