Fix secret name encoding, Retry-After handling, and site_url validation

Author: Abdul-Moiz31

lib/infisical/client.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "uri"
+
 require_relative "http_client"
 require_relative "auth"
 require_relative "secrets"
@@ -9,8 +11,10 @@ module Infisical
   # use `secrets` (and future resource clients) to talk to Infisical.
   class Client
     DEFAULT_SITE_URL = "https://app.infisical.com"
+    ALLOWED_SCHEMES = %w[http https].freeze
 
     def initialize(site_url: DEFAULT_SITE_URL, timeout: HTTPClient::DEFAULT_TIMEOUT)
+      validate_site_url!(site_url)
       @http_client = HTTPClient.new(base_url: site_url, timeout: timeout)
     end
 
@@ -21,5 +25,16 @@ def auth
     def secrets
       @secrets ||= Secrets.new(@http_client)
     end
+
+    private
+
+    def validate_site_url!(site_url)
+      uri = URI.parse(site_url)
+      return if ALLOWED_SCHEMES.include?(uri.scheme) && uri.host
+
+      raise ArgumentError, "site_url must be an http(s) URL, got: #{site_url.inspect}"
+    rescue URI::InvalidURIError
+      raise ArgumentError, "site_url is not a valid URL: #{site_url.inspect}"
+    end
   end
 end

lib/infisical/errors.rb

@@ -22,11 +22,8 @@ def initialize(message, status:, url:, method:, request_id: nil)
   end
 
   # Raised when the request itself fails (network/timeout) before a
-  # response is received.
-  class RequestError < Error
-    def initialize(message, cause: nil)
-      super(message)
-      @cause = cause if cause
-    end
-  end
+  # response is received. Always raised from inside a `rescue` for the
+  # underlying network exception, so Ruby's built-in `Exception#cause`
+  # already carries it through — no need to track it ourselves.
+  class RequestError < Error; end
 end

lib/infisical/http_client.rb

@@ -3,6 +3,7 @@
 require "net/http"
 require "uri"
 require "json"
+require "time"
 
 require_relative "errors"
 
@@ -73,7 +74,7 @@ def request(method, path, body: nil, params: {})
         response = perform(method, uri, body)
         handle_response(response, method: method, uri: uri)
       rescue *RETRYABLE_EXCEPTIONS => e
-        raise RequestError.new("request to #{uri} failed: #{e.message}", cause: e) if attempt >= @max_retries
+        raise RequestError, "request to #{uri} failed: #{e.message}" if attempt >= @max_retries
 
         attempt += 1
         wait_before_retry(attempt)
@@ -82,7 +83,7 @@ def request(method, path, body: nil, params: {})
         raise e.api_error if attempt >= @max_retries
 
         attempt += 1
-        wait_before_retry(attempt)
+        wait_before_retry(attempt, override: e.retry_after)
         retry
       end
     end
@@ -126,12 +127,32 @@ def build_request(method, uri, body)
 
     def handle_response(response, method:, uri:)
       status = response.code.to_i
-      raise RetryableAPIError, build_api_error(response, status, method, uri) if status == 429
+      if status == 429
+        raise RetryableAPIError.new(build_api_error(response, status, method, uri),
+                                    retry_after: parse_retry_after(response["Retry-After"]))
+      end
       raise build_api_error(response, status, method, uri) unless (200..299).cover?(status)
 
       parse_body(response.body)
     end
 
+    # Retry-After is usually an integer/float number of seconds, but per
+    # RFC 9110 it may also be an HTTP-date.
+    def parse_retry_after(value)
+      return nil if value.nil? || value.empty?
+
+      begin
+        Float(value)
+      rescue ArgumentError, TypeError
+        begin
+          seconds = Time.httpdate(value) - Time.now
+          seconds.positive? ? seconds : nil
+        rescue ArgumentError
+          nil
+        end
+      end
+    end
+
     def build_api_error(response, status, method, uri)
       data = parse_body(response.body)
       message = data.is_a?(Hash) ? (data["message"] || data["error"] || response.body) : response.body
@@ -153,19 +174,27 @@ def parse_body(raw)
       raw
     end
 
-    def wait_before_retry(attempt)
+    def wait_before_retry(attempt, override: nil)
+      if override
+        @sleeper.call(override)
+        return
+      end
+
       base = self.class.backoff_delay(attempt - 1, initial_delay: @initial_delay, backoff_factor: @backoff_factor)
       jitter = base * JITTER_RATIO * ((rand * 2) - 1)
       @sleeper.call(base + jitter)
     end
 
     # Internal-only signal so 429s share the same retry path as network
-    # errors without retrying every other 4xx/5xx status.
+    # errors without retrying every other 4xx/5xx status. Carries the
+    # server's Retry-After hint (if any) so the wait honors it instead of
+    # our own computed backoff.
     class RetryableAPIError < StandardError
-      attr_reader :api_error
+      attr_reader :api_error, :retry_after
 
-      def initialize(api_error)
+      def initialize(api_error, retry_after: nil)
         @api_error = api_error
+        @retry_after = retry_after
         super(api_error.message)
       end
     end

lib/infisical/secrets.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "uri"
+
 require_relative "models/secret"
 
 module Infisical
