🔒 Aisle Security Analysis

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

1. 🟠 Unbounded transcript rewrite in context-engine maintenance enables resource exhaustion (DoS) and bypasses size guards

Description

rewriteTranscriptEntriesInSessionManager() can be invoked by any context-engine implementation via runtimeContext.rewriteTranscriptEntries() (new maintain() hook). The rewrite algorithm:

• Finds the first message entry to replace, then branches and re-appends the entire suffix from that point to the end of the active branch (O(n) operations).
• Uses getRawSessionAppendMessage() to bypass persistence hooks and the tool-result size truncation guard, allowing rewritten entries to be arbitrarily large.
• Applies no limits on number of replacements, total bytes written, or suffix length. It also reports bytesFreed as non-negative only (Math.max(0, ...)), so transcript growth is not surfaced.
• maintain() is invoked automatically after bootstrap, after each successful turn, and after compaction; in run/attempt.ts this happens while holding the session write lock, so a malicious/buggy engine can repeatedly rewrite early entries and keep the lock for long periods, blocking other operations.

Vulnerable code (unbounded replay + bypass of guards):

const appendMessage = getRawSessionAppendMessage(params.sessionManager);
for (let index = matchedIndices[0]; index < branch.length; index++) {
  const entry = branch[index];
  const replacement = entry.type === "message" ? replacementsById.get(entry.id) : undefined;
  const newEntryId = replacement === undefined
    ? appendBranchEntry({ sessionManager: params.sessionManager, entry, rewrittenEntryIds, appendMessage })
    : appendMessage(replacement as Parameters<typeof params.sessionManager.appendMessage>[0]);
  rewrittenEntryIds.set(entry.id, newEntryId);
}

Automatic invocation path (amplifies impact):

• After every successful turn: run/attempt.ts calls runContextEngineMaintenance(... reason: "turn" ...) under the session lock.

Impact:

• Availability: large sessions + repeated rewrites can cause high CPU/disk I/O and prolonged lock holds.
• Disk/memory pressure: replacement messages can be made much larger than originals (no cap), potentially growing session JSONL files until storage is exhausted.

Recommendation

Add runtime-enforced limits to prevent unbounded rewrite work and uncontrolled transcript growth. Possible mitigations (can be combined):

1. Cap rewrite scope

• Maximum number of replacements per call
• Maximum suffix entries to replay (or require targets to be near the tail)

2. Cap size increase / total bytes written

• Reject requests where estimated size increase exceeds a threshold
• Track bytesDelta = replacementBytes - originalBytes and fail/limit when positive beyond a cap

3. Do not bypass persistence safety

• Apply the same persistence transformations/guards used for normal writes (at least toolResult truncation) to replacement messages.

Example (sketch):

const MAX_REPLACEMENTS = 25;
const MAX_SUFFIX_ENTRIES = 500;
const MAX_BYTES_GROWTH = 100_000; // 100KB

if (params.replacements.length > MAX_REPLACEMENTS) {
  return { changed: false, bytesFreed: 0, rewrittenEntries: 0, reason: "too many replacements" };
}

​// compute matchedIndices + bytesDelta
let bytesGrowth = 0;
bytesGrowth += Math.max(0, replacementBytes - originalBytes);
if (bytesGrowth > MAX_BYTES_GROWTH) {
  return { changed: false, bytesFreed: 0, rewrittenEntries: 0, reason: "rewrite would grow transcript too much" };
}

const suffixLen = branch.length - matchedIndices[0];
if (suffixLen > MAX_SUFFIX_ENTRIES) {
  return { changed: false, bytesFreed: 0, rewrittenEntries: 0, reason: "rewrite suffix too large" };
}

​// Apply safety caps to replacement messages (e.g. toolResult truncation)
const safeAppend = params.sessionManager.appendMessage.bind(params.sessionManager);

Also consider rate limiting maintenance rewrites (e.g., run at most once per N turns or only when bytesFreed exceeds a threshold) and/or running maintenance outside the longest-lived session lock when possible.

2. 🟡 Context-engine plugins can tamper with persisted session transcripts via runtimeContext.rewriteTranscriptEntries()

Description

The runtime now passes a rewriteTranscriptEntries() capability into ContextEngine.maintain(), allowing any configured context-engine implementation (including third-party plugins) to rewrite arbitrary persisted transcript entries by entryId.

Security impact:

• Audit-log / history tampering (repudiation): a plugin can rewrite past user/assistant messages, altering the authoritative session record.
• Prompt-history manipulation: rewritten content can change what later turns/rehydration treat as “what really happened”.
• Bypass of persistence guards/hooks: the rewrite implementation re-appends entries using the raw SessionManager appendMessage, explicitly bypassing truncation/sanitization hooks (e.g., tool-result size caps, before_message_write hooks), enabling injection of oversized or otherwise disallowed content.

Vulnerable code (capability exposure):

return {
  ...params.runtimeContext,
  rewriteTranscriptEntries: async (request) => {
    if (params.sessionManager) {
      return rewriteTranscriptEntriesInSessionManager({
        sessionManager: params.sessionManager,
        replacements: request.replacements,
      });
    }
    return await rewriteTranscriptEntriesInSessionFile({ ... });
  },
};

Because TranscriptRewriteReplacement.message is typed as AgentMessage with no runtime validation/authorization, a plugin can request replacements for any message role/type.

Recommendation

Treat transcript rewriting as a privileged operation.

Suggested mitigations (pick a combination appropriate for your threat model):

1. Runtime-enforced allowlist: restrict rewrites to specific roles/types (e.g. only toolResult), and validate the replacement message schema.

2. Re-apply persistence guards: do not use raw append for plugin-supplied replacements, or re-run the same truncation/sanitization/before-write hooks used for normal persistence.

3. Capability gating: require an explicit configuration flag / plugin manifest capability to enable transcript rewriting, and default it off for third-party plugins.

4. Auditability: emit structured audit events including which plugin/engine requested rewrites and what entryIds were modified; consider storing immutable history or append-only “corrections” instead of in-place rewrites.

Example (runtime validation sketch):

rewriteTranscriptEntries: async (request) => {
  for (const r of request.replacements) {
    if (r.message.role !== "toolResult") {
      throw new Error("transcript rewrite only allowed for toolResult entries");
    }
  }
  ​// Optionally: run the same toolResult truncation/sanitizers here.
  return rewriteTranscriptEntriesInSessionFile({ ... });
}

Analyzed PR: #51191 at commit b42a3c2

_{Last updated on: 2026-03-21T00:09:32Z}

Greptile Summary

This PR adds a well-designed maintain() lifecycle hook to the ContextEngine interface, enabling plugins like lossless-claw to safely rewrite JSONL transcript entries without coupling to session internals. The branch-and-reappend logic from tool-result-truncation.ts is cleanly extracted into a generic transcript-rewrite.ts module (eliminating ~70 lines of duplication), the new hook is correctly wired at three call sites (bootstrap, post-turn, post-compaction), and the legacy session-key compatibility layer is properly extended to cover maintain.

Key changes:

• TranscriptRewriteRequest/Result/Replacement types and ContextEngineMaintenanceResult exported from both context-engine/index.ts and plugin-sdk/index.ts
• rewriteTranscriptEntriesInSessionManager / rewriteTranscriptEntriesInSessionFile implement the generic rewriter, following the existing SessionManager.open pattern already used by tool-result truncation
• runContextEngineMaintenance bridge correctly swallows errors so maintenance failures never crash sessions
• Legacy session-key compat correctly wraps maintain via SESSION_KEY_COMPAT_METHODS with a new test verifying the retry-and-memoize path

Items to address:

• context-engine-maintenance.test.ts is missing coverage for the three guard/error branches (undefined engine, engine without maintain, and maintain() throwing) despite the PR description claiming they are covered
• ContextEngineMaintenanceResult = TranscriptRewriteResult forces transcript-rewrite return semantics on all maintenance implementations; engines that perform non-transcript housekeeping must return zeros, which is semantically misleading — consider a distinct type or at least a clarifying JSDoc comment
• A dead-code type guard in transcript-rewrite.ts (line 128) is unreachable because matchedIndices is already narrowed to message-type entries

Confidence Score: 4/5

• Safe to merge; the implementation is architecturally sound and follows existing patterns, with only minor test-coverage gaps and a type-alias design concern
• The core rewrite logic is correct and consistent with the existing tool-result truncation pattern. Error handling prevents maintenance failures from affecting sessions. The legacy compatibility layer is correctly extended. The main gaps are: missing tests for error/guard branches in runContextEngineMaintenance, and a type alias that conflates maintenance with transcript-rewrite semantics. Neither is a runtime safety issue.
• src/agents/pi-embedded-runner/context-engine-maintenance.test.ts needs additional test cases for guard and error paths; src/context-engine/types.ts warrants a second look at the ContextEngineMaintenanceResult alias design

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/context-engine-maintenance.test.ts
Line: 135-182

