master
md 473 lines 20 KB
Rendered Raw
1 ---
2 name: coverity-audit
3 description: Triage Coverity Scan defects (https://scan.coverity.com) for this project — fetch defect lists, fetch per-defect details, and apply triage decisions (Bug / FalsePositive / Intentional with severity, action, and a comment). Use when the user asks to "review Coverity defects", "triage Coverity findings", "fetch Coverity outstanding", or anything mentioning Coverity Scan, CIDs, or scan.coverity.com.
4 ---
5
6 # Coverity Scan triage skill
7
8 This skill drives the Coverity Scan unofficial JSON API (the public site has
9 no documented API — the scripts mimic what the browser does) for the project
10 configured in `.env`. Scripts auto-detect the repo root and write all
11 artifacts under `<repo-root>/.local/audits/coverity/`.
12
13 The skill captures **operational knowledge** (how the API works, where it
14 trips up, what the data means). It does NOT prescribe a review pipeline —
15 how you actually triage each defect (single model, multi-model, manual) is
16 adhoc; agree the approach with the user up front.
17
18 ## MANDATORY — keep this skill alive
19
20 If you (the agent) discover a new pattern, gotcha, working flow, correction,
21 or any piece of knowledge while running this skill — update this `SKILL.md`
22 AND commit it BEFORE proceeding. Knowledge that isn't committed is lost.
23
24 Examples of things to capture:
25 - New view IDs encountered (and what each represents)
26 - New API endpoint or parameter behavior
27 - New failure mode (Cloudflare quirks, session expiry signals, rate limits)
28 - New FP guardrail you found via the codebase (idiom that Coverity mismodels)
29 - New severity / classification mapping detail learned the hard way
30
31 ---
32
33 ## MANDATORY — startup sequence when this skill is invoked
34
35 Do these steps in this order. Skipping any step costs the user their session.
36
37 ### Step 1 — agree the triage approach with the user
38
39 Coverity reviews are usually 1-3 defects. Sometimes hundreds. Ask the user:
40
41 - "How many defects are we looking at — one specific CID, or a sweep?"
42 - "How do you want to review them — you read each one, you spawn one agent
43 per defect, you want me to use multiple models for cross-checking, …?"
44
45 Do NOT default to a heavy multi-stage pipeline; that's only worth setting up
46 when there are tens of defects to crunch. For small batches a single agent
47 or the user reading the bundle directly is usually faster.
48
49 If the user does want a multi-model approach, a sensible (not prescribed)
50 shape is: cheap models surface ideas, the strongest coding model produces
51 the actual analysis + fix, the strongest reviewing model sanity-checks the
52 result. The user picks the actual CLIs/models — this skill does not assume
53 any specific tool.
54
55 ### Step 2 — ask the user for a fresh cookie
56
57 Coverity Scan auth is cookie-based and tied to a live browser session. Give
58 the user the exact recipe:
59
60 > 1. Open https://scan.coverity.com/projects/<owner>-<repo>?tab=overview
61 > (replace `<owner>-<repo>` with the project slug, e.g. `netdata-netdata`).
62 > 2. Click **"View Defects"** at the top right -- a new tab opens to
63 > https://scan4.scan.coverity.com/# (or `scanN.scan.coverity.com` --
64 > Coverity load-balances; use whichever URL you land on as `COVERITY_HOST`).
65 > 3. **Keep that tab open the whole time we work.** Closing it kills the
66 > session immediately.
67 > 4. Press F12 to open DevTools, switch to the Network tab.
68 > 5. Click any defect in the list -- 3-4 requests appear in the Network tab.
69 > 6. Right-click any of those requests, select Copy > **Copy as cURL**.
70 > 7. Paste the entire curl command back to me.
71
72 If the curl the user pastes does NOT contain a `-b 'cookie=...'` line, ask
73 again -- they probably picked "Copy as fetch" or "Copy as Node.js fetch".
74
75 ### Step 3 — save the cookie in `.env`
76
77 Extract the value of the `-b` argument from the user's curl and write/update
78 `<repo-root>/.env` with:
79
80 ```bash
81 COVERITY_COOKIE='<paste-the-entire-cookie-blob-here>'
82 COVERITY_PROJECT_ID=<numeric projectId from the URL or table.json query>
83 COVERITY_VIEW_OUTSTANDING=<viewId of the Outstanding view>
84 COVERITY_HOST=https://scan4.scan.coverity.com
85 ```
86
87 `.env` is gitignored. The cookie blob must include both `COVJSESSIONID-build`
88 and `XSRF-TOKEN`. The scripts extract `XSRF-TOKEN` automatically.
89
90 ### Step 4 — start the keepalive (background)
91
92 ```
93 Bash tool with run_in_background=true,
94 command="bash .agents/skills/coverity-audit/scripts/keepalive.sh"
95 ```
96
97 The keepalive **exits non-zero the moment a ping fails** (session expired,
98 browser tab closed, cookie went bad). The orchestrator's background-task
99 notification fires immediately so you know to ask for a fresh cookie and
100 restart.
101
102 **Stop the keepalive at the end of the triage session** by killing its
103 background task.
104
105 ### Step 5 — proceed with the actual triage work
106
107 Now (and only now) is it safe to fetch tables, fetch details, prepare
108 defect bundles, finalize verdicts.
109
110 ---
111
112 ## CRITICAL — server-side view state
113
114 Coverity's table API has a **stateful, server-side view cursor**. The
115 `/views/table.json` POST changes server-state (which page is "current"),
116 then the `/reports/table.json` GET reads whatever the current state is.
117
118 This has two consequences:
119
120 ### 1. Pagination is two-step, not one-shot
121
122 Per page:
123 1. `POST /views/table.json {projectId, viewId, pageNum}` -- moves the cursor
124 2. `GET /reports/table.json?projectId=...&viewId=...` -- reads the page
125
126 `fetch-table.sh` handles both steps and the page loop.
127
128 ### 2. **The user MUST NOT touch the Coverity UI while a fetch is running**
129
130 If the user clicks a different view, scrolls to a different page, sorts a
131 column, applies a filter — the server-side cursor moves under our scripts'
132 feet. The scripts will then fetch garbage (rows from whatever view the user
133 just opened, not the view the script asked for).
134
135 Tell the user explicitly: "I'm about to fetch the defect list. Don't click
136 in the Coverity tab until I say I'm done."
137
138 If the user accidentally interferes, re-run the fetch script — it's
139 idempotent on cached pages but not on already-corrupted ones, so delete the
140 output JSONs and re-fetch from page 1.
141
142 ---
143
144 ## Coverity views (queue scopes)
145
146 Coverity organizes defects into named **views**, each with a numeric
147 `viewId`. The skill operates per-view.
148
149 ### How to find a view's ID
150
151 In the Coverity UI:
152 - Click the project's defects view, then the view selector dropdown.
153 - Each view has a URL like `https://scan4.scan.coverity.com/#viewId=NNNNN`.
154 - That `NNNNN` is the value to put in `.env` (or pass to `fetch-table.sh`).
155
156 ### Common view types you'll encounter
157
158 The default project ships with these (the IDs are project-specific):
159
160 - **Outstanding** — the live queue: defects neither classified nor dismissed.
161 Where day-to-day triage happens. Save its viewId as
162 `COVERITY_VIEW_OUTSTANDING` (also used by the keepalive).
163 - **All in project** — every defect, including already-classified ones.
164 Useful for batch rescans or full audits.
165 - **Dismissed** — defects classified as False Positive / Intentional and
166 ignored. Useful for re-review when the underlying code changed.
167 - **Fixed** — defects Coverity now considers fixed. Verification queue.
168 - **Unclassified non-outstanding** — corner cases.
169
170 When a user asks you to operate on a non-default view, ask them to give you
171 its viewId from the UI. You can have multiple `COVERITY_VIEW_*` env vars.
172
173 ### Switching views without confusing the cursor
174
175 The view-state cursor described above is **per-session**. If you want to
176 fetch view A then view B, do them sequentially (not concurrently): each
177 `fetch-table.sh` call sends its own POST that resets the cursor. There is
178 no need to "go back" -- just call it again with the next viewId.
179
180 ---
181
182 ## Coverity attribute reference
183
184 Triage values are sent as integer **attribute IDs**, not names:
185
186 ### Classification (attribute 3)
187
188 | ID | Name |
189 |----|----------------|
190 | 20 | Unclassified |
191 | 21 | Pending |
192 | 22 | False Positive |
193 | 23 | Intentional |
194 | 24 | Bug |
195
196 ### Severity (attribute 1)
197
198 | ID | Name |
199 |----|-------------|
200 | 10 | Unspecified |
201 | 11 | Major |
202 | 12 | Moderate |
203 | 13 | Minor |
204
205 ### Action (attribute 2)
206
207 | ID | Name |
208 |----|-------------------|
209 | 1 | Undecided |
210 | 2 | Fix Required |
211 | 3 | Fix Submitted |
212 | 4 | Modeling Required |
213 | 5 | Ignore |
214
215 ### External reference (attribute 4)
216
217 Free-form string. The scripts always send `null`.
218
219 ### Sensible (verdict → attributes) mappings
220
221 These are what `finalize-defect.sh` applies when given a verdict from the
222 suggested vocabulary below:
223
224 | Outcome | classification (3) | action (2) |
225 |-------------------|--------------------|------------|
226 | Real bug, fixed | 24 Bug | 3 Fix Submitted |
227 | False positive | 22 False Positive | 5 Ignore |
228 | Cosmetic / harmless | 23 Intentional | 5 Ignore |
229
230 ### Coverity displayImpact → severity ID
231
232 `finalize-defect.sh` maps `defect-summary.json#displayImpact` to severity:
233
234 | displayImpact | severity ID | name |
235 |---------------|-------------|-------------|
236 | `High` | 11 | Major |
237 | `Medium` | 12 | Moderate |
238 | `Low` | 13 | Minor |
239 | `null` / missing | 10 | Unspecified |
240
241 ---
242
243 ## Suggested verdict vocabulary
244
245 This is the taxonomy used historically; you can reuse it or replace it. The
246 `finalize-defect.sh` script understands these names directly:
247
248 ### Real bugs (classification = 24 Bug, action = 3 Fix Submitted)
249
250 | Verdict | When to use |
251 |-------------------------------|----------------------------------------------------------------------|
252 | `TRUE_BUG_MEMORY_CORRUPTION` | OOB read/write, UAF, double-free, type confusion, stack-escape UAF. |
253 | `TRUE_BUG_CRASH` | Reachable NULL deref / div-by-zero / assert / fatal. No corruption. |
254 | `TRUE_BUG_RESOURCE_LEAK` | fd / memory / lock / refcount leak that accumulates on a reachable path. |
255 | `TRUE_BUG_LOGIC` | Wrong result, wrong metric, wrong stored/transmitted data. No crash. |
256 | `TRUE_BUG_UB` | Spec-UB (signed overflow, strict aliasing) compiles today but latent.|
257
258 ### False positives (classification = 22 FP, action = 5 Ignore)
259
260 | Verdict | When to use |
261 |----------------------------------|--------------------------------------------------------------|
262 | `FALSE_POSITIVE_GUARD_EXISTS` | The code has a check Coverity failed to track. |
263 | `FALSE_POSITIVE_UNREACHABLE` | The flagged path cannot be reached under any realistic state.|
264 | `FALSE_POSITIVE_TRUSTED_INPUT` | Tainted source is actually trusted (root-owned local file). |
265 | `FALSE_POSITIVE_TOOL_MODEL` | Coverity's semantic model is wrong (e.g. doesn't know `mallocz` cannot return NULL). |
266 | `IMPOSSIBLE_CONDITIONS` | Combination of states existing invariants prevent. |
267
268 ### Cosmetic (classification = 23 Intentional, action = 5 Ignore)
269
270 | Verdict | When to use |
271 |-------------|--------------------------------------------------------------|
272 | `COSMETIC` | Not a bug: unused value, dead branch, redundant expression. |
273
274 ### Bookkeeping (no Coverity update)
275
276 | Verdict | When to use |
277 |---------------|------------------------------------------------------------------------|
278 | `CODE_GONE` | The flagged file/function no longer exists. (`finalize-defect.sh` skips.) |
279 | `NEEDS_HUMAN` | Genuinely unsure after reasonable investigation. (`finalize-defect.sh` skips.) |
280
281 ---
282
283 ## Per-defect workdir convention
284
285 `prepare-defect.sh` creates a per-defect directory under
286 `.local/audits/coverity/triage/<scope>/cid-<N>/` with:
287
288 - `defect-summary.json` — the row from the table dump
289 - `defect-details.json` — the per-defect details (event trace, CWE, checker)
290 - `source-context.c` — ~150 lines of source around the main event
291 - `TODO.md` — per-defect notes; keep all per-defect artifacts inside
292
293 Whatever review approach the user picks, write its outputs here too (e.g.
294 analyzer reports, decider reasoning, commit message draft, build-verify
295 log). Don't pile per-defect stuff at the repo root.
296
297 ---
298
299 ## CID vs defectInstanceId
300
301 Two IDs for one defect, both flying around the API:
302
303 - **`cid`** is stable across scans. Use it for tracking, comments, your
304 permanent records.
305 - **`defectInstanceId`** is per-scan. The `defectdetails.json` endpoint
306 takes a `defectInstanceId`, NOT a `cid`. Each `table.json` row carries the
307 current `lastDefectInstanceId` for its CID.
308
309 When all you have is a CID (e.g. you're acting on an old list, or working
310 from external triage notes), use:
311
312 ```
313 bash .agents/skills/coverity-audit/scripts/resolve-cid-to-diid.sh <cid>
314 # prints the current defectInstanceId, or "GONE" if Coverity no longer reports it
315 ```
316
317 Internally it queries `/reports/defects.json?cid=N` and parses
318 `defectInstanceId` out of the returned `.url` field.
319
320 ---
321
322 ## Workflow
323
324 ### Fetch a view's table
325
326 ```
327 bash .agents/skills/coverity-audit/scripts/fetch-table.sh \
328 "${COVERITY_VIEW_OUTSTANDING}" 7 .local/audits/coverity/raw/outstanding
329 ```
330
331 Produces `.local/audits/coverity/raw/outstanding-page1.json` ... and a
332 combined flat array at `.local/audits/coverity/raw/outstanding-all.json`.
333 Pass the right page count (visible in the UI) for the view.
334
335 **Reminder**: the user must not touch the UI during a fetch.
336
337 ### Fetch per-defect details
338
339 ```
340 bash .agents/skills/coverity-audit/scripts/fetch-details.sh \
341 .local/audits/coverity/raw/outstanding-all.json \
342 .local/audits/coverity/details/outstanding
343 ```
344
345 One file per CID at `<details>/cid-<N>.json`. Idempotent.
346
347 ### Bundle a defect for review
348
349 ```
350 bash .agents/skills/coverity-audit/scripts/prepare-defect.sh <CID> outstanding
351 ```
352
353 Creates `.local/audits/coverity/triage/outstanding/cid-<N>/` with the bundle.
354 The actual review (single-model, multi-model, human) is **adhoc** — agree
355 the approach with the user.
356
357 ### Apply a verdict
358
359 After review and (if needed) a fix commit:
360
361 ```
362 bash .agents/skills/coverity-audit/scripts/finalize-defect.sh \
363 <CID> <VERDICT> <scope> .local/audits/coverity/triage/<scope>/cid-<N>/comment.txt \
364 [<commit-sha>]
365 ```
366
367 `<scope>` is the same name `prepare-defect.sh` uses (e.g. `outstanding`,
368 `dismissed`, `fixed`, `unclassified`).
369
370 This:
371 - Skips silently for `NEEDS_HUMAN` and `CODE_GONE`.
372 - Reads `displayImpact` from the table dump to derive severity.
373 - Appends `Fix commit: <sha>` to the comment when a SHA is given.
374 - Posts JSON to `/sourcebrowser/updatedefecttriage.json`.
375
376 For scopes other than "outstanding" (re-triaging dismissed/fixed/etc.),
377 finalize-defect prints a warning -- the caller is asserting that the new
378 verdict disagrees with the existing classification. It still applies.
379
380 ---
381
382 ## ASCII-only comments — non-negotiable
383
384 Coverity Scan sits behind Cloudflare. The WAF rejects bodies containing
385 non-ASCII bytes (em-dashes, smart quotes, accented letters) with a 403
386 Cloudflare challenge that looks like an expired-session error but isn't.
387
388 - Use `--` instead of em-dash (U+2014).
389 - Use straight quotes `"` `'` instead of smart quotes.
390 - The scripts reject non-ASCII before the network round-trip.
391
392 ---
393
394 ## Project-specific FP guardrails (Netdata)
395
396 Most of the cost of a Coverity audit is rejecting false positives. Before
397 calling something a bug in this codebase, rule out these idioms — Coverity
398 mis-models all of them:
399
400 1. **`z`-suffix allocators never fail.** `mallocz`, `callocz`, `reallocz`,
401 `strdupz`, `strndupz`, `mallocz_flex` call `fatal()` on OOM. They cannot
402 return NULL. Any "possible NULL deref after `mallocz`" warning is FP.
403 2. **`freez(NULL)` is safe.** Same for `string_freez`, etc.
404 3. **`DOUBLE_LINKED_LIST_*` macros** (`libnetdata/linked-lists.h`) manage
405 prev/next with their own invariants. Raw pointer manipulation inside
406 them is expected.
407 4. **`buffer_*` API** (`libnetdata/buffer/`) auto-grows the underlying
408 `buffer->buffer[]` on write. Direct indexing of `buffer->buffer[N]` from
409 callers is the risk, not the API.
410 5. **`STRING` is refcounted and interned** (`libnetdata/string/`).
411 `string_strdupz` increments refcount on an existing intern;
412 `string_dup` acquires a new reference. Mixing them is a refcount bug.
413 6. **`ARAL` is a slab allocator** (`libnetdata/aral/`). Objects are reused
414 — UAF looks different here: the same address is re-issued. A pointer
415 that "still works" after `aral_freez` may be a reused object.
416 7. **`DICTIONARY`** has `dictionary_acquired_item_get` / `_release` with
417 refcounts. Raw access bypasses the refcount.
418 8. **Custom locks** (`spinlock_lock`, `rw_spinlock_*`) — not pthread.
419 Coverity MISSING_LOCK warnings often misunderstand them.
420 9. **Platform**: glibc + musl. Watch glibc-only assumptions
421 (e.g. `strerror_r` signature).
422 10. **Compilers**: gcc + clang both must compile.
423 11. **Process model**: long-running daemon, spawns plugin subprocesses over
424 a line-based stdin/stdout protocol. Plugin stdin is typically trusted;
425 streaming peers are UNTRUSTED remote peers over TCP.
426
427 ---
428
429 ## Input trust boundaries (Netdata)
430
431 Used to decide whether a tainted-data path is `FALSE_POSITIVE_TRUSTED_INPUT`:
432
433 | Source | Trust | Module |
434 |------------------------------|--------------------------------|------------------------------------|
435 | Local `/proc`, `/sys` | Trusted (root-owned) | `collectors/proc.plugin/`, `cgroups.plugin/` |
436 | Plugin stdin (our plugins) | Trusted | `plugins.d/`, `collectors/*.plugin/` |
437 | Streaming peer (remote agent)| **UNTRUSTED** | `streaming/`, `stream-*` |
438 | HTTP request (dashboard API) | Semi-trusted (usually localhost) | `web/api/`, `web/server/` |
439 | MCP request | Semi-trusted (localhost) | `web/mcp/` |
440 | Cloud (aclk) | Trusted (TLS + token to Netdata Cloud) | `aclk/` |
441 | Config files | Trusted (root-owned) | `daemon/config/`, `health/` |
442
443 ---
444
445 ## Failure modes — quick diagnosis
446
447 | Symptom | Likely cause |
448 |------------------------------------------------------|-------------------------------------------------------------|
449 | HTTP 401 / 403 / 302, response is HTML | Session expired. Recapture cookie from browser. |
450 | HTTP 403 with Cloudflare challenge HTML | Either non-ASCII in comment, or browser tab closed. |
451 | keepalive.sh exits non-zero | Same as above. Ask user for a fresh cookie. |
452 | HTTP 200 but `defectStatus` empty | XSRF token stale. Recapture cookie. |
453 | `Could not parse session from .env` | Wrong cookie format. Paste the FULL `-b 'k=v; ...'` string. |
454 | Fetched rows look wrong (different view) | User clicked in the UI mid-fetch. Delete output JSONs and re-fetch. |
455 | `.url` field missing in `/reports/defects.json` reply | Coverity no longer reports this CID -- treat as `CODE_GONE`. |
456 | Cookie expires in mid-run despite keepalive | Browser tab was closed. Reopen the tab and recapture cookie. |
457
458 ---
459
460 ## Recurring tips
461
462 - The `lastDefectInstanceId` field, NOT `cid`, is what `defectdetails.json` wants.
463 - Coverity caches results aggressively; if a CID disappears from "Outstanding"
464 immediately after finalize, a fresh fetch can take a few seconds to reflect.
465 - Idempotence: `fetch-details.sh` skips files already present, so partial runs
466 are safe to re-invoke.
467 - `prepare-defect.sh` uses Coverity's `displayFile` and the main-event line
468 to extract source context. If Coverity's line numbers are stale (after a
469 refactor), the context may not center on the current code -- use it as a
470 hint, not an authoritative location.
471 - Coverity load-balances across `scanN.scan.coverity.com` hosts. Whatever
472 hostname your browser landed on must be the one in `COVERITY_HOST`;
473 cookies are per-host.