Skip to content

rust-guard: remove org-scope clone in tool labeling and add private list_releases secrecy coverage - #10473

Merged
lpcox merged 3 commits into
mainfrom
copilot/rust-guard-eliminate-clone
Jul 31, 2026
Merged

lpcox merged 3 commits into
mainfrom
copilot/rust-guard-eliminate-clone

Conversation

Copilot AI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Rust guard had an avoidable allocation in org-scoped secret/variable labeling (Cow::Owned(org.clone())) and no test asserting private-repo secrecy propagation for list_releases. This PR removes the clone-by-ordering issue and closes the private-release labeling coverage gap.

  • Tool label ownership cleanup (tool_rules.rs)

    • Reordered org-scope assignments in set_secret | delete_secret | set_variable | delete_variable so borrow-only calls run before ownership transfer.
    • Replaced cloned ownership transfer with direct move into Cow::Owned.
    secrecy = private_scope_label(&org);
    integrity = writer_integrity(&org, ctx);
    baseline_scope = Cow::Owned(org);
  • Private list_releases secrecy coverage (response_paths.rs)

    • Added list_releases_private_repo_gets_private_secrecy.
    • Verifies per-item labels include private:<owner>/<repo> secrecy and merged:<owner>/<repo> integrity.
    • Verifies default_labels.secrecy also carries the private repo label.
  • Test-only visibility cache hook (backend.rs)

    • Added #[cfg(test)] pub(crate) fn set_cached_repo_visibility_for_tests(...) to prime repo visibility deterministically in response-path tests.

Copilot AI changed the title [WIP] Fix needless clone in Cow::Owned(org.clone()) rust-guard: remove org-scope clone in tool labeling and add private list_releases secrecy coverage Jul 31, 2026
Copilot finished work on behalf of lpcox July 31, 2026 20:46
Copilot AI requested a review from lpcox July 31, 2026 20:46
@lpcox
lpcox marked this pull request as ready for review July 31, 2026 21:26
Copilot AI review requested due to automatic review settings July 31, 2026 21:26

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.

Pull request overview

Removes an unnecessary Rust allocation and expands private-repository release labeling coverage.

Changes:

  • Moves org directly into Cow::Owned.
  • Adds private list_releases secrecy tests.
  • Adds a test-only visibility-cache hook.
Show a summary per file
File Description
tool_rules.rs Removes redundant organization cloning.
response_paths.rs Tests private release labels.
backend.rs Adds cache priming for tests.

Review details

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

#[test]
fn list_releases_private_repo_gets_private_secrecy() {
let repo_id = "octocat/private-repo";
crate::labels::backend::set_cached_repo_visibility_for_tests(repo_id, true);
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox

lpcox commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Addressed in bc602f5. The private list_releases test now uses a shared test-only cache guard from backend that holds the visibility-cache lock for the full test and restores/removes the seeded entry on drop.

Copilot finished work on behalf of lpcox July 31, 2026 21:45
@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 AWF (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) unknown tool [-32602] BLOCKED
C CLI reads via github CLI data returned ALLOWED
D CLI REST writes via gh unauthenticated — auth error BLOCKED
E CLI GraphQL mutations via gh unauthenticated — auth error BLOCKED

Notes:

  • Part B: Gateway strips all write tools — every write attempt returns MCP error [-32602] unknown tool. Gateway-enforced denial (tool absent from allowed list), distinct from backend config.
  • Parts D/E: gh CLI has no token in this runtime; all REST and GraphQL write attempts rejected before reaching the API.

Overall: PASS

References: §30667571982

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

@github-actions

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc) kernel-level isolation

Part Surface Op Result Expected Status
A MCP reads (list_issues/PRs/commits, get_file) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) unknown tool — gateway allowlist (22 read tools only) BLOCKED
C CLI reads (list_issues, get_file via MCP bridge) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) gh unauthenticated (no GH_TOKEN) BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated (no GH_TOKEN) BLOCKED

Notes:

  • Part B: Gateway enforces read-only by exposing only 22 read tools; write tool calls return Error [-32602]: unknown tool from the gateway within ~7–30ms (short-circuit, no backend contact).
  • Parts D/E: gh CLI has no GH_TOKEN in this environment; all REST and GraphQL write attempts are rejected at authentication level.

Overall: PASS

References: §30667571839

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

@github-actions

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) unknown tool [-32602] on all 7 BLOCKED
C CLI reads (list_issues, get_file_contents) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) gh unauthenticated — all refused BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated — all refused BLOCKED

Overall: PASS

All 7 MCP write tools absent from gateway tool list (gateway enforcement via tool filtering). CLI/GraphQL writes rejected due to unauthenticated gh token. No writes leaked on docker-sbx runtime.

References: §30667571933

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

@lpcox
lpcox merged commit 35f08fc into main Jul 31, 2026
38 checks passed
@lpcox
lpcox deleted the copilot/rust-guard-eliminate-clone branch July 31, 2026 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rust-guard] Rust Guard: Eliminate needless clone in Cow::Owned(org.clone()) via reorder

3 participants