Skip to content

fix(security): object-level ownership on the compliance surfaces (1a.1)#263

Open
MoltyCel wants to merge 1 commit into
mainfrom
fix/compliance-object-ownership
Open

fix(security): object-level ownership on the compliance surfaces (1a.1)#263
MoltyCel wants to merge 1 commit into
mainfrom
fix/compliance-object-ownership

Conversation

@MoltyCel

Copy link
Copy Markdown
Owner

1a.1 — Compliance IDOR / ownership (Welle 1a)

The four endpoints authenticated the caller () but performed no object-level ownership check. Any valid API-key holder could act on any DID:

Endpoint Before Impact
any key ⟶ any mint a MolTrust-signed conformity VC for someone elses DID (impersonation of the signed attestation)
any key ⟶ any write an assessment into any DIDs history
any key ⟶ any file an Art 73 incident against any DID
any key ⟶ any read any DIDs compliance record (IDOR)

Fix

One shared guard :

  • caller passes iff their API key resolves to that exact DID () or they present the admin key ();
  • uniform 403 whether the caller is the wrong owner or the DID does not exist — checked before the existence lookup on , so the endpoints are not a DID-enumeration oracle;
  • admin bypasses the DB lookup (no dependency for the admin path).

Mirrors the owner-or-admin idiom already used at (main.py:6085) and (main.py:6572).

Design decisions (flagged for review)

  • ** is now owner-or-admin, not public** — per instruction. Conformity VCs are independently verifiable, but incident/assessment records are sensitive, so the report aggregate is closed.
  • ** with no stays open** — stateless classification, nothing persisted, no ownership needed. Ownership is enforced only when a is supplied (i.e. when it would write to that DIDs history).
  • Operator→agent delegation is NOT honored here (an operator key that operates an agent still cannot issue for it unless it is the owner or admin). Tightest correct fix; broadening to operator scope is a policy call left to you.

Tests —

own→200 · foreign→403 (assess/declaration/incident/report) · no- assess open · report no-oracle (foreign == ghost, same status+body) · admin bypass→200. Existing retargeted to the admin 404 path.
**Verified on sandbox DB: ** (test_compliance + test_incidents_anchors + test_compliance_ownership).

🤖 Generated with Claude Code

Do not merge without review — Welle 1a security fix, build-to-PR only per mandate.

The four /compliance/* endpoints authenticated the caller (verify_api_key)
but never checked object-level ownership, so any valid API-key holder could:

  * POST /compliance/declaration — mint a MolTrust-signed conformity VC for
    ANY subject_did (impersonation of the signed attestation),
  * POST /compliance/assess      — write an assessment into ANY DID's history,
  * POST /compliance/incident    — file an Art 73 incident against ANY DID,
  * GET  /compliance/report/{did}— read ANY DID's compliance record (IDOR).

Add a shared _require_did_owner_or_admin(request, api_key, subject_did): the
caller's API key must resolve to that exact DID, or they present the admin key.
The 403 is uniform whether the caller is the wrong owner or the DID does not
exist, so the endpoints are not a DID-enumeration oracle. assess with no `did`
stays open (stateless classification, nothing persisted). /compliance/report
is now owner-or-admin, not a public trust surface.

Tests: tests/test_compliance_ownership.py (own->200, foreign->403,
no-oracle, admin bypass); existing report-not-found retargeted to the admin
404 path. Mirrors the owner-or-admin idiom already used at agents/revoke and
delegation/create.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant