Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cc565c6a-8344-4af7-b51b-4250984bc465
…ers and approval-labels - Add §4.4.4 min-integrity field documentation (was in code, missing from spec) - Add §4.4.5 blocked-users field — unconditional user blocking below "none" integrity - Add §4.4.6 approval-labels field — label-based integrity promotion to "approved" - Add §4.6 Integrity Level Model with formal hierarchy and evaluation algorithm - Add §8 Integrity Level Management — runtime enforcement specification - Update §10 integration examples to show new fields inside allow-only guard policy - Add compliance tests T-GH-41–060 covering new fields - Add Appendix examples A.12–A.14 and error messages B.6–B.8 - Add v1.1.0 change log entry Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cc565c6a-8344-4af7-b51b-4250984bc465
There was a problem hiding this comment.
Pull request overview
Updates the GitHub MCP access-control specification to v1.1.0, adding integrity-level management concepts (min-integrity, blocked-users, approval-labels) and updating the documented compilation into MCP Gateway guard-policies.
Changes:
- Bumps spec version to 1.1.0 and expands the model to include integrity-level hierarchy, evaluation order, and enforcement rules.
- Documents new frontmatter fields (
blocked-users,approval-labels) plus formalizesmin-integrity, including validation rules, error codes, and compliance tests. - Tightens the
add_smoked_labelsafe-output action tool schema in the smoke workflow and maps parsed payload fields to action inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| scratchpad/github-mcp-access-control-specification.md | Extends the spec with integrity-level management fields, algorithms, examples, validation rules, error codes, and compliance tests; updates section numbering and gateway compilation examples. |
| .github/workflows/smoke-codex.lock.yml | Restricts the add_smoked_label tool input schema and changes the workflow step to pass labels/number inputs derived from the safe-output payload JSON. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #### 4.6.4 Gateway Configuration Placement | ||
|
|
||
| `min-integrity`, `blocked-users`, and `approval-labels` are siblings to `repos` inside the `allow-only` guard policy object in the MCP Gateway configuration. See [Section 10.3](#103-frontmatter-to-gateway-configuration) for the compiled representation. |
There was a problem hiding this comment.
Section 4.6.4 links to “Section 10.3” using the anchor #103-frontmatter-to-gateway-configuration, but the “Frontmatter to Gateway Configuration” section is now numbered 10.4, so this link will be broken/misleading. Update the reference and anchor to match the actual section number/heading.
| `min-integrity`, `blocked-users`, and `approval-labels` are siblings to `repos` inside the `allow-only` guard policy object in the MCP Gateway configuration. See [Section 10.3](#103-frontmatter-to-gateway-configuration) for the compiled representation. | |
| `min-integrity`, `blocked-users`, and `approval-labels` are siblings to `repos` inside the `allow-only` guard policy object in the MCP Gateway configuration. See [Section 10.4](#104-frontmatter-to-gateway-configuration) for the compiled representation. |
| | `approved` | `["bot"]` | `["approved"]` | `bot` | `["approved"]` | DENY (blocked user) | | ||
| | `approved` | — | — | `alice` | — | DENY (base=none < approved) | | ||
| | `none` | — | — | `alice` | — | ALLOW (none ≥ none) | | ||
| | `approved` | — | `["approved"]` | `alice` | `["merged"]` | ALLOW (merged > approved) | |
There was a problem hiding this comment.
In the integrity decision examples table, the last row uses Labels: ["merged"] to justify “merged > approved”, but merged is described elsewhere as a content state/base integrity level (not a label). This makes the example internally inconsistent; adjust the row to represent a merged item via its state/base integrity (and keep labels separate), or change the label example to one that exists in approval-labels.
| | `approved` | — | `["approved"]` | `alice` | `["merged"]` | ALLOW (merged > approved) | | |
| | `approved` | — | `["approved"]` | `alice` | — | ALLOW (base=merged > approved) | |
| ## 8. Security Model | ||
| ## 8. Integrity Level Management | ||
|
|
||
| This section specifies runtime behavior for `min-integrity`, `blocked-users`, and `approval-labels`. These fields are siblings to `repos` and `min-integrity` in the frontmatter and compile to the same `allow-only` guard policy in the MCP Gateway configuration. |
There was a problem hiding this comment.
Section 8 intro says these fields are “siblings to repos and min-integrity” in frontmatter, but min-integrity is one of the fields being described, so the wording is self-referential and confusing. Reword to something like “siblings to repos in frontmatter …” or explicitly list the sibling set (repos, min-integrity, blocked-users, approval-labels).
| This section specifies runtime behavior for `min-integrity`, `blocked-users`, and `approval-labels`. These fields are siblings to `repos` and `min-integrity` in the frontmatter and compile to the same `allow-only` guard policy in the MCP Gateway configuration. | |
| This section specifies runtime behavior for `min-integrity`, `blocked-users`, and `approval-labels`. These fields are siblings to `repos` in the frontmatter (`repos`, `min-integrity`, `blocked-users`, `approval-labels`) and compile to the same `allow-only` guard policy in the MCP Gateway configuration. |
| | Item explicitly approved via GitHub review mechanism | `approved` | | ||
| | Pull request merged into target branch | `merged` | | ||
|
|
||
| The exact mapping from GitHub metadata to integrity levels is implementation-defined. Implementations SHOULD use GitHub's native review states (`APPROVED`, `CHANGES_REQUESTED`, `COMMENTED`) and merge status as primary signals. |
There was a problem hiding this comment.
The “Base Level Mapping” table describes approved as “Item explicitly approved via GitHub review mechanism”, but GitHub’s native “review” state is specific to pull requests (issues/comments don’t have an approval review mechanism). Since this section applies to issues/comments too, clarify how approved is determined for non‑PR items (e.g., only via approval-labels, or via some other explicit signal), or scope the row text to PRs.
| | Item explicitly approved via GitHub review mechanism | `approved` | | |
| | Pull request merged into target branch | `merged` | | |
| The exact mapping from GitHub metadata to integrity levels is implementation-defined. Implementations SHOULD use GitHub's native review states (`APPROVED`, `CHANGES_REQUESTED`, `COMMENTED`) and merge status as primary signals. | |
| | Item explicitly approved (see note below) | `approved` | | |
| | Pull request merged into target branch | `merged` | | |
| The exact mapping from GitHub metadata to integrity levels is implementation-defined. For pull requests, implementations SHOULD use GitHub's native review states (`APPROVED`, `CHANGES_REQUESTED`, `COMMENTED`) and merge status as primary signals for determining `approved` and `merged`. For issues, comments, and other non–pull request items, implementations SHOULD treat `approved` as requiring an explicit approval signal such as a configured `approval-labels` match or an equivalent implementation-defined mechanism; otherwise such items SHOULD remain at or below `unapproved`. |
| "payload": { | ||
| "description": "JSON-encoded payload to pass to the action", | ||
| "labels": { | ||
| "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", |
There was a problem hiding this comment.
The labels field description has grammatical issues and is a bit unclear. Consider rewriting to something like “Label names to add (one per line for multiple labels)”.
| "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", | |
| "description": "Label names to add (one per line for multiple labels).", |
| "description": "Add the 'smoked' label to the current pull request (can only be called once)", | ||
| "inputSchema": { | ||
| "additionalProperties": true, | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "payload": { | ||
| "description": "JSON-encoded payload to pass to the action", | ||
| "labels": { | ||
| "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", | ||
| "type": "string" | ||
| }, | ||
| "number": { | ||
| "description": "The number of the issue or pull request.", | ||
| "type": "string" | ||
| } |
There was a problem hiding this comment.
The tool description says this action adds the “smoked” label to the current pull request, but the input schema exposes a number parameter that could target an arbitrary issue/PR. Either tighten the schema/handler so it can only affect the current PR, or update the description to reflect that number can be provided to choose the target.
) ## Summary Implements the `blocked-users` and `approval-labels` additions from the MCP gateway spec ([github/gh-aw#22023](github/gh-aw#22023)). ### What changed **New integrity level: `blocked`** A new integrity level `blocked` sits below `none` in the hierarchy: ``` blocked < none < unapproved < approved < merged ``` Items authored by users in the `blocked-users` list receive `blocked` integrity tags (e.g. `blocked:owner/repo`). Since no agent is ever assigned a `blocked:` tag, these items are unconditionally denied by the DIFC filter. **`blocked-users` field** An optional array of GitHub usernames whose content items are **always blocked** regardless of labels or `min-integrity`. `approval-labels` cannot override a blocked-user exclusion. **`approval-labels` field** An optional array of GitHub label names that **promote an item's effective integrity to at least `approved`** when present on the item. Uses `max(base_integrity, approved)` so it never lowers an item already at `merged`. ### Effective integrity computation (per spec §4.6.2) ``` 1. Start with base integrity (from GitHub metadata) 2. IF author is in blocked-users → effective = blocked (always denied) 3. ELSE IF item has any approval label → effective = max(base, approved) 4. ELSE → effective = base ``` ### Files changed | File | Change | |------|--------| | `guards/github-guard/rust-guard/src/labels/constants.rs` | Add `BLOCKED_PREFIX`/`BLOCKED_BASE` | | `guards/github-guard/rust-guard/src/labels/helpers.rs` | Add fields to `PolicyContext`; add `blocked_integrity`, `is_blocked_user`, `has_approval_label` helpers; update `pr_integrity`, `issue_integrity`, `commit_integrity` | | `guards/github-guard/rust-guard/src/lib.rs` | Update `AllowOnlyPolicy` deserialization and `PolicyContext` construction | | `guards/github-guard/rust-guard/src/labels/mod.rs` | Re-export new functions; add tests (90 total pass) | | `internal/config/guard_policy.go` | Add `BlockedUsers`/`ApprovalLabels` to `AllowOnlyPolicy` and `NormalizedGuardPolicy`; update marshal/unmarshal/normalize | | `internal/config/guard_policy_test.go` | Tests for new fields | | `internal/guard/wasm.go` | Update `buildStrictLabelAgentPayload` to allow new optional keys | | `internal/guard/wasm_test.go` | Tests for new keys | | `guards/github-guard/docs/agentic-workflow-policy.schema.json` | Add new fields to schema | | `guards/github-guard/docs/AGENTIC_WORKFLOW_POLICY_FRONTMATTER.md` | Document new fields | | `guards/github-guard/docs/GATEWAY_ALLOWONLY_INTEGRATION_SPEC.md` | Update integration spec | ### Security Summary No new security vulnerabilities introduced. CodeQL scan: 0 alerts. The `blocked` integrity level specifically adds a new denial mechanism — items with `blocked:scope` tags can never pass the DIFC filter since no agent is assigned that tag.
Summary
Updates
scratchpad/github-mcp-access-control-specification.mdfrom v1.0.0 → v1.1.0 to add two new guard-policy fields (blocked-usersandapproval-labels) that are siblings toreposandmin-integrityin the frontmatter schema and compile to the sameallow-onlyguard-policy object in the MCP Gateway configuration.New Fields
blocked-users(§4.4.5)A list of GitHub usernames whose content items are unconditionally blocked. This represents a trust level below
nonein the integrity hierarchy — not merely "no trust signal" but an explicit negative trust decision that cannot be overridden by approval labels.approval-labels(§4.4.6)A list of GitHub label names that promote a content item's effective integrity level to
approved. Enables human-review gate workflows where a trusted reviewer applies a label to signal that the content is safe for agent processing.Specification Changes
min-integrity(was in the codebase but absent from the spec)blocked-usersfield definitionapproval-labelsfield definitionallow-onlyguard policy; added error codes-32005and-32006Gateway Configuration Placement
Both new fields are siblings to
reposandmin-integrityinside theallow-onlyguard policy:{ "guard-policies": { "allow-only": { "repos": ["myorg/*"], "min-integrity": "approved", "blocked-users": ["external-bot"], "approval-labels": ["human-reviewed"] } } }