fix: use Windows ACLs for security audit

This commit is contained in:
Peter Steinberger
2026-01-26 18:19:58 +00:00
parent b9098f3401
commit ab73aceb27
8 changed files with 760 additions and 110 deletions

View File

@@ -24,14 +24,11 @@ import {
import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
import { resolveNativeCommandsEnabled, resolveNativeSkillsEnabled } from "../config/commands.js";
import {
formatOctal,
isGroupReadable,
isGroupWritable,
isWorldReadable,
isWorldWritable,
modeBits,
safeStat,
formatPermissionDetail,
formatPermissionRemediation,
inspectPathPermissions,
} from "./audit-fs.js";
import type { ExecFn } from "./windows-acl.js";
export type SecurityAuditSeverity = "info" | "warn" | "critical";
@@ -66,6 +63,8 @@ export type SecurityAuditReport = {
export type SecurityAuditOptions = {
config: ClawdbotConfig;
env?: NodeJS.ProcessEnv;
platform?: NodeJS.Platform;
deep?: boolean;
includeFilesystem?: boolean;
includeChannelSecurity?: boolean;
@@ -79,6 +78,8 @@ export type SecurityAuditOptions = {
plugins?: ReturnType<typeof listChannelPlugins>;
/** Dependency injection for tests. */
probeGatewayFn?: typeof probeGateway;
/** Dependency injection for tests (Windows ACL checks). */
execIcacls?: ExecFn;
};
function countBySeverity(findings: SecurityAuditFinding[]): SecurityAuditSummary {
@@ -119,13 +120,19 @@ function classifyChannelWarningSeverity(message: string): SecurityAuditSeverity
async function collectFilesystemFindings(params: {
stateDir: string;
configPath: string;
env?: NodeJS.ProcessEnv;
platform?: NodeJS.Platform;
execIcacls?: ExecFn;
}): Promise<SecurityAuditFinding[]> {
const findings: SecurityAuditFinding[] = [];
const stateDirStat = await safeStat(params.stateDir);
if (stateDirStat.ok) {
const bits = modeBits(stateDirStat.mode);
if (stateDirStat.isSymlink) {
const stateDirPerms = await inspectPathPermissions(params.stateDir, {
env: params.env,
platform: params.platform,
exec: params.execIcacls,
});
if (stateDirPerms.ok) {
if (stateDirPerms.isSymlink) {
findings.push({
checkId: "fs.state_dir.symlink",
severity: "warn",
@@ -133,37 +140,58 @@ async function collectFilesystemFindings(params: {
detail: `${params.stateDir} is a symlink; treat this as an extra trust boundary.`,
});
}
if (isWorldWritable(bits)) {
if (stateDirPerms.worldWritable) {
findings.push({
checkId: "fs.state_dir.perms_world_writable",
severity: "critical",
title: "State dir is world-writable",
detail: `${params.stateDir} mode=${formatOctal(bits)}; other users can write into your Clawdbot state.`,
remediation: `chmod 700 ${params.stateDir}`,
detail: `${formatPermissionDetail(params.stateDir, stateDirPerms)}; other users can write into your Clawdbot state.`,
remediation: formatPermissionRemediation({
targetPath: params.stateDir,
perms: stateDirPerms,
isDir: true,
posixMode: 0o700,
env: params.env,
}),
});
} else if (isGroupWritable(bits)) {
} else if (stateDirPerms.groupWritable) {
findings.push({
checkId: "fs.state_dir.perms_group_writable",
severity: "warn",
title: "State dir is group-writable",
detail: `${params.stateDir} mode=${formatOctal(bits)}; group users can write into your Clawdbot state.`,
remediation: `chmod 700 ${params.stateDir}`,
detail: `${formatPermissionDetail(params.stateDir, stateDirPerms)}; group users can write into your Clawdbot state.`,
remediation: formatPermissionRemediation({
targetPath: params.stateDir,
perms: stateDirPerms,
isDir: true,
posixMode: 0o700,
env: params.env,
}),
});
} else if (isGroupReadable(bits) || isWorldReadable(bits)) {
} else if (stateDirPerms.groupReadable || stateDirPerms.worldReadable) {
findings.push({
checkId: "fs.state_dir.perms_readable",
severity: "warn",
title: "State dir is readable by others",
detail: `${params.stateDir} mode=${formatOctal(bits)}; consider restricting to 700.`,
remediation: `chmod 700 ${params.stateDir}`,
detail: `${formatPermissionDetail(params.stateDir, stateDirPerms)}; consider restricting to 700.`,
remediation: formatPermissionRemediation({
targetPath: params.stateDir,
perms: stateDirPerms,
isDir: true,
posixMode: 0o700,
env: params.env,
}),
});
}
}
const configStat = await safeStat(params.configPath);
if (configStat.ok) {
const bits = modeBits(configStat.mode);
if (configStat.isSymlink) {
const configPerms = await inspectPathPermissions(params.configPath, {
env: params.env,
platform: params.platform,
exec: params.execIcacls,
});
if (configPerms.ok) {
if (configPerms.isSymlink) {
findings.push({
checkId: "fs.config.symlink",
severity: "warn",
@@ -171,29 +199,47 @@ async function collectFilesystemFindings(params: {
detail: `${params.configPath} is a symlink; make sure you trust its target.`,
});
}
if (isWorldWritable(bits) || isGroupWritable(bits)) {
if (configPerms.worldWritable || configPerms.groupWritable) {
findings.push({
checkId: "fs.config.perms_writable",
severity: "critical",
title: "Config file is writable by others",
detail: `${params.configPath} mode=${formatOctal(bits)}; another user could change gateway/auth/tool policies.`,
remediation: `chmod 600 ${params.configPath}`,
detail: `${formatPermissionDetail(params.configPath, configPerms)}; another user could change gateway/auth/tool policies.`,
remediation: formatPermissionRemediation({
targetPath: params.configPath,
perms: configPerms,
isDir: false,
posixMode: 0o600,
env: params.env,
}),
});
} else if (isWorldReadable(bits)) {
} else if (configPerms.worldReadable) {
findings.push({
checkId: "fs.config.perms_world_readable",
severity: "critical",
title: "Config file is world-readable",
detail: `${params.configPath} mode=${formatOctal(bits)}; config can contain tokens and private settings.`,
remediation: `chmod 600 ${params.configPath}`,
detail: `${formatPermissionDetail(params.configPath, configPerms)}; config can contain tokens and private settings.`,
remediation: formatPermissionRemediation({
targetPath: params.configPath,
perms: configPerms,
isDir: false,
posixMode: 0o600,
env: params.env,
}),
});
} else if (isGroupReadable(bits)) {
} else if (configPerms.groupReadable) {
findings.push({
checkId: "fs.config.perms_group_readable",
severity: "warn",
title: "Config file is group-readable",
detail: `${params.configPath} mode=${formatOctal(bits)}; config can contain tokens and private settings.`,
remediation: `chmod 600 ${params.configPath}`,
detail: `${formatPermissionDetail(params.configPath, configPerms)}; config can contain tokens and private settings.`,
remediation: formatPermissionRemediation({
targetPath: params.configPath,
perms: configPerms,
isDir: false,
posixMode: 0o600,
env: params.env,
}),
});
}
}
@@ -850,7 +896,9 @@ async function maybeProbeGateway(params: {
export async function runSecurityAudit(opts: SecurityAuditOptions): Promise<SecurityAuditReport> {
const findings: SecurityAuditFinding[] = [];
const cfg = opts.config;
const env = process.env;
const env = opts.env ?? process.env;
const platform = opts.platform ?? process.platform;
const execIcacls = opts.execIcacls;
const stateDir = opts.stateDir ?? resolveStateDir(env);
const configPath = opts.configPath ?? resolveConfigPath(env, stateDir);
@@ -873,11 +921,23 @@ export async function runSecurityAudit(opts: SecurityAuditOptions): Promise<Secu
: null;
if (opts.includeFilesystem !== false) {
findings.push(...(await collectFilesystemFindings({ stateDir, configPath })));
findings.push(
...(await collectFilesystemFindings({
stateDir,
configPath,
env,
platform,
execIcacls,
})),
);
if (configSnapshot) {
findings.push(...(await collectIncludeFilePermFindings({ configSnapshot })));
findings.push(
...(await collectIncludeFilePermFindings({ configSnapshot, env, platform, execIcacls })),
);
}
findings.push(...(await collectStateDeepFilesystemFindings({ cfg, env, stateDir })));
findings.push(
...(await collectStateDeepFilesystemFindings({ cfg, env, stateDir, platform, execIcacls })),
);
findings.push(...(await collectPluginsTrustFindings({ cfg, stateDir })));
}