Skip to content

Commit eabec91

Browse files
fix(cli): make the CI gate fail on an unreadable findings response
countFindings treated any response it could not parse as zero findings, so `nullify ci gate` printed "Gate passed: no findings above threshold". Parse the real {findings, numItems, nextToken} envelope, return an error for anything else, and exit ExitNetworkError rather than counting an unreadable response as clean. Drop the dead items/total shapes and the duplicate totalFindingsCount. The gate requests limit=1 and the API has no grand total, so the printed counts were the number of failing scanner/severity pairs; the messages now say that instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 731c73e commit eabec91

4 files changed

Lines changed: 127 additions & 105 deletions

File tree

‎cmd/cli/cmd/ci.go‎

Lines changed: 56 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cmd
22

33
import (
44
"encoding/json"
5+
"errors"
56
"fmt"
67
"os"
78
"sync"
@@ -80,8 +81,9 @@ Exit codes:
8081
}
8182
}
8283

83-
var totalFindings int64
84+
var findingsFound int64
8485
var apiErrors int64
86+
var unreadable int64
8587
var mu sync.Mutex
8688
g, gctx := errgroup.WithContext(ctx)
8789

@@ -104,14 +106,19 @@ Exit codes:
104106
return nil
105107
}
106108

107-
// limit=1 keeps the payload small; the accurate count
108-
// comes from the response's "total" field, not the
109-
// truncated items array.
110-
count := totalFindingsCount(body)
109+
count, err := countFindings(body)
110+
if err != nil {
111+
mu.Lock()
112+
fmt.Fprintf(os.Stderr, "Error: unreadable response from %s (%s): %v\n", ep.name, sev, err)
113+
mu.Unlock()
114+
atomic.AddInt64(&unreadable, 1)
115+
return nil
116+
}
117+
111118
if count > 0 {
112-
atomic.AddInt64(&totalFindings, int64(count))
119+
atomic.AddInt64(&findingsFound, 1)
113120
mu.Lock()
114-
fmt.Printf("FAIL: %s has %d %s findings\n", ep.name, count, sev)
121+
fmt.Printf("FAIL: %s has open %s findings\n", ep.name, sev)
115122
mu.Unlock()
116123
}
117124
return nil
@@ -126,10 +133,15 @@ Exit codes:
126133
return withExitCode(ExitNetworkError, err)
127134
}
128135

