🦀 Rust Guard Improvement Report
Improvement 1: Replace Magic String Literals with policy_integrity Constants
Category: Centralization
File(s): guards/github-guard/rust-guard/src/labels/helpers.rs
Effort: Small (< 15 min)
Risk: Low
Problem
Two private helper functions in helpers.rs — effective_disapproval_integrity (line ~453) and effective_endorser_min_integrity (line ~459) — use raw string literals "none" and "approved" as their fallback values. Every other part of the codebase that references these integrity levels uses the policy_integrity::NONE and policy_integrity::APPROVED constants from constants.rs. The two literals stand out as inconsistent outliers and create a subtle drift risk: if the canonical string ever changes (or a typo is introduced), these two defaults would silently produce wrong labels.
Before
fn effective_disapproval_integrity<'a>(ctx: &'a PolicyContext) -> &'a str {
let v = ctx.disapproval_integrity.trim();
if v.is_empty() { "none" } else { v }
}
/// Return the effective `endorser_min_integrity` level from context, defaulting to "approved".
fn effective_endorser_min_integrity<'a>(ctx: &'a PolicyContext) -> &'a str {
let v = ctx.endorser_min_integrity.trim();
if v.is_empty() { "approved" } else { v }
}
After
fn effective_disapproval_integrity<'a>(ctx: &'a PolicyContext) -> &'a str {
let v = ctx.disapproval_integrity.trim();
if v.is_empty() { super::constants::policy_integrity::NONE } else { v }
}
/// Return the effective `endorser_min_integrity` level from context, defaulting to "approved".
fn effective_endorser_min_integrity<'a>(ctx: &'a PolicyContext) -> &'a str {
let v = ctx.endorser_min_integrity.trim();
if v.is_empty() { super::constants::policy_integrity::APPROVED } else { v }
}
Or, since policy_integrity is already imported at the top of the file via use super::constants::policy_integrity as pi in adjacent functions, add a module-level alias and use pi::NONE / pi::APPROVED.
Why This Matters
- Makes the default values grep-able alongside every other use of
policy_integrity::NONE and policy_integrity::APPROVED
- Eliminates two magic strings; a future rename of these constants would be caught at compile time
- Zero behavior change —
policy_integrity::NONE == "none" and policy_integrity::APPROVED == "approved" are verified by the existing order_low_to_high_piped_matches_order_high_to_low test
Improvement 2: Add Unit Tests for Security-Sensitive apply_tool_labels Arms
Category: Test Coverage
File(s): guards/github-guard/rust-guard/src/labels/tool_rules.rs
Effort: Medium (15–30 min)
Risk: Low
Problem
apply_tool_labels in tool_rules.rs (971 lines, 14 tests) includes several security-sensitive match arms that force private: secrecy labels regardless of repository visibility — yet none of these arms have any unit tests:
| Tool(s) |
Lines |
Security property |
list_secret_scanning_alerts, get_secret_scanning_alert |
~323–329 |
always policy_private_scope_label — secrets in public repos still private |
list_code_scanning_alerts, get_code_scanning_alert, list_dependabot_alerts, get_dependabot_alert |
~332–340 |
always policy_private_scope_label |
get_job_logs |
~343–349 |
always policy_private_scope_label — CI logs may contain leaked tokens |
All three blocks share the same policy: secrecy is unconditionally policy_private_scope_label(owner, repo, repo_id, ctx) and integrity is writer_integrity(repo_id, ctx). A refactor that accidentally applies apply_repo_visibility_secrecy instead would silently expose these tools for public repos — there is currently no test to catch that regression.
Suggested Change
Add one test per security-sensitive group to tool_rules.rs (inside the existing #[cfg(test)] block). The pattern mirrors the existing apply_tool_labels_list_repository_collaborators_is_repo_scoped_metadata test:
#[test]
fn apply_tool_labels_secret_scanning_is_always_private() {
let ctx = test_ctx();
let args = serde_json::json!({"owner": "octocat", "repo": "hello-world"});
let repo_id = "octocat/hello-world";
for tool in &["list_secret_scanning_alerts", "get_secret_scanning_alert"] {
let (secrecy, integrity, _desc) = apply_tool_labels(
tool, &args, repo_id, vec![], vec![], String::new(), &ctx,
);
// Must be private even for a public repo — never empty secrecy
assert!(
secrecy.iter().any(|s| s.starts_with("private:")),
"{tool}: expected private secrecy, got {secrecy:?}",
);
assert!(
integrity.iter().any(|s| s.starts_with("approved:")),
"{tool}: expected writer integrity, got {integrity:?}",
);
}
}
#[test]
fn apply_tool_labels_code_scanning_and_dependabot_are_always_private() {
let ctx = test_ctx();
let args = serde_json::json!({"owner": "octocat", "repo": "hello-world"});
let repo_id = "octocat/hello-world";
for tool in &[
"list_code_scanning_alerts",
"get_code_scanning_alert",
"list_dependabot_alerts",
"get_dependabot_alert",
] {
let (secrecy, integrity, _desc) = apply_tool_labels(
tool, &args, repo_id, vec![], vec![], String::new(), &ctx,
);
assert!(
secrecy.iter().any(|s| s.starts_with("private:")),
"{tool}: expected private secrecy, got {secrecy:?}",
);
assert!(
integrity.iter().any(|s| s.starts_with("approved:")),
"{tool}: expected writer integrity, got {integrity:?}",
);
}
}
#[test]
fn apply_tool_labels_get_job_logs_is_always_private() {
let ctx = test_ctx();
let args = serde_json::json!({"owner": "octocat", "repo": "hello-world"});
let repo_id = "octocat/hello-world";
let (secrecy, integrity, _desc) = apply_tool_labels(
"get_job_logs", &args, repo_id, vec![], vec![], String::new(), &ctx,
);
assert!(
secrecy.iter().any(|s| s.starts_with("private:")),
"get_job_logs: expected private secrecy, got {secrecy:?}",
);
assert!(
integrity.iter().any(|s| s.starts_with("approved:")),
"get_job_logs: expected writer integrity, got {integrity:?}",
);
}
Why This Matters
- Security regressions in these arms would silently expose secret-scanning alerts, Dependabot findings, and CI logs on public repos — exactly the kind of data leakage DIFC is designed to prevent
- The existing 2-test coverage for
apply_tool_labels is proportionally very low for a 971-line function with dozens of match arms; adding tests for the highest-risk arms first is the right priority
Codebase Health Summary
- Total Rust files: 10
- Total lines: 14,627
- Areas analyzed: lib.rs, tools.rs, labels/mod.rs, labels/helpers.rs, labels/backend.rs, labels/tool_rules.rs, labels/response_items.rs, labels/response_paths.rs, labels/constants.rs
- Areas with no further improvements:
labels/backend.rs (all dead code and duplication items addressed in prior runs)
Generated by Rust Guard Improver • Run: §26396258859
References:
Generated by Rust Guard Improver · ● 1.9M · ◷
🦀 Rust Guard Improvement Report
Improvement 1: Replace Magic String Literals with
policy_integrityConstantsCategory: Centralization
File(s):
guards/github-guard/rust-guard/src/labels/helpers.rsEffort: Small (< 15 min)
Risk: Low
Problem
Two private helper functions in
helpers.rs—effective_disapproval_integrity(line ~453) andeffective_endorser_min_integrity(line ~459) — use raw string literals"none"and"approved"as their fallback values. Every other part of the codebase that references these integrity levels uses thepolicy_integrity::NONEandpolicy_integrity::APPROVEDconstants fromconstants.rs. The two literals stand out as inconsistent outliers and create a subtle drift risk: if the canonical string ever changes (or a typo is introduced), these two defaults would silently produce wrong labels.Before
After
Or, since
policy_integrityis already imported at the top of the file viause super::constants::policy_integrity as piin adjacent functions, add a module-level alias and usepi::NONE/pi::APPROVED.Why This Matters
policy_integrity::NONEandpolicy_integrity::APPROVEDpolicy_integrity::NONE == "none"andpolicy_integrity::APPROVED == "approved"are verified by the existingorder_low_to_high_piped_matches_order_high_to_lowtestImprovement 2: Add Unit Tests for Security-Sensitive
apply_tool_labelsArmsCategory: Test Coverage
File(s):
guards/github-guard/rust-guard/src/labels/tool_rules.rsEffort: Medium (15–30 min)
Risk: Low
Problem
apply_tool_labelsintool_rules.rs(971 lines, 14 tests) includes several security-sensitive match arms that forceprivate:secrecy labels regardless of repository visibility — yet none of these arms have any unit tests:list_secret_scanning_alerts,get_secret_scanning_alertpolicy_private_scope_label— secrets in public repos still privatelist_code_scanning_alerts,get_code_scanning_alert,list_dependabot_alerts,get_dependabot_alertpolicy_private_scope_labelget_job_logspolicy_private_scope_label— CI logs may contain leaked tokensAll three blocks share the same policy: secrecy is unconditionally
policy_private_scope_label(owner, repo, repo_id, ctx)and integrity iswriter_integrity(repo_id, ctx). A refactor that accidentally appliesapply_repo_visibility_secrecyinstead would silently expose these tools for public repos — there is currently no test to catch that regression.Suggested Change
Add one test per security-sensitive group to
tool_rules.rs(inside the existing#[cfg(test)]block). The pattern mirrors the existingapply_tool_labels_list_repository_collaborators_is_repo_scoped_metadatatest:Why This Matters
apply_tool_labelsis proportionally very low for a 971-line function with dozens of match arms; adding tests for the highest-risk arms first is the right priorityCodebase Health Summary
labels/backend.rs(all dead code and duplication items addressed in prior runs)Generated by Rust Guard Improver • Run: §26396258859
References: