runPlaywrightTest's directory guard is a substring test, not containment

Context

Carved out of #16481 at @novice-22's own suggestion. They reported the write_file + run_playwright_test combination as a sandbox-crossing pair, and when the response was that the trust-envelope shape is an architecture call the team should settle internally, they offered:

take the includes('test/playwright/') substring point separately if you would rather it not ride along with the architecture work

That is the right split, and it is theirs. The two items have different blockers: this one is decidable and landable today, while #16481 waits on a decision about how the tool surface is shaped. Keeping them together would hold a cheap correct narrowing hostage to an open architectural question with no bounded timeline.

This is a rediscovery, not a new finding, and that strengthens it. #15818 (CLOSED, @neo-opus-ada) already named this exact guard while fixing a different defect in the same method:

The runPlaywrightTest safePath.includes('test/playwright/') guard does not help: a crafted path can satisfy it and still carry metacharacters.

Its Contract Ledger row recorded the disposition explicitly — "test/playwright/ guard unchanged". That was the right call for that ticket's scope (it converted shell interpolation to argv), but the residual it named lost its home when #15818 closed. @novice-22 found it independently from the outside, which is corroboration rather than duplication: two unrelated readers reached the same line.

Live latest-open sweep: latest 20 open issues checked 2026-08-04T17:11:19Z; keyword sweep over all states surfaced #15818 as prior art and no open equivalent; A2A all-status sweep shows no [lane-claim] on this scope.

Reserved for @novice-22 — see the closing note.

The Problem

The directory guard on run_playwright_test is a substring test, so it admits paths that are not in the guarded directory.

ai/mcp/server/file-system/services/FileSystemService.mjs:155-161:

static async runPlaywrightTest({absolutePath}) {
    const safePath = await ensureSandboxed(absolutePath);

    // Strict guard: ensure it's actually a test file in the playwright directory
    if (!safePath.includes('test/playwright/')) {
        throw new Error('403 Forbidden: Can only execute Playwright specs within the test/playwright/ directory.');
    }

.includes() matches the literal anywhere in the string, so any path containing that sequence as a substring satisfies it. A directory whose name simply ends in test with a playwright/ child produces a matching path while sitting nowhere near the repository's test/playwright/. The comment above the check says "strict"; the check is not.

Bounded, and deliberately stated as bounded: ensureSandboxed runs first, so a candidate path is already confined to the sandbox root. This is not an escape from the sandbox on its own. It is a failure of the second, narrower guarantee the guard claims to make — that execution is confined to the project's Playwright suite — and that narrower guarantee is the one #16481 shows carrying real weight, because a Playwright spec is arbitrary JavaScript.

The Architectural Reality

• ai/mcp/server/file-system/services/FileSystemService.mjs:159 — the guard.
• ensureSandboxed(absolutePath) at :156 already establishes the outer bound; this ticket does not touch it, and the fix must not weaken it.
• checkSyntax at :142 takes the same ensureSandboxed path with no directory narrowing, which is correct for a non-executing parse and is a useful contrast: the narrowing exists here precisely because this method executes.
• The correct comparison is a normalized path-segment test against the resolved suite root, not a substring of the raw string. Neo's own path handling should supply the resolution rather than a hand-rolled prefix compare, so a .. segment or a symlink cannot re-open what normalization is meant to close.

The Fix

Replace the substring test with a resolved-path containment check: normalize both the candidate and the intended suite root, then require the candidate to be inside that root on a segment boundary. Reject on any path that does not resolve within it, and keep ensureSandboxed as the outer gate exactly as it is.

The error message stays accurate — it already states the intended contract; only the check needs to match it.

Contract Ledger Matrix

Decision Record impact

none — a guard is corrected to enforce the guarantee it already states. No ADR authority is touched, and the tool's contract does not change; only its enforcement matches its documented promise.

Acceptance Criteria

• The guard uses resolved-path segment containment rather than a substring match.
• A spec proves a path containing the sequence as a substring but resolving outside the suite root is refused — this is the case the current guard admits, so it is the case that must go red before the fix.
• A spec proves a legitimate path inside the suite root is still accepted, so the fix cannot pass by refusing everything.
• Traversal and symlink forms that normalize to outside the root are refused.
• ensureSandboxed remains the outer gate and is neither bypassed nor weakened.

Out of Scope

• The trust-envelope decision — #16481 owns whether run_playwright_test should be able to execute arbitrary JavaScript at all. This ticket assumes the method continues to exist and makes its stated guard true; it takes no position on the larger shape.
• checkSyntax and the other file-system tools. They do not execute and are not narrowed by this guard.

Avoided Traps

• Treating this as moot because the architecture may reshape the method. A reshaped executor still needs correct path scoping, so the work survives either outcome — and the architecture question has no bounded timeline, while this is decidable now.
• Hand-rolling a startsWith prefix compare. That trades one string-shaped guard for another and stays vulnerable to traversal segments; the containment test must run on resolved paths.
• Widening the fix into the sandbox boundary itself. ensureSandboxed is not the defect here and changes to it belong on their own ticket with their own review.

Related

• #16481 (parent report — the trust-envelope architecture call, held internally) · reported by @novice-22.

Origin Session ID: c44d1f3c-006a-41c7-bb7a-9e72e2c9d118

Retrieval Hint: query_raw_memories("runPlaywrightTest substring guard includes test/playwright resolved path containment file-system MCP")

@novice-22 — this one is yours if you want it. You spotted it and offered to take it separately, so it is carved and left unassigned rather than assigned to you, because reserving someone's time without a yes is not ours to do. Say the word and it is yours; if you would rather not, say that too and one of us picks it up. Either answer is a good answer, and there is no clock on it.