Skip to content

Commit 90a358e

Browse files
committed
feat(file-edit): add input validation for edit parameters to prevent errors
1 parent ac3e558 commit 90a358e

3 files changed

Lines changed: 52 additions & 4 deletions

File tree

src/tools/file/file-edit.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,47 @@ export const edit = defineTool({
7373
const code = params.code;
7474
const relativePath = path.relative(getHostWorkspace(), filePath).replace(/\\/g, '/');
7575

76-
// Determine edit range based on targeting priority
76+
// ── Input validation ──────────────────────────────────────────
77+
78+
// Bug #5: startLine > endLine
79+
if (params.startLine !== undefined && params.endLine !== undefined && params.startLine > params.endLine) {
80+
response.appendResponseLine(
81+
`❌ Invalid line range: startLine (${params.startLine}) is greater than endLine (${params.endLine}).`,
82+
);
83+
return;
84+
}
85+
86+
// Bug #2: Empty code targeting a symbol = accidental deletion
87+
if (params.target && code.trim().length === 0) {
88+
response.appendResponseLine(
89+
`❌ Refusing to apply empty code to symbol "${params.target}" — this would delete it. ` +
90+
`If deletion is intended, remove the symbol explicitly or use startLine/endLine.`,
91+
);
92+
return;
93+
}
94+
95+
// Bug #6: Validate line ranges are within file bounds
96+
if (params.startLine !== undefined || params.endLine !== undefined) {
97+
try {
98+
const contentResult = await fileReadContent(filePath);
99+
const totalLines = contentResult.totalLines;
100+
101+
if (params.startLine !== undefined && (params.startLine < 1 || params.startLine > totalLines)) {
102+
response.appendResponseLine(
103+
`❌ startLine ${params.startLine} is out of bounds (file has ${totalLines} lines).`,
104+
);
105+
return;
106+
}
107+
if (params.endLine !== undefined && (params.endLine < 1 || params.endLine > totalLines)) {
108+
response.appendResponseLine(
109+
`❌ endLine ${params.endLine} is out of bounds (file has ${totalLines} lines).`,
110+
);
111+
return;
112+
}
113+
} catch {
114+
// File might not exist yet; proceed and let the edit fail naturally
115+
}
116+
}
77117
let editStartLine: number;
78118
let editEndLine: number;
79119
let targetLabel: string | undefined;

src/tools/file/safety-layer.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export async function executeEditWithSafetyLayer(
5656
let preExistingErrors = 0;
5757
try {
5858
const preDiags = await fileGetDiagnostics(filePath);
59-
preExistingErrors = preDiags.diagnostics.filter(d => d.severity === 'Error').length;
59+
preExistingErrors = preDiags.diagnostics.filter(d => d.severity.toLowerCase() === 'error').length;
6060
} catch {
6161
// Best-effort
6262
}
@@ -128,7 +128,7 @@ export async function executeEditWithSafetyLayer(
128128

129129
try {
130130
const postDiags = await fileGetDiagnostics(filePath);
131-
const newErrors = postDiags.diagnostics.filter(d => d.severity === 'Error');
131+
const newErrors = postDiags.diagnostics.filter(d => d.severity.toLowerCase() === 'error');
132132

133133
let fixAttempts = 0;
134134
for (const error of newErrors) {
@@ -167,7 +167,7 @@ export async function executeEditWithSafetyLayer(
167167
file: filePath,
168168
line: d.line,
169169
message: d.message,
170-
severity: d.severity === 'Error' ? 'error' : 'warning',
170+
severity: d.severity.toLowerCase() === 'error' ? 'error' : 'warning',
171171
});
172172
}
173173
} catch {

src/tools/file/symbol-diff.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ export function diffSymbols(
2222
): DetectedIntent[] {
2323
const intents: DetectedIntent[] = [];
2424

25+
// Guard: all old symbols gone with no replacements → bulk deletion
26+
if (oldSymbols.length > 0 && newSymbols.length === 0) {
27+
for (const s of oldSymbols) {
28+
intents.push({type: 'delete', symbol: s.name});
29+
}
30+
return intents;
31+
}
32+
2533
const oldByName = new Map<string, FileSymbol>();
2634
for (const s of oldSymbols) oldByName.set(s.name, s);
2735

0 commit comments

Comments
 (0)