Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion guards/github-guard/rust-guard/src/labels/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,10 @@ pub fn commit_integrity(
/// - github-actions: GitHub Actions workflow actor (without [bot] suffix, as returned by some APIs)
/// - app/github-actions: GitHub Actions workflow actor (with app/ prefix, as returned by gh CLI)
/// - github-merge-queue[bot]: GitHub merge queue automation
/// - copilot: GitHub Copilot AI assistant
/// - copilot: GitHub Copilot coding agent (app login)
/// - copilot-swe-agent[bot]: GitHub Copilot SWE agent (bot user login from REST API)
/// - copilot-swe-agent: GitHub Copilot SWE agent (without [bot] suffix)
/// - app/copilot-swe-agent: GitHub Copilot SWE agent (with app/ prefix, as returned by gh CLI)
pub fn is_trusted_first_party_bot(username: &str) -> bool {
let lower = username.to_lowercase();
lower == "dependabot[bot]"
Expand All @@ -1238,6 +1241,9 @@ pub fn is_trusted_first_party_bot(username: &str) -> bool {
|| lower == "app/github-actions"
|| lower == "github-merge-queue[bot]"
|| lower == "copilot"
|| lower == "copilot-swe-agent[bot]"
|| lower == "copilot-swe-agent"
|| lower == "app/copilot-swe-agent"
}

/// Check if a user is in the gateway-configured trusted bot list.
Expand Down
14 changes: 9 additions & 5 deletions guards/github-guard/rust-guard/src/labels/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,11 +833,15 @@ mod tests {
assert!(is_trusted_first_party_bot("github-actions[bot]"));
assert!(is_trusted_first_party_bot("github-merge-queue[bot]"));
assert!(is_trusted_first_party_bot("copilot"));
assert!(is_trusted_first_party_bot("copilot-swe-agent[bot]"));
assert!(is_trusted_first_party_bot("copilot-swe-agent"));
assert!(is_trusted_first_party_bot("app/copilot-swe-agent"));

// Case-insensitive
assert!(is_trusted_first_party_bot("Dependabot[bot]"));
assert!(is_trusted_first_party_bot("GitHub-Actions[bot]"));
assert!(is_trusted_first_party_bot("Copilot"));
assert!(is_trusted_first_party_bot("Copilot-Swe-Agent[bot]"));

// Not trusted (third-party bots)
assert!(!is_trusted_first_party_bot("renovate[bot]"));
Expand Down Expand Up @@ -1020,13 +1024,13 @@ mod tests {
let repo = "github/copilot";

let ctx_with_bots = PolicyContext {
trusted_bots: vec!["copilot-swe-agent[bot]".to_string()],
trusted_bots: vec!["my-deploy-bot[bot]".to_string()],
..Default::default()
};

// A configured trusted bot issue gets approved (writer) integrity even with NONE association
let configured_bot_issue = json!({
"user": {"login": "copilot-swe-agent[bot]"},
"user": {"login": "my-deploy-bot[bot]"},
"author_association": "NONE"
});
assert_eq!(
Expand All @@ -1036,7 +1040,7 @@ mod tests {

// Case-insensitive match
let upper_bot_issue = json!({
"user": {"login": "COPILOT-SWE-AGENT[BOT]"},
"user": {"login": "MY-DEPLOY-BOT[BOT]"},
"author_association": "NONE"
});
assert_eq!(
Expand All @@ -1057,13 +1061,13 @@ mod tests {
let repo = "github/copilot";

let ctx_with_bots = PolicyContext {
trusted_bots: vec!["copilot-swe-agent[bot]".to_string()],
trusted_bots: vec!["my-deploy-bot[bot]".to_string()],
..Default::default()
};

// A configured trusted bot PR gets approved (writer) integrity even with NONE association
let configured_bot_pr = json!({
"user": {"login": "copilot-swe-agent[bot]"},
"user": {"login": "my-deploy-bot[bot]"},
"author_association": "NONE"
});
assert_eq!(
Expand Down
3 changes: 3 additions & 0 deletions internal/proxy/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ var graphqlPatterns = []graphqlPattern{
{queryPattern: regexp.MustCompile(`(?i)repository\s*\([^)]*\)\s*\{[^}]*\bpullRequest\s*\(`), toolName: "pull_request_read"},
{queryPattern: regexp.MustCompile(`(?i)repository\s*\([^)]*\)\s*\{[^}]*\bpullRequests\s*[\({]`), toolName: "list_pull_requests"},

// Commit history operations
{queryPattern: regexp.MustCompile(`(?i)\bhistory\s*[\({]`), toolName: "list_commits"},

Comment on lines +50 to +52

Copilot AI Mar 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new list_commits GraphQL routing pattern is introduced here, but there isn’t corresponding unit test coverage in the existing TestMatchGraphQL tables to ensure commit-history queries are classified as list_commits (and that other patterns still win when combined). Adding a focused test case for a typical ... on Commit { history(first:...) { nodes { ... } } } query would prevent regressions.

Copilot uses AI. Check for mistakes.
// Discussion operations
{queryPattern: regexp.MustCompile(`(?i)repository\s*\([^)]*\)\s*\{[^}]*\bdiscussion\s*\(`), toolName: "list_discussions"},
{queryPattern: regexp.MustCompile(`(?i)repository\s*\([^)]*\)\s*\{[^}]*\bdiscussions\s*[\({]`), toolName: "list_discussions"},
Expand Down
61 changes: 41 additions & 20 deletions internal/proxy/graphql_rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,57 @@ import (
"strings"
)

// guardRequiredFields lists the GraphQL selection fields the DIFC guard needs
// for accurate integrity labeling. author{login} enables trusted-bot detection;
// authorAssociation provides the integrity level directly (MEMBER, CONTRIBUTOR,
// etc.) so the guard doesn't need extra enrichment REST round-trips.
var guardRequiredFields = []struct {
// guardFieldSet defines the GraphQL fields the DIFC guard needs for a
// specific class of GitHub objects.
type guardFieldSet struct {
field string // field text to inject
present *regexp.Regexp // pattern that indicates the field is already selected
}{
}

// issueAndPRFields are required for Issue and PullRequest types.
// author{login} enables trusted-bot detection; authorAssociation provides the
// integrity level directly so the guard doesn't need enrichment REST round-trips.
var issueAndPRFields = []guardFieldSet{
{"author{login}", regexp.MustCompile(`\bauthor\s*\{[^}]*\blogin\b`)},
{"authorAssociation", regexp.MustCompile(`\bauthorAssociation\b`)},
}

// allGuardFieldsPresent returns true if the query already contains every
// required guard field.
func allGuardFieldsPresent(query string) bool {
for _, f := range guardRequiredFields {
// commitFields are required for Commit types.
// author{user{login}} enables trusted-bot detection. Commits don't have an
// authorAssociation field in the GraphQL schema.
var commitFields = []guardFieldSet{
{"author{user{login}}", regexp.MustCompile(`\bauthor\s*\{[^}]*\buser\s*\{[^}]*\blogin\b`)},

Copilot AI Mar 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The present regexp for commit fields will never match a real author{user{login}} selection because it uses [^}]* around nested braces; the first [^}]* will greedily consume through user{login and then stop before the inner }, leaving no user{ token for the remainder of the pattern. This causes InjectGuardFields to think the field is missing and inject it again, potentially producing invalid GraphQL or duplicated selections. Consider using non-greedy quantifiers (e.g. [^}]*?) or a different presence check that can handle nested selections reliably.

Suggested change
{"author{user{login}}", regexp.MustCompile(`\bauthor\s*\{[^}]*\buser\s*\{[^}]*\blogin\b`)},
{"author{user{login}}", regexp.MustCompile(`(?s)\bauthor\b.*\buser\b.*\blogin\b`)},

Copilot uses AI. Check for mistakes.
}

// fieldsForTool returns the guard fields applicable to the given tool name,
// or nil if no injection is needed.
func fieldsForTool(toolName string) []guardFieldSet {
switch toolName {
case "list_issues", "list_pull_requests", "issue_read", "pull_request_read",
"search_issues":
return issueAndPRFields
case "list_commits":
return commitFields
default:
return nil
}
}

// allFieldsPresent returns true if the query already contains every
// required guard field from the given set.
func allFieldsPresent(query string, fields []guardFieldSet) bool {
for _, f := range fields {
if !f.present.MatchString(query) {
return false
}
}
return true
}

// missingGuardFields returns the field strings not yet present in the query.
func missingGuardFields(query string) []string {
// missingFields returns the field strings from the set not yet present in the query.
func missingFields(query string, fields []guardFieldSet) []string {
var missing []string
for _, f := range guardRequiredFields {
for _, f := range fields {
if !f.present.MatchString(query) {
missing = append(missing, f.field)
}
Expand All @@ -45,11 +69,8 @@ func missingGuardFields(query string) []string {
// Returns the (possibly modified) body. If injection is not needed or fails,
// the original body is returned unchanged.
func InjectGuardFields(body []byte, toolName string) []byte {
// Only rewrite for tools that need author info
switch toolName {
case "list_issues", "list_pull_requests", "issue_read", "pull_request_read",
"search_issues":
default:
fields := fieldsForTool(toolName)
if fields == nil {
return body
}

Expand All @@ -58,11 +79,11 @@ func InjectGuardFields(body []byte, toolName string) []byte {
return body
}

if gql.Query == "" || allGuardFieldsPresent(gql.Query) {
if gql.Query == "" || allFieldsPresent(gql.Query, fields) {
return body
}

missing := missingGuardFields(gql.Query)
missing := missingFields(gql.Query, fields)
modified := injectFieldsIntoQuery(gql.Query, missing)
if modified == gql.Query {
return body
Expand Down
44 changes: 44 additions & 0 deletions internal/proxy/graphql_rewrite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ func TestInjectGuardFields_SkipsWhenFieldsPresent(t *testing.T) {
assert.Equal(t, body, result)
}

func TestInjectGuardFields_SkipsWhenCommitFieldsPresent(t *testing.T) {
query := `query { repository(owner:"o", name:"r") { defaultBranchRef { target { ... on Commit { history(first:10) { nodes { oid author{user{login}} } } } } } } }`
body, _ := json.Marshal(GraphQLRequest{Query: query})
result := InjectGuardFields(body, "list_commits")
assert.Equal(t, body, result)
}

func TestInjectGuardFields_InjectsIntoNodes(t *testing.T) {
query := `query { repository(owner:"o", name:"r") { pullRequests(first:10) { nodes { number title } } } }`
body, _ := json.Marshal(GraphQLRequest{Query: query})
Expand Down Expand Up @@ -147,6 +154,43 @@ query { repository(owner:"o",name:"r") { pullRequests(first:1) { nodes { ...pr }
assert.Contains(t, result, "author{login},authorAssociation}")
}

func TestInjectGuardFields_CommitInjectsAuthorUser(t *testing.T) {
query := `query { repository(owner:"o", name:"r") { defaultBranchRef { target { ... on Commit { history(first:10) { nodes { oid messageHeadline } } } } } } }`
body, _ := json.Marshal(GraphQLRequest{Query: query})

result := InjectGuardFields(body, "list_commits")

var gql GraphQLRequest
require.NoError(t, json.Unmarshal(result, &gql))
assert.Contains(t, gql.Query, "author{user{login}}")
// Should NOT inject issue/PR fields
assert.NotContains(t, gql.Query, "authorAssociation")
// Original fields still present
assert.Contains(t, gql.Query, "oid")
assert.Contains(t, gql.Query, "messageHeadline")
}

func TestInjectGuardFields_CommitFragment(t *testing.T) {
query := `fragment c on Commit{oid,messageHeadline}
query { repository(owner:"o", name:"r") { defaultBranchRef { target { ... on Commit { history(first:10) { nodes { ...c } } } } } } }`
body, _ := json.Marshal(GraphQLRequest{Query: query})

result := InjectGuardFields(body, "list_commits")

var gql GraphQLRequest
require.NoError(t, json.Unmarshal(result, &gql))
assert.Contains(t, gql.Query, "author{user{login}}")
assert.Contains(t, gql.Query, "fragment c on Commit{oid,messageHeadline,author{user{login}}}")
}

func TestInjectGuardFields_CommitSkipsWhenAuthorUserPresent(t *testing.T) {
// Has author{user{login}} already — should be a no-op
query := `query { repository(owner:"o", name:"r") { defaultBranchRef { target { ... on Commit { history(first:10) { nodes { oid author { user { login } } } } } } } } }`
body, _ := json.Marshal(GraphQLRequest{Query: query})
result := InjectGuardFields(body, "list_commits")
assert.Equal(t, body, result)
}

func countOccurrences(s, substr string) int {
count := 0
for i := 0; i+len(substr) <= len(s); i++ {
Expand Down
Loading