Warn at load when two profiles share a tokenEnv but quarantine under different credential ids #411

Open
opened 2026-09-10 04:35:16 +02:00 by ltms · 0 comments
Owner

The defect this catches

credentialId defaults to the profile's own name (FleetConfig.java:765):

if (credentialId != null && !credentialId.isBlank()) return credentialId;
return isSubscription() ? SUBSCRIPTION_CREDENTIAL_ID : profile;

That default is right only when profile identity happens to equal credential identity. Nothing
warns when it does not, and it is wrong in both directions:

  • Under-quarantine. Two profiles share one token but get separate keys. An exhaustion seen by one
    leaves the other free, so the fleet walks straight onto the same dead backend. Silent — the
    operator sees a spawn succeed and then fail for a reason that looks unrelated.
  • Over-quarantine. Two profiles get one key when they are actually independent credentials, so
    one exhaustion locks out both. Loud but wasteful.

Measured on both fleets, same day

Found by the fleet01 lead, who checked their own host after I warned them about the opposite
direction. My warning was wrong for their host and right for nothing — the real defect there was
under-quarantine:

fleet01:  gx   opencode     tokenEnv AI_GATEWAY_TOKEN  -> key "gx"
          local claude-code tokenEnv AI_GATEWAY_TOKEN  -> key "local"

One AI_GATEWAY_TOKEN, one gateway, two paths on it, two quarantine keys.

I then ran the same check here with the production effectiveCredentialId() method rather than a
reimplementation of it, and this host had the identical defect:

profile        kind          tokenEnv              explicitCredId  EFFECTIVE
local          claude-code   AI_GATEWAY_TOKEN      null            local
gx             opencode      AI_GATEWAY_TOKEN      null            gx      <== same token, 2 keys

Fixed here by setting credentialId: ai-gateway on both. Two independent fleets, same misconfiguration,
neither operator noticed until someone wrote the check.

The goal

At load, report when two profiles name the same tokenEnv but resolve to different
effectiveCredentialId() values. That combination is almost certainly a mistake.

Warn, do not refuse

validateModels refuses to start, and that is right for a model allow-list — an unlisted model is a
typo or a withdrawn id, and there is no legitimate reason to boot with one.

This one must not refuse. Two profiles can legitimately share a tokenEnv and still be separate
credentials: the same variable name can hold different values in different environments, and an
operator may deliberately want independent cooldowns. A refusal would break working fleets on an
inference. Log it clearly, name both profiles and both keys, and let the operator decide — the same
shape as the exhaustedPattern gap reporter added in #395.

Acceptance

  • A load-time report naming each offending pair: both profile names, the shared tokenEnv name, and
    both resolved credential ids.
  • Never print the token's value. tokenEnv is a variable name and is safe to log; the value it
    holds is a credential and must not appear.
  • A config with no such pair produces no output at all — silence must mean "checked and clean", so
    the check has to run unconditionally rather than behind a flag.
  • Tests covering: a clean config, a same-token/different-key pair, a same-token/same-key pair (no
    warning — this is the fixed state), and profiles with no tokenEnv at all (subscription: true
    profiles and opencode free providers both have none, and neither should warn).
  • The report must state its own denominator — how many profiles were examined. A checker that cannot
    say what it looked at cannot be trusted when it says nothing; see #395 and #404.

Note for whoever picks this up

Two traps, both already paid for in this repo:

  1. Derive the key from effectiveCredentialId(), never from a copy of its logic. #404 was exactly
    a second implementation of a lookup that agreed with the real one until it did not. I wrote my
    audit script against the production method for this reason, after starting to reimplement it and
    stopping.
  2. This is a log-only reporter, which is the category with no test coverage today. #407 records
    five existing startup reporters that no test pins — deleting any of their bodies leaves the suite
    green. Do not add a sixth. Pin the output.

Related: #407 (unpinned log-only reporters), #395 (the reporter shape to copy), #404 (why not to
reimplement the lookup).

## The defect this catches `credentialId` defaults to the profile's own name (`FleetConfig.java:765`): ```java if (credentialId != null && !credentialId.isBlank()) return credentialId; return isSubscription() ? SUBSCRIPTION_CREDENTIAL_ID : profile; ``` That default is right only when profile identity happens to equal credential identity. Nothing warns when it does not, and **it is wrong in both directions**: - **Under-quarantine.** Two profiles share one token but get separate keys. An exhaustion seen by one leaves the other free, so the fleet walks straight onto the same dead backend. Silent — the operator sees a spawn succeed and then fail for a reason that looks unrelated. - **Over-quarantine.** Two profiles get one key when they are actually independent credentials, so one exhaustion locks out both. Loud but wasteful. ## Measured on both fleets, same day Found by the fleet01 lead, who checked their own host after I warned them about the *opposite* direction. My warning was wrong for their host and right for nothing — the real defect there was under-quarantine: ``` fleet01: gx opencode tokenEnv AI_GATEWAY_TOKEN -> key "gx" local claude-code tokenEnv AI_GATEWAY_TOKEN -> key "local" ``` One `AI_GATEWAY_TOKEN`, one gateway, two paths on it, two quarantine keys. I then ran the same check here with the production `effectiveCredentialId()` method rather than a reimplementation of it, and **this host had the identical defect**: ``` profile kind tokenEnv explicitCredId EFFECTIVE local claude-code AI_GATEWAY_TOKEN null local gx opencode AI_GATEWAY_TOKEN null gx <== same token, 2 keys ``` Fixed here by setting `credentialId: ai-gateway` on both. Two independent fleets, same misconfiguration, neither operator noticed until someone wrote the check. ## The goal At load, report when two profiles name the **same `tokenEnv`** but resolve to **different** `effectiveCredentialId()` values. That combination is almost certainly a mistake. ## Warn, do not refuse `validateModels` refuses to start, and that is right for a model allow-list — an unlisted model is a typo or a withdrawn id, and there is no legitimate reason to boot with one. **This one must not refuse.** Two profiles can legitimately share a `tokenEnv` and still be separate credentials: the same variable name can hold different values in different environments, and an operator may deliberately want independent cooldowns. A refusal would break working fleets on an inference. Log it clearly, name both profiles and both keys, and let the operator decide — the same shape as the `exhaustedPattern` gap reporter added in #395. ## Acceptance - A load-time report naming each offending pair: both profile names, the shared `tokenEnv` name, and both resolved credential ids. - **Never print the token's value.** `tokenEnv` is a variable *name* and is safe to log; the value it holds is a credential and must not appear. - A config with no such pair produces no output at all — silence must mean "checked and clean", so the check has to run unconditionally rather than behind a flag. - Tests covering: a clean config, a same-token/different-key pair, a same-token/same-key pair (no warning — this is the fixed state), and profiles with no `tokenEnv` at all (`subscription: true` profiles and opencode free providers both have none, and neither should warn). - The report must state its own denominator — how many profiles were examined. A checker that cannot say what it looked at cannot be trusted when it says nothing; see #395 and #404. ## Note for whoever picks this up Two traps, both already paid for in this repo: 1. **Derive the key from `effectiveCredentialId()`, never from a copy of its logic.** #404 was exactly a second implementation of a lookup that agreed with the real one until it did not. I wrote my audit script against the production method for this reason, after starting to reimplement it and stopping. 2. **This is a log-only reporter, which is the category with no test coverage today.** #407 records five existing startup reporters that no test pins — deleting any of their bodies leaves the suite green. Do not add a sixth. Pin the output. Related: #407 (unpinned log-only reporters), #395 (the reporter shape to copy), #404 (why not to reimplement the lookup).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#411