fix(classifier): fix 3 safety bugs, extract const arrays, make tier UI dynamic
All checks were successful
Test / frontend-typecheck (pull_request) Successful in 1m36s
Test / frontend-tests (pull_request) Successful in 1m40s
PR Review Automation / review (pull_request) Successful in 10m27s
Test / rust-fmt-check (pull_request) Successful in 11m4s
Test / rust-clippy (pull_request) Successful in 12m50s
Test / rust-tests (pull_request) Successful in 14m20s
All checks were successful
Test / frontend-typecheck (pull_request) Successful in 1m36s
Test / frontend-tests (pull_request) Successful in 1m40s
PR Review Automation / review (pull_request) Successful in 10m27s
Test / rust-fmt-check (pull_request) Successful in 11m4s
Test / rust-clippy (pull_request) Successful in 12m50s
Test / rust-tests (pull_request) Successful in 14m20s
Bug 1 — Dead multi-word tier3 entries / missing single-token commands parse_single_command() extracts only the first token as `command`, so multi-word entries like "kill -9", "init 0", "service stop" in the tier3 array never matched. Adding the single-token forms "kill", "pkill", "killall", "init" to TIER3_COMMANDS ensures these commands are always denied. Removed all dead multi-word entries. Bug 2 — systemctl Tier 1 special case was dead code systemctl was not in tier1_general, so the block that was supposed to auto-execute `systemctl status` never ran. Moved systemctl handling into its own block (TIER1_SYSTEMCTL_SUBCOMMANDS / TIER2_SYSTEMCTL_SUBCOMMANDS) evaluated before the general tier checks. status, is-active, is-enabled, list-units, list-unit-files → Tier 1; all others → Tier 2. Bug 3 — ldapmodify / ldapdelete / ldapadd misclassified as Tier 1 Both appeared in the old tier1_general and tier2_general arrays; the tier1 check ran first, so LDAP write operations auto-executed. Removed them from tier1. ldapsearch (read-only) remains Tier 1. Dynamic Safety Architecture UI Extracted all tier classification arrays to module-level pub const slices (TIER3_COMMANDS, TIER1_KUBECTL_SUBCOMMANDS, etc.) so both the classifier logic and a new get_classifier_rules() Tauri command share a single source of truth. ShellExecution.tsx now calls getClassifierRulesCmd() on mount and renders the actual command lists in collapsible per-tier cards — any change to the const arrays is automatically reflected in the UI with no manual documentation update needed. Also fixes the cargo fmt CI failure introduced in the previous commit (ClusterClient::new call reformatted to a single line).
This commit is contained in:
parent
ef3709ffe9
commit
7d8d5bdbba
@ -180,13 +180,7 @@ pub async fn connect_cluster_from_kubeconfig(
|
||||
let context = extract_context(&content)?;
|
||||
let server_url = extract_server_url(&content).unwrap_or_default();
|
||||
|
||||
let client = ClusterClient::new(
|
||||
id.clone(),
|
||||
name,
|
||||
context,
|
||||
server_url,
|
||||
Arc::new(content),
|
||||
);
|
||||
let client = ClusterClient::new(id.clone(), name, context, server_url, Arc::new(content));
|
||||
|
||||
let mut clusters = state.clusters.lock().await;
|
||||
clusters.insert(id, client);
|
||||
|
||||
@ -232,3 +232,11 @@ pub async fn check_kubectl_installed(_state: State<'_, AppState>) -> Result<Kube
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// Return the live classifier rule lists so the UI can render them dynamically.
|
||||
/// The data derives directly from the module-level const arrays in classifier.rs,
|
||||
/// so any addition or removal there is automatically reflected in the UI.
|
||||
#[tauri::command]
|
||||
pub fn get_classifier_rules() -> crate::shell::classifier::ClassifierRules {
|
||||
crate::shell::classifier::CommandClassifier::get_rules()
|
||||
}
|
||||
|
||||
@ -177,6 +177,7 @@ pub fn run() {
|
||||
commands::shell::respond_to_shell_approval,
|
||||
commands::shell::list_command_executions,
|
||||
commands::shell::check_kubectl_installed,
|
||||
commands::shell::get_classifier_rules,
|
||||
// Kubernetes Management
|
||||
commands::kube::add_cluster,
|
||||
commands::kube::connect_cluster_from_kubeconfig,
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@ -739,6 +739,21 @@ export const listCommandExecutionsCmd = (issueId?: string) =>
|
||||
export const checkKubectlInstalledCmd = () =>
|
||||
invoke<KubectlStatus>("check_kubectl_installed");
|
||||
|
||||
export interface ClassifierRules {
|
||||
tier1_kubectl: string[];
|
||||
tier1_systemctl: string[];
|
||||
tier1_proxmox: string[];
|
||||
tier1_general: string[];
|
||||
tier2_kubectl: string[];
|
||||
tier2_systemctl: string[];
|
||||
tier2_proxmox: string[];
|
||||
tier2_general: string[];
|
||||
tier3: string[];
|
||||
}
|
||||
|
||||
export const getClassifierRulesCmd = () =>
|
||||
invoke<ClassifierRules>("get_classifier_rules");
|
||||
|
||||
// ─── Kubernetes Management Types ──────────────────────────────────────────────
|
||||
|
||||
export interface ClusterInfo {
|
||||
|
||||
@ -1,17 +1,165 @@
|
||||
import { useState, useEffect } from 'react';
|
||||
import { Terminal, CheckCircle, XCircle, Shield, History } from 'lucide-react';
|
||||
import { Terminal, CheckCircle, XCircle, Shield, History, ChevronDown } from 'lucide-react';
|
||||
import { Button, Card, CardHeader, CardTitle, CardContent, Badge } from '@/components/ui';
|
||||
import { Link } from 'react-router-dom';
|
||||
import {
|
||||
checkKubectlInstalledCmd,
|
||||
listCommandExecutionsCmd,
|
||||
getClassifierRulesCmd,
|
||||
type KubectlStatus,
|
||||
type CommandExecution,
|
||||
type ClassifierRules,
|
||||
} from '@/lib/tauriCommands';
|
||||
|
||||
// ── Tier display config ───────────────────────────────────────────────────────
|
||||
|
||||
interface TierConfig {
|
||||
label: string;
|
||||
behavior: string;
|
||||
colorBg: string;
|
||||
colorBorder: string;
|
||||
colorHeading: string;
|
||||
colorText: string;
|
||||
badgeClass: string;
|
||||
tier: 1 | 2 | 3;
|
||||
}
|
||||
|
||||
const TIER_CONFIG: TierConfig[] = [
|
||||
{
|
||||
tier: 1,
|
||||
label: 'Tier 1',
|
||||
behavior: 'Auto-execute (Read-only)',
|
||||
colorBg: 'bg-green-50',
|
||||
colorBorder: 'border-green-200',
|
||||
colorHeading: 'text-green-900',
|
||||
colorText: 'text-green-800',
|
||||
badgeClass: 'bg-green-100 text-green-700 border-green-300',
|
||||
},
|
||||
{
|
||||
tier: 2,
|
||||
label: 'Tier 2',
|
||||
behavior: 'Require approval (Mutating)',
|
||||
colorBg: 'bg-yellow-50',
|
||||
colorBorder: 'border-yellow-200',
|
||||
colorHeading: 'text-yellow-900',
|
||||
colorText: 'text-yellow-800',
|
||||
badgeClass: 'bg-yellow-100 text-yellow-700 border-yellow-300',
|
||||
},
|
||||
{
|
||||
tier: 3,
|
||||
label: 'Tier 3',
|
||||
behavior: 'Always deny (Destructive)',
|
||||
colorBg: 'bg-red-50',
|
||||
colorBorder: 'border-red-200',
|
||||
colorHeading: 'text-red-900',
|
||||
colorText: 'text-red-800',
|
||||
badgeClass: 'bg-red-100 text-red-700 border-red-300',
|
||||
},
|
||||
];
|
||||
|
||||
// ── Helper: build per-tier category groups from ClassifierRules ───────────────
|
||||
|
||||
interface CategoryGroup {
|
||||
label: string;
|
||||
commands: string[];
|
||||
}
|
||||
|
||||
function buildTier1Groups(rules: ClassifierRules): CategoryGroup[] {
|
||||
return [
|
||||
{ label: 'kubectl', commands: rules.tier1_kubectl.map((c) => `kubectl ${c}`) },
|
||||
{ label: 'systemctl', commands: rules.tier1_systemctl.map((c) => `systemctl ${c}`) },
|
||||
{ label: 'proxmox', commands: rules.tier1_proxmox.map((c) => `<cmd> ${c}`) },
|
||||
{ label: 'general', commands: rules.tier1_general },
|
||||
].filter((g) => g.commands.length > 0);
|
||||
}
|
||||
|
||||
function buildTier2Groups(rules: ClassifierRules): CategoryGroup[] {
|
||||
return [
|
||||
{ label: 'kubectl', commands: rules.tier2_kubectl.map((c) => `kubectl ${c}`) },
|
||||
{ label: 'systemctl', commands: rules.tier2_systemctl.map((c) => `systemctl ${c}`) },
|
||||
{ label: 'proxmox', commands: rules.tier2_proxmox.map((c) => `<cmd> ${c}`) },
|
||||
{ label: 'general', commands: rules.tier2_general },
|
||||
].filter((g) => g.commands.length > 0);
|
||||
}
|
||||
|
||||
function buildTier3Groups(rules: ClassifierRules): CategoryGroup[] {
|
||||
return [{ label: 'all', commands: rules.tier3 }];
|
||||
}
|
||||
|
||||
const PREVIEW_COUNT = 6;
|
||||
|
||||
// ── Sub-components ────────────────────────────────────────────────────────────
|
||||
|
||||
function CommandChip({ cmd, colorText }: { cmd: string; colorText: string }) {
|
||||
return (
|
||||
<code
|
||||
className={`inline-block rounded px-1.5 py-0.5 text-xs font-mono border border-current/20 ${colorText}`}
|
||||
>
|
||||
{cmd}
|
||||
</code>
|
||||
);
|
||||
}
|
||||
|
||||
interface TierCardProps {
|
||||
config: TierConfig;
|
||||
groups: CategoryGroup[];
|
||||
}
|
||||
|
||||
function TierCard({ config, groups }: TierCardProps) {
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
|
||||
const allCommands = groups.flatMap((g) => g.commands);
|
||||
const total = allCommands.length;
|
||||
const previewCommands = allCommands.slice(0, PREVIEW_COUNT);
|
||||
const hasMore = total > PREVIEW_COUNT;
|
||||
|
||||
return (
|
||||
<div
|
||||
className={`rounded-lg p-3 border ${config.colorBg} ${config.colorBorder}`}
|
||||
data-testid={`tier${config.tier}-card`}
|
||||
>
|
||||
<div className="flex items-start justify-between gap-2">
|
||||
<div className="flex items-start gap-3 min-w-0">
|
||||
<Badge className={config.badgeClass}>{config.label}</Badge>
|
||||
<div className="min-w-0">
|
||||
<div className={`font-medium ${config.colorHeading}`}>{config.behavior}</div>
|
||||
<div
|
||||
className={`mt-1.5 flex flex-wrap gap-1 ${config.colorText}`}
|
||||
data-testid={`tier${config.tier}-commands`}
|
||||
>
|
||||
{(expanded ? allCommands : previewCommands).map((cmd) => (
|
||||
<CommandChip key={cmd} cmd={cmd} colorText={config.colorText} />
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<span className={`shrink-0 text-xs font-mono tabular-nums ${config.colorText} opacity-70`}>
|
||||
{total}
|
||||
</span>
|
||||
</div>
|
||||
|
||||
{hasMore && (
|
||||
<button
|
||||
onClick={() => setExpanded((p) => !p)}
|
||||
className={`mt-2 flex items-center gap-1 text-xs ${config.colorText} hover:opacity-80 transition-opacity`}
|
||||
data-testid={`tier${config.tier}-toggle`}
|
||||
>
|
||||
<ChevronDown
|
||||
className={`h-3 w-3 transition-transform ${expanded ? 'rotate-180' : ''}`}
|
||||
/>
|
||||
{expanded ? 'Show fewer' : `Show all ${total} commands`}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// ── Main component ────────────────────────────────────────────────────────────
|
||||
|
||||
export default function ShellExecution() {
|
||||
const [kubectlStatus, setKubectlStatus] = useState<KubectlStatus | null>(null);
|
||||
const [executions, setExecutions] = useState<CommandExecution[]>([]);
|
||||
const [classifierRules, setClassifierRules] = useState<ClassifierRules | null>(null);
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
const [error, setError] = useState('');
|
||||
|
||||
@ -36,30 +184,44 @@ export default function ShellExecution() {
|
||||
}
|
||||
};
|
||||
|
||||
const loadClassifierRules = async () => {
|
||||
try {
|
||||
const rules = await getClassifierRulesCmd();
|
||||
setClassifierRules(rules);
|
||||
} catch {
|
||||
// Non-fatal — fall back to empty state; the tier cards will just show 0 commands
|
||||
}
|
||||
};
|
||||
|
||||
useEffect(() => {
|
||||
loadKubectlStatus();
|
||||
loadExecutions();
|
||||
loadClassifierRules();
|
||||
}, []);
|
||||
|
||||
const getTierBadge = (tier: number) => {
|
||||
const colors = {
|
||||
const colors: Record<number, string> = {
|
||||
1: 'bg-green-100 text-green-700 border-green-300',
|
||||
2: 'bg-yellow-100 text-yellow-700 border-yellow-300',
|
||||
3: 'bg-red-100 text-red-700 border-red-300',
|
||||
};
|
||||
return colors[tier as keyof typeof colors] || colors[1];
|
||||
return colors[tier] ?? colors[1];
|
||||
};
|
||||
|
||||
const getStatusBadge = (status: string) => {
|
||||
const config = {
|
||||
const config: Record<string, { label: string; color: string }> = {
|
||||
auto: { label: 'Auto-executed', color: 'bg-blue-100 text-blue-700 border-blue-300' },
|
||||
approved: { label: 'Approved', color: 'bg-green-100 text-green-700 border-green-300' },
|
||||
denied: { label: 'Denied', color: 'bg-red-100 text-red-700 border-red-300' },
|
||||
};
|
||||
const statusConfig = config[status as keyof typeof config] || config.auto;
|
||||
return statusConfig;
|
||||
return config[status] ?? config.auto;
|
||||
};
|
||||
|
||||
// Build grouped command lists for each tier (empty arrays when rules not loaded)
|
||||
const tier1Groups = classifierRules ? buildTier1Groups(classifierRules) : [];
|
||||
const tier2Groups = classifierRules ? buildTier2Groups(classifierRules) : [];
|
||||
const tier3Groups = classifierRules ? buildTier3Groups(classifierRules) : [];
|
||||
|
||||
return (
|
||||
<div className="p-6 space-y-6">
|
||||
<div>
|
||||
@ -126,7 +288,7 @@ export default function ShellExecution() {
|
||||
</CardContent>
|
||||
</Card>
|
||||
|
||||
{/* Safety Architecture */}
|
||||
{/* Safety Architecture — driven by live classifier data */}
|
||||
<Card>
|
||||
<CardHeader>
|
||||
<CardTitle className="flex items-center gap-2">
|
||||
@ -134,42 +296,21 @@ export default function ShellExecution() {
|
||||
Safety Architecture
|
||||
</CardTitle>
|
||||
</CardHeader>
|
||||
<CardContent className="space-y-4">
|
||||
<CardContent className="space-y-3">
|
||||
<p className="text-sm text-muted-foreground">
|
||||
Commands are automatically classified into three safety tiers:
|
||||
Commands are automatically classified into three safety tiers. The lists below
|
||||
reflect the active classifier rules — they update whenever a rule is added or removed.
|
||||
</p>
|
||||
|
||||
<div className="space-y-3">
|
||||
<div className="flex items-start gap-3 p-3 rounded-lg bg-green-50 border border-green-200">
|
||||
<Badge className={getTierBadge(1)}>Tier 1</Badge>
|
||||
<div className="space-y-1">
|
||||
<div className="font-medium text-green-900">Auto-execute (Read-only)</div>
|
||||
<div className="text-sm text-green-800">
|
||||
kubectl get, describe, logs | cat, grep, ls
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{!classifierRules && (
|
||||
<p className="text-xs text-muted-foreground">Loading classifier rules…</p>
|
||||
)}
|
||||
|
||||
<div className="flex items-start gap-3 p-3 rounded-lg bg-yellow-50 border border-yellow-200">
|
||||
<Badge className={getTierBadge(2)}>Tier 2</Badge>
|
||||
<div className="space-y-1">
|
||||
<div className="font-medium text-yellow-900">Require approval (Mutating)</div>
|
||||
<div className="text-sm text-yellow-800">
|
||||
kubectl apply, delete, scale | ssh, chmod, systemctl restart
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div className="flex items-start gap-3 p-3 rounded-lg bg-red-50 border border-red-200">
|
||||
<Badge className={getTierBadge(3)}>Tier 3</Badge>
|
||||
<div className="space-y-1">
|
||||
<div className="font-medium text-red-900">Always deny (Destructive)</div>
|
||||
<div className="text-sm text-red-800">
|
||||
rm -rf, shutdown, mkfs, dd
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{TIER_CONFIG.map((cfg) => {
|
||||
const groups =
|
||||
cfg.tier === 1 ? tier1Groups : cfg.tier === 2 ? tier2Groups : tier3Groups;
|
||||
return <TierCard key={cfg.tier} config={cfg} groups={groups} />;
|
||||
})}
|
||||
</CardContent>
|
||||
</Card>
|
||||
|
||||
@ -201,12 +342,8 @@ export default function ShellExecution() {
|
||||
</code>
|
||||
</div>
|
||||
<div className="flex gap-2 ml-3 flex-shrink-0">
|
||||
<Badge className={getTierBadge(exec.tier)}>
|
||||
T{exec.tier}
|
||||
</Badge>
|
||||
<Badge className={statusConfig.color}>
|
||||
{statusConfig.label}
|
||||
</Badge>
|
||||
<Badge className={getTierBadge(exec.tier)}>T{exec.tier}</Badge>
|
||||
<Badge className={statusConfig.color}>{statusConfig.label}</Badge>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
93
tests/unit/shellCommands.test.ts
Normal file
93
tests/unit/shellCommands.test.ts
Normal file
@ -0,0 +1,93 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { invoke } from "@tauri-apps/api/core";
|
||||
import * as tauriCommands from "@/lib/tauriCommands";
|
||||
|
||||
vi.mock("@tauri-apps/api/core");
|
||||
|
||||
type MockedFunction<T = (...args: unknown[]) => unknown> = T & {
|
||||
mockResolvedValue: (value: unknown) => void;
|
||||
mockRejectedValue: (error: Error) => void;
|
||||
};
|
||||
|
||||
const mockClassifierRules = {
|
||||
tier1_kubectl: ["get", "describe", "logs", "explain", "api-resources", "api-versions", "cluster-info", "top", "version"],
|
||||
tier1_systemctl: ["status", "is-active", "is-enabled", "list-units", "list-unit-files"],
|
||||
tier1_proxmox: ["status", "get"],
|
||||
tier1_general: ["cat", "grep", "ls", "find", "df", "free", "ps", "dig", "nslookup", "ldapsearch"],
|
||||
tier2_kubectl: ["apply", "delete", "edit", "scale", "rollout", "exec", "cp", "port-forward"],
|
||||
tier2_systemctl: ["restart", "stop", "start", "enable", "disable", "reload", "mask", "unmask"],
|
||||
tier2_proxmox: ["migrate", "create", "set", "delete", "start", "stop"],
|
||||
tier2_general: ["ssh", "scp", "chmod", "chown", "curl", "wget", "ldapmodify", "ldapdelete", "ldapadd"],
|
||||
tier3: ["rm", "mkfs", "dd", "fdisk", "kill", "pkill", "killall", "init", "shutdown", "reboot", "halt", "poweroff"],
|
||||
};
|
||||
|
||||
describe("Shell Classifier Commands", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("getClassifierRulesCmd", () => {
|
||||
it("should call invoke with correct command name", async () => {
|
||||
(invoke as MockedFunction).mockResolvedValue(mockClassifierRules);
|
||||
|
||||
await tauriCommands.getClassifierRulesCmd();
|
||||
|
||||
expect(invoke).toHaveBeenCalledWith("get_classifier_rules");
|
||||
});
|
||||
|
||||
it("should return the classifier rules structure", async () => {
|
||||
(invoke as MockedFunction).mockResolvedValue(mockClassifierRules);
|
||||
|
||||
const result = await tauriCommands.getClassifierRulesCmd();
|
||||
|
||||
expect(result.tier1_kubectl).toContain("get");
|
||||
expect(result.tier1_kubectl).toContain("logs");
|
||||
expect(result.tier2_kubectl).toContain("apply");
|
||||
expect(result.tier2_kubectl).toContain("delete");
|
||||
expect(result.tier3).toContain("rm");
|
||||
expect(result.tier3).toContain("kill");
|
||||
expect(result.tier3).toContain("init");
|
||||
});
|
||||
|
||||
it("should include fix for Bug 1 — kill and init in tier3", async () => {
|
||||
(invoke as MockedFunction).mockResolvedValue(mockClassifierRules);
|
||||
|
||||
const result = await tauriCommands.getClassifierRulesCmd();
|
||||
|
||||
expect(result.tier3).toContain("kill");
|
||||
expect(result.tier3).toContain("pkill");
|
||||
expect(result.tier3).toContain("killall");
|
||||
expect(result.tier3).toContain("init");
|
||||
});
|
||||
|
||||
it("should include fix for Bug 2 — systemctl read-only subcommands in tier1", async () => {
|
||||
(invoke as MockedFunction).mockResolvedValue(mockClassifierRules);
|
||||
|
||||
const result = await tauriCommands.getClassifierRulesCmd();
|
||||
|
||||
expect(result.tier1_systemctl).toContain("status");
|
||||
expect(result.tier1_systemctl).toContain("is-active");
|
||||
expect(result.tier2_systemctl).toContain("restart");
|
||||
expect(result.tier2_systemctl).toContain("stop");
|
||||
});
|
||||
|
||||
it("should include fix for Bug 3 — ldap mutating ops in tier2 not tier1", async () => {
|
||||
(invoke as MockedFunction).mockResolvedValue(mockClassifierRules);
|
||||
|
||||
const result = await tauriCommands.getClassifierRulesCmd();
|
||||
|
||||
expect(result.tier2_general).toContain("ldapmodify");
|
||||
expect(result.tier2_general).toContain("ldapdelete");
|
||||
expect(result.tier2_general).toContain("ldapadd");
|
||||
// ldapsearch must NOT appear in tier2 (it's read-only, belongs in tier1)
|
||||
expect(result.tier2_general).not.toContain("ldapsearch");
|
||||
expect(result.tier1_general).toContain("ldapsearch");
|
||||
});
|
||||
|
||||
it("should propagate errors from invoke", async () => {
|
||||
(invoke as MockedFunction).mockRejectedValue(new Error("IPC error"));
|
||||
|
||||
await expect(tauriCommands.getClassifierRulesCmd()).rejects.toThrow("IPC error");
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user