mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
fix: bound workspace repo discovery so symlink escapes can't stall startup
Workspace discovery follows symlinks since #1060, which means the walk can leave the workspace root by design. A link into a huge unrelated tree was enumerated synchronously before the server bound its port, hanging startup with no output. The walk now shares the PLANNOTATOR_FILE_BROWSER_MAX_FILES budget: it visits at most that many directories and returns what it found. Symlinked repos are still discovered under the default budget. Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk
This commit is contained in:
@@ -141,7 +141,7 @@ claude --plugin-dir ./apps/hook
|
||||
| `PLANNOTATOR_ANNOTATE_HISTORY` | Set to `0` / `false` to disable per-file version history in annotate mode (no copies of annotated files are written to the data dir; the annotate version diff is unavailable). Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "annotateHistory": false }`); the env var takes precedence. |
|
||||
| `JINA_API_KEY` | Optional Jina Reader API key for higher rate limits (500 RPM vs 20 RPM unauthenticated). Free keys include 10M tokens. |
|
||||
| `PLANNOTATOR_DATA_DIR` | Override the base data directory. Supports `~` expansion. Default: `~/.plannotator`. All data (plans, history, drafts, config, hooks, sessions, debug logs, IPC registry) is stored under this directory. |
|
||||
| `PLANNOTATOR_FILE_BROWSER_MAX_FILES` | File-discovery limit: regular files inspected by CLI markdown/folder resolution and startup code-file warming, and supported files returned by the file browser. Must be a positive integer; invalid, zero, or negative values use the default of `5000`. |
|
||||
| `PLANNOTATOR_FILE_BROWSER_MAX_FILES` | File-discovery limit: regular files inspected by CLI markdown/folder resolution and startup code-file warming, supported files returned by the file browser, and directories scanned during multi-repo workspace discovery (symlinks may point outside the workspace, so the budget — not the root — bounds that walk). Must be a positive integer; invalid, zero, or negative values use the default of `5000`. |
|
||||
| `PLANNOTATOR_GLIMPSE` | Set to `0` / `false` to disable the Glimpse native window even when `glimpseui` is installed. Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "glimpse": false }`). |
|
||||
| `PLANNOTATOR_GLIMPSE_WIDTH` | Width in pixels for the Glimpse native window. Default: `1280`. |
|
||||
| `PLANNOTATOR_GLIMPSE_HEIGHT` | Height in pixels for the Glimpse native window. Default: `900`. |
|
||||
|
||||
@@ -745,6 +745,49 @@ describe("review-workspace", () => {
|
||||
expect(discoverWorkspaceRepoPaths(root)).toEqual([]);
|
||||
});
|
||||
|
||||
it("bounds discovery when a symlink escapes into a huge external tree", () => {
|
||||
// Regression for the #1060 follow-up: a symlink inside the workspace
|
||||
// pointing at a large unrelated tree must not be enumerated unboundedly
|
||||
// before the server starts. The walk shares the file-discovery budget.
|
||||
const root = makeTempDir("plannotator-workspace-symlink-budget-");
|
||||
const huge = makeTempDir("plannotator-workspace-symlink-budget-target-");
|
||||
// 40 directories, each with 5 subdirectories — 200+ nodes, no repos.
|
||||
for (let i = 0; i < 40; i++) {
|
||||
for (let j = 0; j < 5; j++) {
|
||||
mkdirSync(join(huge, `dir-${String(i).padStart(2, "0")}`, `sub-${j}`), { recursive: true });
|
||||
}
|
||||
}
|
||||
linkDirectory(huge, join(root, "escape"));
|
||||
// A legitimate repo that sorts BEFORE the escape link so it is found
|
||||
// within the budget.
|
||||
const repo = join(root, "app");
|
||||
mkdirSync(repo, { recursive: true });
|
||||
initRepo(repo);
|
||||
|
||||
const prior = process.env.PLANNOTATOR_FILE_BROWSER_MAX_FILES;
|
||||
process.env.PLANNOTATOR_FILE_BROWSER_MAX_FILES = "25";
|
||||
try {
|
||||
const repos = discoverWorkspaceRepoPaths(root);
|
||||
expect(repos).toEqual([repo]);
|
||||
} finally {
|
||||
if (prior === undefined) delete process.env.PLANNOTATOR_FILE_BROWSER_MAX_FILES;
|
||||
else process.env.PLANNOTATOR_FILE_BROWSER_MAX_FILES = prior;
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps discovering symlinked repos under the default budget", () => {
|
||||
// The budget must not break the feature it bounds: an external
|
||||
// symlinked repo is still found with default limits.
|
||||
const root = makeTempDir("plannotator-workspace-symlink-budget-ok-");
|
||||
const targetRoot = makeTempDir("plannotator-workspace-symlink-budget-ok-target-");
|
||||
const targetRepo = join(targetRoot, "service");
|
||||
mkdirSync(targetRepo, { recursive: true });
|
||||
initRepo(targetRepo);
|
||||
linkDirectory(targetRepo, join(root, "linked"));
|
||||
|
||||
expect(discoverWorkspaceRepoPaths(root)).toEqual([join(root, "linked")]);
|
||||
});
|
||||
|
||||
it("chooses the first logical alias deterministically for duplicate targets", () => {
|
||||
const root = makeTempDir("plannotator-workspace-symlink-duplicates-");
|
||||
const targetRoot = makeTempDir("plannotator-workspace-symlink-duplicates-target-");
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
parsePatchPathToken,
|
||||
} from "./diff-paths";
|
||||
import { validateFilePath } from "./review-core";
|
||||
import { getFileBrowserMaxFiles } from "./resolve-file";
|
||||
|
||||
const SKIP_DIRS = new Set([
|
||||
".git",
|
||||
@@ -174,15 +175,21 @@ function compareDirentsByName(a: Dirent, b: Dirent): number {
|
||||
return 0;
|
||||
}
|
||||
|
||||
interface WorkspaceScanState {
|
||||
visitedRealPaths: Set<string>;
|
||||
readonly limit: number;
|
||||
}
|
||||
|
||||
function collectWorkspaceRepos(
|
||||
root: string,
|
||||
current: string,
|
||||
visitedRealPaths: Set<string>,
|
||||
state: WorkspaceScanState,
|
||||
results: string[],
|
||||
): void {
|
||||
if (state.visitedRealPaths.size >= state.limit) return;
|
||||
const realPath = resolveDirectoryRealPath(current);
|
||||
if (!realPath || visitedRealPaths.has(realPath)) return;
|
||||
visitedRealPaths.add(realPath);
|
||||
if (!realPath || state.visitedRealPaths.has(realPath)) return;
|
||||
state.visitedRealPaths.add(realPath);
|
||||
|
||||
let entries: Dirent[];
|
||||
try {
|
||||
@@ -197,9 +204,10 @@ function collectWorkspaceRepos(
|
||||
}
|
||||
|
||||
for (const entry of entries) {
|
||||
if (state.visitedRealPaths.size >= state.limit) return;
|
||||
if (SKIP_DIRS.has(entry.name)) continue;
|
||||
if (!entry.isDirectory() && !entry.isSymbolicLink()) continue;
|
||||
collectWorkspaceRepos(root, resolve(current, entry.name), visitedRealPaths, results);
|
||||
collectWorkspaceRepos(root, resolve(current, entry.name), state, results);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -210,11 +218,22 @@ function collectWorkspaceRepos(
|
||||
* retained for labels and file routing. Each canonical directory is traversed
|
||||
* at most once, so cycles and duplicate aliases cannot duplicate repositories.
|
||||
* Unreadable, broken, and non-directory entries are ignored.
|
||||
*
|
||||
* The walk visits at most `PLANNOTATOR_FILE_BROWSER_MAX_FILES` directories.
|
||||
* Symlinks may legitimately lead outside the workspace root (that is how
|
||||
* symlinked repos are discovered), so a link into a huge unrelated tree cannot
|
||||
* be fenced by path — the visit budget is what keeps discovery from scanning
|
||||
* indefinitely before the server starts.
|
||||
*/
|
||||
export function discoverWorkspaceRepoPaths(root: string): string[] {
|
||||
const resolvedRoot = resolve(root);
|
||||
const results: string[] = [];
|
||||
collectWorkspaceRepos(resolvedRoot, resolvedRoot, new Set<string>(), results);
|
||||
collectWorkspaceRepos(
|
||||
resolvedRoot,
|
||||
resolvedRoot,
|
||||
{ visitedRealPaths: new Set<string>(), limit: getFileBrowserMaxFiles() },
|
||||
results,
|
||||
);
|
||||
return results.sort();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user