Skip to content

fix(cli): delete only removes tb when it is tracebloc's own alias - #171

Merged
LukasWodka merged 1 commit into
developfrom
fix/delete-tb-alias-guard
Jul 7, 2026
Merged

fix(cli): delete only removes tb when it is tracebloc's own alias#171
LukasWodka merged 1 commit into
developfrom
fix/delete-tb-alias-guard

Conversation

@LukasWodka

Copy link
Copy Markdown
Contributor

Summary

tracebloc delete (offboarding) removed any sibling file named tb, guarded only by tb != exe — with no check that it was tracebloc's own alias. If a user had an unrelated tb on the same PATH dir tracebloc installed into (another tool's binary, or a symlink pointing elsewhere), offboarding deleted it — wrong-target deletion of a file tracebloc never created.

This came out of the FR review of the offboarding surface (§7.10). It's not a Bugbot finding — Bugbot's five findings on #164 are all resolved (by #165, #168, #169); this is the one the review caught that they didn't.

Fix — make delete symmetric with install

The installer already refuses to clobber a pre-existing tb (scripts/install.sh: creates it only when absent, or when readlink tb == PREFIX/tracebloc). Delete now mirrors that ownership test:

  • New aliasStatus(tb, exe) (exists, ours bool)tb is ours only when it's a symlink resolving to this binary.
  • ours → remove it (unchanged behavior for a real install).
  • exists but not ours (regular file, or symlink elsewhere) → left in place, with a note: "Left … in place — it isn't tracebloc's tb alias." (mirrors the installer's "already exists and isn't ours" message).
  • Relative symlink targets resolve against the link's dir; an EvalSymlinks fallback covers the binary itself being a symlink (Intel Homebrew).

Tests

  • TestDelete_OwnTBAlias_Removed — a real tb → tracebloc symlink is removed.
  • TestDelete_ForeignTBAlias_Left — a foreign regular-file tb is not removed, is reported, and is untouched on disk. Verified to fail without the guard (unfixed removeSelf deletes it).
  • The full-sequence test now lays down a real symlink so the removal path is exercised honestly.
  • Full internal/... suite + go vet green.

Targets develop per convention.

🤖 Generated with Claude Code

`removeSelf` deleted any sibling file named `tb` (guard was only `tb !=
exe`), with no check that it was tracebloc's own symlink. If a user had an
unrelated `tb` on the same PATH dir tracebloc installed into (another
tool's binary, or a symlink elsewhere), `tracebloc delete` removed it —
wrong-target deletion of a file tracebloc never created.
Make delete symmetric with install. The installer refuses to clobber a
pre-existing `tb` (install.sh: creates it only when absent or when
`readlink tb == PREFIX/tracebloc`). Delete now mirrors that: new
aliasStatus() removes `tb` only when it is a symlink resolving to this
binary; a regular file or a symlink pointing elsewhere is left in place
with a note ("isn't tracebloc's `tb` alias"). Relative targets resolve
against the link dir, and an EvalSymlinks fallback covers the case where
the binary itself is a symlink (Intel Homebrew).
Found in the FR review of the offboarding surface; not a Bugbot finding.
Tests: TestDelete_OwnTBAlias_Removed (real tb->binary symlink is removed),
TestDelete_ForeignTBAlias_Left (a foreign regular-file `tb` is NOT removed
and is reported), and the full-sequence test now lays down a real symlink.
The foreign-tb test is verified to fail without the guard. Suite + vet green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Jul 7, 2026
@LukasWodka
LukasWodka requested a review from saadqbalJuly 7, 2026 15:08
@LukasWodka
LukasWodka merged commit a2c5816 into developJul 7, 2026
17 checks passed
saadqbal added a commit that referenced this pull request Jul 8, 2026
…atch phase)
Four low/medium follow-ups from the Bugbot review of PR #164; the other
five Bugbot items were already fixed by #165/#168/#171.
- delete: fail fast on a 401/403 from the pre-offboard online check
(revoked/expired credential) with a `tracebloc login` hint, instead of
warning and marching the user through the confirm only to fail at revoke.
- delete: clear + persist the active-client pointer IMMEDIATELY after the
revoke succeeds (before the best-effort teardown), so a failed or
interrupted teardown can't leave the host looking enrolled under a dead
credential.
- delete: mark the offboard degraded when removeSelf can't remove the CLI
binary / own `tb` alias, so the closing line stops claiming a clean
offboard while the CLI is still on disk.
- data ingest: a pod already in Succeeded prints "Ingestion complete —
showing its logs:" (replayed) instead of the "live progress:" line.
Tests added for each; full internal/... suite + go vet + gofmt green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka
LukasWodka deleted the fix/delete-tb-alias-guard branch July 9, 2026 11:41
aptracebloc pushed a commit that referenced this pull request Jul 13, 2026
Adds real e2e coverage of the top-level `tracebloc delete` offboard
(RFC-0001 §7.10), which CI's e2e.yml previously left untested (it only
exercised `data ingest`). New test/integration/delete_e2e_test.go:
- TestE2E_RevokeUsesPostNotDelete: a real HTTP round-trip against a
recording stub asserts acceptance (a) — the credential is REVOKED via
POST /edge-device/<id>/revoke/, never a hard DELETE of the row.
- TestE2E_DeleteTeardown: builds the real binary and runs
`tracebloc delete --yes --force` black-box against a throwaway k3d
cluster with a real Helm release, asserting (b) the release is
uninstalled, (c) the k3d cluster is deleted, (d) ~/.tracebloc is wiped,
and (e) the foreign-`tb` guard (#171) leaves a `tb` it didn't create in
place. Opt-in via TB_E2E_K3D=1 and refuses a pre-existing `tracebloc`
cluster, so it never clobbers a dev machine.
The black-box run is kept fully offline (egress through a dead proxy) so
the revoke takes its documented best-effort transport-failure path while
the local teardown runs for real; the POST-not-DELETE contract a live
backend enforces is covered by the stub test. Revoke against a live
backend is left to the pre-prod FR (CI can't provide one — no base-URL
override on the CLI).
Wires a new `delete-teardown` job (k3d + helm) into e2e.yml, mirroring
the ingest e2e job's nightly/dispatch/`e2e`-label gating.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for freeto 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

@LukasWodka@aptracebloc@saadqbal