Skip to content

feat: add --disable-addon CLI option to selectively disable addons#8

Merged
bcdonadio merged 2 commits into
masterfrom
feature/disable-addon-config
Nov 2, 2025
Merged

feat: add --disable-addon CLI option to selectively disable addons#8
bcdonadio merged 2 commits into
masterfrom
feature/disable-addon-config

Conversation

@bcdonadio

Copy link
Copy Markdown
Contributor

Summary

Add the ability to disable specific addons via --disable-addon CLI argument or YAML configuration. This provides users with granular control over which security headers are removed or modified by the proxy.

Key Features

  • Flexible addon naming: Accepts both short names (CSP, COEP, COOP, CORP, CORSInserter, CORSPreflight) and full class names (CSPRemoverAddon, etc.)
  • Multiple input formats:
    • Comma-separated: --disable-addon CSP,COEP
    • Repeated flag: --disable-addon CSP --disable-addon COEP
    • YAML configuration: disabled_addons: [CSP, COEP]
  • Case-insensitive matching: csp, CSP, and CsP all work
  • Intelligent error handling: Provides "Did you mean X?" suggestions for typos using fuzzy matching
  • Configuration precedence: CLI arguments > YAML file > defaults

Implementation Details

Core Changes

  1. devrelay/addons.py (+85 lines)

    • Added ADDON_NAME_MAP for short name to class name mapping
    • Added validate_addon_names() function with fuzzy matching for error suggestions
    • Uses difflib.get_close_matches() for typo detection
  2. devrelay/config.py (+84 lines)

    • Extended ConfigLoader with disabled_addons parameter (type: list)
    • Added _parse_addon_list() to handle both CLI and YAML formats
    • Updated _validate_value() to validate and normalize addon names
    • Modified _build_parser() to use action='append' for repeated flag support
  3. devrelay/proxy.py (+24 lines)

    • Added disabled_addons parameter to ProxyServer.__init__()
    • Updated start() to conditionally load addons based on disabled list
    • All 6 addons can now be individually disabled
  4. devrelay/cli.py (+13 lines)

    • Updated display_startup_info() to show disabled addons
    • Updated run_server() to pass disabled_addons to ProxyServer
    • Updated execute() to extract disabled_addons from config

Documentation & Testing

  1. README.md (+81 lines)

    • Added "Disabling Addons" section with usage examples
    • Added "Configuration File" section explaining YAML config
    • Updated CLI options documentation
    • Documented all available addon names
  2. Comprehensive test coverage (+436 lines across 4 test files)

    • 16 new test cases added
    • 147 total tests passing
    • 100% code coverage (line and branch)
    • Tests cover all addon name formats, validation errors, fuzzy matching, CLI parsing, YAML config, and ProxyServer integration

Usage Examples

CLI

# Disable single addon
devrelay --disable-addon CSP

# Disable multiple addons (comma-separated)
devrelay --disable-addon CSP,COEP,COOP

# Disable multiple addons (repeated flag)
devrelay --disable-addon CSP --disable-addon COEP

# Case-insensitive
devrelay --disable-addon csp --disable-addon COEP

# Combine with other options
devrelay --host 0.0.0.0 --port 9090 --disable-addon CSP

YAML Configuration (~/.mitmproxy/devrelay.yaml)

host: 127.0.0.1
port: 8080
certdir: /home/user/.mitmproxy
disabled_addons:
  - CSP
  - COEP

Error Handling

If you type an invalid addon name, you'll get a helpful suggestion:

$ devrelay --disable-addon CS
Configuration error: Unknown addon 'CS'. Did you mean 'CSP'?

Test Plan

  • All 147 tests pass
  • 100% code coverage achieved (both line and branch)
  • make check passes (format, lint, lintmd, typecheck, test)
  • Pre-commit hooks pass
  • Manual testing of CLI with various addon combinations
  • Manual testing of YAML configuration
  • Error handling tested with invalid addon names
  • Documentation verified for accuracy and completeness

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings November 2, 2025 13:33
@codecov

codecov Bot commented Nov 2, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds the ability to selectively disable proxy addons through a new --disable-addon CLI option. Users can now choose which security header manipulation addons to run, providing more granular control over the proxy's behavior.

