Verification reads versions/latest instead of the newly stored version

After writing the new secret version, the verification step reads versions/latest rather than the specific versionName returned by addSecretVersion. In a concurrent environment (e.g., two rotation jobs running in parallel), a second write could land between the addSecretVersion call and this accessSecretVersion call, causing the verification to confirm the wrong version while the config update proceeds with the original newToken.

This is a comment left during a code review.
Path: src/config/auto-rotation.ts
Line: 203-207

Comment:
**Verification reads `versions/latest` instead of the newly stored version**

After writing the new secret version, the verification step reads `versions/latest` rather than the specific `versionName` returned by `addSecretVersion`. In a concurrent environment (e.g., two rotation jobs running in parallel), a second write could land between the `addSecretVersion` call and this `accessSecretVersion` call, causing the verification to confirm the wrong version while the config update proceeds with the original `newToken`.

```suggestion
    await client.accessSecretVersion({
      name: versionName,
    });
```

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

Hardcoded GCP project "n30-agents" leaks into production defaults

createDefaultDeps ships project: "n30-agents" as the default. Any user who calls this function without overriding project will silently write new secret versions to n30-agents instead of their own GCP project. This value looks like a test/staging project that was never replaced before shipping.

The project should be a required field (no default), or at minimum read from the loaded openclaw.json config at call time rather than being baked into the factory.

This is a comment left during a code review.
Path: src/config/auto-rotation.ts
Line: 244-247

Comment:
**Hardcoded GCP project `"n30-agents"` leaks into production defaults**

`createDefaultDeps` ships `project: "n30-agents"` as the default. Any user who calls this function without overriding `project` will silently write new secret versions to `n30-agents` instead of their own GCP project. This value looks like a test/staging project that was never replaced before shipping.

The project should be a required field (no default), or at minimum read from the loaded `openclaw.json` config at call time rather than being baked into the factory.

```suggestion
export function createDefaultDeps(overrides?: Partial<RotationDeps>): RotationDeps {
  if (!overrides?.project) {
    throw new Error("project is required in RotationDeps — set it via openclaw.json secrets.providers.gcp.project");
  }
  return {
    project: overrides.project,
    secretName: "openclaw-main-gateway-token",
    configPath: `${process.env.HOME}/.openclaw/openclaw.json`,
    intervalDays: 30,
    readConfig: defaultReadConfig,
    writeConfig: defaultWriteConfig,
    getClient: defaultGetClient,
    ...overrides,
  };
}
```

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

Wildcard * in IAM policy ARN is not valid

arn:aws:iam::*:policy/${policyName} uses * for the account ID, which is not a valid ARN for aws iam attach-user-policy. AWS requires the exact 12-digit account ID. This command will fail at runtime for any user.

The account ID should be extracted from the aws sts get-caller-identity response that was already called a few lines above:

const callerIdentity = JSON.parse(stsCheck.stdout) as { Account: string };
const accountId = callerIdentity.Account;
// ...
await exec(
  `aws iam attach-user-policy --user-name openclaw-${agent} --policy-arn arn:aws:iam::${accountId}:policy/${policyName}`,
);

This is a comment left during a code review.
Path: src/commands/secrets.ts
Line: 226-229

Comment:
**Wildcard `*` in IAM policy ARN is not valid**

`arn:aws:iam::*:policy/${policyName}` uses `*` for the account ID, which is not a valid ARN for `aws iam attach-user-policy`. AWS requires the exact 12-digit account ID. This command will fail at runtime for any user.

The account ID should be extracted from the `aws sts get-caller-identity` response that was already called a few lines above:

```typescript
const callerIdentity = JSON.parse(stsCheck.stdout) as { Account: string };
const accountId = callerIdentity.Account;
// ...
await exec(
  `aws iam attach-user-policy --user-name openclaw-${agent} --policy-arn arn:aws:iam::${accountId}:policy/${policyName}`,
);
```

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

scanForSensitiveValues only skips ${gcp:} references

