From d63b636d87412b20edb913871d4579e7c8860c72 Mon Sep 17 00:00:00 2001 From: Aiman Malib <84276911+aimanmalib@users.noreply.github.com> Date: Fri, 6 Nov 2026 23:27:31 +0000 Subject: [PATCH] Add SARIF 2.1.0 output to review (-f sarif) for GitHub code scanning `crewforge review PATH -f sarif` writes one SARIF result per finding so CI can upload review findings to GitHub code scanning. Severity maps to level (critical/error -> error, warning -> warning, info -> note); the rule is the finding's CWE, else the reviewer. In a directory review the line a reviewer reports refers to the joined `# File:` text, so `Sources` now records where each file sits and maps the line back; unmappable lines keep the reviewed path without a region. Failed reviewers appear in `invocations`; exit codes are unchanged. Release 0.3.0. Closes #3 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 ++ CITATION.cff | 2 +- README.md | 30 ++++- pyproject.toml | 2 +- src/crewforge/__init__.py | 2 +- src/crewforge/recipes/review/cli.py | 27 +++- src/crewforge/recipes/review/report.py | 90 +++++++++++++- src/crewforge/recipes/review/sources.py | 36 +++++- tests/recipes/review/test_sarif.py | 159 ++++++++++++++++++++++++ uv.lock | 2 +- 10 files changed, 346 insertions(+), 15 deletions(-) create mode 100644 tests/recipes/review/test_sarif.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f4db4a2..9ee1b33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project uses [Semantic Versioning](https://semver.org/). +## [0.3.0] - 2026-11-07 + +### Added +- `crewforge review -f sarif`: a Static Analysis Results Interchange Format (`SARIF`) 2.1.0 report for GitHub code + scanning (#3). One result per finding, with `critical`/`error` as level `error`, `warning` as `warning` and `info` + as `note`; the rule is the finding's CWE, or the reviewer's name when there is none. In a directory review, a finding's line is mapped back to the file it came + from; a line that cannot be mapped keeps the reviewed path without a region. Failed reviewers are listed in the + run's `invocations`, and exit codes are unchanged. Inspired by scanaislop/aislop and agent-sh/agentsys, which offer + the same output for the same purpose; written from the published specification, with no new dependency. + ## [0.2.2] - 2026-11-06 Documentation only; no change to the code or to what the recipes do. @@ -87,6 +97,7 @@ First version: one framework replacing four separate tools ### Removed - apiforge's web dashboard and the illustrative token "stats"/daily estimates. +[0.3.0]: https://github.com/aimanmalib/crewforge/compare/v0.2.2...v0.3.0 [0.2.2]: https://github.com/aimanmalib/crewforge/compare/v0.2.1...v0.2.2 [0.2.1]: https://github.com/aimanmalib/crewforge/compare/v0.2.0...v0.2.1 [0.2.0]: https://github.com/aimanmalib/crewforge/compare/v0.1.0...v0.2.0 diff --git a/CITATION.cff b/CITATION.cff index fd25fc0..d06634e 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -5,7 +5,7 @@ type: software authors: - given-names: Muhammad family-names: Aiman -version: 0.2.2 +version: 0.3.0 license: MIT repository-code: "https://github.com/aimanmalib/crewforge" abstract: >- diff --git a/README.md b/README.md index 28be7b8..377e13d 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ tables are trimmed to their totals. | Command | Agents | What you get | |---|---|---| -| `review PATH` | security, style, complexity, logic, testing, documentation (parallel) → summary | Findings by severity, a decision and a score; `-f json` for CI | +| `review PATH` | security, style, complexity, logic, testing, documentation (parallel) → summary | Findings by severity, a decision and a score; `-f json` for CI, `-f sarif` for GitHub code scanning | | `ingest FILES` | ingester, indexer (no LLM) | How each file was parsed and chunked | | `ask FILES -q Q [--verify] [--rerank]` | retriever → answerer → fact-checker | An answer with `[Source N]` citations and a claim-by-claim verdict | | `summarize FILES` | summarizer | Summary and key points per document | @@ -119,6 +119,30 @@ $ crewforge review app.py -f markdown -o review.md # app.py builds SQL with total 7 calls 22,366 tokens (provider-reported) ``` +`-f sarif` writes a [Static Analysis Results Interchange Format 2.1.0](https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html) +log (`sarif`), the format GitHub code scanning reads, so findings can appear in a repository's Security tab. Each finding becomes one +result: `critical` and `error` map to level `error`, `warning` to `warning`, `info` to `note`; the rule is the +finding's CWE when it has one, otherwise the reviewer's name. When you review a directory, the line a reviewer +reports is mapped back to the file it came from. A finding whose line is missing or outside the code points at the +reviewed path without a line. Reviewers that fail are listed in `invocations`, and the exit code is the same as for +the other formats. + +```yaml +# .github/workflows/review.yml (needs an API key secret, e.g. OPENAI_API_KEY) +permissions: + contents: read + security-events: write +steps: + - uses: actions/checkout@v4 + - run: pipx run crewforge review src/ -f sarif -o crewforge.sarif + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + - uses: github/codeql-action/upload-sarif@v3 + if: always() + with: + sarif_file: crewforge.sarif +``` + ### `ask` and `summarize`

@@ -415,9 +439,13 @@ pip install crewforge # Python 3.10 hingga 3.14 pip install "crewforge[docs,api]" # pilihan: PDF/DOCX dan semakan aplikasi FastAPI export OPENAI_API_KEY=sk-... # atau penyedia lain, lihat `crewforge providers` crewforge review src/ +crewforge review src/ -f sarif -o crewforge.sarif # untuk GitHub code scanning crewforge academic feedback draf.md --brief taklimat.md --language ms ``` +`review -f sarif` menulis laporan `SARIF` 2.1.0 yang dibaca GitHub code scanning: setiap dapatan menjadi satu +keputusan dalam tab Security, dan baris dalam semakan folder dipetakan kembali ke fail asalnya. + Senarai penyedia, contoh setiap resipi dan seni bina ada dalam bahagian bahasa Inggeris di atas ([Providers](https://github.com/aimanmalib/crewforge#providers), [Recipes](https://github.com/aimanmalib/crewforge#recipes)). diff --git a/pyproject.toml b/pyproject.toml index 30ca010..c8debcc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "crewforge" -version = "0.2.2" +version = "0.3.0" description = "One crew of LLM agents, five recipes: code review, document Q&A, API scaffolding, content and honest academic-writing help — on any OpenAI-compatible provider." readme = "README.md" license = "MIT" diff --git a/src/crewforge/__init__.py b/src/crewforge/__init__.py index 3560360..582e0c9 100644 --- a/src/crewforge/__init__.py +++ b/src/crewforge/__init__.py @@ -1,3 +1,3 @@ """crewforge — one crew of LLM agents, five recipes, any OpenAI-compatible provider.""" -__version__ = "0.2.2" +__version__ = "0.3.0" diff --git a/src/crewforge/recipes/review/cli.py b/src/crewforge/recipes/review/cli.py index 0138508..40dff00 100644 --- a/src/crewforge/recipes/review/cli.py +++ b/src/crewforge/recipes/review/cli.py @@ -22,8 +22,14 @@ from ...core.config import LLMConfig from .agents import REVIEW_AGENTS from .pipeline import CodeReviewCrew, ReviewResult -from .report import report_dict, report_markdown -from .sources import DEFAULT_MAX_FILE_BYTES, SourceError, collect, detect_language +from .report import report_dict, report_markdown, report_sarif +from .sources import ( + DEFAULT_MAX_FILE_BYTES, + SourceError, + Sources, + collect, + detect_language, +) # Kept under the old private name: codeguard's tests and users imported it. _detect_language = detect_language @@ -60,7 +66,8 @@ async def _run_review( "fmt", default="text", show_default=True, - type=click.Choice(["text", "json", "markdown"]), + type=click.Choice(["text", "json", "markdown", "sarif"]), + help="sarif = SARIF 2.1.0 for GitHub code scanning.", ) @click.option("--output", "-o", default="", help="Write the report to this file.") @click.option( @@ -77,6 +84,7 @@ def review(ctx, path, path_opt, language, fmt, output, allow_partial): Example: crewforge review app.py crewforge review src/ -f json -o review.json + crewforge review src/ -f sarif -o review.sarif """ target = path or path_opt if not target: @@ -116,11 +124,18 @@ def review(ctx, path, path_opt, language, fmt, output, allow_partial): allow_partial=allow_partial, ) ) - _display_results(result, fmt, output) + _display_results(result, fmt, output, sources, target) -def _display_results(result: ReviewResult, fmt: str, output: str) -> None: - text = json.dumps(report_dict(result), indent=2) if fmt == "json" else report_markdown(result) +def _display_results( + result: ReviewResult, fmt: str, output: str, sources: Sources, target: str +) -> None: + if fmt == "json": + text = json.dumps(report_dict(result), indent=2) + elif fmt == "sarif": + text = json.dumps(report_sarif(result, sources, target), indent=2) + else: + text = report_markdown(result) if output: Path(output).write_text(text, encoding="utf-8") err_console.print(f"[green]Report written to {output}[/]") diff --git a/src/crewforge/recipes/review/report.py b/src/crewforge/recipes/review/report.py index aa27352..a65234b 100644 --- a/src/crewforge/recipes/review/report.py +++ b/src/crewforge/recipes/review/report.py @@ -1,11 +1,18 @@ -"""Render a :class:`ReviewResult` as JSON or Markdown.""" +"""Render a :class:`ReviewResult` as JSON, SARIF or Markdown.""" from __future__ import annotations +from pathlib import Path from typing import Any +from ... import __version__ from .agents import SEVERITIES from .pipeline import ReviewResult +from .sources import Sources + +SARIF_SCHEMA = "https://json.schemastore.org/sarif-2.1.0.json" +#: Finding severity -> SARIF ``level`` (GitHub code scanning shows error/warning/note). +SARIF_LEVELS = {"critical": "error", "error": "error", "warning": "warning", "info": "note"} def report_dict(result: ReviewResult) -> dict[str, Any]: @@ -101,3 +108,84 @@ def report_markdown(result: ReviewResult) -> str: f"Duration: {result.duration_s:.1f}s · Agents: {agents_run}_" ) return "\n".join(lines) + "\n" + + +def _uri(path: Path) -> str: + """Artifact URI: relative paths stay relative (GitHub resolves them from the repo root).""" + return path.as_uri() if path.is_absolute() else path.as_posix() + + +def _int_line(value: Any) -> int | None: + if isinstance(value, bool): + return None + if isinstance(value, int): + return value if value > 0 else None + if isinstance(value, str) and value.strip().isdigit(): + return int(value) or None + return None + + +def report_sarif(result: ReviewResult, sources: Sources, target: str | Path) -> dict[str, Any]: + """SARIF 2.1.0 log for GitHub code scanning: one ``result`` per finding. + + A finding's line refers to the text the reviewers saw. For a directory that + is every file joined under ``# File:`` headers, so the line is mapped back to + its file; a line that cannot be mapped keeps the reviewed path, without a region. + """ + results: list[dict[str, Any]] = [] + rules: dict[str, dict[str, Any]] = {} + for agent, r in result.agent_results.items(): + for f in r.findings: + rule_id = str(f.get("cwe") or agent) + rules.setdefault(rule_id, {"id": rule_id}) + title = str(f.get("title") or "Untitled") + message = f"{title}: {f['description']}" if f.get("description") else title + if f.get("suggestion"): + message += f"\nFix: {f['suggestion']}" + physical: dict[str, Any] = {"artifactLocation": {"uri": _uri(Path(target))}} + line = _int_line(f.get("line")) + spot = sources.locate(line) if line is not None else None + if spot is not None: + physical = { + "artifactLocation": {"uri": _uri(spot[0])}, + "region": {"startLine": spot[1]}, + } + results.append( + { + "ruleId": rule_id, + "level": SARIF_LEVELS.get(str(f.get("severity")), "note"), + "message": {"text": message}, + "locations": [{"physicalLocation": physical}], + "properties": {"agent": agent, "severity": f.get("severity", "info")}, + } + ) + return { + "$schema": SARIF_SCHEMA, + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "crewforge", + "version": __version__, + "informationUri": "https://github.com/aimanmalib/crewforge", + "rules": list(rules.values()), + } + }, + "invocations": [ + { + "executionSuccessful": not result.failed_agents, + "toolExecutionNotifications": [ + { + "level": "error", + "message": {"text": f"agent '{name}' failed: {r.error}"}, + } + for name, r in result.agent_results.items() + if not r.ok + ], + } + ], + "results": results, + } + ], + } diff --git a/src/crewforge/recipes/review/sources.py b/src/crewforge/recipes/review/sources.py index b44c03e..d925f50 100644 --- a/src/crewforge/recipes/review/sources.py +++ b/src/crewforge/recipes/review/sources.py @@ -52,6 +52,19 @@ class Sources: language: str files: list[Path] = field(default_factory=list) skipped: list[str] = field(default_factory=list) + #: ``(file, first, last)``: lines ``first..last`` of :attr:`code` hold line 1.. of ``file``. + spans: list[tuple[Path, int, int]] = field(default_factory=list) + + def locate(self, line: int) -> tuple[Path, int] | None: + """Map a line of :attr:`code` to ``(file, line in that file)``. + + ``None`` when the line is outside the code or falls on a ``# File:`` + header or the blank lines between two files. + """ + for path, first, last in self.spans: + if first <= line <= last: + return path, line - first + 1 + return None def collect(path: str | Path, *, max_file_bytes: int = DEFAULT_MAX_FILE_BYTES) -> Sources: @@ -60,13 +73,20 @@ def collect(path: str | Path, *, max_file_bytes: int = DEFAULT_MAX_FILE_BYTES) - code = target.read_text(encoding="utf-8", errors="replace") if not code.strip(): raise SourceError(f"{target} is empty") - return Sources(code=code, language=detect_language(target.suffix), files=[target]) + return Sources( + code=code, + language=detect_language(target.suffix), + files=[target], + spans=[(target, 1, _line_count(code))], + ) if not target.is_dir(): raise SourceError(f"path not found: {target}") parts: list[str] = [] files: list[Path] = [] skipped: list[str] = [] + spans: list[tuple[Path, int, int]] = [] + start = 1 # line of ``code`` where the next "# File:" header goes languages: set[str] = set() for f in sorted(target.rglob("*")): rel = f.relative_to(target) @@ -80,10 +100,20 @@ def collect(path: str | Path, *, max_file_bytes: int = DEFAULT_MAX_FILE_BYTES) - text = f.read_text(encoding="utf-8", errors="replace") if not text.strip(): continue - parts.append(f"# File: {rel}\n{text}") + part = f"# File: {rel}\n{text}" + parts.append(part) files.append(f) + spans.append((f, start + 1, start + _line_count(text))) + start += part.count("\n") + 2 # the part, then the blank line joining the next one languages.add(detect_language(f.suffix)) if not parts: raise SourceError(f"no supported source files found under {target}") language = languages.pop() if len(languages) == 1 else "" - return Sources(code="\n\n".join(parts), language=language, files=files, skipped=skipped) + return Sources( + code="\n\n".join(parts), language=language, files=files, skipped=skipped, spans=spans + ) + + +def _line_count(text: str) -> int: + """Lines in ``text`` as an editor numbers them (a final newline adds none).""" + return text.count("\n") + (0 if text.endswith("\n") else 1) diff --git a/tests/recipes/review/test_sarif.py b/tests/recipes/review/test_sarif.py new file mode 100644 index 0000000..8ec124c --- /dev/null +++ b/tests/recipes/review/test_sarif.py @@ -0,0 +1,159 @@ +"""`crewforge review -f sarif`: SARIF 2.1.0 for GitHub code scanning (issue #3).""" + +from __future__ import annotations + +import json +from pathlib import Path + +import httpx +import pytest +from click.testing import CliRunner + +from crewforge import __version__ +from crewforge.cli import main +from crewforge.recipes.review.agents import AgentResult +from crewforge.recipes.review.pipeline import ReviewResult +from crewforge.recipes.review.report import report_sarif +from crewforge.recipes.review.sources import collect +from tests.recipes.review.fakes import router + + +def _result(*findings: dict, failed: str | None = None) -> ReviewResult: + agents = {"security": AgentResult("security", findings=list(findings))} + if failed: + agents[failed] = AgentResult(failed, status="failed", error="HTTP 500") + return ReviewResult(agent_results=agents) + + +def _sarif(tmp_path: Path, *args: str): + src = tmp_path / "a.py" + src.write_text('import sqlite3\n\ndef find(conn, name):\n return conn.execute(f"{name}")\n') + out = tmp_path / "r.sarif" + res = CliRunner().invoke(main, ["review", str(src), "-f", "sarif", "-o", str(out), *args]) + return res, src, out + + +class TestCli: + def test_single_file_report_is_sarif_with_one_result_per_finding(self, fake_llm, tmp_path): + fake_llm.responder = router() + res, src, out = _sarif(tmp_path) + assert res.exit_code == 0, res.output + log = json.loads(out.read_text()) + assert log["version"] == "2.1.0" + assert log["$schema"].endswith("sarif-2.1.0.json") + run = log["runs"][0] + assert run["tool"]["driver"]["name"] == "crewforge" + assert run["tool"]["driver"]["version"] == __version__ + assert run["invocations"][0]["executionSuccessful"] is True + # The fake security reviewer reports one critical CWE-89 on line 4; the rest are clean. + [only] = run["results"] + assert only["ruleId"] == "CWE-89" + assert only["level"] == "error" + assert only["message"]["text"] == "SQL injection: query built with f-string" + loc = only["locations"][0]["physicalLocation"] + assert loc["artifactLocation"]["uri"] == src.as_uri() + assert loc["region"] == {"startLine": 4} + assert {"id": "CWE-89"} in run["tool"]["driver"]["rules"] + + def test_failed_reviewer_still_exits_1(self, fake_llm, tmp_path): + fake_llm.responder = router({"style": httpx.Response(500, json={"error": "boom"})}) + res, _, out = _sarif(tmp_path) + assert res.exit_code == 1 + assert "agent 'style'" in res.output + assert not out.exists() + + def test_allow_partial_marks_the_run_unsuccessful_and_names_the_agent(self, fake_llm, tmp_path): + fake_llm.responder = router({"style": httpx.Response(500, json={"error": "boom"})}) + res, _, out = _sarif(tmp_path, "--allow-partial") + assert res.exit_code == 0, res.output + inv = json.loads(out.read_text())["runs"][0]["invocations"][0] + assert inv["executionSuccessful"] is False + [note] = inv["toolExecutionNotifications"] + assert note["message"]["text"].startswith("agent 'style' failed") + + def test_sarif_to_stdout_is_plain_json(self, fake_llm, tmp_path): + fake_llm.responder = router() + src = tmp_path / "a.py" + src.write_text("x = 1\n") + res = CliRunner().invoke(main, ["review", str(src), "-f", "sarif"]) + assert res.exit_code == 0, res.output + start = res.output.index("{\n") + log, _ = json.JSONDecoder().raw_decode(res.output[start:]) + assert log["version"] == "2.1.0" + + +class TestReport: + @pytest.mark.parametrize( + ("severity", "level"), + [("critical", "error"), ("error", "error"), ("warning", "warning"), ("info", "note")], + ) + def test_severity_maps_to_level(self, tmp_path, severity, level): + src = tmp_path / "a.py" + src.write_text("x = 1\n") + log = report_sarif(_result({"severity": severity, "title": "t"}), collect(src), src) + assert log["runs"][0]["results"][0]["level"] == level + + def test_rule_is_the_agent_without_a_cwe_and_fix_is_in_the_message(self, tmp_path): + src = tmp_path / "a.py" + src.write_text("x = 1\n") + finding = {"severity": "warning", "title": "", "suggestion": "rename it"} + [r] = report_sarif(_result(finding), collect(src), src)["runs"][0]["results"] + assert r["ruleId"] == "security" + assert r["message"]["text"] == "Untitled\nFix: rename it" + + @pytest.mark.parametrize("line", [None, "", "4-6", 0, -1, True, "abc", 99]) + def test_line_that_is_not_a_line_of_the_file_has_no_region(self, tmp_path, line): + src = tmp_path / "a.py" + src.write_text("x = 1\ny = 2\n") + log = report_sarif(_result({"title": "t", "line": line}), collect(src), src) + loc = log["runs"][0]["results"][0]["locations"][0]["physicalLocation"] + assert loc == {"artifactLocation": {"uri": src.as_uri()}} + + def test_relative_paths_stay_relative(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + Path("src").mkdir() + Path("src/a.py").write_text("x = 1\ny = 2\n") + log = report_sarif(_result({"title": "t", "line": "2"}), collect("src/a.py"), "src/a.py") + loc = log["runs"][0]["results"][0]["locations"][0]["physicalLocation"] + assert loc == {"artifactLocation": {"uri": "src/a.py"}, "region": {"startLine": 2}} + + +class TestDirectoryLines: + """In a directory review the line refers to the joined text; map it back to the file.""" + + def _tree(self, root: Path) -> None: + (root / "pkg").mkdir() + (root / "a.py").write_text("a1 = 1\na2 = 2\n") + (root / "pkg" / "b.py").write_text("b1 = 1\nb2 = 2\nb3 = 3") # no final newline + (root / "c.py").write_text("c1 = 1\n") + + def test_every_code_line_maps_to_the_same_text_in_its_file(self, tmp_path): + self._tree(tmp_path) + sources = collect(tmp_path) + mapped = 0 + for n, text in enumerate(sources.code.split("\n"), start=1): + spot = sources.locate(n) + if text.startswith("# File:") or text == "": + assert spot is None, (n, text) + continue + path, line = spot # type: ignore[misc] + assert path.read_text().split("\n")[line - 1] == text + mapped += 1 + assert mapped == 6 + + def test_finding_in_second_file_points_at_that_file(self, tmp_path): + self._tree(tmp_path) + sources = collect(tmp_path) + n = sources.code.split("\n").index("b2 = 2") + 1 + log = report_sarif(_result({"title": "t", "line": n}), sources, tmp_path) + loc = log["runs"][0]["results"][0]["locations"][0]["physicalLocation"] + assert loc == { + "artifactLocation": {"uri": (tmp_path / "pkg" / "b.py").as_uri()}, + "region": {"startLine": 2}, + } + + def test_finding_on_a_header_keeps_the_directory_without_region(self, tmp_path): + self._tree(tmp_path) + log = report_sarif(_result({"title": "t", "line": 1}), collect(tmp_path), tmp_path) + loc = log["runs"][0]["results"][0]["locations"][0]["physicalLocation"] + assert loc == {"artifactLocation": {"uri": tmp_path.as_uri()}} diff --git a/uv.lock b/uv.lock index a949222..7c85a63 100644 --- a/uv.lock +++ b/uv.lock @@ -275,7 +275,7 @@ toml = [ [[package]] name = "crewforge" -version = "0.2.2" +version = "0.3.0" source = { editable = "." } dependencies = [ { name = "click" },