Skip to content

Implemented cloud shell policy, which overrides local policy#73

Open
zmaj1231 wants to merge 1 commit into
mainfrom
feature/cloud-shell-policy
Open

Implemented cloud shell policy, which overrides local policy#73
zmaj1231 wants to merge 1 commit into
mainfrom
feature/cloud-shell-policy

Conversation

@zmaj1231

Copy link
Copy Markdown
Contributor

Reins Cloud is now the main policy source for OpenClaw runtime shell and MCP enforcement, instead of relying on the inbuilt reins policy. The policies.json file, which comes from Reins Cloud, is the source of policy unless it doesn't exist, in which case the inbuilt policy is used. protected_paths was removed from Reins Cloud interfacing because that functionality does not exist in Reins Cloud.

@greptile-apps

greptile-apps Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes Reins Cloud (policies.json) the authoritative policy source for OpenClaw's shell and MCP enforcement, falling back to local policy only when the file is absent. It introduces two new modules (cloud-policy.ts, cloud-policy-evaluator.ts) and wires them into the existing tool-interceptor hook.

  • Shell bypass: When policies.json is present, the shell code path in tool-interceptor.ts always returns early — before irreversibility scoring, destructive gating, memory-risk forecasting, and rate-limiting execute. Any shell command that isn't explicitly BLOCKed by a cloud rule will skip all of those safety checks entirely.
  • Empty-file takeover: loadCloudPolicyCache returns a non-null cache (with empty rule arrays) for any valid JSON object, including {}. Because the shell branch treats any non-null cache as authoritative, an empty or zero-rule policies.json fully disables local DENY rules for shell commands.
  • Unanchored MCP regex: evaluateCloudMcpRule matches tool names against an unanchored regex, so a pattern like exec matches reexec or context in addition to exec.

Confidence Score: 2/5

Not safe to merge as-is — the cloud policy path short-circuits destructive gating and irreversibility checks for all shell commands, and an empty policies.json disables local shell enforcement entirely.

The shell interceptor now unconditionally returns early whenever a cloud policy cache is loaded, bypassing every downstream safety mechanism (destructive classification, irreversibility scoring, memory risk, rate limiting). A minimal or empty policies.json — whether the result of a sync failure, a deliberate erasure, or a bug in the cloud-side writer — turns off all of those protections silently. Combined with the unanchored MCP regex that can block unintended tools, these are present defects in the changed paths that need to be resolved before this code should run in production.

src/plugin/tool-interceptor.ts and src/plugin/cloud-policy.ts require the most attention; the early-return logic and the empty-cache promotion are the root causes of the main issues.

Important Files Changed

Filename Overview
src/plugin/tool-interceptor.ts Added cloud policy evaluation blocks that short-circuit the entire downstream safety pipeline (destructive gating, irreversibility scoring, memory risk forecasting) for all shell commands when a cloud policy cache is present; also calls readFileSync on every invocation.
src/plugin/cloud-policy.ts New file that loads policies.json and resolves runtime policy; returns a non-null authoritative cloud policy (with defaultAction ALLOW) even when the file contains no rules, silently overriding any local DENY configuration.
src/plugin/cloud-policy-evaluator.ts New evaluator for shell and MCP cloud rules; MCP rule matching uses an unanchored regex that can match tool names beyond the intended prefix pattern.
src/plugin/index.ts Plugin loader updated to prefer cloud policy over local policy when policies.json exists; change is straightforward and correctly falls back to local policy when the file is absent.
src/lib/watchtower-client.ts Removed protected_paths field from PolicyBundle and fetchPolicies; updated fetchShellPolicies to use new /api/policies/shell_sync endpoint and handle items[] response shape.
test/policy.test.mjs Added two new tests for cloud policy override behaviour; tests cover the happy path (cloud authoritative when file exists) but do not cover the empty-file bypass scenario or MCP regex matching edge cases.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[before_tool_call event] --> B[Load cloud policy cache\npolicies.json]
    B --> C{Cloud policy\nexists?}

    C -- No --> D[Use local Interceptor\nevaluate]
    C -- Yes\nShell/bash/exec --> E{Cloud shell rule\nmatches?}
    C -- Yes\nAny other tool --> F{Cloud MCP rule\nmatches?}

    E -- BLOCK --> G[Return block=true\nLog BLOCKED]
    E -- WARN --> H[Log warning\nLog ALLOWED\nReturn early]
    E -- No match --> I[Log ALLOWED\nReturn early]

    F -- BLOCK --> J[Return block=true\nLog BLOCKED]
    F -- WARN / LOG --> K[Log ALLOWED\nReturn early]
    F -- No match --> L[Continue to full\nlocal pipeline]

    D --> L
    L --> M[Irreversibility scoring]
    M --> N[Destructive classification]
    N --> O[Memory risk forecasting]
    O --> P[Rate limiter check]
    P --> Q[interceptor.evaluate]
    Q --> R{Allowed?}
    R -- Yes --> S[Return allowed]
    R -- No --> T[Return block=true]
Loading

Reviews (1): Last reviewed commit: "Implemented cloud shell policy, which ov..." | Re-trigger Greptile

Comment on lines +130 to +180
if (
cloudPolicy &&
moduleName === 'Shell' &&
['bash', 'exec'].includes(methodName)
) {
const command = typeof params.command === 'string' ? params.command : '';
const matchedRule = evaluateCloudShellRule(command, cloudPolicy);

if (matchedRule?.action === 'BLOCK') {
const reason =
`Reins Cloud policy blocked shell command. ` +
`${matchedRule.description || 'No description provided.'} ` +
`[rule: ${matchedRule.rule}; severity: ${matchedRule.severity || 'unknown'}]`;
await DecisionLog.append({
timestamp: new Date().toISOString(),
module: moduleName,
method: methodName,
args: [params],
decision: 'BLOCKED',
decisionTime: 0,
reason,
eventType: 'tool_blocked',
tool: toolName,
});
return { block: true, blockReason: reason };
}

if (matchedRule?.action === 'WARN') {
logger.warn('Reins Cloud shell rule matched with WARN action', {
toolName,
command,
rule: matchedRule.rule,
severity: matchedRule.severity,
});
}

await DecisionLog.append({
timestamp: new Date().toISOString(),
module: moduleName,
method: methodName,
args: [params],
decision: 'ALLOWED',
decisionTime: 0,
reason: matchedRule
? 'allowed by Reins Cloud shell policy cache'
: 'allowed by Reins Cloud shell policy cache (no matching block rule)',
eventType: 'tool_executed',
tool: toolName,
});
return shouldReturnParams ? { params } : {};
}

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.

P1 Shell safety pipeline bypassed when cloud policy is active

When cloudPolicy is loaded and the tool is a shell command, the hook always returns at line 179 — before irreversibility scoring, destructive gating, memory risk forecasting, and rate-limiting logic run. This means any bash or exec call that doesn't match a BLOCK rule (including all WARN/LOG rule hits and zero-rule matches) skips every downstream safety check. A policies.json with an empty shell_rules array effectively disables catastrophic-action gating for all shell commands.

Comment on lines +24 to +41
export function loadCloudPolicyCache(): CloudPolicyCache | null {
try {
const raw = readFileSync(getDataPath('policies.json'), 'utf8');
const parsed = JSON.parse(raw) as unknown;
if (!parsed || typeof parsed !== 'object') {
return null;
}

const record = parsed as Record<string, unknown>;
return {
shell_rules: Array.isArray(record['shell_rules']) ? (record['shell_rules'] as ShellRule[]) : [],
mcp_rules: Array.isArray(record['mcp_rules']) ? (record['mcp_rules'] as McpRule[]) : [],
updated_at: typeof record['updated_at'] === 'string' ? record['updated_at'] : undefined,
};
} catch {
return null;
}
}

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.

P1 Empty or minimal policies.json silently disables local shell enforcement

loadCloudPolicyCache returns a non-null CloudPolicyCache — with empty arrays — when the file contains {} or any valid JSON object with no recognised keys. The caller in tool-interceptor.ts treats any non-null return as authoritative cloud policy, causing all shell commands to return early without going through the local interceptor. A corrupt, zero-byte (invalid JSON is caught, but {} is not), or deliberately emptied policies.json will fully bypass the local DENY/allow-path rules configured by the operator.

Comment on lines +30 to +48
export function evaluateCloudMcpRule(toolName: string, cloudPolicy: CloudPolicyCache): CloudRuleMatch | null {
for (const rule of cloudPolicy.mcp_rules) {
try {
const matches =
toolName.startsWith(rule.tool_pattern) ||
new RegExp(rule.tool_pattern, 'i').test(toolName);
if (matches) {
return {
action: rule.action,
severity: rule.severity,
rule: rule.tool_pattern,
description: rule.description,
};
}
} catch {
// Ignore invalid regex patterns from cache.
}
}

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.

P1 Unanchored regex in MCP rule matching can match unintended tools

The regex path in evaluateCloudMcpRule is only reached when startsWith returns false, but the constructed RegExp is not anchored (^/$). A tool_pattern of "exec" would match reexec, context, or any tool whose name contains the substring. An operator who intends a prefix-only rule will get broader matching than expected, potentially triggering BLOCK or WARN actions on unrelated tools. Consider anchoring the regex (e.g. ^${pattern}) or documenting that patterns are treated as unanchored substrings.


let params = event.params;
let shouldReturnParams = false;
const cloudPolicy = loadCloudPolicyCache();

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.

P2 readFileSync called on every tool invocation

loadCloudPolicyCache() synchronously reads and parses policies.json on every single before_tool_call event. In a busy session this adds a synchronous disk-read to every tool call, blocking the event loop. Consider caching the result in memory (with a short TTL or file-watch invalidation) so that repeated calls are served from the in-process cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant