Skip to content

[rust-guard] Rust Guard: Remove dead permissions.rs module and de-duplicate username lookup #2830

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Remove Dead permissions.rs Module

Category: Dead Code
File(s): guards/github-guard/rust-guard/src/permissions.rs, guards/github-guard/rust-guard/src/lib.rs
Effort: Small (< 15 min)
Risk: Low

Problem

permissions.rs is a 426-line module that is entirely suppressed with #![allow(dead_code)] at the top and is explicitly documented as:

"Note: This module is scaffolding for future permission-based integrity labeling. Not yet wired into production code paths."

The only reference in the entire codebase is mod permissions; on line 13 of lib.rs. No other module imports or uses permissions:: anything. Because this is a cdylib crate compiled to WASM, every unused function that Rust can't dead-strip at link time bloats the binary. The #![allow(dead_code)] lint suppression also masks any future accidental dead code in that file.

Suggested Change

  1. Delete guards/github-guard/rust-guard/src/permissions.rs
  2. Remove line 13 from lib.rs: mod permissions;

Before

// lib.rs line 13
mod permissions;
// permissions.rs — entire 426-line file
#![allow(dead_code)]
//! Note: This module is scaffolding for future permission-based
//! integrity labeling. Not yet wired into production code paths.
// ...PermissionLevel, Collaborator, RepoPermissions, fetch_repo_permissions,
//    get_author_integrity_tags, get_bot_integrity_tags — none referenced externally

After

// lib.rs line 13 — removed
// permissions.rs — file deleted

Why This Matters

  • WASM binary size: Dead code compiled to WASM that LTO can't strip adds unnecessary bytes to the .wasm output.
  • Maintenance burden: Future developers will waste time understanding code that has no effect.
  • Lint hygiene: The #![allow(dead_code)] suppression hides any future real dead code in the file.
  • If fetch_repo_permissions is ever needed, it can be reintroduced at that time from git history with proper integration.

Improvement 2: De-duplicate Case-Insensitive Username Membership Check

Category: Code Duplication
File(s): guards/github-guard/rust-guard/src/labels/helpers.rs
Effort: Small (< 15 min)
Risk: Low

Problem

The same pattern — lower-case a username, then check if any item in a Vec<String> matches case-insensitively — appears three times in three consecutive public functions:

  • is_blocked_user (line 231)
  • is_configured_trusted_bot (line 1355)
  • is_trusted_user (line 1369)

Each function body is structurally identical:

if <list>.is_empty() { return false; }
let lower = username.to_lowercase();
<list>.iter().any(|u| u.to_lowercase() == lower)

This also has a subtle inefficiency: .to_lowercase() is called once per list item, allocating a new String for every comparison, instead of using eq_ignore_ascii_case which compares without allocation.

Before

pub fn is_blocked_user(username: &str, ctx: &PolicyContext) -> bool {
    if ctx.blocked_users.is_empty() {
        return false;
    }
    let lower = username.to_lowercase();
    ctx.blocked_users.iter().any(|u| u.to_lowercase() == lower)
}

pub fn is_configured_trusted_bot(username: &str, ctx: &PolicyContext) -> bool {
    if ctx.trusted_bots.is_empty() {
        return false;
    }
    let lower = username.to_lowercase();
    ctx.trusted_bots.iter().any(|b| b.to_lowercase() == lower)
}

pub fn is_trusted_user(username: &str, ctx: &PolicyContext) -> bool {
    if ctx.trusted_users.is_empty() {
        return false;
    }
    let lower = username.to_lowercase();
    ctx.trusted_users.iter().any(|u| u.to_lowercase() == lower)
}

After

/// Returns true if `username` matches any entry in `list` (case-insensitive).
fn username_in_list(username: &str, list: &[String]) -> bool {
    list.iter().any(|u| u.eq_ignore_ascii_case(username))
}

pub fn is_blocked_user(username: &str, ctx: &PolicyContext) -> bool {
    username_in_list(username, &ctx.blocked_users)
}

pub fn is_configured_trusted_bot(username: &str, ctx: &PolicyContext) -> bool {
    username_in_list(username, &ctx.trusted_bots)
}

pub fn is_trusted_user(username: &str, ctx: &PolicyContext) -> bool {
    username_in_list(username, &ctx.trusted_users)
}

Why This Matters

  • Eliminates duplication: Three identical 6-line bodies collapse to one 2-line private helper.
  • Reduces allocations: eq_ignore_ascii_case compares bytes directly without allocating a lowercased String per item — a win in WASM where allocations are more expensive.
  • Single point of change: If the comparison semantics ever need to change (e.g., Unicode folding), there is one place to update instead of three.
  • Passes all existing tests unchanged: The public API and behavior are identical.

Codebase Health Summary

  • Total Rust files: 10
  • Total lines: 10,151
  • Areas analyzed: lib.rs, permissions.rs, tools.rs, labels/mod.rs, labels/backend.rs, labels/constants.rs, labels/helpers.rs, labels/response_items.rs, labels/response_paths.rs, labels/tool_rules.rs
  • Areas with no further improvements: (none yet — first run)

Generated by Rust Guard Improver • Run: §23738165481

Generated by Rust Guard Improver ·

  • expires on Apr 6, 2026, 9:44 AM UTC

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions