fix: correct baseline_scope for scoped integrity labels and discussion tool integrity#2281
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/5d3a1a5e-292f-45e0-8856-10ea2317ed26
There was a problem hiding this comment.
Pull request overview
This PR fixes integrity label downgrades caused by ensure_integrity_baseline when tool rules emit scoped integrity labels (e.g., approved:github, unapproved:user) but baseline_scope remains the default repo_id (often empty), causing integrity to collapse to none.
Changes:
- Set
baseline_scopeexplicitly forproject:github-scoped tools anduser-scoped gist tools so scoped integrity labels are preserved. - Change discussion-related tools to assign
writer_integrityat the resource level (instead ofprivate_writer_integrity) to avoidnoneintegrity in public repos. - Update unit tests to remove prior
repo_id = "github"/"user"workarounds and to reflect the new discussion integrity behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
guards/github-guard/rust-guard/src/labels/tool_rules.rs |
Aligns baseline_scope with integrity label scope for several tools; changes discussion tool integrity assignment. |
guards/github-guard/rust-guard/src/labels/mod.rs |
Updates tests to use realistic empty repo_id and adjusts discussion integrity expectations. |
Comments suppressed due to low confidence (1)
guards/github-guard/rust-guard/src/labels/tool_rules.rs:435
get_discussion_commentsnow useswriter_integrity(repo_id, ctx)for discussion comments as well. Since these are explicitly user-submitted and can be the lowest-trust content, treating them as approved at the repo scope can let untrusted public comments pass filters whenmin-integrity > none. A safer pattern would mirror issues: keep resource-level integrity conservative (e.g.,private_writer_integrityorreader_integrity) and add response labeling to compute integrity from author association / approval labels per comment.
"get_discussion_comments" => {
// Discussion comments are user-submitted, lowest-trust user content.
// S = inherits from repo visibility
// I = approved — treat discussion comments as approved at the resource level
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "list_discussions" | "get_discussion" => { | ||
| // Discussions are user-submitted content, similar to issues. | ||
| // S = inherits from repo visibility | ||
| // I = private repos get approved; public repos deferred to response labeling | ||
| // I = approved — treat discussion content as approved at the resource level | ||
| secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); | ||
| integrity = private_writer_integrity(repo_id, repo_private, ctx); | ||
| integrity = writer_integrity(repo_id, ctx); |
There was a problem hiding this comment.
list_discussions / get_discussion now assign writer_integrity(repo_id, ctx) unconditionally. This elevates user-generated discussion content to approved integrity even for public repos, which differs from the existing approach for other user-submitted repo content (e.g., issues use private_writer_integrity + per-item issue_integrity in response labeling). Consider keeping resource-level integrity at private_writer_integrity(repo_id, repo_private, ctx) (or reader_integrity for public) and adding response labeling for discussions to compute integrity per item (similar to issue_integrity), so min-integrity policies aren’t bypassed for untrusted public content.
This issue also appears on line 430 of the same file.
See below for a potential fix:
// I = reader — resource-level integrity reflects read access; per-item
// labeling should be used to elevate trusted content if needed
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = reader_integrity(repo_id, ctx);
}
"get_discussion_comments" => {
// Discussion comments are user-submitted, lowest-trust user content.
// S = inherits from repo visibility
// I = reader — resource-level integrity reflects read access; per-item
// labeling should be used to elevate trusted content if needed
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = reader_integrity(repo_id, ctx);
ensure_integrity_baselineranks integrity labels by matching thebaseline_scopeagainst label scopes. Whenbaseline_scopestays asrepo_id(often empty) butintegritywas assigned with a different scope, the rank comparison returns 0 and the labels get replaced bynone.Fixes
baseline_scopenot set forproject:github-scoped toolsproject_github_label(ctx)callswriter_integrity("github", ctx), producing labels likeapproved:github. Withoutbaseline_scope = "github",ensure_integrity_baselinedowngrades these tonone:.Affected tools:
get_me,get_teams,get_team_members,list_starred_repositories,search_orgs,list_global_security_advisories,get_global_security_advisory.baseline_scopenot set foruser-scoped toolsreader_integrity("user", ctx)producesunapproved:user— same downgrade issue withoutbaseline_scope = "user".Affected tools:
list_gists,get_gist.Discussion tools silently producing
noneintegrity for public reposprivate_writer_integrityreturns[]for public repos, whichensure_integrity_baselineconverts tonone. Nolabel_response_paths/label_response_itemsfallback exists for these tools, so public discussion content was permanently stuck atnoneand filtered at anymin-integrity > none. Replaced withwriter_integrity(repo_id, ctx)to assign approved at the resource level unconditionally.Affected tools:
list_discussions,get_discussion,get_discussion_comments.Test updates
repo_id = "github"/repo_id = "user"workarounds in 9 tests that were compensating for the missingbaseline_scopeassignments — tests now use realistic emptyrepo_idnone_integrity→writer_integrity📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.