Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 21 additions & 3 deletions src/lib/shields/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,20 @@ 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 describeShieldsMode(mode: ShieldsPostureMode): Omit<ShieldsPosture, "state"> {
switch (mode) {
case "mutable_default":
Expand Down Expand Up @@ -4602,6 +4616,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 +4685,10 @@ function shieldsDownWithoutHostLock(sandboxName: string, opts: ShieldsDownOpts =
console.log(` Recovered interrupted config unlock for ${sandboxName}.`);
return;
}
if (isEquivalentShieldsDownRequest(state, timeoutSeconds, reason, policyName)) {
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 +4720,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
85 changes: 85 additions & 0 deletions src/lib/shields/openclaw-transition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,18 @@ describe("OpenClaw shields flow rollback and recovery", () => {
return output;
}

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

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

it("accepts an equivalent repeated shieldsDown request without changing state or timer authority (#8806)", {
timeout: 30_000,
}, () => {
const harness = createHarness({
confirmOpenClawInodeFlags: true,
processStartIdentity: "issue-8806-owner",
});

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

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

harness.shieldsDown("openclaw", {
timeout: "5m",
reason: "retry-safe",
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 openclaw; equivalent request accepted.",
);
});

it("rejects a conflicting repeated shieldsDown request without changing state or timer authority (#8806)", {
timeout: 30_000,
}, () => {
const harness = createHarness({
confirmOpenClawInodeFlags: true,
processStartIdentity: "issue-8806-owner",
});

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

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

expect(() =>
harness.shieldsDown("openclaw", {
timeout: "5m",
reason: "changed-reason",
throwOnError: true,
}),
).toThrow(/Config is already unlocked for openclaw/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");
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

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