Security audit + SEC-001 fix: shared agent loader with validation#3
Open
roybotbot wants to merge 5 commits into
Open
Security audit + SEC-001 fix: shared agent loader with validation#3roybotbot wants to merge 5 commits into
roybotbot wants to merge 5 commits into
Conversation
Comprehensive security analysis of all extensions, agent definitions, configuration files, and damage-control rules. Covers: - 4 critical (subprocess injection, env inheritance, path bypass, ReDoS) - 4 high (env sample, session files, input validation, foreign commands) - 4 medium (swallowed errors, global trust, race conditions, weak heuristics) - 3 low/informational (argv coupling, no sandboxing, parent path ref)
Replace duplicate parseAgentFile/scanAgentDirs across agent-team, agent-chain, and pi-pi with a shared agent-loader.ts that validates: - Agent names (alphanumeric + dashes/underscores/dots, max 64 chars) - Tools (checked against known allowlist) - System prompts (scanned for shell injection patterns, max 50K chars) Invalid agents (error-severity) are rejected at load time. Suspicious content (warning-severity) allows loading but surfaces warnings. Includes 42 tests covering all validation logic. All 18 existing agent .md files load with zero warnings.
ndizazzo
reviewed
Feb 24, 2026
scanAgentDirectory now walks directories recursively using
readdirSync({ recursive: true }), enabling nested organization:
.pi/agents/
├── review_agents/
│ ├── code_reviewer.md
│ └── security_reviewer.md
└── build_agents/
└── ts_builder.md
Collision detection: when duplicate agent names (case-insensitive)
are found, first-wins and a CollisionWarning is returned with both
file paths. All three extension call sites (agent-team, agent-chain,
pi-pi) surface collisions via ui.notify() at session start.
New exports from agent-loader.ts:
- CollisionWarning: { name, duplicatePath, originalPath }
- ScanResult: { agents: Map, collisions: CollisionWarning[] }
7 new tests covering recursive walk, deep nesting, collision
detection, mixed flat+nested, frontmatter-less .md skipping,
and collision path validity. All 49 tests pass.
jetnet
pushed a commit
to jetnet/pi-coms
that referenced
this pull request
May 23, 2026
New extensions/utils/agent-loader.ts validates agent .md files: - Name: alphanumeric + dashes/underscores/dots, max 64 chars - Tools: checked against known allowlist, warns on unknowns - Prompt: scanned for injection patterns ($(…), backtick subst, pipe to shell, null bytes, eval(), chained commands), 50K cap Error-severity issues reject the agent; warnings allow loading. 22 tests in tests/agent-loader.test.ts — all passing. Addresses PR disler#3 (security audit).
jetnet
pushed a commit
to jetnet/pi-coms
that referenced
this pull request
May 23, 2026
agent-team.ts, agent-chain.ts, pi-pi.ts, subagent-widget.ts: - Deduplicate parseAgentFile() → use shared agent-loader (SEC-001, PR disler#3) - Clamp renderCard width to prevent RangeError: Invalid count -1 on narrow terminals (Issue disler#17, PR disler#20) - Use process.execPath + findPiCli() for snap-safe subprocess spawning (PR disler#13) - Forward damage-control.ts extension to subagent processes when present, falling back to --no-extensions (Issue disler#24) - Normalize CRLF line endings in all readFileSync calls (PR disler#1) - Wrap 'No experts found' message with truncateToWidth() (PR disler#20)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security audit of the full codebase identifying 15 issues, plus a fix for the highest-priority one.
Security Audit (SECURITY_AUDIT.md)
Comprehensive analysis covering all extensions, agent definitions, configuration files, and damage-control rules:
Each issue includes a description, fix plan, and behavior change analysis.
SEC-001 Fix: Shared Agent Loader
agent-team.ts,agent-chain.ts, andpi-pi.tseach had duplicateparseAgentFile()functions that loaded agent.mdfiles and passed raw system prompts intospawn()with no validation. A malicious.mdfile could inject content into subprocess arguments.Fix: New shared loader at
extensions/utils/agent-loader.tsthat validates:$(…), pipe to shell, null bytes,eval(), etc.), capped at 50K charsError-severity issues reject the agent at load time. Warning-severity issues allow loading but surface warnings. All 18 existing agent
.mdfiles load with zero warnings.Tests
42 tests covering all validation logic:
Files Changed
extensions/utils/agent-loader.ts— shared validated loadertests/agent-loader.test.ts— 42 testsSECURITY_AUDIT.md— full audit documentextensions/agent-team.ts— uses shared loader, removed duplicate codeextensions/agent-chain.ts— uses shared loader, removed duplicate codeextensions/pi-pi.ts— uses shared loader, removed duplicate codeREADME.md— documents validation, tests, and audit