🔒 Aisle Security Analysis

We found 3 potential security issue(s) in this PR:

1. 🟠 allowedPaths secret scoping bypass: gateway assignments outside scope are still merged into config

Description

resolveCommandSecretRefsViaGateway introduces an allowedPaths parameter intended to scope which SecretRef paths are enforced/resolved (e.g., per-channel/per-account scoping). However, in the gateway-backed path, all assignments returned by the gateway are applied to resolvedConfig without filtering by allowedPaths.

This means a caller can request broad targetIds (e.g., channels.discord.accounts.*.token) while supplying a narrow allowedPaths (e.g., one account token), but the client will still:

• ask the gateway to resolve all secrets matching the broad targetIds
• merge all returned assignments into resolvedConfig, including secrets for disallowed paths
• then run unresolved/enforcement analysis only over allowedPaths, so the extra secrets are not reflected in targetStatesByPath

Impact:

• Secret isolation between accounts/channels is broken: secrets outside the intended scope may be resolved and become available in-process to downstream code.
• If any later code path exposes parts of the resolved config (debug/status output, tool responses, plugin errors, etc.), this becomes an information disclosure risk.
• A compromised/malicious gateway could also inject assignments to any existing secret path in config (since the client does not verify assignments correspond to requested/allowed targets).

Vulnerable code:

for (const assignment of parsed.assignments) {
  const pathSegments = assignment.pathSegments.filter((segment) => segment.length > 0);
  if (pathSegments.length === 0) {
    continue;
  }
  setPathExistingStrict(resolvedConfig, pathSegments, assignment.value);
}

No check is performed against params.allowedPaths before mutating resolvedConfig.

Recommendation

Filter (and ideally validate) gateway assignments before applying them.

Minimum fix (enforce allowedPaths when present):

for (const assignment of parsed.assignments) {
  const pathSegments = assignment.pathSegments.filter((s) => s.length > 0);
  if (pathSegments.length === 0) continue;

  const path = pathSegments.join(".");
  if (params.allowedPaths && !params.allowedPaths.has(path)) {
    continue; // ignore out-of-scope assignments
  }

  setPathExistingStrict(resolvedConfig, pathSegments, assignment.value);
}

Stronger fix (also prevent unexpected assignments even when allowedPaths is undefined):

• Build an allowedAssignmentPaths set from discoverConfigSecretTargetsByIds(config, targetIds) (optionally intersect with allowedPaths) and only apply assignments whose path is in that set.
• Consider extending the gateway protocol to accept allowedPaths so the gateway itself does not resolve/return out-of-scope secrets.

2. 🟡 Sensitive data exposure via unredacted plugin error stack/message logging (message-actions)

Description

listChannelMessageActions() / listChannelMessageCapabilities() now catch exceptions thrown by channel plugins and log raw Error.stack (or Error.message) via defaultRuntime.error.

This is risky because:

• defaultRuntime.error writes directly to console.error (stderr) and may be captured by log aggregation/telemetry.
• Stack traces and error messages commonly include sensitive values such as:
  • request URLs containing embedded credentials (e.g., Telegram Bot API URLs embed the bot token as /bot<token>/...),
  • Authorization: Bearer ... headers,
  • API keys/tokens included by third-party plugin error messages.
• This code does not apply any existing redaction utilities (e.g. redactSensitiveText() in src/logging/redact.ts, already used in src/infra/errors.ts).

Vulnerable code:

const stack = params.error instanceof Error && params.error.stack ? params.error.stack : null;
defaultRuntime.error?.(
  `[message-actions] ${params.pluginId}.actions.${params.operation} failed: ${stack ?? message}`,
);

Impact: secrets present in plugin exception messages/stacks can be written to logs.

Recommendation

Redact sensitive data before logging and avoid logging full stacks by default.

• Prefer logging a short, redacted message.
• If you must log stacks, redact them first.

Example using existing redaction helper:

