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
2 changes: 2 additions & 0 deletions scripts/checks/openshell-policy-mutation-read.mts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export const MUTATION_READS: readonly AuditedPolicyReadFile[] = [
{
relativePath: "src/lib/shields/index.ts",
expectedReads: [
ignoredBase("readDeferredPermissivePolicyReceipt"),
preservingBase("resolveExactManagedMcpPolicies"),
ignoredBase("resolveProvableManagedMcpPoliciesForDeadline"),
ignoredBase("shieldsDownWithoutHostLock"),
Expand Down Expand Up @@ -521,6 +522,7 @@ function classifyIgnoreErrorOption(expression: ts.Expression): IgnoreErrorOption
const POLICY_READ_RUNNERS = new Set([
"captureOpenshell",
"captureOpenshellForStatus",
"run",
"runCapture",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
"runCmd",
]);
Expand Down
20 changes: 15 additions & 5 deletions src/lib/sandbox/privileged-exec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,10 +634,15 @@ describe("privileged sandbox exec routing", () => {
throw new Error("docker daemon unavailable");
},
},
({ privilegedSandboxExecArgv }) => {
expect(() => privilegedSandboxExecArgv("alpha", ["id"])).toThrow(
"docker daemon unavailable",
);
({ isMissingDirectSandboxContainerError, privilegedSandboxExecArgv }) => {
let refusal: unknown;
try {
privilegedSandboxExecArgv("alpha", ["id"]);
} catch (error) {
refusal = error;
}
expect(String(refusal)).toContain("docker daemon unavailable");
expect(isMissingDirectSandboxContainerError(refusal)).toBe(false);
},
);
});
Expand All @@ -652,7 +657,11 @@ describe("privileged sandbox exec routing", () => {
}),
dockerCapture: () => "",
},
({ isDirectSandboxFallbackUnavailableError, privilegedSandboxExecArgv }) => {
({
isDirectSandboxFallbackUnavailableError,
isMissingDirectSandboxContainerError,
privilegedSandboxExecArgv,
}) => {
let refusal: unknown;
try {
privilegedSandboxExecArgv("alpha", ["id"]);
Expand All @@ -664,6 +673,7 @@ describe("privileged sandbox exec routing", () => {
/No running direct OpenShell sandbox container found for 'alpha'/,
);
expect(isDirectSandboxFallbackUnavailableError(refusal)).toBe(true);
expect(isMissingDirectSandboxContainerError(refusal)).toBe(true);
},
);
});
Expand Down
16 changes: 15 additions & 1 deletion src/lib/sandbox/privileged-exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ class DirectSandboxFallbackUnavailableError extends Error {
}
}

class MissingDirectSandboxContainerError extends DirectSandboxFallbackUnavailableError {
constructor(message: string) {
super(message);
this.name = "MissingDirectSandboxContainerError";
}
}

