Skip to content

build: verify gem integrity at install time + green the unit-test baseline #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Open
07souravkunda wants to merge 2 commits into
base: master
Choose a base branch
from
+37 −8
Open

build: verify gem integrity at install time + green the unit-test baseline #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee74327
test: fix pre-existing unit-test failures (green the suite)
07souravkunda Aug 7, 2026
524db87
build: verify gem integrity at install time (CVE-2020-8130 hardening)
07souravkunda Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
dist/*
*.log
browserstack.err

# Local Bundler state. .bundle/config can carry settings that weaken install-time
# integrity checks (e.g. disable_checksum_validation), so it must never be committed.
.bundle/
vendor/bundle/
6 changes: 4 additions & 2 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
source "http://rubygems.org"
source "https://rubygems.org"
gem "minitest"
gem "rake"
gem "json"
# "json" is intentionally NOT listed: lib/ uses the `json` default gem that ships
# with Ruby, and the gemspec declares no dependency on it, so a third-party json
# build is a redundant build-time dependency (and a native extension) to pull in.
23 changes: 17 additions & 6 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
GEM
remote: http://rubygems.org/
remote: https://rubygems.org/
specs:
json (1.8.3)
minitest (5.8.4)
rake (12.3.3)
drb (2.2.3)
minitest (6.0.6)
drb (~> 2.0)
prism (~> 1.5)
prism (1.9.0)

@07souravkunda 07souravkunda Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[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 ruby platform only — there are no precompiled x86_64-linux / aarch64-linux / darwin variants. That is consistent with this lockfile carrying a single generic prism (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.

@07souravkunda 07souravkunda Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

@07souravkunda 07souravkunda Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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 gems is correct, not an off-by-one. The lockfile has 4 specs; Bundler counts itself — cli/install.rb:119 uses definition.specs.count, and definition.rb:621,628 fold a bundler dependency into the resolution via dependencies_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:127 is create_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 rake digest aborts with exit 37, nothing installed, error anchored at Gemfile.lock:25:17 — the same line/column quoted in the PR body. Bundler's own error text suggests bundle config set --local disable_checksum_validation true, which independently vindicates the .gitignore precaution.

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.

rake (13.4.2)

PLATFORMS
aarch64-linux
arm64-darwin-24

@07souravkunda 07souravkunda Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[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.

@07souravkunda 07souravkunda Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

ruby
x86_64-linux

DEPENDENCIES
json
minitest
rake

CHECKSUMS
drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373
minitest (6.0.6) sha256=153ea36d1d987a62942382b61075745042a2b3123b1cd48f4c3675af9cc7d6f1
prism (1.9.0) sha256=7b530c6a9f92c24300014919c9dcbc055bf4cdf51ec30aed099b06cd6674ef85
rake (13.4.2) sha256=cb825b2bd5f1f8e91ca37bddb4b9aaf345551b4731da62949be002fa89283701

BUNDLED WITH
1.11.2
2.7.1
11 changes: 11 additions & 0 deletions test/browserstack-local-test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,28 @@ def setup
@bs_local = BrowserStack::Local.new
end

# The tests below actually start the BrowserStackLocal binary and open a
# tunnel, so they need a valid BROWSERSTACK_ACCESS_KEY and network access.
# 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?

@07souravkunda 07souravkunda Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[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:

  1. 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 whenever BROWSERSTACK_ACCESS_KEY is absent, so 0 failures, 0 errors, 3 skips reflects 20 executed tests, not 23. No coverage is lost relative to master — there, bundle install failed outright so nothing ran at all — and the author covered that same code path out-of-band with a real tunnel through BrowserStack::Local#start plus 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 the start/stop path is verified manually rather than by this suite.

  2. The dev-dependency bumps riding along are broader than the advisory required. minitest 5.8.4 → 6.0.6 and rake 12.3.3 → 13.4.2 are both major, dev/test-only. The remediation itself only needed the HTTPS source plus a Bundler ≥ 2.5 regeneration to get a CHECKSUMS block. 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.

@07souravkunda 07souravkunda Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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:

  1. "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: the start/isRunning/stop path is verified, just not by this suite — real tunnel through BrowserStack::Local#start (pid 32416, isRunning=true), live Automate session 067614082ba6420bfdbe389adb246bab15a48439 fetching a page served only on my machine, #stop graceful with the pid gone; corroborated by 4 Host: bs-local.com:45678 hits 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).

  2. 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 CHECKSUMS block needs Bundler ≥ 2.5 regeneration, and json 1.8.3 can't build on Ruby 3.x — so a minimal regeneration lands on current versions regardless. minitest 6 is the one genuinely discretionary piece (it's what drags in drb + prism, hence the native build in the other thread). If the approver wants the narrowest possible diff, pinning minitest ~> 5.x would 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.

end

def test_check_pid
skip_without_credentials
@bs_local.start
refute_nil @bs_local.pid, 0
end

def test_is_running
skip_without_credentials
@bs_local.start
assert_equal true, @bs_local.isRunning
end

def test_multiple_binary
skip_without_credentials
@bs_local.start
bs_local_2 = BrowserStack::Local.new
second_log_file = File.join(Dir.pwd, 'local2.log')
Expand Down
Loading