release-train: develop -> staging - #528
Merged
Merged
Conversation
…cosign (#523) install.ps1 gated cosign verification on `if (Get-Command cosign)`. On a fresh Windows box cosign is never present, so the default path printed (cosign not installed; SHA256 verified, signature skipped) and installed. The SHA256 it kept is same-channel — SHA256SUMS ships from the same release as the binary, so whoever could swap one could swap the other. It proves the download finished, not who built it. That made README.md:75 ("Signature verification is mandatory ... fails closed", no platform qualifier) false on Windows, and only on Windows: install.sh has bootstrapped a pinned cosign and failed closed since backend#889. The README needs no edit — this makes the existing claim true. Windows now does what Linux and macOS do: bootstrap a pinned, checksum-verified cosign, and refuse to install when it can't. TRACEBLOC_ALLOW_UNVERIFIED=1 is the one escape, and it covers "cannot verify" only — never a verification that ran and FAILED, which is tampering evidence no env var should override. Two things this does that install.sh does not have to: - The bootstrap fetches cosign-windows-amd64.exe on BOTH architectures. Sigstore has never published a Windows arm64 build, so a per-arch name 404s and locks Windows-on-ARM out permanently (the same bug as tracebloc/client#734). Verification is over bytes, so the verifier's instruction set cannot change the verdict. - Test-CosignRuns separates "cosign won't start here" from "the signature is bad". They arrive through the same channel and warrant opposite messages — only one means the artifact may be tampered with. Absent x64 emulation on Windows-on-ARM is the case that makes this real, and "install cosign" would be useless advice there. Also sets a TLS 1.2 floor: PS 5.1 defaults to SSL3/TLS1.0 on older Windows, and every fetch here carries either the binary or the verifier that authenticates it. Tests — scripts/tests/install-ps1-verify.sh, install-verify.sh's sibling, wired into the same CI job. Against develop's installer: 0 passed, 5 failed. After: 5 passed, 0 failed. install.ps1 cannot be driven end-to-end on the Linux runner (it ends in Windows-registry PATH writes), so the behavioural tier extracts the helpers from the real file BY AST and executes them — a copy would pass while production broke. Six guards, each mutation-proven, each mutation asserted to have applied: [bool]$env:... instead of -eq '1' -> 4 fail checksum mismatch stops refusing -> 1 fail $LASTEXITCODE not armed before the probe -> 1 fail an opt-out on a FAILED verification -> 1 fail the no-cosign degrade restored -> 1 fail a \$ escape in a message -> 1 fail Three of those are bugs this change made and this tier caught before review: - $AllowUnverified was [bool]$env:TRACEBLOC_ALLOW_UNVERIFIED. Every non-empty string casts to $true in PowerShell, so setting it to 0 would have switched the bypass ON. - The bootstrap restated Get-Arch's logic minus its PROCESSOR_ARCHITEW6432 handling, so a 32-bit PowerShell host would have refused. Removed: the asset is arch-independent, there was nothing to branch on. - "Pin a signed \$env:RELEASE_VERSION" rendered as "Pin a signed ," — PowerShell escapes with a backtick, so \$ prints a backslash and expands the variable. Now a check of its own. One test in this tier was vacuous on its first writing: a "stale exit code" assertion built on an absent path, which throws and returns before ever reading $LASTEXITCODE. The mutation caught it — M3 applied and nothing reddened. Replaced with the input that is actually reachable: a PowerShell shim (scoop and chocolatey install cosign.ps1), which `&` dispatches in-process and which sets no $LASTEXITCODE at all. backend#2078
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
tracebloc-release-train
Bot
added
gate-nudge
… path (backend#1907) (#527) The CLI emits nothing today, so the backend#736 class — the binary landing on a PATH the shell does not read, `cluster info` reading a kubeconfig context nobody meant — is only ever visible when a customer mentions it. This wires the #1897 helper to a single terminal event per invocation: command, duration, exit code, OS/arch, version, error class. The ticket's "no arguments, no paths, no data" is built as a structure rather than a rule, because a rule is a thing every future call site has to remember: * the command is a LOOKUP into the set of paths enumerated from the live cobra tree, so a value that is not a command the CLI dispatches cannot be emitted at all — it reports `unregistered`, which stays countable; * the error class is keyed on an INT, the CLI's own frozen exit-code contract. The classifier is never handed an error message, so there is nothing for a path or a cell value to travel down; * everything else is an int. There is no redaction regex anywhere in the change. A sanitiser has to anticipate what it strips; a closed set only admits what was enumerated. os.type / host.arch go in the RESOURCE layer under OpenTelemetry's own names (§1.1 forbids re-inventing them as tracebloc.os): they are compile-time constants of the binary, so they describe the process, not the occurrence, and adding them to resourceScope means a call site still cannot set them. The guard is derived, not restated. TestEveryEmittedStringComesFromAClosedSet walks what the code ACTUALLY emits and requires every value to be an int or a member of a set assembled from the producer's own declarations — so a free-text channel fails it whether or not anyone thought to forbid the thing travelling down it. Thirteen mutations were run against it; each reddened, and each anchor was asserted to have applied. One of them (smuggling the raw command into a second attribute) was caught only by the telemetry-side test and NOT by the cli-side one, which was inspecting a single key — that test now sweeps the whole payload. WHAT IS NOT CONNECTED. The transport. The ticket says "rides the gateway and token"; the 17 Aug decision (rfcs#28) replaced the Collector gateway with an ingest endpoint on the backend, which is backend#1905 and does not exist yet. pendingSink() returns nil, so every event is validated and dropped. Validation runs regardless, so a malformed event fails in CI wherever the binary was built, and connecting #1905 is one function body. Opt-out (default on) via TRACEBLOC_NO_TELEMETRY or DO_NOT_TRACK, documented in docs/troubleshooting.md — and the document's claim about which variables work is itself a test, because a user who exports a stale name believes they have opted out and nothing else would ever tell them. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
3 tasks
* fix(install.ps1): verify-blob must not inherit a stale exit code `$LASTEXITCODE` persists from the previous command. `Test-CosignRuns` runs `cosign version` immediately before the verification, and presets 255 precisely so a binary that never starts cannot leave a stale 0 behind. The `verify-blob` call that actually GATES the install had no such preset. So a cosign shim that exits 0 on `version` and then no-ops on `verify-blob` leaves `$LASTEXITCODE` at 0, the `-ne 0` gate reads that as success, and the installer prints "cosign signature valid" and installs a binary nothing verified. That is RFC-0001 R8 defeated by a stale variable, one line from the guard that exists. Reproduced before fixing, driving the real block extracted from install.ps1 under pwsh with a no-op verifier: with fix -> REFUSED (LASTEXITCODE=255) without fix -> INSTALLED-UNVERIFIED (LASTEXITCODE=0) The regression assertion is a SOURCE check, and the limitation is stated rather than hidden: install.ps1 has NO behavioural coverage -- there is no pwsh or Pester anywhere in this repo's CI, which is why a signature gate that does not gate reached a promotion PR. Case 19 asserts the preset sits inside the `$sigDownloaded` block and before the invocation, by line number, so a preset elsewhere cannot satisfy it. It closes this hole; it does not make the Windows installer tested. Worth its own ticket. Mutation-proved: removing the preset reddens case 19 with the message naming the consequence. 34 passed / 0 failed; the mutation gives 33/1. One self-inflicted detail recorded because it is the repo's own failure class: the first version of the check grepped for `verify-blob` and matched the explanatory comment written directly above the call, so it failed on correct code. Anchored on the `& $cosign` invocation instead -- prose is not wiring. Found by Bugbot on release-train promotion PR cli#528 (High). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(install-verify): match the STATEMENT, not the text (shujaatTracebloc, #529) Both review findings were right, and both are the failure this PR is about -- left open on the side the PR did not anchor. A COMMENTED-OUT PRESET SATISFIED CASE 19. The check matched the substring `$global:LASTEXITCODE = 255` anywhere on a line, so `# $global:LASTEXITCODE = 255` passed it: the gate dead, the suite green. That is the likelier human mutation -- commenting the line out while debugging the installer -- and it was exactly the one not covered. The PR proved the DELETE mutation and missed this one. THE MIRROR IMAGE, ON THE SAME LINE. Hard-coded single spaces meant `$global:LASTEXITCODE=255` -- correct, equivalent PowerShell -- turned case 19 RED on a working gate, with a message asserting the installer would install unverified. A false alarm that names a supply-chain failure is worse than none. Anchoring the whole statement start-to-end, with flexible spacing, closes both. Suggestion taken as written from the review. AND THE `$` IS ESCAPED in the `if ($sigDownloaded)` grep, matching the two sibling patterns in the same block. In a POSIX BRE a `$` that is not at the end is undefined; an implementation treating it as an anchor matches nothing, `blk_line` comes back empty, and case 19 fails on correct code. Verified all three directions on this branch, reproducing the reviewer's results first: commented-out preset before 34/0 (escaped) -> after 33/1 (caught) no-space variant before 33/1 (false) -> after 34/0 (correct) preset deleted before 33/1 -> after 33/1 (still caught) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The last board writer on PROJECTS_KANBAN_TOKEN. A per-repo COPY, so one PR per repo; the content stays byte-identical across the fleet because the guard compares it that way. `owner:` makes the installation token ORG-scoped -- a repo-scoped one cannot write the org project. No fallback to the PAT: a fallback would let a broken App path keep working silently. This workflow also fires on DEPENDABOT PRs, which GitHub gates on a separate secret scope. Both app secrets are set there too; without that, Dependabot PRs would stop reaching the board with `Input required and not supplied` -- the exact failure PROJECTS_KANBAN_TOKEN already had to be dual-scoped to avoid. Refs backend#2036 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d18a520. Configure here.
tracebloc-release-train
Bot
added
the
gate-nudge
tracebloc-release-train
Bot
removed
the
gate-nudge
#531) * fix(telemetry): do not file a run signed into an unknown env under prod telemetryEnv repaired a present-but-unrecognised signed-in environment through api.ResolveEnv(""), which returns prod when $CLIENT_ENV is unset. New() then saw a known env and exported — filing a run signed into an unknown backend under prod, the exact guess §3.2 forbids and this function's own doc disclaims. Distinguish the two cases: empty (not signed in) still resolves via $CLIENT_ENV then the prod default; a present-but-unknown value is passed through unchanged so New() disables export. Correct TestTheEnvironmentIsNeverGuessed, which asserted the buggy prod answer for a signed-in "staging", and add an end-to-end regression (TestASignedInUnknownEnvironmentDeliversNothing). Bugbot (Medium), cli#528 staging mirror. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): label the record with the backend the client actually uses Reversing the direction of the first commit, per @saadqbal's review. The premise there — an unknown signed-in env is an unknown backend, so withhold — does not hold: sessionEnv (client.go) hands cfg.CurrentEnv to api.New verbatim and api.BaseURL routes every unrecognised value to prod. So a run signed into an unknown env genuinely hits prod, prod is the ACCURATE label, and withholding drops exactly the failed-install-on-prod runs this feature exists to see. telemetryEnv now mirrors api.BaseURL: resolve (CurrentEnv, else $CLIENT_ENV/prod), then known -> itself, unknown -> prod. The real bug it fixes is the old code reading $CLIENT_ENV for a signed-in env while the client ignores it — filing a run under 'dev' while every request went to prod. Rename the param env -> drop the shadow of signedInEnv(). Tests flip from 'delivers nothing' to 'labelled prod', plus TestASignedInUnknownEnvIgnoresClientEnv pinning the divergence (fails against the old code). Root cause — BaseURL silently routing unknown envs to prod — filed separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7f1fc28. Configure here.
Contributor
Author
|
bugbot run |
…#532) * fix(install.ps1): SET the TLS 1.2 floor, don't OR it onto the default The floor bitwise-OR-ed Tls12 onto [Net.ServicePointManager]::SecurityProtocol, which on PowerShell 5.1 already advertises SSL3/TLS1.0/1.1 — so those stay on and a fetch of the binary or the cosign verifier can still negotiate down, the exact downgrade the floor's own comment says it prevents (cli#528 Bugbot, Medium). Assign the protocol to Tls12 (dropping the weak ones), adding Tls13 only where the runtime defines the enum member (absent on older 5.1 hosts, where naming it throws). New install-ps1-verify assertion fails on the OR-onto-default form (mutation-proved); collapses newlines first since the old form spanned two lines. install-ps1-verify 6/6, behavioural tier 22/22. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(install.ps1): set only the Tls12 floor, drop the throwing Tls13 add Bugbot (Medium) on the first push: [Enum]::IsDefined([Net.SecurityProtocolType], 'Tls13') is true on .NET 4.8 even where Schannel cannot negotiate TLS 1.3 (Win10 21H1, Server 2019). Assigning Tls12 -bor Tls13 then THROWS, the empty catch swallows it, and SecurityProtocol is never set — so the Tls13 decoration could defeat the very Tls12 floor it was meant to extend. Assign Tls12 alone: it is the floor, always negotiable, and secure for these fetches. Verify assertion now pins the direct Tls12 assignment; still fails on the OR-onto-default form (mutation-proved). 6/6 verify, 22/22 behavioural. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d443509. Configure here.
Contributor
Author
|
Resolving the Bugbot thread to unblock the staging promotion under the non-High hop policy. Finding is tracked, not dropped:
|
tracebloc-release-train
Bot
added
the
gate-nudge
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
1 participant
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-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Opt-out telemetry is on by default with strong structural privacy guards and extensive tests, but it still touches every CLI exit path; Windows installer changes are security-positive but affect a privileged install path relied on by customers.
Overview
This promotion bundles CLI usage reporting, Windows installer hardening, and workflow/CI updates.
Command-outcome telemetry (backend#1907) adds one best-effort event per invocation from
mainafter the command tree returns: registered command path, exit code, duration, version, OS/arch, and error class from the frozen exit-code map—no args or paths by design. Opt-out viaTRACEBLOC_NO_TELEMETRYorDO_NOT_TRACK; events are validated in-process but not sent yet (pendingSinkis nil until backend#1905). Docs add a usage-reporting section;internal/telemetrygains outcome recording,os.type/host.archon the resource, and a 95% coverage floor.Windows
install.ps1(RFC-0001 R8 / backend#2078) aligns withinstall.sh: mandatory cosign (pinned bootstrap ofcosign-windows-amd64.exe, TLS 1.2 assignment,TRACEBLOC_ALLOW_UNVERIFIED=1only for “cannot verify”), presets$LASTEXITCODEbeforeverify-blob, and refuses failed verification. Newinstall-ps1-verify.shand AST-basedinstall-ps1-functions.tests.ps1run in CI;install-verify.shgains a source check for the verify-blob preset.GitHub Actions: the kanban workflow mints an org-scoped release-train app token (no PAT fallback); Dependabot needs the same app secrets.
Reviewed by Cursor Bugbot for commit d443509. Bugbot is set up for automated code reviews on this repo. Configure here.