sdk fixes

[khaliqgant]

CLI → SpawnManager → AgentSpawner → RelayPtyOrchestrator → RelayClient

• Spawned agents couldn't connect to the daemon sockets

[my-senior-dev-pr-review]

🤖 My Senior Dev — Analysis Complete

👤 For @khaliqgant

📁 Expert in src (497 edits) • ⚡ 138th PR this month

View your contributor analytics →

---

📊 25 files reviewed • 2 high risk • 1 need attention

🚨 High Risk:

• .trajectories/index.json — Modifications in the index structure can lead to critical runtime issues that may negatively impact trajectory tracking and performance.

⚠️ Needs Attention:

• .trajectories/completed/2026-01/traj_6ddibplo00du.json — While this is a data file, it's essential to confirm that it aligns with the new system protocols and doesn't lead to undefined behavior.

---

🚀 Open Interactive Review →

The full interface unlocks features not available in GitHub:

• 💬 AI Chat — Ask questions on any file, get context-aware answers
• 🔍 Smart Hovers — See symbol definitions and usage without leaving the diff
• 📚 Code Archeology — Understand how files evolved over time (/archeology)
• 🎯 Learning Insights — See how this PR compares to similar changes

---

💬 Chat here: @my-senior-dev explain this change — or try @chaos-monkey @security-auditor @optimizer @skeptic @junior-dev

📖 View all 12 personas & slash commands

You can interact with me by mentioning @my-senior-dev in any comment:

In PR comments or on any line of code:

• Ask questions about the code or PR
• Request explanations of specific changes
• Get suggestions for improvements

Slash commands:

• /help — Show all available commands
• /archeology — See the history and evolution of changed files
• /profile — Performance analysis and suggestions
• /expertise — Find who knows this code best
• /personas — List all available AI personas

AI Personas (mention to get their perspective):

Persona	Focus
@chaos-monkey 🐵	Edge cases & failure scenarios
@skeptic 🤨	Challenge assumptions
@optimizer ⚡	Performance & efficiency
@security-auditor 🔒	Security vulnerabilities
@accessibility-advocate ♿	Inclusive design
@junior-dev 🌱	Simple explanations
@tech-debt-collector 💳	Code quality & shortcuts
@ux-champion 🎨	User experience
@devops-engineer 🚀	Deployment & scaling
@documentation-nazi 📚	Documentation gaps
@legacy-whisperer 🏛️	Working with existing code
@test-driven-purist ✅	Testing & TDD

For the best experience, view this PR on myseniordev.com — includes AI chat, file annotations, and interactive reviews.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional flags.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View issue and 5 additional flags in Devin Review.

In general terms, the problem is that the cwd parameter from SpawnRequest is treated as an arbitrary filesystem root and passed down to getProjectPaths, which then builds data directories and socket paths, ultimately leading to creation of directories and symlinks. To fix this, we need to: (1) constrain what values of cwd are allowed (for example, only allow it if it is within a safe base project root, or disallow it entirely), and (2) normalize it before use (via path.resolve) and ensure the resulting path is still under that safe root. This should be done as close as possible to where cwd is first accepted from the client, so the rest of the code can continue to assume config.cwd is safe. Since CodeQL’s reported sink is in relay-pty-orchestrator.ts, we can add a defensive check there as well, in case other callers bypass the HTTP layer.

The single best, minimal-change fix here—without altering external behavior for valid inputs—is:

1. In packages/bridge/src/spawner.ts, inside AgentSpawner.spawn, when computing agentCwd, validate and normalize any request.cwd before using it. The AgentSpawner already has a known-safe this.projectRoot (it is used as default), so we can:

   • Resolve both this.projectRoot and request.cwd with path.resolve.
   • Require that the resolved cwd starts with the resolved projectRoot plus a path separator, or is exactly equal to it. If not, reject the spawn request with an error, similar to the existing validation for name.
   • Use the normalized resolvedCwd as agentCwd (so literals like . or ./subdir become canonical).
     This keeps existing behavior for callers that pass a subdirectory of the project root but prevents escapes like /etc, ../../.., or similar.

2. Optionally, for extra defense, in RelayPtyOrchestrator’s constructor in packages/wrapper/src/relay-pty-orchestrator.ts, we can normalize config.cwd with path.resolve (via join’s sibling resolve), so even if another caller passes a relative path, getProjectPaths always receives a canonical absolute path. However, because getProjectPaths already calls path.resolve inside hashPath and findProjectRoot, this extra normalization is not strictly required for security; the main risk is from the choice of base directory, which we address in the spawner.

3. We do not need to modify getProjectPaths itself, as it is a general-purpose utility that may intentionally accept arbitrary roots. The risk is in how it is called with user-controlled data; hence the fix belongs in the bridging/spawn layer that mediates between HTTP and worker orchestration.

Concretely:

• Edit packages/bridge/src/spawner.ts in the spawn method around lines 887–889, where agentCwd is currently defined as request.cwd || this.projectRoot. Insert a small block to:
  • If request.cwd is provided:
    • Resolve this.projectRoot and request.cwd via path.resolve.
    • If resolvedCwd === resolvedProjectRoot or resolvedCwd.startsWith(resolvedProjectRoot + path.sep), accept and set agentCwd to resolvedCwd.
    • Otherwise, return an error SpawnResult indicating invalid cwd (similar style to the existing name validation).
  • If request.cwd is not provided, keep using this.projectRoot as before.
• No new imports are needed: this file already imports path from node:path at the top, and we reuse that.

This change ensures that any user-supplied working directory (via req.body.cwd in both dashboard servers) is constrained to the agent-relay project tree, eliminating the uncontrolled-path issue that ultimately affected linkParent in RelayPtyOrchestrator.start.

---

General approach: Treat the user-provided cwd as either (a) a relative subdirectory under a trusted project root, or (b) a path that must be validated and normalized to stay within a trusted base. The simplest non-breaking fix here is to constrain cwd to a safe root in the spawner layer, before passing it into RelayPtyOrchestratorConfig and getProjectPaths. This keeps all existing behavior for normal relative values (like "." or "subdir") but prevents traversal outside the project root.

Best concrete fix without changing existing functionality:

1. In packages/bridge/src/spawner.ts, when computing agentCwd, normalize and bound request.cwd to this.projectRoot:
   • If request.cwd is falsy, keep the existing behavior (agentCwd = this.projectRoot).
   • Otherwise, resolve request.cwd relative to this.projectRoot via path.resolve(this.projectRoot, request.cwd).
   • Ensure the resulting resolvedCwd is still under this.projectRoot by checking that it starts with this.projectRoot + path.sep or equals this.projectRoot. If not, reject the spawn request with an error.
2. Use this sanitized agentCwd as before when creating ptyConfig.cwd. No changes are needed in RelayPtyOrchestrator or project-namespace.ts, because they will now receive a trusted project-root-based path.

This fix is localized to packages/bridge/src/spawner.ts inside the shown spawn method. We do not need new imports because path is already imported in that file.

---

General approach: validate and normalize the user-provided cwd before it is allowed to influence filesystem paths. We can either (a) disallow arbitrary cwd values altogether, or (b) allow them only if they resolve inside a trusted base directory (for example, the project root) and are free of traversal tricks. Since we should not change existing behavior more than necessary, the best fix is to keep the cwd feature but bound it to the project root in AgentSpawner.spawn, and ensure we resolve it to an absolute path there. This avoids threading validation logic through multiple modules and centralizes the trust boundary in the spawner.

Concrete fix:

1. In packages/bridge/src/spawner.ts, inside AgentSpawner.spawn, where agentCwd is computed:

   // Use request.cwd if specified, otherwise use projectRoot
   const agentCwd = request.cwd || this.projectRoot;

   replace this with logic that:

   • If request.cwd is not provided, use this.projectRoot as before.
   • If request.cwd is provided but is not a string, reject the request.
   • Resolve request.cwd against this.projectRoot using path.resolve(this.projectRoot, request.cwd), so relative paths are anchored under the project root.
   • Optionally, allow absolute paths by checking that path.resolve(request.cwd) starts with this.projectRoot, but to be safest and simplest, we can require cwd to be either relative or, if absolute, still contained within this.projectRoot.
   • If the resolved agentCwd is not inside this.projectRoot (string prefix check after path.resolve), return an error SpawnResult instead of proceeding.
     This ensures that the effective cwd and hence the root passed into getProjectPaths(agentCwd) is always within the project root.

2. Because spawner.ts already imports path as import path from 'node:path';, we can use it directly; no new imports are required.

3. We do not need to modify relay-pty-orchestrator.ts or project-namespace.ts themselves: once config.cwd has been sanitized and constrained in the spawner, getProjectPaths(config.cwd) will only ever see a project-root-contained directory, so the creation of outbox directories and symlinks will be within that allowed tree.

4. The dashboard and dashboard-server POST /api/spawn handlers can remain unchanged; they simply pass cwd through. The trust boundary and validation is now in the bridge layer where SpawnRequest is consumed.

Line-level changes:

• File packages/bridge/src/spawner.ts, around lines 887–889:
  • Replace the simple assignment const agentCwd = request.cwd || this.projectRoot; with the new validation logic as described.
  • Ensure that on invalid cwd (non-string, traversal outside project root), we return a SpawnResult object with success: false and a clear error message instead of continuing.

This single change ensures that RelayPtyOrchestratorConfig.cwd is always an absolute path within the project root, preventing the uncontrolled path expression issue that appears later when creating directories and symlinks.

---

General approach: Constrain or validate the user-provided cwd before it is used to derive project paths and file-system locations. We should prevent traversal outside an intended base directory, and/or fall back to a safe project root when cwd is invalid. Because cwd is first consumed in AgentSpawner.spawn, that is the ideal central point to enforce validation for all call sites (dashboard and dashboard-server). Additionally, we can make getProjectPaths safer by rejecting obviously dangerous projectRoot inputs.

Best concrete fix:

1. In packages/bridge/src/spawner.ts inside AgentSpawner.spawn:

   • Import path is already present; reuse it.
   • Right after the existing agent name validation, add validation/normalization for request.cwd:
     • If request.cwd is defined and is not a string, reject with an error.
     • Resolve it to an absolute path with path.resolve.
     • Optionally, enforce that this path is inside this.projectRoot (the known safe project root of the spawner). If it is not, either:
       • reject the spawn request with a 400-style error, or
       • ignore the user-provided cwd and fall back to this.projectRoot.
     • For minimal behavior change while still addressing the vulnerability, I’ll choose to reject clearly unsafe cwd (non-string or path that escapes the project root) with a descriptive error. That keeps behavior unchanged for legitimate cwd values under the project root but prevents using arbitrary filesystem directories.
   • Then, when computing agentCwd, use the validated safeCwd instead of raw request.cwd.

   This directly breaks the taint flow: cwd coming from req.body cannot drive the filesystem paths unless it is a normalized path within this.projectRoot.

2. In packages/config/src/project-namespace.ts in getProjectPaths(projectRoot?: string):

   • Add a lightweight guard: if projectRoot is provided and is not a string or is empty/whitespace, throw an error. Legitimate callers already pass either undefined or a directory path; this will catch badly-typed tainted input early.
   • Then proceed with const root = projectRoot ?? findProjectRoot(); as before.

   This is mainly defense-in-depth. With the primary fix in the spawner, we still ensure this utility doesn’t silently accept nonsense projectRoot values if misused elsewhere.

3. We do not need to touch relay-pty-orchestrator.ts symlink creation logic: once cwd is validated in the spawner and getProjectPaths is only given acceptable roots, the resulting _canonicalOutboxPath, _outboxPath, and _legacyOutboxPath will be under the expected project tree or /tmp/relay/... space, and not attacker-chosen arbitrary paths. The linkPath used in symlinkSync will therefore no longer be influenced by untrusted input in an uncontrolled manner.

Files/regions to change:

• packages/bridge/src/spawner.ts

  • In AgentSpawner.spawn, just after the existing agent name validation (around lines 689–696) add a cwd validation block and compute a safeCwd.
  • Update the agentCwd assignment (around line 888) to use safeCwd instead of request.cwd.

• packages/config/src/project-namespace.ts

  • In getProjectPaths (around lines 101–105), add a type/emptiness check on projectRoot when provided.

No new imports are needed beyond what’s already in these files.

---

