control-plane: guarded, auto-expiring temporary support access to restricted tenants

[skord]

What

Adds declarative expiration timestamps to the grant tables, and a guarded,
auto-expiring mechanism for temporary support access to restricted tenants
that are deliberately excluded from the standing estuary_support/ role.

Why

Today, giving a restricted tenant temporary support access means an operator with
broad database access hand-writes a role_grants row and is trusted to remove it.
There is no expiry, no record of who granted it or why, and it requires write
access to the system-wide authorization table, which is also the ability to grant
anyone admin on anything. See ADR estuary/security#746.

What's in this PR

One migration plus its pgTAP tests. No Rust, snapshot, RLS, or
authorization-function changes.

• role_grants.expires_at and user_grants.expires_at (nullable timestamptz).
  NULL means permanent, which is every pre-existing row. SELECT-only column
  grants for PostgREST-facing roles: readable (required for
  RETURNING * dashboard mutations) but writable only through the functions
  below.
• internal.support_access: append-only audit log of who granted or revoked
  what, when, and why. Doubles as access-review evidence.
• internal.grant_support_access(tenant, reason, duration): attaches
  estuary_support/ admin to the tenant with an expiry, as one race-free
  upsert. A repeat call extends the window (later expiry wins); a tenant that
  already has permanent support access is refused.
• internal.revoke_support_access(tenant): detaches immediately and marks the
  audit rows revoked.
• internal.expire_support_access(): deletes rows in both grant tables whose
  expires_at has passed. Run on a schedule (see below).
• EXECUTE revoked from PUBLIC on all three functions.

Why it's safe

• Operators get EXECUTE on the functions and never write role_grants
  directly, so they cannot escalate their own access. The functions only attach
  or detach estuary_support/ on a named, existing tenant, and record
  session_user, reason, and expiry.
• Permanent grants are protected structurally: every destructive statement
  (revoke, sweep) matches only rows with a non-NULL expires_at, so the
  permanent grants normal tenants receive can never be touched. The grant
  function's conflict guard likewise refuses to convert a permanent grant into
  a temporary one.
• Because expiry is enforced by deleting the grant row, it propagates to every
  consumer of these tables (RLS, the authorization snapshot, publications) with
  no changes to any of them.

Deliberately not in this PR

Read-side filtering of expires_at (in user_roles/task_roles, RLS, or the
Rust snapshot loader). That would allow expiry without deletion, and composes
cleanly on top of these columns as a follow-up if we want it. Nothing writes
user_grants.expires_at yet; the column and its sweep exist so the semantics
are uniform and ready for future use (e.g. customer-initiated support access).

Required at rollout (out of band, not in this migration)

1. Register the expiry sweep once on the Supabase database, after the
   migration is applied (pg_cron is absent from the sqlx test cluster, so the
   migration cannot do it):

   select cron.schedule('expire-support-access', '* * * * *',
                        $$ select internal.expire_support_access() $$);

   Verify it's scheduled and firing:

   select jobid, jobname, schedule, active from cron.job
     where jobname = 'expire-support-access';
   select * from cron.job_run_details order by start_time desc limit 5;

   Note the sweep is the only enforcement of expiry: if the job stops, lapsed
   grants stay live until revoked manually. Check cron.job_run_details as
   part of access reviews.

2. Grant EXECUTE on the grant/revoke functions to operator roles via the
   departmental database-roles tooling (a separate private repo, not flow
   migrations).

Testing

• supabase db reset applies all migrations cleanly.
• pgTAP covers: grant attaches with a future expiry and logs the audit row;
  input validation with exact error messages; refusal to grant over or revoke a
  permanent grant; window extension (later expiry wins, single row per tenant,
  audit records effective expiry); re-grant over a lapsed-but-unswept window;
  immediate revoke; and the sweep across both tables, sparing still-open
  windows and permanent grants. Full pgTAP suite passes.
• The control-plane-api Rust test suite passes against the migrated schema
  with zero Rust changes, confirming the added columns are forward-compatible.

Related

• Implements ADR estuary/security#746.

[qaoj]

I think everything here makes sense- SQL edge cases are hard to reason about without actually running it...

[jgraettinger]

/cc @GregorShear

We've had conversations about adding expires_at / revoked_at or similar as nullable new columns of user_grants and role_grants. Thoughts on doing that instead, and making it the primary lever for support access? In particular we can have the Snapshot loader push down filters on these columns so they're reacted too immediately with each Snapshot refresh.

[jgraettinger]

