test: speed up security audit tests

This commit is contained in:
Peter Steinberger
2026-04-26 02:51:13 +01:00
parent ec56dd3116
commit 1bc9bada65
6 changed files with 211 additions and 211 deletions

View File

@@ -39,48 +39,6 @@ type ExecDockerRawFn = (
) => Promise<import("../agents/sandbox/docker.js").ExecDockerRawResult>;
type CodeSafetySummaryCache = Map<string, Promise<unknown>>;
type WorkspaceSkillScanLimits = {
maxFiles?: number;
maxDirVisits?: number;
};
const MAX_WORKSPACE_SKILL_SCAN_FILES_PER_WORKSPACE = 2_000;
const MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS = 12;
/**
* Resolves the realpath of `p` with a 2 s timeout.
*
* Returns the realpath string on success, or `null` if realpath fails or the
* timeout fires first. Note: fs.realpath cannot be cancelled once submitted to
* libuv — the underlying OS call continues running in the background after the
* timeout resolves. Callers make sequential (not concurrent) calls so at most
* one libuv thread is occupied at a time; the OS will eventually time out the
* stuck NFS/SMB call independently.
*
* Timer cleanup: when realpath resolves before the deadline the timer is
* cleared immediately so it does not linger across the rest of the audit run.
* The timer is also unref'd so it cannot prevent process exit even if it fires
* late (e.g. the process finishes while a hang is still in-flight).
*/
function realpathWithTimeout(p: string, timeoutMs = 2000): Promise<string | null> {
let timerHandle: ReturnType<typeof setTimeout> | undefined;
const realpathPromise = fs
.realpath(p)
.catch(() => null)
.then((result) => {
clearTimeout(timerHandle);
return result;
});
const timeoutPromise = new Promise<null>((resolve) => {
timerHandle = setTimeout(() => resolve(null), timeoutMs);
// Prevent the timer from keeping the process alive while waiting on a
// potentially hanging NFS/SMB path during a large audit run.
timerHandle.unref?.();
});
return Promise.race([realpathPromise, timeoutPromise]);
}
let skillsModulePromise: Promise<typeof import("../agents/skills.js")> | undefined;
let configModulePromise: Promise<typeof import("../config/config.js")> | undefined;
let agentScopeModulePromise: Promise<typeof import("../agents/agent-scope.js")> | undefined;
@@ -302,66 +260,6 @@ async function getCodeSafetySummary(params: {
});
}
async function listWorkspaceSkillMarkdownFiles(
workspaceDir: string,
limits: WorkspaceSkillScanLimits = {},
): Promise<{ skillFilePaths: string[]; truncated: boolean }> {
const skillsRoot = path.join(workspaceDir, "skills");
const rootStat = await safeStat(skillsRoot);
if (!rootStat.ok || !rootStat.isDir) {
return { skillFilePaths: [], truncated: false };
}
const maxFiles = limits.maxFiles ?? MAX_WORKSPACE_SKILL_SCAN_FILES_PER_WORKSPACE;
const maxTotalDirVisits = limits.maxDirVisits ?? maxFiles * 20;
const skillFiles: string[] = [];
const queue: string[] = [skillsRoot];
const visitedDirs = new Set<string>();
let totalDirVisits = 0;
while (queue.length > 0 && skillFiles.length < maxFiles && totalDirVisits++ < maxTotalDirVisits) {
const dir = queue.shift()!;
// Use the module-level realpathWithTimeout so a hanging network FS doesn't
// block the BFS indefinitely (same 2 s guard as the outer escape-detection loop).
const dirRealPath = (await realpathWithTimeout(dir)) ?? path.resolve(dir);
if (visitedDirs.has(dirRealPath)) {
continue;
}
visitedDirs.add(dirRealPath);
const entries = await fs.readdir(dir, { withFileTypes: true }).catch(() => []);
for (const entry of entries) {
if (entry.name.startsWith(".") || entry.name === "node_modules") {
continue;
}
const fullPath = path.join(dir, entry.name);
if (entry.isDirectory()) {
queue.push(fullPath);
continue;
}
if (entry.isSymbolicLink()) {
const stat = await fs.stat(fullPath).catch(() => null);
if (!stat) {
continue;
}
if (stat.isDirectory()) {
queue.push(fullPath);
continue;
}
if (stat.isFile() && entry.name === "SKILL.md") {
skillFiles.push(fullPath);
}
continue;
}
if (entry.isFile() && entry.name === "SKILL.md") {
skillFiles.push(fullPath);
}
}
}
return { skillFilePaths: skillFiles, truncated: queue.length > 0 };
}
// --------------------------------------------------------------------------
// Exported collectors
// --------------------------------------------------------------------------
@@ -552,110 +450,6 @@ export async function collectSandboxBrowserHashLabelFindings(params?: {
return findings;
}
export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
cfg: OpenClawConfig;
skillScanLimits?: WorkspaceSkillScanLimits;
}): Promise<SecurityAuditFinding[]> {
const findings: SecurityAuditFinding[] = [];
const { listAgentWorkspaceDirs } = await loadAgentWorkspaceDirsModule();
const workspaceDirs = listAgentWorkspaceDirs(params.cfg);
if (workspaceDirs.length === 0) {
return findings;
}
const escapedSkillFiles: Array<{
workspaceDir: string;
skillFilePath: string;
skillRealPath: string;
}> = [];
const seenSkillPaths = new Set<string>();
for (const workspaceDir of workspaceDirs) {
const workspacePath = path.resolve(workspaceDir);
const workspaceRealPath = (await realpathWithTimeout(workspacePath)) ?? workspacePath;
const { skillFilePaths, truncated } = await listWorkspaceSkillMarkdownFiles(
workspacePath,
params.skillScanLimits,
);
if (truncated) {
// The BFS visit cap was hit before the full skills/ tree was scanned.
// Escaped SKILL.md symlinks in the unvisited portion will not be detected.
// Surface this as a warning so the user knows coverage was incomplete.
findings.push({
checkId: "skills.workspace.scan_truncated",
severity: "warn",
title: "Workspace skill scan reached the directory visit limit",
detail:
`The skills/ directory scan in ${workspacePath} stopped early after reaching the ` +
`BFS visit cap. Skill files in the unscanned portion of the tree were not checked ` +
"for symlink escapes.",
remediation:
"Flatten or simplify the skills/ directory hierarchy to stay within the scan budget, " +
"or move deeply-nested skill collections to a managed skill location.",
});
}
for (const skillFilePath of skillFilePaths) {
const canonicalSkillPath = path.resolve(skillFilePath);
if (seenSkillPaths.has(canonicalSkillPath)) {
continue;
}
seenSkillPaths.add(canonicalSkillPath);
const skillRealPath = await realpathWithTimeout(canonicalSkillPath);
if (!skillRealPath) {
// realpath timed out or failed — cannot verify the symlink target.
// Treat as a potential escape rather than silently bypassing the check.
// An attacker on a slow/network FS could otherwise hang realpath to
// prevent escape detection.
escapedSkillFiles.push({
workspaceDir: workspacePath,
skillFilePath: canonicalSkillPath,
skillRealPath: "(realpath timed out \u2014 symlink target unverifiable)",
});
continue;
}
if (isPathInside(workspaceRealPath, skillRealPath)) {
continue;
}
escapedSkillFiles.push({
workspaceDir: workspacePath,
skillFilePath: canonicalSkillPath,
skillRealPath,
});
}
}
if (escapedSkillFiles.length === 0) {
return findings;
}
findings.push({
checkId: "skills.workspace.symlink_escape",
severity: "warn",
title: "Workspace skill files resolve outside the workspace root",
detail:
"Detected workspace `skills/**/SKILL.md` paths whose realpath escapes their workspace root:\n" +
escapedSkillFiles
.slice(0, MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS)
.map(
(entry) =>
`- workspace=${entry.workspaceDir}\n` +
` skill=${entry.skillFilePath}\n` +
` realpath=${entry.skillRealPath}`,
)
.join("\n") +
(escapedSkillFiles.length > MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS
? `\n- +${escapedSkillFiles.length - MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS} more`
: ""),
remediation:
"Keep workspace skills inside the workspace root (replace symlinked escapes with real in-workspace files), or move trusted shared skills to managed/bundled skill locations.",
});
return findings;
}
export async function collectIncludeFilePermFindings(params: {
configSnapshot: ConfigFileSnapshot;
env?: NodeJS.ProcessEnv;

View File

@@ -4,7 +4,6 @@ import path from "node:path";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../config/config.js";
import type { PluginInstallRecord } from "../config/types.plugins.js";
import { writePersistedInstalledPluginIndex } from "../plugins/installed-plugin-index-store.js";
import type { InstalledPluginIndex } from "../plugins/installed-plugin-index.js";
import { createPathResolutionEnv, withEnvAsync } from "../test-utils/env.js";
import { collectPluginsTrustFindings } from "./audit-plugins-trust.js";
@@ -132,7 +131,9 @@ describe("security audit install metadata findings", () => {
})),
diagnostics: [],
};
await writePersistedInstalledPluginIndex(index, { stateDir });
const filePath = path.join(stateDir, "plugins", "installs.json");
await fs.mkdir(path.dirname(filePath), { recursive: true, mode: 0o700 });
await fs.writeFile(filePath, `${JSON.stringify(index, null, 2)}\n`, { mode: 0o600 });
};
beforeAll(async () => {

View File

@@ -7,7 +7,7 @@ import type { OpenClawConfig } from "../config/config.js";
import type { AgentToolsConfig } from "../config/types.tools.js";
import { readInstalledPackageVersion } from "../infra/package-update-utils.js";
import { normalizePluginId, normalizePluginsConfig } from "../plugins/config-state.js";
import { loadInstalledPluginIndexInstallRecords } from "../plugins/installed-plugin-index-records.js";
import { loadInstalledPluginIndexInstallRecords } from "../plugins/installed-plugin-index-record-reader.js";
import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js";
import type { SecurityAuditFinding } from "./audit.types.js";

View File

@@ -2,7 +2,7 @@ import fs from "node:fs/promises";
import path from "node:path";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../config/config.js";
import { collectWorkspaceSkillSymlinkEscapeFindings } from "./audit-extra.async.js";
import { collectWorkspaceSkillSymlinkEscapeFindings } from "./audit-workspace-skills.js";
import { AsyncTempCaseFactory } from "./test-temp-cases.js";
const isWindows = process.platform === "win32";

View File

@@ -0,0 +1,205 @@
import fs from "node:fs/promises";
import path from "node:path";
import { listAgentWorkspaceDirs } from "../agents/workspace-dirs.js";
import type { OpenClawConfig } from "../config/config.js";
import type { SecurityAuditFinding } from "./audit.types.js";
import { isPathInside } from "./scan-paths.js";
type WorkspaceSkillScanLimits = {
maxFiles?: number;
maxDirVisits?: number;
};
const MAX_WORKSPACE_SKILL_SCAN_FILES_PER_WORKSPACE = 2_000;
const MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS = 12;
async function safeStat(targetPath: string): Promise<{
ok: boolean;
isDir: boolean;
}> {
try {
const lst = await fs.lstat(targetPath);
return {
ok: true,
isDir: lst.isDirectory(),
};
} catch {
return {
ok: false,
isDir: false,
};
}
}
function realpathWithTimeout(p: string, timeoutMs = 2000): Promise<string | null> {
let timerHandle: ReturnType<typeof setTimeout> | undefined;
const realpathPromise = fs
.realpath(p)
.catch(() => null)
.then((result) => {
clearTimeout(timerHandle);
return result;
});
const timeoutPromise = new Promise<null>((resolve) => {
timerHandle = setTimeout(() => resolve(null), timeoutMs);
timerHandle.unref?.();
});
return Promise.race([realpathPromise, timeoutPromise]);
}
async function listWorkspaceSkillMarkdownFiles(
workspaceDir: string,
limits: WorkspaceSkillScanLimits = {},
): Promise<{ skillFilePaths: string[]; truncated: boolean }> {
const skillsRoot = path.join(workspaceDir, "skills");
const rootStat = await safeStat(skillsRoot);
if (!rootStat.ok || !rootStat.isDir) {
return { skillFilePaths: [], truncated: false };
}
const maxFiles = limits.maxFiles ?? MAX_WORKSPACE_SKILL_SCAN_FILES_PER_WORKSPACE;
const maxTotalDirVisits = limits.maxDirVisits ?? maxFiles * 20;
const skillFiles: string[] = [];
const queue: string[] = [skillsRoot];
const visitedDirs = new Set<string>();
let totalDirVisits = 0;
while (queue.length > 0 && skillFiles.length < maxFiles && totalDirVisits++ < maxTotalDirVisits) {
const dir = queue.shift()!;
const dirRealPath = (await realpathWithTimeout(dir)) ?? path.resolve(dir);
if (visitedDirs.has(dirRealPath)) {
continue;
}
visitedDirs.add(dirRealPath);
const entries = await fs.readdir(dir, { withFileTypes: true }).catch(() => []);
for (const entry of entries) {
if (entry.name.startsWith(".") || entry.name === "node_modules") {
continue;
}
const fullPath = path.join(dir, entry.name);
if (entry.isDirectory()) {
queue.push(fullPath);
continue;
}
if (entry.isSymbolicLink()) {
const stat = await fs.stat(fullPath).catch(() => null);
if (!stat) {
continue;
}
if (stat.isDirectory()) {
queue.push(fullPath);
continue;
}
if (stat.isFile() && entry.name === "SKILL.md") {
skillFiles.push(fullPath);
}
continue;
}
if (entry.isFile() && entry.name === "SKILL.md") {
skillFiles.push(fullPath);
}
}
}
return { skillFilePaths: skillFiles, truncated: queue.length > 0 };
}
export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
cfg: OpenClawConfig;
skillScanLimits?: WorkspaceSkillScanLimits;
}): Promise<SecurityAuditFinding[]> {
const findings: SecurityAuditFinding[] = [];
const workspaceDirs = listAgentWorkspaceDirs(params.cfg);
if (workspaceDirs.length === 0) {
return findings;
}
const escapedSkillFiles: Array<{
workspaceDir: string;
skillFilePath: string;
skillRealPath: string;
}> = [];
const seenSkillPaths = new Set<string>();
for (const workspaceDir of workspaceDirs) {
const workspacePath = path.resolve(workspaceDir);
const workspaceRealPath = (await realpathWithTimeout(workspacePath)) ?? workspacePath;
const { skillFilePaths, truncated } = await listWorkspaceSkillMarkdownFiles(
workspacePath,
params.skillScanLimits,
);
if (truncated) {
findings.push({
checkId: "skills.workspace.scan_truncated",
severity: "warn",
title: "Workspace skill scan reached the directory visit limit",
detail:
`The skills/ directory scan in ${workspacePath} stopped early after reaching the ` +
`BFS visit cap. Skill files in the unscanned portion of the tree were not checked ` +
"for symlink escapes.",
remediation:
"Flatten or simplify the skills/ directory hierarchy to stay within the scan budget, " +
"or move deeply-nested skill collections to a managed skill location.",
});
}
for (const skillFilePath of skillFilePaths) {
const canonicalSkillPath = path.resolve(skillFilePath);
if (seenSkillPaths.has(canonicalSkillPath)) {
continue;
}
seenSkillPaths.add(canonicalSkillPath);
const skillRealPath = await realpathWithTimeout(canonicalSkillPath);
if (!skillRealPath) {
escapedSkillFiles.push({
workspaceDir: workspacePath,
skillFilePath: canonicalSkillPath,
skillRealPath: "(realpath timed out - symlink target unverifiable)",
});
continue;
}
if (isPathInside(workspaceRealPath, skillRealPath)) {
continue;
}
escapedSkillFiles.push({
workspaceDir: workspacePath,
skillFilePath: canonicalSkillPath,
skillRealPath,
});
}
}
if (escapedSkillFiles.length === 0) {
return findings;
}
findings.push({
checkId: "skills.workspace.symlink_escape",
severity: "warn",
title: "Workspace skill files resolve outside the workspace root",
detail:
"Detected workspace `skills/**/SKILL.md` paths whose realpath escapes their workspace root:\n" +
escapedSkillFiles
.slice(0, MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS)
.map(
(entry) =>
`- workspace=${entry.workspaceDir}\n` +
` skill=${entry.skillFilePath}\n` +
` realpath=${entry.skillRealPath}`,
)
.join("\n") +
(escapedSkillFiles.length > MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS
? `\n- +${escapedSkillFiles.length - MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS} more`
: ""),
remediation:
"Keep workspace skills inside the workspace root (replace symlinked escapes with real in-workspace files), or move trusted shared skills to managed/bundled skill locations.",
});
return findings;
}

View File

@@ -23,7 +23,7 @@ export {
collectSandboxBrowserHashLabelFindings,
collectIncludeFilePermFindings,
collectStateDeepFilesystemFindings,
collectWorkspaceSkillSymlinkEscapeFindings,
readConfigSnapshotForAudit,
} from "./audit-extra.async.js";
export { collectWorkspaceSkillSymlinkEscapeFindings } from "./audit-workspace-skills.js";
export { collectPluginsTrustFindings } from "./audit-plugins-trust.js";