follow-up: align ingress, atomic paths, and channel tests with credential semantics (#33733)

Merged via squash.

Prepared head SHA: c290c2ab6a
Co-authored-by: joshavant <830519+joshavant@users.noreply.github.com>
Co-authored-by: joshavant <830519+joshavant@users.noreply.github.com>
Reviewed-by: @joshavant
This commit is contained in:
Josh Avant
2026-03-03 20:29:46 -06:00
committed by GitHub
parent 6842877b2e
commit 1c200ca7ae
36 changed files with 1130 additions and 219 deletions

View File

@@ -0,0 +1,77 @@
import { describe, expect, it } from "vitest";
import {
evaluateStoredCredentialEligibility,
resolveTokenExpiryState,
} from "./credential-state.js";
describe("resolveTokenExpiryState", () => {
const now = 1_700_000_000_000;
it("treats undefined as missing", () => {
expect(resolveTokenExpiryState(undefined, now)).toBe("missing");
});
it("treats non-finite and non-positive values as invalid_expires", () => {
expect(resolveTokenExpiryState(0, now)).toBe("invalid_expires");
expect(resolveTokenExpiryState(-1, now)).toBe("invalid_expires");
expect(resolveTokenExpiryState(Number.NaN, now)).toBe("invalid_expires");
expect(resolveTokenExpiryState(Number.POSITIVE_INFINITY, now)).toBe("invalid_expires");
});
it("returns expired when expires is in the past", () => {
expect(resolveTokenExpiryState(now - 1, now)).toBe("expired");
});
it("returns valid when expires is in the future", () => {
expect(resolveTokenExpiryState(now + 1, now)).toBe("valid");
});
});
describe("evaluateStoredCredentialEligibility", () => {
const now = 1_700_000_000_000;
it("marks api_key with keyRef as eligible", () => {
const result = evaluateStoredCredentialEligibility({
credential: {
type: "api_key",
provider: "anthropic",
keyRef: {
source: "env",
provider: "default",
id: "ANTHROPIC_API_KEY",
},
},
now,
});
expect(result).toEqual({ eligible: true, reasonCode: "ok" });
});
it("marks tokenRef with missing expires as eligible", () => {
const result = evaluateStoredCredentialEligibility({
credential: {
type: "token",
provider: "github-copilot",
tokenRef: {
source: "env",
provider: "default",
id: "GITHUB_TOKEN",
},
},
now,
});
expect(result).toEqual({ eligible: true, reasonCode: "ok" });
});
it("marks token with invalid expires as ineligible", () => {
const result = evaluateStoredCredentialEligibility({
credential: {
type: "token",
provider: "github-copilot",
token: "tok",
expires: 0,
},
now,
});
expect(result).toEqual({ eligible: false, reasonCode: "invalid_expires" });
});
});

View File

@@ -0,0 +1,74 @@
import { coerceSecretRef, normalizeSecretInputString } from "../../config/types.secrets.js";
import type { AuthProfileCredential } from "./types.js";
export type AuthCredentialReasonCode =
| "ok"
| "missing_credential"
| "invalid_expires"
| "expired"
| "unresolved_ref";
export type TokenExpiryState = "missing" | "valid" | "expired" | "invalid_expires";
export function resolveTokenExpiryState(expires: unknown, now = Date.now()): TokenExpiryState {
if (expires === undefined) {
return "missing";
}
if (typeof expires !== "number") {
return "invalid_expires";
}
if (!Number.isFinite(expires) || expires <= 0) {
return "invalid_expires";
}
return now >= expires ? "expired" : "valid";
}
function hasConfiguredSecretRef(value: unknown): boolean {
return coerceSecretRef(value) !== null;
}
function hasConfiguredSecretString(value: unknown): boolean {
return normalizeSecretInputString(value) !== undefined;
}
export function evaluateStoredCredentialEligibility(params: {
credential: AuthProfileCredential;
now?: number;
}): { eligible: boolean; reasonCode: AuthCredentialReasonCode } {
const now = params.now ?? Date.now();
const credential = params.credential;
if (credential.type === "api_key") {
const hasKey = hasConfiguredSecretString(credential.key);
const hasKeyRef = hasConfiguredSecretRef(credential.keyRef);
if (!hasKey && !hasKeyRef) {
return { eligible: false, reasonCode: "missing_credential" };
}
return { eligible: true, reasonCode: "ok" };
}
if (credential.type === "token") {
const hasToken = hasConfiguredSecretString(credential.token);
const hasTokenRef = hasConfiguredSecretRef(credential.tokenRef);
if (!hasToken && !hasTokenRef) {
return { eligible: false, reasonCode: "missing_credential" };
}
const expiryState = resolveTokenExpiryState(credential.expires, now);
if (expiryState === "invalid_expires") {
return { eligible: false, reasonCode: "invalid_expires" };
}
if (expiryState === "expired") {
return { eligible: false, reasonCode: "expired" };
}
return { eligible: true, reasonCode: "ok" };
}
if (
normalizeSecretInputString(credential.access) === undefined &&
normalizeSecretInputString(credential.refresh) === undefined
) {
return { eligible: false, reasonCode: "missing_credential" };
}
return { eligible: true, reasonCode: "ok" };
}

View File

@@ -16,7 +16,7 @@ function cfgFor(profileId: string, provider: string, mode: "api_key" | "token" |
function tokenStore(params: {
profileId: string;
provider: string;
token: string;
token?: string;
expires?: number;
}): AuthProfileStore {
return {
@@ -132,6 +132,45 @@ describe("resolveApiKeyForProfile config compatibility", () => {
});
describe("resolveApiKeyForProfile token expiry handling", () => {
it("accepts token credentials when expires is undefined", async () => {
const profileId = "anthropic:token-no-expiry";
const result = await resolveWithConfig({
profileId,
provider: "anthropic",
mode: "token",
store: tokenStore({
profileId,
provider: "anthropic",
token: "tok-123",
}),
});
expect(result).toEqual({
apiKey: "tok-123",
provider: "anthropic",
email: undefined,
});
});
it("accepts token credentials when expires is in the future", async () => {
const profileId = "anthropic:token-valid-expiry";
const result = await resolveWithConfig({
profileId,
provider: "anthropic",
mode: "token",
store: tokenStore({
profileId,
provider: "anthropic",
token: "tok-123",
expires: Date.now() + 60_000,
}),
});
expect(result).toEqual({
apiKey: "tok-123",
provider: "anthropic",
email: undefined,
});
});
it("returns null for expired token credentials", async () => {
const profileId = "anthropic:token-expired";
const result = await resolveWithConfig({
@@ -148,7 +187,7 @@ describe("resolveApiKeyForProfile token expiry handling", () => {
expect(result).toBeNull();
});
it("accepts token credentials when expires is 0", async () => {
it("returns null for token credentials when expires is 0", async () => {
const profileId = "anthropic:token-no-expiry";
const result = await resolveWithConfig({
profileId,
@@ -161,11 +200,30 @@ describe("resolveApiKeyForProfile token expiry handling", () => {
expires: 0,
}),
});
expect(result).toEqual({
apiKey: "tok-123",
expect(result).toBeNull();
});
it("returns null for token credentials when expires is invalid (NaN)", async () => {
const profileId = "anthropic:token-invalid-expiry";
const store = tokenStore({
profileId,
provider: "anthropic",
email: undefined,
token: "tok-123",
});
store.profiles[profileId] = {
...store.profiles[profileId],
type: "token",
provider: "anthropic",
token: "tok-123",
expires: Number.NaN,
};
const result = await resolveWithConfig({
profileId,
provider: "anthropic",
mode: "token",
store,
});
expect(result).toBeNull();
});
});
@@ -237,6 +295,39 @@ describe("resolveApiKeyForProfile secret refs", () => {
}
});
it("resolves token tokenRef without inline token when expires is absent", async () => {
const profileId = "github-copilot:no-inline-token";
const previous = process.env.GITHUB_TOKEN;
process.env.GITHUB_TOKEN = "gh-ref-token";
try {
const result = await resolveApiKeyForProfile({
cfg: cfgFor(profileId, "github-copilot", "token"),
store: {
version: 1,
profiles: {
[profileId]: {
type: "token",
provider: "github-copilot",
tokenRef: { source: "env", provider: "default", id: "GITHUB_TOKEN" },
},
},
},
profileId,
});
expect(result).toEqual({
apiKey: "gh-ref-token",
provider: "github-copilot",
email: undefined,
});
} finally {
if (previous === undefined) {
delete process.env.GITHUB_TOKEN;
} else {
process.env.GITHUB_TOKEN = previous;
}
}
});
it("resolves inline ${ENV} api_key values", async () => {
const profileId = "openai:inline-env";
const previous = process.env.OPENAI_API_KEY;

View File

@@ -11,6 +11,7 @@ import { refreshQwenPortalCredentials } from "../../providers/qwen-portal-oauth.
import { resolveSecretRefString, type SecretRefResolveCache } from "../../secrets/resolve.js";
import { refreshChutesTokens } from "../chutes-oauth.js";
import { AUTH_STORE_LOCK_OPTIONS, log } from "./constants.js";
import { resolveTokenExpiryState } from "./credential-state.js";
import { formatAuthDoctorHint } from "./doctor.js";
import { ensureAuthStoreFile, resolveAuthStorePath } from "./paths.js";
import { suggestOAuthProfileIdForLegacyDefault } from "./repair.js";
@@ -86,12 +87,6 @@ function buildOAuthProfileResult(params: {
});
}
function isExpiredCredential(expires: number | undefined): boolean {
return (
typeof expires === "number" && Number.isFinite(expires) && expires > 0 && Date.now() >= expires
);
}
type ResolveApiKeyForProfileParams = {
cfg?: OpenClawConfig;
store: AuthProfileStore;
@@ -332,6 +327,10 @@ export async function resolveApiKeyForProfile(
return buildApiKeyProfileResult({ apiKey: key, provider: cred.provider, email: cred.email });
}
if (cred.type === "token") {
const expiryState = resolveTokenExpiryState(cred.expires);
if (expiryState === "expired" || expiryState === "invalid_expires") {
return null;
}
const token = await resolveProfileSecretString({
profileId,
provider: cred.provider,
@@ -346,9 +345,6 @@ export async function resolveApiKeyForProfile(
if (!token) {
return null;
}
if (isExpiredCredential(cred.expires)) {
return null;
}
return buildApiKeyProfileResult({ apiKey: token, provider: cred.provider, email: cred.email });
}

View File

@@ -4,6 +4,10 @@ import {
normalizeProviderId,
normalizeProviderIdForAuth,
} from "../model-selection.js";
import {
evaluateStoredCredentialEligibility,
type AuthCredentialReasonCode,
} from "./credential-state.js";
import { dedupeProfileIds, listProfilesForProvider } from "./profiles.js";
import type { AuthProfileStore } from "./types.js";
import {
@@ -12,6 +16,54 @@ import {
resolveProfileUnusableUntil,
} from "./usage.js";
export type AuthProfileEligibilityReasonCode =
| AuthCredentialReasonCode
| "profile_missing"
| "provider_mismatch"
| "mode_mismatch";
export type AuthProfileEligibility = {
eligible: boolean;
reasonCode: AuthProfileEligibilityReasonCode;
};
export function resolveAuthProfileEligibility(params: {
cfg?: OpenClawConfig;
store: AuthProfileStore;
provider: string;
profileId: string;
now?: number;
}): AuthProfileEligibility {
const providerAuthKey = normalizeProviderIdForAuth(params.provider);
const cred = params.store.profiles[params.profileId];
if (!cred) {
return { eligible: false, reasonCode: "profile_missing" };
}
if (normalizeProviderIdForAuth(cred.provider) !== providerAuthKey) {
return { eligible: false, reasonCode: "provider_mismatch" };
}
const profileConfig = params.cfg?.auth?.profiles?.[params.profileId];
if (profileConfig) {
if (normalizeProviderIdForAuth(profileConfig.provider) !== providerAuthKey) {
return { eligible: false, reasonCode: "provider_mismatch" };
}
if (profileConfig.mode !== cred.type) {
const oauthCompatible = profileConfig.mode === "oauth" && cred.type === "token";
if (!oauthCompatible) {
return { eligible: false, reasonCode: "mode_mismatch" };
}
}
}
const credentialEligibility = evaluateStoredCredentialEligibility({
credential: cred,
now: params.now,
});
return {
eligible: credentialEligibility.eligible,
reasonCode: credentialEligibility.reasonCode,
};
}
export function resolveAuthProfileOrder(params: {
cfg?: OpenClawConfig;
store: AuthProfileStore;
@@ -42,48 +94,14 @@ export function resolveAuthProfileOrder(params: {
return [];
}
const isValidProfile = (profileId: string): boolean => {
const cred = store.profiles[profileId];
if (!cred) {
return false;
}
if (normalizeProviderIdForAuth(cred.provider) !== providerAuthKey) {
return false;
}
const profileConfig = cfg?.auth?.profiles?.[profileId];
if (profileConfig) {
if (normalizeProviderIdForAuth(profileConfig.provider) !== providerAuthKey) {
return false;
}
if (profileConfig.mode !== cred.type) {
const oauthCompatible = profileConfig.mode === "oauth" && cred.type === "token";
if (!oauthCompatible) {
return false;
}
}
}
if (cred.type === "api_key") {
return Boolean(cred.key?.trim());
}
if (cred.type === "token") {
if (!cred.token?.trim()) {
return false;
}
if (
typeof cred.expires === "number" &&
Number.isFinite(cred.expires) &&
cred.expires > 0 &&
now >= cred.expires
) {
return false;
}
return true;
}
if (cred.type === "oauth") {
return Boolean(cred.access?.trim() || cred.refresh?.trim());
}
return false;
};
const isValidProfile = (profileId: string): boolean =>
resolveAuthProfileEligibility({
cfg,
store,
provider: providerAuthKey,
profileId,
now,
}).eligible;
let filtered = baseOrder.filter(isValidProfile);
// Repair config/store profile-id drift from older onboarding flows:

View File

@@ -19,7 +19,7 @@ export type TokenCredential = {
*/
type: "token";
provider: string;
token: string;
token?: string;
tokenRef?: SecretRef;
/** Optional expiry timestamp (ms since epoch). */
expires?: number;