SonarCloud first scan: five valid findings #153

First SonarCloud scan (#149's second half) reported 118 issues. Triaged: 8 are valid across five distinct fixes, 110 are false positives in four groups.

** Valid

1. Protocol-relative redirect (gosecurity:S5146, BLOCKER ×2). internal/httpd/pages.go:80 and :107 pass r.URL.Path to http.Redirect unnormalised, while the sibling reqPath on line 71 is already path.Cleaned. A request for //evil.example produces Location: //evil.example/, which a browser follows off-site — confirmed by running http.Redirect directly. Reaching it needs a public repo whose name is the target host, and repo names permit dots, so evil.com is legal. Low exploitability, real mechanism.

2. Predictable runner workspace (go:S5445, CRITICAL). cmd/gitbay-runner/main.go defaults -workdir to /tmp/gitbay-runner and creates it 0o755 with MkdirAll, which succeeds against a directory someone else already owns. bay1 is not exposed — its unit passes -workdir /var/lib/gitbay-runner/work — but the default is what anyone gets running the binary by hand, and this is the process that executes untrusted build steps.

3. Cookie clearing drops its attributes (go:S3330 ×2, go:S2092 ×2). Logout (accounts.go:106) and flash consumption (flash.go:38) clear with a bare cookie while the setting calls specify HttpOnly, SameSite and conditional Secure. Deletion still works, so this is consistency rather than a live bug — four findings for a two-line change, and it stops a reviewer comparing the two paths and wondering.

4. Unlabelled input (Web:InputWithoutLabelCheck). settings.html topics-remove has neither id nor label while the input above it has both. A placeholder is not an accessible name. The class #133 fixed elsewhere and missed here.

5. PL/SQL rules on SQLite migrations — config, not code. internal/store/migrations/** is analysed as PL/SQL, where '' is NULL; SQLite's is not, and both flagged lines compare to '' on NOT NULL DEFAULT '' columns. Excluded in sonar-project.properties so it does not recur.

** Dismissed, with reasons recorded in the scan

• go:S4036 ×74 — PATH is systemd's and /usr is read-only under ProtectSystem, including for build steps.
• go:S2077 ×31 — all 31 checked: 29 join adjacent literals, 2 join a const and two parameters whose only callers pass literals. No user value reaches SQL text.
• go:S2092 ×2 (the setting calls) — Secure is set, conditionally on TLS being on.
• Web:S5256 ×1 — the diff table is a code layout with no header semantics.

Worth deciding separately: 63% of the dashboard is go:S4036, and a dashboard that is mostly permanent noise stops being read — the same failure mode as #152. Resolving git once with exec.LookPath at startup would remove all 74 and turn a missing git into an explicit startup failure. Mechanical across ~74 call sites; not done here.