Skip to content

Fix Dev API and KG locked data bypass (#6146) #6147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
beastoin merged 11 commits into from
Mar 29, 2026
Merged

Fix Dev API and KG locked data bypass (#6146) #6147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
143388b
Add is_locked guards to Developer API write endpoints (#6146)
beastoin Mar 29, 2026
3dd5f21
Filter locked memories from knowledge graph rebuild (#6146)
beastoin Mar 29, 2026
2e55f99
Skip locked memories during KG extraction in process_conversation (#6…
beastoin Mar 29, 2026
98427cf
Add unit tests for Dev API and KG lock bypass fixes (#6146)
beastoin Mar 29, 2026
cd89c44
Add test_dev_api_lock_bypass.py to test.sh (#6146)
beastoin Mar 29, 2026
b1edd01
Tighten test assertions per review feedback (#6146)
beastoin Mar 29, 2026
7b4321e
Use AST parsing to verify KG extraction guard structure (#6146)
beastoin Mar 29, 2026
a8c1d45
Tighten AST guard check to exact shape per review (#6146)
beastoin Mar 29, 2026
ff7f4d9
Require both AST operands are ast.Name on same variable (#6146)
beastoin Mar 29, 2026
1290dbe
Add boundary tests: action-item 404, all-locked rebuild, missing fiel…
beastoin Mar 29, 2026
9cf1f6b
Add real execution tests for locked memory KG extraction skip (#6146)
beastoin Mar 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion backend/routers/developer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ def delete_memory(
memory = memories_db.get_memory(uid, memory_id)
if not memory:
raise HTTPException(status_code=404, detail="Memory not found")
if memory.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this memory.")

memories_db.delete_memory(uid, memory_id)
return {"success": True}
Expand All @@ -329,6 +331,8 @@ def update_memory(
memory = memories_db.get_memory(uid, memory_id)
if not memory:
raise HTTPException(status_code=404, detail="Memory not found")
if memory.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this memory.")

if request.content is None and request.visibility is None and request.tags is None and request.category is None:
raise HTTPException(
Expand Down Expand Up @@ -534,8 +538,13 @@ def delete_action_item(

- **action_item_id**: The ID of the action item to delete
"""
if not action_items_db.delete_action_item(uid, action_item_id):
action_item = action_items_db.get_action_item(uid, action_item_id)
if not action_item:
raise HTTPException(status_code=404, detail="Action item not found")
if action_item.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this action item.")

action_items_db.delete_action_item(uid, action_item_id)
return {"success": True}
Comment on lines +541 to 548
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Double Firestore read and silently ignored return value

The new pattern introduces two Firestore reads where one was sufficient before:

  1. get_action_item internally calls action_item_ref.get() to fetch the document.
  2. action_items_db.delete_action_item also calls action_item_ref.get().exists before deleting.

The return value of delete_action_item (which is False when the document has already disappeared between the two calls) is now silently discarded, so a concurrent deletion between the fetch and the delete would still return {"success": True}. While this race window is tiny, the previous single-call pattern avoided it entirely.

Consider at least checking the return value:

Suggested change
action_item = action_items_db.get_action_item(uid, action_item_id)
if not action_item:
raise HTTPException(status_code=404, detail="Action item not found")
if action_item.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this action item.")
action_items_db.delete_action_item(uid, action_item_id)
return {"success": True}
action_item = action_items_db.get_action_item(uid, action_item_id)
if not action_item:
raise HTTPException(status_code=404, detail="Action item not found")
if action_item.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this action item.")
deleted = action_items_db.delete_action_item(uid, action_item_id)
if not deleted:
raise HTTPException(status_code=404, detail="Action item not found")
return {"success": True}



Expand All @@ -557,6 +566,8 @@ def update_action_item(
action_item = action_items_db.get_action_item(uid, action_item_id)
if not action_item:
raise HTTPException(status_code=404, detail="Action item not found")
if action_item.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this action item.")

# Build update data from non-None fields
update_data = {}
Expand Down Expand Up @@ -1028,6 +1039,8 @@ def delete_conversation_endpoint(
conversation = conversations_db.get_conversation(uid, conversation_id)
if not conversation:
raise HTTPException(status_code=404, detail="Conversation not found")
if conversation.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this conversation.")

conversations_db.delete_conversation(uid, conversation_id)
return {"success": True}
Expand All @@ -1049,6 +1062,8 @@ def update_conversation_endpoint(
conversation = conversations_db.get_conversation(uid, conversation_id)
if not conversation:
raise HTTPException(status_code=404, detail="Conversation not found")
if conversation.get('is_locked', False):
raise HTTPException(status_code=402, detail="A paid plan is required to access this conversation.")

if request.title is None and request.discarded is None:
raise HTTPException(status_code=422, detail="At least one field (title or discarded) must be provided")
Expand Down
1 change: 1 addition & 0 deletions backend/routers/knowledge_graph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def get_knowledge_graph(uid: str = Depends(auth.get_current_user_uid)):

def _rebuild_graph_task(uid: str, user_name: str):
memories = memories_db.get_memories(uid, limit=500)
memories = [m for m in memories if not m.get('is_locked', False)]
rebuild_knowledge_graph(uid, memories, user_name)


Expand Down
1 change: 1 addition & 0 deletions backend/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pytest tests/unit/test_fair_use_free_tier.py -v
pytest tests/unit/test_timeout_middleware.py -v
pytest tests/unit/test_pusher_circuit_breaker.py -v
pytest tests/unit/test_lock_bypass_fixes.py -v
pytest tests/unit/test_dev_api_lock_bypass.py -v

# Fair-use integration tests (require Redis; skip gracefully if unavailable)
if redis-cli ping >/dev/null 2>&1; then
Expand Down
Loading
Loading