diff --git a/README.md b/README.md index bff902b..9f66976 100644 --- a/README.md +++ b/README.md @@ -80,10 +80,13 @@ Example workflow: [`examples/github-actions/scan.yml`](examples/github-actions/s | `httpctx/outbound-call-without-context` | Go | `http.Get`/`Post`/`Head`/`PostForm` | | `otel/sdk-missing-resource-attrs` | Go, TS | Provider/SDK without `service.name` | | `otel/sdk-missing-shutdown` | Go, TS | Provider without Shutdown/forceFlush (medium) | -| `otel/semconv-drift` | Go, TS | Deprecated HTTP attribute names (advisory) | +| `otel/semconv-drift` | Go, TS | Deprecated HTTP attribute names (medium; TAR-77 precision gate) | | `errors/swallowed-on-critical-path` | Go, Java | `_ = err` / empty `catch` (medium) | | `traces/error-path-not-recorded-on-span` | Go | return err after `.Start(` without RecordError/SetStatus (medium) | | `logs/missing-trace-correlation` | Go, TS | slog/pino alongside OTel without a bridge (medium) | +| `logs/slog-sprintf-message` | Go | `slog.*(fmt.Sprintf(...))` — prefer structured attrs (medium; TAR-77) | + +Precision process for high-confidence rules: [`docs/PRECISION.md`](docs/PRECISION.md). ## Demo fixture diff --git a/docs/PRECISION.md b/docs/PRECISION.md new file mode 100644 index 0000000..a475988 --- /dev/null +++ b/docs/PRECISION.md @@ -0,0 +1,29 @@ +# Precision gate (TAR-77) + +Public high-confidence rules must earn that label. This gate applies to **new** and **changed** rules in [`Pieczasz/tarsier`](https://github.com/Pieczasz/tarsier). + +## Rule + +1. **Default to `medium`.** Ship new pattern rules at `confidence: medium` unless evidence says otherwise. +2. **Promote to `high` only with precision evidence:** ruletest fixtures (valid + invalid), at least one `_badshop` / corpus `notwant` FP posture note, and a short note in the rule YAML. +3. **Demote when notes admit the ceiling.** If a rule says “advisory until corpus precision is measured” or “demote if FP rate is bad”, confidence must not stay `high`. +4. **`--fail-on` / policy block** only fire on findings that are not medium-only noise paths (medium never trips `--fail-on` by design). + +## Provider slices + +When adding a provider family (e.g. slog bridge, TS OTel): + +| Step | Done when | +| --- | --- | +| Fixtures | `fixtures/ruletests/-.yml` has valid + invalid cases | +| FP note | Rule `note:` documents the precision ceiling and stays `medium` until measured | +| README | Row added under “What it detects” | +| Evidence | Comment on TAR-77 (or child) with FP triage summary before promoting to `high` | + +## This pass (TAR-77) + +* Documented this gate. +* Demoted `otel/semconv-drift` (Go + TS) from `high` → `medium` (notes already said precision unmeasured). +* Added Go **slog sprintf-message** provider (`logs/slog-sprintf-message`) at **medium** with ruletests + FP note. + +No phones-home / learning telemetry in the public CLI. diff --git a/fixtures/_badshop/api/slog_sprintf.go b/fixtures/_badshop/api/slog_sprintf.go new file mode 100644 index 0000000..a4d9566 --- /dev/null +++ b/fixtures/_badshop/api/slog_sprintf.go @@ -0,0 +1,23 @@ +package main + +import ( + "context" + "fmt" + "log/slog" +) + +func logOrderFailed(id string, err error) { + slog.Info(fmt.Sprintf("order %s failed: %v", id, err)) // want: logs/slog-sprintf-message +} + +func logBoom(ctx context.Context, id string) { + slog.ErrorContext(ctx, fmt.Sprintf("boom %s", id)) // want: logs/slog-sprintf-message +} + +func logStructured(id string) { + slog.Info("order failed", "order_id", fmt.Sprintf("%s", id)) // notwant: logs/slog-sprintf-message +} + +func logStructuredCtx(ctx context.Context, id string) { + slog.ErrorContext(ctx, "boom", "id", fmt.Sprintf("%s", id)) // notwant: logs/slog-sprintf-message +} diff --git a/fixtures/ruletests/__snapshots__/logs-slog-sprintf-message-go-snapshot.yml b/fixtures/ruletests/__snapshots__/logs-slog-sprintf-message-go-snapshot.yml new file mode 100644 index 0000000..747ab65 --- /dev/null +++ b/fixtures/ruletests/__snapshots__/logs-slog-sprintf-message-go-snapshot.yml @@ -0,0 +1,142 @@ +id: logs-slog-sprintf-message-go +snapshots: + ? | + package main + import ( + "context" + "fmt" + "log/slog" + ) + func f(ctx context.Context, id string) { + slog.ErrorContext(ctx, fmt.Sprintf("boom %s", id)) + } + : labels: + - source: slog.ErrorContext(ctx, fmt.Sprintf("boom %s", id)) + style: primary + start: 100 + end: 150 + - source: slog.ErrorContext + style: secondary + start: 100 + end: 117 + - source: fmt.Sprintf + style: secondary + start: 123 + end: 134 + - source: fmt.Sprintf("boom %s", id) + style: secondary + start: 123 + end: 149 + - source: fmt.Sprintf("boom %s", id) + style: secondary + start: 123 + end: 149 + - source: '"log/slog"' + style: secondary + start: 44 + end: 54 + - source: | + package main + import ( + "context" + "fmt" + "log/slog" + ) + func f(ctx context.Context, id string) { + slog.ErrorContext(ctx, fmt.Sprintf("boom %s", id)) + } + style: secondary + start: 0 + end: 153 + ? | + package main + import ( + "fmt" + "log/slog" + ) + func f(id string) { + slog.Warn(fmt.Sprintf("x %s", id), "k", 1) + } + : labels: + - source: slog.Warn(fmt.Sprintf("x %s", id), "k", 1) + style: primary + start: 67 + end: 109 + - source: slog.Warn + style: secondary + start: 67 + end: 76 + - source: fmt.Sprintf + style: secondary + start: 77 + end: 88 + - source: fmt.Sprintf("x %s", id) + style: secondary + start: 77 + end: 100 + - source: fmt.Sprintf("x %s", id) + style: secondary + start: 77 + end: 100 + - source: '"log/slog"' + style: secondary + start: 32 + end: 42 + - source: | + package main + import ( + "fmt" + "log/slog" + ) + func f(id string) { + slog.Warn(fmt.Sprintf("x %s", id), "k", 1) + } + style: secondary + start: 0 + end: 112 + ? | + package main + import ( + "fmt" + "log/slog" + ) + func f(id string, err error) { + slog.Info(fmt.Sprintf("order %s failed: %v", id, err)) + } + : labels: + - source: 'slog.Info(fmt.Sprintf("order %s failed: %v", id, err))' + style: primary + start: 78 + end: 132 + - source: slog.Info + style: secondary + start: 78 + end: 87 + - source: fmt.Sprintf + style: secondary + start: 88 + end: 99 + - source: 'fmt.Sprintf("order %s failed: %v", id, err)' + style: secondary + start: 88 + end: 131 + - source: 'fmt.Sprintf("order %s failed: %v", id, err)' + style: secondary + start: 88 + end: 131 + - source: '"log/slog"' + style: secondary + start: 32 + end: 42 + - source: | + package main + import ( + "fmt" + "log/slog" + ) + func f(id string, err error) { + slog.Info(fmt.Sprintf("order %s failed: %v", id, err)) + } + style: secondary + start: 0 + end: 135 diff --git a/fixtures/ruletests/logs-slog-sprintf-message-go.yml b/fixtures/ruletests/logs-slog-sprintf-message-go.yml new file mode 100644 index 0000000..833ac72 --- /dev/null +++ b/fixtures/ruletests/logs-slog-sprintf-message-go.yml @@ -0,0 +1,68 @@ +id: logs-slog-sprintf-message-go +valid: + - | + package main + import "log/slog" + func f(id string, err error) { + slog.Info("order failed", "order_id", id, "err", err) + } + - | + package main + import ( + "fmt" + "log/slog" + ) + func f(id string) { + // sprintf in attrs is structured — not the message argument + slog.Info("order failed", "order_id", fmt.Sprintf("%s", id)) + } + - | + package main + import ( + "context" + "fmt" + "log/slog" + ) + func f(ctx context.Context, id string) { + slog.ErrorContext(ctx, "boom", "id", fmt.Sprintf("%s", id)) + } + - | + package main + import ( + "fmt" + "log" + ) + func f(id string) { + // not slog — unstructured-logging may still fire when slog is imported + log.Printf("order %s", id) + _ = fmt.Sprintf("x") + } +invalid: + - | + package main + import ( + "fmt" + "log/slog" + ) + func f(id string, err error) { + slog.Info(fmt.Sprintf("order %s failed: %v", id, err)) + } + - | + package main + import ( + "context" + "fmt" + "log/slog" + ) + func f(ctx context.Context, id string) { + slog.ErrorContext(ctx, fmt.Sprintf("boom %s", id)) + } + - | + package main + import ( + "fmt" + "log/slog" + ) + func f(id string) { + slog.Warn(fmt.Sprintf("x %s", id), "k", 1) + } diff --git a/rules/logs/slog-sprintf-message.go.yml b/rules/logs/slog-sprintf-message.go.yml new file mode 100644 index 0000000..4828faf --- /dev/null +++ b/rules/logs/slog-sprintf-message.go.yml @@ -0,0 +1,60 @@ +id: logs-slog-sprintf-message-go +language: go +severity: warning + +metadata: + rule: logs/slog-sprintf-message + requires: log_correlation + expectation_layer: "2" + confidence: medium + spec_ref: instrumentation-score/LOG-001 + +message: "slog message is built with fmt.Sprintf; prefer structured attrs." +note: | + slog.Info/Error/Warn/Debug with fmt.Sprintf as the message argument collapses + fields into an unstructured string and fights log<->trace correlation. + + Fix: slog.Info("order failed", "order_id", id, "err", err) — not + slog.Info(fmt.Sprintf("order %s failed: %v", id, err)). + + Precision (TAR-77): medium until corpus FP measured. Only the message + argument is matched (nth child 1 for Info/…, nth child 2 for *Context); + fmt.Sprintf inside structured attrs is notwant. Logger method receivers + (l.Info) and slog.Default().Info are out of scope. Do not promote to high + without evidence (docs/PRECISION.md). + +rule: + kind: call_expression + all: + - any: + - all: + - has: + field: function + regex: '^slog\.(Info|Error|Warn|Debug)$' + - has: + field: arguments + has: + nthChild: 1 + kind: call_expression + has: + field: function + regex: '^fmt\.Sprintf$' + - all: + - has: + field: function + regex: '^slog\.(InfoContext|ErrorContext|WarnContext|DebugContext)$' + - has: + field: arguments + has: + nthChild: 2 + kind: call_expression + has: + field: function + regex: '^fmt\.Sprintf$' + - inside: + stopBy: end + kind: source_file + has: + stopBy: end + kind: import_spec + regex: '"log/slog"' diff --git a/rules/otel/semconv-drift.go.yml b/rules/otel/semconv-drift.go.yml index 5ba1eb1..b24302c 100644 --- a/rules/otel/semconv-drift.go.yml +++ b/rules/otel/semconv-drift.go.yml @@ -6,7 +6,7 @@ metadata: rule: otel/semconv-drift requires: attributes expectation_layer: "2" - confidence: high + confidence: medium spec_ref: otel/semconv-http-migration message: "Deprecated OTel HTTP attribute name; TraceQL/PromQL joins break across SDK versions." @@ -24,8 +24,9 @@ note: | http.user_agent -> user_agent.original Flagged when go.opentelemetry.io/otel/attribute (or semconv) is imported - and a deprecated key appears as a string literal. Advisory severity until - corpus precision is measured. + and a deprecated key appears as a string literal. Confidence is medium + (TAR-77): string-literal matches can FP on non-attribute uses; promote to + high only after corpus precision evidence (see docs/PRECISION.md). rule: any: diff --git a/rules/otel/semconv-drift.ts.yml b/rules/otel/semconv-drift.ts.yml index c83265b..9af6bc0 100644 --- a/rules/otel/semconv-drift.ts.yml +++ b/rules/otel/semconv-drift.ts.yml @@ -6,7 +6,7 @@ metadata: rule: otel/semconv-drift requires: attributes expectation_layer: "2" - confidence: high + confidence: medium spec_ref: otel/semconv-http-migration message: "Deprecated OTel HTTP attribute name; TraceQL/PromQL joins break across SDK versions." @@ -24,8 +24,9 @@ note: | http.user_agent -> user_agent.original Flagged when @opentelemetry/api or @opentelemetry/semantic-conventions - is imported and a deprecated key appears as a string literal. Advisory - severity until corpus precision is measured. + is imported and a deprecated key appears as a string literal. Confidence + is medium (TAR-77): string-literal matches can FP on non-attribute uses; + promote to high only after corpus precision evidence (see docs/PRECISION.md). rule: any: