fix(cli#125): write the UUID username (not the numeric id) as TRACEBLOC_CLIENT_ID#126
Merged
Merged
Conversation
…OC_CLIENT_ID
`client create` wrote strconv.Itoa(pc.ID) — the numeric dashboard id — as
TRACEBLOC_CLIENT_ID in both credential-file paths (mint + adopt) and in the
printed credential. But TRACEBLOC_CLIENT_ID is the *auth username*: it flows
cred → helm clientId → secret CLIENT_ID → pod env → controller.py
os.getenv("CLIENT_ID"), which is POSTed to api-token-auth as the login
username. The backend authenticates an EdgeDevice by its UUID username
(str(uuid4)), never the numeric id, so a freshly-provisioned client always
failed auth ("Unable to log in with provided credentials") and crash-looped.
Write pc.Username instead. The numeric id stays display-only (shown as
"dashboard id"). Both credential-file tests asserted the buggy numeric id and
are updated to require the username; the print-path test now asserts the
username is shown as the client id.
This is what client PR #293's skip-verify masked — verify_credentials was a
correct canary. #293 will be reverted once this ships (v0.5.1).
Refs: RFC-0001 backend#830, installer credential handoff #838.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
👋 Heads-up — Code review queue is at 31 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
saadqbal
added a commit
that referenced
this pull request
Jul 1, 2026
* Merge pull request #111 from tracebloc/sec/rfc-0001-r8-mandatory-verify fix(security): mandatory / fail-closed cosign verification in the CLI installer (RFC-0001 R8) * fix: validate COSIGN_VERSION and enforce TLS 1.2 on installer downloads (#114) Bugbot (promotion PR #113), two findings in scripts/install.sh: - Unvalidated cosign version in URL: COSIGN_VERSION (env-overridable) was interpolated into the Sigstore download URL without the semver/path-traversal gate applied to RELEASE_VERSION. Generalized validate_tag into validate_version_tag and apply it to COSIGN_VERSION before building the URL. (scripts/install.sh:45) - Signature download curls omit TLS: the .sig/.cert fetches (and the binary, SHA256SUMS, and latest-redirect curls) lacked --tlsv1.2, unlike the new cosign bootstrap fetches. Add --tlsv1.2 to every security-sensitive download. (scripts/install.sh:335) Co-authored-by: shujaat hasan <shujaathasan@shujaats-MacBook-Pro.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore(schema): re-vendor ingest.v1.json — register seq2seq as recognize-but-unsupported (#118) The CI "Schema drift check" (scripts/sync-schema.sh --check) was failing on develop itself and therefore every PR — the vendored internal/schema/ingest.v1.json had drifted from data-ingestors master, which added the `seq2seq` task category (enum entry + an if/then requiring `texts`, membership in the self-supervised text group, and an updated `texts` description). The drift is purely additive — it does not change validation for any already-supported category. - Re-vendored the schema (sync-schema.sh) → --check now passes. - Registered seq2seq in internal/push/category.go as recognize-but-not-yet- CLI-supported (CLISupported:false + UnsupportedNote), Family text: the CLI's discover/build for its raw-.txt / source\ttarget `texts` layout isn't implemented, so push reports it as pending rather than leaving a schema<->registry gap (the cli#74 drift class that TestRegistryCoversSchema- Categories pins). Updated the parity tests. Mirrors #103, which did exactly this for causal_language_modeling. Full seq2seq push support (discover/build, flip CLISupported) is a follow-up feature — the sibling of cli#105. go build / vet / test ./... green; drift check green. Closes #117 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(auth): send User-Agent version header + handle 426 Upgrade Required (cli#98) (#115) RFC-0001 §13 / §14 R11 / Appendix C.1 — the CLI half of the minimum-supported-CLI-version handshake (server half: backend#888, shipped in backend#916). - Every backend request now carries `User-Agent: tracebloc-cli/<ver> (<os>/<arch>)`, injected by a transport wrapper on the shared internal/api client so login + provisioning are both covered without threading the version through each command. The version is the ldflags build info, recorded once via api.SetUserAgent in NewRootCmd; a build with no version reports "dev" (unparseable server-side → fails open). - A 426 from any endpoint is detected centrally in post/get and surfaced as a typed *UpgradeRequiredError carrying min_version, so every command degrades to the same actionable "your CLI is too old — upgrade to >= X" message and a clean non-zero exit (no stack trace), in both interactive and --plain modes. Tests: UA on the wire, dev fallback, 426 → UpgradeRequiredError (GET + POST), unparseable-426 body, message actionability. Closes #98. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(auth): logout revokes the token server-side (cli#112) (#116) * feat(auth): logout revokes the token server-side via POST /auth/revoke (cli#112) RFC-0001 §7.5 / R2 — the CLI half of the logout-revoke gap (server endpoint: backend#887, shipped in backend#903). Until now `logout` was local-only: it cleared ~/.tracebloc but a copied/leaked token kept authenticating (confirmed in the 2026-06-25 connect-flow FR). - New api.Client.RevokeToken → POST /auth/revoke (Bearer in, 204 out, idempotent); a non-2xx surfaces as *APIError. - logout now revokes server-side before clearing local state. Best-effort by contract: on failure (offline / already-revoked / 5xx) it logs a hint and still clears local state — the user must always be able to log out locally. Tests: RevokeToken 204→nil (Bearer + POST on the wire) and 5xx→APIError; logout calls revoke (now routed at a stub — it was about to hit real prod) and still clears local state when the revoke fails. Closes #112. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(auth): address Bugbot on logout revoke — env resolution + save ordering (cli#112) Two Medium findings on the revoke wiring: - Resolve the revoke env consistently with authedClient via a new shared sessionEnv(cfg) helper (saved env → $CLIENT_ENV → prod), instead of hardcoding prod for an empty cfg.Env — which could revoke against the wrong host and leave the real session token valid after sign-out. - Clear + persist local state BEFORE the revoke call, so a failed Save can't leave a token that's already been revoked server-side on disk as a broken "signed in" state. Revoke stays best-effort, after the local clear. Test: TestLogout_RevokesAgainstSessionEnv pins empty cfg.Env → $CLIENT_ENV. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(cli): rename dataset → data (ingest/list/delete; keep aliases) (cli#99) (#119) Rename the `dataset` command group to `data` and its verbs: - `push` → `ingest` (canonical: `tracebloc data ingest <path>`) - `rm` → `delete` (canonical: `tracebloc data delete <table>`) - `list` stays `list` Deprecated aliases retained for one cycle via cobra's Aliases field: - `data` has `Aliases: []string{"dataset"}` - `ingest` has `Aliases: []string{"push"}` - `delete` has `Aliases: []string{"rm"}` All user-facing text updated (Short, Long, Banner, home screen, examples). Internal Go symbols renamed accordingly (newDataCmd, newDataIngestCmd, newDataDeleteCmd, runDataIngestArgs, runDataIngest, runDataDelete, etc.). Tests updated to use canonical names; alias-resolution tests added. No behaviour change beyond naming; ingestion logic and cli#67–#77 fixes are untouched. The separate top-level `ingest validate` command is unmodified. Closes #99 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(cli): env-scoped config v2 — per-env profiles fix the cross-env clobber (cli#100) (#120) RFC-0001 §7.5 / §14 R10 / Appendix C.8. The v1 ~/.tracebloc/config.json held a single flat {env,token,active_client_id}, so `login --env X` overwrote env+token but stranded the previous env's active_client_id — a dev/stg/prod user could silently target the wrong client. - Config is now v2: { version, current_env, profiles: { <env>: {email, token, expires_at, active_client_id} } }. One active_client_id PER env. - `login --env X` switches current_env and writes X's profile via Profile(env) (which returns X's existing profile), so a re-login preserves its active_client_id instead of clobbering it; other envs are untouched. - v1 files auto-migrate to v2 on first read (the single record wrapped under profiles[env]); the first Save rewrites the file as v2. No data loss. - `logout` clears only the current env's profile; other envs untouched. (Still revokes server-side, best-effort — from cli#112.) - File stays 0600. Call sites (login / logout / auth status / client create·list·use) now read and write via cfg.Current() / Profile(env) / CurrentEnv. expires_at is persisted per profile and shown by `auth status` when present (login doesn't capture it yet). Tests: v1→v2 migration (+ empty-env→prod), the dev→prod→dev no-clobber (R10) guarantee, SignedIn semantics, 0600, and every login/logout/auth/client path on v2. Closes #100. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(cli): --verbose + cluster doctor auth checks + resume hint & install log (cli#101) (#121) * feat(cli): --verbose + cluster doctor auth checks + resume hint & install log (cli#101) RFC-0001 §8.5 — make the zero-prompt connect flow's FAILURE path stand on its own (the installer-side streaming stays with backend#838, per the ticket). - `--verbose` / $TRACEBLOC_LOG_LEVEL: a root persistent flag that streams the device-flow → provision detail via a new verbose-gated ui.Detailf. Default output stays quiet (~the usual handful of ✔ lines). - `cluster doctor` now runs an "Auth & config" section FIRST — before the cluster checks, so it works even when no cluster is reachable (the failed-provision case): signed in? which env + account? active client set? plus a live token check (WhoAmI) — 401 → ✖ re-login, network error → ⚠. Its status folds into the overall verdict. - `client create` failure prints the exact, idempotent resume command (§7.2) + a `tracebloc cluster doctor` pointer, so a broken headless connect isn't a dead end. - Every `client create` run writes ~/.tracebloc/install-<ts>.log (0600) — a full trace on disk even when the terminal stayed quiet. Tests: ui Detailf gating; doctor auth (not-signed-in / valid / 401 / no-active-client); verbose-streams vs quiet-default; provision failure → resume command + install log. Closes #101. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): don't log a cancelled provision as "done" (cli#101, Bugbot) The success-path defer blanket-logged "done" on err==nil, so declining the confirm prompt (which returns nil after "Cancelled.") recorded a user abort as a successful run in install-<ts>.log — misleading for support / post-mortems. Log the terminal outcome at each branch instead (minted / adopted / cancelled); the defer now only handles the failure case. Test: TestClientCreate_CancelLogsCancelledNotDone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): resume command includes prompted name/location, not just flags (cli#101, Bugbot) resumeCommand(opts) read opts, which carries only the flag values — so a failed interactive provision (name/location typed at a prompt) printed a resume command missing --name/--location, defeating the copy-paste-to-retry goal. Write the resolved values back into opts after gathering them, so the defer's resumeCommand reflects what the user actually entered. Test: TestClientCreate_ResumeCommandIncludesPromptedValues. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): doctor folds auth into the kubeconfig-exit code; install log advertises no path on open failure (cli#101, Bugbot) Two more Bugbot findings: - `cluster doctor` returned exit 3 on a kubeconfig load / clientset failure even when the auth section had failed (e.g. a 401), so automation could read a bad token as a kubeconfig-only problem. A failed auth section now escalates the exit to 2; exit 3 is kept for the auth-OK case (the documented contract). - newInstallLog returned the intended path even when opening the file failed, so the failure hint could print "Full log:" for a file that was never written. It now returns an empty path, and the caller's guard skips the hint. Tests: doctor kubeconfig-fail → exit 2 (auth fail) / exit 3 (auth OK); install log returns an empty path when the file can't be opened. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): cluster doctor treats a 426 as a hard upgrade failure, not a transient warning (Bugbot/#113) (#122) doctor's live token check (WhoAmI) treated every non-401 error as a transient "couldn't verify — check your network" warning. A 426 (the server enforces a newer CLI, surfaced as *api.UpgradeRequiredError by the #98 handling) is a definite, actionable problem — now reported as a hard ✖ "this CLI is too old — upgrade to >= X" failure with the min-version message. Test: TestRunAuthChecks_426IsHardFailure. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): don't misread a v2-shaped config as v1 (Bugbot/#113) (#123) Load routed any config with version < 2 to migrateV1, which reads only the flat v1 fields — so a v2-shaped file with a missing/wrong version would be migrated to an empty record, silently dropping its profiles. migrateV1 now fires only for a genuine v1 record (old version AND no `profiles` object); a profiles-bearing file is always parsed as v2. Test: TestLoadV2ShapedWithoutVersion_NotMigrated. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli#125): write the UUID username (not the numeric id) as TRACEBLOC_CLIENT_ID (#126) `client create` wrote strconv.Itoa(pc.ID) — the numeric dashboard id — as TRACEBLOC_CLIENT_ID in both credential-file paths (mint + adopt) and in the printed credential. But TRACEBLOC_CLIENT_ID is the *auth username*: it flows cred → helm clientId → secret CLIENT_ID → pod env → controller.py os.getenv("CLIENT_ID"), which is POSTed to api-token-auth as the login username. The backend authenticates an EdgeDevice by its UUID username (str(uuid4)), never the numeric id, so a freshly-provisioned client always failed auth ("Unable to log in with provided credentials") and crash-looped. Write pc.Username instead. The numeric id stays display-only (shown as "dashboard id"). Both credential-file tests asserted the buggy numeric id and are updated to require the username; the print-path test now asserts the username is shown as the client id. This is what client PR #293's skip-verify masked — verify_credentials was a correct canary. #293 will be reverted once this ships (v0.5.1). Refs: RFC-0001 backend#830, installer credential handoff #838. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Asad Iqbal (Saadi) <asad.dsoft@gmail.com> Co-authored-by: shujaat_tracebloc <153823837+shujaatTracebloc@users.noreply.github.com> Co-authored-by: shujaat hasan <shujaathasan@shujaats-MacBook-Pro.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
5 tasks
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.
Closes #125.
The bug
A freshly-provisioned client (
tracebloc client create --credential-file …, the RFC-0001 installer flow) deploys but crash-loops on auth:client createwrote the numeric dashboard id asTRACEBLOC_CLIENT_ID:But
TRACEBLOC_CLIENT_IDis the auth username, not a display id. The chain:The backend's
EdgeDevice.usernameis a UUID (str(uuid.uuid4())), never the numeric id — so the pod sentusername="1052", matched no user, and auth always failed. Password + everything else were correct.The fix
Write
pc.Username(the UUID) asTRACEBLOC_CLIENT_IDin both credential-file paths (mint + adopt). The numeric id becomes display-only — the printed credential now shows the username as client id and the number as dashboard id (a human reference at ai.tracebloc.io/clients, never an installer input).Tests
Both existing credential-file tests codified the bug (
TestClientCreate_CredentialFileMintassertedid=="5",..._CredentialFileAdoptassertedid=="8"— the numeric ids, while the mocks carry distinct usernamesu-5/u-8). Updated to require the username, so they now guard the invariant.TestClientCreate_AnchorMintadditionally asserts the username is printed as the client id. Full suite green.Follow-ups
verify_credentialswas a correct canary that #293 masked; restoring it, the installer e2e on dev should go green and the pod should authenticate.Refs: RFC-0001 backend#830, installer credential handoff #838.
Note
Medium Risk
Wrong credentials broke all new RFC-0001 installs; the change is small but sits on the critical provisioning→auth path for every new client.
Overview
Fixes client crash-loops after
tracebloc client create --credential-fileby aligningTRACEBLOC_CLIENT_IDwith what the jobs-manager pod actually sends toapi-token-authas the login username.Mint and adopt credential-file paths now set
TRACEBLOC_CLIENT_IDfrompc.Username(the EdgeDevice UUID) instead ofstrconv.Itoa(pc.ID). When credentials are printed to the terminal, client id shows that username and the numeric id is labeled dashboard id only.Tests that previously expected the numeric id in the env file and stdout were updated to require the username (
u-5/u-8), locking in the installer invariant.Reviewed by Cursor Bugbot for commit 9fe85c5. Bugbot is set up for automated code reviews on this repo. Configure here.