Uh oh!
There was an error while loading. Please reload this page.
feat: implement blocked-users and approval-labels in GitHub guard - #2241
Conversation
- Rust guard: add blocked_users/approval_labels to PolicyContext; add blocked_integrity(), is_blocked_user(), has_approval_label() helpers; apply blocked-user check (new integrity level below none) and approval-label promotion in pr_integrity, issue_integrity, commit_integrity; update AllowOnlyPolicy deserialization in lib.rs; re-export new public functions from labels/mod.rs; add tests - Go config: add BlockedUsers/ApprovalLabels to AllowOnlyPolicy, update UnmarshalJSON/MarshalJSON, validate and deduplicate entries in NormalizeGuardPolicy; add to NormalizedGuardPolicy; add tests - Go wasm: update buildStrictLabelAgentPayload to allow blocked-users and approval-labels as optional allow-only keys with validation; add tests - Schema: add blocked-users and approval-labels to schema.json - Docs: update AGENTIC_WORKFLOW_POLICY_FRONTMATTER.md and GATEWAY_ALLOWONLY_INTEGRATION_SPEC.md Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/e77aee07-131a-47b6-a84f-9908229ff314
…r cleanups when blocked-user or approval-label rules fire, logging author/item/reason - Fix case-sensitive dedup mismatch: NormalizeGuardPolicy now deduplicates blocked-users and approval-labels using lowercased keys to match the Rust guard's case-insensitive comparison - Document that commit_integrity skips approval-labels (commits lack labels) - Improve error message for missing required repos/min-integrity fields - Add tests for case-insensitive deduplication Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8e18310 to
3e64f64CompareUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Implements MCP gateway spec additions for GitHub guard policies by introducing an unconditional author blocklist and label-driven integrity promotion, plus the new blocked integrity level to ensure blocked content is always denied by DIFC.
Changes:
- Add
blockedintegrity tags (blocked:) and compute effective integrity usingblocked-usersandapproval-labels. - Extend gateway policy parsing/normalization and WASM payload validation to accept the new fields.
- Update schemas/docs and add Go + Rust test coverage for the new behaviors.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/guard/wasm.go | Accepts/validates new allow-only keys (blocked-users, approval-labels) in strict payload builder. |
| internal/guard/wasm_test.go | Adds coverage for new allow-only keys and unknown-key rejection. |
| internal/config/guard_policy.go | Extends AllowOnlyPolicy + normalization to include/validate/dedup new fields. |
| internal/config/guard_policy_test.go | Adds tests for parsing/normalization/marshalling of new fields. |
| guards/github-guard/rust-guard/src/lib.rs | Extends allow-only policy deserialization and PolicyContext construction. |
| guards/github-guard/rust-guard/src/labels/constants.rs | Adds BLOCKED_PREFIX / BLOCKED_BASE. |
| guards/github-guard/rust-guard/src/labels/helpers.rs | Implements blocked_integrity, blocked-user checks, and approval-label promotion logic. |
| guards/github-guard/rust-guard/src/labels/mod.rs | Re-exports helpers and adds extensive tests for blocked-users/approval-labels semantics. |
| guards/github-guard/docs/agentic-workflow-policy.schema.json | Adds schema fields for blocked-users and approval-labels. |
| guards/github-guard/docs/AGENTIC_WORKFLOW_POLICY_FRONTMATTER.md | Documents new policy fields and effective integrity computation. |
| guards/github-guard/docs/GATEWAY_ALLOWONLY_INTEGRATION_SPEC.md | Updates integration spec to describe new integrity level and policy fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Validate blocked-users if present: must be a non-empty array of non-empty strings. | ||
| if blockedUsersRaw, ok := allowOnly["blocked-users"]; ok { | ||
| arr, ok := blockedUsersRaw.([]interface{}) | ||
| if !ok { | ||
| return nil, fmt.Errorf("invalid blocked-users value: expected array of strings") |
There was a problem hiding this comment.
The inline comment says blocked-users must be a non-empty array, but the code currently allows an empty array (it only validates element types/emptiness). Since other parts of the PR treat an empty list as equivalent to omitting the field, update the comment (or enforce non-empty if that’s truly required) to keep behavior and documentation consistent.
| // Validate approval-labels if present: must be a non-empty array of non-empty strings. | ||
| if approvalLabelsRaw, ok := allowOnly["approval-labels"]; ok { | ||
| arr, ok := approvalLabelsRaw.([]interface{}) | ||
| if !ok { | ||
| return nil, fmt.Errorf("invalid approval-labels value: expected array of strings") |
There was a problem hiding this comment.
The inline comment says approval-labels must be a non-empty array, but this function currently permits an empty array (it only validates element types/emptiness). Please align the comment with the intended semantics (empty list behaves like omitted) or add an explicit non-empty check if empties should be rejected.
| // Validate and normalize blocked-users. | ||
| // Dedup uses lowercased keys to match Rust guard's case-insensitive comparison. | ||
| if len(policy.AllowOnly.BlockedUsers) > 0 { | ||
| seen := make(map[string]struct{}, len(policy.AllowOnly.BlockedUsers)) | ||
| for _, u := range policy.AllowOnly.BlockedUsers { | ||
| u = strings.TrimSpace(u) |
There was a problem hiding this comment.
NormalizeGuardPolicy claims to produce a canonical representation for caching/observability, but BlockedUsers/ApprovalLabels are deduplicated without being sorted or otherwise canonicalized. That means semantically identical policies can normalize to different JSON depending on input order. Consider lowercasing + sorting these lists (similar to ScopeValues) after trimming/dedup to make normalization deterministic.
| - New required policy field `Integrity` | ||
| - New integrity hierarchy with baseline `none` | ||
| - New integrity hierarchy with baseline `none` and `blocked` level below `none` | ||
| - Guard-side policy initialization via a new `label_agent` interface | ||
| - Blocked-user enforcement via `blocked-users` | ||
| - Label-based integrity promotion via `approval-labels` |
There was a problem hiding this comment.
This section still states the required policy field is Integrity, but the examples and implementation use min-integrity (and the PR adds fields under that same allow-only shape). Please update the doc to consistently use the correct field/key names throughout (including later label_agent payload examples) to avoid integrators wiring the wrong JSON keys.
…bels to README (#2250) ## Changes Updates the README Guard Policies section to document the new integrity features: ### Added - **`blocked-users`** option — array of usernames whose content gets unconditional `blocked` integrity (below `none`) - **`approval-labels`** option — array of labels that elevate items to `approved` integrity (human-review gate) - **`blocked` integrity level** in the `min-integrity` hierarchy - Example config showing both options in context - Link to the [Integrity Filtering Reference](https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/integrity.md) in the Further Reading table ### Improved - Reordered integrity levels from highest to lowest for clarity - Added notes on private repo items and trusted bots qualifying as `approved` ### Related - PR #2241 implements blocked-users and approval-labels in the GitHub guard - PR #22044 (gh-aw) adds the Integrity Filtering Reference documentation
Summary
Implements the
blocked-usersandapproval-labelsadditions from the MCP gateway spec (github/gh-aw#22023).What changed
New integrity level:
blockedA new integrity level
blockedsits belownonein the hierarchy:Items authored by users in the
blocked-userslist receiveblockedintegrity tags (e.g.blocked:owner/repo). Since no agent is ever assigned ablocked:tag, these items are unconditionally denied by the DIFC filter.blocked-usersfieldAn optional array of GitHub usernames whose content items are always blocked regardless of labels or
min-integrity.approval-labelscannot override a blocked-user exclusion.approval-labelsfieldAn optional array of GitHub label names that promote an item's effective integrity to at least
approvedwhen present on the item. Usesmax(base_integrity, approved)so it never lowers an item already atmerged.Effective integrity computation (per spec §4.6.2)
Files changed
guards/github-guard/rust-guard/src/labels/constants.rsBLOCKED_PREFIX/BLOCKED_BASEguards/github-guard/rust-guard/src/labels/helpers.rsPolicyContext; addblocked_integrity,is_blocked_user,has_approval_labelhelpers; updatepr_integrity,issue_integrity,commit_integrityguards/github-guard/rust-guard/src/lib.rsAllowOnlyPolicydeserialization andPolicyContextconstructionguards/github-guard/rust-guard/src/labels/mod.rsinternal/config/guard_policy.goBlockedUsers/ApprovalLabelstoAllowOnlyPolicyandNormalizedGuardPolicy; update marshal/unmarshal/normalizeinternal/config/guard_policy_test.gointernal/guard/wasm.gobuildStrictLabelAgentPayloadto allow new optional keysinternal/guard/wasm_test.goguards/github-guard/docs/agentic-workflow-policy.schema.jsonguards/github-guard/docs/AGENTIC_WORKFLOW_POLICY_FRONTMATTER.mdguards/github-guard/docs/GATEWAY_ALLOWONLY_INTEGRATION_SPEC.mdSecurity Summary
No new security vulnerabilities introduced. CodeQL scan: 0 alerts. The
blockedintegrity level specifically adds a new denial mechanism — items withblocked:scopetags can never pass the DIFC filter since no agent is assigned that tag.