Skip to content

[Repo Assist] fix: prevent findFiles from escaping parent when directory is outside workspace#94

Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/fix-findfiles-escape-2026-03-17-6660966bbfcb574d
Draft

[Repo Assist] fix: prevent findFiles from escaping parent when directory is outside workspace#94
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/fix-findfiles-escape-2026-03-17-6660966bbfcb574d

Conversation

@github-actions
Copy link
Contributor

🤖 This is an automated draft PR from Repo Assist, an AI assistant.

Summary

Fixes a bug in lib/utils.js where findFiles() could search for config files outside the workspace root when a PHP file is opened from a path that resolves to outside the workspace.

Root Cause

getStandard() computes the relative directory via path.relative(workspaceRoot, path.dirname(filePath)). When a PHP file sits outside the workspace (e.g. opened directly from the filesystem), this produces a traversal path like "../../outside-project".

findFiles(workspaceRoot, "../../outside-project", confFileNames) then calls path.resolve(workspaceRoot, "../../outside-project") which yields a path that is entirely outside the workspace. The boundary check if (parent === currentDir) { break; } compares the raw parent string against currentDir derived from the resolved path — they can never match, so the loop walks all the way up to the filesystem root, potentially returning a phpcs.xml from an unrelated project.

Fix

Two changes to lib/utils.js:

  1. Early escape guard: Before starting the walk, check whether resolvedDir is inside resolvedParent. If not, return null immediately — no cross-project config file can be inadvertently applied.
  2. Normalise boundary comparison: Use resolvedParent (result of path.resolve(parent)) in the loop's boundary check instead of the raw parent string, so trailing-slash differences or redundant segments (e.g. "/workspace/") never cause the guard to miss.
// Guard: directory outside workspace → bail out
if (resolvedDir !== resolvedParent &&
    !resolvedDir.startsWith(resolvedParent + path.sep)) {
    return null;
}
```

## Test Added

A new unit test, **"returns null when directory resolves outside parent (no workspace escape)"**, exercises the escape path directly. The old code would have searched outside and (if a matching file existed) returned it; the new code returns `null`.

## Test Status

All 8 unit tests pass:

```
 findFiles
   returns path when file exists in exact directory
   returns null when file does not exist anywhere in tree
   walks up the directory tree to find file
   returns file in the deepest matching directory when multiple exist
   accepts an array of file names and returns first match found
   stops at parent boundary  does not escape above parent
   handles a single-segment directory (file at root)
   returns null when directory resolves outside parent (no workspace escape)
 findFiles (8 pass, 0 fail)

Run with npm run test:unit.

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f

… workspace

If path.relative() produces a path like '../../outside' (when a PHP file
is opened from outside the workspace root), findFiles was walking up from
outside the parent boundary and could return a config file from an
unrelated project.

Add an early-exit guard: if the resolved starting directory does not sit
inside resolvedParent, return null immediately. Also replace the raw
string comparison 'parent === currentDir' with the normalised equivalent
'resolvedParent === currentDir' (carries forward the normalisation from
the resolvedParent variable).

Add a unit test that demonstrates the previous escape and confirms the
fix: 'returns null when directory resolves outside parent'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants