From bbdcf2963b98293eea386c294f4cdb03201fede4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 11 Apr 2026 01:50:05 +0100 Subject: [PATCH] refactor: reduce unsafe assertions in secrets --- src/secrets/apply.ts | 48 +++++++++++++------------------- src/secrets/channel-env-vars.ts | 2 +- src/secrets/json-pointer.ts | 18 +++++++----- src/secrets/provider-env-vars.ts | 2 +- src/secrets/resolve.ts | 10 ++++--- src/secrets/storage-scan.ts | 10 +++++-- 6 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/secrets/apply.ts b/src/secrets/apply.ts index 3dd95f6d2ef..9fa3152a16b 100644 --- a/src/secrets/apply.ts +++ b/src/secrets/apply.ts @@ -6,6 +6,7 @@ import { resolveAgentConfig } from "../agents/agent-scope.js"; import { loadAuthProfileStoreForSecretsRuntime } from "../agents/auth-profiles.js"; import { AUTH_STORE_VERSION } from "../agents/auth-profiles/constants.js"; import { resolveAuthStorePath } from "../agents/auth-profiles/paths.js"; +import { coercePersistedAuthProfileStore } from "../agents/auth-profiles/persisted.js"; import { normalizeProviderId } from "../agents/model-selection.js"; import { resolveStateDir, type OpenClawConfig } from "../config/config.js"; import type { ConfigWriteOptions } from "../config/io.js"; @@ -384,11 +385,13 @@ function scrubAuthStoresForProviderTargets(params: { if (!parsed || !isRecord(parsed.profiles)) { continue; } - const nextStore = structuredClone(parsed) as Record & { - profiles: Record; - }; + const nextStore = structuredClone(parsed); + const profiles = nextStore.profiles; + if (!isRecord(profiles)) { + continue; + } let mutated = false; - for (const profile of iterateAuthProfileCredentials(nextStore.profiles)) { + for (const profile of iterateAuthProfileCredentials(profiles)) { const provider = normalizeProviderId(profile.provider); if (!params.providerTargets.has(provider)) { continue; @@ -426,13 +429,11 @@ function ensureMutableAuthStore( store: Record | undefined, ): MutableAuthProfileStore { const next: Record = store ? structuredClone(store) : {}; - if (!isRecord(next.profiles)) { - next.profiles = {}; - } + const profiles = isRecord(next.profiles) ? next.profiles : {}; if (typeof next.version !== "number" || !Number.isFinite(next.version)) { next.version = AUTH_STORE_VERSION; } - return next as MutableAuthProfileStore; + return { ...next, profiles }; } function resolveAuthStoreForTarget(params: { @@ -457,10 +458,6 @@ function resolveAuthStoreForTarget(params: { return { path: authStorePath, store }; } -function asConfigPathRoot(store: MutableAuthProfileStore): OpenClawConfig { - return store as unknown as OpenClawConfig; -} - function resolveAuthStorePathForAgent(params: { nextConfig: OpenClawConfig; stateDir: string; @@ -507,7 +504,7 @@ function ensureAuthProfileContainer(params: { isNonEmptyString(params.target.authProfileProvider) ) { const wroteProvider = setPathCreateStrict( - asConfigPathRoot(params.store), + params.store, [...profilePathSegments, "provider"], params.target.authProfileProvider, ); @@ -526,7 +523,7 @@ function ensureAuthProfileContainer(params: { `Cannot create auth profile "${profileId}" for ${params.target.path} without authProfileProvider.`, ); } - const wroteProfile = setPathCreateStrict(asConfigPathRoot(params.store), profilePathSegments, { + const wroteProfile = setPathCreateStrict(params.store, profilePathSegments, { type: expectedType, provider, }); @@ -567,12 +564,8 @@ function applyAuthProfileTargetMutation(params: { if (!refPathSegments) { throw new Error(`Missing sibling ref path for auth-profiles target ${params.target.path}.`); } - const wroteRef = setPathCreateStrict( - asConfigPathRoot(store), - refPathSegments, - params.target.ref, - ); - const deletedPlaintext = deletePathStrict(asConfigPathRoot(store), targetPathSegments); + const wroteRef = setPathCreateStrict(store, refPathSegments, params.target.ref); + const deletedPlaintext = deletePathStrict(store, targetPathSegments); changed = changed || wroteRef || deletedPlaintext; return changed; } @@ -580,11 +573,7 @@ function applyAuthProfileTargetMutation(params: { if (isNonEmptyString(previous)) { params.scrubbedValues.add(previous.trim()); } - const wroteRef = setPathCreateStrict( - asConfigPathRoot(store), - targetPathSegments, - params.target.ref, - ); + const wroteRef = setPathCreateStrict(store, targetPathSegments, params.target.ref); changed = changed || wroteRef; return changed; } @@ -706,9 +695,12 @@ async function validateProjectedSecretsState(params: { const storePath = resolveUserPath(resolveAuthStorePath(agentDir)); const override = authStoreLookup.get(storePath); if (override) { - return structuredClone(override) as unknown as ReturnType< - typeof loadAuthProfileStoreForSecretsRuntime - >; + return ( + coercePersistedAuthProfileStore(structuredClone(override)) ?? { + version: AUTH_STORE_VERSION, + profiles: {}, + } + ); } return loadAuthProfileStoreForSecretsRuntime(agentDir); }, diff --git a/src/secrets/channel-env-vars.ts b/src/secrets/channel-env-vars.ts index d68b6d02c61..c0f18cb74f9 100644 --- a/src/secrets/channel-env-vars.ts +++ b/src/secrets/channel-env-vars.ts @@ -36,7 +36,7 @@ export function resolveChannelEnvVars( workspaceDir: params?.workspaceDir, env: params?.env, }); - const candidates: Record = Object.create(null) as Record; + const candidates: Record = {}; for (const plugin of registry.plugins) { if (!plugin.channelEnvVars) { continue; diff --git a/src/secrets/json-pointer.ts b/src/secrets/json-pointer.ts index c9761af61c5..6e70b1add6c 100644 --- a/src/secrets/json-pointer.ts +++ b/src/secrets/json-pointer.ts @@ -5,6 +5,10 @@ function failOrUndefined(params: { onMissing: "throw" | "undefined"; message: st return undefined; } +function isJsonObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + export function decodeJsonPointerToken(token: string): string { return token.replace(/~1/g, "/").replace(/~0/g, "~"); } @@ -45,20 +49,19 @@ export function readJsonPointer( current = current[index]; continue; } - if (typeof current !== "object" || current === null || Array.isArray(current)) { + if (!isJsonObject(current)) { return failOrUndefined({ onMissing, message: `JSON pointer segment "${token}" does not exist.`, }); } - const record = current as Record; - if (!Object.hasOwn(record, token)) { + if (!Object.hasOwn(current, token)) { return failOrUndefined({ onMissing, message: `JSON pointer segment "${token}" does not exist.`, }); } - current = record[token]; + current = current[token]; } return current; } @@ -86,9 +89,10 @@ export function setJsonPointer( return; } const child = current[token]; - if (typeof child !== "object" || child === null || Array.isArray(child)) { - current[token] = {}; + const next: Record = isJsonObject(child) ? child : {}; + if (next !== child) { + current[token] = next; } - current = current[token] as Record; + current = next; } } diff --git a/src/secrets/provider-env-vars.ts b/src/secrets/provider-env-vars.ts index 63c3e74c60b..316a65a20e6 100644 --- a/src/secrets/provider-env-vars.ts +++ b/src/secrets/provider-env-vars.ts @@ -51,7 +51,7 @@ function resolveManifestProviderAuthEnvVarCandidates( workspaceDir: params?.workspaceDir, env: params?.env, }); - const candidates: Record = Object.create(null) as Record; + const candidates: Record = {}; for (const plugin of registry.plugins) { if (!plugin.providerAuthEnvVars) { continue; diff --git a/src/secrets/resolve.ts b/src/secrets/resolve.ts index 808a18d0cb9..fe2e2eeb1e5 100644 --- a/src/secrets/resolve.ts +++ b/src/secrets/resolve.ts @@ -276,8 +276,9 @@ async function readFileProviderPayload(params: { }): Promise { const cacheKey = params.providerName; const cache = params.cache; - if (cache?.filePayloadByProvider?.has(cacheKey)) { - return await (cache.filePayloadByProvider.get(cacheKey) as Promise); + const cachedFilePayload = cache?.filePayloadByProvider?.get(cacheKey); + if (cachedFilePayload) { + return await cachedFilePayload; } const filePath = resolveUserPath(params.providerConfig.path); @@ -915,8 +916,9 @@ export async function resolveSecretRefValue( ): Promise { const cache = options.cache; const key = secretRefKey(ref); - if (cache?.resolvedByRefKey?.has(key)) { - return await (cache.resolvedByRefKey.get(key) as Promise); + const cachedResolvedValue = cache?.resolvedByRefKey?.get(key); + if (cachedResolvedValue) { + return await cachedResolvedValue; } const promise = (async () => { diff --git a/src/secrets/storage-scan.ts b/src/secrets/storage-scan.ts index d415a3e44f2..0be01b8ce0d 100644 --- a/src/secrets/storage-scan.ts +++ b/src/secrets/storage-scan.ts @@ -7,6 +7,10 @@ import { resolveUserPath } from "../utils.js"; import { listAuthProfileStorePaths as listAuthProfileStorePathsFromAuthStorePaths } from "./auth-store-paths.js"; import { parseEnvValue } from "./shared.js"; +function isJsonObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + export function parseEnvAssignmentValue(raw: string): string { return parseEnvValue(raw); } @@ -119,11 +123,11 @@ export function readJsonObjectIfExists( }; } const raw = fs.readFileSync(filePath, "utf8"); - const parsed = JSON.parse(raw) as unknown; - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + const parsed: unknown = JSON.parse(raw); + if (!isJsonObject(parsed)) { return { value: null }; } - return { value: parsed as Record }; + return { value: parsed }; } catch (err) { return { value: null,