| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🟡 The PR description says this change points CONFORMANCE_PKG back at the registry spec and drops CONFORMANCE_PKG_SHA256 plus the fetch-and-verify steps, but the head commit (40328e7) deliberately reversed that plan — the final diff keeps the sha256 byte-pin and both fetch-and-verify steps, pointing CONFORMANCE_PKG at the registry tarball URL instead. A squash-merge message derived from the description would misstate the supply-chain posture (implying npm's weaker integrity check applies when a byte-level pin is actually in force). The workflow code itself is correct (the pinned digest matches the published alpha.10 tarball); only the PR description needs updating.
Extended reasoning...What the description claims vs. what the diff does: The PR description's bullet list says the change will "point CONFORMANCE_PKG back at the registry spec" and "drop CONFORMANCE_PKG_SHA256 and the fetch-and-verify steps that existed only to pin the tarball bytes (as the comment they came with prescribed)". The final diff does neither: CONFORMANCE_PKG is set to the registry tarball URL (https://registry.npmjs.org/@modelcontextprotocol/conformance/-/conformance-0.2.0-alpha.10.tgz), not a bare version spec, and CONFORMANCE_PKG_SHA256 plus both jobs' "Fetch and verify conformance harness" steps are retained as the core pinning mechanism.\n\nHow the mismatch arose: The PR consists of two commits. The first, 3774e8d ("ci: repin conformance harness to published 0.2.0-alpha.10"), matched the description exactly — it switched to the bare @modelcontextprotocol/conformance@0.2.0-alpha.10 registry spec and removed the sha256 pin and fetch-and-verify steps, following the instruction the old comment prescribed. The head commit 40328e7 ("ci: pin conformance harness by tarball URL + sha256, not version spec"), authored after the PR was opened, deliberately reversed that decision; its commit message explains: "Registry version specs and dist-tags are movable, so keep the fetch-and-verify machinery from the previous pin and point it at the published 0.2.0-alpha.10 registry tarball with its digest." The description was never updated to reflect the changed approach.\n\nStep-by-step proof:\n1. PR opened 2026-07-27T13:12Z with head 3774e8d; the description's bullets accurately describe that commit (registry spec, pin dropped).\n2. Commit 40328e7 pushed afterwards (14:39 +0100) reverses the approach: the new env comment block in .github/workflows/conformance.yml (lines 17–26) reads "Pinned to the published release tarball by registry URL, with CONFORMANCE_PKG_SHA256 pinning the bytes (registry version specs and dist-tags are movable; the digest is not)".\n3. The final cumulative diff therefore keeps CONFORMANCE_PKG_SHA256 (new value fd3795fd…) and both fetch-and-verify steps — directly contradicting the description's "drop" bullet.\n4. Downloading the alpha.10 tarball from the registry and hashing it confirms the pinned digest fd3795fd3f55ee7c3395710282c3f9739ec8bffe98898bd36eca0c142e2a4234 matches exactly — the code is correct.\n\nWhy it matters: The contradiction is on a security-relevant point. A reader of the description — or of a squash-merge commit message derived from it, which is the common merge mode — would conclude the byte-level pin was removed and that only npm's weaker registry integrity check now applies. In fact the opposite is true: the byte-level pin is the retained, load-bearing mechanism, chosen precisely because registry version specs are movable. Future maintainers auditing the supply-chain posture from git history would be misled.\n\nWhy nothing else prevents this: Nothing in CI validates the PR description against the diff, and a squash merge will propagate the stale text verbatim into the commit message unless it is edited at merge time.\n\nFix: Update the PR description to match the final approach — e.g. replace the two stale bullets with: "point CONFORMANCE_PKG at the published alpha.10 registry tarball URL, keeping CONFORMANCE_PKG_SHA256 and the fetch-and-verify steps as the byte-level pin (registry version specs and dist-tags are movable; the digest is not)". No code change is needed; all six CI legs pass against the pinned tarball as-is. Nit severity: description accuracy only — nothing breaks if merged as-is.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.