Skip to content

Conversation

@aegilops
Copy link
Collaborator

No description provided.

…y default, allow switching off either, and listing a custom set of types if needed
@aegilops aegilops requested a review from a team as a code owner November 21, 2025 11:24
@aegilops aegilops requested review from adrienpessu, Copilot and felickz and removed request for a team November 21, 2025 11:24
Copilot finished reviewing on behalf of aegilops November 21, 2025 11:27
Copy link

Copilot AI left a comment

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 refactors the secret scanning alerts listing functionality to provide more granular control over which types of secrets are included in the output. Instead of a single --generic flag to include generic secrets, the new approach allows users to independently control default secrets, generic secrets, and specify custom secret types.

Key changes:

  • Changed from single --generic flag to --no-generic and --no-default flags with --include-types for custom types
  • Modified the alert collection to support fetching multiple categories of secrets (default, generic, and specific types)
  • Updated the API to accept a secret_types parameter for custom secret type filtering

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
list_secret_scanning_alerts.py Added support for separate default/generic/specific secret type filtering; changed parameter signature of decorate_alerts to accept Iterable instead of Generator; updated argument parsing to use --no-generic, --no-default, and --include-types
githubapi.py Added secret_types parameter to list_secret_scanning_alerts method to support custom secret type filtering
README.md Updated documentation to reflect the new command-line interface with --no-generic, --no-default, and --include-types options

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

Comment on lines +241 to +261
alerts = []

if default:
default_alerts = g.list_secret_scanning_alerts(
name, state=state, since=since, scope=scope, bypassed=bypassed, generic=False, progress=progress
)

alerts.append(default_alerts)

if generic:
generic_alerts = g.list_secret_scanning_alerts(
name, state=state, since=since, scope=scope, bypassed=bypassed, generic=True, progress=progress
)

alerts.append(generic_alerts)

if specific:
specific_alerts = g.list_secret_scanning_alerts(
name, state=state, since=since, scope=scope, bypassed=bypassed, secret_types=specific, progress=progress
)
alerts.append(specific_alerts)
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code appends generators to a list, but decorate_alerts expects a flat iterable of alert dictionaries. When decorate_alerts iterates over alerts, it will iterate over the generator objects themselves, not the alert dictionaries within them.

Consider using itertools.chain to chain the generators together:

from itertools import chain

alert_generators = []

if default:
    default_alerts = g.list_secret_scanning_alerts(...)
    alert_generators.append(default_alerts)

if generic:
    generic_alerts = g.list_secret_scanning_alerts(...)
    alert_generators.append(generic_alerts)

if specific:
    specific_alerts = g.list_secret_scanning_alerts(...)
    alert_generators.append(specific_alerts)

alerts = chain.from_iterable(alert_generators)

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

@aegilops aegilops merged commit 0405604 into fix-quality Nov 21, 2025
7 checks passed
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.

2 participants