Skip to content

fix(mcp): enforce write-tag permission on save-memory and add_memory - #1654

Open
ygd58 wants to merge 1 commit into
supermemoryai:mainfrom
ygd58:fix/mcp-enforce-write-access-on-save
Open

ygd58 wants to merge 1 commit into
supermemoryai:mainfrom
ygd58:fix/mcp-enforce-write-access-on-save

Conversation

@ygd58

@ygd58 ygd58 commented Sep 9, 2026

Copy link
Copy Markdown

fix(mcp): enforce write-tag permission on save-memory and add_memory

effectiveContainerTagAccess() computes per-tag read/write permission
from the session's RBAC state (restricted member roles, scoped
OAuth grants), but it was only ever consumed by select-space,
guided-save, and upload-file -- all three use it purely to build the
list of tags to offer in a picker/dropdown widget. The two tools
that actually perform a write, save-memory and add_memory, accept a
containerTag argument directly from the caller and never check it
against the session's permissions before calling createMemory /
forgetMemory.

Since containerTags are discoverable (listSpaces / listContainerTags
returns them) and not secret, any client can call these tools
directly -- bypassing the widget entirely -- with a containerTag it
only has read access to (a restricted member) or that isn't in its
assigned set (a scoped OAuth grant), and the write still goes
through today.

Add assertWriteAccess(containerTag, session) in rbac.ts, built on
the existing effectiveContainerTagAccess, and call it at the start
of both tools before any client call is made. A denied call now
returns a normal tool error instead of writing.

Covered by 5 new rbac.test.ts cases (full access, restricted
read-only, scoped-out tag, scoped read-only own tag, restricted
explicit write). apps/mcp's pre-existing e2e/auth.test.ts failures
are unrelated -- they hit a live OAuth origin this sandbox doesn't
have and fail identically on main before this change.

effectiveContainerTagAccess() computes per-tag read/write permission
from the session's RBAC state (restricted member roles, scoped
OAuth grants), but it was only ever consumed by select-space,
guided-save, and upload-file -- all three use it purely to build the
list of tags to offer in a picker/dropdown widget. The two tools
that actually perform a write, save-memory and add_memory, accept a
containerTag argument directly from the caller and never check it
against the session's permissions before calling createMemory /
forgetMemory.

Since containerTags are discoverable (listSpaces / listContainerTags
returns them) and not secret, any client can call these tools
directly -- bypassing the widget entirely -- with a containerTag it
only has read access to (a restricted member) or that isn't in its
assigned set (a scoped OAuth grant), and the write still goes
through today.

Add assertWriteAccess(containerTag, session) in rbac.ts, built on
the existing effectiveContainerTagAccess, and call it at the start
of both tools before any client call is made. A denied call now
returns a normal tool error instead of writing.

Covered by 5 new rbac.test.ts cases (full access, restricted
read-only, scoped-out tag, scoped read-only own tag, restricted
explicit write). apps/mcp's pre-existing e2e/auth.test.ts failures
are unrelated -- they hit a live OAuth origin this sandbox doesn't
have and fail identically on main before this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant