From 9529fed3014290a3e7184460398ab2f6c4bea5d0 Mon Sep 17 00:00:00 2001
From: 07souravkunda <souravkunda00@gmail.com>
Date: Mon, 21 Sep 2026 17:06:24 +0530
Subject: [PATCH] fix: redact access key in public command accessor and inspect
 (CWE-312)

The public `command` method returned the start command string with the
BrowserStack access key interpolated verbatim, so any caller that logged
it (CI output, test runner logs, APM/error trackers) leaked the credential
to a wider audience than the key itself. Ruby's default #inspect had the
same problem, dumping @key when a Local instance was logged or raised.

- command now returns the command with the key masked as [REDACTED]
- start_command takes an optional redact flag; the execution path
  (start_command_args array, and the string form on legacy Ruby) keeps
  the real key, so the tunnel is unaffected
- add a redacting #inspect so the key is never dumped by object inspection

Proxy password is intentionally left visible (existing behaviour/tests).
Adds regression tests that fail on the pre-fix code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 lib/browserstack/local.rb       | 21 ++++++++++++++++++---
 test/browserstack-local-test.rb | 21 +++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/lib/browserstack/local.rb b/lib/browserstack/local.rb
index 0a01b1c..5e1417d 100644
--- a/lib/browserstack/local.rb
+++ b/lib/browserstack/local.rb
@@ -121,12 +121,27 @@ def stop
     @pid = nil
   end
 
+  # Public accessor used by callers for debugging/logging. Return the command
+  # with the access key masked so it is never written to logs, CI artifacts or
+  # error trackers (CWE-312). The real key is still used for execution via
+  # start_command_args / start_command(false).
   def command
-    start_command
+    start_command(true)
   end
 
-  def start_command
-    cmd = "#{@binary_path} -d start -logFile '#{@logfile}' #{@folder_flag} #{@key} #{@folder_path} #{@force_local_flag}"
+  # Prevent Ruby's default #inspect from dumping @key when a Local instance is
+  # logged or included in an exception payload (CWE-312).
+  def inspect
+    redacted = instance_variables.map do |var|
+      value = var == :@key && !@key.to_s.empty? ? "[REDACTED]" : instance_variable_get(var)
+      "#{var}=#{value.inspect}"
+    end.join(", ")
+    "#<#{self.class}:0x#{format('%016x', object_id << 1)} #{redacted}>"
+  end
+
+  def start_command(redact = false)
+    key = redact && !@key.to_s.empty? ? "[REDACTED]" : @key
+    cmd = "#{@binary_path} -d start -logFile '#{@logfile}' #{@folder_flag} #{key} #{@folder_path} #{@force_local_flag}"
     cmd += " -localIdentifier #{@local_identifier_flag}" if @local_identifier_flag
     cmd += " #{@only_flag} #{@only_automate_flag}"
     cmd += " -proxyHost #{@proxy_host}" if @proxy_host
diff --git a/test/browserstack-local-test.rb b/test/browserstack-local-test.rb
index 2c6218b..12c96ed 100644
--- a/test/browserstack-local-test.rb
+++ b/test/browserstack-local-test.rb
@@ -96,6 +96,27 @@ def test_hosts
     assert_match /localhost\,8080\,0/, @bs_local.command
   end
 
+  # Regression for CWE-312: the public #command accessor must NOT expose the
+  # access key — callers routinely log it to CI output / APM / error trackers.
+  def test_command_redacts_access_key
+    bs = BrowserStack::Local.new("MY_SECRET_ACCESS_KEY")
+    refute_match /MY_SECRET_ACCESS_KEY/, bs.command
+    assert_match /\[REDACTED\]/, bs.command
+  end
+
+  # The real key must still reach the binary on the execution path.
+  def test_start_command_keeps_key_for_execution
+    bs = BrowserStack::Local.new("MY_SECRET_ACCESS_KEY")
+    assert_match /MY_SECRET_ACCESS_KEY/, bs.start_command
+  end
+
+  # Regression for CWE-312: default object inspection must not dump the key.
+  def test_inspect_redacts_access_key
+    bs = BrowserStack::Local.new("MY_SECRET_ACCESS_KEY")
+    refute_match /MY_SECRET_ACCESS_KEY/, bs.inspect
+    assert_match /\[REDACTED\]/, bs.inspect
+  end
+
   def teardown
     @bs_local.stop
   end