Key changes:

  • Added disabled_addons parameter to configuration system with validation against known addon names
  • Implemented fuzzy matching for addon name suggestions when invalid names are provided
  • Extended CLI, proxy server, and configuration components to support the new functionality

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
devrelay/addons.py Added validate_addon_names() function with fuzzy matching and addon name mapping for short/full name support
devrelay/config.py Added disabled_addons parameter with custom parsing for comma-separated and repeated CLI arguments
devrelay/cli.py Updated methods to accept and display disabled addons
devrelay/proxy.py Modified addon loading to conditionally skip disabled addons
README.md Added documentation for the new --disable-addon option with usage examples
tests/test_proxy.py Added comprehensive tests for disabled addons functionality
tests/test_devrelay.py Updated tests to pass disabled_addons parameter
tests/test_config.py Added extensive tests for addon list parsing and validation
tests/test_addons.py Added tests for validate_addon_names() function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread devrelay/addons.py Outdated
Comment thread devrelay/addons.py Outdated
Comment thread devrelay/config.py
@claude

claude Bot commented Nov 2, 2025

Copy link
Copy Markdown

PR Review: Add --disable-addon CLI option

Summary

This is an excellent, production-ready PR that implements selective addon disabling with robust error handling, comprehensive testing, and thorough documentation. The implementation demonstrates high code quality and follows best practices throughout.

Code Quality & Best Practices ✅

Strengths

  1. Clean Architecture: The feature is well-integrated across all layers (CLI → Config → Proxy → Addons) with clear separation of concerns.

  2. Type Safety: Strict type hints throughout (list[str], dict[str, Any], etc.) align with the project's pyright strict mode.

  3. Excellent Error Handling:

    • Fuzzy matching with difflib.get_close_matches() for typo suggestions (addons.py:68)
    • User-friendly error messages: "Unknown addon 'CS'. Did you mean 'CSP'?"
    • Graceful handling of edge cases (empty strings, None values, mixed formats)
  4. Comprehensive Documentation:

    • All new functions have detailed docstrings with Args/Returns/Raises
    • README.md expanded with clear usage examples (+76 lines)
    • Inline comments explain non-obvious logic
  5. Flexible API Design:

    • Supports multiple input formats (comma-separated, repeated flags, YAML)
    • Case-insensitive matching for better UX
    • Both short names (CSP) and full class names (CSPRemoverAddon) accepted
  6. Configuration Precedence: Properly implements CLI > YAML > defaults (config.py:271-284)

Minor Suggestions

  1. Magic Number (addons.py:76):

    if value == suggested_canonical and len(key) <= 15:  # Prefer short names

    Consider extracting 15 to a named constant:

    SHORT_NAME_MAX_LENGTH = 15
    if value == suggested_canonical and len(key) <= SHORT_NAME_MAX_LENGTH:
  2. Hardcoded List (addons.py:85):

    valid_short_names = ["CSP", "COEP", "COOP", "CORP", "CORSInserter", "CORSPreflight"]

    This duplicates knowledge already in ADDON_NAME_MAP. Consider deriving it programmatically:

    valid_short_names = [k for k in ADDON_NAME_MAP.keys() if len(k) <= SHORT_NAME_MAX_LENGTH]
  3. Unused Constant (addons.py:26-33):

    ALL_ADDON_NAMES = [...]

    This constant is defined but never used. Either use it in validation logic or remove it to follow YAGNI.

Potential Bugs 🐛

None Found

The code handles edge cases extremely well:

  • ✅ Empty lists, None values, empty strings (config.py:117-136)
  • ✅ Mixed CLI formats (comma + repeated flags)
  • ✅ Invalid addon names with suggestions
  • ✅ Type mismatches in YAML

Performance Considerations ⚡

Excellent

  1. Efficient Lookups: Uses dict lookups (ADDON_NAME_MAP) instead of linear searches - O(1) time complexity.

  2. Minimal Overhead: Fuzzy matching only runs on error path (addons.py:68), not happy path.

  3. No Premature Optimization: Code prioritizes clarity over micro-optimizations, which is appropriate for configuration parsing (runs once at startup).

  4. Dict Iteration (proxy.py:92): Iterating over available_addons.items() to conditionally load addons is efficient for 6 items.

Notes

  • Fuzzy matching uses cutoff=0.6 which is reasonable for typo detection
  • The implementation is appropriate for the scale (6 addons)

Security Concerns 🔒

None Identified

  1. Input Validation: All user input is validated before use (config.py:171-220).

  2. No Injection Risks: Addon names are validated against a whitelist (ADDON_NAME_MAP), preventing arbitrary code execution.

  3. Path Handling: Certificate directory properly uses Path objects and doesn't allow path traversal.

  4. YAML Safety: Uses ruamel.yaml which is safe against YAML deserialization attacks (unlike yaml.unsafe_load).

Test Coverage 🧪

Outstanding (100% coverage maintained)

New Tests Added: 147 total tests (+16 new test cases)

Coverage by Component:

  1. addons.py (test_addons.py):

    • ✅ All short names, full class names, case variations
    • ✅ Fuzzy matching suggestions
    • ✅ Invalid names with/without close matches
    • ✅ Edge cases: empty lists, duplicates, mixed formats
    • 64 test functions in test_addons.py
  2. config.py (test_config.py):

    • ✅ CLI parsing (single, comma-separated, repeated flags)
    • ✅ YAML parsing (lists, comma-separated strings)
    • ✅ Validation errors
    • ✅ Precedence (CLI overrides YAML)
    • 50 test functions in test_config.py
  3. proxy.py (test_proxy.py):

    • ✅ Conditional addon loading
    • ✅ All addons disabled
    • ✅ Partial disabling
    • 13 test functions in test_proxy.py
  4. cli.py (test_devrelay.py):

    • ✅ Startup info display
    • ✅ End-to-end CLI flow
    • 20 test functions in test_devrelay.py

Test Quality:

  • Uses proper fixtures (tempfile.TemporaryDirectory)
  • Tests both happy paths and error conditions
  • Follows existing test patterns (mitmproxy.test.tflow)

Additional Observations

Excellent Practices

  1. Git Commit Message: Follows conventional commits (feat: add --disable-addon CLI option)

  2. PR Description: Outstanding - includes summary, features, implementation details, usage examples, and test plan

  3. Backwards Compatibility: Feature is additive - existing users not affected (default: no addons disabled)

  4. User Experience:

    • Error messages are actionable
    • Multiple input formats reduce friction
    • Documentation includes examples for all use cases
  5. Maintainability:

    • Single source of truth for addon names (ADDON_NAME_MAP)
    • Configuration logic centralized in ConfigLoader
    • Clear separation between parsing and validation

Alignment with Project Standards

AGENTS.md: Follows all coding standards
Pyright strict mode: All type hints correct
100% coverage: Maintained (both line and branch)
Docstrings: All public APIs documented
Code style: Follows black formatting (120 char lines)

Recommendation

APPROVE

This PR is exemplary and ready to merge. The implementation is thoughtful, well-tested, and follows all project conventions. The minor suggestions above are optional refinements, not blockers.

Optional Follow-ups (Non-blocking)

  1. Extract magic number 15 to named constant
  2. Derive valid_short_names from ADDON_NAME_MAP or use ALL_ADDON_NAMES
  3. Remove ALL_ADDON_NAMES constant if unused

Great work! 🎉


🤖 Generated with Claude Code

- 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 <noreply@anthropic.com>
@claude

claude Bot commented Nov 2, 2025

Copy link
Copy Markdown

Pull Request Review: Add --disable-addon CLI Option

Overall Assessment

Verdict: Excellent - Ready to Merge

This is a high-quality PR that adds a valuable feature with excellent implementation. The code demonstrates strong software engineering practices with comprehensive testing, thoughtful design, and attention to detail.

Strengths

1. Code Quality & Best Practices - 5/5 Stars

Excellent architecture and design patterns:

  • Clean separation of concerns across addons.py, config.py, proxy.py, and cli.py
  • Proper use of type hints throughout (critical for strict pyright compliance)
  • Well-structured dataclasses and clear function signatures
  • Consistent error handling with helpful error messages
  • Good use of Python idioms (list comprehensions, dict comprehensions, early returns)

Strong naming conventions:

  • validate_addon_names() clearly describes its purpose
  • ADDON_NAME_MAP is descriptive and self-documenting
  • Variable names like user_friendly, canonical_name, normalized communicate intent well

Documentation:

  • Comprehensive docstrings with Args, Returns, and Raises sections
  • Inline comments explain complex logic (fuzzy matching, precedence rules)
  • README.md thoroughly updated with multiple usage examples

2. Security & Robustness - 5/5 Stars

No security concerns identified:

  • Input validation using validate_addon_names() prevents invalid addon references
  • No SQL injection, command injection, or path traversal vulnerabilities
  • Proper exception handling prevents information leakage
  • YAML parsing uses ruamel.yaml with safe defaults

Robust error handling:

  • Fuzzy matching with difflib.get_close_matches() provides helpful suggestions
  • Port range validation (1-65535) in config.py:204-205
  • Graceful handling of None values and missing configuration keys
  • Type conversion errors are caught and re-raised with context

3. Test Coverage - 5/5 Stars

Outstanding test coverage - 100% line and branch coverage:

  • 147 total tests passing (up from 131 - added 16 new tests)
  • Comprehensive test scenarios covering all addon name formats, fuzzy matching, CLI parsing, YAML configuration, configuration precedence, and edge cases
  • Tests follow AAA pattern (Arrange-Act-Assert)
  • Clear test names describing expected behavior
  • Good use of pytest features (fixtures, parametrization, exception matching)

4. Performance Considerations - 4/5 Stars

Efficient implementation:

  • O(1) lookup time using dictionary-based ADDON_NAME_MAP
  • Lazy loading of addons (only enabled ones instantiated)
  • Single-pass validation in validate_addon_names()
  • No unnecessary file I/O or repeated YAML parsing

Minor optimization opportunity (not blocking):
Line 58 in addons.py creates a list of all possible names on each validation call. Could be computed once as a module-level constant, though this is a micro-optimization.

Issues Found

Critical Issues: None

Major Issues: None

Minor Issues & Suggestions

1. Potential UX Improvement (devrelay/addons.py:7)

The SHORT_NAME_MAX_LENGTH of 15 feels arbitrary. Consider documenting why 15 was chosen, or use an explicit set of user-facing names.

  • Impact: Low
  • Recommendation: Document rationale or refactor if time permits

2. Edge Case in _parse_addon_list (devrelay/config.py:127-129)

The method handles non-string items with else: result.append(item). Consider adding type validation for defensive programming.

  • Impact: Very low
  • Recommendation: Optional enhancement

3. Documentation Clarity (devrelay/proxy.py:92-93)

The comment could be slightly clearer about the double-negative logic.

  • Impact: Very low
  • Recommendation: Optional wording improvement

Detailed Code Review

devrelay/addons.py (+79 lines) - Excellent

  • validate_addon_names() is well-designed with clear error messages
  • Fuzzy matching is a nice UX touch
  • Case-insensitive matching is appropriate

devrelay/config.py (+77 lines) - Excellent

  • _parse_addon_list() handles multiple input formats gracefully
  • Configuration precedence logic is correct and well-commented
  • Port validation prevents common errors

devrelay/proxy.py (+18 lines) - Excellent

  • Dictionary-based addon registry is extensible
  • Conditional loading is straightforward
  • Maintains backward compatibility

devrelay/cli.py (+9 lines) - Excellent

  • Proper parameter threading through call chain
  • Good user feedback

Tests (+436 lines) - Excellent

  • Comprehensive edge case coverage
  • Thorough fuzzy matching tests
  • Advanced testing techniques

Documentation (README.md +76 lines) - Excellent

  • Multiple usage examples
  • Clear precedence explanation

Best Practices Checklist

All items passed:

  • Code follows style guidelines (black, flake8, pymarkdownlnt)
  • Type hints present and correct (pyright strict)
  • All functions have docstrings
  • User-friendly error messages
  • 100% test coverage (line and branch)
  • No code duplication
  • Proper separation of concerns
  • Backward compatible
  • Documentation updated

Performance

Negligible impact:

  • Startup time: O(1) dictionary lookups
  • Memory usage: Minimal (only enabled addons instantiated)
  • Runtime overhead: None (validation once at startup)

Recommendations

Before Merge

None - This PR is ready to merge as-is.

Future Enhancements (Post-Merge)

  1. Consider adding --list-addons CLI flag to show available addons
  2. Could add config validation command: devrelay --validate-config
  3. Might support regex patterns: --disable-addon ".*Remover"

Conclusion

This PR exemplifies excellent software engineering:

  • Solves a real user need (granular addon control)
  • Implementation is clean, tested, and documented
  • No security concerns or bugs identified
  • Performance impact is negligible
  • Test coverage is comprehensive (100%)
  • Follows all project conventions

Recommendation: Approve and merge immediately!

The author has done an outstanding job addressing all aspects of quality software development. The minor suggestions above are truly optional refinements that could be addressed in future PRs if desired.


Review conducted following AGENTS.md guidelines with focus on code quality, security, performance, and test coverage.

@bcdonadio bcdonadio merged commit 13dcd7e into master Nov 2, 2025
12 checks passed
@bcdonadio bcdonadio deleted the feature/disable-addon-config branch November 2, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants