[bug-bash] Deleted-and-recreated atespace can inherit its previous owner (or be deleted by its previous owner) #3255

Activity

1. Max Thompson (thompsonmax)

   added

   kind/bugSomething isn't working / bugfixesSomething isn't working / bugfixes

   on Oct 26, 2026
2. SHRUTI6991 commented on Oct 26, 2026

   

   Shruti Nair (SHRUTI6991)

   on Oct 26, 2026

   Last edited by SHRUTI6991

   Collaborator

   More actions

   It's unfortunate but our decision to use "name" instead of "uid" to store the resource results in known ABA problems. I think I will just document this as a known limitation since we anyway enforce min 26 characters and it's unlikely that the higher order systems would generate the same 26 char every single time.

   cc: Taahir Ahmed (@ahmedtd) Julian Gutierrez Oschmann (@juli4n) Greg Castle (@destijl)
3. thompsonmax commented on Oct 27, 2026

   

   Max Thompson (thompsonmax)

   on Oct 27, 2026

   CollaboratorAuthor

   More actions

   I haven't really dug too deep into either of these, but Shruti Nair (@SHRUTI6991) would either of these options resolve this?

   Why Keying by name Doesn't Force an Unfixable ABA Problem Here

   Two things are already true in atepg:

   1. The atespaces table already stores a per-incarnation uid column (cmd/ateapi/internal/store/atepg/migrations/000001_initial.sql:17-22: name text PRIMARY KEY, uid text NOT NULL, ...).
   2. DeleteAtespace (cmd/ateapi/internal/store/atepg/atespace.go:121-152) already deletes the atespaces row and its OpenFGA tuples atomically in a single PostgreSQL transaction tx (DELETE FROM atespaces at line 130 + DeleteAtespacePolicies(ctx, tx, name) at line 147 + tx.Commit at line 150).
      Because atespaces(name) is a PRIMARY KEY, incarnation 2 (uid_2) can never exist in atespaces until the transaction that deleted incarnation 1 (uid_1) and wiped uid_1's OpenFGA tuples has already committed.
      The ABA race only happens today because UnaryServerInterceptor (cmd/ateapi/internal/authz/interceptor.go:50-56) runs authorizer.Check outside the handler's DB transaction without tying the write to the atespace incarnation that existed when Check ran.

   Option 1: Read atespaces.uid Before authorizer.Check, and Assert That uid in the Write Transaction (~20 lines, zero schema or OpenFGA changes)

   For any atespace-scoped mutation (CreateAtespaceAccessPolicy, DeleteAtespace, etc.):

   1. Step 1 (Before authorizer.Check): Query SELECT uid FROM atespaces WHERE name = $1 -> expectedUID.
   2. Step 2: Run authorizer.Check(ctx, relation, "atespace:"+name) as today.
   3. Step 3 (Inside the write tx): Assert that the atespaces row still has uid = expectedUID:
      • In DeleteAtespace (cmd/ateapi/internal/store/atepg/atespace.go:130-134): WHERE name = $1 AND ($2::text = '' OR uid = $2::text) already exists—we just populate precondition.UID = expectedUID when the caller didn't supply one.
      • In CreateAtespaceAccessPolicy (cmd/ateapi/internal/store/atepg/access_policy.go:120-143): Run:

        SELECT 1 FROM atespaces WHERE name = $1 AND uid = $2 FOR NO KEY UPDATE

        inside tx before INSERT INTO atespace_access_policies and ReconcileAtespaceBindings, returning Aborted / NotFound if zero rows match.
        Why this eliminates all ABA interleavings:

   • If Step 1 ran before DeleteAtespace(uid_1) committed: expectedUID is uid_1. Even if Step 2 (authorizer.Check) passes against uid_1's tuples and the atespace is then deleted and recreated as uid_2, Step 3 checks WHERE name = $1 AND uid = 'uid_1', sees uid_2 != uid_1, and aborts without touching uid_2.
   • If Step 1 ran after CreateAtespace(uid_2) committed: expectedUID is uid_2. Because uid_2 is already committed in atespaces, DeleteAtespace(uid_1) (and its atomic OpenFGA tuple wipe) must have already committed before Step 1 started — which means Step 2 (authorizer.Check, which starts after Step 1) is guaranteed to see 0 tuples for the old owner and reject with PermissionDenied.

   Option 2: Lock the atespaces Row and Run authorizer.Check on tx

   In cmd/ateapi/internal/authz/datastore.go:62-63, transactionalDatastore already routes Write, ReadPage, and ReadAuthorizationModel through TxFromContext(ctx), and only leaves Read, ReadUserTuple, and ReadUsersetTuples (used by fgaServer.Check) on the pool.
   If those three read methods also use TxFromContext(ctx) when present (see also #3205), any mutation that locks SELECT 1 FROM atespaces WHERE name = $1 FOR NO KEY UPDATE in tx and calls authorizer.Check(ContextWithTx(ctx, tx), ...) is strictly serialized in PostgreSQL either before DeleteAtespace (on uid_1) or after DeleteAtespace commits (when uid_1's tuples are already gone).
4. SHRUTI6991 commented on Oct 27, 2026

   

   Shruti Nair (SHRUTI6991)

   on Oct 27, 2026

   Last edited by SHRUTI6991

   Collaborator

   More actions

   I don't like both the options.

   Option 1 is reading UID from the store and passing that down to handler, that's additional check + we have to maintain some caching for it ourselves.
   Option 2 is worse I feel, since now we need transactional read which might make parallel reads slower.

   Option 3: document the limitation since we anyway enforce 26 bytes of characters and it's unlikely that the higher order systems would generate the same 26 char every single time.
5. Shruti Nair (SHRUTI6991)

   added a parent issue on Oct 27, 2026

   • Substrate Authorization Bug Bash Outcome Tracker #3190
6. Bowei Du (bowei)

   added

   area/api-machinery

   area/securitySecurity related issue/prSecurity related issue/pr

   on Oct 27, 2026
7. ahmedtd commented on Oct 28, 2026

   

   Taahir Ahmed (ahmedtd)

   on Oct 28, 2026

   Collaborator

   More actions

   I suspect this means we need to turn Postgres up to Serializable isolation? Otherwise I'm not sure how these two things that happen in transactions can conflict?
8. SHRUTI6991 commented on Oct 29, 2026

   

   Shruti Nair (SHRUTI6991)

   on Oct 29, 2026

   Collaborator

   More actions

   The issue

   ate-api-server handles an atespace-scoped write in two separate steps:

   1. Authorize. UnaryServerInterceptor (cmd/ateapi/internal/authz/interceptor.go) calls authorizer.Check("can_create_access_policy", "atespace:config-race2"). OpenFGA reads the tuples straight from the connection pool, so this is its own autocommit transaction.
   2. Write. The handler opens a new transaction and inserts the policy and owner tuples by name: atespace_name = 'config-race2'. The FK on atespace_access_policies references atespaces(name).

   Nothing ties step 2 to the atespace that step 1 approved. Both steps refer only to the name, and the name can point to a different atespace by the time step 2 runs.

   Timeline

   t0  mallory: Check(owner on atespace:config-race2)    -> ALLOWED  (old tuples, uid_1)
   t1  admin:   DeleteAtespace(config-race2)              -> row uid_1 + tuples deleted, commit
   t2  admin:   CreateAtespace(config-race2)              -> row uid_2 inserted, commit
   t3  mallory: handler tx INSERT policy(config-race2)    -> FK on name matches uid_2, commit
   

   Result: the new atespace (uid_2) starts with mallory as owner, though she was never granted anything on it.

   DeleteAtespace has the same race. Mallory's delete is authorized against uid_1 but runs DELETE ... WHERE name = 'config-race2', so it deletes uid_2.

   The 26-character name rule doesn't cover this. The attacker doesn't guess a name. The trigger is an admin recreating the same name, which is normal for deterministic or GitOps-managed names.

   Why Serializable won't fix it

   Serializable guarantees the result is equivalent to some one-at-a-time order of the transactions. Postgres sees four separate transactions here:

   Txn	Who	What
   T1	mallory's Check (autocommit, on the pool)	reads owner tuple for atespace:config-race2, finds it
   T2	admin DeleteAtespace	deletes the atespace row and its tuples
   T3	admin CreateAtespace	inserts a new config-race2 row
   T4	mallory's handler tx	inserts the policy and owner tuple

   Running them one at a time as T1 → T2 → T3 → T4 gives exactly the bad outcome. That history is already serializable, so Postgres has nothing to detect and aborts nothing.

   The bug is that the application treats "check, then write" as one operation while the database sees two. Isolation levels only protect what happens inside one transaction.

   Short-term fix: pin the write to the UID that was checked

   This needs no schema change and no OpenFGA change.

   1. Interceptor: before Check, run SELECT uid FROM atespaces WHERE name = $1, then put expectedUID in the context.
   2. Handler transaction: fail unless the atespace still has that UID.
      • DeleteAtespace: set precondition.UID = expectedUID when the caller left it empty. The AND uid = $2 clause already exists.
      • Create, update and delete of the atespace access policy: run SELECT 1 FROM atespaces WHERE name = $1 AND uid = $2 FOR SHARE before writing. Return Aborted if no row matches.

   Why it's correct:

   • UID read before the delete: expectedUID = uid_1. At t3 the row is uid_2, the assert fails, and the write aborts.
   • UID read after the recreate: expectedUID = uid_2. The delete at t1 already removed mallory's tuples in the same transaction, so Check denies.

   It costs one primary-key lookup per request, with nothing to cache.

   Also in this change:

   • Add a regression test that runs delete and recreate alongside a concurrent policy create, and asserts the new incarnation has no policy.
   • Note in docs/authorization.md that other atespace-scoped writes are still affected until the long-term fix.

   Long-term fix: key atespace identity by UID

   Make the race structurally impossible for every atespace-scoped RPC:

   • OpenFGA objects: use atespace:<uid> instead of atespace:<name>.
   • Child tables: atespace_access_policies, actors, and the rest reference atespaces(uid) with ON DELETE CASCADE.
   • Interceptor: resolve name to UID once, run Check against atespace:<uid>, and pass the UID to the handler. Store writes take the UID.
   • A late write for uid_1 then fails on the FK, because that row is gone. uid_2 has no tuples for anyone the admin didn't grant.

   We're pre-v1.0.0, so this can change the schema and tuples directly, with no migration or compatibility code. It touches every atespace-scoped table and RPC, so it should get its own issue and design discussion.
9. SHRUTI6991 commented on Oct 29, 2026

   

   Shruti Nair (SHRUTI6991)

   on Oct 29, 2026

   Collaborator

   More actions

   Synched offline. We are documenting this as a known limitation.
10. Yuan Gao (ygao-g)

    added a commit that references this issue on Nov 3, 2026

    docs: Document that atespace names must not be reused (agent-substrat…

    ...

    9d16b69
11. Eitan Yarmush (EItanya)

    added a commit that references this issue on Nov 3, 2026

    docs: Document that atespace names must not be reused (agent-substrat…

    ...

    4e3209f
12. Dmitry Berkovich (dberkov)

    added a commit that references this issue on Nov 3, 2026

    docs: Document that atespace names must not be reused (#3450)

    ...

    4b61cf5