-
Notifications
You must be signed in to change notification settings - Fork 21
use dedicated database for advisories #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
69994dd
000a3c6
6e42a64
eb3656c
5f8057b
bb4403b
abba1f5
a678b71
d6e1866
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,3 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # SPDX-FileCopyrightText: GitHub, Inc. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # SPDX-License-Identifier: MIT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from fastmcp import FastMCP | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -6,7 +9,12 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from urllib.parse import urlparse, parse_qs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from .gh_code_scanning import call_api | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from seclab_taskflow_agent.path_utils import log_file_name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from seclab_taskflow_agent.path_utils import mcp_data_dir, log_file_name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from .ghsa_models import GHSA, GHSASummary, Base | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy import create_engine | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Session | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from .utils import process_repo | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logging.basicConfig( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| level=logging.DEBUG, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -17,17 +25,153 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mcp = FastMCP("GitHubRepoAdvisories") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MEMORY = mcp_data_dir("seclab-taskflows", "ghsa", "GHSA_DIR") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def ghsa_to_dict(result): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "id": result.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "ghsa_id": result.ghsa_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "repo": result.repo.lower(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "severity": result.severity, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "cve_id": result.cve_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "description": result.description, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "summary": result.summary, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "published_at": result.published_at, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "state": result.state, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def ghsa_summary_to_dict(summary): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "id": summary.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "repo": summary.repo.lower(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "total_advisories": summary.total_advisories, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "high_severity_count": summary.high_severity_count, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "medium_severity_count": summary.medium_severity_count, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "low_severity_count": summary.low_severity_count, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "summary_notes": summary.summary_notes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class GHSABackend: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, db_dir: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Directory in which the GHSA SQLite database file will be stored. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.db_dir = db_dir | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not Path(self.db_dir).exists(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db_uri = "sqlite://" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db_uri = f"sqlite:///{self.db_dir}/ghsa.db" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not Path(self.db_dir).exists(): | |
| db_uri = "sqlite://" | |
| else: | |
| db_uri = f"sqlite:///{self.db_dir}/ghsa.db" | |
| db_dir_path = Path(self.db_dir) | |
| try: | |
| db_dir_path.mkdir(parents=True, exist_ok=True) | |
| except OSError as e: | |
| logging.error("Failed to create GHSA database directory '%s': %s", self.db_dir, e) | |
| raise RuntimeError(f"Cannot create GHSA database directory: {self.db_dir}") from e | |
| db_path = db_dir_path / "ghsa.db" | |
| db_uri = f"sqlite:///{db_path}" |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
store_new_ghsa_summary appends to summary_notes even on the "store" path (existing.summary_notes = ... + ...). This makes add_ghsa_summary_notes largely redundant and can cause unbounded growth / stale errors being carried forward. Consider making store_new_ghsa_summary overwrite summary_notes and keeping all appending behavior in add_ghsa_summary_notes (ideally with a delimiter like a newline).
| existing.summary_notes = (existing.summary_notes or "") + (summary_notes or "") | |
| existing.summary_notes = summary_notes |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetch_and_store_GHSA_list returns raw error strings from fetch_GHSA_list_from_gh (e.g., "Request error: ...") without normalizing them. Since the taskflow prompt requires errors to start with "Error:", consider wrapping/standardizing error returns here (and/or in fetch_GHSA_list_from_gh) so downstream prompts can reliably detect failures.
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetch_and_store_GHSA_list returns raw error strings from fetch_GHSA_list_from_gh unchanged. Those errors currently come back as e.g. "Request error: ..." / "HTTP error: ..." (no leading "Error:"), which conflicts with the updated taskflow prompt that requires error messages to start with "Error:". Consider normalizing error returns here (and/or in fetch_GHSA_list_from_gh) to consistently prefix errors with "Error:".
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetch_and_store_GHSA_list calls backend.store_new_ghsa in a loop, and store_new_ghsa opens a new SQLAlchemy Session and commits per advisory. This is likely to be slow for repos with many advisories. Prefer doing the upsert work in a single session/transaction for the whole batch.
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetch_and_store_GHSA_list stores each advisory by calling backend.store_new_ghsa(...) inside a loop, which opens a new Session and commits for every row. This is unnecessarily expensive for large advisory lists; prefer using a single Session/transaction for the whole batch (and commit once), or add a backend method that upserts the whole list in one go.
| for advisory in results: | |
| backend.store_new_ghsa( | |
| process_repo(owner, repo), | |
| advisory["ghsa_id"], | |
| advisory["severity"], | |
| advisory["cve_id"], | |
| advisory["description"], | |
| advisory["summary"], | |
| advisory["published_at"], | |
| advisory["state"], | |
| ) | |
| # Use a single Session/transaction for the whole batch to avoid per-row session/commit overhead. | |
| repo_identifier = process_repo(owner, repo) | |
| with Session(bind=engine) as session: | |
| for advisory in results: | |
| ghsa_obj = GHSA( | |
| repo=repo_identifier, | |
| ghsa_id=advisory["ghsa_id"], | |
| severity=advisory["severity"], | |
| cve_id=advisory["cve_id"], | |
| description=advisory["description"], | |
| summary=advisory["summary"], | |
| published_at=advisory["published_at"], | |
| state=advisory["state"], | |
| ) | |
| # merge provides upsert-like behaviour if a matching row already exists | |
| session.merge(ghsa_obj) | |
| session.commit() |
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetch_and_store_GHSA_list stores new GHSA rows but never clears existing rows for the repo. If the repo’s advisories are removed/withdrawn, or if a fetch returns "No advisories found." / an error, stale advisories (and any previous summary) will remain in the DB and be treated as current by downstream prompts. Consider clearing existing GHSA/GHSASummary for the repo at the start of this tool (or explicitly handling the no-advisories/error cases by clearing/updating the summary).
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_ghsa_with_id_from_db appears to be functionally identical to get_ghsa_from_db (same parameters and behavior). Consider removing one, or making one an alias/wrapper, to avoid divergence and extra maintenance.
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_ghsa_summary_notes is documented/named as an append operation, but it overwrites summary_notes by calling store_new_ghsa_summary(..., summary_notes) without combining with the existing notes. This makes it impossible to accumulate notes and is inconsistent with the tool name/docstring. Consider appending to existing["summary_notes"] (with a separator) rather than replacing it.
| return backend.store_new_ghsa_summary( | |
| repo_name, | |
| existing["total_advisories"], | |
| existing["high_severity_count"], | |
| existing["medium_severity_count"], | |
| existing["low_severity_count"], | |
| summary_notes, | |
| # Combine existing summary notes with the new notes, if any. | |
| existing_notes = existing.get("summary_notes") or "" | |
| if summary_notes: | |
| if existing_notes: | |
| combined_notes = existing_notes + "\n" + summary_notes | |
| else: | |
| combined_notes = summary_notes | |
| else: | |
| combined_notes = existing_notes | |
| return backend.store_new_ghsa_summary( | |
| repo_name, | |
| existing["total_advisories"], | |
| existing["high_severity_count"], | |
| existing["medium_severity_count"], | |
| existing["low_severity_count"], | |
| combined_notes, |
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_ghsa_summary_notes is documented/named as appending notes, but it overwrites summary_notes by passing only the new notes into store_new_ghsa_summary. Either actually append to the existing summary_notes value (e.g., concatenate with a separator) or rename/update the docstring to reflect overwrite behavior.
| return backend.store_new_ghsa_summary( | |
| repo_name, | |
| existing["total_advisories"], | |
| existing["high_severity_count"], | |
| existing["medium_severity_count"], | |
| existing["low_severity_count"], | |
| summary_notes, | |
| existing_notes = existing.get("summary_notes") or "" | |
| new_notes = summary_notes or "" | |
| if existing_notes and new_notes: | |
| combined_notes = existing_notes.rstrip() + "\n\n" + new_notes.lstrip() | |
| elif new_notes: | |
| combined_notes = new_notes | |
| else: | |
| combined_notes = existing_notes | |
| return backend.store_new_ghsa_summary( | |
| repo_name, | |
| existing["total_advisories"], | |
| existing["high_severity_count"], | |
| existing["medium_severity_count"], | |
| existing["low_severity_count"], | |
| combined_notes, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # SPDX-FileCopyrightText: GitHub, Inc. | ||
| # SPDX-License-Identifier: MIT | ||
|
|
||
| from sqlalchemy import Text | ||
| from sqlalchemy.orm import DeclarativeBase, mapped_column, Mapped | ||
| from typing import Optional | ||
|
|
||
|
|
||
| class Base(DeclarativeBase): | ||
| pass | ||
|
|
||
| class GHSA(Base): | ||
| __tablename__ = "ghsa" | ||
|
|
||
| id: Mapped[int] = mapped_column(primary_key=True) | ||
| ghsa_id: Mapped[str] | ||
| repo: Mapped[str] | ||
| severity: Mapped[str] | ||
| cve_id: Mapped[Optional[str]] = mapped_column(nullable=True) | ||
| description: Mapped[Optional[str]] = mapped_column(Text, nullable=True) | ||
| summary: Mapped[Optional[str]] = mapped_column(Text, nullable=True) | ||
| published_at: Mapped[Optional[str]] = mapped_column(nullable=True) | ||
| state: Mapped[Optional[str]] = mapped_column(nullable=True) | ||
|
|
||
| def __repr__(self): | ||
| return ( | ||
| f"<GHSA(id={self.id}, ghsa_id={self.ghsa_id}, repo={self.repo}, " | ||
| f"severity={self.severity}, cve_id={self.cve_id}, description={self.description}, summary={self.summary}, " | ||
| f"published_at={self.published_at}, state={self.state})>" | ||
| ) | ||
|
|
||
| class GHSASummary(Base): | ||
| __tablename__ = "ghsa_summary" | ||
|
Comment on lines
+9
to
+33
|
||
|
|
||
| id: Mapped[int] = mapped_column(primary_key=True) | ||
| repo: Mapped[str] | ||
| total_advisories: Mapped[int] | ||
| high_severity_count: Mapped[int] | ||
| medium_severity_count: Mapped[int] | ||
| low_severity_count: Mapped[int] | ||
| summary_notes: Mapped[Optional[str]] = mapped_column(Text, nullable=True) | ||
|
|
||
| def __repr__(self): | ||
| return ( | ||
| f"<GHSASummary(id={self.id}, repo={self.repo}, total_advisories={self.total_advisories}, " | ||
| f"high_severity_count={self.high_severity_count}, medium_severity_count={self.medium_severity_count}, " | ||
| f"low_severity_count={self.low_severity_count}, summary_notes={self.summary_notes})>" | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file appears to lack the standard SPDX copyright/license header used by other modules in
src/seclab_taskflows/mcp_servers/. Consider adding the SPDX header above the imports to keep licensing metadata consistent.