import { redactSensitiveText } from "../../logging/redact.js";

const message = params.error instanceof Error ? params.error.message : String(params.error);
const stackOrMessage =
  params.error instanceof Error && params.error.stack ? params.error.stack : message;

defaultRuntime.error?.(
  `[message-actions] ${params.pluginId}.actions.${params.operation} failed: ` +
    redactSensitiveText(stackOrMessage),
);

Optionally:

• gate stack logging behind a verbose/debug flag
• log an error code/classification rather than the raw stack

3. 🔵 Sensitive data exposure via unredacted plugin config error logging (channel-selection)

Description

listConfiguredMessageChannels() now catches plugin resolveAccount() / isConfigured() exceptions and logs the raw Error.message via defaultRuntime.error.

This is risky because:

• These errors originate in plugin code and may include sensitive values (tokens, secrets, credential-bearing URLs, or headers) in their message text.
• The project already has a redaction mechanism (redactSensitiveText() in src/logging/redact.ts, used e.g. by src/infra/errors.ts), but it is not applied here.
• defaultRuntime.error writes directly to stderr (console.error), which may be collected/forwarded.

Vulnerable code:

const message = params.error instanceof Error ? params.error.message : String(params.error);
...
defaultRuntime.error?.(
  `[channel-selection] ${params.pluginId}(${params.accountId}) ${params.operation} failed: ${message}`,
);

Impact: secrets embedded in plugin exception messages can be written to logs.

Recommendation

Redact sensitive strings before logging, using the existing project helper.

import { redactSensitiveText } from "../../logging/redact.js";

const message = params.error instanceof Error ? params.error.message : String(params.error);
defaultRuntime.error?.(
  `[channel-selection] ${params.pluginId}(${params.accountId}) ${params.operation} failed: ` +
    redactSensitiveText(message),
);

Additionally consider:

• logging a stable error code/classification rather than arbitrary plugin messages
• bounding the dedupe Set (LRU/TTL/max size) to avoid retaining large/unique messages

Analyzed PR: #48728 at commit 1501008

_{Last updated on: 2026-03-17T05:59:13Z}

Greptile Summary

This PR hardens the SecretRef resolution pipeline by scoping secret enforcement to the active message target (channel + account) and guarding several previously-fatal paths in doctor, status, channel selection, and message action discovery. The overall approach is well-structured and the new abstractions (resolveMessageSecretScope, getScopedChannelsCommandSecretTargets) are clean.

Key changes and findings:

• New scoped resolution flow (message-secret-scope.ts, command-secret-targets.ts, command-secret-gateway.ts): Channel/account scope is inferred from CLI inputs and threaded through the gateway resolver. One edge-case logic bug exists in getScopedChannelsCommandSecretTargets (see inline comment): when the selected account has no configured SecretRefs but a sibling account on the same channel does, allowedPaths comes back empty and the function falls back to the unscoped { targetIds } return, which means the sibling's unresolved secret can still trigger an enforce_resolved failure—defeating the scoping goal.
• Discord cfg threading (discord-actions-messaging.ts): Consistent { ...cfgOptions, accountId } spread applied to all read/probe/write actions. Clean and well-tested.
• Hardened error paths (message-actions.ts, channel-selection.ts): Per-key deduplication logging + __testing reset hooks follow a consistent pattern. Errors are now non-fatal, with once-per-key logging to defaultRuntime.error.
• Doctor/status improvements (doctor-config-flow.ts, status-all.ts, diagnosis.ts): Telegram token inspection failures are now warnings, and status --all surfaces secret diagnostics without crashing. Both are straightforward and safe.
• Coverage test (command-secret-resolution.coverage.test.ts): The assertion was relaxed from "targetIds: get" to "targetIds:" to accommodate the new scopedTargets.targetIds access pattern. The relaxation is functionally justified by the existing code, but it weakens the gate for future callsites.

Confidence Score: 3/5

• Safe to merge with the noted edge-case fix; the fallback in getScopedChannelsCommandSecretTargets can cause enforce_resolved failures for unrelated accounts that the PR is explicitly trying to prevent.
• The broad hardening changes (doctor, status, channel-selection, message-actions) are well-guarded and clearly tested. The core new abstraction (resolveMessageSecretScope + getScopedChannelsCommandSecretTargets) is sound for the common cases covered by tests. However, the allowedPaths.size === 0 fallback to { targetIds } is a real logic regression in the scoping contract: a user targeting an account with no configured secrets but whose channel has other accounts with unresolved secrets will still hit enforce_resolved failures, which is the exact scenario this PR aims to fix. This is a correctness issue that warrants a one-line fix before merging.
• src/cli/command-secret-targets.ts (line 910) — the allowedPaths.size > 0 guard causes the scoping to silently collapse back to full-channel enforcement in the edge case where the selected account has no SecretRefs configured.

This is a comment left during a code review.
Path: src/cli/command-secret-targets.ts
Line: 910

Comment:
**Scoping fallback breaks isolation when selected account has no secrets**

When both `channel` and `accountId` are specified but the targeted account has no SecretRefs in the config (while a _sibling_ account on the same channel does), `allowedPaths` ends up empty and the function returns `{ targetIds }` with no `allowedPaths`. This means `collectConfiguredTargetRefPaths` (in `command-secret-gateway.ts`) has no restriction and includes every configured secret under `targetIds`—including the sibling account's tokens.

With `mode: "enforce_resolved"` (used in `message-tool.ts`), an unresolved sibling account secret then causes the command to fail even though that account was never the target, defeating the core isolation goal of this PR.

For example:
- Channel `discord`, selected `accountId: "ops"`, but `ops` has no SecretRef tokens
- `accounts.chat.token` IS a SecretRef and is unresolved
- `allowedPaths` → empty → `{ targetIds }` returned → `chat.token` included in enforcement → command fails

The fix is to return `{ targetIds, allowedPaths }` unconditionally when both `channel` and `accountId` are scoped, letting an empty `allowedPaths` act as "enforce nothing for this scope". `collectConfiguredTargetRefPaths` already handles this correctly: an empty but truthy `Set` causes the inner `if (!params.allowedPaths.has(...))` branch to skip every target, producing an empty `configuredTargetRefPaths` and a no-op resolution.

```suggestion
  return { targetIds, allowedPaths };
```

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

---

This is a comment left during a code review.
Path: src/cli/command-secret-resolution.coverage.test.ts
Line: 776

Comment:
**Coverage contract weakened by loosened assertion**

The original `"targetIds: get"` check enforced that every `resolveCommandSecretRefsViaGateway` callsite sourced `targetIds` from a designated registry getter (e.g. `getChannelsCommandSecretTargetIds()`). Changing to `"targetIds:"` now accepts any assignment—including inline `Set` construction or direct property access like `scopedTargets.targetIds`—so a future callsite that bypasses the target registry entirely would pass this check.

The new callers in `message.ts` and `message-tool.ts` correctly go through `getScopedChannelsCommandSecretTargets` (which internally calls `selectChannelTargetIds` → `COMMAND_SECRET_TARGETS`), so they respect the registry. Consider tightening the assertion to cover both the legacy and new patterns:

```suggestion
      expect(source).toContain("targetIds: get") || expect(source).toContain("targetIds: scopedTargets.targetIds") || expect(source).toContain("targetIds:");
```

Or, if the test is intentionally an open-ended coverage gate, a comment explaining the relaxed intent would help future readers understand the trade-off.

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

_{Last reviewed commit: 82b9f9c}

Addressed both Greptile points in ce5fd6034a.

What changed:

• src/cli/command-secret-targets.ts

  • getScopedChannelsCommandSecretTargets(...) now always returns { targetIds, allowedPaths } when both channel/account scope are present.
  • This preserves scope semantics even when allowedPaths is empty, instead of collapsing back to unscoped channel enforcement.

• src/cli/command-secret-targets.test.ts

  • Added/updated coverage to assert scoped empty-set behavior is preserved (allowedPaths remains defined and empty when scoped target paths are absent).

• src/cli/command-secret-resolution.coverage.test.ts

  • Tightened the relaxed targetIds check to only accept supported wiring patterns:
    • registry getter callsites (targetIds: get...())
    • scoped message callsites (targetIds: scopedTargets.targetIds)
  • This restores the gate intent without breaking the new scoped pattern.

Validation run:

• pnpm test -- src/cli/command-secret-targets.test.ts src/cli/command-secret-resolution.coverage.test.ts src/cli/command-secret-gateway.test.ts src/cli/message-secret-scope.test.ts src/commands/message.test.ts src/agents/tools/message-tool.test.ts src/infra/outbound/channel-selection.test.ts src/channels/plugins/message-actions.test.ts src/commands/doctor-config-flow.test.ts src/commands/doctor-config-flow.include-warning.test.ts src/commands/doctor-config-flow.missing-default-account-bindings.test.ts src/commands/status-all/report-lines.test.ts
• Result: pass.

I think I’m running into the same underlying SecretRef/runtime-resolution problem in a few other CLI paths, not just the ones this PR touches @joshavant

I’m seeing similar failures in other commands where an unrelated unresolved channel SecretRef seems to stop the command entirely.

The examples below are from 2026.3.13 but I get the same errors when running from git-main as well

user@host:~$ openclaw agents

🦞 OpenClaw 2026.3.13 (61d171a) — Say "stop" and I'll stop—say "ship" and we'll both learn a lesson.

14:21:31 [plugins] memory-lancedb: plugin registered (db: /home/user/.openclaw/memory/lancedb, lazy init)
Error: channels.discord.accounts.coder.token: unresolved SecretRef "file:openclaw:/channels/discord/coder". Resolve this command against an active gateway runtime snapshot before reading it.

user@host:~$ openclaw doctor

🦞 OpenClaw 2026.3.13 (61d171a) — Half butler, half debugger, full crustacean.

┌  OpenClaw doctor
14:22:31 [plugins] memory-lancedb: plugin registered (db: /home/user/.openclaw/memory/lancedb, lazy init)
│
◇  Update
│  This install is not a git checkout.
│  Run `openclaw update` to update via your package manager (npm/pnpm), then rerun doctor.
│
◇  Gateway auth
│  Gateway token is managed via SecretRef and is currently unavailable.
│  Doctor will not overwrite gateway.auth.token with a plaintext value.
│  Resolve/rotate the external secret source, then rerun doctor.
│
Error: channels.discord.accounts.default.token: unresolved SecretRef "file:openclaw:/channels/discord/default". Resolve this command against an active gateway runtime snapshot before reading it.

user@host:~$ openclaw gateway restart

🦞 OpenClaw 2026.3.13 (61d171a) — I've survived more breaking changes than your last three relationships.

14:22:41 [plugins] memory-lancedb: plugin registered (db: /home/user/.openclaw/memory/lancedb, lazy init)

⚠️  Unable to verify gateway token drift: gateway.auth.token SecretRef is configured but unavailable in this command path.

Restarted systemd service: openclaw-gateway.service

My question is whether the same fix pattern from this PR should be applied more broadly to other CLI commands too.

More specifically:

• scope SecretRef resolution to only the config paths a command actually needs
• don’t fail a command because of unrelated unresolved channel/account SecretRefs
• for read-only or diagnostic paths, prefer a warning or degraded output over a hard failure

doctor looks directly in scope for this PR. agents feels like the same root problem showing up in a different command path. gateway restart seems related, though maybe not exactly the same case.

If that read is right, I’d expect there are probably a few more commands that still need the same treatment.