All secret-tool errors coerced to "not found"

The bare catch block discards the real error from execFileAsync and rethrows a generic "not found" message. If secret-tool fails for any reason other than the secret not existing — e.g. the GNOME Keyring is locked, the D-Bus session bus is not running, or the daemon crashes — users will see Secret "x" not found in keyring rather than the actual problem, making it very hard to diagnose.

Consider rethrowing the original error for non-"not-found" failures:

  } catch (err: unknown) {
    const msg = err instanceof Error ? err.message : String(err);
    // secret-tool exits 1 with empty stdout when key doesn't exist
    if (!msg || msg.includes("No such attribute")) {
      throw new Error(`Secret "${name}" not found in keyring`);
    }
    throw new Error(`Failed to retrieve secret "${name}" from keyring: ${msg}`, { cause: err instanceof Error ? err : undefined });
  }

This is a comment left during a code review.
Path: src/config/keyring-secret-provider.ts
Line: 193-196

Comment:
**All `secret-tool` errors coerced to "not found"**

The bare `catch` block discards the real error from `execFileAsync` and rethrows a generic "not found" message. If `secret-tool` fails for any reason other than the secret not existing — e.g. the GNOME Keyring is locked, the D-Bus session bus is not running, or the daemon crashes — users will see `Secret "x" not found in keyring` rather than the actual problem, making it very hard to diagnose.

Consider rethrowing the original error for non-"not-found" failures:

```ts
  } catch (err: unknown) {
    const msg = err instanceof Error ? err.message : String(err);
    // secret-tool exits 1 with empty stdout when key doesn't exist
    if (!msg || msg.includes("No such attribute")) {
      throw new Error(`Secret "${name}" not found in keyring`);
    }
    throw new Error(`Failed to retrieve secret "${name}" from keyring: ${msg}`, { cause: err instanceof Error ? err : undefined });
  }
```

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

Malformed YAML frontmatter pasted as plain markdown

The block starting here contains what looks like document-level YAML frontmatter fields (summary:, read_when:, title:) rendered as raw markdown text. This is likely a copy-paste artefact from a separate doc that was accidentally appended inline rather than structured as proper markdown.

In addition, the "Supported Providers" table that follows only lists GCP, AWS, HashiCorp Vault, and Azure Key Vault, with no mention of the keyring and 1password providers actually being introduced by this PR. The table should be updated to reflect the new providers:

This is a comment left during a code review.
Path: docs/gateway/secrets.md
Line: 454-462

Comment:
**Malformed YAML frontmatter pasted as plain markdown**

The block starting here contains what looks like document-level YAML frontmatter fields (`summary:`, `read_when:`, `title:`) rendered as raw markdown text. This is likely a copy-paste artefact from a separate doc that was accidentally appended inline rather than structured as proper markdown.

In addition, the "Supported Providers" table that follows only lists GCP, AWS, HashiCorp Vault, and Azure Key Vault, with no mention of the `keyring` and `1password` providers actually being introduced by this PR. The table should be updated to reflect the new providers:

| Provider            | Status       |
| ------------------- | ------------ |
| GCP Secret Manager  | ✅ Supported |
| OS Keyring          | ✅ Supported |
| 1Password           | ✅ Supported |
| AWS Secrets Manager | Planned      |
| HashiCorp Vault     | Planned      |
| Azure Key Vault     | Planned      |

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

1password provider name never matched by regex

The SECRET_REF_PATTERN uses [a-z]+ for the provider group, which only matches lowercase ASCII letters. The provider name 1password starts with the digit 1, so any config reference using this provider will never match — the reference is silently left unresolved and the 1password provider is completely non-functional.

This bug affects all three resolution paths: extractSecretReferences, containsSecretReference, and resolveString.

The stripEscapedRefs helper at line 263 has the same [a-z]+ pattern and needs the same fix.

Fix: expand the provider character class to allow a leading digit:

This is a comment left during a code review.
Path: src/config/secret-resolution.ts
Line: 17

Comment:
**`1password` provider name never matched by regex**

The `SECRET_REF_PATTERN` uses `[a-z]+` for the provider group, which only matches lowercase ASCII letters. The provider name `1password` starts with the digit `1`, so any config reference using this provider will **never match** — the reference is silently left unresolved and the `1password` provider is completely non-functional.

This bug affects all three resolution paths: `extractSecretReferences`, `containsSecretReference`, and `resolveString`.

The `stripEscapedRefs` helper at line 263 has the same `[a-z]+` pattern and needs the same fix.

Fix: expand the provider character class to allow a leading digit:

```suggestion
const SECRET_REF_PATTERN = /\$\{([a-z0-9][a-z0-9]*):([a-zA-Z0-9_\-/.]+?)(?:#([a-zA-Z0-9]+))?\}/g;
```

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

Expand provider regex to resolve 1password references

The secret reference pattern only accepts provider names matching [a-z]+, so ${1password:...} never matches. Both extraction and substitution rely on this regex, which means 1Password placeholders are silently left unresolved instead of being fetched (or even rejected), making the newly added 1Password provider effectively unusable in real configs.

Useful? React with 👍 / 👎.

Allow 1Password item and URI syntax in secret names

The name token class excludes spaces and :, so common 1Password inputs like item titles with spaces (for example My Item.api-key) and full op://... URIs cannot be parsed by ${provider:name} references. Even after provider matching is fixed, these valid 1Password lookup forms remain unresolved and will propagate as literal placeholders into runtime auth fields.

Useful? React with 👍 / 👎.

Align provider config shape with validated secrets schema

buildSecretProviders assumes secrets.providers contains provider-specific objects like { gcp: { project }, keyring: { ... }, "1password": { ... } }, but the validated config schema still enforces secrets.providers.* to match SecretProviderSchema (src/config/zod-schema.core.ts), which requires a source discriminator (env/file/exec) and rejects these fields. In normal openclaw.json loading, configs matching the new provider docs fail validation before resolution runs, so the new providers cannot be configured through the standard path.

Useful? React with 👍 / 👎.

Replace Unix-only which probe for 1Password CLI

ensureOp shells out to which op, which is not available on Windows, so OnePasswordSecretProvider reports the CLI as missing even when op.exe is installed and in PATH. This makes the provider unusable on Windows hosts despite the implementation not otherwise being platform-gated.

Useful? React with 👍 / 👎.

Deduplicate identical secret refs before parallel fetch

resolveString resolves every regex match concurrently without deduplicating identical references, so repeated placeholders in one config value can trigger multiple simultaneous getSecret calls for the same key before cache population. This can unnecessarily amplify external secret-store traffic and hit rate limits/timeouts when the same secret is reused across fields.

Useful? React with 👍 / 👎.

Preserve raw secret text during placeholder substitution

resolveString substitutes with working.replace(full, sv), but JavaScript treats $ patterns in replacement strings specially ($$, $&, $1, etc.). That means secrets containing dollar-sign sequences are mutated during resolution (for example, pa$$word becomes pa$word), which can silently corrupt credentials and cause authentication failures when those values are used downstream.

Useful? React with 👍 / 👎.

Scope cache keys to provider configuration

fetchWithCache keys entries only by provider name, secret name, and version, so cached values survive across provider reconfiguration (for example changing GCP project, keyring account, or 1Password vault/field) within the same process. On reloads that keep the same ref names, resolution can return secrets from the previous provider context until TTL expiry, causing the app to authenticate against the wrong credentials.

Useful? React with 👍 / 👎.

Wire secret resolver into config loading pipeline

resolveConfigSecrets is only re-exported here and never invoked in the normal config read path (repo-wide rg "resolveConfigSecrets\(" only finds its definition), while loadConfig returns the validated object directly, so ${gcp:...}, ${keyring:...}, and ${1password:...} values remain literal strings in runtime config. This makes the new provider implementations effectively unreachable for standard openclaw.json usage.

Useful? React with 👍 / 👎.

Update secrets docs to use existing CLI subcommands

This quick-start step documents openclaw secrets setup (and later set/test), but the CLI in src/cli/secrets-cli.ts only registers reload, audit, configure, and apply, so users following these instructions will hit unknown-command errors and cannot complete the documented flow.

Useful? React with 👍 / 👎.

Restrict stale-cache fallback to transient provider failures

fetchWithCache currently returns expired cached values for every provider error, not only transient outages. When a secret is deleted/disabled or permissions are revoked after a value was cached, provider.getSecret will throw but this branch still serves the stale credential, so revocation and rotation failures are silently masked and can persist indefinitely.

Useful? React with 👍 / 👎.