Skip to content

Commit aa1472d

Browse files
committed
fix(mcp): harden workspace permalink routing
Signed-off-by: phernandez <paul@basicmachines.co>
1 parent bcc5758 commit aa1472d

8 files changed

Lines changed: 205 additions & 13 deletions

File tree

src/basic_memory/api/app.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
from basic_memory.workspace_context import (
3434
WORKSPACE_SLUG_HEADER,
3535
WORKSPACE_TYPE_HEADER,
36+
validate_workspace_permalink_context_values,
3637
workspace_permalink_context,
3738
)
3839

@@ -100,19 +101,19 @@ async def workspace_permalink_context_middleware(request: Request, call_next):
100101
workspace_slug = request.headers.get(WORKSPACE_SLUG_HEADER)
101102
workspace_type = request.headers.get(WORKSPACE_TYPE_HEADER)
102103

103-
if bool(workspace_slug) != bool(workspace_type):
104+
try:
105+
validate_workspace_permalink_context_values(workspace_slug, workspace_type)
106+
except ValueError as exc:
104107
return JSONResponse(
105108
status_code=400,
106-
content={
107-
"detail": (
108-
f"{WORKSPACE_SLUG_HEADER} and {WORKSPACE_TYPE_HEADER} must be provided together"
109-
)
110-
},
109+
content={"detail": str(exc)},
111110
)
112111

113112
if not workspace_slug:
114113
return await call_next(request)
115114

