🔐 fix: Address Codex re-review on memory capability (round 4)

- Detect inline memory by tool NAME (set_memory/delete_memory) across an initialized agent's tools + toolDefinitions, since the 'memory' marker is expanded at init and the prior string check never matched; inject the keyed memory context for any primary OR sub-agent that carries the inline memory tools (api/server/controllers/agents/client.js).
- Enforce memory WRITE permissions in the inline tool gate: set_memory requires CREATE+UPDATE and delete_memory requires UPDATE (matching the REST memory routes), so a USE-only role can't mutate/delete memories via agent tool calls (api/app/clients/tools/util/handleTools.js).
This commit is contained in:
Danny Avila 2026-06-20 23:17:30 -04:00
parent d596f3a869
commit e52ca9d3d2
2 changed files with 48 additions and 11 deletions

View file

@ -59,11 +59,13 @@ const { getRoleByName, setMemory, deleteMemory, getFormattedMemories } = require
* Re-checks the full memory gate before constructing inline memory tools.
* The event-driven executor loads tools by name, so an unsolicited
* `set_memory`/`delete_memory` call must not bypass the config, opt-out, and
* permission checks enforced when the tools were registered.
* permission checks enforced when the tools were registered. `writePermissions`
* mirror the REST memory routes: writes require CREATE/UPDATE, not just USE.
* @param {ServerRequest} [req]
* @param {string[]} [writePermissions]
* @returns {Promise<boolean>}
*/
async function isMemoryToolUsable(req) {
async function isMemoryToolUsable(req, writePermissions = []) {
if (!isMemoryEnabled(req?.config?.memory)) {
return false;
}
@ -74,7 +76,7 @@ async function isMemoryToolUsable(req) {
return await checkAccess({
user: req.user,
permissionType: PermissionTypes.MEMORIES,
permissions: [Permissions.USE],
permissions: [Permissions.USE, ...writePermissions],
getRoleByName,
});
} catch (error) {
@ -395,7 +397,7 @@ const loadTools = async ({
const validKeys = memoryConfig?.validKeys;
const tokenLimit = memoryConfig?.tokenLimit;
requestedTools[tool] = async () => {
if (!(await isMemoryToolUsable(options.req))) {
if (!(await isMemoryToolUsable(options.req, [Permissions.CREATE, Permissions.UPDATE]))) {
return null;
}
let totalTokens = 0;
@ -413,7 +415,7 @@ const loadTools = async ({
} else if (tool === DELETE_MEMORY_TOOL_NAME) {
const memoryConfig = options.req?.config?.memory;
requestedTools[tool] = async () => {
if (!(await isMemoryToolUsable(options.req))) {
if (!(await isMemoryToolUsable(options.req, [Permissions.UPDATE]))) {
return null;
}
return createDeleteMemoryTool({

View file

@ -17,6 +17,8 @@ const {
omitTitleOptions,
getProviderConfig,
memoryInstructions,
SET_MEMORY_TOOL_NAME,
DELETE_MEMORY_TOOL_NAME,
createTokenCounter,
applyContextToAgent,
isMemoryAgentEnabled,
@ -84,6 +86,31 @@ const loadAgent = (params) => loadAgentFn(params, { getAgent: db.getAgent, getMC
const MEMORY_INPUT_CHARS_PER_TOKEN = 8;
/** Names that mark inline memory tooling on an initialized agent. After
* `initializeAgent` the `memory` capability marker is expanded into the
* `set_memory`/`delete_memory` definitions, so a string `includes('memory')`
* on `agent.tools` (now loaded instances/definitions) no longer matches. */
const INLINE_MEMORY_TOOL_NAMES = new Set([
AgentCapabilities.memory,
SET_MEMORY_TOOL_NAME,
DELETE_MEMORY_TOOL_NAME,
]);
/**
* Whether an initialized agent carries the inline memory tools, checked across
* both its loaded tool instances and its serializable tool definitions.
* @param {Agent & { toolDefinitions?: Array<{ name?: string }> }} [agent]
* @returns {boolean}
*/
function agentHasInlineMemoryTools(agent) {
if (!agent) {
return false;
}
const matches = (entry) =>
INLINE_MEMORY_TOOL_NAMES.has(typeof entry === 'string' ? entry : entry?.name);
return (agent.toolDefinitions ?? []).some(matches) || (agent.tools ?? []).some(matches);
}
class AgentClient extends BaseClient {
constructor(options = {}) {
super(null, options);
@ -514,7 +541,12 @@ class AgentClient extends BaseClient {
await Promise.all(
allAgents.map(({ agent, agentId }) => {
const agentRunContextParts = [sharedRunContext];
if (memoryContext && (agentId === this.options.agent.id || memoryAgentEnabled)) {
if (
memoryContext &&
(agentId === this.options.agent.id ||
memoryAgentEnabled ||
agentHasInlineMemoryTools(agent))
) {
agentRunContextParts.push(memoryContext);
}
const scopedContext = agentScopedContext.get(agentId);
@ -595,11 +627,14 @@ class AgentClient extends BaseClient {
const userId = this.options.req.user.id + '';
this.processMemory = undefined;
/** Inline memory tools (the `memory` capability on this agent) let the main
* agent read/write memory itself, so it needs the keyed memory context
* otherwise `delete_memory` has no visible key to target. */
const agentHasMemoryTools =
this.options.agent?.tools?.includes(AgentCapabilities.memory) === true;
/** Inline memory tools (the `memory` capability) let an agent read/write
* memory itself, so the run needs the keyed memory context otherwise
* `delete_memory` has no visible key to target. Checked across the primary
* and every sub-agent, since a handoff agent may carry memory alone. */
const agentHasMemoryTools = [
this.options.agent,
...(this.agentConfigs ? Array.from(this.agentConfigs.values()) : []),
].some(agentHasInlineMemoryTools);
if (!isMemoryAgentEnabled(memoryConfig)) {
try {