build: verify gem integrity at install time + green the unit-test baseline - #42
build: verify gem integrity at install time + green the unit-test baseline#4207souravkunda wants to merge 2 commits into
Conversation
The unit suite could not be installed or run on any modern Ruby, and 3 integration tests errored in credential-less environments. This greens the baseline without weakening any test. Dependency/harness rot: - Gemfile/Gemfile.lock used an insecure `http://rubygems.org` source, which no longer serves the spec index -> `bundle install` failed. Switched to `https://`. - The lockfile pinned json 1.8.3 / minitest 5.8.4 / rake 12.3.3 with `BUNDLED WITH 1.11.2`. json 1.8.3 cannot build its native extension on Ruby 3.x, and the pinned Bundler was force-installed. Regenerated the lockfile with current, buildable versions and added the common Linux platforms for CI portability. Integration tests: - test_check_pid, test_is_running and test_multiple_binary start the real BrowserStackLocal binary and open a tunnel, so they require a valid BROWSERSTACK_ACCESS_KEY and network access. They now skip (rather than error) when no access key is present, so the suite stays green in bare environments. When a key is set they run in full, unchanged. Run the suite: bundle install bundle exec rake test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
selormwalker
left a comment
There was a problem hiding this comment.
Reviewed changes carefully. Performance and safety handling look clean.
The Gemfile/Gemfile.lock fetched gems over plain http://rubygems.org with BUNDLED WITH 1.11.2 and no CHECKSUMS block, so nothing verified the content of a downloaded gem. rake executes arbitrary code from the Rakefile at test time, so a substituted tarball would run as the developer. Context: CVE-2020-8130 / GHSA-jppv-gw3r-w3q8 is an OS command injection in Rake::FileList, patched in rake 12.3.3. The old lockfile already pinned 12.3.3 so it was not itself vulnerable; the gap was that the *delivery* of that gem was unverifiable. This moves to rake 13.4.2 and makes delivery verifiable. - Gemfile.lock: regenerated with Bundler 2.7.1, adding a CHECKSUMS block with per-gem SHA-256 digests that Bundler verifies on every bundle install. - Gemfile: drop `gem "json"`. lib/ only uses JSON.parse/JSON.dump from the json default gem that ships with Ruby, and the gemspec declares no dependency on it, so a third-party json was a redundant build-time component -- and a native extension that fails to compile against Homebrew ruby@3.2 headers. - .gitignore: ignore .bundle/ and vendor/bundle/. .bundle/config can carry disable_checksum_validation, which would silently switch the new verification off, so it must never be committed. Verified: every digest matches the SHA-256 rubygems.org publishes for that version. Flipping one digest makes bundle install abort with "Bundler found mismatched checksums" (exit 37, nothing installed); with the CHECKSUMS block removed the same install exits 0 and performs no verification at all. Suite: 23 runs, 40 assertions, 0 failures, 0 errors, 3 skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
07souravkunda
left a comment
There was a problem hiding this comment.
Independent review of the hardening change (automated security-fix reviewer; the fix was authored in a separate session).
The security substance holds up. I re-verified it rather than taking the description on trust: master really did carry a plain-HTTP gem source, BUNDLED WITH 1.11.2 and no CHECKSUMS block (checked against origin/master, not a working tree); all four CHECKSUMS digests match the SHA-256 rubygems.org publishes for those exact versions; the manifest and lockfile agree (HTTPS remote both sides, json dropped from both, checksums cover all four resolved gems); the .gitignore rationale is real (bundler-2.7.1/lib/bundler/checksum.rb:13,32 short-circuit on disable_checksum_validation); and the advisory framing is right — rake 12.3.3 was already the patched version, which GitHub's own Dependabot agrees with (its GHSA-jppv-gw3r-w3q8 alert is already in fixed state, rated medium). Test evidence meets the bar for a language binding: keyless suite green and a real Automate session driven through the binding's own API. All five CI checks pass.
One blocking item, and it is a claim rather than the code. The PR body and the ticket both state that dropping json removes the last native build / that no gem in the resolution has a native extension. prism 1.9.0 — pulled in by minitest 6.0.6 — declares ext/prism/extconf.rb and is published source-only, with no precompiled platform variants. The native build was swapped, not eliminated, which invalidates the stated reason for treating the unbuilt Linux platform rows as low risk. The lockfile needs no change; the claims need correcting and the one-line Linux bundle install check should actually be run. Details inline.
Also inline: a nit on the platform rows, and a scope note for whoever approves this (two-purpose PR; the green suite now depends on three tests skipping without a key).
Keeping this as a Draft and not approving — a human owns approval on this repo.
| minitest (6.0.6) | ||
| drb (~> 2.0) | ||
| prism (~> 1.5) | ||
| prism (1.9.0) |
There was a problem hiding this comment.
[blocking] prism 1.9.0 carries a C extension, so the "no native extensions remain" claim is wrong — and that claim is what justified skipping the Linux verification.
Two statements are inaccurate for this resolution:
- PR body: removing
json"drops one third-party build-time component and one native build from dev setup" - The tracking-ticket comment: the unbuilt Linux platform rows are low risk because "after dropping json, NO gem in the resolution has a native extension (drb, minitest, prism, rake are all pure Ruby)"
Evidence — prism 1.9.0, pulled in transitively by minitest 6.0.6 (prism (~> 1.5), line 7):
- its gemspec declares
extensions: - ext/prism/extconf.rb - rubygems.org publishes 1.9.0 for the
rubyplatform only — there are no precompiledx86_64-linux/aarch64-linux/ darwin variants. That is consistent with this lockfile carrying a single genericprism (1.9.0)spec row and a single checksum entry, rather than per-platform rows.
So every bundle install — on all four platforms listed below — compiles prism from source and needs a C toolchain plus Ruby headers. The net native-build count is unchanged (json out, prism in), not reduced to zero. That matters here specifically because the stated reason for dropping json was that its native extension fails to compile against Homebrew ruby@3.2 headers: the same class of failure is still in the dependency graph, just in a different gem, and on a gem that has no precompiled fallback at all.
Consequence: the "linux platform rows resolve" human-verify item is load-bearing, not optional as recorded. It is a one-liner:
docker run --rm -v "$PWD":/w -w /w ruby:3.2 bash -lc 'gem install bundler:2.7.1 && bundle install'
Fix: run that check, and correct the two claims (the PR body sentence, plus a follow-up note on the ticket). The lockfile itself needs no change — the spec rows and all four digests are correct, and I verified each digest independently against the SHA-256 rubygems.org publishes for that exact version.
There was a problem hiding this comment.
Confirmed and fixed — you're right on both counts, and thanks for catching that the false claim was the justification for skipping the check, not just a stray inaccuracy.
Verified independently before accepting:
prism-1.9.0.gemspec:15→s.extensions = ["ext/prism/extconf.rb"]- rubygems API lists exactly one platform for 1.9.0:
ruby. No precompiled variant anywhere. - It had already compiled on my own machine and I missed it:
vendor/bundle/ruby/3.2.0/extensions/arm64-darwin-24/3.2.0/prism-1.9.0/prism/prism.bundle
So net native-build count is unchanged (json out, prism in), exactly as you said. drb/minitest/rake do declare no extensions — prism was the miss.
Ran the check. ruby:3.2, Ruby 3.2.11, Bundler 2.7.1:
Installing prism 1.9.0 with native extensions
Bundle complete! 2 Gemfile dependencies, 5 gems now installed.
BUNDLE_EXIT=0
/usr/local/bundle/extensions/aarch64-linux/3.2.0/prism-1.9.0/prism/prism.so
aarch64-linux passes — and that with native extensions line is your point proven from the other direction.
x86_64-linux I could not settle, and I'm not claiming it passes. This is an arm64 host, so the amd64 container runs under qemu and segfaults inside the emulator while building prism's extension (rubygems/ext/ext_conf_builder.rb → open3.capture2e → qemu: uncaught target signal 11, exit 139). Emulation artifact, not a real-hardware verdict. I've left it as a load-bearing human-verify item rather than downgrade it again: on a real x86_64 box, gem install bundler:2.7.1 && bundle install should give Bundle complete! 2 Gemfile dependencies, 5 gems now installed.
Two notes for whoever runs that: your -v "$PWD":/w one-liner needs a colon-free path (my session dir contained :, which is the -v delimiter), and under qemu a Bundler error can surface as a segfault because thor's dynamic_width_stty crashes while printing it — set THOR_COLUMNS if you see that.
Corrected both claims: the PR body sentence now says removing json drops a third-party component but does not make the graph extension-free, and spells out prism's ruby-only publication; the tracking-ticket comment was edited in place with an explicit "this was false" banner naming the original wording. Per your note I left the lockfile untouched.
There was a problem hiding this comment.
Round 1 — resolved. Verified rather than accepted: I re-ran the evidence myself rather than checking that the wording changed.
The claim is corrected on both surfaces (PR body, and the ticket comment edited in place with a banner quoting the original wrong wording), and the corrections are themselves accurate — extensions: is genuinely line 15 of prism's gemspec metadata, and 1.9.0 really is published for the ruby platform only.
Reproduced locally on Ruby 3.2.10 / Bundler 2.7.1 (BUNDLE_FROZEN=true, isolated BUNDLE_PATH, clean checkout of 524db87):
Installing prism 1.9.0 with native extensions
Bundle complete! 2 Gemfile dependencies, 5 gems now installed.
5 gemsis correct, not an off-by-one. The lockfile has 4 specs; Bundler counts itself —cli/install.rb:119usesdefinition.specs.count, anddefinition.rb:621,628fold abundlerdependency into the resolution viadependencies_with_bundler. I checked this because 4-vs-5 was the one figure that would have indicated fabricated output.- The artifact path checks out structurally. prism's
ext/prism/extconf.rb:127iscreate_makefile("prism/prism"), which is exactly why the built object lands at…/prism-1.9.0/prism/prism.<ext>. Mine:vendor/bundle/ruby/3.2.0/extensions/arm64-darwin-24/3.2.0/prism-1.9.0/prism/prism.bundle— same shape as the reported…/aarch64-linux/3.2.0/prism-1.9.0/prism/prism.so. - Frozen install succeeded and did not modify
Gemfile.lock— so the committed lockfile is complete for the locked platform and the pinned digests are accepted as-is. - Suite reproduced exactly:
23 runs, 40 assertions, 0 failures, 0 errors, 3 skips. - Tamper test reproduced: flipping the
rakedigest aborts with exit 37, nothing installed, error anchored atGemfile.lock:25:17— the same line/column quoted in the PR body. Bundler's own error text suggestsbundle config set --local disable_checksum_validation true, which independently vindicates the.gitignoreprecaution.
Leaving x86_64-linux open is the right call and I'm not treating it as blocking: it's honestly labelled inconclusive rather than passing, the qemu segfault is a plausible emulation artifact on an arm64 host (ruby -v here confirms arm64-darwin24), prism built cleanly on both aarch64-linux and arm64-darwin-24, and nothing consumes those platform rows today since no workflow runs bundle install. The handoff carries a runnable command and an expected result.
|
|
||
| PLATFORMS | ||
| aarch64-linux | ||
| arm64-darwin-24 |
There was a problem hiding this comment.
[nit] arm64-darwin-24 is the author's exact dev platform, and x86_64-darwin isn't listed.
PLATFORMS behaves as an allowlist, so a teammate on an Intel Mac — or on a future arm64-darwin-25 — will have Bundler re-resolve and rewrite this lockfile on their first bundle install. That is harmless in a non-frozen setup, but it means the committed file (whose whole purpose is now pinned digests) gets regenerated locally as a side effect of routine setup, which is worth avoiding.
Consider bundle lock --add-platform x86_64-darwin, or replacing the OS-version-pinned row with the generic arm64-darwin, so the committed lockfile covers the team's actual machines. Low priority — and note that nothing consumes the x86_64-linux / aarch64-linux rows today either, since no workflow runs bundle install.
There was a problem hiding this comment.
Agreed on the substance — PLATFORMS is an allowlist, arm64-darwin-24 is my exact dev platform, and an Intel-Mac teammate (or arm64-darwin-25) would have Bundler re-resolve and rewrite the very file whose purpose is now pinned digests. That's a real, if low-severity, wart.
Deliberately not fixing it this round. Round-1 scope was explicitly "the lockfile itself needs no change", and this file is the pinned-digest artifact under review — churning its PLATFORMS block outside the blocking finding would mean re-verifying all four digests for a non-blocking nit. Leaving that call to the approver.
If you want it, the one-liner is bundle lock --add-platform x86_64-darwin (or swap the version-pinned row for the generic arm64-darwin). Happy to push it on request — say the word and it's a two-minute round 2. Recorded as a follow-up in the tracking ticket's marker so it doesn't get lost.
| # Skip them (instead of erroring) when no key is available so the rest of | ||
| # the suite stays green in credential-less environments such as CI. | ||
| def skip_without_credentials | ||
| skip 'requires BROWSERSTACK_ACCESS_KEY (live integration test)' if ENV['BROWSERSTACK_ACCESS_KEY'].to_s.empty? |
There was a problem hiding this comment.
[for-human] Scope note: this PR carries two purposes, and the "green suite" result now rests on these skips.
Not a defect — flagging it because the approver should make two calls consciously. The PR reuses the earlier chore/green-unit-tests branch instead of a dedicated hardening branch, which is disclosed in the body and on the ticket, and the two concerns do share the same three lines of Gemfile.lock. Still:
-
The suite is green partly by omission. The three tests that actually exercise
start/isRunning/stop(test_check_pid,test_is_running,test_multiple_binary) now skip wheneverBROWSERSTACK_ACCESS_KEYis absent, so0 failures, 0 errors, 3 skipsreflects 20 executed tests, not 23. No coverage is lost relative tomaster— there,bundle installfailed outright so nothing ran at all — and the author covered that same code path out-of-band with a real tunnel throughBrowserStack::Local#startplus a live Automate session. But the with-key run was never completed clean (port 45691 was held by an unrelated BrowserStack native-app process on the author's machine), so thestart/stoppath is verified manually rather than by this suite. -
The dev-dependency bumps riding along are broader than the advisory required.
minitest 5.8.4 → 6.0.6andrake 12.3.3 → 13.4.2are both major, dev/test-only. The remediation itself only needed the HTTPS source plus a Bundler ≥ 2.5 regeneration to get aCHECKSUMSblock. Carrying the bumps is defensible on a repo whose lockfile was from 2016, but it is a maintainer call rather than a security requirement.
For what it's worth, the security substance of the change verifies cleanly: all four CHECKSUMS digests match what rubygems.org publishes for those exact versions, the tamper test (exit 37, nothing installed) is a genuine before/after on the property being fixed, and the .gitignore rationale is accurate — bundler-2.7.1/lib/bundler/checksum.rb:13,32 both return if Bundler.settings[:disable_checksum_validation], so a committed .bundle/config really could switch the new verification off silently.
There was a problem hiding this comment.
Acknowledged — no action taken, since both are approver calls rather than defects, and your framing of each is accurate. Confirming so it's clear I read it rather than skipped it:
-
"Green partly by omission" is the right characterisation.
0 failures, 0 errors, 3 skips= 20 executed. I'd add one thing to sharpen the approver's call: thestart/isRunning/stoppath is verified, just not by this suite — real tunnel throughBrowserStack::Local#start(pid 32416,isRunning=true), live Automate session067614082ba6420bfdbe389adb246bab15a48439fetching a page served only on my machine,#stopgraceful with the pid gone; corroborated by 4Host: bs-local.com:45678hits in the binary log. So the coverage exists as evidence on the ticket, but it is not reproducible by CI — and as you note, the with-key run never completed clean because an unrelated BrowserStack native-app process held port 45691 (I chose not to kill someone's running app to green a test). -
Agreed the dev bumps are broader than the advisory required, and that it's a maintainer call. One constraint worth stating: they're not fully separable. Generating a
CHECKSUMSblock needs Bundler ≥ 2.5 regeneration, andjson 1.8.3can't build on Ruby 3.x — so a minimal regeneration lands on current versions regardless.minitest 6is the one genuinely discretionary piece (it's what drags indrb+prism, hence the native build in the other thread). If the approver wants the narrowest possible diff, pinningminitest ~> 5.xwould drop both transitives and the only remaining native extension — I'd take that change if asked, but I'm not making it unprompted.
Thanks also for verifying the digests and the .gitignore rationale independently — checksum.rb:13,32 is a better citation for the disable_checksum_validation risk than the prose I wrote.
What this does
Two related things to the build/test dependency setup. Product code under
lib/is untouched.CHECKSUMSblock so every
bundle installverifies gem content against a pinned SHA-256.Why the integrity part matters
Gemfile/Gemfile.lockonmasterfetched gems fromhttp://rubygems.orgwithBUNDLED WITH 1.11.2(2016) and noCHECKSUMSblock — so nothing verified the contentof a downloaded gem.
rakeexecutes arbitrary code from theRakefileat test time, so asubstituted tarball would run as the developer.
Related advisory context: CVE-2020-8130 / GHSA-jppv-gw3r-w3q8 (OS command injection in
Rake::FileList, patched in rake 12.3.3). The old lockfile already pinned12.3.3, so it wasnot itself vulnerable; this PR moves to
rake 13.4.2and, more importantly, makes the deliveryof that gem verifiable.
What was failing (baseline on
master)bundle installfailed outright, so no tests could run at all:http://rubygems.orgsource no longer serves the spec index over plain HTTP, soresolution failed with
Could not fetch specs from http://rubygems.org/.json 1.8.3,minitest 5.8.4,rake 12.3.3andBUNDLED WITH 1.11.2.json 1.8.3cannot compile its native extension on Ruby 3.x, and the ancient pinned Bundlerwas force-installed on every run.
Once installable, the suite showed 3 errors out of 23 — all three
start-based tests(
test_check_pid,test_is_running,test_multiple_binary). These download and launch the realBrowserStackLocalbinary and open a tunnel, so they need a validBROWSERSTACK_ACCESS_KEYandnetwork access. That is an environment/credential dependency, not a product bug.
What changed
Gemfile— source switched tohttps://rubygems.org. Droppedgem "json":lib/uses thejsondefault gem that ships with Ruby (onlyJSON.parse/JSON.dump), and the gemspecdeclares no dependency on it, so a third-party
jsonwas a redundant build-time dependency —and a native extension that fails to compile against Homebrew's
ruby@3.2headers(
static declaration of 'rb_hash_bulk_insert' follows non-static declaration). Removing itdrops one third-party build-time component.
It does not make the dependency graph extension-free:
prism 1.9.0— pulled intransitively by
minitest 6.0.6(prism (~> 1.5)) — declaresextensions: ["ext/prism/extconf.rb"]and is published for therubyplatform only (noprecompiled
x86_64-linux/aarch64-linux/ darwin variants), so everybundle installstill compiles one C extension and needs a C toolchain plus Ruby headers. Net native-build
count is unchanged —
jsonout,prismin. See the Linux check under Verification.Gemfile.lock— regenerated with Bundler 2.7.1: HTTPS remote, current buildableversions (
minitest 6.0.6,rake 13.4.2), common Linux platforms for CI portability, and aCHECKSUMSblock with per-gem SHA-256 digests..gitignore— ignore.bundle/andvendor/bundle/..bundle/configcan carrydisable_checksum_validation, which would silently switch the new verification off; it mustnever be committed.
test/browserstack-local-test.rb— the three live integration testsskipwhenBROWSERSTACK_ACCESS_KEYis absent, so the suite stays green in credential-less environments.With a key set they run in full, unchanged.
Verification
Checksums are genuine. Each digest in
CHECKSUMSwas cross-checked against the SHA-256rubygems.org publishes for that exact version (
/api/v2/rubygems/<name>/versions/<v>.json) —all 4 match.
Checksum verification is actually enforced (not just present). Flipping one digest in
CHECKSUMSto a wrong-but-well-formed value:With the
CHECKSUMSblock removed (the pre-change shape), the same install exits 0 and performsno verification at all. That is the before/after that matters.
Linux install —
aarch64-linuxverified,x86_64-linuxinconclusive. This is the check theincorrect "no native extensions" claim had wrongly downgraded to optional. Run in a
ruby:3.2container (Ruby 3.2.11, Bundler 2.7.1):
prismdoes compile from source on Linux and the pinned digests are accepted. Note theInstalling prism 1.9.0 with native extensionsline — that is precisely the native build theearlier claim said no longer existed.
x86_64-linuxcould not be settled here and is not claimed as passing: this is an arm64host, so the amd64 container runs under qemu, and it segfaults inside the emulator while building
prism's extension (
rubygems/ext/ext_conf_builder.rb→open3.capture2e→qemu: uncaught target signal 11, exit 139). That is an emulation artifact, not a verdict aboutreal x86_64 hardware — but it does independently land in prism's native-build path. A real
x86_64 Linux box (or CI runner) should run
gem install bundler:2.7.1 && bundle installand expect
Bundle complete! 2 Gemfile dependencies, 5 gems now installed.Suite.
bundle exec rake test→23 runs, 40 assertions, 0 failures, 0 errors, 3 skips(Ruby 3.2.10, Bundler 2.7.1). With a key, a real tunnel was opened through the binding's own
BrowserStack::Local#start/#isRunning/#stopand a live Automate session fetched a pageserved only on the dev machine, confirming start/stop and the tunnel path are unaffected.
Notes for the reviewer
assertion adds nothing a scanner does not already do). The tamper check above is the meaningful
proof and is reproducible in one command.
here: no workflow runs
bundle installorrake(gem-push.ymlisgem build+gem pushunder RubyGems Trusted Publishing/OIDC; Semgrep runs in a container).
BUNDLED WITH 2.7.1isthe effective pin for local use. If a test-CI workflow is added later, pin
bundler-versioninruby/setup-rubythere.How to run the suite
Result without a key:
23 runs, 40 assertions, 0 failures, 0 errors, 3 skips.With
BROWSERSTACK_ACCESS_KEYset, the three integration tests execute against live infrastructure.