mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-04-29 10:26:23 +02:00
Add security allowlist for command execution and update copilot instructions
- Add security.ts with allowlist configuration for shell commands - Update command-executor.ts to enforce security policy (exit code 126 for blocked commands) - Update copilot instructions to clarify builtin tools vs shell commands - Document that builtin tools (deleteFile, createFile, etc.) bypass security filtering - Only executeCommand (shell commands) requires security.json allowlist entries
This commit is contained in:
parent
570543e1c7
commit
28488d5fd1
4 changed files with 183 additions and 1 deletions
|
|
@ -26,4 +26,19 @@ Always consult this catalog first so you load the right skills before taking act
|
||||||
- Explore existing files and structure before creating new assets.
|
- Explore existing files and structure before creating new assets.
|
||||||
- Use relative paths (no \${BASE_DIR} prefixes) when running commands or referencing files.
|
- Use relative paths (no \${BASE_DIR} prefixes) when running commands or referencing files.
|
||||||
- Keep user data safe—double-check before editing or deleting important resources.
|
- Keep user data safe—double-check before editing or deleting important resources.
|
||||||
|
|
||||||
|
## Builtin Tools vs Shell Commands
|
||||||
|
|
||||||
|
**IMPORTANT**: Rowboat provides builtin tools that are internal and do NOT require security allowlist entries:
|
||||||
|
- \`deleteFile\`, \`createFile\`, \`updateFile\`, \`readFile\` - File operations
|
||||||
|
- \`listFiles\`, \`exploreDirectory\` - Directory exploration
|
||||||
|
- \`analyzeAgent\` - Agent analysis
|
||||||
|
- \`listMcpServers\`, \`listMcpTools\` - MCP server management
|
||||||
|
- \`loadSkill\` - Skill loading
|
||||||
|
|
||||||
|
These tools work directly and are NOT filtered by \`.rowboat/config/security.json\`.
|
||||||
|
|
||||||
|
**Only \`executeCommand\` (shell/bash commands) is filtered** by the security allowlist. If you need to delete a file, use the \`deleteFile\` builtin tool, not \`executeCommand\` with \`rm\`. If you need to create a file, use \`createFile\`, not \`executeCommand\` with \`touch\` or \`echo >\`.
|
||||||
|
|
||||||
|
The security allowlist in \`security.json\` only applies to shell commands executed via \`executeCommand\`, not to Rowboat's internal builtin tools.
|
||||||
`;
|
`;
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,8 @@ Agents can use builtin tools by declaring them in the \`"tools"\` object with \`
|
||||||
### executeCommand
|
### executeCommand
|
||||||
**The most powerful and versatile builtin tool** - Execute any bash/shell command and get the output.
|
**The most powerful and versatile builtin tool** - Execute any bash/shell command and get the output.
|
||||||
|
|
||||||
|
**Security note:** Commands are filtered through \`.rowboat/config/security.json\`. Populate this file with allowed command names (array or dictionary entries). Any command not present is blocked and returns exit code 126 so the agent knows it violated the policy.
|
||||||
|
|
||||||
**Agent tool declaration:**
|
**Agent tool declaration:**
|
||||||
\`\`\`json
|
\`\`\`json
|
||||||
"tools": {
|
"tools": {
|
||||||
|
|
@ -176,4 +178,3 @@ There are no separate "workflow" files - everything is an agent!
|
||||||
`;
|
`;
|
||||||
|
|
||||||
export default skill;
|
export default skill;
|
||||||
|
|
||||||
|
|
|
||||||
90
apps/cli/src/application/config/security.ts
Normal file
90
apps/cli/src/application/config/security.ts
Normal file
|
|
@ -0,0 +1,90 @@
|
||||||
|
import path from "path";
|
||||||
|
import fs from "fs";
|
||||||
|
import { WorkDir } from "./config.js";
|
||||||
|
|
||||||
|
export const SECURITY_CONFIG_PATH = path.join(WorkDir, "config", "security.json");
|
||||||
|
|
||||||
|
const DEFAULT_ALLOW_LIST = ["ls", "pwd", "cat", "echo", "whoami"];
|
||||||
|
|
||||||
|
let cachedAllowList: string[] | null = null;
|
||||||
|
let cachedMtimeMs: number | null = null;
|
||||||
|
|
||||||
|
function ensureSecurityConfig() {
|
||||||
|
if (!fs.existsSync(SECURITY_CONFIG_PATH)) {
|
||||||
|
fs.writeFileSync(
|
||||||
|
SECURITY_CONFIG_PATH,
|
||||||
|
JSON.stringify(DEFAULT_ALLOW_LIST, null, 2) + "\n",
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeList(commands: unknown[]): string[] {
|
||||||
|
const seen = new Set<string>();
|
||||||
|
for (const entry of commands) {
|
||||||
|
if (typeof entry !== "string") continue;
|
||||||
|
const normalized = entry.trim().toLowerCase();
|
||||||
|
if (!normalized) continue;
|
||||||
|
seen.add(normalized);
|
||||||
|
}
|
||||||
|
|
||||||
|
return Array.from(seen);
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseSecurityPayload(payload: unknown): string[] {
|
||||||
|
if (Array.isArray(payload)) {
|
||||||
|
return normalizeList(payload);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (payload && typeof payload === "object") {
|
||||||
|
const maybeObject = payload as Record<string, unknown>;
|
||||||
|
if (Array.isArray(maybeObject.allowedCommands)) {
|
||||||
|
return normalizeList(maybeObject.allowedCommands);
|
||||||
|
}
|
||||||
|
|
||||||
|
const dynamicList = Object.entries(maybeObject)
|
||||||
|
.filter(([, value]) => Boolean(value))
|
||||||
|
.map(([key]) => key);
|
||||||
|
|
||||||
|
return normalizeList(dynamicList);
|
||||||
|
}
|
||||||
|
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
|
function readAllowList(): string[] {
|
||||||
|
ensureSecurityConfig();
|
||||||
|
|
||||||
|
try {
|
||||||
|
const configContent = fs.readFileSync(SECURITY_CONFIG_PATH, "utf8");
|
||||||
|
const parsed = JSON.parse(configContent);
|
||||||
|
return parseSecurityPayload(parsed);
|
||||||
|
} catch (error) {
|
||||||
|
console.warn(`Failed to read security config at ${SECURITY_CONFIG_PATH}: ${error instanceof Error ? error.message : error}`);
|
||||||
|
return DEFAULT_ALLOW_LIST;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export function getSecurityAllowList(): string[] {
|
||||||
|
ensureSecurityConfig();
|
||||||
|
try {
|
||||||
|
const stats = fs.statSync(SECURITY_CONFIG_PATH);
|
||||||
|
if (cachedAllowList && cachedMtimeMs === stats.mtimeMs) {
|
||||||
|
return cachedAllowList;
|
||||||
|
}
|
||||||
|
|
||||||
|
const allowList = readAllowList();
|
||||||
|
cachedAllowList = allowList;
|
||||||
|
cachedMtimeMs = stats.mtimeMs;
|
||||||
|
return allowList;
|
||||||
|
} catch {
|
||||||
|
cachedAllowList = null;
|
||||||
|
cachedMtimeMs = null;
|
||||||
|
return readAllowList();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export function resetSecurityAllowListCache() {
|
||||||
|
cachedAllowList = null;
|
||||||
|
cachedMtimeMs = null;
|
||||||
|
}
|
||||||
|
|
@ -1,7 +1,73 @@
|
||||||
import { exec, execSync } from 'child_process';
|
import { exec, execSync } from 'child_process';
|
||||||
import { promisify } from 'util';
|
import { promisify } from 'util';
|
||||||
|
import { getSecurityAllowList, SECURITY_CONFIG_PATH } from '../config/security.js';
|
||||||
|
|
||||||
const execPromise = promisify(exec);
|
const execPromise = promisify(exec);
|
||||||
|
const COMMAND_SPLIT_REGEX = /(?:\|\||&&|;|\||\n)/;
|
||||||
|
const ENV_ASSIGNMENT_REGEX = /^[A-Za-z_][A-Za-z0-9_]*=.*/;
|
||||||
|
const WRAPPER_COMMANDS = new Set(['sudo', 'env', 'time', 'command']);
|
||||||
|
|
||||||
|
function sanitizeToken(token: string): string {
|
||||||
|
return token.trim().replace(/^['"]+|['"]+$/g, '');
|
||||||
|
}
|
||||||
|
|
||||||
|
function extractCommandNames(command: string): string[] {
|
||||||
|
const discovered = new Set<string>();
|
||||||
|
const segments = command.split(COMMAND_SPLIT_REGEX);
|
||||||
|
|
||||||
|
for (const segment of segments) {
|
||||||
|
const tokens = segment.trim().split(/\s+/).filter(Boolean);
|
||||||
|
if (!tokens.length) continue;
|
||||||
|
|
||||||
|
let index = 0;
|
||||||
|
while (index < tokens.length && ENV_ASSIGNMENT_REGEX.test(tokens[index])) {
|
||||||
|
index++;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (index >= tokens.length) continue;
|
||||||
|
|
||||||
|
const primary = sanitizeToken(tokens[index]).toLowerCase();
|
||||||
|
if (!primary) continue;
|
||||||
|
|
||||||
|
discovered.add(primary);
|
||||||
|
|
||||||
|
if (WRAPPER_COMMANDS.has(primary) && index + 1 < tokens.length) {
|
||||||
|
const wrapped = sanitizeToken(tokens[index + 1]).toLowerCase();
|
||||||
|
if (wrapped) {
|
||||||
|
discovered.add(wrapped);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return Array.from(discovered);
|
||||||
|
}
|
||||||
|
|
||||||
|
function findBlockedCommands(command: string): string[] {
|
||||||
|
const invoked = extractCommandNames(command);
|
||||||
|
if (!invoked.length) return [];
|
||||||
|
|
||||||
|
const allowList = getSecurityAllowList();
|
||||||
|
if (!allowList.length) return invoked;
|
||||||
|
|
||||||
|
const allowSet = new Set(allowList);
|
||||||
|
if (allowSet.has('*')) return [];
|
||||||
|
|
||||||
|
return invoked.filter((cmd) => !allowSet.has(cmd));
|
||||||
|
}
|
||||||
|
|
||||||
|
function enforceSecurity(command: string): CommandResult | null {
|
||||||
|
const blocked = findBlockedCommands(command);
|
||||||
|
|
||||||
|
if (!blocked.length) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
stdout: '',
|
||||||
|
stderr: `Command blocked by security policy. Blocked command(s): ${blocked.join(', ')}. Update ${SECURITY_CONFIG_PATH} to allow them before retrying.`,
|
||||||
|
exitCode: 126,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
export interface CommandResult {
|
export interface CommandResult {
|
||||||
stdout: string;
|
stdout: string;
|
||||||
|
|
@ -23,6 +89,11 @@ export async function executeCommand(
|
||||||
maxBuffer?: number; // max buffer size in bytes
|
maxBuffer?: number; // max buffer size in bytes
|
||||||
}
|
}
|
||||||
): Promise<CommandResult> {
|
): Promise<CommandResult> {
|
||||||
|
const securityResult = enforceSecurity(command);
|
||||||
|
if (securityResult) {
|
||||||
|
return securityResult;
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const { stdout, stderr } = await execPromise(command, {
|
const { stdout, stderr } = await execPromise(command, {
|
||||||
cwd: options?.cwd,
|
cwd: options?.cwd,
|
||||||
|
|
@ -57,6 +128,11 @@ export function executeCommandSync(
|
||||||
timeout?: number;
|
timeout?: number;
|
||||||
}
|
}
|
||||||
): CommandResult {
|
): CommandResult {
|
||||||
|
const securityResult = enforceSecurity(command);
|
||||||
|
if (securityResult) {
|
||||||
|
return securityResult;
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const stdout = execSync(command, {
|
const stdout = execSync(command, {
|
||||||
cwd: options?.cwd,
|
cwd: options?.cwd,
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue