Skip to content

Fix author-aware integrity labeling - #14935

Merged
lpcox merged 1 commit into
mainfrom
lpcox-author-aware-labels
Oct 15, 2026
Merged

lpcox merged 1 commit into
mainfrom
lpcox-author-aware-labels

Conversation

@lpcox

@lpcox lpcox commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Remediates the author-provenance integrity bypasses reported in GHSA-5x56-229p-xf9f and the unresolved author-labeling portions of GHSA-g63x-69j3-vxf5.

  • makes blocked-users take precedence in resolve_author_integrity before trusted-user or collaborator elevation
  • moves discussion bodies and comments off unconditional repository-writer integrity and labels each REST/GraphQL item from its own author
  • makes non-get issue_read and pull_request_read coarse labels fail closed instead of inheriting the parent issue/PR author's trust
  • labels PR comments, reviews, review comments, and commits from each returned item's provenance
  • assigns authorless PR files/diffs none integrity while retaining writer integrity for maintainer-controlled labels, check runs, and status metadata
  • expands shared REST/GraphQL collection extraction so equivalent envelopes cannot bypass the same rules

Advisory mapping

GHSA-5x56-229p-xf9f

Condition Remediation Coverage
list_discussions trusted every discussion as repository-writer content coarse integrity is none; each discussion uses its own author REST root array and GraphQL discussions.nodes
get_discussion trusted the body as repository-writer content singleton response labeling uses the discussion author REST singleton and GraphQL repository.discussion
get_discussion_comments trusted all comments as repository-writer content each comment uses its own author REST root array and GraphQL nested discussion.comments.nodes
blocked/trusted dual-listed authors could be elevated shared user-authored helper checks blocked users first dual-listed blocked+trusted regression cases
discussion categories are maintainer-controlled metadata list_discussion_categories remains writer integrity existing category regression test

GHSA-g63x-69j3-vxf5

Condition Remediation Coverage
resolve_author_integrity skipped blocked-users returns blocked integrity before association/trusted/collaborator elevation direct blocked+trusted+OWNER test
PR get_comments inherited parent PR integrity each comment uses its own author GraphQL comments connection
PR get_reviews inherited parent PR integrity each review uses its own author GraphQL reviews connection
PR get_review_comments inherited parent PR integrity each review comment uses its own author REST review-comment envelope
PR get_commits inherited parent PR integrity each commit uses its own author, without default-branch promotion REST commits envelope and GraphQL commit connection
PR get_files / get_diff lacked per-item author evidence file/diff content defaults to none object envelope and scalar diff cases
PR get_check_runs / get_status are maintainer-controlled metadata retain writer integrity check-runs and statuses envelopes
Issue get_sub_issues inherited parent issue integrity each sub-issue uses its own author GraphQL subIssues.nodes
Issue get_labels is maintainer-controlled metadata retains writer integrity labels envelope
Dispatched methods could fall back to parent resource integrity on unrecognized shapes non-get resource labels now fail closed table covering every advisory-named issue/PR method

The previously fixed issue_read(method=get_comments) behavior remains intact: comments continue through the existing issue-item author-aware path, and a NONE outsider remains unapproved rather than inheriting the parent issue's integrity.

Validation

  • cargo test --manifest-path guards/github-guard/rust-guard/Cargo.toml --lib --quiet (682 passed)
  • make agent-finished (✓ All agent-finished checks passed!)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 954e6260-61f4-4b25-9bf5-99fdd0b623d0
Copilot AI max review requested due to automatic review settings October 15, 2026 05:01
@github-actions

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — default

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (list_issues, list_pull_requests, get_file_contents, list_commits) data returned (HTTP 200) ALLOWED ✅
B MCP writes (reaction/star/issue/comment/branch/file/PR) all 7 write tools absent from the 23-tool catalog (only read tools exposed: issue_read, pull_request_read, list_, get_, search_*) BLOCKED ⚠️
C CLI reads via proxied github CLI data returned ALLOWED ✅
D CLI REST writes gh not authenticated in this environment (no GH_TOKEN) BLOCKED ⚠️
E CLI GraphQL mutations gh not authenticated in this environment (no GH_TOKEN) BLOCKED ⚠️

Overall: INCONCLUSIVE

⚠️ Part B: none of the 7 targeted write tools (add_issue_comment, star_repository, issue_write, create_branch, create_or_update_file, create_pull_request) appear in the gateway's exposed tool catalog for this tools.github: config — only 23 read-only tools are registered. Per the architectural note, this reflects gh-aw's backend-level GITHUB_READ_ONLY=1 enforcement, not an independent gateway-level DIFC check, so it cannot confirm mcpg's own blocking layer. No write was attempted since no write tool existed to call.

⚠️ Part D/E: gh CLI is unauthenticated in this runner (gh: To use GitHub CLI in a GitHub Actions workflow, set the GH_TOKEN environment variable), so REST/GraphQL write-rejection could not be exercised against a real token boundary this run.

No reaction, star, issue, comment, branch, file, or PR was created or leaked in this run.

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

@lpcox
lpcox merged commit b573674 into main Oct 15, 2026
38 of 39 checks passed
@lpcox
lpcox deleted the lpcox-author-aware-labels branch October 15, 2026 05:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved trust bypasses and response-shape regressions can expose blocked content or discard resource data.

8 open findings
What changed in this PR

Hardens the Rust GitHub guard’s integrity labeling against author-provenance bypasses.

Changes:

  • Adds blocked-user precedence and per-author content labeling.
  • Introduces conservative sub-resource defaults, broader response extraction, and regression tests.
  • Updates integrity documentation and release notes.
File Description
guards/​github-guard/​rust-guard/​src/​labels/​tool_rules.rs Updates coarse integrity rules and blocking checks.
guards/​github-guard/​rust-guard/​src/​labels/​response_paths.rs Adds author-aware collection and sub-resource labels.
guards/​github-guard/​rust-guard/​src/​labels/​response_items.rs Extends author-aware legacy item labeling.
guards/​github-guard/​rust-guard/​src/​labels/​mod.rs Updates discussion integrity expectations.
guards/​github-guard/​rust-guard/​src/​labels/​helpers.rs Adds author helpers and response-envelope extraction.
guards/​github-guard/​docs/​INTEGRITY_TAG_SPEC.md Documents revised sub-resource integrity rules.
docs/​RELEASE_NOTES.md Announces author-aware integrity changes.

🧠 Review effort: Max


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

integrity,
ctx,
);
ensure_integrity_baseline(repo_full_name, &integrity, ctx)
("review_comments", "/review_comments"),
("commits", "/commits"),
("files", "/files"),
("labels", "/labels"),
Comment on lines +309 to +312
"get_comments" | "get_reviews" | "get_review_comments"
)
{
return label_author_items(tool_name, tool_args, &actual_response, method, ctx);
Comment on lines +336 to +340
integrity: commit_integrity_with_visibility(
item,
&repo_full,
repo_private,
false,
Comment on lines +404 to +408
labels: repo_default_labels(
"pull-request-status",
secrecy.clone(),
writer_integrity(&repo_full, ctx),
),
Comment on lines +195 to +196
if author_login.is_some_and(|login| is_blocked_user(login, ctx)) {
return blocked_integrity(repo_id, ctx);
return blocked_integrity(repo_full_name, ctx);
}

let mut integrity = author_association_floor(item, repo_full_name, ctx);
let (owner, repo, repo_full) = extract_repo_info(tool_args);
let secrecy: crate::SharedLabels =
repo_visibility_secrecy(&owner, &repo, &repo_full, ctx).into();
let (items, items_path) = extract_items_array(&actual_response);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants