Conversation
- Updated documentation to reflect the change from `collect_alert.sh` to `collect_alert.py`. - Introduced a new script `sync_security_alerts.py` to orchestrate the security alert synchronization pipeline, including label checks, alert collection, and issue promotion. - Added unit tests for the new `sync_security_alerts.py` functionality. - Created a new test suite for `collect_alert.py` to ensure proper functionality and coverage. - Enhanced argument parsing in `sync_security_alerts.py` to support additional options such as severity priority mapping and project details. - Implemented logic to handle existing output files with a force option.
There was a problem hiding this comment.
Pull request overview
This PR completes “phase 2” of the refactor by replacing the remaining shell-based security automation entrypoints with Python equivalents, adding unit tests, and updating docs/workflows to point at the new scripts.
Changes:
- Replace
sync_security_alerts.shandcheck_labels.shwith Python implementations and add unit tests. - Extend
collect_alert.pynormalization and add unit tests for alert collection/serialization. - Update documentation and GitHub workflow wiring to reference the new Python entrypoints.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/security/sync_security_alerts.sh |
Removed legacy bash orchestrator. |
src/security/sync_security_alerts.py |
Added Python orchestrator to run label-check → collect → promote pipeline. |
src/security/check_labels.sh |
Removed legacy bash label checker. |
src/security/check_labels.py |
Added Python label checker using gh. |
src/security/collect_alert.py |
Updated alert normalization/output schema and added parsing helpers. |
src/security/promote_alerts.py |
Updated CLI entrypoints to accept argv (easier orchestration/testing) and updated references to collect_alert.py. |
src/security/utils/issue_sync.py |
Doc reference updated to collect_alert.py. |
src/security/utils/alert_parser.py |
Doc reference updated to collect_alert.py. |
tests/security/test_sync_security_alerts.py |
Added unit tests for the new orchestrator wiring/flags. |
tests/security/test_collect_alert.py |
Added unit tests for collect_alert.py parsing, GH API helpers, normalization, and output. |
tests/security/test_check_labels.py |
Added unit tests for check_labels.py. |
docs/security/security.md |
Updated documentation to use Python scripts and removed jq prerequisite. |
DEVELOPERS.md |
Updated developer docs to reflect new script layout and tools. |
.github/workflows/aquasec-scan.yml |
Updated workflow to run Python orchestrator. |
.gitignore |
Ignored root .coverage. |
Comments suppressed due to low confidence (1)
docs/security/security.md:220
- This section says the automation requires “exactly these five labels”, but
src/security/check_labels.pycurrently enforces only 4 labels (scope:security, type:tech-debt, epic, sec:adept-to-close) and there’s no reference tosec:src/aquasec-sarifin the codebase. Please either updatecheck_labels.py/constants to enforce the fifth label, or adjust the documentation/table to match the actual required label contract.
The automation requires exactly these five labels to exist in the target repository (enforced by `check_labels.py`):
| Label | Purpose |
| --- | --- |
| `scope:security` | Applied to every security Issue; used by `promote_alerts.py --issue-label` to discover existing Issues |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 23 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Added AlertMetadata, AlertDetails, RuleDetails, and Alert classes to encapsulate alert data. - Implemented from_dict method for Alert class to facilitate construction from raw data. - Updated LoadedAlerts class to store alerts in a structured manner. - Refactored tests to utilize the new Alert data models, ensuring consistency and type safety. - Modified existing alert enrichment logic to align with the new data structure. - Enhanced test cases to validate the new alert model integration across various scenarios.
- Added multiple issue templates for bug reports, feature requests, documentation tasks, DevOps tasks, and more to streamline issue tracking. - Introduced CODEOWNERS file to define code ownership for better collaboration. - Updated requirements.txt to include additional dependencies: pytest-mock, pylint, black, and mypy for improved code quality and testing. - Created GitHub Actions workflows for checking PR release notes, static code analysis with Pylint, code formatting with Black, unit testing with pytest, and type checking with mypy. - Added .pylintrc configuration file to enforce coding standards and linting rules.
- Updated string formatting to use f-strings for better readability. - Standardized the use of double quotes for strings across multiple files. - Removed unnecessary blank lines and added missing newlines at the end of files. - Improved logging messages for clarity and consistency. - Simplified list comprehensions and dictionary initializations for better performance. - Added comments to clarify the purpose of certain functions and classes. - Ensured that all environment variable defaults are handled consistently. - Enhanced the handling of optional fields in data classes to use NOT_AVAILABLE defaults. - Cleaned up imports and removed unused ones across various modules.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 54 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 55 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…nd correct label count in security automation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 55 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
tmikula-dev
left a comment
tmikula-dev
left a comment
There was a problem hiding this comment.
Please look at the review comments, if you find they should be currently implemented, please do so. The code quality looks much much better now. Thanks!
Release Notes
sync_security_alerts.shtosync_security_alerts.pyand provided unit tests.check_labels.shtocheck_labels.pyand provided unit tests.collect_alert.py.