Comment:
**Missing test coverage for guard and error branches**

The PR description states this file covers "missing engine, no maintain method, successful maintenance, error handling" — but there are only two `it` blocks, both testing the successful path. The three guard/error branches in `runContextEngineMaintenance` have no tests:

1. `contextEngine` is `undefined` — should return `undefined` immediately
2. Engine exists but `maintain` is not a function — should return `undefined` immediately
3. `maintain()` throws — should swallow the error, emit `log.warn`, and return `undefined`

Without these tests the catch-block and early-return paths are entirely unexercised:

```ts
// Currently untested
it("returns undefined when no context engine is provided", async () => {
  const result = await runContextEngineMaintenance({
    contextEngine: undefined,
    sessionId: "s1",
    sessionFile: "/tmp/session.jsonl",
    reason: "turn",
  });
  expect(result).toBeUndefined();
});

it("returns undefined when engine has no maintain method", async () => {
  const result = await runContextEngineMaintenance({
    contextEngine: { info: { id: "test" }, ingest: async () => ({ ingested: true }), assemble: async ({ messages }) => ({ messages, estimatedTokens: 0 }), compact: async () => ({ ok: true, compacted: false }) },
    sessionId: "s1",
    sessionFile: "/tmp/session.jsonl",
    reason: "turn",
  });
  expect(result).toBeUndefined();
});

it("returns undefined and logs a warning when maintain() throws", async () => {
  const result = await runContextEngineMaintenance({
    contextEngine: { ..., maintain: async () => { throw new Error("oops"); } },
    sessionId: "s1",
    sessionFile: "/tmp/session.jsonl",
    reason: "turn",
  });
  expect(result).toBeUndefined();
});
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/context-engine/types.ts
Line: 938

Comment:
**`ContextEngineMaintenanceResult` conflates maintenance with transcript-rewrite stats**

`ContextEngineMaintenanceResult` is a direct type alias for `TranscriptRewriteResult`:

```ts
export type ContextEngineMaintenanceResult = TranscriptRewriteResult;
```

This forces any engine implementing `maintain()` — even one that performs non-transcript housekeeping (e.g., pruning an external vector store, rotating DAG nodes) — to return `{ changed, bytesFreed, rewrittenEntries }`. Engines that don't perform transcript rewrites must return zeros, which is semantically misleading.

Consider either:
- Making it a distinct type with `transcriptRewrite?: TranscriptRewriteResult` as an optional field for engines that do perform rewrites, or
- Adding a JSDoc note making it explicit that non-rewriting engines should return zeros and `changed: false`.

At minimum, a comment explaining the contract would help future plugin authors understand what to return when `rewriteTranscriptEntries` is not called.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/transcript-rewrite.ts
Line: 127-135

Comment:
**Unreachable guard after confirmed-type narrowing**

`matchedIndices` is populated only when `entry.type === "message"` (line 111), so `branch[matchedIndices[0]]` is guaranteed to be a message-type entry when `matchedIndices.length > 0`. The `firstMatchedEntry.type !== "message"` check on line 128 can never be true and creates dead code that obscures the actual invariant:

```ts
const firstMatchedEntry = branch[matchedIndices[0]];
if (!firstMatchedEntry || firstMatchedEntry.type !== "message") {
  // This branch is unreachable
  return { changed: false, ... reason: "invalid first rewrite target" };
}
```

Consider removing the redundant guard and adding a short comment explaining why the type check is unnecessary, so future readers don't have to re-derive this:

```suggestion
  const firstMatchedEntry = branch[matchedIndices[0]];
  // matchedIndices only contains indices of "message" type entries (see loop above).
  if (!firstMatchedEntry) {
    return {
      changed: false,
      bytesFreed: 0,
      rewrittenEntries: 0,
      reason: "invalid first rewrite target",
    };
  }
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "feat: add context en..."}

Merged via squash.

• Prepared head SHA: b42a3c28b4395bd8a253c7728080f09100d02f42
• Merge commit: 751d5b7849cab4c8f21380cb77c946e78e5490f2

Thanks @jalehman!