Conversation
m-y-mo
requested review from
JarLob,
kevinbackhouse,
p- and
sylwia-budzynska
as code owners
There was a problem hiding this comment.
Pull request overview
Adds a new audit taskflow to identify “low severity” vulnerabilities and introduces persistence + MCP tooling to record reasons for classifying an audit result as low severity.
Changes:
- Add
filter_severity.yamltaskflow to review vulnerable audit results against “low severity” criteria and store a reason when applicable. - Introduce
LowSeverityAuditResultSQLAlchemy model/table to persist low-severity classification reasons. - Extend repo-context MCP server to include
result_idin returned audit results and add astore_low_severity_reasonMCP tool.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/seclab_taskflows/taskflows/audit/filter_severity.yaml |
New taskflow to triage vulnerable issues and (optionally) store low-severity reasons. |
src/seclab_taskflows/mcp_servers/repo_context_models.py |
Adds LowSeverityAuditResult ORM model/table. |
src/seclab_taskflows/mcp_servers/repo_context.py |
Creates the new table, exposes a new MCP tool to store reasons, and adds result_id to audit result payloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with Session(self.engine) as session: | ||
| existing = session.query(LowSeverityAuditResult).filter_by(repo=repo, result_id=result_id).first() | ||
| if existing: | ||
| existing.reason_for_rejection += reason |
There was a problem hiding this comment.
store_low_severity_reason updates existing.reason_for_rejection, but LowSeverityAuditResult defines the column as reason. This will raise an AttributeError at runtime and prevent saving updates. Use the correct attribute (and consider adding a separator like a newline if appending multiple reasons).
| existing.reason_for_rejection += reason | |
| if existing.reason: | |
| existing.reason = f"{existing.reason}\n{reason}" | |
| else: | |
| existing.reason = reason |
| Base.metadata.create_all( | ||
| self.engine, | ||
| tables=[ | ||
| Application.__table__, | ||
| EntryPoint.__table__, | ||
| UserAction.__table__, | ||
| WebEntryPoint.__table__, | ||
| ApplicationIssue.__table__, | ||
| AuditResult.__table__, | ||
| LowSeverityAuditResult.__table__, | ||
| ], |
There was a problem hiding this comment.
clear_repo() currently deletes rows from AuditResult but not from the new low_severity_audit_result table. Because this is SQLite and bulk deletes are used, relying on ON DELETE CASCADE may not actually remove low-severity rows, so repo cleanup can leave orphan/old data behind. Consider explicitly deleting LowSeverityAuditResult rows for the repo (or enabling/enforcing FK cascades consistently).
| "repo": app.repo, | ||
| "issue_type": issue.issue_type, | ||
| "issue_id": issue.issue_id, | ||
| "result_id" : issue.id, |
There was a problem hiding this comment.
There’s a formatting inconsistency in the returned dict key ("result_id" : issue.id has an extra space before the colon). This is minor, but keeping key formatting consistent improves readability and reduces noisy diffs later.
| "result_id" : issue.id, | |
| "result_id": issue.id, |
| store a low severity reason for the result with the id {{ result.result_id }}. Otherwise, | ||
| you can finish the task. | ||
|
|
||
| DO NOT change or store anything for the current audit result. |
There was a problem hiding this comment.
The task prompt instructs the agent to store a low-severity reason (and even names the result id), but later says "DO NOT change or store anything for the current audit result." This is contradictory and may prevent the intended store_low_severity_reason call. Clarify that the agent should not modify the AuditResult itself, but should store a low-severity reason record when applicable.
| DO NOT change or store anything for the current audit result. | |
| Do not modify the existing AuditResult record itself (for example, do not change its severity, status, or notes); only store a separate low-severity reason record for the current audit result when applicable. |
No description provided.