What happens
api.BaseURL maps an env name to a backend host with no error case
(internal/api/client.go:86):
switch strings.ToLower(env) {
case EnvDev: return "https://dev-api.tracebloc.io"
case EnvStg: return "https://stg-api.tracebloc.io"
default: return "https://api.tracebloc.io"
}
So every unrecognised value — a typo, an alias, a future env name, a plausible-looking
staging instead of stg — resolves to production. The doc comment says
"Unknown / empty → prod", so this was a choice rather than an oversight, which is why
this is a discussion issue and not a bug report.
The distinction being lost
"Unset" and "set to something I do not recognise" are different inputs, and they
currently produce the same host.
api.ResolveEnv defaulting to prod when nothing is configured is reasonable — no
config means the real thing. But it returns strings.ToLower(explicit) for whatever
it is given, and BaseURL then silently accepts it. A user who typed an env name
intended a specific backend; falling back to prod gives them the one place where
being wrong is most expensive.
Why the existing validator does not catch it
api.IsKnownEnv exists (client.go:115) and is correct. It has exactly one caller:
internal/cli/auth.go:63, inside login.
IsKnownEnv -> auth.go:63 (login) only
ResolveEnv -> auth.go:58, auth.go:314, client.go:136
client.go:136 returns api.ResolveEnv("") with no validation, and that value
reaches BaseURL. So the validator sits on the path a caller takes when it is
already being careful, and the silent default sits on the path everything else
takes. Any caller that configures a session without going through login inherits
the fallback.
That shape is the actual finding. A guard that only runs on the careful path is
close to no guard: the careless path is the one that needs it.
Suggested change, and the trade-off
Have BaseURL (or a new BaseURLFor) return an error for an env that is neither
empty nor known, and keep empty → prod. Concretely:
- empty / unset → prod, unchanged. This is the real default and should stay.
- known (
dev, stg, prod) → its host, unchanged.
- anything else → an error naming the value and listing the known set.
The cost is that BaseURL gains an error return and its callers have to handle it.
Given there is currently one non-test caller (client.go:140), that is small today
and only gets larger.
A cheaper half-measure, if the signature change is unwelcome: call IsKnownEnv
wherever a session is constructed rather than only in login. That closes the same
hole without touching BaseURL, at the cost of relying on every future
session-construction site remembering to call it — which is the property that failed
here in the first place.
Also worth a look: aliases
If names like staging / production are ones users plausibly type, an explicit
alias table mapping them to stg / prod would be friendlier than either an error
or a silent prod fallback. Rejecting a typo is good; rejecting a reasonable synonym
is a papercut. That is a product call, not a correctness one.
How this surfaced
A caller that wrote a CLI config directly, rather than running login, passed its own
environment name and every request went to prod with a non-prod token. The caller was
wrong to assume its name matched the CLI's vocabulary — that half is fixed on the
caller's side. This issue is about the other half: the CLI resolved an env name it did
not recognise to the most dangerous available default and said nothing.
No repro against a customer environment is needed: BaseURL("staging") returning
https://api.tracebloc.io is the whole of it.
What happens
api.BaseURLmaps an env name to a backend host with no error case(
internal/api/client.go:86):So every unrecognised value — a typo, an alias, a future env name, a plausible-looking
staginginstead ofstg— resolves to production. The doc comment says"Unknown / empty → prod", so this was a choice rather than an oversight, which is why
this is a discussion issue and not a bug report.
The distinction being lost
"Unset" and "set to something I do not recognise" are different inputs, and they
currently produce the same host.
api.ResolveEnvdefaulting to prod when nothing is configured is reasonable — noconfig means the real thing. But it returns
strings.ToLower(explicit)for whateverit is given, and
BaseURLthen silently accepts it. A user who typed an env nameintended a specific backend; falling back to prod gives them the one place where
being wrong is most expensive.
Why the existing validator does not catch it
api.IsKnownEnvexists (client.go:115) and is correct. It has exactly one caller:internal/cli/auth.go:63, inside login.client.go:136returnsapi.ResolveEnv("")with no validation, and that valuereaches
BaseURL. So the validator sits on the path a caller takes when it isalready being careful, and the silent default sits on the path everything else
takes. Any caller that configures a session without going through
logininheritsthe fallback.
That shape is the actual finding. A guard that only runs on the careful path is
close to no guard: the careless path is the one that needs it.
Suggested change, and the trade-off
Have
BaseURL(or a newBaseURLFor) return an error for an env that is neitherempty nor known, and keep empty → prod. Concretely:
dev,stg,prod) → its host, unchanged.The cost is that
BaseURLgains an error return and its callers have to handle it.Given there is currently one non-test caller (
client.go:140), that is small todayand only gets larger.
A cheaper half-measure, if the signature change is unwelcome: call
IsKnownEnvwherever a session is constructed rather than only in
login. That closes the samehole without touching
BaseURL, at the cost of relying on every futuresession-construction site remembering to call it — which is the property that failed
here in the first place.
Also worth a look: aliases
If names like
staging/productionare ones users plausibly type, an explicitalias table mapping them to
stg/prodwould be friendlier than either an erroror a silent prod fallback. Rejecting a typo is good; rejecting a reasonable synonym
is a papercut. That is a product call, not a correctness one.
How this surfaced
A caller that wrote a CLI config directly, rather than running
login, passed its ownenvironment name and every request went to prod with a non-prod token. The caller was
wrong to assume its name matched the CLI's vocabulary — that half is fixed on the
caller's side. This issue is about the other half: the CLI resolved an env name it did
not recognise to the most dangerous available default and said nothing.
No repro against a customer environment is needed:
BaseURL("staging")returninghttps://api.tracebloc.iois the whole of it.