Skip to content

Commit ae5ae50

Browse files
author
preflight-agent
committed
fix: remove shell syntax from git.run() calls across 8 tools (#172)
Replace shell pipes, redirects, and non-git commands that were passed as literal args to execFileSync('git', args): - verify-completion: use fs.readFileSync for package.json, execFileSync for tsc/test/build commands instead of run() with pipes - token-audit: use fs for line counting and file size instead of wc/tail - session-handoff: use 'which' + execFileSync('gh') instead of shell - audit-workspace: use fs.readdirSync for test file counting - sharpen-followup: use array args for git diff/status - scope-work: use JS regex filter instead of shell grep pipes - enrich-agent-task: use JS filter/slice instead of grep/head pipes - sequence-tasks: use array args + JS slice instead of shell pipes All 43 tests pass, clean build.
1 parent 4637eaf commit ae5ae50

9 files changed

Lines changed: 165 additions & 76 deletions

package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/tools/audit-workspace.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
22
import { run } from "../lib/git.js";
3-
import { readIfExists, findWorkspaceDocs } from "../lib/files.js";
3+
import { readIfExists, findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js";
4+
import { readdirSync, statSync, existsSync } from "fs";
5+
import { join } from "path";
46

57
/** Extract top-level work areas from file paths generically */
68
function detectWorkAreas(files: string[]): Set<string> {
@@ -36,7 +38,8 @@ export function registerAuditWorkspace(server: McpServer): void {
3638
{},
3739
async () => {
3840
const docs = findWorkspaceDocs();
39-
const recentFiles = run("git diff --name-only HEAD~10 2>/dev/null || echo ''").split("\n").filter(Boolean);
41+
const recentFilesRaw = run(["diff", "--name-only", "HEAD~10"]);
42+
const recentFiles = recentFilesRaw.startsWith("[") ? [] : recentFilesRaw.split("\n").filter(Boolean);
4043
const sections: string[] = [];
4144

4245
// Doc freshness
@@ -75,7 +78,22 @@ export function registerAuditWorkspace(server: McpServer): void {
7578
// Check for gap trackers or similar tracking docs
7679
const trackingDocs = Object.entries(docs).filter(([n]) => /gap|track|progress/i.test(n));
7780
if (trackingDocs.length > 0) {
78-
const testFilesCount = parseInt(run("find tests -name '*.spec.ts' -o -name '*.test.ts' 2>/dev/null | wc -l").trim()) || 0;
81+
// Count test files using Node.js fs instead of shell find|wc
82+
let testFilesCount = 0;
83+
const testsDir = join(PROJECT_DIR, "tests");
84+
if (existsSync(testsDir)) {
85+
const countTests = (dir: string): number => {
86+
let count = 0;
87+
try {
88+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
89+
if (entry.isDirectory()) count += countTests(join(dir, entry.name));
90+
else if (/\.(spec|test)\.(ts|tsx|js|jsx)$/.test(entry.name)) count++;
91+
}
92+
} catch { /* skip unreadable dirs */ }
93+
return count;
94+
};
95+
testFilesCount = countTests(testsDir);
96+
}
7997
sections.push(`## Tracking Docs\n${trackingDocs.map(([n]) => {
8098
const age = docStatus.find(d => d.name === n)?.ageHours ?? "?";
8199
return `- .claude/${n} — last updated ${age}h ago`;

src/tools/enrich-agent-task.ts

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ import { execFileSync } from "child_process";
88
import { join, basename } from "path";
99
import { createHash } from "crypto";
1010

11-
/** Sanitize user input for safe use in shell commands */
12-
function shellEscape(s: string): string {
13-
return s.replace(/[^a-zA-Z0-9_\-./]/g, "");
14-
}
15-
1611
/** Detect package manager from lockfiles */
1712
function detectPackageManager(): string {
1813
if (existsSync(join(PROJECT_DIR, "pnpm-lock.yaml"))) return "pnpm";
@@ -25,35 +20,48 @@ function detectPackageManager(): string {
2520
function findAreaFiles(area: string): string {
2621
if (!area) return getDiffFiles("HEAD~3");
2722

28-
const safeArea = shellEscape(area);
29-
30-
// If area looks like a path, search directly
23+
// If area looks like a path, search directly with git ls-files glob
3124
if (area.includes("/")) {
32-
return run(`git ls-files -- '${safeArea}*' 2>/dev/null | head -20`);
25+
const result = run(["ls-files", "--", `${area}*`]);
26+
if (result && !result.startsWith("[")) {
27+
return result.split("\n").filter(Boolean).slice(0, 20).join("\n");
28+
}
3329
}
3430

35-
// Search for area keyword in git-tracked file paths
36-
const files = run(`git ls-files 2>/dev/null | grep -i '${safeArea}' | head -20`);
37-
if (files && !files.startsWith("[command failed")) return files;
31+
// Search for area keyword in git-tracked file paths using JS filter
32+
const allFiles = run(["ls-files"]);
33+
if (allFiles && !allFiles.startsWith("[")) {
34+
const areaLower = area.toLowerCase();
35+
const matched = allFiles.split("\n").filter(f => f.toLowerCase().includes(areaLower)).slice(0, 20);
36+
if (matched.length > 0) return matched.join("\n");
37+
}
3838

3939
// Fallback to recently changed files
4040
return getDiffFiles("HEAD~3");
4141
}
4242

4343
/** Find related test files for an area */
4444
function findRelatedTests(area: string): string {
45-
if (!area) return run("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10");
46-
47-
const safeArea = shellEscape(area.split(/\s+/)[0]);
48-
const tests = run(`git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | grep -i '${safeArea}' | head -10`);
49-
return tests || run("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10");
45+
const allFiles = run(["ls-files"]);
46+
if (!allFiles || allFiles.startsWith("[")) return "";
47+
const testFiles = allFiles.split("\n").filter(f => /\.(spec|test)\.(ts|tsx|js|jsx)$/.test(f));
48+
if (!area) return testFiles.slice(0, 10).join("\n");
49+
50+
const areaLower = area.split(/\s+/)[0].toLowerCase();
51+
const matched = testFiles.filter(f => f.toLowerCase().includes(areaLower)).slice(0, 10);
52+
return matched.length > 0 ? matched.join("\n") : testFiles.slice(0, 10).join("\n");
5053
}
5154

5255
/** Get an example pattern from the first matching file */
5356
function getExamplePattern(files: string): string {
5457
const firstFile = files.split("\n").filter(Boolean)[0];
5558
if (!firstFile) return "no pattern available";
56-
return run(`head -30 '${shellEscape(firstFile)}' 2>/dev/null || echo 'could not read file'`);
59+
try {
60+
const filePath = join(PROJECT_DIR, firstFile);
61+
if (!existsSync(filePath)) return "could not read file";
62+
const content = readFileSync(filePath, "utf-8");
63+
return content.split("\n").slice(0, 30).join("\n");
64+
} catch { return "could not read file"; }
5765
}
5866

5967
// ---------------------------------------------------------------------------

src/tools/scope-work.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@ const STOP_WORDS = new Set([
1717
"like", "some", "each", "only", "need", "want", "please", "update", "change",
1818
]);
1919

20-
/** Shell-escape a string for use inside single quotes */
21-
function shellEscape(s: string): string {
22-
return s.replace(/'/g, "'\\''");
23-
}
20+
/* shellEscape removed — no longer needed */
2421

2522
/** Safely parse git porcelain status lines */
2623
function parsePortelainFiles(porcelain: string): string[] {
@@ -127,8 +124,12 @@ export function registerScopeWork(server: McpServer): void {
127124
.filter((k) => k.length > 2)
128125
.slice(0, 5);
129126
if (grepTerms.length > 0) {
130-
const pattern = shellEscape(grepTerms.join("|"));
131-
matchedFiles = run(`git ls-files | head -500 | grep -iE '${pattern}' | head -30`);
127+
const allFiles = run(["ls-files"]);
128+
if (allFiles && !allFiles.startsWith("[")) {
129+
const regex = new RegExp(grepTerms.join("|"), "i");
130+
matchedFiles = allFiles.split("\n").filter(Boolean).slice(0, 500)
131+
.filter(f => regex.test(f)).slice(0, 30).join("\n");
132+
}
132133
}
133134

134135
// Check which relevant dirs actually exist (with path traversal protection)

src/tools/sequence-tasks.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ export function registerSequenceTasks(server: McpServer): void {
9090
// For locality: infer directories from path-like tokens in task text
9191
if (strategy === "locality") {
9292
// Use git ls-files with a depth limit instead of find for performance
93-
const gitFiles = run("git ls-files 2>/dev/null | head -1000");
93+
const gitFilesRaw = run(["ls-files"]);
94+
const gitFiles = gitFilesRaw.startsWith("[") ? "" : gitFilesRaw.split("\n").slice(0, 1000).join("\n");
9495
const knownDirs = new Set<string>();
9596
for (const f of gitFiles.split("\n").filter(Boolean)) {
9697
const parts = f.split("/");

src/tools/session-handoff.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
import { z } from "zod";
22
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
33
import { existsSync, readFileSync } from "fs";
4+
import { execFileSync } from "child_process";
45
import { join } from "path";
56
import { run, getBranch, getRecentCommits, getStatus } from "../lib/git.js";
6-
import { readIfExists, findWorkspaceDocs } from "../lib/files.js";
7+
import { readIfExists, findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js";
78
import { STATE_DIR, now } from "../lib/state.js";
89

910
/** Check if a CLI tool is available */
1011
function hasCommand(cmd: string): boolean {
11-
const result = run(`command -v ${cmd} 2>/dev/null`);
12-
return !!result && !result.startsWith("[command failed");
12+
try {
13+
execFileSync("which", [cmd], { encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] });
14+
return true;
15+
} catch { return false; }
1316
}
1417

1518
export function registerSessionHandoff(server: McpServer): void {
@@ -44,7 +47,13 @@ export function registerSessionHandoff(server: McpServer): void {
4447

4548
// Only try gh if it exists
4649
if (hasCommand("gh")) {
47-
const openPRs = run("gh pr list --state open --json number,title,headRefName 2>/dev/null || echo '[]'");
50+
let openPRs = "[]";
51+
try {
52+
openPRs = execFileSync("gh", ["pr", "list", "--state", "open", "--json", "number,title,headRefName"], {
53+
cwd: PROJECT_DIR, encoding: "utf-8", timeout: 10000,
54+
stdio: ["pipe", "pipe", "pipe"],
55+
}).trim();
56+
} catch { /* gh not available or not in a repo */ }
4857
if (openPRs && openPRs !== "[]") {
4958
sections.push(`## Open PRs\n\`\`\`json\n${openPRs}\n\`\`\``);
5059
}

src/tools/sharpen-followup.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ function parsePortelainFiles(output: string): string[] {
2727
/** Get recently changed files, safe for first commit / shallow clones */
2828
function getRecentChangedFiles(): string[] {
2929
// Try HEAD~1..HEAD, fall back to just staged, then unstaged
30-
const commands = [
31-
"git diff --name-only HEAD~1 HEAD 2>/dev/null",
32-
"git diff --name-only --cached 2>/dev/null",
33-
"git diff --name-only 2>/dev/null",
30+
const commands: string[][] = [
31+
["diff", "--name-only", "HEAD~1", "HEAD"],
32+
["diff", "--name-only", "--cached"],
33+
["diff", "--name-only"],
3434
];
3535
const results = new Set<string>();
36-
for (const cmd of commands) {
37-
const out = run(cmd);
38-
if (out) out.split("\n").filter(Boolean).forEach((f) => results.add(f));
36+
for (const args of commands) {
37+
const out = run(args);
38+
if (out && !out.startsWith("[")) out.split("\n").filter(Boolean).forEach((f) => results.add(f));
3939
if (results.size > 0) break; // first successful source is enough
4040
}
4141
return [...results];
@@ -87,7 +87,7 @@ export function registerSharpenFollowup(server: McpServer): void {
8787
// Gather context to resolve ambiguity
8888
const contextFiles: string[] = [...(previous_files ?? [])];
8989
const recentChanged = getRecentChangedFiles();
90-
const porcelainOutput = run("git status --porcelain 2>/dev/null");
90+
const porcelainOutput = run(["status", "--porcelain"]);
9191
const untrackedOrModified = parsePortelainFiles(porcelainOutput);
9292

9393
const allKnownFiles = [...new Set([...contextFiles, ...recentChanged, ...untrackedOrModified])].filter(Boolean);

src/tools/token-audit.ts

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,9 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
44
import { run } from "../lib/git.js";
55
import { readIfExists, findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js";
66
import { loadState, saveState, now, STATE_DIR } from "../lib/state.js";
7-
import { readFileSync, existsSync, statSync } from "fs";
7+
import { readFileSync, existsSync, statSync, openSync, readSync, closeSync } from "fs";
88
import { join } from "path";
99

10-
/** Shell-escape a filename for safe interpolation */
11-
function shellEscape(s: string): string {
12-
return s.replace(/'/g, "'\\''");
13-
}
14-
1510
/**
1611
* Grade thresholds rationale:
1712
* - A (0-10): Minimal waste — small diffs, targeted reads, lean context
@@ -39,8 +34,8 @@ export function registerTokenAudit(server: McpServer): void {
3934
let wasteScore = 0;
4035

4136
// 1. Git diff size & dirty file count
42-
const diffStat = run("git diff --stat --no-color 2>/dev/null");
43-
const dirtyFiles = run("git diff --name-only 2>/dev/null");
37+
const diffStat = run(["diff", "--stat", "--no-color"]);
38+
const dirtyFiles = run(["diff", "--name-only"]);
4439
const dirtyList = dirtyFiles.split("\n").filter(Boolean);
4540
const dirtyCount = dirtyList.length;
4641

@@ -62,9 +57,15 @@ export function registerTokenAudit(server: McpServer): void {
6257
const largeFiles: string[] = [];
6358

6459
for (const f of dirtyList.slice(0, 30)) {
65-
// Use shell-safe quoting instead of interpolation
66-
const wc = run(`wc -l < '${shellEscape(f)}' 2>/dev/null`);
67-
const lines = parseInt(wc) || 0;
60+
// Count lines using Node.js fs instead of shell wc
61+
let lines = 0;
62+
try {
63+
const filePath = join(PROJECT_DIR, f);
64+
if (existsSync(filePath)) {
65+
const content = readFileSync(filePath, "utf-8");
66+
lines = content.split("\n").length;
67+
}
68+
} catch { /* skip unreadable files */ }
6869
estimatedContextTokens += lines * AVG_LINE_BYTES * AVG_TOKENS_PER_BYTE;
6970
if (lines > 500) {
7071
largeFiles.push(`${f} (${lines} lines)`);
@@ -80,8 +81,11 @@ export function registerTokenAudit(server: McpServer): void {
8081
// 3. CLAUDE.md bloat check
8182
const claudeMd = readIfExists("CLAUDE.md", 1);
8283
if (claudeMd !== null) {
83-
const stat = run(`wc -c < '${shellEscape("CLAUDE.md")}' 2>/dev/null`);
84-
const bytes = parseInt(stat) || 0;
84+
let bytes = 0;
85+
try {
86+
const claudePath = join(PROJECT_DIR, "CLAUDE.md");
87+
if (existsSync(claudePath)) bytes = statSync(claudePath).size;
88+
} catch { /* ignore */ }
8589
if (bytes > 5120) {
8690
patterns.push(`CLAUDE.md is ${(bytes / 1024).toFixed(1)}KB — injected every session, burns tokens on paste`);
8791
recommendations.push("Trim CLAUDE.md to essentials (<5KB). Move reference docs to files read on-demand");
@@ -137,9 +141,16 @@ export function registerTokenAudit(server: McpServer): void {
137141
}
138142

139143
// Read with size cap: take the tail if too large
140-
const raw = stat.size <= MAX_TOOL_LOG_BYTES
141-
? readFileSync(toolLogPath, "utf-8")
142-
: run(`tail -c ${MAX_TOOL_LOG_BYTES} '${shellEscape(toolLogPath)}'`);
144+
let raw: string;
145+
if (stat.size <= MAX_TOOL_LOG_BYTES) {
146+
raw = readFileSync(toolLogPath, "utf-8");
147+
} else {
148+
const fd = openSync(toolLogPath, "r");
149+
const buf = Buffer.alloc(MAX_TOOL_LOG_BYTES);
150+
readSync(fd, buf, 0, MAX_TOOL_LOG_BYTES, stat.size - MAX_TOOL_LOG_BYTES);
151+
closeSync(fd);
152+
raw = buf.toString("utf-8");
153+
}
143154

144155
const lines = raw.trim().split("\n").filter(Boolean);
145156
totalToolCalls = lines.length;

0 commit comments

Comments
 (0)