Skip to content

Fix code review issues, CVE-2026-33210 pin, and Ruby 4.0 test suite#1

Merged
cmwright33 merged 2 commits into
mainfrom
fix/review-and-test-suite
May 12, 2026
Merged

Fix code review issues, CVE-2026-33210 pin, and Ruby 4.0 test suite#1
cmwright33 merged 2 commits into
mainfrom
fix/review-and-test-suite

Conversation

@cmwright33
Copy link
Copy Markdown
Owner

Summary

  • Fixes a load-order bug that caused NoMethodError on Ruby 4.0 at require time
  • Adds nil/empty api_key guard in send_batch to avoid sending broken Authorization: Bearer headers
  • Brings flush! (at-exit path) in line with safe_flush: now calls on_flush_error and increments consecutive_failures on failure
  • Closes HTTP connection leak in RegistryLoader.fetch_remote
  • Raises json floor to >= 2.19.2 to exclude CVE-2026-33210 (CVSS 9.1)
  • Deletes lib/apidepth/core.rb tombstone file
  • Fixes test suite to pass on Ruby 4.0 (leaked doubles, be_in without ActiveSupport, SSL consistency error from started? stub, WebMock 0ms duration)

What changed and why

lib/apidepth/vendor_registry.rb

initialize_registry called replace(BUNDLED_BASELINE) at require time, which logs via Apidepth.logger. But logger is defined in apidepth.rb's class << self block, which hasn't been evaluated yet when vendor_registry.rb is first required. Fixed by inlining the state assignment in initialize_registry without the log call.

lib/apidepth/collector.rb

  • send_batch: returns early when api_key is nil or empty — avoids a wasted round-trip that was guaranteed to 401 and burn a failure increment
  • flush!: rescue block now calls on_flush_error and increments consecutive_failures, matching safe_flush behavior. The at-exit path is the one users are most likely to notice failures in
  • Removed respond_to?(:name=) guards — Thread#name= has been available since Ruby 2.3; gem requires 2.7+
  • Added comment documenting collector_url memoization intent

lib/apidepth/registry_loader.rb

fetch_remote creates a new Net::HTTP per call and was never calling finish. Added http&.finish rescue nil in the ensure block. Also removed dead respond_to?(:name=) guard.

apidepth.gemspec

  • Raised json floor to >= 2.19.2CVE-2026-33210 (CVSS 9.1 Critical) affects json 2.14–2.15.2.0, 2.16–2.17.1.1, and 2.18–2.19.1. Patched in 2.19.2
  • Removed fabricated CVE reference placeholder

spec/spec_helper.rb

Added a safety nil-out of @http on the singleton Collector instance before reset! runs in after(:each). Tests that set a mock Net::HTTP double on the singleton were leaving it alive past example teardown, causing "expired test double" failures in subsequent tests.

spec/sdk_spec.rb

  • Added before { Apidepth.configuration.api_key = "sk_test" } to the Collector describe block — required now that send_batch skips when api_key is nil
  • cold_start: false test: removed allow_any_instance_of stub. Net::HTTP.start establishes a real connection before yielding to our instrumented request, so started? is naturally true. The stub was unnecessary and caused a Ruby 4.0 SSL consistency IOError
  • Changed be_positivebe >= 0 for duration_ms checks: WebMock returns with no delay; elapsed rounds to 0ms on fast hardware
  • Replaced be_in([true, false]) with include matcher: be_in requires ActiveSupport which is not loaded in the test suite
  • Updated SSRF flush! test: consecutive_failures should be 1 after a failed flush now that flush! tracks failures correctly
  • Fixed event() helper to include outcome: :success (required field)
  • Fixed env fallback test to expect "unknown" (Rails is not loaded in specs)
  • Added tests for empty api_key guard and corrected env fallback

Test plan

  • bundle exec rspec passes — 118 examples, 0 failures on Ruby 4.0.3
  • No warnings about missing api_key at suite startup
  • Security tests (SSRF, header injection, log injection) all pass

🤖 Generated with Claude Code

cmwright33 and others added 2 commits May 11, 2026 22:06
vendor_registry.rb — load-order bug
  initialize_registry ran at require time and called Apidepth.logger before
  it was defined (Apidepth class << self block hadn't been evaluated yet).
  Replaced the replace(BUNDLED_BASELINE) call with a direct assignment so
  the bundled baseline loads silently without touching the logger.

spec/spec_helper.rb — expired double safety net
  Tests that set a mock @http on the singleton Collector instance leaked
  the double into the next example's after(:each) teardown, causing
  "expired test double" failures. Nil out @http on the singleton before
  reset! so teardown never calls finish on an expired double.

spec/sdk_spec.rb — Ruby 4.0 compatibility fixes
  - Add api_key before hook to Collector describe block: the new
    send_batch nil/empty guard skips the send when no key is set, so
    tests that expect network calls must configure one.
  - cold_start: false test no longer stubs started?. Net::HTTP.start
    establishes the real connection before yielding to our instrumented
    request, so started? is naturally true — the stub was both unnecessary
    and caused a Ruby 4.0 SSL consistency IOError.
  - Change be_positive to be >= 0 for duration_ms checks: WebMock returns
    immediately with no delay, so elapsed rounds to 0ms on fast hardware.
  - Replace be_in([true, false]) with include matcher: be_in requires
    ActiveSupport which is not loaded in the test suite.
  - Update SSRF flush! test expectation: flush! now correctly increments
    consecutive_failures on failure (matching safe_flush behavior), so the
    old eq(0) assertion was wrong.
  - Add outcome: :success to event() helper: outcome is a REQUIRED field
    on Event; the helper was producing hashes that bypass Event.build.
  - Fix env fallback test: Rails is not loaded in specs; the fallback is
    "unknown", not Rails.env.
  - Add tests for empty api_key guard and corrected env fallback.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents all changes from this session: load-order bug fix, empty
api_key guard, flush! on_flush_error parity, HTTP connection leak fix,
CVE-2026-33210 json pin, core.rb removal, and thread-name guard cleanup.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cmwright33 cmwright33 merged commit 460d5e6 into main May 12, 2026
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.

1 participant