Skip to content

Add pre-emptive guard entries for 3 CLI write operations missing from WRITE_OPERATIONS#3609

Merged
lpcox merged 3 commits intomainfrom
copilot/guard-coverage-gap-fix
Apr 11, 2026
Merged

Add pre-emptive guard entries for 3 CLI write operations missing from WRITE_OPERATIONS#3609
lpcox merged 3 commits intomainfrom
copilot/guard-coverage-gap-fix

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 11, 2026

Three GitHub CLI write operations had no guard coverage — meaning if a corresponding MCP tool lands in github-mcp-server, it would pass through unclassified. All three are high/medium risk operations involving repo settings, PR reversion, and SSH key grants.

tools.rs — WRITE_OPERATIONS additions

  • edit_repositoryPATCH /repos/{owner}/{repo}; can flip visibility private→public
  • revert_pull_request — GraphQL revertPullRequest; creates branch + PR
  • add_deploy_key / delete_deploy_keyPOST/DELETE /repos/{owner}/{repo}/keys; grants persistent SSH access

tool_rules.rs — DIFC label rules

"edit_repository" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
    integrity = writer_integrity(repo_id, ctx);
}
"revert_pull_request" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
    integrity = writer_integrity(repo_id, ctx);
}
"add_deploy_key" | "delete_deploy_key" => {
    // Always private regardless of repo visibility — deploy key secrets are sensitive
    secrecy = policy_private_scope_label(&owner, &repo, repo_id, ctx);
    integrity = writer_integrity(repo_id, ctx);
}

Deploy key operations use policy_private_scope_label unconditionally (not apply_repo_visibility_secrecy) since the key material itself is always sensitive regardless of whether the repo is public.

Warning

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
    • Triggering command: /tmp/go-build3092088994/b514/launcher.test /tmp/go-build3092088994/b514/launcher.test -test.testlogfile=/tmp/go-build3092088994/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -W .cfg Bmen/MKrSXkkW-_Q-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet 9006�� .cfg olang.org/grpc@v1.80.0/internal/transport/client_stream.go x_amd64/vet --gdwarf-5 g/grpc/attribute-qE -o x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3092088994/b496/config.test /tmp/go-build3092088994/b496/config.test -test.testlogfile=/tmp/go-build3092088994/b496/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3092088994/b323/vet.cfg 5.0/deviceauth.g-errorsas 5.0/oauth2.go x_amd64/vet --gdwarf-5 nal/protolazy -o x_amd64/vet -I g_.a -I x_amd64/vet --gdwarf-5 go-sdk/auth -o x_amd64/vet (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build3092088994/b514/launcher.test /tmp/go-build3092088994/b514/launcher.test -test.testlogfile=/tmp/go-build3092088994/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -W .cfg Bmen/MKrSXkkW-_Q-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet 9006�� .cfg olang.org/grpc@v1.80.0/internal/transport/client_stream.go x_amd64/vet --gdwarf-5 g/grpc/attribute-qE -o x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build3092088994/b514/launcher.test /tmp/go-build3092088994/b514/launcher.test -test.testlogfile=/tmp/go-build3092088994/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -W .cfg Bmen/MKrSXkkW-_Q-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet 9006�� .cfg olang.org/grpc@v1.80.0/internal/transport/client_stream.go x_amd64/vet --gdwarf-5 g/grpc/attribute-qE -o x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3092088994/b523/mcp.test /tmp/go-build3092088994/b523/mcp.test -test.testlogfile=/tmp/go-build3092088994/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3092088994/b517/_pkg_.a elemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp@v1.43.0/internal/otlpconfig/envconfig.go/usr/bin/runc.original elemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp@v1.43.0/internal/otlpconfig/options.go x_amd64/vet --gdwarf-5 g/grpc/internal -o x_amd64/vet -obj�� .cfg -importpath x_amd64/vet -ldflags="-O2" "/usr/libexec/docker/docker-init ecosystem/grpc-g--version -I x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

…tory, revert_pull_request, add_deploy_key, delete_deploy_key)

Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/17c13f5c-2bc5-46db-8a22-089994e14c83

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix guard coverage gap for CLI write operations Add pre-emptive guard entries for 3 CLI write operations missing from WRITE_OPERATIONS Apr 11, 2026
Copilot AI requested a review from lpcox April 11, 2026 21:09
@lpcox lpcox marked this pull request as ready for review April 11, 2026 21:16
Copilot AI review requested due to automatic review settings April 11, 2026 21:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds guard classification and DIFC labeling for three previously-uncovered GitHub CLI write operations so that if/when matching MCP tools are introduced, they won’t be treated as unclassified pass-throughs.

Changes:

  • Added edit_repository, revert_pull_request, add_deploy_key, and delete_deploy_key to WRITE_OPERATIONS, plus a regression test asserting they’re treated as writes.
  • Added explicit apply_tool_labels rules for the new operations (writer integrity; deploy keys forced to private secrecy).
  • Added unit tests validating labeling behavior for the new tools.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/tools.rs Classifies the new operations as write operations and adds a test to prevent regressions.
guards/github-guard/rust-guard/src/labels/tool_rules.rs Applies tool-specific DIFC labels for repo edit, PR revert, and deploy key management.
guards/github-guard/rust-guard/src/labels/mod.rs Adds tests ensuring the new tool labeling rules yield expected integrity/secrecy.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

// === Deploy key management (SSH key with optional write access) ===
"add_deploy_key" | "delete_deploy_key" => {
// Manages SSH deploy keys — `add_deploy_key` may grant persistent write access.
// S = private:owner/repo (deploy key secrets should be restricted)
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

The comment claims secrecy is private:owner/repo, but this arm uses policy_private_scope_label(...), which can return unscoped private (or owner-scoped) depending on the cached policy scope kind. Update the comment to reflect the actual behavior (i.e., always at least private, scope may vary).

Suggested change
// S = private:owner/repo (deploy key secrets should be restricted)
// S = at least private; scope is policy-dependent (may be unscoped, owner-scoped, or repo-scoped)

Copilot uses AI. Check for mistakes.
The comment claimed secrecy was always private:owner/repo, but
policy_private_scope_label() returns a scope that depends on the
cached policy scope kind (unscoped, owner-scoped, or repo-scoped).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 4c7872d into main Apr 11, 2026
14 checks passed
@lpcox lpcox deleted the copilot/guard-coverage-gap-fix branch April 11, 2026 21:24
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.

[guard-coverage] Guard coverage gap: 3 CLI write operations missing pre-emptive WRITE_OPERATIONS entries

3 participants