Security sub-agent should detect CI trust model expansions from dynamic config discovery

[fullsend-ai-retro]

What happened

On PR #145, the fullsend review agent's security sub-agent reviewed a change that replaced hardcoded agent names in functional-tests.yml with dynamic selection via select-eval-agents.sh. The agent found a GHEOF heredoc injection risk (medium) but did not flag a broader trust model concern.

A human reviewer (waynesun09's review squad) identified that the dynamic pattern means adding a harness/<name>.yaml + eval/<name>/eval.yaml pair in a single PR grants that eval config code execution with GCP WIF credentials and EVAL_GH_TOKEN — with no workflow file diff to signal the trust expansion to reviewers doing ok-to-test authorization.

The security sub-agent's prompt (skills/pr-review/sub-agents/security.md) has explicit guidance for permission expansion in workflow permissions: blocks and secrets: blocks, but does not cover the pattern where dynamic dispatch extends an existing trust boundary to new configuration files without modifying the workflow file itself.

What could go better

The security sub-agent should recognize that dynamic CI dispatch patterns (where configuration files control which code runs with secrets) create a new attack surface: any PR that adds a qualifying config file inherits the credential scope of the dispatch mechanism. This is analogous to the existing secrets: block check but operates at a higher level of abstraction — the workflow file grants credentials once, and configuration files determine who benefits.

Confidence: High. The human reviewer explicitly identified this as an unstated security tradeoff. The security sub-agent's own rules require flagging permission expansions even when internally consistent, and this qualifies — the blast radius of ok-to-test labeling expanded without a workflow diff to surface it.

Uncertainty: The security sub-agent may not have had visibility into the full pull_request_target trust model to reason about this. It may also be that this class of finding (architectural trust model shift rather than code-level vulnerability) is genuinely out of scope for a code-level security review.

Proposed change

Add a heuristic to the security sub-agent prompt (skills/pr-review/sub-agents/security.md) under a new section titled Dynamic dispatch trust expansion:

## Dynamic dispatch trust expansion

**Category:** `trust-model-expansion`

When the diff introduces or modifies a pattern where configuration files
(YAML, JSON, etc.) control which code runs in a CI workflow that has
access to secrets or cloud credentials, flag it as a trust model
expansion:

- Identify which secrets/credentials the workflow grants
- Determine whether adding a new configuration file would grant a new
  code path access to those credentials
- If yes, flag as **medium** with a note about the review safeguard
  implications (e.g., no workflow file diff for reviewers to notice)

This pattern commonly appears as: glob-based or directory-enumeration
loops that execute scripts based on config file presence, matrix
strategies populated from repository contents, or dynamic workflow
dispatch based on file discovery.

This complements the existing secrets: block and permissions: block checks by covering the indirect case where credentials are granted once and configuration files determine scope.

Validation criteria

On the next PR that introduces dynamic CI dispatch with credential inheritance (or when PR #148 is re-reviewed), the security sub-agent should produce a finding about trust model expansion. Verify by checking the security sub-agent's output in the review comment for a trust-model-expansion category finding.

---

Generated by retro agent from #145

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 5:08 PM UTC · Completed 5:11 PM UTC
Commit: bd28f41 · View workflow run →

[fullsend-ai-triage]

Triage Summary

Severity: Medium · Category: Feature (prompt enhancement)

Problem

The security sub-agent prompt covers permission expansion in workflow permissions: and secrets: blocks but misses an indirect pattern: dynamic dispatch mechanisms (glob-based config discovery, directory enumeration, matrix strategies from repo contents) that extend an existing trust boundary to new configuration files without modifying the workflow file itself.

PR #145 demonstrated this gap — adding a harness/<name>.yaml + eval/<name>/eval.yaml pair silently grants code execution with GCP WIF credentials and EVAL_GH_TOKEN, with no workflow diff to alert reviewers.

Recommended Fix

Add a Dynamic dispatch trust expansion section to skills/pr-review/sub-agents/security.md with:

• Category: trust-model-expansion
• Heuristic: when config files control which code runs in a CI workflow with secret access, flag it
• Guidance to identify granted credentials, assess whether new config files inherit them, and flag with review-safeguard implications

The proposed prompt text in the issue body is well-specified and complements the existing permission/role checks.

Proposed Test Case

Scenario: Dynamic dispatch trust expansion detection
  Given a workflow that grants secrets and discovers execution targets
        via glob or directory enumeration
  When a PR adds a new configuration file that qualifies for execution
       by the dispatch mechanism
  Then the security sub-agent should produce a finding with category
       "trust-model-expansion"
  And the finding should identify which credentials the new config
      file would inherit
  And severity should be at least "medium"

Validation can be done by re-reviewing PR #148 (which introduces the dynamic matrix strategy) with the updated prompt, or via a synthetic functional test case.

---

Labels: This is an enhancement to the security sub-agent prompt. Removing ready-for-triage as triage is complete.

[fullsend-ai-prioritize]

RICE Priority Score: 3.6

Score breakdown

Dimension	Score	Reasoning
Reach	1.5	The security sub-agent reviews all PRs to the agents repo, so this gap affects every contributor relying on automated security review. Dynamic dispatch patterns are an emerging pattern in this repo (PR #145, #148), and the trust model gap means any future PR adding config files to these patterns would bypass automated security scrutiny. Scored at 1.5 because it affects the security posture across the project but the specific pattern frequency is moderate.
Impact	1.5	A missed trust model expansion means new config files can silently inherit GCP WIF credentials and EVAL_GH_TOKEN access with no workflow diff to trigger reviewer attention during ok-to-test authorization. This is a significant security gap — the human reviewer explicitly called it out as an unstated security tradeoff. Scored at 1.5 because it's a meaningful security improvement, though human reviewers provide a backstop.
Confidence	0.8	High confidence at 0.8. The issue is exceptionally well-described: it identifies the exact file to modify, provides ready-to-use prompt text, specifies validation criteria, and is backed by a concrete real-world miss on PR #145. The triage agent confirmed the analysis. The only gap is whether the prompt addition will reliably trigger the desired LLM behavior in practice.
Effort	0.5	This is a prompt-only change to a single file (skills/pr-review/sub-agents/security.md), and the proposed text is already written in the issue body. The work is adding a new section with clear heuristics. Scored at 0.5 because it's a small, well-scoped change with minimal implementation risk.

Formula: (1.5 x 1.5 x 0.8) / 0.5 = 3.6