Conversation
…rity elevation Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/2146d0f4-315e-43bc-9df7-2cec71523762 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/2146d0f4-315e-43bc-9df7-2cec71523762 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds support for a trusted-users allowlist to the GitHub guard’s allow-only policy so specific external collaborators can be elevated to approved integrity without lowering min-integrity globally.
Changes:
- Extends Go guard policy parsing/normalization/serialization to include
trusted-users, with validation + case-insensitive deduplication. - Plumbs
trusted-usersthrough proxy/gateway initialization into the WASM guard (nested underallow-only), and updates the Go payload builder signature accordingly. - Updates Rust WASM guard policy context and integrity elevation logic to treat configured trusted users similarly to trusted bots; adds tests for detection/elevation/precedence.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/server/guard_init.go | Updates LabelAgent payload construction call site (passes nil trusted-users for gateway mode). |
| internal/proxy/proxy.go | Adds TrustedUsers to proxy config and forwards it into guard initialization. |
| internal/guard/wasm.go | Accepts/validates trusted-users in strict payload validation; injects trusted users into allow-only during payload build. |
| internal/guard/wasm_test.go | Adds Go tests for strict validation and payload injection of trusted-users. |
| internal/config/guard_policy.go | Adds TrustedUsers to policy structs + JSON (un)marshal; normalizes/dedups trusted users. |
| internal/config/guard_policy_test.go | Adds Go tests for parsing, normalization, and marshal behavior around trusted-users. |
| internal/cmd/proxy.go | Adds --trusted-users flag and wires it into proxy config. |
| guards/github-guard/rust-guard/src/lib.rs | Extends allow-only policy deserialization and passes trusted users into PolicyContext. |
| guards/github-guard/rust-guard/src/labels/helpers.rs | Adds trusted_users to context + is_trusted_user() and uses it in integrity elevation paths. |
| guards/github-guard/rust-guard/src/labels/tool_rules.rs | Extends tool-level elevation checks to include is_trusted_user(). |
| guards/github-guard/rust-guard/src/labels/mod.rs | Adds Rust unit tests for trusted user detection/elevation and precedence. |
Comments suppressed due to low confidence (1)
guards/github-guard/rust-guard/src/labels/tool_rules.rs:195
- Blocked-users precedence is not enforced in this trusted elevation block: a login present in blocked_users but also in trusted_users (or a trusted bot) will be promoted to approved integrity. This violates the stated precedence (blocked-users > trusted-users). Consider checking is_blocked_user(login, ctx) first and returning blocked_integrity (or preventing any max_integrity elevation) before applying trusted user/bot promotion.
// Elevate trusted first-party bots and trusted users to approved
if let Some(ref login) = facts.author_login {
if is_trusted_first_party_bot(login)
|| is_configured_trusted_bot(login, ctx)
|| is_trusted_user(login, ctx)
{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If allow-only is absent, the injection is skipped and buildStrictLabelAgentPayload | ||
| // will reject the payload when called with the missing allow-only key. | ||
| users := make([]interface{}, len(trustedUsers)) | ||
| for i, u := range trustedUsers { | ||
| users[i] = u | ||
| } | ||
| // Inject into allow-only object if present | ||
| if allowOnly, ok := payload["allow-only"].(map[string]interface{}); ok { | ||
| allowOnly["trusted-users"] = users |
There was a problem hiding this comment.
BuildLabelAgentPayload injects trusted-users only when the policy uses the canonical "allow-only" key. However, buildStrictLabelAgentPayload explicitly supports the legacy top-level key "allowonly" for backward compatibility, so a legacy policy would silently skip trusted-users injection. Consider injecting into "allowonly" as well (or normalizing the key first) and add a test covering legacy allowonly + --trusted-users.
| // If allow-only is absent, the injection is skipped and buildStrictLabelAgentPayload | |
| // will reject the payload when called with the missing allow-only key. | |
| users := make([]interface{}, len(trustedUsers)) | |
| for i, u := range trustedUsers { | |
| users[i] = u | |
| } | |
| // Inject into allow-only object if present | |
| if allowOnly, ok := payload["allow-only"].(map[string]interface{}); ok { | |
| allowOnly["trusted-users"] = users | |
| // If allow-only/allowonly is absent, the injection is skipped and | |
| // buildStrictLabelAgentPayload will reject the payload when called | |
| // with the missing allow-only key. | |
| users := make([]interface{}, len(trustedUsers)) | |
| for i, u := range trustedUsers { | |
| users[i] = u | |
| } | |
| // Inject into canonical allow-only object if present | |
| if allowOnly, ok := payload["allow-only"].(map[string]interface{}); ok { | |
| allowOnly["trusted-users"] = users | |
| } else if allowOnlyLegacy, ok := payload["allowonly"].(map[string]interface{}); ok { | |
| // Also support legacy top-level allowonly key for backward compatibility. | |
| allowOnlyLegacy["trusted-users"] = users |
| } | ||
|
|
||
| // Validate trusted-users if present inside allow-only. | ||
| // Must be a non-empty array of non-empty strings when present. |
There was a problem hiding this comment.
The trusted-users validation comment says the list "must be a non-empty array" when present, but the implementation currently accepts an empty array (it only type-checks and validates entries). Either enforce len(arr) > 0 or adjust the comment to match the actual behavior.
| // Must be a non-empty array of non-empty strings when present. | |
| // Must be an array of non-empty strings when present; an empty array is allowed. |
| // Elevate trusted first-party bots and trusted users to approved | ||
| if let Some(ref login) = info.author_login { | ||
| if is_trusted_first_party_bot(login) | ||
| || is_configured_trusted_bot(login, ctx) | ||
| || is_trusted_user(login, ctx) | ||
| { |
There was a problem hiding this comment.
Blocked-users precedence is not enforced here: a login that is in blocked_users but also matches is_trusted_user/is_trusted_* will be elevated to approved integrity in apply_tool_labels. This conflicts with the documented rule that blocked_users overrides trusted_users/bots. Consider importing is_blocked_user (and/or blocked_integrity) and short-circuiting to blocked integrity before applying any trusted elevation, or at least gating the elevation on !is_blocked_user(login, ctx).
This issue also appears on line 190 of the same file.
External users (contractors, cross-org collaborators) couldn't be elevated to
approvedintegrity without globally loweringmin-integrity.trusted-usersadds a targeted allowlist insideallow-only, analogous totrusted-botsbut for regular human users.Policy shape
{ "allow-only": { "repos": "public", "min-integrity": "approved", "trusted-users": ["contractor-1", "partner-dev"] } }Precedence:
blocked-users>trusted-users>approval-labels>author_associationChanges
Go (config/guard/proxy)
internal/config/guard_policy.go—TrustedUsers []stringonAllowOnlyPolicyandNormalizedGuardPolicy; addedUnmarshalJSONcase,MarshalJSONserialization, and normalization (whitespace trim, empty-entry rejection, case-insensitive dedup)internal/guard/wasm.go—BuildLabelAgentPayloadgainstrustedUsers []stringparameter; injects into theallow-onlyobject (not top-level, unliketrusted-bots);buildStrictLabelAgentPayloadvalidatestrusted-usersentriesinternal/cmd/proxy.go—--trusted-usersflag, mirrors--trusted-botsinternal/proxy/proxy.go—TrustedUsersinConfig; forwarded throughinitGuardPolicyinternal/server/guard_init.go—BuildLabelAgentPayloadcall updated;nilfor injected trusted-users because gateway mode embeds them directly in the policy JSONRust (WASM guard)
src/lib.rs—trusted_users: Vec<String>onAllowOnlyPolicy; passed intoPolicyContextsrc/labels/helpers.rs—trusted_usersfield onPolicyContext; newis_trusted_user()(case-insensitive);author_association_floor()andpr_integrity()checkis_trusted_user()alongside existing bot checkssrc/labels/tool_rules.rs— issue/PR elevation checks (get_issue,pull_request_read) extended tois_trusted_user()Tests
BuildLabelAgentPayloadinjectionFIRST_TIMERelevation,blocked-usersprecedence overtrusted-usersWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build967590049/b330/launcher.test /tmp/go-build967590049/b330/launcher.test -test.testlogfile=/tmp/go-build967590049/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-p s/block.go 64/pkg/tool/linu-lang=go1.25(dns block)/tmp/go-build2594907654/b334/launcher.test /tmp/go-build2594907654/b334/launcher.test -test.testlogfile=/tmp/go-build2594907654/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s -gua�� -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.05.rcgu/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.06.rcgu/tmp/go-build702816338/b309/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.07.rcgu.o -guard/target/de/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/de/tmp/go-build702816338/b209/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.10.rcgu.o -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.11.rcgu.o -gua�� -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.13.rcgugrep -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.14.rcgu-qE d30fe3.rlib ithub-guard/rust/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/link vendor/golang.or-o -lang=go1.25 known-linux-gnu/-importcfg(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build967590049/b315/config.test /tmp/go-build967590049/b315/config.test -test.testlogfile=/tmp/go-build967590049/b315/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go 7luPlZJ6G ache/uv/0.11.1/x-I(dns block)/tmp/go-build2594907654/b319/config.test /tmp/go-build2594907654/b319/config.test -test.testlogfile=/tmp/go-build2594907654/b319/testlog.txt -test.paniconexit0 -test.timeout=10m0s rds/�� lib/rustlib/x86_embed-bitcode=no lib/rustlib/x86_-C -guard/target/debug/deps/rustcME6R4b/symbols.o -guard/target/de/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/de-unsafeptr=false -guard/target/de/tmp/go-build702816338/b094/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.03.rcgu.o -gua�� -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.05.rcgu/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.06.rcgu/tmp/go-build702816338/b314/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.07.rcgu.o -guard/target/de/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/de/tmp/go-build702816338/b238/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.10.rcgu.o -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.11.rcgu.o(dns block)nonexistent.local/tmp/go-build967590049/b330/launcher.test /tmp/go-build967590049/b330/launcher.test -test.testlogfile=/tmp/go-build967590049/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-p s/block.go 64/pkg/tool/linu-lang=go1.25(dns block)/tmp/go-build2594907654/b334/launcher.test /tmp/go-build2594907654/b334/launcher.test -test.testlogfile=/tmp/go-build2594907654/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s -gua�� -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.05.rcgu/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.06.rcgu/tmp/go-build702816338/b309/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.07.rcgu.o -guard/target/de/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/de/tmp/go-build702816338/b209/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.10.rcgu.o -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.11.rcgu.o -gua�� -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.13.rcgugrep -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.14.rcgu-qE d30fe3.rlib ithub-guard/rust/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/link vendor/golang.or-o -lang=go1.25 known-linux-gnu/-importcfg(dns block)slow.example.com/tmp/go-build967590049/b330/launcher.test /tmp/go-build967590049/b330/launcher.test -test.testlogfile=/tmp/go-build967590049/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-p s/block.go 64/pkg/tool/linu-lang=go1.25(dns block)/tmp/go-build2594907654/b334/launcher.test /tmp/go-build2594907654/b334/launcher.test -test.testlogfile=/tmp/go-build2594907654/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s -gua�� -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.05.rcgu/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.06.rcgu/tmp/go-build702816338/b309/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.07.rcgu.o -guard/target/de/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -guard/target/de/tmp/go-build702816338/b209/vet.cfg -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.10.rcgu.o -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.11.rcgu.o -gua�� -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.13.rcgugrep -guard/target/debug/deps/serde_derive-f8a9da973ea849b8.serde_derive.291b3f11bb186a3e-cgu.14.rcgu-qE d30fe3.rlib ithub-guard/rust/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/link vendor/golang.or-o -lang=go1.25 known-linux-gnu/-importcfg(dns block)this-host-does-not-exist-12345.com/tmp/go-build967590049/b339/mcp.test /tmp/go-build967590049/b339/mcp.test -test.testlogfile=/tmp/go-build967590049/b339/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas(dns block)/tmp/go-build2594907654/b343/mcp.test /tmp/go-build2594907654/b343/mcp.test -test.testlogfile=/tmp/go-build2594907654/b343/testlog.txt -test.paniconexit0 -test.timeout=10m0s -gua�� -guard/target/debug/deps/github_guard-d6a03f414dc506a0.2j8pqcxyyfybe7nghv8aqe5ar.1du650a.rcgu.o -guard/target/debug/deps/github_guard-d6a03f414dc506a0.2qy5imqpflz26hi4smzxan76o.1du650a.rcgu.o d30fe3.rlib ebug.go mime/multipart known-linux-gnu/--version known-linux-gnu/lib/rustlib/x86_prefer-dynamic know�� ithub-guard/rust-guard/target/debug/deps/rustc0eIFPy/symbols.o ithub-guard/rust-guard/target/debug/deps/github_guard-d6a03f414dc506a0.0bxgpkkxlbr5ndvint8fbouk8test x_amd64/vet ithub-guard/rustgrep ithub-guard/rust-qE ithub-guard/rust(create|run) x_amd64/vet(dns block)If you need me to access, download, or install something from one of these locations, you can either:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.