Fail closed on malformed trust role values

.github/workflows/downstream-seam-audit.yml

@@ -11,6 +11,7 @@ on:
       - bin/agent-workflow-writing-style
       - bin/agent-workflow-seam-doctor-test.rb
       - bin/agent_doctor/**
+      - skills/pr-batch/lib/github_actor_trust.rb
       - skills/secure-github-actions/lib/**
       - downstream.yml
       - seam-presets.yml

bin/agent-workflow-seam-doctor

@@ -342,8 +342,12 @@ module AgentWorkflowSeamDoctor
   end
 
   def validate_trust_mapping!(trust)
-    trusted_bots = Array(trust.fetch("trusted_bots", [])).map { |login| normalized_trust_bot_login(login) }
-    metadata_bots = Array(trust.fetch("trusted_metadata_bots", [])).map { |login| normalized_trust_bot_login(login) }
+    trusted_bots = strict_trust_role_list(
+      trust.fetch("trusted_bots", []), name: "trusted_bots"
+    ).map { |login| normalized_trust_bot_login(login) }
+    metadata_bots = strict_trust_role_list(
+      trust.fetch("trusted_metadata_bots", []), name: "trusted_metadata_bots"
+    ).map { |login| normalized_trust_bot_login(login) }
     overlap = (trusted_bots & metadata_bots).sort
     return if overlap.empty?
 
@@ -352,6 +356,16 @@ module AgentWorkflowSeamDoctor
           "trusted_metadata_bots: #{overlap.join(', ')}"
   end
 
+  # This executable is intentionally installable without the skill pack, so it
+  # keeps this small validation boundary local while matching GithubActorTrust.
+  def strict_trust_role_list(value, name:)
+    values = value.is_a?(Array) ? value : [value]
+    return [] if value.nil?
+    return values if values.all? { |entry| entry.is_a?(String) && !entry.strip.empty? }
+
+    raise InitError, "#{name} must be a nonempty string or an array of nonempty strings"
+  end
+
   def normalized_trust_bot_login(login)
     login.to_s.delete_prefix("@").downcase.delete_suffix("[bot]")
   end

bin/agent-workflow-seam-doctor-test.rb

@@ -2225,6 +2225,36 @@ def test_regular_check_accepts_scalar_trust_values_for_preflight_compatibility
     end
   end
 
+  def test_regular_check_rejects_malformed_trust_role_values_before_normalization
+    malformed_cases = [
+      ["trusted_bots", { "deploy" => true }],
+      ["trusted_bots", 42],
+      ["trusted_metadata_bots", ["deploy", 42]],
+      ["trusted_metadata_bots", [""]]
+    ]
+
+    malformed_cases.each do |key, value|
+      with_repo("agent-workflow-seam-doctor-trust-role-shape") do |root|
+        write_valid_binstub_contract(root)
+        write_skill(root, "No commands here.\n")
+        trust = {
+          "trusted_users" => [],
+          "trusted_bots" => [],
+          "trusted_metadata_bots" => [],
+          "trusted_teams" => []
+        }
+        trust[key] = value
+        File.write(File.join(root, ".agents/trusted-github-actors.yml"), trust.to_yaml)
+
+        out, status = run_doctor(root)
+
+        refute status.success?, out
+        assert_includes out, "FAIL agent workflow seam has 1 issue(s)"
+        assert_includes out, "#{key} must be a nonempty string or an array of nonempty strings"
+      end
+    end
+  end
+
   def test_regular_check_rejects_overlapping_trust_bot_roles
     with_repo do |root|
       write_valid_binstub_contract(root)

bin/push-downstream

@@ -12,6 +12,7 @@ require "tmpdir"
 require "yaml"
 
 load File.expand_path("agent-workflow-seam-doctor", __dir__)
+require_relative "../skills/pr-batch/lib/github_actor_trust"
 require_relative "../skills/secure-github-actions/lib/secure_github_actions_scanner"
 
 module PushDownstream
@@ -596,9 +597,20 @@ module PushDownstream
   def normalize_trust_config(trust)
     trust = stringify_keys(trust || {})
     TRUST_KEYS.to_h do |key|
-      values = Array(trust[key]).map { |value| normalize_trust_value(key, value) }.reject(&:empty?)
+      raw_values =
+        case key
+        when "trusted_bots", "trusted_metadata_bots"
+          GithubActorTrust.strict_string_list(trust[key], name: key)
+        else
+          Array(trust[key])
+        end
+      values = raw_values.map { |value| normalize_trust_value(key, value) }.reject(&:empty?)
       [key, values.uniq]
     end
+  rescue GithubActorTrust::Error => e
+    # Fleet callers rescue RuntimeError per repository; preserve that public
+    # failure boundary so one malformed consumer contract cannot abort the run.
+    raise e.message.to_s
   end
 
   def normalize_trust_value(key, value)
@@ -2658,87 +2670,92 @@ if $PROGRAM_NAME == __FILE__
   end.parse!
 
   code =
-    if options[:audit]
-      if options[:root] || options[:policy_fleet]
-        warn "--audit cannot be combined with --root or --policy-fleet"
-        1
+    begin
+      if options[:audit]
+        if options[:root] || options[:policy_fleet]
+          warn "--audit cannot be combined with --root or --policy-fleet"
+          1
+        elsif options[:security_audit_fleet]
+          warn "--audit cannot be combined with --security-audit-fleet"
+          1
+        elsif options[:apply]
+          warn "--audit is read-only and cannot be combined with --apply"
+          1
+        elsif !PushDownstream.trust_config_empty?(options[:trust])
+          warn "--trusted-* flags require --root; --audit never writes to a consumer"
+          1
+        else
+          PushDownstream.run_audit(
+            options[:config],
+            options[:presets],
+            only: options[:only],
+            include_disabled: options[:include_disabled]
+          )
+        end
+      elsif options[:root]
+        if options[:policy_fleet] || options[:security_audit_fleet]
+          warn "--policy-fleet and --security-audit-fleet cannot be combined with --root"
+          1
+        else
+          PushDownstream.run_local(
+            options[:root],
+            base_branch: options[:base_branch],
+            trust: options[:trust],
+            apply: options[:apply]
+          )
+        end
       elsif options[:security_audit_fleet]
-        warn "--audit cannot be combined with --security-audit-fleet"
-        1
-      elsif options[:apply]
-        warn "--audit is read-only and cannot be combined with --apply"
-        1
-      elsif !PushDownstream.trust_config_empty?(options[:trust])
-        warn "--trusted-* flags require --root; --audit never writes to a consumer"
-        1
-      else
-        PushDownstream.run_audit(
-          options[:config],
-          options[:presets],
-          only: options[:only],
-          include_disabled: options[:include_disabled]
-        )
-      end
-    elsif options[:root]
-      if options[:policy_fleet] || options[:security_audit_fleet]
-        warn "--policy-fleet and --security-audit-fleet cannot be combined with --root"
-        1
-      else
-        PushDownstream.run_local(
-          options[:root],
-          base_branch: options[:base_branch],
-          trust: options[:trust],
-          apply: options[:apply]
-        )
-      end
-    elsif options[:security_audit_fleet]
-      if options[:apply]
-        warn "--apply cannot be combined with --security-audit-fleet"
-        1
+        if options[:apply]
+          warn "--apply cannot be combined with --security-audit-fleet"
+          1
+        elsif options[:policy_fleet]
+          warn "--policy-fleet cannot be combined with --security-audit-fleet"
+          1
+        elsif options[:include_disabled]
+          warn "--all cannot be combined with --security-audit-fleet; fleet membership is explicit"
+          1
+        elsif !PushDownstream.trust_config_empty?(options[:trust])
+          warn "--trusted-* flags cannot be combined with --security-audit-fleet"
+          1
+        else
+          PushDownstream.run_security_audit_fleet(
+            options[:config],
+            fleet_name: options[:security_audit_fleet],
+            only: options[:only]
+          )
+        end
       elsif options[:policy_fleet]
-        warn "--policy-fleet cannot be combined with --security-audit-fleet"
-        1
-      elsif options[:include_disabled]
-        warn "--all cannot be combined with --security-audit-fleet; fleet membership is explicit"
-        1
-      elsif !PushDownstream.trust_config_empty?(options[:trust])
-        warn "--trusted-* flags cannot be combined with --security-audit-fleet"
-        1
-      else
-        PushDownstream.run_security_audit_fleet(
-          options[:config],
-          fleet_name: options[:security_audit_fleet],
-          only: options[:only]
-        )
-      end
-    elsif options[:policy_fleet]
-      if options[:include_disabled]
-        warn "--all cannot be combined with --policy-fleet; fleet membership is explicit"
-        1
-      elsif !PushDownstream.trust_config_empty?(options[:trust])
-        warn "--trusted-* flags require --root; policy fleets read only their registered policy values"
-        1
+        if options[:include_disabled]
+          warn "--all cannot be combined with --policy-fleet; fleet membership is explicit"
+          1
+        elsif !PushDownstream.trust_config_empty?(options[:trust])
+          warn "--trusted-* flags require --root; policy fleets read only their registered policy values"
+          1
+        else
+          PushDownstream.run_policy_fleet(
+            options[:config],
+            fleet_name: options[:policy_fleet],
+            only: options[:only],
+            apply: options[:apply]
+          )
+        end
       else
-        PushDownstream.run_policy_fleet(
+        unless PushDownstream.trust_config_empty?(options[:trust])
+          warn "--trusted-* flags require --root; use downstream.yml or seam-presets.yml trust blocks in registry mode"
+          exit 1
+        end
+
+        PushDownstream.run_registry(
           options[:config],
-          fleet_name: options[:policy_fleet],
+          options[:presets],
           only: options[:only],
+          include_disabled: options[:include_disabled],
           apply: options[:apply]
         )
       end
-    else
-      unless PushDownstream.trust_config_empty?(options[:trust])
-        warn "--trusted-* flags require --root; use downstream.yml or seam-presets.yml trust blocks in registry mode"
-        exit 1
-      end
-
-      PushDownstream.run_registry(
-        options[:config],
-        options[:presets],
-        only: options[:only],
-        include_disabled: options[:include_disabled],
-        apply: options[:apply]
-      )
+    rescue RuntimeError => e
+      warn "FAIL: #{e.message}"
+      1
     end
 
   exit code

bin/push-downstream-test.rb

@@ -72,6 +72,7 @@ def test_workflow_is_a_read_only_audit_without_a_publisher_surface
       "bin/agent-workflow-writing-style",
       "bin/agent-workflow-seam-doctor-test.rb",
       "bin/agent_doctor/**",
+      "skills/pr-batch/lib/github_actor_trust.rb",
       "skills/secure-github-actions/lib/**",
       "downstream.yml",
       "seam-presets.yml"
@@ -242,8 +243,7 @@ def test_registry_dry_run_reports_invalid_contract_without_aborting_valid_repos
         - repo: bad
           overrides:
             trust:
-              trusted_bots: [github-actions]
-              trusted_metadata_bots: [github-actions]
+              trusted_metadata_bots: [github-actions, 42]
     YAML
 
     with_config(yaml) do |path|
@@ -255,7 +255,7 @@ def test_registry_dry_run_reports_invalid_contract_without_aborting_valid_repos
       assert_equal 1, @registry_status
       assert_includes out, "shakacode/good"
       refute_includes out, "shakacode/bad"
-      assert_includes err, "FAIL shakacode/bad: invalid trust config"
+      assert_includes err, "FAIL shakacode/bad: trusted_metadata_bots must be a nonempty string or an array of nonempty strings"
     end
   end
 
@@ -270,8 +270,7 @@ def test_registry_apply_continues_after_invalid_contract
         - repo: bad
           overrides:
             trust:
-              trusted_bots: [github-actions]
-              trusted_metadata_bots: [github-actions]
+              trusted_metadata_bots: [github-actions, 42]
     YAML
 
     with_config(yaml) do |path|
@@ -289,7 +288,7 @@ def test_registry_apply_continues_after_invalid_contract
 
         assert_equal 1, @registry_status
         assert_equal ["shakacode/good"], calls
-        assert_includes err, "FAIL shakacode/bad: invalid trust config"
+        assert_includes err, "FAIL shakacode/bad: trusted_metadata_bots must be a nonempty string or an array of nonempty strings"
       end
     end
   end
@@ -968,6 +967,27 @@ def test_resolve_contract_rejects_bot_metadata_overlap
     assert_match(/bot\(s\) listed in both trusted_bots and trusted_metadata_bots: github-actions/, error.message)
   end
 
+  def test_resolve_contract_rejects_malformed_bot_role_values
+    presets = {
+      "defaults" => {
+        "trust" => {
+          "trusted_bots" => ["dependabot"],
+          "trusted_metadata_bots" => ["github-actions", 42]
+        }
+      }
+    }
+    repo = {
+      repo: "rsc", base_branch: "main", preset: nil,
+      overrides: { "trust" => {} }
+    }
+
+    error = assert_raises(RuntimeError) do
+      PushDownstream.resolve_contract(repo, presets)
+    end
+
+    assert_match(/trusted_metadata_bots must be a nonempty string or an array of nonempty strings/, error.message)
+  end
+
   def test_resolve_contract_unknown_preset_raises
     error = assert_raises(RuntimeError) do
       PushDownstream.resolve_contract(
@@ -3974,6 +3994,14 @@ def test_registry_mode_rejects_cli_trust_flags
     assert_includes out, "--trusted-* flags require --root"
   end
 
+  def test_empty_trusted_bot_flag_fails_without_a_backtrace
+    out, status = run_cli("--trusted-bot", "")
+
+    refute status.success?, out
+    assert_includes out, "FAIL: trusted_bots must be a nonempty string or an array of nonempty strings"
+    refute_match(/(?:RuntimeError|Traceback|from .*push-downstream)/, out)
+  end
+
   def test_local_apply_reports_invalid_trust_config_without_backtrace
     Dir.mktmpdir("push-downstream-cli") do |root|
       FileUtils.mkdir_p(File.join(root, ".agents"))

skills/pr-batch/bin/github-actor-trust-test.rb

@@ -68,6 +68,27 @@ def test_overlapping_bot_classification_fails_closed
     assert_match(/listed in both/, error.message)
   end
 
+  def test_bot_roles_reject_malformed_values_before_normalization
+    {
+      "trusted_bots: { deploy: true }\n" => "trusted_bots",
+      "trusted_bots: 42\n" => "trusted_bots",
+      "trusted_metadata_bots: [github-actions, 42]\n" => "trusted_metadata_bots",
+      "trusted_metadata_bots: ['']\n" => "trusted_metadata_bots",
+      "trusted_bots: ['   ']\n" => "trusted_bots"
+    }.each do |yaml, role|
+      error = assert_raises(GithubActorTrust::Error) { config(yaml) }
+
+      assert_equal "#{role} must be a nonempty string or an array of nonempty strings", error.message
+    end
+  end
+
+  def test_bot_roles_preserve_legacy_scalar_string_compatibility
+    loaded = config("trusted_bots: deploy\ntrusted_metadata_bots: github-actions\n")
+
+    assert_equal Set["deploy"], loaded.fetch(:trusted_bots)
+    assert_equal Set["github-actions"], loaded.fetch(:trusted_metadata_bots)
+  end
+
   # A stray blank list item parses to nil; that must not crash out of the
   # Error contract both callers rescue on.
   def test_blank_team_entry_is_ignored_rather_than_crashing