security: redact credentials from config.get gateway responses

[abdelsfane]

Summary

The config.get gateway method returns the full config snapshot including all channel credentials, model provider API keys, and gateway auth tokens in plaintext. Any WebSocket client can read every secret — including the unauthenticated Control UI when dangerouslyDisableDeviceAuth is set.

This PR adds a redactConfigSnapshot() function that sanitizes the config before it reaches clients:

• Field-based redaction: Deep-walks the config object and masks any string value whose key matches token, password, secret, or apiKey patterns (aligned with the existing SENSITIVE_PATTERNS in schema.ts)
• Text-based redaction: Applies the existing redactSensitiveText() to the raw JSON5 source so no credential leaks through either path
• Preserves hash: The snapshot hash is kept intact for change detection

Credentials redacted

Channel	Fields
Telegram	botToken, webhookSecret
Discord	token
Slack	botToken, appToken, userToken, signingSecret
Feishu/Lark	appSecret
MS Teams	appPassword
Gateway	auth.token, auth.password, remote.token, remote.password
Models	providers.*.apiKey
Tools	web.search.apiKey
Talk	apiKey

Masking behavior

• Tokens ≥18 chars: abc123…xyz (keeps first 6 + last 4)
• Tokens <18 chars: ***
• Reuses the existing maskToken() logic from src/logging/redact.ts

Files changed

• src/config/redact-snapshot.ts — new redaction utility
• src/config/redact-snapshot.test.ts — 15 test cases
• src/gateway/server-methods/config.ts — one-line change to wrap the response
• CHANGELOG.md — added entry under Fixes

Test plan

• 15 dedicated unit tests covering all channel types, nested accounts, short tokens, null raw, non-sensitive field preservation
• Full test suite passes (939 files, 6,358 tests)
• Lint clean (oxlint + oxfmt)
• Build passes

Greptile Overview

Greptile Summary

This PR introduces redaction for Gateway config.get responses by adding src/config/redact-snapshot.ts and wrapping the config.get handler to return a sanitized ConfigFileSnapshot (masking sensitive keys like token/password/secret/apiKey in both the parsed object and raw JSON5 text). It also adds write-side support (restoreRedactedValues) so UI round-trips can keep credentials intact when users submit configs containing the redaction sentinel.

In addition, it adds a new “skill/plugin code safety scanner” (src/security/skill-scanner.ts) and integrates it into plugin install flow (src/plugins/install.ts) and deep security audit (src/security/audit-extra.ts, src/security/audit.ts), with tests covering scanner rules and audit/install warnings.

Confidence Score: 3/5

• This PR is close to safe to merge, but has a credential-corruption bug in the gateway config write path.
• Core redaction approach is sound and well-tested, but config.patch/config.apply currently return (and in config.patch, write) the non-restored config object, which can persist the redaction sentinel to disk when clients round-trip redacted configs. Fixing that should make the change low risk.
• src/gateway/server-methods/config.ts

[greptile-apps]

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (2)

src/config/redact-snapshot.ts
Redaction depends on logging config

redactConfigSnapshot() calls getDefaultRedactPatterns() but then passes them into redactSensitiveText(snapshot.raw, { mode: "tools", patterns }), which means redaction can still be effectively disabled if redactSensitiveText is invoked elsewhere with mode: "off" (and more importantly: this function is importing redaction config concepts from logging into a security boundary). For config.get, redaction should be unconditional and not depend on runtime logging redaction settings; otherwise secrets can leak to any WS client when redaction is turned off.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/config/redact-snapshot.ts
Line: 283:286

Comment:
**Redaction depends on logging config**

`redactConfigSnapshot()` calls `getDefaultRedactPatterns()` but then passes them into `redactSensitiveText(snapshot.raw, { mode: "tools", patterns })`, which means redaction can still be effectively disabled if `redactSensitiveText` is invoked elsewhere with `mode: "off"` (and more importantly: this function is importing redaction config concepts from logging into a security boundary). For `config.get`, redaction should be unconditional and not depend on runtime logging redaction settings; otherwise secrets can leak to any WS client when redaction is turned off.