In general, the fix is to validate and constrain the user-supplied cwd before it is used to derive filesystem paths. The safest approach here is: (1) only allow cwd to be an absolute path; (2) normalize it with path.resolve; (3) optionally ensure it lies within an allowed base directory; and (4) reject or clear it if it contains traversal indicators like .. or attempts to escape the permitted base. This should be enforced on the server endpoints that accept cwd (both dashboard variants) so that the rest of the pipeline (AgentSpawner, RelayPtyOrchestrator, getProjectPaths) receives either a safe normalized path or undefined, preserving existing behavior when cwd is not provided.

Concretely, the minimal, non-functional-breaking fix is:

• In packages/dashboard/src/server.ts, inside the /api/spawn handler, validate cwd from req.body before including it in the SpawnRequest. Use Node’s path module (already imported) to:
  • Ensure it is a string.
  • Ensure it is absolute (path.isAbsolute), otherwise reject with 400.
  • Normalize it with path.resolve(cwd).
  • Optionally guard against traversal by checking for .. segments in the normalized path (for belt-and-suspenders).
• Do the same in packages/dashboard-server/src/server.ts’s /api/spawn handler.
• Replace cwd: cwd || undefined with cwd: safeCwd where safeCwd is either a validated, normalized path or undefined if invalid/absent; reject obviously malicious input with a 400 rather than silently ignoring.

We do not need to modify AgentSpawner or RelayPtyOrchestrator because they already treat cwd as an opaque base directory; hardening the ingestion point is sufficient to stop tainted data flowing into unsafe path expressions. No additional imports are required beyond path which is already in both server files.

---

In general terms, the fix is to validate and constrain the user-provided cwd before it is used to derive filesystem paths. We should either (a) reject cwd values that are not simple, safe paths or that contain traversal characters, or (b) resolve cwd against a known safe base directory and ensure the resolved path remains under that base. Since the current semantics allow specifying a full working directory, the least disruptive change is to enforce that cwd is an absolute path and disallow obvious traversal constructs; optionally, we can normalize it.

The single best place to fix this without changing existing functionality is in the HTTP layer, where cwd first enters the system (/api/spawn handlers in packages/dashboard/src/server.ts and packages/dashboard-server/src/server.ts). There we can:

• Ensure cwd is a string if present.
• Optionally normalize with path.resolve.
• Reject values that contain .., \ on POSIX, or null bytes, or that are not absolute paths on the current platform.
  If validation fails, we respond with 400 and do not call spawner.spawn. This prevents tainted cwd from reaching getProjectPaths and RelayPtyOrchestrator. No changes are needed in the deeper layers (spawner.ts, relay-pty-orchestrator.ts, project-namespace.ts), which can continue to assume cwd is a reasonable directory.

Concretely, in both dashboard servers we will:

• Import path only if not already imported (it already is).
• Before building the SpawnRequest, validate cwd (if provided):
  • typeof cwd === 'string'
  • use path.resolve(cwd) to normalize
  • check that the normalized path is absolute (path.isAbsolute)
  • check that it does not contain the substring .. path segments (e.g. by splitting and checking segments), and does not contain null bytes.
  • On failure, immediately return a 400 JSON error explaining that cwd is invalid.
    We’ll also ensure both packages/dashboard/src/server.ts and packages/dashboard-server/src/server.ts get the same logic, covering both alert variants.

---

In general, the problem is that cwd from req.body is treated as a trusted project root and passed down to getProjectPaths, which then uses it to derive data directories and sockets. To fix this, we must either (a) restrict cwd to a safe subset of values (for example, only allow absolute paths under the actual server project root, or disallow setting it entirely via the API), or (b) sanitize and normalize it and then enforce that the resolved path is inside a known-safe base directory before using it. The least invasive fix that preserves functionality is to keep supporting client-specified cwd but to validate it in AgentSpawner.spawn before using it, replacing unsafe values with the existing safe default (this.projectRoot) and optionally logging or returning an error.

Concrete best fix:

1. In packages/bridge/src/spawner.ts, inside AgentSpawner.spawn, validate request.cwd:
   • Only allow string values.
   • Resolve it to an absolute path with path.resolve.
   • Compute the real project base (this.projectRoot) and ensure the resolved cwd is within that base using a prefix check that accounts for path separators.
   • If the check fails, either reject the spawn request or, more conservatively, ignore the provided cwd and fall back to this.projectRoot. This avoids changing external API semantics while preventing arbitrary directory selection.
