Skip to content

Commit f7a5d45

Browse files
feat(auth): add create bank validation hook
Add a precise operation-validator hook for bank creation, with a no-op default so deployments without custom validators keep existing behavior. Route lazy bank creation through the hook from retain, imports, MCP create_bank, and the default get_bank_profile auto-create path. This keeps create-bank authorization separate from bank-scoped write validation, which often assumes the target bank already exists. Add regression coverage for rejected creation, existing-bank skips, HTTP create/import paths, async retain, profile auto-create, and MCP create_bank.
1 parent 9dafadc commit f7a5d45

7 files changed

Lines changed: 227 additions & 20 deletions

File tree

hindsight-api-slim/hindsight_api/api/http.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5588,8 +5588,12 @@ async def api_create_or_update_bank(
55885588
):
55895589
"""Create or update an agent with disposition and mission."""
55905590
try:
5591-
# Ensure bank exists by getting profile (auto-creates with defaults)
5592-
await app.state.memory.get_bank_profile(bank_id, request_context=request_context)
5591+
# Ensure bank exists, validating create_bank only when this call
5592+
# actually creates a missing bank.
5593+
await app.state.memory._ensure_bank_exists(
5594+
bank_id,
5595+
request_context,
5596+
)
55935597

55945598
# Update name if provided (stored in DB for display only, deprecated)
55955599
if request.name is not None:
@@ -5772,8 +5776,12 @@ async def api_import_bank_template(
57725776
dry_run=True,
57735777
)
57745778

5775-
# Ensure bank exists (auto-creates with defaults if needed)
5776-
await app.state.memory.get_bank_profile(bank_id, request_context=request_context)
5779+
# Ensure bank exists, validating create_bank only when this import
5780+
# actually creates a missing target bank.
5781+
await app.state.memory._ensure_bank_exists(
5782+
bank_id,
5783+
request_context,
5784+
)
57775785

