Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
31 changes: 31 additions & 0 deletions apps/desktop/e2e/approvals.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ test("Settings scopes new rules, previews without saving, keeps inactive rules v
await expect(preview()).toHaveAttribute("data-decision", "allow");
await expect(preview()).toContainText("Exact match");
await expect(preview()).toContainText("Preview only");
await page.getByLabel("Sample action").fill("npm test && rm -rf src");
await page.getByRole("button", { name: "Try it", exact: true }).click();
await expect(preview()).toHaveAttribute("data-decision", "ask");
await expect(preview()).toContainText("Allow became Ask because the command chains several commands.");
await expect(preview()).toContainText("Jev not consulted");
expect(await storedRules()).toHaveLength(1);
expect((await page.evaluate(() => window.modex!.invoke("state:get", undefined))).threads).toHaveLength(0);
await page.getByLabel("Sample action").fill("npm build");
Expand Down Expand Up @@ -121,6 +126,8 @@ test("automatic allow and never persist compact receipts; ask and project isolat
await approval.getByRole("button", { name: "Deny", exact: true }).click();
} else {
await expect(tid(approval, "approval-receipt")).toContainText(decision === "allow" ? "Allowed:" : "Refused by rule");
// No Jev in this environment: the allow had no destructive check, and its receipt says so.
if (decision === "allow") await expect(tid(approval, "receipt-meta")).toContainText("Jev not consulted");
await expect(approval.getByRole("button", { name: "Approve", exact: true })).toHaveCount(0);
await tid(approval, "item-toggle").click();
await expect(tid(approval, "item-body")).toContainText("write the sample");
Expand Down Expand Up @@ -159,3 +166,27 @@ test("a safety-downgraded receipt explains why a human must answer", async () =>
await expect(card.getByRole("button", { name: "Deny", exact: true })).toBeVisible();
await expect(card.getByRole("button", { name: "Always", exact: true })).toHaveCount(0);
});

test("an allow rule covers one command: a chained shell command asks and says why", async () => {
const script = path.join(home, "chained-script.json");
fs.writeFileSync(script, JSON.stringify({ steps: [
{ tool_calls: [{ name: "shell", arguments: { command: "echo one; touch chained.txt" } }] },
{ content: "Finished." },
] }));
await page.evaluate(async ({ script, repo }) => {
await window.modex!.invoke("settings:update", { mock_script: script, approval_gate: { enabled: true, threshold: 0.8, timeout_ms: 3000 }, approval_rules: [
{ id: "echo", when: "print text", decision: "allow", project: repo, match: "shell: echo *", enabled: true },
] });
}, { script, repo });
await page.keyboard.press("Meta+n");
await tid(page, "composer-input").fill("chain two commands");
await page.keyboard.press("Meta+Enter");
const card = items(page, "approval");
await expect(card).toHaveCount(1);
await expect(card).toHaveAttribute("data-decided-by", "ask");
await expect(currentRow(page)).toHaveAttribute("data-status", "waiting");
await expect(tid(card, "approval-reason")).toHaveText('Asked because: the command chains several commands; rule "print text" would have allowed it · Jev not consulted');
await card.getByRole("button", { name: "Deny", exact: true }).click();
await expect(currentRow(page)).toHaveAttribute("data-status", "idle");
expect(fs.existsSync(path.join(repo, "chained.txt"))).toBe(false);
});
24 changes: 19 additions & 5 deletions apps/desktop/src/main/engine/approvals/digest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface DigestContext {
}

export const MAX_COMMAND = 500;
const MAX_TITLE = 200;
export const MAX_TITLE = 200;
const MAX_PATHS = 20;
const MAX_PATH = 300;
export const REDACTED = "«redacted»";
Expand Down Expand Up @@ -91,11 +91,25 @@ function pathsOf(action: ApprovalAction): string[] {
return out.slice(0, MAX_PATHS);
}

/** The title Jev sees, before clipping to MAX_TITLE. */
function titleOf(action: ApprovalAction, command: string | undefined, paths: string[]): string {
if (TITLE_FROM_TOOL.has(action.tool)) return clip(`${action.tool === "Write" ? "write" : "edit"} ${paths[0] ?? ""}`.trim(), MAX_TITLE);
if (TITLE_FROM_TOOL.has(action.tool)) return `${action.tool === "Write" ? "write" : "edit"} ${paths[0] ?? ""}`.trim();
// A title that repeats the command gets the scrubbed command, never the raw one.
if (command && action.title.includes(command)) return clip(action.title.replace(command, scrubSecrets(command)), MAX_TITLE);
return clip(scrubSecrets(action.title), MAX_TITLE);
if (command && action.title.includes(command)) return action.title.replace(command, scrubSecrets(command));
return scrubSecrets(action.title);
}

/**
* True when `digestAction` clips text Jev needs to judge the action: a command longer than
* MAX_COMMAND, or a title longer than MAX_TITLE. A title that is only the command line ("$ cmd"
* for Claude, the bare command for Codex) does not count when the command itself fits, because
* Jev then reads the whole command in `command`. Paths are not considered.
*/
export function digestClipped(action: ApprovalAction): boolean {
const raw = commandOf(action);
if (raw && scrubSecrets(raw).length > MAX_COMMAND) return true;
if (raw && (action.title === raw || action.title === `$ ${raw}`)) return false;
return titleOf(action, raw, pathsOf(action)).length > MAX_TITLE;
}

/** Builds the state Jev sees for one approval. Pure: same action, same digest. */
Expand All @@ -107,7 +121,7 @@ export function digestAction(action: ApprovalAction, ctx: DigestContext): Approv
action: {
backend: action.backend,
tool: action.tool,
title: titleOf(action, rawCommand, paths),
title: clip(titleOf(action, rawCommand, paths), MAX_TITLE),
...(action.cwd ? { cwd_name: path.basename(action.cwd) } : {}),
...(action.escalation ? { escalation: true } : {}),
},
Expand Down
67 changes: 53 additions & 14 deletions apps/desktop/src/main/engine/approvals/gate.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ApprovalAnswer, ApprovalGateConfig, ApprovalReceipt, ApprovalRule, Mode, RuleDecision } from "../../../shared/types.js";
import type { ApprovalAnswer, ApprovalDowngrade, ApprovalGateConfig, ApprovalReceipt, ApprovalRule, Mode, RuleDecision } from "../../../shared/types.js";
import type { ApprovalAction } from "../backends/types.js";
import { JevError, type JevQuestion, type JevTransport } from "../routing/jev.js";
import { digestAction } from "./digest.js";
import { digestAction, digestClipped } from "./digest.js";

/**
* The approval gate: answers an approval from the user's plain-language rules before it reaches a
Expand All @@ -13,14 +13,19 @@ import { digestAction } from "./digest.js";
* 3. Rules without one go to Jev (one request, one noul per rule, plus `destructive`), capped at
* MAX_JEV_RULES. A rule applies when its noul ≥ threshold.
* 4. Applying rules combine as never > ask > allow; nothing applies → ask.
* 5. Safety downgrades no rule can override: allow on something Jev finds destructive (≥ 0.5) or on a
* sandbox/permission escalation → ask.
* 6. Jev unavailable → only the deterministic result counts, and the receipt says so.
* 5. Safety downgrades no rule can override, labelled in this order: allow on a sandbox/permission
* escalation, on something Jev finds destructive (≥ 0.5), on a command line that chains or
* redirects commands, or on an action whose digest clipped text Jev needed → ask.
* 6. Jev needed (language rules, or the destructive check on a match allow) but not configured or
* unavailable → only the deterministic result counts, and the decision and receipt say Jev was
* not consulted.
* 7. Aborted while deciding → the approval is answered "no".
*/

export const MAX_JEV_RULES = 12;
export const DESTRUCTIVE_THRESHOLD = 0.5;
/** The reason recorded when Jev was needed and there is no transport to ask. */
export const JEV_NOT_CONFIGURED = "Jev is not configured or unavailable.";

export const DESTRUCTIVE_QUESTION: JevQuestion = {
type: "noul",
Expand Down Expand Up @@ -56,8 +61,10 @@ export interface GateDecision {
ms: number;
/** Jev's `destructive` probability, when it was asked. */
destructive?: number;
/** Why a rule's allow became an ask, or that Jev could not be consulted. */
downgraded?: "destructive" | "escalation" | "jev-unavailable";
/** Why a rule's allow became an ask. */
downgraded?: ApprovalDowngrade;
/** Jev was needed for this decision but was not consulted; independent of `downgraded`. */
jevNotConsulted?: true;
/** Why Jev could not be consulted, when it could not. */
jevError?: string;
/** The turn stopped while the gate was deciding: the approval is answered "no". */
Expand Down Expand Up @@ -85,6 +92,7 @@ export function receiptFor(d: GateDecision): ApprovalReceipt | undefined {
ms: d.ms,
...(d.destructive !== undefined ? { destructive: round(d.destructive) } : {}),
...(d.downgraded ? { downgraded: d.downgraded } : {}),
...(d.jevNotConsulted ? { jevNotConsulted: true as const } : {}),
};
}

Expand Down Expand Up @@ -116,6 +124,37 @@ export function matchesRule(match: string, action: Pick<ApprovalAction, "tool" |
return candidates.some((c) => re.test(c));
}

/** Tools whose approval runs a shell command line: Claude's Bash, Codex's command, the scripted engine's shell. */
const COMMAND_TOOLS = new Set(["Bash", "command", "shell"]);

/**
* Shell control operators and line breaks: `;`, `&` (so `&&`, `&`, `2>&1`), `|` (so `||`), backtick,
* `$(`, `<` and `>` (so `<(` and `>(`), CR and LF. Quoting is deliberately not parsed, so
* `grep "a|b"` counts too: a false positive only means a person is asked.
*/
const CHAINING = /[;&|`<>\r\n]|\$\(/;

/**
* True when a command action's full command line (never the digest's clipped copy) chains,
* redirects or substitutes commands. An allow rule covers one command, so such a line is asked.
* Both the input's `command` and the title are checked; either one chaining is enough.
*/
export function chainsCommands(action: Pick<ApprovalAction, "tool" | "title" | "input">): boolean {
if (!COMMAND_TOOLS.has(action.tool)) return false;
const command = action.input?.command;
const lines = [action.title.replace(/^\$ /, ""), ...(typeof command === "string" ? [command] : [])];
return lines.some((line) => CHAINING.test(line));
}

/** Why an allow must be asked instead; the first reason wins the label. */
function downgradeOf(action: ApprovalAction, destructive: number | undefined): ApprovalDowngrade | undefined {
if (action.escalation) return "escalation";
if (destructive !== undefined && destructive >= DESTRUCTIVE_THRESHOLD) return "destructive";
if (chainsCommands(action)) return "compound";
if (digestClipped(action)) return "incomplete";
return undefined;
}

/** Rules sent to Jev: project-scoped first, then newest (later in the list), at most MAX_JEV_RULES. */
export function jevRules(rules: ApprovalRule[]): ApprovalRule[] {
return rules
Expand Down Expand Up @@ -184,9 +223,11 @@ export async function decide(action: ApprovalAction | undefined, ctx: GateContex
// so the destructive downgrade can apply to an exact-match allow too.
const unmatched = jevRules(rules.filter((r) => !r.match));
const wouldAllow = combine(hits)?.rule.decision === "allow";
const needsJev = unmatched.length > 0 || wouldAllow;
let destructive: number | undefined;
let jevError: string | undefined;
if (ctx.transport && (unmatched.length || wouldAllow)) {
if (needsJev && !ctx.transport) jevError = JEV_NOT_CONFIGURED;
else if (needsJev && ctx.transport) {
const questions: Record<string, JevQuestion> = { destructive: DESTRUCTIVE_QUESTION };
unmatched.forEach((r, n) => { questions[`rule_${n}`] = ruleQuestion(r); });
const state = digestAction(action, { project: { name: ctx.project.name, branch: ctx.project.branch }, mode: ctx.mode });
Expand All @@ -206,12 +247,10 @@ export async function decide(action: ApprovalAction | undefined, ctx: GateContex
if (signal?.aborted) return { decision: "ask", source: "none", ms: ms(), aborted: true };

const best = combine(hits);
const base = { ms: ms(), ...(destructive !== undefined ? { destructive } : {}), ...(jevError ? { jevError, downgraded: "jev-unavailable" as const } : {}) };
const base = { ms: ms(), ...(destructive !== undefined ? { destructive } : {}), ...(jevError ? { jevError, jevNotConsulted: true as const } : {}) };
if (!best) return { decision: "ask", source: "none", ...base };
const decided: GateDecision = { decision: best.rule.decision, rule: best.rule, source: best.via, p: best.p, ...base };
if (decided.decision === "allow") {
if (action.escalation) return { ...decided, decision: "ask", downgraded: "escalation" };
if (destructive !== undefined && destructive >= DESTRUCTIVE_THRESHOLD) return { ...decided, decision: "ask", downgraded: "destructive" };
}
return decided;
// Never and Ask rules stand as they are; only a rule-driven allow can be downgraded.
const downgraded = decided.decision === "allow" ? downgradeOf(action, destructive) : undefined;
return downgraded ? { ...decided, decision: "ask", downgraded } : decided;
}
11 changes: 8 additions & 3 deletions apps/desktop/src/main/engine/approvals/policy.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
import type { ApprovalPolicy, ApprovalReceipt } from "../../../shared/types.js";
import type { ApprovalDowngrade, ApprovalPolicy, ApprovalReceipt } from "../../../shared/types.js";
import type { ApprovalAction } from "../backends/types.js";

/** Downgrades Always allow must not override. */
const HELD: ReadonlySet<string> = new Set<ApprovalDowngrade>(["destructive", "escalation", "compound", "incomplete"]);

/**
* A thread's standing answer to approvals, applied only where the rules gate would have left the
* decision to a human (an explicit "Never" rule has already refused by then).
*
* - `ask`: nothing; the card appears.
* - `always`: yes, unless the request wants more sandbox access, or we cannot tell what it wants
* (no structured action), or the rules gate sent it here (an Ask rule, or a destructive/escalation downgrade).
* (no structured action), or the rules gate sent it here (an Ask rule, or a safety downgrade:
* destructive, escalation, a chained command, or a command Jev could not see in full).
* Jev not being consulted is not itself a reason to ask.
* - `yolo`: yes to everything.
*/
export function policyAnswer(policy: ApprovalPolicy | undefined, action: ApprovalAction | undefined, gated?: ApprovalReceipt): "yes" | null {
if (policy === "yolo") return "yes";
if (policy !== "always") return null;
if (!action || action.escalation) return null;
// A rule that said "ask" or a safety downgrade is the user's own caution; only YOLO overrides it.
if (gated && (gated.decision === "ask" || gated.downgraded === "destructive" || gated.downgraded === "escalation")) return null;
if (gated && (gated.decision === "ask" || (gated.downgraded !== undefined && HELD.has(gated.downgraded)))) return null;
return "yes";
}

Expand Down
3 changes: 2 additions & 1 deletion apps/desktop/src/main/engine/approvals/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,6 @@ export async function previewApproval(raw: unknown, ctx: Context): Promise<Appro
const result = await decide({ backend: req.backend, tool, title, cwd: project.path, escalation: req.escalation,
...(tool === "Bash" || tool === "command" ? { input: { command: title.replace(/^\$ /, "") } } : {}),
}, { rules, config: { ...ctx.config, enabled: true }, project: { root: project.path, name: project.name }, mode: req.mode, transport, model });
return { ...result, ...(setupError ? { jevError: setupError, downgraded: result.downgraded ?? "jev-unavailable" } : {}), gateEnabled: ctx.config.enabled, jevAvailable: transport !== null };
// A judge that failed to resolve is reported by name rather than as "not configured".
return { ...result, ...(setupError ? { jevError: setupError } : {}), gateEnabled: ctx.config.enabled, jevAvailable: transport !== null };
}
4 changes: 3 additions & 1 deletion apps/desktop/src/renderer/components/ApprovalRules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ function PreviewReceipt({ result: r }: { result: ApprovalPreview }) {
<div>{r.source === "match" ? "Exact match" : r.source === "jev" ? `Jev ${r.p?.toFixed(2) ?? ""}` : "No match"} · {r.ms} ms{r.destructive !== undefined ? ` · destructive ${r.destructive.toFixed(2)}` : ""}</div>
{r.downgraded === "destructive" && <p>Allow became Ask because the action may be destructive.</p>}
{r.downgraded === "escalation" && <p>Allow became Ask because only you can grant additional permissions.</p>}
{(!r.jevAvailable || r.downgraded === "jev-unavailable") && <p>Jev unavailable; only exact-match rules were considered.{r.jevError ? ` ${r.jevError}` : ""}</p>}
{r.downgraded === "compound" && <p>Allow became Ask because the command chains several commands.</p>}
{r.downgraded === "incomplete" && <p>Allow became Ask because Jev could not see the whole command.</p>}
{(!r.jevAvailable || r.jevNotConsulted) && <p>Jev not consulted; only exact-match rules were considered.{r.jevError ? ` ${r.jevError}` : ""}</p>}
{!r.gateEnabled && <p>Preview only — the approval gate is off.</p>}
</div>;
}
6 changes: 3 additions & 3 deletions apps/desktop/src/renderer/components/ThreadView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { toolSummary } from "../../shared/tool-summary";
import { Fragment, useEffect, useMemo, useRef, useState } from "react";
import type { ApprovalPolicy, ApprovalReceipt, BackendId, FollowUp, Mode, ModelInfo, Project, Thread, ThreadItem, ThreadPatch, TurnFailure, TurnFix } from "../../shared/types";
import { failureReport } from "../../shared/failures";
import { askedBecause, receiptVia } from "../../shared/approval-receipt";
import { askedBecause, jevNotConsulted, receiptVia } from "../../shared/approval-receipt";
import { safeHref } from "../../shared/inline";
import { parseWebSearch, splitSources, type SourceLink, type WebSearch } from "../../shared/sources";
import type { AttachmentActions } from "../composer-attachments";
Expand Down Expand Up @@ -392,7 +392,7 @@ function ApprovalReceiptRow({ item, receipt: r }: { item: ApprovalEntry; receipt
const [open, setOpen] = useState(false);
const allowed = r.decision === "allow";
const title = item.title ?? item.question;
const via = `${receiptVia(r)} · ${r.ms} ms${r.downgraded === "jev-unavailable" ? " · Jev unavailable" : ""}`;
const via = `${receiptVia(r)} · ${r.ms} ms${jevNotConsulted(r) ? " · Jev not consulted" : ""}`;
return (
<div className={`approval-receipt ${allowed ? "allowed" : "refused"} ${open ? "open" : ""}`} data-testid="item" data-item-kind="approval" data-answered="true" data-decided-by={r.decision}>
<button className="receipt-head" data-testid="item-toggle" onClick={() => setOpen((v) => !v)} aria-expanded={open}>
Expand All @@ -411,7 +411,7 @@ function ApprovalReceiptRow({ item, receipt: r }: { item: ApprovalEntry; receipt
{r.source === "policy"
? `Approved because this thread is set to "${r.when}".`
: <>{allowed ? "Approved once" : "Denied"} by your rule "{r.when}" ({receiptVia(r)}, {r.ms} ms){r.destructive !== undefined ? `; Jev put the chance that it is destructive at ${r.destructive.toFixed(2)}` : ""}.
{r.downgraded === "jev-unavailable" ? " Jev was unavailable, so only rules with an exact match could apply." : ""}</>}
{jevNotConsulted(r) ? " Jev was not consulted (not configured or unavailable), so only rules with an exact match could apply and nothing was checked for destructive effects." : ""}</>}
</div>
</div>
)}
Expand Down
Loading
Loading