Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 41 additions & 3 deletions src/lib/shields/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,30 @@ function deriveShieldsMode(state: ShieldsState, hasStateFile: boolean): ShieldsM
return "mutable_default";
}

function isEquivalentShieldsDownRequest(
state: ShieldsState,
timeoutSeconds: number,
reason: string | null,
policyName: string,
): boolean {
return (
state.shieldsDown === true &&
state.shieldsDownTimeout === timeoutSeconds &&
state.shieldsDownReason === reason &&
state.shieldsDownPolicy === policyName
);
}

function hasEquivalentShieldsDownTimerAuthority(sandboxName: string, state: ShieldsState): boolean {
const marker = readTimerMarker(sandboxName);
return (
marker !== null &&
marker.sandboxName === sandboxName &&
marker.snapshotPath === state.shieldsPolicySnapshotPath &&
isExactLiveFutureTimerAuthority(marker)
);
}

function describeShieldsMode(mode: ShieldsPostureMode): Omit<ShieldsPosture, "state"> {
switch (mode) {
case "mutable_default":
Expand Down Expand Up @@ -4602,6 +4626,9 @@ function shieldsDownWithoutHostLock(sandboxName: string, opts: ShieldsDownOpts =
recoveredProviderTarget = retainedProviderTarget;
}
const initialMode = deriveShieldsMode(state, state._hasStateFile);
const timeoutSeconds = parseDuration(opts.timeout || `${DEFAULT_TIMEOUT_SECONDS}`);
const reason = opts.reason || null;
const policyName = opts.policy || "permissive";
if (state.shieldsDown) {
// Provider release deliberately precedes route convergence and the final
// timer-bound transition commit. A process can therefore die after the
Expand Down Expand Up @@ -4668,6 +4695,20 @@ function shieldsDownWithoutHostLock(sandboxName: string, opts: ShieldsDownOpts =
console.log(` Recovered interrupted config unlock for ${sandboxName}.`);
return;
}
if (isEquivalentShieldsDownRequest(state, timeoutSeconds, reason, policyName)) {
if (!hasEquivalentShieldsDownTimerAuthority(sandboxName, state)) {
recoverExpiredAutoRestoreInline(sandboxName, state);
console.error(
" Cannot accept equivalent shields down request without live auto-restore timer authority.",
);
return failShieldsCommand(
`Cannot accept equivalent shields down request without live auto-restore timer authority for ${sandboxName}`,
opts.throwOnError,
);
}
console.log(` Shields already down for ${sandboxName}; equivalent request accepted.`);
return;
}
console.error(
` Config is already unlocked for ${sandboxName} (since ${state.shieldsDownAt}).`,
);
Expand Down Expand Up @@ -4699,9 +4740,6 @@ function shieldsDownWithoutHostLock(sandboxName: string, opts: ShieldsDownOpts =
);
}

const timeoutSeconds = parseDuration(opts.timeout || `${DEFAULT_TIMEOUT_SECONDS}`);
const reason = opts.reason || null;
const policyName = opts.policy || "permissive";
const processToken = opts.processToken ?? randomBytes(16).toString("hex");
if (!/^[0-9a-f]{32}$/.test(processToken)) {
throw new Error("Invalid shields-down recovery process token");
Expand Down
234 changes: 234 additions & 0 deletions src/lib/shields/openclaw-transition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
createShieldsFlowHarness,
type ShieldsFlowHarnessOptions,
} from "../../../test/helpers/shields-flow-harness";
import type { AgentConfigTarget } from "../sandbox/agent-config";

const requireSource = createRequire(import.meta.url);
const INDEX_MODULE = "./index.js";
Expand All @@ -26,6 +27,15 @@ const STATE_LOCK_PLAN = {
writableSubpaths: [],
};

const RETRY_STATE_LOCK_PLAN = {
version: 1 as const,
readOnlyRoots: ["skills"],
confidentialRoots: [],
readOnlyPrefixes: [],
confidentialPrefixes: [],
writableSubpaths: [],
};

function openClawTarget() {
return {
agentName: "openclaw",
Expand All @@ -39,6 +49,64 @@ function openClawTarget() {
};
}

const retryAgentCases: ReadonlyArray<
readonly [label: string, sandboxName: string, target: AgentConfigTarget]
> = [
[
"OpenClaw",
"openclaw",
{
agentName: "openclaw",
configPath: "/sandbox/.openclaw/openclaw.json",
configDir: "/sandbox/.openclaw",
format: "json",
configFile: "openclaw.json",
sensitiveFiles: ["/sandbox/.openclaw/.config-hash"],
stateLockPlan: RETRY_STATE_LOCK_PLAN,
stateLockPlanInImage: true,
},
],
[
"Hermes",
"hermes",
{
agentName: "hermes",
configPath: "/sandbox/.hermes/config.yaml",
configDir: "/sandbox/.hermes",
format: "yaml",
configFile: "config.yaml",
sensitiveFiles: ["/sandbox/.hermes/.config-hash"],
stateLockPlan: RETRY_STATE_LOCK_PLAN,
stateLockPlanInImage: true,
},
],
[
"DCode",
"dcode",
{
agentName: "langchain-deepagents-code",
configPath: "/sandbox/.dcode/config.json",
configDir: "/sandbox/.dcode",
format: "json",
configFile: "config.json",
sensitiveFiles: ["/sandbox/.dcode/.config-hash"],
stateLockPlan: RETRY_STATE_LOCK_PLAN,
stateLockPlanInImage: true,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
},
],
];

const retryConflictCases: ReadonlyArray<
readonly [
label: string,
retryOptions: { timeout?: string; reason?: string; policy?: string; throwOnError: true },
]
> = [
["timeout", { timeout: "6m", reason: "retry-safe", policy: "permissive", throwOnError: true }],
["reason", { timeout: "5m", reason: "changed-reason", policy: "permissive", throwOnError: true }],
["policy", { timeout: "5m", reason: "retry-safe", policy: "custom-policy", throwOnError: true }],
];

describe("OpenClaw shields top-config transaction", () => {
let homeDir: string;
let shields: ShieldsModule;
Expand Down Expand Up @@ -386,6 +454,27 @@ describe("OpenClaw shields flow rollback and recovery", () => {
return output;
}

function readStateAndTimer(sandboxName: string) {
const stateDir = path.join(tmpDir, ".nemoclaw", "state");
const statePath = path.join(stateDir, `shields-${sandboxName}.json`);
const timerPath = path.join(stateDir, `shields-timer-${sandboxName}.json`);
return {
statePath,
timerPath,
state: fs.readFileSync(statePath, "utf-8"),
timer: fs.readFileSync(timerPath, "utf-8"),
};
}

function createRetryHarness(sandboxName: string, target: AgentConfigTarget) {
return createHarness({
agentConfigTarget: target,
confirmOpenClawInodeFlags: true,
processStartIdentity: `issue-8806-${sandboxName}-owner`,
sandboxName,
});
}

beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "shields-openclaw-recovery-"));
vi.stubEnv("HOME", tmpDir);
Expand All @@ -403,6 +492,151 @@ describe("OpenClaw shields flow rollback and recovery", () => {
delete require.cache[requireSource.resolve("../cli/branding.js")];
});

it.each(retryAgentCases)(
"accepts an equivalent repeated shieldsDown request for %s without changing state or timer authority (#8806)",
{ timeout: 30_000 },
(_label, sandboxName, target) => {
const harness = createRetryHarness(sandboxName, target);

harness.shieldsDown(sandboxName, {
timeout: "5m",
reason: "retry-safe",
policy: "permissive",
throwOnError: true,
});

const before = readStateAndTimer(sandboxName);
harness.logSpy.mockClear();
harness.errorSpy.mockClear();
harness.runCaptureSpy.mockClear();
harness.runSpy.mockClear();
harness.auditSpy.mockClear();

harness.shieldsDown(sandboxName, {
timeout: "5m",
reason: "retry-safe",
policy: "permissive",
throwOnError: true,
});

expect(fs.readFileSync(before.statePath, "utf-8")).toBe(before.state);
expect(fs.readFileSync(before.timerPath, "utf-8")).toBe(before.timer);
expect(harness.runCaptureSpy).not.toHaveBeenCalled();
expect(harness.runSpy).not.toHaveBeenCalled();
expect(harness.auditSpy).not.toHaveBeenCalled();
expect(harness.errorSpy).not.toHaveBeenCalled();
expect(harness.logSpy.mock.calls.flat().join("\n")).toContain(
`Shields already down for ${sandboxName}; equivalent request accepted.`,
);
},
);

it.each(
retryAgentCases.flatMap(([label, sandboxName, target]) =>
retryConflictCases.map(
([conflict, retryOptions]) => [label, conflict, sandboxName, target, retryOptions] as const,
),
),
)(
"rejects a repeated shieldsDown request for %s with a conflicting %s without changing state or timer authority (#8806)",
{ timeout: 30_000 },
(_label, _conflict, sandboxName, target, retryOptions) => {
const harness = createRetryHarness(sandboxName, target);

harness.shieldsDown(sandboxName, {
timeout: "5m",
reason: "retry-safe",
policy: "permissive",
throwOnError: true,
});

const before = readStateAndTimer(sandboxName);
harness.runCaptureSpy.mockClear();
harness.runSpy.mockClear();
harness.auditSpy.mockClear();

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
expect(() => harness.shieldsDown(sandboxName, retryOptions)).toThrow(
new RegExp(`Config is already unlocked for ${sandboxName}`, "u"),
);

expect(fs.readFileSync(before.statePath, "utf-8")).toBe(before.state);
expect(fs.readFileSync(before.timerPath, "utf-8")).toBe(before.timer);
expect(harness.runCaptureSpy).not.toHaveBeenCalled();
expect(harness.runSpy).not.toHaveBeenCalled();
expect(harness.auditSpy).not.toHaveBeenCalled();
expect(harness.errorSpy.mock.calls.flat().join("\n")).toContain("already unlocked");
},
);

it("rejects a Hermes repeated shieldsDown request with no options without changing state or timer authority (#8806)", {
timeout: 30_000,
}, () => {
const [, sandboxName, target] = retryAgentCases[1]!;
const harness = createRetryHarness(sandboxName, target);

harness.shieldsDown(sandboxName, {
timeout: "5m",
reason: "retry-safe",
policy: "permissive",
throwOnError: true,
});

const before = readStateAndTimer(sandboxName);
harness.runCaptureSpy.mockClear();
harness.runSpy.mockClear();
harness.auditSpy.mockClear();

expect(() => harness.shieldsDown(sandboxName, { throwOnError: true })).toThrow(
/Config is already unlocked for hermes/u,
);

expect(fs.readFileSync(before.statePath, "utf-8")).toBe(before.state);
expect(fs.readFileSync(before.timerPath, "utf-8")).toBe(before.timer);
expect(harness.runCaptureSpy).not.toHaveBeenCalled();
expect(harness.runSpy).not.toHaveBeenCalled();
expect(harness.auditSpy).not.toHaveBeenCalled();
expect(harness.errorSpy.mock.calls.flat().join("\n")).toContain("already unlocked");
});

it("rejects an equivalent repeated shieldsDown request with missing timer authority and restores lockdown (#8806)", {
timeout: 30_000,
}, () => {
const harness = createRetryHarness("openclaw", retryAgentCases[0]![2]);

harness.shieldsDown("openclaw", {
timeout: "5m",
reason: "retry-safe",
policy: "permissive",
throwOnError: true,
});

const before = readStateAndTimer("openclaw");
fs.rmSync(before.timerPath, { force: true });
harness.runCaptureSpy.mockClear();
harness.auditSpy.mockClear();

expect(() =>
harness.shieldsDown("openclaw", {
timeout: "5m",
reason: "retry-safe",
policy: "permissive",
throwOnError: true,
}),
).toThrow(/Cannot accept equivalent shields down request without live auto-restore timer/u);

expect(JSON.parse(fs.readFileSync(before.statePath, "utf-8"))).toMatchObject({
shieldsDown: false,
shieldsDownAt: null,
});
expect(fs.existsSync(before.timerPath)).toBe(false);
expect(harness.auditSpy).toHaveBeenCalledWith(
expect.objectContaining({ action: "shields_auto_restore", sandbox: "openclaw" }),
);
expect(harness.errorSpy.mock.calls.flat().join("\n")).toContain(
"Cannot accept equivalent shields down request without live auto-restore timer authority.",
);
});

it("shields down removes the permissive runtime temp directory when the auto-restore timer fails (#7964)", () => {
const mkdtempSpy = vi.spyOn(fs, "mkdtempSync");
// shieldsDown builds the temp policy before it forks the auto-restore timer,
Expand Down
Loading
Loading