Skip to content

Commit 81b777c

Browse files
joshavantjoshavant
authored
fix(config): harden SecretRef round-trip handling in Control UI and RPC writes (#58044)
* Config: harden SecretRef round-trip handling * Gateway: test SecretRef preflight on config writes * Agents: align skill loader with upstream Skill type * Docs: align SecretRef write semantics with Control UI and RPC behavior * Config: add UI and gateway regression evidence for SecretRef hardening * Config: add token SecretRef restore regression and skill sourceInfo compat * UI: scope structured-value lockout to SecretRef fields * Agents: remove out-of-scope skill loader compat edits * UI: reduce app-render churn to rawAvailable-only changes * Gateway: scope SecretRef preflight to submitted config * Docs: clarify config write SecretRef preflight scope * changelog Signed-off-by: joshavant <830519+joshavant@users.noreply.github.com> --------- Signed-off-by: joshavant <830519+joshavant@users.noreply.github.com>
1 parent f7ced43 commit 81b777c

20 files changed

Lines changed: 1115 additions & 273 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ Docs: https://docs.openclaw.ai
128128
- Matrix/direct rooms: recover fresh auto-joined 1:1 DMs without eagerly persisting invite-only `m.direct` mappings, while keeping named, aliased, and explicitly configured rooms on the room path. (#58024) Thanks @gumadeiras.
129129
- TTS: Restore 3.28 schema compatibility and fallback observability. (#57953) Thanks @joshavant.
130130
- Telegram/forum topics: restore reply routing to the active topic and keep ACP `sessions_spawn(..., thread=true, mode="session")` bound to that same topic instead of falling back to root chat or losing follow-up routing. (#56060) Thanks @one27001.
131+
- Config/SecretRef + Control UI: harden SecretRef redaction round-trip restore, block unsafe raw fallback (force Form mode when raw is unavailable), and preflight submitted-config SecretRefs before config write RPC persistence. (#58044) Thanks @joshavant.
131132

132133
## 2026.3.28
133134

‎docs/cli/index.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,12 +905,14 @@ Subcommands:
905905

906906
Common RPCs:
907907

908+
- `config.set` (validate + write full config; use `baseHash` for optimistic concurrency)
908909
- `config.apply` (validate + write config + restart + wake)
909910
- `config.patch` (merge a partial update + restart + wake)
910911
- `update.run` (run update + restart + wake)
911912

912913
Tip: when calling `config.set`/`config.apply`/`config.patch` directly, pass `baseHash` from
913914
`config.get` if a config already exists.
915+
Tip: these config write RPCs preflight active SecretRef resolution for refs in the submitted config payload and reject writes when an effectively active submitted ref is unresolved.
914916

915917
## Models
916918

‎docs/gateway/secrets.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ Runtime-minted or rotating credentials and OAuth refresh material are intentiona
364364
- Field without a ref: unchanged.
365365
- Field with a ref: required on active surfaces during activation.
366366
- If both plaintext and ref are present, ref takes precedence on supported precedence paths.
367+
- The redaction sentinel `__OPENCLAW_REDACTED__` is reserved for internal config redaction/restore and is rejected as literal submitted config data.
367368

368369
Warning and audit signals:
369370

@@ -383,12 +384,14 @@ Secret activation runs on:
383384
- Config reload hot-apply path
384385
- Config reload restart-check path
385386
- Manual reload via `secrets.reload`
387+
- Gateway config write RPC preflight (`config.set` / `config.apply` / `config.patch`) for active-surface SecretRef resolvability within the submitted config payload before persisting edits
386388

387389
Activation contract:
388390

389391
- Success swaps the snapshot atomically.
390392
- Startup failure aborts gateway startup.
391393
- Runtime reload failure keeps the last-known-good snapshot.
394+
- Write-RPC preflight failure rejects the submitted config and keeps both disk config and active runtime snapshot unchanged.
392395
- Providing an explicit per-call channel token to an outbound helper/tool call does not trigger SecretRef activation; activation points remain startup, reload, and explicit `secrets.reload`.
393396

394397
## Degraded and recovered signals

‎docs/web/control-ui.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,10 @@ The Control UI can localize itself on first load based on your browser locale, a
8787
- Config: view/edit `~/.openclaw/openclaw.json` (`config.get`, `config.set`)
8888
- Config: apply + restart with validation (`config.apply`) and wake the last active session
8989
- Config writes include a base-hash guard to prevent clobbering concurrent edits
90-
- Config schema + form rendering (`config.schema`, including plugin + channel schemas); Raw JSON editor remains available
90+
- Config writes (`config.set`/`config.apply`/`config.patch`) also preflight active SecretRef resolution for refs in the submitted config payload; unresolved active submitted refs are rejected before write
91+
- Config schema + form rendering (`config.schema`, including plugin + channel schemas); Raw JSON editor is available only when the snapshot has a safe raw round-trip
92+
- If a snapshot cannot safely round-trip raw text, Control UI forces Form mode and disables Raw mode for that snapshot
93+
- Structured SecretRef object values are rendered read-only in form text inputs to prevent accidental object-to-string corruption
9194
- Debug: status/health/models snapshots + event log + manual RPC calls (`status`, `health`, `models.list`)
9295
- Logs: live tail of gateway file logs with filter/export (`logs.tail`)
9396
- Update: run a package/git update + restart (`update.run`) with a restart report

‎extensions/discord/src/config-ui-hints.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,5 +192,6 @@ export const discordChannelConfigUiHints = {
192192
token: {
193193
label: "Discord Bot Token",
194194
help: "Discord bot token used for gateway and REST API authentication for this provider account. Keep this secret out of committed config and rotate immediately after any leak.",
195+
sensitive: true,
195196
},
196197
} satisfies Record<string, ChannelConfigUiHint>;

‎src/config/redact-snapshot.restore.test.ts‎

Lines changed: 104 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ describe("restoreRedactedValues", () => {
121121
expect(result.humanReadableMessage).toContain("channels.newChannel.token");
122122
});
123123

124-
it("keeps unmatched wildcard array entries unchanged outside extension paths", () => {
124+
it("rejects sentinel literals that survive restore", () => {
125125
const hints: ConfigUiHints = {
126126
"custom.*": { sensitive: true },
127127
};
@@ -131,8 +131,9 @@ describe("restoreRedactedValues", () => {
131131
const original = {
132132
custom: { items: ["original-secret-value"] },
133133
};
134-
const result = restoreRedactedValues(incoming, original, hints) as typeof incoming;
135-
expect(result.custom.items[0]).toBe(REDACTED_SENTINEL);
134+
const result = restoreRedactedValues_orig(incoming, original, hints);
135+
expect(result.ok).toBe(false);
136+
expect(result.humanReadableMessage).toContain("Reserved redaction sentinel");
136137
});
137138

138139
it("round-trips config through redact → restore", () => {
@@ -183,7 +184,7 @@ describe("restoreRedactedValues", () => {
183184
expect(restored).toEqual(originalConfig);
184185
});
185186

186-
it("restores with uiHints respecting sensitive:false override", () => {
187+
it("rejects sentinel literals even when uiHints mark the path non-sensitive", () => {
187188
const hints: ConfigUiHints = {
188189
"gateway.auth.token": { sensitive: false },
189190
};
@@ -193,8 +194,9 @@ describe("restoreRedactedValues", () => {
193194
const original = {
194195
gateway: { auth: { token: "real-secret" } },
195196
};
196-
const result = restoreRedactedValues(incoming, original, hints) as typeof incoming;
197-
expect(result.gateway.auth.token).toBe(REDACTED_SENTINEL);
197+
const result = restoreRedactedValues_orig(incoming, original, hints);
198+
expect(result.ok).toBe(false);
199+
expect(result.humanReadableMessage).toContain("Reserved redaction sentinel");
198200
});
199201

200202
it("restores array items using wildcard uiHints", () => {
@@ -225,4 +227,100 @@ describe("restoreRedactedValues", () => {
225227
expect(result.channels.slack.accounts[0].botToken).toBe("original-token-first-account");
226228
expect(result.channels.slack.accounts[1].botToken).toBe("user-provided-new-token-value");
227229
});
230+
231+
it("restores redacted SecretRef ids for channels token paths", () => {
232+
const hints: ConfigUiHints = {
233+
"channels.discord.token": { sensitive: true },
234+
};
235+
const incoming = {
236+
channels: {
237+
discord: {
238+
token: {
239+
source: "env",
240+
provider: "default",
241+
id: REDACTED_SENTINEL,
242+
},
243+
},
244+
},
245+
};
246+
const original = {
247+
channels: {
248+
discord: {
249+
token: {
250+
source: "env",
251+
provider: "default",
252+
id: "DISCORD_BOT_TOKEN",
253+
},
254+
},
255+
},
256+
};
257+
const result = restoreRedactedValues(incoming, original, hints);
258+
expect(result.channels.discord.token).toEqual({
259+
source: "env",
260+
provider: "default",
261+
id: "DISCORD_BOT_TOKEN",
262+
});
263+
});
264+
265+
it("rejects SecretRef source/provider changes when id is still redacted", () => {
266+
const incoming = {
267+
models: {
268+
providers: {
269+
default: {
270+
apiKey: {
271+
source: "file",
272+
provider: "vault",
273+
id: REDACTED_SENTINEL,
274+
},
275+
},
276+
},
277+
},
278+
};
279+
const original = {
280+
models: {
281+
providers: {
282+
default: {
283+
apiKey: {
284+
source: "env",
285+
provider: "default",
286+
id: "OPENAI_API_KEY",
287+
},
288+
},
289+
},
290+
},
291+
};
292+
const result = restoreRedactedValues_orig(incoming, original, mainSchemaHints);
293+
expect(result.ok).toBe(false);
294+
expect(result.humanReadableMessage).toContain("changed source/provider");
295+
});
296+
297+
it("reports a provider-focused error when original SecretRefs lack provider", () => {
298+
const incoming = {
299+
models: {
300+
providers: {
301+
default: {
302+
apiKey: {
303+
source: "env",
304+
id: REDACTED_SENTINEL,
305+
},
306+
},
307+
},
308+
},
309+
};
310+
const original = {
311+
models: {
312+
providers: {
313+
default: {
314+
apiKey: {
315+
source: "env",
316+
id: "OPENAI_API_KEY",
317+
},
318+
},
319+
},
320+
},
321+
};
322+
const result = restoreRedactedValues_orig(incoming, original, mainSchemaHints);
323+
expect(result.ok).toBe(false);
324+
expect(result.humanReadableMessage).toContain("requires a provider field");
325+
});
228326
});

‎src/config/redact-snapshot.test.ts‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ describe("redactConfigSnapshot", () => {
317317
expect(result.raw).toContain(REDACTED_SENTINEL);
318318
});
319319

320-
it("keeps non-sensitive raw fields intact when secret values overlap", () => {
320+
it("drops raw text when overlap fallback triggers", () => {
321321
const config = {
322322
gateway: {
323323
mode: "local",
@@ -326,12 +326,13 @@ describe("redactConfigSnapshot", () => {
326326
};
327327
const snapshot = makeSnapshot(config, JSON.stringify(config));
328328
const result = redactConfigSnapshot(snapshot, mainSchemaHints);
329-
const parsed: {
329+
expect(result.raw).toBeNull();
330+
const cfg = result.config as {
330331
gateway?: { mode?: string; auth?: { password?: string } };
331-
} = JSON5.parse(result.raw ?? "{}");
332-
expect(parsed.gateway?.mode).toBe("local");
333-
expect(parsed.gateway?.auth?.password).toBe(REDACTED_SENTINEL);
334-
const restored = restoreRedactedValues(parsed, snapshot.config, mainSchemaHints);
332+
};
333+
expect(cfg.gateway?.mode).toBe("local");
334+
expect(cfg.gateway?.auth?.password).toBe(REDACTED_SENTINEL);
335+
const restored = restoreRedactedValues(result.config, snapshot.config, mainSchemaHints);
335336
expect(restored.gateway.mode).toBe("local");
336337
expect(restored.gateway.auth.password).toBe("local");
337338
});
@@ -373,13 +374,19 @@ describe("redactConfigSnapshot", () => {
373374
};
374375
const snapshot = makeSnapshot(config, JSON.stringify(config, null, 2));
375376
const result = redactConfigSnapshot(snapshot, mainSchemaHints);
376-
const parsed = JSON5.parse(result.raw ?? "{}");
377-
expect(parsed.gateway?.mode).toBe("default");
378-
expect(parsed.gateway?.auth?.password).toBe(REDACTED_SENTINEL);
379-
expect(parsed.models?.providers?.default?.apiKey?.source).toBe("env");
380-
expect(parsed.models?.providers?.default?.apiKey?.provider).toBe("default");
381-
expect(result.raw).not.toContain("OPENAI_API_KEY");
382-
const restored = restoreRedactedValues(parsed, snapshot.config, mainSchemaHints);
377+
expect(result.raw).toBeNull();
378+
const cfg = result.config as {
379+
gateway?: { mode?: string; auth?: { password?: string } };
380+
models?: {
381+
providers?: { default?: { apiKey?: { source?: string; provider?: string; id?: string } } };
382+
};
383+
};
384+
expect(cfg.gateway?.mode).toBe("default");
385+
expect(cfg.gateway?.auth?.password).toBe(REDACTED_SENTINEL);
386+
expect(cfg.models?.providers?.default?.apiKey?.source).toBe("env");
387+
expect(cfg.models?.providers?.default?.apiKey?.provider).toBe("default");
388+
expect(cfg.models?.providers?.default?.apiKey?.id).toBe(REDACTED_SENTINEL);
389+
const restored = restoreRedactedValues(result.config, snapshot.config, mainSchemaHints);
383390
expect(restored).toEqual(snapshot.config);
384391
});
385392

0 commit comments

Comments
 (0)