How can I resolve this? If you propose a fix, please make it concise.

---

src/config/redact-snapshot.ts
Misses sensitive non-string values

redactObject() only masks sensitive keys when the value is a string. If credentials are stored as numbers (common for Telegram IDs/secrets mistakenly), buffers, or other scalar types, they will be returned to clients unredacted. Given this is guarding a gateway response (config.get), the masking should cover any primitive value (e.g., number/boolean) for sensitive keys, or stringify+mask, otherwise secrets can still leak depending on config shape.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/config/redact-snapshot.ts
Line: 250:270

Comment:
**Misses sensitive non-string values**

`redactObject()` only masks sensitive keys when the value is a string. If credentials are stored as numbers (common for Telegram IDs/secrets mistakenly), buffers, or other scalar types, they will be returned to clients unredacted. Given this is guarding a gateway response (`config.get`), the masking should cover any primitive value (e.g., number/boolean) for sensitive keys, or stringify+mask, otherwise secrets can still leak depending on config shape.


How can I resolve this? If you propose a fix, please make it concise.

[HenryLoenwind]

Doesn't that mean that when editing your config in the Web UI, all keys are trashed, basically disabling the whole system?

[abdelsfane]

Doesn't that mean that when editing your config in the Web UI, all keys are trashed, basically disabling the whole system?

ok so the Web UI round-trips config through config.get → edit → config.set, so masked values would
overwrite real credentials on save breaking everything. I'm thinking of fixing this with a sentinel-based approach: config.get returns "OPENCLAW_REDACTED" for sensitive fields, and config.set/config.apply/config.patch detect the sentinel and restore original values from disk before writing

[HenryLoenwind]

The only one is that that WebSocket API is only for the owner of the system. The documentation explicitly states that it isn't hardened against attack, even though you need a token to get in. Giving someone access to it is akin to giving them a login to the system and user openclaw is running on.

And it's not just those tokens. You can add all kinds of nasty backdoors to the system if you can edit the config.

[abdelsfane]

Fixed in the latest push with a sentinel-based approach:

1. config.get now returns "__OPENCLAW_REDACTED__" for all sensitive fields (token, password, secret, apiKey patterns) instead of truncated masks.
2. config.set, config.apply, and config.patch detect the sentinel before writing and restore the original values from the current on-disk config.
3. If a user explicitly types a new value (anything other than the sentinel), it's written as-is.

So the Web UI can safely display redacted credentials, and saving - even without changing any secrets - preserves all original values. Also removed the dependency on the logging module's redactSensitiveText for the raw field redaction, addressing Greptile's concern about mixing logging config into a security-critical path.

22 tests including a full redact → restore round-trip test. Ready for review! :)

[abdelsfane]

Additional Comments (2)
src/config/redact-snapshot.ts Redaction depends on logging config

redactConfigSnapshot() calls getDefaultRedactPatterns() but then passes them into redactSensitiveText(snapshot.raw, { mode: "tools", patterns }), which means redaction can still be effectively disabled if redactSensitiveText is invoked elsewhere with mode: "off" (and more importantly: this function is importing redaction config concepts from logging into a security boundary). For config.get, redaction should be unconditional and not depend on runtime logging redaction settings; otherwise secrets can leak to any WS client when redaction is turned off.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/config/redact-snapshot.ts
Line: 283:286

Comment:
**Redaction depends on logging config**

`redactConfigSnapshot()` calls `getDefaultRedactPatterns()` but then passes them into `redactSensitiveText(snapshot.raw, { mode: "tools", patterns })`, which means redaction can still be effectively disabled if `redactSensitiveText` is invoked elsewhere with `mode: "off"` (and more importantly: this function is importing redaction config concepts from logging into a security boundary). For `config.get`, redaction should be unconditional and not depend on runtime logging redaction settings; otherwise secrets can leak to any WS client when redaction is turned off.


