wip: native script analysis support#2946
wip: native script analysis support#2946pranavthakur0-0 wants to merge 2 commits intomandiant:masterfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request adds support for analyzing script files, initially focusing on Python. It integrates tree-sitter to parse scripts and extract features, enabling capa to detect capabilities within these files. The changes include new modules for script feature extraction, a language handler interface, and a Python-specific language handler. This enhancement expands capa's ability to analyze a broader range of file types. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces native script analysis support to capa, primarily focusing on Python scripts using tree-sitter. It adds a new ScriptAddress type, integrates script format and backend into the existing capa infrastructure, and provides a modular LanguageHandler interface for extensibility. The Python language handler includes detailed logic for extracting API calls, strings, and numbers from AST nodes, handling various import syntaxes and string literal formats. The changes are well-structured and demonstrate a clear path for supporting additional scripting languages.
| def __lt__(self, other): | ||
| assert isinstance(other, ScriptAddress) | ||
| return (self.line, self.column) < (other.line, other.column) |
There was a problem hiding this comment.
The __lt__ method should return NotImplemented when comparing with an incompatible type, rather than raising an AssertionError. This allows for more graceful handling of mixed-type comparisons in Python.
| def __lt__(self, other): | |
| assert isinstance(other, ScriptAddress) | |
| return (self.line, self.column) < (other.line, other.column) | |
| def __lt__(self, other): | |
| if not isinstance(other, ScriptAddress): | |
| return NotImplemented | |
| return (self.line, self.column) < (other.line, other.column) |
| def get_function_name(self, addr: Address) -> str: | ||
| # not used in the current pipeline for script analysis | ||
| return "" |
There was a problem hiding this comment.
The get_function_name method, which is part of the StaticFeatureExtractor interface, currently returns an empty string. While the comment indicates it's not used in the current pipeline, returning a more descriptive placeholder like "" would be more informative if it were to be used in the future, or if debugging output were to rely on it. This aligns with the fallback behavior in PythonLanguageHandler.get_function_name.
| def get_function_name(self, addr: Address) -> str: | |
| # not used in the current pipeline for script analysis | |
| return "" | |
| def get_function_name(self, addr: Address) -> str: | |
| # not used in the current pipeline for script analysis | |
| return "<unknown>" |
| text = node.text.decode("utf-8") | ||
| # handle triple-quoted strings | ||
| for quote in ('"""', "'''", '"', "'"): | ||
| if text.startswith(quote) and text.endswith(quote): | ||
| return text[len(quote) : -len(quote)] | ||
| # handle prefixed strings like b"...", r"...", f"..." | ||
| # strip the prefix first | ||
| for prefix in ("b", "B", "r", "R", "f", "F", "rb", "Rb", "rB", "RB", "br", "Br", "bR", "BR"): | ||
| if text.startswith(prefix): | ||
| text = text[len(prefix) :] | ||
| break | ||
| for quote in ('"""', "'''", '"', "'"): | ||
| if text.startswith(quote) and text.endswith(quote): | ||
| return text[len(quote) : -len(quote)] | ||
| return "" |
There was a problem hiding this comment.
The logic for stripping prefixes and quotes in _extract_string_value is a bit repetitive and could be refactored for better clarity and robustness. A helper function to strip quotes, combined with a single pass to strip prefixes, would make the code cleaner and easier to maintain.
def _strip_quotes(self, s: str) -> str:
for quote in ('"""', "'''", '"', "'"):
if s.startswith(quote) and s.endswith(quote):
return s[len(quote) : -len(quote)]
return s
def _extract_string_value(self, node: Node) -> str:
text = node.text.decode("utf-8")
# First, try to strip any prefix (e.g., b, r, f, rb, etc.).
# Iterate through common prefixes, prioritizing longer ones.
for prefix in ("rb", "br", "rB", "Rb", "RB", "bR", "Br", "b", "B", "r", "R", "f", "F"):
if text.startswith(prefix):
text = text[len(prefix):]
break
# Then, strip the quotes from the remaining string.
return self._strip_quotes(text)|
dup of #2931 |
Checklist