Skip to content

fix(scanner): address LSP violation and magic number issues#6

Merged
yi-john-huang merged 1 commit into
masterfrom
fix/lsp-and-magic-numbers
Feb 4, 2026
Merged

fix(scanner): address LSP violation and magic number issues#6
yi-john-huang merged 1 commit into
masterfrom
fix/lsp-and-magic-numbers

Conversation

@yi-john-huang

Copy link
Copy Markdown
Owner

Summary

  • Fix LSP violation in NetworkExfilRule by adding source_str parameter to base class _walk()
  • Extract MOCK_CHECKSUM constant for magic number in test fixtures

Changes

LSP Violation Fix

The NetworkExfilRule previously had an empty _walk() stub and used a separate _walk_with_source() method, violating Liskov Substitution Principle.

Fix: Add source_str as a parameter to the base class _walk() signature:

  • Rules that need it (NetworkExfilRule) can use it
  • Rules that don't need it (DangerousAPIRule, FSAbuseRule) mark it as unused with # noqa: ARG002

Magic Number Fix

Extracted MOCK_CHECKSUM = "a" * 64 constant in tests/conftest.py with a comment explaining it represents SHA-256 hex digest length.

Test plan

  • All 175 tests pass
  • Ruff linter passes

🤖 Generated with Claude Code

1. LSP violation fix: Add source_str parameter to _walk() in base class
   so NetworkExfilRule can properly implement the abstract method
   instead of needing a separate _walk_with_source() workaround.

2. Magic number fix: Extract MOCK_CHECKSUM constant for the SHA-256
   hex digest length (64 chars) in test fixtures.

All 175 tests pass.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@yi-john-huang

Copy link
Copy Markdown
Owner Author

Code Review Results

No issues found.

All code changes in this PR meet the quality threshold. The fixes properly address:

  1. LSP Violation Fixed: The base class ASTScanRule._walk() now includes source_str parameter, allowing all subclasses to implement the same signature. NetworkExfilRule properly uses this parameter for URL allowlist checking, while other rules correctly mark it unused with # noqa: ARG002.

  2. Magic Number Eliminated: MOCK_CHECKSUM constant extracted to conftest.py with explanatory comment about SHA-256 producing 64 hex characters.

✅ Ready to merge.

@yi-john-huang
yi-john-huang merged commit 9e73d45 into master Feb 4, 2026
@yi-john-huang
yi-john-huang deleted the fix/lsp-and-magic-numbers branch February 4, 2026 12:35
@nayname nayname mentioned this pull request Feb 22, 2026
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.

1 participant