-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(lib): mitre d3fend #1394
feat(lib): mitre d3fend #1394
Conversation
WalkthroughThe pull request introduces a comprehensive update to the cybersecurity library tools, focusing on the integration of the Mitre D3FEND cybersecurity ontology. A new YAML file for D3FEND has been added, along with a Python script to generate an Excel library. The changes primarily involve adding new functionality for processing cybersecurity techniques, improving code formatting, and standardizing string representations across multiple utility scripts. Changes
Sequence DiagramsequenceDiagram
participant CSV as D3FEND CSV
participant Script as D3FEND Generator
participant Excel as Output Workbook
CSV->>Script: Read Cybersecurity Techniques
Script->>Script: Process Technique Data
Script->>Excel: Create Library Metadata Sheet
Script->>Excel: Create Controls Sheet
Script->>Excel: Populate Sheets with Technique Details
Excel-->>Script: Workbook Completed
Script->>Excel: Save Excel File
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 actionlint (1.7.4).github/workflows/functional-tests.yml120-120: shellcheck reported issue in this script: SC2086:info:1:76: Double quote to prevent globbing and word splitting (shellcheck) 226-226: shellcheck reported issue in this script: SC2086:info:1:76: Double quote to prevent globbing and word splitting (shellcheck) ⏰ Context from checks skipped due to timeout of 90000ms (10)
🔇 Additional comments (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tools/mitre/d3fend.csv
is excluded by!**/*.csv
tools/mitre/d3fend.xlsx
is excluded by!**/*.xlsx
📒 Files selected for processing (7)
backend/library/libraries/d3fend.yaml
(1 hunks)tools/convert_library.py
(7 hunks)tools/mitre/d3fend.py
(1 hunks)tools/mitre/mitre.py
(4 hunks)tools/nzism/prep_nzism.py
(4 hunks)tools/prepare_mapping.py
(3 hunks)tools/tisax/convert_tisax_5.1.py
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- tools/mitre/mitre.py
- tools/nzism/prep_nzism.py
- tools/tisax/convert_tisax_5.1.py
- tools/prepare_mapping.py
🧰 Additional context used
🪛 Ruff (0.8.2)
tools/convert_library.py
845-845: Use is
and is not
for type comparisons, or isinstance()
for isinstance checks
(E721)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: ruff (3.12)
- GitHub Check: test (3.12)
- GitHub Check: build (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
tools/mitre/d3fend.py (3)
1-28
: LGTM! Well-structured initialization.The imports, mapping dictionary, and library metadata are well-organized and appropriate for generating the D3FEND library.
45-45
: Review the hardcoded future publication date.The library_publication_date is set to "2025-01-22", which is a future date. Consider using the current date or making it configurable.
62-82
: LGTM! Well-structured CSV processing logic.The code effectively handles different technique levels and maintains consistent description formatting.
tools/convert_library.py (1)
14-14
: LGTM! Documentation and import changes are appropriate.The datetime import and updated documentation for library_publication_date default value are well-documented.
Also applies to: 115-115
backend/library/libraries/d3fend.yaml (2)
23-23
: Review the future publication date.The publication_date is set to "2025-01-22". Consider using the current date or making it configurable.
26-2214
: LGTM! Well-structured reference controls.The reference controls are consistently formatted and provide comprehensive information including URNs, categories, CSF functions, and detailed descriptions.
with open("d3fend.csv", newline="") as csvfile: | ||
reader = csv.reader(csvfile, delimiter=",") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for CSV file operations.
The script may fail silently if the CSV file is missing or inaccessible. Consider adding proper error handling.
-with open("d3fend.csv", newline="") as csvfile:
+try:
+ with open("d3fend.csv", newline="") as csvfile:
+ reader = csv.reader(csvfile, delimiter=",")
+except FileNotFoundError:
+ print("Error: d3fend.csv file not found")
+ exit(1)
+except PermissionError:
+ print("Error: Permission denied accessing d3fend.csv")
+ exit(1)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
with open("d3fend.csv", newline="") as csvfile: | |
reader = csv.reader(csvfile, delimiter=",") | |
try: | |
with open("d3fend.csv", newline="") as csvfile: | |
reader = csv.reader(csvfile, delimiter=",") | |
except FileNotFoundError: | |
print("Error: d3fend.csv file not found") | |
exit(1) | |
except PermissionError: | |
print("Error: Permission denied accessing d3fend.csv") | |
exit(1) |
(ref_id, name, description, "technical", tactic_to_csf_funtion[tactic]) | ||
) | ||
|
||
wb_output.save(output_file_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for workbook saving.
The workbook save operation could fail due to permission issues or disk space. Add proper error handling.
- wb_output.save(output_file_name)
+ try:
+ wb_output.save(output_file_name)
+ print(f"Successfully generated {output_file_name}")
+ except PermissionError:
+ print(f"Error: Permission denied saving {output_file_name}")
+ exit(1)
+ except Exception as e:
+ print(f"Error saving workbook: {str(e)}")
+ exit(1)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
wb_output.save(output_file_name) | |
try: | |
wb_output.save(output_file_name) | |
print(f"Successfully generated {output_file_name}") | |
except PermissionError: | |
print(f"Error: Permission denied saving {output_file_name}") | |
exit(1) | |
except Exception as e: | |
print(f"Error saving workbook: {str(e)}") | |
exit(1) |
lib_date = library_vars.get("library_publication_date", datetime.datetime.now()) | ||
if type(lib_date) == datetime.datetime: | ||
lib_date = lib_date.date() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use isinstance()
for type checking.
Replace type()
comparison with isinstance()
for better Python practices.
lib_date = library_vars.get("library_publication_date", datetime.datetime.now())
-if type(lib_date) == datetime.datetime:
+if isinstance(lib_date, datetime.datetime):
lib_date = lib_date.date()
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
lib_date = library_vars.get("library_publication_date", datetime.datetime.now()) | |
if type(lib_date) == datetime.datetime: | |
lib_date = lib_date.date() | |
lib_date = library_vars.get("library_publication_date", datetime.datetime.now()) | |
if isinstance(lib_date, datetime.datetime): | |
lib_date = lib_date.date() |
🧰 Tools
🪛 Ruff (0.8.2)
845-845: Use is
and is not
for type comparisons, or isinstance()
for isinstance checks
(E721)
try fix startup test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/functional-tests.yml (1)
Line range hint
145-148
: Maintain consistent action versions across jobs.The
setup-python
action is using different versions across jobs:
functional-tests
job uses v4enterprise-functional-tests
job uses v5This inconsistency could lead to different behavior between jobs.
Either update both jobs to use v5 or keep both at v4. Here's how to update the
functional-tests
job to match:- name: Set up python ${{ matrix.python-version }} - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} cache: "pip"🧰 Tools
🪛 actionlint (1.7.4)
118-118: shellcheck reported issue in this script: SC2086:info:1:44: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC1009:info:1:56: The mentioned syntax error was in this simple command
(shellcheck)
120-120: shellcheck reported issue in this script: SC1073:error:1:68: Couldn't parse this single quoted string. Fix to allow more checks
(shellcheck)
120-120: shellcheck reported issue in this script: SC1072:error:2:1: Expected end of single quoted string. Fix any mentioned problems and try again
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/functional-tests.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/functional-tests.yml
120-120: shellcheck reported issue in this script: SC1009:info:1:56: The mentioned syntax error was in this simple command
(shellcheck)
120-120: shellcheck reported issue in this script: SC1073:error:1:68: Couldn't parse this single quoted string. Fix to allow more checks
(shellcheck)
120-120: shellcheck reported issue in this script: SC1072:error:2:1: Expected end of single quoted string. Fix any mentioned problems and try again
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: test (3.12)
- GitHub Check: build (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
@@ -117,7 +117,7 @@ jobs: | |||
- name: Set current date as env variable | |||
run: echo "NOW=$(date +'%Y-%m-%dT%H-%M-%S')" >> $GITHUB_ENV | |||
- name: Sanitize branch name | |||
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/" "_")" >> $GITHUB_ENV | |||
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()'' '_')" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix shell script syntax error in branch name sanitization.
The current command has a syntax error with single quotes that could cause the workflow to fail.
Apply this fix:
- run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()'' '_')" >> $GITHUB_ENV
+ run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()\'' '_')" >> $GITHUB_ENV
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()'' '_')" >> $GITHUB_ENV | |
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()\'' '_')" >> $GITHUB_ENV |
🧰 Tools
🪛 actionlint (1.7.4)
120-120: shellcheck reported issue in this script: SC1009:info:1:56: The mentioned syntax error was in this simple command
(shellcheck)
120-120: shellcheck reported issue in this script: SC1073:error:1:68: Couldn't parse this single quoted string. Fix to allow more checks
(shellcheck)
120-120: shellcheck reported issue in this script: SC1072:error:2:1: Expected end of single quoted string. Fix any mentioned problems and try again
(shellcheck)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/functional-tests.yml (1)
120-120
:⚠️ Potential issueFix shell script syntax error in branch name sanitization.
The command has a syntax error with single quotes that could cause the workflow to fail.
Apply this fix:
- run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()'' '_')" >> $GITHUB_ENV + run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()\'' '_')" >> $GITHUB_ENV🧰 Tools
🪛 actionlint (1.7.4)
120-120: shellcheck reported issue in this script: SC1009:info:1:56: The mentioned syntax error was in this simple command
(shellcheck)
120-120: shellcheck reported issue in this script: SC1073:error:1:68: Couldn't parse this single quoted string. Fix to allow more checks
(shellcheck)
120-120: shellcheck reported issue in this script: SC1072:error:2:1: Expected end of single quoted string. Fix any mentioned problems and try again
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/functional-tests.yml
(2 hunks).github/workflows/startup-tests.yml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/startup-tests.yml
105-105: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
245-245: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/functional-tests.yml
120-120: shellcheck reported issue in this script: SC1009:info:1:56: The mentioned syntax error was in this simple command
(shellcheck)
120-120: shellcheck reported issue in this script: SC1073:error:1:68: Couldn't parse this single quoted string. Fix to allow more checks
(shellcheck)
120-120: shellcheck reported issue in this script: SC1072:error:2:1: Expected end of single quoted string. Fix any mentioned problems and try again
(shellcheck)
226-226: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: build (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: test (3.12)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
.github/workflows/functional-tests.yml (2)
Line range hint
165-168
: LGTM! Good practice to update the action version.The update to
actions/setup-python@v5
keeps the workflow current with the latest stable version.🧰 Tools
🪛 actionlint (1.7.4)
118-118: shellcheck reported issue in this script: SC2086:info:1:44: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC1009:info:1:56: The mentioned syntax error was in this simple command
(shellcheck)
120-120: shellcheck reported issue in this script: SC1073:error:1:68: Couldn't parse this single quoted string. Fix to allow more checks
(shellcheck)
120-120: shellcheck reported issue in this script: SC1072:error:2:1: Expected end of single quoted string. Fix any mentioned problems and try again
(shellcheck)
226-226
:⚠️ Potential issueUse single quotes to prevent globbing and word splitting.
The command should use single quotes for the tr pattern to ensure reliable string handling.
Apply this fix:
- run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/()" "_")" >> $GITHUB_ENV + run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()\'' '_')" >> $GITHUB_ENVLikely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
226-226: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/startup-tests.yml
Outdated
@@ -102,7 +102,7 @@ jobs: | |||
- name: Set current date as env variable | |||
run: echo "NOW=$(date +'%Y-%m-%dT%H-%M-%S')" >> $GITHUB_ENV | |||
- name: Sanitize branch name | |||
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/" "_")" >> $GITHUB_ENV | |||
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/()" "_")" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use single quotes to prevent globbing and word splitting.
The command should use single quotes for the tr pattern to ensure reliable string handling.
Apply this fix:
- run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/()" "_")" >> $GITHUB_ENV
+ run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()\'' '_')" >> $GITHUB_ENV
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/()" "_")" >> $GITHUB_ENV | |
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()\'' '_')" >> $GITHUB_ENV |
🧰 Tools
🪛 actionlint (1.7.4)
105-105: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/startup-tests.yml
Outdated
@@ -242,7 +242,7 @@ | |||
- name: Set current date as env variable | |||
run: echo "NOW=$(date +'%Y-%m-%dT%H-%M-%S')" >> $GITHUB_ENV | |||
- name: Sanitize branch name | |||
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/" "_")" >> $GITHUB_ENV | |||
run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/()" "_")" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use single quotes to prevent globbing and word splitting.
The command should use single quotes for the tr pattern to ensure reliable string handling.
Apply this fix:
- run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr "/()" "_")" >> $GITHUB_ENV
+ run: echo "BRANCH_SANITIZED=$(echo ${{ env.BRANCH_NAME }} | tr '/()\'' '_')" >> $GITHUB_ENV
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
245-245: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
should be ok now
Add Mitre D3FEND
Add default value for library_publication_date
ruff format on tools
Summary by CodeRabbit
New Features
Chores
Documentation