Skip to content

Review agent should use comment-only disposition for PRs that expand fullsend trust boundaries #240

Description

@fullsend-ai-retro

What happened

On PR #49 (review workflow run), the review agent analyzed a change that added a new external organization (redhat-community-ai-tools/qualityflow-fullsend) to both the agents list and allowed_remote_resources in .fullsend/config.yaml, plus a new .fullsend/qualityflow.yaml with a base: directive pointing to that external repo. The agent's thinking log noted: 'the allowed_remote_resources entry uses a prefix match without a commit pin, so the base agent could pull in other resources from the repository without that same level of pinning protection.' Despite identifying this trust boundary expansion, the agent chose 'approve' because 'the existing entries follow the same pattern.' The PR was then self-merged within 5 minutes of opening, with no human review.

What could go better

Changes to .fullsend/config.yaml that add new organizations to allowed_remote_resources or register new agent source URLs from previously-untrusted repos are security-sensitive — they expand which external code can execute in the fullsend pipeline. The review agent's own analysis correctly flagged this as a trust boundary concern, but its approval policy treated pattern conformance as sufficient for approval. For this class of change, the agent should signal that human sign-off is warranted by using 'comment' disposition rather than 'approve', while still providing its full analysis. This is distinct from agents#155 (which covers dynamic CI dispatch trust model shifts in workflow files) — this proposal targets the review agent's verdict policy specifically for .fullsend configuration changes that widen the remote resource trust boundary. Confidence: medium. The change in PR #49 was likely fine (well-known org, commit-pinned with SHA256), but the agent's own security analysis supported a more cautious disposition.

Proposed change

Add guidance to the review agent's policy or skill definitions (e.g., policies/review.yaml or skills/pr-review/SKILL.md) specifying that when a PR modifies .fullsend/config.yaml to add a new prefix to allowed_remote_resources or adds a new agent source: URL referencing an organization not already present in the existing config, the review agent should use 'comment' disposition rather than 'approve'. The agent should still provide its full analysis (hash consistency checks, pattern conformance, trust boundary assessment) but explicitly note that trust boundary expansions require human approval. Example policy clause: 'If the diff adds a new organization-level prefix to allowed_remote_resources or introduces an agent source from a previously-unlisted organization, use comment disposition and include a finding noting the trust boundary expansion.'

Validation criteria

On a test PR that adds a new organization to allowed_remote_resources in .fullsend/config.yaml, the review agent should produce a 'comment' disposition (not 'approve') with a finding that flags the trust boundary expansion. The review should still contain the full analysis. PRs that only update existing pinned references (same organization, new commit SHA) should continue to be eligible for approval.


Generated by retro agent from rh-hemartin-fullsendai/standalone-fullsend#49

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions