Skip to content

fix(delete): don't brick local teardown when the revoke isn't a 403 - #205

Merged
LukasWodka merged 2 commits into
developfrom
fix/delete-revoke-best-effort
Jul 10, 2026
Merged

fix(delete): don't brick local teardown when the revoke isn't a 403#205
LukasWodka merged 2 commits into
developfrom
fix/delete-revoke-best-effort

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

tracebloc delete treated the server-side credential revoke as a hard gate: only HTTP 403 was special-cased (ask-an-admin); every otherRevokeClient error returned exitError{code:1} and short-circuited the entire offboard — helm uninstall, k3d teardown, image prune, ~/.tracebloc wipe, and self-remove never ran. The machine was left fully installed with no escape (--force only skips the online guard).

Realistic triggers, all leaving the user stuck:

  • a stale / wrong-account active-client pointer → account-scoped 404;
  • a backend predating/edge-device/<id>/revoke404;
  • a transient network/5xx blip.

This directly contradicts the same function's online guard 40 lines above, which treats 5xx/429/network as "warn and continue — the teardown is the real gate," and its own comment anticipating a 404 from the revoke.

(Found by an adversarial bug-hunt of the installer + CLI.)

Fix

Non-403 revoke failures now warn and continue into the offline-capable local teardown (removing tracebloc from this machine is the command's primary job — it can't be held hostage to a best-effort remote call). 403 still routes to ask-an-admin — a genuine authorization decision — unchanged. "Revoked …" prints only on actual success; the warn path is honest that the credential may still be live (revoke from the dashboard; the orphan reaper backend#970 sweeps a never-torn-down record later).

Tests

TestDelete_RevokeNon403_ContinuesTeardown — a 404 revoke → teardown still runs, honest warning, no false "revoked" claim. Existing TestDelete_RevokeForbidden_OffboardCopy (403 → ask-an-admin) unchanged. Full go test ./... green; gofmt -s / errcheck / ineffassign clean.

Type

Bug fix (offboard reliability) · cli · HIGH.


Note

Medium Risk
Changes offboard control flow and credential lifecycle messaging; a failed revoke can leave a live server credential until dashboard/reaper cleanup, but 401/426 paths prevent silent teardown with stale sessions.

Overview
tracebloc delete no longer aborts the whole offboard when server-side RevokeClient fails with anything other than a terminal auth/upgrade case. Previously only HTTP 403 was special-cased; 404, 5xx, and network errors returned exit 1 and skipped helm uninstall, cluster teardown, data wipe, and self-remove.

Non-403 failures (except 401 and 426) now warn and continue into offline-capable local teardown, aligned with the pre-offboard online guard. 403 still routes to ask-an-admin. 426 and 401 on the revoke step fail fast (no teardown)—especially important under --force, which skips the guard—so users aren't left with a wiped machine and a still-live credential.

A revoked flag drives honest UX: success text prints only when revoke actually succeeded; the final summary is a four-way split on revoke outcome × teardown completeness (degraded), instead of always claiming the machine is disconnected.

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

…sn't a 403
runDelete treated the credential revoke as a hard gate: only HTTP 403 was
special-cased (ask-an-admin); every OTHER RevokeClient error returned
exitError{code:1} and short-circuited the entire offboard — helm uninstall, k3d
teardown, image prune, ~/.tracebloc wipe, and self-remove never ran. So a stale/
wrong-account active-client pointer (account-scoped 404), a backend predating the
/edge-device/<id>/revoke route (404), or a transient network/5xx blip left the
machine fully installed with no escape (--force only skips the online guard).
That contradicts the same function's online-guard, which already treats
5xx/429/network as "warn and continue — the teardown is the real gate," and its
own comment anticipating a 404 revoke.
Fix: non-403 revoke failures now warn and continue into the (offline-capable)
local teardown, mirroring the online guard. 403 still routes to ask-an-admin
(unchanged, still tested). "Revoked …" is printed only on actual success; the
warn path says the credential may still be live (revoke from the dashboard; the
orphan reaper backend#970 sweeps a never-torn-down record later).
Test: TestDelete_RevokeNon403_ContinuesTeardown (404 revoke → teardown still runs,
honest warning, no false "revoked" claim). Existing TestDelete_RevokeForbidden
(403 → ask-an-admin) unchanged. Full suite green; gofmt -s / errcheck /
ineffassign clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursorcursorBot 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 2 potential issues.

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 68284b2. Configure here.

Comment threadinternal/cli/delete.go
Comment threadinternal/cli/delete.go
@saadqbal
saadqbal self-requested a review July 10, 2026 10:05
Comment threadinternal/cli/delete.go
Comment threadinternal/cli/delete.go
Addresses @saadqbal's #205 review:
- 426 revoke now fails fast with the upgrade prompt (mirrors the pre-offboard
guard) instead of warn-and-continue into a teardown the backend can't process.
- 401 revoke now fails fast to re-login. The --force footgun: with --force the
online guard is skipped, so an expired session used to silently tear the machine
down while leaving a live credential; now it aborts to sign-in.
- Track whether the server-side revoke actually succeeded; the closing summary is
honest on BOTH axes (revoke status x teardown status) and no longer claims the
credential is revoked / the machine disconnected when the revoke failed.
- Fix the now-false "credential is already revoked" comment on the teardown block.
- Tests: 404 + no-namespace honest closing (the degraded+revoke-failed case the
old test never reached), plus 426 and 401 revoke fail-fast.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Thanks @saadqbal — all addressed in ec884f9.

426 revoke → terminal (Bugbot HIGH). The revoke branch now mirrors the pre-offboard guard: a 426 fails fast with the upgrade prompt instead of warn-and-continuing into a teardown the backend can't process.

Closing summary no longer overclaims (Bugbot MED + your note). Added a revoked flag; the closing is now honest on both axes — server-side revoke status × local teardown status (4 states). It only says "the credential is revoked / no longer connected" when the revoke actually succeeded. When the revoke failed it says the credential may still be live and points at the dashboard + the reaper. Also fixed the now-false "credential is already revoked" comment on the teardown block.

401 → terminal (your minor, but a real --force footgun). With --force the online guard is skipped, so a 401 (expired session) used to silently tear the machine down while leaving a live credential. A 401 revoke now fails fast with the re-login hint, matching the guard. (403 still routes to ask-an-admin, unchanged.)

Test gap closed. You were right that the old 404 test never reached the degraded branch. Added:

  • TestDelete_RevokeNon403_Degraded_HonestClosing — 404 revoke + no namespace (uninstall skipped → degraded); asserts the closing does NOT claim "revoked / no longer connected" and DOES say the revoke didn't complete + the credential may still be live.
  • TestDelete_RevokeUpgradeRequired_FailsFast (426) and TestDelete_RevokeUnauthorized_FailsFast (401) — both under --force, asserting fail-fast with no teardown.

go build / go test ./... / gofmt / go vet all green.

@LukasWodka
LukasWodka merged commit f619db5 into developJul 10, 2026
16 checks passed
@LukasWodka
LukasWodka deleted the fix/delete-revoke-best-effort branch July 10, 2026 11:17
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.

2 participants

@LukasWodka@saadqbal