|
| 1 | +# SOW-0010 - Static Analysis Finding Cleanup |
| 2 | + |
| 3 | +## Status |
| 4 | + |
| 5 | +Status: open |
| 6 | + |
| 7 | +Sub-state: Scanner rollout exposed pre-existing findings that need focused triage before stricter gates are enabled. |
| 8 | + |
| 9 | +## Requirements |
| 10 | + |
| 11 | +### Purpose |
| 12 | + |
| 13 | +Make the SDK clean under the strongest practical local and GitHub static-analysis scanners, so future CI can hard-gate more findings without blocking on existing debt. |
| 14 | + |
| 15 | +### User Request |
| 16 | + |
| 17 | +This follow-on SOW was created from scanner validation during `SOW-0009`; the user approved installing strong GitHub and local scanners. |
| 18 | + |
| 19 | +### Assistant Understanding |
| 20 | + |
| 21 | +Facts: |
| 22 | + |
| 23 | +- `SOW-0009` added GitHub and local scanner coverage. |
| 24 | +- Local `staticcheck` reports Go findings in `src/go`. |
| 25 | +- Local `gosec` reports findings in the SDK, fixture, and benchmark Go modules. |
| 26 | +- Local C scanners report findings in test code when run beyond the initial hard gate. |
| 27 | +- Local full ShellCheck reports existing warning/info findings; the initial scanner rollout gates only ShellCheck errors. |
| 28 | + |
| 29 | +Inferences: |
| 30 | + |
| 31 | +- These findings should be triaged in a dedicated cleanup pass because they affect existing SDK/test/benchmark code rather than the scanner workflow configuration itself. |
| 32 | +- After this cleanup, the static-analysis workflow can likely promote more scanners from reporting mode to hard gates. |
| 33 | + |
| 34 | +Unknowns: |
| 35 | + |
| 36 | +- Which findings are true bugs versus acceptable test/benchmark patterns that need local suppressions with documented rationale. |
| 37 | + |
| 38 | +### Acceptance Criteria |
| 39 | + |
| 40 | +- Triage each scanner finding class with file/line evidence. |
| 41 | +- Fix true bugs and unsafe patterns. |
| 42 | +- Add narrow suppressions only when the finding is intentional, test-only, or a false positive. |
| 43 | +- Re-run C, Rust, Go, shell, and workflow scanners locally. |
| 44 | +- Tighten `.github/workflows/static-analysis.yml` gates where the cleaned results make that safe. |
| 45 | +- Update specs, docs, skills, or SOW artifacts if any cleanup changes public behavior or durable workflow guidance. |
| 46 | + |
| 47 | +## Analysis |
| 48 | + |
| 49 | +Sources checked: |
| 50 | + |
| 51 | +- Local scanner validation from `SOW-0009`. |
| 52 | +- Go SDK source around `src/go/pkg/netipc/protocol/lookup.go:758`, `src/go/pkg/netipc/protocol/lookup.go:1274`, `src/go/pkg/netipc/protocol/lookup.go:1278`, and `src/go/pkg/netipc/transport/posix/uds.go:667`. |
| 53 | +- C test source around `tests/fixtures/c/test_shm.c:1033`, `tests/fixtures/c/test_shm.c:1667`, and `tests/test_protocol.c:369`. |
| 54 | + |
| 55 | +Current state: |
| 56 | + |
| 57 | +- `staticcheck ./...` in `src/go` reports unused assignment findings at `lookup.go:758`, `lookup.go:1274`, and `lookup.go:1278`, plus unused function `maxU32` at `uds.go:667`. |
| 58 | +- `gosec ./...` reports findings across `src/go`, `tests/fixtures/go`, and `bench/drivers/go`; common classes include integer conversion, file path, unsafe block, and unchecked return findings. |
| 59 | +- `cppcheck` against the broader test tree reports uninitialized-buffer findings in test code, including `tests/fixtures/c/test_shm.c:1033` and `tests/test_protocol.c:369`. |
| 60 | +- `flawfinder --minlevel=4` reports `access()` usage in Windows shared-memory cleanup logic and C tests, including `tests/fixtures/c/test_shm.c:1667`. |
| 61 | + |
| 62 | +Risks: |
| 63 | + |
| 64 | +- Fixing findings mechanically can change wire encoding, transport behavior, or benchmark semantics. |
| 65 | +- Suppressing findings too broadly can hide real bugs. |
| 66 | +- Tightening gates before cleanup will make the initial scanner rollout fail immediately. |
| 67 | + |
| 68 | +## Pre-Implementation Gate |
| 69 | + |
| 70 | +Status: needs-user-decision |
| 71 | + |
| 72 | +Problem / root-cause model: |
| 73 | + |
| 74 | +- Stronger scanners now expose existing code patterns that were previously not enforced. The local evidence above shows both likely real cleanup items and scanner findings that may be acceptable in tests or benchmarks. |
| 75 | + |
| 76 | +Evidence reviewed: |
| 77 | + |
| 78 | +- `src/go/pkg/netipc/protocol/lookup.go:758`, `src/go/pkg/netipc/protocol/lookup.go:1274`, and `src/go/pkg/netipc/protocol/lookup.go:1278` from local `staticcheck`. |
| 79 | +- `src/go/pkg/netipc/transport/posix/uds.go:667` from local `staticcheck`. |
| 80 | +- `tests/fixtures/c/test_shm.c:1033` and `tests/test_protocol.c:369` from local `cppcheck`. |
| 81 | +- `tests/fixtures/c/test_shm.c:1667` from local `flawfinder --minlevel=4`. |
| 82 | + |
| 83 | +Affected contracts and surfaces: |
| 84 | + |
| 85 | +- Go SDK source, Go fixtures, Go benchmark drivers, C tests, shell scripts, scanner workflows, and future CI behavior. |
| 86 | +- Public protocol/API behavior may be affected if fixes touch encoding, conversion, or transport paths. |
| 87 | + |
| 88 | +Existing patterns to reuse: |
| 89 | + |
| 90 | +- Existing C, Rust, Go, sanitizer, Valgrind, race, and interop validation commands listed in `AGENTS.md`. |
| 91 | +- Existing SOW validation and artifact-maintenance gates. |
| 92 | + |
| 93 | +Risk and blast radius: |
| 94 | + |
| 95 | +- Medium. The findings span SDK code, tests, fixtures, and benchmarks. Some fixes can be local, but conversion and encoding fixes require cross-language interoperability checks. |
| 96 | + |
| 97 | +Sensitive data handling plan: |
| 98 | + |
| 99 | +- No sensitive data is required. Evidence should cite source paths, line numbers, scanner classes, and sanitized output summaries only. |
| 100 | + |
| 101 | +Implementation plan: |
| 102 | + |
| 103 | +1. Reproduce all scanner findings and classify each as true positive, false positive, intentional test fixture, or acceptable benchmark pattern. |
| 104 | +2. Fix true positives with focused code changes and add narrow suppressions where justified. |
| 105 | +3. Promote static-analysis workflow gates only after local validation is clean enough to avoid immediate CI deadlock. |
| 106 | + |
| 107 | +Validation plan: |
| 108 | + |
| 109 | +- Run the final scanner matrix locally. |
| 110 | +- Run C/Rust/Go test and interoperability commands affected by touched code. |
| 111 | +- Run `bash .agents/sow/audit.sh` and `git diff --check`. |
| 112 | + |
| 113 | +Artifact impact plan: |
| 114 | + |
| 115 | +- AGENTS.md: likely unaffected unless validation policy changes. |
| 116 | +- Runtime project skills: likely unaffected unless repeatable scanner workflow knowledge emerges. |
| 117 | +- Specs: update only if protocol/API behavior changes. |
| 118 | +- End-user/operator docs: likely unaffected unless public SDK guidance changes. |
| 119 | +- End-user/operator skills: update only if public integration workflow changes. |
| 120 | +- SOW lifecycle: complete this SOW only after all deferred scanner findings are implemented, rejected with evidence, or moved to separate SOWs. |
| 121 | + |
| 122 | +Open-source reference evidence: |
| 123 | + |
| 124 | +- None yet for this cleanup. Future implementation should check comparable scanner suppression/fix patterns if a class of finding is ambiguous. |
| 125 | + |
| 126 | +Open decisions: |
| 127 | + |
| 128 | +- None yet. Future implementation may need user decisions if a scanner finding requires accepting a behavior change, broad suppression, or benchmark/test rewrite. |
| 129 | + |
| 130 | +## Implications And Decisions |
| 131 | + |
| 132 | +- No user decision has been requested yet. This SOW records cleanup work discovered by the scanner rollout. |
| 133 | + |
| 134 | +## Plan |
| 135 | + |
| 136 | +1. Reproduce scanner findings and classify them. |
| 137 | +2. Fix true positives and document narrow suppressions. |
| 138 | +3. Tighten CI gates where validation proves they are ready. |
| 139 | + |
| 140 | +## Execution Log |
| 141 | + |
| 142 | +### 2026-06-02 |
| 143 | + |
| 144 | +- Created from `SOW-0009` validation after local scanner installation surfaced pre-existing findings. |
| 145 | + |
| 146 | +## Validation |
| 147 | + |
| 148 | +Acceptance criteria evidence: |
| 149 | + |
| 150 | +- Pending. |
| 151 | + |
| 152 | +Tests or equivalent validation: |
| 153 | + |
| 154 | +- Pending. |
| 155 | + |
| 156 | +Real-use evidence: |
| 157 | + |
| 158 | +- Pending. |
| 159 | + |
| 160 | +Reviewer findings: |
| 161 | + |
| 162 | +- Pending. |
| 163 | + |
| 164 | +Same-failure scan: |
| 165 | + |
| 166 | +- Pending. |
| 167 | + |
| 168 | +Sensitive data gate: |
| 169 | + |
| 170 | +- Pending. |
| 171 | + |
| 172 | +Artifact maintenance gate: |
| 173 | + |
| 174 | +- AGENTS.md: Pending. |
| 175 | +- Runtime project skills: Pending. |
| 176 | +- Specs: Pending. |
| 177 | +- End-user/operator docs: Pending. |
| 178 | +- End-user/operator skills: Pending. |
| 179 | +- SOW lifecycle: Pending. |
| 180 | + |
| 181 | +Specs update: |
| 182 | + |
| 183 | +- Pending. |
| 184 | + |
| 185 | +Project skills update: |
| 186 | + |
| 187 | +- Pending. |
| 188 | + |
| 189 | +End-user/operator docs update: |
| 190 | + |
| 191 | +- Pending. |
| 192 | + |
| 193 | +End-user/operator skills update: |
| 194 | + |
| 195 | +- Pending. |
| 196 | + |
| 197 | +Lessons: |
| 198 | + |
| 199 | +- Pending. |
| 200 | + |
| 201 | +Follow-up mapping: |
| 202 | + |
| 203 | +- Pending. |
| 204 | + |
| 205 | +## Outcome |
| 206 | + |
| 207 | +Pending. |
| 208 | + |
| 209 | +## Lessons Extracted |
| 210 | + |
| 211 | +Pending. |
| 212 | + |
| 213 | +## Followup |
| 214 | + |
| 215 | +None yet. |
| 216 | + |
| 217 | +## Regression Log |
| 218 | + |
| 219 | +None yet. |
0 commit comments