[rust-guard] Move test-only function out of production code; add missing field_names constants (#3096)

Two small code quality improvements to the Rust guard: dead production
code inflating the WASM binary, and JSON field strings scattered as raw
literals instead of using the existing `field_names` convention.

## Changes

### 1. `is_forked_pull_request_with_callback` → `#[cfg(test)]` only
(`backend.rs`)

The function was `pub` in production code with `#[allow(dead_code)]`
suppressing the warning, but was only ever called from tests. Moved it
into the `#[cfg(test)] mod tests` block as a private `fn`, removing the
suppression annotation and eliminating the dead WASM binary code.

```rust
// Before: production code
#[allow(dead_code)]
pub fn is_forked_pull_request_with_callback(...) -> Option<bool> { ... }

// After: test-only
#[cfg(test)]
mod tests {
    fn is_forked_pull_request_with_callback(...) -> Option<bool> { ... }
}
```

### 2. New `field_names` constants + replacement of raw literals
(`constants.rs`, `helpers.rs`, `response_items.rs`, `response_paths.rs`)

Added `FULL_NAME`, `NUMBER`, `PRIVATE`, and `LOGIN` to the `field_names`
module alongside the existing `OWNER`, `REPO`, `SHA`, etc. Replaced all
production-code JSON field string lookups with the new constants across
the four affected files. Added `use super::constants::field_names;` to
`response_items.rs` and `response_paths.rs` which previously lacked the
import.

Note: `"private".to_string()` in secrecy label constructions (not field
lookups) are intentionally left as-is.

> [!WARNING]
>
> <details>
> <summary>Firewall rules blocked me from connecting to one or more
addresses (expand for details)</summary>
>
> #### I tried to connect to the following addresses, but was blocked by
firewall rules:
>
> - `example.com`
> - Triggering command: `/tmp/go-build3562526043/b336/launcher.test
/tmp/go-build3562526043/b336/launcher.test
-test.testlogfile=/tmp/go-build3562526043/b336/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/net
64/src/mime/encodedword.go x_amd64/vet -p crypto/internal/-atomic
-lang=go1.25 x_amd64/vet -o ify@v1.11.1/asse-errorsas -trimpath` (dns
block)
> - `invalid-host-that-does-not-exist-12345.com`
> - Triggering command: `/tmp/go-build3562526043/b318/config.test
/tmp/go-build3562526043/b318/config.test
-test.testlogfile=/tmp/go-build3562526043/b318/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/cgo
2zQN/vEak0ZJ9yzSgbLLs2zQN x_amd64/vet
guard.3t4ldemkr1/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet
mcpgodebug /home/REDACTED/.ru-unreachable=false x_amd64/vet 1226��
122696/b119/_pkg_.a nn_n/bFgeOALOdCKszpT-nn_n x_amd64/vet
/home/REDACTED/wor/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet
/home/REDACTED/wor-unsafeptr=false /home/REDACTED/wor-unreachable=false
x_amd64/vet` (dns block)
> - `nonexistent.local`
> - Triggering command: `/tmp/go-build3562526043/b336/launcher.test
/tmp/go-build3562526043/b336/launcher.test
-test.testlogfile=/tmp/go-build3562526043/b336/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/net
64/src/mime/encodedword.go x_amd64/vet -p crypto/internal/-atomic
-lang=go1.25 x_amd64/vet -o ify@v1.11.1/asse-errorsas -trimpath` (dns
block)
> - `slow.example.com`
> - Triggering command: `/tmp/go-build3562526043/b336/launcher.test
/tmp/go-build3562526043/b336/launcher.test
-test.testlogfile=/tmp/go-build3562526043/b336/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/net
64/src/mime/encodedword.go x_amd64/vet -p crypto/internal/-atomic
-lang=go1.25 x_amd64/vet -o ify@v1.11.1/asse-errorsas -trimpath` (dns
block)
> - `this-host-does-not-exist-12345.com`
> - Triggering command: `/tmp/go-build3562526043/b345/mcp.test
/tmp/go-build3562526043/b345/mcp.test
-test.testlogfile=/tmp/go-build3562526043/b345/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true _.a
58122696/b151//_-ifaceassert x_amd64/vet go1.25.8
ternal/engine/wa--version 122696/b151/ x_amd64/vet ctor�� .cfg
122696/b151/ x_amd64/vet --gdwarf-5 ut-1903621372.c -o x_amd64/vet` (dns
block)
>
> If you need me to access, download, or install something from one of
these locations, you can either:
>
> - Configure [Actions setup
steps](https://gh.io/copilot/actions-setup-steps) to set up my
environment, which run before the firewall is enabled
> - Add the appropriate URLs or hosts to the custom allowlist in this
repository's [Copilot coding agent
settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent)
(admins only)
>
> </details>

Author: lpcox

guards/github-guard/rust-guard/src/labels/backend.rs

@@ -264,75 +264,6 @@ pub fn is_repo_org_owned(owner: &str, repo: &str) -> Option<bool> {
     get_cached_owner_is_org(&repo_id)
 }
 
-/// Determine whether a pull request is from a fork.
-///
-/// This helper calls `pull_request_read` through the provided backend callback,
-/// extracts `base.repo.full_name` and `head.repo.full_name`, and returns:
-/// - `Some(true)` if the PR is from a fork (head repo differs from base repo)
-/// - `Some(false)` if the PR is direct (same repository)
-/// - `None` if the result cannot be determined
-#[allow(dead_code)]
-pub fn is_forked_pull_request_with_callback(
-    callback: GithubMcpCallback,
-    owner: &str,
-    repo: &str,
-    pull_number: &str,
-) -> Option<bool> {
-    if owner.is_empty() || repo.is_empty() || pull_number.is_empty() {
-        return None;
-    }
-
-    let args = serde_json::json!({
-        "owner": owner,
-        "repo": repo,
-        "pullNumber": pull_number,
-        "method": "get",
-    });
-
-    let args_str = args.to_string();
-    let mut result_buffer = vec![0u8; SMALL_BUFFER_SIZE];
-
-    crate::log_debug(&format!(
-        "Checking PR origin for {}/{}#{}",
-        owner, repo, pull_number
-    ));
-
-    let len = match callback("pull_request_read", &args_str, &mut result_buffer) {
-        Ok(len) if len > 0 => len,
-        Ok(_) => return None,
-        Err(code) => {
-            crate::log_warn(&format!(
-                "Failed to check PR origin for {}/{}#{}: error code {}",
-                owner, repo, pull_number, code
-            ));
-            return None;
-        }
-    };
-
-    let response_str = std::str::from_utf8(&result_buffer[..len]).ok()?;
-    let response = serde_json::from_str::<Value>(response_str).ok()?;
-    let pr = super::extract_mcp_response(&response);
-
-    let base_full_name = pr
-        .get("base")
-        .and_then(|b| b.get("repo"))
-        .and_then(|r| r.get("full_name"))
-        .and_then(|v| v.as_str());
-
-    let head_full_name = pr
-        .get("head")
-        .and_then(|h| h.get("repo"))
-        .and_then(|r| r.get("full_name"))
-        .and_then(|v| v.as_str());
-
-    match (base_full_name, head_full_name) {
-        (Some(base), Some(head)) if !base.is_empty() && !head.is_empty() => {
-            Some(!base.eq_ignore_ascii_case(head))
-        }
-        _ => None,
-    }
-}
-
 /// Fetch pull request facts used for integrity derivation.
 pub fn get_pull_request_facts_with_callback(
     callback: GithubMcpCallback,
@@ -674,9 +605,78 @@ fn extract_rate_reset_seconds(error_text: &str) -> Option<u64> {
 
 #[cfg(test)]
 mod tests {
+    use super::super::constants::field_names;
     use super::*;
     use std::sync::atomic::{AtomicUsize, Ordering};
 
+    /// Determine whether a pull request is from a fork.
+    ///
+    /// This helper calls `pull_request_read` through the provided backend callback,
+    /// extracts `base.repo.full_name` and `head.repo.full_name`, and returns:
+    /// - `Some(true)` if the PR is from a fork (head repo differs from base repo)
+    /// - `Some(false)` if the PR is direct (same repository)
+    /// - `None` if the result cannot be determined
+    fn is_forked_pull_request_with_callback(
+        callback: GithubMcpCallback,
+        owner: &str,
+        repo: &str,
+        pull_number: &str,
+    ) -> Option<bool> {
+        if owner.is_empty() || repo.is_empty() || pull_number.is_empty() {
+            return None;
+        }
+
+        let args = serde_json::json!({
+            "owner": owner,
+            "repo": repo,
+            "pullNumber": pull_number,
+            "method": "get",
+        });
+
+        let args_str = args.to_string();
+        let mut result_buffer = vec![0u8; SMALL_BUFFER_SIZE];
+
+        crate::log_debug(&format!(
+            "Checking PR origin for {}/{}#{}",
+            owner, repo, pull_number
+        ));
+
+        let len = match callback("pull_request_read", &args_str, &mut result_buffer) {
+            Ok(len) if len > 0 => len,
+            Ok(_) => return None,
+            Err(code) => {
+                crate::log_warn(&format!(
+                    "Failed to check PR origin for {}/{}#{}: error code {}",
+                    owner, repo, pull_number, code
+                ));
+                return None;
+            }
+        };
+
+        let response_str = std::str::from_utf8(&result_buffer[..len]).ok()?;
+        let response = serde_json::from_str::<Value>(response_str).ok()?;
+        let pr = crate::labels::extract_mcp_response(&response);
+
+        let base_full_name = pr
+            .get("base")
+            .and_then(|b| b.get("repo"))
+            .and_then(|r| r.get(field_names::FULL_NAME))
+            .and_then(|v| v.as_str());
+
+        let head_full_name = pr
+            .get("head")
+            .and_then(|h| h.get("repo"))
+            .and_then(|r| r.get(field_names::FULL_NAME))
+            .and_then(|v| v.as_str());
+
+        match (base_full_name, head_full_name) {
+            (Some(base), Some(head)) if !base.is_empty() && !head.is_empty() => {
+                Some(!base.eq_ignore_ascii_case(head))
+            }
+            _ => None,
+        }
+    }
+
     fn copy_payload(payload: Value, buffer: &mut [u8]) -> Result<usize, i32> {
         let serialized = payload.to_string();
         let bytes = serialized.as_bytes();

guards/github-guard/rust-guard/src/labels/constants.rs

@@ -39,6 +39,11 @@ pub mod field_names {
     pub const SHA: &str = "sha";
     pub const MERGED_AT: &str = "merged_at";
     pub const MERGED: &str = "merged";
+    // Commonly accessed response fields
+    pub const FULL_NAME: &str = "full_name";
+    pub const NUMBER: &str = "number";
+    pub const PRIVATE: &str = "private";
+    pub const LOGIN: &str = "login";
 }
 
 /// Sensitive file patterns for detecting secret-containing files

guards/github-guard/rust-guard/src/labels/helpers.rs

@@ -12,7 +12,7 @@ use super::constants::{field_names, label_constants};
 /// number segment from `html_url` or `url` (e.g. `.../issues/123` → `123`).
 /// Returns "unknown" (with a log warning) if no number can be determined.
 pub(crate) fn extract_resource_number(item: &Value, resource_type: &str, repo: &str) -> String {
-    if let Some(n) = item.get("number").and_then(|v| v.as_u64()) {
+    if let Some(n) = item.get(field_names::NUMBER).and_then(|v| v.as_u64()) {
         return n.to_string();
     }
     // Fallback: extract trailing number from html_url or url
@@ -298,7 +298,7 @@ fn apply_approval_label_promotion(
     ctx: &PolicyContext,
 ) -> Vec<String> {
     if let Some(label) = first_matching_approval_label(item, ctx) {
-        let number = item.get("number").and_then(|v| v.as_u64()).unwrap_or(0);
+        let number = item.get(field_names::NUMBER).and_then(|v| v.as_u64()).unwrap_or(0);
         crate::log_info(&format!(
             "[integrity] {}:{}#{} promoted to approved (label '{}' in approval-labels)",
             resource_type, repo_full_name, number, label
@@ -603,13 +603,13 @@ pub(crate) fn extract_repo_from_github_url(url: &str) -> Option<String> {
 /// Returns empty string if no repo info found
 pub fn extract_repo_from_item(item: &Value) -> String {
     // Direct full_name (repositories)
-    if let Some(name) = item.get("full_name").and_then(|v| v.as_str()) {
+    if let Some(name) = item.get(field_names::FULL_NAME).and_then(|v| v.as_str()) {
         return name.to_string();
     }
     // repository.full_name (issues, PRs with repo info)
     if let Some(name) = item
         .get("repository")
-        .and_then(|r| r.get("full_name"))
+        .and_then(|r| r.get(field_names::FULL_NAME))
         .and_then(|v| v.as_str())
     {
         return name.to_string();
@@ -618,7 +618,7 @@ pub fn extract_repo_from_item(item: &Value) -> String {
     if let Some(name) = item
         .get("base")
         .and_then(|b| b.get("repo"))
-        .and_then(|r| r.get("full_name"))
+        .and_then(|r| r.get(field_names::FULL_NAME))
         .and_then(|v| v.as_str())
     {
         return name.to_string();
@@ -627,7 +627,7 @@ pub fn extract_repo_from_item(item: &Value) -> String {
     if let Some(name) = item
         .get("head")
         .and_then(|h| h.get("repo"))
-        .and_then(|r| r.get("full_name"))
+        .and_then(|r| r.get(field_names::FULL_NAME))
         .and_then(|v| v.as_str())
     {
         return name.to_string();
@@ -925,12 +925,12 @@ pub fn author_association_floor_from_str(
 /// Returns empty string if no login found.
 fn extract_author_login(item: &Value) -> &str {
     // Issues and PRs use user.login
-    let login = get_nested_str(item, "user", "login");
+    let login = get_nested_str(item, "user", field_names::LOGIN);
     if !login.is_empty() {
         return login;
     }
     // Commits use author.login
-    get_nested_str(item, "author", "login")
+    get_nested_str(item, "author", field_names::LOGIN)
 }
 
 /// Check whether an item contains an `author_association` (or `authorAssociation`) field.
@@ -1036,7 +1036,7 @@ pub fn pr_integrity(
     // Step 1: Check if author is in blocked_users — takes precedence over all other rules.
     let author_login = extract_author_login(item);
     if !author_login.is_empty() && is_blocked_user(author_login, ctx) {
-        let number = item.get("number").and_then(|v| v.as_u64()).unwrap_or(0);
+        let number = item.get(field_names::NUMBER).and_then(|v| v.as_u64()).unwrap_or(0);
         crate::log_info(&format!(
             "[integrity] pr:{}#{} → blocked (author '{}' in blocked-users)",
             repo_full_name, number, author_login
@@ -1062,7 +1062,7 @@ pub fn pr_integrity(
     // and merge status.
     if integrity.is_empty() && !has_author_association(item) && !repo_private {
         let number_opt = item
-            .get("number")
+            .get(field_names::NUMBER)
             .and_then(|v| v.as_u64())
             .map(|n| n.to_string())
             .or_else(|| extract_number_from_url(item));
@@ -1176,7 +1176,7 @@ pub fn issue_integrity(
     // Step 1: Check if author is in blocked_users — takes precedence over all other rules.
     let author_login = extract_author_login(item);
     if !author_login.is_empty() && is_blocked_user(author_login, ctx) {
-        let number = item.get("number").and_then(|v| v.as_u64()).unwrap_or(0);
+        let number = item.get(field_names::NUMBER).and_then(|v| v.as_u64()).unwrap_or(0);
         crate::log_info(&format!(
             "[integrity] issue:{}#{} → blocked (author '{}' in blocked-users)",
             repo_full_name, number, author_login
@@ -1192,7 +1192,7 @@ pub fn issue_integrity(
     // incorrectly assigning "none" integrity to members/collaborators.
     if integrity.is_empty() && !has_author_association(item) && !repo_private {
         let number_opt = item
-            .get("number")
+            .get(field_names::NUMBER)
             .and_then(|v| v.as_u64())
             .map(|n| n.to_string())
             .or_else(|| extract_number_from_url(item));

guards/github-guard/rust-guard/src/labels/response_items.rs

@@ -10,6 +10,7 @@
 //! Use path-based labeling (`label_response_paths`) when possible for better
 //! performance with large result sets.
 
+use super::constants::field_names;
 use super::extract_mcp_response;
 use super::helpers::*;
 use crate::{LabeledItem, ResourceLabels};
@@ -53,8 +54,8 @@ pub fn label_response_items(
 
                 let mut private_count = 0;
                 for (i, item) in items_to_process.iter().enumerate() {
-                    let is_private = get_bool_or(item, "private", false);
-                    let full_name = get_str_or(item, "full_name", "unknown");
+                    let is_private = get_bool_or(item, field_names::PRIVATE, false);
+                    let full_name = get_str_or(item, field_names::FULL_NAME, "unknown");
 
                     // Repository metadata has approved-level integrity (endorsed by maintainers)
                     let integrity = writer_integrity(full_name, ctx);
@@ -165,12 +166,12 @@ pub fn label_response_items(
                     let base_head_repo = item
                         .get("base")
                         .and_then(|b| b.get("repo"))
-                        .and_then(|r| r.get("full_name"))
+                        .and_then(|r| r.get(field_names::FULL_NAME))
                         .and_then(|v| v.as_str())
                         .or_else(|| {
                             item.get("head")
                                 .and_then(|h| h.get("repo"))
-                                .and_then(|r| r.get("full_name"))
+                                .and_then(|r| r.get(field_names::FULL_NAME))
                                 .and_then(|v| v.as_str())
                         })
                         .unwrap_or("");
@@ -192,12 +193,12 @@ pub fn label_response_items(
                     let is_forked = item
                         .get("base")
                         .and_then(|b| b.get("repo"))
-                        .and_then(|r| r.get("full_name"))
+                        .and_then(|r| r.get(field_names::FULL_NAME))
                         .and_then(|v| v.as_str())
                         .zip(
                             item.get("head")
                                 .and_then(|h| h.get("repo"))
-                                .and_then(|r| r.get("full_name"))
+                                .and_then(|r| r.get(field_names::FULL_NAME))
                                 .and_then(|v| v.as_str()),
                         )
                         .map(|(base, head)| !base.eq_ignore_ascii_case(head));

guards/github-guard/rust-guard/src/labels/response_paths.rs

@@ -7,6 +7,7 @@
 //! Returns JSON paths like `/items/0`, `/items/1` pointing to labeled objects
 //! in the response, rather than cloning the entire data.
 
+use super::constants::field_names;
 use super::extract_mcp_response;
 use super::helpers::*;
 use serde_json::Value;
@@ -70,8 +71,8 @@ pub fn label_response_paths(
                 let mut labeled_paths = Vec::with_capacity(limited_items.len());
 
                 for (i, item) in limited_items.iter().enumerate() {
-                    let is_private = get_bool_or(item, "private", false);
-                    let full_name = get_str_or(item, "full_name", "unknown");
+                    let is_private = get_bool_or(item, field_names::PRIVATE, false);
+                    let full_name = get_str_or(item, field_names::FULL_NAME, "unknown");
                     let integrity = writer_integrity(full_name, ctx);
 
                     let secrecy = if is_private {
@@ -171,13 +172,13 @@ pub fn label_response_paths(
                     let base_repo = item
                         .get("base")
                         .and_then(|b| b.get("repo"))
-                        .and_then(|r| r.get("full_name"))
+                        .and_then(|r| r.get(field_names::FULL_NAME))
                         .and_then(|v| v.as_str())
                         .unwrap_or("");
                     let head_repo = item
                         .get("head")
                         .and_then(|h| h.get("repo"))
-                        .and_then(|r| r.get("full_name"))
+                        .and_then(|r| r.get(field_names::FULL_NAME))
                         .and_then(|v| v.as_str())
                         .unwrap_or("");
                     let is_forked = if !base_repo.is_empty() && !head_repo.is_empty() {