From 22e892a4eaa07b2428d9599735af4bd56a32806a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 02:52:38 -0700 Subject: [PATCH] fix(secrets): preserve canonical configuration path identity (#128318) * fix(secrets): scope manifest-owned credentials * fix(secrets): isolate plugin owner domains * fix(secrets): preserve manifest target path identity * fix(config): preserve typed secret target paths * fix(secrets): preserve dotted header target paths * test(secrets): expect canonical auth profile paths * test(secrets): prove plugin owner authority chain * fix(secrets): preserve canonical configuration path identity * docs(plugins): clarify canonical secret paths and route ownership * fix(cli): keep configuration path parsing out of bootstrap dependencies --- docs/plugins/manifest.md | 2 + ...mmand-secret-targets.agent-runtime.test.ts | 39 ++- src/cli/command-secret-targets.import.test.ts | 5 +- src/cli/command-secret-targets.ts | 79 ++---- src/cli/config-cli-input.ts | 82 +++++- src/cli/config-cli-path.ts | 152 ++--------- src/cli/config-cli-runner.ts | 16 +- src/cli/config-cli.test.ts | 208 +++++++++++--- src/cli/config-cli.ts | 6 +- src/config/env-substitution.test.ts | 115 ++++++++ src/config/env-substitution.ts | 11 +- src/plugins/config-contract-matches.ts | 26 +- src/plugins/config-contracts.test.ts | 59 +++- src/secrets/apply.test.ts | 30 +- src/secrets/apply.ts | 16 +- src/secrets/exec-secret-ref-id-parity.test.ts | 4 +- src/secrets/path-utils.test.ts | 31 ++- src/secrets/path-utils.ts | 51 ++-- src/secrets/plan.ts | 32 ++- src/secrets/plugin-setup-plan.ts | 27 +- .../runtime-auth-refresh-failure.test.ts | 13 + ...-config-collectors-plugins.bundled.test.ts | 17 +- .../runtime-config-collectors-plugins.test.ts | 124 ++++++++- .../runtime-config-collectors-plugins.ts | 56 +--- src/secrets/runtime.coverage.test.ts | 6 +- src/secrets/runtime.ts | 2 +- ...get-registry-data.current-snapshot.test.ts | 253 ++++++++++++++++- src/secrets/target-registry-data.ts | 9 +- src/secrets/target-registry-pattern.test.ts | 78 +++++- src/secrets/target-registry-pattern.ts | 68 +++-- src/secrets/target-registry-query.ts | 63 +++-- src/secrets/target-registry-types.ts | 8 + src/secrets/target-registry.test.ts | 33 +++ src/shared/dot-path.ts | 257 ++++++++++++++++++ src/wizard/setup.plugin-config.test.ts | 81 +++++- src/wizard/setup.plugin-config.ts | 57 +++- 36 files changed, 1674 insertions(+), 442 deletions(-) diff --git a/docs/plugins/manifest.md b/docs/plugins/manifest.md index f0cdb99b97b0..a0e5684cd832 100644 --- a/docs/plugins/manifest.md +++ b/docs/plugins/manifest.md @@ -853,6 +853,8 @@ Each `dangerousFlags` entry supports: | `bundledDefaultEnabled` | No | `boolean` | Override bundled-plugin default enablement when deciding whether this SecretRef surface is active. Use this when the plugin is bundled but the surface should stay inactive until explicitly enabled in config. | | `paths` | Yes | `object[]` | Secret-shaped config paths, each with `path` (dot-separated, relative to `plugins.entries..config`, supports `*` wildcards), optional `expected` (currently only `"string"`), and optional `ownerKind` (currently only `"route"`). A declared owner isolates only that exact matched path when resolution fails; its owner id is the full config path. | +Concrete paths preserve literal record keys and array indices: `headers["X.Trace"]` remains distinct from `headers.X.Trace`, and record key `["0"]` remains distinct from array index `[0]`. Plugin IDs containing dots are quoted the same way, such as `plugins.entries["example.plugin"].config.headers["X.Trace"]`. + ## mediaUnderstandingProviderMetadata reference Use `mediaUnderstandingProviderMetadata` when a media-understanding provider has default models, auto-auth fallback priority, or native document support that generic core helpers need before runtime loads. Keys must also be declared in `contracts.mediaUnderstandingProviders`. diff --git a/src/cli/command-secret-targets.agent-runtime.test.ts b/src/cli/command-secret-targets.agent-runtime.test.ts index c2be3b30af37..e212aa3bdde3 100644 --- a/src/cli/command-secret-targets.agent-runtime.test.ts +++ b/src/cli/command-secret-targets.agent-runtime.test.ts @@ -3,11 +3,20 @@ import { describe, expect, it, vi } from "vitest"; const firecrawlPath = "plugins.entries.firecrawl.config.webFetch.apiKey"; const exaPath = "plugins.entries.exa.config.webSearch.apiKey"; +const dottedExaPath = 'plugins.entries["exa.internal"].config.webSearch.apiKey'; vi.mock("../secrets/target-registry.js", () => ({ listSecretTargetRegistryEntries: () => [ - { id: firecrawlPath }, - { id: exaPath }, + { id: firecrawlPath, pathPatternSegments: firecrawlPath.split(".") }, + { id: exaPath, pathPatternSegments: exaPath.split(".") }, + { + id: dottedExaPath, + pathPatternSegments: ["plugins", "entries", "exa.internal", "config", "webSearch", "apiKey"], + }, + { + id: "plugins.entries.example.config.accounts[].token", + pathPatternSegments: ["plugins", "entries", "example", "config", "accounts[]", "token"], + }, { id: "plugins.entries.example.config.other.apiKey" }, ], discoverConfigSecretTargetsByIds: ( @@ -21,6 +30,7 @@ vi.mock("../secrets/target-registry.js", () => ({ const ids = new Set(targetIds); const firecrawlValue = config.plugins?.entries?.firecrawl?.config?.webFetch?.apiKey; const exaValue = config.plugins?.entries?.exa?.config?.webSearch?.apiKey; + const dottedExaValue = config.plugins?.entries?.["exa.internal"]?.config?.webSearch?.apiKey; return [ ...(ids.has(firecrawlPath) && firecrawlValue !== undefined ? [ @@ -42,11 +52,22 @@ vi.mock("../secrets/target-registry.js", () => ({ }, ] : []), + ...(ids.has(dottedExaPath) && dottedExaValue !== undefined + ? [ + { + entry: { id: dottedExaPath }, + path: dottedExaPath, + pathSegments: ["plugins", "entries", "exa.internal", "config", "webSearch", "apiKey"], + value: dottedExaValue, + }, + ] + : []), ]; }, })); -const { getAgentRuntimeOptionalCommandSecretPaths } = await import("./command-secret-targets.js"); +const { getAgentRuntimeCommandSecretTargetIds, getAgentRuntimeOptionalCommandSecretPaths } = + await import("./command-secret-targets.js"); describe("agent runtime command secret targets", () => { it("marks only configured web SecretRefs optional for generic agent startup", () => { @@ -65,10 +86,20 @@ describe("agent runtime command secret targets", () => { webSearch: { apiKey: "inline-key" }, }, }, + "exa.internal": { + config: { + webSearch: { + apiKey: { source: "env", provider: "default", id: "INTERNAL_EXA_API_KEY" }, + }, + }, + }, }, }, } as never); - expect(paths).toEqual(new Set([firecrawlPath])); + expect(paths).toEqual(new Set([firecrawlPath, dottedExaPath])); + const runtimeTargetIds = getAgentRuntimeCommandSecretTargetIds(); + expect(runtimeTargetIds.has(firecrawlPath)).toBe(true); + expect(runtimeTargetIds.has(dottedExaPath)).toBe(true); }); }); diff --git a/src/cli/command-secret-targets.import.test.ts b/src/cli/command-secret-targets.import.test.ts index 7b9c45dcd8d6..70c6feb5e2b4 100644 --- a/src/cli/command-secret-targets.import.test.ts +++ b/src/cli/command-secret-targets.import.test.ts @@ -47,7 +47,10 @@ describe("command secret targets module import", () => { it("loads registry lazily for agent runtime plugin credential targets", async () => { const listSecretTargetRegistryEntries = vi.fn(() => [ - { id: "plugins.entries.example.config.webSearch.apiKey" }, + { + id: "plugins.entries.example.config.webSearch.apiKey", + pathPatternSegments: ["plugins", "entries", "example", "config", "webSearch", "apiKey"], + }, { id: "plugins.entries.example.config.other.apiKey" }, { id: "channels.telegram.botToken" }, ]); diff --git a/src/cli/command-secret-targets.ts b/src/cli/command-secret-targets.ts index 97c2205c0d6f..f54465406f18 100644 --- a/src/cli/command-secret-targets.ts +++ b/src/cli/command-secret-targets.ts @@ -16,10 +16,12 @@ import { resolvePluginWebSearchProviders } from "../plugins/web-search-providers import { sortWebSearchProvidersForAutoDetect } from "../plugins/web-search-providers.shared.js"; import { normalizeOptionalAccountId } from "../routing/session-key.js"; import { loadChannelSecretContractApi } from "../secrets/channel-contract-api.js"; +import { compileTargetRegistryEntry, matchPathTokens } from "../secrets/target-registry-pattern.js"; import { discoverConfigSecretTargetsByIds, listSecretTargetRegistryEntries, } from "../secrets/target-registry.js"; +import { parseConcreteConfigPathTokens } from "../shared/dot-path.js"; const STATIC_QR_REMOTE_TARGET_IDS = ["gateway.remote.token", "gateway.remote.password"] as const; const STATIC_MODEL_TARGET_IDS = [ @@ -107,36 +109,27 @@ function getChannelSecretTargetIds(): string[] { return cachedChannelSecretTargetIds; } -function isPluginWebCredentialTargetId(id: string): boolean { - const segments = id.split("."); - if (segments[0] !== "plugins" || segments[1] !== "entries" || segments[3] !== "config") { - return false; +function pluginWebCredentialConfigPath(entry: { + id: string; + pathPatternSegments?: string[]; +}): string | undefined { + const segments = entry.pathPatternSegments; + if (segments?.[0] === "plugins" && segments[1] === "entries" && segments[3] === "config") { + return segments.slice(4).join("."); } - const configPath = segments.slice(4).join("."); - return configPath === "webSearch.apiKey" || configPath === "webFetch.apiKey"; -} - -function isPluginWebSearchCredentialTargetId(id: string): boolean { - const segments = id.split("."); - if (segments[0] !== "plugins" || segments[1] !== "entries" || segments[3] !== "config") { - return false; + for (const configPath of ["webSearch.apiKey", "webFetch.apiKey"] as const) { + if (entry.id.startsWith("plugins.entries.") && entry.id.endsWith(`.config.${configPath}`)) { + return configPath; + } } - return segments.slice(4).join(".") === "webSearch.apiKey"; -} - -function isPluginWebFetchCredentialTargetId(id: string): boolean { - const segments = id.split("."); - if (segments[0] !== "plugins" || segments[1] !== "entries" || segments[3] !== "config") { - return false; - } - return segments.slice(4).join(".") === "webFetch.apiKey"; + return undefined; } function getCapabilityWebSearchTargetIds(): string[] { cachedCapabilityWebSearchTargetIds ??= sortUniqueStrings( listSecretTargetRegistryEntries() - .map((entry) => entry.id) - .filter(isPluginWebSearchCredentialTargetId), + .filter((entry) => pluginWebCredentialConfigPath(entry) === "webSearch.apiKey") + .map((entry) => entry.id), ); return cachedCapabilityWebSearchTargetIds; } @@ -144,8 +137,8 @@ function getCapabilityWebSearchTargetIds(): string[] { function getCapabilityWebFetchTargetIds(): string[] { cachedCapabilityWebFetchTargetIds ??= sortUniqueStrings( listSecretTargetRegistryEntries() - .map((entry) => entry.id) - .filter(isPluginWebFetchCredentialTargetId), + .filter((entry) => pluginWebCredentialConfigPath(entry) === "webFetch.apiKey") + .map((entry) => entry.id), ); return cachedCapabilityWebFetchTargetIds; } @@ -171,38 +164,11 @@ function resolveSearchConfig(config: OpenClawConfig): Record | : undefined; } -function pathPatternMatchesConcretePath(pathPattern: string, path: string): boolean { - const pathSegments = path.split("."); - const patternSegments = pathPattern.split("."); - let pathIndex = 0; - for (const segment of patternSegments) { - if (segment === "*") { - if (!pathSegments[pathIndex]) { - return false; - } - pathIndex += 1; - continue; - } - if (segment.endsWith("[]")) { - const field = segment.slice(0, -2); - if (pathSegments[pathIndex] !== field || !/^\d+$/.test(pathSegments[pathIndex + 1] ?? "")) { - return false; - } - pathIndex += 2; - continue; - } - if (pathSegments[pathIndex] !== segment) { - return false; - } - pathIndex += 1; - } - return pathIndex === pathSegments.length; -} - // Registry entries use wildcard path patterns; command inputs often identify one concrete config path. function targetIdsForConfigPath(path: string): string[] { + const pathSegments = parseConcreteConfigPathTokens(path); return listSecretTargetRegistryEntries() - .filter((entry) => pathPatternMatchesConcretePath(entry.pathPattern ?? entry.id, path)) + .filter((entry) => matchPathTokens(pathSegments, compileTargetRegistryEntry(entry).pathTokens)) .map((entry) => entry.id) .toSorted(); } @@ -621,8 +587,11 @@ function getAgentRuntimeBaseTargetIds(): string[] { cachedAgentRuntimeBaseTargetIds ??= [ ...STATIC_AGENT_RUNTIME_BASE_TARGET_IDS, ...listSecretTargetRegistryEntries() + .filter((entry) => { + const configPath = pluginWebCredentialConfigPath(entry); + return configPath === "webSearch.apiKey" || configPath === "webFetch.apiKey"; + }) .map((entry) => entry.id) - .filter(isPluginWebCredentialTargetId) .toSorted(), ]; return cachedAgentRuntimeBaseTargetIds; diff --git a/src/cli/config-cli-input.ts b/src/cli/config-cli-input.ts index 473bac636bf5..15514d52665b 100644 --- a/src/cli/config-cli-input.ts +++ b/src/cli/config-cli-input.ts @@ -20,9 +20,14 @@ import { validateExecSecretRefId, } from "../secrets/ref-contract.js"; import { resolveConfigSecretTargetByPath } from "../secrets/target-registry.js"; -import { toDotPath } from "../shared/dot-path.js"; +import { + parseConcreteConfigPathWithProvenance, + toDotPath, + type ConcreteConfigPathSegment, +} from "../shared/dot-path.js"; import { formatCliCommand } from "./command-format.js"; import { + formatConfigSetPath, parseConfigSetPath, parseConfigSetValue, type PathSegment, @@ -44,6 +49,8 @@ const CONFIG_PATCH_STDIN_MAX_BYTES = 1024 * 1024; export type ConfigSetOperation = { inputMode: ConfigSetDryRunInputMode; requestedPath: PathSegment[]; + pathTokens?: readonly ConcreteConfigPathSegment[]; + quotedNumericSegments?: ReadonlySet; setPath: PathSegment[]; value: unknown; mutation?: "set" | "merge" | "replace" | "delete"; @@ -315,18 +322,24 @@ function touchesSecretDefaults(path: PathSegment[]): boolean { function buildRefAssignmentOperation(params: { requestedPath: PathSegment[]; + pathTokens?: readonly ConcreteConfigPathSegment[]; + quotedNumericSegments?: ReadonlySet; ref: SecretRef; inputMode: ConfigSetDryRunInputMode; }): ConfigSetOperation { - const resolved = resolveConfigSecretTargetByPath(params.requestedPath); + const resolved = resolveConfigSecretTargetByPath(params.requestedPath, params.pathTokens); if (resolved?.entry.secretShape === "sibling_ref" && resolved.refPathSegments) { return { inputMode: params.inputMode, requestedPath: params.requestedPath, + ...(params.pathTokens ? { pathTokens: params.pathTokens } : {}), + ...(params.quotedNumericSegments + ? { quotedNumericSegments: params.quotedNumericSegments } + : {}), setPath: resolved.refPathSegments, value: params.ref, schemaValidated: true, - touchedSecretTargetPath: toDotPath(resolved.pathSegments), + touchedSecretTargetPath: formatConfigSetPath(resolved.pathSegments, params.pathTokens), assignedRef: params.ref, ...(resolved.providerId ? { touchedProviderAlias: resolved.providerId } : {}), }; @@ -334,10 +347,17 @@ function buildRefAssignmentOperation(params: { return { inputMode: params.inputMode, requestedPath: params.requestedPath, + ...(params.pathTokens ? { pathTokens: params.pathTokens } : {}), + ...(params.quotedNumericSegments + ? { quotedNumericSegments: params.quotedNumericSegments } + : {}), setPath: params.requestedPath, value: params.ref, ...(resolved ? { schemaValidated: true } : {}), - touchedSecretTargetPath: toDotPath(resolved?.pathSegments ?? params.requestedPath), + touchedSecretTargetPath: formatConfigSetPath( + resolved?.pathSegments ?? params.requestedPath, + params.pathTokens, + ), assignedRef: params.ref, ...(resolved?.providerId ? { touchedProviderAlias: resolved.providerId } : {}), }; @@ -345,18 +365,29 @@ function buildRefAssignmentOperation(params: { function buildValueAssignmentOperation(params: { requestedPath: PathSegment[]; + pathTokens?: readonly ConcreteConfigPathSegment[]; + quotedNumericSegments?: ReadonlySet; value: unknown; inputMode: ConfigSetDryRunInputMode; }): ConfigSetOperation { - const resolved = resolveConfigSecretTargetByPath(params.requestedPath); + const resolved = resolveConfigSecretTargetByPath(params.requestedPath, params.pathTokens); const providerAlias = parseProviderAliasFromTargetPath(params.requestedPath); const coercedRef = coerceSecretRef(params.value); return { inputMode: params.inputMode, requestedPath: params.requestedPath, - setPath: params.requestedPath, + ...(params.pathTokens ? { pathTokens: params.pathTokens } : {}), + ...(params.quotedNumericSegments + ? { quotedNumericSegments: params.quotedNumericSegments } + : {}), + setPath: + coercedRef && resolved?.entry.secretShape === "sibling_ref" && resolved.refPathSegments + ? resolved.refPathSegments + : params.requestedPath, value: params.value, - ...(resolved ? { touchedSecretTargetPath: toDotPath(resolved.pathSegments) } : {}), + ...(resolved + ? { touchedSecretTargetPath: formatConfigSetPath(resolved.pathSegments, params.pathTokens) } + : {}), ...(providerAlias ? { touchedProviderAlias: providerAlias } : {}), ...(coercedRef ? { assignedRef: coercedRef } : {}), }; @@ -364,10 +395,15 @@ function buildValueAssignmentOperation(params: { function parseBatchOperations(entries: ConfigSetBatchEntry[]): ConfigSetOperation[] { return entries.map((entry, index) => { - const path = parseConfigSetPath(entry.path); + const { tokens: pathTokens, quotedNumericSegments } = parseConcreteConfigPathWithProvenance( + entry.path, + ); + const path = pathTokens.map(String); if (entry.ref !== undefined) { return buildRefAssignmentOperation({ requestedPath: path, + pathTokens, + quotedNumericSegments, ref: parseSecretRefFromUnknown(entry.ref, `batch[${index}].ref`), inputMode: "json", }); @@ -384,6 +420,8 @@ function parseBatchOperations(entries: ConfigSetBatchEntry[]): ConfigSetOperatio return { inputMode: "json", requestedPath: path, + pathTokens, + quotedNumericSegments, setPath: path, value: validated.data, schemaValidated: true, @@ -392,6 +430,8 @@ function parseBatchOperations(entries: ConfigSetBatchEntry[]): ConfigSetOperatio } return buildValueAssignmentOperation({ requestedPath: path, + pathTokens, + quotedNumericSegments, value: entry.value, inputMode: "json", }); @@ -404,7 +444,11 @@ function buildSingleSetOperations(params: { opts: ConfigSetOptions; }): ConfigSetOperation[] { const pathProvided = typeof params.path === "string" && params.path.trim().length > 0; - const parsedPath = pathProvided ? parseConfigSetPath(params.path as string) : null; + const parsedConcretePath = pathProvided + ? parseConcreteConfigPathWithProvenance(params.path as string) + : null; + const pathTokens = parsedConcretePath?.tokens ?? null; + const parsedPath = pathTokens?.map(String) ?? null; const strictJson = Boolean(params.opts.strictJson || params.opts.json); const modeResolution = resolveConfigSetMode({ hasBatchMode: false, @@ -431,6 +475,8 @@ function buildSingleSetOperations(params: { return [ buildRefAssignmentOperation({ requestedPath: parsedPath, + pathTokens: pathTokens ?? undefined, + quotedNumericSegments: parsedConcretePath?.quotedNumericSegments, ref: parseSecretRefBuilder({ provider: params.opts.refProvider, source: params.opts.refSource, @@ -453,6 +499,10 @@ function buildSingleSetOperations(params: { { inputMode: "builder", requestedPath: parsedPath, + ...(pathTokens ? { pathTokens } : {}), + ...(parsedConcretePath + ? { quotedNumericSegments: parsedConcretePath.quotedNumericSegments } + : {}), setPath: parsedPath, value: buildProviderFromBuilder(params.opts), schemaValidated: true, @@ -470,6 +520,8 @@ function buildSingleSetOperations(params: { return [ buildValueAssignmentOperation({ requestedPath: parsedPath, + pathTokens: pathTokens ?? undefined, + quotedNumericSegments: parsedConcretePath?.quotedNumericSegments, value: parseConfigSetValue(params.value, strictJson), inputMode: modeResolution.mode === "json" ? "json" : "value", }), @@ -537,19 +589,25 @@ function buildDeleteOperation(path: PathSegment[]): ConfigSetOperation { }; } -export function buildUnsetOperation(path: PathSegment[]): ConfigSetOperation { - const resolved = resolveConfigSecretTargetByPath(path); +export function buildUnsetOperation( + path: PathSegment[], + pathTokens?: readonly ConcreteConfigPathSegment[], +): ConfigSetOperation { + const resolved = resolveConfigSecretTargetByPath(path, pathTokens); const providerAlias = parseProviderAliasFromTargetPath(path); return { inputMode: "unset", requestedPath: path, + ...(pathTokens ? { pathTokens } : {}), setPath: path, value: undefined, mutation: "delete", ...(touchesSecretProviderCollection(path) || touchesSecretDefaults(path) ? { touchesAllSecretRefs: true } : {}), - ...(resolved ? { touchedSecretTargetPath: toDotPath(resolved.pathSegments) } : {}), + ...(resolved + ? { touchedSecretTargetPath: formatConfigSetPath(resolved.pathSegments, pathTokens) } + : {}), ...(providerAlias ? { touchedProviderAlias: providerAlias } : {}), }; } diff --git a/src/cli/config-cli-path.ts b/src/cli/config-cli-path.ts index c8202e617ead..083d10ffb54a 100644 --- a/src/cli/config-cli-path.ts +++ b/src/cli/config-cli-path.ts @@ -2,13 +2,27 @@ import { isRecord as isPlainRecord } from "@openclaw/normalization-core/record-c import JSON5 from "json5"; import { rejectConfigNonFiniteNumbers } from "../config/io.read-helpers.js"; import { isBlockedObjectKey } from "../infra/prototype-keys.js"; -import { toDotPath } from "../shared/dot-path.js"; +import { + formatConcreteConfigPath, + toDotPath, + type ConcreteConfigPathSegment, +} from "../shared/dot-path.js"; import { parseConfigPathArrayIndex } from "../shared/path-array-index.js"; import { formatCliCommand } from "./command-format.js"; import { formatStrictJsonParseFailure } from "./error-format.js"; +export { parseConcreteConfigPath as parseConfigSetPath } from "../shared/dot-path.js"; + export type PathSegment = string; +export function formatConfigSetPath( + path: readonly PathSegment[], + pathTokens?: readonly ConcreteConfigPathSegment[], + source?: unknown, +): string { + return formatConcreteConfigPath(pathTokens ?? path, source); +} + export type JsonSchemaRecord = { type?: unknown; properties?: unknown; @@ -21,6 +35,8 @@ export type JsonSchemaRecord = { type SetAtPathOptions = { numericObjectKeys?: boolean; + pathTokens?: readonly ConcreteConfigPathSegment[]; + quotedNumericSegments?: ReadonlySet; schema?: JsonSchemaRecord; }; @@ -32,133 +48,6 @@ function isIndexSegment(raw: string): boolean { return parseIndexSegment(raw) !== undefined; } -function parseBracketPathSegment(raw: string, fullPath: string): string { - const trimmed = raw.trim(); - if (!trimmed) { - throw new Error(`Invalid path (empty "[]"): ${fullPath}`); - } - if (trimmed.startsWith('"') || trimmed.startsWith("'")) { - try { - const parsed = JSON5.parse(trimmed) as unknown; - if (typeof parsed === "string" && parsed.trim()) { - return parsed; - } - } catch (err) { - throw new Error(`Invalid path bracket string (${trimmed}): ${fullPath}`, { cause: err }); - } - throw new Error(`Invalid path bracket string (${trimmed}): ${fullPath}`); - } - return trimmed; -} - -function assertNotWhitespaceSegment(current: string, raw: string): void { - if (current.length > 0 && !current.trim()) { - throw new Error(`Invalid path (empty segment): ${raw}`); - } -} - -function findBracketPathClose(path: string, open: number): number { - let quote: '"' | "'" | undefined; - for (let index = open + 1; index < path.length; index += 1) { - const character = path[index]; - if (quote) { - if (character === "\\") { - index += 1; - } else if (character === quote) { - quote = undefined; - } - continue; - } - if (character === "]") { - return index; - } - if ((character === '"' || character === "'") && !path.slice(open + 1, index).trim()) { - quote = character; - } - } - return -1; -} - -function parsePath(raw: string): PathSegment[] { - const trimmed = raw.trim(); - if (!trimmed) { - return []; - } - const parts: string[] = []; - let current = ""; - let segmentEmitted = false; - let i = 0; - while (i < trimmed.length) { - const ch = trimmed[i]; - if (ch === "\\") { - const next = trimmed[i + 1]; - if (next === undefined) { - throw new Error(`Invalid path (trailing escape): ${raw}`); - } - current += next; - i += 2; - continue; - } - if (ch === ".") { - assertNotWhitespaceSegment(current, raw); - if (!segmentEmitted && !current.trim()) { - throw new Error(`Invalid path (empty segment): ${raw}`); - } - if (current) { - parts.push(current.trim()); - } - current = ""; - segmentEmitted = false; - i += 1; - continue; - } - if (ch === "[") { - assertNotWhitespaceSegment(current, raw); - if (!current.trim() && !segmentEmitted && parts.length > 0) { - throw new Error(`Invalid path (empty segment): ${raw}`); - } - if (current) { - parts.push(current.trim()); - } - current = ""; - const close = findBracketPathClose(trimmed, i); - if (close === -1) { - throw new Error(`Invalid path (missing "]"): ${raw}`); - } - const inside = trimmed.slice(i + 1, close).trim(); - if (!inside) { - throw new Error(`Invalid path (empty "[]"): ${raw}`); - } - parts.push(parseBracketPathSegment(inside, raw)); - const next = trimmed[close + 1]; - if (next !== undefined && next !== "." && next !== "[") { - throw new Error(`Invalid path (missing separator after bracket): ${raw}`); - } - segmentEmitted = true; - i = close + 1; - continue; - } - current += ch; - i += 1; - } - if (!segmentEmitted && !current.trim()) { - throw new Error(`Invalid path (empty segment): ${raw}`); - } - if (current) { - parts.push(current.trim()); - } - return parts; -} - -export function parseConfigSetPath(path: string): string[] { - const parsedPath = parsePath(path); - if (parsedPath.length === 0) { - throw new Error("Path is empty."); - } - validatePathSegments(parsedPath); - return parsedPath; -} - export function parseConfigSetValue(raw: string, strictJson: boolean): unknown { const trimmed = raw.trim(); if (strictJson) { @@ -364,6 +253,13 @@ function shouldCreateArrayForMissingPathSegment(params: { if (!params.next || params.options?.numericObjectKeys || !isIndexSegment(params.next)) { return false; } + const nextToken = params.options?.pathTokens?.[params.segmentIndex + 1]; + if (typeof nextToken === "number") { + return true; + } + if (params.options?.quotedNumericSegments?.has(params.segmentIndex + 1)) { + return false; + } const parentPath = params.path.slice(0, params.segmentIndex + 1); return schemaPrefersArrayAtPath(params.options?.schema, parentPath) ?? true; } diff --git a/src/cli/config-cli-runner.ts b/src/cli/config-cli-runner.ts index f3a0e9d8a86b..c92a6da89fcb 100644 --- a/src/cli/config-cli-runner.ts +++ b/src/cli/config-cli-runner.ts @@ -29,6 +29,7 @@ import { } from "./config-cli-model-normalization.js"; import { assertNonDestructiveReplacement, + formatConfigSetPath, getAtPath, mergeAtPath, setAtPath, @@ -316,6 +317,10 @@ export async function runConfigOperations(params: { if (operation.mutation === "merge" || (options.merge && operation.mutation !== "replace")) { mergeAtPath(next, operation.setPath, operation.value, { numericObjectKeys: params.successMode === "patch", + ...(operation.pathTokens ? { pathTokens: operation.pathTokens } : {}), + ...(operation.quotedNumericSegments + ? { quotedNumericSegments: operation.quotedNumericSegments } + : {}), schema: mutationSchema, }); } else { @@ -327,6 +332,10 @@ export async function runConfigOperations(params: { }); setAtPath(next, operation.setPath, operation.value, { numericObjectKeys: params.successMode === "patch", + ...(operation.pathTokens ? { pathTokens: operation.pathTokens } : {}), + ...(operation.quotedNumericSegments + ? { quotedNumericSegments: operation.quotedNumericSegments } + : {}), schema: mutationSchema, }); } @@ -501,7 +510,12 @@ export async function runConfigOperations(params: { if (params.successMode === "set" && operations.length === 1) { const operation = operations[0]; const action = operation?.mutation === "delete" ? "Removed" : "Updated"; - runtime.log(info(`${action} ${toDotPath(operation?.requestedPath ?? [])}. ${hint}`)); + const requestedPath = formatConfigSetPath( + operation?.requestedPath ?? [], + operation?.pathTokens, + nextConfig, + ); + runtime.log(info(`${action} ${requestedPath}. ${hint}`)); } else if (params.successMode === "set") { runtime.log(info(`Updated ${operations.length} config paths. ${hint}`)); } else { diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts index b2d908fcde4d..d1912e403478 100644 --- a/src/cli/config-cli.test.ts +++ b/src/cli/config-cli.test.ts @@ -45,20 +45,38 @@ const mockLoadChannelSecretContractApi = vi.hoisted(() => telegram: ["botToken"], }; return { - secretTargetRegistryEntries: (fields[channelId] ?? []).map((field) => { - const pathPattern = `channels.${channelId}.${field}`; - return { - id: pathPattern, - targetType: pathPattern, - configFile: "openclaw.json" as const, - pathPattern, - secretShape: "secret_input" as const, - expectedResolvedValue: "string" as const, - includeInPlan: true, - includeInConfigure: true, - includeInAudit: true, - }; - }), + secretTargetRegistryEntries: [ + ...(fields[channelId] ?? []).map((field) => { + const pathPattern = `channels.${channelId}.${field}`; + return { + id: pathPattern, + targetType: pathPattern, + configFile: "openclaw.json" as const, + pathPattern, + secretShape: "secret_input" as const, + expectedResolvedValue: "string" as const, + includeInPlan: true, + includeInConfigure: true, + includeInAudit: true, + }; + }), + ...(channelId === "discord" + ? [ + { + id: "channels.discord.accounts[].token", + targetType: "channels.discord.accounts[].token", + configFile: "openclaw.json" as const, + pathPattern: "channels.discord.accounts[].token", + refPathPattern: "channels.discord.accounts[].tokenRef", + secretShape: "sibling_ref" as const, + expectedResolvedValue: "string" as const, + includeInPlan: true, + includeInConfigure: true, + includeInAudit: true, + }, + ] + : []), + ], }; }), ); @@ -1908,6 +1926,114 @@ describe("config cli", () => { }); }); + it.each(["ref builder", "JSON value", "batch ref", "batch value"] as const)( + "writes array-indexed sibling SecretRefs to their registered ref path in %s mode", + async (mode) => { + const resolved = { + channels: { discord: { accounts: [{ token: "existing-token" }] } }, + } as unknown as OpenClawConfig; + const ref = { source: "env", provider: "default", id: "DISCORD_ACCOUNT_TOKEN" }; + const configPath = "channels.discord.accounts[0].token"; + setSnapshot(resolved, resolved); + + const args = + mode === "ref builder" + ? [ + configPath, + "--ref-provider", + ref.provider, + "--ref-source", + ref.source, + "--ref-id", + ref.id, + ] + : mode === "JSON value" + ? [configPath, JSON.stringify(ref), "--strict-json"] + : [ + "--batch-json", + JSON.stringify([ + mode === "batch ref" + ? { path: configPath, ref } + : { path: configPath, value: ref }, + ]), + ]; + await runConfigSet(...args); + + expect(mockWriteConfigFile).toHaveBeenCalledTimes(1); + const written = firstWrittenConfig() as { + channels?: { discord?: { accounts?: Array<{ token?: unknown; tokenRef?: unknown }> } }; + }; + expect(written.channels?.discord?.accounts?.[0]).toEqual({ + token: "existing-token", + tokenRef: ref, + }); + expect(requireWriteOptions().explicitSetPaths).toEqual([ + ["channels", "discord", "accounts", "0", "tokenRef"], + ]); + }, + ); + + it("keeps a quoted numeric record key distinct from an array-indexed secret target", async () => { + const resolved = { + channels: { discord: { accounts: { "0": { token: "existing-token" } } } }, + } as unknown as OpenClawConfig; + const ref = { source: "env", provider: "default", id: "DISCORD_ACCOUNT_TOKEN" }; + setSnapshot(resolved, resolved); + + await runConfigSet( + 'channels.discord.accounts["0"].token', + "--ref-provider", + ref.provider, + "--ref-source", + ref.source, + "--ref-id", + ref.id, + ); + + const written = firstWrittenConfig() as { + channels?: { discord?: { accounts?: Record } }; + }; + expect(written.channels?.discord?.accounts?.["0"]).toEqual({ token: ref }); + expect(requireWriteOptions().explicitSetPaths).toEqual([ + ["channels", "discord", "accounts", "0", "token"], + ]); + }); + + it.each([ + [ + 'agents.defaults.models["fixture/model.v1"].params["literal.dot"]', + "LITERAL", + { "literal.dot": "LITERAL" }, + ], + [ + 'agents.defaults.models["fixture/model.v1"].params.literal.dot', + "NESTED", + { literal: { dot: "NESTED" } }, + ], + [ + 'agents.defaults.models["fixture/model.v1"].params.record["0"]', + "RECORD-ZERO", + { record: { "0": "RECORD-ZERO" } }, + ], + [ + 'agents.defaults.models["fixture/model.v1"].params.list[0]', + "ARRAY-ZERO", + { list: ["ARRAY-ZERO"] }, + ], + ])("preserves generic config path identity for %s", async (configPath, value, expected) => { + const resolved = { + agents: { defaults: { models: { "fixture/model.v1": { params: {} } } } }, + } as unknown as OpenClawConfig; + setSnapshot(resolved, resolved); + + await runConfigSet(configPath, JSON.stringify(value), "--strict-json"); + + expect(firstWrittenConfig().agents?.defaults?.models?.["fixture/model.v1"]?.params).toEqual( + expected, + ); + expectLogIncludes(`Updated ${configPath}`); + }); + it("keeps numeric config set path segments as object keys for schema-backed Discord guild records", async () => { setConfigMutationShapeSchema(); const resolved: OpenClawConfig = { @@ -3771,6 +3897,11 @@ describe("config cli", () => { args: ["config", "set", "gateway.[port]", "23456"], error: "Invalid path (empty segment): gateway.[port]", }, + { + name: "rejects registry array patterns as concrete config paths", + args: ["config", "get", "plugins.entries.example.config.accounts[].token"], + error: 'Invalid path (empty "[]"): plugins.entries.example.config.accounts[].token', + }, { name: "rejects a trailing escape for config get before reading another key", args: ["config", "get", "gateway.port\\"], @@ -3855,6 +3986,14 @@ describe("config cli", () => { ["agents.list[0].id", ["agents", "list", "0", "id"]], ["agents.list[0][1]", ["agents", "list", "0", "1"]], ["[0]", ["0"]], + [ + 'plugins.entries.example.config.accounts["0"].token', + ["plugins", "entries", "example", "config", "accounts", "0", "token"], + ], + [ + 'plugins.entries["foo.config.bar"].config.token', + ["plugins", "entries", "foo.config.bar", "config", "token"], + ], [" gateway.port ", ["gateway", "port"]], ["channels.discord.guilds.prod\\.guild", ["channels", "discord", "guilds", "prod.guild"]], [ @@ -4494,7 +4633,7 @@ describe("config cli", () => { "--strict-json", ]); - expectLogIncludes("Updated agents.list.1.model.primary"); + expectLogIncludes("Updated agents.list[1].model.primary"); expectLogIncludes("Change will apply without restarting the gateway."); expectLogExcludes("Restart the gateway to apply."); }); @@ -4544,7 +4683,7 @@ describe("config cli", () => { "--strict-json", ]); - expectLogIncludes("Updated agents.list.0.model.primary"); + expectLogIncludes("Updated agents.list[0].model.primary"); expectLogIncludes("Restart the gateway to apply."); expectLogExcludes("Change will apply without restarting the gateway."); }); @@ -4568,7 +4707,7 @@ describe("config cli", () => { "--strict-json", ]); - expectLogIncludes("Updated agents.list.0.model.primary"); + expectLogIncludes("Updated agents.list[0].model.primary"); expectLogIncludes("Change will apply without restarting the gateway."); expectLogExcludes("Restart the gateway to apply."); }); @@ -4674,23 +4813,30 @@ describe("config cli", () => { expectLogExcludes("Change will apply without restarting the gateway."); }); - it("keeps plugin entry config writes restart-backed when reload metadata is absent", async () => { - const resolved: OpenClawConfig = { - plugins: { - entries: { - canvas: { enabled: true }, + it.each([ + ["canvas", "plugins.entries.canvas.enabled"], + ["canvas.internal", 'plugins.entries["canvas.internal"].enabled'], + ["canvas", "plugins.entries.canvas.config.accounts[0].enabled"], + ])( + "keeps plugin entry %s writes unambiguous and restart-backed", + async (pluginId, configPath) => { + const resolved = { + plugins: { + entries: { + [pluginId]: { enabled: true, config: { accounts: [{ enabled: true }] } }, + }, }, - }, - } as unknown as OpenClawConfig; - setSnapshot(resolved, resolved); + } as unknown as OpenClawConfig; + setSnapshot(resolved, resolved); - await runConfigSet("plugins.entries.canvas.enabled", "false"); + await runConfigSet(configPath, "false"); - expectLogIncludes("Updated plugins.entries.canvas.enabled"); - expectLogIncludes("Restart the gateway to apply."); - expectLogExcludes("Change will apply without restarting the gateway."); - expectLogExcludes("No gateway restart needed."); - }); + expectLogIncludes(`Updated ${configPath}`); + expectLogIncludes("Restart the gateway to apply."); + expectLogExcludes("Change will apply without restarting the gateway."); + expectLogExcludes("No gateway restart needed."); + }, + ); it("keeps the restart hint for mixed hot and restart batch updates", async () => { const resolved: OpenClawConfig = { diff --git a/src/cli/config-cli.ts b/src/cli/config-cli.ts index 1481311f21b2..d403b57c884a 100644 --- a/src/cli/config-cli.ts +++ b/src/cli/config-cli.ts @@ -22,6 +22,7 @@ import { writeRuntimeJson, writeRuntimeStdout, } from "../runtime.js"; +import { parseConcreteConfigPathTokens } from "../shared/dot-path.js"; import { shortenHomePath } from "../utils.js"; import { formatCliCommand } from "./command-format.js"; import { @@ -229,7 +230,8 @@ export async function runConfigUnset(opts: { if (cliOptions.json && !cliOptions.dryRun) { throw new Error("--json can only be used with --dry-run."); } - const parsedPath = parseConfigSetPath(opts.path); + const pathTokens = parseConcreteConfigPathTokens(opts.path); + const parsedPath = pathTokens.map(String); assertConfigPathIsNotAutoManaged(parsedPath); const mutationStart = cliOptions.dryRun ? { snapshot: await loadValidConfig(runtime), writeOptions: {} } @@ -276,7 +278,7 @@ export async function runConfigUnset(opts: { runtime.exit(1); return; } - const operation = buildUnsetOperation(parsedPath); + const operation = buildUnsetOperation(parsedPath, pathTokens); if (cliOptions.dryRun) { await runConfigOperations({ runtime, diff --git a/src/config/env-substitution.test.ts b/src/config/env-substitution.test.ts index e4e43b43eab9..f5ad62c7dbad 100644 --- a/src/config/env-substitution.test.ts +++ b/src/config/env-substitution.test.ts @@ -6,6 +6,11 @@ import { containsEnvVarReference, resolveConfigEnvVars, } from "./env-substitution.js"; +import { + createConfigResolutionFacts, + getAuthoredConfigSecretRef, + setConfigResolutionFacts, +} from "./resolution-facts.js"; type SubstitutionScenario = { name: string; @@ -138,6 +143,65 @@ describe("resolveConfigEnvVars", () => { varName: "MISSING", configPath: "items[1]", }, + { + name: "dotted plugin ID remains one record key", + config: { plugins: { entries: { "foo.config.bar": { token: "${MISSING}" } } } }, + env: {}, + varName: "MISSING", + configPath: 'plugins.entries["foo.config.bar"].token', + }, + { + name: "dotted header remains one record key", + config: { + plugins: { entries: { fixture: { config: { headers: { "X.Trace": "${MISSING}" } } } } }, + }, + env: {}, + varName: "MISSING", + configPath: 'plugins.entries.fixture.config.headers["X.Trace"]', + }, + { + name: "nested header segments remain dotted", + config: { + plugins: { + entries: { fixture: { config: { headers: { X: { Trace: "${MISSING}" } } } } }, + }, + }, + env: {}, + varName: "MISSING", + configPath: "plugins.entries.fixture.config.headers.X.Trace", + }, + { + name: "numeric-looking record key is not an array index", + config: { + plugins: { entries: { fixture: { config: { headers: { "0": "${MISSING}" } } } } }, + }, + env: {}, + varName: "MISSING", + configPath: 'plugins.entries.fixture.config.headers["0"]', + }, + { + name: "existing non-plugin root record paths stay unchanged", + config: { "root.key": "${MISSING}" }, + env: {}, + varName: "MISSING", + configPath: "root.key", + }, + { + name: "plugin config array indices remain canonical", + config: { + plugins: { entries: { fixture: { config: { headers: ["${MISSING}"] } } } }, + }, + env: {}, + varName: "MISSING", + configPath: "plugins.entries.fixture.config.headers[0]", + }, + { + name: "hyphenated record key keeps its existing dotted spelling", + config: { providers: { "vercel-gateway": { apiKey: "${MISSING}" } } }, + env: {}, + varName: "MISSING", + configPath: "providers.vercel-gateway.apiKey", + }, { name: "empty string env value treated as missing", config: { key: "${EMPTY}" }, @@ -272,6 +336,57 @@ describe("resolveConfigEnvVars", () => { }); describe("graceful missing env var handling (onMissing)", () => { + it("keeps authored SecretRef provenance distinct across dotted plugin and header keys", () => { + const pendingEnvSecretRefs = new Map(); + const config = resolveConfigEnvVars( + { + plugins: { + entries: { + "foo.config.bar": { config: { token: "$ATTACKER" } }, + foo: { + config: { + bar: { config: { token: "$VICTIM" } }, + headers: { + "X.Trace": "$DOTTED_HEADER", + X: { Trace: "$NESTED_HEADER" }, + }, + }, + }, + }, + }, + models: { + providers: { + "alpha:beta": { + apiKey: "$CORE_PROVIDER", + headers: { "X.Trace": "$CORE_HEADER" }, + }, + }, + }, + }, + {}, + { + onPendingEnvSecretRef: (id, configPath) => pendingEnvSecretRefs.set(configPath, id), + }, + ); + setConfigResolutionFacts(config, createConfigResolutionFacts([], pendingEnvSecretRefs)); + + expect([...pendingEnvSecretRefs]).toEqual([ + ['plugins.entries["foo.config.bar"].config.token', "ATTACKER"], + ["plugins.entries.foo.config.bar.config.token", "VICTIM"], + ['plugins.entries.foo.config.headers["X.Trace"]', "DOTTED_HEADER"], + ["plugins.entries.foo.config.headers.X.Trace", "NESTED_HEADER"], + ["models.providers.alpha:beta.apiKey", "CORE_PROVIDER"], + ["models.providers.alpha:beta.headers.X.Trace", "CORE_HEADER"], + ]); + for (const [configPath, id] of pendingEnvSecretRefs) { + expect(getAuthoredConfigSecretRef(config, configPath), configPath).toEqual({ + source: "env", + provider: "default", + id, + }); + } + }); + it("collects warnings and preserves placeholder when onMissing is set", () => { const warnings: EnvSubstitutionWarning[] = []; const result = resolveConfigEnvVars( diff --git a/src/config/env-substitution.ts b/src/config/env-substitution.ts index 193d70777148..ff59288d1ed4 100644 --- a/src/config/env-substitution.ts +++ b/src/config/env-substitution.ts @@ -22,6 +22,7 @@ // Pattern for valid uppercase env var names: starts with letter or underscore, // followed by letters, numbers, or underscores (all uppercase) +import { appendConfigPathSegment } from "../shared/dot-path.js"; import { isPlainObject } from "../utils.js"; import { parseEnvTemplateSecretRef } from "./types.secrets.js"; @@ -189,7 +190,15 @@ function substituteAny( if (isPlainObject(value)) { const result: Record = {}; for (const [key, val] of Object.entries(value)) { - const childPath = path ? `${path}.${key}` : key; + const isPluginConfigPath = + path === "plugins.entries" || + path.startsWith("plugins.entries.") || + path.startsWith("plugins.entries["); + const childPath = isPluginConfigPath + ? appendConfigPathSegment(path, key) + : path + ? `${path}.${key}` + : key; result[key] = substituteAny(val, env, childPath, opts); } return result; diff --git a/src/plugins/config-contract-matches.ts b/src/plugins/config-contract-matches.ts index 7576b818b837..bf38dcc2e37b 100644 --- a/src/plugins/config-contract-matches.ts +++ b/src/plugins/config-contract-matches.ts @@ -1,5 +1,6 @@ // Matches plugin config contracts against config paths and values. import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization"; +import { appendConfigPathSegment } from "../shared/dot-path.js"; import { parseConfigPathArrayIndex } from "../shared/path-array-index.js"; import { isRecord } from "../utils.js"; @@ -8,24 +9,21 @@ type PluginConfigContractMatch = { path: string; /** Config value stored at the matched path. */ value: unknown; + /** Exact matched container and key so assignments update the original location directly. */ + parent: Record | unknown[]; + key: string; }; type TraversalState = { - segments: string[]; + segments: Array; value: unknown; + parent?: Record | unknown[]; }; function normalizePathPattern(pathPattern: string): string[] { return normalizeStringEntries(pathPattern.split(".")); } -function appendPathSegment(path: string, segment: string): string { - if (!path) { - return segment; - } - return /^\d+$/.test(segment) ? `${path}[${segment}]` : `${path}.${segment}`; -} - function parseCanonicalArrayIndex(segment: string, length: number): number | null { const index = parseConfigPathArrayIndex(segment); return index !== undefined && index < length ? index : null; @@ -50,8 +48,9 @@ export function collectPluginConfigContractMatches(params: { if (Array.isArray(state.value)) { for (const [index, value] of state.value.entries()) { nextStates.push({ - segments: [...state.segments, String(index)], + segments: [...state.segments, index], value, + parent: state.value, }); } continue; @@ -61,6 +60,7 @@ export function collectPluginConfigContractMatches(params: { nextStates.push({ segments: [...state.segments, key], value, + parent: state.value, }); } } @@ -70,8 +70,9 @@ export function collectPluginConfigContractMatches(params: { const index = parseCanonicalArrayIndex(segment, state.value.length); if (index !== null) { nextStates.push({ - segments: [...state.segments, segment], + segments: [...state.segments, index], value: state.value[index], + parent: state.value, }); } continue; @@ -82,6 +83,7 @@ export function collectPluginConfigContractMatches(params: { nextStates.push({ segments: [...state.segments, segment], value: state.value[segment], + parent: state.value, }); } states = nextStates; @@ -91,7 +93,9 @@ export function collectPluginConfigContractMatches(params: { } return states.map((state) => ({ - path: state.segments.reduce(appendPathSegment, ""), + path: state.segments.reduce(appendConfigPathSegment, ""), value: state.value, + parent: state.parent!, + key: String(state.segments.at(-1)!), })); } diff --git a/src/plugins/config-contracts.test.ts b/src/plugins/config-contracts.test.ts index 75de3cbcfc63..f5093f0836d9 100644 --- a/src/plugins/config-contracts.test.ts +++ b/src/plugins/config-contracts.test.ts @@ -375,7 +375,7 @@ describe("collectPluginConfigContractMatches", () => { root, pathPattern: "items.1", }), - ).toEqual([{ path: "items[1]", value: "second" }]); + ).toEqual([{ path: "items[1]", value: "second", parent: root.items, key: "1" }]); expect( collectPluginConfigContractMatches({ root, @@ -390,6 +390,63 @@ describe("collectPluginConfigContractMatches", () => { ).toEqual([]); }); + it("preserves exact dotted wildcard keys and array-index parents", () => { + const headers = { "X.Trace": "trace-value" }; + const entries = [{ headers }]; + + expect( + collectPluginConfigContractMatches({ + root: { "sales.eu": { entries } }, + pathPattern: "*.entries.*.headers.*", + }), + ).toEqual([ + { + path: '["sales.eu"].entries[0].headers["X.Trace"]', + value: "trace-value", + parent: headers, + key: "X.Trace", + }, + ]); + expect( + collectPluginConfigContractMatches({ + root: { entries }, + pathPattern: "entries.*", + }), + ).toEqual([{ path: "entries[0]", value: entries[0], parent: entries, key: "0" }]); + }); + + it.each([ + { key: "X.Trace", path: 'headers["X.Trace"]' }, + { key: "0", path: 'headers["0"]' }, + { key: "01", path: 'headers["01"]' }, + { key: "value[0]", path: 'headers["value[0]"]' }, + { key: 'quoted"key', path: 'headers["quoted\\"key"]' }, + { key: "escaped\\key", path: 'headers["escaped\\\\key"]' }, + { key: "safe-header", path: "headers.safe-header" }, + ])("renders wildcard record key $key without path ambiguity", ({ key, path }) => { + const headers = { [key]: "value" }; + + expect( + collectPluginConfigContractMatches({ root: { headers }, pathPattern: "headers.*" }), + ).toEqual([{ path, value: "value", parent: headers, key }]); + }); + + it("keeps dotted wildcard keys distinct from explicitly nested record keys", () => { + const root = { + "alpha.beta": { token: "dotted" }, + alpha: { beta: { token: "nested" } }, + }; + + expect( + collectPluginConfigContractMatches({ root, pathPattern: "*.token" }).map(({ path }) => path), + ).toEqual(['["alpha.beta"].token']); + expect( + collectPluginConfigContractMatches({ root, pathPattern: "*.*.token" }).map( + ({ path }) => path, + ), + ).toEqual(["alpha.beta.token"]); + }); + it("rejects array indexes outside canonical config path bounds", () => { const items = Array(100_002); items[100_001] = "too far"; diff --git a/src/secrets/apply.test.ts b/src/secrets/apply.test.ts index bc72858b0af9..a1b84762e804 100644 --- a/src/secrets/apply.test.ts +++ b/src/secrets/apply.test.ts @@ -1263,7 +1263,12 @@ describe("secrets apply", () => { openai: { ...createOpenAiProviderConfig(), headers: { - "x-api-key": "sk-header-plaintext", + "X.Trace": "sk-header-plaintext", + }, + request: { + headers: { + "X.Request.Trace": "sk-request-header-plaintext", + }, }, }, }, @@ -1273,8 +1278,23 @@ describe("secrets apply", () => { const plan = createPlan({ targets: [ createOpenAiProviderHeaderTarget({ - pathSegments: ["models", "providers", "openai", "headers", "x-api-key"], + path: 'models.providers.openai.headers["X.Trace"]', + pathSegments: ["models", "providers", "openai", "headers", "X.Trace"], }), + { + ...createOpenAiProviderHeaderTarget({ + path: 'models.providers.openai.request.headers["X.Request.Trace"]', + pathSegments: [ + "models", + "providers", + "openai", + "request", + "headers", + "X.Request.Trace", + ], + }), + type: "models.providers.request.headers", + }, ], options: { scrubEnv: false, @@ -1291,11 +1311,15 @@ describe("secrets apply", () => { providers?: { openai?: { headers?: Record; + request?: { headers?: Record }; }; }; }; }; - expect(nextConfig.models?.providers?.openai?.headers?.["x-api-key"]).toEqual( + expect(nextConfig.models?.providers?.openai?.headers?.["X.Trace"]).toEqual( + OPENAI_API_KEY_ENV_REF, + ); + expect(nextConfig.models?.providers?.openai?.request?.headers?.["X.Request.Trace"]).toEqual( OPENAI_API_KEY_ENV_REF, ); }); diff --git a/src/secrets/apply.ts b/src/secrets/apply.ts index af6c68c64c83..1c5202116e7b 100644 --- a/src/secrets/apply.ts +++ b/src/secrets/apply.ts @@ -420,11 +420,11 @@ function applyConfigTargetMutations(params: { if (isNonEmptyString(previous)) { scrubbedValues.add(previous.trim()); } - const refPathSegments = resolved.refPathSegments; - if (!refPathSegments) { + const refPathTokens = resolved.refPathTokens; + if (!refPathTokens) { throw new Error(`Missing sibling ref path for target ${target.type}.`); } - const wroteRef = setPathCreateStrict(params.nextConfig, refPathSegments, target.ref); + const wroteRef = setPathCreateStrict(params.nextConfig, refPathTokens, target.ref); const deletedLegacy = deletePathStrict(params.nextConfig, targetPathSegments); if (wroteRef || deletedLegacy) { configChanged = true; @@ -436,7 +436,7 @@ function applyConfigTargetMutations(params: { if (isNonEmptyString(previous)) { scrubbedValues.add(previous.trim()); } - const wroteRef = setPathCreateStrict(params.nextConfig, targetPathSegments, target.ref); + const wroteRef = setPathCreateStrict(params.nextConfig, resolved.pathTokens, target.ref); if (wroteRef) { configChanged = true; } @@ -678,11 +678,11 @@ function applyAuthProfileTargetMutation(params: { if (isNonEmptyString(previous)) { params.scrubbedValues.add(previous.trim()); } - const refPathSegments = params.resolved.refPathSegments; - if (!refPathSegments) { + const refPathTokens = params.resolved.refPathTokens; + if (!refPathTokens) { throw new Error(`Missing sibling ref path for auth-profiles target ${params.target.path}.`); } - const wroteRef = setPathCreateStrict(store, refPathSegments, params.target.ref); + const wroteRef = setPathCreateStrict(store, refPathTokens, params.target.ref); const deletedPlaintext = deletePathStrict(store, targetPathSegments); changed = changed || wroteRef || deletedPlaintext; return changed; @@ -691,7 +691,7 @@ function applyAuthProfileTargetMutation(params: { if (isNonEmptyString(previous)) { params.scrubbedValues.add(previous.trim()); } - const wroteRef = setPathCreateStrict(store, targetPathSegments, params.target.ref); + const wroteRef = setPathCreateStrict(store, params.resolved.pathTokens, params.target.ref); changed = changed || wroteRef; return changed; } diff --git a/src/secrets/exec-secret-ref-id-parity.test.ts b/src/secrets/exec-secret-ref-id-parity.test.ts index 9f00d01ad543..06f9095c858a 100644 --- a/src/secrets/exec-secret-ref-id-parity.test.ts +++ b/src/secrets/exec-secret-ref-id-parity.test.ts @@ -237,13 +237,13 @@ describe("exec SecretRef id parity", () => { if (token.kind === "literal") { return []; } - return [token.kind === "array" ? "0" : "sample"]; + return [token.kind === "array" ? 0 : "sample"]; }); const segments = materializePathTokens(tokens, captures); if (!segments) { throw new Error(`failed to sample path segments for pattern "${entry.pathPattern}"`); } - return segments; + return segments.map(String); } const registryPlanTargets = listSecretTargetRegistryEntries().filter( diff --git a/src/secrets/path-utils.test.ts b/src/secrets/path-utils.test.ts index bfac82b3e0bd..c0cd63156136 100644 --- a/src/secrets/path-utils.test.ts +++ b/src/secrets/path-utils.test.ts @@ -49,9 +49,9 @@ describe("secrets path utils", () => { const config = createAgentListConfig(); expect(() => - setPathCreateStrict(config, ["agents", "list", "9007199254740993", "id"], "b"), + setPathCreateStrict(config, ["agents", "list", Number.MAX_SAFE_INTEGER + 2, "id"], "b"), ).toThrow(/Invalid array index segment/); - expect(() => setPathCreateStrict(config, ["agents", "list", "4294967294", "id"], "b")).toThrow( + expect(() => setPathCreateStrict(config, ["agents", "list", 4294967294, "id"], "b")).toThrow( /Invalid array index segment/, ); expect(() => setPathCreateStrict(config, ["agents", "list", "+0", "id"], "b")).toThrow( @@ -119,6 +119,33 @@ describe("secrets path utils", () => { expect(getPath(config, ["talk", "provider", "apiKey"])).toBe("x"); }); + it.each([ + { + name: "array index", + segment: 0, + expected: { accounts: [{ token: "secret" }] }, + mismatched: { accounts: { "0": { token: "old" } } }, + }, + { + name: "numeric record key", + segment: "0", + expected: { accounts: { "0": { token: "secret" } } }, + mismatched: { accounts: [{ token: "old" }] }, + }, + ])( + "setPathCreateStrict preserves the $name container contract", + ({ segment, expected, mismatched }) => { + const config = asConfig({}); + + expect(setPathCreateStrict(config, ["accounts", segment, "token"], "secret")).toBe(true); + expect(config).toEqual(expected); + expect(() => + setPathCreateStrict(asConfig(mismatched), ["accounts", segment, "token"], "secret"), + ).toThrow(/Invalid path shape/); + expect(mismatched.accounts[0].token).toBe("old"); + }, + ); + it("setPathCreateStrict leaves value unchanged when equal", () => { const config = asConfig({ talk: { diff --git a/src/secrets/path-utils.ts b/src/secrets/path-utils.ts index 409c89df7892..589754eace74 100644 --- a/src/secrets/path-utils.ts +++ b/src/secrets/path-utils.ts @@ -1,13 +1,10 @@ /** Strict dotted-path get/set/delete helpers for secrets migration targets. */ import { isDeepStrictEqual } from "node:util"; import { isBlockedObjectKey } from "../infra/prototype-keys.js"; +import type { ConcreteConfigPathSegment } from "../shared/dot-path.js"; import { parseConfigPathArrayIndex } from "../shared/path-array-index.js"; import { isRecord } from "./shared.js"; -function looksLikeArrayIndexSegment(segment: string): boolean { - return /^\d+$/.test(segment); -} - function parseArrayIndexSegment(segment: string): number | undefined { return parseConfigPathArrayIndex(segment); } @@ -20,15 +17,13 @@ function requireArrayIndexSegment(segment: string, pathLabel: string): number { return index; } -function expectedContainer(nextSegment: string): "array" | "object" { - return looksLikeArrayIndexSegment(nextSegment) ? "array" : "object"; -} - -function assertSafeMutationPath(segments: string[]): void { +function assertSafeMutationPath(segments: readonly ConcreteConfigPathSegment[]): void { if (segments.length === 0) { throw new Error("Target path is empty."); } - const blockedSegment = segments.find(isBlockedObjectKey); + const blockedSegment = segments.find( + (segment) => typeof segment === "string" && isBlockedObjectKey(segment), + ); if (blockedSegment) { throw new Error(`Refusing to mutate prototype-polluting path segment "${blockedSegment}".`); } @@ -36,13 +31,13 @@ function assertSafeMutationPath(segments: string[]): void { function parseArrayLeafTarget( cursor: unknown, - leaf: string, - segments: string[], + leaf: ConcreteConfigPathSegment, + segments: readonly ConcreteConfigPathSegment[], ): { array: unknown[]; index: number } | null { if (!Array.isArray(cursor)) { return null; } - return { array: cursor, index: requireArrayIndexSegment(leaf, segments.join(".")) }; + return { array: cursor, index: requireArrayIndexSegment(String(leaf), segments.join(".")) }; } function traverseToLeafParent(params: { @@ -109,12 +104,11 @@ export function getPath(root: unknown, segments: string[]): unknown { } /** - * Sets a config path, creating missing object or array containers from the next path segment. - * Existing non-container parents fail so callers cannot silently change config shape. + * Sets a config path using token types as the sole authority for object-versus-array shape. */ export function setPathCreateStrict( root: Record, - segments: string[], + segments: readonly ConcreteConfigPathSegment[], value: unknown, ): boolean { assertSafeMutationPath(segments); @@ -123,38 +117,41 @@ export function setPathCreateStrict( for (let index = 0; index < segments.length - 1; index += 1) { const segment = segments[index] ?? ""; - const nextSegment = segments[index + 1] ?? ""; - const needs = expectedContainer(nextSegment); + const needsArray = typeof segments[index + 1] === "number"; - // Numeric next segments create arrays; named next segments create objects. - // This keeps registry wildcard paths and config array paths materialized consistently. if (Array.isArray(cursor)) { - const arrayIndex = requireArrayIndexSegment(segment, segments.join(".")); + if (typeof segment !== "number") { + throw new Error(`Invalid path shape at ${segments.slice(0, index).join(".") || ""}.`); + } + const arrayIndex = requireArrayIndexSegment(String(segment), segments.join(".")); const existing = cursor[arrayIndex]; if (existing === undefined || existing === null) { - cursor[arrayIndex] = needs === "array" ? [] : {}; + cursor[arrayIndex] = needsArray ? [] : {}; changed = true; - } else if (needs === "array" ? !Array.isArray(existing) : !isRecord(existing)) { + } else if (needsArray ? !Array.isArray(existing) : !isRecord(existing)) { throw new Error(`Invalid path shape at ${segments.slice(0, index + 1).join(".")}.`); } cursor = cursor[arrayIndex]; continue; } - if (!isRecord(cursor)) { + if (!isRecord(cursor) || typeof segment !== "string") { throw new Error(`Invalid path shape at ${segments.slice(0, index).join(".") || ""}.`); } const existing = cursor[segment]; if (existing === undefined || existing === null) { - cursor[segment] = needs === "array" ? [] : {}; + cursor[segment] = needsArray ? [] : {}; changed = true; - } else if (needs === "array" ? !Array.isArray(existing) : !isRecord(existing)) { + } else if (needsArray ? !Array.isArray(existing) : !isRecord(existing)) { throw new Error(`Invalid path shape at ${segments.slice(0, index + 1).join(".")}.`); } cursor = cursor[segment]; } const leaf = segments[segments.length - 1] ?? ""; + if (Array.isArray(cursor) !== (typeof leaf === "number")) { + throw new Error(`Invalid path shape at ${segments.slice(0, -1).join(".") || ""}.`); + } const arrayTarget = parseArrayLeafTarget(cursor, leaf, segments); if (arrayTarget) { if (!isDeepStrictEqual(arrayTarget.array[arrayTarget.index], value)) { @@ -163,7 +160,7 @@ export function setPathCreateStrict( } return changed; } - if (!isRecord(cursor)) { + if (!isRecord(cursor) || typeof leaf !== "string") { throw new Error(`Invalid path shape at ${segments.slice(0, -1).join(".") || ""}.`); } if (!isDeepStrictEqual(cursor[leaf], value)) { diff --git a/src/secrets/plan.ts b/src/secrets/plan.ts index 150f290881af..30a719112eb6 100644 --- a/src/secrets/plan.ts +++ b/src/secrets/plan.ts @@ -4,9 +4,11 @@ import { normalizeStringEntries } from "@openclaw/normalization-core/string-norm import type { SecretProviderConfig, SecretRef } from "../config/types.secrets.js"; import { SecretProviderSchema } from "../config/zod-schema.core.js"; import { isBlockedObjectKey } from "../infra/prototype-keys.js"; -import { toDotPath } from "../shared/dot-path.js"; +import { + parseConcreteConfigPathTokens, + type ConcreteConfigPathSegment, +} from "../shared/dot-path.js"; import { isValidSecretProviderAlias, isValidSecretRef } from "./ref-contract.js"; -import { parseDotPath } from "./shared.js"; import { resolvePlanTargetAgainstRegistry, type ResolvedPlanTarget } from "./target-registry.js"; /** Registry target id accepted by a secrets apply plan. */ @@ -83,11 +85,26 @@ export function resolveValidatedPlanTarget(candidate: { if (!path) { return null; } - const segments = - Array.isArray(candidate.pathSegments) && candidate.pathSegments.length > 0 + let parsedTokens: ConcreteConfigPathSegment[]; + let segments: string[]; + const hasPathSegments = + Array.isArray(candidate.pathSegments) && candidate.pathSegments.length > 0; + try { + parsedTokens = parseConcreteConfigPathTokens(path); + segments = hasPathSegments ? normalizeStringEntries(candidate.pathSegments) - : parseDotPath(path); - if (segments.length === 0 || segments.some(isBlockedObjectKey) || path !== toDotPath(segments)) { + : parsedTokens.map(String); + } catch { + return null; + } + const parsedPathMatches = + segments.length === parsedTokens.length && + segments.every((segment, index) => segment === String(parsedTokens[index])); + if ( + segments.length === 0 || + segments.some(isBlockedObjectKey) || + (!parsedPathMatches && path !== segments.join(".")) + ) { return null; } // Registry resolution is the ownership gate; caller-provided paths must map to a known @@ -95,6 +112,9 @@ export function resolveValidatedPlanTarget(candidate: { return resolvePlanTargetAgainstRegistry({ type: candidate.type, pathSegments: segments, + pathTokens: parsedPathMatches ? parsedTokens : segments, + // Only an authored array pattern can disambiguate indices in shipped v1 dotted plans. + allowLegacyArrayString: path === segments.join("."), providerId: candidate.providerId, accountId: candidate.accountId, }); diff --git a/src/secrets/plugin-setup-plan.ts b/src/secrets/plugin-setup-plan.ts index 90ecca800514..6cac60afc018 100644 --- a/src/secrets/plugin-setup-plan.ts +++ b/src/secrets/plugin-setup-plan.ts @@ -1,7 +1,7 @@ /** Shared plan construction for plugin-owned SecretRef setup commands. */ import { isValidAgentId } from "@openclaw/normalization-core/agent-id"; import type { PluginIntegrationSecretProviderConfig, SecretRef } from "../config/types.secrets.js"; -import { toDotPath } from "../shared/dot-path.js"; +import { formatConcreteConfigPath, parseConcreteConfigPathTokens } from "../shared/dot-path.js"; import type { SecretsApplyPlan, SecretsPlanTarget } from "./plan.js"; import { resolveSecretPlanTargetByPathCore } from "./target-registry-query.js"; @@ -18,14 +18,6 @@ type PluginSecretRefConfigTargetMapping = { const SECRET_PROVIDER_ALIAS_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; const MODEL_PROVIDER_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,127}$/; -const FORBIDDEN_PATH_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]); - -function parseDotPath(pathname: string): string[] { - return pathname - .split(".") - .map((segment) => segment.trim()) - .filter((segment) => segment.length > 0); -} export function assertValidPluginSecretProviderAlias(value: string): void { if (!SECRET_PROVIDER_ALIAS_PATTERN.test(value)) { @@ -89,18 +81,21 @@ function createPluginConfigSecretTarget(params: { if (params.agentId && !isValidAgentId(params.agentId)) { throw new Error(`Invalid ${params.productName} setup agent id: ${params.agentId}`); } - const pathSegments = parseDotPath(params.path); - const normalizedPath = toDotPath(pathSegments); - if ( - pathSegments.length === 0 || - normalizedPath !== params.path || - pathSegments.some((segment) => FORBIDDEN_PATH_SEGMENTS.has(segment)) - ) { + let parsedPath: Array; + try { + parsedPath = parseConcreteConfigPathTokens(params.path); + } catch { + throw new Error(`Invalid --target config path: ${params.path}`); + } + const pathSegments = parsedPath.map(String); + const normalizedPath = formatConcreteConfigPath(parsedPath); + if (normalizedPath !== params.path) { throw new Error(`Invalid --target config path: ${params.path}`); } const resolved = resolveSecretPlanTargetByPathCore({ configFile: params.agentId ? "auth-profile-store" : "openclaw.json", pathSegments, + pathTokens: parsedPath, }); if (!resolved) { throw new Error( diff --git a/src/secrets/runtime-auth-refresh-failure.test.ts b/src/secrets/runtime-auth-refresh-failure.test.ts index b495dfaeda4c..ec7795f2de9c 100644 --- a/src/secrets/runtime-auth-refresh-failure.test.ts +++ b/src/secrets/runtime-auth-refresh-failure.test.ts @@ -3,6 +3,11 @@ import fs from "node:fs/promises"; import os from "node:os"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { withTempHome } from "../config/home-env.test-harness.js"; +import { + createConfigResolutionFacts, + getAuthoredConfigSecretRef, + setConfigResolutionFacts, +} from "../config/resolution-facts.js"; import { resolveAuthProfileSecretOwnerId } from "./runtime-auth-profile-owner.js"; import { beginSecretsRuntimeIsolationForTest, @@ -136,6 +141,11 @@ describe("secrets runtime snapshot auth refresh failure", () => { loadablePluginOrigins: EMPTY_LOADABLE_PLUGIN_ORIGINS, loadAuthStore, }); + const discordTokenPath = "channels.discord.accounts.ops.token"; + setConfigResolutionFacts( + prepared.config, + createConfigResolutionFacts([], new Map([[discordTokenPath, "DISCORD_BOT_TOKEN"]])), + ); prepared.secretOwners = [ ...(prepared.secretOwners ?? []), { @@ -166,6 +176,9 @@ describe("secrets runtime snapshot auth refresh failure", () => { activeRef = secondRef; await expect(refreshActiveProviderAuthRuntimeSnapshot()).resolves.toBe(true); + expect( + getAuthoredConfigSecretRef(expectActiveSecretsRuntimeSnapshot().config, discordTokenPath), + ).toEqual({ source: "env", provider: "default", id: "DISCORD_BOT_TOKEN" }); expect(expectActiveSecretsRuntimeSnapshot().secretOwners).toContainEqual({ ownerKind: "account", ownerId: "discord:ops", diff --git a/src/secrets/runtime-config-collectors-plugins.bundled.test.ts b/src/secrets/runtime-config-collectors-plugins.bundled.test.ts index 2ca7ef181665..3cdd95208b98 100644 --- a/src/secrets/runtime-config-collectors-plugins.bundled.test.ts +++ b/src/secrets/runtime-config-collectors-plugins.bundled.test.ts @@ -16,6 +16,7 @@ function envRef(id: string) { const explicitMainRoster: NonNullable = { list: [{ id: "main", default: true }], }; +const isolatedEnv: NodeJS.ProcessEnv = { OPENCLAW_STATE_DIR: process.env.OPENCLAW_TEST_HOME }; describe("collectPluginConfigAssignments bundled plugin manifests", () => { it("assigns each webhooks route SecretRef to its exact runtime owner", () => { @@ -43,7 +44,7 @@ describe("collectPluginConfigAssignments bundled plugin manifests", () => { }, }, } as OpenClawConfig; - const context = createResolverContext({ sourceConfig: config, env: {} }); + const context = createResolverContext({ sourceConfig: config, env: isolatedEnv }); collectPluginConfigAssignments({ config, @@ -98,7 +99,7 @@ describe("collectPluginConfigAssignments bundled plugin manifests", () => { resolvePluginConfigContractsById({ config, workspaceDir: resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config)), - env: {}, + env: isolatedEnv, fallbackToBundledMetadata: true, fallbackToBundledMetadataForResolvedBundled: true, pluginIds: ["codex"], @@ -110,7 +111,7 @@ describe("collectPluginConfigAssignments bundled plugin manifests", () => { ]); const context = createResolverContext({ sourceConfig: config, - env: {}, + env: isolatedEnv, }); collectPluginConfigAssignments({ @@ -169,7 +170,7 @@ describe("collectPluginConfigAssignments bundled plugin manifests", () => { }, }, } as OpenClawConfig; - const env = { GEMINI_GATEWAY_TOKEN: "resolved-gateway-token" }; + const env = { ...isolatedEnv, GEMINI_GATEWAY_TOKEN: "resolved-gateway-token" }; const context = createResolverContext({ sourceConfig: config, env }); collectPluginConfigAssignments({ @@ -252,7 +253,7 @@ describe("collectPluginConfigAssignments bundled plugin manifests", () => { resolvePluginConfigContractsById({ config, workspaceDir: resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config)), - env: {}, + env: isolatedEnv, fallbackToBundledMetadata: true, fallbackToBundledMetadataForResolvedBundled: true, pluginIds: ["voice-call"], @@ -266,7 +267,7 @@ describe("collectPluginConfigAssignments bundled plugin manifests", () => { ]); const context = createResolverContext({ sourceConfig: config, - env: {}, + env: isolatedEnv, }); collectPluginConfigAssignments({ @@ -321,13 +322,13 @@ describe("collectPluginConfigAssignments bundled plugin manifests", () => { expect( resolvePluginConfigContractsById({ config, - env: {}, + env: isolatedEnv, pluginIds: ["google-meet"], }).get("google-meet")?.configContracts.secretInputs?.paths, ).toEqual([{ path: "realtime.providers.*.apiKey", expected: "string" }]); const context = createResolverContext({ sourceConfig: config, - env: {}, + env: isolatedEnv, }); collectPluginConfigAssignments({ diff --git a/src/secrets/runtime-config-collectors-plugins.test.ts b/src/secrets/runtime-config-collectors-plugins.test.ts index 169c8dcde7f6..3779902f4a63 100644 --- a/src/secrets/runtime-config-collectors-plugins.test.ts +++ b/src/secrets/runtime-config-collectors-plugins.test.ts @@ -36,7 +36,7 @@ function makeContext( ): ResolverContext { return createResolverContext({ sourceConfig, - env: {}, + env: { OPENCLAW_STATE_DIR: process.env.OPENCLAW_TEST_HOME }, ...(manifestRegistry ? { manifestRegistry } : {}), }); } @@ -164,6 +164,128 @@ describe("collectPluginConfigAssignments", () => { expect(assignment.expected).toBe("string"); }); + it("keeps dotted plugin identities and plugin-local route paths distinct", () => { + const pluginIds = ["foo.config.bar", "foo"] as const; + loadPluginManifestRegistryForPluginRegistryMock.mockReturnValue({ + plugins: pluginIds.map((id) => ({ + id, + origin: "config", + configContracts: { + secretInputs: { + paths: [ + { + path: id === "foo" ? "bar.config.token" : "token", + ownerKind: "route", + }, + ], + }, + }, + })), + diagnostics: [], + }); + + const context = collectAssignments( + asConfig({ + plugins: { + entries: { + "foo.config.bar": { enabled: true, config: { token: envRef("DOTTED_TOKEN") } }, + foo: { + enabled: true, + config: { bar: { config: { token: envRef("NESTED_TOKEN") } } }, + }, + }, + }, + }), + pluginIds.map((id) => [id, "config"]), + ); + + expect(context.assignments).toMatchObject([ + { + path: 'plugins.entries["foo.config.bar"].config.token', + ownerKind: "route", + ownerId: 'plugins.entries["foo.config.bar"].config.token', + ref: { id: "DOTTED_TOKEN" }, + }, + { + path: "plugins.entries.foo.config.bar.config.token", + ownerKind: "route", + ownerId: "plugins.entries.foo.config.bar.config.token", + ref: { id: "NESTED_TOKEN" }, + }, + ]); + expect(new Set(context.assignments.map(({ ownerId }) => ownerId)).size).toBe(2); + }); + + it("collects and applies dotted wildcard keys separately from nested record keys", () => { + loadPluginManifestRegistryForPluginRegistryMock.mockReturnValue({ + plugins: [ + { + id: "distinct-keys", + origin: "config", + configContracts: { + secretInputs: { + paths: [{ path: "*.token" }, { path: "*.*.token" }], + }, + }, + }, + ], + diagnostics: [], + }); + const config = createPluginConfig("distinct-keys", { + "alpha.beta": { token: envRef("DOTTED_TOKEN") }, + alpha: { beta: { token: envRef("NESTED_TOKEN") } }, + }); + const context = collectAssignments(config, [["distinct-keys", "config"]]); + + expect(context.assignments).toMatchObject([ + { + path: 'plugins.entries.distinct-keys.config["alpha.beta"].token', + ref: { id: "DOTTED_TOKEN" }, + }, + { + path: "plugins.entries.distinct-keys.config.alpha.beta.token", + ref: { id: "NESTED_TOKEN" }, + }, + ]); + requireAssignment(context, 0).apply("resolved-dotted"); + requireAssignment(context, 1).apply("resolved-nested"); + expect(config.plugins?.entries?.["distinct-keys"]?.config).toMatchObject({ + "alpha.beta": { token: "resolved-dotted" }, + alpha: { beta: { token: "resolved-nested" } }, + }); + }); + + it("keeps installed web-provider headers unknown while applying exact dotted keys", () => { + loadPluginManifestRegistryForPluginRegistryMock.mockReturnValue({ + plugins: [ + { + id: "custom-search", + origin: "config", + contracts: { webSearchProviders: ["custom-search"] }, + configContracts: { + secretInputs: { paths: [{ path: "webSearch.headers.*", expected: "string" }] }, + }, + }, + ], + diagnostics: [], + }); + const config = createPluginConfig("custom-search", { + webSearch: { headers: { "X.Trace": envRef("CUSTOM_TRACE") } }, + }); + const context = collectAssignments(config, [["custom-search", "config"]]); + + expect(context.assignments).toMatchObject([ + { + path: 'plugins.entries.custom-search.config.webSearch.headers["X.Trace"]', + ownerKind: "unknown", + }, + ]); + requireAssignment(context, 0).apply("resolved-trace"); + expect(config.plugins?.entries?.["custom-search"]?.config).toMatchObject({ + webSearch: { headers: { "X.Trace": "resolved-trace" } }, + }); + }); + it("collects contracts from a secondary agent workspace registry", () => { loadPluginManifestRegistryForPluginRegistryMock.mockReturnValue({ plugins: [ diff --git a/src/secrets/runtime-config-collectors-plugins.ts b/src/secrets/runtime-config-collectors-plugins.ts index b6876a70c691..2fbc64b7d46e 100644 --- a/src/secrets/runtime-config-collectors-plugins.ts +++ b/src/secrets/runtime-config-collectors-plugins.ts @@ -1,5 +1,4 @@ /** Collects plugin config secret refs from runtime plugin metadata. */ -import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization"; import { resolveConfigWidePluginManifestRegistry } from "../config/io.plugin-metadata.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { @@ -8,7 +7,7 @@ import { } from "../plugins/config-contracts.js"; import { normalizePluginsConfig, resolveEnableState } from "../plugins/config-state.js"; import type { PluginOrigin } from "../plugins/plugin-origin.types.js"; -import { parseConfigPathArrayIndex } from "../shared/path-array-index.js"; +import { formatConcreteConfigPath } from "../shared/dot-path.js"; import { collectRuntimeSecretInputAssignment, type ResolverContext, @@ -16,10 +15,6 @@ import { } from "./runtime-shared.js"; import { isRecord } from "./shared.js"; -function parsePluginConfigArrayIndex(segment: string): number | undefined { - return parseConfigPathArrayIndex(segment); -} - /** * Walk manifest-declared plugin config SecretRef surfaces and collect * assignments for runtime materialization. Plugin-owned metadata controls which @@ -143,13 +138,20 @@ function collectConfiguredPluginSecretAssignments(params: { defaults: SecretDefaults | undefined; context: ResolverContext; }): void { + const pluginConfigPath = formatConcreteConfigPath([ + "plugins", + "entries", + params.pluginId, + "config", + ]); const seenPaths = new Set(); for (const secretPath of params.secretPaths) { for (const match of collectPluginConfigContractMatches({ root: params.pluginConfig, pathPattern: secretPath.path, })) { - const fullPath = `plugins.entries.${params.pluginId}.config.${match.path}`; + const relativePath = match.path.startsWith("[") ? match.path : `.${match.path}`; + const fullPath = `${pluginConfigPath}${relativePath}`; if (seenPaths.has(fullPath)) { continue; } @@ -177,44 +179,10 @@ function collectConfiguredPluginSecretAssignments(params: { }, } : {}), - apply: createPluginConfigAssignmentApply(params.pluginConfig, match.path), + apply: (value) => { + Reflect.set(match.parent, match.key, value); + }, }); } } } - -function createPluginConfigAssignmentApply( - pluginConfig: Record, - relativePath: string, -): (value: unknown) => void { - return (value) => { - // Manifest paths use dotted/bracket notation; assignment writes need concrete object/array steps. - const segments = normalizeStringEntries(relativePath.replace(/\[(\d+)\]/g, ".$1").split(".")); - if (segments.length === 0) { - return; - } - let current: unknown = pluginConfig; - for (const segment of segments.slice(0, -1)) { - if (Array.isArray(current)) { - const index = parsePluginConfigArrayIndex(segment); - current = index !== undefined && index < current.length ? current[index] : undefined; - continue; - } - current = isRecord(current) ? current[segment] : undefined; - } - const finalSegment = segments.at(-1); - if (!finalSegment) { - return; - } - if (Array.isArray(current)) { - const index = parsePluginConfigArrayIndex(finalSegment); - if (index !== undefined && index < current.length) { - current[index] = value; - } - return; - } - if (isRecord(current)) { - current[finalSegment] = value; - } - }; -} diff --git a/src/secrets/runtime.coverage.test.ts b/src/secrets/runtime.coverage.test.ts index 9c412bc736fa..d121dbedc408 100644 --- a/src/secrets/runtime.coverage.test.ts +++ b/src/secrets/runtime.coverage.test.ts @@ -418,7 +418,7 @@ function addCoveragePluginLoadPath(config: OpenClawConfig, pluginId: string): vo return; } const nextIndex = Array.isArray(existing) ? existing.length : 0; - setPathCreateStrict(config, ["plugins", "load", "paths", String(nextIndex)], loadPath); + setPathCreateStrict(config, ["plugins", "load", "paths", nextIndex], loadPath); } function resolveCoverageLoadablePluginOrigins( @@ -578,7 +578,7 @@ function applyConfigForOpenClawTarget( } } if (entry.id === "memory.search.remote.apiKey") { - setPathCreateStrict(config, ["agents", "list", "0", "id"], "sample-agent"); + setPathCreateStrict(config, ["agents", "list", 0, "id"], "sample-agent"); } if (entry.id === "gateway.auth.password") { setPathCreateStrict(config, ["gateway", "auth", "mode"], "password"); @@ -848,7 +848,7 @@ async function expectOpenClawCoverageBatchResolved( ): Promise { logCoverageBatch(label, batch); const config = {} as OpenClawConfig; - const env: Record = {}; + const env: NodeJS.ProcessEnv = { OPENCLAW_STATE_DIR: process.env.OPENCLAW_TEST_HOME }; for (const [index, entry] of batch.entries()) { const envId = toCoverageEnvRefId("OPENCLAW_SECRET_TARGET", entry.id); const runtimeEnvId = resolveCoverageEnvId(entry, envId); diff --git a/src/secrets/runtime.ts b/src/secrets/runtime.ts index 08b4dead17ca..63d91e2d3fd9 100644 --- a/src/secrets/runtime.ts +++ b/src/secrets/runtime.ts @@ -655,7 +655,7 @@ export async function refreshActiveProviderAuthRuntimeSnapshot(): Promise ({ listBundledPluginMetadata: metadataMocks.listBundledPluginMetadata, })); -vi.mock("../plugins/plugin-metadata-snapshot.js", () => ({ +vi.mock("../plugins/plugin-metadata-snapshot.js", async (importOriginal) => ({ + ...(await importOriginal()), resolvePluginMetadataSnapshot: metadataMocks.resolvePluginMetadataSnapshot, })); @@ -132,6 +133,256 @@ describe("getSecretTargetRegistry metadata reuse", () => { expect(metadataMocks.listBundledPluginMetadata).not.toHaveBeenCalled(); }); + it("preserves plugin, array, and record identity across discovery, setup, and apply", async () => { + const rootDir = makeTrackedTempDir("openclaw-target-registry-plugin-identity", tempDirs); + const pluginContracts = [ + { id: "foo.config.bar", secretPath: "token", refId: "DOTTED_PLUGIN_TOKEN" }, + { id: "foo", secretPath: "bar.config.token", refId: "NESTED_PLUGIN_TOKEN" }, + { id: "array-plugin", secretPath: "accounts[].token", refId: "ARRAY_PLUGIN_TOKEN" }, + { id: "record-plugin", secretPath: "accounts.*.token", refId: "RECORD_PLUGIN_TOKEN" }, + { + id: "wildcard-array-plugin", + secretPath: "accounts.*.token", + refId: "WILDCARD_ARRAY_PLUGIN_TOKEN", + }, + ]; + const plugins = pluginContracts.map(({ id, secretPath }, index) => { + const pluginRoot = path.join(rootDir, `plugin-${index}`); + fs.mkdirSync(pluginRoot); + fs.writeFileSync( + path.join(pluginRoot, "index.js"), + `export default { id: ${JSON.stringify(id)}, register() {} };`, + ); + const manifest = { + id, + configSchema: { type: "object", additionalProperties: true }, + configContracts: { secretInputs: { paths: [{ path: secretPath }] } }, + }; + fs.writeFileSync(path.join(pluginRoot, "openclaw.plugin.json"), JSON.stringify(manifest)); + return { ...manifest, origin: "config", channels: [], rootDir: pluginRoot }; + }); + metadataMocks.resolvePluginMetadataSnapshot.mockReturnValue({ + plugins, + manifestRegistry: { plugins, diagnostics: [] }, + } as never); + + const config = { + plugins: { + load: { paths: plugins.map((plugin) => plugin.rootDir) }, + entries: { + "foo.config.bar": { enabled: true, config: { token: "dotted-plaintext" } }, + foo: { enabled: true, config: { bar: { config: { token: "nested-plaintext" } } } }, + "array-plugin": { enabled: true, config: { accounts: [{ token: "array-plaintext" }] } }, + "record-plugin": { + enabled: true, + config: { + accounts: { + "0": { token: "numeric-record-plaintext" }, + "foo.bar": { token: "dotted-record-plaintext" }, + }, + }, + }, + "wildcard-array-plugin": { + enabled: true, + config: { accounts: [{ token: "wildcard-array-plaintext" }] }, + }, + }, + }, + }; + const expectedTargets = [ + { + path: 'plugins.entries["foo.config.bar"].config.token', + pathSegments: ["plugins", "entries", "foo.config.bar", "config", "token"], + }, + { + path: "plugins.entries.foo.config.bar.config.token", + pathSegments: ["plugins", "entries", "foo", "config", "bar", "config", "token"], + }, + { + path: "plugins.entries.array-plugin.config.accounts[0].token", + pathSegments: ["plugins", "entries", "array-plugin", "config", "accounts", "0", "token"], + }, + { + path: 'plugins.entries.record-plugin.config.accounts["0"].token', + pathSegments: ["plugins", "entries", "record-plugin", "config", "accounts", "0", "token"], + }, + { + path: 'plugins.entries.record-plugin.config.accounts["foo.bar"].token', + pathSegments: [ + "plugins", + "entries", + "record-plugin", + "config", + "accounts", + "foo.bar", + "token", + ], + }, + { + path: "plugins.entries.wildcard-array-plugin.config.accounts[0].token", + pathSegments: [ + "plugins", + "entries", + "wildcard-array-plugin", + "config", + "accounts", + "0", + "token", + ], + }, + ]; + const { getSecretTargetRegistry } = await import("./target-registry-data.js"); + const { discoverConfigSecretTargets } = await import("./target-registry-query.js"); + const { buildConfigureCandidatesForScope, buildSecretsConfigurePlan } = + await import("./configure-plan.js"); + const { isSecretsApplyPlan, resolveValidatedPlanTarget } = await import("./plan.js"); + + expect(getSecretTargetRegistry({ config, env: {} })).toEqual( + expect.arrayContaining( + pluginContracts.map(({ id, secretPath }) => { + const pluginPath = id.includes(".") ? `[${JSON.stringify(id)}]` : `.${id}`; + const pathPattern = `plugins.entries${pluginPath}.config.${secretPath}`; + return expect.objectContaining({ + id: pathPattern, + pathPattern, + pathPatternSegments: ["plugins", "entries", id, "config", ...secretPath.split(".")], + }); + }), + ), + ); + expect(discoverConfigSecretTargets(config)).toEqual( + expect.arrayContaining(expectedTargets.map((target) => expect.objectContaining(target))), + ); + + const candidates = buildConfigureCandidatesForScope({ config }).filter((candidate) => + candidate.path.startsWith("plugins.entries"), + ); + expect(candidates).toEqual( + expect.arrayContaining(expectedTargets.map((target) => expect.objectContaining(target))), + ); + const plan = buildSecretsConfigurePlan({ + selectedTargets: new Map( + candidates.map((candidate) => { + const plugin = pluginContracts.find(({ id }) => candidate.pathSegments[2] === id)!; + return [ + candidate.path, + { + ...candidate, + ref: { source: "env" as const, provider: "default", id: plugin.refId }, + }, + ]; + }), + ), + providerChanges: { upserts: {}, deletes: [] }, + }); + expect(plan.targets).toHaveLength(expectedTargets.length); + const arrayTarget = plan.targets.find((target) => target.pathSegments?.[2] === "array-plugin")!; + const recordTarget = plan.targets.find( + (target) => target.pathSegments?.[2] === "record-plugin" && target.pathSegments[5] === "0", + )!; + const wildcardArrayTarget = plan.targets.find( + (target) => target.pathSegments?.[2] === "wildcard-array-plugin", + )!; + const dottedPluginTarget = plan.targets.find( + (target) => target.pathSegments?.[2] === "foo.config.bar", + )!; + const quotedArrayPath = 'plugins.entries.array-plugin.config.accounts["0"].token'; + const legacyArrayTarget = { + ...arrayTarget, + path: arrayTarget.pathSegments!.join("."), + }; + const legacyRecordTarget = { + ...recordTarget, + path: recordTarget.pathSegments!.join("."), + }; + expect(resolveValidatedPlanTarget(arrayTarget)?.pathTokens[5]).toBe(0); + expect(resolveValidatedPlanTarget(recordTarget)?.pathTokens[5]).toBe("0"); + expect(resolveValidatedPlanTarget(wildcardArrayTarget)?.pathTokens[5]).toBe(0); + expect(Object.hasOwn(arrayTarget, "pathTokens")).toBe(false); + expect(resolveValidatedPlanTarget({ ...arrayTarget, path: quotedArrayPath })).toBeNull(); + expect(resolveValidatedPlanTarget(legacyArrayTarget)?.pathTokens[5]).toBe(0); + expect(resolveValidatedPlanTarget(legacyRecordTarget)?.pathTokens[5]).toBe("0"); + expect( + resolveValidatedPlanTarget({ ...legacyArrayTarget, pathSegments: undefined })?.pathTokens[5], + ).toBe(0); + expect( + resolveValidatedPlanTarget({ ...legacyRecordTarget, pathSegments: undefined })?.pathTokens[5], + ).toBe("0"); + expect( + resolveValidatedPlanTarget({ + ...dottedPluginTarget, + path: dottedPluginTarget.pathSegments!.join("."), + }), + ).not.toBeNull(); + const forgedArrayPlan = { + ...plan, + generatedBy: "manual" as const, + targets: [{ ...arrayTarget, path: quotedArrayPath }], + }; + expect(isSecretsApplyPlan(forgedArrayPlan)).toBe(false); + const { buildPluginSecretRefSetupPlan } = await import("./plugin-setup-plan.js"); + const setupPlan = buildPluginSecretRefSetupPlan({ + productName: "Fixture", + providerAlias: "fixture", + providerConfig: { + source: "exec", + pluginIntegration: { pluginId: "fixture", integrationId: "fixture" }, + }, + providerSecrets: [], + configTargetSecrets: expectedTargets.map(({ path: targetPath }, index) => ({ + path: targetPath, + secretId: `credentials/${index}`, + })), + }); + expect(setupPlan.targets).toEqual( + expect.arrayContaining(expectedTargets.map((target) => expect.objectContaining(target))), + ); + expect(isSecretsApplyPlan(plan)).toBe(true); + + const configPath = path.join(rootDir, "openclaw.json"); + fs.writeFileSync(configPath, JSON.stringify(config)); + const { testing } = await import("./apply.js"); + const env = { + OPENCLAW_STATE_DIR: rootDir, + OPENCLAW_CONFIG_PATH: configPath, + DOTTED_PLUGIN_TOKEN: "dotted-secret", + NESTED_PLUGIN_TOKEN: "nested-secret", + ARRAY_PLUGIN_TOKEN: "array-secret", + RECORD_PLUGIN_TOKEN: "record-secret", + WILDCARD_ARRAY_PLUGIN_TOKEN: "wildcard-array-secret", + }; + await expect(testing.projectConfigForTest({ plan: forgedArrayPlan, env })).rejects.toThrow( + /Invalid plan target path/, + ); + const projected = await testing.projectConfigForTest({ + plan, + env, + }); + + expect(projected.plugins?.entries?.["foo.config.bar"]?.config).toEqual({ + token: { source: "env", provider: "default", id: "DOTTED_PLUGIN_TOKEN" }, + }); + expect(projected.plugins?.entries?.foo?.config).toEqual({ + bar: { + config: { token: { source: "env", provider: "default", id: "NESTED_PLUGIN_TOKEN" } }, + }, + }); + expect(projected.plugins?.entries?.["array-plugin"]?.config).toEqual({ + accounts: [{ token: { source: "env", provider: "default", id: "ARRAY_PLUGIN_TOKEN" } }], + }); + expect(projected.plugins?.entries?.["record-plugin"]?.config).toEqual({ + accounts: { + "0": { token: { source: "env", provider: "default", id: "RECORD_PLUGIN_TOKEN" } }, + "foo.bar": { token: { source: "env", provider: "default", id: "RECORD_PLUGIN_TOKEN" } }, + }, + }); + expect(projected.plugins?.entries?.["wildcard-array-plugin"]?.config).toEqual({ + accounts: [ + { token: { source: "env", provider: "default", id: "WILDCARD_ARRAY_PLUGIN_TOKEN" } }, + ], + }); + }); + it("keeps official external channel secret targets without installed plugin metadata", async () => { const { getSecretTargetRegistry } = await import("./target-registry-data.js"); diff --git a/src/secrets/target-registry-data.ts b/src/secrets/target-registry-data.ts index 7656d5bbea90..8e1d19e2b8b7 100644 --- a/src/secrets/target-registry-data.ts +++ b/src/secrets/target-registry-data.ts @@ -2,8 +2,10 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { PluginManifestRecord } from "../plugins/manifest-registry.js"; import { resolvePluginMetadataSnapshot } from "../plugins/plugin-metadata-snapshot.js"; +import { formatConcreteConfigPath } from "../shared/dot-path.js"; import { loadChannelSecretContractApiForRecord } from "./channel-contract-api.js"; import { listOfficialExternalChannelSecretTargetRegistryEntries } from "./official-external-channel-secret-contract.js"; +import { parseDotPath } from "./shared.js"; import type { SecretTargetRegistryEntry } from "./target-registry-types.js"; const SECRET_INPUT_SHAPE = "secret_input"; // pragma: allowlist secret @@ -20,14 +22,15 @@ function createPluginOpenClawConfigSecretTargetEntry( pluginId: string, configPath: string, ): SecretTargetRegistryEntry { - const pathPattern = ["plugins", "entries", pluginId, "config", ...configPath.split(".")].join( - ".", - ); + const pluginConfigPath = ["plugins", "entries", pluginId, "config"]; + const pathPatternSegments = [...pluginConfigPath, ...parseDotPath(configPath)]; + const pathPattern = `${formatConcreteConfigPath(pluginConfigPath)}.${configPath}`; return { id: pathPattern, targetType: pathPattern, configFile: "openclaw.json", pathPattern, + pathPatternSegments, secretShape: SECRET_INPUT_SHAPE, expectedResolvedValue: "string", includeInPlan: true, diff --git a/src/secrets/target-registry-pattern.test.ts b/src/secrets/target-registry-pattern.test.ts index f8c1a54a8e9b..cbb60680d4f8 100644 --- a/src/secrets/target-registry-pattern.test.ts +++ b/src/secrets/target-registry-pattern.test.ts @@ -26,13 +26,26 @@ describe("target registry pattern helpers", () => { it("matches wildcard and array tokens with stable capture ordering", () => { const tokens = compilePattern("agents.list[].memory.search.providers.*.apiKey").pathTokens; const match = matchPathTokens( - ["agents", "list", "2", "memory", "search", "providers", "openai", "apiKey"], + ["agents", "list", 2, "memory", "search", "providers", "openai", "apiKey"], tokens, ); expect(match).toEqual({ - captures: ["2", "openai"], + captures: [2, "openai"], }); + expect( + matchPathTokens( + ["agents", "list", "2", "memory", "search", "providers", "openai", "apiKey"], + tokens, + { allowLegacyArrayString: true }, + ), + ).toEqual({ captures: [2, "openai"] }); + expect( + matchPathTokens( + ["agents", "list", "2", "memory", "search", "providers", "openai", "apiKey"], + tokens, + ), + ).toBeNull(); expect( matchPathTokens( ["agents", "list", "x", "memory", "search", "providers", "openai", "apiKey"], @@ -65,10 +78,10 @@ describe("target registry pattern helpers", () => { "agents.list[].memory.search.providers.*.apiKeyRef", ).refPathTokens; expect(refTokens).toBeDefined(); - expect(materializePathTokens(refTokens ?? [], ["1", "anthropic"])).toEqual([ + expect(materializePathTokens(refTokens ?? [], [1, "anthropic"])).toEqual([ "agents", "list", - "1", + 1, "memory", "search", "providers", @@ -76,6 +89,7 @@ describe("target registry pattern helpers", () => { "apiKeyRef", ]); expect(materializePathTokens(refTokens ?? [], ["anthropic"])).toBeNull(); + expect(materializePathTokens(refTokens ?? [], ["1", "anthropic"])).toBeNull(); expect(materializePathTokens(refTokens ?? [], ["01", "anthropic"])).toBeNull(); expect(materializePathTokens(refTokens ?? [], ["+1", "anthropic"])).toBeNull(); expect(materializePathTokens(refTokens ?? [], ["4294967294", "anthropic"])).toBeNull(); @@ -92,6 +106,48 @@ describe("target registry pattern helpers", () => { }); }); + it("keeps wildcard record keys distinct from array indices without excluding arrays", () => { + const tokens = compilePattern("accounts.*.token").pathTokens; + + expect(matchPathTokens(["accounts", "0", "token"], tokens)).toEqual({ captures: ["0"] }); + expect(matchPathTokens(["accounts", 0, "token"], tokens)).toEqual({ captures: [0] }); + expect( + matchPathTokens(["accounts", 0, "token"], compilePattern("accounts.0.token").pathTokens), + ).toBeNull(); + }); + + it("normalizes legacy numeric strings only for declared array captures", () => { + const arrayTokens = compilePattern("accounts[].token").pathTokens; + const wildcardTokens = compilePattern("accounts.*.token").pathTokens; + const options = { allowLegacyArrayString: true }; + + expect(matchPathTokens(["accounts", "0", "token"], arrayTokens)).toBeNull(); + expect(matchPathTokens(["accounts", "0", "token"], arrayTokens, options)).toEqual({ + captures: [0], + }); + expect(matchPathTokens(["accounts", "0", "token"], wildcardTokens, options)).toEqual({ + captures: ["0"], + }); + for (const invalid of ["01", "+1", "4294967294"]) { + expect(matchPathTokens(["accounts", invalid, "token"], arrayTokens, options)).toBeNull(); + } + }); + + it("materializes wildcard sibling ref paths with their original container shape", () => { + const { pathTokens, refPathTokens } = compilePattern("accounts.*.token", "accounts.*.tokenRef"); + + for (const segment of ["0", 0] as const) { + const matched = matchPathTokens(["accounts", segment, "token"], pathTokens); + + expect(matched).not.toBeNull(); + expect(materializePathTokens(refPathTokens ?? [], matched!.captures)).toEqual([ + "accounts", + segment, + "tokenRef", + ]); + } + }); + it("expands wildcard and array patterns over config objects", () => { const root = { agents: { @@ -156,4 +212,18 @@ describe("target registry pattern helpers", () => { }, ]); }); + + it("preserves numeric indices when expanding array and wildcard patterns", () => { + const root = { accounts: [{ token: "array-secret" }] }; + + for (const pattern of ["accounts[].token", "accounts.*.token"]) { + expect(expandPathTokens(root, compilePattern(pattern).pathTokens)).toEqual([ + { + segments: ["accounts", 0, "token"], + captures: [0], + value: "array-secret", + }, + ]); + } + }); }); diff --git a/src/secrets/target-registry-pattern.ts b/src/secrets/target-registry-pattern.ts index 2d91beaa9d9e..c7f05481d8bf 100644 --- a/src/secrets/target-registry-pattern.ts +++ b/src/secrets/target-registry-pattern.ts @@ -1,4 +1,5 @@ /** Compiles, matches, and expands secret target registry path patterns. */ +import type { ConcreteConfigPathSegment } from "../shared/dot-path.js"; import { parseConfigPathArrayIndex } from "../shared/path-array-index.js"; import { isRecord, parseDotPath } from "./shared.js"; import type { SecretTargetRegistryEntry } from "./target-registry-types.js"; @@ -19,8 +20,8 @@ export type CompiledTargetRegistryEntry = SecretTargetRegistryEntry & { /** Concrete config value matched by expanding a path pattern. */ type ExpandedPathMatch = { - segments: string[]; - captures: string[]; + segments: ConcreteConfigPathSegment[]; + captures: ConcreteConfigPathSegment[]; value: unknown; }; @@ -31,8 +32,8 @@ function countDynamicPatternTokens(tokens: PathPatternToken[]): number { /** * Parses a dotted target pattern into literal, wildcard, and array traversal tokens. */ -function parsePathPattern(pathPattern: string): PathPatternToken[] { - const segments = parseDotPath(pathPattern); +function parsePathPattern(pathPattern: string, pathSegments?: string[]): PathPatternToken[] { + const segments = pathSegments ?? parseDotPath(pathPattern); return segments.map((segment) => { if (segment === "*") { return { kind: "wildcard" } as const; @@ -54,7 +55,7 @@ function parsePathPattern(pathPattern: string): PathPatternToken[] { export function compileTargetRegistryEntry( entry: SecretTargetRegistryEntry, ): CompiledTargetRegistryEntry { - const pathTokens = parsePathPattern(entry.pathPattern); + const pathTokens = parsePathPattern(entry.pathPattern, entry.pathPatternSegments); const pathDynamicTokenCount = countDynamicPatternTokens(pathTokens); const refPathTokens = entry.refPathPattern ? parsePathPattern(entry.refPathPattern) : undefined; const refPathDynamicTokenCount = refPathTokens ? countDynamicPatternTokens(refPathTokens) : 0; @@ -79,16 +80,17 @@ export function compileTargetRegistryEntry( * Matches concrete path segments against compiled pattern tokens and returns dynamic captures. */ export function matchPathTokens( - segments: string[], + segments: readonly ConcreteConfigPathSegment[], tokens: PathPatternToken[], + options?: { allowLegacyArrayString?: boolean }, ): { - captures: string[]; + captures: ConcreteConfigPathSegment[]; } | null { - const captures: string[] = []; + const captures: ConcreteConfigPathSegment[] = []; let index = 0; for (const token of tokens) { if (token.kind === "literal") { - if (segments[index] !== token.value) { + if (typeof segments[index] !== "string" || segments[index] !== token.value) { return null; } index += 1; @@ -96,7 +98,7 @@ export function matchPathTokens( } if (token.kind === "wildcard") { const value = segments[index]; - if (!value) { + if (value === undefined || value === "") { return null; } // Capture order must match materializePathTokens for sibling ref path reconstruction. @@ -108,10 +110,16 @@ export function matchPathTokens( return null; } const next = segments[index + 1]; - if (!next || parseConfigPathArrayIndex(next) === undefined) { + const arrayIndex = + typeof next === "number" + ? next + : options?.allowLegacyArrayString && typeof next === "string" + ? parseConfigPathArrayIndex(next) + : undefined; + if (arrayIndex === undefined || parseConfigPathArrayIndex(String(arrayIndex)) !== arrayIndex) { return null; } - captures.push(next); + captures.push(arrayIndex); index += 2; } return index === segments.length ? { captures } : null; @@ -122,9 +130,9 @@ export function matchPathTokens( */ export function materializePathTokens( tokens: PathPatternToken[], - captures: string[], -): string[] | null { - const out: string[] = []; + captures: ConcreteConfigPathSegment[], +): ConcreteConfigPathSegment[] | null { + const out: ConcreteConfigPathSegment[] = []; let captureIndex = 0; for (const token of tokens) { if (token.kind === "literal") { @@ -133,7 +141,7 @@ export function materializePathTokens( } if (token.kind === "wildcard") { const value = captures[captureIndex]; - if (!value) { + if (value === undefined || value === "") { return null; } out.push(value); @@ -141,7 +149,10 @@ export function materializePathTokens( continue; } const arrayIndex = captures[captureIndex]; - if (!arrayIndex || parseConfigPathArrayIndex(arrayIndex) === undefined) { + if ( + typeof arrayIndex !== "number" || + parseConfigPathArrayIndex(String(arrayIndex)) !== arrayIndex + ) { return null; } out.push(token.field, arrayIndex); @@ -158,8 +169,8 @@ export function expandPathTokens(root: unknown, tokens: PathPatternToken[]): Exp const walk = ( node: unknown, tokenIndex: number, - segments: string[], - captures: string[], + segments: ConcreteConfigPathSegment[], + captures: ConcreteConfigPathSegment[], ): void => { const token = tokens[tokenIndex]; if (!token) { @@ -188,10 +199,13 @@ export function expandPathTokens(root: unknown, tokens: PathPatternToken[]): Exp } if (token.kind === "wildcard") { - if (!isRecord(node)) { + if (!Array.isArray(node) && !isRecord(node)) { return; } - for (const [key, value] of Object.entries(node)) { + const entries: Iterable<[ConcreteConfigPathSegment, unknown]> = Array.isArray(node) + ? node.entries() + : Object.entries(node); + for (const [key, value] of entries) { if (isLeaf) { out.push({ segments: [...segments, key], @@ -214,21 +228,15 @@ export function expandPathTokens(root: unknown, tokens: PathPatternToken[]): Exp } for (let index = 0; index < items.length; index += 1) { const item = items[index]; - const indexString = String(index); if (isLeaf) { out.push({ - segments: [...segments, token.field, indexString], - captures: [...captures, indexString], + segments: [...segments, token.field, index], + captures: [...captures, index], value: item, }); continue; } - walk( - item, - tokenIndex + 1, - [...segments, token.field, indexString], - [...captures, indexString], - ); + walk(item, tokenIndex + 1, [...segments, token.field, index], [...captures, index]); } }; walk(root, 0, [], []); diff --git a/src/secrets/target-registry-query.ts b/src/secrets/target-registry-query.ts index 01f7300e6a6e..3f486fa41045 100644 --- a/src/secrets/target-registry-query.ts +++ b/src/secrets/target-registry-query.ts @@ -1,6 +1,7 @@ /** Query helpers for discovering secret target registry entries. */ import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { PluginManifestRegistry } from "../plugins/manifest-registry.js"; +import { formatConcreteConfigPath, type ConcreteConfigPathSegment } from "../shared/dot-path.js"; import { loadChannelSecretContractApi } from "./channel-contract-api.js"; import { getPath } from "./path-utils.js"; import { @@ -241,17 +242,19 @@ function discoverSecretTargetsFromEntries( source: unknown, discoveryEntries: CompiledTargetRegistryEntry[], ): DiscoveredConfigSecretTarget[] { + const formatDiscoveredPath = (segments: readonly ConcreteConfigPathSegment[]) => + formatConcreteConfigPath(segments, source); const out: DiscoveredConfigSecretTarget[] = []; const seen = new Set(); for (const entry of discoveryEntries) { const expanded = expandPathTokens(source, entry.pathTokens); for (const match of expanded) { - const resolved = toResolvedPlanTarget(entry, match.segments, match.captures); + const resolved = toResolvedPlanTarget(entry, match.captures); if (!resolved) { continue; } - const key = `${entry.id}:${resolved.pathSegments.join(".")}`; + const key = JSON.stringify([entry.id, ...resolved.pathTokens]); if (seen.has(key)) { continue; } @@ -261,12 +264,12 @@ function discoverSecretTargetsFromEntries( : undefined; out.push({ entry, - path: resolved.pathSegments.join("."), + path: formatDiscoveredPath(match.segments), pathSegments: resolved.pathSegments, ...(resolved.refPathSegments ? { refPathSegments: resolved.refPathSegments, - refPath: resolved.refPathSegments.join("."), + refPath: formatDiscoveredPath(resolved.refPathTokens ?? resolved.refPathSegments), } : {}), value: match.value, @@ -282,9 +285,13 @@ function discoverSecretTargetsFromEntries( function toResolvedPlanTarget( entry: CompiledTargetRegistryEntry, - pathSegments: string[], - captures: string[], + captures: ConcreteConfigPathSegment[], ): ResolvedPlanTarget | null { + const pathTokens = materializePathTokens(entry.pathTokens, captures); + if (!pathTokens) { + return null; + } + const pathSegments = pathTokens.map(String); const providerId = entry.providerIdPathSegmentIndex !== undefined ? pathSegments[entry.providerIdPathSegmentIndex] @@ -293,16 +300,17 @@ function toResolvedPlanTarget( entry.accountIdPathSegmentIndex !== undefined ? pathSegments[entry.accountIdPathSegmentIndex] : undefined; - const refPathSegments = entry.refPathTokens + const refPathTokens = entry.refPathTokens ? materializePathTokens(entry.refPathTokens, captures) : undefined; - if (entry.refPathTokens && !refPathSegments) { + if (entry.refPathTokens && !refPathTokens) { return null; } return { entry, pathSegments, - ...(refPathSegments ? { refPathSegments } : {}), + pathTokens, + ...(refPathTokens ? { refPathTokens, refPathSegments: refPathTokens.map(String) } : {}), ...(providerId ? { providerId } : {}), ...(accountId ? { accountId } : {}), }; @@ -318,6 +326,7 @@ export function listSecretTargetRegistryEntries(): SecretTargetRegistryEntry[] { { id: entry.id, targetType: entry.targetType }, entry.targetTypeAliases ? { targetTypeAliases: [...entry.targetTypeAliases] } : {}, { configFile: entry.configFile, pathPattern: entry.pathPattern }, + entry.pathPatternSegments ? { pathPatternSegments: [...entry.pathPatternSegments] } : {}, entry.refPathPattern ? { refPathPattern: entry.refPathPattern } : {}, { secretShape: entry.secretShape, @@ -360,6 +369,8 @@ export function isKnownCoreSecretTargetId(value: unknown): value is string { export function resolvePlanTargetAgainstRegistry(candidate: { type: string; pathSegments: string[]; + pathTokens?: readonly ConcreteConfigPathSegment[]; + allowLegacyArrayString?: boolean; providerId?: string; accountId?: string; }): ResolvedPlanTarget | null { @@ -387,6 +398,8 @@ function resolvePlanTargetAgainstEntries( candidate: { type: string; pathSegments: string[]; + pathTokens?: readonly ConcreteConfigPathSegment[]; + allowLegacyArrayString?: boolean; providerId?: string; accountId?: string; }, @@ -396,15 +409,18 @@ function resolvePlanTargetAgainstEntries( return null; } + const pathTokens = candidate.pathTokens ?? candidate.pathSegments; for (const entry of entries) { if (!entry.includeInPlan) { continue; } - const matched = matchPathTokens(candidate.pathSegments, entry.pathTokens); + const matched = matchPathTokens(pathTokens, entry.pathTokens, { + allowLegacyArrayString: candidate.allowLegacyArrayString, + }); if (!matched) { continue; } - const resolved = toResolvedPlanTarget(entry, candidate.pathSegments, matched.captures); + const resolved = toResolvedPlanTarget(entry, matched.captures); if (!resolved) { continue; } @@ -429,19 +445,21 @@ function resolvePlanTargetAgainstEntries( export function resolveSecretPlanTargetByPathCore(params: { configFile: SecretTargetConfigFile; pathSegments: string[]; + pathTokens?: readonly ConcreteConfigPathSegment[]; }): ResolvedPlanTarget | null { if (params.configFile === "openclaw.json") { - return resolveConfigSecretTargetByPath(params.pathSegments); + return resolveConfigSecretTargetByPath(params.pathSegments, params.pathTokens); } + const pathTokens = params.pathTokens ?? params.pathSegments; for (const entry of getCompiledSecretTargetRegistryState().authProfilesCompiledSecretTargets) { if (!entry.includeInPlan) { continue; } - const matched = matchPathTokens(params.pathSegments, entry.pathTokens); + const matched = matchPathTokens(pathTokens, entry.pathTokens); if (!matched) { continue; } - const resolved = toResolvedPlanTarget(entry, params.pathSegments, matched.captures); + const resolved = toResolvedPlanTarget(entry, matched.captures); if (resolved) { return resolved; } @@ -452,16 +470,19 @@ export function resolveSecretPlanTargetByPathCore(params: { /** * Resolves an openclaw.json config path to the matching plan-capable secrets target. */ -export function resolveConfigSecretTargetByPath(pathSegments: string[]): ResolvedPlanTarget | null { +export function resolveConfigSecretTargetByPath( + pathSegments: string[], + pathTokens: readonly ConcreteConfigPathSegment[] = pathSegments, +): ResolvedPlanTarget | null { for (const entry of getCompiledCoreOpenClawTargetState().openClawCompiledSecretTargets) { if (!entry.includeInPlan) { continue; } - const matched = matchPathTokens(pathSegments, entry.pathTokens); + const matched = matchPathTokens(pathTokens, entry.pathTokens); if (!matched) { continue; } - const resolved = toResolvedPlanTarget(entry, pathSegments, matched.captures); + const resolved = toResolvedPlanTarget(entry, matched.captures); if (!resolved) { continue; } @@ -477,11 +498,11 @@ export function resolveConfigSecretTargetByPath(pathSegments: string[]): Resolve if (!entry.includeInPlan) { continue; } - const matched = matchPathTokens(pathSegments, entry.pathTokens); + const matched = matchPathTokens(pathTokens, entry.pathTokens); if (!matched) { continue; } - const resolved = toResolvedPlanTarget(entry, pathSegments, matched.captures); + const resolved = toResolvedPlanTarget(entry, matched.captures); if (!resolved) { continue; } @@ -492,11 +513,11 @@ export function resolveConfigSecretTargetByPath(pathSegments: string[]): Resolve if (!entry.includeInPlan) { continue; } - const matched = matchPathTokens(pathSegments, entry.pathTokens); + const matched = matchPathTokens(pathTokens, entry.pathTokens); if (!matched) { continue; } - const resolved = toResolvedPlanTarget(entry, pathSegments, matched.captures); + const resolved = toResolvedPlanTarget(entry, matched.captures); if (!resolved) { continue; } diff --git a/src/secrets/target-registry-types.ts b/src/secrets/target-registry-types.ts index 9431efdfd360..c316f2231d12 100644 --- a/src/secrets/target-registry-types.ts +++ b/src/secrets/target-registry-types.ts @@ -1,3 +1,5 @@ +import type { ConcreteConfigPathSegment } from "../shared/dot-path.js"; + /** Config document that owns a registered secret-bearing target. */ export type SecretTargetConfigFile = "openclaw.json" | "auth-profile-store"; // pragma: allowlist secret /** Storage shape used by a target: inline SecretInput or a sibling `*Ref` field. */ @@ -20,6 +22,8 @@ export type SecretTargetRegistryEntry = { configFile: SecretTargetConfigFile; /** Dot-path pattern for the secret-bearing value; `*` captures path segments. */ pathPattern: string; + /** Structured pattern segments preserve literal plugin IDs containing dots. */ + pathPatternSegments?: string[]; /** Optional sibling SecretRef path materialized from the same captures as `pathPattern`. */ refPathPattern?: string; /** Whether the registered value stores a SecretInput directly or via a sibling ref field. */ @@ -49,8 +53,12 @@ export type ResolvedPlanTarget = { entry: SecretTargetRegistryEntry; /** Concrete path to the secret-bearing value in the owning config document. */ pathSegments: string[]; + /** Internal mutation path preserving the registered record-key versus array-index shape. */ + pathTokens: ConcreteConfigPathSegment[]; /** Concrete sibling SecretRef path when `entry.secretShape` is `sibling_ref`. */ refPathSegments?: string[]; + /** Internal sibling mutation path preserving captured container shape. */ + refPathTokens?: ConcreteConfigPathSegment[]; /** Provider id captured from `pathSegments`, if the registry entry declares one. */ providerId?: string; /** Account/profile id captured from `pathSegments`, if the registry entry declares one. */ diff --git a/src/secrets/target-registry.test.ts b/src/secrets/target-registry.test.ts index e12d641664ef..cad6605795f7 100644 --- a/src/secrets/target-registry.test.ts +++ b/src/secrets/target-registry.test.ts @@ -35,6 +35,39 @@ describe("secret target registry", () => { expect(targets[0]?.path).toBe(TALK_TEST_PROVIDER_API_KEY_PATH); }); + it("preserves dotted provider header keys during discovery", () => { + const config = { + models: { + providers: { + openai: { + headers: { + "X.Trace": { source: "env", provider: "default", id: "TRACE_HEADER" }, + }, + request: { + headers: { + "X.Request.Trace": { + source: "env", + provider: "default", + id: "REQUEST_TRACE_HEADER", + }, + }, + }, + }, + }, + }, + } as unknown as OpenClawConfig; + + const targets = discoverConfigSecretTargetsByIds( + config, + new Set(["models.providers.*.headers.*", "models.providers.*.request.headers.*"]), + ); + + expect(targets.map(({ path }) => path).toSorted()).toEqual([ + 'models.providers.openai.headers["X.Trace"]', + 'models.providers.openai.request.headers["X.Request.Trace"]', + ]); + }); + it("resolves talk realtime provider api key targets", () => { const target = resolveConfigSecretTargetByPath([ "talk", diff --git a/src/shared/dot-path.ts b/src/shared/dot-path.ts index 42e28e144d4f..dacf7cdecc2d 100644 --- a/src/shared/dot-path.ts +++ b/src/shared/dot-path.ts @@ -1,3 +1,260 @@ +import { isBlockedObjectKey } from "../infra/prototype-keys.js"; +import { parseConfigPathArrayIndex } from "./path-array-index.js"; + +export type ConcreteConfigPathSegment = string | number; +export type ParsedConcreteConfigPath = { + tokens: ConcreteConfigPathSegment[]; + quotedNumericSegments: ReadonlySet; +}; + +function parseQuotedBracketPathSegment(value: string): unknown { + const quote = value.startsWith('"') ? '"' : "'"; + if (quote === '"') { + try { + return JSON.parse(value) as unknown; + } catch { + // Quoted config paths also accept the JSON5 string escapes used by shipped CLI paths. + } + } + if (!value.endsWith(quote)) { + throw new SyntaxError("Unterminated quoted path segment"); + } + + let normalized = '"'; + for (let index = 1; index < value.length - 1; index += 1) { + const character = value[index]; + if (character === quote) { + throw new SyntaxError("Unexpected quote in path segment"); + } + if (character === '"') { + normalized += '\\"'; + continue; + } + if (character === "\\") { + const escaped = value[index + 1]; + if (escaped === undefined || index + 1 >= value.length - 1) { + throw new SyntaxError("Unterminated escape in path segment"); + } + index += 1; + if (escaped === "\n" || escaped === "\u2028" || escaped === "\u2029") { + continue; + } + if (escaped === "\r") { + if (value[index + 1] === "\n") { + index += 1; + } + continue; + } + if (escaped === "x") { + const hex = value.slice(index + 1, index + 3); + if (!/^[\da-f]{2}$/i.test(hex)) { + throw new SyntaxError("Invalid hexadecimal escape in path segment"); + } + normalized += `\\u00${hex}`; + index += 2; + continue; + } + if (escaped === "0") { + if (/\d/.test(value[index + 1] ?? "")) { + throw new SyntaxError("Invalid numeric escape in path segment"); + } + normalized += "\\u0000"; + continue; + } + if (escaped === "v") { + normalized += "\\u000b"; + continue; + } + if (/[1-9]/.test(escaped)) { + throw new SyntaxError("Invalid numeric escape in path segment"); + } + if (escaped === "'") { + normalized += "'"; + } else if ('"\\/bfnrtu'.includes(escaped)) { + normalized += `\\${escaped}`; + } else { + normalized += escaped; + } + continue; + } + normalized += character; + } + return JSON.parse(`${normalized}"`) as unknown; +} + +function parseBracketPathSegment(raw: string, fullPath: string): ConcreteConfigPathSegment { + const trimmed = raw.trim(); + if (!trimmed) { + throw new Error(`Invalid path (empty "[]"): ${fullPath}`); + } + if (trimmed.startsWith('"') || trimmed.startsWith("'")) { + try { + const parsed = parseQuotedBracketPathSegment(trimmed); + if (typeof parsed === "string" && parsed.trim()) { + return parsed; + } + } catch (err) { + throw new Error(`Invalid path bracket string (${trimmed}): ${fullPath}`, { cause: err }); + } + throw new Error(`Invalid path bracket string (${trimmed}): ${fullPath}`); + } + return parseConfigPathArrayIndex(trimmed) ?? trimmed; +} + +function assertNotWhitespaceSegment(current: string, raw: string): void { + if (current.length > 0 && !current.trim()) { + throw new Error(`Invalid path (empty segment): ${raw}`); + } +} + +function findBracketPathClose(path: string, open: number): number { + let quote: '"' | "'" | undefined; + for (let index = open + 1; index < path.length; index += 1) { + const character = path[index]; + if (quote) { + if (character === "\\") { + index += 1; + } else if (character === quote) { + quote = undefined; + } + continue; + } + if (character === "]") { + return index; + } + if ((character === '"' || character === "'") && !path.slice(open + 1, index).trim()) { + quote = character; + } + } + return -1; +} + +/** Retains quoted numeric-key provenance alongside the public concrete path tokens. */ +export function parseConcreteConfigPathWithProvenance(raw: string): ParsedConcreteConfigPath { + const trimmed = raw.trim(); + if (!trimmed) { + throw new Error("Path is empty."); + } + const parts: ConcreteConfigPathSegment[] = []; + const quotedNumericSegments = new Set(); + let current = ""; + let segmentEmitted = false; + let index = 0; + while (index < trimmed.length) { + const character = trimmed[index]; + if (character === "\\") { + const next = trimmed[index + 1]; + if (next === undefined) { + throw new Error(`Invalid path (trailing escape): ${raw}`); + } + current += next; + index += 2; + continue; + } + if (character === ".") { + assertNotWhitespaceSegment(current, raw); + if (!segmentEmitted && !current.trim()) { + throw new Error(`Invalid path (empty segment): ${raw}`); + } + if (current) { + parts.push(current.trim()); + } + current = ""; + segmentEmitted = false; + index += 1; + continue; + } + if (character === "[") { + assertNotWhitespaceSegment(current, raw); + if (!current.trim() && !segmentEmitted && parts.length > 0) { + throw new Error(`Invalid path (empty segment): ${raw}`); + } + if (current) { + parts.push(current.trim()); + } + current = ""; + const close = findBracketPathClose(trimmed, index); + if (close === -1) { + throw new Error(`Invalid path (missing "]"): ${raw}`); + } + const inside = trimmed.slice(index + 1, close).trim(); + if (!inside) { + throw new Error(`Invalid path (empty "[]"): ${raw}`); + } + const segment = parseBracketPathSegment(inside, raw); + if ( + typeof segment === "string" && + (inside.startsWith('"') || inside.startsWith("'")) && + parseConfigPathArrayIndex(segment) !== undefined + ) { + quotedNumericSegments.add(parts.length); + } + parts.push(segment); + const next = trimmed[close + 1]; + if (next !== undefined && next !== "." && next !== "[") { + throw new Error(`Invalid path (missing separator after bracket): ${raw}`); + } + segmentEmitted = true; + index = close + 1; + continue; + } + current += character; + index += 1; + } + if (!segmentEmitted && !current.trim()) { + throw new Error(`Invalid path (empty segment): ${raw}`); + } + if (current) { + parts.push(current.trim()); + } + for (const segment of parts) { + if (typeof segment === "string" && isBlockedObjectKey(segment)) { + throw new Error(`Invalid path segment: ${segment}`); + } + } + return { tokens: parts, quotedNumericSegments }; +} + +/** Parses one concrete path while keeping explicit array brackets distinct from quoted keys. */ +export function parseConcreteConfigPathTokens(raw: string): ConcreteConfigPathSegment[] { + return parseConcreteConfigPathWithProvenance(raw).tokens; +} + +/** Parses one concrete config path into the existing string-segment CLI contract. */ +export function parseConcreteConfigPath(raw: string): string[] { + return parseConcreteConfigPathTokens(raw).map(String); +} + +/** Appends one config path segment without confusing literal record keys with traversal. */ +export function appendConfigPathSegment(path: string, segment: string | number): string { + if (typeof segment === "number") { + return `${path}[${segment}]`; + } + if (!/^[A-Za-z_$][A-Za-z0-9_$:-]*$/.test(segment)) { + return `${path}[${JSON.stringify(segment)}]`; + } + return path ? `${path}.${segment}` : segment; +} + +/** Formats concrete tokens, recovering array indices from their actual source containers. */ +export function formatConcreteConfigPath( + segments: readonly ConcreteConfigPathSegment[], + source?: unknown, +): string { + let cursor = source; + return segments.reduce((path, segment) => { + const concreteSegment = + typeof segment === "string" && Array.isArray(cursor) + ? (parseConfigPathArrayIndex(segment) ?? segment) + : segment; + cursor = + cursor !== null && typeof cursor === "object" + ? Reflect.get(cursor, String(segment)) + : undefined; + return appendConfigPathSegment(path, concreteSegment); + }, ""); +} + /** Joins path segments into their dotted-path representation. */ export function toDotPath(segments: readonly string[]): string { return segments.join("."); diff --git a/src/wizard/setup.plugin-config.test.ts b/src/wizard/setup.plugin-config.test.ts index df8493027823..6cac5d1224f8 100644 --- a/src/wizard/setup.plugin-config.test.ts +++ b/src/wizard/setup.plugin-config.test.ts @@ -357,6 +357,85 @@ describe("setupPluginConfig", () => { expect(result.plugins?.entries?.brave?.config?.["webSearch.mode"]).toBeUndefined(); }); + it.each([ + { + name: "an existing array through a dotted index", + field: "accounts.0.token", + existing: { accounts: [{}] }, + expected: { accounts: [{ token: "configured" }] }, + }, + { + name: "a missing schema-declared array through a dotted index", + field: "accounts.0.token", + schema: { + type: "object", + properties: { + accounts: { + type: "array", + items: { type: "object", properties: { token: { type: "string" } } }, + }, + }, + }, + expected: { accounts: [{ token: "configured" }] }, + }, + { + name: "a numeric record key through a dotted path", + field: "accounts.0.token", + schema: { + type: "object", + properties: { + accounts: { + type: "object", + properties: { + "0": { type: "object", properties: { token: { type: "string" } } }, + }, + }, + }, + }, + expected: { accounts: { "0": { token: "configured" } } }, + }, + { + name: "an explicit bracketed array index without a schema", + field: "accounts[0].token", + expected: { accounts: [{ token: "configured" }] }, + }, + { + name: "an explicitly quoted numeric record key without a schema", + field: 'accounts["0"].token', + expected: { accounts: { "0": { token: "configured" } } }, + }, + { + name: "a quoted record key containing a literal dot", + field: 'accounts["primary.backup"].token', + expected: { accounts: { "primary.backup": { token: "configured" } } }, + }, + ])("writes $name", async ({ field, existing, schema, expected }) => { + const pluginId = "indexed-plugin"; + loadPluginManifestRegistryCore.mockReturnValue({ + plugins: [makeManifestPlugin(pluginId, { [field]: { label: "Token" } }, schema)], + }); + + const result = await setupPluginConfig({ + config: { + plugins: { + entries: { [pluginId]: { enabled: true, ...(existing && { config: existing }) } }, + }, + }, + prompter: { + intro: vi.fn(async () => {}), + outro: vi.fn(async () => {}), + note: vi.fn(async () => {}), + select: vi.fn(async () => "") as unknown as WizardPrompter["select"], + multiselect: vi.fn(async () => [pluginId]) as unknown as WizardPrompter["multiselect"], + text: vi.fn(async () => "configured") as unknown as WizardPrompter["text"], + confirm: vi.fn(async () => true), + progress: vi.fn(() => ({ update: vi.fn(), stop: vi.fn() })), + }, + }); + + expect(result.plugins?.entries?.[pluginId]?.config).toEqual(expected); + }); + it("rejects prototype-polluting dotted uiHint paths without mutating config", async () => { const pollutionProbe = "openclawPluginPollutionProbe"; loadPluginManifestRegistryCore.mockReturnValue({ @@ -389,7 +468,7 @@ describe("setupPluginConfig", () => { progress: vi.fn(() => ({ update: vi.fn(), stop: vi.fn() })), }, }), - ).rejects.toThrow(/prototype-polluting/); + ).rejects.toThrow(/Invalid path segment/); expect(config.plugins?.entries?.["unsafe-plugin"]?.config).toBeUndefined(); expect(({} as Record)[pollutionProbe]).toBeUndefined(); }); diff --git a/src/wizard/setup.plugin-config.ts b/src/wizard/setup.plugin-config.ts index 821e82fdde8a..cd392ad59c9c 100644 --- a/src/wizard/setup.plugin-config.ts +++ b/src/wizard/setup.plugin-config.ts @@ -4,8 +4,13 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { PluginManifestRecord } from "../plugins/manifest-registry.js"; import type { PluginConfigUiHint } from "../plugins/types.js"; import { getPath, setPathCreateStrict } from "../secrets/path-utils.js"; +import { + parseConcreteConfigPathTokens, + type ConcreteConfigPathSegment, +} from "../shared/dot-path.js"; import type { JsonSchemaObject } from "../shared/json-schema.types.js"; import { createLazyRuntimeModule } from "../shared/lazy-runtime.js"; +import { parseConfigPathArrayIndex } from "../shared/path-array-index.js"; import { t } from "./i18n/index.js"; import type { WizardPrompter } from "./prompts.js"; @@ -33,21 +38,24 @@ type JsonSchemaProperty = { function resolveJsonSchemaProperty( jsonSchema: JsonSchemaObject | undefined, - fieldKey: string, + pathSegments: readonly ConcreteConfigPathSegment[], ): JsonSchemaProperty | undefined { if (!jsonSchema) { return undefined; } let cursor: unknown = jsonSchema; - for (const segment of fieldKey.split(".")) { + for (const segment of pathSegments) { if (!cursor || typeof cursor !== "object") { return undefined; } - const properties = (cursor as Record).properties; - if (!properties || typeof properties !== "object") { - return undefined; - } - cursor = (properties as Record)[segment]; + const schema = cursor as Record; + const properties = schema.properties; + cursor = + schema.type === "array" + ? schema.items + : properties && typeof properties === "object" + ? (properties as Record)[String(segment)] + : undefined; } return cursor && typeof cursor === "object" ? (cursor as JsonSchemaProperty) : undefined; } @@ -59,8 +67,29 @@ function getExistingPluginConfig( return (config.plugins?.entries?.[pluginId]?.config as Record) ?? {}; } -function toPathSegments(fieldKey: string): string[] { - return fieldKey.split(".").filter(Boolean); +function toPathSegments( + fieldKey: string, + existing: Record, + jsonSchema?: JsonSchemaObject, +): ConcreteConfigPathSegment[] { + const segments = parseConcreteConfigPathTokens(fieldKey); + let value: unknown = existing; + + return segments.map((segment, index) => { + const schema = resolveJsonSchemaProperty(jsonSchema, segments.slice(0, index)); + // Existing containers own their shape; the schema recovers arrays not created yet. + const arrayContainer = Array.isArray(value) || (value == null && schema?.type === "array"); + const arrayIndex = + typeof segment === "string" && arrayContainer + ? parseConfigPathArrayIndex(segment) + : undefined; + const resolved = arrayIndex ?? segment; + value = + value !== null && typeof value === "object" + ? Reflect.get(value, String(resolved)) + : undefined; + return resolved; + }); } function formatCurrentValue(value: unknown): string { @@ -144,7 +173,7 @@ export function discoverUnconfiguredPlugins(params: { return all.filter((plugin) => { const existing = getExistingPluginConfig(params.config, plugin.id); return Object.keys(plugin.uiHints).some((key) => { - const val = getPath(existing, toPathSegments(key)); + const val = getPath(existing, toPathSegments(key, existing, plugin.jsonSchema).map(String)); return val === undefined || val === null || val === ""; }); }); @@ -183,8 +212,8 @@ async function promptPluginFields(params: { let changed = false; for (const [key, hint] of Object.entries(plugin.uiHints)) { - const pathSegments = toPathSegments(key); - const currentValue = getPath(existing, pathSegments); + const pathSegments = toPathSegments(key, existing, plugin.jsonSchema); + const currentValue = getPath(existing, pathSegments.map(String)); const hasValue = currentValue !== undefined && currentValue !== null && currentValue !== ""; // In onboard mode, skip already-configured fields @@ -192,7 +221,7 @@ async function promptPluginFields(params: { continue; } - const schemaProp = resolveJsonSchemaProperty(plugin.jsonSchema, key); + const schemaProp = resolveJsonSchemaProperty(plugin.jsonSchema, pathSegments); const label = hint.label ?? key; const helpSuffix = hint.help ? ` — ${hint.help}` : ""; @@ -410,7 +439,7 @@ export async function configurePluginConfig(params: { ...configurable.map((p) => { const existing = getExistingPluginConfig(params.config, p.id); const configuredCount = Object.keys(p.uiHints).filter((k) => { - const val = getPath(existing, toPathSegments(k)); + const val = getPath(existing, toPathSegments(k, existing, p.jsonSchema).map(String)); return val !== undefined && val !== null && val !== ""; }).length; const totalCount = Object.keys(p.uiHints).length;