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.
|
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.
|
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:
|
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 * as projectMemory from "../project-memory.js";
|
||||||
import { AgentStore } from "../agent-store.js";
|
import { AgentStore } from "../agent-store.js";
|
||||||
import { CentralDatabase } from "../central-db.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 { buildResearchDocumentKey, type Task } from "../types.js";
|
||||||
import { createTaskStoreTestHarness, makeTmpDir } from "./store-test-helpers.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", () => {
|
describe("parseFileScopeFromPrompt", () => {
|
||||||
it("returns paths when File Scope is followed by another heading", async () => {
|
it("returns paths when File Scope is followed by another heading", async () => {
|
||||||
const task = await store.createTask({ description: "Mid-file scope" });
|
const task = await store.createTask({ description: "Mid-file scope" });
|
||||||
@@ -265,7 +299,58 @@ describe("TaskStore", () => {
|
|||||||
"packages/engine/src/**/*.ts",
|
"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> {
|
export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||||
static async getOrCreateForProject(
|
static async getOrCreateForProject(
|
||||||
projectId?: string,
|
projectId?: string,
|
||||||
@@ -2855,6 +2952,16 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
?? (task.column === "triage"
|
?? (task.column === "triage"
|
||||||
? buildBootstrapPrompt(id, task.title, task.description)
|
? buildBootstrapPrompt(id, task.title, task.description)
|
||||||
: this.generateSpecifiedPrompt(task));
|
: 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 mkdir(dir, { recursive: true });
|
||||||
await writeFile(join(dir, "PROMPT.md"), prompt);
|
await writeFile(join(dir, "PROMPT.md"), prompt);
|
||||||
|
|
||||||
@@ -2897,6 +3004,15 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
|
|
||||||
const newDir = this.taskDir(newId);
|
const newDir = this.taskDir(newId);
|
||||||
await this.atomicCreateTaskJson(newDir, newTask, "duplicateTask");
|
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 mkdir(newDir, { recursive: true });
|
||||||
await writeFile(join(newDir, "PROMPT.md"), sourceTask.prompt);
|
await writeFile(join(newDir, "PROMPT.md"), sourceTask.prompt);
|
||||||
|
|
||||||
@@ -2963,6 +3079,16 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
const newDir = this.taskDir(newId);
|
const newDir = this.taskDir(newId);
|
||||||
await this.atomicCreateTaskJson(newDir, newTask, "refineTask");
|
await this.atomicCreateTaskJson(newDir, newTask, "refineTask");
|
||||||
const prompt = `# ${newTask.title}\n\n${newTask.description}\n`;
|
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 mkdir(newDir, { recursive: true });
|
||||||
await writeFile(join(newDir, "PROMPT.md"), prompt);
|
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 (this.isWatching) this.taskCache.set(id, { ...task });
|
||||||
|
|
||||||
if (updates.prompt !== undefined) {
|
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 mkdir(dir, { recursive: true });
|
||||||
await writeFile(join(dir, "PROMPT.md"), updates.prompt);
|
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");
|
const content = await readFile(promptPath, "utf-8");
|
||||||
|
|
||||||
// Find the ## File Scope section.
|
const paths = extractFileScopeTokens(content);
|
||||||
// We locate the heading then slice to the next heading (or end of file)
|
return paths.filter((path) => isValidFileScopeEntry(path));
|
||||||
// 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;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async deleteTask(
|
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[] {
|
private rewriteDependentsAndDeleteTask(taskId: string, dependentIds: string[]): Task[] {
|
||||||
const rewrittenDependents: Task[] = [];
|
const rewrittenDependents: Task[] = [];
|
||||||
|
|
||||||
@@ -6875,6 +6995,10 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
|
|
||||||
// Generate PROMPT.md with preserved steps
|
// Generate PROMPT.md with preserved steps
|
||||||
const prompt = entry.prompt ?? this.generatePromptFromArchiveEntry(entry);
|
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 mkdir(dir, { recursive: true });
|
||||||
await writeFile(join(dir, "PROMPT.md"), prompt);
|
await writeFile(join(dir, "PROMPT.md"), prompt);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user