From 7cce4d6d6c1de621473539b26b082e329d601baf Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Wed, 3 Jun 2026 00:04:36 +0200 Subject: [PATCH 1/4] feat(docs): bootstrap RULE blocks in go-logging-guide.md (+6 rules) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule count: 124 -> 130. Added 6 RULE blocks to a previously 0-rule doc: - go-logging/no-mixing-slog-and-glog (MUST) - go-logging/no-log-and-return-error (MUST) - go-logging/external-call-logs-response (MUST) - go-logging/no-sensitive-data-in-logs (MUST) - go-logging/lowercase-log-messages (SHOULD) - go-logging/no-tight-loop-without-sampler (SHOULD) All carry judgment-tier enforcement with ast-grep partial hints where applicable; the 4 MUSTs are semantic (project-wide logger choice, inner-vs-boundary intent, boundary-call recognition, credential-shaped variable detection) and need agent adjudication to avoid false-positive fatigue. Existing operator-content (slog/glog usage, sampling, runtime log level, related rules) preserved verbatim below the Rules section — this is a doc bootstrap, not a rewrite. make precommit clean: 130 rules, 20 mechanical YAMLs, no drift. Closes part of 'All 67 docs carry rule-block structure for their MUST/SHOULD content' success criterion. 0-rule docs remaining: 15 (was 16). --- docs/go-logging-guide.md | 251 ++++++++++++++++++++++++++++++--------- rules/index.json | 54 +++++++++ 2 files changed, 251 insertions(+), 54 deletions(-) diff --git a/docs/go-logging-guide.md b/docs/go-logging-guide.md index aa3150e..940fea4 100644 --- a/docs/go-logging-guide.md +++ b/docs/go-logging-guide.md @@ -2,6 +2,199 @@ **New projects:** `log/slog` (stdlib). **Existing with glog:** keep `glog`, don't mix. +## Rules + +### RULE go-logging/no-mixing-slog-and-glog (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a Go file in a project that already uses one logging library (`log/slog` OR `github.com/golang/glog`) imports the other one — meaning the codebase emits both stdlib structured logs and glog's V-leveled string logs from the same binary. +**Enforcement**: judgment (semantic — distinguishing "this binary already uses X; the new import of Y is wrong" from "this is a new binary that happens to coexist with X-using siblings in the monorepo" requires checking which logger the rest of the same module uses; ast-grep partial: detect both `import "log/slog"` AND `import "github.com/golang/glog"` co-occurring in any single `.go` file) +**Why**: Mixing the two loggers in one binary fragments the operator's view: log aggregators see two different formats (slog's structured key-values vs glog's free-form Errorf strings), grep patterns from one side miss the other, and verbosity gating (`-v` for glog, log level for slog) controls only half the output. Migration is an explicit project-level decision — either all-slog or all-glog — not a per-file choice that creeps in during PR review. + +#### Bad + +```go +import ( + "log/slog" + "github.com/golang/glog" +) + +func process() { + slog.Info("starting", "items", 42) // structured + glog.Infof("done with %d items", 42) // unstructured — different format same binary +} +``` + +#### Good + +```go +// Existing glog project — keep glog throughout +import "github.com/golang/glog" + +func process() { + glog.Infof("starting items=%d", 42) + glog.Infof("done items=%d", 42) +} +``` + +### RULE go-logging/no-log-and-return-error (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a Go function logs an error (via `glog.Errorf` / `slog.Error` / `log.Printf` / similar) and then also returns the same error to the caller — meaning the error is reported twice: once at the inner site and again upstream when the caller (or its caller) logs the propagated error. +**Enforcement**: judgment (semantic — distinguishing "intentional inner log + return for diagnostic visibility" from "accidental double-log" requires reading the caller's handling; ast-grep partial: `if err != nil { glog.Errorf(...); return ..., err }` pattern in any non-main, non-test Go file) +**Why**: Logging at the inner site AND returning means the same error lands in the log twice — once with the inner context, once with the outer wrapper. Aggregators count it as two events, alerts double-fire, and the stack-trace breadcrumbs disagree about where the error actually originated. Pick one: either log at the boundary (`main.go`, top-level handler) and let the inner code return; OR consume the error inline (log it, recover, return success). Never both. + +#### Bad + +```go +func fetchUser(ctx context.Context, id string) (*User, error) { + user, err := store.Get(ctx, id) + if err != nil { + glog.Errorf("failed to fetch user %s: %v", id, err) // logged here + return nil, errors.Wrapf(ctx, err, "fetch user") // AND returned — logged again upstream + } + return user, nil +} +``` + +#### Good + +```go +// Inner: return only — the boundary decides what to log +func fetchUser(ctx context.Context, id string) (*User, error) { + user, err := store.Get(ctx, id) + if err != nil { + return nil, errors.Wrapf(ctx, err, "fetch user %s", id) + } + return user, nil +} + +// Boundary (HTTP handler / main.go): log once with full context +func (h *handler) Get(w http.ResponseWriter, r *http.Request) { + user, err := h.svc.FetchUser(r.Context(), id) + if err != nil { + glog.Errorf("get user %s: %+v", id, err) // log once at the boundary + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } +} +``` + +### RULE go-logging/external-call-logs-response (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a Go function makes a call that crosses the process boundary (HTTP client, gRPC client, database query, message-bus publish, subprocess `exec`) without emitting a single log line summarising the call's outcome (method/op + status/result + latency, plus error message on failure). +**Enforcement**: judgment (semantic — identifying "this is a boundary call" requires recognising the client package (`http.Client.Do`, `*sql.DB.Query`, `client.SomeRPC`, `kafka.Producer.Send`, `exec.Command`) and verifying no log statement follows; ast-grep partial: `call_expression` matching known boundary clients without a sibling glog/slog call in the same block) +**Why**: Boundary calls are the audit trail. Without a log line per call, runtime mysteries — "did the payment send? did the webhook deliver? did the job enqueue?" — become guesswork from indirect signals (downstream alerts, partial state in a DB, support tickets). The log line lets operators answer "what crossed the wire?" without redeploying with extra instrumentation. Minimum payload: method + path/op + status code + latency; add error message on non-success. Never log credentials, request bodies with secrets, or full response bodies — log lengths/counts instead. + +#### Bad + +```go +// Silent boundary — caller has no audit trail +status, body, err := doRequest(ctx, client, token, "POST", url, payload) +if err != nil { + return err +} +return parseBody(body) +``` + +#### Good + +```go +status, body, err := doRequest(ctx, client, token, "POST", url, payload) +glog.Infof("http POST %s status=%d body_len=%d", path, status, len(body)) +if err != nil { + glog.Warningf("http POST %s failed: %v", path, err) + return err +} +return parseBody(body) +``` + +### RULE go-logging/no-sensitive-data-in-logs (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material (password, token, secret, key, PEM, JWT, raw request body containing credentials, full session cookie). +**Enforcement**: judgment (semantic — distinguishing "credential" from "credential-shaped name on a public value (e.g. `publicKey`)" requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.*` / `slog.*` with format args including identifiers matching `(?i)password|token|secret|.*Key|PEM|JWT|credential|authorization`) +**Why**: Logged credentials land in stdout, log aggregators, cloud logging backends — searchable, indexed, and impossible to redact once the batch has shipped. A single `glog.Infof("config: %+v", cfg)` on a struct that contains a `PEMKey` field leaks the key to every operator with log access. Use `display:"length"` tags (see `go-k8s-binary/secret-fields-need-display-length`), log lengths instead of values (`body_len=%d`), and never interpolate raw credential-shaped variables into log strings. + +#### Bad + +```go +// %+v dumps the whole struct including the PEMKey field — secret in the log +glog.Infof("config: %+v", cfg) + +// Direct interpolation +glog.Infof("authenticated with token=%s", token) +``` + +#### Good + +```go +// Length only — operators know it's set without seeing the value +glog.Infof("config: stage=%s pem_key_len=%d", cfg.Stage, len(cfg.PEMKey)) +glog.Infof("authenticated token_len=%d", len(token)) +``` + +### RULE go-logging/lowercase-log-messages (SHOULD) + +**Owner**: go-quality-assistant +**Applies when**: a Go log call's message string starts with an uppercase letter (`glog.Infof("Service started ...")`, `slog.Info("Failed to ...")`). +**Enforcement**: judgment (ast-grep follow-up: pattern over `glog.{Info,Warning,Error,Fatal}{,f}` / `slog.{Info,Warn,Error,Debug}` first-string-literal argument matching `^"[A-Z]`) +**Why**: Convention — log lines are streamed mid-sentence into structured-log search tools; lowercase makes the multi-source stream look uniform (no random capitalised mid-line "Failed to" tokens), and matches Go's stdlib `log` package convention. Cheap to maintain via grep; consistency pays off when operators eyeball thousands of log lines per minute. + +#### Bad + +```go +glog.Infof("Service started on port %d", port) +slog.Error("Failed to parse request", "error", err) +``` + +#### Good + +```go +glog.Infof("service started on port %d", port) +slog.Error("failed to parse request", "error", err) +``` + +### RULE go-logging/no-tight-loop-without-sampler (SHOULD) + +**Owner**: go-quality-assistant +**Applies when**: a Go log statement appears inside a `for` loop body that processes more than ~100 items per iteration without being gated by a `log.Sampler` / `IsSample()` check or a `glog.V(N)` verbosity guard. +**Enforcement**: judgment (semantic — distinguishing "tight inner loop" from "small bounded outer loop" requires reading the loop bound; ast-grep partial: `call_expression` matching `glog.{Info,Warning}{,f}` / `slog.{Info,Warn}` inside a `for_statement` body without a sibling `if .IsSample()` / `glog.V(N)` guard) +**Why**: Unsampled log calls in hot paths produce log gigabytes per minute — drowning real operator signal, inflating cloud-logging cost, and adding non-trivial latency to the loop itself (sync.Mutex contention inside `glog` on heavy concurrent writes). Sampling preserves the "is something happening?" signal at sustainable volume. `github.com/bborbe/log` provides `NewSampleTime(d)` (once per duration), `NewSampleMod(n)` (every N-th), `NewSamplerGlogLevel(n)` (gated by verbosity). + +#### Bad + +```go +for _, item := range items { + if err := process(item); err != nil { + glog.Warningf("process %s failed: %v", item.ID, err) // every iteration, no sampler + } +} +``` + +#### Good + +```go +for _, item := range items { + if err := process(item); err != nil { + if h.logSampler.IsSample() { + glog.Warningf("process %s failed: %v", item.ID, err) + } + } +} +// Or: aggregate and log once after the loop +var failures int +for _, item := range items { + if err := process(item); err != nil { + failures++ + } +} +if failures > 0 { + glog.Warningf("processed %d items, %d failures", len(items), failures) +} +``` + ## slog (New Projects) ```go @@ -77,58 +270,8 @@ curl http://pod:9090/setloglevel/4 # 4 = +V(4) debug paths Wiring: see [go-http-service-guide.md](go-http-service-guide.md) for the canonical admin endpoint block (router registration, baseline level, TTL). -## External Calls — Always Log - -Any call that crosses the process boundary (HTTP, gRPC, DB, message bus, subprocess) is logged on response. Without this, runtime mysteries — "did the payment send? did the webhook deliver? did the job enqueue?" — become guesswork from indirect signals. The log line is the audit trail. - -Minimum payload: method + path/op + status code + latency. Add error message on non-success. Never log credentials, request bodies with secrets, or full response bodies — log lengths/counts instead. - -```go -// HTTP client — one log line per call -// [GOOD] -status, body, err := doRequest(ctx, client, token, "POST", url, payload) -glog.Infof("http POST %s status=%d body_len=%d", path, status, len(body)) -if err != nil { - glog.Warningf("http POST %s failed: %v", path, err) -} - -// [BAD] — silent boundary: caller has no audit trail -status, body, err := doRequest(ctx, client, token, "POST", url, payload) -if err != nil { - return err -} - -// gRPC client — log on response -// [GOOD] -resp, err := client.SomeRPC(ctx, req) -glog.Infof("rpc %s.%s status=%v", service, method, statusOf(err)) - -// DB query — log latency on hot queries -// [GOOD] -start := time.Now() -rows, err := db.QueryContext(ctx, query, args...) -glog.Infof("db query=%s rows=%d elapsed=%v", queryName, count, time.Since(start)) -``` - -**Hot path?** Wrap with a sampler so high-frequency calls don't drown the log: - -```go -if c.logSampler.IsSample() { - glog.Infof("queue publish topic=%s partition=%d offset=%d", topic, p, offset) -} -``` - -**Verbosity choice**: `glog.Infof` (V0) for low-frequency external calls where every call matters (payments, deploys, webhook sends, audit-relevant API writes). `glog.V(2)` + sampler for high-frequency (message-bus publishes, cache lookups, polling). Avoid `V(3)+` for boundary calls — they're operational signal, not debug detail. - -**What to grep for later**: pick a consistent prefix per boundary so `kubectl logs ... | grep http` returns everything. Examples: `http POST`, `rpc UserService.GetUser`, `db query`, `subprocess exec`. - -## Rules +## Related Rules -- **Don't mix** slog and glog in the same project -- **Don't log + return error** — do one or the other -- **Lowercase messages** -- **No sensitive data** — no tokens, passwords, PII -- **Log at boundaries** — handlers, processors, startup — not deep internals -- **Don't log in tight loops** — log aggregated result, or use sampler -- **V(2) with nothing to report** — skip the log line, or use sampling -- **Every external call logs its response** — see "External Calls" above. Default V0 for low-frequency; sampled V(2) for hot paths. No silent boundaries. +- [`go-cli/slog-not-glog-in-new-projects`](go-cli-guide.md) — new projects should pick `slog`, not introduce `glog` +- [`go-glog/use-v-for-debug-not-info`](go-glog-guide.md) — V0 (bare `glog.Info`) is operator-default; debug-shaped lines go behind `V(N)` +- [`go-k8s-binary/secret-fields-need-display-length`](go-k8s-binary-conventions.md) — application-config secret fields carry `display:"length"` so `argument.Parse()` startup dump never prints values diff --git a/rules/index.json b/rules/index.json index 2955cf6..c23aba0 100644 --- a/rules/index.json +++ b/rules/index.json @@ -611,6 +611,60 @@ "level": "MUST", "owner": "go-quality-assistant" }, + { + "anchor": "go-logging/external-call-logs-response", + "applies_when": "a Go function makes a call that crosses the process boundary (HTTP client, gRPC client, database query, message-bus publish, subprocess `exec`) without emitting a single log line summarising the call's outcome (method/op + status/result + latency, plus error message on failure).", + "doc_path": "docs/go-logging-guide.md", + "enforcement": "judgment (semantic — identifying \"this is a boundary call\" requires recognising the client package (`http.Client.Do`, `*sql.DB.Query`, `client.SomeRPC`, `kafka.Producer.Send`, `exec.Command`) and verifying no log statement follows; ast-grep partial: `call_expression` matching known boundary clients without a sibling glog/slog call in the same block)", + "id": "go-logging/external-call-logs-response", + "level": "MUST", + "owner": "go-quality-assistant" + }, + { + "anchor": "go-logging/lowercase-log-messages", + "applies_when": "a Go log call's message string starts with an uppercase letter (`glog.Infof(\"Service started ...\")`, `slog.Info(\"Failed to ...\")`).", + "doc_path": "docs/go-logging-guide.md", + "enforcement": "judgment (ast-grep follow-up: pattern over `glog.{Info,Warning,Error,Fatal}{,f}` / `slog.{Info,Warn,Error,Debug}` first-string-literal argument matching `^\"[A-Z]`)", + "id": "go-logging/lowercase-log-messages", + "level": "SHOULD", + "owner": "go-quality-assistant" + }, + { + "anchor": "go-logging/no-log-and-return-error", + "applies_when": "a Go function logs an error (via `glog.Errorf` / `slog.Error` / `log.Printf` / similar) and then also returns the same error to the caller — meaning the error is reported twice: once at the inner site and again upstream when the caller (or its caller) logs the propagated error.", + "doc_path": "docs/go-logging-guide.md", + "enforcement": "judgment (semantic — distinguishing \"intentional inner log + return for diagnostic visibility\" from \"accidental double-log\" requires reading the caller's handling; ast-grep partial: `if err != nil { glog.Errorf(...); return ..., err }` pattern in any non-main, non-test Go file)", + "id": "go-logging/no-log-and-return-error", + "level": "MUST", + "owner": "go-quality-assistant" + }, + { + "anchor": "go-logging/no-mixing-slog-and-glog", + "applies_when": "a Go file in a project that already uses one logging library (`log/slog` OR `github.com/golang/glog`) imports the other one — meaning the codebase emits both stdlib structured logs and glog's V-leveled string logs from the same binary.", + "doc_path": "docs/go-logging-guide.md", + "enforcement": "judgment (semantic — distinguishing \"this binary already uses X; the new import of Y is wrong\" from \"this is a new binary that happens to coexist with X-using siblings in the monorepo\" requires checking which logger the rest of the same module uses; ast-grep partial: detect both `import \"log/slog\"` AND `import \"github.com/golang/glog\"` co-occurring in any single `.go` file)", + "id": "go-logging/no-mixing-slog-and-glog", + "level": "MUST", + "owner": "go-quality-assistant" + }, + { + "anchor": "go-logging/no-sensitive-data-in-logs", + "applies_when": "a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material (password, token, secret, key, PEM, JWT, raw request body containing credentials, full session cookie).", + "doc_path": "docs/go-logging-guide.md", + "enforcement": "judgment (semantic — distinguishing \"credential\" from \"credential-shaped name on a public value (e.g. `publicKey`)\" requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.*` / `slog.*` with format args including identifiers matching `(?i)password|token|secret|.*Key|PEM|JWT|credential|authorization`)", + "id": "go-logging/no-sensitive-data-in-logs", + "level": "MUST", + "owner": "go-security-specialist" + }, + { + "anchor": "go-logging/no-tight-loop-without-sampler", + "applies_when": "a Go log statement appears inside a `for` loop body that processes more than ~100 items per iteration without being gated by a `log.Sampler` / `IsSample()` check or a `glog.V(N)` verbosity guard.", + "doc_path": "docs/go-logging-guide.md", + "enforcement": "judgment (semantic — distinguishing \"tight inner loop\" from \"small bounded outer loop\" requires reading the loop bound; ast-grep partial: `call_expression` matching `glog.{Info,Warning}{,f}` / `slog.{Info,Warn}` inside a `for_statement` body without a sibling `if .IsSample()` / `glog.V(N)` guard)", + "id": "go-logging/no-tight-loop-without-sampler", + "level": "SHOULD", + "owner": "go-quality-assistant" + }, { "anchor": "go-makefile/test-not-build-for-verification", "applies_when": "a Go project's CI script, contributor onboarding doc, or commit message references `go build ./...` as the verification step for \"this change compiles and is safe to merge\", instead of `make test` (which wraps Ginkgo + race detection + coverage thresholds).", From 1bba0f4f562ce4ec9479b906a9b32bc97144d2af Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Wed, 3 Jun 2026 00:23:55 +0200 Subject: [PATCH 2/4] fix(docs): add skip-empty-v2-heartbeats rule per PR #35 bot review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot flagged that 'V(2) with nothing to report — skip the log line, or use sampling' bullet from the original go-logging-guide.md Rules section was dropped during the bootstrap conversion. Added as the 7th formal RULE block. Owner: go-quality-assistant. SHOULD-level (not MUST — it's a production-tuning recommendation, not a correctness issue). Enforcement is judgment-tier with ast-grep partial hint: 'call_expression matching glog.V(2).Info*(...) inside for_statement body without a preceding guard'. Bad/Good examples show the typical scan-cycle-zero-changed pattern. Bot MINOR re backticks in JSON applies_when string: not addressing — JSON spec doesn't treat backticks as special, no real parser issue. Multiple existing rules in index.json use backtick-wrapped code in applies_when text already; precedent stands. Rule count: 130 -> 131. make precommit clean, no drift. --- docs/go-logging-guide.md | 37 +++++++++++++++++++++++++++++++++++++ rules/index.json | 9 +++++++++ 2 files changed, 46 insertions(+) diff --git a/docs/go-logging-guide.md b/docs/go-logging-guide.md index 940fea4..95e465a 100644 --- a/docs/go-logging-guide.md +++ b/docs/go-logging-guide.md @@ -195,6 +195,43 @@ if failures > 0 { } ``` +### RULE go-logging/skip-empty-v2-heartbeats (SHOULD) + +**Owner**: go-quality-assistant +**Applies when**: a Go function emits a `glog.V(2).Infof(...)` heartbeat log line unconditionally — meaning the line runs every iteration / cycle / poll regardless of whether anything observable happened (zero items processed, no state change, no signal worth recording). +**Enforcement**: judgment (semantic — distinguishing "this V(2) heartbeat carries real signal every iteration" from "this fires every loop body and contributes nothing" requires reading the surrounding code; ast-grep partial: `call_expression` matching `glog.V(2).Info*(...)` inside a `for_statement` body without a preceding guard `if N > 0` / `if changed` / sampler check) +**Why**: V(2) is the production-heartbeat level (default `LOGLEVEL=2`). Unconditional V(2) heartbeats produce log volume proportional to iteration frequency, even when nothing happened — `scan cycle: 0 changed` lines drowning the actual state changes that V(2) exists to surface. Guard each V(2) heartbeat with a "something happened" check (`if changed > 0`, `if n > 0`) or wrap it in a sampler (`log.NewSampleTime(10s)`). Operators reading V(2) want to know what changed, not that the loop ran. + +#### Bad + +```go +for { + changed, err := scanForUpdates(ctx) + if err != nil { + glog.Errorf("scan failed: %v", err) + continue + } + glog.V(2).Infof("scan cycle: %d changed", changed) // fires even when changed=0 + time.Sleep(interval) +} +``` + +#### Good + +```go +for { + changed, err := scanForUpdates(ctx) + if err != nil { + glog.Errorf("scan failed: %v", err) + continue + } + if changed > 0 { + glog.V(2).Infof("scan cycle: %d changed", changed) // only when there's something to report + } + time.Sleep(interval) +} +``` + ## slog (New Projects) ```go diff --git a/rules/index.json b/rules/index.json index c23aba0..18396dc 100644 --- a/rules/index.json +++ b/rules/index.json @@ -665,6 +665,15 @@ "level": "SHOULD", "owner": "go-quality-assistant" }, + { + "anchor": "go-logging/skip-empty-v2-heartbeats", + "applies_when": "a Go function emits a `glog.V(2).Infof(...)` heartbeat log line unconditionally — meaning the line runs every iteration / cycle / poll regardless of whether anything observable happened (zero items processed, no state change, no signal worth recording).", + "doc_path": "docs/go-logging-guide.md", + "enforcement": "judgment (semantic — distinguishing \"this V(2) heartbeat carries real signal every iteration\" from \"this fires every loop body and contributes nothing\" requires reading the surrounding code; ast-grep partial: `call_expression` matching `glog.V(2).Info*(...)` inside a `for_statement` body without a preceding guard `if N > 0` / `if changed` / sampler check)", + "id": "go-logging/skip-empty-v2-heartbeats", + "level": "SHOULD", + "owner": "go-quality-assistant" + }, { "anchor": "go-makefile/test-not-build-for-verification", "applies_when": "a Go project's CI script, contributor onboarding doc, or commit message references `go build ./...` as the verification step for \"this change compiles and is safe to merge\", instead of `make test` (which wraps Ginkgo + race detection + coverage thresholds).", From 9a5bb2b20b777eecafd1db067ff999dce087c0d2 Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Wed, 3 Jun 2026 00:43:34 +0200 Subject: [PATCH 3/4] fix(docs): address PR #35 bot CRITICAL+MAJOR review on go-logging-guide.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5 CRITICAL fixed: - no-log-and-return-error Good example: added 'id := chi.URLParam(r, "id")' so the snippet is self-contained - no-tight-loop-without-sampler Good example: added comment pointing reader to the Log Sampling section for h.logSampler wiring context - no-sensitive-data-in-logs Enforcement regex: rewrote with word boundaries and explicit credential-shaped key list. Was: (?i)password|token|secret|.*Key|PEM|JWT|credential|authorization Now: (?i)(password|passphrase|token|credential|authorization|jwt|pem|dsn|connection[_ ]?string)\b|(private|signing|encryption|api)Key\b - Drops bare '.*Key' (was matching partitionKey/sortKey/lookupKey) - Adds \b on 'secret' (was matching secretName/secretRef/secretNamespace) - Groups (?i) so case-insensitivity covers the whole alternation - Explicit credential-shaped key list (private/signing/encryption/api) 6 MAJOR fixed: - no-mixing-slog-and-glog: added rule-boundary paragraph vs go-cli/slog-not-glog-in-new-projects (new-binary check vs same-binary-mixes-both check) - no-log-and-return-error: cross-link to go-errors/no-bare-return-err in Related rules note - external-call-logs-response: added 'Carve-out from no-log-and-return-error' paragraph; restructured Good example to emit ONE audit line per call (success branch OR failure branch, never both); cross-link to no-tight-loop-without-sampler for hot paths - no-sensitive-data-in-logs applies_when: expanded to name Authorization headers, DSNs, OAuth client secrets, X.509 certs, whole-request/response struct dumps via %+v - no-sensitive-data-in-logs Good examples: added slog key-value pair examples (rule covered slog but examples were glog-only) - no-sensitive-data-in-logs Bad examples: added 'glog.Infof("request: %+v", r)' as PRIMARY bad example (most common Authorization-header leak vector per bot) 4 NITs fixed: - no-tight-loop-without-sampler applies_when: replaced '~100 items' threshold with 'iteration count unbounded at compile time' + judgment guidance tied to ops/sec - skip-empty-v2-heartbeats Bad/Good: added 'select+ctx.Done()' for graceful shutdown; cross-link to go-context-cancellation-in-loops - external-call-logs-response: added Related cross-link to no-tight-loop-without-sampler for sampled audit lines in hot paths - Related Rules section: added go-k8s-binary/secret-fields-need-display-length, go-errors/no-bare-return-err, go-context-cancellation-in-loops NIT not addressed: bot raised concern about backticks in applies_when string causing JSON validation tool issues. JSON spec doesn't treat backticks as special — multiple existing rules use backtick-wrapped code in applies_when. Precedent stands. Rule count unchanged: 131. make precommit clean. --- docs/go-logging-guide.md | 65 ++++++++++++++++++++++++++++++++-------- rules/index.json | 6 ++-- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/docs/go-logging-guide.md b/docs/go-logging-guide.md index 95e465a..aaf943c 100644 --- a/docs/go-logging-guide.md +++ b/docs/go-logging-guide.md @@ -11,6 +11,8 @@ **Enforcement**: judgment (semantic — distinguishing "this binary already uses X; the new import of Y is wrong" from "this is a new binary that happens to coexist with X-using siblings in the monorepo" requires checking which logger the rest of the same module uses; ast-grep partial: detect both `import "log/slog"` AND `import "github.com/golang/glog"` co-occurring in any single `.go` file) **Why**: Mixing the two loggers in one binary fragments the operator's view: log aggregators see two different formats (slog's structured key-values vs glog's free-form Errorf strings), grep patterns from one side miss the other, and verbosity gating (`-v` for glog, log level for slog) controls only half the output. Migration is an explicit project-level decision — either all-slog or all-glog — not a per-file choice that creeps in during PR review. +**Rule boundary vs `go-cli/slog-not-glog-in-new-projects`**: that rule fires on *new* binaries that pick `glog` (the "you should have picked slog" check). This rule fires on *any* binary — new or existing — that uses both loggers (the "you've mixed them in one binary" check). The two are independent: a new project mixing both triggers both rules; an existing all-glog project introducing slog triggers only this one. + #### Bad ```go @@ -71,15 +73,19 @@ func fetchUser(ctx context.Context, id string) (*User, error) { // Boundary (HTTP handler / main.go): log once with full context func (h *handler) Get(w http.ResponseWriter, r *http.Request) { + id := chi.URLParam(r, "id") user, err := h.svc.FetchUser(r.Context(), id) if err != nil { glog.Errorf("get user %s: %+v", id, err) // log once at the boundary http.Error(w, err.Error(), http.StatusInternalServerError) return } + respondJSON(w, user) } ``` +**Related rules**: see [`go-errors/no-bare-return-err`](go-error-wrapping-guide.md) and the surrounding error-wrapping family — the boundary-logging pattern depends on inner callers properly wrapping errors via `errors.Wrapf(ctx, err, ...)` so the boundary log line carries the full context chain. + ### RULE go-logging/external-call-logs-response (MUST) **Owner**: go-quality-assistant @@ -87,6 +93,8 @@ func (h *handler) Get(w http.ResponseWriter, r *http.Request) { **Enforcement**: judgment (semantic — identifying "this is a boundary call" requires recognising the client package (`http.Client.Do`, `*sql.DB.Query`, `client.SomeRPC`, `kafka.Producer.Send`, `exec.Command`) and verifying no log statement follows; ast-grep partial: `call_expression` matching known boundary clients without a sibling glog/slog call in the same block) **Why**: Boundary calls are the audit trail. Without a log line per call, runtime mysteries — "did the payment send? did the webhook deliver? did the job enqueue?" — become guesswork from indirect signals (downstream alerts, partial state in a DB, support tickets). The log line lets operators answer "what crossed the wire?" without redeploying with extra instrumentation. Minimum payload: method + path/op + status code + latency; add error message on non-success. Never log credentials, request bodies with secrets, or full response bodies — log lengths/counts instead. +**Carve-out from `go-logging/no-log-and-return-error`**: this rule is the documented exception. A boundary-audit log line (one event per wire crossing) is a different concern from error-propagation logging (which fires once at the boundary that decides what to do). Emit exactly one audit line per boundary call — success path or failure path, never both for the same call — and then return the error for the caller to handle. + #### Bad ```go @@ -101,38 +109,53 @@ return parseBody(body) #### Good ```go +// One audit line per call — success branch status, body, err := doRequest(ctx, client, token, "POST", url, payload) -glog.Infof("http POST %s status=%d body_len=%d", path, status, len(body)) if err != nil { - glog.Warningf("http POST %s failed: %v", path, err) + // One audit line per call — failure branch (transport failure status known here, not at caller) + glog.Warningf("http POST %s err=%v", path, err) return err } +glog.Infof("http POST %s status=%d body_len=%d", path, status, len(body)) return parseBody(body) ``` +**Related**: pair with [`go-logging/no-tight-loop-without-sampler`](#rule-go-loggingno-tight-loop-without-sampler) when the boundary call is in a hot path (e.g. message-bus publishes); sample the audit line via `log.NewSampleTime` instead of emitting once per send. + ### RULE go-logging/no-sensitive-data-in-logs (MUST) **Owner**: go-security-specialist -**Applies when**: a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material (password, token, secret, key, PEM, JWT, raw request body containing credentials, full session cookie). -**Enforcement**: judgment (semantic — distinguishing "credential" from "credential-shaped name on a public value (e.g. `publicKey`)" requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.*` / `slog.*` with format args including identifiers matching `(?i)password|token|secret|.*Key|PEM|JWT|credential|authorization`) -**Why**: Logged credentials land in stdout, log aggregators, cloud logging backends — searchable, indexed, and impossible to redact once the batch has shipped. A single `glog.Infof("config: %+v", cfg)` on a struct that contains a `PEMKey` field leaks the key to every operator with log access. Use `display:"length"` tags (see `go-k8s-binary/secret-fields-need-display-length`), log lengths instead of values (`body_len=%d`), and never interpolate raw credential-shaped variables into log strings. +**Applies when**: a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material — password, passphrase, token (access/refresh/bearer/CSRF/session/JWT), secret (raw value, not k8s `secretName`/`secretRef`), credential-shaped key (privateKey, signingKey, encryptionKey, apiKey, PEM key — NOT publicKey / partitionKey / sortKey / lookupKey), Authorization header, DB connection string / DSN, OAuth client secret, X.509 certificate body, full request/response struct that contains any of the above as a field. +**Enforcement**: judgment (semantic — distinguishing real credential from credential-shaped name on a public value, e.g. `publicKey` / `partitionKey`, requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.{Info,Warning,Error}{,f}` / `slog.{Info,Warn,Error,Debug}` / `log.Printf` with format args including identifiers matching `(?i)(password|passphrase|token|credential|authorization|jwt|pem|dsn|connection[_ ]?string)\b|(private|signing|encryption|api)Key\b`. Word-boundary anchors required: bare `key` matches `partitionKey` / `sortKey` / `lookupKey` (over-flag); bare `secret` matches `secretName` / `secretRef` / `secretNamespace` (over-flag). Whole-struct dumps via `%+v` / `%#v` of any request / response / config struct also flag.) +**Why**: Logged credentials land in stdout, log aggregators, cloud logging backends — searchable, indexed, and impossible to redact once the batch has shipped. A single `glog.Infof("request: %+v", r)` dumps the `Authorization` header to every operator with log access; `glog.Infof("config: %+v", cfg)` on a struct that contains a `PEMKey` field leaks the key. Use `display:"length"` tags (see [`go-k8s-binary/secret-fields-need-display-length`](go-k8s-binary-conventions.md)), log lengths instead of values (`body_len=%d`), never interpolate raw credential-shaped variables, and never `%+v` whole request/response/config structs that contain authorization headers or secret fields. #### Bad ```go -// %+v dumps the whole struct including the PEMKey field — secret in the log +// Most common leak: whole-struct %+v dumps the Authorization header +glog.Infof("request: %+v", r) // r.Header["Authorization"] in the log + +// Config struct with PEMKey field — secret in the log glog.Infof("config: %+v", cfg) -// Direct interpolation +// Direct interpolation of credential-shaped variables glog.Infof("authenticated with token=%s", token) +slog.Info("connecting", "dsn", connectionString) // structured key-value still leaks the value +slog.Info("oauth refresh", "refresh_token", refreshToken) ``` #### Good ```go -// Length only — operators know it's set without seeing the value +// glog projects — log identifier + lengths, never the values +glog.Infof("request: method=%s path=%s authz_len=%d", r.Method, r.URL.Path, len(r.Header.Get("Authorization"))) glog.Infof("config: stage=%s pem_key_len=%d", cfg.Stage, len(cfg.PEMKey)) glog.Infof("authenticated token_len=%d", len(token)) + +// slog projects — structured key-value with length, not value +slog.Info("request", "method", r.Method, "path", r.URL.Path, "authz_len", len(r.Header.Get("Authorization"))) +slog.Info("connecting", "dsn_len", len(connectionString), "driver", driverName) +slog.Info("oauth refresh", "refresh_token_len", len(refreshToken)) ``` ### RULE go-logging/lowercase-log-messages (SHOULD) @@ -159,7 +182,7 @@ slog.Error("failed to parse request", "error", err) ### RULE go-logging/no-tight-loop-without-sampler (SHOULD) **Owner**: go-quality-assistant -**Applies when**: a Go log statement appears inside a `for` loop body that processes more than ~100 items per iteration without being gated by a `log.Sampler` / `IsSample()` check or a `glog.V(N)` verbosity guard. +**Applies when**: a Go log statement appears inside a `for` loop body whose iteration count is unbounded at compile time (range over an externally-sourced slice / channel; classic for-condition driven by external state) without being gated by a `log.Sampler` / `IsSample()` check or a `glog.V(N)` verbosity guard. The judgment threshold is "would this log line out-pace what an operator can scan at expected load?" — a poll-loop running once per second is fine to log every iteration; a per-message Kafka consumer running 10k msg/s is not. **Enforcement**: judgment (semantic — distinguishing "tight inner loop" from "small bounded outer loop" requires reading the loop bound; ast-grep partial: `call_expression` matching `glog.{Info,Warning}{,f}` / `slog.{Info,Warn}` inside a `for_statement` body without a sibling `if .IsSample()` / `glog.V(N)` guard) **Why**: Unsampled log calls in hot paths produce log gigabytes per minute — drowning real operator signal, inflating cloud-logging cost, and adding non-trivial latency to the loop itself (sync.Mutex contention inside `glog` on heavy concurrent writes). Sampling preserves the "is something happening?" signal at sustainable volume. `github.com/bborbe/log` provides `NewSampleTime(d)` (once per duration), `NewSampleMod(n)` (every N-th), `NewSamplerGlogLevel(n)` (gated by verbosity). @@ -176,6 +199,9 @@ for _, item := range items { #### Good ```go +// h.logSampler is wired through the handler constructor — see the +// "Log Sampling (glog projects)" section below for the canonical +// constructor shape: log.SamplerFactory -> Sampler stored on struct. for _, item := range items { if err := process(item); err != nil { if h.logSampler.IsSample() { @@ -205,14 +231,21 @@ if failures > 0 { #### Bad ```go +// Use select+ctx.Done() for graceful shutdown — bare for+time.Sleep +// leaks the goroutine on process termination. Production loops also +// pair this rule with go-context-cancellation-in-loops. for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(interval): + } changed, err := scanForUpdates(ctx) if err != nil { glog.Errorf("scan failed: %v", err) continue } glog.V(2).Infof("scan cycle: %d changed", changed) // fires even when changed=0 - time.Sleep(interval) } ``` @@ -220,6 +253,11 @@ for { ```go for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(interval): + } changed, err := scanForUpdates(ctx) if err != nil { glog.Errorf("scan failed: %v", err) @@ -228,7 +266,6 @@ for { if changed > 0 { glog.V(2).Infof("scan cycle: %d changed", changed) // only when there's something to report } - time.Sleep(interval) } ``` @@ -309,6 +346,8 @@ Wiring: see [go-http-service-guide.md](go-http-service-guide.md) for the canonic ## Related Rules -- [`go-cli/slog-not-glog-in-new-projects`](go-cli-guide.md) — new projects should pick `slog`, not introduce `glog` +- [`go-cli/slog-not-glog-in-new-projects`](go-cli-guide.md) — new projects should pick `slog`, not introduce `glog` (boundary with `go-logging/no-mixing-slog-and-glog` documented in the latter's Why paragraph) - [`go-glog/use-v-for-debug-not-info`](go-glog-guide.md) — V0 (bare `glog.Info`) is operator-default; debug-shaped lines go behind `V(N)` -- [`go-k8s-binary/secret-fields-need-display-length`](go-k8s-binary-conventions.md) — application-config secret fields carry `display:"length"` so `argument.Parse()` startup dump never prints values +- [`go-k8s-binary/secret-fields-need-display-length`](go-k8s-binary-conventions.md) — application-config secret fields carry `display:"length"` so `argument.Parse()` startup dump never prints values; companion to `go-logging/no-sensitive-data-in-logs` +- [`go-errors/no-bare-return-err`](go-error-wrapping-guide.md) and the surrounding error-wrapping family — `go-logging/no-log-and-return-error` depends on inner callers properly wrapping errors so the boundary log line carries full context +- [`go-context-cancellation-in-loops`](go-context-cancellation-in-loops.md) — pair with `go-logging/skip-empty-v2-heartbeats` for any V(2) heartbeat loop, so the goroutine exits cleanly on shutdown diff --git a/rules/index.json b/rules/index.json index 18396dc..5be9f74 100644 --- a/rules/index.json +++ b/rules/index.json @@ -649,16 +649,16 @@ }, { "anchor": "go-logging/no-sensitive-data-in-logs", - "applies_when": "a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material (password, token, secret, key, PEM, JWT, raw request body containing credentials, full session cookie).", + "applies_when": "a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material — password, passphrase, token (access/refresh/bearer/CSRF/session/JWT), secret (raw value, not k8s `secretName`/`secretRef`), credential-shaped key (privateKey, signingKey, encryptionKey, apiKey, PEM key — NOT publicKey / partitionKey / sortKey / lookupKey), Authorization header, DB connection string / DSN, OAuth client secret, X.509 certificate body, full request/response struct that contains any of the above as a field.", "doc_path": "docs/go-logging-guide.md", - "enforcement": "judgment (semantic — distinguishing \"credential\" from \"credential-shaped name on a public value (e.g. `publicKey`)\" requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.*` / `slog.*` with format args including identifiers matching `(?i)password|token|secret|.*Key|PEM|JWT|credential|authorization`)", + "enforcement": "judgment (semantic — distinguishing real credential from credential-shaped name on a public value, e.g. `publicKey` / `partitionKey`, requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.{Info,Warning,Error}{,f}` / `slog.{Info,Warn,Error,Debug}` / `log.Printf` with format args including identifiers matching `(?i)(password|passphrase|token|credential|authorization|jwt|pem|dsn|connection[_ ]?string)\\b|(private|signing|encryption|api)Key\\b`. Word-boundary anchors required: bare `key` matches `partitionKey` / `sortKey` / `lookupKey` (over-flag); bare `secret` matches `secretName` / `secretRef` / `secretNamespace` (over-flag). Whole-struct dumps via `%+v` / `%#v` of any request / response / config struct also flag.)", "id": "go-logging/no-sensitive-data-in-logs", "level": "MUST", "owner": "go-security-specialist" }, { "anchor": "go-logging/no-tight-loop-without-sampler", - "applies_when": "a Go log statement appears inside a `for` loop body that processes more than ~100 items per iteration without being gated by a `log.Sampler` / `IsSample()` check or a `glog.V(N)` verbosity guard.", + "applies_when": "a Go log statement appears inside a `for` loop body whose iteration count is unbounded at compile time (range over an externally-sourced slice / channel; classic for-condition driven by external state) without being gated by a `log.Sampler` / `IsSample()` check or a `glog.V(N)` verbosity guard. The judgment threshold is \"would this log line out-pace what an operator can scan at expected load?\" — a poll-loop running once per second is fine to log every iteration; a per-message Kafka consumer running 10k msg/s is not.", "doc_path": "docs/go-logging-guide.md", "enforcement": "judgment (semantic — distinguishing \"tight inner loop\" from \"small bounded outer loop\" requires reading the loop bound; ast-grep partial: `call_expression` matching `glog.{Info,Warning}{,f}` / `slog.{Info,Warn}` inside a `for_statement` body without a sibling `if .IsSample()` / `glog.V(N)` guard)", "id": "go-logging/no-tight-loop-without-sampler", From 464d1929bd144d8bd4e0bafe35f1e0852d7173e7 Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Wed, 3 Jun 2026 01:01:23 +0200 Subject: [PATCH 4/4] fix(docs): address PR #35 round-3 bot review on go-logging-guide.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MAJOR resolved: - cookie credential type re-added to no-sensitive-data-in-logs applies_when list (was 'full session cookie' in the original draft; got dropped during the round-2 regex/applies_when expansion). Enforcement regex now includes 'cookie|set[_-]?cookie'. - compound *Secret (clientSecret, webhookSecret, signingSecret) added to enforcement regex via '\b(private|signing|encryption| api|client|webhook)(Key|Secret)\b' alternation. Bot MAJOR #1 not addressed (owner mismatch claim): bot reported 'docs line 127 says Owner: go-quality-assistant'. Verified — doc line 127 is 'Owner: go-security-specialist' (correct), matching index.json. The bot's claim is a hallucination on this round. NITs addressed: - external-call-logs-response Good example: added 'start := time.Now()' + elapsed_ms to both branches per rule's Why paragraph stating latency is part of the minimum payload - Added DB / gRPC / subprocess Good-example variants showing the same one-log-line-per-call shape (replaces the prose 'External Calls' section that was lost during bootstrap) - slog 'connecting' Good example now logs host + db name in addition to dsn_len (operator can identify which database) - Enforcement field clarification: whole-struct %+v / %#v dumps are judgment-tier (ast-grep cannot mechanically correlate the format verb with the argument's struct type — agent reads fields) NIT not addressed: bot's 'add bare secret to regex' — already covered by the new (Key|Secret) compound alternation. Rule count unchanged: 131. make precommit clean. --- docs/go-logging-guide.md | 30 ++++++++++++++++++++++++------ rules/index.json | 4 ++-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/docs/go-logging-guide.md b/docs/go-logging-guide.md index aaf943c..4f5a87c 100644 --- a/docs/go-logging-guide.md +++ b/docs/go-logging-guide.md @@ -109,24 +109,42 @@ return parseBody(body) #### Good ```go -// One audit line per call — success branch +// HTTP — one audit line per call, with latency +start := time.Now() status, body, err := doRequest(ctx, client, token, "POST", url, payload) if err != nil { // One audit line per call — failure branch (transport failure status known here, not at caller) - glog.Warningf("http POST %s err=%v", path, err) + glog.Warningf("http POST %s elapsed_ms=%d err=%v", path, time.Since(start).Milliseconds(), err) return err } -glog.Infof("http POST %s status=%d body_len=%d", path, status, len(body)) +glog.Infof("http POST %s status=%d body_len=%d elapsed_ms=%d", path, status, len(body), time.Since(start).Milliseconds()) return parseBody(body) ``` +DB queries, gRPC calls, and subprocess `exec` follow the same shape — one log line per call with op + result + latency: + +```go +// DB +start := time.Now() +rows, err := db.QueryContext(ctx, query, args...) +glog.Infof("db query=%s rows=%d elapsed_ms=%d", queryName, count, time.Since(start).Milliseconds()) + +// gRPC +resp, err := client.GetUser(ctx, req) +glog.Infof("rpc UserService.GetUser status=%v elapsed_ms=%d", statusOf(err), time.Since(start).Milliseconds()) + +// subprocess +out, err := exec.CommandContext(ctx, cmd, args...).Output() +glog.Infof("exec %s exit=%v elapsed_ms=%d", cmd, exitCodeOf(err), time.Since(start).Milliseconds()) +``` + **Related**: pair with [`go-logging/no-tight-loop-without-sampler`](#rule-go-loggingno-tight-loop-without-sampler) when the boundary call is in a hot path (e.g. message-bus publishes); sample the audit line via `log.NewSampleTime` instead of emitting once per send. ### RULE go-logging/no-sensitive-data-in-logs (MUST) **Owner**: go-security-specialist -**Applies when**: a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material — password, passphrase, token (access/refresh/bearer/CSRF/session/JWT), secret (raw value, not k8s `secretName`/`secretRef`), credential-shaped key (privateKey, signingKey, encryptionKey, apiKey, PEM key — NOT publicKey / partitionKey / sortKey / lookupKey), Authorization header, DB connection string / DSN, OAuth client secret, X.509 certificate body, full request/response struct that contains any of the above as a field. -**Enforcement**: judgment (semantic — distinguishing real credential from credential-shaped name on a public value, e.g. `publicKey` / `partitionKey`, requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.{Info,Warning,Error}{,f}` / `slog.{Info,Warn,Error,Debug}` / `log.Printf` with format args including identifiers matching `(?i)(password|passphrase|token|credential|authorization|jwt|pem|dsn|connection[_ ]?string)\b|(private|signing|encryption|api)Key\b`. Word-boundary anchors required: bare `key` matches `partitionKey` / `sortKey` / `lookupKey` (over-flag); bare `secret` matches `secretName` / `secretRef` / `secretNamespace` (over-flag). Whole-struct dumps via `%+v` / `%#v` of any request / response / config struct also flag.) +**Applies when**: a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material — password, passphrase, token (access/refresh/bearer/CSRF/session/JWT), secret (raw value, not k8s `secretName`/`secretRef`), compound `*Secret` (`clientSecret`, `webhookSecret`, `signingSecret`), credential-shaped key (privateKey, signingKey, encryptionKey, apiKey, PEM key — NOT publicKey / partitionKey / sortKey / lookupKey), Authorization header, cookie (session / auth / tracking, including `Set-Cookie` header value and individual cookie values), DB connection string / DSN, OAuth client secret, X.509 certificate body, full request/response struct that contains any of the above as a field. +**Enforcement**: judgment (semantic — distinguishing real credential from credential-shaped name on a public value, e.g. `publicKey` / `partitionKey`, requires reading the source and intent; ast-grep partial — identifier filter: `call_expression` matching `glog.{Info,Warning,Error}{,f}` / `slog.{Info,Warn,Error,Debug}` / `log.Printf` with format args including identifiers matching `(?i)(password|passphrase|token|credential|authorization|jwt|pem|dsn|connection[_ ]?string|cookie|set[_-]?cookie)\b|\b(private|signing|encryption|api|client|webhook)(Key|Secret)\b`. Word-boundary anchors required: bare `key` matches `partitionKey` / `sortKey` / `lookupKey` (over-flag); bare `secret` without prefix matches `secretName` / `secretRef` / `secretNamespace` (over-flag — k8s reference fields, safe to log). Whole-struct dumps via `%+v` / `%#v` are a separate judgment-tier check — ast-grep can flag the format verb but cannot mechanically correlate it with the argument's struct type; the agent reads the struct fields to decide.) **Why**: Logged credentials land in stdout, log aggregators, cloud logging backends — searchable, indexed, and impossible to redact once the batch has shipped. A single `glog.Infof("request: %+v", r)` dumps the `Authorization` header to every operator with log access; `glog.Infof("config: %+v", cfg)` on a struct that contains a `PEMKey` field leaks the key. Use `display:"length"` tags (see [`go-k8s-binary/secret-fields-need-display-length`](go-k8s-binary-conventions.md)), log lengths instead of values (`body_len=%d`), never interpolate raw credential-shaped variables, and never `%+v` whole request/response/config structs that contain authorization headers or secret fields. #### Bad @@ -154,7 +172,7 @@ glog.Infof("authenticated token_len=%d", len(token)) // slog projects — structured key-value with length, not value slog.Info("request", "method", r.Method, "path", r.URL.Path, "authz_len", len(r.Header.Get("Authorization"))) -slog.Info("connecting", "dsn_len", len(connectionString), "driver", driverName) +slog.Info("connecting", "driver", driverName, "host", dbHost, "db", dbName, "dsn_len", len(connectionString)) // host+db identify the target; dsn_len signals "is the secret set" slog.Info("oauth refresh", "refresh_token_len", len(refreshToken)) ``` diff --git a/rules/index.json b/rules/index.json index 5be9f74..aa3dea6 100644 --- a/rules/index.json +++ b/rules/index.json @@ -649,9 +649,9 @@ }, { "anchor": "go-logging/no-sensitive-data-in-logs", - "applies_when": "a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material — password, passphrase, token (access/refresh/bearer/CSRF/session/JWT), secret (raw value, not k8s `secretName`/`secretRef`), credential-shaped key (privateKey, signingKey, encryptionKey, apiKey, PEM key — NOT publicKey / partitionKey / sortKey / lookupKey), Authorization header, DB connection string / DSN, OAuth client secret, X.509 certificate body, full request/response struct that contains any of the above as a field.", + "applies_when": "a Go log statement (`glog.Infof` / `slog.Info` / `log.Printf` / similar) interpolates a value whose name or content suggests sensitive material — password, passphrase, token (access/refresh/bearer/CSRF/session/JWT), secret (raw value, not k8s `secretName`/`secretRef`), compound `*Secret` (`clientSecret`, `webhookSecret`, `signingSecret`), credential-shaped key (privateKey, signingKey, encryptionKey, apiKey, PEM key — NOT publicKey / partitionKey / sortKey / lookupKey), Authorization header, cookie (session / auth / tracking, including `Set-Cookie` header value and individual cookie values), DB connection string / DSN, OAuth client secret, X.509 certificate body, full request/response struct that contains any of the above as a field.", "doc_path": "docs/go-logging-guide.md", - "enforcement": "judgment (semantic — distinguishing real credential from credential-shaped name on a public value, e.g. `publicKey` / `partitionKey`, requires reading the source and intent; ast-grep partial: `call_expression` matching `glog.{Info,Warning,Error}{,f}` / `slog.{Info,Warn,Error,Debug}` / `log.Printf` with format args including identifiers matching `(?i)(password|passphrase|token|credential|authorization|jwt|pem|dsn|connection[_ ]?string)\\b|(private|signing|encryption|api)Key\\b`. Word-boundary anchors required: bare `key` matches `partitionKey` / `sortKey` / `lookupKey` (over-flag); bare `secret` matches `secretName` / `secretRef` / `secretNamespace` (over-flag). Whole-struct dumps via `%+v` / `%#v` of any request / response / config struct also flag.)", + "enforcement": "judgment (semantic — distinguishing real credential from credential-shaped name on a public value, e.g. `publicKey` / `partitionKey`, requires reading the source and intent; ast-grep partial — identifier filter: `call_expression` matching `glog.{Info,Warning,Error}{,f}` / `slog.{Info,Warn,Error,Debug}` / `log.Printf` with format args including identifiers matching `(?i)(password|passphrase|token|credential|authorization|jwt|pem|dsn|connection[_ ]?string|cookie|set[_-]?cookie)\\b|\\b(private|signing|encryption|api|client|webhook)(Key|Secret)\\b`. Word-boundary anchors required: bare `key` matches `partitionKey` / `sortKey` / `lookupKey` (over-flag); bare `secret` without prefix matches `secretName` / `secretRef` / `secretNamespace` (over-flag — k8s reference fields, safe to log). Whole-struct dumps via `%+v` / `%#v` are a separate judgment-tier check — ast-grep can flag the format verb but cannot mechanically correlate it with the argument's struct type; the agent reads the struct fields to decide.)", "id": "go-logging/no-sensitive-data-in-logs", "level": "MUST", "owner": "go-security-specialist"