rust-guard: centralize repo-visibility fallback semantics in labeling helpers - #10624
Conversation
There was a problem hiding this comment.
Pull request overview
Centralizes repository-visibility fallback policies in Rust guard labeling helpers.
Changes:
- Adds fail-open and fail-secure visibility helpers with tests.
- Reuses helpers across response and tool labeling paths.
- One tool-rule call site unintentionally retries visibility lookup.
Show a summary per file
| File | Description |
|---|---|
labels/helpers.rs |
Adds fallback helpers and unit tests. |
labels/response_items.rs |
Uses helpers for item labeling. |
labels/response_paths.rs |
Uses helpers for path labeling. |
labels/tool_rules.rs |
Uses secure fallback for commit rules. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
| let repo_private_effective = repo_private | ||
| .unwrap_or_else(|| repo_private_or_secure_default(&owner, &repo)); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot resolve merge conflicts and address review feedback |
…ure_default signature The helper now accepts Option<bool> (already-fetched visibility) instead of (owner, repo) to avoid a second backend lookup. This addresses the review feedback that calling the helper when repo_private is None was triggering a duplicate backend request, which could dowgrade integrity if the second lookup succeeded with a 'public' result.
Resolved in commit Merged The key fix in // Before (double lookup — wrong):
let repo_private_effective = repo_private
.unwrap_or_else(|| repo_private_or_secure_default(&owner, &repo));
// After (uses already-fetched value — correct):
let repo_private_effective = repo_private_or_secure_default(repo_private);The same |
🔒 mcpg Read-Only Stress — gVisorSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Notes:
Overall: PASS References: §30833270945
|
🔒 mcpg Read-Only Stress — default AWFSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Notes:
Overall: PASS References: §30833275013
|
🔒 mcpg Read-Only Stress — docker-sbxSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Gateway tool surface (B detail): Gateway exposes exactly 21 read-only tools. All 7 write tool names returned MCP error -32602 (unknown tool), confirming gateway-level enforcement. Overall: PASS References: §30833271178
|
Rust guard had duplicated repo-visibility fallback logic across response labeling paths, with both fail-open (
unwrap_or(false)) and fail-secure (None => !cfg!(test)) variants repeated inline. This PR consolidates those semantics into named helpers to keep policy behavior explicit and auditable in one place.Helper centralization (single source of truth)
repo_private_fallback(owner, repo)inlabels/helpers.rsfor best-effort fail-open defaulting.repo_private_or_secure_default(owner, repo)inlabels/helpers.rsfor security-sensitive fail-secure defaulting (test-mode exception preserved).Call-site deduplication
labels/response_items.rs(PR/issues collection defaults)labels/response_paths.rs(repo item context defaults)labels/response_items.rs(commit labeling)labels/response_paths.rs(commit path labeling)labels/tool_rules.rs(commit tool-rule integrity fallback)Behavior-preserving clarity