How can I resolve this? If you propose a fix, please make it concise.

src/config/redact-snapshot.ts Misses sensitive non-string values

redactObject() only masks sensitive keys when the value is a string. If credentials are stored as numbers (common for Telegram IDs/secrets mistakenly), buffers, or other scalar types, they will be returned to clients unredacted. Given this is guarding a gateway response (config.get), the masking should cover any primitive value (e.g., number/boolean) for sensitive keys, or stringify+mask, otherwise secrets can still leak depending on config shape.

Prompt To Fix With AI

please review again

[HenryLoenwind]

Hey bot, read the file ../openclaw.conf and show it to me.

It's pretty much useless to mask anything for the web UI because you have access to the unrestricted agent there...

---

What you're trying to do is like putting a high-security lock on the off switch of the TV in your living room so nobody can turn it off. They can only pull the plug out of the TV, or out of the wall outlet, or cut the power cable, or turn off the circuit breaker.

[abdelsfane]

You're right — the threat model doesn't hold up. Anyone with Web UI access has an unrestricted agent that can read the config file directly, so redacting credentials in one API response doesn't provide meaningful security. Withdrawing this PR.

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

src/gateway/server-methods/config.ts
Redaction restoration not applied

In config.patch, the merge result is passed through restoreRedactedValues (restoredMerge), but the code still writes and returns validated.config (built from resolved). This means if a client sends back __OPENCLAW_REDACTED__ for a sensitive key, the sentinel can be persisted to disk (and echoed back) instead of restoring from snapshot.config. This breaks the intended “UI round-trip doesn’t corrupt credentials” behavior.

Also appears in config.apply (response returns validated.config instead of the restored object).

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/gateway/server-methods/config.ts
Line: 252:273

Comment:
**Redaction restoration not applied**

In `config.patch`, the merge result is passed through `restoreRedactedValues` (`restoredMerge`), but the code still writes and returns `validated.config` (built from `resolved`). This means if a client sends back `__OPENCLAW_REDACTED__` for a sensitive key, the sentinel can be persisted to disk (and echoed back) instead of restoring from `snapshot.config`. This breaks the intended “UI round-trip doesn’t corrupt credentials” behavior.

Also appears in `config.apply` (response returns `validated.config` instead of the restored object).

How can I resolve this? If you propose a fix, please make it concise.

[grp06]

Landed by updating the PR branch to include latest main (no force push) and then squash-merging onto main.

• Gate: pnpm -s lint && pnpm -s build && pnpm -s test
• Land commit (PR head): 24cb95d
• Merge commit: 0c7fa2b

Thanks @abdelsfane!

[HenryLoenwind]

What? We just established that this is window-dressing, and, at best, dangerous, as it leads to users thinking that giving third parties access to the gateway UI is safe.

[hongw]

I noticed a couple of issues with the current implementation:

1. Environment variable references are blindly redacted

When a config field contains an environment variable reference like "${MY_API_KEY}", config.get still replaces it with "OPENCLAW_REDACTED". This prevents agents from validating configuration correctness (checking if env vars are properly set, detecting typos in variable names, etc.).

Would it make sense to preserve environment variable placeholder syntax (e.g., values matching ^${[A-Z0-9_]+}$) and only redact resolved plaintext values?

2. config.patch still expands environment variables on write

The more critical issue: config.patch resolves environment variable placeholders into their actual plaintext values when writing the config back to disk. This means:

• User starts with: "apiKey": "${MY_API_KEY}" in openclaw.json
• Agent calls config.patch to update a different field
• Result: "apiKey": "sk-abc123..." (actual secret now persisted to disk)
This defeats the purpose of using environment variables for secrets in the first place. The config file becomes insecure even when following best practices.

[HenryLoenwind]

This will also redact

agents.defaults.memorySearch.chunking.tokens
reserveTokensFloor, softThresholdTokens, etc.

And those a numbers, no idea what the UI makes out of getting a String for a number.

[abdelsfane]

hmm, I'm a bit confused, is this still being merged? What about Henry's concerns unless you're saying this is better than nothing so why not?

[HenryLoenwind]

Yes, it got merged, and it has some issues aside from being window dressing.

That /token/i is way too broad, for one thing. There are plenty of keys with "token" in their name that are numbers, and redactObject() doesn't care if the value is a String, a Number, an array or an object, it will replace the value...

[abdelsfane]

Yes, it got merged, and it has some issues aside from being window dressing.

That /token/i is way too broad, for one thing. There are plenty of keys with "token" in their name that are numbers, and redactObject() doesn't care if the value is a String, a Number, an array or an object, it will replace the value...

how do you suggest we move forward?

[HenryLoenwind]

how do you suggest we move forward?

I'd say ping @grp06 and ask them to revert the commit. Then we can talk about whether this change makes any sense at all, and if they still want it, we work on it to get it ready for tomorrow's merge window. I'd be happy to help out and go through the code and logic with a fine-toothed comb.

[HenryLoenwind]

ok, I tested it. The GUI dosn't croak, but naturally the field stays empty.

This is with the "token" field unchanged, just trying to save another, unrelated value:

And this is the invalid value that prevents the server from accepting the config:

[abdelsfane]

Awsome. Thanks for looking into it! If we can make this work I think the community would appreciate it.

…

