release-train: staging -> main - #510
Merged
Merged
Conversation
* sec(deps): bump the build toolchain to go1.26.6 (backend#1972)
`govulncheck` reports four CALLED stdlib vulnerabilities on go1.26.5 — reached
from this module's own code, not merely present in the dependency graph:
GO-2026-6218 net/url quadratic complexity in resolvePath
GO-2026-6090 crypto/tls post-handshake message flood
GO-2026-5972 encoding/asn1 unbounded recursion depth
GO-2026-5026 net/http x/net/idna Punycode label handling
All four are fixed in go1.26.6, and none needs a code or dependency change.
Every workflow resolves Go through `go-version-file: go.mod`, so this one line
moves build.yml, golangci.yml, e2e.yml, mutation.yml, release.yml and
chart-drift.yml together.
`go 1.26.0` is deliberately untouched — that is the language floor, not the
toolchain, and .golangci.yml's `go: "1.26"` tracks it. Only the compiler moves.
## Evidence, on a real toolchain rather than from release notes
GOTOOLCHAIN=auto fetched go1.26.6 and used it:
go version go1.26.6 darwin/amd64 (downloaded on demand)
go build ./... exit 0
go vet ./... exit 0
go test ./... exit 0 (all packages ok)
govulncheck ./... exit 0 "No vulnerabilities found."
Counterfactual, so the bump is demonstrably what fixed it rather than something
incidental — reverted the single line and re-ran the same binary:
toolchain go1.26.5 -> exit 3, 4 vulnerabilities (the four above)
toolchain go1.26.6 -> exit 0, none
Exit codes read explicitly: govulncheck exits 3 on findings, so "looks clean"
in the output is not the same as passing.
## What is NOT in this PR
Making `govulncheck` a required status check. It is red-on-main today and the
prod promotion merged straight past it, which is the actual defect — filed as
backend#1972. It must be armed only AFTER this reaches `main`: adding the
context while main is still on go1.26.5 would block every release-train
promotion on the branch the train has to merge into.
Fix first, arm second.
* chore(release): bump VERSION to 0.10.7 for the toolchain fix
The version-bump gate caught what I left out: v0.10.6 is already released, and
this PR changes `go.mod` — a published path — so the binary the train ships
would differ from the one v0.10.6 tagged.
The gate's own wording is the reason this matters: the train reads VERSION and
cuts the tag from it, never bumping for you, so a stale file does not fail here
— it fails the next prod hop, days later, on somebody else (backend#1561).
Patch bump: stdlib security fixes via go1.26.6, no behaviour or API change.
…hat keeps it (backend#1972) (#501) * ci(vulncheck): one declaration for the govulncheck pin, and a check that keeps it (backend#1972) The version lived in THREE places -- build.yml's job, vulncheck.yml's job, and GOVULNCHECK_VERSION in the Makefile -- held in step by a comment reading "keep the job in lockstep". All three happened to read v1.1.4, which is exactly what made it look fine. Two copies kept in sync by a request is not a mechanism. Both workflows now run `make vulncheck`, so the Makefile is the single declaration, and a green `make ci` locally cannot disagree with the PR gate about which govulncheck ran -- the same reasoning already applied to lint-full/GOLANGCI_LINT_VERSION. The "keep in lockstep" comment is replaced by a machine check rather than deleted: scripts/check-tool-pins.sh PARSES GOVULNCHECK_VERSION out of the Makefile and fails the Lint job if any workflow hardcodes that module with an @Version. The guard holds no version of its own, so it cannot agree with itself while disagreeing with reality. Mutation-proved, anchors asserted: reintroduce `go install ...govulncheck@v1.1.4` in build.yml -> exit 1, names file and line delete GOVULNCHECK_VERSION from the Makefile -> exit 2, refuses to report clean run from an unrelated cwd -> still correct (cd's to its root) The two jobs still differ, deliberately, and now ONLY in the ref: vulncheck.yml checks out develop on a schedule, build.yml judges the PR head. That difference is stated in the file instead of being buried in duplicated steps. DRAFT until cli#500 lands. `govulncheck` is already red on develop -- go1.26.5 carries 4 reachable stdlib CVEs -- so this branch inherits that failure. Verified locally that the folded target reproduces it exactly: make vulncheck -> exit status 3, "affected by 4 vulnerabilities from the Go standard library", each "Fixed in: <pkg>@go1.26.6" which also independently confirms backend#1972's counterfactual. This PR does not change WHAT govulncheck finds, only where its version is declared. NOT in this PR, and both belong to the ticket rather than here: adding govulncheck to `main`'s required contexts (needs #500 on main first, plus the break-glass decision -- enforce_admins is true there and skip-fr-gate does not bypass a required check), and the same three-copies shape for GOLANGCI_LINT_VERSION, which TOOLS in the new guard is structured to take as a second row. The two actionlint SC2001 findings on build.yml are pre-existing on develop (verified against the unmodified base) and untouched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci(vulncheck): run the pin guard in make ci/check + fail it closed on grep errors Two Bugbot findings on the govulncheck single-declaration change (backend#1972): - check-tool-pins ran only in the CI Lint job, so a green local `make ci` / `make check` could disagree with the PR about a restated pin — the exact local/CI divergence the Makefile-as-entry-point exists to prevent. Add it to both aggregate targets, beside check-style/file-budget. - The workflow scan used `grep ... 2>/dev/null || true`, laundering a real grep error (rc>=2: unreadable tree, bad invocation) into an empty hit list and an unearned clean pass — the fail-open this guard was written against. Capture rc and fail closed on rc>=2, matching scan() in check-style.sh. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
release-train: develop -> staging
…ckend#1897) (#503) RFC-BACKEND-1872 D2 for the CLI, and D12's host-process path. The Go half of what backend#1896 does for the Python services. Unblocks #1907. The CLI emits nothing today — it is not a pod, so the edge Collector's filelog receiver cannot reach it, and a field failure is only ever a support thread. e := telemetry.New(cfg.CurrentEnv, version, hostname) err := e.Emit("cli.command.failed", telemetry.Attrs{"error.type": "network"}) ENFORCED, NOT DOCUMENTED. The contract's mechanically-checkable rules run at the call site and return an error: the <domain>.<object>.<outcome> grammar with its closed vocabularies, the attribute-key namespace, retired names, value types, and the error set a failure must carry — stacktrace included. It returns rather than panics, because a CLI must never die of telemetry; but a malformed event must not pass silently either, and the caller's tests are where it fails. THE ENVIRONMENT IS DERIVED, NOT RESTATED. `New` classifies via `api.IsKnownEnv`, the same function that rejects a `--env staging` typo at the CLI's front door, and a test asserts the two agree across dev/stg/prod/staging/ prd/PROD/"". One saying yes while the other says no is precisely how records acquire a guessed environment. The domain vocabulary is narrower than the full registry — `cli` and `auth` only. The CLI is not the installer and not the backend, so admitting domains it cannot legitimately produce would make a typo look plausible. That is the failure already visible in the browser leg, where 461 of 484 events are named `not_specified`. 25 tests, 100% statement coverage, `make check` green. Twelve rules mutation-proved. ONE SURVIVED THE FIRST PASS, and it was a weak test rather than weak code — the same one the Python side hit. Every key I had tried was caught by the retired or namespace rule, so nothing exercised the key-SHAPE check; it needed a key that passes every other rule and is still badly shaped (`tracebloc.clientID`). Added, and the mutation now bites. NOTHING IMPORTS THIS YET, deliberately: #1907 is the consumer ticket. One consequence worth recording rather than discovering — `make deadcode` scans reachability from ./cmd/tracebloc, so a package outside that import graph is invisible to it. The gate is SILENT on this package, not passing it, and will start covering it the moment #1907 wires the first call site. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
release-train: develop -> staging
…#1897) (#507) * fix(telemetry): five review findings on the merged Go helper (backend#1897) cli#503 merged before its review was resolved. All five findings are real; each was reproduced against the merged code before anything changed, and each is mutation-proved. Nothing imports this package yet (#1907 is the consumer), so there is no production impact — but three of these would have shaped the first call sites, which is exactly when they would have been expensive. 1. RESOURCE-SCOPE KEYS WERE ACCEPTED AS RECORD ATTRIBUTES. One flat `otelAttrs` allowlist mixed the layers, so a call site could smuggle `service.name` into the record and contradict its own process identity — the cloud_RoleName cross-layer confusion, at the call site of the package meant to close it. `TestTheSinkReceivesResourceAndRecordSeparately` passed only because the caller happened not to pass one. Now two sets. `tracebloc.component`/`tracebloc.tenant.id` are included even though correctly prefixed — the namespace rule alone waved them past. And `event.name` is refused: it is Emit's first ARGUMENT, so accepting it let a caller replace the name after the grammar and failure-set checks had run. 2. THE PRIMITIVE SWITCH WAS NARROWER THAN GO'S SCALARS. A type switch matches the DYNAMIC type, so `case int64` never matched `time.Duration` — an idiomatic caller writing `Attrs{"tracebloc.elapsed": elapsed}` was told their duration was the retired extraData defect. Now switches on reflect.Kind, so int8/32, uint*, float32 and named types over them all pass. Added `Duration(d) int64` returning MILLISECONDS, and the reason is that time.Duration is an int64 kind and would otherwise pass as a raw nanosecond count — a number nobody reading a dashboard can interpret. 3. AN EMPTY service.instance.id WAS STAMPED. os.Hostname() returns "" on error. Omitted now: the "sent as empty rather than omitted" defect the record layer already refused, which the resource layer did not. 4. DELIVERY IGNORED Exports(). An emitter for an unrecognised env delivered anyway if a sink was installed, so "unknown never exports" lived in caller discipline at every #1907 call site. Gated now — validation still always runs, so a bad event fails in CI wherever the binary is built. 5. A BAD KEY WITH AN EMPTY VALUE PASSED SILENTLY. `normalise` continued on nil/empty BEFORE validating the key, so `Attrs{"experimentKey": nil}` raised nothing — contradicting this package's own "a malformed event must not pass silently". The key is validated first now; a GOOD key with an empty value is still dropped rather than rejected. Finding 1 is the same defect Bugbot found independently in the Python sibling (backend#1996); both are fixed the same way. 31 tests, 100% statement coverage, `make check` green. Six mutations, all caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): the omit rule must know what a string is, same as the value check Bugbot, and it is fallout from finding 2 in this same PR — the honest kind. I widened `checkAttrValue` to accept every string KIND so `type Reason string` would pass, and left the omit rule type-asserting to builtin `string`. So an empty named string was accepted as a value and never recognised as absent: `Attrs{"tracebloc.reason": Reason(" ")}` landed on the record, reopening §1.2 for exactly the callers the widening was for. The consequence is worse one line down. On a failure, an empty named `error.type` satisfies `checkFailureSet` by key presence alone — a failure that cannot be grouped, reported as one that can. That is the whole point of the required-error.type rule, defeated by a type assertion. `absentValue` now asks by reflect.Kind, the same question `checkAttrValue` asks, because the two have to agree on what a string is. Zero numbers and false bools stay data: they are measurements that happen to be falsey. Three tests — the drop, the failure-set consequence, and the other half (a named string with content arrives, 0 and false are kept). Mutation-proved: restoring the `value.(string)` assertion reddens two of them. The Python sibling had the same class in a different container (arrays bypassed both the omit and size rules); fixed there in backend#1996. Coverage stays 100%. `make check` green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
release-train: develop -> staging
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit cb1fb54. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-mainbranch (a mirror ofstaging), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Go patch and CI-only pin consolidation are low risk; the new telemetry package will affect observability correctness once wired into commands, and the release bundles security-sensitive toolchain changes with a large new validation surface.
Overview
Release train promotion (0.10.6 → 0.10.7) bundling security tooling fixes, CI hygiene, and the first CLI telemetry contract library.
Go toolchain:
go.modpins go1.26.6 so reachable stdlib CVEs reported by govulncheck (tls, asn1, http/idna, url) clear without application code changes; workflows that usego-version-file: go.modpick this up together.backend#1972 — single source for govulncheck:
build.ymlandvulncheck.ymlnow runmake vulncheckinstead of inlinego install …@v1.1.4. Newscripts/check-tool-pins.sh(wired intomake check,make ci, Lint + installer shellcheck) fails if workflows restategolang.org/x/vuln/cmd/govulncheck@…while the Makefile ownsGOVULNCHECK_VERSION.Telemetry (RFC / backend#1897): Adds
internal/telemetry— anEmitterthat validates contract rules at emit time (event name grammar, CLI-only domains, attribute namespaces, retired keys, resource vs record layering, failureerror.type/ exception set) and only delivers whenExports()is true for known envs viaapi.IsKnownEnv. Comprehensive tests; no command wiring in this diff.Reviewed by Cursor Bugbot for commit cb1fb54. Bugbot is set up for automated code reviews on this repo. Configure here.