The early-return guard only checks for ${gcp: prefix. If a user has already migrated some secrets to AWS (${aws:...}) or Vault (${vault:...}), those values would still be scanned and reported as plaintext secrets to migrate. This would produce false positives in openclaw secrets migrate.

This is a comment left during a code review.
Path: src/commands/secrets.ts
Line: 97-98

Comment:
**`scanForSensitiveValues` only skips `${gcp:}` references**

The early-return guard only checks for `${gcp:` prefix. If a user has already migrated some secrets to AWS (`${aws:...}`) or Vault (`${vault:...}`), those values would still be scanned and reported as plaintext secrets to migrate. This would produce false positives in `openclaw secrets migrate`.

```suggestion
    if (obj.match(/^\$\{[a-z]+:/)) {
      return results;
    }
```

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

Wrong event emitted for the expired rotation state

The expired case in emitRotationEvents calls the listener using the same event name as the expiring-soon case. Any consumer that registers a handler specifically for the expired event will never receive it — fully expired credentials will silently appear to be merely expiring-soon. The event name passed to the listener in this branch should be "secret:" + "expired" to match the actual state, consistent with how the other cases are handled.

This is a comment left during a code review.
Path: src/config/rotation-reminders.ts
Line: 198-203

Comment:
**Wrong event emitted for the `expired` rotation state**

The `expired` case in `emitRotationEvents` calls the listener using the same event name as the `expiring-soon` case. Any consumer that registers a handler specifically for the `expired` event will never receive it — fully expired credentials will silently appear to be merely expiring-soon. The event name passed to the listener in this branch should be `"secret:" + "expired"` to match the actual state, consistent with how the other cases are handled.

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

Make rotation project configurable

createDefaultDeps hard-codes project: "n30-agents", and secretsRotateCommand does not expose a project override in its public options, so rotations always target that single GCP project. In any deployment using a different project this either fails outright or rotates the wrong secret, which makes the new rotation command unusable outside the maintainer environment.

Useful? React with 👍 / 👎.

Fail setup when provisioning commands return non-zero

The setup flow runs critical provisioning commands (gcloud iam service-accounts create, IAM policy binding, and similarly in the AWS branch) without checking exitCode, then still returns success and writes config. If permissions/API setup fail, users are told setup succeeded even though runtime secret access is broken; each provisioning step should be validated and abort on failure.

Useful? React with 👍 / 👎.

Provide a non-mock execution path for secrets setup

This command exits with code 1 whenever _mockExec is not injected, which means the implementation currently has no production path to run real setup logic. The same _mock* pattern is used across the new secrets command functions, so invoking these commands outside tests cannot perform the promised operations.

Useful? React with 👍 / 👎.

Rewrite config to secret refs before purging

The migrate flow uploads detected secrets and then proceeds to purge plaintext values, but it never updates the loaded config to replace those plaintext fields with SecretRef values (for example ${gcp:...}) before purge. In any run where purge is enabled, this can remove the only usable credentials while leaving config entries unresolved, which turns a successful migration into broken auth at runtime. Add an explicit rewrite-and-save step before purge and fail the command if that rewrite cannot be completed.

Useful? React with 👍 / 👎.

Preserve gateway token SecretRef during rotation

This always writes the freshly generated token value directly into gateway.auth.token. When users are configured with external secret refs (for example gateway.auth.token: "${gcp:openclaw-main-gateway-token}"), running rotation replaces that reference with plaintext in openclaw.json, which defeats the security model and stops future rotations from being centrally managed by the provider. The update path should preserve/refill the existing ref instead of persisting the raw token.

Useful? React with 👍 / 👎.

Handle config/client init failures in token rotation

rotateGatewayToken returns a RotationResult, but both readConfig(configPath) and getClient() are awaited outside any error handling. If the config file is missing/invalid JSON or the Secret Manager client cannot initialize, this throws and bypasses the function’s structured failure path, so callers like secretsRotateCommand cannot report a controlled rotation failure.

Useful? React with 👍 / 👎.

Require project before building gcloud setup commands

options.project is optional, but the GCP setup path interpolates it directly into every provisioning command. When --project is omitted, commands are emitted with --project=undefined (and IAM principals containing @undefined), so setup targets an invalid project and persists an unusable provider config instead of failing fast.

Useful? React with 👍 / 👎.

Derive default config path from homedir, not HOME only

The default rotation deps build configPath from process.env.HOME only. In environments where HOME is unset (for example some Windows/service contexts), this becomes undefined/.openclaw/openclaw.json, causing rotation to fail before it can read config. Use os.homedir() (or an existing path helper) for a portable default.

Useful? React with 👍 / 👎.

Register AWS provider in buildSecretProviders

buildSecretProviders currently only instantiates gcp and env, so an aws entry under secrets.providers is silently ignored. That makes ${aws:...} references fail with UnknownSecretProviderError even when AWS is configured, which means the new AWS provider path is unusable in normal runtime resolution.

Useful? React with 👍 / 👎.

Scope shared cache keys by provider config

The shared cache key is only ${provider.name}:${secret}#${version}, so it does not distinguish different GCP projects or AWS accounts/regions using the same provider alias. In a long-lived process that resolves multiple configs, a secret fetched for one tenant can be returned for another tenant with the same name, producing incorrect values and cross-tenant secret leakage until the TTL expires.

Useful? React with 👍 / 👎.

Remove old token value from rotation error text

When rotateGatewayToken hits a config write failure (for example, permission denied on openclaw.json), it embeds oldToken directly into the returned error string, and secretsRotateCommand prints that string to stderr. This leaks a live gateway credential into terminal history/CI logs in a failure path, which is a security regression even when rotation itself fails.

Useful? React with 👍 / 👎.

Treat all SecretRef templates as already-migrated values

scanForSensitiveValues only skips strings containing ${gcp:, so other reference formats (e.g. ${aws:...} or ${env:...} with long IDs) still satisfy the secret heuristic and get re-uploaded as literal text during secretsMigrateCommand. In configs that already use non-GCP refs, migration will incorrectly process those refs as plaintext secrets and can lead to bad uploads (and risky purge behavior if enabled).

Useful? React with 👍 / 👎.

Wire new secrets subcommands into CLI registration

The handlers added in this file are not reachable from the actual CLI entrypoints: in this commit, registerSecretsCli still only registers reload, audit, configure, and apply (src/cli/secrets-cli.ts), and a repo-wide search shows src/commands/secrets.ts is only imported by its own test file. That means openclaw secrets users cannot invoke the new setup/migrate/rotate/reminder flows in production, so the feature ships as dead code.

Useful? React with 👍 / 👎.