Skip to content

fix: Bugbot findings from promotion PR #397 (TLS floor + delete cache)#404

Merged
shujaatTracebloc merged 2 commits into
developfrom
fix/bugbot-cli-tls-delete
Jul 24, 2026
Merged

fix: Bugbot findings from promotion PR #397 (TLS floor + delete cache)#404
shujaatTracebloc merged 2 commits into
developfrom
fix/bugbot-cli-tls-delete

Conversation

@shujaatTracebloc

@shujaatTracebloc shujaatTracebloc commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Resolves the second round of Cursor Bugbot findings surfaced on the cli promotion PR (#397, develop → main), both introduced/adjacent to the earlier upgrade fix (#402).

Fixes:

  • Installer curl omits TLS floor — the shared installerRunScript (used by tracebloc upgrade and tracebloc prepare-host) now downloads the installer with curl -fsSL --tlsv1.2, matching the TLS 1.2 floor scripts/install.sh enforces on every security-sensitive fetch, so this privileged download can never negotiate a weaker protocol. (Release develop → main #397 (comment))
  • Nudge recreates wiped config dir — after tracebloc delete wipes ~/.tracebloc, the post-command update check no longer resurrects it. SkipUpdateNudge now also suppresses the nudge (and its cache write) after delete — reusing the existing skip mechanism — and writeUpdateCache no longer MkdirAlls a missing dir (it only writes when the dir already exists), so a throttle cache can never recreate a just-offboarded host data dir. (Release develop → main #397 (comment))

Lands on develop; the promotion PR head picks these up on the next sync and Bugbot re-reviews.

🤖 Generated with Claude Code


Note

Low Risk
Targeted CLI hygiene and TLS hardening on installer fetch; behavior change is suppressing update nudge/cache after offboard and skipping cache writes when the data dir is gone.

Overview
Addresses Bugbot findings from the promotion PR around privileged installer downloads and offboard hygiene after tracebloc delete.

Installer download — Shared installerRunScript (used by upgrade and prepare-host) now passes curl --tlsv1.2, aligned with scripts/install.sh, so those downloads cannot negotiate below TLS 1.2. A test locks this in for both subcommand variants.

After offboardwriteUpdateCache no longer MkdirAll the config parent; it only writes when that directory already exists, so a post-command update check cannot resurrect a wiped ~/.tracebloc. SkipUpdateNudge moves from a hard-coded upgrade name check to a Cobra tracebloc.io/skip-update-nudge annotation on upgrade and top-level delete only (not data delete). Tests cover the missing-dir cache behavior and the annotation vs. name collision.

Reviewed by Cursor Bugbot for commit db9c2ab. Bugbot is set up for automated code reviews on this repo. Configure here.

… cache)

- installer: pin the TLS 1.2 floor (--tlsv1.2) on the shared installerRunScript
  download, matching scripts/install.sh, so `tracebloc upgrade` and
  `prepare-host` can never negotiate a weaker protocol
- update-check: skip the post-command nudge after `tracebloc delete` (reuse
  SkipUpdateNudge) AND make writeUpdateCache refuse to recreate a missing
  ~/.tracebloc, so a clean offboard is never resurrected by the throttle cache

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTracebloc shujaatTracebloc self-assigned this Jul 24, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5023869. Configure here.

Comment thread internal/cli/delete.go Outdated
…404)

isDeleteCommand matched any leaf named "delete", so the nudge was wrongly
skipped after `data delete` too (which neither offboards nor wipes ~/.tracebloc).
Replace the name-based upgrade/delete predicates with a self-declared cobra
annotation (skipUpdateNudgeAnnotation) set on the top-level upgrade and delete
commands, so `data delete` is unaffected. Add a table-guard test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTracebloc
shujaatTracebloc merged commit fcdefca into develop Jul 24, 2026
20 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the fix/bugbot-cli-tls-delete branch July 24, 2026 14:48
@LukasWodka

Copy link
Copy Markdown
Contributor

Functional review \u2014 passed\n\nBasis: the behavioural suites that ran on this PR at merge against real environments, not mocks:\n\n- + \u2014 the CLI exercised against a real kind cluster, plus the installer suite\n\nI could not reach the dev API (no dev credentials), so rather than rubber-stamp I used the strongest evidence available: these suites exercise the actual behaviour this change alters, on real infrastructure. Advancing \u2192 .\n\nIf the functional reviewer wants a manual pass on dev in addition, please move it back and say so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants