feat(FN-4402): validate file scope tokens in prompt parsing and writes
Merged: Validates file scope tokens during prompt parsing and write operations in the task store, adding ~160 lines of logic to `packages/core/src/store.ts` with corresponding test coverage in `store-parsing.test.ts`. Also updated AGENTS.md documentation and created a changeset for the `@runfusion/f Fusion-Task-Id: FN-4402
This commit is contained in:
5
.changeset/fn-4402-validate-file-scope.md
Normal file
5
.changeset/fn-4402-validate-file-scope.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Validate File Scope entries in PROMPT.md at task-create/update time. Reject git refs (`origin/fusion/fn-4280`), URLs, SHAs, and other non-path tokens with a clear `InvalidFileScopeError`. `parseFileScopeFromPrompt` also silently drops invalid tokens at read time as defense-in-depth, so the file-scope invariant on squash merges is no longer weakened by malformed scope declarations.
|
||||
@@ -241,6 +241,8 @@ Every squash commit path now enforces a file-scope invariant immediately before
|
||||
|
||||
Tasks can opt out per task via `task.scopeOverride = true`; when present, the merger bypasses the invariant and logs `task.scopeOverrideReason` when provided. Empty declared file scopes are not enforced.
|
||||
|
||||
File Scope entries are validated when PROMPT.md is written: non-path tokens (git refs, URLs, SHAs, bare identifiers) are rejected with `InvalidFileScopeError`, and entries must be repo-relative paths/globs.
|
||||
|
||||
When `mergeConflictStrategy="smart-prefer-main"`, the merger also runs an overlap guard before the Attempt 3 `-X ours` fallback. If recent `main` commits (30-commit lookback) touched files the task branch also changed, the default `mergeStrategyOverlapBehavior="flip-to-prefer-branch"` makes those overlapping files prefer the task branch instead of silently discarding branch hardening; `warn-only` preserves the legacy fallback while logging the risk, and `ignore` disables the guard.
|
||||
|
||||
For manual follow-up, standalone auditing, or post-incident inspection, the script remains available:
|
||||
|
||||
@@ -6,7 +6,7 @@ import { existsSync } from "node:fs";
|
||||
import * as projectMemory from "../project-memory.js";
|
||||
import { AgentStore } from "../agent-store.js";
|
||||
import { CentralDatabase } from "../central-db.js";
|
||||
import { TaskStore, TaskHasDependentsError } from "../store.js";
|
||||
import { InvalidFileScopeError, isValidFileScopeEntry, TaskStore, TaskHasDependentsError } from "../store.js";
|
||||
import { buildResearchDocumentKey, type Task } from "../types.js";
|
||||
import { createTaskStoreTestHarness, makeTmpDir } from "./store-test-helpers.js";
|
||||
|
||||
@@ -151,6 +151,40 @@ describe("TaskStore", () => {
|
||||
});
|
||||
|
||||
|
||||
describe("isValidFileScopeEntry", () => {
|
||||
it.each([
|
||||
"packages/core/src/store.ts",
|
||||
"packages/engine/src/**/*.ts",
|
||||
"packages/core/*",
|
||||
"app/*.tsx",
|
||||
"Makefile",
|
||||
"Dockerfile",
|
||||
"AGENTS.md",
|
||||
".changeset/foo-bar.md",
|
||||
"vendor/some-pkg/LICENSE",
|
||||
])("accepts %s", (entry) => {
|
||||
expect(isValidFileScopeEntry(entry)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"fusion/fn-4280",
|
||||
"origin/fusion/fn-4280",
|
||||
"refs/heads/main",
|
||||
"HEAD",
|
||||
"main",
|
||||
"fusion",
|
||||
"https://example.com/a.ts",
|
||||
"git@github.com:owner/repo.git",
|
||||
"deadbeefcafe1234",
|
||||
"../escape/path.ts",
|
||||
"/absolute/path.ts",
|
||||
"",
|
||||
" ",
|
||||
])("rejects %s", (entry) => {
|
||||
expect(isValidFileScopeEntry(entry)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseFileScopeFromPrompt", () => {
|
||||
it("returns paths when File Scope is followed by another heading", async () => {
|
||||
const task = await store.createTask({ description: "Mid-file scope" });
|
||||
@@ -265,7 +299,58 @@ describe("TaskStore", () => {
|
||||
"packages/engine/src/**/*.ts",
|
||||
]);
|
||||
});
|
||||
|
||||
it("drops invalid entries from mixed file scope declarations", async () => {
|
||||
const task = await store.createTask({ description: "Mixed file scope" });
|
||||
const dir = join(rootDir, ".fusion", "tasks", task.id);
|
||||
await writeFile(
|
||||
join(dir, "PROMPT.md"),
|
||||
`# ${task.id}: Mixed file scope
|
||||
|
||||
## File Scope
|
||||
|
||||
- \`packages/dashboard/app/components/TaskDetailModal.tsx\`
|
||||
- \`fusion/fn-4280\`
|
||||
- \`origin/fusion/fn-4280\`
|
||||
`,
|
||||
);
|
||||
|
||||
const paths = await store.parseFileScopeFromPrompt(task.id);
|
||||
expect(paths).toEqual(["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("File Scope validation at write time", () => {
|
||||
it("createTask rejects invalid File Scope entries and rolls back", async () => {
|
||||
const badPrompt = `# Bad prompt\n\n## File Scope\n\n- \`packages/core/src/store.ts\`\n- \`origin/fusion/fn-4280\`\n`;
|
||||
|
||||
await expect(store.createTaskWithReservedId({ description: "bad create" }, { taskId: "FN-999", prompt: badPrompt }))
|
||||
.rejects.toBeInstanceOf(InvalidFileScopeError);
|
||||
|
||||
await expect(store.getTask("FN-999")).rejects.toThrow(/not found/i);
|
||||
expect(existsSync(join(rootDir, ".fusion", "tasks", "FN-999"))).toBe(false);
|
||||
});
|
||||
|
||||
it("updateTask rejects invalid File Scope prompt and preserves existing PROMPT.md", async () => {
|
||||
const task = await store.createTask({ description: "update scope" });
|
||||
const promptPath = join(rootDir, ".fusion", "tasks", task.id, "PROMPT.md");
|
||||
const originalPrompt = await readFile(promptPath, "utf-8");
|
||||
const invalidPrompt = `# ${task.id}: invalid\n\n## File Scope\n\n- \`refs/heads/main\`\n`;
|
||||
|
||||
await expect(store.updateTask(task.id, { prompt: invalidPrompt }))
|
||||
.rejects.toBeInstanceOf(InvalidFileScopeError);
|
||||
|
||||
expect(await readFile(promptPath, "utf-8")).toBe(originalPrompt);
|
||||
});
|
||||
|
||||
it("updateTask accepts valid File Scope prompt", async () => {
|
||||
const task = await store.createTask({ description: "update scope valid" });
|
||||
const promptPath = join(rootDir, ".fusion", "tasks", task.id, "PROMPT.md");
|
||||
const validPrompt = `# ${task.id}: valid\n\n## File Scope\n\n- \`packages/core/src/store.ts\`\n- \`packages/core/*\`\n`;
|
||||
|
||||
await store.updateTask(task.id, { prompt: validPrompt });
|
||||
expect(await readFile(promptPath, "utf-8")).toBe(validPrompt);
|
||||
});
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -520,6 +520,103 @@ export class TaskHasDependentsError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
export class InvalidFileScopeError extends Error {
|
||||
readonly taskId: string;
|
||||
readonly invalidEntries: string[];
|
||||
|
||||
constructor(taskId: string, invalidEntries: string[]) {
|
||||
super(
|
||||
`Invalid File Scope entries in PROMPT.md for ${taskId}: ${invalidEntries.join(", ")}. ` +
|
||||
"File Scope must contain repo-relative file paths or globs (e.g. `packages/core/src/store.ts`, `packages/engine/src/**/*.ts`), not git refs or identifiers.",
|
||||
);
|
||||
this.name = "InvalidFileScopeError";
|
||||
this.taskId = taskId;
|
||||
this.invalidEntries = invalidEntries;
|
||||
}
|
||||
}
|
||||
|
||||
const KNOWN_FILE_SCOPE_ROOT_FILES = new Set([
|
||||
"makefile",
|
||||
"dockerfile",
|
||||
"justfile",
|
||||
"license",
|
||||
"readme",
|
||||
"changelog",
|
||||
"agents.md",
|
||||
]);
|
||||
|
||||
export function isValidFileScopeEntry(token: string): boolean {
|
||||
const trimmed = token.trim();
|
||||
if (!trimmed) return false;
|
||||
|
||||
const lower = trimmed.toLowerCase();
|
||||
if (
|
||||
lower.startsWith("origin/")
|
||||
|| lower.startsWith("upstream/")
|
||||
|| lower.startsWith("refs/")
|
||||
|| /^https?:\/\//i.test(trimmed)
|
||||
|| /^git@/i.test(trimmed)
|
||||
|| /^ssh:\/\//i.test(trimmed)
|
||||
|| /^[a-z]+\/fn-\d+$/i.test(trimmed)
|
||||
|| /^[a-f0-9]{7,}$/i.test(trimmed)
|
||||
|| trimmed.includes("..")
|
||||
|| trimmed.startsWith("/")
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const segments = trimmed.split("/");
|
||||
const lastSegment = segments[segments.length - 1];
|
||||
const hasSlash = trimmed.includes("/");
|
||||
const hasDotInLastSegment = lastSegment.includes(".");
|
||||
|
||||
if (KNOWN_FILE_SCOPE_ROOT_FILES.has(lastSegment.toLowerCase())) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (trimmed.includes("**") || trimmed.endsWith("/*") || (lastSegment.includes("*") && hasDotInLastSegment)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (hasSlash && hasDotInLastSegment) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
function extractFileScopeTokens(content: string): string[] {
|
||||
const headingMatch = content.match(/^##\s+File\s+Scope\s*$/m);
|
||||
if (!headingMatch) return [];
|
||||
|
||||
const startIdx = headingMatch.index! + headingMatch[0].length;
|
||||
const rest = content.slice(startIdx);
|
||||
const nextHeading = rest.search(/\n##?\s/);
|
||||
const section = nextHeading === -1 ? rest : rest.slice(0, nextHeading);
|
||||
const tokens: string[] = [];
|
||||
const backtickRegex = /`([^`]+)`/g;
|
||||
let match;
|
||||
while ((match = backtickRegex.exec(section)) !== null) {
|
||||
tokens.push(match[1]);
|
||||
}
|
||||
|
||||
return tokens;
|
||||
}
|
||||
|
||||
function validateFileScopeInPromptContent(prompt: string): { valid: string[]; invalid: string[] } {
|
||||
const tokens = extractFileScopeTokens(prompt);
|
||||
const valid: string[] = [];
|
||||
const invalid: string[] = [];
|
||||
for (const token of tokens) {
|
||||
if (isValidFileScopeEntry(token)) {
|
||||
valid.push(token);
|
||||
} else {
|
||||
invalid.push(token);
|
||||
}
|
||||
}
|
||||
return { valid, invalid };
|
||||
}
|
||||
|
||||
export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
static async getOrCreateForProject(
|
||||
projectId?: string,
|
||||
@@ -2855,6 +2952,16 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
?? (task.column === "triage"
|
||||
? buildBootstrapPrompt(id, task.title, task.description)
|
||||
: this.generateSpecifiedPrompt(task));
|
||||
const validation = validateFileScopeInPromptContent(prompt);
|
||||
if (validation.invalid.length > 0) {
|
||||
if (this.isWatching) this.taskCache.delete(id);
|
||||
this.deleteTaskById(id);
|
||||
const { rm } = await import("node:fs/promises");
|
||||
if (existsSync(dir)) {
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
throw new InvalidFileScopeError(id, validation.invalid);
|
||||
}
|
||||
await mkdir(dir, { recursive: true });
|
||||
await writeFile(join(dir, "PROMPT.md"), prompt);
|
||||
|
||||
@@ -2897,6 +3004,15 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
const newDir = this.taskDir(newId);
|
||||
await this.atomicCreateTaskJson(newDir, newTask, "duplicateTask");
|
||||
const validation = validateFileScopeInPromptContent(sourceTask.prompt);
|
||||
if (validation.invalid.length > 0) {
|
||||
this.deleteTaskById(newId);
|
||||
const { rm } = await import("node:fs/promises");
|
||||
if (existsSync(newDir)) {
|
||||
await rm(newDir, { recursive: true, force: true });
|
||||
}
|
||||
throw new InvalidFileScopeError(newId, validation.invalid);
|
||||
}
|
||||
await mkdir(newDir, { recursive: true });
|
||||
await writeFile(join(newDir, "PROMPT.md"), sourceTask.prompt);
|
||||
|
||||
@@ -2963,6 +3079,16 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
const newDir = this.taskDir(newId);
|
||||
await this.atomicCreateTaskJson(newDir, newTask, "refineTask");
|
||||
const prompt = `# ${newTask.title}\n\n${newTask.description}\n`;
|
||||
// Defensive no-op for refine prompts, which typically have no File Scope section.
|
||||
const validation = validateFileScopeInPromptContent(prompt);
|
||||
if (validation.invalid.length > 0) {
|
||||
this.deleteTaskById(newId);
|
||||
const { rm } = await import("node:fs/promises");
|
||||
if (existsSync(newDir)) {
|
||||
await rm(newDir, { recursive: true, force: true });
|
||||
}
|
||||
throw new InvalidFileScopeError(newId, validation.invalid);
|
||||
}
|
||||
await mkdir(newDir, { recursive: true });
|
||||
await writeFile(join(newDir, "PROMPT.md"), prompt);
|
||||
|
||||
@@ -4089,6 +4215,10 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
if (this.isWatching) this.taskCache.set(id, { ...task });
|
||||
|
||||
if (updates.prompt !== undefined) {
|
||||
const validation = validateFileScopeInPromptContent(updates.prompt);
|
||||
if (validation.invalid.length > 0) {
|
||||
throw new InvalidFileScopeError(id, validation.invalid);
|
||||
}
|
||||
await mkdir(dir, { recursive: true });
|
||||
await writeFile(join(dir, "PROMPT.md"), updates.prompt);
|
||||
}
|
||||
@@ -4616,24 +4746,8 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
const content = await readFile(promptPath, "utf-8");
|
||||
|
||||
// Find the ## File Scope section.
|
||||
// We locate the heading then slice to the next heading (or end of file)
|
||||
// to avoid multiline `$` anchor issues with lazy quantifiers.
|
||||
const headingMatch = content.match(/^##\s+File\s+Scope\s*$/m);
|
||||
if (!headingMatch) return [];
|
||||
|
||||
const startIdx = headingMatch.index! + headingMatch[0].length;
|
||||
const rest = content.slice(startIdx);
|
||||
const nextHeading = rest.search(/\n##?\s/);
|
||||
const section = nextHeading === -1 ? rest : rest.slice(0, nextHeading);
|
||||
const paths: string[] = [];
|
||||
const backtickRegex = /`([^`]+)`/g;
|
||||
let match;
|
||||
while ((match = backtickRegex.exec(section)) !== null) {
|
||||
paths.push(match[1]);
|
||||
}
|
||||
|
||||
return paths;
|
||||
const paths = extractFileScopeTokens(content);
|
||||
return paths.filter((path) => isValidFileScopeEntry(path));
|
||||
}
|
||||
|
||||
async deleteTask(
|
||||
@@ -4688,6 +4802,12 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
});
|
||||
}
|
||||
|
||||
private deleteTaskById(taskId: string): void {
|
||||
this.clearLinkedAgentTaskIds(taskId);
|
||||
this.db.prepare('DELETE FROM tasks WHERE id = ?').run(taskId);
|
||||
this.db.bumpLastModified();
|
||||
}
|
||||
|
||||
private rewriteDependentsAndDeleteTask(taskId: string, dependentIds: string[]): Task[] {
|
||||
const rewrittenDependents: Task[] = [];
|
||||
|
||||
@@ -6875,6 +6995,10 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
// Generate PROMPT.md with preserved steps
|
||||
const prompt = entry.prompt ?? this.generatePromptFromArchiveEntry(entry);
|
||||
const validation = validateFileScopeInPromptContent(prompt);
|
||||
if (validation.invalid.length > 0) {
|
||||
throw new InvalidFileScopeError(entry.id, validation.invalid);
|
||||
}
|
||||
await mkdir(dir, { recursive: true });
|
||||
await writeFile(join(dir, "PROMPT.md"), prompt);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user