@@ -31,7 +33,7 @@ def list(project_id:, environment:, secret_path: "/", expand_secret_references:
     def get(secret_name, project_id:, environment:, secret_path: "/",
             expand_secret_references: true, include_imports: false)
       response = @http_client.get(
-        "#{BASE_PATH}/#{secret_name}",
+        secret_path_for(secret_name),
         params: {
           workspaceId: project_id,
           environment: environment,
@@ -46,7 +48,7 @@ def get(secret_name, project_id:, environment:, secret_path: "/",
 
     def create(secret_name, secret_value, project_id:, environment:, secret_path: "/", secret_comment: nil)
       response = @http_client.post(
-        "#{BASE_PATH}/#{secret_name}",
+        secret_path_for(secret_name),
         body: {
           workspaceId: project_id,
           environment: environment,
@@ -60,8 +62,12 @@ def create(secret_name, secret_value, project_id:, environment:, secret_path: "/
     end
 
     def update(secret_name, project_id:, environment:, secret_value: nil, new_secret_name: nil, secret_path: "/")
+      if secret_value.nil? && new_secret_name.nil?
+        raise ArgumentError, "update requires at least one of secret_value: or new_secret_name:"
+      end
+
       response = @http_client.patch(
-        "#{BASE_PATH}/#{secret_name}",
+        secret_path_for(secret_name),
         body: {
           workspaceId: project_id,
           environment: environment,
@@ -76,7 +82,7 @@ def update(secret_name, project_id:, environment:, secret_value: nil, new_secret
 
     def delete(secret_name, project_id:, environment:, secret_path: "/")
       response = @http_client.delete(
-        "#{BASE_PATH}/#{secret_name}",
+        secret_path_for(secret_name),
         body: {
           workspaceId: project_id,
           environment: environment,
@@ -86,5 +92,14 @@ def delete(secret_name, project_id:, environment:, secret_path: "/")
 
       Models::Secret.from_api(response["secret"])
     end
+
+    private
+
+    # Escapes a secret name for safe use as a single URI path segment, so
+    # names containing "/", "?", "#", or "%" can't be misread as path
+    # separators or query-string tokens.
+    def secret_path_for(secret_name)
+      "#{BASE_PATH}/#{URI::DEFAULT_PARSER.escape(secret_name.to_s, /[^A-Za-z0-9\-._~]/)}"
+    end
   end
 end

spec/infisical/client_spec.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+RSpec.describe Infisical::Client do
+  it "builds successfully with the default site_url" do
+    expect { described_class.new }.not_to raise_error
+  end
+
+  it "builds successfully with a valid https site_url" do
+    expect { described_class.new(site_url: "https://self-hosted.example.com") }.not_to raise_error
+  end
+
+  it "rejects a non-http(s) scheme" do
+    expect { described_class.new(site_url: "file:///etc/passwd") }
+      .to raise_error(ArgumentError, /must be an http\(s\) URL/)
+  end
+
+  it "rejects a malformed URL" do
+    expect { described_class.new(site_url: "not a url") }
+      .to raise_error(ArgumentError, /not a valid URL/)
+  end
+end

spec/infisical/http_client_spec.rb

@@ -64,6 +64,20 @@
       expect(stub).to have_been_requested.times(2)
     end
 
+    it "honors a Retry-After header instead of computing its own backoff" do
+      waits = []
+      patient_client = described_class.new(base_url: base_url, max_retries: 1, sleeper: lambda { |seconds|
+        waits << seconds
+      })
+      stub_request(:get, url).to_return(
+        { status: 429, body: '{"message":"rate limited"}', headers: { "Retry-After" => "5" } },
+        { status: 200, body: '{"secrets":[]}' }
+      )
+
+      expect(patient_client.get("api/v3/secrets/raw")).to eq({ "secrets" => [] })
+      expect(waits).to eq([5.0])
+    end
+
     it "raises Infisical::APIError after exhausting retries on persistent 429s" do
       stub = stub_request(:get, url).to_return(status: 429, body: '{"message":"rate limited"}')
 

spec/infisical/secrets_spec.rb

@@ -35,6 +35,16 @@
       expect(secret.secret_key).to eq("FOO")
       expect(secret.secret_value).to eq("bar")
     end
+
+    it "URL-encodes secret names containing reserved characters" do
+      stub = stub_request(:get, "#{base_url}/api/v3/secrets/raw/FOO%2FBAR")
+             .with(query: hash_including("workspaceId" => "proj-1"))
+             .to_return(status: 200, body: { secret: { id: "1", secretKey: "FOO/BAR" } }.to_json)
+
+      secrets.get("FOO/BAR", project_id: "proj-1", environment: "dev")
+
+      expect(stub).to have_been_requested
+    end
   end
 
   describe "#create" do
@@ -71,6 +81,11 @@
 
       expect(stub).to have_been_requested
     end
+
+    it "raises ArgumentError when neither secret_value nor new_secret_name is given" do
+      expect { secrets.update("FOO", project_id: "proj-1", environment: "dev") }
+        .to raise_error(ArgumentError, /requires at least one of/)
+    end
   end
 
   describe "#delete" do