129-
if totalFindings > 0 {
130-
fmt.Printf("\nGate failed: %d findings at or above %s severity\n", totalFindings, severityThreshold)
136+
if unreadable > 0 {
137+
err := fmt.Errorf("%d scanner response(s) could not be read; failing the gate (cannot confirm a clean result)", unreadable)
138+
return withExitCode(ExitNetworkError, err)
139+
}
140+
141+
if findingsFound > 0 {
142+
fmt.Printf("\nGate failed: open findings at or above %s severity\n", severityThreshold)
131143
cmd.SilenceErrors = true
132-
return withExitCode(ExitFindings, fmt.Errorf("gate failed: %d findings at or above %s severity", totalFindings, severityThreshold))
144+
return withExitCode(ExitFindings, fmt.Errorf("gate failed: open findings at or above %s severity", severityThreshold))
133145
}
134146

135147
fmt.Println("Gate passed: no findings above threshold")
@@ -202,12 +214,20 @@ format emits a SARIF v2.1.0 document for upload to code-scanning tools.`,
202214
mu.Unlock()
203215
return nil
204216
}
217+
count, err := countFindings(body)
218+
if err != nil {
219+
atomic.AddInt64(&apiErrors, 1)
220+
mu.Lock()
221+
fmt.Fprintf(os.Stderr, "Warning: unreadable response from %s (%s): %v\n", ep.name, sev, err)
222+
mu.Unlock()
223+
return nil
224+
}
205225
atomic.AddInt64(&successCount, 1)
206226

207227
rows[i*len(severities)+j] = reportRow{
208228
scanner: ep.name,
209229
severity: sev,
210-
count: totalFindingsCount(body),
230+
count: count,
211231
findings: extractFindings(body),
212232
}
213233
return nil
@@ -278,55 +298,37 @@ func severitiesAboveThreshold(threshold string) []string {
278298
return []string{"critical", "high"}
279299
}
280300

281-
// countFindings extracts a count from API response JSON. When used with limit=1,
282-
// it returns 0 or 1 to indicate whether findings exist at a given severity.
283-
func countFindings(body string) int {
284-
var result any
285-
if err := json.Unmarshal([]byte(body), &result); err != nil {
286-
return 0
301+
// errUnreadableFindings reports a findings response that does not carry the
302+
// scanner envelope. It is never reported as a count of zero: a gate that cannot
303+
// read the response must block the build rather than declare the repo clean.
304+
var errUnreadableFindings = errors.New("unrecognised findings response envelope")
305+
306+
// countFindings returns the number of findings in a scanner API response. Every
307+
// endpoint in allScannerEndpoints returns {"findings":[...],"numItems":N,
308+
// "nextToken":"..."}, except /dast/bughunt/findings which omits numItems. The
309+
// count is page-scoped and therefore bounded by the request's limit; the API
310+
// exposes no grand total.
311+
func countFindings(body string) (int, error) {
312+
var envelope struct {
313+
Findings json.RawMessage `json:"findings"`
314+
NumItems *int `json:"numItems"`
287315
}
288-
289-
switch v := result.(type) {
290-
case []any:
291-
return len(v)
292-
case map[string]any:
293-
if items, ok := v["items"].([]any); ok {
294-
return len(items)
295-
}
296-
if total, ok := v["total"].(float64); ok {
297-
return int(total)
298-
}
316+
if err := json.Unmarshal([]byte(body), &envelope); err != nil {
317+
return 0, fmt.Errorf("%w: %w", errUnreadableFindings, err)
299318
}
300-
301-
return 0
302-
}
303-
304-
// totalFindingsCount extracts an accurate finding count from an API response.
305-
// It prefers the response's "total" field (which reflects the full result set
306-
// regardless of the request's limit) over the length of the truncated items
307-
// array. Falls back to array/items length when no total is present.
308-
func totalFindingsCount(body string) int {
309-
var result any
310-
if err := json.Unmarshal([]byte(body), &result); err != nil {
311-
return 0
319+
if len(envelope.Findings) == 0 {
320+
return 0, fmt.Errorf("%w: no \"findings\" field", errUnreadableFindings)
312321
}
313322

314-
switch v := result.(type) {
315-
case []any:
316-
return len(v)
317-
case map[string]any:
318-
if total, ok := v["total"].(float64); ok {
319-
return int(total)
320-
}
321-
if items, ok := v["items"].([]any); ok {
322-
return len(items)
323-
}
324-
if findings, ok := v["findings"].([]any); ok {
325-
return len(findings)
326-
}
323+
if envelope.NumItems != nil {
324+
return *envelope.NumItems, nil
327325
}
328326

329-
return 0
327+
var findings []json.RawMessage
328+
if err := json.Unmarshal(envelope.Findings, &findings); err != nil {
329+
return 0, fmt.Errorf("%w: \"findings\" is not an array: %w", errUnreadableFindings, err)
330+
}
331+
return len(findings), nil
330332
}
331333

332334
// extractFindings pulls the finding objects out of an API response body so they

‎cmd/cli/cmd/ci_test.go‎

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,41 +33,75 @@ func TestCountFindings(t *testing.T) {
3333
expected int
3434
}{
3535
{
36-
name: "empty array",
37-
body: `[]`,
38-
expected: 0,
36+
name: "sast envelope",
37+
body: `{"version":"1","findings":[{"id":"f1"}],"numItems":1,"nextToken":""}`,
38+
expected: 1,
3939
},
4040
{
41-
name: "array with items",
42-
body: `[{"id": 1}, {"id": 2}, {"id": 3}]`,
43-
expected: 3,
41+
name: "sca dependencies envelope",
42+
body: `{"version":"1","findings":[{"id":"f1"},{"id":"f2"}],"numItems":2,"nextToken":"abc"}`,
43+
expected: 2,
4444
},
4545
{
46-
name: "object with items array",
47-
body: `{"items": [{"id": 1}]}`,
46+
name: "sca containers envelope",
47+
body: `{"findings":[{"id":"f1"}],"numItems":1,"nextToken":""}`,
4848
expected: 1,
4949
},
5050
{
51-
name: "object with total field",
52-
body: `{"total": 42}`,
53-
expected: 42,
51+
name: "secrets envelope",
52+
body: `{"findings":[],"numItems":0,"nextToken":""}`,
53+
expected: 0,
5454
},
5555
{
56-
name: "invalid json",
57-
body: `not json`,
58-
expected: 0,
56+
name: "pentest envelope",
57+
body: `{"findings":[{"id":"f1"},{"id":"f2"},{"id":"f3"}],"numItems":3,"nextToken":""}`,
58+
expected: 3,
59+
},
60+
{
61+
name: "bughunt envelope omits numItems",
62+
body: `{"findings":[{"id":"f1"},{"id":"f2"}]}`,
63+
expected: 2,
64+
},
65+
{
66+
name: "cspm envelope omits empty nextToken",
67+
body: `{"findings":[{"id":"f1"}],"numItems":1,"version":"1"}`,
68+
expected: 1,
5969
},
6070
{
61-
name: "empty object",
62-
body: `{}`,
71+
name: "nil findings slice serialises as null",
72+
body: `{"findings":null,"numItems":0,"nextToken":""}`,
6373
expected: 0,
6474
},
6575
}
6676

6777
for _, tt := range tests {
6878
t.Run(tt.name, func(t *testing.T) {
69-
result := countFindings(tt.body)
79+
result, err := countFindings(tt.body)
80+
require.NoError(t, err)
7081
require.Equal(t, tt.expected, result)
7182
})
7283
}
7384
}
85+
86+
func TestCountFindingsRejectsUnknownEnvelopes(t *testing.T) {
87+
tests := []struct {
88+
name string
89+
body string
90+
}{
91+
{name: "invalid json", body: `not json`},
92+
{name: "empty object", body: `{}`},
93+
{name: "bare array", body: `[{"id":"f1"}]`},
94+
{name: "items envelope", body: `{"items":[{"id":"f1"}]}`},
95+
{name: "total envelope", body: `{"total":42}`},
96+
{name: "findings is not an array", body: `{"findings":{"id":"f1"}}`},
97+
{name: "empty body", body: ``},
98+
}
99+
100+
for _, tt := range tests {
101+
t.Run(tt.name, func(t *testing.T) {
102+
count, err := countFindings(tt.body)
103+
require.ErrorIs(t, err, errUnreadableFindings)
104+
require.Zero(t, count)
105+
})
106+
}
107+
}

‎cmd/cli/cmd/status.go‎

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -113,33 +113,14 @@ func init() {
113113

114114
// summarizeFindingsResponse extracts a human-readable summary from a findings API response.
115115
func summarizeFindingsResponse(body string) string {
116-
var result any
117-
if err := json.Unmarshal([]byte(body), &result); err != nil {
118-
return "data available"
116+
count, err := countFindings(body)
117+
if err != nil {
118+
return "unreadable response"
119119
}
120-
121-
switch v := result.(type) {
122-
case []any:
123-
if len(v) == 1 {
124-
return "1 finding returned"
125-
}
126-
return fmt.Sprintf("%d findings returned", len(v))
127-
case map[string]any:
128-
if items, ok := v["items"].([]any); ok {
129-
if len(items) == 1 {
130-
return "1 finding returned"
131-
}
132-
return fmt.Sprintf("%d findings returned", len(items))
133-
}
134-
if total, ok := v["total"].(float64); ok {
135-
if total == 1 {
136-
return "1 total finding"
137-
}
138-
return fmt.Sprintf("%.0f total findings", total)
139-
}
120+
if count == 1 {
121+
return "1 finding returned"
140122
}
141-
142-
return "data available"
123+
return fmt.Sprintf("%d findings returned", count)
143124
}
144125

145126
type securityStatusOutput struct {

‎cmd/cli/cmd/status_test.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,34 @@ func TestSummarizeFindingsResponse(t *testing.T) {
1414
expected string
1515
}{
1616
{
17-
name: "array response",
18-
body: `[{"id":1},{"id":2}]`,
17+
name: "scanner envelope with findings",
18+
body: `{"findings":[{"id":"f1"},{"id":"f2"}],"numItems":2,"nextToken":""}`,
1919
expected: "2 findings returned",
2020
},
2121
{
22-
name: "object with items",
23-
body: `{"items":[{"id":1}]}`,
22+
name: "scanner envelope with one finding",
23+
body: `{"findings":[{"id":"f1"}],"numItems":1,"nextToken":""}`,
2424
expected: "1 finding returned",
2525
},
2626
{
27-
name: "object with total",
28-
body: `{"total":99}`,
29-
expected: "99 total findings",
27+
name: "bughunt envelope omits numItems",
28+
body: `{"findings":[{"id":"f1"},{"id":"f2"},{"id":"f3"}]}`,
29+
expected: "3 findings returned",
30+
},
31+
{
32+
name: "empty scanner envelope",
33+
body: `{"findings":[],"numItems":0,"nextToken":""}`,
34+
expected: "0 findings returned",
3035
},
3136
{
3237
name: "empty object",
3338
body: `{}`,
34-
expected: "data available",
39+
expected: "unreadable response",
3540
},
3641
{
3742
name: "invalid json",
3843
body: `nope`,
39-
expected: "data available",
44+
expected: "unreadable response",
4045
},
4146
}
4247

0 commit comments

Comments
 (0)