LIVE-20561 + LIVE-20566: Bump yajl-ruby to >= 1.4.3 (CVE-2022-24795) and commit Gemfile.lock

.github/workflows/Semgrep.yml

@@ -27,7 +27,10 @@ jobs:
 
     container:
       # A Docker image with Semgrep installed. Do not change this.
-      image: returntocorp/semgrep
+      # Pinned to a specific tag + immutable digest to prevent supply-chain
+      # tampering via floating tag mutation. Bump both the tag and the
+      # digest together when updating.
+      image: returntocorp/semgrep:1.163.0@sha256:7cad2bc2d1e44f87f0bf4be6d1fa23aa90fb72015bebc89fb91385d813987a03
 
     # Skip any PR created by dependabot to avoid permission issues:
     if: (github.actor != 'dependabot[bot]')

.gitignore

@@ -3,7 +3,6 @@
 .bundle
 .config
 .yardoc
-Gemfile.lock
 InstalledFiles
 _yardoc
 coverage

Gemfile.lock

@@ -0,0 +1,21 @@
+PATH
+  remote: .
+  specs:
+    browserstack-screenshot (0.0.3)
+      yajl-ruby (>= 1.4.3)
+
+GEM
+  remote: https://rubygems.org/
+  specs:
+    rake (13.4.2)
+    yajl-ruby (1.4.3)
+
+PLATFORMS
+  ruby
+
+DEPENDENCIES
+  browserstack-screenshot!
+  rake
+
+BUNDLED WITH
+   2.1.4

Gemfile.test

@@ -0,0 +1,18 @@
+source 'https://rubygems.org'
+
+# LOCAL-ONLY test bundle — not part of the gem distribution. Lives in a
+# separate Gemfile so it never lands in a PR or a published .gem.
+#
+# Usage:
+#   BUNDLE_GEMFILE=Gemfile.test bundle install
+#   BUNDLE_GEMFILE=Gemfile.test bundle exec rspec spec/
+#
+# Or use the bin/test wrapper which handles the env var for you.
+
+gemspec
+
+group :test do
+  gem 'rspec',   '~> 3.13'
+  gem 'webmock', '~> 3.19'
+  gem 'rake',    '>= 13.0'
+end

Gemfile.test.lock

@@ -0,0 +1,50 @@
+PATH
+  remote: .
+  specs:
+    browserstack-screenshot (0.0.3)
+      yajl-ruby (= 1.3.1)
+
+GEM
+  remote: https://rubygems.org/
+  specs:
+    addressable (2.9.0)
+      public_suffix (>= 2.0.2, < 8.0)
+    bigdecimal (4.1.2)
+    crack (1.0.1)
+      bigdecimal
+      rexml
+    diff-lcs (1.6.2)
+    hashdiff (1.2.1)
+    public_suffix (5.1.1)
+    rake (13.4.2)
+    rexml (3.4.4)
+    rspec (3.13.2)
+      rspec-core (~> 3.13.0)
+      rspec-expectations (~> 3.13.0)
+      rspec-mocks (~> 3.13.0)
+    rspec-core (3.13.6)
+      rspec-support (~> 3.13.0)
+    rspec-expectations (3.13.5)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.13.0)
+    rspec-mocks (3.13.8)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.13.0)
+    rspec-support (3.13.7)
+    webmock (3.26.2)
+      addressable (>= 2.8.0)
+      crack (>= 0.3.2)
+      hashdiff (>= 0.4.0, < 2.0.0)
+    yajl-ruby (1.3.1)
+
+PLATFORMS
+  ruby
+
+DEPENDENCIES
+  browserstack-screenshot!
+  rake (>= 13.0)
+  rspec (~> 3.13)
+  webmock (~> 3.19)
+
+BUNDLED WITH
+   2.1.4

bin/test

@@ -0,0 +1,46 @@
+#!/usr/bin/env bash
+# Local-only test runner for ruby-screenshots. Not part of the gem.
+#
+# What it does:
+#   1. Sets BUNDLE_GEMFILE to Gemfile.test so rspec + webmock don't leak
+#      into the published gem's dependency tree.
+#   2. Pins PATH to the Ruby `ruby` currently resolves to, so machines
+#      with both rvm and rbenv installed don't end up using a different
+#      Ruby for `bundle exec` than for the `bundle` lookup. Mixing
+#      Rubies leads to "incompatible library version" errors on native
+#      extensions like yajl-ruby and bigdecimal.
+#   3. Installs test deps if missing.
+#   4. Runs rspec against spec/ (passes through any extra args).
+#
+# Usage:
+#   bin/test                                              # all specs
+#   bin/test spec/client_spec.rb -e 'job_id allowlist'    # focused
+#   bin/test --format documentation                       # verbose
+
+set -euo pipefail
+
+cd "$(dirname "$0")/.."
+
+# Resolve the bin dir of whichever Ruby is on PATH and pin it so
+# `bundle`, `bundle exec ruby`, and `rspec` all use the same interpreter.
+RUBY_BINDIR="$(ruby -e 'puts RbConfig::CONFIG["bindir"]')"
+export PATH="$RUBY_BINDIR:$PATH"
+
+export BUNDLE_GEMFILE="$PWD/Gemfile.test"
+
+# Ensure the bundler version recorded in Gemfile.test.lock is installed
+# for the current Ruby. Without this, machines that pick up a different
+# Ruby than the one that generated the lockfile fail with
+# "Could not find bundler (X) required by your Gemfile.lock".
+LOCKED_BUNDLER="$(awk '/^BUNDLED WITH$/{getline; gsub(/^ +/, ""); print; exit}' Gemfile.test.lock 2>/dev/null || true)"
+if [ -n "$LOCKED_BUNDLER" ] && ! gem list -i bundler -v "$LOCKED_BUNDLER" >/dev/null 2>&1; then
+  echo "==> Installing bundler $LOCKED_BUNDLER (required by Gemfile.test.lock) ..."
+  gem install bundler -v "$LOCKED_BUNDLER" --no-document --quiet
+fi
+
+if ! bundle check --quiet >/dev/null 2>&1; then
+  echo "==> Installing test deps via Gemfile.test ..."
+  bundle install --quiet
+fi
+
+exec bundle exec rspec "${@:-spec/}"

browserstack-screenshot.gemspec

@@ -19,5 +19,5 @@ Gem::Specification.new do |spec|
   spec.require_paths = ["lib"]
 
   spec.add_development_dependency "rake"
-  spec.add_dependency("yajl-ruby", "1.3.1")
+  spec.add_dependency("yajl-ruby", ">= 1.4.3")
 end

lib/screenshot/client.rb

@@ -1,8 +1,14 @@
 module Screenshot
 	class Client
-          
+
     API = "https://www.browserstack.com/screenshots"
-
+
+    # Allowlist for job IDs accepted by screenshots_status/screenshots.
+    # Alphanumeric, underscore, and hyphen only — blocks path traversal,
+    # CRLF injection, and other URL-path tampering before the value is
+    # interpolated into the API request path.
+    JOB_ID_FORMAT = /\A[\w\-]{1,64}\z/
+
     def initialize(options={})
       options = symbolize_keys options
       unless options[:username] && options[:password]
@@ -15,9 +21,14 @@ def initialize(options={})
 
     def get_os_and_browsers
       res = http_get_request :extend_uri => "browsers.json"
+      # Empirically the production /screenshots/browsers.json endpoint
+      # returns a JSON object (Hash). The published API doc at
+      # https://www.browserstack.com/screenshots/api#list-os-browsers
+      # shows a top-level array, but a curl against production returns
+      # a Hash — trust reality over docs.
       parse res
     end
-    
+
     def generate_screenshots configHash={}
       res = http_post_request :data => Yajl::Encoder.encode(configHash)
       responseJson = parse res
@@ -29,18 +40,35 @@ def screenshots_done? job_id
     end
 
      def screenshots_status job_id