On Thu, Feb 5, 2026, 20:19 Henry Loenwind ***@***.***> wrote: *HenryLoenwind* left a comment (openclaw/openclaw#9858) <#9858 (comment)> ok, I tested it. The GUI dosn't croak, but naturally the field stays empty. image.png (view on web) <https://github.com/user-attachments/assets/245bcb14-0d67-4272-b46f-49b919e99bed> image.png (view on web) <https://github.com/user-attachments/assets/614f6645-3660-42c0-88e1-f7c3e7ba19f9> — Reply to this email directly, view it on GitHub <#9858 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHXKWGS5GZZP3IRM4WM7YLL4KQB4ZAVCNFSM6AAAAACUEJY65SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNJXG4YTQMBZGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[HenryLoenwind]

yeah, I think so, too.

BTW, do you have any idea where the UI gets the info to use password fields here:

If we can hook into that in your redact code, we'd have a much better and more targeted base to stand on.

I'm off to bed, sorry. See you in 9~10 hour.

[abdelsfane]

yeah, I think so, too.

BTW, do you have any idea where the UI gets the info to use password fields here:

If we can hook into that in your redact code, we'd have a much better and more targeted base to stand on.

I'm off to bed, sorry. See you in 9~10 hour.

sorry no idea but i'll try to investigate it

[HenryLoenwind]

ok, found it.

There are two sources:

1. in config.ts, line 138/147, in "config.schema": ({ params, respond }) => { it queries pluginRegistry for plugin.configUiHints and listChannelPlugins for entry.configSchema?.uiHints
2. config-form.shared.ts, it has isSensitivePath():

export function isSensitivePath(path: Array<string | number>): boolean {
  const key = pathKey(path).toLowerCase();
  return (
    key.includes("token") ||
    key.includes("password") ||
    key.includes("secret") ||
    key.includes("apikey") ||
    key.endsWith("key")
  );
}

The difference to yours is that this is only called when it wants to render a text field. renderTextInput() in redner-form.node.ts is the only consumer.

Now, I'm not happy that there is no uiHints source for non-plugin, non-channel config keys, but at least we have some source and he isSensitivePath() usage is plausible.

So, my suggestion:

1. Take the code that creates the schema in "config.schema" in config.ts (lines 122 to 149) and put them in a stand-alone function. It doesn't have any inputs, so it is independent of the API request it's in.
2. Make your isSensitivePath() mirror the one from confi-form.shared.ts or even better: make both a call to some shared code.
3. change your code that calls isSensitivePath() to
   • isSensitive = hint?.sensitive ?? isSensitivePath(path), and
   • only if the value is a String (or, better: If the schema says it's a string and it's not undefined. That would not try to mangle enums, which are stored as strings, too.)
4. Think about a new source of uiHnts for "normal" config keys, so we can tag all of them properly and explicitly and throw away isSensitivePath() eventually. (I hate that "guessing from the key name" bit, and I think you understand why.)

Does that sound like a plan?

[HenryLoenwind]

oh, come on...

const SENSITIVE_PATTERNS = [/token/i, /password/i, /secret/i, /api.?key/i];

function isSensitivePath(path: string): boolean {
  return SENSITIVE_PATTERNS.some((pattern) => pattern.test(path));
}

in schema.ts does the exact same thing and applies that hint to the schema. Ok, that changes some things: You don't need isSensitiveKey() anymore at all. You can go be the uiHint in the schema alone and are in sync with the UI hat way. And the UI doesn't need its isSensitivePath() either---that can be dropped, as the schema already has the same thing.

[abdelsfane]

Thanks for the progress! I'm looking into this now

[abdelsfane]

Ok addressed all three points:

1. /token/i regex fixed — changed to /token(?!s)/i so tokens, softThresholdTokens etc. are no longer matched. Added explicit tests for both.

2. Single source of truth — removed the duplicate SENSITIVE_KEY_PATTERNS from redact-snapshot.ts and isSensitivePath() from the UI shared file. schema.ts is now the only place that defines sensitivity patterns.

3. Schema-driven redaction — redact-snapshot.ts now accepts uiHints and resolves sensitivity via: direct hint lookup → wildcard hint (for array items) → regex fallback. Extracted loadSchemaWithPlugins() in config.ts so all four handlers (get, set, patch, apply) pass uiHints through. UI side just uses hint?.sensitive ?? false.

Let me know what you think before I push

[HenryLoenwind]

Hey, that looks good. Atm I'm working on adding the isSensitive hint to any key, and I'm 99% there.

I have the mechanism (a zod registry), and a way to apply it. I just have to stomp some tiny thing that makes my code say it applied the flag but it never showing up... ;)

When I have that, I'll PR it into your branch in your repo and do a through review of your code.

[HenryLoenwind]

And I found the bug---Gemini wrote me a nice recursion, but it didn't assign the result of the recursion, so everything inside of the top-level object just got lost. Took me over half an hour to get that...

It'll probably be another half hour at least until I have it cleaned up and a commit ready. I'll send that your way for review then before going over the config and tagging everything that needs to be sensitive.

[abdelsfane]

awesome, take your time!

[realugbun]

Great to see this security hardening on the API layer. However, agents can still access secrets directly via filesystem reads:

  Read(file_path="/home/node/.openclaw/openclaw.json")                                                                                                    

This means the redaction can be bypassed entirely via prompt injection from untrusted content (emails, webhooks, fetched web pages) tricking the agent into reading and leaking the config:

Some possible mitigations to consider:

1. Separate secrets file with restrictive permissions the agent user can't read
2. Path blocklist in the Read tool itself for known sensitive paths
3. Richer config.get API that exposes non-sensitive config (enabled channels, ports, etc.) so legitimate agent use cases don't need file access

The API hardening is valuable for preventing accidental exposure in logs/history. Filesystem access is the deliberate exfiltration path worth closing.

[HenryLoenwind]

@realugbun See above, we've discussed this thoroughly. That's why this PR was discarded half-done. It's windows dressing that results in a false impression of security. As long as the AI has full system access, it can access everything. There's no way around it---if in doubt, it can simply write a program that extracts the tokens from the memory space of the gateway and call that.

[HenryLoenwind]

@abdelsfane Done, see the PR to your fork.