From b20d5bc1226ebeef650ee5178d2449c3f6b81b1a Mon Sep 17 00:00:00 2001 From: leet-c1 <264029741+leet-c1@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:35:11 +0000 Subject: [PATCH 1/4] feat: report the resolved tenant from auth whoami, machine-readably The tenant a write would land on was readable only from `auth status`'s prose or from the stderr warning that fires only when --url is omitted -- so the signal vanished exactly when a caller passed --url, and toolkits gating writes on "confirm the tenant first" had to sed that warning line. `auth whoami` now adds two client-resolved keys to its summary and to --verbose: `tenant` (the resolved base URL) and `tenantSource` (flag/env/ config). Both go through writeObject, so `--fields tenant` is the check. They are emitted only after credentials are proven against that tenant, so an auth failure still exits 3 with no tenant. `auth status`'s text output is unchanged; no second JSON surface (`auth status --json`) was added for the same fact. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 16 +++ cmd/auth_whoami.go | 28 ++++- cmd/auth_whoami_tenant_test.go | 220 +++++++++++++++++++++++++++++++++ cmd/docs_guide.go | 7 +- cmd/root.go | 30 ++++- 5 files changed, 290 insertions(+), 11 deletions(-) create mode 100644 cmd/auth_whoami_tenant_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cfc38e6..6716791 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,22 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). whose cursor never advances it aborts with the existing stuck-cursor error, which is still better than the silent truncation it replaces. +- **`auth whoami` now reports the tenant it resolved, machine-readably.** + Two client-resolved keys join its JSON summary (and its `--verbose` dump): + `tenant`, the base URL this invocation resolved, and `tenantSource`, where + that URL came from (`flag`, `env`, or `config`). Before this, the resolved + tenant was readable only from `auth status`'s prose or from the stderr + warning that fires *only* when `--url` was omitted -- so the signal + disappeared exactly when a caller did the right thing and passed `--url`, + and toolkits that gate writes on "confirm the tenant first" had to `sed` + that warning line. `c1i auth whoami --url --fields tenant` is now + the check. The keys are emitted only after the credentials are proven + against that tenant, so an auth failure still exits 3 with no tenant rather + than naming a target the caller cannot reach. `auth status`'s text output + is unchanged -- anything parsing it keeps working -- and no `--json` flag + was added to it, since a second JSON surface for the same fact is exactly + the duplication that drifts. + - **`apps owners`, `apps add-owner`, and `apps remove-owner`.** `apps get`'s `appOwners` field was empty on every app checked, while `GET .../owners` returns the owners `apps set-owners` had already written, but reading diff --git a/cmd/auth_whoami.go b/cmd/auth_whoami.go index 6b467b0..f295207 100644 --- a/cmd/auth_whoami.go +++ b/cmd/auth_whoami.go @@ -10,19 +10,30 @@ import ( var authWhoamiCmd = &cobra.Command{ Use: "whoami", - Short: "Show the authenticated principal's user ID and tenant scope", + Short: "Show the authenticated principal and the tenant being targeted", Long: `Calls /api/v1/auth/introspect and returns a compact summary of the authenticated principal: userId, principleId, and counts of roles, permissions, and feature flags. +Two client-resolved keys are added to that summary, and to --verbose: +"tenant" is the base URL this invocation resolved, and "tenantSource" is +where it came from ("flag", "env", or "config"). This is the machine-readable +form of the tenant "auth status" prints as text — check it before a write: + + c1i auth whoami --url https://mycompany.conductor.one --fields tenant + +Both keys report where a request WOULD go; they are only emitted once the +credentials are proven against that tenant, so an auth failure exits nonzero +with no tenant rather than reporting an unusable target. + The full introspect payload can include hundreds of roles and over a thousand permissions — pass --verbose to dump it all.`, RunE: func(cmd *cobra.Command, args []string) error { - baseURL, err := GetBaseURL() + baseURL, urlSource, err := requireBaseURL() if err != nil { return err } - c, err := newClient(cmd, baseURL) + c, err := newWhoamiClient(cmd, baseURL) if err != nil { return fmt.Errorf("not authenticated: %w", err) } @@ -58,10 +69,14 @@ thousand permissions — pass --verbose to dump it all.`, } } - var obj any = summarize(payload, displayName, email) + obj := summarize(payload, displayName, email) if verbose { obj = payload } + // Always the client-resolved values, never the payload's: the question + // is which host this invocation would write to. + obj["tenant"] = baseURL + obj["tenantSource"] = urlSourceToken(urlSource) out, err := json.Marshal(obj) if err != nil { return err @@ -90,6 +105,11 @@ func summarize(p map[string]any, displayName, email string) map[string]any { return out } +// newWhoamiClient is a var, not a direct newClient call, so a test can +// substitute an httptest-backed client — mirroring newListClient (cmd/client.go) +// and newAPIClient (cmd/api.go). +var newWhoamiClient = newClient + func sliceLen(v any) int { if s, ok := v.([]any); ok { return len(s) diff --git a/cmd/auth_whoami_tenant_test.go b/cmd/auth_whoami_tenant_test.go new file mode 100644 index 0000000..daf18c4 --- /dev/null +++ b/cmd/auth_whoami_tenant_test.go @@ -0,0 +1,220 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/ConductorOne/c1i/internal/client" + "github.com/spf13/cobra" + "github.com/spf13/viper" +) + +// stubWhoamiServer answers introspect (and the follow-up user lookup) with a +// fixed payload, and points newWhoamiClient at it. status is the code the +// introspect call answers with. +func stubWhoamiServer(t *testing.T, status int) { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.HasPrefix(r.URL.Path, "/api/v1/users/") { + _, _ = w.Write([]byte(`{"userView":{"user":{"displayName":"Ada","email":"ada@example.invalid"}}}`)) + return + } + if status != http.StatusOK { + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"message":"denied"}`)) + return + } + _, _ = w.Write([]byte(`{"userId":"u1","principleId":"p1","tenantId":"t1","roles":["r"],"permissions":[],"features":[]}`)) + })) + t.Cleanup(srv.Close) + + orig := newWhoamiClient + newWhoamiClient = func(_ *cobra.Command, _ string) (*client.Client, error) { + return client.NewForTesting(srv.URL, srv.Client()), nil + } + t.Cleanup(func() { newWhoamiClient = orig }) +} + +// runWhoami executes `auth whoami` with args and returns stdout plus the +// command error, isolating the flag state each case sets up. +func runWhoami(t *testing.T, args []string) (string, error) { + t.Helper() + resetRootURLFlag(t) + resetCmdFlags(t, authWhoamiCmd) + + var out bytes.Buffer + rootCmd.SetOut(&out) + rootCmd.SetErr(&out) + t.Cleanup(func() { + rootCmd.SetOut(nil) + rootCmd.SetErr(nil) + rootCmd.SetArgs(nil) + }) + rootCmd.SetArgs(append([]string{"auth", "whoami"}, args...)) + err := rootCmd.ExecuteContext(context.Background()) + return out.String(), err +} + +// TestWhoamiReportsResolvedTenant is the point of the feature: the tenant a +// write would land on must be readable from JSON, not only from `auth +// status`'s prose or the stderr warning that fires only when --url is +// omitted. tenantSource names the resolution path, so an agent can tell an +// explicit --url from a silent ~/.c1i.yaml fall-through. +func TestWhoamiReportsResolvedTenant(t *testing.T) { + cases := []struct { + name string + setup func(t *testing.T) + args []string + wantTenant string + wantSource string + }{ + { + name: "flag", + setup: func(t *testing.T) { t.Setenv("C1I_URL", "") }, + args: []string{"--url", "https://acme.conductor.one"}, + wantTenant: "https://acme.conductor.one", + wantSource: "flag", + }, + { + name: "env", + setup: func(t *testing.T) { t.Setenv("C1I_URL", "https://acme-env.conductor.one") }, + wantTenant: "https://acme-env.conductor.one", + wantSource: "env", + }, + { + name: "config", + setup: func(t *testing.T) { + t.Setenv("C1I_URL", "") + orig := viper.GetString("url") + viper.Set("url", "https://acme-config.conductor.one") + t.Cleanup(func() { viper.Set("url", orig) }) + }, + wantTenant: "https://acme-config.conductor.one", + wantSource: "config", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tc.setup(t) + stubWhoamiServer(t, http.StatusOK) + + out, err := runWhoami(t, tc.args) + if err != nil { + t.Fatalf("auth whoami: %v (output %q)", err, out) + } + var got map[string]any + if uerr := json.Unmarshal([]byte(out), &got); uerr != nil { + t.Fatalf("output is not JSON: %v (%q)", uerr, out) + } + if got["tenant"] != tc.wantTenant { + t.Errorf("tenant = %v, want %q", got["tenant"], tc.wantTenant) + } + if got["tenantSource"] != tc.wantSource { + t.Errorf("tenantSource = %v, want %q", got["tenantSource"], tc.wantSource) + } + if got["userId"] != "u1" { + t.Errorf("userId = %v, want u1 (the identity summary must survive)", got["userId"]) + } + }) + } +} + +// TestWhoamiVerboseReportsResolvedTenant pins that --verbose carries the same +// two keys: `--fields tenant` must not depend on whether --verbose was +// passed. The server's own tenantId is a different fact (an id, not a host) +// and must still come through untouched. +func TestWhoamiVerboseReportsResolvedTenant(t *testing.T) { + t.Setenv("C1I_URL", "") + stubWhoamiServer(t, http.StatusOK) + + out, err := runWhoami(t, []string{"--url", "https://acme.conductor.one", "--verbose"}) + if err != nil { + t.Fatalf("auth whoami --verbose: %v (output %q)", err, out) + } + var got map[string]any + if uerr := json.Unmarshal([]byte(out), &got); uerr != nil { + t.Fatalf("output is not JSON: %v (%q)", uerr, out) + } + if got["tenant"] != "https://acme.conductor.one" { + t.Errorf("tenant = %v, want the resolved base URL", got["tenant"]) + } + if got["tenantSource"] != "flag" { + t.Errorf("tenantSource = %v, want flag", got["tenantSource"]) + } + if got["tenantId"] != "t1" { + t.Errorf("tenantId = %v, want t1 (the payload's own key must survive)", got["tenantId"]) + } +} + +// TestWhoamiTenantSurvivesFieldsProjection drives the exact guardrail an +// agent runs before a write. `--fields tenant` must yield the tenant and exit +// 0 — if the key were missing, writeObject would classify it as a zero-match +// usage error instead. +func TestWhoamiTenantSurvivesFieldsProjection(t *testing.T) { + t.Setenv("C1I_URL", "") + stubWhoamiServer(t, http.StatusOK) + + orig := viper.GetString("fields") + viper.Set("fields", "tenant") + t.Cleanup(func() { viper.Set("fields", orig) }) + + out, err := runWhoami(t, []string{"--url", "https://acme.conductor.one"}) + if err != nil { + t.Fatalf("auth whoami --fields tenant: %v (output %q)", err, out) + } + var got map[string]any + if uerr := json.Unmarshal([]byte(out), &got); uerr != nil { + t.Fatalf("output is not JSON: %v (%q)", uerr, out) + } + if len(got) != 1 || got["tenant"] != "https://acme.conductor.one" { + t.Errorf("projected output = %v, want exactly {tenant: https://acme.conductor.one}", got) + } +} + +// TestWhoamiReportsNoTenantWhenUnauthenticated pins the fail-closed half of +// the contract: the tenant is only reported once the credentials are proven +// against it, so a 401 exits 3 with no tenant rather than naming a target the +// caller cannot actually reach. +func TestWhoamiReportsNoTenantWhenUnauthenticated(t *testing.T) { + t.Setenv("C1I_URL", "") + stubWhoamiServer(t, http.StatusUnauthorized) + + out, err := runWhoami(t, []string{"--url", "https://acme.conductor.one"}) + if err == nil { + t.Fatalf("expected an error for a 401 introspect, got nil (output %q)", out) + } + if got, want := exitCode(err), exitAuth; got != want { + t.Errorf("exitCode(%v) = %d, want %d (exitAuth)", err, got, want) + } + if strings.Contains(out, `"tenant"`) { + t.Errorf("stdout = %q, an unauthenticated whoami must not report a tenant", out) + } +} + +// TestURLSourceTokenIsStable guards the machine-readable identifiers against +// being reworded the way the human-facing urlSourceLabel strings can be: they +// are a parsed value, so a prose label must never leak into tenantSource. +func TestURLSourceTokenIsStable(t *testing.T) { + want := map[URLSource]string{ + URLSourceFlag: "flag", + URLSourceEnv: "env", + URLSourceConfig: "config", + URLSourceNone: "unknown", + } + for source, token := range want { + got := urlSourceToken(source) + if got != token { + t.Errorf("urlSourceToken(%d) = %q, want %q", source, got, token) + } + if strings.ContainsAny(got, " -~") { + t.Errorf("urlSourceToken(%d) = %q, want a bare identifier, not prose like urlSourceLabel's", source, got) + } + } +} diff --git a/cmd/docs_guide.go b/cmd/docs_guide.go index 1239942..e01b5c3 100644 --- a/cmd/docs_guide.go +++ b/cmd/docs_guide.go @@ -365,10 +365,11 @@ via toolset sync). ## Prerequisites - Pointed at the tenant you intend to change, and authenticated. This guide - creates objects, so confirm the target before you start — "auth whoami" - reports the identity but not the tenant: + creates objects, so confirm the target before you start. "auth whoami" + reports both the identity and the resolved tenant, the latter machine- + readably ("auth status" prints the same tenant as text): - c1i auth status + c1i auth whoami --fields tenant c1i auth whoami - At least one candidate owner already exists as a C1 user (owners are existing users, never created here — C1 users come from a connected diff --git a/cmd/root.go b/cmd/root.go index 8ad0fd5..afcceb5 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -155,18 +155,25 @@ func initConfig() { // GetBaseURL returns the configured base URL or exits with an error. Embedded // credentials are dropped with a warning to stderr rather than an error; a -// non-https scheme is an error. Delegates to GetBaseURLWithSource so a ParseURL +// non-https scheme is an error. Delegates to requireBaseURL so a ParseURL // error (e.g. a retired bare short name) is reported with the same // source-naming used everywhere else. func GetBaseURL() (string, error) { + baseURL, _, err := requireBaseURL() + return baseURL, err +} + +// requireBaseURL is GetBaseURL plus where the URL came from, for commands that +// report the tenant they resolved instead of only using it. +func requireBaseURL() (string, URLSource, error) { baseURL, source, err := GetBaseURLWithSource() if err != nil { - return "", err + return "", source, err } if source == URLSourceNone { - return "", &usageError{fmt.Errorf("url is required: set --url flag, C1I_URL env var, or url in ~%s.c1i.yaml", string(filepath.Separator))} + return "", source, &usageError{fmt.Errorf("url is required: set --url flag, C1I_URL env var, or url in ~%s.c1i.yaml", string(filepath.Separator))} } - return baseURL, nil + return baseURL, source, nil } // warnAboutURL prints any ParseURL warnings to stderr, one per line. @@ -217,6 +224,21 @@ func urlSourceLabel(source URLSource) string { } } +// urlSourceToken is urlSourceLabel's machine-readable twin: stable identifiers +// for JSON output (auth whoami's tenantSource), not prose that may be reworded. +func urlSourceToken(source URLSource) string { + switch source { + case URLSourceFlag: + return "flag" + case URLSourceEnv: + return "env" + case URLSourceConfig: + return "config" + default: + return "unknown" + } +} + // GetBaseURLWithSource returns the configured base URL and where it came // from, warning to stderr about anything ParseURL dropped. Looks up // the "url" flag on rootCmd.PersistentFlags() directly (not a passed-in From f94c66f7327b27c47e7a9b0239ae77fa97dee595 Mon Sep 17 00:00:00 2001 From: leet-c1 <264029741+leet-c1@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:49:25 +0000 Subject: [PATCH 2/4] fix: reject a null introspect body, and correct the shipped tenant docs Review of ab5c1c9 found three real problems. 1. `--verbose` panicked on a 200 whose body is `null`. That body unmarshals into a NIL map with no error, so `obj["tenant"] = baseURL` was an assignment to entry in a nil map -- a regression this branch introduced (the pre-image never wrote into the map). Non-verbose was quieter but no better: it printed `userId: null` and exited 0, i.e. a pre-write check reading "confirmed" off a response with no identity in it. Both modes now return a *nonJSONResponseError (exit 6), matching the other unusable-200 cases. Covered by TestWhoamiNullIntrospectBodyIsC1Failure; the suite had no degenerate-body case at all before. 2. `cmd/agents.md` -- go:embed'ed and shipped as `c1i docs agents`, the bootstrap doc for exactly this feature's audience -- still said whoami has "no tenant URL in its output". An agent reading that keeps sed-ing the stderr warning this change exists to replace. Corrected there and in README.md's whoami enumeration. 3. `--verbose` precedence for a payload-owned `tenant` key was undocumented. The client-resolved value must win in every mode or `--fields tenant` stops meaning one thing; that is now stated in `Long` and pinned by TestWhoamiVerboseTenantIsClientResolved. `tenantId` is untouched. Also corrects the CHANGELOG's claim about when the stderr tenant warning fires: only for a `~/.c1i.yaml` fall-through, never for `--url` OR `C1I_URL` (warnAboutURLSource returns early unless source == URLSourceConfig), so `C1I_URL` callers had no signal either. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 29 +++++++++----- README.md | 6 ++- cmd/agents.md | 19 +++++++-- cmd/auth_whoami.go | 10 ++++- cmd/auth_whoami_tenant_test.go | 73 ++++++++++++++++++++++++++++++++-- 5 files changed, 118 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6716791..2e483db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,16 +26,18 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). `tenant`, the base URL this invocation resolved, and `tenantSource`, where that URL came from (`flag`, `env`, or `config`). Before this, the resolved tenant was readable only from `auth status`'s prose or from the stderr - warning that fires *only* when `--url` was omitted -- so the signal - disappeared exactly when a caller did the right thing and passed `--url`, - and toolkits that gate writes on "confirm the tenant first" had to `sed` - that warning line. `c1i auth whoami --url --fields tenant` is now - the check. The keys are emitted only after the credentials are proven - against that tenant, so an auth failure still exits 3 with no tenant rather - than naming a target the caller cannot reach. `auth status`'s text output - is unchanged -- anything parsing it keeps working -- and no `--json` flag - was added to it, since a second JSON surface for the same fact is exactly - the duplication that drifts. + warning that fires *only* when the URL came from `~/.c1i.yaml` -- never for + `--url` or `C1I_URL` -- so no machine-readable signal existed on the two + explicit paths, and toolkits that gate writes on "confirm the tenant first" + had to `sed` that warning line. `c1i auth whoami --url --fields + tenant` is now the check. The keys are emitted only after the credentials + are proven against that tenant, so an auth failure still exits 3 with no + tenant rather than naming a target the caller cannot reach; `tenant` is + always the client-resolved URL, including under `--verbose`, so the check + reads the same in either mode. `auth status`'s text output is unchanged -- + anything parsing it keeps working -- and no `--json` flag was added to it, + since a second JSON surface for the same fact is exactly the duplication + that drifts. Docs updated: `cmd/agents.md`, `README.md`, `cmd/docs_guide.go`. - **`apps owners`, `apps add-owner`, and `apps remove-owner`.** `apps get`'s `appOwners` field was empty on every app checked, while `GET .../owners` @@ -162,6 +164,13 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). from the OpenAPI spec; it is published (`DELETE /api/v1/apps/{id}`, operation `c1.api.app.v1.Apps.Delete`), so that sentence is gone from the help too. +- **`auth whoami` no longer reports success on an empty introspect body.** A + 200 whose body is `null` unmarshals into a nil map without error, so the + command printed a summary of nulls (`userId: null`) and exited 0 -- a + pre-write check reading "confirmed" off a response carrying no identity at + all. It is now a `nonJSONResponseError` (exit 6, "C1 failed"), matching the + other unusable-200 cases, in both plain and `--verbose` output. + - **`apps set-owners` no longer claims new owners appear in `apps get`'s `appOwners` field.** Measured against a live tenant, `appOwners` was empty on every app checked -- all 47, spanning 45 connector-managed apps across diff --git a/README.md b/README.md index 7efe1da..1a98e42 100644 --- a/README.md +++ b/README.md @@ -692,9 +692,13 @@ c1i auth login --client-id --client-secret # Check credential status (also reports the storage backend) c1i auth status -# Show the authenticated principal: user ID, display name, email, role/permission/feature counts +# Show the authenticated principal (user ID, display name, email, role/permission/feature +# counts) plus the resolved tenant: "tenant" (base URL) and "tenantSource" (flag/env/config) c1i auth whoami # add --verbose for full roles/permissions/features arrays +# Machine-readable "which tenant am I about to write to?" — the pre-write check +c1i auth whoami --url https://mycompany.conductor.one --fields tenant + # Mint a short-lived bearer token for driving raw API calls yourself c1i auth token # add --json for token type and absolute expiry (RFC3339) diff --git a/cmd/agents.md b/cmd/agents.md index c180f27..1aece79 100644 --- a/cmd/agents.md +++ b/cmd/agents.md @@ -40,10 +40,21 @@ subcommands that also take an external server's address — that one is Credentials resolve in this order: `C1I_CLIENT_ID` + `C1I_CLIENT_SECRET` env vars (read-only — c1i never writes them), the OS keyring, then a `0600` file used automatically where no keyring exists (headless Linux, CI, containers). -`c1i auth login` to authenticate; `c1i auth status` to confirm which tenant -you're pointed at (it prints the base URL) and `c1i auth whoami` to confirm -which identity you're acting as (userId, principleId, email, displayName — -no tenant URL in its output) before doing anything else. +`c1i auth login` to authenticate; then `c1i auth whoami` before doing +anything else — it reports both the identity you're acting as (userId, +principleId, email, displayName) and the tenant you're pointed at, the +latter machine-readably: `tenant` is the resolved base URL and +`tenantSource` is where that URL came from (`flag`, `env`, or `config`, the +last meaning it fell through to `~/.c1i.yaml`). Gate any write on it: + +```sh +c1i auth whoami --url https://mycompany.conductor.one --fields tenant +``` + +Both keys are emitted only once the credentials are proven against that +tenant, so a failure exits nonzero with no tenant rather than naming a +target you can't reach. `c1i auth status` prints the same tenant as plain +text, plus which credential store served it. ## Global flags diff --git a/cmd/auth_whoami.go b/cmd/auth_whoami.go index f295207..26fa4a7 100644 --- a/cmd/auth_whoami.go +++ b/cmd/auth_whoami.go @@ -24,7 +24,10 @@ form of the tenant "auth status" prints as text — check it before a write: Both keys report where a request WOULD go; they are only emitted once the credentials are proven against that tenant, so an auth failure exits nonzero -with no tenant rather than reporting an unusable target. +with no tenant rather than reporting an unusable target. "tenant" always +means the client-resolved URL, including under --verbose: if the introspect +payload ever carries a key of that name, this one wins, so the check reads +the same in either mode. The payload's own "tenantId" is untouched. The full introspect payload can include hundreds of roles and over a thousand permissions — pass --verbose to dump it all.`, @@ -47,6 +50,11 @@ thousand permissions — pass --verbose to dump it all.`, if err := json.Unmarshal(body, &payload); err != nil { return fmt.Errorf("parsing introspect response: %w", err) } + // A `null` body unmarshals into a nil map with no error: a 200 carrying + // no identity at all, which must not read as a confirmed tenant. + if payload == nil { + return &nonJSONResponseError{fmt.Errorf("introspect returned a JSON null body")} + } // Best-effort: enrich with display_name + email from /api/v1/users/{id}. // If that call fails (permissions, network), we still return the diff --git a/cmd/auth_whoami_tenant_test.go b/cmd/auth_whoami_tenant_test.go index daf18c4..92f1dc5 100644 --- a/cmd/auth_whoami_tenant_test.go +++ b/cmd/auth_whoami_tenant_test.go @@ -14,10 +14,21 @@ import ( "github.com/spf13/viper" ) -// stubWhoamiServer answers introspect (and the follow-up user lookup) with a -// fixed payload, and points newWhoamiClient at it. status is the code the +// defaultIntrospectBody is the shape the live endpoint returns: no "tenant" +// key of its own, but a "tenantId" that must survive untouched. +const defaultIntrospectBody = `{"userId":"u1","principleId":"p1","tenantId":"t1","roles":["r"],"permissions":[],"features":[]}` + +// stubWhoamiServer answers introspect (and the follow-up user lookup) with the +// default payload, and points newWhoamiClient at it. status is the code the // introspect call answers with. func stubWhoamiServer(t *testing.T, status int) { + t.Helper() + stubWhoamiServerBody(t, status, defaultIntrospectBody) +} + +// stubWhoamiServerBody is stubWhoamiServer with the introspect body chosen by +// the caller, for the degenerate bodies a 200 can still carry. +func stubWhoamiServerBody(t *testing.T, status int, introspectBody string) { t.Helper() srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -30,7 +41,7 @@ func stubWhoamiServer(t *testing.T, status int) { _, _ = w.Write([]byte(`{"message":"denied"}`)) return } - _, _ = w.Write([]byte(`{"userId":"u1","principleId":"p1","tenantId":"t1","roles":["r"],"permissions":[],"features":[]}`)) + _, _ = w.Write([]byte(introspectBody)) })) t.Cleanup(srv.Close) @@ -198,6 +209,62 @@ func TestWhoamiReportsNoTenantWhenUnauthenticated(t *testing.T) { } } +// TestWhoamiNullIntrospectBodyIsC1Failure covers the degenerate body a 200 can +// still carry: `null` unmarshals into a map[string]any without error, leaving +// the payload a NIL map. Writing the tenant into it would panic (assignment to +// entry in nil map), and printing it would be a bare "null" with exit 0 — a +// guardrail reporting success on a body carrying no identity at all. It is the +// remote failing its JSON contract, so it belongs in exitServer with the other +// unusable 200s, in both output modes. +func TestWhoamiNullIntrospectBodyIsC1Failure(t *testing.T) { + for _, args := range [][]string{ + {"--url", "https://acme.conductor.one"}, + {"--url", "https://acme.conductor.one", "--verbose"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + t.Setenv("C1I_URL", "") + stubWhoamiServerBody(t, http.StatusOK, `null`) + + out, err := runWhoami(t, args) + if err == nil { + t.Fatalf("expected an error for a null introspect body, got nil (output %q)", out) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(%v) = %d, want %d (exitServer)", err, got, want) + } + if strings.Contains(out, `"tenant"`) || strings.Contains(out, "null") { + t.Errorf("stdout = %q, an unusable introspect body must not print a tenant or a bare null", out) + } + }) + } +} + +// TestWhoamiVerboseTenantIsClientResolved pins the documented precedence: if +// the payload ever grows a "tenant" key of its own, the client-resolved base +// URL still wins, because `--fields tenant` is a pre-write guardrail and must +// mean the same thing in every mode. The server's value would be a different +// fact under the same name; tenantId (a real payload key) is unaffected. +func TestWhoamiVerboseTenantIsClientResolved(t *testing.T) { + t.Setenv("C1I_URL", "") + stubWhoamiServerBody(t, http.StatusOK, + `{"userId":"u1","principleId":"p1","tenantId":"t1","tenant":{"name":"acme"},"roles":[],"permissions":[],"features":[]}`) + + out, err := runWhoami(t, []string{"--url", "https://acme.conductor.one", "--verbose"}) + if err != nil { + t.Fatalf("auth whoami --verbose: %v (output %q)", err, out) + } + var got map[string]any + if uerr := json.Unmarshal([]byte(out), &got); uerr != nil { + t.Fatalf("output is not JSON: %v (%q)", uerr, out) + } + if got["tenant"] != "https://acme.conductor.one" { + t.Errorf("tenant = %v, want the client-resolved base URL to win", got["tenant"]) + } + if got["tenantId"] != "t1" { + t.Errorf("tenantId = %v, want t1", got["tenantId"]) + } +} + // TestURLSourceTokenIsStable guards the machine-readable identifiers against // being reworded the way the human-facing urlSourceLabel strings can be: they // are a parsed value, so a prose label must never leak into tenantSource. From 795c62b32a8b5bbaa1534947cb329f67554bff7f Mon Sep 17 00:00:00 2001 From: leet-c1 <264029741+leet-c1@users.noreply.github.com> Date: Thu, 27 Aug 2026 00:02:10 +0000 Subject: [PATCH 3/4] fix: key the whoami guard on a usable identity, not on a non-nil map The previous guard rejected `null` and nothing else, so `{}` -- literally "the empty introspect body" the CHANGELOG headline claimed to have fixed -- still printed a tenant beside a null identity and exited 0, byte-identical to the pre-fix `null` output. Under --verbose it was worse: `{}` printed the two tenant keys and nothing else, a "confirmed" target backed by no identity at all. The invariant this feature rests on is that the credentials proved something against the tenant, so the guard now asks for a usable identity (hasIdentity) rather than a non-nil map. Either id satisfies it: a service principal can carry principleId with no userId, and rejecting that would break callers whoami works for today -- a worse failure than the degenerate body being caught. I could not obtain service-principal credentials to confirm that shape live, so the guard is deliberately permissive in that direction. The neighbouring unmarshal failure is now a *nonJSONResponseError too. A truncated, empty, array, or scalar body on a 200 was exiting 1 through a bare fmt.Errorf, one statement above an exit-6 classification for `null` -- the pair now agrees, and matches CLAUDE.md's "a 200 with a non-JSON body" rule. Docs: cmd/agents.md and README.md no longer promise email/displayName unhedged (they come from a best-effort secondary lookup and vanish under --verbose, which is a different projection, not a superset), the whoami help says the same, and the precedence note in Long now covers tenantSource as well as tenant. The CHANGELOG headline was rewritten to match what the code actually rejects, and the test comment that mis-stated the plain-mode symptom was corrected. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 17 +++--- README.md | 10 ++-- cmd/agents.md | 8 +-- cmd/auth_whoami.go | 41 +++++++++++---- cmd/auth_whoami_tenant_test.go | 96 +++++++++++++++++++++++++--------- 5 files changed, 123 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e483db..c224b27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,12 +164,17 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). from the OpenAPI spec; it is published (`DELETE /api/v1/apps/{id}`, operation `c1.api.app.v1.Apps.Delete`), so that sentence is gone from the help too. -- **`auth whoami` no longer reports success on an empty introspect body.** A - 200 whose body is `null` unmarshals into a nil map without error, so the - command printed a summary of nulls (`userId: null`) and exited 0 -- a - pre-write check reading "confirmed" off a response carrying no identity at - all. It is now a `nonJSONResponseError` (exit 6, "C1 failed"), matching the - other unusable-200 cases, in both plain and `--verbose` output. +- **`auth whoami` no longer reports success on a 200 that carries no usable + identity.** `null`, `{}`, and `{"userId":null,"principleId":null}` all + decode without error, so the command printed a summary of nulls (`userId: + null`) -- or, under `--verbose`, nothing but the tenant keys -- and exited + 0: a pre-write check reading "confirmed" off a response that proves + nothing. All three are now a `nonJSONResponseError` (exit 6, "C1 failed"), + in both output modes, and so is a body that isn't a JSON object at all (a + truncated, empty, array, or scalar body), which previously fell through a + bare `fmt.Errorf` to the generic exit 1. The check is deliberately + permissive about *which* id is present -- either `userId` or `principleId` + is enough, since a service principal can carry `principleId` alone. - **`apps set-owners` no longer claims new owners appear in `apps get`'s `appOwners` field.** Measured against a live tenant, `appOwners` was empty diff --git a/README.md b/README.md index 1a98e42..4bc0d54 100644 --- a/README.md +++ b/README.md @@ -692,9 +692,13 @@ c1i auth login --client-id --client-secret # Check credential status (also reports the storage backend) c1i auth status -# Show the authenticated principal (user ID, display name, email, role/permission/feature -# counts) plus the resolved tenant: "tenant" (base URL) and "tenantSource" (flag/env/config) -c1i auth whoami # add --verbose for full roles/permissions/features arrays +# Show the authenticated principal (principle/user ID, role/permission/feature counts, and +# display name + email when a best-effort secondary lookup succeeds) plus the resolved +# tenant: "tenant" (base URL) and "tenantSource" (flag/env/config) +c1i auth whoami +# --verbose swaps the summary for the raw introspect payload (full roles/permissions/ +# features arrays, but no display name or email) -- a different projection, not a superset +c1i auth whoami --verbose # Machine-readable "which tenant am I about to write to?" — the pre-write check c1i auth whoami --url https://mycompany.conductor.one --fields tenant diff --git a/cmd/agents.md b/cmd/agents.md index 1aece79..a81e99c 100644 --- a/cmd/agents.md +++ b/cmd/agents.md @@ -41,9 +41,11 @@ Credentials resolve in this order: `C1I_CLIENT_ID` + `C1I_CLIENT_SECRET` env vars (read-only — c1i never writes them), the OS keyring, then a `0600` file used automatically where no keyring exists (headless Linux, CI, containers). `c1i auth login` to authenticate; then `c1i auth whoami` before doing -anything else — it reports both the identity you're acting as (userId, -principleId, email, displayName) and the tenant you're pointed at, the -latter machine-readably: `tenant` is the resolved base URL and +anything else — it reports both the identity you're acting as (principleId, +plus userId when the principal has one; email and displayName only when a +best-effort secondary lookup succeeds, and never under `--verbose`, which is +a different projection, not a superset) and the tenant you're pointed at, +the latter machine-readably: `tenant` is the resolved base URL and `tenantSource` is where that URL came from (`flag`, `env`, or `config`, the last meaning it fell through to `~/.c1i.yaml`). Gate any write on it: diff --git a/cmd/auth_whoami.go b/cmd/auth_whoami.go index 26fa4a7..cbe4660 100644 --- a/cmd/auth_whoami.go +++ b/cmd/auth_whoami.go @@ -12,8 +12,11 @@ var authWhoamiCmd = &cobra.Command{ Use: "whoami", Short: "Show the authenticated principal and the tenant being targeted", Long: `Calls /api/v1/auth/introspect and returns a compact summary of the -authenticated principal: userId, principleId, and counts of roles, -permissions, and feature flags. +authenticated principal: principleId, userId (a service principal may carry +only the former), and counts of roles, permissions, and feature flags -- plus +displayName and email when the secondary /api/v1/users/{id} lookup they come +from succeeds, omitted when it does not. --verbose is not a superset of that +summary: it replaces it with the raw introspect payload, which has neither. Two client-resolved keys are added to that summary, and to --verbose: "tenant" is the base URL this invocation resolved, and "tenantSource" is @@ -24,10 +27,10 @@ form of the tenant "auth status" prints as text — check it before a write: Both keys report where a request WOULD go; they are only emitted once the credentials are proven against that tenant, so an auth failure exits nonzero -with no tenant rather than reporting an unusable target. "tenant" always -means the client-resolved URL, including under --verbose: if the introspect -payload ever carries a key of that name, this one wins, so the check reads -the same in either mode. The payload's own "tenantId" is untouched. +with no tenant rather than reporting an unusable target. Both keys always +hold the client-resolved values, including under --verbose: if the introspect +payload ever carries a key of either name, these win, so the check reads the +same in either mode. The payload's own "tenantId" is untouched. The full introspect payload can include hundreds of roles and over a thousand permissions — pass --verbose to dump it all.`, @@ -48,12 +51,15 @@ thousand permissions — pass --verbose to dump it all.`, verbose, _ := cmd.Flags().GetBool("verbose") var payload map[string]any if err := json.Unmarshal(body, &payload); err != nil { - return fmt.Errorf("parsing introspect response: %w", err) + // A 200 whose body isn't a JSON object is C1 failing its contract, + // not a usage error: exitServer, like the guard below. + return &nonJSONResponseError{fmt.Errorf("parsing introspect response: %w", err)} } - // A `null` body unmarshals into a nil map with no error: a 200 carrying - // no identity at all, which must not read as a confirmed tenant. - if payload == nil { - return &nonJSONResponseError{fmt.Errorf("introspect returned a JSON null body")} + // A 200 carrying no identity proves nothing about the tenant, so it must + // not read as a confirmed target -- `null` unmarshals into a nil map and + // `{}` into an empty one, both without error. + if !hasIdentity(payload) { + return &nonJSONResponseError{fmt.Errorf("introspect returned no usable identity (neither userId nor principleId)")} } // Best-effort: enrich with display_name + email from /api/v1/users/{id}. @@ -118,6 +124,19 @@ func summarize(p map[string]any, displayName, email string) map[string]any { // and newAPIClient (cmd/api.go). var newWhoamiClient = newClient +// hasIdentity reports whether an introspect payload identifies anyone at all. +// EITHER id is enough: a service principal can legitimately carry principleId +// with no userId, so requiring userId would break whoami for it -- a worse +// failure than the degenerate body this rejects. +func hasIdentity(p map[string]any) bool { + for _, k := range []string{"userId", "principleId"} { + if s, ok := p[k].(string); ok && s != "" { + return true + } + } + return false +} + func sliceLen(v any) int { if s, ok := v.([]any); ok { return len(s) diff --git a/cmd/auth_whoami_tenant_test.go b/cmd/auth_whoami_tenant_test.go index 92f1dc5..a086bd2 100644 --- a/cmd/auth_whoami_tenant_test.go +++ b/cmd/auth_whoami_tenant_test.go @@ -209,33 +209,77 @@ func TestWhoamiReportsNoTenantWhenUnauthenticated(t *testing.T) { } } -// TestWhoamiNullIntrospectBodyIsC1Failure covers the degenerate body a 200 can -// still carry: `null` unmarshals into a map[string]any without error, leaving -// the payload a NIL map. Writing the tenant into it would panic (assignment to -// entry in nil map), and printing it would be a bare "null" with exit 0 — a -// guardrail reporting success on a body carrying no identity at all. It is the -// remote failing its JSON contract, so it belongs in exitServer with the other -// unusable 200s, in both output modes. -func TestWhoamiNullIntrospectBodyIsC1Failure(t *testing.T) { - for _, args := range [][]string{ - {"--url", "https://acme.conductor.one"}, - {"--url", "https://acme.conductor.one", "--verbose"}, - } { - t.Run(strings.Join(args, " "), func(t *testing.T) { - t.Setenv("C1I_URL", "") - stubWhoamiServerBody(t, http.StatusOK, `null`) +// TestWhoamiUnusableIntrospectBodyIsC1Failure walks the degenerate bodies a +// 200 can still carry. The invariant is a usable identity, not a non-nil map: +// `null` leaves the payload nil (writing the tenant into it panics), `{}` +// leaves it empty and non-nil, and an all-null identity leaves the keys +// present but useless — all three produced, or would have produced, a tenant +// reported next to a null identity with exit 0, which is a guardrail +// confirming a target off a response that proves nothing. A body that isn't a +// JSON object at all belongs in the same bucket rather than the generic exit +// 1 a bare fmt.Errorf gives. All of it is the remote failing its JSON +// contract: exitServer, in both output modes. +func TestWhoamiUnusableIntrospectBodyIsC1Failure(t *testing.T) { + bodies := map[string]string{ + "null": `null`, + "empty object": `{}`, + "null identity": `{"userId":null,"principleId":null,"roles":[]}`, + "empty identity": `{"userId":"","principleId":""}`, + "json array": `[]`, + "json scalar": `0`, + "truncated": `{"userId":"u1"`, + "empty body": ``, + "whitespace body": " \n", + } + for name, body := range bodies { + for _, args := range [][]string{ + {"--url", "https://acme.conductor.one"}, + {"--url", "https://acme.conductor.one", "--verbose"}, + } { + t.Run(name+" "+strings.Join(args, " "), func(t *testing.T) { + t.Setenv("C1I_URL", "") + stubWhoamiServerBody(t, http.StatusOK, body) - out, err := runWhoami(t, args) - if err == nil { - t.Fatalf("expected an error for a null introspect body, got nil (output %q)", out) - } - if got, want := exitCode(err), exitServer; got != want { - t.Errorf("exitCode(%v) = %d, want %d (exitServer)", err, got, want) - } - if strings.Contains(out, `"tenant"`) || strings.Contains(out, "null") { - t.Errorf("stdout = %q, an unusable introspect body must not print a tenant or a bare null", out) - } - }) + out, err := runWhoami(t, args) + if err == nil { + t.Fatalf("expected an error for introspect body %q, got nil (output %q)", body, out) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("body %q: exitCode(%v) = %d, want %d (exitServer)", body, err, got, want) + } + if strings.Contains(out, `"tenant"`) { + t.Errorf("stdout = %q, an unusable introspect body must not report a tenant", out) + } + }) + } + } +} + +// TestWhoamiServicePrincipalWithoutUserIDSucceeds is the other side of that +// guard, and the more dangerous direction to get wrong: a principal with a +// principleId but no userId is a legitimate caller (a service principal), and +// rejecting it would break whoami for callers it works for today — a worse +// failure than the degenerate body the guard exists to catch. The identity +// enrichment is skipped without a userId, so displayName/email are absent; +// the tenant keys must still be reported. +func TestWhoamiServicePrincipalWithoutUserIDSucceeds(t *testing.T) { + t.Setenv("C1I_URL", "") + stubWhoamiServerBody(t, http.StatusOK, + `{"principleId":"service:svc1","tenantId":"t1","roles":[],"permissions":[],"features":[]}`) + + out, err := runWhoami(t, []string{"--url", "https://acme.conductor.one"}) + if err != nil { + t.Fatalf("auth whoami for a service principal: %v (output %q)", err, out) + } + var got map[string]any + if uerr := json.Unmarshal([]byte(out), &got); uerr != nil { + t.Fatalf("output is not JSON: %v (%q)", uerr, out) + } + if got["principleId"] != "service:svc1" { + t.Errorf("principleId = %v, want service:svc1", got["principleId"]) + } + if got["tenant"] != "https://acme.conductor.one" { + t.Errorf("tenant = %v, want the resolved base URL", got["tenant"]) } } From fb691ce3a5bf1b66cadd639199130235bf92fc87 Mon Sep 17 00:00:00 2001 From: leet-c1 <264029741+leet-c1@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:37:57 +0000 Subject: [PATCH 4/4] fix: restore the dropped stdout assertion, and correct two contracts Three review findings, none of them behavioral. The degenerate-body test lost its `strings.Contains(out, "null")` clause when it grew from one body to nine. That was gratuitous -- the clause passes unchanged against the current guard -- and it was the only assertion covering the symptom the CHANGELOG names: a summary of nulls printed with no tenant keys in it, which is what a session-wide C1I_FIELDS would leave behind if a refactor ever printed the summary before returning the guard error. Restored, with a comment saying why it is not redundant, and mutation-checked: printing `{"userId": null}` ahead of the guard's return fails it while the tenant-only check still passes. The CHANGELOG body described a per-body `--verbose` symptom that held for only one of the three bodies it named -- `null` panicked on this branch before the round-2 fix, and the all-null body echoed its whole payload. It now states only the plain-mode symptom, which was measured for all three. nonJSONResponseError's doc comment defined it as marking a body that isn't valid JSON; whoami now also returns it for `{}` and an all-null identity, which are valid JSON. Widened to "a response the caller cannot use" -- the exitServer rationale below it already covered this case. Also, per the review's optional note: the whoami help said displayName and email are omitted when the secondary lookup fails; they are equally omitted when it succeeds with empty strings, and it is skipped entirely when there is no userId to look up. Says so now. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 10 +++++----- cmd/auth_whoami.go | 3 ++- cmd/auth_whoami_tenant_test.go | 9 +++++++-- cmd/errors.go | 14 ++++++++------ 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c224b27..2eac57c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -166,11 +166,11 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - **`auth whoami` no longer reports success on a 200 that carries no usable identity.** `null`, `{}`, and `{"userId":null,"principleId":null}` all - decode without error, so the command printed a summary of nulls (`userId: - null`) -- or, under `--verbose`, nothing but the tenant keys -- and exited - 0: a pre-write check reading "confirmed" off a response that proves - nothing. All three are now a `nonJSONResponseError` (exit 6, "C1 failed"), - in both output modes, and so is a body that isn't a JSON object at all (a + decode without error, so each was accepted as a valid response: the summary + printed with its identity fields null (`userId: null`) and exited 0 -- a + pre-write check reading "confirmed" off a response that proves nothing. + All three are now a `nonJSONResponseError` (exit 6, "C1 failed") in both + output modes, and so is a body that isn't a JSON object at all (a truncated, empty, array, or scalar body), which previously fell through a bare `fmt.Errorf` to the generic exit 1. The check is deliberately permissive about *which* id is present -- either `userId` or `principleId` diff --git a/cmd/auth_whoami.go b/cmd/auth_whoami.go index cbe4660..ab17e28 100644 --- a/cmd/auth_whoami.go +++ b/cmd/auth_whoami.go @@ -15,7 +15,8 @@ var authWhoamiCmd = &cobra.Command{ authenticated principal: principleId, userId (a service principal may carry only the former), and counts of roles, permissions, and feature flags -- plus displayName and email when the secondary /api/v1/users/{id} lookup they come -from succeeds, omitted when it does not. --verbose is not a superset of that +from succeeds with values, omitted when it fails, returns them empty, or is +skipped for want of a userId to look up. --verbose is not a superset of that summary: it replaces it with the raw introspect payload, which has neither. Two client-resolved keys are added to that summary, and to --verbose: diff --git a/cmd/auth_whoami_tenant_test.go b/cmd/auth_whoami_tenant_test.go index a086bd2..0420151 100644 --- a/cmd/auth_whoami_tenant_test.go +++ b/cmd/auth_whoami_tenant_test.go @@ -247,8 +247,13 @@ func TestWhoamiUnusableIntrospectBodyIsC1Failure(t *testing.T) { if got, want := exitCode(err), exitServer; got != want { t.Errorf("body %q: exitCode(%v) = %d, want %d (exitServer)", body, err, got, want) } - if strings.Contains(out, `"tenant"`) { - t.Errorf("stdout = %q, an unusable introspect body must not report a tenant", out) + // The "null" half is not redundant with the tenant check: a + // refactor that printed the summary before returning the guard + // error would, under a session-wide C1I_FIELDS projecting the + // tenant keys away, leave a summary of nulls carrying no + // "tenant" substring at all. + if strings.Contains(out, `"tenant"`) || strings.Contains(out, "null") { + t.Errorf("stdout = %q, an unusable introspect body must print nothing: no tenant, and no summary of nulls", out) } }) } diff --git a/cmd/errors.go b/cmd/errors.go index 0a162f0..e207f05 100644 --- a/cmd/errors.go +++ b/cmd/errors.go @@ -52,12 +52,14 @@ type toolExecutionError struct{ err error } func (e *toolExecutionError) Error() string { return e.err.Error() } func (e *toolExecutionError) Unwrap() error { return e.err } -// nonJSONResponseError marks an HTTP-success response whose body isn't valid -// JSON (e.g. a path that escapes the API prefix and lands on a server that -// answers 200 with an HTML document). Maps to exitServer: the request itself -// was well-formed, so this isn't a usage error; it's the remote side failing -// to honor the JSON contract this CLI depends on, the same "remote responded -// but not usefully" bucket exitServer already covers for 5xx. +// nonJSONResponseError marks an HTTP-success response the caller cannot use: +// a body that isn't valid JSON (e.g. a path that escapes the API prefix and +// lands on a server answering 200 with an HTML document), or one that is valid +// JSON but carries none of what the endpoint promises (e.g. an introspect 200 +// with no identity in it). Maps to exitServer: the request itself was +// well-formed, so this isn't a usage error; it's the remote side failing to +// honor the contract this CLI depends on, the same "remote responded but not +// usefully" bucket exitServer already covers for 5xx. type nonJSONResponseError struct{ err error } func (e *nonJSONResponseError) Error() string { return e.err.Error() }