2. Use the validated value (safeCwd) when computing agentCwd instead of using request.cwd directly.
3. This keeps all downstream behavior (including getProjectPaths(config.cwd) and the symlink creation in RelayPtyOrchestrator) constrained to the server’s known project tree.

We do not need to change getProjectPaths or RelayPtyOrchestrator themselves; by constraining the cwd at the spawner layer, the taint flow to symlinkSync(targetPath, linkPath) is effectively cut off for attacker-controlled filesystem locations.

Specific edits:

• File: packages/bridge/src/spawner.ts
  • Add a small normalization/validation block near the existing agent name validation in spawn to compute safeCwd.
  • Update the later assignment const agentCwd = request.cwd || this.projectRoot; to use safeCwd instead.
  • Use the existing path import at the top of the file; no new imports are necessary.

No changes are required in the two server files or in project-namespace.ts and relay-pty-orchestrator.ts, because once request.cwd is validated/normalized in the spawner, the path expressions in those modules will only operate on trusted roots.

---

In general, the fix is to validate and constrain the user‑supplied cwd before it is ever used to construct filesystem paths. We should treat cwd as an optional directory hint, but force it to be (a) an absolute path, (b) normalized, and (c) contained within a safe root directory that the service is allowed to operate in (for example, the spawner’s projectRoot). If validation fails, we should reject the request with a 400 error rather than passing the unsafe path into the rest of the system.

The best minimal change, without altering external behavior for valid inputs, is to validate cwd at the HTTP boundary in both packages/dashboard/src/server.ts and packages/dashboard-server/src/server.ts. For each /api/spawn handler, we can:

1. Compute a safe root for child working directories. A natural choice is the dashboard server’s current working directory (process.cwd()), since that is presumably the project being managed. This avoids changing any other modules.
2. If cwd is provided:
   • Ensure it is a string.
   • Normalize it with path.resolve, resolving relative paths against safeRoot.
   • Check that the resolved path is inside safeRoot by verifying resolved.startsWith(safeRoot + path.sep) or equals safeRoot.
   • Optionally disallow path traversal tokens (..) and null bytes up front.
3. If validation fails, respond with a 400 and a clear error; do not construct SpawnRequest.
4. When building the SpawnRequest, pass validatedCwd instead of raw cwd. If cwd is absent, keep existing behavior (cwd: undefined).

This keeps all downstream code (in AgentSpawner, RelayPtyOrchestrator, and getProjectPaths) unchanged; they will now only ever receive either undefined or a safe, normalized path under the permitted root. There is no need to modify packages/wrapper/src/relay-pty-orchestrator.ts or packages/config/src/project-namespace.ts; the sink is made safe by sanitizing its inputs before they reach it.

Concretely:

• In packages/dashboard/src/server.ts, in the /api/spawn route, insert validation logic just after destructuring req.body, and use a safeCwd (or similar) when building the SpawnRequest.
• Mirror the same logic in packages/dashboard-server/src/server.ts’s /api/spawn route.
• Reuse the already imported path module in both files; no new imports are needed.

---

General approach: Replace all uses of Math.random() in the name generator with a cryptographically secure source of randomness. In Node.js, this is best done via crypto.randomInt (or crypto.randomBytes), which directly yields unbiased integers in a range. This lets us preserve existing behavior (randomly choosing indices and a numeric suffix) while upgrading to secure randomness, without changing the rest of the codebase.

Concrete fix in this codebase:

• In packages/utils/src/name-generator.ts:
  • Import Node’s crypto module.
  • Introduce a small helper (or inline usage) that calls crypto.randomInt(max) to pick indices in ADJECTIVES and NOUNS instead of Math.floor(Math.random() * length).
  • For the fallback numeric suffix in generateUniqueAgentName, replace Math.floor(Math.random() * 1000) with crypto.randomInt(1000).
• No changes are needed in src/cli/index.ts, packages/config/src/shadow-config.ts, or packages/bridge/src/spawner.ts; they will automatically get secure names by calling the updated generateAgentName/generateUniqueAgentName.

All functionality (format and range of generated names) stays the same; only the randomness source changes.

---