function normalizeDriver(driver: unknown): string | null {
return typeof driver === "string" && driver.trim() ? driver.trim().toLowerCase() : null;
}
Expand Down Expand Up @@ -178,14 +185,20 @@ function findDirectSandboxContainer(sandboxName: string): string | null {

function missingDirectContainerError(sandboxName: string, driver: string | null): Error {
const driverLabel = driver ?? "unspecified";
return new DirectSandboxFallbackUnavailableError(
return new MissingDirectSandboxContainerError(
`No running direct OpenShell sandbox container found for '${sandboxName}' ` +
`(driver: ${driverLabel}). Expected one OpenShell-managed container labeled ` +
`'${OPENSHELL_SANDBOX_NAME_LABEL}=${sandboxName}' and named ` +
`${expectedDirectContainerPattern(sandboxName)}. Is the sandbox running?`,
);
}

function isMissingDirectSandboxContainerError(
error: unknown,
): error is MissingDirectSandboxContainerError {
return error instanceof MissingDirectSandboxContainerError;
}

function isDirectSandboxFallbackUnavailableError(
error: unknown,
): error is DirectSandboxFallbackUnavailableError {
Expand Down Expand Up @@ -328,6 +341,7 @@ function privilegedSandboxExecArgv(
export {
containerNameMatchesSandbox,
isDirectSandboxFallbackUnavailableError,
isMissingDirectSandboxContainerError,
privilegedSandboxExecArgv,
resolveDirectSandboxContainer,
selectDirectSandboxContainer,
Expand Down
45 changes: 44 additions & 1 deletion src/lib/shields/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const {
}: typeof import("../adapters/docker/exec") = require("../adapters/docker/exec");
const {
isDirectSandboxFallbackUnavailableError,
isMissingDirectSandboxContainerError,
privilegedSandboxExecArgv,
withPrivilegedSandboxExecutionLease,
}: typeof import("../sandbox/privileged-exec") = require("../sandbox/privileged-exec");
Expand Down Expand Up @@ -73,6 +74,7 @@ const {
const { cleanupTempDir } = require("../onboard/temp-files");
const { verifyShieldsLockState }: typeof import("./verify-lock") = require("./verify-lock");
const {
confirmDeferredPermissivePolicyForStoppedSandbox,
relockAndReconfirm,
waitForHermesInferenceRouteConvergence,
}: typeof import("./relock-reconfirm") = require("./relock-reconfirm");
Expand Down Expand Up @@ -4247,6 +4249,35 @@ interface ShieldsDownOpts {
processToken?: string;
}

function proveNoRunningDirectSandbox(sandboxName: string): boolean {
try {
// Argv resolution performs the exact registry-bound Docker discovery but
// does not execute the command. A live container therefore returns false;
// only the dedicated missing-container error admits deferred activation.
privilegedSandboxExecArgv(sandboxName, ["/usr/bin/true"], false, true);
return false;
} catch (error) {
return isMissingDirectSandboxContainerError(error);
}
}

function readDeferredPermissivePolicyReceipt(sandboxName: string): ReturnType<typeof run> {
// invalidState: a failed or malformed gateway read is mistaken for proof
// that the timed-out forward policy was committed.
// sourceBoundary: OpenShell owns the base-policy receipt; the canonical
// parser and exact version/hash verifier admit it below.
// whyNotSourceFix: supported OpenShell releases expose this receipt only via
// the CLI after a stopped sandbox cannot acknowledge policy activation.
// regressionTest: relock-reconfirm.test.ts covers failed reads and receipt
// mismatches; policy-transition.test.ts covers the full Shields state flow.
// removalCondition: remove when policy set returns a structured committed
// result independently of live-load acknowledgement.
return run(buildPolicyGetCommand(sandboxName), {
ignoreError: true,
suppressOutput: true,
});
}

type RecoveredShieldsDownCompletion = {
readonly alreadyCommitted: boolean;
readonly audit: Parameters<typeof appendAuditEntry>[0];
Expand Down Expand Up @@ -4884,7 +4915,14 @@ function shieldsDownWithoutHostLock(sandboxName: string, opts: ShieldsDownOpts =
} finally {
cleanupRuntimePolicyFile();
}
if (policySetResult.status !== 0) {
const deferredPolicyConfirmed =
policySetResult.status !== 0 &&
confirmDeferredPermissivePolicyForStoppedSandbox(policySetResult, {
proveNoRunningDirectSandbox: () => proveNoRunningDirectSandbox(sandboxName),
readPolicyReceipt: () => readDeferredPermissivePolicyReceipt(sandboxName),
parsePolicy: parseCurrentPolicy,
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
if (policySetResult.status !== 0 && !deferredPolicyConfirmed) {
// The permissive policy was rejected before it applied — for example,
// OpenShell refuses a live Landlock change on a sandbox whose policy is
// sealed at startup (Deep Agents). Nothing was weakened: configuration is
Expand Down Expand Up @@ -4938,6 +4976,11 @@ function shieldsDownWithoutHostLock(sandboxName: string, opts: ShieldsDownOpts =
console.error(" Shields down did not take effect. `shields status` continues to report `UP`.");
return failShieldsCommand(`Could not apply ${policyName} policy`, opts.throwOnError);
}
if (deferredPolicyConfirmed) {
console.log(
" Policy committed for the stopped sandbox and will load when the sandbox starts.",
);
}

// 2b. Return config to default mutable state.
// OpenClaw uses sandbox:sandbox 0660/2770 here so the gateway UID, which
Expand Down
35 changes: 35 additions & 0 deletions src/lib/shields/policy-transition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,41 @@ describe("shields down policy rejection", () => {
expect(state).toMatchObject({ shieldsDown: false, shieldsDownAt: null });
});

it("finishes Shields down when a terminal sandbox has the exact committed policy receipt (#8304)", () => {
const hash = "d71616b0333b";
const harness = createShieldsFlowHarness(requireSource, tmpDir, {
deferredPolicyContainerMissing: true,
initialOpenClawPosture: "locked",
run: (cmd) => {
const args = Array.isArray(cmd) ? cmd.map(String) : [];
if (args.includes("policy") && args.includes("get")) {
return {
status: 0,
stdout: `Version: 11\nHash: ${hash}\nStatus: pending\n---\nversion: 1\nnetwork_policies: {}\n`,
stderr: "",
};
}
if (args.includes("policy") && args.includes("set")) {
return {
status: 1,
stdout: `Policy version 11 submitted (hash: ${hash})\n`,
stderr: "Timeout waiting for policy version 11 to load\n",
};
}
return { status: 0 };
},
});

expect(() =>
harness.shieldsDown("openclaw", { reason: "failed startup recovery", throwOnError: true }),
).not.toThrow();
expect(harness.isShieldsDown("openclaw")).toBe(true);
expect(harness.getOpenClawPosture()).toBe("mutable");
expect(harness.logSpy.mock.calls.flat().join("\n")).toContain(
"Policy committed for the stopped sandbox",
);
});

it("retains auto-restore authority when rejected policy state cleanup fails (#8198)", () => {
const stateDir = path.join(tmpDir, ".nemoclaw", "state");
const statePath = path.join(stateDir, "shields-openclaw.json");
Expand Down
85 changes: 84 additions & 1 deletion src/lib/shields/relock-reconfirm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,90 @@

import { afterEach, describe, expect, it, vi } from "vitest";

import { relockAndReconfirm, resolveSettleMs } from "./relock-reconfirm";
import {
confirmDeferredPermissivePolicyForStoppedSandbox,
relockAndReconfirm,
resolveSettleMs,
} from "./relock-reconfirm";

const SUBMITTED_POLICY_HASH = "d71616b0333b";

function timedOutPolicySet(overrides: { stdout?: string; stderr?: string } = {}) {
return {
status: 1,
stdout: overrides.stdout ?? `Policy version 11 submitted (hash: ${SUBMITTED_POLICY_HASH})\n`,
stderr: overrides.stderr ?? "Timeout waiting for policy version 11 to load\n",
};
}

function policyReceipt(hash = SUBMITTED_POLICY_HASH, version = "11", status = "pending") {
return {
status: 0,
stdout: `Version: ${version}\nHash: ${hash}\nStatus: ${status}\n---\nversion: 1\nnetwork_policies: {}\n`,
stderr: "",
};
}

describe("deferred permissive policy confirmation", () => {
it("accepts an exact committed receipt only after proving no sandbox container runs (#8304)", () => {
const proveNoRunningDirectSandbox = vi.fn(() => true);
const readPolicyReceipt = vi.fn(() => policyReceipt());

expect(
confirmDeferredPermissivePolicyForStoppedSandbox(timedOutPolicySet(), {
proveNoRunningDirectSandbox,
readPolicyReceipt,
parsePolicy: () => "version: 1\nnetwork_policies: {}",
}),
).toBe(true);
expect(proveNoRunningDirectSandbox).toHaveBeenCalledOnce();
expect(readPolicyReceipt).toHaveBeenCalledOnce();
});

it.each([
["a live sandbox", false, policyReceipt(), timedOutPolicySet()],
["a mismatched hash", true, policyReceipt("aaaaaaaaaaaa"), timedOutPolicySet()],
["a mismatched version", true, policyReceipt(SUBMITTED_POLICY_HASH, "12"), timedOutPolicySet()],
[
"a rejected receipt",
true,
policyReceipt(SUBMITTED_POLICY_HASH, "11", "rejected"),
timedOutPolicySet(),
],
[
"submission without a timeout",
true,
policyReceipt(),
timedOutPolicySet({ stderr: "policy rejected\n" }),
],
[
"duplicate submission evidence",
true,
policyReceipt(),
timedOutPolicySet({
stdout: `Policy version 11 submitted (hash: ${SUBMITTED_POLICY_HASH})\nPolicy version 11 submitted (hash: ${SUBMITTED_POLICY_HASH})\n`,
}),
],
])("rejects %s (#8304)", (_name, stopped, receipt, policySetResult) => {
expect(
confirmDeferredPermissivePolicyForStoppedSandbox(policySetResult, {
proveNoRunningDirectSandbox: () => stopped,
readPolicyReceipt: () => receipt,
parsePolicy: () => "version: 1\nnetwork_policies: {}",
}),
).toBe(false);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Cover both permitted receipt states and duplicate receipt fields.

The success case covers pending, but the helper also accepts active. The rejection cases do not cover duplicate Version, Hash, or Status receipt fields. Add output-level cases for these conditions.

As per path instructions, tests must give behavioral confidence.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/shields/relock-reconfirm.test.ts` around lines 46 - 78, Extend the
parameterized rejection cases in
confirmDeferredPermissivePolicyForStoppedSandbox to include an accepted active
receipt state alongside pending, and add output-level fixtures containing
duplicate Version, Hash, and Status receipt fields. Assert each case rejects
while preserving the existing behavioral test structure.

Source: Path instructions


it("rejects a receipt whose policy body is invalid (#8304)", () => {
expect(
confirmDeferredPermissivePolicyForStoppedSandbox(timedOutPolicySet(), {
proveNoRunningDirectSandbox: () => true,
readPolicyReceipt: () => policyReceipt(),
parsePolicy: () => "",
}),
).toBe(false);
});
});

const sealedHashes = {
"/sandbox/.openclaw/openclaw.json":
Expand Down
65 changes: 64 additions & 1 deletion src/lib/shields/relock-reconfirm.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0
//
// Re-lock-and-reconfirm helper for shields (#4663).
// Shields transition confirmation helpers (#4663, #8304).
//
// `lockAgentConfig` chmod 444 / chown root:root the config files and verifies
// the on-disk state once — a single instantaneous snapshot. On DGX Station /
Expand Down Expand Up @@ -33,6 +33,69 @@ const DEFAULT_MAX_ATTEMPTS = 3;
const DEFAULT_SETTLE_MS = 750;
const MIN_SETTLE_MS = 0;
const MAX_SETTLE_MS = 10_000;
const POLICY_ANSI_RE = /\x1b\[[0-9;]*m/g;

export interface PolicyCommandResult {
status: number | null;
stdout?: string | Buffer | null;
stderr?: string | Buffer | null;
}

export interface DeferredPermissivePolicyConfirmationOptions {
proveNoRunningDirectSandbox: () => boolean;
readPolicyReceipt: () => PolicyCommandResult;
parsePolicy: (raw: string) => string;
}

function policyCommandOutput(result: PolicyCommandResult): string {
return [result.stdout, result.stderr]
.map((value) => String(value ?? ""))
.filter(Boolean)
.join("\n")
.replace(POLICY_ANSI_RE, "");
}

/**
* Confirm the one safe deferred-load case for a user-authorized Shields-down
* transition. OpenShell may commit the relaxed policy but return nonzero when
* a failed-startup sandbox has no running container to acknowledge loading it.
* The caller must independently prove that exact stopped-container state; a
* discovery error, live container, malformed receipt, or mismatch fails closed.
*/
export function confirmDeferredPermissivePolicyForStoppedSandbox(
policySetResult: PolicyCommandResult,
options: DeferredPermissivePolicyConfirmationOptions,
): boolean {
const output = policyCommandOutput(policySetResult);
const submissions = [
...output.matchAll(/^.*Policy version ([1-9]\d*) submitted \(hash: ([0-9a-f]{8,64})\)\s*$/gim),
];
if (submissions.length !== 1) return false;
const version = submissions[0]?.[1];
const hash = submissions[0]?.[2]?.toLowerCase();
if (!version || !hash) return false;
const timeouts = output.match(
new RegExp(`^.*Timeout waiting for policy version ${version} to load\\s*$`, "gim"),
);
if (timeouts?.length !== 1 || !options.proveNoRunningDirectSandbox()) return false;

const receipt = options.readPolicyReceipt();
if (receipt.status !== 0) return false;
const receiptOutput = policyCommandOutput(receipt);
const separator = /(?:^|\r?\n)---[ \t]*(?:\r?\n|$)/.exec(receiptOutput);
if (!separator || !options.parsePolicy(receiptOutput)) return false;
const header = receiptOutput.slice(0, separator.index);
const versions = [...header.matchAll(/^\s*Version:\s*([1-9]\d*)\s*$/gim)];
const hashes = [...header.matchAll(/^\s*Hash:\s*([0-9a-f]{8,64})\s*$/gim)];
const statuses = [...header.matchAll(/^\s*Status:\s*(pending|active)\s*$/gim)];
return (
versions.length === 1 &&
hashes.length === 1 &&
statuses.length === 1 &&
versions[0]?.[1] === version &&
hashes[0]?.[1]?.toLowerCase() === hash
);
}

/** Result of a single `lockAgentConfig` call: apply + verify. */
export interface LockResult {
Expand Down
Loading
Loading