Skip to content

Commit b45274a

Browse files
CopilotCopilot
authored
Merge remote-tracking branch 'origin/copilot/add-sarif-to-git-diff-tool' into copilot/fix-code-for-review-comments
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
2 parents 05302bb + 7a7f8df commit b45274a

15 files changed

Lines changed: 1104 additions & 15 deletions

File tree

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Integration Test: sarif_diff_by_commits - file_level_classification
2+
3+
## Purpose
4+
5+
Validates that the `sarif_diff_by_commits` tool correctly partitions
6+
SARIF results into "new" vs "pre-existing" based on file-level overlap
7+
with a git diff. Uses `HEAD..HEAD` (empty diff) so all results are
8+
classified as pre-existing.
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`: `file`
15+
16+
## Expected Behavior
17+
18+
Returns structured output with all 3 results in `preExistingResults`
19+
and 0 results in `newResults`, since the empty diff has no changed files.

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/file_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"
5+
}

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/file_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/file_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 classifies all results as pre-existing with empty diff"
5+
}

‎client/integration-tests/primitives/tools/sarif_diff_by_commits/file_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/file_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": "file"
6+
},
7+
"assertions": {
8+
"responseContains": [
9+
"\"granularity\": \"file\"",
10+
"\"preExistingResults\"",
11+
"\"newResults\"",
12+
"\"totalPreExisting\": 3",
13+
"\"totalNew\": 0",
14+
"\"totalResults\": 3"
15+
]
16+
}
17+
}

‎client/internal/testing/params.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,27 @@ func buildToolParams(repoRoot, toolName, testCase, testDir string) (map[string]a
290290
}
291291
}
292292

293+
case "sarif_diff_by_commits":
294+
sarifFiles := findFilesByExt(beforeDir, ".sarif")
295+
if len(sarifFiles) > 0 {
296+
params["sarifPath"] = filepath.Join(beforeDir, sarifFiles[0])
297+
}
298+
// Merge refRange, repoPath, and granularity from test-config.json
299+
if configData, err := os.ReadFile(configPath); err == nil {
300+
var cfg TestConfig
301+
if json.Unmarshal(configData, &cfg) == nil {
302+
if refRange, ok := cfg.Arguments["refRange"]; ok {
303+
params["refRange"] = refRange
304+
}
305+
if repoPath, ok := cfg.Arguments["repoPath"]; ok {
306+
params["repoPath"] = repoPath
307+
}
308+
if granularity, ok := cfg.Arguments["granularity"]; ok {
309+
params["granularity"] = granularity
310+
}
311+
}
312+
}
313+
293314
case "sarif_diff_runs":
294315
sarifFiles := findFilesByExt(beforeDir, ".sarif")
295316
sort.Strings(sarifFiles)

‎client/internal/testing/params_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,3 +227,40 @@ func TestBuildToolParams_SARIFCompareAlertsWithConfig(t *testing.T) {
227227
t.Error("expected sarifPath injected into alertB")
228228
}
229229
}
230+
231+
func TestBuildToolParams_SARIFDiffByCommitsWithConfig(t *testing.T) {
232+
dir := t.TempDir()
233+
testDir := filepath.Join(dir, "tools", "sarif_diff_by_commits", "file_level_classification")
234+
beforeDir := filepath.Join(testDir, "before")
235+
os.MkdirAll(beforeDir, 0o755)
236+
os.MkdirAll(filepath.Join(testDir, "after"), 0o755)
237+
238+
// Write test-config.json with refRange and granularity but no sarifPath
239+
os.WriteFile(filepath.Join(testDir, "test-config.json"),
240+
[]byte(`{"toolName":"sarif_diff_by_commits","arguments":{"refRange":"HEAD..HEAD","granularity":"file"}}`), 0o600)
241+
242+
// Write a SARIF file in before/
243+
os.WriteFile(filepath.Join(beforeDir, "results.sarif"),
244+
[]byte(`{"version":"2.1.0","runs":[{"tool":{"driver":{"name":"CodeQL","rules":[]}},"results":[]}]}`), 0o600)
245+
246+
params, err := buildToolParams(dir, "sarif_diff_by_commits", "file_level_classification", testDir)
247+
if err != nil {
248+
t.Fatalf("unexpected error: %v", err)
249+
}
250+
251+
// Should have sarifPath injected from before/
252+
sarifPath, ok := params["sarifPath"].(string)
253+
if !ok || sarifPath == "" {
254+
t.Error("expected sarifPath to be injected from before/ directory")
255+
}
256+
257+
// Should have refRange from config
258+
if params["refRange"] != "HEAD..HEAD" {
259+
t.Errorf("params[refRange] = %v, want HEAD..HEAD", params["refRange"])
260+
}
261+
262+
// Should have granularity from config
263+
if params["granularity"] != "file" {
264+
t.Errorf("params[granularity] = %v, want file", params["granularity"])
265+
}
266+
}

0 commit comments

Comments
 (0)