release-train: develop -> staging

.cursor/BUGBOT.md

@@ -125,3 +125,11 @@ consequence — what they see, and which exit code they get — not just the cod
 
 This repo is **public**: never put a customer name, internal hostname, or internal-only ticket
 detail in a finding. A bare `tracebloc/backend#NNNN` reference is fine.
+
+## Working with Bugbot findings (team norm)
+
+Every Bugbot review thread gets a reply, then gets resolved:
+- **Fixed**: say what changed and in which commit.
+- **False positive**: say why, with evidence (file/line, measured behavior).
+Unresolved cursor threads HOLD release-train promotions (soft gate) — an
+unaddressed finding blocks the fleet, not just this PR.

.github/workflows/code-quality-caller.yml

@@ -0,0 +1,22 @@
+name: Code quality
+
+on:
+  pull_request:
+    types: [opened, reopened, synchronize, ready_for_review]
+
+# Supersede the previous run when a branch is pushed again. Measured:
+# workflows missing this stack ~10-minute duplicate runs per push.
+concurrency:
+  group: code-quality-${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+
+jobs:
+  quality:
+    uses: tracebloc/.github/.github/workflows/code-quality.yml@main
+    with:
+      python: true          # repos with Python
+      shell: true           # repos with shell scripts
+      # soft-fail: false    # flip once the backlog is clear

.github/workflows/golangci.yml

@@ -0,0 +1,66 @@
+name: golangci-lint
+
+# Runs golangci-lint (with the gosec security linter) against the
+# repo's .golangci.yml on every PR + push to develop/main. This is the
+# "one tool, one config" successor being sized up for the standalone
+# lint steps in build.yml, and the first thing in this repo that scans
+# our own code for insecure patterns — govulncheck (build.yml +
+# vulncheck.yml) only covers known CVEs in dependencies.
+#
+# Why the action is safe now: the golangci-lint-action timeout story
+# (#6) was the SSA linters (staticcheck, unused) choking on the
+# k8s.io/* dep tree. The current .golangci.yml enables no SSA linters;
+# a full run measures ~11s wall locally / ~62s on the runner (#423).
+#
+# Part of backend#1305 (epic #930, Layer 1).
+
+on:
+  push:
+    branches: [develop, main]
+  pull_request:
+    branches: [develop, main]
+
+permissions:
+  contents: read
+
+concurrency:
+  group: golangci-${{ github.ref }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+jobs:
+  golangci:
+    timeout-minutes: 10
+    name: golangci-lint
+    runs-on: ubuntu-latest
+    # ============================= GATE ==============================
+    # Blocking by exit code since the backlog hit zero: the gosec
+    # findings sized on #423 were all resolved with per-site reviewed
+    # #nosec waivers (#427), so any finding this job reports from now
+    # on is NEW and fails the job — including typecheck errors, which
+    # ride the same exit path. History of the advisory era (the
+    # --issues-exit-code=0 flag, why job-level continue-on-error was
+    # not the tool) is in the #423/#426 discussions if you need it.
+    # Final step of the flip = marking this check required in branch
+    # protection (backend#1305 / epic #930).
+    # ==================================================================
+    steps:
+      - uses: actions/checkout@v7
+
+      - name: Set up Go
+        uses: actions/setup-go@v7
+        with:
+          go-version-file: go.mod
+          cache: true
+
+      # Both versions pinned for reproducibility, same policy as the
+      # standalone tools in build.yml (#127). golangci-lint v2.12.2 is
+      # built with Go 1.26 (required: go.mod says `go 1.26.0`; v1-era
+      # binaries can't typecheck this module). Bump deliberately, and
+      # keep the version in step with the format expectations noted in
+      # .golangci.yml AND with GOLANGCI_LINT_VERSION in the Makefile
+      # (make lint-full runs the same pinned version -- the local/CI
+      # mirror depends on the two never drifting).
+      - name: golangci-lint run (.golangci.yml)
+        uses: golangci/golangci-lint-action@v9.3.0
+        with:
+          version: v2.12.2

.golangci.yml

@@ -4,9 +4,24 @@
 # bugs without flooding PRs with style noise. Tune up over time
 # rather than turning everything on day one and quarantining half of
 # them.
+#
+# Format: golangci-lint v2 (`version: "2"`). The v1-format file died
+# with golangci-lint v1: v1 binaries are EOL and predate Go 1.26, so
+# they can't typecheck this module at all, and v2 binaries (what
+# `brew install golangci-lint` ships) refuse v1 configs. Migrated via
+# `golangci-lint migrate` (backend#1305, epic #930 Layer 1); needs
+# golangci-lint >= v2.
+#
+# CI: run by the advisory `golangci-lint` job in
+# .github/workflows/golangci.yml (pinned version there; keep it in
+# lockstep when the format needs a newer binary). The old
+# action-times-out story (#6) was about the SSA linters (staticcheck,
+# unused) on the k8s.io dep tree — this set has none, so the action
+# is safe with it.
+
+version: "2"
 
 run:
-  timeout: 5m
   # Track go.mod's `go` directive. Go's release cadence + `go mod
   # tidy`'s aggressive bumping (especially when k8s.io/* deps want
   # newer Go) keep dragging go.mod's minimum up; pinning a stale
@@ -17,42 +32,73 @@ run:
   go: "1.26"
 
 linters:
-  disable-all: true
+  default: none
   enable:
     # The set is trimmed to the linters that DON'T do full whole-program
     # SSA analysis. `staticcheck` and `unused` were in the original set
     # and reproducibly caused the GitHub-hosted runner to time-budget
     # the job (~2 min then shutdown signal) on this module — k8s.io/*
     # transitive deps inflate the analysis graph enough to OOM-or-stall
-    # the standard 4-CPU/16GB runner. Re-enabling them is a v0.2
-    # follow-up that needs either a larger runner, a much narrower
-    # scope (e.g. only `./internal/...`), or a faster successor like
-    # govulncheck for the security-only subset.
+    # the standard 4-CPU/16GB runner (#6). staticcheck now runs
+    # standalone in build.yml's Lint job instead.
 
     # Cheap, per-file checks — catch real bugs without SSA.
     - errcheck       # unchecked error returns
     - govet          # `go vet`
     - ineffassign    # assignments that go nowhere
 
+    # Security: insecure code patterns (G1xx-G6xx) — command injection,
+    # path traversal, weak crypto, world-writable files. Complements
+    # govulncheck (known CVEs in deps) with our-own-code checks; this is
+    # a customer-installed binary that shells out and writes to disk, so
+    # both halves matter (backend#1305, epic #930 Layer 1).
+    - gosec
+
     # Style / hygiene that pays for itself in code review time.
-    - gofmt
-    - goimports
     - misspell
     - unconvert      # unnecessary type conversions
 
-linters-settings:
-  goimports:
-    # Group imports: stdlib, third-party, our own. Keeps diffs
-    # readable when adding new imports.
-    local-prefixes: github.com/tracebloc/cli
+  exclusions:
+    # v1 had `exclude-use-default: false` — the default exclusions hide
+    # a lot of real findings, so keep opting back in (no presets).
+    generated: lax
+    rules:
+      # Test files often deliberately ignore err returns from
+      # bytes.Buffer / strings.Builder / fmt.Fprintf, which never fail.
+      # gosec is likewise test-exempt per convention: tests use fixed
+      # temp paths, os.Setenv, and relaxed perms that G-rules flag but
+      # that never ship in the binary.
+      - path: _test\.go
+        linters:
+          - errcheck
+          - gosec
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+
+# v2 moved the formatters out of `linters`. Same tools as before —
+# build.yml's Lint job runs the standalone equivalents (`gofmt -s`,
+# `goimports -local`).
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  settings:
+    goimports:
+      # Group imports: stdlib, third-party, our own. Keeps diffs
+      # readable when adding new imports.
+      local-prefixes:
+        - github.com/tracebloc/cli
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
 
 issues:
-  # Default exclusions hide a lot of real findings — opt back in.
-  exclude-use-default: false
-
-  exclude-rules:
-    # Test files often deliberately ignore err returns from
-    # bytes.Buffer / strings.Builder / fmt.Fprintf, which never fail.
-    - path: _test\.go
-      linters:
-        - errcheck
+  # Never cap repeated findings: the default (3) hid 10 of the 13 G304s
+  # behind a cache-flappy sample — "8 findings" were really 18 (#427).
+  # A gate must see the whole backlog, every run.
+  max-same-issues: 0

CONTRIBUTING.md

@@ -105,3 +105,7 @@ Coverage says a line *ran*; mutation testing says a test would *fail* if the lin
 4. File one issue per real gap, titled `test(<pkg>): pin <behavior> (mutation survivor)`, quoting the gremlins line (mutant type + file:line) and what behavior the missing test must pin. That issue then flows through the kanban like any other test ticket — #262, #263, #264 are the pattern.
 
 Survivors are *findings to triage*, not build failures — the workflow stays green even when mutants live, on purpose.
+
+## Bugbot findings
+
+See `.cursor/BUGBOT.md` — every thread gets a reply (fixed / false-positive-with-evidence), then resolved.

Makefile

@@ -9,11 +9,12 @@
 # ---- toggles -----------------------------------------------------
 
 GO            ?= go
-GOLANGCI_LINT ?= golangci-lint
 PKGS          := ./...
 
 # Pinned lint/analysis tool versions (reproducibility — no more @latest drift).
-# Keep these in lockstep with .github/workflows/build.yml. Bump deliberately.
+# Keep these in lockstep with .github/workflows/build.yml — and
+# GOLANGCI_LINT_VERSION with .github/workflows/golangci.yml. Bump deliberately.
+GOLANGCI_LINT_VERSION ?= v2.12.2
 ERRCHECK_VERSION    ?= v1.20.0
 INEFFASSIGN_VERSION ?= v0.2.0
 MISSPELL_VERSION    ?= v0.3.4
@@ -24,8 +25,11 @@ GOIMPORTS_VERSION   ?= v0.48.0
 
 # ---- top-level targets -------------------------------------------
 
+# ci mirrors the PR gates exactly — including golangci-lint (lint-full),
+# which fails on findings since #430. A green `make ci` must imply a green
+# PR; lint-full's own guard tells you how to install the tool if missing.
 .PHONY: ci
-ci: vet test lint fmt-check schema-check vulncheck file-budget deadcode check-style
+ci: vet test lint lint-full fmt-check schema-check vulncheck file-budget deadcode check-style
 	@echo "==> ci: all green"
 
 .PHONY: build
@@ -126,15 +130,14 @@ deadcode:
 vulncheck:
 	$(GO) run golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) ./...
 
+# Pinned to the exact version the golangci CI job runs (see
+# .github/workflows/golangci.yml), via the same `go run tool@version`
+# pattern as the tools above — no PATH dependency, so a green
+# `make ci` and the PR gate can never disagree on golangci version.
+# First run builds from source (~1-2 min); cached afterwards.
 .PHONY: lint-full
 lint-full:
-	@command -v $(GOLANGCI_LINT) >/dev/null 2>&1 || { \
-	  echo "==> $(GOLANGCI_LINT) not on PATH"; \
-	  echo "    install via:  brew install golangci-lint"; \
-	  echo "    or see:       https://golangci-lint.run/usage/install/"; \
-	  exit 1; \
-	}
-	$(GOLANGCI_LINT) run
+	$(GO) run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) run
 
 .PHONY: fmt
 fmt:

internal/cli/data.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -165,7 +166,7 @@ func runDataIngest(ctx context.Context, out, errOut io.Writer, a runDataIngestAr
 		// DROP/rm and then "succeed".
 		plan := push.PlanTeardown(existingTable)
 		rmSpin := a.Printer.Spinner(fmt.Sprintf("Removing the existing %q first", existingTable), "")
-		_, terr := push.Teardown(ctx, cs, &push.SPDYExecutor{Config: resolved.RestConfig, Client: cs}, resolved.Namespace, plan, push.PodSpecOptions{
+		tres, terr := push.Teardown(ctx, cs, &push.SPDYExecutor{Config: resolved.RestConfig, Client: cs}, resolved.Namespace, plan, push.PodSpecOptions{
 			Namespace:          resolved.Namespace,
 			PVCClaimName:       pvc.ClaimName,
 			PVCMountPath:       pvc.MountPath,
@@ -185,6 +186,13 @@ func runDataIngest(ctx context.Context, out, errOut io.Writer, a runDataIngestAr
 					"first, then re-run this ingest. Nothing new was staged. (%w)",
 				existingTable, existingTable, terr)}
 		}
+		if !tres.BookkeepingCleaned {
+			// Same surfacing `data delete` does (Bugbot on the PR): the
+			// overwrite pre-clean runs the identical teardown, and a silent
+			// bookkeeping failure here would hide the same schema-drift
+			// regression on this path.
+			a.Printer.Warnf("Bookkeeping cleanup incomplete — the old table is gone, but its run-journal/salt rows may remain: %s", strings.Join(tres.BookkeepingErrs, "; "))
+		}
 		a.Printer.Successf("Removed the old %q — ingesting the new data.", existingTable)
 	}
 

internal/cli/data_delete.go

@@ -212,7 +212,7 @@ undone — re-ingesting the data is the only way back.`)
 		p.Newline()
 		p.Successf("Dry-run — nothing was deleted.")
 		if a.OutputJSON {
-			writeDataDeleteJSON(a.JSONOut, "dry-run", resolved.Namespace, release.ReleaseName, plan, nil)
+			writeDataDeleteJSON(a.JSONOut, "dry-run", resolved.Namespace, release.ReleaseName, plan, nil, false)
 			jsonEmitted = true
 		}
 		return nil
@@ -234,7 +234,7 @@ undone — re-ingesting the data is the only way back.`)
 		// exit 0. One closure so the pair can't drift apart.
 		declined := func() error {
 			if a.OutputJSON {
-				writeDataDeleteJSON(a.JSONOut, "declined", resolved.Namespace, release.ReleaseName, plan, nil)
+				writeDataDeleteJSON(a.JSONOut, "declined", resolved.Namespace, release.ReleaseName, plan, nil, false)
 				jsonEmitted = true
 			}
 			return cleanCancel(p, "nothing was deleted.")
@@ -283,9 +283,15 @@ undone — re-ingesting the data is the only way back.`)
 
 	p.Newline()
 	p.Successf("Deleted %s.%s and %d PVC path(s).", plan.Database, plan.Table, len(res.RemovedPaths))
+	if !res.BookkeepingCleaned {
+		// Best-effort cleanup failed — say so, or a schema-drift regression
+		// (a renamed keying column) is indistinguishable from a legacy
+		// cluster without the bookkeeping tables (review, Saqlain).
+		p.Warnf("Bookkeeping cleanup incomplete — the table is gone, but its run-journal/salt rows may remain: %s", strings.Join(res.BookkeepingErrs, "; "))
+	}
 	p.Infof("The dataset's catalog metadata is kept as a record on tracebloc, marked unavailable — never removed.")
 	if a.OutputJSON {
-		writeDataDeleteJSON(a.JSONOut, "deleted", resolved.Namespace, release.ReleaseName, plan, res.RemovedPaths)
+		writeDataDeleteJSON(a.JSONOut, "deleted", resolved.Namespace, release.ReleaseName, plan, res.RemovedPaths, res.BookkeepingCleaned)
 		jsonEmitted = true
 	}
 	return nil
@@ -302,12 +308,17 @@ type dataDeleteJSON struct {
 	Table        string   `json:"table"` // the REAL (case-resolved) spelling, not the raw argument
 	PVCPaths     []string `json:"pvc_paths"`
 	RemovedPaths []string `json:"removed_paths"`
+	// BookkeepingCleaned mirrors push.TeardownResult: whether the
+	// run-journal/salt rows were removed with the table. Always false for
+	// dry-run/declined — nothing was attempted, and a strict consumer must
+	// never read "cleanup happened" out of a run that deleted nothing.
+	BookkeepingCleaned bool `json:"bookkeeping_cleaned"`
 }
 
 // writeDataDeleteJSON serializes the delete result to w (stdout in
 // --output-json mode). Marshal errors are dropped: marshaling our own
 // struct can't fail in practice, and the exit code remains the contract.
-func writeDataDeleteJSON(w io.Writer, status, namespace, release string, plan push.TeardownPlan, removed []string) {
+func writeDataDeleteJSON(w io.Writer, status, namespace, release string, plan push.TeardownPlan, removed []string, bookkeepingCleaned bool) {
 	pvcPaths := plan.PVCPaths
 	if pvcPaths == nil {
 		pvcPaths = []string{} // emit [] not null
@@ -316,13 +327,14 @@ func writeDataDeleteJSON(w io.Writer, status, namespace, release string, plan pu
 		removed = []string{} // emit [] not null
 	}
 	res := dataDeleteJSON{
-		Status:       status,
-		Namespace:    namespace,
-		Release:      release,
-		Database:     plan.Database,
-		Table:        plan.Table,
-		PVCPaths:     pvcPaths,
-		RemovedPaths: removed,
+		Status:             status,
+		Namespace:          namespace,
+		Release:            release,
+		Database:           plan.Database,
+		Table:              plan.Table,
+		PVCPaths:           pvcPaths,
+		RemovedPaths:       removed,
+		BookkeepingCleaned: bookkeepingCleaned,
 	}
 	b, err := json.MarshalIndent(res, "", "  ")
 	if err != nil {

internal/cli/home_local_fallback.go

@@ -110,7 +110,7 @@ func tbAliasAvailable() bool {
 // different tracebloc at another path (Bugbot). Case-insensitive: .cmd is a
 // Windows artifact and NTFS paths are case-insensitive.
 func tbCmdAliasOurs(dir, exe string) bool {
-	b, err := os.ReadFile(filepath.Join(dir, binTB+".cmd"))
+	b, err := os.ReadFile(filepath.Join(dir, binTB+".cmd")) // #nosec G304 -- fixed name next to os.Executable(): inspects the install dir's own tb.cmd shim; whoever controls that dir already controls the binary.
 	if err != nil {
 		return false
 	}