Fix trusted publisher reusable workflow refs (#6346)

* Fix trusted publisher verification for cross-repo reusable workflows

The job_workflow_ref OIDC claim contains the reusable workflow's ref, not the caller's ref/sha. Extract the ref from job_workflow_ref and job_workflow_sha instead when verifying cross-repo reusable workflows.

* Fix trusted publisher token exchange for cross-repo reusable workflows

Also fixes inconsistent error handling

* Harden job_workflow_refs prefix validation and improve test coverage

* Fixup linting

Author: colby-swandale

app/models/oidc/trusted_publisher/github_action.rb

@@ -128,8 +128,8 @@ def job_workflow_ref_condition(ref)
 
   def to_access_policy(jwt)
     common_conditions = [repository_condition, environment_condition, repository_owner_id_condition, audience_condition].compact
-    refs = [jwt.fetch(:ref), jwt.fetch(:sha)].compact_blank
-    raise OIDC::AccessPolicy::AccessError, "ref and sha are both missing" if refs.empty?
+    refs = job_workflow_refs(jwt)
+    raise OIDC::AccessPolicy::AccessError, "no workflow refs could be extracted from the JWT claims" if refs.empty?
     OIDC::AccessPolicy.new(
       statements: refs.map do |ref|
         OIDC::AccessPolicy::Statement.new(
@@ -183,6 +183,31 @@ def owns_gem?(rubygem) = rubygem_trusted_publishers.exists?(rubygem: rubygem)
 
   private
 
+  # For same-repo workflows, the JWT's ref/sha (from the caller repo) match
+  # the workflow's ref in job_workflow_ref, so we use them directly.
+  #
+  # For cross-repo reusable workflows, the JWT's ref/sha belong to the caller
+  # repo, while job_workflow_ref contains the reusable workflow's own ref.
+  # We strip the known prefix to extract the ref, validating it matches the
+  # loaded publisher's workflow_repository/workflow_slug.
+  def job_workflow_refs(jwt)
+    if workflow_repository_owner.present?
+      refs = []
+      if (jwf_ref = jwt[:job_workflow_ref])
+        expected_prefix = "#{workflow_repository}/#{workflow_slug}@"
+        unless jwf_ref.start_with?(expected_prefix)
+          raise OIDC::AccessPolicy::AccessError,
+            "job_workflow_ref #{jwf_ref} does not match expected prefix #{expected_prefix}"
+        end
+        refs << jwf_ref.delete_prefix(expected_prefix)
+      end
+      refs << jwt[:job_workflow_sha] if jwt[:job_workflow_sha].present?
+      refs.compact_blank
+    else
+      [jwt[:ref], jwt[:sha]].compact_blank
+    end
+  end
+
   def find_github_repository_owner_id
     return if repository_owner.blank?
     return if repository_owner_id.present?

test/integration/api/v1/oidc/trusted_publisher_controller_test.rb

@@ -274,7 +274,9 @@ def jwt(claims = @claims, key: @pkey)
       @claims["repository"] = "cseeman/my-gem"
       @claims["repository_owner"] = "cseeman"
       @claims["repository_owner_id"] = "1946610"
-      @claims["job_workflow_ref"] = "rubygems/shared-workflows/.github/workflows/release.yml@refs/heads/main"
+      # Reusable workflow ref differs from caller's ref (refs/heads/main)
+      @claims["job_workflow_ref"] = "rubygems/shared-workflows/.github/workflows/release.yml@refs/tags/v2.0"
+      @claims["job_workflow_sha"] = "aaabbbccc111222333"
 
       trusted_publisher = build(:oidc_trusted_publisher_github_action,
         repository_name: "my-gem",
@@ -293,6 +295,10 @@ def jwt(claims = @claims, key: @pkey)
       resp = response.parsed_body
 
       assert_match(/^rubygems_/, resp["rubygems_api_key"])
+
+      api_key = trusted_publisher.api_keys.sole
+
+      assert_equal api_key.owner, trusted_publisher
     end
 
     should "return not found when reusable workflow does not match configured workflow_repository" do
@@ -316,6 +322,49 @@ def jwt(claims = @claims, key: @pkey)
       assert_response :not_found
     end
 
+    should "succeed but only grant access to attacker's own publisher when using same reusable workflow" do
+      # Two publishers share the same reusable workflow but are bound to different caller repos.
+      # An attacker's JWT from their own repo must not match the victim's publisher.
+      victim_publisher = build(:oidc_trusted_publisher_github_action,
+        repository_name: "victim-gem",
+        repository_owner_id: "111111",
+        workflow_filename: "release.yml",
+        workflow_repository_owner: "rubygems",
+        workflow_repository_name: "shared-workflows")
+      victim_publisher.repository_owner = "victim-org"
+      victim_publisher.save!
+
+      attacker_publisher = build(:oidc_trusted_publisher_github_action,
+        repository_name: "attacker-repo",
+        repository_owner_id: "999999",
+        workflow_filename: "release.yml",
+        workflow_repository_owner: "rubygems",
+        workflow_repository_name: "shared-workflows")
+      attacker_publisher.repository_owner = "attacker-org"
+      attacker_publisher.save!
+
+      # JWT comes from the attacker's repo using the same shared workflow
+      @claims["repository"] = "attacker-org/attacker-repo"
+      @claims["repository_owner"] = "attacker-org"
+      @claims["repository_owner_id"] = "999999"
+      @claims["job_workflow_ref"] = "rubygems/shared-workflows/.github/workflows/release.yml@refs/heads/main"
+      @claims["job_workflow_sha"] = "aaa111bbb222"
+
+      post api_v1_oidc_trusted_publisher_exchange_token_path,
+        params: { jwt: jwt.to_s }
+
+      # Should succeed — but only grant access to the attacker's own publisher, not the victim's
+      assert_response :success
+
+      resp = response.parsed_body
+
+      assert_match(/^rubygems_/, resp["rubygems_api_key"])
+
+      # The API key must belong to the attacker's publisher, not the victim's
+      assert_equal 1, attacker_publisher.api_keys.count
+      assert_equal 0, victim_publisher.api_keys.count
+    end
+
     should "return not found when reusable workflow used but trusted publisher expects same-repo workflow" do
       @claims["repository"] = "cseeman/my-gem"
       @claims["repository_owner"] = "cseeman"

test/models/oidc/trusted_publisher/github_action_test.rb

@@ -355,26 +355,100 @@ class OIDC::TrustedPublisher::GitHubActionTest < ActiveSupport::TestCase
     assert_equal "example/my-gem", publisher.workflow_repository
   end
 
-  test "#to_access_policy with reusable workflow" do
+  test "#to_access_policy with reusable workflow uses reusable workflow refs" do
     publisher = create(:oidc_trusted_publisher_github_action,
       repository_owner: "caller-org",
       repository_name: "caller-repo",
       workflow_filename: "shared-release.yml",
       workflow_repository_owner: "shared-org",
       workflow_repository_name: "shared-workflows")
 
-    policy = publisher.to_access_policy({ ref: "refs/heads/main", sha: "abc123" })
+    # Caller ref/sha differ from the reusable workflow's ref/sha
+    policy = publisher.to_access_policy(
+      ref: "refs/heads/main", sha: "caller-sha-111",
+      job_workflow_ref: "shared-org/shared-workflows/.github/workflows/shared-release.yml@refs/tags/v1.0",
+      job_workflow_sha: "reusable-sha-222"
+    )
+
+    # Should create statements using the reusable workflow's ref and sha, NOT the caller's
+    assert_equal 2, policy.statements.length
 
-    # Verify job_workflow_ref points to the shared workflow repo, not the caller repo
-    first_statement = policy.statements.first
-    job_workflow_ref_condition = first_statement.conditions.find { |c| c.claim == "job_workflow_ref" }
+    first_condition = policy.statements.first.conditions.find { |c| c.claim == "job_workflow_ref" }
 
-    assert_equal "shared-org/shared-workflows/.github/workflows/shared-release.yml@refs/heads/main",
-                 job_workflow_ref_condition.value
+    assert_equal "shared-org/shared-workflows/.github/workflows/shared-release.yml@refs/tags/v1.0",
+                 first_condition.value
+
+    second_condition = policy.statements.second.conditions.find { |c| c.claim == "job_workflow_ref" }
+
+    assert_equal "shared-org/shared-workflows/.github/workflows/shared-release.yml@reusable-sha-222",
+                 second_condition.value
 
     # Verify repository condition still points to caller repo (security)
-    repository_condition = first_statement.conditions.find { |c| c.claim == "repository" }
+    repository_condition = policy.statements.first.conditions.find { |c| c.claim == "repository" }
 
     assert_equal "caller-org/caller-repo", repository_condition.value
   end
+
+  test "#to_access_policy with reusable workflow without job_workflow_sha" do
+    publisher = create(:oidc_trusted_publisher_github_action,
+      repository_owner: "caller-org",
+      repository_name: "caller-repo",
+      workflow_filename: "shared-release.yml",
+      workflow_repository_owner: "shared-org",
+      workflow_repository_name: "shared-workflows")
+
+    policy = publisher.to_access_policy(
+      ref: "refs/heads/main", sha: "caller-sha-111",
+      job_workflow_ref: "shared-org/shared-workflows/.github/workflows/shared-release.yml@refs/tags/v1.0"
+    )
+
+    assert_equal 1, policy.statements.length
+
+    condition = policy.statements.first.conditions.find { |c| c.claim == "job_workflow_ref" }
+
+    assert_equal "shared-org/shared-workflows/.github/workflows/shared-release.yml@refs/tags/v1.0",
+                 condition.value
+  end
+
+  test "#to_access_policy same-repo workflow uses caller ref and sha" do
+    publisher = create(:oidc_trusted_publisher_github_action,
+      repository_owner: "example",
+      repository_name: "my-gem",
+      workflow_filename: "release.yml",
+      workflow_repository_owner: nil,
+      workflow_repository_name: nil)
+
+    policy = publisher.to_access_policy({ ref: "refs/heads/main", sha: "abc123" })
+
+    assert_equal 2, policy.statements.length
+
+    first_condition = policy.statements.first.conditions.find { |c| c.claim == "job_workflow_ref" }
+
+    assert_equal "example/my-gem/.github/workflows/release.yml@refs/heads/main",
+                 first_condition.value
+
+    second_condition = policy.statements.second.conditions.find { |c| c.claim == "job_workflow_ref" }
+
+    assert_equal "example/my-gem/.github/workflows/release.yml@abc123",
+                 second_condition.value
+  end
+
+  test "#to_access_policy reusable workflow rejects mismatched prefix" do
+    publisher = create(:oidc_trusted_publisher_github_action,
+      repository_owner: "caller-org",
+      repository_name: "caller-repo",
+      workflow_filename: "shared-release.yml",
+      workflow_repository_owner: "shared-org",
+      workflow_repository_name: "shared-workflows")
+
+    # job_workflow_ref has a different workflow repo than expected
+    assert_raises(OIDC::AccessPolicy::AccessError) do
+      publisher.to_access_policy(
+        ref: "refs/heads/main",
+        sha: "caller-sha",
+        job_workflow_ref: "attacker-org/evil-workflows/.github/workflows/shared-release.yml@refs/heads/main",
+        job_workflow_sha: "attacker-sha-123"
+      )
+    end
+  end
 end