feat(misc): condition-based runs, chunked_by opts, dynamic task profiles, cli improvements #659
Conversation
|
@coderabbitai please review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 69871 @ocervell I've reviewed the PR and here's my assessment: This is a significant enhancement that adds several valuable features to Secator:
The PR improves both the developer and user experience while maintaining backward compatibility. The condition-based execution feature is especially valuable as it allows for more sophisticated and targeted security scanning workflows. Overall, the changes are well-designed and represent a significant improvement to the tool's flexibility and usability. |
WalkthroughThis update introduces a broad refactor and feature expansion across the codebase. Key changes include enhanced CLI aliasing and grouping, dynamic runner tree visualization, improved debug logging and option handling, conditional task/workflow execution, more robust extraction and reporting logic, and updated configuration schemas. Several modules are introduced or restructured to support these features. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant OrderedGroup
participant Runner
participant Tree
participant Report
User->>CLI: Invokes command (with aliases)
CLI->>OrderedGroup: Resolves command/group/alias
CLI->>Runner: Initializes runner with options
Runner->>Tree: Optionally builds runner tree (if tree flag)
Tree-->>Runner: Returns tree structure
Runner->>Runner: Resolves options, profiles, hooks, validators
Runner->>Runner: Executes tasks/workflows (conditional on 'if' expressions)
Runner->>Report: Builds report with extractors
Report-->>Runner: Returns filtered/deduped results
Runner->>CLI: Outputs results (with enhanced formatting)
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: 14
🔭 Outside diff range comments (2)
secator/celery.py (1)
297-327: 🛠️ Refactor suggestion
optsobject reused across chunks – each Celery signature should receive its own copy
optsis mutated inside the loop (chunk,chunk_count,results).
Since the same dict is passed to every signature, later iterations overwrite the values captured by earlier ones, leading to inconsistent behaviour when workers unpickle the tasks.- # Clone opts - opts = task_opts.copy() + # Base template for every chunk + base_opts = task_opts.copy() ... - if len(chunks) > 0: - opts['chunk'] = ix + 1 - opts['chunk_count'] = len(chunks) + # Build a fresh copy for this specific chunk + opts = base_opts.copy() + opts.update({'chunk': ix + 1, 'chunk_count': len(chunks)}) ... - opts['results'] = chunked_results - sig = type(task).si(chunk, **opts).set(task_id=task_id) + opts['results'] = chunked_results + sig = type(task).si(chunk, **opts).set(task_id=task_id)secator/decorators.py (1)
291-325:⚠️ Potential issueDuplicate “show version” block and potential
NameErrorThe version-handling logic appears twice (l. 302-313 & 314-324).
Additionally, it referencestask_cls, which is undefined forscanandworkflowcommands, raisingNameErrorif--versionis used there.Please de-duplicate and scope by runner type:
- version = opts['version'] + version_requested = opts['version'] ... - if version: + if version_requested and runner_cls is Task: ...This removes the redundancy and prevents runtime crashes.
♻️ Duplicate comments (1)
secator/runners/_helpers.py (1)
65-74: Replicate the “mutable-default” fix for the other helpersBoth
extract_from_results(l. 65) andprocess_extractor(l. 115) repeat thectx={}anti-pattern.
For consistency and safety, patch them the same way as suggested above.Also applies to: 115-124
🧰 Tools
🪛 Ruff (0.11.9)
65-65: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
🧹 Nitpick comments (18)
tests/integration/test_workflows.py (1)
58-61: Consider updating remaining numeric DEBUG comparisonsWhile the logging level check was updated to use the new string-based approach
DEBUG == ["1"], these lines still use numeric comparisons (DEBUG > 1,DEBUG > 2). For consistency, consider updating these to match the new string-based debug configuration pattern.- 'print_item': DEBUG > 1, - 'print_line': DEBUG > 2, - 'table': DEBUG > 1, - 'output': 'table' if DEBUG > 1 else '' + 'print_item': "2" in DEBUG or "3" in DEBUG, + 'print_line': "3" in DEBUG, + 'table': "2" in DEBUG or "3" in DEBUG, + 'output': 'table' if "2" in DEBUG or "3" in DEBUG else ''secator/tasks/httpx.py (1)
80-90: Implemented dynamic profile selection based on task optionsThe static profile assignment has been replaced with a dynamic approach that selects the appropriate processing profile based on the task's options. This implementation:
- Uses a lambda function to call the new
dynamic_profilestatic method- Selects 'cpu' profile when screenshot functionality is enabled, otherwise defaults to 'io'
- Demonstrates the "dynamic task profiles" feature mentioned in the PR objectives
This is an excellent enhancement that allows for more efficient resource allocation based on the specific requirements of each task execution.
However, consider using a regular method instead of a lambda function for better readability:
-profile = lambda opts: httpx.dynamic_profile(opts) # noqa: E731 +profile = httpx.dynamic_profileThis would be slightly cleaner and avoid the need for the E731 linter exclusion.
secator/utils.py (1)
430-431: Optimize boolean logic with inequality operator.The code uses
not DEBUG == ['all']andnot DEBUG == ['1']which can be simplified.- if not DEBUG == ['all'] and not DEBUG == ['1']: + if DEBUG != ['all'] and DEBUG != ['1']:🧰 Tools
🪛 Ruff (0.11.9)
430-430: Use
DEBUG != ['all']instead ofnot DEBUG == ['all']Replace with
!=operator(SIM201)
430-430: Use
DEBUG != ['1']instead ofnot DEBUG == ['1']Replace with
!=operator(SIM201)
secator/tasks/ffuf.py (1)
99-102: Improved Host header fuzzing condition.The condition has been refined to specifically check for HTTP URLs rather than just checking if inputs exist, which is more precise.
Consider simplifying the nested if statements for better readability:
- if self.get_opt_value('fuzz_host_header'): - if 'http://' in self.inputs[0]: + if self.get_opt_value('fuzz_host_header') and 'http://' in self.inputs[0]:🧰 Tools
🪛 Ruff (0.11.9)
99-100: Use a single
ifstatement instead of nestedifstatements(SIM102)
secator/runners/workflow.py (1)
142-144: Provide a deterministic fallback queue whenprofilereturnsNoneIf
task.profile(opts)legitimately returnsNone, the resultingSignature.set(queue=None)lets Celery pick the default queue implicitly. That works, but makes routing harder to reason about and can hide mis-configurations.-profile = task.profile(opts) if callable(task.profile) else task.profile -sig = task.s(inputs, **opts).set(queue=profile, task_id=task_id) +queue = task.profile(opts) if callable(task.profile) else task.profile +if not queue: + queue = CONFIG.celery.default_queue # or a hard-coded "default" +sig = task.s(inputs, **opts).set(queue=queue, task_id=task_id)This turns a silent fallback into an explicit, auditable decision.
secator/template.py (1)
130-137: Memoise workflow extraction for speed & recursion safety
_extract_workflows()instantiatesTemplateLoaderon every call (invoked by bothsupported_optsandflat_tasks). In large repos this is noticeable, and repeated parsing risks cyclic loads.Consider caching the result per instance:
- def _extract_workflows(self): + from functools import lru_cache + + @lru_cache(maxsize=1) + def _extract_workflows(self):A one-line decorator yields O(1) subsequent look-ups without altering call-sites.
secator/report.py (1)
95-117: Duplicates may re-appear after extractor union/intersection – runremove_duplicatesonce more
remove_duplicates()is executed before the extractor pipeline, but the"or"union path (l. 109-111) can introduce the same object several times.
A lightweight second pass keeps the final payload clean and predictable:- items = all_res + items = remove_duplicates(all_res) if dedupe else all_ressecator/celery.py (1)
311-319:list.removein filtering loop may raiseValueErrorIf the same object occurs multiple times in
results, callingremovefor each occurrence oftempwill fail after the first removal.
Safer (and faster) to build a new list:- chunked_results = results.copy() - ... - for item in temp: - item_attr = getattr(item, attr) - if item_attr not in chunk: - chunked_results.remove(item) + chunked_results = [ + r for r in results + if not (r._type == _type and getattr(r, attr) not in chunk) + ]secator/click.py (1)
12-15: HandleNone/ empty values & strip whitespace inListParamType.convert
clickcan passNone(e.g. option omitted) which will raise.
Also trailing spaces remain in elements.- if isinstance(value, list): - return value - return value.split(',') + if value is None: + return [] + if isinstance(value, list): + return value + return [v.strip() for v in value.split(',') if v.strip()]secator/tasks/katana.py (2)
101-108: Path construction is not OS-agnosticHard-coding the
"/.outputs"separator makes the command brittle on Windows paths.
Preferos.path.join(or evenPath) to assemble the path.- self.cmd += f' -srd {self.reports_folder}/.outputs' + output_dir = os.path.join(self.reports_folder, ".outputs") + self.cmd += f" -srd {output_dir}"
139-150: Minor optimisation — avoid rewriting unchanged files
os.path.existsis checked, but we always open the file and rewrite its
contents even when the first line is already at the bottom.
Consider comparing before rewriting to save a disk write on large runs.secator/configs/workflows/host_recon.yaml (1)
23-29: Quote boolean expressions to avoid YAML quirksSome YAML parsers coerce unquoted scalars such as
not opts.full
into unexpected types. Wrapping the expression in quotes makes the
intent crystal-clear.- if: not opts.full + if: "not opts.full"Do the same for the
naabu/fullcondition.secator/tree.py (2)
118-121: Variable shadowing hampers readabilityRe-using the name
configfor the newly-loaded workflow overrides the
function argument, making the code harder to follow.- config = TemplateLoader(name=f'workflow/{workflow_name}') - workflow_tree = build_runner_tree(config, condition) + wf_config = TemplateLoader(name=f'workflow/{workflow_name}') + workflow_tree = build_runner_tree(wf_config, condition)
62-64: Tiny micro-perf: cache render function
child_str = self.render_opts.get(child.type, lambda x: str(x))
is looked up for every child. Caching outside the loop or using
functools.lru_cachecould shave a few µs on very large trees, though
this is definitely optional.secator/decorators.py (1)
60-87:decorate_command_options– short option generation can violate Click rulesWhen no explicit
shortkey is given the code falls back to-<opt_name>even if the name is multi-character (e.g.--profiles⇒-profiles). Click only allows a single character short flag and will raise.Consider guarding it:
if short_opt: short = f'-{short_opt}' elif len(opt_name) == 1: short = f'-{opt_name}' else: short = None ... params = [long] if short is None else [long, short] f = click.option(*params, **conf)(f)secator/runners/_base.py (1)
775-778: Prefer direct comparison over double negation
not self.config.type == 'task'is harder to read and was flagged by Ruff.- if not self.has_parent and not self.config.type == 'task': + if not self.has_parent and self.config.type != 'task':Pure readability win, no behavioural change.
🧰 Tools
🪛 Ruff (0.11.9)
776-776: Use
self.config.type != 'task'instead ofnot self.config.type == 'task'Replace with
!=operator(SIM201)
secator/cli.py (2)
765-768: Unused loop indexix– simplify the loop
ixisn’t used; dropenumerateto silence B007 and reduce noise:- for ix, extractor in enumerate(extractors): + for extractor in extractors:[nitpick but worth tightening]
🧰 Tools
🪛 Ruff (0.11.9)
767-767: Loop control variable
ixnot used within loop body(B007)
1498-1526: Three almost-identical test sub-commands – refactor to a helper
test integration,test template, andtest performanceduplicateenv-var setup ➔ shutil.rmtree ➔ coverage + pytest command building
Extract the common body into a private helper (e.g.
_run_pytest(kind, folder, tasks, workflows, scans, test)), then each command becomes a 3-liner. This
cuts >100 lines and future-proofs new kinds of tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
secator/celery.py(3 hunks)secator/celery_utils.py(2 hunks)secator/cli.py(17 hunks)secator/click.py(1 hunks)secator/config.py(2 hunks)secator/configs/profiles/default.yaml(1 hunks)secator/configs/scans/domain.yaml(1 hunks)secator/configs/scans/host.yaml(1 hunks)secator/configs/scans/network.yaml(1 hunks)secator/configs/scans/subdomain.yaml(1 hunks)secator/configs/scans/url.yaml(1 hunks)secator/configs/workflows/cidr_recon.yaml(0 hunks)secator/configs/workflows/code_scan.yaml(1 hunks)secator/configs/workflows/host_recon.yaml(1 hunks)secator/configs/workflows/subdomain_recon.yaml(1 hunks)secator/configs/workflows/url_crawl.yaml(1 hunks)secator/configs/workflows/url_dirsearch.yaml(0 hunks)secator/configs/workflows/url_fuzz.yaml(0 hunks)secator/configs/workflows/url_nuclei.yaml(1 hunks)secator/configs/workflows/user_hunt.yaml(1 hunks)secator/configs/workflows/wordpress.yaml(1 hunks)secator/decorators.py(12 hunks)secator/definitions.py(1 hunks)secator/exporters/console.py(1 hunks)secator/output_types/_base.py(1 hunks)secator/output_types/info.py(2 hunks)secator/output_types/tag.py(2 hunks)secator/output_types/url.py(3 hunks)secator/report.py(3 hunks)secator/rich.py(3 hunks)secator/runners/_base.py(34 hunks)secator/runners/_helpers.py(3 hunks)secator/runners/command.py(17 hunks)secator/runners/scan.py(3 hunks)secator/runners/task.py(1 hunks)secator/runners/workflow.py(6 hunks)secator/tasks/dnsx.py(1 hunks)secator/tasks/ffuf.py(3 hunks)secator/tasks/httpx.py(1 hunks)secator/tasks/katana.py(2 hunks)secator/template.py(2 hunks)secator/tree.py(1 hunks)secator/utils.py(3 hunks)tests/integration/test_scans.py(1 hunks)tests/integration/test_worker.py(2 hunks)tests/integration/test_workflows.py(1 hunks)tests/unit/test_command.py(1 hunks)tests/unit/test_runners.py(1 hunks)tests/unit/test_tasks.py(2 hunks)
💤 Files with no reviewable changes (3)
- secator/configs/workflows/cidr_recon.yaml
- secator/configs/workflows/url_dirsearch.yaml
- secator/configs/workflows/url_fuzz.yaml
🧰 Additional context used
🧬 Code Graph Analysis (7)
tests/integration/test_scans.py (2)
secator/utils.py (1)
setup_logging(44-60)secator/utils_test.py (1)
CommandOutputTester(161-270)
secator/tasks/dnsx.py (1)
secator/utils.py (1)
extract_domain_info(512-530)
secator/template.py (2)
secator/output_types/_base.py (1)
toDict(104-108)secator/utils.py (1)
convert_functions_to_strings(818-834)
secator/celery.py (3)
secator/utils.py (1)
debug(428-465)secator/decorators.py (1)
task(42-46)secator/runners/command.py (1)
si(237-241)
secator/report.py (3)
secator/utils.py (2)
get_file_timestamp(353-355)traceback_as_string(647-656)
secator/runners/_base.py (1)
errors(237-238)
secator/runners/_helpers.py (1)
extract_from_results(65-87)
secator/decorators.py (8)
secator/runners/task.py (1)
Task(8-80)secator/runners/workflow.py (1)
Workflow(12-149)secator/tree.py (1)
build_runner_tree(73-125)secator/utils.py (3)
deduplicate(128-146)expand_input(63-108)get_command_category(263-274)secator/click.py (2)
decorator(29-53)decorator(59-81)secator/runners/_base.py (1)
toDict(604-631)secator/template.py (2)
flat_tasks(57-59)_extract_workflows(130-137)secator/config.py (1)
get(203-226)
secator/tree.py (1)
secator/template.py (1)
TemplateLoader(17-137)
🪛 Ruff (0.11.9)
secator/utils.py
430-430: Use DEBUG != ['all'] instead of not DEBUG == ['all']
Replace with != operator
(SIM201)
430-430: Use DEBUG != ['1'] instead of not DEBUG == ['1']
Replace with != operator
(SIM201)
secator/tasks/ffuf.py
99-100: Use a single if statement instead of nested if statements
(SIM102)
secator/runners/_helpers.py
7-7: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
7-7: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
65-65: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
115-115: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
secator/runners/_base.py
776-776: Use self.config.type != 'task' instead of not self.config.type == 'task'
Replace with != operator
(SIM201)
secator/runners/command.py
237-237: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
secator/cli.py
693-693: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
767-767: Loop control variable ix not used within loop body
(B007)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (80)
secator/configs/workflows/wordpress.yaml (2)
17-17: Approve addition oftagsto thewpprobetask.
Addingtags: wordpressunder thewpprobetask improves categorization and aligns it with the other tasks in this workflow.
18-18: Trailing newline only.
The blank line at the end is a formatting update for consistency across YAML files.secator/configs/workflows/user_hunt.yaml (2)
10-10: Add a description for themaigrettask.
Includingdescription: Hunt user accountsenhances clarity and documentation consistency.
11-11: Trailing newline only.
Formatting update to ensure consistent file endings.secator/configs/scans/host.yaml (2)
14-14: Define default target forurl_vuln.
Adding- url.urlunderurl_vuln.targets_ensures the vulnerability scan has a concrete input.
15-15: Trailing newline only.
Consistent formatting update across scan configuration files.secator/configs/workflows/url_nuclei.yaml (2)
11-11: Introduceexclude_tagsfor thenucleitask.
The newexclude_tagslist provides finer control over skipped templates during the HTTP vulnerability scan.
12-12: Trailing newline only.
Pure formatting change for consistency.secator/configs/scans/url.yaml (2)
12-12: Define default target forurl_vuln.
Specifying- url.urlensures the URL vulnerability scan has a defined input.
13-13: Trailing newline only.
Just a formatting fix for consistent file endings.secator/configs/workflows/code_scan.yaml (1)
11-11: Add trailing newline for consistency
This change only appends a blank line after thedescriptionfield under thegrypetask to ensure all workflow YAMLs end with a newline. No functional or structural modifications.secator/configs/scans/network.yaml (1)
17-17: Add trailing newline for consistency
A newline was added at the end of this scan configuration to align formatting with the rest of the scan YAML files. No content or behavior changes.secator/configs/scans/domain.yaml (1)
18-18: Add trailing newline for consistency
Only a trailing newline was introduced at the end of the file to maintain uniform file endings across domain scan configurations. No functional changes.secator/configs/profiles/default.yaml (1)
3-8: Clear profile configuration changesThe profile description update and the change to an empty
optsdictionary with commented-out options provides a good balance between simplification and documentation. This approach gives users the flexibility to enable specific options when needed while maintaining a cleaner default configuration.secator/output_types/info.py (2)
4-4: Updated imports for simplified formattingThe removal of the
rich_escapeimport aligns with the simplified string formatting approach in the__repr__method.
23-23: Simplified message formatting in__repr__The direct embedding of
self.messagein the formatted string removes an unnecessary escaping step, streamlining the output generation.secator/output_types/_base.py (1)
15-17: Added useful__str__methodThe new string representation method provides a clean and consistent way to identify output type instances, which improves debugging and display consistency across the application.
secator/exporters/console.py (2)
2-2: Updated console importThe change to import
console_stdoutinstead ofconsolealigns with broader improvements to output handling.
10-10: Using dedicated stdout consoleUsing
console_stdoutfor printing ensures consistent formatting across the application and properly separates standard output from other console output types.secator/tasks/dnsx.py (1)
69-70: Improved subdomain source tracking with DNS taggingThe addition of
sources=['dns']enhances observability by explicitly tracking subdomains discovered through DNS resolution. This change aligns with the conditional execution features in the broader PR, allowing for better filtering and traceability of subdomain origins.tests/unit/test_command.py (1)
14-14: Updated debug configuration to use string-based list matchingThe condition for setting the logging level has been changed from a numeric comparison to a list equality check, aligning with the new string-based debug configuration approach introduced in this PR. This standardizes debug level handling across the codebase.
tests/integration/test_scans.py (2)
9-9: Removed unused importsClean-up of unused imports from
secator.richandsecator.utilsimproves code cleanliness.
15-15: Updated debug configuration to use string-based list matchingThe condition for setting the logging level has been changed from a numeric comparison to a list equality check, aligning with the new string-based debug configuration approach introduced in this PR.
tests/integration/test_workflows.py (1)
18-18: Updated debug configuration to use string-based list matchingThe condition for setting the logging level has been changed from a numeric comparison to a list equality check, aligning with the new string-based debug configuration approach.
tests/unit/test_runners.py (1)
95-96: Updated test expectations to match refactored hook implementationThe assertions for
run_hookscalls now include thesub='init'parameter, matching the enhanced hook invocation system that provides more granular debug context during command lifecycle events.tests/unit/test_tasks.py (2)
12-12: Updated debug flag checking to use new string-based formatThe condition for setting the logging level has been modified to check if
DEBUGequals the list["1"]rather than checking if it's greater than 0. This change aligns with the refactoring of debug configuration to use a string-based approach for finer control.
33-33: Updated debug command configuration pathThe check for
debug_commandnow looks directly inCONFIG.debuginstead ofCONFIG.debug.component, reflecting the simplified debug configuration structure where debug is now a string rather than an object with components.secator/output_types/tag.py (2)
13-13: Added stored response path field to Tag dataclassNew field allows tracking the location of stored responses associated with a tag, enhancing data linkage capabilities.
36-37: Enhanced Tag representation with clickable file linkThe
__repr__method now includes a clickable envelope icon that links to the stored response file when available, improving user experience by providing direct access to associated content.secator/configs/scans/subdomain.yaml (2)
7-10: Added test option for conditional workflow executionNew string option
testwith default value "test" enables configuration-based control of workflow execution.
12-13: Implemented condition-based workflow executionAdded conditional execution for the
subdomain_reconworkflow using the newifstatement syntax. The workflow will only run ifopts.test == 'test', demonstrating the new condition-based execution feature.tests/integration/test_worker.py (2)
28-28: Parameter changed fromno_process=Falsetoprocess=TrueThis change aligns with the updated API in the
Command.executemethod that now uses a positiveprocessflag instead of the previous negatedno_processflag. The boolean semantics have been inverted but the behavior remains consistent.
48-51: Test updated to verify new nuclei integration optionThe test has been properly updated to:
- Include the new
--nucleiflag in the command string to test the conditional nuclei scanning feature- Use the new
process=Trueparameter instead of the previousno_process=FalseThese changes ensure the test aligns with the updated command execution API and the new workflow option for conditionally enabling nuclei scanning.
secator/definitions.py (1)
23-23: Debug configuration simplified to string-based approachThe debug configuration has been refactored from a structured model approach (
CONFIG.debug.levelandCONFIG.debug.component) to a simpler string-based approach usingCONFIG.debug.split(','). This change:
- Makes debugging more flexible by allowing arbitrary debug categories
- Simplifies configuration by consolidating the debug settings into a single string
- Enables more granular control over what gets logged
This aligns with the PR's goal of improving configuration handling.
secator/config.py (2)
172-172: Debug configuration type changed from nested model to stringThe debug attribute in SecatorConfig has been changed from a structured Pydantic model to a simple string. This simplification:
- Makes the configuration more intuitive and easier to modify
- Reduces complexity by eliminating nested configuration objects
- Aligns with the string-based debug approach seen in definitions.py
This is a positive change that improves the configuration interface.
626-627: Debug condition check updated for string-based approachThe conditional check for debug configuration has been updated to use a substring presence test (
'config' in CONFIG.debug) instead of checking a nested attribute. This correctly implements the new string-based debug configuration approach.secator/utils.py (4)
29-29: Good update to use the consolidated DEBUG variable.The import statement now correctly uses
DEBUGinstead of the removedDEBUG_COMPONENTfromsecator.definitions, which aligns with the consolidated debug configuration approach described in the PR objectives.
422-422: Enhanced console formatting for better readability.The updated formatting with bold blue keys and yellow arrows creates better visual hierarchy in debug output.
434-443: Improved debug component checking logic.The code now correctly iterates through the
DEBUGlist to check for debugging components, supporting the new string-based debug configuration approach.🧰 Tools
🪛 Ruff (0.11.9)
436-439: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
438-441: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
464-465: Properly guarded exception raising for rich markup issues.The condition now checks if 'rich' is in the
DEBUGlist before re-raising exceptions, which aligns with the updated debug configuration.secator/celery_utils.py (3)
8-8: Good addition of TaskRevokedError import.This import is necessary for the improved task revocation handling implemented below.
242-245: Updated comment to reflect new task revocation handling.The comment now correctly explains that the exception could specifically be a TaskRevokedError, which is handled differently than other exceptions.
247-253: Enhanced task revocation handling.This change properly distinguishes between revoked tasks and other exceptions, providing clearer state management and better error reporting. For revoked tasks, it sets a descriptive error message, updates the state to 'REVOKED', and marks the task as ready without raising the exception.
The improved handling of revoked tasks supports the condition-based execution features introduced in this PR, making task and workflow management more robust.
secator/tasks/ffuf.py (2)
37-37: Good addition of stop_on_error option.This new boolean option gives users more control over ffuf task execution behavior.
65-65: Proper mapping to ffuf's native flag.The
stop_on_erroroption is correctly mapped to ffuf's-saflag in the option key map.secator/runners/task.py (2)
52-56: Clarified responsibility for print and report handling.These explicit flags ensure that the task class itself manages the printing and reporting behaviors rather than the Celery runner, making the code more maintainable and reducing potential for conflicts.
62-63: Implemented dynamic queue assignment.This enhancement allows for flexible queue routing based on runtime options. If
task_cls.profileis callable, it's invoked with the options to determine the queue name dynamically; otherwise, it uses the static profile string.This change aligns perfectly with the PR objective of enabling dynamic task profiles, allowing for more sophisticated task routing based on runtime conditions.
secator/output_types/url.py (6)
5-5: Expanded imports to include METHOD constant.Good addition of the METHOD constant from secator.definitions to support the new method field in the Url class.
22-22: Added HTTP method field to Url dataclass.Great enhancement to the Url dataclass to track the HTTP method used for the request. This provides more comprehensive information about the URL interaction.
41-41: Added METHOD to table fields.Appropriate update to include the METHOD field in the table display, ensuring the new field is visible in tabular outputs.
48-49: Replaced TIME with stored_response_path and screenshot_path in table fields.Good replacement of the TIME field with more useful information about stored responses and screenshots, enhancing the information displayed in tables.
64-65: Added conditional display of HTTP method.Nice enhancement to only show the method in the string representation when it's not GET, keeping the output clean while still surfacing non-standard methods.
90-93: Added clickable links for screenshots and stored responses.Excellent improvement to the repr method to render screenshot_path and stored_response_path as clickable icon links, enhancing user interaction with the results.
secator/configs/workflows/url_crawl.yaml (5)
7-12: Added configurable crawler selection option.Good addition of the
crawlersoption to dynamically control which crawler tasks are enabled. This provides flexibility for users to customize their crawling strategy.
19-24: Added conditional crawler tasks with if statements.Excellent implementation of condition-based execution for the
gauandgospidertasks. The conditional execution using theifstatement withopts.crawlerscheck aligns with the PR objective of adding condition-based runs.
27-27: Added conditional execution for cariddi crawler.Consistent implementation of conditional execution for the cariddi task, maintaining the pattern established for other crawlers.
30-30: Added conditional execution for katana crawler.Consistent implementation of conditional execution for the katana task, ensuring all crawler tasks follow the same pattern of conditional execution.
34-34: Simplified targets_ specification.Good simplification of the
targets_field from a dictionary to a direct list containingurl.url. This makes the configuration more concise and easier to understand.secator/runners/scan.py (5)
3-6: Added imports for DotMap and Info output type.Good addition of necessary imports to support the new conditional execution feature and output of informational messages when workflows are skipped.
37-37: Initialized sig variable.Good initialization of the
sigvariable to None, ensuring it has a valid value even if no signatures are added to the sigs list.
43-43: Added has_parent flag to run options.Good addition of the
has_parentflag to the run options, which helps in hierarchical workflow execution context awareness.
48-54: Implemented conditional workflow execution.Excellent implementation of condition-based workflow execution. The code properly:
- Extracts the 'if' condition from workflow options
- Creates a restricted evaluation context with only the opts variable
- Evaluates the condition safely
- Skips the workflow with an informative message when the condition is not met
This directly implements the PR objective of adding condition-based runs for workflows.
69-75: Added null check before creating the chain.Good addition of a check to ensure there are signatures before creating the Celery chain. This prevents attempting to create an empty chain when all workflows are skipped.
secator/rich.py (6)
2-2: Added Path import from pathlib.Good addition of the Path import to support checking file existence for the new clickable file link formatters.
41-41: Enhanced confidence formatter.Good improvement to the confidence formatter to display values in dim uppercase text, making them visually distinct.
45-45: Enhanced URL formatter with clickable links.Excellent enhancement to make URLs clickable in the UI, improving user experience when interacting with URL data.
46-48: Added formatters for file paths with clickable icons.Great addition of formatters for stored_response_path and screenshot_path that render as clickable icons when the files exist. This enhances the interactivity of the output and provides quick access to response data and screenshots.
51-52: Added formatters for conditional clickable links.Excellent addition of formatters for matched_at and match fields that render as clickable links when they are URLs. This improves navigation between related resources.
97-106: Simplified table column creation.Good simplification of the table column creation by removing complex wrapping and overflow logic. The commented code with TODO indicates this was intentional and the simpler approach is preferred.
secator/runners/workflow.py (2)
38-47: 👍 Cleaner option handlingRemoving
print_profilesearly and deferring report generation to the workflow runner keeps concerns well-separated.
147-149: Guard againstNonesignatures – nice touchOnly appending truthy signatures prevents
Noneplaceholders leaking into Celery chains. Saves a lot of head-scratching later.secator/template.py (1)
118-123: Good namespacing of tasks inside scansPrefixing tasks with their workflow name (
wf/task) prevents key collisions when multiple workflows share task IDs – solid choice.secator/configs/workflows/subdomain_recon.yaml (3)
9-27: Verify “internal + default=True” usability
probeis flaggedinternal: Trueyet defaults toTrue. Because internal options are usually hidden from the CLI, users have no direct way to disable probing (--no-probe) unless the framework still auto-generates a negated flag for internal options. Please confirm that:
secatorhonoursis_flagnegation for internal options.- Documentation clarifies the feature.
If not, consider removing
internal: Trueor documenting the implicit behaviour.
37-45: Conditional expressions align with new runner logicUsing
opts.brute_dns,opts.probe or opts.brute_http, etc., matches the DotMap evaluation introduced in the runner – nice consistency.
51-56:stop_on_error: Trueis a welcome safety valveExplicitly halting
ffufon first error avoids noisy cascades when the HTTP brute stage fails. Good addition.secator/click.py (1)
22-25: Overwritingself.commandsdrops theOrderedDictinitialised byclick
click.Groupalready setsself.commands(ordered since Python 3.7).
Re-assigning here risks losing commands registered beforeOrderedGroup’s__init__completes.
Prefer in-place conversion only when needed:- super(OrderedGroup, self).__init__(name, commands, **attrs) - self.commands = commands or OrderedDict() + super().__init__(name, commands or OrderedDict(), **attrs)secator/tasks/katana.py (1)
89-100: Dynamic profile – double-check queue naming consistencyThe lambda correctly defers profile selection until runtime, nice!
However, please ensure the runner queues are actually declared as"cpu"/"io"elsewhere – a mismatch will silently fall back to the default queue.
If the queues are centrally defined, feel free to ignore this.secator/configs/workflows/host_recon.yaml (2)
10-19: Great addition of toggleable options :+1Introducing
nucleiandfullflags increases flexibility while
preserving previous defaults – looks good.
34-41: Verifytargetsbinding insideports_condition
item.host in targetsassumes the template engine provides atargets
variable in this context. If that symbol is not injected the
expression will raise at runtime, skipping the task entirely.secator/runners/command.py (1)
325-329:❌ Incorrect review comment
processvsno_processflag mismatch may break execution
execute()now injectsprocess:kwargs['process'] = kwargs.get('process', False)but the rest of the class (e.g.
yielder,process_line) still checks
self.no_process. UnlessRunner.__init__mapsprocess → no_process,
commands launched viaCommand.execute()will always think they are in
“no-process” mode.Quick fix:
kwargs['print_cmd'] = not kwargs.get('quiet', False) kwargs['print_line'] = True -kwargs['process'] = kwargs.get('process', False) +# keep both for backward compat +process_flag = kwargs.pop('process', False) +kwargs['no_process'] = not process_flagPlease verify the mapping in
Runner.
🏁 Script executed:
#!/bin/bash set -e # Search for Runner class definitions in secator/runners rg -n "class Runner" -n secator/runners # For each file containing Runner, display its __init__ method and surrounding lines for file in $(rg -l "class Runner" secator/runners); do echo "=== File: $file ===" rg -n "def __init__" -A10 "$file" done # Search for no_process flag usage in secator/runners rg -n "no_process" -n secator/runnersLength of output: 2317
#!/bin/bash set -e # Show the Command class definition with context rg -n -C10 "class Command" secator/runners/command.py # Show the __init__ method in Command with context rg -n -C10 "def __init__" secator/runners/command.py # Show any super(...) calls in Command.__init__ rg -n -C5 "super" secator/runners/command.py
🏁 Script executed:
#!/bin/bash set -e # Display the Command class declaration with context rg -n -C5 "class Command" secator/runners/command.py # Display the __init__ method in Command with surrounding lines for context rg -n -C20 "def __init__" secator/runners/command.pyLength of output: 1336
🏁 Script executed:
#!/bin/bash set -e # Display full Command.__init__ method to find super() call and mapping of run_opts sed -n '120,260p' secator/runners/command.pyLength of output: 3979
#!/bin/bash set -e # Display context around lines 320 in secator/runners/command.py to locate the execute() snippet sed -n '300,360p' secator/runners/command.py
🏁 Script executed:
#!/bin/bash set -e # Locate the execute() method in secator/runners/command.py rg -n "def execute" -n secator/runners/command.py -A5 # Show the lines around the kwargs['process'] injection rg -n "kwargs\['process'\]" -n secator/runners/command.py -A5Length of output: 656
No changes needed:
processis correctly mapped tono_process
Runner.init already invertsrun_opts['process']intoself.no_process:
- In
secator/runners/_base.pyline 117:self.no_process = not self.run_opts.get('process', True)- The
execute()method’s injection ofkwargs['process']flows throughrun_optsinto that mapping, so downstream checks ofself.no_processbehave as intended.You can safely ignore the suggested diff.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
secator/cli.py (2)
693-707:⚠️ Potential issueFix mutable default argument and shadowed built-in
The function uses a mutable default argument (
fields=[]) which can lead to unexpected behavior if the function is called multiple times. Additionally, the variabletypeshadows the built-in Python function.Apply this fix to address both issues:
-def process_query(query, fields=[]): +def process_query(query, fields=None): otypes = [o.__name__.lower() for o in FINDING_TYPES] extractors = [] - # Process fields + # Process fields + if fields is None: + fields = [] fields_filter = {} if fields: for field in fields: parts = field.split('.') if len(parts) == 2: - type, field = parts + obj_type, field = parts else: - type = parts[0] + obj_type = parts[0] field = None - if type not in otypes: - console.print(Error(message='Invalid output type: ' + type)) + if obj_type not in otypes: + console.print(Error(message='Invalid output type: ' + obj_type)) sys.exit(1) - fields_filter[type] = field + fields_filter[obj_type] = field🧰 Tools
🪛 Ruff (0.11.9)
693-693: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
719-735:⚠️ Potential issueImprove operator inference in process_query
The current logic silently falls back to
||when no operator is present and gives precedence to&&when both are present, which could lead to unexpected behavior.Implement a more explicit parsing approach:
- operator = '&&' if '&&' in query else '||' + if '&&' in query and '||' in query: + console.print(Error(message='Cannot mix && and || in the same query')) + sys.exit(1) + elif '&&' in query: + operator = '&&' + elif '||' in query: + operator = '||' + else: + operator = '&&' # sensible default
🧹 Nitpick comments (4)
.github/workflows/tests.yml (1)
119-119: Update the GitHub Action checkout versionThe runner for "actions/checkout@v3" is considered outdated. GitHub recommends using the latest versions of actions for security and functionality improvements.
- uses: actions/checkout@v3 + uses: actions/checkout@v4🧰 Tools
🪛 actionlint (1.7.7)
119-119: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
tests/template/test_templates.py (2)
64-78: Consider removing or completing commented-out test methodsThese commented-out test methods (
test_runner_configs) might be intended for future use, but leaving incomplete code in production can lead to confusion. Either complete and uncomment these methods or remove them if they're no longer needed.
79-101: Consider removing or completing commented-out test methodsSimilar to the previous comment, these commented-out test methods (
test_runner_dynamic) should either be completed and uncommented or removed to maintain code clarity.secator/cli.py (1)
767-767: Remove unused loop variableThe loop variable
ixis not used in the loop body, which could cause confusion.- for ix, extractor in enumerate(extractors): + for extractor in extractors: console.print(f':zap: [bold blue]{extractor["type"].title()}[/] [bold orange3]->[/] [bold green]{extractor["condition"]}[/]', highlight=False) # noqa: E501🧰 Tools
🪛 Ruff (0.11.9)
767-767: Loop control variable
ixnot used within loop body(B007)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/tests.yml(1 hunks)secator/cli.py(18 hunks)tests/template/test_templates.py(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/tests.yml
119-119: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Ruff (0.11.9)
secator/cli.py
693-693: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
767-767: Loop control variable ix not used within loop body
(B007)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (15)
.github/workflows/tests.yml (1)
110-147: Good addition of the template test job to CI workflowThe new template job follows good practices by using the same matrix configuration as the integration job and properly sets up the environment with all necessary dependencies. The reuse of setup steps and consistent structure with other jobs shows attention to maintainability.
The addition aligns well with the new template test command in the CLI and helps ensure template integrity through automated testing.
🧰 Tools
🪛 actionlint (1.7.7)
119-119: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
tests/template/test_templates.py (1)
1-101: Well-structured template tests with good isolationThe test module is designed well with:
- Proper use of subtests to isolate each test case
- Redirection of stdout/stderr to prevent test output pollution
- Consistent testing pattern across different template types
- Appropriate dry-run mode to verify template integrity without execution
- Good coverage of both regular and dynamically imported templates
This systematic approach ensures all templates can be instantiated and executed without errors, which is crucial for validating the new condition-based execution feature.
secator/cli.py (13)
20-21: Good module reorganizationMoving the
OrderedGroupclass to a dedicatedclick.pyfile improves code organization and separation of concerns, aligning with the PR's objective of "extracting Click-related functionality into a dedicated file".
73-75: Improved CLI structure with consistent aliasingThe addition of consistent aliases (
x,t,tasks) for the task command improves usability by providing both short-form aliases for power users and descriptive aliases for better discoverability.
90-92: Enhanced workflow command with OrderedGroup and aliasesUsing
OrderedGroupfor the workflow command ensures consistent command ordering in help output, while adding aliases (w,workflows) provides better usability. This is a good application of the refactored CLI infrastructure.
533- A93C 534: Consistent aliasing for workspace commandAdding consistent aliases (
ws,workspaces) to the workspace command maintains the same aliasing pattern used throughout the CLI, enhancing user experience through consistency.
570-587: Well-implemented profiles groupThe new profiles group with its list command provides a clean way to explore available profiles, showing relevant information (name, description, options) in a well-formatted table. This enhances usability for profile management.
593-680: Comprehensive alias management functionalityThe new alias group with commands for enabling, disabling, and listing aliases provides powerful functionality for users to customize their experience. The implementation is thorough, generating appropriate shell commands and providing clear instructions for users.
813-816: Improved UX with clearer loading indicatorsThe enhanced loading indicators with progress tracking (
[1/n]) and result counts provide better user feedback during report loading, especially important for unified reports that might process many files.
841-843: Enhanced report building feedbackThe improved feedback during report building provides users with clear expectations about potentially time-consuming operations, enhancing the overall user experience.
868-873: Good CLI adaptation for piped outputAdding support for piped output in the report_list command enhances CLI interoperability, allowing the command to be used in shell pipelines for further processing.
957-964: Improved health command with aliasesAdding aliases (
h) to the health command improves command discoverability and usability, consistent with the aliasing pattern used throughout the CLI.
1124-1127: Improved install command with aliasesAdding aliases (
i) to the install command and improving the help text enhances command usability and maintains consistency with the aliasing pattern used throughout the CLI.
1498-1527: Well-implemented template test commandThe new template test command provides a comprehensive way to test all templates systematically. It's well-integrated with the existing test infrastructure and provides appropriate environment setup and coverage tracking.
1706-1709: Enhanced coverage command with template supportAdding the
--template-onlyoption to the coverage command provides a clean way to focus on template test coverage, complementing the existing unit and integration options.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
secator/runners/command.py (2)
215-217:self.chunkmay still be undefined
A previous review already raised this; the code still referencesself.chunkbefore guaranteeing it exists, riskingAttributeErrorifRunner.__init__does not initialise it.Please ensure
self.chunkis always set (e.g.self.chunk = getattr(self, "chunk", False)inRunner.__init__).
237-242:⚠️ Potential issueMutable default argument
results=[]Using a list as a default argument shares the same list between calls (classic Python pitfall ‑ Ruff B006). Replace with
Noneand initialise inside:- def si(cls, *args, results=[], **kwargs): + def si(cls, *args, results=None, **kwargs): # TODO: Move this to TaskBase from secator.celery import run_command - profile = cls.profile(kwargs) if callable(cls.profile) else cls.profile - return run_command.si(results, cls.__name__, *args, opts=kwargs).set(queue=profile) + profile = cls.profile(kwargs) if callable(cls.profile) else cls.profile + return run_command.si(results or [], cls.__name__, *args, opts=kwargs).set(queue=profile)Prevents hard-to-trace state leakage between tasks.
🧰 Tools
🪛 Ruff (0.11.9)
237-237: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
secator/cli.py (2)
693-707:⚠️ Potential issueFix mutable default argument and shadowed built-in
The function uses a mutable default argument (
fields=[]) which can cause unexpected behavior if called multiple times, and shadows the built-intypefunction.Apply this fix:
- def process_query(query, fields=[]): + def process_query(query, fields=None): + if fields is None: + fields = [] otypes = [o.__name__.lower() for o in FINDING_TYPES] extractors = [] # Process fields fields_filter = {} if fields: for field in fields: parts = field.split('.') if len(parts) == 2: - type, field = parts + obj_type, field = parts else: - type = parts[0] + obj_type = parts[0] field = None - if type not in otypes: - console.print(Error(message='Invalid output type: ' + type)) + if obj_type not in otypes: + console.print(Error(message='Invalid output type: ' + obj_type)) sys.exit(1) - fields_filter[type] = field + fields_filter[obj_type] = field🧰 Tools
🪛 Ruff (0.11.9)
693-693: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
718-735:⚠️ Potential issueImprove operator inference with explicit error handling
The current operator inference logic silently falls back to
||when no operator is present and gives precedence to&&when both are present, which could lead to unexpected behavior.Consider implementing explicit parsing with error handling:
- operator = '&&' if '&&' in query else '||' + if '&&' in query and '||' in query: + console.print(Error(message='Cannot mix && and || in the same query')) + sys.exit(1) + elif '&&' in query: + operator = '&&' + elif '||' in query: + operator = '||' + else: + operator = '&&' # sensible default - consider what makes most sense for your use case
🧹 Nitpick comments (5)
secator/utils.py (1)
430-444: Simplify equality checks and remove double-negationThe current guard
if not DEBUG == ['all'] and not DEBUG == ['1']:adds an unnecessary cognitive load and was flagged by Ruff (SIM201).
A more idiomatic and readable alternative is to use!=:-if not DEBUG == ['all'] and not DEBUG == ['1']: +if DEBUG not in (['all'], ['1']):This keeps the exact same semantics while improving clarity and avoids the double negation pattern that static-analysis tools complain about.
No functional impact, just readability.🧰 Tools
🪛 Ruff (0.11.9)
430-430: Use
DEBUG != ['all']instead ofnot DEBUG == ['all']Replace with
!=operator(SIM201)
430-430: Use
DEBUG != ['1']instead ofnot DEBUG == ['1']Replace with
!=operator(SIM201)
436-439: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
438-441: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
tests/unit/test_cli.py (2)
8-9: Remove unused imports
PackageInstallerandSourceInstallerare imported but never used, triggering Ruff F401 and unnecessarily increasing module load time.-from secator.installer import InstallerStatus, PackageInstaller, SourceInstaller +from secator.installer import InstallerStatusNo behaviour changes; just a cleanup.
🧰 Tools
🪛 Ruff (0.11.9)
8-8:
secator.installer.PackageInstallerimported but unusedRemove unused import
(F401)
8-8:
secator.installer.SourceInstallerimported but unusedRemove unused import
(F401)
88-102: Collapse nestedwithstatements for brevityThe inner nesting can be flattened for better readability (Ruff SIM117):
-with mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}): - with mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)): - result = self.runner.invoke(cli, ['util', 'build']) +with ( + mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}), + mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)), +): + result = self.runner.invoke(cli, ['util', 'build'])Apply the same pattern to the analogous block at lines 96-102 and 203-206.
🧰 Tools
🪛 Ruff (0.11.9)
88-89: Use a single
withstatement with multiple contexts instead of nestedwithstatements(SIM117)
96-97: Use a single
withstatement with multiple contexts instead of nestedwithstatements(SIM117)
secator/cli.py (2)
920-924: Consider using ternary operator for cleaner codeThe if-else block for setting
workspace_namecould be simplified using a ternary operator.- split = json_path.split('/') - if len(split) > 4: - workspace_name = '/'.join(split[:-4]) - else: - workspace_name = '_default' + split = json_path.split('/') + workspace_name = '/'.join(split[:-4]) if len(split) > 4 else '_default'🧰 Tools
🪛 Ruff (0.11.9)
920-923: Use ternary operator
workspace_name = '/'.join(split[:-4]) if len(split) > 4 else '_default'instead ofif-else-blockReplace
if-else-block withworkspace_name = '/'.join(split[:-4]) if len(split) > 4 else '_default'(SIM108)
767-767: Unused loop index variableThe loop index variable
ixis not used within the loop body. You can use an underscore_to indicate an unused variable.- for ix, extractor in enumerate(extractors): + for _, extractor in enumerate(extractors):Alternatively, if you plan to use the index in the future, add a comment explaining its intended purpose.
🧰 Tools
🪛 Ruff (0.11.9)
767-767: Loop control variable
ixnot used within loop body(B007)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
secator/cli.py(24 hunks)secator/runners/command.py(17 hunks)secator/utils.py(4 hunks)tests/unit/test_cli.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
secator/cli.py (13)
secator/click.py (3)
OrderedGroup(21-85)group(56-82)command(26-54)secator/decorators.py (2)
register_runner(224-421)task(42-46)secator/serializers/dataclass.py (2)
default(8-16)loads_dataclass(38-39)secator/output_types/error.py (1)
Error(8-43)secator/report.py (2)
is_empty(122-123)send(48-56)secator/exporters/console.py (1)
send(6-10)secator/exporters/txt.py (1)
send(7-28)secator/exporters/csv.py (1)
send(12-37)secator/exporters/table.py (1)
send(9-35)secator/output_types/info.py (1)
Info(8-24)secator/utils.py (1)
debug(428-465)secator/installer.py (4)
install(60-110)install(126-169)install(176-227)install(234-257)secator/runners/command.py (2)
Command(28-993)execute(308-333)
🪛 Ruff (0.11.9)
tests/unit/test_cli.py
8-8: secator.installer.PackageInstaller imported but unused
Remove unused import
(F401)
8-8: secator.installer.SourceInstaller imported but unused
Remove unused import
(F401)
88-89: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
96-97: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
203-204: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
secator/cli.py
693-693: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
767-767: Loop control variable ix not used within loop body
(B007)
920-923: Use ternary operator workspace_name = '/'.join(split[:-4]) if len(split) > 4 else '_default' instead of if-else-block
Replace if-else-block with workspace_name = '/'.join(split[:-4]) if len(split) > 4 else '_default'
(SIM108)
secator/runners/command.py
237-237: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
secator/utils.py
430-430: Use DEBUG != ['all'] instead of not DEBUG == ['all']
Replace with != operator
(SIM201)
430-430: Use DEBUG != ['1'] instead of not DEBUG == ['1']
Replace with != operator
(SIM201)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (18)
tests/unit/test_cli.py (1)
184-192: Temporary file not removed
tmp_report.jsonis created but never deleted, which will pollute the project root when the suite is run locally or in CI.Consider
tempfile.NamedTemporaryFile(delete=False)ortmp_pathfrompytest, or explicitlyos.remove('tmp_report.json')in afinallyblock.secator/cli.py (17)
19-20: Good refactoring of CLI-related codeMoving the
OrderedGroupclass to a dedicatedsecator/click.pyfile is a good example of separation of concerns and aligns with the PR objective of extracting Click-related functionality into a dedicated module.
73-75: Improved CLI usability with consistent aliasingAdding multiple aliases (
x,t,tasks) to the task command group enhances usability and provides flexibility for users. This is consistent with the PR objectives of improved command organization and consistent aliases.
90-95: Enhanced workflow group with OrderedGroup and aliasesUsing the
OrderedGroupclass for the workflow group and adding aliases improves command organization and makes the CLI more intuitive. The aliaseswandworkflowsprovide consistent naming conventions across the CLI.
533-536: Consistent aliasing convention for workspace groupAdding aliases
wsandworkspacesto the workspace group maintains the consistent naming pattern established with other command groups, improving overall CLI usability.
569-587: Well-structured profile command groupThe addition of a dedicated profile command group with aliases improves organization of CLI commands. The implementation of the
listcommand with a clean tabular output makes it easy for users to view available profiles and their configurations.
589-680: Comprehensive alias management systemThe new alias group with enable, disable, and list commands provides a well-structured way for users to manage shell aliases. The implementation includes:
- Generation of shell script files for enabling/disabling aliases
- Helpful instructions on how to use the generated files
- Comprehensive listing of all available aliases with clear formatting
This significantly improves user experience by making it easier to work with Secator commands from the shell.
756-760: Good support for piped input in report show commandAdding support for piped input in the
report_showcommand improves CLI interoperability by allowing the use of the command in shell pipelines. Settingunified=Truein this case is a sensible default that enhances usability.
784-786: Improved report query handlingThe enhanced report query handling with fuzzy matching logic improves user experience by allowing more flexible query inputs and providing better error handling when queries don't match.
813-816: Enhanced report loading feedbackThe improved reporting during unified report loading provides better user feedback, especially for large reports, by showing progress information and result counts. This enhances the user experience for complex operations.
841-843: Improved user feedback for report processingAdding informative messages about the report building process with the number of results being processed and a friendly warning about potential processing time improves the user experience.
868-876: Effective piped output handling in report listAdding support for piped output in the
report_listcommand improves shell interoperability, allowing the output to be easily consumed by other commands. The early return pattern after printing paths ensures efficient processing.
906-908: Consistent error handling for report listUsing
Erroroutput type for the "No reports found" message instead of a plain console print ensures consistent error formatting across the application, improving user experience.
965-971: Expanded health command with aliasAdding the
halias to the health command improves usability with a short, memorable command alternative. The command docstring is now properly aligned with the rest of the codebase style.
1132-1135: Improved install command with aliasAdding the
ialias to the install command and clarifying its purpose in the docstring improves usability and makes the command easier to discover and use.
1402-1409: Improved test command error handlingThe enhanced error handling for the test command with clear error messages and early exit using
sys.exit(1)ensures users receive proper feedback when attempting to run tests without the required development environment.
1506-1535: Well-structured template testing commandThe new template test command follows the same pattern as other test commands, promoting consistency in the codebase. It properly sets up the test environment variables and handles test selection based on input options, enhancing the testing capabilities of the framework.
1716-1728: Expanded coverage reporting optionsAdding the
--template-onlyoption to the coverage command enhances the test reporting capabilities by allowing users to focus specifically on template test coverage, which complements the newly added template test command.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
secator/utils.py (1)
527-529: Fixed property mismatch in domain validation.Great! This change addresses the previous review comment about a potential security issue where validation used
top_domain_under_public_suffixbut returnedregistered_domain. The code now consistently usestop_domain_under_public_suffixfor both validation and return.
🧹 Nitpick comments (1)
secator/utils.py (1)
430-431: Simplify boolean expression with!=operator.The current expressions can be simplified for better readability.
- if not DEBUG == ['all'] and not DEBUG == ['1']: + if DEBUG != ['all'] and DEBUG != ['1']:🧰 Tools
🪛 Ruff (0.11.9)
430-430: Use
DEBUG != ['all']instead ofnot DEBUG == ['all']Replace with
!=operator(SIM201)
430-430: Use
DEBUG != ['1']instead ofnot DEBUG == ['1']Replace with
!=operator(SIM201)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
secator/utils.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
secator/utils.py
430-430: Use DEBUG != ['all'] instead of not DEBUG == ['all']
Replace with != operator
(SIM201)
430-430: Use DEBUG != ['1'] instead of not DEBUG == ['1']
Replace with != operator
(SIM201)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (4)
secator/utils.py (4)
29-29: Import updated to align with new debug configuration approach.The change from
DEBUG_COMPONENTtoDEBUGaligns with the PR's objective of making debug configuration string-based for finer control, as mentioned in the PR summary.
422-422: Improved debug output styling.Changed styling from dim cyan/yellow/green to bold blue/yellow/blue for better readability of debug output.
435-436: Updated debug component iteration.The code now correctly iterates over the new string-based debug configuration values.
464-465: Updated rich markup exception handling.The condition now correctly checks for 'rich' in the new
DEBUGlist.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/tests.yml (1)
< 10000 /details>
1-3: 🛠️ Refactor suggestionAdd a minimal permissions block to tighten GITHUB_TOKEN scope
The workflow currently defaults to full
GITHUB_TOKENpermissions. For security best practices, explicitly define a minimal permissions block (e.g.,contents: read) at the top level to avoid over-permission.
♻️ Duplicate comments (1)
.github/workflows/tests.yml (1)
150-150: Confirm inclusion oftemplatein coverage dependenciesThis correctly updates the
coveragejob’sneedsarray to include thetemplatejob, ensuring template tests are factored into the aggregated coverage report.
🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)
110-147: DRY up common setup steps in thetemplatejobThe
templatejob duplicates the same sequence of steps from theintegrationjob (checkout, install, Docker Compose setup, PATH exports). Consider extracting these into a reusable workflow or composite action (or using YAML anchors) to reduce maintenance overhead and keep the CI DRY.🧰 Tools
🪛 actionlint (1.7.7)
119-119: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/tests.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/tests.yml
119-119: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 GitHub Check: CodeQL
.github/workflows/tests.yml
[warning] 111-148: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (1)
.github/workflows/tests.yml (1)
118-119: Skip updatingactions/checkout@v3The actionlint warning about
actions/checkout@v3being “too old” is a false positive; v3 is the current stable branch. No changes are needed here.🧰 Tools
🪛 actionlint (1.7.7)
119-119: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
secator/decorators.py (1)
303-326:⚠️ Potential issueRemove duplicated version check code
There's an exact duplicate of the version checking code block (lines 303-313 and 315-325). This is likely an oversight and could cause confusion during maintenance.
# Show version if version: data = task_cls.get_version_info() current = data['version'] latest = data['latest_version'] installed = data['installed'] if not installed: console.print(f'[bold red]{task_cls.__name__} is not installed.[/]') else: console.print(f'{task_cls.__name__} version: [bold green]{current}[/] (recommended: [bold green]{latest}[/])') sys.exit(0) - # Show version - if version: - data = task_cls.get_version_info() - current = data['version'] - latest = data['latest_version'] - installed = data['installed'] - if not installed: - console.print(f'[bold red]{task_cls.__name__} is not installed.[/]') - else: - console.print(f'{task_cls.__name__} version: [bold green]{current}[/] (recommended: [bold green]{latest}[/])') - sys.exit(0)
🧹 Nitpick comments (1)
secator/decorators.py (1)
339-345: Consider removing or documenting commented codeThere's a block of commented code related to removing default-valued options. Either remove it if it's no longer needed or add a comment explaining why it's kept and when it might be uncommented.
- # Remove options whose values are default values - # for k, v in options.items(): - # if k in config.options.toDict(): - # continue - # opt_name = k.replace('-', '_') - # if opt_name in opts and opts[opt_name] == v.get('default', None): - # del opts[opt_name] + # TODO: Remove this commented code block or document why it's kept
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
secator/decorators.py(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (9)
secator/decorators.py (9)
25-26: Implementation properly addresses the previous flag/description mismatchThe process flag has been updated to fix the previous issue by changing the help text to "Enable secator processing" and adding
reverse=True, which properly aligns the flag behavior with its description. This maintains backward compatibility while making the flag's purpose clearer to users.
30-30: Good addition of the tree visualization featureThe new
treeflag allows users to visualize the runner tree structure, which enhances debugging and understanding of complex workflows. This is a useful usability improvement.
43-48: Clean task decorator implementationThe new
taskdecorator provides a simple and clear way to mark classes as tasks, which improves code organization and makes task registration more explicit.
50-89: Improved option decoration with better handling of flag oppositesThe refactored
decorate_command_optionsfunction now properly handles option flags with opposites and reverse flags, including fixing the short option for opposites. This makes the CLI more intuitive for users who want to toggle options.
110-121: Good addition of recursive option merging from child workflowsThe implementation now correctly merges options from child workflows, enabling better inheritance of configuration options through the workflow hierarchy. This enhances the flexibility of the configuration system.
185-186: Good fix for required flags defined in configThe code now correctly handles the case where a required option is defined in the YAML configuration, making it no longer required in the CLI. This prevents unnecessary input and improves user experience.
332-337: Well-implemented tree visualization featureThe implementation of the tree visualization feature is clean and self-contained. It builds the runner tree, renders it to the console, and exits, providing a nice user experience enhancement.
358-370: Good enhancement to driver trackingThe addition of tracking actual drivers in a separate list is a good improvement that allows for better reporting and validation of used drivers.
406-408: Enhanced print options for better debuggingThe addition of more granular print options (
print_profiles,print_start,print_end) gives users better control over the verbosity and content of the output, which is especially useful for debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
secator/runners/_helpers.py (4)
7-7: Fix mutable default argumentsUsing mutable objects (
[]and{}) as default parameters is dangerous as they are shared across function calls, which can lead to unexpected state leakage.-def run_extractors(results, opts, inputs=[], ctx={}, dry_run=False): +def run_extractors(results, opts, inputs=None, ctx=None, dry_run=False): + if inputs is None: + inputs = [] + if ctx is None: + ctx = {}🧰 Tools
🪛 Ruff (0.11.9)
7-7: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
7-7: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
65-65: Fix mutable default argument-def extract_from_results(results, extractors, ctx={}): +def extract_from_results(results, extractors, ctx=None): + if ctx is None: + ctx = {}🧰 Tools
🪛 Ruff (0.11.9)
65-65: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
115-115: Fix mutable default argument-def process_extractor(results, extractor, ctx={}): +def process_extractor(results, extractor, ctx=None): + if ctx is None: + ctx = {}🧰 Tools
🪛 Ruff (0.11.9)
115-115: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
139-147: 💡 Verification agent🧩 Analysis chain
Security vulnerability with
eval()While you've restricted the builtins to only include
len(), the current implementation ofeval()is still potentially unsafe. A more robust approach would be to use a dedicated expression evaluation library.- safe_globals = {'__builtins__': {'len': len}} - eval_result = eval(_condition, safe_globals, ctx) + # Option 1: Use simpleeval library + from simpleeval import SimpleEval + evaluator = SimpleEval(names=ctx, functions={"len": len}) + eval_result = evaluator.eval(_condition) + + # Option 2: Use asteval library + # from asteval import Interpreter + # interpreter = Interpreter(usersyms=ctx, minimal=True) + # interpreter.symtable['len'] = len + # eval_result = interpreter.eval(_condition)Consider using
simpleeval,asteval, or a similar library designed for safe expression evaluation to prevent arbitrary code execution.
🌐 Web query:
Python simpleeval vs asteval for safe expression evaluation💡 Result:
When choosing between
simpleevalandastevalfor safe expression evaluation in Python, here's a comparison to help you decide which might best suit your needs:simpleeval
- Purpose: A minimalistic and safe expression evaluator that evaluates simple Python expressions.
- Safety: Designed to be safe and prevent execution of arbitrary code. It only evaluates expressions, not statements.
- Features: Supports arithmetic operations, comparisons, boolean logic, and basic math functions. It allows adding custom functions and names.
- Ease of Use: Very straightforward API, easy to integrate and use.
- Limitations: Not designed for complex code or statements, only expressions.
- Use Case: Ideal when you want to evaluate simple, user-generated expressions safely without the risk of code injection.
asteval
- Purpose: A more comprehensive evaluator that interprets a subset of Python's abstract syntax tree (AST).
- Safety: More flexible and powerful but tries to keep evaluation safe by limiting the accessible built-ins and modules. However, due to its complexity, it can potentially be less secure than
simpleevalif not carefully managed.- Features: Supports assignment, function calls, loops, and more complex Python constructs beyond simple expressions.
- Ease of Use: More powerful but more complex API. Requires careful setup to maintain security.
- Limitations: Larger attack surface due to broader feature set; security needs more attention.
- Use Case: Suitable for scenarios needing more than just expressions—where small scripts or more complex logic need safe evaluation.
Summary
- Use
simpleevalif you need to safely evaluate user-defined expressions with a clean, minimal API and strong security.- Use
astevalif you need to evaluate more complex code snippets safely but be prepared to handle security carefully.If you want, I can provide simple example codes for both to demonstrate their usage. Would you like that?
Replace
eval()withsimpleevalfor safe, minimal expression evaluationThe current
eval(_condition, safe_globals, ctx)—even with restricted builtins—can still allow attribute traversal on objects inctxand poses a security risk. Since you only need to evaluate simple boolean and comparison expressions, switch tosimpleevalwhich is designed for this use case.Files to update:
- secator/runners/_helpers.py (around lines 139–147)
Suggested changes:
--- a/secator/runners/_helpers.py +++ b/secator/runners/_helpers.py @@ -1,3 +1,4 @@ +from simpleeval import SimpleEval import ... @@ -135,15 +136,17 @@ def _apply_condition(results, _condition, _type, debug): tmp_results = [] - ctx['item'] = item - ctx[f'{_type}'] = item - safe_globals = {'__builtins__': {'len': len}} - eval_result = eval(_condition, safe_globals, ctx) + # inject the current item into the evaluation context + ctx['item'] = item + ctx[f'{_type}'] = item + + # evaluate condition safely using simpleeval + evaluator = SimpleEval(names=ctx, functions={'len': len}) + eval_result = evaluator.eval(_condition) + if eval_result: tmp_results.append(item) del ctx['item'] del ctx[f'{_type}'] - debug(f'kept {len(tmp_results)} out of {len(results)} items after condition [bold]{_condition}[/bold]', sub='extractor') + debug( + f'kept {len(tmp_results)} out of {len(results)} items after condition [bold]{_condition}[/bold]', + sub='extractor' + )• Move the
SimpleEvalimport to the module level for clarity and performance.
• Remove the manualsafe_globalsandeval()call entirely.
•simpleevalrestricts operations to expressions only, eliminating risks of arbitrary code execution—perfect for user‐provided filter logic.
🧹 Nitpick comments (2)
secator/runners/_helpers.py (1)
137-137: Use!=operator instead ofnot ==- if not item._type == _type: + if item._type != _type:Using
!=is more idiomatic and clearer thannot ==.🧰 Tools
🪛 Ruff (0.11.9)
137-137: Use
item._type != _typeinstead ofnot item._type == _typeReplace with
!=operator(SIM201)
tests/unit/test_runners_helpers.py (1)
136-161: TODO comment needs addressingThere's a TODO comment about testing nested field access. Consider implementing this test to ensure complete coverage of the field formatting functionality.
def test_process_extractor_with_nested_field(self): """Test process_extractor with nested field access.""" extractor = { 'type': 'mock', 'field': '{nested.subfield}' } result = process_extractor(self.results, extractor) self.assertEqual(result, ['nested_value', 'nested_value', 'nested_value'])Would you like me to implement the commented-out test for nested field access?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
secator/configs/workflows/cidr_recon.yaml(1 hunks)secator/configs/workflows/url_crawl.yaml(1 hunks)secator/configs/workflows/url_dirsearch.yaml(2 hunks)secator/configs/workflows/url_fuzz.yaml(2 hunks)secator/decorators.py(12 hunks)secator/runners/_helpers.py(3 hunks)tests/unit/test_runners_helpers.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- secator/configs/workflows/url_dirsearch.yaml
- secator/configs/workflows/cidr_recon.yaml
- secator/configs/workflows/url_fuzz.yaml
- secator/configs/workflows/url_crawl.yaml
- secator/decorators.py
🧰 Additional context used
🪛 Ruff (0.11.9)
secator/runners/_helpers.py
7-7: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
7-7: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
65-65: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
115-115: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
137-137: Use item._type != _type instead of not item._type == _type
Replace with != operator
(SIM201)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (16)
secator/runners/_helpers.py (7)
26-27: LGTM: Good context tracking enhancementTracking the current key in the context and passing it to downstream functions improves state management and enables better debug logging.
30-32: LGTM: Improved dry run formatting and context managementNice improvement to use
fmt_extractorfor more descriptive placeholder values in dry run mode, and storing computed inputs in the context makes them available to downstream processors.
46-63: LGTM: Well-designed formatter functionThe new
fmt_extractorfunction provides a clean way to format extractors for display, handling both string and dict formats with proper condition representation.
76-87: LGTM: Improved results collectionUsing
extendinstead of individualappendoperations is a cleaner approach for collecting results from multiple extractors.
90-113: LGTM: Clean extractor parsing logicGood refactoring to extract the parsing logic into its own function, which improves code organization and reusability.
125-131: LGTM: Good debug logging and parsingAdding debug logging with context details and using the new parser function improves code readability and diagnostics.
148-157: LGTM: Improved filtering and debug loggingGood implementation of type filtering and field formatting with helpful debug logging.
tests/unit/test_runners_helpers.py (9)
1-50: LGTM: Well-structured test setupGood test class organization with a comprehensive setup method that creates various mock objects for testing different scenarios.
52-64: LGTM: Thorough string extractor parsing testsThe tests cover valid, invalid, and empty string formats for extractor parsing, which ensures robustness.
66-83: LGTM: Complete dict extractor parsing testsGood test coverage for dictionary-based extractors with both full and minimal field sets.
84-102: LGTM: Comprehensive formatter testsThe tests verify formatter behavior for string and dictionary extractors, as well as invalid formats.
103-114: LGTM: Good type filtering testsThorough verification of type-based filtering for both mock and URL objects.
115-135: LGTM: Comprehensive condition testingTests cover both simple conditions and more complex ones using functions like
len().
162-182: LGTM: Good error handling testsThe tests properly verify both successful extraction and error handling scenarios using mocking.
183-203: LGTM: Thorough run_extractors testsGood tests for both normal and dry run modes, including verification of deduplication by mocking.
204-238: LGTM: Comprehensive folder ID testsThe tests cover all important scenarios for task folder ID generation, including non-existent paths, empty directories, and directories with numeric names.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tests/unit/test_runners_helpers.py (1)
187-192: Use a temporary file to avoid polluting the project rootCreating
tmp_report.jsonin the repo root may leave garbage if the test crashes and also forces the test runner to have write permissions on the repository.
Switching totempfile.NamedTemporaryFile()(or patchingopen) keeps the FS clean and makes the test less brittle.tests/unit/test_cli.py (3)
88-93: Collapse nestedwithstatements for better readabilityRuff SIM117 flags the nested context-managers. A single
withline is shorter and easier to read:- with mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}): - with mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)): - result = self.runner.invoke(cli, ['util', 'build']) + with mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}), \ + mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)): + result = self.runner.invoke(cli, ['util', 'build'])🧰 Tools
🪛 Ruff (0.11.9)
88-89: Use a single
withstatement with multiple contexts instead of nestedwithstatements(SIM117)
96-101: Same comment as above – merge the nestedwithblocksThis will satisfy SIM117 and keep the style consistent across the module.
🧰 Tools
🪛 Ruff (0.11.9)
96-97: Use a single
withstatement with multiple contexts instead of nestedwithstatements(SIM117)
187-191: Clean up the temporary file created during the testAfter the
report showassertion succeeds, deletetmp_report.json(or usetempfile.NamedTemporaryFile) so repeated test runs don’t reuse stale artefacts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/unit/test_cli.py(2 hunks)tests/unit/test_runners_helpers.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/unit/test_cli.py (2)
secator/cli.py (1)
cli(54-66)secator/installer.py (3)
InstallerStatus(29-44)PackageInstaller(122-169)SourceInstaller(172-227)
🪛 Ruff (0.11.9)
tests/unit/test_runners_helpers.py
12-12: secator.output_types.Error imported but unused
Remove unused import: secator.output_types.Error
(F401)
tests/unit/test_cli.py
8-8: secator.installer.PackageInstaller imported but unused
Remove unused import
(F401)
8-8: secator.installer.SourceInstaller imported but unused
Remove unused import
(F401)
88-89: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
96-97: Use a s
47C5
ingle with statement with multiple contexts instead of nested with statements
(SIM117)
203-204: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (1)
tests/unit/test_cli.py (1)
203-206: CLI option is probably--output, not-outputMost Click options are prefixed with a double dash (
--).
If the CLI really expects--outputor-o, the current call will raiseNo such optionand the test will pass falsely because the command exits with code 2 (Click usage error) rather than executing the logic.- result = self.runner.invoke(cli, ['report', 'export', 'test.json', '-output', 'console']) + result = self.runner.invoke(cli, ['report', 'export', 'test.json', '--output', 'console'])Please double-check the actual option name in
secator.cli.report.export.🧰 Tools
🪛 Ruff (0.11.9)
203-204: Use a single
withstatement with multiple contexts instead of nestedwithstatements(SIM117)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
secator/report.py (1)
107-107:⚠️ Potential issue
data['info']['errors']is overwritten – append instead to avoid silently dropping earlier errorsThe current implementation overwrites previously collected errors instead of appending new ones. This could cause runner-level errors (collected at line 78) to be lost.
Apply this fix to preserve all errors:
- if errors: - data['info']['errors'] = errors + if errors: + # keep any previously recorded errors + data['info'].setdefault('errors', []).extend(errors)secator/tree.py (1)
95-97:⚠️ Potential issueGuard against non-mapping sub-tasks
subtask_detailsmay beNoneor aboolif the user disables the task in YAML, which would cause anAttributeErrorwhen calling.get().Apply this fix:
- condition = subtask_details.get('if') + condition = ( + subtask_details.get('if') + if isinstance(subtask_details, dict) + else None + )tests/unit/test_cli.py (1)
95-103: Improve nested context managersMultiple nested
withstatements reduce code readability and maintainability.Consider combining the nested
withstatements for better readability:-with mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}): - with mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)): - with mock.patch.dict(os.environ, {'HATCH_INDEX_AUTH': 'test_token'}): +with mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}), \ + mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)), \ + mock.patch.dict(os.environ, {'HATCH_INDEX_AUTH': 'test_token'}):🧰 Tools
🪛 Ruff (0.11.9)
97-98: Use a single
withstatement with multiple contexts instead of nestedwithstatements(SIM117)
🧹 Nitpick comments (4)
tests/unit/test_runners_helpers.py (1)
177-183: Implement commented-out error handling testThe test for error handling during extraction is commented out. Consider implementing this test to ensure error handling works correctly.
Uncomment and implement the error handling test to ensure robust error capture and reporting in the extraction process.
secator/runners/_helpers.py (1)
121-166: Security improvement in condition evaluationThe condition evaluation now uses a restricted
__builtins__dictionary that only includes thelenfunction, significantly reducing the risk of arbitrary code execution through YAML-supplied conditions.Good security practice with the restricted globals. Consider adding a docstring comment highlighting this security feature for future maintainers.
tests/unit/test_cli.py (2)
87-94: Improve nested context managersMultiple nested
withstatements reduce code readability and maintainability.Consider combining the nested
withstatements for better readability:-with mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}): - with mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)): +with mock.patch('secator.cli.ADDONS_ENABLED', {'build': True}), \ + mock.patch('secator.cli.Command.execute', return_value=mock.MagicMock(return_code=0)):🧰 Tools
🪛 Ruff (0.11.9)
89-90: Use a single
withstatement with multiple contexts instead of nestedwithstatements(SIM117)
204-211: Improve multi-line context managersUsing backslash line continuation makes the code less readable and more error-prone.
Consider using parentheses to group the context managers for better readability:
-with mock.patch('builtins.open', mock.mock_open(read_data='{"info":{"name":"test", "title": "test"}, "results":{}}')), \ - mock.patch('secator.cli.loads_dataclass', return_value={"info": {"name": "test", "title": "test"}, "results": {}}): +with ( + mock.patch('builtins.open', mock.mock_open(read_data='{"info":{"name":"test", "title": "test"}, "results":{}}')), + mock.patch('secator.cli.loads_dataclass', return_value={"info": {"name": "test", "title": "test"}, "results": {}}) +):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
secator/celery.py(3 hunks)secator/cli.py(24 hunks)secator/click.py(1 hunks)secator/configs/workflows/wordpress.yaml(0 hunks)secator/decorators.py(12 hunks)secator/report.py(3 hunks)secator/runners/_base.py(34 hunks)secator/runners/_helpers.py(2 hunks)secator/runners/command.py(17 hunks)secator/tasks/ffuf.py(3 hunks)secator/tree.py(1 hunks)tests/integration/test_workflows.py(1 hunks)tests/template/test_templates.py(1 hunks)tests/unit/test_cli.py(2 hunks)tests/unit/test_runners_helpers.py(1 hunks)
💤 Files with no reviewable changes (1)
- secator/configs/workflows/wordpress.yaml
✅ Files skipped from review due to trivial changes (1)
- tests/template/test_templates.py
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/integration/test_workflows.py
- secator/tasks/ffuf.py
- secator/celery.py
- secator/click.py
- secator/runners/command.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
secator/tree.py (1)
secator/template.py (1)
TemplateLoader(17-137)
🪛 Ruff (0.11.9)
tests/unit/test_runners_helpers.py
12-12: SyntaxError: Expected an identifier, but found a keyword 'from' that cannot be used here
12-12: SyntaxError: Simple statements must be separated by newlines or semicolons
12-12: SyntaxError: Simple statements must be separated by newlines or semicolons
13-13: SyntaxError: Expected an identifier, but found a keyword 'from' that cannot be used here
13-13: SyntaxError: Simple statements must be separated by newlines or semicolons
13-13: SyntaxError: Simple statements must be separated by newlines or semicolons
tests/unit/test_cli.py
89-90: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
97-98: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
188-188: Use a context manager for opening files
(SIM115)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (55)
secator/report.py (2)
90-94: Good addition of error handling for sorting operationsThe try-except block for catching TypeError during sorting is a valuable addition that prevents the report generation from failing completely if items can't be sorted properly.
97-116: Enhanced extractor processing with type-aware grouping and logical operationsThe new implementation significantly improves extractor processing by:
- Grouping extractors by type
- Supporting logical operations ('or'/'and') for combining results
- Applying proper deduplication
This provides greater flexibility and control over how extractors filter and combine results.
secator/tree.py (4)
4-10: Well-structured formatting options for tree visualizationThe
DEFAULT_RENDER_OPTSdictionary provides a clean, configurable way to define rendering styles for different node types using Rich formatting.
13-30: Clean implementation of TaskNode with proper typingThe
TaskNodeclass provides a simple yet effective representation of workflow/scan task nodes with proper type annotations and a clear string representation.
109-110: Good defensive check for dictionary typeThe proper guard to check if
workflow_detailsis a dictionary before calling.get()is a good practice that prevents potentialAttributeErrorexceptions.
44-71: Well-implemented tree rendering with proper recursion and formattingThe tree rendering logic elegantly handles:
- Recursive rendering of nested structures
- Proper indentation and branch characters
- Conditional expressions in nodes
- Custom formatting based on node types
tests/unit/test_runners_helpers.py (4)
12-13: Unused import removedGood cleanup removing the unused
Errorimport.🧰 Tools
🪛 Ruff (0.11.9)
12-12: SyntaxError: Expected an identifier, but found a keyword 'from' that cannot be used here
12-12: SyntaxError: Simple statements must be separated by newlines or semicolons
12-12: SyntaxError: Simple statements must be separated by newlines or semicolons
13-13: SyntaxError: Expected an identifier, but found a keyword 'from' that cannot be used here
13-13: SyntaxError: Simple statements must be separated by newlines or semicolons
13-13: SyntaxError: Simple statements must be separated by newlines or semicolons
17-24: Well-designed mock class for testingThe
MockOutputTypeclass provides a clean test double with appropriate default values and nested structure for thorough testing of the extractor functions.
53-183: Comprehensive test coverage for extractor functionsThe tests thoroughly validate all aspects of the extractor functionality:
- Parsing from strings and dictionaries
- Formatting for display
- Type filtering
- Conditional expressions
- Field formatting and extraction
- Error handling
This ensures the extractor code is robust and behaves as expected.
184-203: Thorough testing of the run_extractors functionThe tests cover both normal extraction and dry-run mode, ensuring all aspects of the function work correctly including option updating and error handling.
secator/decorators.py (6)
25-25: Fixed flag/description mismatch forprocessoptionThe
processflag now correctly usesreverse: Trueand the help text matches its behavior.
29-29: Good addition of tree visualization flagThe new
treeflag enables users to visualize the task structure, which is a valuable feature for understanding complex workflows.
42-47: Clean implementation of task decoratorThe
taskdecorator provides a simple way to mark classes as tasks with minimal overhead.
124-126: Security improvement: removed eval in favor of explicit mappingReplacing
evalwith an explicit type mapping dictionary eliminates a potential security risk while maintaining the same functionality.
322-325: Well-integrated tree visualization implementationThe tree visualization feature is cleanly integrated into the CLI command function, building and rendering the runner tree with proper exits.
387-389: Enhanced output control with additional print optionsAdding
print_profiles,print_start, andprint_endoptions gives users more granular control over the output.secator/runners/_helpers.py (6)
7-37: Great fix for mutable default argumentsThe change from using mutable default arguments (
inputs=[],ctx={}) toNonewith proper initialization inside the function prevents shared state bugs that can lead to unexpected behavior.-def run_extractors(results, opts, inputs=[], ctx={}, dry_run=False): +def run_extractors(results, opts, inputs=None, ctx=None, dry_run=False): + if inputs is None: + inputs = [] + if ctx is None: + ctx = {}
30-37: Enhanced context-aware extractionAdding the current key to the context and storing computed inputs under
targetsenables better state management between extractors and downstream processing.
34-36: Improved dry run output with formatted extractorsUsing the new
fmt_extractorfunction to format extractors in dry run mode provides more readable and informative output for debugging.
50-67: Well-designed formatter for extractor representationThis new helper function converts extractor definitions into human-readable string format, making debug output and dry run results more informative.
69-94: Context-aware extraction improvementsThe updated function signature now accepts and passes a context parameter, enabling better state sharing between extractors and more informative debug output.
96-119: Good modularization with extracted parsing logicMoving the parsing logic to a dedicated
parse_extractorfunction improves code organization and maintainability. The function properly handles both dictionary and string extractor formats.tests/unit/test_cli.py (4)
14-26: Good test setup with proper mockingMocking the installer methods prevents actual installation during tests, making the tests more reliable and faster.
27-30: Proper tearDown implementationAdding tearDown to stop the mocks prevents test leakage that could affect other tests.
31-36: Enhanced version test assertionThe test now verifies the specific version string is in the output, making it more precise.
62-66: Better offline mode testingThe test now properly verifies that the command fails with the expected exit code and error message when in offline mode.
secator/runners/_base.py (10)
42-50: Good addition of colorized runner name formattingAdding a helper function to format runner names with consistent colors based on their type improves visual cues in the output.
77-79: New profiles supportAdding profiles support enhances configuration management and reusability.
117-118: Improved process option handlingConverting the 'process' run option to 'no_process' with logical negation makes the code more intuitive.
133-134: Added print_profiles optionThis new option allows controlling profile information visibility, enhancing flexibility in output control.
144-160: Enhanced debug logging for initializationAdding detailed debug information with categorized subsystems improves observability and troubleshooting capabilities.
202-213: Good use of properties for option categorizationThese properties provide a cleaner interface for accessing different categories of options, making the code more maintainable.
307-310: Helpful ID propertyThis property abstracts away the complexity of determining the runner ID from various context sources.
379-392: Improved context handling with DotMapUsing DotMap for the context provides more structured access to nested values and better integration with the extraction system.
774-787: Enhanced visual output with runner treeThe integration with build_runner_tree provides a more informative visual representation of runner hierarchies.
1023-1050: Improved profile resolution with feedbackThe resolve_profiles method now returns loaded profiles and optionally provides user feedback about loaded profiles.
secator/cli.py (19)
73-78: Enhanced CLI with command aliasesAdding multiple aliases for the task command improves usability by providing shorter alternatives.
90-96: Ordered workflow commands with aliasesUsing OrderedGroup and adding aliases improves the command organization and usability.
106-112: Consistent command aliasing patternAdding aliases to scan commands maintains a consistent pattern across the CLI.
207-241: Added explicit exit codesAdding explicit sys.exit(1) calls ensures proper process termination on errors.
533-536: Consistent workspace command aliasingAdding aliases for the workspace command maintains a consistent pattern across the CLI.
566-587: New profiles management commandsAdding a profile command group with list functionality improves configuration management capabilities.
589-680: New shell alias management commandsThe alias command group provides a robust way to manage shell aliases for Secator commands, improving workflow integration.
687-688: Report command aliasingAdding aliases for the report command maintains a consistent pattern across the CLI.
693-752: Well-designed query processing functionThe new
process_queryfunction properly handles null defaults, explicitly checks for mixing operators, and provides clear error messages.
767-771: Improved piped input handlingThe report_show command now intelligently handles piped input by reading from stdin and enabling unified mode.
795-797: Better report query handlingThe command now properly handles comma-separated report queries and determines whether to load all reports based on path existence.
823-827: Enhanced user feedback during report loadingAdding detailed progress information makes the report loading process more transparent to users.
852-854: Improved feedback for unified report buildingAdding detailed messaging with a progress indicator improves the user experience for potentially long-running operations.
879-887: Smart handling of piped outputThe report_list command now detects piped output and adjusts its output format accordingly.
917-919: Better report count feedbackAdding an informational message about the number of reports found improves user feedback.
973-979: Health command aliasingAdding an alias for the health command and improving the docstring enhances usability.
1140-1143: Install command aliasingAdding an alias for the install command and improving the docstring maintains a consistent pattern across the CLI.
1514-1543: New template test commandAdding a dedicated command for template testing improves test coverage and organization.
1724-1736: Enhanced coverage reportingAdding template-only coverage option provides more granular control over coverage reporting.
| def test_report_show_command(self, mock_list_reports): | ||
| mock_list_reports.return_value = [] | ||
| tf = tempfile.NamedTemporaryFile(delete=False) | ||
| with open(tf.name, 'w') as f: | ||
| f.write('{"info":{"name":"test", "title": "test"}, "results":{}}') | ||
| result = self.runner.invoke(cli, ['report', 'show', tf.name]) | ||
| os.remove(tf.name) | ||
| assert not result.exception | ||
| assert result.exit_code == 0 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use context manager for temporary file
The temporary file is created but not properly managed with a context manager, which could lead to resource leaks.
Use a context manager to ensure proper file cleanup:
-tf = tempfile.NamedTemporaryFile(delete=False)
-with open(tf.name, 'w') as f:
- f.write('{"info":{"name":"test", "title": "test"}, "results":{}}')
-result = self.runner.invoke(cli, ['report', 'show', tf.name])
-os.remove(tf.name)
+with tempfile.NamedTemporaryFile(delete=False) as tf:
+ tf.write(b'{"info":{"name":"test", "title": "test"}, "results":{}}')
+ tf.flush()
+ result = self.runner.invoke(cli, ['report', 'show', tf.name])
+os.remove(tf.name)📝 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.
| def test_report_show_command(self, mock_list_reports): | |
| mock_list_reports.return_value = [] | |
| tf = tempfile.NamedTemporaryFile(delete=False) | |
| with open(tf.name, 'w') as f: | |
| f.write('{"info":{"name":"test", "title": "test"}, "results":{}}') | |
| result = self.runner.invoke(cli, ['report', 'show', tf.name]) | |
| os.remove(tf.name) | |
| assert not result.exception | |
| assert result.exit_code == 0 | |
| def test_report_show_command(self, mock_list_reports): | |
| mock_list_reports.return_value = [] | |
| - tf = tempfile.NamedTemporaryFile(delete=False) | |
| - with open(tf.name, 'w') as f: | |
| - f.write('{"info":{"name":"test", "title": "test"}, "results":{}}') | |
| - result = self.runner.invoke(cli, ['report', 'show', tf.name]) | |
| - os.remove(tf.name) | |
| + with tempfile.NamedTemporaryFile(delete=False) as tf: | |
| + tf.write(b'{"info":{"name":"test", "title": "test"}, "results":{}}') | |
| + tf.flush() | |
| + result = self.runner.invoke(cli, ['report', 'show', tf.name]) | |
| + os.remove(tf.name) | |
| assert not result.exception | |
| assert result.exit_code == 0 |
🧰 Tools
🪛 Ruff (0.11.9)
188-188: Use a context manager for opening files
(SIM115)
🤖 Prompt for AI Agents
In tests/unit/test_cli.py around lines 186 to 195, the temporary file is created
without using a context manager, which risks resource leaks. Refactor the test
to create and write to the temporary file within a with statement that manages
the NamedTemporaryFile context, ensuring the file is properly closed and deleted
after use. Adjust the code to remove manual file deletion and rely on the
context manager for cleanup.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
secator/celery.py (1)
170-170: Fix mutable default argument issueUsing a mutable default argument like
opts={}can cause unexpected behavior when state from one call leaks into another.Change the signature to use
Noneand create a fresh dict:-@app.task(bind=True) -def run_command(self, results, name, targets, opts={}): +@app.task(bind=True) +def run_command(self, results, name, targets, opts=None): + if opts is None: + opts = {}🧰 Tools
🪛 Ruff (0.11.9)
170-170: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
secator/runners/_base.py (1)
128-128: Fix duplicated condition in print_start initializationThere's a duplicated condition (
not self.piped_output) in the print_start initialization.-self.print_start = self.run_opts.get('print_start', False) and not self.piped_output and not self.piped_output and not self.dry_run +self.print_start = self.run_opts.get('print_start', False) and not self.piped_input and not self.piped_output and not self.dry_run
🧹 Nitpick comments (1)
secator/runners/_base.py (1)
308-311: Good addition of ID propertyThe new
idproperty centralizes the logic for extracting task/workflow/scan IDs from the context, which is used for tagging debug messages.Consider handling the case when no ID is found in the context more explicitly:
@property def id(self): - return self.context.get('task_id', '') or self.context.get('workflow_id', '') or self.context.get('scan_id', '') + id_value = self.context.get('task_id', '') or self.context.get('workflow_id', '') or self 10000 .context.get('scan_id', '') + return id_value if id_value else f"runner_{self.unique_name}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
secator/celery.py(4 hunks)secator/runners/_base.py(34 hunks)secator/tasks/nmap.py(1 hunks)secator/tree.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- secator/tree.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
secator/runners/_base.py (13)
secator/tree.py (1)
build_runner_tree(73-115)secator/utils.py (3)
debug(428-465)rich_to_ansi(376-393)should_update(659-676)secator/output_types/target.py (1)
Target(9-30)secator/output_types/_base.py (4)
keys(101-102)OutputType(11-108)toDict(104-108)load(61-94)secator/output_types/error.py (2)
Error(8-43)from_exception(23-30)secator/runners/celery.py (1)
yielder(6-15)secator/runners/_helpers.py (1)
run_extractors(7-47)secator/runners/scan.py (1)
build_celery_workflow(23-75)secator/runners/task.py (1)
build_celery_workflow(17-65)secator/runners/workflow.py (1)
build_celery_workflow(26-81)secator/output_types/info.py (1)
Info(8-24)secator/tasks/bbot.py (1)
output_discriminator(172-179)secator/output_types/warning.py (1)
Warning(8-24)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (10)
secator/tasks/nmap.py (1)
415-415: Simplified vulnerability taggingThe change to remove
vuln_idfrom the tags list simplifies the tagging structure while maintaining all necessary information, as the vulnerability ID remains accessible through theIDfield (line 411). This refinement aligns with the broader workflow enhancements in the PR.secator/celery.py (3)
172-175: Good improvement to options management and task debuggingAdding print options and routing key information improves observability, and setting the quiet flag based on global configuration is more consistent.
The debug log with the routing key is particularly valuable for understanding task routing and troubleshooting distribution issues.
192-193: Good optimization for chunking checkCaching the result of
task.needs_chunking(sync)in a local variable and settingtask.has_childrenavoids redundant calls and improves code readability.
309-332: Great implementation of chunked results filteringThe added
chunk_byoption allows filtering results for each chunk based on a specified attribute, which is more efficient than passing all results to each chunk.The implementation properly:
- Creates a new options copy for each chunk to prevent mutation issues
- Adds chunk metadata to options
- Filters results based on chunk attributes
- Logs the chunked targets and filtered results for debugging
secator/runners/_base.py (6)
43-51: Good addition of runner name formattingThe
format_runner_namefunction improves readability by adding consistent color-coding based on runner type.This enhances the user experience by making it easier to distinguish between different types of runners in the console output.
203-214: Well-structured property accessors for option managementThe new properties provide a clean way to categorize and access different types of options:
resolved_opts: Non-null, non-print, non-extractor optionsresolved_print_opts: Only print options that are enableddynamic_opts: Only extractor options (ending with '_')This organization makes the code more maintainable and supports the improved option handling mentioned in the PR objectives.
634-657: Improved hook execution with better debug contextThe updated
run_hooksmethod now includes asubparameter for debug context, which enhances traceability of hook execution in logs.The additional debug messages provide valuable insights into hook execution, including:
- When hooks are skipped due to frequency limitations
- When hooks are disabled or no_process is set
- When hooks succeed or fail
This is particularly useful for understanding the complex lifecycle of runners.
773-789: Enhanced start logging with visual tree representationThe completely rewritten
log_startmethod now includes tree visualization of tasks and workflows, providing a much clearer picture of the execution plan.The use of the
build_runner_treefunction to render a hierarchical view of the runner configuration is an excellent usability improvement, making complex workflows more understandable.
1027-1054: Improved profile resolution with optional printingThe updated
resolve_profilesmethod now supports the newprint_profilesoption, providing better visibility into which profiles are being applied.This change enhances usability by making it clearer to users which profiles are active and what their descriptions are.
382-392: Enhanced result filtering with context-aware extractionThe
filter_resultsmethod now passes aDotMapcontext torun_extractors, enabling more sophisticated extraction based on context.Using
DotMapallows for dot notation access to nested structures, which makes the code more readable and less error-prone when working with complex contexts.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
secator/utils.py (1)
527-529: Fixed domain validation consistencyNow the function validates and returns the same property (
top_domain_under_public_suffix) instead of validating one property and returning another. This prevents potential security issues where an unvalidated domain might be returned.secator/decorators.py (2)
16-30: Renamed and enhanced CLI optionsRenamed
RUNNER_OPTStoCLI_OPTSfor better clarity, fixed theprocessflag to usereverse=Truefor more intuitive behavior, and added a newtreeflag to visualize runner hierarchies. The help text for theprocessflag now correctly describes that it enables processing (with--no-processdisabling it).
101-127: Replaced eval with explicit type mappingReplaced the use of
evalfor type conversion with a safer explicit mapping approach, eliminating a potential security risk while maintaining the same functionality.
🧹 Nitpick comments (2)
secator/utils.py (1)
430-431: Simplify DEBUG check conditionThe condition uses
not DEBUG == ['all']andnot DEBUG == ['1']which could be simplified toDEBUG != ['all']andDEBUG != ['1']for better readability.- if not DEBUG == ['all'] and not DEBUG == ['1']: - if not DEBUG or DEBUG == [""]: + if DEBUG != ['all'] and DEBUG != ['1']: + if not DEBUG or DEBUG == [""]:🧰 Tools
🪛 Ruff (0.11.9)
430-430: Use
DEBUG != ['all']instead ofnot DEBUG == ['all']Replace with
!=operator(SIM201)
430-430: Use
DEBUG != ['1']instead ofnot DEBUG == ['1']Replace with
!=operator(SIM201)
secator/decorators.py (1)
91-100: TODO comment for future refactoringThere's a TODO comment suggesting refactoring the
get_command_optionsfunction to userunner.supported_optsinstead. Consider creating a follow-up task to implement this refactoring once the current changes are stable.Would you like me to create a separate issue to track this future refactoring task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
secator/celery.py(4 hunks)secator/cli.py(26 hunks)secator/config.py(6 hunks)secator/decorators.py(12 hunks)secator/output_types/error.py(1 hunks)secator/output_types/url.py(3 hunks)secator/runners/_base.py(34 hunks)secator/tasks/katana.py(3 hunks)secator/utils.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- secator/output_types/url.py
- secator/tasks/katana.py
- secator/celery.py
- secator/runners/_base.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
secator/decorators.py (9)
secator/runners/task.py (1)
Task(8-80)secator/tree.py (2)
build_runner_tree(73-115)render_tree(44-54)secator/utils.py (3)
expand_input(63-108)get_command_category(263-274)debug(428-465)tests/performance/loadtester.py (1)
cli(51-74)secator/output_types/_base.py (2)
keys(101-102)toDict(104-108)secator/click.py (2)
decorator(31-55)decorator(61-83)secator/runners/_base.py (2)
debug(484-501)toDict(605-632)secator/template.py (2)
flat_tasks(57-59)_extract_workflows(130-137)secator/config.py (1)
get(207-230)
🪛 Ruff (0.11.9)
secator/utils.py
430-430: Use DEBUG != ['all'] instead of not DEBUG == ['all']
Replace with != operator
(SIM201)
430-430: Use DEBUG != ['1'] instead of not DEBUG == ['1']
Replace with != operator
(SIM201)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (58)
secator/output_types/error.py (2)
29-29: Improved error message formattingThis change ensures consistent and proper escaping of error messages when creating error objects from exceptions. By applying the rich text escape function
_s()at the creation point, the message is correctly formatted for display.
36-36: Enhanced error display formatThe direct use of the message string in the repr output without additional escaping prevents double-escaping issues, resulting in cleaner and more consistent error presentation in the console.
secator/utils.py (5)
29-29: Updated import to use new DEBUG structureThe import has been updated to use
DEBUGinstead ofDEBUG_COMPONENT, aligning with the broader refactoring of debug configuration to use string-based flags.
422-422: Improved visual distinction in debug outputThe styling for dictionary keys and values has been updated from dim cyan/yellow/green to bold blue/yellow/blue, providing better visual distinction and readability in debug output.
435-436: Updated debug component checkingThe code now iterates over the elements in the
DEBUGlist to check if the component should be logged, replacing the previousDEBUG_COMPONENTapproach. This provides more flexible and granular control over debug output.
464-464: Consistent debug flag checkingException handling in the debug function now checks for
'rich'inDEBUGrather than using the removedDEBUG_COMPONENT, maintaining the same behavior while aligning with the new debug configuration approach.
807-812: Simplified wordlist processing logicThe
process_wordlistfunction has been simplified by removing the explicit existence check and always callingdownload_filewith the given parameters. This leverages the improveddownload_filefunction insecator/config.pythat now handles local file security checks and URL validation more robustly.secator/config.py (11)
7-9: Added validators and switched to shutilAdded
validatorspackage for URL validation and switched fromos.symlinktoshutilfor more robust file operations. This improves security by properly validating URLs before download attempts.
75-76: Added worker command verbosity controlAdded a new configuration option to control worker command output verbosity, providing better flexibility for debugging and production environments.
82-84: Enhanced HTTP response control optionsAdded new configuration options to control HTTP response header visibility and filtering, giving users more granular control over output verbosity and sensitive information display.
176-176: Simplified debug configurationChanged the
debugattribute inSecatorConfigfrom a complex object to a simple string, supporting the broader refactoring of debug configuration to use string-based flags that can be parsed into a list of components.
537-537: Using structured output typesNow importing
InfoandErrorfromsecator.output_typesto provide consistent, structured output for download operations, improving error handling and user feedback.
546-547: Improved Git clone error handlingEnhanced error reporting for Git clone operations using the structured
Errortype, providing more consistent and visually clear error messages to users.Also applies to: 554-556
565-569: Added security validation for local file accessAdded a security check that prevents access to files outside the data directory when
security.allow_local_file_accessis disabled. This is an important security enhancement to prevent potential directory traversal vulnerabilities.
570-576: Changed from symlinking to moving filesReplaced the symlink operation with a file move operation, which is more secure and portable across different operating systems. This also uses structured
Infooutput for consistent user feedback.
577-599: Enhanced URL download with better error handlingImproved the URL download logic with proper validation, existence checking, and structured error handling. The function now skips downloads for existing files and provides clear feedback using
Infoobjects.
602-603: Improved invalid resource error reportingAdded structured error reporting for invalid resources (not a valid git repository, URL, or local path), providing clearer feedback to users about the nature of the error.
640-641: Updated debug condition checkUpdated the debug print condition to check for the substring 'config' in the debug string instead of comparing with a boolean, consistent with the new string-based debug flag approach.
secator/decorators.py (15)
9-10: Imported refined CLI componentsNow importing
CLICK_LISTfrom the dedicatedsecator.clickmodule andbuild_runner_treefromsecator.tree, reflecting a better code organization where CLI-related functionality is consolidated in dedicated modules.
33-35: Renamed global options and improved execution controlsRenamed
RUNNER_GLOBAL_OPTStoCLI_GLOBAL_OPTSfor clarity and replacedworkerwithsyncflag, using theoppositeparameter to maintain backward compatibility. This makes the execution mode selection more intuitive.
42-47: Added task decoratorIntroduced a new
taskdecorator to mark classes as tasks, providing a cleaner and more explicit way to identify task classes in the codebase.
49-89: Improved command option decorationCompletely rewrote the
decorate_command_optionsfunction with comprehensive handling for reverse flags and opposites. The function now correctly processes option configurations and generates appropriate Click option decorators with proper help text and flag combinations.
110-120: Enhanced options gathering from child workflowsAdded logic to collect options from child workflows (for scans), preserving the workflow name as metadata. This ensures that all relevant options are available at the CLI level and properly attributed to their source.
130-131: Unified option collectionUpdated the options collection logic to use the renamed option dictionaries (
CLI_OPTSandCLI_GLOBAL_OPTS), ensuring consistent option handling across the application.
147-152: Updated option support checksModified the option support checking to use the renamed option dictionaries, maintaining the same functionality with the new naming convention.
164-169: Improved option prefix determinationUpdated the prefix determination logic to handle the renamed option dictionaries, ensuring options are correctly categorized in the CLI help output.
172-173: Simplified option value retrievalSimplified the retrieval of option values from the YAML configuration, making the code more readable while maintaining the same functionality.
186-188: Fixed required option handlingModified the handling of required options to properly set
required=Falsewhen the option is defined in the configuration, preventing unnecessary prompts for values that are already provided.
202-204: Improved boolean option handlingEnhanced the handling of boolean options by adding reverse flags, providing users with more intuitive CLI options (e.g.,
--flag/--no-flag).
300-301: Added tree visualizationAdded support for the new
treeflag that allows users to visualize the runner hierarchy using thebuild_runner_treefunction. This provides a helpful overview of task relationships and execution flow.Also applies to: 322-326
339-339: Improved driver trackingAdded tracking of actual drivers used, which improves reporting and debugging capabilities by maintaining a list of active drivers.
Also applies to: 351-351
362-370: Enhanced synchronous mode logicUpdated the logic for enabling synchronous mode to use the
syncflag, improving clarity and consistency with the new CLI option naming.
387-390: Added print control optionsAdded new print control options (
print_profiles,print_start,print_end) to the run options, providing finer-grained control over output verbosity and presentation.secator/cli.py (25)
19-21: Improved module organization with better separation of concernsThe import changes reflect a good architecture improvement, separating Click-related functionality into a dedicated module (
secator.click) instead of keeping it indecorators. This aligns with the PR objective of code reorganization for better structure.
73-75: Improved CLI usability with consistent aliasesAdding aliases (
'x', 't', 'tasks') to the task group improves usability by providing both short and descriptive alternatives. This change aligns with the PR objective of enhancing CLI command organization.
90-92: Consistent use of OrderedGroup with aliasesUsing the new
OrderedGroupclass for the workflow group with aliases follows the same pattern as the task group, maintaining consistency across the CLI interface.
106-108: CLI command consistency maintained for scan groupThe scan group follows the same pattern as the task and workflow groups, using OrderedGroup and adding an alias, ensuring a consistent user experience.
208-208: Improved error handling with explicit exit codesReplacing
returnwithsys.exit(1)for error cases ensures the CLI properly signals failures to the calling process or shell, which is a best practice for command-line tools.Also applies to: 228-228, 237-237, 1129-1129, 1309-1309
303-308: Enhanced UX with automatic payload downloadThe changes to the
servecommand now check if the payload directory is empty and automatically download payload files, improving the user experience by eliminating manual steps.
540-540: Consistent command naming with workspace aliasesAdding aliases (
'ws', 'workspaces') to the workspace group follows the same pattern as other command groups, maintaining CLI consistency.
573-594: Added profiles management functionalityThe new
profilecommand group with itslistcommand enhances the CLI by providing a way to view available profiles with their descriptions and options, which aligns with the PR objective of improving configuration handling.
596-687: Added alias management functionalityThe new
aliascommand group with enable, disable, and list commands provides a comprehensive way to manage shell aliases for Secator commands, improving the overall CLI usability as mentioned in the PR objectives.
694-694: Consistent report command group with aliasesAdding aliases (
'r', 'reports') to the report group maintains the CLI consistency pattern established with other command groups.
700-759: Improved query processing with better validationThe new
process_queryfunction provides structured query parsing and validation, preventing mixed operators and handling field filtering effectively. This implementation addresses the previous review comment about operator inference.The error handling for mixed operators (&&, ||) in the same query is now explicit, improving the user experience by providing clear error messages instead of silent fallbacks.
700-702: Fixed mutable default argument issueThe implementation correctly addresses the previous review comment about mutable default arguments by using
Noneas the default and initializingfieldsto an empty list inside the function.
711-715: Avoid shadowing built-in type nameUsing
_typeinstead oftypeprevents shadowing the built-in Pythontype()function, addressing the previous review comment.
728-736: Improved operator detection with explicit error handlingThe operator detection logic now properly checks for mixed operators and provides clear error messages, addressing the previous review comment about logical operator inference.
774-778: Enhanced CLI with piped input supportAdding support for reading report queries from piped input improves the CLI's interoperability with other command-line tools, enabling more complex automation workflows.
831-833: Improved user feedback when loading multiple reportsThe updated output formatting when loading multiple reports provides better visibility into the loading progress, enhancing the user experience especially for large report sets.
847-849: Only show report path for non-empty reportsThis change improves the output by only showing the report path information for non-empty reports, reducing noise in the output.
859-860: Better user feedback during report processingImproved progress messaging during report building provides better user feedback, especially for time-consuming operations with large result sets.
871-872: Smart CLI output based on contextThe
report_listcommand now detects if its output is being piped and adjusts the output format accordingly, printing only report paths when piped. This is a good practice for CLI tools that may be used in automation scripts.Also applies to: 886-894
924-926: Consistent error messagingUsing
Errormessage for no reports found instead of plain text maintains consistency with the error reporting style used elsewhere in the codebase.
1147-1149: Removed excessive styling from docstringsRemoving the dim styling from the
installcommand docstring improves readability while maintaining a clean CLI help output.
1417-1417: Removed excessive styling from test docstringSimilar to the install command, removing the dim styling from the test command docstring improves readability.
1483-1483: Updated test commands with better coverage trackingThe test commands now use more specific coverage data files, improving the organization of test coverage data.
Also applies to: 1514-1514
1521-1550: Added template testing functionalityThe new
templatetest command enhances the testing capabilities by specifically focusing on templates, with proper environment setup and coverage tracking, which aligns with the PR objective of improved configuration handling.
1731-1743: Enhanced coverage command with template supportThe coverage command now includes a
--template-onlyoption to generate coverage reports specifically for template tests, providing more granular control over coverage reporting.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
secator/utils.py (1)
430-431: Use!=operator for better readability.The current boolean expression uses
not x == ywhich is less readable than the equivalentx != y.- if not DEBUG == ['all'] and not DEBUG == ['1']: + if DEBUG != ['all'] and DEBUG != ['1']:🧰 Tools
🪛 Ruff (0.11.9)
430-430: Use
DEBUG != ['all']instead ofnot DEBUG == ['all']Replace with
!=operator(SIM201)
430-430: Use
DEBUG != ['1']instead ofnot DEBUG == ['1']Replace with
!=operator(SIM201)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
secator/config.py(6 hunks)secator/runners/_base.py(34 hunks)secator/tasks/bbot.py(1 hunks)secator/utils.py(5 hunks)tests/unit/test_template.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/unit/test_template.py
🧰 Additional context used
🪛 Ruff (0.11.9)
secator/utils.py
430-430: Use DEBUG != ['all'] instead of not DEBUG == ['all']
Replace with != operator
(SIM201)
430-430: Use DEBUG != ['1'] instead of not DEBUG == ['1']
Replace with != operator
(SIM201)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: integration (3.11, ubuntu-latest)
- GitHub Check: test-tasks (ubuntu-latest)
🔇 Additional comments (24)
secator/tasks/bbot.py (1)
353-353: Good improvement usingshutil.copyfileinstead ofshutil.copyThis change improves reliability when copying screenshot files. Using
shutil.copyfile()instead ofshutil.copy()ensures only the file content is copied without preserving metadata like permissions, which can help avoid potential permission issues in cross-platform environments.secator/utils.py (5)
29-29: Import update for unified debug handling.The change from
DEBUG_COMPONENTtoDEBUGaligns with the PR's objective of making debug configuration string-based for finer control.
422-422: Improved debug output formatting.Changed color styling from dim cyan/yellow/green to bold blue/yellow/blue, making debug output more readable.
463-464: Updated debug configuration check.Updated to check for 'rich' in the DEBUG string list, aligning with the new debug configuration approach.
526-528: Fixed property mismatch in domain validation.This change ensures that the same property (
top_domain_under_public_suffix) is both validated and returned, eliminating a potential security/logic bug where an unvalidated domain might be returned.
806-811: Simplified wordlist processing.Removed redundant file existence check since
download_filenow handles all necessary validation and error reporting with structured output types.secator/config.py (8)
7-9: Added essential imports for enhanced functionality.
validatorsfor improved URL validationshutilfor file operations in the enhanceddownload_filefunction
75-75: Added Celery worker command verbosity control.This addition allows users to control the verbosity of Celery worker commands, supporting the CLI improvements goal of the PR.
82-84: Enhanced CLI configuration with header controls.Added options to:
- Show HTTP response headers (
show_http_response_headers)- Show command output (
show_command_output)- Exclude specific HTTP headers from display (
exclude_http_response_headers)These additions provide more fine-grained control over CLI output display.
176-176: Simplified debug configuration.Changed
debugfrom a complexDebugclass to a simple string, making it easier to customize debug levels and components through string parsing.
537-538: Added structured output types for improved error handling.Now using
InfoandErrortypes fromsecator.output_typesto provide consistent, well-formatted feedback.
564-574: Added security check for local file access.Enhanced
download_fileto validate that files accessed outside the data directory are only accessed when explicitly allowed by the security configuration, preventing potential security risks.
576-602: Improved URL validation and error handling.Enhanced
download_fileto:
- Validate URLs properly using the validators package
- Provide structured error reporting
- Skip downloads for existing files
- Handle request exceptions gracefully
These changes make the function more robust and user-friendly.
639-640: Updated debug configuration check.Changed to check if 'config' is contained in the CONFIG.debug string, aligning with the new string-based debug configuration approach.
secator/runners/_base.py (10)
4-21: Added imports for enhanced visualization and data handling.
textwrapfor indenting tree outputDotMapfor structured context databuild_runner_treefrom the new tree module for hierarchical task visualizationThese additions support the runner tree visualization feature and improved context handling.
43-51: Added helpful runner name formatting utility.The new
format_runner_namefunction provides consistent, color-coded runner names based on type (task, workflow, scan), improving readability in logs and console output.
128-130: Added print control options for enhanced output management.New options:
print_start: Controls printing of runner start informationprint_end: Controls printing of runner end informationprint_profiles: Controls printing of profile informationThese options align with the PR's goal of improving CLI output control and provide more granular control over what gets displayed.
Also applies to: 134-134
203-214: Added properties for cleaner option categorization.New properties:
resolved_opts: Filters run options to only non-None, non-print, non-dynamic optionsresolved_print_opts: Extracts print-related optionsdynamic_opts: Extracts dynamic options (ending with underscore)This change improves code organization and makes option handling more maintainable.
309-311: Added ID property for improved debug tagging.Extracts a task/workflow/scan ID from context for enhanced debug message tagging, making it easier to trace log messages to specific operations.
382-392: Enhanced filter_results with context-aware extraction.Modified to pass a
DotMapcontext torun_extractors, allowing for more sophisticated context-aware extraction logic that can leverage run options and targets.
634-635: Improved hook and validator function signatures.Added the
subparameter torun_hooksandrun_validatorsto provide consistent debug context information, making debugging more structured and informative.Also applies to: 672-673
774-798: Enhanced runner start/end logging with tree visualization.The updated logging now:
- Checks for print control flags
- Renders a hierarchical tree of the runner's structure for non-task runners
- Uses consistent formatting with the new
format_runner_namefunction- Provides more context through formatted messages
This aligns with the PR's goal of improved command organization and visibility.
1027-1053: Improved profile resolution with optional output.Enhanced
resolve_profilesto:
- Return the loaded profiles (instead of None)
- Only print profile information when
print_profilesis enabled- Use consistent formatting for profile messages
This improves the function's utility while maintaining control over output verbosity.
118-118:❓ Verification inconclusive
Changed process flag from opt-out to opt-in.
The initialization of
no_processhas been inverted to use the negation ofprocess, changing from an opt-out to an opt-in approach.Verify that all callers set the
processparameter correctly when needed, as this change could affect existing code that relies on the default behavior.
🏁 Script executed:
#!/bin/bash # Search for instances where run_opts is created and check if 'process' is set echo "Searching for run_opts assignments that might need updating..." rg -A 5 "run_opts\s*=\s*\{" --glob "*.py" | grep -i -E "process|no_process"Length of output: 330
🏁 Script executed:
#!/bin/bash echo "=== Inspecting BaseRunner class and its run method ===" rg -n "class BaseRunner" -A 200 secator/runners/_base.py echo -e "\n=== Showing run() method in BaseRunner ===" rg -n "def run" -A 50 secator/runners/_base.py echo -e "\n=== Context around self.no_process assignment ===" rg -n "self.no_process" -A 10 secator/runners/_base.py echo -e "\n=== Inspecting _process_opts implementation ===" rg -n "def _process_opts" -A 200 . echo -e "\n=== Checking CLI definitions for 'process' flag ===" rg -n "add_argument.*process" -A 5 .Length of output: 22799
Verify that all callers set
processcorrectlyWe’ve inverted the
no_processflag insecator/runners/_base.py(line 118):self.no_process = not self.run_opts.get('process', True)This means:
- By default
process=True→no_process=False(processing enabled).- Callers now must pass
process=Falseto disable processing.We did not find any existing use of a
processkey inrun_opts, nor a corresponding CLI flag. Please confirm that:
- Your CLI definitions (argparse/click) expose
--process/--no-processand populaterun_opts['process'].- Any direct instantiations of runners that used to rely on the old default now explicitly pass
process=Falsewhen needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
secator/cli.py (2)
303-308: Nice enhancement to handle empty payload directoriesThe automatic download of payload files when the directory is empty improves user experience by eliminating manual steps. Consider adding a check if the directory exists before listing its contents to prevent errors.
fnames = list(os.listdir(directory)) + if not os.path.exists(directory): + os.makedirs(directory, exist_ok=True) + console.print(Warning(message=f'Creating payload directory {directory}.')) + fnames = [] if not fnames: console.print(Warning(message=f'No payloads found in {directory}.')) download_files(CONFIG.payloads.templates, CONFIG.dirs.payloads, CONFIG.offline_mode, 'payload') fnames = list(os.listdir(directory))
727-736: Improved operator detection logicThe operator detection logic now properly handles mixed operators and provides better error messages, addressing the issues identified in previous reviews. This prevents potential confusion and unexpected behavior.
# Get operator operator = '||' if '&&' in query and '||' in query: - console.print(Error(message='Cannot mix && and || in the same query')) - sys.exit(1) + console.print(Error(message='Cannot mix && and || in the same query')) + sys.exit(1) elif '&&' in query: - operator = '&&' + operator = '&&' elif '||' in query: - operator = '||' + operator = '||'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
secator/cli.py(26 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (12)
secator/cli.py (12)
19-21: Good module organization improvementThe imports have been updated to reflect the code restructuring, particularly importing
OrderedGroupfrom the dedicatedsecator.clickmodule and addingdownload_filesfrom config. This aligns with the PR objective of extracting Click-related functionality into a separate module.
73-75: Well-implemented command aliasingAdding consistent aliases for the task group (
x,t,tasks) improves the CLI usability by providing shorter command alternatives while maintaining the descriptive primary command name.
90-92: Good use of OrderedGroup for workflow commandsUsing the
OrderedGroupclass for the workflow group with appropriate aliases ensures commands appear in a logical order in help text while maintaining consistent aliasing patterns across the CLI.
573-594: Good addition of profiles command groupThe new
profilegroup with aliases improves command organization and provides a clear way to list and manage profiles, which enhances the CLI's usability. The table formatting for displaying profiles is also well-done.
596-687: Excellent alias management implementationThe new
aliasgroup withenable,disable, andlistcommands provides a powerful way to manage shell aliases for Secator commands. The implementation is thorough, including helpful instructions for users on how to use the generated alias files.
700-720: Fixed mutable default argument issueThe rewritten
process_queryfunction properly initializes the default value forfieldsparameter, addressing the potential issues with mutable default arguments that were flagged in previous reviews.
771-787: Improved report query handling with piped inputThe enhancement to handle piped input for report queries and the improved formatting of the query summary provide a more flexible and user-friendly interface for report filtering and extraction.
802-804: Enhanced report path handlingThe improved report query handling supports comma-separated queries and fuzzy matching, making it easier for users to work with multiple reports or locate reports without specifying exact paths.
831-833: Improved progress indicators for report processingThe added progress indicators provide better feedback during report loading and processing, especially helpful for large reports or when processing multiple reports.
Also applies to: 847-849, 859-860
887-894: Good support for piped output in report listingAdding support for piped output enhances the CLI's flexibility and enables integration with other tools in command pipelines, which is a valuable feature for a CLI tool.
1521-1550: Well-implemented template test commandThe addition of a template test command enhances the testing capabilities and provides a dedicated way to run integration tests on templates, which is valuable for template development and validation.
1731-1743: Improved coverage command with template supportThe enhanced coverage command with template-only option provides more flexibility for test coverage analysis, which is important for comprehensive testing of the codebase.
🤖 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
New Features
Improvements
Bug Fixes
Documentation
Chores
Tests