From c09a1a449ad5cd03bcc566689c7005ba4a9d67ba Mon Sep 17 00:00:00 2001 From: Alex Kenion <alexk@wordfence.com> Date: Mon, 30 Oct 2023 14:15:37 -0400 Subject: [PATCH 1/7] Allowed scans to continue when IO errors occur by default --- wordfence/cli/malwarescan/definition.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wordfence/cli/malwarescan/definition.py b/wordfence/cli/malwarescan/definition.py index 90dd250f..d63393a2 100644 --- a/wordfence/cli/malwarescan/definition.py +++ b/wordfence/cli/malwarescan/definition.py @@ -115,7 +115,7 @@ "be skipped and a warning will be logged.", "context": "ALL", "argument_type": "FLAG", - "default": False + "default": True }, "chunk-size": { "short_name": "z", From 80876ca9a660a7e69e2fc5304e5833c1ae9ff465 Mon Sep 17 00:00:00 2001 From: Alex Kenion <alexk@wordfence.com> Date: Mon, 30 Oct 2023 14:18:32 -0400 Subject: [PATCH 2/7] Fixed report headers --- wordfence/cli/reporting.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wordfence/cli/reporting.py b/wordfence/cli/reporting.py index 831041ce..187c7491 100644 --- a/wordfence/cli/reporting.py +++ b/wordfence/cli/reporting.py @@ -198,9 +198,10 @@ def _write_row(self, data: List[str], record: ReportRecord): def _write_headers(self) -> None: if self.headers_written or not self.write_headers: return + headers = [column.header for column in self.columns] for writer in self.writers: if writer.allows_headers(): - writer.write_row(self.columns) + writer.write_row(headers) self.headers_written = True def _format_record(self, record: ReportRecord) -> List[str]: From 37eaf62e43909b89dbd995ccd14184fed814aee8 Mon Sep 17 00:00:00 2001 From: Alex Kenion <alexk@wordfence.com> Date: Mon, 30 Oct 2023 14:54:42 -0400 Subject: [PATCH 3/7] Adjusted requirement of paths for vuln-scan subcommand --- wordfence/cli/context.py | 9 +++++ wordfence/cli/vulnscan/definition.py | 8 ++++ .../vulnscan}/exceptions.py | 0 wordfence/cli/vulnscan/vulnscan.py | 40 ++++++++++++++----- wordfence/util/terminal.py | 2 +- 5 files changed, 48 insertions(+), 11 deletions(-) rename wordfence/{vulnscanning => cli/vulnscan}/exceptions.py (100%) diff --git a/wordfence/cli/context.py b/wordfence/cli/context.py index 6d47c1d3..d5fefe3a 100644 --- a/wordfence/cli/context.py +++ b/wordfence/cli/context.py @@ -1,3 +1,4 @@ +import sys from typing import Optional, Any from ..version import __version__, __version_name__ @@ -74,3 +75,11 @@ def display_version(self) -> None: f"PCRE Version: {pcre.VERSION} - " f"JIT Supported: {jit_support_text}" ) + + def has_terminal_output(self) -> bool: + return sys.stdout is not None \ + and sys.stdout.isatty() + + def has_terminal_input(self) -> bool: + return sys.stdin is not None \ + and sys.stdin.isatty() diff --git a/wordfence/cli/vulnscan/definition.py b/wordfence/cli/vulnscan/definition.py index 16f875bc..623d514c 100644 --- a/wordfence/cli/vulnscan/definition.py +++ b/wordfence/cli/vulnscan/definition.py @@ -135,6 +135,14 @@ variant.path for variant in VulnerabilityFeedVariant ] } + }, + "require-path": { + "description": "When enabled, an error will be issued if at least one " + "path to scan is not specified. This is the default " + "behavior when running in a terminal.", + "context": "CLI", + "argument_type": "OPTIONAL_FLAG", + "default": None } } diff --git a/wordfence/vulnscanning/exceptions.py b/wordfence/cli/vulnscan/exceptions.py similarity index 100% rename from wordfence/vulnscanning/exceptions.py rename to wordfence/cli/vulnscan/exceptions.py diff --git a/wordfence/cli/vulnscan/vulnscan.py b/wordfence/cli/vulnscan/vulnscan.py index 3afa8d5b..099289e2 100644 --- a/wordfence/cli/vulnscan/vulnscan.py +++ b/wordfence/cli/vulnscan/vulnscan.py @@ -11,7 +11,7 @@ from ...logging import log from ..subcommands import Subcommand from .reporting import VulnScanReportManager -from ...vulnscanning.exceptions import VulnScanningConfigurationException +from .exceptions import VulnScanningConfigurationException class VulnScanSubcommand(Subcommand): @@ -157,14 +157,33 @@ def _scan_site( structure_options=structure_options ) - def invoke(self) -> int: - if (len(self.config.trailing_arguments) + + def _requires_paths(self) -> bool: + required = self.config.require_path + return ( + required is True or + (required is None and self.context.has_terminal_input) + ) + + def _check_required_paths(self) -> bool: + if not self._requires_paths(): + return True + return (len(self.config.trailing_arguments) + len(self.config.wordpress_path) + len(self.config.plugin_directory) + - len(self.config.theme_directory)) < 1: - raise VulnScanningConfigurationException( - 'At least one WordPress path must be specified' - ) + len(self.config.theme_directory)) > 0 + + def _raise_path_error(self) -> None: + raise VulnScanningConfigurationException( + 'At least one WordPress path must be specified' + ) + + def invoke(self) -> int: + feed_variant = VulnerabilityFeedVariant.for_path(self.config.feed) + report_manager = VulnScanReportManager(self.config, feed_variant) + io_manager = report_manager.get_io_manager() + if not io_manager.should_read_stdin() and \ + not self._check_required_paths(): + self._raise_path_error() if self.config.output_format == 'human' \ and not self.context.allows_color: log.warning( @@ -172,9 +191,6 @@ def invoke(self) -> int: 'support to function properly. See --output-format for ' 'other options.' ) - feed_variant = VulnerabilityFeedVariant.for_path(self.config.feed) - report_manager = VulnScanReportManager(self.config, feed_variant) - io_manager = report_manager.get_io_manager() vulnerability_index = self._load_vulnerability_index(feed_variant) vulnerability_filter = self._initialize_filter(feed_variant) for invalid_id in vulnerability_filter.get_invalid_ids( @@ -204,12 +220,16 @@ def invoke(self) -> int: ) if io_manager.should_read_stdin(): reader = io_manager.get_input_reader() + path_count = 0 for path in reader.read_all_entries(): self._scan_site( path, scanner, structure_options=structure_options ) + path_count += 1 + if self._requires_paths() and path_count == 0: + self._raise_path_error() for path in self.config.wordpress_path: log.info(f'Scanning core installation at {path}...') self._scan( diff --git a/wordfence/util/terminal.py b/wordfence/util/terminal.py index 58f24d09..dd904bec 100644 --- a/wordfence/util/terminal.py +++ b/wordfence/util/terminal.py @@ -4,7 +4,7 @@ def supports_colors() -> bool: # TODO: Implement more detailed checks for color support - return sys.stdout.isatty() + return sys.stdout is not None and sys.stdout.isatty() class Color(IntEnum): From 6ca316f392f39a3e309c66c2cddbe57d6140e218 Mon Sep 17 00:00:00 2001 From: Alex Kenion <alexk@wordfence.com> Date: Mon, 30 Oct 2023 16:06:33 -0400 Subject: [PATCH 4/7] Improved help messaging and added warnings around format compatibiltiy with header/column options --- wordfence/cli/config/__init__.py | 8 ++++ .../cli/config/base_config_definitions.py | 3 +- wordfence/cli/config/config.py | 4 ++ wordfence/cli/helper.py | 25 ++++++++--- wordfence/cli/malwarescan/reporting.py | 3 +- wordfence/cli/reporting.py | 45 ++++++++++++++++--- wordfence/cli/vulnscan/reporting.py | 4 +- 7 files changed, 77 insertions(+), 15 deletions(-) diff --git a/wordfence/cli/config/__init__.py b/wordfence/cli/config/__init__.py index f7541dbc..000e1df6 100644 --- a/wordfence/cli/config/__init__.py +++ b/wordfence/cli/config/__init__.py @@ -55,6 +55,12 @@ def create_config_object( # later values always replace previous values if new_value is not not_set_token: setattr(target, item_definition.property_name, new_value) + try: + target.defaulted_options.remove( + item_definition.property_name + ) + except KeyError: + pass # Ignore options that weren't previously defaulted elif not hasattr(target, item_definition.property_name): default = item_definition.default if item_definition.has_separator() and \ @@ -62,7 +68,9 @@ def create_config_object( default = default.split(item_definition.meta.separator) setattr(target, item_definition.property_name, default) + target.defaulted_options.add(item_definition.property_name) target.trailing_arguments = trailing_arguments + print(target.defaulted_options) return target diff --git a/wordfence/cli/config/base_config_definitions.py b/wordfence/cli/config/base_config_definitions.py index 57a7339f..cdf5be45 100644 --- a/wordfence/cli/config/base_config_definitions.py +++ b/wordfence/cli/config/base_config_definitions.py @@ -4,8 +4,7 @@ config_definitions = { "configuration": { "short_name": "c", - "description": "Path to a configuration INI file to use (defaults to" - f" \"{INI_DEFAULT_PATH}\").", + "description": "Path to a configuration INI file to use.", "context": "CLI", "argument_type": "OPTION", "default": INI_DEFAULT_PATH diff --git a/wordfence/cli/config/config.py b/wordfence/cli/config/config.py index 2684000f..43a1eab1 100644 --- a/wordfence/cli/config/config.py +++ b/wordfence/cli/config/config.py @@ -20,6 +20,7 @@ def __init__( self.subcommand = subcommand self.ini_path = ini_path self.trailing_arguments = None + self.defaulted_options = set() def values(self) -> Dict[str, Any]: result: Dict[str, Any] = dict() @@ -42,3 +43,6 @@ def has_ini_file(self) -> bool: def display_help(self) -> None: self._parser.print_help() print() + + def is_specified(self, option: str) -> bool: + return option not in self.defaulted_options diff --git a/wordfence/cli/helper.py b/wordfence/cli/helper.py index 21123adc..d817275c 100644 --- a/wordfence/cli/helper.py +++ b/wordfence/cli/helper.py @@ -17,6 +17,7 @@ def __init__( short_name: Optional[str], description: str, category: str, + default: str, valid_values: Optional[List[str]], is_flag: bool = False ): @@ -24,6 +25,7 @@ def __init__( self.short_name = short_name self.description = description self.category = category + self.default = default self.valid_values = valid_values self.is_flag = is_flag self.label = self.generate_label() @@ -45,12 +47,12 @@ def split_line( self, line: str, max_length: int, - offset: int = 0 + offset: int = 0, + first: bool = True ) -> List[str]: if offset > max_length: offset = 0 lines = [] - first = True while len(line) > 0: end = max_length - 1 if len(line) > max_length: @@ -59,7 +61,7 @@ def split_line( except ValueError: next_break = end else: - next_break = len(line) - 1 + next_break = len(line) next = line[:next_break] line = line[next_break + 1:] if not first: @@ -79,13 +81,20 @@ def join_lines( final_lines = [] max_length = self.terminal_size.columns for input_line in lines: + initial = True for real_line in input_line.splitlines(): - if len(real_line) > max_length: + if len(real_line) > max_length or not initial: final_lines.extend( - self.split_line(real_line, max_length, offset) + self.split_line( + real_line, + max_length, + offset, + first=initial + ) ) else: final_lines.append(real_line) + initial = False return delimiter.join(final_lines) def join_chunks( @@ -136,6 +145,7 @@ def _load_options( item.short_name, item.description, item.category, + item.default, valid_values, item.is_flag() ) @@ -171,6 +181,11 @@ def format_category( f'(use --no-{option.long_name} to disable)', offset )) + elif isinstance(option.default, str) and len(option.default): + lines.append(self._offset( + f'(default: {option.default})', + offset + )) return self.line_formatter.join_lines(lines, offset=offset) def format_options(self) -> str: diff --git a/wordfence/cli/malwarescan/reporting.py b/wordfence/cli/malwarescan/reporting.py index 08c8dff1..611baf2f 100644 --- a/wordfence/cli/malwarescan/reporting.py +++ b/wordfence/cli/malwarescan/reporting.py @@ -66,7 +66,8 @@ def allows_headers(self) -> bool: REPORT_FORMAT_HUMAN = ReportFormat( 'human', - lambda stream, columns: HumanReadableWriter(stream, columns) + lambda stream, columns: HumanReadableWriter(stream, columns), + allows_headers=False ) diff --git a/wordfence/cli/reporting.py b/wordfence/cli/reporting.py index 187c7491..907c4e6b 100644 --- a/wordfence/cli/reporting.py +++ b/wordfence/cli/reporting.py @@ -5,6 +5,7 @@ from enum import Enum from contextlib import nullcontext +from wordfence.logging import log from .config import Config from .io import IoManager @@ -67,6 +68,9 @@ def write_row(self, data: List[str]): def allows_headers(self) -> bool: return True + def allows_column_customization(self) -> bool: + return True + class CsvReportWriter(ReportWriter): @@ -106,6 +110,9 @@ class RowlessWriter(ReportWriter): def allows_headers(self) -> bool: return False + def allows_column_customization(self) -> bool: + return False + def write_row(self, data: List[str]) -> None: pass @@ -118,10 +125,14 @@ class ReportFormat: def __init__( self, option: str, - initializer: Callable[[IO, List[ReportColumn]], ReportWriter] + initializer: Callable[[IO, List[ReportColumn]], ReportWriter], + allows_headers: bool = True, + allows_column_customization: bool = True ): self.option = option self.initializer = initializer + self.allows_headers = allows_headers + self.allows_column_customization = allows_column_customization def initialize_writer( self, @@ -130,9 +141,6 @@ def initialize_writer( ) -> ReportWriter: return self.initializer(stream, columns) - def get_valid_options() -> List[str]: - return [format.value for format in ReportFormat] - REPORT_FORMAT_CSV = ReportFormat( 'csv', @@ -183,9 +191,21 @@ def __init__( self.write_headers = write_headers self.headers_written = False self.writers = [] + self.has_custom_columns = False def add_target(self, stream: IO) -> None: writer = self.format.initialize_writer(stream, self.columns) + if self.write_headers and not writer.allows_headers(): + log.warning( + 'Headers are not supported when using the ' + f'{self.format.option} output format' + ) + if self.has_custom_columns \ + and not writer.allows_column_customization(): + log.warning( + 'Columns cannot be specified when using the ' + f'{self.format.option} output format' + ) self.writers.append(writer) def _write_row(self, data: List[str], record: ReportRecord): @@ -232,6 +252,15 @@ def get_config_options( default_columns: List[ReportColumnEnum], default_format: str = 'csv' ) -> Dict[str, Dict[str, Any]]: + header_formats = [] + column_formats = [] + for format in formats: + if format.value.allows_headers: + header_formats.append(format.value.option) + if format.value.allows_column_customization: + column_formats.append(format.value.option) + header_format_string = ', '.join(header_formats) + column_format_string = ', '.join(column_formats) return { "output": { "description": "Write results to stdout. This is the default " @@ -252,7 +281,9 @@ def get_config_options( "output-columns": { "description": ("An ordered, comma-delimited list of columns to" " include in the output. Available columns: " - + columns.get_options_as_string()), + + columns.get_options_as_string() + + f"\nCompatible formats: {column_format_string}" + ), "context": "ALL", "argument_type": "OPTION", "default": ','.join([column.header for column in default_columns]), @@ -274,7 +305,8 @@ def get_config_options( }, "output-headers": { "description": "Whether or not to include column headers in " - "output", + "output.\n" + f"Compatible formats: {header_format_string}", "context": "ALL", "argument_type": "FLAG", "default": None, @@ -357,6 +389,7 @@ def initialize_report(self, output_file: Optional[IO] = None) -> Report: columns, self.config.output_headers ) + report.has_custom_columns = self.config.is_specified('output_columns') self._add_targets(report, output_file) if not report.has_writers(): raise ReportingException( diff --git a/wordfence/cli/vulnscan/reporting.py b/wordfence/cli/vulnscan/reporting.py index ed6266d8..f7e3d5f9 100644 --- a/wordfence/cli/vulnscan/reporting.py +++ b/wordfence/cli/vulnscan/reporting.py @@ -109,7 +109,9 @@ def write_record(self, record) -> None: REPORT_FORMAT_HUMAN = ReportFormat( 'human', - lambda stream, columns: HumanReadableWriter(stream) + lambda stream, columns: HumanReadableWriter(stream), + allows_headers=False, + allows_column_customization=False ) From e3470e29b7e14d80b015677b87a5a1acfbb5a7e8 Mon Sep 17 00:00:00 2001 From: Alex Kenion <alexk@wordfence.com> Date: Mon, 30 Oct 2023 16:11:09 -0400 Subject: [PATCH 5/7] Updated messaging around terminal size --- wordfence/cli/malwarescan/malwarescan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wordfence/cli/malwarescan/malwarescan.py b/wordfence/cli/malwarescan/malwarescan.py index 41f02da4..48adab50 100644 --- a/wordfence/cli/malwarescan/malwarescan.py +++ b/wordfence/cli/malwarescan/malwarescan.py @@ -204,8 +204,8 @@ def generate_exception_message( ) -> Optional[str]: if isinstance(exception, ProgressException): return ( - 'The current terminal size is inadequate for ' - 'displaying progress output for the current scan ' + 'The current terminal is too small to ' + 'display progress output with the current scan ' 'options' ) return super().generate_exception_message(exception) From 257e31ea8cea5e93ab868637194530246c53d3b5 Mon Sep 17 00:00:00 2001 From: Alex Kenion <alexk@wordfence.com> Date: Mon, 30 Oct 2023 16:33:23 -0400 Subject: [PATCH 6/7] Differential terms update prompt for paid and free licenses --- wordfence/api/noc1.py | 12 ++++++++---- wordfence/cli/config/__init__.py | 1 - wordfence/cli/terms.py | 12 ++++++++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/wordfence/api/noc1.py b/wordfence/api/noc1.py index 4d377efa..6f43de53 100644 --- a/wordfence/api/noc1.py +++ b/wordfence/api/noc1.py @@ -23,16 +23,19 @@ def build_query(self, action: str, base_query: dict = None) -> dict: query['s'] = self._generate_site_stats() return query - def register_terms_update_hook(self, callable: Callable[[], None]) -> None: + def register_terms_update_hook( + self, + callable: Callable[[bool], None] + ) -> None: if not hasattr(self, 'terms_update_hooks'): self.terms_update_hooks = [] self.terms_update_hooks.append(callable) - def _trigger_terms_update_hooks(self) -> None: + def _trigger_terms_update_hooks(self, paid: bool = False) -> None: if not hasattr(self, 'terms_update_hooks'): return for hook in self.terms_update_hooks: - hook() + hook(paid) def validate_response(self, response, validator: Validator) -> None: if isinstance(response, dict): @@ -42,7 +45,8 @@ def validate_response(self, response, validator: Validator) -> None: response['errorMsg'] ) if '_termsUpdated' in response: - self._trigger_terms_update_hooks() + paid = '_isPaidKey' in response and response['_isPaidKey'] + self._trigger_terms_update_hooks(paid) return super().validate_response(response, validator) def process_simple_request(self, action: str) -> bool: diff --git a/wordfence/cli/config/__init__.py b/wordfence/cli/config/__init__.py index 000e1df6..74f03f78 100644 --- a/wordfence/cli/config/__init__.py +++ b/wordfence/cli/config/__init__.py @@ -70,7 +70,6 @@ def create_config_object( default) target.defaulted_options.add(item_definition.property_name) target.trailing_arguments = trailing_arguments - print(target.defaulted_options) return target diff --git a/wordfence/cli/terms.py b/wordfence/cli/terms.py index 6ce71e0c..a047f4e2 100644 --- a/wordfence/cli/terms.py +++ b/wordfence/cli/terms.py @@ -18,9 +18,9 @@ def prompt_acceptance_if_needed(self): if not accepted: self.prompt_acceptance() - def trigger_update(self): + def trigger_update(self, paid: bool = False): self.context.cache.put(TERMS_CACHE_KEY, False) - self.prompt_acceptance() + self.prompt_acceptance(paid) def record_acceptance(self, remote: bool = True): if remote: @@ -28,11 +28,15 @@ def record_acceptance(self, remote: bool = True): client.record_toupp() self.context.cache.put(TERMS_CACHE_KEY, True) - def prompt_acceptance(self): + def prompt_acceptance(self, paid: bool = False): if not (sys.stdout.isatty() and sys.stdin.isatty()): return + if paid: + edition = '' + else: + edition = ' Free edition' terms_accepted = prompt_yes_no( - 'Your access to and use of Wordfence CLI Free edition is ' + f'Your access to and use of Wordfence CLI{edition} is ' 'subject to the updated Wordfence CLI License Terms and ' f'Conditions set forth at {TERMS_URL}. By entering "y" and ' 'selecting Enter, you agree that you have read and accept the ' From 5dcccf94d607491e3ed6f53eeaa6f1c943f7f94e Mon Sep 17 00:00:00 2001 From: Alex Kenion <alexk@wordfence.com> Date: Mon, 30 Oct 2023 16:33:44 -0400 Subject: [PATCH 7/7] Upgraded some debug messages to info --- wordfence/scanning/scanner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wordfence/scanning/scanner.py b/wordfence/scanning/scanner.py index d2f7247e..55f909bc 100644 --- a/wordfence/scanning/scanner.py +++ b/wordfence/scanning/scanner.py @@ -189,7 +189,7 @@ def locate(self): real_path = os.path.realpath(self.path) if os.path.isdir(real_path): for path in self.search_directory(real_path): - log.debug(f'File added to scan queue: {path}') + log.info(f'File added to scan queue: {path}') self.queue.put(path) else: if not self._is_loop(self.path): @@ -382,7 +382,7 @@ def _get_next_chunk_size(self, length: int) -> int: return min(self._scanned_content_limit - length, self._chunk_size) def _process_file(self, path: str, jit_stack: PcreJitStack): - log.debug(f'Processing file: {path}') + log.info(f'Processing file: {path}') with open(path, mode='rb') as file, \ self._matcher.create_context() as context: length = 0