Repository navigation
PRD: Squad SDK Hardening — Architecture & Security Improvements #895
Description
Problem Statement
The Squad SDK is installed directly on developer machines and acts as a runtime for AI-powered team agents. Two independent reviews — an architecture review (Flight) and a security audit (RETRO) — surfaced overlapping concerns that compound each other:
- Security gaps in shell execution, hook governance, and path handling expose users to injection and data leakage risks.
- Architectural debt in state management, error hierarchies, and API surface makes the security gaps harder to reason about and fix correctly.
- Contract mismatches between documented behavior and actual implementation erode trust: hooks that don't fire, state backends that silently fall back, lifecycle events that don't exist.
These aren't theoretical. A malicious or misconfigured .squad/ directory could trigger shell injection today. The hook governance pipeline — the SDK's primary safety net — is defined but never wired into execution. This PRD unifies both sets of findings into a single prioritized plan.
Strengths to preserve: The SDK has a clean dependency graph (zero circular deps, only 2 hard deps), solid credential handling (env vars, 0o600 perms), zero npm audit vulnerabilities, TLS on all external calls, and a well-designed 3-tier state layer with a clean builder API.
Prioritized Work Items
P0 — Critical (must fix before any release)
| # | Theme | Finding | Source | Risk |
|---|---|---|---|---|
| 1 | Shell Injection | execSync(entry.task.ref) in scheduler passes unvalidated ref directly to shell |
RETRO | RCE — any agent task definition can execute arbitrary commands |
| 2 | Shell Injection | execSync with template literals in git state backend; user-controlled keys not shell-escaped |
RETRO | RCE via crafted state keys |
| 3 | Dead Security Pipeline | Hook governance (file-write guards, PII scrubbing, shell blocking) is defined but NEVER wired into tool execution — entire hook system is dead code | RETRO | All hook-based safety guarantees are illusory |
| 4 | Silent State Fallback | external state backend silently falls back to worktree when config fails — data goes to wrong backend without warning |
Flight | Data written to unintended location; state corruption |
| 5 | Swallowed Errors | resolveStateBackend() silently swallows errors — callers don't know their config failed |
Flight | Masks configuration bugs; compounds P0-4 |
| 6 | Ghost API Surface | Hook docs/API advertise lifecycle hooks (onSessionStart, onSessionEnd, onErrorOccurred) that don't exist in implementation |
Flight | Users build integrations against non-functional contracts |
P1 — High (fix in next milestone)
| # | Theme | Finding | Source | Risk |
|---|---|---|---|---|
| 7 | Prompt Injection | Charter template injection — charter.md content concatenated directly into LLM prompts without sanitization |
RETRO | Prompt manipulation via agent charters |
| 8 | Path Traversal | Agent name not validated against ../, absolute paths, null bytes |
RETRO | Directory escape; read/write outside .squad/ |
| 9 | Path Traversal | teamRoot from config.json resolved without boundary check |
RETRO | Config-driven escape from project root |
| 10 | Network Exposure | Unauthenticated EventBus WebSocket on port 6277 — any local process can eavesdrop | RETRO | Local privilege escalation; session hijacking |
| 11 | Race Condition | No atomic writes / file locking for .squad/ state files |
RETRO | State corruption under concurrent access |
| 12 | Sync Debt | ~34 files still use deprecated sync storage methods — need async migration | Flight | Blocks event loop; prevents proper error handling |
| 13 | API Sprawl | ~52 export paths in package.json with no stability tiers |
Flight | Breaking changes impossible to communicate; semver meaningless |
| 14 | Error Fragmentation | Three independent error hierarchies (SquadError, StorageError, StateError) with fragile ErrorFactory string matching |
Flight | Error handling is unreliable; catch blocks miss errors |
| 15 | Test Isolation | SDK not independently testable — no npm test in package; all tests in root test/ |
Flight | Can't verify SDK correctness in isolation; slows CI |
| 16 | Dual Abstractions | StateBackend parallels StorageProvider without extending it — two storage abstractions |
Flight | Confusion about which abstraction to use; duplicated bugs |
P2 — Medium (address in planning)
| # | Theme | Finding | Source | Risk |
|---|---|---|---|---|
| 17 | PII Exposure | PII scrubbing disabled by default; email-only regex pattern | RETRO + Flight | Sensitive data leaks to logs/state |
| 18 | Guard Bypass | Hook guards default to allow when arguments are null instead of block | RETRO | Safety checks bypassed by omission |
| 19 | Schema Validation | Minimal JSON schema validation for config files | RETRO | Malformed configs cause runtime errors instead of clear failures |
| 20 | TOCTOU | Symlink check race condition in FSStorageProvider |
RETRO | Symlink-based file access bypass |
| 21 | Module Sprawl | runtime/ module handles 11+ concerns across 20+ files |
Flight | Hard to reason about; change amplification |
| 22 | Domain Sprawl | One package mixes runtime + state + platform + build + marketplace | Flight | Everything coupled; can't version independently |
| 23 | Glob Implementation | Hand-rolled glob matching in hooks (should use picomatch) |
Flight | Edge cases in pattern matching; maintenance burden |
Implementation Phases
Phase 1: Critical Security Fixes
Timeline target: Immediate
Theme: Stop the bleeding — eliminate RCE vectors and wire the safety net.
| Item | Work | Effort | Notes |
|---|---|---|---|
| #1, #2 | Replace all execSync with execFileSync / spawn with explicit argv arrays; validate and shell-escape all dynamic inputs |
M | Audit every execSync call site; add input validation |
| #3 | Wire hook governance pipeline into tool execution path; add integration tests proving guards fire | L | Largest item — requires tracing the full tool execution flow |
| #4, #5 | Make state backend throw on misconfiguration instead of falling back silently; surface errors to callers | S | Fail-fast with clear error messages |
| #6 | Remove or implement advertised lifecycle hooks; update docs to match reality | S | Audit hook type definitions vs. actual emit sites |
Exit criteria: Zero shell injection vectors. Hook governance provably active. State backend fails loudly on bad config. No ghost APIs.
Phase 2: Contract Truthfulness & Input Validation
Timeline target: Next milestone
Theme: Every API does what it says. Every input is validated.
| Item | Work | Effort | Notes |
|---|---|---|---|
| #7 | Sanitize charter content before LLM prompt injection; implement allow-list or escaping strategy | M | Requires deciding on sanitization approach |
| #8, #9 | Add path validation (no .., no absolute, no null bytes) for agent names and teamRoot |
S | Centralized validation function |
| #10 | Add authentication to EventBus WebSocket (token or Unix socket) | M | May require protocol version bump |
| #11 | Implement atomic writes (write-to-temp-then-rename) for state files | S | Standard pattern; apply to FSStorageProvider |
| #13 | Define stability tiers (stable, beta, internal) for exports; document in package.json |
M | Requires API surface audit |
| #14 | Unify error hierarchies under single SquadError base with typed codes instead of string matching |
M | Coordinate with CLI error handling |
| #16 | Merge StateBackend into StorageProvider or define clear layering contract |
M | Design decision needed first |
Exit criteria: All inputs validated. Error hierarchy unified. API surface documented with stability guarantees.
Phase 3: Structural Improvements
Timeline target: Following milestone
Theme: Pay down structural debt; enable future velocity.
| Item | Work | Effort | Notes |
|---|---|---|---|
| #12 | Migrate ~34 files from sync to async storage methods | L | Mechanical but wide-reaching; do in batches |
| #15 | Add npm test to squad-sdk package; move/mirror relevant tests |
M | Enables independent SDK CI |
| #17 | Expand PII patterns (phone, SSN, API keys); enable scrubbing by default | S | Incremental; add patterns over time |
| #18 | Change hook guard defaults from allow to block when arguments are null | S | Breaking change — needs migration path |
| #19 | Add JSON schema validation for all config files | M | Use ajv or similar |
| #20 | Fix TOCTOU in symlink check (use O_NOFOLLOW or fstat after open) |
S | Surgical fix |
| #21, #22 | Plan runtime module decomposition; split package if warranted | L | Design RFC needed; don't rush |
| #23 | Replace hand-rolled glob with picomatch |
S | Drop-in replacement |
Exit criteria: All storage calls async. SDK independently testable. PII scrubbing on by default. Config validated at load time.
Cost & Risk Analysis
| Phase | Total Effort | Risk if Deferred |
|---|---|---|
| Phase 1 | ~2-3 weeks | Severe — RCE via shell injection is exploitable today. Hook governance being dead code means every safety claim in docs is false. Silent state fallback causes data loss. |
| Phase 2 | ~3-4 weeks | High — Prompt injection, path traversal, and unauthenticated WebSocket are exploitable by malicious .squad/ repos or local processes. API sprawl makes semver promises meaningless. |
| Phase 3 | ~4-6 weeks | Moderate — Technical debt slows development velocity and makes bugs harder to find, but doesn't directly expose users to security risk (except PII scrubbing). |
Success Criteria
Phase 1
- Zero
execSynccalls with string interpolation or unvalidated input - Hook governance integration tests pass — guards provably fire on tool execution
- State backend throws
StateErroron misconfiguration (no silent fallback) - All documented hooks either implemented or removed from type definitions and docs
- Security-focused test suite added with injection/traversal test cases
Phase 2
- Path validation rejects
../, absolute paths, and null bytes in agent names andteamRoot - Charter content sanitized before prompt insertion (test with adversarial inputs)
- EventBus WebSocket requires authentication token
- State file writes are atomic (write-tmp-rename pattern)
- Export paths annotated with stability tiers in package.json
- Single unified error hierarchy with typed error codes
Phase 3
- Zero remaining sync storage method calls
-
cd packages/squad-sdk && npm testruns and passes independently - PII scrubbing enabled by default with expanded patterns
- All config files validated against JSON schema at load time
- Hook guards default to block (not allow) on missing arguments
Dependencies & Recommended Order
Phase 1 (all can parallelize):
#1,#2 Shell injection ──────────────────────────────┐
#3 Wire hook governance ─────────────────────────────┤
#4,#5 State backend fail-fast ───────────────────────┤── Gate: all P0 done
#6 Remove ghost APIs ───────────────────────────────┘
Phase 2 (after Phase 1 gate):
#14 Error unification ──► #16 Merge state/storage abstractions
#8,#9 Path validation (independent)
#7 Charter sanitization (independent)
#10 WebSocket auth (independent)
#11 Atomic writes ──► #12 Async migration (Phase 3)
#13 API stability tiers (independent)
Phase 3 (after Phase 2):
#12 Async migration (depends on #11 atomic writes)
#15 SDK test isolation (independent, can start earlier)
#17-#23 Remaining items (independent, parallelize)
Key dependency: Error unification (#14) should land before merging state/storage abstractions (#16) — otherwise the new abstraction inherits the fragmented error model. Atomic writes (#11) should land before async migration (#12) to avoid introducing new race conditions during the migration.
Sources
- Architecture Review: Flight (Lead) — full SDK architecture audit
- Security Audit: RETRO (Security) — threat model and code-level security review
Labels
This PRD covers squad-sdk package scope. Individual work items should be broken into separate issues and linked back here.
"The SDK's safety net exists in code but isn't plugged in. The architecture is sound in structure but dishonest in contracts. We fix the lies first, then the debt." — Flight
Activity
fix(sdk): eliminate shell injection vectors in scheduler and state ba…
diberry commentedon Apr 10, 2026
🔒 On hold — focusing on low-touch and medium-touch issues first.