Skip to content

fix(plugins): enforce strict validation and integrity checks for missing custom parser #1812 - #2041

Merged
utksh1 merged 2 commits into
utksh1:mainfrom
i-OmSharma:feature/plugins-custom-parser-validation
Jul 20, 2026
Merged

fix(plugins): enforce strict validation and integrity checks for missing custom parser #1812#2041
utksh1 merged 2 commits into
utksh1:mainfrom
i-OmSharma:feature/plugins-custom-parser-validation

Conversation

@i-OmSharma

Copy link
Copy Markdown
Contributor

📝 Description

Fixes #1812. This PR replaces the soft-warning behavior with a hard validation failure during the plugin loading phase when a plugin explicitly sets output.parser == 'custom' but the physical parser.py file is missing from its directory. This prevents invalid/broken plugins from silently loading and crashing downstream at execution time.

🛠️ Key Changes

  1. backend/secuscan/plugins.py (_validate_plugin):

    • Upgraded logger.warning to a explicit logger.error.
    • Returns False early to guarantee the plugin framework skips loading the broken instance.
  2. compute_plugin_digest:

    • Introduced a require_parser: bool = False defensive keyword argument.
    • Raises a explicit FileNotFoundError if a custom parser is missing instead of hashing an empty fallback digest string.
  3. _verify_plugin_integrity:

    • Dynamically tracks requirements using require_parser=(plugin.output.get("parser") == "custom").
    • This creates a double-assertion mechanism catching post-metadata-load file modifications (TOCTOU).

🧪 Testing & Verification

  • Created a highly resilient test suite at testing/backend/unit/test_plugins_validation.py containing 15 unit tests.
  • Scenarios covered:
    • Happy path: Custom parser present passes validation and registers correctly.
    • Failure paths: Validations drop bad plugins, log clean errors, and raise proper exceptions.
    • Race Conditions: Specifically tests runtime integrity failures if parser.py is removed post-metadata parsing.
    • Backward Compatibility: Parametrized matrix verifying that standard built-in pars (default, json, regex, builtin) continue working seamlessly without a local file presence.
  • Local regression checks run across all 38 related plugin core tests—everything passes cleanly without issues.

Copilot AI review requested due to automatic review settings July 20, 2026 06:45

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 hardens SecuScan’s plugin-loading and integrity-verification flow by treating output.parser == "custom" without a corresponding parser.py as an invalid plugin, preventing broken/tampered plugins from being loaded and failing later at execution time.

Changes:

  • Make _validate_plugin emit an error and hard-fail validation when a custom parser is declared but parser.py is missing.
  • Extend compute_plugin_digest with require_parser to raise FileNotFoundError when a custom parser is required but absent, and wire this into _verify_plugin_integrity.
  • Add a dedicated unit test suite covering validation failures, digest behavior, and integrity verification when parser.py is removed post-metadata load.

Reviewed changes

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

File Description
backend/secuscan/plugins.py Enforces custom-parser presence at validation time and strengthens integrity hashing to fail loudly when required files are missing.
testing/backend/unit/test_plugins_validation.py Adds regression tests for missing custom parser handling and integrity verification behavior.

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

# ---------------------------------------------------------------------------


@pytest.mark.parametrize("parser_type", ["default", "json", "regex", "builtin"])


def test_non_custom_parser_plugin_loads_via_load_plugins(tmp_path):
make_plugin(tmp_path, "builtin-ok", parser_type="default", parser_content=None, checksum=None)
Comment on lines +231 to +233
plugin_dir = make_plugin(
tmp_path, "builtin-digest", parser_type="default", parser_content=None, checksum=None
)

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Excellent security fix. This properly enforces validation for custom parsers by:

  • Upgrading soft warning to hard failure in _validate_plugin
  • Adding require_parser flag to compute_plugin_digest with FileNotFoundError
  • Double-assertion in _verify_plugin_integrity to catch TOCTOU
  • 240 lines of comprehensive test coverage (15 tests covering happy path, failures, race conditions, backward compat)

All CI passing. Merging.

@utksh1
utksh1 merged commit 06a5a91 into utksh1:main Jul 20, 2026
21 checks passed
@utksh1 utksh1 added level:advanced 55 pts difficulty label for advanced contributor PRs type:security Security work category bonus label type:bug Bug fix work category bonus label area:backend Backend API, database, or service work area:plugins Scanner plugin metadata, schemas, or plugin runtime work gssoc:approved Admin validation: approved for GSSoC scoring labels Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:plugins Scanner plugin metadata, schemas, or plugin runtime work gssoc:approved Admin validation: approved for GSSoC scoring level:advanced 55 pts difficulty label for advanced contributor PRs type:bug Bug fix work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[#39] custom parser declared but missing parser.py still validates

3 participants