DSL Engine - #1587
Conversation
create pr
Document all modules ( fix OWASP#1269 ) (OWASP#1270)
Signed-off-by: Aarush <cs24b064@smail.iitm.ac.in>
Removed entry for FortiWeb authentication bypass vulnerability. Signed-off-by: Aarush <cs24b064@smail.iitm.ac.in>
Signed-off-by: Aarush <cs24b064@smail.iitm.ac.in>
Signed-off-by: Aarush <cs24b064@smail.iitm.ac.in>
Merge new changes
Merge new modules
Pull the changes
merge the changes
Add the new changes
MERGE NEW changes
Merge changes
Merge the changes
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a ChangesVersion DSL Engine and HTTP Integration
Estimated code review effort: 4 (Complex) | ~50 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
46-69:⚠️ Potential issue | 🔴 CriticalAdd missing
packagingruntime dependency to resolve import error in execution paths.
nettacker/core/utils/dsl_matcher.pyimportsfrom packaging.version import Version, InvalidVersionat line 12. This module is imported bycommon.py(line 17), which is in turn imported across all core entry points (config.py,app.py,graph.py,http.py, etc.). Thepackaginglibrary is not declared in[tool.poetry.dependencies], causing aModuleNotFoundErrorwhen these execution paths are triggered.Suggested fix
[tool.poetry.dependencies] python = "^3.10, <3.13" aiohttp = "^3.9.5" @@ zipp = "^3.19.1" semver = "^3.0.0" +packaging = "^24.0"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 46 - 69, The packaging library is missing from the runtime dependencies in pyproject.toml. Add the packaging dependency to the [tool.poetry.dependencies] section with an appropriate version constraint. This will ensure that when dsl_matcher.py imports from packaging.version (which is transitively required by common.py and used across all core entry points), the module will be available at runtime.Source: Pipeline failures
🧹 Nitpick comments (1)
nettacker/core/utils/common.py (1)
456-493: ⚡ Quick winAdd type hints to the new public wrapper APIs.
Lines 456 and 482 expose public helpers in
nettacker/core/utils/common.pybut currently omit practical type hints.Suggested update
+from typing import Iterable, Optional @@ -def version_matches_dsl(detected_version, dsl_expression): +def version_matches_dsl(detected_version: str, dsl_expression: str) -> bool: @@ -def extract_version_from_content(content, patterns): +def extract_version_from_content( + content: str, patterns: Iterable[str] +) -> Optional[str]:As per coding guidelines,
nettacker/**/*.py: "Keep functions small, use type hints where practical, and add docstrings for public APIs."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nettacker/core/utils/common.py` around lines 456 - 493, The functions version_matches_dsl and extract_version_from_content lack type hints for their parameters and return types. Add proper type hints to both functions: for version_matches_dsl, add type hints indicating both detected_version and dsl_expression are strings, and the return type is bool; for extract_version_from_content, add type hints indicating content is a string, patterns is a list, and the return type is an optional string. This aligns with the coding guidelines requiring type hints for public APIs in the nettacker module.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nettacker/core/lib/http.py`:
- Around line 96-121: In the version_match condition block, the code lowercases
the header_name variable but fails to normalize the response headers dictionary
itself, causing case-sensitive lookup failures. When extracting the header from
response.get("headers", {}), first convert the headers dictionary to have all
lowercase keys before accessing it with header_name.lower(), so that the lookup
succeeds regardless of the original casing in the response headers (e.g.,
"Server" vs "server").
In `@nettacker/core/utils/dsl_matcher.py`:
- Around line 447-454: The exception handling in the try-except block (lines
453-454) silently catches all exceptions without logging, which hides debugging
information about invalid regex patterns or configuration issues. Add logging to
the except block that captures the actual exception details and relevant context
(such as the pattern being attempted) before executing the continue statement.
This will make it easier to diagnose why pattern matching fails during
extraction.
- Around line 442-446: The issue in the extract_version_from_response method is
that line 445 unconditionally wraps patterns in a list with patterns =
[patterns], which creates a nested list structure when callers already pass a
list of patterns. This causes re.search to receive a list instead of a string
regex pattern, breaking the matching logic. Fix this by conditionally wrapping
patterns only if it is not already a list; check if patterns is a string and
only then wrap it in a list, otherwise leave it as-is.
---
Outside diff comments:
In `@pyproject.toml`:
- Around line 46-69: The packaging library is missing from the runtime
dependencies in pyproject.toml. Add the packaging dependency to the
[tool.poetry.dependencies] section with an appropriate version constraint. This
will ensure that when dsl_matcher.py imports from packaging.version (which is
transitively required by common.py and used across all core entry points), the
module will be available at runtime.
---
Nitpick comments:
In `@nettacker/core/utils/common.py`:
- Around line 456-493: The functions version_matches_dsl and
extract_version_from_content lack type hints for their parameters and return
types. Add proper type hints to both functions: for version_matches_dsl, add
type hints indicating both detected_version and dsl_expression are strings, and
the return type is bool; for extract_version_from_content, add type hints
indicating content is a string, patterns is a list, and the return type is an
optional string. This aligns with the coding guidelines requiring type hints for
public APIs in the nettacker module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45891a2c-51a7-4c9d-b836-c097700375d7
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
nettacker/api/database.sqlite3nettacker/core/lib/http.pynettacker/core/utils/common.pynettacker/core/utils/dsl_matcher.pypyproject.toml
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/OWASP/Nettacker/blob/8371b67aaef5bc7c7b87cc5997bccff0296a8a04/nettacker/api/database.sqlite3#L1
The binary SQLite diff is not just metadata: dumping it shows a new hosts_log row for 210.65.127.194 with scan id manual_test. This file is documented as the empty sample API database, so shipping it with a pre-populated scan result leaks a real target and makes fresh installs/tests start with stale findings instead of an empty database.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| next_major = v2_parts.copy() | ||
| next_major[0] += 1 | ||
| for i in range(1, len(next_major)): | ||
| next_major[i] = 0 |
There was a problem hiding this comment.
When a DSL expression uses a caret constraint on a pre-1.0 version, this always advances to the next major version, so ^0.2.3 accepts targets up to <1.0.0 (for example 0.9.0). Caret semantics for 0.x versions only allow changes within the next non-zero component, so modules using this matcher for vulnerability ranges can falsely flag unaffected pre-1.0 services.
Useful? React with 👍 / 👎.
| else: | ||
| condition_results["responsetime"] = [] | ||
| # DSL version matching support | ||
| if condition == "version_match": |
There was a problem hiding this comment.
The new version_match/version_dsl runtime conditions are still absent from tests/test_yaml_schema_and_regex.py's HTTP_CONDITION_SCHEMA, which validates HTTP conditions with ignore_extra_keys=False. Any module that actually uses this DSL condition will therefore fail the repo's YAML schema test, so the feature cannot be added to Nettacker modules until that whitelist is updated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We will update the schema
| version_config = conditions[condition] | ||
| # Extract version from response using patterns | ||
| version_patterns = version_config.get("patterns", []) | ||
| dsl_expressions = version_config.get("expressions", []) |
There was a problem hiding this comment.
When a module defines version_dsl with the same single-expression shape as version_match (for example expression: '>=1.2.3'), this lookup leaves dsl_expressions empty and the condition always returns no match even after extracting a valid version. Either accepting the singular key or clearly normalizing it to a list is needed, otherwise one-expression version_dsl checks are silently unusable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
its not an issue, user can use "expressions" always
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
nettacker/core/utils/dsl_matcher.py (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd docstrings to public methods.
parse_dsl_expression(Line 19) andextract_version_from_response(Line 442) are public entry points but lack docstrings, unlike the private helpers below them. A short docstring documenting args/return would align them with the rest of the module.As per coding guidelines: "add docstrings for public APIs".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nettacker/core/utils/dsl_matcher.py` at line 19, Add concise docstrings to the public methods parse_dsl_expression and extract_version_from_response, documenting their parameters and return values. Match the module’s existing documentation style and leave the private helper implementations unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nettacker/core/lib/http.py`:
- Around line 16-22: Resolve the import conflict at the module’s import block by
removing all raw merge markers and retaining all four required symbols:
replace_dependent_response, reverse_and_regex_condition, version_matches_dsl,
and extract_version_from_content. Ensure the resulting imports are valid Python
and preserve their existing usages.
---
Nitpick comments:
In `@nettacker/core/utils/dsl_matcher.py`:
- Line 19: Add concise docstrings to the public methods parse_dsl_expression and
extract_version_from_response, documenting their parameters and return values.
Match the module’s existing documentation style and leave the private helper
implementations unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 639b7bd4-1b1c-462f-99ff-5a65b9ad26cd
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
nettacker/core/lib/http.pynettacker/core/utils/common.pynettacker/core/utils/dsl_matcher.pypyproject.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- nettacker/core/utils/common.py
- pyproject.toml
3 participants
Proposed change
This PR introduces DSL (Domain Specific Language) support for service/version extraction in Nettacker.
(This work is derived from the DSL PR by Rishi after minor changes)
Fixes #1586
Type of change
Checklist
make pre-commitand confirm it didn't generate any warnings/changesmake testand I confirm all tests passed locallydocs/folder