57785786
return await apply_bank_template_manifest(
57795787
memory=app.state.memory,

hindsight-api-slim/hindsight_api/engine/memory_engine.py

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3223,6 +3223,8 @@ async def retain_batch_async(
32233223
if result and result.contents is not None:
32243224
contents = cast(list[RetainContentDict], result.contents)
32253225

3226+
await self._ensure_bank_exists(bank_id, request_context)
3227+
32263228
# Engine-owned copy: the orchestrator clears per-item "content" strings
32273229
# after building the document's combined text (memory pressure
32283230
# optimization, see retain/orchestrator.py). Without an internal copy
@@ -3709,6 +3711,14 @@ async def import_bank_async(
37093711
# target bank's config before the restore.
37103712
parsed = parse_bank_archive(archive_bytes)
37113713
bank_id = target_bank_id or parsed.manifest.source_bank_id
3714+
if self._operation_validator and await bank_utils.get_bank_profile_if_exists(backend, bank_id) is None:
3715+
from hindsight_api.extensions import CreateBankContext
3716+
3717+
ctx = CreateBankContext(
3718+
bank_id=bank_id,
3719+
request_context=request_context,
3720+
)
3721+
await self._validate_operation(self._operation_validator.validate_create_bank(ctx))
37123722
resolved_config = await self._config_resolver.resolve_full_config(bank_id, request_context)
37133723
return await import_bank(
37143724
backend=backend,
@@ -8401,16 +8411,12 @@ async def get_bank_profile(
84018411
existing = await bank_utils.get_bank_profile_if_exists(backend, bank_id)
84028412
if existing is None:
84038413
return None
8404-
profile, created = existing, False
8414+
profile = existing
84058415
else:
8406-
result = await bank_utils.get_or_create_bank_profile(backend, bank_id)
8407-
profile, created = result.profile, result.created
8408-
8409-
# Apply HINDSIGHT_API_DEFAULT_BANK_TEMPLATE to freshly-created banks. Done
8410-
# before reading the resolved config below so the template's overrides
8411-
# (e.g. reflect_mission, dispositions) are visible on this very call.
8412-
if created:
8413-
await self._apply_default_bank_template(bank_id, request_context)
8416+
await self._ensure_bank_exists(bank_id, request_context)
8417+
profile = await bank_utils.get_bank_profile_if_exists(backend, bank_id)
8418+
if profile is None:
8419+
raise RuntimeError(f"Bank '{bank_id}' was not found after ensuring it exists")
84148420

84158421
# reflect_mission and disposition in config take precedence over the legacy DB columns
84168422
config_dict = await self._config_resolver.get_bank_config(bank_id, request_context)
@@ -8466,6 +8472,20 @@ async def _ensure_bank_exists(
84668472
True if the bank was freshly created on this call.
84678473
"""
84688474
backend = await self._get_backend()
8475+
if self._operation_validator:
8476+
if conn is not None:
8477+
exists = await conn.fetchval(f"SELECT 1 FROM {fq_table('banks')} WHERE bank_id = $1", bank_id)
8478+
else:
8479+
exists = await bank_utils.get_bank_profile_if_exists(backend, bank_id)
8480+
if not exists:
8481+
from hindsight_api.extensions import CreateBankContext
8482+
8483+
ctx = CreateBankContext(
8484+
bank_id=bank_id,
8485+
request_context=request_context,
8486+
)
8487+
await self._validate_operation(self._operation_validator.validate_create_bank(ctx))
8488+
84698489
if conn is not None:
84708490
result = await bank_utils.get_or_create_bank_profile_on_conn(conn, bank_id, ops=backend.ops)
84718491
return result.created
@@ -10181,7 +10201,11 @@ async def create_mental_model(
1018110201
# outlives a mental-model insert that ultimately fails.
1018210202
async with acquire_with_retry(backend) as conn:
1018310203
async with conn.transaction():
10184-
created = await self._ensure_bank_exists(bank_id, request_context, conn=conn)
10204+
created = await self._ensure_bank_exists(
10205+
bank_id,
10206+
request_context,
10207+
conn=conn,
10208+
)
1018510209
if mental_model_id:
1018610210
row = await conn.fetchrow(
1018710211
f"""
@@ -11773,7 +11797,11 @@ async def create_webhook(
1177311797
# commit (or roll back) atomically.
1177411798
async with acquire_with_retry(backend) as conn:
1177511799
async with conn.transaction():
11776-
created = await self._ensure_bank_exists(bank_id, request_context, conn=conn)
11800+
created = await self._ensure_bank_exists(
11801+
bank_id,
11802+
request_context,
11803+
conn=conn,
11804+
)
1177711805
row = await backend.ops.create_webhook(
1177811806
conn,
1177911807
fq_table("webhooks"),
@@ -12214,7 +12242,11 @@ async def submit_async_retain(
1221412242
# async_operations.bank_id has a FK to banks. Create the bank
1221512243
# lazily inside this same transaction so it is atomic with the
1221612244
# parent + child operation rows.
12217-
created = await self._ensure_bank_exists(bank_id, request_context, conn=conn)
12245+
created = await self._ensure_bank_exists(
12246+
bank_id,
12247+
request_context,
12248+
conn=conn,
12249+
)
1221812250
await conn.execute(
1221912251
f"""
1222012252
INSERT INTO {fq_table("async_operations")} (operation_id, bank_id, operation_type, result_metadata, status)

hindsight-api-slim/hindsight_api/extensions/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
# Consolidation operation
4444
ConsolidateContext,
4545
ConsolidateResult,
46+
CreateBankContext,
4647
# File Conversion
4748
FileConvertResult,
4849
# Mental Model operations
@@ -99,6 +100,7 @@
99100
"BankListResult",
100101
"BankReadContext",
101102
"BankWriteContext",
103+
"CreateBankContext",
102104
# Operation Validator - Consolidation
103105
"ConsolidateContext",
104106
"ConsolidateResult",

hindsight-api-slim/hindsight_api/extensions/operation_validator.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,14 @@ class BankWriteContext:
321321
request_context: "RequestContext"
322322

323323

324+
@dataclass
325+
class CreateBankContext:
326+
"""Context for validating creation of a new bank."""
327+
328+
bank_id: str
329+
request_context: "RequestContext"
330+
331+
324332
@dataclass
325333
class BankListContext:
326334
"""Context for filtering the bank list (post-query)."""
@@ -805,6 +813,23 @@ async def validate_bank_write(self, ctx: BankWriteContext) -> ValidationResult:
805813
"""
806814
return ValidationResult.accept()
807815

816+
async def validate_create_bank(self, ctx: CreateBankContext) -> ValidationResult:
817+
"""
818+
Validate creation of a new bank before the bank row is inserted.
819+
820+
Override to implement custom validation logic for operations that
821+
explicitly or implicitly create a bank.
822+
823+
Args:
824+
ctx: Context containing:
825+
- bank_id: Bank identifier
826+
- request_context: Request context with auth info
827+
828+
Returns:
829+
ValidationResult indicating whether the bank may be created.
830+
"""
831+
return ValidationResult.accept()
832+
808833
async def filter_bank_list(self, ctx: BankListContext) -> BankListResult:
809834
"""
810835
Filter the bank list after querying.

hindsight-api-slim/hindsight_api/mcp_tools.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,9 @@ async def create_bank(bank_id: str, name: str | None = None, mission: str | None
12091209
"""
12101210
try:
12111211
request_context = _get_request_context(config)
1212-
# get_bank_profile auto-creates bank if it doesn't exist
1212+
# create_bank may auto-create the bank; validate that explicit
1213+
# creation permission before reading the resulting profile.
1214+
await memory._ensure_bank_exists(bank_id, request_context)
12131215
profile = await memory.get_bank_profile(bank_id, request_context=request_context)
12141216

12151217
# Update name/mission if provided

hindsight-api-slim/tests/test_extensions.py

Lines changed: 132 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
from hindsight_api.extensions import (
1010
ApiKeyTenantExtension,
1111
AuthenticationError,
12+
# Consolidation operation
13+
ConsolidateContext,
14+
ConsolidateResult,
15+
CreateBankContext,
1216
Extension,
1317
HttpExtension,
1418
OperationValidationError,
@@ -25,9 +29,6 @@
2529
TenantExtension,
2630
ValidationResult,
2731
load_extension,
28-
# Consolidation operation
29-
ConsolidateContext,
30-
ConsolidateResult,
3132
)
3233

3334

@@ -169,6 +170,30 @@ async def on_consolidate_complete(self, result: ConsolidateResult) -> None:
169170
self.post_consolidate_calls.append(result)
170171

171172

173+
class CreateBankRejectingValidator(OperationValidatorExtension):
174+
"""Validator that records create-bank checks and can reject them."""
175+
176+
def __init__(self, *, reject: bool = True):
177+
super().__init__({})
178+
self.reject = reject
179+
self.create_bank_calls: list[CreateBankContext] = []
180+
181+
async def validate_retain(self, ctx: RetainContext) -> ValidationResult:
182+
return ValidationResult.accept()
183+
184+
async def validate_recall(self, ctx: RecallContext) -> ValidationResult:
185+
return ValidationResult.accept()
186+
187+
async def validate_reflect(self, ctx: ReflectContext) -> ValidationResult:
188+
return ValidationResult.accept()
189+
190+
async def validate_create_bank(self, ctx: CreateBankContext) -> ValidationResult:
191+
self.create_bank_calls.append(ctx)
192+
if self.reject:
193+
return ValidationResult.reject("bank creation not allowed", status_code=402)
194+
return ValidationResult.accept()
195+
196+
172197
class TestMemoryEngineValidation:
173198
"""Tests for validation integration with MemoryEngine.
174199
@@ -264,6 +289,109 @@ async def test_reflect_validation(self, memory_with_validator):
264289

265290
assert "limit exceeded" in str(exc_info.value).lower()
266291

292+
@pytest.mark.asyncio
293+
async def test_retain_validates_create_bank_for_missing_bank(self, memory):
294+
"""Retain may lazily create a bank, so missing-bank retain validates create_bank."""
295+
bank_id = "test-retain-create-bank-validation"
296+
ctx = RequestContext()
297+
validator = CreateBankRejectingValidator()
298+
memory._operation_validator = validator
299+
300+
with pytest.raises(OperationValidationError) as exc_info:
301+
await memory.retain_batch_async(
302+
bank_id=bank_id,
303+
contents=[{"content": "Should not create a bank"}],
304+
request_context=ctx,
305+
)
306+
307+
assert "bank creation not allowed" in str(exc_info.value)
308+
assert len(validator.create_bank_calls) == 1
309+
assert validator.create_bank_calls[0].bank_id == bank_id
310+
assert await memory.get_bank_profile(bank_id, request_context=ctx, create_if_missing=False) is None
311+
312+
@pytest.mark.asyncio
313+
async def test_async_retain_validates_create_bank_for_missing_bank(self, memory):
314+
"""Async retain validates create_bank before creating its parent operation."""
315+
bank_id = "test-async-retain-create-bank-validation"
316+
ctx = RequestContext()
317+
validator = CreateBankRejectingValidator()
318+
memory._operation_validator = validator
319+
320+
with pytest.raises(OperationValidationError) as exc_info:
321+
await memory.submit_async_retain(
322+
bank_id=bank_id,
323+
contents=[{"content": "Should not create a bank"}],
324+
request_context=ctx,
325+
)
326+
327+
assert "bank creation not allowed" in str(exc_info.value)
328+
assert len(validator.create_bank_calls) == 1
329+
assert validator.create_bank_calls[0].bank_id == bank_id
330+
assert await memory.get_bank_profile(bank_id, request_context=ctx, create_if_missing=False) is None
331+
332+
@pytest.mark.asyncio
333+
async def test_create_bank_validation_skips_existing_bank(self, memory):
334+
"""Existing banks do not require create_bank validation."""
335+
bank_id = "test-create-bank-existing"
336+
ctx = RequestContext()
337+
await memory.get_bank_profile(bank_id, request_context=ctx)
338+
339+
validator = CreateBankRejectingValidator()
340+
memory._operation_validator = validator
341+
342+
created = await memory._ensure_bank_exists(bank_id, ctx)
343+
344+
assert created is False
345+
assert validator.create_bank_calls == []
346+
347+
@pytest.mark.asyncio
348+
async def test_get_bank_profile_validates_create_bank_for_missing_bank(self, memory):
349+
"""The default auto-create profile path validates create_bank."""
350+
bank_id = "test-profile-create-bank-validation"
351+
ctx = RequestContext()
352+
validator = CreateBankRejectingValidator()
353+
memory._operation_validator = validator
354+
355+
with pytest.raises(OperationValidationError) as exc_info:
356+
await memory.get_bank_profile(bank_id, request_context=ctx)
357+
358+
assert "bank creation not allowed" in str(exc_info.value)
359+
assert len(validator.create_bank_calls) == 1
360+
assert validator.create_bank_calls[0].bank_id == bank_id
361+
assert await memory.get_bank_profile(bank_id, request_context=ctx, create_if_missing=False) is None
362+
363+
@pytest.mark.asyncio
364+
async def test_http_create_or_update_validates_create_bank(self, api_client, memory):
365+
bank_id = "test-http-put-create-bank-validation"
366+
validator = CreateBankRejectingValidator()
367+
memory._operation_validator = validator
368+
369+
resp = await api_client.put(
370+
f"/v1/default/banks/{bank_id}",
371+
json={"name": "Blocked Bank"},
372+
)
373+
374+
assert resp.status_code == 402
375+
assert resp.json()["detail"] == "bank creation not allowed"
376+
assert len(validator.create_bank_calls) == 1
377+
assert validator.create_bank_calls[0].bank_id == bank_id
378+
379+
@pytest.mark.asyncio
380+
async def test_http_template_import_validates_create_bank(self, api_client, memory):
381+
bank_id = "test-http-import-create-bank-validation"
382+
validator = CreateBankRejectingValidator()
383+
memory._operation_validator = validator
384+
385+
resp = await api_client.post(
386+
f"/v1/default/banks/{bank_id}/import",
387+
json={"version": "1"},
388+
)
389+
390+
assert resp.status_code == 402
391+
assert resp.json()["detail"] == "bank creation not allowed"
392+
assert len(validator.create_bank_calls) == 1
393+
assert validator.create_bank_calls[0].bank_id == bank_id
394+
267395

268396
@pytest.fixture
269397
def memory_with_validator(memory):
@@ -356,6 +484,7 @@ async def test_retain_post_hook_receives_all_parameters_and_result(self, memory_
356484
async def test_recall_pre_hook_receives_all_parameters(self, memory_with_tracking_validator):
357485
"""Pre-recall hook receives all user-provided parameters."""
358486
from datetime import datetime, timezone
487+
359488
from hindsight_api.engine.memory_engine import Budget
360489

361490
memory, validator = memory_with_tracking_validator

hindsight-api-slim/tests/test_mcp_tools.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ async def _get_mental_model(**kwargs):
188188

189189
# Tags & bank methods
190190
memory.list_tags = AsyncMock(return_value={"items": ["tag1", "tag2"], "total": 2})
191+
memory._ensure_bank_exists = AsyncMock(return_value=True)
191192
memory.get_bank_profile = AsyncMock(return_value={"id": "test-bank", "name": "Test Bank", "mission": "Testing"})
192193
memory.get_bank_stats = AsyncMock(return_value={"nodes": 100, "links": 50})
193194
memory.update_bank = AsyncMock(return_value={"id": "test-bank", "name": "Updated"})
@@ -1512,6 +1513,14 @@ async def test_get_bank(self, mock_memory):
15121513
result = await _tools(mcp)["get_bank"].fn()
15131514
assert '"test-bank"' in result or "test-bank" in result
15141515

1516+
async def test_create_bank_validates_creation(self, mock_memory):
1517+
mcp = _make_mcp_server(mock_memory, {"create_bank"}, include_bank_id=True)
1518+
result = await _tools(mcp)["create_bank"].fn(bank_id="new-bank")
1519+
assert '"test-bank"' in result or "test-bank" in result
1520+
call = mock_memory._ensure_bank_exists.call_args
1521+
assert call.args[0] == "new-bank"
1522+
assert "operation" not in call.kwargs
1523+
15151524
async def test_get_bank_stats(self, mock_memory):
15161525
mcp = _make_mcp_server(mock_memory, {"get_bank_stats"}, include_bank_id=True)
15171526
result = await _tools(mcp)["get_bank_stats"].fn()

0 commit comments

Comments
 (0)