Skip to content
Open
Changes from all commits
Commits
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
77 changes: 77 additions & 0 deletions __tests__/lib/resolve-subagent-path.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// @vitest-environment node
import { mkdtemp, mkdir, rm, writeFile } from "fs/promises";
import { tmpdir } from "os";
import { dirname, join, relative } from "path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";

import { resolveSubagentPath } from "@/lib/resolve-subagent-path";

describe("resolveSubagentPath", () => {
let fixtureRoot: string;
let projectsPath: string;

const projectName = "project";
const sessionId = "session";
const agentId = "abc123";

beforeEach(async () => {
fixtureRoot = await mkdtemp(join(tmpdir(), "failproofai-subagents-"));
projectsPath = join(fixtureRoot, "projects");
await mkdir(join(projectsPath, projectName, sessionId, "subagents"), { recursive: true });
});

afterEach(async () => {
await rm(fixtureRoot, { recursive: true, force: true });
});

async function touch(path: string): Promise<void> {
await mkdir(dirname(path), { recursive: true });
await writeFile(path, "");
}

it("returns candidate 1 when the project-level agent log exists", async () => {
const candidate = join(projectsPath, projectName, `agent-${agentId}.jsonl`);
await touch(candidate);

await expect(resolveSubagentPath(projectsPath, projectName, sessionId, agentId)).resolves.toBe(candidate);
});

it("returns candidate 2 when candidate 1 is missing", async () => {
const candidate = join(projectsPath, projectName, sessionId, `agent-${agentId}.jsonl`);
await touch(candidate);

await expect(resolveSubagentPath(projectsPath, projectName, sessionId, agentId)).resolves.toBe(candidate);
});

it("returns candidate 3 when candidates 1 and 2 are missing", async () => {
const candidate = join(projectsPath, projectName, sessionId, "subagents", `agent-${agentId}.jsonl`);
await touch(candidate);

await expect(resolveSubagentPath(projectsPath, projectName, sessionId, agentId)).resolves.toBe(candidate);
});

it("returns candidate 1 when all candidate paths exist", async () => {
const candidate1 = join(projectsPath, projectName, `agent-${agentId}.jsonl`);
const candidate2 = join(projectsPath, projectName, sessionId, `agent-${agentId}.jsonl`);
const candidate3 = join(projectsPath, projectName, sessionId, "subagents", `agent-${agentId}.jsonl`);
await touch(candidate3);
await touch(candidate2);
await touch(candidate1);

await expect(resolveSubagentPath(projectsPath, projectName, sessionId, agentId)).resolves.toBe(candidate1);
});

it("returns null when none of the candidate paths exist", async () => {
await expect(resolveSubagentPath(projectsPath, projectName, sessionId, agentId)).resolves.toBeNull();
});

it("does not resolve an existing candidate outside the projects path", async () => {
const traversalAgentId = "../../../../escape";
const escapedCandidate = join(projectsPath, projectName, `agent-${traversalAgentId}.jsonl`);
expect(relative(projectsPath, escapedCandidate).startsWith("..")).toBe(true);
expect(relative(fixtureRoot, escapedCandidate).startsWith("..")).toBe(false);
await touch(escapedCandidate);

await expect(resolveSubagentPath(projectsPath, projectName, sessionId, traversalAgentId)).resolves.toBeNull();
});
Comment on lines +68 to +76
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Test leak: escaped file not cleaned up.

The path traversal test creates a file outside fixtureRoot (at /tmp/escape.jsonl or similar), but the afterEach hook only removes fixtureRoot. This leaves the escaped file behind, polluting the test environment.

The calculation:

  • join(projectsPath, projectName, "agent-../../../../escape.jsonl")
  • With projectsPath = "/tmp/failproofai-subagents-XXXXXX/projects"
  • Normalizes to: /tmp/escape.jsonl (outside the fixture)
🧹 Proposed fix: deepen fixture structure to contain the escaped path
  beforeEach(async () => {
    fixtureRoot = await mkdtemp(join(tmpdir(), "failproofai-subagents-"));
-   projectsPath = join(fixtureRoot, "projects");
+   // Use a deeper structure so that even with traversal, we stay within fixtureRoot
+   projectsPath = join(fixtureRoot, "a", "b", "c", "d", "projects");
    await mkdir(join(projectsPath, projectName, sessionId, "subagents"), { recursive: true });
  });

With this change:

  • escapedCandidate normalizes to /tmp/failproofai-subagents-XXXXXX/a/b/c/escape.jsonl
  • Still outside projectsPath (satisfies line 60's assertion)
  • But inside fixtureRoot (properly cleaned up)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/lib/resolve-subagent-path.test.ts` around lines 57 - 64, The test
currently leaves an escaped file outside fixtureRoot because traversalAgentId
("../../../../escape") normalizes above projectsPath; change the fixture so the
`escapedCandidate` path normalizes inside `fixtureRoot` while still being
outside `projectsPath` (e.g., deepen the temp fixture directory structure and
pick a traversalAgentId that climbs out of `projectsPath` but remains within the
fixtureRoot tree), ensure any directories for `escapedCandidate` are created
before calling `touch`, and/or explicitly remove the created escaped file in the
test teardown (afterEach) to guarantee cleanup; update references in the test to
use the adjusted traversalAgentId/fixture layout (symbols: traversalAgentId,
escapedCandidate, projectsPath, fixtureRoot, touch, afterEach,
resolveSubagentPath).

});