(marking request changes because I'd like to get group consensus on where these new expiration columns live)

[jshearer]

As far as I see it, we want to accomplish two things here: as we're moving to individual database users for support access, we need a way for staff to record and time-limit support-role use ASAP. Simultaneously, we also want this design to lead into the feature that will allow customers to ask for support themselves, when we get there.

After doing some digging and remembering/loading this into context, I agree with @jgraettinger -- I think it would be better to frame this as plumbing overtop of a new declarative expiration timestamp on grants, rather than as an imperative periodic cron task to delete specific grants.

The internal.support_access table is good because it acts as an audit log of who took what action when: it's possible to have multiple support access requests targeting a specific grant(s), and we'll want to store a log of that full history. I also think the SECURITY DEFINER functions are a great way to enable support staff (via their individual LOGON roles) to record access decisions.

In addition to adding support for expiring grants to the snapshot logic in rust, we'll also need to add it to RLS as we still rely on that for a bunch of authorization decisions today

[GregorShear]

I don't think I have much to contribute here - adding the expiration directly to the user/role_grants sounded like a pretty lightweight change to make on the snapshot. The expiration mechanism is easy to reason about, though I don't have a good sense of how big or risky of a change is needed on the RLS side while it's still alive.

[jgraettinger]

In addition to adding support for expiring grants to the snapshot logic in rust, we'll also need to add it to RLS as we still rely on that for a bunch of authorization decisions today

To my knowledge, RLS is used only for AuthZ internal to a publication (e.x. task to task access) today. All gate-keeping about what a user may access from a data-plane is already brokered by the authorization API and its snapshots.

I'd recommend a) not touching RLS, b) extending role_grants and user_grants with expiration timestamps (that RLS checks wouldn't use, but snapshots fetches filter on), and c) retaining a cron that clears out grants which have expired (which bounds how long the legacy RLS could continue to allow them).

[skord]

PTAL @jshearer or @jgraettinger. The natives who would like access to support customers are restless.

[skord]

@jshearer I've reworked this to reflect what we discussed on the phone today, at least as I understood it; tell me if I got it wrong.

• expires_at (nullable timestamptz) now lives on role_grants and user_grants directly. NULL means permanent, which is every pre-existing row.
• Enforcement is unchanged from the original design: the scheduled sweeper deletes lapsed rows, so expiry propagates to every consumer with no changes anywhere else. No Rust/snapshot changes, no RLS or authorization-function changes; the PR is one migration plus its pgTAP tests.
• The columns make the safety guards structural: nothing with a NULL expires_at can ever be deleted by the grant/revoke/sweep paths, so permanent grants are protected by shape rather than by bookkeeping. Extending a window is a greatest() upsert on the tenant's single grant row.
• internal.support_access stays as the append-only audit log of who granted/revoked what and why.

Read-side filtering of expires_at (SQL cores, snapshot push-down) composes cleanly on top of these columns later if we want expiry without deletion. Deliberately not in this PR.

[jgraettinger]

Confirming your summary matches my understanding 👍

[jshearer]

LGTM % two small pieces of feedback neither of which I believe are blocking. The overlapping/extended-grants finding seems legit but also probably unlikely to actually happen in real life? WDYT

[jshearer]

Claude found the following review around handling of multiple overlapping (or subsequently extended) support access grants and whether we should mark them revoked_at=now() if we don't actually revoke anything. I'm happy for this to be a nothing burger, just thought I'd raise it

---

The revoked_at/revoked_by update here matches on the audit row's own expires_at, which we freeze at grant time and never move when a window gets extended. So the audit row and the role_grants row it's tracking can end up with different expiries, and the sweep stamps a row revoked while support access is still live.

Say you grant an hour, then extend to 24. The extension bumps the single role_grants row to +24h but leaves the first audit row sitting at +1h. An hour in, the sweep deletes nothing from role_grants (access is still live) but marks that first audit row revoked_by = 'internal.expire_support_access', so the access-review trail says support lost access at +1h when it really had another 23 hours. Same thing the other way: if the grant row gets deleted early out of band, the audit rows sit open until their expires_at and then get stamped as if the window ran its full course.

revoked_at is null and expires_at <= now() already tells you a row has lapsed, so I'd drop this update and only set revoked_at/revoked_by from revoke_support_access (which also kills the sentinel revoked_by string).

[skord]

Not a nothing burger, it was real: the sweep could stamp a subsumed window as revoked by enforcement while the tenant's access was still live. Fixed in 29fd9eb by going one further than the suggestion: the sweep no longer writes internal.support_access at all. revoked_at/revoked_by now strictly mean a named person revoked explicitly, a lapsed window is self-describing (revoked_at IS NULL and expires_at <= now()), and the sentinel revoked_by string is gone. Tests assert lapsed audit rows stay unstamped.

[jshearer]

nit: should we be asserting that the duration is positive?

if p_duration <= interval '0' then
  raise exception 'support access duration must be positive';
end if;

[skord]

Taken in 29fd9eb, with a NULL check too: now() + NULL computes a NULL expires_at, i.e. a permanent-looking grant. It could not actually escape (the audit insert's NOT NULL aborts the transaction) but that was safety by accident with a confusing error. Both cases now raise 'support access duration must be positive', and tests cover each.

[jshearer]

LGTM!