Add pub-dev-search skill - #3
Conversation
Search pub.dev for Dart/Flutter packages and recommend based on quality score, popularity, likes, download counts, and publication recency.
Summary of ChangesHello @csells, 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 integrates a new Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new pub-dev-search skill, which includes a Python script for querying the pub.dev API. While the implementation is well-structured, two critical security concerns have been identified: a potential command injection vulnerability in the skill's usage template in SKILL.md and a risk of indirect prompt injection in the handling of untrusted package descriptions in search_pub_dev.py. Additionally, there are opportunities to improve the skill's robustness and performance through comprehensive error handling for network requests, concurrent API calls, and more robust tag parsing against malformed data.
| Run the search script with the user's query: | ||
|
|
||
| ```bash | ||
| python3 <skill-dir>/scripts/search_pub_dev.py "<query>" -n <count> |
There was a problem hiding this comment.
The usage template in the SKILL.md file is vulnerable to command injection. The <query> placeholder is placed directly inside double quotes in a shell command context: python3 <skill-dir>/scripts/search_pub_dev.py "<query>" -n <count>. If the agent executor replaces this placeholder with unsanitized user input and executes the resulting string in a shell (e.g., using os.system or subprocess.run(shell=True)), an attacker can escape the double quotes and execute arbitrary commands on the host system. For example, a query like "; touch /tmp/pwned; " would result in the execution of touch /tmp/pwned. To remediate this, the skill definition should avoid suggesting shell-style command construction with string interpolation, or the agent framework should be configured to pass arguments safely (e.g., as a list to subprocess.run with shell=False).
| def get_package_score(package_name): | ||
| """Get the score/metrics for a package.""" | ||
| url = f"{BASE_URL}/packages/{urllib.parse.quote(package_name)}/metrics" | ||
| req = urllib.request.Request(url, headers={"Accept": "application/json"}) | ||
| with urllib.request.urlopen(req) as resp: | ||
| return json.loads(resp.read().decode()) |
There was a problem hiding this comment.
The script currently doesn't handle network errors (e.g., timeouts, HTTP errors like 404 or 500) when making API requests to pub.dev. An unhandled error will cause the script to crash. It's important to wrap these requests in try...except blocks to gracefully handle failures. Also, it's good practice to set a User-Agent header and a timeout for requests.
This same feedback applies to the search_packages and get_package_info functions.
| def get_package_score(package_name): | |
| """Get the score/metrics for a package.""" | |
| url = f"{BASE_URL}/packages/{urllib.parse.quote(package_name)}/metrics" | |
| req = urllib.request.Request(url, headers={"Accept": "application/json"}) | |
| with urllib.request.urlopen(req) as resp: | |
| return json.loads(resp.read().decode()) | |
| def get_package_score(package_name): | |
| """Get the score/metrics for a package.""" | |
| url = f"{BASE_URL}/packages/{urllib.parse.quote(package_name)}/metrics" | |
| req = urllib.request.Request( | |
| url, headers={"Accept": "application/json", "User-Agent": "pub-dev-search-skill/1.0"} | |
| ) | |
| try: | |
| with urllib.request.urlopen(req, timeout=15) as resp: | |
| return json.loads(resp.read().decode()) | |
| except (urllib.error.URLError, urllib.error.HTTPError) as e: | |
| print(f"Warning: Could not fetch score for {package_name}: {e}", file=sys.stderr) | |
| return {} |
| ) | ||
| # Print description on next line, truncated | ||
| desc = p["description"][:100] + ("..." if len(p["description"]) > 100 else "") | ||
| print(f" {desc}") |
There was a problem hiding this comment.
The script prints package descriptions directly from the pub.dev API to stdout. These descriptions are untrusted and could contain malicious instructions designed to manipulate the LLM's behavior (Indirect Prompt Injection). While the script already truncates the description to 100 characters, this does not fully eliminate the risk. Consider wrapping untrusted content in clear delimiters (e.g., [START OF UNTRUSTED CONTENT] ... [END OF UNTRUSTED CONTENT]) and providing instructions to the LLM to treat content within those delimiters as potentially untrusted.
| for pkg in packages: | ||
| name = pkg["package"] | ||
| info = get_package_info(name) | ||
| metrics = get_package_score(name) |
There was a problem hiding this comment.
The script currently fetches details for each package sequentially within a for loop. This results in 2 * N sequential network requests, which can be very slow if N is large. To significantly improve performance, consider fetching the details for all packages concurrently.
You can use concurrent.futures.ThreadPoolExecutor to parallelize the API calls. For example, you could define a function that fetches both info and metrics for a single package, and then use a ThreadPoolExecutor to run this function for all packages in parallel.
| "publisher": next( | ||
| ( | ||
| t.split(":")[1] | ||
| for t in tags | ||
| if t.startswith("publisher:") | ||
| ), | ||
| "", | ||
| ), | ||
| "quality_points": f"{granted}/{max_pts}", | ||
| "quality_pct": quality_pct, | ||
| "likes": likes, | ||
| "downloads_30d": downloads_30d, | ||
| "published_date": pub_date, | ||
| "published_age": pub_age, | ||
| "flutter_favorite": is_flutter_fav, | ||
| "dart3_compatible": is_dart3, | ||
| "sdks": [ | ||
| t.split(":")[1] for t in tags if t.startswith("sdk:") | ||
| ], | ||
| "platforms": [ | ||
| t.split(":")[1] for t in tags if t.startswith("platform:") | ||
| ], |
There was a problem hiding this comment.
The current code for parsing tags like publisher, sdks, and platforms could raise an IndexError if a tag is malformed (e.g., publisher: without a value). To make this more robust, it's safer to check for the existence of a colon before splitting and accessing the value. Using t.split(':', 1) is also safer than t.split(':') as it only splits on the first occurrence.
"publisher": next(
(
t.split(":", 1)[1]
for t in tags
if t.startswith("publisher:") and ":" in t
),
"",
),
"quality_points": f"{granted}/{max_pts}",
"quality_pct": quality_pct,
"likes": likes,
"downloads_30d": downloads_30d,
"published_date": pub_date,
"published_age": pub_age,
"flutter_favorite": is_flutter_fav,
"dart3_compatible": is_dart3,
"sdks": [
t.split(":", 1)[1] for t in tags if t.startswith("sdk:") and ":" in t
],
"platforms": [
t.split(":", 1)[1] for t in tags if t.startswith("platform:") and ":" in t
],1 participant
Summary
pub-dev-searchskill that searches pub.dev for Dart/Flutter packages and provides recommendationsTest plan
state management,http client,json)