115+
# ContextVar state remains active across the awaited downstream handler while
116+
# this context manager is open, so entity creation can see request metadata.
116117
with workspace_permalink_context(
117118
workspace_slug=workspace_slug,
118119
workspace_type=workspace_type,

src/basic_memory/mcp/project_context.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,15 @@ async def resolve_workspace_qualified_memory_url(
483483
f"Project '{project_identifier}' was not found in workspace "
484484
f"'{workspace.name}' ({workspace.slug}). Available projects: {available}"
485485
)
486+
if len(matches) > 1:
487+
details = ", ".join(
488+
f"{entry.qualified_name} ({entry.project.external_id})" for entry in matches
489+
)
490+
raise ValueError(
491+
f"Project '{project_identifier}' matched multiple projects in workspace "
492+
f"'{workspace.name}' ({workspace.slug}). Project permalinks must be unique. "
493+
f"Matches: {details}"
494+
)
486495

487496
entry = matches[0]
488497
canonical_path = f"{entry.workspace.slug}/{entry.project.permalink}/{remainder}"
@@ -707,6 +716,15 @@ async def resolve_workspace_project_identifier(
707716
f"Project '{project_identifier}' was not found in workspace "
708717
f"'{workspace.name}' ({workspace.slug}). Available projects: {available}"
709718
)
719+
if len(matches) > 1:
720+
details = ", ".join(
721+
f"{entry.qualified_name} ({entry.project.external_id})" for entry in matches
722+
)
723+
raise ValueError(
724+
f"Project '{project_identifier}' matched multiple projects in workspace "
725+
f"'{workspace.name}' ({workspace.slug}). Project permalinks must be unique. "
726+
f"Matches: {details}"
727+
)
710728
return matches[0]
711729

712730
matches = index.entries_by_permalink.get(project_permalink, ())

src/basic_memory/utils.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,14 @@ def build_canonical_permalink(
245245
return normalized_path
246246

247247
normalized_project = generate_permalink(project_permalink)
248+
normalized_workspace = generate_permalink(workspace_permalink) if workspace_permalink else None
249+
if normalized_workspace:
250+
workspace_project_prefix = f"{normalized_workspace}/{normalized_project}"
251+
if normalized_path == workspace_project_prefix or normalized_path.startswith(
252+
f"{workspace_project_prefix}/"
253+
):
254+
return normalized_path
255+
248256
if normalized_path == normalized_project or normalized_path.startswith(
249257
f"{normalized_project}/"
250258
):
@@ -255,10 +263,6 @@ def build_canonical_permalink(
255263
if not workspace_permalink:
256264
return project_path
257265

258-
normalized_workspace = generate_permalink(workspace_permalink)
259-
if project_path == normalized_workspace or project_path.startswith(f"{normalized_workspace}/"):
260-
return project_path
261-
262266
return f"{normalized_workspace}/{project_path}"
263267

264268

src/basic_memory/workspace_context.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
"""Request-local workspace context for canonical permalink generation."""
22

3+
import re
34
from contextlib import contextmanager
45
from contextvars import ContextVar
56
from dataclasses import dataclass
67
from typing import Iterator
78

89
WORKSPACE_SLUG_HEADER = "X-Basic-Memory-Workspace-Slug"
910
WORKSPACE_TYPE_HEADER = "X-Basic-Memory-Workspace-Type"
11+
_WORKSPACE_SLUG_PATTERN = re.compile(r"^[a-z0-9_-]+$")
12+
_WORKSPACE_TYPES = {"personal", "organization"}
1013

1114

1215
@dataclass(frozen=True)
@@ -32,6 +35,25 @@ def current_workspace_permalink_context() -> WorkspacePermalinkContext | None:
3235
return _workspace_permalink_context.get()
3336

3437

38+
def validate_workspace_permalink_context_values(
39+
workspace_slug: str | None,
40+
workspace_type: str | None,
41+
) -> None:
42+
"""Validate workspace permalink metadata before it can affect stored permalinks."""
43+
if bool(workspace_slug) != bool(workspace_type):
44+
raise ValueError("workspace_slug and workspace_type must be provided together")
45+
46+
if not workspace_slug or not workspace_type:
47+
return
48+
49+
if _WORKSPACE_SLUG_PATTERN.fullmatch(workspace_slug) is None:
50+
raise ValueError(f"{WORKSPACE_SLUG_HEADER} must match [a-z0-9_-]+")
51+
52+
if workspace_type not in _WORKSPACE_TYPES:
53+
allowed = ", ".join(sorted(_WORKSPACE_TYPES))
54+
raise ValueError(f"{WORKSPACE_TYPE_HEADER} must be one of: {allowed}")
55+
56+
3557
@contextmanager
3658
def workspace_permalink_context(
3759
workspace_slug: str | None,
@@ -42,8 +64,7 @@ def workspace_permalink_context(
4264
Cloud can populate this per request without storing workspace metadata in
4365
local project config. The slug/type pair is all permalink generation needs.
4466
"""
45-
if bool(workspace_slug) != bool(workspace_type):
46-
raise ValueError("workspace_slug and workspace_type must be provided together")
67+
validate_workspace_permalink_context_values(workspace_slug, workspace_type)
4768

4869
if not workspace_slug or not workspace_type:
4970
yield
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
"""Tests for workspace permalink context headers."""
2+
3+
import pytest
4+
from httpx import AsyncClient
5+
6+
from basic_memory.workspace_context import WORKSPACE_SLUG_HEADER, WORKSPACE_TYPE_HEADER
7+
8+
9+
@pytest.mark.asyncio
10+
@pytest.mark.parametrize(
11+
"headers, expected_detail",
12+
[
13+
(
14+
{WORKSPACE_SLUG_HEADER: "team-paul"},
15+
"workspace_slug and workspace_type must be provided together",
16+
),
17+
(
18+
{
19+
WORKSPACE_SLUG_HEADER: "../team-paul",
20+
WORKSPACE_TYPE_HEADER: "organization",
21+
},
22+
f"{WORKSPACE_SLUG_HEADER} must match [a-z0-9_-]+",
23+
),
24+
(
25+
{
26+
WORKSPACE_SLUG_HEADER: "team-paul",
27+
WORKSPACE_TYPE_HEADER: "enterprise",
28+
},
29+
f"{WORKSPACE_TYPE_HEADER} must be one of: organization, personal",
30+
),
31+
],
32+
)
33+
async def test_workspace_permalink_headers_fail_fast(
34+
client: AsyncClient,
35+
headers: dict[str, str],
36+
expected_detail: str,
37+
):
38+
response = await client.get("/v2/projects/", headers=headers)
39+
40+
assert response.status_code == 400
41+
assert response.json()["detail"] == expected_detail

tests/mcp/test_project_context.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,45 @@ async def fake_index(context=None):
663663
assert resolved == "team-paul/main"
664664

665665

666+
@pytest.mark.asyncio
667+
async def test_resolve_workspace_qualified_memory_url_fails_on_duplicate_project_permalink(
668+
monkeypatch,
669+
):
670+
import basic_memory.mcp.project_context as project_context
671+
from basic_memory.mcp.project_context import (
672+
WorkspaceProjectEntry,
673+
_build_workspace_project_index,
674+
resolve_workspace_qualified_memory_url,
675+
)
676+
677+
team = _workspace(
678+
tenant_id="team-tenant",
679+
workspace_type="organization",
680+
slug="team-paul",
681+
name="Team Paul",
682+
role="editor",
683+
)
684+
entries = (
685+
WorkspaceProjectEntry(
686+
workspace=team,
687+
project=_project("main", id=1, external_id="team-main-id-1"),
688+
),
689+
WorkspaceProjectEntry(
690+
workspace=team,
691+
project=_project("Main", id=2, external_id="team-main-id-2"),
692+
),
693+
)
694+
index = _build_workspace_project_index((team,), entries)
695+
696+
async def fake_index(context=None):
697+
return index
698+
699+
monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index)
700+
701+
with pytest.raises(ValueError, match="matched multiple projects"):
702+
await resolve_workspace_qualified_memory_url("memory://team-paul/main/notes/foo")
703+
704+
666705
@pytest.mark.asyncio
667706
async def test_get_project_client_routes_duplicate_project_through_workspace_slug(
668707
config_manager,

tests/utils/test_permalink_formatting.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from basic_memory.config import ProjectConfig
88
from basic_memory.services import EntityService
99
from basic_memory.sync.sync_service import SyncService
10-
from basic_memory.utils import generate_permalink
10+
from basic_memory.utils import build_canonical_permalink, generate_permalink
1111

1212

1313
async def create_test_file(path: Path, content: str = "test content") -> None:
@@ -124,3 +124,27 @@ def test_chinese_character_preservation(input_path, expected):
124124
def test_mixed_character_sets(input_path, expected):
125125
"""Test handling of mixed character sets and edge cases."""
126126
assert generate_permalink(input_path) == expected
127+
128+
129+
def test_build_canonical_permalink_prefixes_same_workspace_and_project_slug():
130+
"""Workspace and project slugs may be equal but remain distinct permalink segments."""
131+
assert (
132+
build_canonical_permalink(
133+
"acme",
134+
"notes/foo.md",
135+
workspace_permalink="acme",
136+
)
137+
== "acme/acme/notes/foo"
138+
)
139+
140+
141+
def test_build_canonical_permalink_preserves_complete_workspace_prefix():
142+
"""Already workspace-qualified canonical paths should not gain duplicate prefixes."""
143+
assert (
144+
build_canonical_permalink(
145+
"main",
146+
"team-paul/main/notes/foo.md",
147+
workspace_permalink="team-paul",
148+
)
149+
== "team-paul/main/notes/foo"
150+
)
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
"""Tests for request-local workspace permalink context."""
2+
3+
import pytest
4+
5+
from basic_memory.workspace_context import (
6+
WORKSPACE_SLUG_HEADER,
7+
WORKSPACE_TYPE_HEADER,
8+
current_workspace_permalink_context,
9+
workspace_permalink_context,
10+
workspace_permalink_headers,
11+
)
12+
13+
14+
def test_workspace_permalink_context_requires_slug_and_type_together():
15+
with pytest.raises(ValueError, match="provided together"):
16+
with workspace_permalink_context("team-paul", None):
17+
pass
18+
19+
20+
def test_workspace_permalink_context_rejects_unsafe_slug():
21+
with pytest.raises(ValueError, match=WORKSPACE_SLUG_HEADER):
22+
with workspace_permalink_context("../team-paul", "organization"):
23+
pass
24+
25+
26+
def test_workspace_permalink_context_rejects_unknown_type():
27+
with pytest.raises(ValueError, match=WORKSPACE_TYPE_HEADER):
28+
with workspace_permalink_context("team-paul", "enterprise"):
29+
pass
30+
31+
32+
def test_workspace_permalink_headers_reflect_active_context():
33+
assert workspace_permalink_headers() == {}
34+
35+
with workspace_permalink_context("team-paul", "organization"):
36+
context = current_workspace_permalink_context()
37+
assert context is not None
38+
assert context.workspace_slug == "team-paul"
39+
assert workspace_permalink_headers() == {
40+
WORKSPACE_SLUG_HEADER: "team-paul",
41+
WORKSPACE_TYPE_HEADER: "organization",
42+
}
43+
44+
assert current_workspace_permalink_context() is None

0 commit comments

Comments
 (0)