fix(gateway): drop role/principalId from POST /v1/contacts body (ATL-515)#30372
Merged
Conversation
The native handler introduced in #30141 accepted role and principalId from the request body and passed them straight into ContactStore.upsertContact. The route is protected only by generic edge auth, so any authenticated caller could rebind the guardian by POSTing the guardian's contact id plus role:"guardian" + their own principalId — elevating to guardian for every guardian-only flow. Fix: strip role and principalId from the route input AND from ContactStore.upsertContact's params surface (the route is the only caller; guardian binding is owned by guardian-bootstrap, which writes guardian role via raw SQL with its own privileged path). On update, existing role/principalId are preserved. On create, role defaults to "contact" and principalId to null. Test added: privilege-escalation regression asserting POST /v1/contacts with role:"guardian" + principalId in body does not forward those fields to the service layer.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
dvargasfuertes
approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes ATL-515 — P2 / High privilege-escalation finding from Carson's Shard audit.
The vulnerability
The native
POST /v1/contactshandler introduced in #30141 forwardedbody.roleandbody.principalIdstraight intoContactStore.upsertContact(). The route's auth is generic"edge"— no guardian-specific gate. So any authenticated caller could:GET /v1/contacts→ find the guardian contact id.POST /v1/contactswith{ id: <guardian id>, displayName: "x", role: "guardian", principalId: "<attacker principal>" }.ContactStore.upsertContactmatch-by-id path overwrites the existing row's role + principalId.guardian-bootstraplookup (WHERE role = 'guardian') returns the attacker's principalId — full privilege elevation for every guardian-only flow.A new guardian could also be created from scratch by sending a fresh
{ role: "guardian", principalId: ..., channels: [...] }.The fix
roleandprincipalIdare removed from the route handler and fromContactStore.upsertContact's param surface. The store-level removal is the structural guarantee — there is no longer a code path that lets a caller ofupsertContact()set those fields, so a future regression in the route handler can't reintroduce the vector either.role/principalIdare preserved (display name + channels still update).roledefaults to"contact",principalIdtonull.Guardian role is set exclusively through
guardian-bootstrap, which uses raw SQL and runs on a privileged path. The only call site ofContactStore.upsertContact()was the route handler itself — no other internal caller is affected. Confirmed no caller inassistant/orclients/POSTs role/principalId to/v1/contacts.Tests
Added
strips role and principalId from request body (privilege escalation guard)tocontacts-control-plane-proxy.test.ts— asserts that when a request body containsrole: "guardian"+principalId: "attacker-principal-id", those fields are not present in the params the route passes toContactStore.upsertContact. Other 19 tests in the file pass; adjacentcontact-store-mark-channel-verified+ipc-contact-routessuites pass;bunx tsc --noEmitclean.