diff --git a/CHANGELOG.md b/CHANGELOG.md index 35124a4..15916ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## [Unreleased] +### 수정 +- SARIF 출력 견고성: (1) `startLine`을 방어적으로 coerce합니다 — 외부 엔진(Trivy 등)이 `"12-14"`·`"n/a"` 같은 비정수 line을 내면 `int()`가 던져 리포트 전체가 크래시했습니다(불량 finding 1개 → 리포트 전멸). (2) 공백만 있는 message의 shortDescription 추출 시 IndexError를 방지합니다. (3) `ruleIndex` 계산을 O(n²)에서 O(1)로 바꿨습니다(대량 finding 성능). + ### 추가 - `appguardrail fix` 명령 — 안전하고 결정적인 자동 수정을 적용합니다(기본 dry-run diff, `--apply`로 기록). 의미를 바꾸지 않는 순수 additive 변환만 수행하며, 첫 변환으로 외부 `target="_blank"` 링크에 `rel="noopener noreferrer"`를 추가합니다(reverse tabnabbing 방지). 동작을 바꾸는 수정(시크릿→env 등)은 위험하므로 자동 적용하지 않고 fix-pack 프롬프트로 남깁니다. scan→fix→verify 루프를 안전하게 닫습니다. diff --git a/appguardrail_core/sarif.py b/appguardrail_core/sarif.py index 70c10e1..c32e79a 100644 --- a/appguardrail_core/sarif.py +++ b/appguardrail_core/sarif.py @@ -32,6 +32,24 @@ def _tags(finding: dict[str, Any]) -> list[str]: return tags +def _safe_line(value: Any) -> int: + """SARIF startLine must be a positive int; external tools emit ranges + ("12-14") or "n/a", so coerce defensively — one bad line must not sink + the whole report.""" + try: + return max(1, int(value)) + except (ValueError, TypeError): + return 1 + + +def _first_line(message: str) -> str: + """First non-empty line of a message, capped; never IndexErrors on blanks.""" + for line in message.splitlines(): + if line.strip(): + return line.strip()[:200] + return message.strip()[:200] or "Security finding" + + def findings_to_sarif( findings: Iterable[dict[str, Any]], *, tool_version: str = "0.0.0" ) -> dict[str, Any]: @@ -45,10 +63,11 @@ def findings_to_sarif( severity = f["severity"] refs = f.get("references") or () if rule_id not in rules: + rule_index = len(rules) rule: dict[str, Any] = { "id": rule_id, "name": rule_id, - "shortDescription": {"text": f["message"].strip().splitlines()[0][:200]}, + "shortDescription": {"text": _first_line(f["message"])}, "fullDescription": {"text": f["message"].strip()}, "helpUri": refs[0] if refs else "https://github.com/ContextualWisdomLab/appguardrail", "defaultConfiguration": {"level": _LEVEL.get(severity, "note")}, @@ -57,19 +76,20 @@ def findings_to_sarif( "security-severity": _SECURITY_SEVERITY.get(severity, "2.0"), }, } + rule["_index"] = rule_index rules[rule_id] = rule results.append( { "ruleId": rule_id, - "ruleIndex": list(rules).index(rule_id), + "ruleIndex": rules[rule_id]["_index"], "level": _LEVEL.get(severity, "note"), "message": {"text": f["message"].strip()}, "locations": [ { "physicalLocation": { "artifactLocation": {"uri": f["file"]}, - "region": {"startLine": max(1, int(f["line"] or 1))}, + "region": {"startLine": _safe_line(f["line"])}, } } ], @@ -96,7 +116,7 @@ def findings_to_sarif( "name": "AppGuardrail", "informationUri": "https://github.com/ContextualWisdomLab/appguardrail", "version": tool_version, - "rules": list(rules.values()), + "rules": [{k: v for k, v in r.items() if k != "_index"} for r in rules.values()], } }, "results": results, diff --git a/tests/test_sarif.py b/tests/test_sarif.py index d989019..904b3ea 100644 --- a/tests/test_sarif.py +++ b/tests/test_sarif.py @@ -54,3 +54,40 @@ def test_empty_findings_valid(): run = findings_to_sarif([])["runs"][0] assert run["results"] == [] assert run["tool"]["driver"]["rules"] == [] + + +# ---- robustness: one malformed finding must not sink the report ---- + +def test_non_integer_line_does_not_crash(): + from appguardrail_core.sarif import findings_to_sarif + log = findings_to_sarif([ + {"severity": "HIGH", "rule_id": "trivy-range", "message": "range", + "file": "a.tf", "line": "12-14", "context": "app-code"}, + {"severity": "INFO", "rule_id": "na", "message": "x", + "file": "b", "line": "n/a", "context": "doc"}, + ]) + regions = [r["locations"][0]["physicalLocation"]["region"]["startLine"] + for r in log["runs"][0]["results"]] + assert regions == [1, 1] # coerced, not crashed + + +def test_blank_message_gets_fallback_short_description(): + from appguardrail_core.sarif import findings_to_sarif + log = findings_to_sarif([ + {"severity": "HIGH", "rule_id": "blank", "message": " ", + "file": "a", "line": 3, "context": "app-code"}, + ]) + rule = log["runs"][0]["tool"]["driver"]["rules"][0] + assert rule["shortDescription"]["text"] # non-empty, no IndexError + + +def test_private_index_not_leaked_and_ruleindex_correct(): + from appguardrail_core.sarif import findings_to_sarif + log = findings_to_sarif([ + {"severity": "HIGH", "rule_id": "a", "message": "m", "file": "f", "line": 1, "context": "app-code"}, + {"severity": "HIGH", "rule_id": "b", "message": "m", "file": "f", "line": 2, "context": "app-code"}, + {"severity": "HIGH", "rule_id": "a", "message": "m", "file": "f", "line": 3, "context": "app-code"}, + ]) + run = log["runs"][0] + assert all("_index" not in r for r in run["tool"]["driver"]["rules"]) + assert [r["ruleIndex"] for r in run["results"]] == [0, 1, 0] # deduped, stable