Skip to content

Commit cbe5f6a

Browse files
data-douserdata-douser
committed
Extend client tests -> sarif_diff_by_commits tool
1 parent be5096d commit cbe5f6a

File tree

12 files changed

+335
-14
lines changed

12 files changed

+335
-14
lines changed

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/line_level_classification/README.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Integration Test: sarif_diff_by_commits - line_level_classification
2+
3+
## Purpose
4+
5+
Validates that the `sarif_diff_by_commits` tool correctly handles
6+
line-level granularity classification. Uses `HEAD..HEAD` (empty diff)
7+
so all results are classified as pre-existing regardless of their
8+
line positions.
9+
10+
## Inputs
11+
12+
- `results.sarif`: SARIF with 3 results across 2 rules in 3 files
13+
- `refRange`: `HEAD..HEAD` (produces an empty diff)
14+
- `granularity`: `line`
15+
16+
## Expected Behavior
17+
18+
Returns structured output with `"granularity": "line"`, all 3 results
19+
in `preExistingResults`, and 0 results in `newResults`, since the
20+
empty diff has no changed files or hunks to match against.

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/line_level_classification/after/monitoring-state.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"toolName": "sarif_diff_by_commits",
3+
"success": true,
4+
"description": "Successfully classified all results as pre-existing with empty diff (line granularity)"
5+
}

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/line_level_classification/after/results.sarif‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
{
2+
"version": "2.1.0",
3+
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
4+
"runs": [
5+
{
6+
"tool": {
7+
"driver": {
8+
"name": "CodeQL",
9+
"version": "2.20.4",
10+
"rules": [
11+
{
12+
"id": "js/sql-injection",
13+
"name": "js/sql-injection",
14+
"shortDescription": {
15+
"text": "Database query built from user-controlled sources"
16+
},
17+
"properties": {
18+
"tags": ["security"],
19+
"kind": "path-problem",
20+
"precision": "high",
21+
"security-severity": "8.8"
22+
}
23+
},
24+
{
25+
"id": "js/xss",
26+
"name": "js/xss",
27+
"shortDescription": {
28+
"text": "Cross-site scripting"
29+
},
30+
"properties": {
31+
"tags": ["security"],
32+
"kind": "path-problem",
33+
"precision": "high",
34+
"security-severity": "6.1"
35+
}
36+
}
37+
]
38+
}
39+
},
40+
"results": [
41+
{
42+
"ruleId": "js/sql-injection",
43+
"ruleIndex": 0,
44+
"message": {
45+
"text": "SQL injection from user input."
46+
},
47+
"locations": [
48+
{
49+
"physicalLocation": {
50+
"artifactLocation": {
51+
"uri": "src/db.js"
52+
},
53+
"region": {
54+
"startLine": 42,
55+
"startColumn": 5,
56+
"endColumn": 38
57+
}
58+
}
59+
}
60+
]
61+
},
62+
{
63+
"ruleId": "js/sql-injection",
64+
"ruleIndex": 0,
65+
"message": {
66+
"text": "SQL injection from request body."
67+
},
68+
"locations": [
69+
{
70+
"physicalLocation": {
71+
"artifactLocation": {
72+
"uri": "src/api.js"
73+
},
74+
"region": {
75+
"startLine": 15,
76+
"startColumn": 3,
77+
"endColumn": 40
78+
}
79+
}
80+
}
81+
]
82+
},
83+
{
84+
"ruleId": "js/xss",
85+
"ruleIndex": 1,
86+
"message": {
87+
"text": "XSS vulnerability."
88+
},
89+
"locations": [
90+
{
91+
"physicalLocation": {
92+
"artifactLocation": {
93+
"uri": "src/views.js"
94+
},
95+
"region": {
96+
"startLine": 30,
97+
"startColumn": 10,
98+
"endColumn": 50
99+
}
100+
}
101+
}
102+
]
103+
}
104+
]
105+
}
106+
]
107+
}

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/line_level_classification/before/monitoring-state.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"toolName": "sarif_diff_by_commits",
3+
"expectedSuccess": true,
4+
"description": "Test sarif_diff_by_commits line-level classification with empty diff"
5+
}

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/line_level_classification/before/results.sarif‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
{
2+
"version": "2.1.0",
3+
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
4+
"runs": [
5+
{
6+
"tool": {
7+
"driver": {
8+
"name": "CodeQL",
9+
"version": "2.20.4",
10+
"rules": [
11+
{
12+
"id": "js/sql-injection",
13+
"name": "js/sql-injection",
14+
"shortDescription": {
15+
"text": "Database query built from user-controlled sources"
16+
},
17+
"properties": {
18+
"tags": ["security"],
19+
"kind": "path-problem",
20+
"precision": "high",
21+
"security-severity": "8.8"
22+
}
23+
},
24+
{
25+
"id": "js/xss",
26+
"name": "js/xss",
27+
"shortDescription": {
28+
"text": "Cross-site scripting"
29+
},
30+
"properties": {
31+
"tags": ["security"],
32+
"kind": "path-problem",
33+
"precision": "high",
34+
"security-severity": "6.1"
35+
}
36+
}
37+
]
38+
}
39+
},
40+
"results": [
41+
{
42+
"ruleId": "js/sql-injection",
43+
"ruleIndex": 0,
44+
"message": {
45+
"text": "SQL injection from user input."
46+
},
47+
"locations": [
48+
{
49+
"physicalLocation": {
50+
"artifactLocation": {
51+
"uri": "src/db.js"
52+
},
53+
"region": {
54+
"startLine": 42,
55+
"startColumn": 5,
56+
"endColumn": 38
57+
}
58+
}
59+
}
60+
]
61+
},
62+
{
63+
"ruleId": "js/sql-injection",
64+
"ruleIndex": 0,
65+
"message": {
66+
"text": "SQL injection from request body."
67+
},
68+
"locations": [
69+
{
70+
"physicalLocation": {
71+
"artifactLocation": {
72+
"uri": "src/api.js"
73+
},
74+
"region": {
75+
"startLine": 15,
76+
"startColumn": 3,
77+
"endColumn": 40
78+
}
79+
}
80+
}
81+
]
82+
},
83+
{
84+
"ruleId": "js/xss",
85+
"ruleIndex": 1,
86+
"message": {
87+
"text": "XSS vulnerability."
88+
},
89+
"locations": [
90+
{
91+
"physicalLocation": {
92+
"artifactLocation": {
93+
"uri": "src/views.js"
94+
},
95+
"region": {
96+
"startLine": 30,
97+
"startColumn": 10,
98+
"endColumn": 50
99+
}
100+
}
101+
}
102+
]
103+
}
104+
]
105+
}
106+
]
107+
}

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/line_level_classification/test-config.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"toolName": "sarif_diff_by_commits",
3+
"arguments": {
4+
"refRange": "HEAD..HEAD",
5+
"granularity": "line"
6+
},
7+
"assertions": {
8+
"responseContains": [
9+
"\"granularity\": \"line\"",
10+
"\"preExistingResults\"",
11+
"\"newResults\"",
12+
"\"totalPreExisting\": 3",
13+
"\"totalNew\": 0",
14+
"\"totalResults\": 3"
15+
]
16+
}
17+
}

‎client/internal/testing/params_test.go‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,42 @@
11
package testing
22

33
import (
4+
"fmt"
45
"os"
56
"path/filepath"
7+
"runtime"
68
"testing"
79
)
810

11+
// projectTmpDir creates a temporary directory under the project-local .tmp/
12+
// directory instead of the OS temp directory (avoids CWE-377/CWE-378).
13+
// Registers t.Cleanup to remove the directory after the test.
14+
func projectTmpDir(t *testing.T, name string) string {
15+
t.Helper()
16+
// Walk up from this test file to find the repo root (contains codeql-workspace.yml)
17+
_, thisFile, _, ok := runtime.Caller(0)
18+
if !ok {
19+
t.Fatal("runtime.Caller failed")
20+
}
21+
dir := filepath.Dir(thisFile)
22+
for {
23+
if _, err := os.Stat(filepath.Join(dir, "codeql-workspace.yml")); err == nil {
24+
break
25+
}
26+
parent := filepath.Dir(dir)
27+
if parent == dir {
28+
t.Fatal("could not find repo root (codeql-workspace.yml)")
29+
}
30+
dir = parent
31+
}
32+
tmpBase := filepath.Join(dir, ".tmp", fmt.Sprintf("test-params-%s-%d", name, os.Getpid()))
33+
if err := os.MkdirAll(tmpBase, 0o755); err != nil {
34+
t.Fatalf("create project tmp dir: %v", err)
35+
}
36+
t.Cleanup(func() { os.RemoveAll(tmpBase) })
37+
return tmpBase
38+
}
39+
940
func TestBuildToolParams_TestConfig(t *testing.T) {
1041
// Create a temp test fixture with test-config.json
1142
dir := t.TempDir()
@@ -229,7 +260,7 @@ func TestBuildToolParams_SARIFCompareAlertsWithConfig(t *testing.T) {
229260
}
230261

231262
func TestBuildToolParams_SARIFDiffByCommitsWithConfig(t *testing.T) {
232-
dir := t.TempDir()
263+
dir := projectTmpDir(t, "sarif-diff-by-commits")
233264
testDir := filepath.Join(dir, "tools", "sarif_diff_by_commits", "file_level_classification")
234265
beforeDir := filepath.Join(testDir, "before")
235266
os.MkdirAll(beforeDir, 0o755)

‎server/dist/codeql-development-mcp-server.js‎

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

‎server/dist/codeql-development-mcp-server.js.map‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎server/src/resources/server-tools.md‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,16 @@ This resource provides a complete reference of the default tools exposed by the
6060

6161
## SARIF Analysis Tools
6262

63-
| Tool | Description |
64-
| ------------------------ | ---------------------------------------------------------------------------------------------------- |
65-
| `sarif_compare_alerts` | Compare code locations of two SARIF alerts for overlap (sink, source, any-location, full-path modes) |
66-
| `sarif_diff_by_commits` | Correlate SARIF results with a git diff to classify findings as "new" or "pre-existing" |
67-
| `sarif_diff_runs` | Diff two SARIF files to find added, removed, and changed rules/results across analysis runs |
68-
| `sarif_extract_rule` | Extract all data for a specific rule from multi-rule SARIF. Returns a valid SARIF JSON subset |
69-
| `sarif_list_rules` | List all rules in a SARIF file with result counts, severity, precision, and tags |
70-
| `sarif_rule_to_markdown` | Convert per-rule SARIF data to markdown with Mermaid dataflow diagrams |
63+
| Tool | Description |
64+
| ------------------------- | ---------------------------------------------------------------------------------------------------- |
65+
| `sarif_compare_alerts` | Compare code locations of two SARIF alerts for overlap (sink, source, any-location, full-path modes) |
66+
| `sarif_deduplicate_rules` | Find duplicate rules across two SARIF files using fingerprint-first, full-path-fallback overlap |
67+
| `sarif_diff_by_commits` | Correlate SARIF results with a git diff to classify findings as "new" or "pre-existing" |
68+
| `sarif_diff_runs` | Diff two SARIF files to find added, removed, and changed rules/results across analysis runs |
69+
| `sarif_extract_rule` | Extract all data for a specific rule from multi-rule SARIF. Returns a valid SARIF JSON subset |
70+
| `sarif_list_rules` | List all rules in a SARIF file with result counts, severity, precision, and tags |
71+
| `sarif_rule_to_markdown` | Convert per-rule SARIF data to markdown with Mermaid dataflow diagrams |
72+
| `sarif_store` | Cache SARIF content in the session store and return a cache key for use by downstream SARIF tools |
7173

7274
### `sarif_list_rules` Response Format
7375

0 commit comments

Comments
 (0)