From dd0885885df08c803baf21cd7ca3de2283edfe11 Mon Sep 17 00:00:00 2001 From: Bernardo Donadio Date: Sun, 2 Nov 2025 10:32:26 -0300 Subject: [PATCH 1/2] feat: add --disable-addon CLI option to selectively disable addons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ability to disable specific addons via CLI argument or YAML config. This allows users to run the proxy with only the addons they need, providing more granular control over which security headers are removed or modified. Features: - CLI option --disable-addon accepts short names (CSP, COEP, etc.) or full class names - Supports comma-separated values: --disable-addon CSP,COEP - Supports repeated usage: --disable-addon CSP --disable-addon COEP - Case-insensitive addon name matching - YAML config support via disabled_addons list - Fuzzy matching with "Did you mean?" suggestions for typos - Configuration precedence: CLI > YAML > defaults Implementation: - Added addon name mapping and validation in devrelay/addons.py - Extended ConfigLoader to parse and validate disabled_addons - Updated ProxyServer to conditionally load addons - Updated CLI to display disabled addons on startup - Comprehensive test coverage (147 tests, 100% coverage) - Updated README.md with usage examples and configuration docs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- README.md | 81 ++++++++++++++++- devrelay/addons.py | 85 +++++++++++++++++ devrelay/cli.py | 13 ++- devrelay/config.py | 84 +++++++++++++++-- devrelay/proxy.py | 24 +++-- tests/test_addons.py | 115 +++++++++++++++++++++++ tests/test_config.py | 201 +++++++++++++++++++++++++++++++++++++++++ tests/test_devrelay.py | 80 +++++++++++++--- tests/test_proxy.py | 121 +++++++++++++++++++++++++ 9 files changed, 770 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index c533c0a..175d938 100644 --- a/README.md +++ b/README.md @@ -96,15 +96,86 @@ To use the proxy, configure your browser to use it: ## CLI Options ```text -devrelay [-h] [--host HOST] [--port PORT] [--confdir CONFDIR] +devrelay [-h] [--host HOST] [--port PORT] [--certdir CERTDIR] [--disable-addon ADDON] Options: - -h, --help Show help message - --host HOST Host address to bind to (default: 127.0.0.1) - --port PORT Port to listen on (default: 8080) - --confdir CONFDIR Certificate directory (default: ~/.mitmproxy) + -h, --help Show help message + --host HOST Host address to bind to (default: 127.0.0.1) + --port PORT Port to listen on (default: 8080) + --certdir CERTDIR Certificate directory (default: ~/.mitmproxy) + --disable-addon ADDON Disable specific addon(s) (can be used multiple times) ``` +### Disabling Addons + +You can selectively disable specific addons using the `--disable-addon` option. +This is useful when you only need to remove specific security headers. + +**Available addons:** + +- `CSP` - Content-Security-Policy remover +- `COEP` - Cross-Origin-Embedder-Policy remover +- `COOP` - Cross-Origin-Opener-Policy remover +- `CORP` - Cross-Origin-Resource-Policy inserter +- `CORSInserter` - CORS headers inserter for webhooks +- `CORSPreflight` - CORS preflight handler for webhooks + +**Examples:** + +Disable CSP and COEP addons: + +```bash +devrelay --disable-addon CSP --disable-addon COEP +``` + +Disable multiple addons with comma-separated values: + +```bash +devrelay --disable-addon CSP,COEP,COOP +``` + +Combine addon disabling with other options: + +```bash +devrelay --host 0.0.0.0 --port 9090 --disable-addon CSP +``` + +You can also use full addon class names: + +```bash +devrelay --disable-addon CSPRemoverAddon --disable-addon COEPRemoverAddon +``` + +Addon names are case-insensitive: + +```bash +devrelay --disable-addon csp --disable-addon COEP +``` + +## Configuration File + +DevRelay supports configuration via a YAML file located at `~/.mitmproxy/devrelay.yaml`. +The file is automatically created with default values on first run. + +**Example configuration:** + +```yaml +host: 127.0.0.1 +port: 8080 +certdir: /home/user/.mitmproxy +disabled_addons: + - CSP + - COEP +``` + +Configuration precedence (highest to lowest): + +1. Command-line arguments +2. YAML configuration file +3. Default values + +This means CLI arguments will override values in the YAML file. + ## Development ### Available Make Targets diff --git a/devrelay/addons.py b/devrelay/addons.py index 747c6f2..0bbe46d 100644 --- a/devrelay/addons.py +++ b/devrelay/addons.py @@ -1,7 +1,92 @@ """DevRelay proxy addons for security header manipulation""" +import difflib from mitmproxy import http +# Addon name mapping for user-friendly names +# Maps both short names and full class names to canonical class names +ADDON_NAME_MAP = { + # Short names (case will be normalized to uppercase for lookup) + "CSP": "CSPRemoverAddon", + "COEP": "COEPRemoverAddon", + "COOP": "COOPRemoverAddon", + "CORP": "CORPInserterAddon", + "CORSINSERTER": "CORSInserterForWebhooksAddon", + "CORSPREFLIGHT": "CORSPreflightForWebhooksAddon", + # Full class names (for completeness) + "CSPREMOVERADDON": "CSPRemoverAddon", + "COEPREMOVERADDON": "COEPRemoverAddon", + "COOPREMOVERADDON": "COOPRemoverAddon", + "CORPINSERTERADDON": "CORPInserterAddon", + "CORSINSERTERFORWEBHOOKSADDON": "CORSInserterForWebhooksAddon", + "CORSPREFLIGHTFORWEBHOOKSADDON": "CORSPreflightForWebhooksAddon", +} + +# All valid addon class names +ALL_ADDON_NAMES = [ + "CSPRemoverAddon", + "COEPRemoverAddon", + "COOPRemoverAddon", + "CORPInserterAddon", + "CORSInserterForWebhooksAddon", + "CORSPreflightForWebhooksAddon", +] + + +def validate_addon_names(addon_names: list[str]) -> list[str]: + """ + Validate and normalize addon names to canonical class names. + + Accepts both short names (e.g., "CSP", "COEP") and full class names + (e.g., "CSPRemoverAddon"). Case-insensitive matching is supported. + + Args: + addon_names: List of addon names to validate + + Returns: + List of canonical addon class names + + Raises: + ValueError: If any addon name is invalid, with a suggestion for the first invalid name + """ + validated = [] + + for name in addon_names: + # Normalize to uppercase for case-insensitive lookup + normalized = name.upper() + + # Check if it's a valid addon name + if normalized in ADDON_NAME_MAP: + canonical_name = ADDON_NAME_MAP[normalized] + validated.append(canonical_name) + else: + # Invalid name - provide a suggestion using fuzzy matching + # Get all possible input names (both short and full) + all_possible_names = list(ADDON_NAME_MAP.keys()) + + # Find close matches (case-insensitive) + close_matches = difflib.get_close_matches(normalized, all_possible_names, n=1, cutoff=0.6) + + if close_matches: + # Suggest the canonical class name for the close match + suggested_canonical = ADDON_NAME_MAP[close_matches[0]] + # Find a user-friendly version (prefer short names) + user_friendly = None + for key, value in ADDON_NAME_MAP.items(): + if value == suggested_canonical and len(key) <= 15: # Prefer short names + user_friendly = key + break + if user_friendly is None: + user_friendly = suggested_canonical + + raise ValueError(f"Unknown addon '{name}'. Did you mean '{user_friendly}'?") + else: + # No close match - list all valid options + valid_short_names = ["CSP", "COEP", "COOP", "CORP", "CORSInserter", "CORSPreflight"] + raise ValueError(f"Unknown addon '{name}'. Valid addons: {', '.join(valid_short_names)}") + + return validated + class CSPRemoverAddon: """ diff --git a/devrelay/cli.py b/devrelay/cli.py index b12f0cc..93d3981 100644 --- a/devrelay/cli.py +++ b/devrelay/cli.py @@ -20,7 +20,7 @@ def __init__(self, config_path: Path | None = None) -> None: """ self.config_loader = ConfigLoader(config_path=config_path) - def display_startup_info(self, host: str, port: int, certdir: Path) -> None: + def display_startup_info(self, host: str, port: int, certdir: Path, disabled_addons: list[str]) -> None: """ Display startup information to the user. @@ -28,12 +28,15 @@ def display_startup_info(self, host: str, port: int, certdir: Path) -> None: host: Host address being used port: Port number being used certdir: Certificate directory path + disabled_addons: List of disabled addon names """ print(f"Starting DevRelay proxy on {host}:{port}") print(f"Certificate directory: {certdir}") + if disabled_addons: + print(f"Disabled addons: {', '.join(disabled_addons)}") print("\nPress Ctrl+C to stop the proxy\n") - def run_server(self, host: str, port: int, certdir: Path) -> int: + def run_server(self, host: str, port: int, certdir: Path, disabled_addons: list[str]) -> int: """ Start and run the proxy server. @@ -41,6 +44,7 @@ def run_server(self, host: str, port: int, certdir: Path) -> int: host: Host address to bind to port: Port number to listen on certdir: Certificate directory + disabled_addons: List of addon class names to disable Returns: Exit code (0 for success, 1 for error) @@ -50,6 +54,7 @@ def run_server(self, host: str, port: int, certdir: Path) -> int: host=host, port=port, certdir=certdir, + disabled_addons=disabled_addons, ) server.run() except KeyboardInterrupt: @@ -73,8 +78,8 @@ def execute(self, args: list[str] | None = None) -> int: """ try: config = self.config_loader.get_config(args) - self.display_startup_info(config.host, config.port, config.certdir) - return self.run_server(config.host, config.port, config.certdir) + self.display_startup_info(config.host, config.port, config.certdir, config.disabled_addons) + return self.run_server(config.host, config.port, config.certdir, config.disabled_addons) except ValueError as e: print(f"Configuration error: {e}", file=sys.stderr) return 1 diff --git a/devrelay/config.py b/devrelay/config.py index 5851ad0..bf4eab4 100644 --- a/devrelay/config.py +++ b/devrelay/config.py @@ -7,6 +7,8 @@ from ruamel.yaml import YAML +from devrelay.addons import validate_addon_names + @dataclass class Parameter: @@ -53,6 +55,12 @@ def __init__(self, config_path: Path | None = None) -> None: default=Path.home() / ".mitmproxy", help="Certificate directory", ), + Parameter( + name="disabled_addons", + type=list, + default=[], + help="Comma-separated list of addons to disable (e.g., CSP,COEP)", + ), ] self.parser = self._build_parser() @@ -70,15 +78,63 @@ def _build_parser(self) -> argparse.ArgumentParser: ) for param in self.parameters: - parser.add_argument( - f"--{param.name}", - type=param.type, - default=param.default, - help=param.help, - ) + # Special handling for list types (disabled_addons) + if param.type == list: + # Use --disable-addon (singular) for better UX + arg_name = "--disable-addon" + # action='append' allows repeated usage: --disable-addon CSP --disable-addon COEP + parser.add_argument( + arg_name, + action="append", + default=None, # Use None to detect if user provided values + help=param.help, + dest=param.name, # Store in disabled_addons + ) + else: + parser.add_argument( + f"--{param.name}", + type=param.type, + default=param.default, + help=param.help, + ) return parser + def _parse_addon_list(self, raw_value: Any) -> list[str]: + """ + Parse addon list from CLI or YAML format. + + Handles both comma-separated strings and lists. + CLI format: --disable-addon CSP,COEP or --disable-addon CSP --disable-addon COEP + YAML format: disabled_addons: [CSP, COEP] or disabled_addons: CSP,COEP + + Args: + raw_value: Raw value from CLI or YAML (None, str, or list) + + Returns: + Parsed list of addon names + """ + if raw_value is None: + return [] + + # If it's already a list (from action='append' or YAML) + if isinstance(raw_value, list): + # Flatten and split comma-separated values + result = [] + for item in raw_value: + if isinstance(item, str): + # Split by comma and strip whitespace + result.extend([x.strip() for x in item.split(",") if x.strip()]) + else: + result.append(item) + return result + + # If it's a string (from YAML), split by comma + if isinstance(raw_value, str): + return [x.strip() for x in raw_value.split(",") if x.strip()] + + return [] + def _load_yaml(self) -> dict[str, Any]: """ Load configuration from YAML file. @@ -127,6 +183,9 @@ def _validate_value(self, param: Parameter, value: Any) -> Any: ValueError: If value is invalid """ if value is None: + # For list types, return empty list instead of None + if param.type == list: + return [] return None # Type conversion @@ -147,6 +206,13 @@ def _validate_value(self, param: Parameter, value: Any) -> Any: return result elif param.type == str: return str(value) + elif param.type == list: + # Handle list types (currently only disabled_addons) + parsed_list = self._parse_addon_list(value) + # Validate addon names if this is the disabled_addons parameter + if param.name == "disabled_addons": + return validate_addon_names(parsed_list) + return parsed_list else: # pragma: no cover return value except (ValueError, TypeError): @@ -203,7 +269,11 @@ def get_config(self, args: list[str] | None = None) -> argparse.Namespace: cli_value = getattr(cli_args, param.name) # Check if CLI value is the default (meaning user didn't provide it) - is_cli_default = cli_value == param.default + # For list types, CLI default is None, so check for None explicitly + if param.type == list: + is_cli_default = cli_value is None + else: + is_cli_default = cli_value == param.default if is_cli_default and param.name in yaml_config: # Use YAML value if CLI wasn't provided diff --git a/devrelay/proxy.py b/devrelay/proxy.py index 010146d..f832433 100644 --- a/devrelay/proxy.py +++ b/devrelay/proxy.py @@ -40,6 +40,7 @@ def __init__( host: str = "127.0.0.1", port: int = 8080, certdir: Path = Path.home() / ".mitmproxy", + disabled_addons: list[str] | None = None, ) -> None: """ Initialize the proxy server. @@ -48,10 +49,12 @@ def __init__( host: Host address to bind to (default: 127.0.0.1) port: Port to listen on (default: 8080) certdir: Certificate directory (default: ~/.mitmproxy) + disabled_addons: List of addon class names to disable (default: None) """ self.host = host self.port = port self.certdir = certdir + self.disabled_addons = disabled_addons or [] async def start(self) -> None: """Start the proxy server.""" @@ -74,12 +77,21 @@ async def start(self) -> None: with_termlog=True, with_dumper=False, ) - master.addons.add(CSPRemoverAddon()) - master.addons.add(COEPRemoverAddon()) - master.addons.add(COOPRemoverAddon()) - master.addons.add(CORPInserterAddon()) - master.addons.add(CORSInserterForWebhooksAddon()) - master.addons.add(CORSPreflightForWebhooksAddon()) + + # Define all available addons + available_addons = { + "CSPRemoverAddon": CSPRemoverAddon(), + "COEPRemoverAddon": COEPRemoverAddon(), + "COOPRemoverAddon": COOPRemoverAddon(), + "CORPInserterAddon": CORPInserterAddon(), + "CORSInserterForWebhooksAddon": CORSInserterForWebhooksAddon(), + "CORSPreflightForWebhooksAddon": CORSPreflightForWebhooksAddon(), + } + + # Load only enabled addons (not in disabled list) + for addon_name, addon_instance in available_addons.items(): + if addon_name not in self.disabled_addons: + master.addons.add(addon_instance) try: await master.run() diff --git a/tests/test_addons.py b/tests/test_addons.py index 9caf5b4..e887224 100644 --- a/tests/test_addons.py +++ b/tests/test_addons.py @@ -5,6 +5,7 @@ and CORSPreflightForWebhooksAddon. """ +import pytest from mitmproxy.test import tflow from devrelay.addons import ( @@ -14,6 +15,7 @@ CORSInserterForWebhooksAddon, CORSPreflightForWebhooksAddon, CSPRemoverAddon, + validate_addon_names, ) @@ -792,3 +794,116 @@ def test_preserves_existing_headers(self) -> None: assert flow.response.headers["Cache-Control"] == "no-cache" # And CORS headers were added assert flow.response.headers["Access-Control-Allow-Origin"] == "*" + + +class TestValidateAddonNames: + """Test cases for validate_addon_names function.""" + + def test_validate_short_names(self) -> None: + """Test that short addon names are validated and normalized.""" + result = validate_addon_names(["CSP", "COEP"]) + assert result == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_validate_full_class_names(self) -> None: + """Test that full class names are validated.""" + result = validate_addon_names(["CSPRemoverAddon", "COEPRemoverAddon"]) + assert result == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_validate_case_insensitive(self) -> None: + """Test that validation is case-insensitive.""" + result = validate_addon_names(["csp", "COEP", "CsP"]) + assert result == ["CSPRemoverAddon", "COEPRemoverAddon", "CSPRemoverAddon"] + + def test_validate_all_addon_names(self) -> None: + """Test that all valid addon names can be validated.""" + result = validate_addon_names(["CSP", "COEP", "COOP", "CORP", "CORSInserter", "CORSPreflight"]) + assert result == [ + "CSPRemoverAddon", + "COEPRemoverAddon", + "COOPRemoverAddon", + "CORPInserterAddon", + "CORSInserterForWebhooksAddon", + "CORSPreflightForWebhooksAddon", + ] + + def test_validate_empty_list(self) -> None: + """Test that empty list returns empty list.""" + result = validate_addon_names([]) + assert result == [] + + def test_validate_mixed_formats(self) -> None: + """Test that mixed short and full names work together.""" + result = validate_addon_names(["CSP", "COEPRemoverAddon", "coop"]) + assert result == ["CSPRemoverAddon", "COEPRemoverAddon", "COOPRemoverAddon"] + + def test_invalid_addon_name_with_close_match(self) -> None: + """Test that invalid addon name raises ValueError with suggestion.""" + with pytest.raises(ValueError) as excinfo: + validate_addon_names(["CS"]) + assert "Unknown addon 'CS'" in str(excinfo.value) + assert "Did you mean 'CSP'?" in str(excinfo.value) + + def test_invalid_addon_name_with_typo(self) -> None: + """Test that typo in addon name suggests correct name.""" + with pytest.raises(ValueError) as excinfo: + validate_addon_names(["CEOP"]) # Typo in COEP + assert "Unknown addon 'CEOP'" in str(excinfo.value) + assert "Did you mean" in str(excinfo.value) + + def test_invalid_addon_name_no_close_match(self) -> None: + """Test that completely invalid name lists all valid options.""" + with pytest.raises(ValueError) as excinfo: + validate_addon_names(["InvalidAddon"]) + assert "Unknown addon 'InvalidAddon'" in str(excinfo.value) + assert "Valid addons:" in str(excinfo.value) + assert "CSP" in str(excinfo.value) + + def test_first_invalid_name_reported_only(self) -> None: + """Test that only the first invalid name is reported.""" + with pytest.raises(ValueError) as excinfo: + validate_addon_names(["CSP", "Invalid1", "Invalid2"]) + # Should only mention Invalid1, not Invalid2 + assert "Invalid1" in str(excinfo.value) + assert "Invalid2" not in str(excinfo.value) + + def test_validate_duplicate_names(self) -> None: + """Test that duplicate names are preserved in output.""" + result = validate_addon_names(["CSP", "CSP", "COEP"]) + assert result == ["CSPRemoverAddon", "CSPRemoverAddon", "COEPRemoverAddon"] + + def test_validate_cors_inserter_short_name(self) -> None: + """Test that CORSInserter short name works.""" + result = validate_addon_names(["CORSInserter"]) + assert result == ["CORSInserterForWebhooksAddon"] + + def test_validate_cors_preflight_short_name(self) -> None: + """Test that CORSPreflight short name works.""" + result = validate_addon_names(["CORSPreflight"]) + assert result == ["CORSPreflightForWebhooksAddon"] + + def test_invalid_addon_name_suggests_short_name_when_available(self) -> None: + """Test that suggestions prefer short names over full class names.""" + with pytest.raises(ValueError) as excinfo: + validate_addon_names(["COEP123"]) # Close to COEP but not exact + # Should suggest a short name like COEP, not the full class name + error_msg = str(excinfo.value) + assert "Unknown addon 'COEP123'" in error_msg + assert "Did you mean" in error_msg + + def test_invalid_addon_name_suggests_full_name_when_no_short_name(self) -> None: + """Test that suggestions use full class name when no short name found.""" + # Mock ADDON_NAME_MAP to have only long names (> 15 chars, no short names) + from unittest.mock import patch + + mock_map = { + "VERYLONGADDONNAMEWITHOUTSHORTVERSION": "CSPRemoverAddon", + "CSPREMOVERADDONVERYLONGNAME": "CSPRemoverAddon", # 27 chars, > 15 + } + with patch("devrelay.addons.ADDON_NAME_MAP", mock_map): + with pytest.raises(ValueError) as excinfo: + validate_addon_names(["CSPREMOVERADDONVERYLONGNAMETYPO"]) # Close to long name + error_msg = str(excinfo.value) + assert "Unknown addon 'CSPREMOVERADDONVERYLONGNAMETYPO'" in error_msg + assert "Did you mean" in error_msg + # Should suggest the full canonical name since no short name <= 15 chars exists + assert "CSPRemoverAddon" in error_msg diff --git a/tests/test_config.py b/tests/test_config.py index 475ff7a..f138a64 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -383,6 +383,7 @@ def test_parameter_definitions_match_defaults(self) -> None: assert "host" in param_names assert "port" in param_names assert "certdir" in param_names + assert "disabled_addons" in param_names # Verify defaults for param in loader.parameters: @@ -395,3 +396,203 @@ def test_parameter_definitions_match_defaults(self) -> None: elif param.name == "certdir": assert param.default == Path.home() / ".mitmproxy" assert param.type == Path + elif param.name == "disabled_addons": + assert param.default == [] + assert param.type == list + + def test_parse_addon_list_with_none(self) -> None: + """Test that parse_addon_list handles None value.""" + loader = ConfigLoader() + result = loader._parse_addon_list(None) # pyright: ignore[reportPrivateUsage] + assert result == [] + + def test_parse_addon_list_with_list(self) -> None: + """Test that parse_addon_list handles list value.""" + loader = ConfigLoader() + result = loader._parse_addon_list(["CSP", "COEP"]) # pyright: ignore[reportPrivateUsage] + assert result == ["CSP", "COEP"] + + def test_parse_addon_list_with_comma_separated_string(self) -> None: + """Test that parse_addon_list handles comma-separated string.""" + loader = ConfigLoader() + result = loader._parse_addon_list("CSP,COEP,COOP") # pyright: ignore[reportPrivateUsage] + assert result == ["CSP", "COEP", "COOP"] + + def test_parse_addon_list_with_whitespace(self) -> None: + """Test that parse_addon_list strips whitespace.""" + loader = ConfigLoader() + result = loader._parse_addon_list("CSP, COEP , COOP") # pyright: ignore[reportPrivateUsage] + assert result == ["CSP", "COEP", "COOP"] + + def test_parse_addon_list_with_mixed_format(self) -> None: + """Test that parse_addon_list handles list with comma-separated items.""" + loader = ConfigLoader() + result = loader._parse_addon_list(["CSP,COEP", "COOP"]) # pyright: ignore[reportPrivateUsage] + assert result == ["CSP", "COEP", "COOP"] + + def test_parse_addon_list_with_empty_string(self) -> None: + """Test that parse_addon_list handles empty string.""" + loader = ConfigLoader() + result = loader._parse_addon_list("") # pyright: ignore[reportPrivateUsage] + assert result == [] + + def test_parse_addon_list_filters_empty_items(self) -> None: + """Test that parse_addon_list filters out empty items.""" + loader = ConfigLoader() + result = loader._parse_addon_list("CSP,,COEP,") # pyright: ignore[reportPrivateUsage] + assert result == ["CSP", "COEP"] + + def test_validate_value_for_disabled_addons_valid(self) -> None: + """Test that validate_value validates disabled_addons correctly.""" + loader = ConfigLoader() + param = Parameter(name="disabled_addons", type=list, default=[], help="help") + + result = loader._validate_value(param, ["CSP", "COEP"]) # pyright: ignore[reportPrivateUsage] + assert result == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_validate_value_for_disabled_addons_invalid(self) -> None: + """Test that validate_value raises on invalid addon name.""" + loader = ConfigLoader() + param = Parameter(name="disabled_addons", type=list, default=[], help="help") + + with pytest.raises(ValueError, match="Unknown addon"): + loader._validate_value(param, ["InvalidAddon"]) # pyright: ignore[reportPrivateUsage] + + def test_validate_value_for_disabled_addons_none(self) -> None: + """Test that validate_value returns empty list for None.""" + loader = ConfigLoader() + param = Parameter(name="disabled_addons", type=list, default=[], help="help") + + result = loader._validate_value(param, None) # pyright: ignore[reportPrivateUsage] + assert result == [] + + def test_get_config_with_disabled_addons_from_cli(self) -> None: + """Test getting configuration with disabled_addons from CLI.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + config = loader.get_config(["--disable-addon", "CSP", "--disable-addon", "COEP"]) + + assert config.disabled_addons == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_get_config_with_disabled_addons_comma_separated(self) -> None: + """Test getting configuration with comma-separated disabled_addons.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + config = loader.get_config(["--disable-addon", "CSP,COEP"]) + + assert config.disabled_addons == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_get_config_with_disabled_addons_mixed_format(self) -> None: + """Test getting configuration with mixed CLI format.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + config = loader.get_config(["--disable-addon", "CSP,COEP", "--disable-addon", "COOP"]) + + assert config.disabled_addons == ["CSPRemoverAddon", "COEPRemoverAddon", "COOPRemoverAddon"] + + def test_get_config_with_disabled_addons_from_yaml(self) -> None: + """Test getting configuration with disabled_addons from YAML.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + # Create YAML with disabled_addons + config_path.parent.mkdir(parents=True, exist_ok=True) + with open(config_path, "w") as f: + loader.yaml.dump({"disabled_addons": ["CSP", "COEP"]}, f) + + config = loader.get_config([]) + + assert config.disabled_addons == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_get_config_with_disabled_addons_cli_overrides_yaml(self) -> None: + """Test that CLI disabled_addons override YAML.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + # Create YAML with disabled_addons + config_path.parent.mkdir(parents=True, exist_ok=True) + with open(config_path, "w") as f: + loader.yaml.dump({"disabled_addons": ["CSP"]}, f) + + config = loader.get_config(["--disable-addon", "COEP"]) + + # CLI should override YAML + assert config.disabled_addons == ["COEPRemoverAddon"] + + def test_get_config_with_disabled_addons_invalid_name(self) -> None: + """Test that invalid addon name in CLI raises error.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + with pytest.raises(ValueError, match="Unknown addon"): + loader.get_config(["--disable-addon", "InvalidAddon"]) + + def test_get_config_with_disabled_addons_case_insensitive(self) -> None: + """Test that disabled_addons are case-insensitive.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + config = loader.get_config(["--disable-addon", "csp,COEP"]) + + assert config.disabled_addons == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_get_config_with_empty_disabled_addons(self) -> None: + """Test that empty disabled_addons returns empty list.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + loader = ConfigLoader(config_path=config_path) + + config = loader.get_config([]) + + assert config.disabled_addons == [] + + def test_build_parser_includes_disable_addon_argument(self) -> None: + """Test that parser includes --disable-addon argument.""" + loader = ConfigLoader() + parser = loader.parser + + # Test that --disable-addon is accepted + args = parser.parse_args(["--disable-addon", "CSP"]) + assert args.disabled_addons == ["CSP"] + + def test_build_parser_disable_addon_allows_multiple(self) -> None: + """Test that --disable-addon can be used multiple times.""" + loader = ConfigLoader() + parser = loader.parser + + args = parser.parse_args(["--disable-addon", "CSP", "--disable-addon", "COEP"]) + assert args.disabled_addons == ["CSP", "COEP"] + + def test_parse_addon_list_with_non_string_items(self) -> None: + """Test that parse_addon_list handles non-string items in list.""" + loader = ConfigLoader() + # This tests the else branch at line 129 where item is not a string + result = loader._parse_addon_list([123, 456]) # pyright: ignore[reportPrivateUsage] + assert result == [123, 456] + + def test_parse_addon_list_with_non_standard_type(self) -> None: + """Test that parse_addon_list returns empty list for non-standard types.""" + loader = ConfigLoader() + # This tests line 136 where raw_value is neither None, list, nor string + result = loader._parse_addon_list(123) # pyright: ignore[reportPrivateUsage] + assert result == [] + + def test_validate_value_for_list_type_not_disabled_addons(self) -> None: + """Test that validate_value returns parsed list for non-disabled_addons list params.""" + loader = ConfigLoader() + # Create a parameter with type list but not named disabled_addons + param = Parameter(name="other_list", type=list, default=[], help="help") + + result = loader._validate_value(param, ["item1", "item2"]) # pyright: ignore[reportPrivateUsage] + # Should return the parsed list without validation + assert result == ["item1", "item2"] diff --git a/tests/test_devrelay.py b/tests/test_devrelay.py index 3963280..749dd8c 100644 --- a/tests/test_devrelay.py +++ b/tests/test_devrelay.py @@ -25,7 +25,7 @@ def test_display_startup_info(self) -> None: """Test display_startup_info outputs correct information.""" cli_instance = cli.DevRelayCLI() with patch("builtins.print") as mock_print: - cli_instance.display_startup_info("127.0.0.1", 8080, Path.home() / ".mitmproxy") + cli_instance.display_startup_info("127.0.0.1", 8080, Path.home() / ".mitmproxy", []) assert mock_print.call_count == 3 mock_print.assert_has_calls( [ @@ -40,7 +40,7 @@ def test_display_startup_info_with_custom_certdir(self) -> None: cli_instance = cli.DevRelayCLI() custom_certdir = Path("/tmp/custom") with patch("builtins.print") as mock_print: - cli_instance.display_startup_info("0.0.0.0", 9090, custom_certdir) + cli_instance.display_startup_info("0.0.0.0", 9090, custom_certdir, []) assert mock_print.call_count == 3 mock_print.assert_has_calls( [ @@ -50,6 +50,23 @@ def test_display_startup_info_with_custom_certdir(self) -> None: ] ) + def test_display_startup_info_with_disabled_addons(self) -> None: + """Test display_startup_info shows disabled addons.""" + cli_instance = cli.DevRelayCLI() + with patch("builtins.print") as mock_print: + cli_instance.display_startup_info( + "127.0.0.1", 8080, Path.home() / ".mitmproxy", ["CSPRemoverAddon", "COEPRemoverAddon"] + ) + assert mock_print.call_count == 4 + mock_print.assert_has_calls( + [ + call("Starting DevRelay proxy on 127.0.0.1:8080"), + call(f"Certificate directory: {Path.home() / '.mitmproxy'}"), + call("Disabled addons: CSPRemoverAddon, COEPRemoverAddon"), + call("\nPress Ctrl+C to stop the proxy\n"), + ] + ) + def test_run_server_success(self) -> None: """Test run_server starts server successfully.""" cli_instance = cli.DevRelayCLI() @@ -57,9 +74,27 @@ def test_run_server_success(self) -> None: mock_server_instance = MagicMock() mock_proxy_server.return_value = mock_server_instance - result = cli_instance.run_server("127.0.0.1", 8080, Path.home() / ".mitmproxy") + result = cli_instance.run_server("127.0.0.1", 8080, Path.home() / ".mitmproxy", []) + + mock_proxy_server.assert_called_once_with( + host="127.0.0.1", port=8080, certdir=Path.home() / ".mitmproxy", disabled_addons=[] + ) + mock_server_instance.run.assert_called_once() + assert result == 0 + + def test_run_server_with_disabled_addons(self) -> None: + """Test run_server passes disabled_addons to ProxyServer.""" + cli_instance = cli.DevRelayCLI() + disabled_addons = ["CSPRemoverAddon", "COEPRemoverAddon"] + with patch("devrelay.cli.ProxyServer") as mock_proxy_server: + mock_server_instance = MagicMock() + mock_proxy_server.return_value = mock_server_instance + + result = cli_instance.run_server("127.0.0.1", 8080, Path.home() / ".mitmproxy", disabled_addons) - mock_proxy_server.assert_called_once_with(host="127.0.0.1", port=8080, certdir=Path.home() / ".mitmproxy") + mock_proxy_server.assert_called_once_with( + host="127.0.0.1", port=8080, certdir=Path.home() / ".mitmproxy", disabled_addons=disabled_addons + ) mock_server_instance.run.assert_called_once() assert result == 0 @@ -72,7 +107,7 @@ def test_run_server_keyboard_interrupt(self) -> None: mock_proxy_server.return_value = mock_server_instance with patch("builtins.print"): - result = cli_instance.run_server("127.0.0.1", 8080, Path.home() / ".mitmproxy") + result = cli_instance.run_server("127.0.0.1", 8080, Path.home() / ".mitmproxy", []) assert result == 0 @@ -85,7 +120,7 @@ def test_run_server_exception(self) -> None: mock_proxy_server.return_value = mock_server_instance with patch("builtins.print"): - result = cli_instance.run_server("127.0.0.1", 8080, Path.home() / ".mitmproxy") + result = cli_instance.run_server("127.0.0.1", 8080, Path.home() / ".mitmproxy", []) assert result == 1 @@ -104,8 +139,8 @@ def test_execute_with_defaults(self) -> None: result = cli_instance.execute([]) - mock_display.assert_called_once_with("127.0.0.1", 8080, Path.home() / ".mitmproxy") - mock_run.assert_called_once_with("127.0.0.1", 8080, Path.home() / ".mitmproxy") + mock_display.assert_called_once_with("127.0.0.1", 8080, Path.home() / ".mitmproxy", []) + mock_run.assert_called_once_with("127.0.0.1", 8080, Path.home() / ".mitmproxy", []) assert result == 0 def test_execute_with_custom_args(self) -> None: @@ -123,8 +158,29 @@ def test_execute_with_custom_args(self) -> None: result = cli_instance.execute(["--host", "0.0.0.0", "--port", "9090", "--certdir", "/tmp/certs"]) - mock_display.assert_called_once_with("0.0.0.0", 9090, Path("/tmp/certs")) - mock_run.assert_called_once_with("0.0.0.0", 9090, Path("/tmp/certs")) + mock_display.assert_called_once_with("0.0.0.0", 9090, Path("/tmp/certs"), []) + mock_run.assert_called_once_with("0.0.0.0", 9090, Path("/tmp/certs"), []) + assert result == 0 + + def test_execute_with_disabled_addons(self) -> None: + """Test execute method with disabled_addons.""" + import tempfile + + with tempfile.TemporaryDirectory() as tmpdir: + config_path = Path(tmpdir) / "test.yaml" + cli_instance = cli.DevRelayCLI(config_path=config_path) + with ( + patch.object(cli_instance, "display_startup_info") as mock_display, + patch.object(cli_instance, "run_server") as mock_run, + ): + mock_run.return_value = 0 + + result = cli_instance.execute(["--disable-addon", "CSP", "--disable-addon", "COEP"]) + + # Verify disabled_addons were validated and normalized + call_args = mock_display.call_args[0] + assert call_args[3] == ["CSPRemoverAddon", "COEPRemoverAddon"] + mock_run.assert_called_once() assert result == 0 def test_execute_with_config_error(self) -> None: @@ -197,8 +253,8 @@ def test_main_starts_server_with_custom_args(self) -> None: result = test_cli.execute() # Verify display was called with custom args - mock_display.assert_called_once_with("0.0.0.0", 9090, Path.home() / ".mitmproxy") - mock_run.assert_called_once_with("0.0.0.0", 9090, Path.home() / ".mitmproxy") + mock_display.assert_called_once_with("0.0.0.0", 9090, Path.home() / ".mitmproxy", []) + mock_run.assert_called_once_with("0.0.0.0", 9090, Path.home() / ".mitmproxy", []) # Verify exit code assert result == 0 diff --git a/tests/test_proxy.py b/tests/test_proxy.py index a4a3e10..06ff07f 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -114,3 +114,124 @@ def fake_asyncio_run(coro: Coroutine[Any, Any, Any]) -> None: # Verify asyncio.run was called with start coroutine mock_asyncio_run.assert_called_once() assert captured["coro"] is mock_asyncio_run.call_args[0][0] + + def test_initialization_with_disabled_addons(self) -> None: + """Test ProxyServer initialization with disabled_addons.""" + server = ProxyServer(disabled_addons=["CSPRemoverAddon", "COEPRemoverAddon"]) + + assert server.disabled_addons == ["CSPRemoverAddon", "COEPRemoverAddon"] + + def test_initialization_with_none_disabled_addons(self) -> None: + """Test ProxyServer initialization with None disabled_addons defaults to empty list.""" + server = ProxyServer(disabled_addons=None) + + assert server.disabled_addons == [] + + def test_initialization_disabled_addons_defaults_to_empty_list(self) -> None: + """Test that disabled_addons defaults to empty list when not specified.""" + server = ProxyServer() + + assert server.disabled_addons == [] + + @pytest.mark.asyncio + async def test_start_with_disabled_addons_excludes_addons(self) -> None: + """Test that start() excludes disabled addons.""" + server = ProxyServer(disabled_addons=["CSPRemoverAddon", "COEPRemoverAddon"]) + + with ( + patch("devrelay.proxy.options.Options"), + patch("devrelay.proxy.dump.DumpMaster") as mock_dump_master, + ): + # Setup mocks + mock_master_instance = MagicMock() + mock_master_instance.run = AsyncMock() + mock_dump_master.return_value = mock_master_instance + + # Run start + await server.start() + + # Verify only 4 addons were added (6 total - 2 disabled) + assert mock_master_instance.addons.add.call_count == 4 + + # Verify disabled addons were NOT added + added_addon_types = [type(call[0][0]).__name__ for call in mock_master_instance.addons.add.call_args_list] + assert "CSPRemoverAddon" not in added_addon_types + assert "COEPRemoverAddon" not in added_addon_types + # Verify enabled addons were added + assert "COOPRemoverAddon" in added_addon_types + assert "CORPInserterAddon" in added_addon_types + + @pytest.mark.asyncio + async def test_start_with_all_addons_disabled(self) -> None: + """Test that start() works when all addons are disabled.""" + server = ProxyServer( + disabled_addons=[ + "CSPRemoverAddon", + "COEPRemoverAddon", + "COOPRemoverAddon", + "CORPInserterAddon", + "CORSInserterForWebhooksAddon", + "CORSPreflightForWebhooksAddon", + ] + ) + + with ( + patch("devrelay.proxy.options.Options"), + patch("devrelay.proxy.dump.DumpMaster") as mock_dump_master, + ): + # Setup mocks + mock_master_instance = MagicMock() + mock_master_instance.run = AsyncMock() + mock_dump_master.return_value = mock_master_instance + + # Run start + await server.start() + + # Verify no addons were added + assert mock_master_instance.addons.add.call_count == 0 + + @pytest.mark.asyncio + async def test_start_with_one_addon_disabled(self) -> None: + """Test that start() excludes only one disabled addon.""" + server = ProxyServer(disabled_addons=["CSPRemoverAddon"]) + + with ( + patch("devrelay.proxy.options.Options"), + patch("devrelay.proxy.dump.DumpMaster") as mock_dump_master, + ): + # Setup mocks + mock_master_instance = MagicMock() + mock_master_instance.run = AsyncMock() + mock_dump_master.return_value = mock_master_instance + + # Run start + await server.start() + + # Verify 5 addons were added (6 total - 1 disabled) + assert mock_master_instance.addons.add.call_count == 5 + + # Verify CSP was NOT added but others were + added_addon_types = [type(call[0][0]).__name__ for call in mock_master_instance.addons.add.call_args_list] + assert "CSPRemoverAddon" not in added_addon_types + assert "COEPRemoverAddon" in added_addon_types + assert "COOPRemoverAddon" in added_addon_types + + @pytest.mark.asyncio + async def test_start_with_empty_disabled_addons(self) -> None: + """Test that start() adds all addons when disabled_addons is empty.""" + server = ProxyServer(disabled_addons=[]) + + with ( + patch("devrelay.proxy.options.Options"), + patch("devrelay.proxy.dump.DumpMaster") as mock_dump_master, + ): + # Setup mocks + mock_master_instance = MagicMock() + mock_master_instance.run = AsyncMock() + mock_dump_master.return_value = mock_master_instance + + # Run start + await server.start() + + # Verify all 6 addons were added + assert mock_master_instance.addons.add.call_count == 6 From ba6177fa2c3e7a6f92ef29c3d90a6dd3da683199 Mon Sep 17 00:00:00 2001 From: Bernardo Donadio Date: Sun, 2 Nov 2025 10:40:23 -0300 Subject: [PATCH 2/2] refactor: address PR review suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract magic number 15 to SHORT_NAME_MAX_LENGTH constant for clarity - Derive valid_short_names from ADDON_NAME_MAP programmatically instead of hardcoding - Remove unused ALL_ADDON_NAMES constant (was defined but never used) These changes improve maintainability by ensuring a single source of truth for addon names and making the code more self-documenting. All tests still pass with 100% coverage maintained. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- devrelay/addons.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/devrelay/addons.py b/devrelay/addons.py index 0bbe46d..b587f84 100644 --- a/devrelay/addons.py +++ b/devrelay/addons.py @@ -3,6 +3,9 @@ import difflib from mitmproxy import http +# Maximum length for addon names to be considered "short" for user-friendly suggestions +SHORT_NAME_MAX_LENGTH = 15 + # Addon name mapping for user-friendly names # Maps both short names and full class names to canonical class names ADDON_NAME_MAP = { @@ -22,16 +25,6 @@ "CORSPREFLIGHTFORWEBHOOKSADDON": "CORSPreflightForWebhooksAddon", } -# All valid addon class names -ALL_ADDON_NAMES = [ - "CSPRemoverAddon", - "COEPRemoverAddon", - "COOPRemoverAddon", - "CORPInserterAddon", - "CORSInserterForWebhooksAddon", - "CORSPreflightForWebhooksAddon", -] - def validate_addon_names(addon_names: list[str]) -> list[str]: """ @@ -73,7 +66,7 @@ def validate_addon_names(addon_names: list[str]) -> list[str]: # Find a user-friendly version (prefer short names) user_friendly = None for key, value in ADDON_NAME_MAP.items(): - if value == suggested_canonical and len(key) <= 15: # Prefer short names + if value == suggested_canonical and len(key) <= SHORT_NAME_MAX_LENGTH: user_friendly = key break if user_friendly is None: @@ -81,8 +74,9 @@ def validate_addon_names(addon_names: list[str]) -> list[str]: raise ValueError(f"Unknown addon '{name}'. Did you mean '{user_friendly}'?") else: - # No close match - list all valid options - valid_short_names = ["CSP", "COEP", "COOP", "CORP", "CORSInserter", "CORSPreflight"] + # No close match - list all valid short names + # Derive short names from ADDON_NAME_MAP (names <= SHORT_NAME_MAX_LENGTH chars) + valid_short_names = sorted(set(k for k in ADDON_NAME_MAP.keys() if len(k) <= SHORT_NAME_MAX_LENGTH)) raise ValueError(f"Unknown addon '{name}'. Valid addons: {', '.join(valid_short_names)}") return validated