feat: improve template loading flow #667
Conversation
WalkthroughThis change refactors Secator's configuration and task discovery system, replacing static, global lists and caches with dynamic, function-based loading via a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Loader
participant Runner
participant Template
CLI->>Loader: get_configs_by_type('task'/etc)
Loader->>Loader: find_templates() / discover_tasks()
Loader-->>CLI: List of configs/tasks
CLI->>CLI Helper: register_runner(config)
CLI Helper->>Runner: Instantiate/configure runner
CLI Helper->>Runner: Execute runner (sync/async)
Runner->>Template: Load templates as needed
Runner-->>CLI Helper: Execution results
CLI Helper-->>CLI: Output/results
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Actionable comments posted: 5
🧹 Nitpick comments (7)
secator/cli_helper.py (3)
131-133: Loop variable unused – rename to_for clarity
kis never referenced inside the loop; keeping it confuses readers and triggers Ruff B007.- for k, v in tasks.items(): + for _, v in tasks.items():🧰 Tools
🪛 Ruff (0.11.9)
131-131: Loop control variable
knot used within loop body(B007)
368-371: Collapse nestedifstatementsLines 368-371 can be flattened, improving readability (Ruff SIM102):
- if CONFIG.celery.broker_url: - if (broker_protocol == 'redis' or backend_protocol == 'redis') and not ADDONS_ENABLED['redis']: + if CONFIG.celery.broker_url and \ + (broker_protocol == 'redis' or backend_protocol == 'redis') and \ + not ADDONS_ENABLED['redis']:🧰 Tools
🪛 Ruff (0.11.9)
368-369: Use a single
ifstatement instead of nestedifstatements(SIM102)
334-347:actual_driversis dead code
actual_driversis populated but never read afterwards. Remove it or use it (e.g., to display which hooks were loaded).- actual_drivers = [] ... - actual_drivers.append(driver)tests/integration/test_workflows.py (1)
85-85: Consider fixing the Yoda condition for better readability.The condition works correctly but could be more readable by reversing the comparison order.
- if TEST_WORKFLOWS != get_configs_by_type('workflow'): + if get_configs_by_type('workflow') != TEST_WORKFLOWS:🧰 Tools
🪛 Ruff (0.11.9)
85-85: Yoda condition detected
Rewrite as
get_configs_by_type('workflow') != TEST_WORKFLOWS(SIM300)
scripts/generate_tools_md_table.py (1)
1-1: Consider lazy loading to improve import performance.Calling
discover_tasks()at module import time could slow down imports if the discovery process is expensive. Consider moving this to a function that's called when needed, or ensure the loader module implements proper caching.-from secator.loader import discover_tasks +from secator.loader import discover_tasks -TASKS = discover_tasks() +def get_tasks(): + return discover_tasks()Then update the usage in
get_tools_data():- for task in TASKS: + for task in get_tasks():Also applies to: 10-10
secator/workflows/__init__.py (1)
1-1: Consider lazy loading to improve import performance.Similar to the previous file, calling
get_configs_by_type('workflow')at module import time could impact performance. Consider implementing lazy loading or ensuring the loader functions are properly cached.-WORKFLOWS = get_configs_by_type('workflow') +def get_workflows(): + return get_configs_by_type('workflow')Then update the iteration:
-for workflow in WORKFLOWS: +for workflow in get_workflows():Also applies to: 5-5
secator/template.py (1)
55-66: Track deprecated methods for future cleanup.Multiple methods and properties are marked with
# TODO: deprecatecomments. Consider creating a tracking issue to ensure these are properly deprecated and removed in a future release to complete the refactoring.Would you like me to help create a deprecation plan or open an issue to track the removal of these deprecated methods?
Also applies to: 67-67, 87-87, 128-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
scripts/generate_tools_md_table.py(3 hunks)secator/cli.py(11 hunks)secator/cli_helper.py(1 hunks)secator/decorators.py(0 hunks)secator/loader.py(1 hunks)secator/runners/_base.py(5 hunks)secator/runners/task.py(2 hunks)secator/scans/__init__.py(2 hunks)secator/tasks/__init__.py(1 hunks)secator/template.py(6 hunks)secator/tree.py(3 hunks)secator/utils.py(0 hunks)secator/utils_test.py(3 hunks)secator/workflows/__init__.py(2 hunks)tests/fixtures/ls.yml(1 hunks)tests/integration/test_tasks.py(2 hunks)tests/integration/test_workflows.py(2 hunks)tests/template/test_templates.py(4 hunks)tests/unit/test_cli.py(1 hunks)tests/unit/test_template.py(4 hunks)
💤 Files with no reviewable changes (2)
- secator/utils.py
- secator/decorators.py
🧰 Additional context used
🧬 Code Graph Analysis (10)
secator/tasks/__init__.py (1)
secator/loader.py (1)
discover_tasks(46-48)
secator/runners/task.py (2)
secator/loader.py (1)
discover_tasks(46-48)secator/runners/_base.py (1)
add_subtask(437-454)
tests/integration/test_workflows.py (2)
secator/loader.py (1)
get_configs_by_type(37-42)secator/utils_test.py (1)
CommandOutputTester(165-274)
tests/integration/test_tasks.py (1)
secator/loader.py (1)
discover_tasks(46-48)
tests/template/test_templates.py (2)
secator/loader.py (2)
get_configs_by_type(37-42)discover_tasks(46-48)secator/decorators.py (1)
task(3-7)
secator/workflows/__init__.py (1)
secator/loader.py (1)
get_configs_by_type(37-42)
secator/tree.py (1)
secator/config.py (1)
get(207-230)
secator/template.py (3)
secator/output_types/error.py (1)
Error(8-43)secator/utils.py (1)
convert_functions_to_strings(728-744)secator/loader.py (1)
find_templates(15-33)
secator/runners/_base.py (4)
secator/loader.py (1)
get_configs_by_type(37-42)secator/output_types/_base.py (1)
toDict(104-108)secator/output_types/warning.py (1)
Warning(8-24)secator/output_types/info.py (1)
Info(8-24)
secator/cli_helper.py (7)
secator/runners/task.py (2)
Task(8-81)get_task_class(69-81)secator/tree.py (1)
build_runner_tree(75-117)secator/utils.py (3)
expand_input(62-107)get_command_category(177-188)deep_merge_dicts(671-703)secator/loader.py (1)
get_configs_by_type(37-42)secator/template.py (1)
_extract_workflows(129-136)secator/click.py (1)
command(28-56)secator/celery.py (1)
is_celery_worker_alive(282-290)
🪛 Ruff (0.11.9)
tests/integration/test_workflows.py
85-85: Yoda condition detected
Rewrite as get_configs_by_type('workflow') != TEST_WORKFLOWS
(SIM300)
secator/cli_helper.py
131-131: Loop control variable k not used within loop body
(B007)
368-369: Use a single if statement instead of nested if statements
(SIM102)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (38)
tests/fixtures/ls.yml (1)
8-9: File terminates correctly – no further action requiredOnly a trailing newline was added, which is a conventional POSIX friendly practice and prevents certain tools from warning about “no-newline-at-EOF”.
secator/tasks/__init__.py (1)
1-1: LGTM! Import refactoring looks correct.The import change from
secator.utilstosecator.loaderaligns with the centralization of task discovery logic. Thediscover_tasks()function maintains the same interface and behavior.secator/runners/task.py (2)
4-4: LGTM! Import refactoring looks correct.The import change from
secator.utilstosecator.loaderaligns with the centralization of task discovery logic.
65-65:✅ Verification successful
Verify that
self.descriptionis always initialized.The change from
self.config.description or ''toself.descriptionsuggests that the runner base class now properly initializes this attribute. Please ensure thatself.descriptionis always available and not None to prevent potential AttributeError.
🏁 Script executed:
#!/bin/bash # Description: Verify that self.description is properly initialized in the runner base class # Expected: Find initialization of self.description in the base runner class # Search for description initialization in runner base classes ast-grep --pattern 'class $CLASS($PARENT): $$$ def __init__($SELF, $$$): $$$ $SELF.description = $$$ $$$ $$$' # Also search for description property definitions rg -A 5 "def description" --type pyLength of output: 167
🏁 Script executed:
#!/bin/bash # Gather all references to 'description' in runner implementations echo "=== All 'description' occurrences in secator/runners ===" rg -n "description" -g 'secator/runners/*.py' || true # List runner class files echo echo "=== Runner class files ===" rg -l "class .*Runner" -g 'secator/runners/*.py' || true # Show __init__ signatures and nearby lines for each runner class echo echo "=== __init__ contexts in runner classes ===" for file in $(rg -l "class .*Runner" -g 'secator/runners/*.py'); do echo echo "---- $file ----" rg -n "def __init__" -B2 -A8 "$file" || true doneLength of output: 4087
🏁 Script executed:
#!/bin/bash # Show __init__ of CommandRunner for context echo "---- secator/runners/command.py:100-200 ----" sed -n '100,200p' secator/runners/command.py || true # Find super().__init__ calls in CommandRunner echo echo "---- super calls in command.py ----" rg -n "super" -A5 -B2 secator/runners/command.py || trueLength of output: 2822
No Action Required:
self.descriptionis always initialized
I verified that insecator/runners/_base.pythe base class’s__init__always setsself.description = run_opts.get('description', config.description or '')which guarantees a string (never
None). All runner subclasses (Task, Workflow, Command, etc.) either inherit this or pass theirrun_optsthrough tosuper().__init__, soself.descriptioncannot be uninitialized.tests/integration/test_workflows.py (1)
13-13: LGTM! Dynamic loading refactoring looks correct.The import change from static
ALL_WORKFLOWSto dynamicget_configs_by_typealigns with the overall refactoring to use dynamic configuration loading.secator/scans/__init__.py (3)
1-1: LGTM! Import refactoring looks correct.The import change from
secator.clitosecator.loaderaligns with the centralization of configuration loading logic.
5-5: LGTM! Dynamic loading implementation looks correct.The replacement of static
ALL_SCANSwith dynamicget_configs_by_type('scan')is consistent with the overall refactoring pattern and maintains the same functionality.
27-27: LGTM! Loop update looks correct.The change from iterating over
ALL_SCANStoSCANScorrectly uses the new dynamically loaded scan configurations.scripts/generate_tools_md_table.py (1)
38-38: Good defensive programming practice.Adding the null-safety check
task.__doc__ or ''prevents potential issues when a task's docstring isNone. This improves the robustness of the generated documentation.tests/integration/test_tasks.py (1)
77-80:❓ Verification inconclusive
Potential attribute mismatch in filtering logic.
There appears to be an inconsistency in the filtering logic. Line 78 extracts names using
t.namefromTEST_TASKS, but line 79 compares againstt.__name__from the discovered tasks. This could cause the filtering to fail if the attribute names don't match.Please verify that
TEST_TASKSobjects have a.nameattribute and that it corresponds to the.__name__attribute of the discovered task classes. If not, the filtering logic needs to be corrected.
🏁 Script executed:
#!/bin/bash # Description: Verify the attributes used in the filtering logic # Expected: TEST_TASKS should have .name attribute and discover_tasks() should return classes with .__name__ # Check the TEST_TASKS definition and usage rg -A 10 -B 5 "TEST_TASKS.*=" # Check how TEST_TASKS is populated and what attributes the objects have ast-grep --pattern 'TEST_TASKS = $_'Length of output: 5027
I need to inspect how
TASKSand its items are defined to confirm thatconfig.namematches the class__name__. Let's view the top ofsecator/utils_test.py, whereTASKSis imported and used.
🏁 Script executed:
#!/bin/bash # Display imports and TASKS usage in secator/utils_test.py sed -n '1,80p' secator/utils_test.pyLength of output: 2268
Let’s verify how configs and task discovery align:
🏁 Script executed:
#!/bin/bash # 1. Show get_configs_by_type definition to confirm config.name semantics rg -n "def get_configs_by_type" -C10 secator/loader.py # 2. Show discover_tasks implementation in the integration test rg -n "def discover_tasks" -C10 tests/integration/test_tasks.pyLength of output: 923
I wasn’t able to locate the
TemplateLoaderdefinition to confirm whether it exposes a.nameattribute (vs only['name']). That’s essential to validate the filter logic.Please verify manually that
TemplateLoaderinstances support attribute access (config.name) rather than only mapping access (config['name']). If they only support__getitem__, then in tests/integration/test_tasks.py you must replace:- test_tasks_names = [t.name for t in TEST_TASKS] + test_tasks_names = [t['name'] for t in TEST_TASKS]and ensure consistency between how
nameis accessed on configs and on classes.tests/template/test_templates.py (1)
3-3: LGTM! Clean refactoring to dynamic loading.The changes consistently replace static imports with dynamic discovery functions. The test logic remains intact while adopting the new loading mechanism, which aligns well with the overall refactoring objectives.
Also applies to: 16-16, 26-26, 36-36
tests/unit/test_cli.py (1)
212-214: LGTM! Test correctly updated for dynamic loading.The test has been properly updated to mock the new
get_configs_by_typefunction instead of the staticALL_TASKSlist, aligning with the architectural shift to dynamic configuration loading.secator/tree.py (4)
3-3: LGTM! DotMap import aligns with dynamic configuration approach.The addition of DotMap import supports the shift from static template loading to dynamic configuration objects.
75-80: LGTM! Function signature correctly updated for dynamic configuration.The parameter type change from
TemplateLoadertoDotMapaligns with the architectural shift to dynamic configuration loading, and the docstring has been appropriately updated.
101-101: LGTM! Good defensive programming practice.The added null check prevents potential errors when
task_detailsisNone, improving the robustness of the function.
112-112:❌ Incorrect review comment
Inconsistent usage of TemplateLoader after refactor.
This line still uses
TemplateLoaderwhich conflicts with the function's new parameter typeDotMap. This could cause runtime errors since the function signature has been changed to expectDotMapobjects.Consider updating this line to use the dynamic configuration approach:
-wf_config = TemplateLoader(name=f'workflow/{workflow_name}') +# Use dynamic config loading instead of TemplateLoader +wf_configs = get_configs_by_type('workflow') +wf_config = next((c for c in wf_configs if c.name == workflow_name), None) +if not wf_config: + continueLet me verify how TemplateLoader is being used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash # Search for TemplateLoader usage to understand the refactoring scope rg -A 3 "TemplateLoader\(" --type pyLength of output: 4639
No change required:
TemplateLoaderis compatible withDotMapThe
TemplateLoaderclass insecator/template.pyinherits fromDotMap, so passing aTemplateLoaderinstance intobuild_runner_tree(which expects aDotMap) is perfectly valid. All existing usage sites and unit/integration tests rely on this behavior, and there is noget_configs_by_typehelper implemented in the codebase. You can safely leave the line as-is:wf_config = TemplateLoader(name=f'workflow/{workflow_name}')Likely an incorrect or invalid review comment.
secator/utils_test.py (6)
15-17: LGTM! Import correctly updated for dynamic loading.The import changes from static lists to the dynamic
get_configs_by_typefunction align with the architectural refactoring.
28-31: LGTM! Global variables correctly updated for dynamic configuration loading.The replacement of static imported lists (
ALL_TASKS,ALL_WORKFLOWS,ALL_SCANS) with dynamic calls toget_configs_by_type()is consistent with the PR objectives.
38-41: LGTM! Filtering logic correctly adapted for configuration objects.The filtering logic has been properly updated to use
config.nameinstead of class names, which is appropriate for the new configuration objects returned byget_configs_by_type().
47-50: LGTM! Workflow filtering consistently updated.The filtering logic for workflows has been correctly adapted to work with configuration objects.
56-59: LGTM! Scan filtering consistently updated.The filtering logic for scans has been correctly adapted to work with configuration objects.
79-81: LGTM! Fixtures dictionary correctly adapted for dynamic loading.The fixture dictionary construction has been properly updated to use
Task.get_task_class(task.name)to work with the new configuration objects instead of direct class references.tests/unit/test_template.py (4)
10-10: LGTM! Import correctly updated for dynamic loading functions.The import changes to use
get_configs_by_type,find_templates, anddiscover_tasksalign with the architectural shift to dynamic configuration loading.
49-53: LGTM! Test correctly updated for dynamic workflow loading.The cache clearing ensures fresh data and the replacement of
ALL_WORKFLOWSwithget_configs_by_type('workflow')is consistent with the refactoring approach.
119-128: LGTM! Template handling correctly updated for dynamic loading.The test has been properly updated to use
find_templates()dynamically instead of relying on a globalTEMPLATESlist, which aligns with the architectural changes.
137-139: LGTM! Consistent template handling throughout the test.The template handling continues to use the dynamic approach consistently throughout the test method.
secator/runners/_base.py (6)
22-22: LGTM: Import aligns with dynamic loading refactoring.The import of
get_configs_by_typefromsecator.loaderis consistent with the broader refactoring to replace static global lists with dynamic loading mechanisms.
93-93: LGTM: Improved description handling with proper fallback.The fallback to empty string prevents potential issues with None or undefined descriptions and improves robustness.
305-305: LGTM: Consistent use of processed description.Using
self.descriptioninstead ofconfig.descriptionensures consistency with the fallback handling established in the constructor.
813-814: LGTM: Improved conditional description formatting.The conditional check prevents empty parentheses in log output when no description is available, enhancing user experience.
1095-1095: LGTM: Dynamic profile loading with improved formatting.The replacement of static profile lists with
get_configs_by_type('profile')aligns with the dynamic loading refactoring. The conditional description formatting prevents empty parentheses in output, consistent with similar improvements elsewhere in the file.Also applies to: 1103-1106
91-91:✅ Verification successful
Verify config objects have toDict() method.
The conversion to
DotMap(config.toDict())assumes all config objects passed to the Runner constructor have atoDict()method. Please ensure this is consistently available across all configuration types.
🏁 Script executed:
#!/bin/bash # Description: Verify that all config objects used with Runner have toDict() method # Expected: Find TemplateLoader class definition and other config classes that should have toDict() # Search for classes that might be passed as config to Runner ast-grep --pattern 'class $_ { $$$ def toDict($$$) { $$$ } $$$ }' # Also search for TemplateLoader class specifically since it's mentioned in relevant code snippets rg -A 10 "class TemplateLoader"Length of output: 832
🏁 Script executed:
#!/bin/bash set -e echo "=== 1. Runner class definition ===" rg -n "class Runner" -A 20 secator/runners/_base.py || true echo echo "=== 2. toDict method definitions in repo ===" rg -n "def toDict" . || true echo echo "=== 3. to_dict method definitions in repo ===" rg -n "def to_dict" . || true echo echo "=== 4. Config classes in repo ===" rg -n "^class .*Config" . || true echo echo "=== 5. Runner usage throughout code ===" rg -n "Runner\(" . || true echo echo "=== 6. Verify DotMap has toDict ===" pip install dotmap >/dev/null 2>&1 || true python3 - <<'EOF' try: from dotmap import DotMap methods = dir(DotMap) print("DotMap.toDict available:", "toDict" in methods) print("DotMap.to_dict available:", "to_dict" in methods) except ImportError as e: print("Error importing DotMap:", e) EOFLength of output: 1962
Verified:
.toDict()is safe.
Confirmed thatTemplateLoader(and theConfigclass) inherit fromDotMap, which provides atoDict()method. No changes required.secator/template.py (2)
1-1: LGTM: Import cleanup aligns with refactoring.The simplified imports remove unused dependencies and clean up the module structure as part of the transition to dynamic loading.
Also applies to: 5-5, 8-9
101-102: LGTM: Good defensive programming with fallback.Using
TemplateLoader()as a fallback for empty values prevents potential issues with None or empty configuration values during parsing.secator/cli.py (4)
21-21: LGTM: Import changes align with refactoring architecture.The import changes properly reflect the new architecture:
- CLI registration moved from
secator.decoratorstosecator.cli_helper- Dynamic loading functions imported from
secator.loader- Unused imports cleaned up
Also applies to: 29-29, 31-31
81-82: LGTM: Consistent registration using dynamic configurations.The registration loops properly use the dynamically loaded configurations, maintaining the same functionality while leveraging the new loading architecture.
Also applies to: 97-98, 113-114
589-589: LGTM: Consistent usage of dynamic loading throughout CLI commands.All CLI commands have been consistently updated to use dynamic loading:
- Profile operations use the dynamically loaded
PROFILES- Alias generation uses dynamic configurations for tasks, workflows, and scans
- Health checks and tool installation use
discover_tasks()The changes maintain functionality while leveraging the new dynamic loading architecture.
Also applies to: 656-656, 660-660, 664-664, 668-668, 986-986, 1322-1322, 1332-1332, 1402-1402, 1586-1586
39-42:✅ Verification successful
Dynamic loading at module import time - consider performance impact.
The replacement of static lists with dynamic loading calls is consistent with the refactoring goals. However, since these are called at module import time, ensure that
get_configs_by_type()is efficiently implemented to avoid CLI startup delays.
🏁 Script executed:
#!/bin/bash # Description: Check the performance characteristics of get_configs_by_type function # Expected: Review the implementation to assess if it's suitable for module import time # Find the get_configs_by_type function implementation ast-grep --pattern 'def get_configs_by_type($$$) { $$$ }' # Check if there are any caching mechanisms rg -A 10 -B 5 "cache\|Cache"Length of output: 92
🏁 Script executed:
#!/bin/bash # Locate all occurrences of get_configs_by_type (uses and definition) rg -n "get_configs_by_type" . # Find the function definition specifically rg -n "def get_configs_by_type" . # Check for common caching decorators (e.g., functools.lru_cache) rg -n "lru_cache" .Length of output: 2606
🏁 Script exec 8000 uted:
#!/bin/bash # Display imports and decorator usage in secator/loader.py rg -n "from .*functools" secator/loader.py rg -n "@\(cache\|lru_cache\)" secator/loader.py rg -n "cache_clear" secator/loader.py # Print get_configs_by_type implementation sed -n '1,20p' secator/loader.py sed -n '37,100p' secator/loader.pyLength of output: 2926
Dynamic loading is efficiently cached
The underlying discover_tasks and find_templates functions are decorated with functools.cache, so the expensive discovery and template‐loading work only happens once. While get_configs_by_type itself isn’t cached, its per-call filtering and sorting overhead is trivial for the expected number of configs. Approved.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
secator/template.py (1)
23-24: Address potential circular import issue.The dynamic import of
find_templatesfromsecator.loaderinside the__init__method could cause circular import issues sincesecator.loaderimportsTemplateLoaderfrom this module.Consider moving this import to the top of the file or refactoring to eliminate the circular dependency:
+from secator.loader import find_templates + class TemplateLoader(DotMap): def __init__(self, input=None, name=None, **kwargs): if name: split = name.split('/') if len(split) != 2: console.print(Error(message=f'Cannot load {name}: you should specify a type for the template when loading by name (e.g. workflow/<workflow_name>)')) # noqa: E501 return _type, _name = tuple(split) if _type.endswith('s'): _type = _type[:-1] - from secator.loader import find_templates config = next((p for p in find_templates() if p['type'] == _type and p['name'] == _name), None)
🧹 Nitpick comments (1)
secator/cli_helper.py (1)
18-31: Multi-character “short” flags are unconventional and clutter help outputSeveral entries in
CLI_OPTSuse values like'json','stat','fmt', etc. as theshortvariant.
These are rendered as-json,-stat, … which (a) diverge from common CLI expectations (single-letter short flags), (b) make auto-completion verbose, and (c) interact badly with the reverse-flag logic described above.Consider:
- Using actual single letters (e.g.,
-jfor--print-json) and exposing the mnemonic throughmetavarorhelp.- Or remove the
shortkey altogether and rely solely on long options for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
scripts/generate_tools_md_table.py(3 hunks)secator/cli.py(12 hunks)secator/cli_helper.py(1 hunks)secator/loader.py(1 hunks)secator/scans/__init__.py(2 hunks)secator/template.py(5 hunks)secator/workflows/__init__.py(2 hunks)tests/integration/test_workflows.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- secator/workflows/init.py
- tests/integration/test_workflows.py
- secator/scans/init.py
- scripts/generate_tools_md_table.py
- secator/loader.py
- secator/cli.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
secator/template.py (3)
secator/output_types/error.py (1)
Error(8-43)secator/utils.py (1)
convert_functions_to_strings(728-744)secator/loader.py (1)
find_templates(15-33)
🪛 Ruff (0.11.9)
secator/template.py
14-14: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (5)
secator/template.py (3)
49-56: LGTM! Clean output formatting improvement.The logic to temporarily remove the
_pathkey before printing provides cleaner YAML output while preserving the original configuration. Good implementation.
58-70: Good deprecation strategy for backward compatibility.Marking these methods with "# TODO: deprecate" comments provides a clear migration path while maintaining backward compatibility during the transition to the new loader system.
104-105: Defensive programming improvement.The change
value = value or TemplateLoader()is a good defensive programming practice that ensures falsy values are replaced with a valid TemplateLoader instance, preventing potential AttributeError exceptions downstream.secator/cli_helper.py (2)
283-289: Nice work fixing the invalid Click argument nameReplacing the dynamic name with a constant
'inputs'and keeping the original pattern inmetavarfully resolves the previousKeyError/startup crash.
300-310: Guarding version display withcli_endpoint.name == 'task'avoids the priorUnboundLocalErrorThis conditional removes the crash path highlighted in the previous review. ✅
🤖 I have created a release *beep* *boop* --- ## [0.16.0](v0.15.1...v0.16.0) (2025-06-05) ### Features * **`dnsx`:** merge `dnsxbrute` into `dnsx` ([#571](#571)) ([d30a497](d30a497)) * add task revoke state and perf improvements ([#678](#678)) ([2a3bf08](2a3bf08)) * allow returning errors in hooks ([#632](#632)) ([39a56bd](39a56bd)) * improve bbot output types ([#627](#627)) ([3b0aa5d](3b0aa5d)) * improve runner logic, workflow building, results filtering logic; and add config defaults for profiles & drivers ([#673](#673)) ([df94657](df94657)) * improve template loading flow ([#667](#667)) ([f223120](f223120)) * memory optimizations ([#681](#681)) ([d633133](d633133)) * **misc:** condition-based runs, chunked_by opts, dynamic task profiles, cli improvements ([#659](#659)) ([e8225cd](e8225cd)) * **runner:** add input validation to all tasks and workflows ([#663](#663)) ([8392551](8392551)) * **runner:** improve option handling ([#670](#670)) ([59b1c68](59b1c68)) * **scans:** improve scans ([#660](#660)) ([bdd38ec](bdd38ec)) * use os system for CLI and better labs ([#649](#649)) ([8b49912](8b49912)) * **workflow:** improve subdomain_recon workflow ([#657](#657)) ([bc65092](bc65092)) ### Bug Fixes * allow dry-run mode to work without targets ([#624](#624)) ([cccffb9](cccffb9)) * check task is registered before running test ([1f5cd83](1f5cd83)) * formatting for dynamic opts ([#628](#628)) ([dcbbfe9](dcbbfe9)) * header options conversion ([#633](#633)) ([6ae8423](6ae8423)) * header parsing ([#629](#629)) ([db2f028](db2f028)) * improve mongodb duplicate checker ([#626](#626)) ([bf277a9](bf277a9)) * **installer:** compound distro.like() on distribs like popos ([#653](#653)) ([3687e1d](3687e1d)) * **installer:** ignore dev/post release from PyPI ([#634](#634)) ([614c3e2](614c3e2)) * **installer:** secator update with correct package version ([#648](#648)) ([a9cf189](a9cf189)) * lab --wait not in gitlab runner ([070ae84](070ae84)) * logic to test all tasks ([3bd7503](3bd7503)) * os.system return code ([02aed75](02aed75)) * progress type fields ([#652](#652)) ([f146914](f146914)) * remove duplicates from txt exporter ([#630](#630)) ([88ba5c5](88ba5c5)) * remove fping -r flag by default, show alive hosts better ([#665](#665)) ([5c945fd](5c945fd)) * remove no-recreate flag in labs as not supported by github runner ([bd997a8](bd997a8)) * short opt incorrectly named ([#631](#631)) ([0c73c60](0c73c60)) * tasks with no file flag need input_chunk_size=1 ([#668](#668)) ([a088c94](a088c94)) * tools in readme, arjun chunk and ffuf header ([#679](#679)) ([654ff30](654ff30)) * tools table generator update ([9420f14](9420f14)) * update ci workflow ([f4c2b13](f4c2b13)) * update generate table workflow ([ff62702](ff62702)) * vulnerability output reference when unset ([#625](#625)) ([a656fbf](a656fbf)) ### Documentation * generate tools table md ([#610](#610)) ([d60f11e](d60f11e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary by CodeRabbit