+      validate_job_id! job_id
       res = http_get_request :extend_uri => "#{job_id}.json"
       responseJson = parse res
       responseJson[:state]
     end
 
     def screenshots job_id
+      validate_job_id! job_id
       res = http_get_request :extend_uri => "#{job_id}.json"
       responseJson = parse res
       responseJson[:screenshots]
     end
-
+
+    # Redact @authentication when the receiver is serialised — APM/error
+    # trackers (Sentry, Bugsnag, Datadog) capture the receiver's inspect
+    # output alongside exception frames, which would otherwise leak the
+    # reversible Base64-encoded Basic Auth credential.
+    def inspect
+      "#<#{self.class.name}:0x#{(object_id << 1).to_s(16)} @authentication=[REDACTED]>"
+    end
+    alias_method :to_s, :inspect
+
     private
+    def validate_job_id!(job_id)
+      unless job_id.is_a?(String) && job_id =~ JOB_ID_FORMAT
+        raise ArgumentError, "Invalid job_id: must match #{JOB_ID_FORMAT.source}"
+      end
+    end
+
     def authenticate options, uri=API
       http_get_request options, uri
     end
@@ -70,7 +98,7 @@ def make_request req, options={}, uri=API
       http_response_code_check res
       res
     end
-    
+
     def add_authentication options, req
       req["Authorization"] = @authentication
       req
@@ -81,19 +109,31 @@ def http_response_code_check res
       when 200
         res
       when 401
-        raise AuthenticationError, encode({:code => res.code, :body => res.body})
+        raise AuthenticationError.new("BrowserStack API responded #{res.code}", res.body)
       when 403
-        raise ScreenshotNotAllowedError, encode({:code => res.code, :body => res.body})
+        raise ScreenshotNotAllowedError.new("BrowserStack API responded #{res.code}", res.body)
       when 422
-        raise InvalidRequestError, encode({:code => res.code, :body => res.body})
+        raise InvalidRequestError.new("BrowserStack API responded #{res.code}", res.body)
       else
-        raise UnexpectedError, encode({:code => res.code, :body => res.body})
+        raise UnexpectedError.new("BrowserStack API responded #{res.code}", res.body)
       end
     end
 
-    def parse(response)
+    def parse(response, expected = Hash)
       parser = Yajl::Parser.new(:symbolize_keys => true)
-      parser.parse(response.body)
+      begin
+        result = parser.parse(response.body)
+      rescue Yajl::ParseError => e
+        # Wrap upstream parser errors (non-JSON 200 bodies — HTML
+        # maintenance pages, plain text, truncated payloads) so callers
+        # see a typed Screenshot::ParseError rather than a yajl-internal
+        # exception that doesn't match `rescue Screenshot::*` blocks.
+        raise ParseError, "BrowserStack API returned invalid JSON: #{e.message}"
+      end
+      unless result.is_a?(expected)
+        raise ParseError, "Expected #{expected} from BrowserStack API, got #{result.class}"
+      end
+      result
     end
 
     def encode(hash)
@@ -105,17 +145,33 @@ def symbolize_keys hash
     end
 
   end #Client
-
-  class AuthenticationError < StandardError  
-  end 
-
-  class InvalidRequestError < StandardError  
-  end 
-
-  class ScreenshotNotAllowedError < StandardError
+
+  # Base class for BrowserStack API errors. Carries the raw response body
+  # behind an opt-in `#body` reader so callers can inspect it deliberately;
+  # the default exception message is a fixed status-code string so that
+  # APM/log capture does not auto-ingest the body alongside the receiver's
+  # instance variables.
+  class APIError < StandardError
+    attr_reader :body
+    def initialize(message = nil, body = nil)
+      super(message)
+      @body = body
+    end
+  end
+
+  class AuthenticationError < APIError
+  end
+
+  class InvalidRequestError < APIError
+  end
+
+  class ScreenshotNotAllowedError < APIError
   end
 
-  class UnexpectedError < StandardError  
+  class UnexpectedError < APIError
   end
-
+
+  class ParseError < StandardError
+  end
+
 end #Screenshots