🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🔵 Sandbox memory flush “append-only” implemented as non-atomic read-then-rewrite (lost updates / overwrite risk)

Description

The sandbox-mode implementation of appendMemoryFlushContent() does not perform a true append. Instead it:

• Reads the full existing file content into memory (readOptionalUtf8File())
• Concatenates the new content in-process
• Calls sandbox.bridge.writeFile() to rewrite the entire file

This creates a race window and breaks the intended append-only guarantee:

• Lost updates / clobbering: concurrent writers (another agent, user process, or another flush) can modify/create the file between the read and the final write, and their changes will be overwritten by the stale existing snapshot.
• TOCTOU: stat() is used as an existence check before readFile(), and if the file is created after stat() returns null, the code will treat it as empty and then rewrite it.
• DoS risk: reading the entire existing file into memory scales with file size; an unexpectedly large memory/YYYY-MM-DD.md can cause high memory usage.

Vulnerable code:

const existing = await readOptionalUtf8File({ ... });
const next = `${existing}${separator}${params.content}`;
await params.sandbox.bridge.writeFile({ filePath: params.relativePath, data: next, ... });

Recommendation

Implement a real append primitive for sandbox mode rather than read+rewrite.

Recommended approach:

1. Add appendFile (or openAppend) to SandboxFsBridge that:
   • Validates path confinement (same boundary checks as writeFile)
   • Opens the destination with O_APPEND and O_NOFOLLOW (and rejects hardlinks) on the host path (similar to host-side appendFileWithinRoot)
   • Writes only the new bytes (optionally checking last byte for newline)

Example sketch:

​// in SandboxFsBridge
async appendFile(params: { filePath: string; cwd?: string; data: Buffer | string }) {
  const target = this.resolveResolvedPath(params);
  this.ensureWriteAccess(target, "append files");
  await this.pathGuard.assertPathSafety(target, { action: "append files", requireWritable: true });

  const fd = fs.openSync(target.hostPath,
    fs.constants.O_WRONLY | fs.constants.O_APPEND | fs.constants.O_NOFOLLOW);
  try {
    fs.writeSync(fd, Buffer.isBuffer(params.data) ? params.data : Buffer.from(params.data, "utf8"));
  } finally {
    fs.closeSync(fd);
  }
}

2. Update appendMemoryFlushContent() sandbox branch to call bridge.appendFile() and avoid reading the entire file.

If an append primitive cannot be added immediately, a mitigation is to do a compare-and-swap style write (e.g., verify mtime/size after read before commit), but true append is preferable.

Analyzed PR: #38574 at commit a0b9a02

_{Last updated on: 2026-03-10T05:29:11Z}

Greptile Summary

This PR hardens the memory-flush feature by injecting three prompt-level guardrails — a target-path hint, an append-only hint, and a read-only hint for workspace bootstrap files — into both the default prompt/systemPrompt and any custom overrides supplied via config. The new ensureMemoryFlushSafetyHints() helper is applied in resolveMemoryFlushSettings before ensureNoReplyHint, ensuring the contract holds regardless of whether a user has customised the flush prompts.

Key changes:

• Three new exported-private constants (MEMORY_FLUSH_TARGET_HINT, MEMORY_FLUSH_APPEND_ONLY_HINT, MEMORY_FLUSH_READ_ONLY_HINT) centralise the guardrail wording so it can't drift between prompt and system-prompt.
• ensureMemoryFlushSafetyHints() iterates MEMORY_FLUSH_REQUIRED_HINTS and appends any hint not already present — a no-op for the defaults, which already embed all hints.
• Unit and e2e tests are updated to assert the hints appear in both prompt and extraSystemPrompt for default and custom-override paths.
• The YYYY-MM-DD placeholder in the user-facing prompt is correctly resolved to an actual date by the pre-existing resolveMemoryFlushPromptForRun; the system-prompt retains the literal placeholder, consistent with its role as format guidance.

Minor observation (not a bug): the ordering of hints in MEMORY_FLUSH_REQUIRED_HINTS (TARGET → APPEND_ONLY → READ_ONLY) differs from the ordering in DEFAULT_MEMORY_FLUSH_PROMPT (TARGET → READ_ONLY → APPEND_ONLY). Because ensureMemoryFlushSafetyHints uses substring includes checks rather than ordering, this has no functional impact — but aligning the array order to match the default-prompt order would remove a small source of confusion for future maintainers.

Confidence Score: 5/5

• This PR is safe to merge — it only hardens prompt text and adds test coverage with no behaviour changes outside the flush-prompt contract.
• Changes are narrowly scoped to prompt-string construction in memory-flush.ts. The new ensureMemoryFlushSafetyHints helper is straightforward, purely additive, and exercised by both unit and e2e tests. No data paths, network calls, or tool-execution surfaces are touched. Backward compatibility is preserved: existing default prompts already contain all three hints, so the helper is a no-op for them; custom-override prompts get the hints appended, which is the intended fix.
• No files require special attention.

_{Last reviewed commit: 8223806}

Follow-up for the Aisle security finding: this no longer relies on prompt hints alone.

This branch now enforces memory-flush writes server-side:

• trigger: "memory" runs force filesystem writes to stay under the workspace root.
• Memory flush now receives a single canonical write target: memory/YYYY-MM-DD.md.
• The generic write tool is wrapped to allow append-only writes to that one path.
• edit and apply_patch are removed from memory-triggered runs.

Local verification I ran before pushing this update:

• pnpm vitest run src/agents/pi-tools.workspace-only-false.test.ts src/auto-reply/reply/memory-flush.test.ts src/auto-reply/reply/reply-state.test.ts
• pnpm exec vitest run --config vitest.e2e.config.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts -t "memory flush"
• pnpm check