Skip to content

Gate HITL actions and memory writes through the control plane - #16

Open
saitej123 wants to merge 1 commit into
theschoolofai:mainfrom
saitej123:fix/control-plane-missed-writes
Open

saitej123 wants to merge 1 commit into
theschoolofai:mainfrom
saitej123:fix/control-plane-missed-writes

Conversation

@saitej123

Copy link
Copy Markdown

What breaks

require_control already guards subscriptions, events, runs, and resume. Four other write paths were left off that list:

  • POST /v1/action — HITL approve / reject / rerun of a parked node
  • POST /v1/agent/facts — inject a fact into tenant memory
  • POST /v1/agent/documents — index a document into tenant memory
  • POST /v1/agent/memory/search — recall across a tenant chosen in the JSON body

tests/test_control_plane_auth.py only enumerated the first four. An anonymous caller on the agent port could approve a waiting run or write memory as Principal("gateway","gateway"). The control token on the other routes does not help if these stay open.

Reproduction

On current main, both of these fail (they return 200 / 404 instead of 503 / 401):

uv run pytest tests/test_control_plane_auth.py -q

Without the test: unset S16_CONTROL_TOKEN and POST /v1/agent/facts with any tenant. Before this change the fact is stored. After it, the route answers 503 until a token is configured, then 401 for the wrong bearer.

Fix

Add dependencies=[Depends(require_control)] on those four routes. Extend WRITE_PATHS so the fail-closed and wrong-token cases cover them.

uv run pytest tests/test_control_plane_auth.py tests/test_runtime.py passes (24). Runtime tests already send the control token via the app_client fixture.

POST /v1/action, /facts, /documents, and /memory/search were left off the
require_control list, so an anonymous caller could approve a parked run or
inject tenant memory.

Co-authored-by: Cursor <cursoragent@cursor.com>
@theschoolofai

Copy link
Copy Markdown
Owner

Session 16 — graded ✅

Score: +100.

HITL actions and memory writes bypassed the control plane, so state-changing operations were reachable without the credential every neighbouring route required. First to file (2026-08-13 16:44).

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.

2 participants