fix(secrets): harden sops migration sops rule matching

This commit is contained in:
joshavant
2026-02-24 20:34:47 -06:00
committed by Peter Steinberger
parent 0e69660c41
commit e8637c79b3
5 changed files with 60 additions and 3 deletions

View File

@@ -57,7 +57,7 @@ openclaw secrets migrate --write --no-scrub-env
- Scrub target is `<config-dir>/.env`.
- Only known secret env keys are considered.
- Entries are removed only when the value exactly matches a migrated plaintext secret.
- If `<config-dir>/.sops.yaml` or `<config-dir>/.sops.yml` exists, migrate passes it explicitly to `sops` so command behavior is cwd-independent.
- If `<config-dir>/.sops.yaml` or `<config-dir>/.sops.yml` exists, migrate passes it explicitly to `sops`, runs `sops` with `cwd=<config-dir>`, and sets `--filename-override` to the absolute target secrets path (for example `/home/user/.openclaw/secrets.enc.json`) so strict `creation_rules` continue to match when OpenClaw encrypts through a temp file.
Common migrate write failure:

View File

@@ -93,7 +93,7 @@ Contract:
- OpenClaw shells out to `sops` for decrypt/encrypt.
- Minimum supported version: `sops >= 3.9.0`.
- For migration, OpenClaw explicitly passes `--config <config-dir>/.sops.yaml` (or `.sops.yml`) when present, so behavior is not dependent on current working directory.
- For migration, OpenClaw explicitly passes `--config <config-dir>/.sops.yaml` (or `.sops.yml`), runs `sops` with `cwd=<config-dir>`, and sets `--filename-override` to the absolute target secrets path (for example `/home/user/.openclaw/secrets.enc.json`) so strict `creation_rules` still match even though encryption uses a temp input file.
- Decrypted payload must be a JSON object.
- `id` is resolved as JSON pointer into decrypted payload.
- Default timeout is `5000ms`.

View File

@@ -46,7 +46,7 @@ export function shouldSpawnWithShell(params: {
export async function runExec(
command: string,
args: string[],
opts: number | { timeoutMs?: number; maxBuffer?: number } = 10_000,
opts: number | { timeoutMs?: number; maxBuffer?: number; cwd?: string } = 10_000,
): Promise<{ stdout: string; stderr: string }> {
const options =
typeof opts === "number"
@@ -54,6 +54,7 @@ export async function runExec(
: {
timeout: opts.timeoutMs,
maxBuffer: opts.maxBuffer,
cwd: opts.cwd,
encoding: "utf8" as const,
};
try {

View File

@@ -228,5 +228,31 @@ describe("secrets migrate", () => {
const configIndex = args.indexOf("--config");
expect(configIndex).toBeGreaterThanOrEqual(0);
expect(args[configIndex + 1]).toBe(sopsConfigPath);
const filenameOverrideIndex = args.indexOf("--filename-override");
expect(filenameOverrideIndex).toBeGreaterThanOrEqual(0);
expect(args[filenameOverrideIndex + 1]).toBe(
path.join(stateDir, "secrets.enc.json").replaceAll(path.sep, "/"),
);
const options = encryptCall?.[2] as { cwd?: string } | undefined;
expect(options?.cwd).toBe(stateDir);
});
it("passes a stable filename override for sops when config file is absent", async () => {
await runSecretsMigration({ env, write: true });
const encryptCall = runExecMock.mock.calls.find((call) =>
(call[1] as string[]).includes("--encrypt"),
);
expect(encryptCall).toBeTruthy();
const args = encryptCall?.[1] as string[];
const configIndex = args.indexOf("--config");
expect(configIndex).toBe(-1);
const filenameOverrideIndex = args.indexOf("--filename-override");
expect(filenameOverrideIndex).toBeGreaterThanOrEqual(0);
expect(args[filenameOverrideIndex + 1]).toBe(
path.join(stateDir, "secrets.enc.json").replaceAll(path.sep, "/"),
);
const options = encryptCall?.[2] as { cwd?: string } | undefined;
expect(options?.cwd).toBe(stateDir);
});
});

View File

@@ -7,6 +7,21 @@ import { ensureDirForFile, normalizePositiveInt } from "./shared.js";
export const DEFAULT_SOPS_TIMEOUT_MS = 5_000;
const MAX_SOPS_OUTPUT_BYTES = 10 * 1024 * 1024;
function toSopsPath(value: string): string {
return value.replaceAll(path.sep, "/");
}
function resolveFilenameOverride(params: { targetPath: string }): string {
return toSopsPath(path.resolve(params.targetPath));
}
function resolveSopsCwd(params: { targetPath: string; configPath?: string }): string {
if (typeof params.configPath === "string" && params.configPath.trim().length > 0) {
return path.dirname(params.configPath);
}
return path.dirname(params.targetPath);
}
function normalizeTimeoutMs(value: number | undefined): number {
return normalizePositiveInt(value, DEFAULT_SOPS_TIMEOUT_MS);
}
@@ -37,6 +52,10 @@ export async function decryptSopsJsonFile(params: {
configPath?: string;
}): Promise<unknown> {
const timeoutMs = normalizeTimeoutMs(params.timeoutMs);
const cwd = resolveSopsCwd({
targetPath: params.path,
configPath: params.configPath,
});
try {
const args: string[] = [];
if (typeof params.configPath === "string" && params.configPath.trim().length > 0) {
@@ -46,6 +65,7 @@ export async function decryptSopsJsonFile(params: {
const { stdout } = await runExec("sops", args, {
timeoutMs,
maxBuffer: MAX_SOPS_OUTPUT_BYTES,
cwd,
});
return JSON.parse(stdout) as unknown;
} catch (err) {
@@ -84,12 +104,21 @@ export async function encryptSopsJsonFile(params: {
fs.chmodSync(tmpPlain, 0o600);
try {
const filenameOverride = resolveFilenameOverride({
targetPath: params.path,
});
const cwd = resolveSopsCwd({
targetPath: params.path,
configPath: params.configPath,
});
const args: string[] = [];
if (typeof params.configPath === "string" && params.configPath.trim().length > 0) {
args.push("--config", params.configPath);
}
args.push(
"--encrypt",
"--filename-override",
filenameOverride,
"--input-type",
"json",
"--output-type",
@@ -101,6 +130,7 @@ export async function encryptSopsJsonFile(params: {
await runExec("sops", args, {
timeoutMs,
maxBuffer: MAX_SOPS_OUTPUT_BYTES,
cwd,
});
fs.renameSync(tmpEncrypted, params.path);
fs.chmodSync(params.path, 0o600);