Skip to content

rust-guard: simplify default_secrecy conditions; remove lifetime noise from integrity helpers#7762

Merged
lpcox merged 3 commits into
mainfrom
copilot/rust-guard-simplify-default-secrecy
Jun 19, 2026
Merged

rust-guard: simplify default_secrecy conditions; remove lifetime noise from integrity helpers#7762
lpcox merged 3 commits into
mainfrom
copilot/rust-guard-simplify-default-secrecy

Conversation

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Two small cleanups in the Rust guard identified by the Rust Guard Improver workflow.

response_paths.rs — negative check for default_secrecy

The PR and Issue match arms computed default_secrecy via a positive list of tool names, but the actual invariant is "everything in this arm gets repo-scoped secrecy except the search tool." The positive list silently breaks whenever a new tool is added to the outer arm.

// Before — must be kept in sync with the outer match arm
let default_secrecy = if tool_name == "list_pull_requests"
    || tool_name == "pull_request_read"
    || tool_name == "get_pull_request"
{}

// After — expresses the invariant directly, future-proof
let default_secrecy = if tool_name != "search_pull_requests" {}

Same change applied to the Issue arm (!= "search_issues").

helpers.rs&'static str + validated fallback for integrity helpers

effective_disapproval_integrity and effective_endorser_min_integrity carried an 'a lifetime tying the return to the ctx borrow, even though all callers only pass the result to functions returning owned values. Beyond the lifetime noise, invalid config values (e.g. "strict") were silently forwarded through integrity_level_rank's generic fallback with no indication of which field was misconfigured.

Both functions now:

  • Return &'static str (no lifetime)
  • Parse through MinIntegrity::from_policy_str, log a named warning on unrecognised values, and canonicalize to the correct default (none / approved) at the boundary

GitHub Advanced Security started work on behalf of lpcox June 19, 2026 04:34 View session
GitHub Advanced Security finished work on behalf of lpcox June 19, 2026 04:36
…in Rust guard

- response_paths.rs: Replace positive-list default_secrecy checks in the
  PR arm and Issue arm with negative checks (!=  "search_pull_requests" /
  != "search_issues"). This directly expresses the invariant and stays
  correct when new tools are added to the outer match arm.

- helpers.rs: Remove unnecessary 'a lifetime from
  effective_disapproval_integrity and effective_endorser_min_integrity.
  Both now return &'static str and validate unrecognized config values via
  MinIntegrity::from_policy_str, logging a warning and canonicalizing to
  the correct default (none / approved) at the boundary.

Closes #7722
GitHub Advanced Security started work on behalf of lpcox June 19, 2026 04:40 View session
Copilot AI changed the title [WIP] Update default_secrecy conditions in response_paths rust-guard: simplify default_secrecy conditions; remove lifetime noise from integrity helpers Jun 19, 2026
Copilot finished work on behalf of lpcox June 19, 2026 04:40
Copilot AI requested a review from lpcox June 19, 2026 04:40
GitHub Advanced Security finished work on behalf of lpcox June 19, 2026 04:41
@lpcox lpcox marked this pull request as ready for review June 19, 2026 13:48
Copilot AI review requested due to automatic review settings June 19, 2026 13:48

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

This PR makes small maintainability improvements to the Rust GitHub guard labeling logic by (1) expressing default secrecy rules for PR/Issue response labeling more robustly, and (2) simplifying integrity helper lifetimes while hardening handling of invalid integrity configuration values.

Changes:

  • Update PR/Issue response-path labeling to compute default_secrecy via a negative check (everything in the arm except the corresponding search_* tool).
  • Update integrity helper functions to return &'static str and to validate/canonicalize configured integrity levels via MinIntegrity::from_policy_str, emitting targeted warnings on invalid values.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/labels/response_paths.rs Simplifies default_secrecy branching for PR/Issue tool match arms by excluding only the search_* variants.
guards/github-guard/rust-guard/src/labels/helpers.rs Removes unnecessary lifetimes from integrity helpers and validates misconfigured integrity strings with a warning + safe default.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment on lines +568 to +576
MinIntegrity::from_policy_str(v)
.unwrap_or_else(|| {
crate::log_warn(&format!(
"effective_disapproval_integrity: unrecognised level {:?}, defaulting to 'none'",
v
));
MinIntegrity::None
})
.as_str()
@lpcox

lpcox commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Done — added test_effective_disapproval_integrity_invalid_value_defaults_to_none and test_effective_endorser_min_integrity_invalid_value_defaults_to_approved in helpers.rs. Both set the respective field to "strict" and assert the canonical fallback value ("none" / "approved"). All four test_effective_* tests pass.

GitHub Advanced Security started work on behalf of lpcox June 19, 2026 14:39 View session
Copilot finished work on behalf of lpcox June 19, 2026 14:39
GitHub Advanced Security finished work on behalf of lpcox June 19, 2026 14:40
@lpcox lpcox merged commit 6552d08 into main Jun 19, 2026
27 checks passed
@lpcox lpcox deleted the copilot/rust-guard-simplify-default-secrecy branch June 19, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants