Skip to content

Security audit + SEC-001 fix: shared agent loader with validation #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roybotbot wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Security audit + SEC-001 fix: shared agent loader with validation #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
73edde7
docs: add security audit with 15 identified issues
roybotbot Feb 24, 2026
ee681c7
fix(SEC-001): shared agent loader with validation
roybotbot Feb 24, 2026
c0836eb
docs: update README with agent validation, tests, and security audit …
roybotbot Feb 24, 2026
ddac702
feat: recursive agent directory scanning with collision warnings
roybotbot Feb 25, 2026
81e904c
feat: add pushover notification extension
roybotbot Mar 3, 2026
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
22 changes: 22 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ just open purpose-gate minimal tool-counter-widget
```
pi-vs-cc/
├── extensions/ # Pi extension source files (.ts) — one file per extension
│ └── utils/ # Shared utilities used by multiple extensions
│ └── agent-loader.ts # Validated agent .md file loader (SEC-001)
├── tests/ # Test suite
│ └── agent-loader.test.ts # 42 tests for agent validation logic
├── specs/ # Feature specifications for extensions
├── .pi/
│ ├── agent-sessions/ # Ephemeral session files (gitignored)
Expand All @@ -165,6 +169,7 @@ pi-vs-cc/
│ └── settings.json # Pi workspace settings
├── justfile # just task definitions
├── CLAUDE.md # Conventions and tooling reference (for agents)
├── SECURITY_AUDIT.md # Security audit — 15 issues tracked with fix plans
├── THEME.md # Color token conventions for extension authors
└── TOOLS.md # Built-in tool function signatures available in extensions
```
Expand Down Expand Up @@ -203,6 +208,22 @@ The `damage-control` extension provides real-time security hooks to prevent cata
- **Read-Only Paths**: Allows reading but blocks modifying system files or lockfiles (`package-lock.json`, `/etc/`).
- **No-Delete Paths**: Allows modifying but prevents deleting critical project configuration (`.git/`, `Dockerfile`, `README.md`).

### Agent Definition Validation

Extensions that spawn subprocesses (`agent-team`, `agent-chain`, `pi-pi`) pass agent `.md` file contents as CLI arguments. To prevent injection via malicious agent definitions, all three use a shared loader (`extensions/utils/agent-loader.ts`) that validates every agent file at load time:

- **Name**: Must be alphanumeric with dashes/underscores/dots, max 64 characters. Names with shell metacharacters (`;`, `$`, `` ` ``, `|`, `&`) are rejected.
- **Tools**: Checked against a known allowlist. Unknown tools produce a warning.
- **System prompt**: Scanned for shell injection patterns (`$(…)`, pipe to shell, chained destructive commands, null bytes, `eval()`). Capped at 50,000 characters.

Agents with error-severity issues are rejected and won't load. Suspicious content produces warnings but still allows loading. See [SECURITY_AUDIT.md](SECURITY_AUDIT.md) for the full audit.

### Running Tests

```bash
npx tsx --test tests/agent-loader.test.ts
```

---

## Extension Author Reference
Expand All @@ -213,6 +234,7 @@ Companion docs cover the conventions used across all extensions in this repo:
- **[RESERVED_KEYS.md](RESERVED_KEYS.md)** — Pi reserved keybindings, overridable keys, and safe keys for extension authors.
- **[THEME.md](THEME.md)** — Color language: which Pi theme tokens (`success`, `accent`, `warning`, `dim`, `muted`) map to which UI roles, with examples.
- **[TOOLS.md](TOOLS.md)** — Function signatures for the built-in tools available inside extensions (`read`, `bash`, `edit`, `write`).
- **[SECURITY_AUDIT.md](SECURITY_AUDIT.md)** — Full security audit: 15 issues identified with severity ratings, fix plans, and behavior impact analysis.

---

Expand Down
628 changes: 628 additions & 0 deletions SECURITY_AUDIT.md
View file
Open in desktop

Large diffs are not rendered by default.

89 changes: 27 additions & 62 deletions extensions/agent-chain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { spawn } from "child_process";
import { readFileSync, existsSync, readdirSync, mkdirSync, unlinkSync } from "fs";
import { join, resolve } from "path";
import { applyExtensionDefaults } from "./themeMap.ts";
import { scanAgentDirectory, type AgentDef as LoaderAgentDef, type ValidationWarning, type CollisionWarning } from "./utils/agent-loader.ts";

// ── Types ────────────────────────────────────────

Expand All @@ -42,12 +43,7 @@ interface ChainDef {
steps: ChainStep[];
}

interface AgentDef {
name: string;
description: string;
tools: string;
systemPrompt: string;
}
type AgentDef = LoaderAgentDef;

interface StepState {
agent: string;
Expand Down Expand Up @@ -130,61 +126,6 @@ function parseChainYaml(raw: string): ChainDef[] {
return chains;
}

// ── Frontmatter Parser ───────────────────────────

function parseAgentFile(filePath: string): AgentDef | null {
try {
const raw = readFileSync(filePath, "utf-8");
const match = raw.match(/^---\n([\s\S]*?)\n---\n([\s\S]*)$/);
if (!match) return null;

const frontmatter: Record<string, string> = {};
for (const line of match[1].split("\n")) {
const idx = line.indexOf(":");
if (idx > 0) {
frontmatter[line.slice(0, idx).trim()] = line.slice(idx + 1).trim();
}
}

if (!frontmatter.name) return null;

return {
name: frontmatter.name,
description: frontmatter.description || "",
tools: frontmatter.tools || "read,grep,find,ls",
systemPrompt: match[2].trim(),
};
} catch {
return null;
}
}

function scanAgentDirs(cwd: string): Map<string, AgentDef> {
const dirs = [
join(cwd, "agents"),
join(cwd, ".claude", "agents"),
join(cwd, ".pi", "agents"),
];

const agents = new Map<string, AgentDef>();

for (const dir of dirs) {
if (!existsSync(dir)) continue;
try {
for (const file of readdirSync(dir)) {
if (!file.endsWith(".md")) continue;
const fullPath = resolve(dir, file);
const def = parseAgentFile(fullPath);
if (def && !agents.has(def.name.toLowerCase())) {
agents.set(def.name.toLowerCase(), def);
}
}
} catch {}
}

return agents;
}

// ── Extension ────────────────────────────────────

export default function (pi: ExtensionAPI) {
Expand All @@ -198,14 +139,34 @@ export default function (pi: ExtensionAPI) {
// Per-step state for the active chain
let stepStates: StepState[] = [];
let pendingReset = false;
let lastCollisions: CollisionWarning[] = [];

function loadChains(cwd: string) {
sessionDir = join(cwd, ".pi", "agent-sessions");
if (!existsSync(sessionDir)) {
mkdirSync(sessionDir, { recursive: true });
}

allAgents = scanAgentDirs(cwd);
allAgents = new Map<string, AgentDef>();
lastCollisions = [];
const agentDirs = [
join(cwd, "agents"),
join(cwd, ".claude", "agents"),
join(cwd, ".pi", "agents"),
];
for (const dir of agentDirs) {
const { agents, collisions } = scanAgentDirectory(dir, (_file, warning) => {
if (warning.severity === "error") {
console.error(`[agent-chain] ${_file}: ${warning.message}`);
}
});
lastCollisions.push(...collisions);
for (const [key, def] of agents) {
if (!allAgents.has(key)) {
allAgents.set(key, def);
}
}
}

agentSessions.clear();
for (const [key] of allAgents) {
Expand Down Expand Up @@ -750,6 +711,10 @@ ${agentCatalog}
// Reload chains + clear agentSessions map (all agents start fresh)
loadChains(_ctx.cwd);

for (const c of lastCollisions) {
_ctx.ui.notify(`⚠️ Agent collision: "${c.name}" in ${c.duplicatePath} — already loaded from ${c.originalPath}`, "warning");
}

if (chains.length === 0) {
_ctx.ui.notify("No chains found in .pi/agents/agent-chain.yaml", "warning");
return;
Expand Down
97 changes: 32 additions & 65 deletions extensions/agent-team.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,11 @@ import { spawn } from "child_process";
import { readdirSync, readFileSync, existsSync, mkdirSync, unlinkSync } from "fs";
import { join, resolve } from "path";
import { applyExtensionDefaults } from "./themeMap.ts";
import { scanAgentDirectory, type AgentDef as LoaderAgentDef, type ValidationWarning, type CollisionWarning } from "./utils/agent-loader.ts";

// ── Types ────────────────────────────────────────

interface AgentDef {
name: string;
description: string;
tools: string;
systemPrompt: string;
file: string;
}
type AgentDef = LoaderAgentDef;

interface AgentState {
def: AgentDef;
Expand Down Expand Up @@ -74,63 +69,7 @@ function parseTeamsYaml(raw: string): Record<string, string[]> {
return teams;
}

// ── Frontmatter Parser ───────────────────────────

function parseAgentFile(filePath: string): AgentDef | null {
try {
const raw = readFileSync(filePath, "utf-8");
const match = raw.match(/^---\n([\s\S]*?)\n---\n([\s\S]*)$/);
if (!match) return null;

const frontmatter: Record<string, string> = {};
for (const line of match[1].split("\n")) {
const idx = line.indexOf(":");
if (idx > 0) {
frontmatter[line.slice(0, idx).trim()] = line.slice(idx + 1).trim();
}
}

if (!frontmatter.name) return null;

return {
name: frontmatter.name,
description: frontmatter.description || "",
tools: frontmatter.tools || "read,grep,find,ls",
systemPrompt: match[2].trim(),
file: filePath,
};
} catch {
return null;
}
}

function scanAgentDirs(cwd: string): AgentDef[] {
const dirs = [
join(cwd, "agents"),
join(cwd, ".claude", "agents"),
join(cwd, ".pi", "agents"),
];

const agents: AgentDef[] = [];
const seen = new Set<string>();

for (const dir of dirs) {
if (!existsSync(dir)) continue;
try {
for (const file of readdirSync(dir)) {
if (!file.endsWith(".md")) continue;
const fullPath = resolve(dir, file);
const def = parseAgentFile(fullPath);
if (def && !seen.has(def.name.toLowerCase())) {
seen.add(def.name.toLowerCase());
agents.push(def);
}
}
} catch {}
}

return agents;
}

// ── Extension ────────────────────────────────────

Expand All @@ -143,6 +82,7 @@ export default function (pi: ExtensionAPI) {
let widgetCtx: any;
let sessionDir = "";
let contextWindow = 0;
let lastCollisions: CollisionWarning[] = [];

function loadAgents(cwd: string) {
// Create session storage dir
Expand All @@ -151,8 +91,31 @@ export default function (pi: ExtensionAPI) {
mkdirSync(sessionDir, { recursive: true });
}

// Load all agent definitions
allAgentDefs = scanAgentDirs(cwd);
// Load all agent definitions (recursive walk with collision detection)
const agentDirs = [
join(cwd, "agents"),
join(cwd, ".claude", "agents"),
join(cwd, ".pi", "agents"),
];

const seen = new Set<string>();
allAgentDefs = [];
lastCollisions = [];

for (const dir of agentDirs) {
const { agents, collisions } = scanAgentDirectory(dir, (_file, warning) => {
if (warning.severity === "error") {
console.error(`[agent-team] ${_file}: ${warning.message}`);
}
});
lastCollisions.push(...collisions);
for (const [key, def] of agents) {
if (!seen.has(key)) {
seen.add(key);
allAgentDefs.push(def);
}
}
}

// Load teams from .pi/agents/teams.yaml
const teamsPath = join(cwd, ".pi", "agents", "teams.yaml");
Expand Down Expand Up @@ -689,6 +652,10 @@ ${agentCatalog}`,

loadAgents(_ctx.cwd);

for (const c of lastCollisions) {
_ctx.ui.notify(`⚠️ Agent collision: "${c.name}" in ${c.duplicatePath} — already loaded from ${c.originalPath}`, "warning");
}

// Default to first team — use /agents-team to switch
const teamNames = Object.keys(teams);
if (teamNames.length > 0) {
Expand Down
Loading