Skip to content

feat(pypi): support non-sha256 --hash pins and index digests - #3974

Open
abcdabcd987 wants to merge 3 commits into
bazel-contrib:mainfrom
abcdabcd987:fix-3972-non-sha256-hash-pins
Open

feat(pypi): support non-sha256 --hash pins and index digests#3974
abcdabcd987 wants to merge 3 commits into
bazel-contrib:mainfrom
abcdabcd987:fix-3972-non-sha256-hash-pins

Conversation

@abcdabcd987

@abcdabcd987 abcdabcd987 commented Jul 29, 2026

Copy link
Copy Markdown

feat(pypi): support non-sha256 --hash pins and index digests

Fixes #3972.

Previously only --hash=sha256: pins and #sha256= URL fragments were
recognized. Pins using any other hash algorithm were silently dropped: the
requirement was treated as hash-less, and in the pip fallback the hashes
were stripped from the requirement line entirely, so the artifact was no
longer verified against what was locked.

Now a distribution digest is carried through the code as one canonical
<algo>:<hex digest> string (the same shape as pip --hash and uv.lock
hash values) for any hashlib algorithm:

  • a new hash.bzl module (single hash symbol) centralizes digest
    parsing, validation and conversion to SRI;
  • parse_simpleapi_html accepts any PEP 503 #<algo>=<digest> fragment
    and keys whls/sdists by the canonical digest; sha256s_by_version
    is renamed to hashes_by_version;
  • matching pins against the index is a direct dict lookup; when the index
    only advertises a different algorithm the requirement falls back to pip
    with the pins kept in the requirement line, so nothing installs
    unverified (pip verifies sha256/384/512 pins and rejects requirement
    lines pinned only with other algorithms);
  • whl_library gains an integrity attribute, and the extension now
    always passes the digest as an SRI integrity value instead of
    sha256, so the lock entries use one format for all algorithms; a
    malformed (non-hex) sha256/384/512 digest fails loudly at extension
    evaluation instead of silently disabling download verification;
  • the lock file facts store <algo>:<digest> values (_FACT_VERSION
    bumped to v2; old facts are refetched once); this also fixes the
    latent bug where dist_filenames was written keyed by URL but read
    back keyed by digest, so stored filenames were never found;
  • uv.lock hash values pass through for any algorithm instead of being
    mangled by a sha256: string replace.

Repo names are unchanged for sha256 digests, and sha256 stays the
preferred algorithm when a direct-URL requirement has several pins.
Digests whose algorithm has no SRI form (e.g. md5) still match the index
but are downloaded without downloader-side verification, with a warning.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@abcdabcd987
abcdabcd987 marked this pull request as ready for review July 29, 2026 03:54
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@aignas aignas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this cleanup/refactor/feature addition.

Overall I am in favour of this, but to make it more maintainable, please switch to using integrity all the time and make the MODULE.bazel.lock file entries more normalized, i.e. use the same format all the time, not just when it is not sha256. If it makes sense, we could use the SRI integrity value to write to the facts if that makes the code any easier to reason about.

Also please change the PR description to be more in style of a commit message. This is too large.

Comment thread python/private/pypi/hashes.bzl Outdated

return ""

def preferred_digest(hashes):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For utility libs like this, it would be good to name the file hash.bzl and have a single exported symbol hash, where it has 3 attributes that are functions: hash.preferred_digest, hash.integrity, hash.hex_to_sri.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Renamed to hash.bzl with a single exported hash struct following the
version.bzl style: hash.preferred_digest, hash.integrity,
hash.hex_to_sri, plus hash.digest(algo, hex) — a validating constructor
for the canonical <algo>:<hex> form that everything now uses internally
(this is where the algorithm allow-list check lives) — and hash.ALGOS,
which the docstrings cross-reference.

Comment thread python/private/pypi/hub_builder.bzl Outdated

args["urls"] = [src.url]
args["sha256"] = src.sha256
if not src.sha256:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor to actually use integrity internally all of the time. This will make the code easier to maintain.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. _whl_repo no longer sets sha256 at all; it always sets
integrity = hash.integrity(src.digest), so the whl_library entries use one
format for every algorithm. hash.integrity() does fail() loudly when a
sha256/384/512 digest is not valid hex, so a corrupted pin cannot silently
disable verification.

Comment thread python/private/pypi/hub_builder.bzl Outdated

return struct(
repo_name = whl_repo_name(src.filename, src.sha256, *target_platforms),
repo_name = whl_repo_name(src.filename, src.sha256 or preferred_digest(src.hashes), *target_platforms),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here as well, lets just use preferred_digest all of the time.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, and it ended up simpler than that: src entries now carry a single
normalized digest string, so this line is just
whl_repo_name(src.filename, src.digest, ...). whl_repo_name strips the
<algo>: prefix internally, so repo names stay byte-for-byte identical for
sha256 digests. hash.preferred_digest is still used at the one place where
several pins can exist for a single artifact (direct-URL requirements with
multiple --hash options), with sha256 preferred to keep names stable.

* `srcs`: {type}`list[struct]` A list of per-distribution source entries, each
containing: `distribution`, `extra_pip_args`, `requirement_line`,
`target_platforms`, `filename`, `sha256`, `url`, `yanked`.
`target_platforms`, `filename`, `sha256`, `hashes`, `url`, `yanked`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use integrity as the name instead of hashes. In bazel-land it is more common.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with digest rather than integrity here and would like your take.
The src entries now carry one field holding the normalized
<algo>:<hex digest> value (the sha256 + hashes pair is gone). I avoided
calling it integrity because in bazel-land that name specifically means the
SRI <algo>-<base64> format accepted by ctx.download(integrity = ...), and
this field is deliberately not that: keeping the hex form makes index
matching, repo naming and lock-file diffs line up with pip's --hash and
uv.lock's hash values with zero conversions. A field named integrity that
cannot be passed to the integrity attribute seemed like a footgun. The SRI
conversion happens in exactly one place — hash.integrity(src.digest) when
building the whl_library args. Happy to rename if you still prefer
integrity.

dist_url = "{}#{}".format(dist_url, fragment)
algo = ""
digest = ""
sha256 = digest if algo == "sha256" else ""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stop setting sha256 internally in here and just use digest everywhere.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The dist structs now only carry digest (<algo>:<hex>, or empty when
the index advertises none) — the sha256 and hashes fields are gone. The
whls/sdists dicts are keyed by the same canonical value, so requirement
pins (already in that format) match with a direct dict lookup and the
prefix-stripping special case in _add_dists is deleted.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the name of attr to hashes_by_version

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — renamed here, in pypi_cache.bzl and in
simpleapi_download._with_index_url. The values are canonical
<algo>:<hex> strings (empty when the index advertises no digest, as
before), so the no-pins fallback path goes through exactly the same lookup as
pinned requirements.

Comment thread python/private/pypi/pypi_cache.bzl Outdated
dists = known_sources.setdefault("sdists", {})

known_sources.setdefault("sha256s_by_version", {}).setdefault(version, []).append(sha256)
known_sources.setdefault("sha256s_by_version", {}).setdefault(version, []).append(digest)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we should change sha256s_by_version here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, together with the rename above — no occurrences of
sha256s_by_version remain.

Comment thread python/private/pypi/pypi_cache.bzl Outdated
# "<index_url>": {
# "<last segment>": {
# "<dist url>": "<sha256>",
# # A bare sha256 hex digest or "<algo>:<digest>" when the index

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets do : all the time to simplify the code.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. dist_hashes values and dist_yanked keys are now always
<algo>:<digest> (or empty when the index advertises no digest, as before),
which deleted both the bare-sha256 legacy branch on read
and the stored-hash reconstruction loop on write. Since this changes the
stored format, I bumped _FACT_VERSION to v2 — existing facts are treated
as a cache miss and refetched/rewritten once. I considered writing SRI values
into the facts as you suggested in the review summary, but <algo>:<digest>
keeps the hex (needed for repo naming and matching) and matches pip/uv lock
formats byte-for-byte, so it avoids base64 round-trips at every boundary.

abcdabcd987 added a commit to abcdabcd987/rules_python that referenced this pull request Jul 30, 2026
…x> strings

Address review feedback from @aignas on bazel-contrib#3974:

- rename hashes.bzl to hash.bzl, exporting a single `hash` struct
  (ALGOS, digest, hex_to_sri, integrity, preferred_digest)
- carry one canonical `<algo>:<hex digest>` string per artifact instead
  of parallel `sha256` + `hashes` fields; requirement pins match the
  index via a direct dict lookup
- rename `sha256s_by_version` to `hashes_by_version` everywhere
- always pass the digest to whl_library as an SRI `integrity` value
  instead of `sha256`; fail loudly when a sha256/384/512 digest is not
  valid hex rather than silently disabling verification
- store facts as `<algo>:<digest>` unconditionally and bump
  _FACT_VERSION to v2, dropping the legacy bare-sha256 read branch

Repo names are unchanged for sha256 digests: whl_repo_name strips the
algo prefix and sha256 stays the preferred algorithm for naming.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@abcdabcd987

Copy link
Copy Markdown
Author

Thanks for the review! All addressed:

  • Everything internal now flows through a single canonical
    <algo>:<hex digest> string per artifact (the pip/uv format); the sha256
    special-casing for distribution digests is gone from
    parse_simpleapi_html, parse_requirements, pypi_cache and
    hub_builder (the PEP 714 metadata_sha256 handling is untouched).
  • whl_library repos always get the digest as an SRI integrity value now —
    sha256 is no longer set by the extension, so the lock entries use the same
    format for every algorithm.
  • Facts always store <algo>:<digest> and the fact version is bumped to v2
    (one-time refetch of the cached index info).
  • hashes.bzlhash.bzl with a single exported hash struct.
  • PR description rewritten as a commit-message-style summary.

One naming question left inline: I used digest for the <algo>:<hex> form
and kept integrity strictly for SRI values — see the
parse_requirements.bzl thread.

abcdabcd987 and others added 3 commits July 30, 2026 22:24
pip.parse only recognized --hash=sha256: pins in requirements files and
#sha256= URL fragments on Simple API pages; pins using any other
algorithm were silently dropped, so they were neither matched against
the index metadata nor verified at download time.

- Parse --hash=<algo>:<digest> for all hashlib algorithms and keep the
  pins in the reconstructed requirement line so that the pip fallback
  verifies them.
- Parse #<algo>=<digest> Simple API URL fragments per PEP 503.
- Match non-sha256 pins against the digests advertised by the index.
- Download with ctx.download(integrity = ...) when only a non-sha256
  digest is known, via a new integrity attribute on whl_library.
- Parse the uv.lock hash field algorithm instead of assuming sha256.
- Store "<algo>:<digest>" values in the lockfile facts for non-sha256
  digests, and fix the dist filename fact lookup to use the URL key the
  filenames are stored under.

Matching still requires the index to advertise a digest with the same
algorithm as the pin; support for the multi-algorithm PEP 691 hashes
dict would be a separate feature.

Fixes bazel-contrib#3972

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…x> strings

Address review feedback from @aignas on bazel-contrib#3974:

- rename hashes.bzl to hash.bzl, exporting a single `hash` struct
  (ALGOS, digest, hex_to_sri, integrity, preferred_digest)
- carry one canonical `<algo>:<hex digest>` string per artifact instead
  of parallel `sha256` + `hashes` fields; requirement pins match the
  index via a direct dict lookup
- rename `sha256s_by_version` to `hashes_by_version` everywhere
- always pass the digest to whl_library as an SRI `integrity` value
  instead of `sha256`; fail loudly when a sha256/384/512 digest is not
  valid hex rather than silently disabling verification
- store facts as `<algo>:<digest>` unconditionally and bump
  _FACT_VERSION to v2, dropping the legacy bare-sha256 read branch

Repo names are unchanged for sha256 digests: whl_repo_name strips the
algo prefix and sha256 stays the preferred algorithm for naming.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The committed lock stored pip facts in the v1 format (bare sha256 hex
values). The facts are now stored as canonical <algo>:<digest> values
under fact version v2, so the lock check aborted analysis.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@abcdabcd987
abcdabcd987 force-pushed the fix-3972-non-sha256-hash-pins branch from cb86791 to 3e3e015 Compare July 30, 2026 22:27
@abcdabcd987

Copy link
Copy Markdown
Author

CI is back to green now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pip.parse: support non-sha256 --hash pins (AWS CodeArtifact serves sha512)

2 participants