What we want
POST /documents, DELETE /documents/{doc_id}, and GET /audit/document/{doc_id} should require a caller identity. Today only /ask does.
Why it matters
app/main.py opens with "The trust boundary lives here", and /ask holds that line: it takes user_id: str = Depends(current_user) (line 116), resolves the principal server-side, and fails closed on an unknown user (lines 120 to 125).
The other three endpoints have no such dependency:
create_document (line 173): async def create_document(doc: DocIn) -> IngestResponsedelete_document (line 197): async def delete_document(doc_id: str) -> Noneaudit_document (line 206): async def audit_document(doc_id: str) -> dict
None of them take Depends(current_user), so with no header at all an anonymous caller can:
- Ingest a document with an arbitrary
acl list (DocIn.acl, line 60), including granting themselves access to a new document, or re-post an existing id. ingest in app/ingest.py is upsert-by-id (line 161) and replaces the ACL wholesale (lines 174 to 179), so re-posting a known doc id rewrites who can see it.
- Soft delete any document by id, removing it from retrieval immediately (
soft_delete, app/ingest.py line 194).
- Read the audit trail for any document: who asked what, and when (
app/main.py lines 216 to 226). That endpoint exists for the "was this leaked" question, and right now it answers that question for anyone.
The retrieval query is genuinely airtight; that is not in dispute and tests/test_acl.py proves it. The gap is that the write and audit surface can rewrite the ACLs that the airtight query reads. A project whose entire premise is permission-aware retrieval should not leave that door open, even in a demo.
Suggested approach
Keeping the same "auth is a header, real deployments swap in OIDC/JWT" stance the module docstring takes (lines 10 to 13), this can stay small:
- Add
Depends(current_user) to all three endpoints so an anonymous caller is rejected at the boundary rather than served.
- For
/audit/document/{doc_id}, at minimum require identity. If you want to go further, gate it on the caller being the document owner or a member of an admin group, and say so in the docstring.
- For the write endpoints, document explicitly in the docstring what the intended authorization model is (for example "any authenticated caller may ingest" versus "only the owner may replace"), so the next reader knows what is deliberate.
- Update
tests/test_api.py. Note that _seed (line 113 onward) posts to /documents with no headers today, so it will need the same header the other requests use. Add a test in the style of test_ask_requires_identity (line 97) asserting each endpoint rejects a request with no identity.
- Update the endpoint table in the README (around lines 220 to 224) to say which endpoints require the identity header.
If you would rather split this, the audit endpoint alone is a reasonable first PR.
Running it
docker compose up -d db
pytest -q
Comment here if you want to pick this up, or if you would like to argue the scope down first. I usually reply within a day.
What we want
POST /documents,DELETE /documents/{doc_id}, andGET /audit/document/{doc_id}should require a caller identity. Today only/askdoes.Why it matters
app/main.pyopens with "The trust boundary lives here", and/askholds that line: it takesuser_id: str = Depends(current_user)(line 116), resolves the principal server-side, and fails closed on an unknown user (lines 120 to 125).The other three endpoints have no such dependency:
create_document(line 173):async def create_document(doc: DocIn) -> IngestResponsedelete_document(line 197):async def delete_document(doc_id: str) -> Noneaudit_document(line 206):async def audit_document(doc_id: str) -> dictNone of them take
Depends(current_user), so with no header at all an anonymous caller can:acllist (DocIn.acl, line 60), including granting themselves access to a new document, or re-post an existingid.ingestinapp/ingest.pyis upsert-by-id (line 161) and replaces the ACL wholesale (lines 174 to 179), so re-posting a known doc id rewrites who can see it.soft_delete,app/ingest.pyline 194).app/main.pylines 216 to 226). That endpoint exists for the "was this leaked" question, and right now it answers that question for anyone.The retrieval query is genuinely airtight; that is not in dispute and
tests/test_acl.pyproves it. The gap is that the write and audit surface can rewrite the ACLs that the airtight query reads. A project whose entire premise is permission-aware retrieval should not leave that door open, even in a demo.Suggested approach
Keeping the same "auth is a header, real deployments swap in OIDC/JWT" stance the module docstring takes (lines 10 to 13), this can stay small:
Depends(current_user)to all three endpoints so an anonymous caller is rejected at the boundary rather than served./audit/document/{doc_id}, at minimum require identity. If you want to go further, gate it on the caller being the documentowneror a member of an admin group, and say so in the docstring.tests/test_api.py. Note that_seed(line 113 onward) posts to/documentswith no headers today, so it will need the same header the other requests use. Add a test in the style oftest_ask_requires_identity(line 97) asserting each endpoint rejects a request with no identity.If you would rather split this, the audit endpoint alone is a reasonable first PR.
Running it
Comment here if you want to pick this up, or if you would like to argue the scope down first. I usually reply within a day.