Hi,
On Mon, Mar 23, 2026 at 8:31 PM Jim Jones <[email protected]> wrote:
>
> This is a step forward in really isolating contents of temp tables from
> other sessions, but the more I think about it, the more I'm concerned
> with the current approach -- I spent some time investigating this
> problem a bit deeper last week.
>
> My main concern is the usage of gram.y, as a parser is arguably fragile
> for this kind of things.
I don't actually want to use gram.y as a main solver of this issue. But
gram.y is setting the "relpersistence" field for the RangeVar and all
subsequent code is treating this value as truthful. In particular, v12 patch
is relying on this field in RangeVar during the resolution of access issues,
and IMHO it's not out of line with the current code base. For instance, we are
considering RangeVar to determine whether we can perform a CREATE operation
within the specified namespace (see RangeVarAdjustRelationPersistence).
Even if we decide not to touch the gram.y in this patch, I still think that
leaving the "relpesistence" field misleading may lead to more bugs appearing
in the future. I.e. it should be fixed anyway (maybe in another thread?).
> For instance, one can always change the
> search_path and bypass this restriction:
>
> (table t was created in a different session)
>
> postgres=# SELECT * FROM pg_temp_81.t;
> ERROR: cannot access temporary relations of other sessions
> LINE 1: SELECT * FROM pg_temp_81.t;
> ^
> postgres=# SET search_path = pg_temp_81, public;
> SET
> postgres=# SELECT * FROM t;
> ?column?
> ----------
> (0 rows)
>
> * See: if (relation->relpersistence == RELPERSISTENCE_TEMP) in
> namespace.c for more details.
Yeah, you are right. It turns out that the current patch doesn't fully
protect other temp tables from superuser. The first thought that comes to me
is to forbid setting other-temp-namespaces in a search_path parameter. I know
it's starting to look ugly. But actually, such a restriction seems quite
logical.
>
> IMO, since it is an access control issue, I guess we better treat it as
> such and modify aclchk.c instead.
>
> Something like this the file attached. This breaks an unrelated test,
> which is potentially a bug in REPACK ... but I'll describe it in another
> thread.
>
> Thoughts?
Thank you for the patch! It seems much more beautiful and convenient to
maintain, but I have a little concern about it.
Actually, in your implementation we can DROP other temp tables not because we
make an exception to the general rule ("don't touch other temp tables"), but
because we just can perform such operations with every object in pg_temp_0.
If other operations with the same access checking as DROP command appear in
the future, the user will be able to perform these operations for
other-temp-tables. I.e. we will need to manually prevent such new operations
from accessing other-temp-tables. Have I gone too far in my reasoning?
BTW, your implementation allows calling a VACUUM for other-temp-tables. I think
that we should forbid that too.
On Mon, Mar 23, 2026 at 8:44 PM Tom Lane <[email protected]> wrote:
>
> Jim Jones <[email protected]> writes:
> > This is a step forward in really isolating contents of temp tables from
> > other sessions, but the more I think about it, the more I'm concerned
> > with the current approach -- I spent some time investigating this
> > problem a bit deeper last week.
>
> Yeah. I think this entire approach is wrongheaded: we do not enforce
> permissions checks against superusers. Moreover, if we try to fix it
> at the permissions level, it seems nearly certain that there will be
> bypass paths, simply because superusers bypass so many other checks.
>
Actually, v12 patch is not about a superuser rights restriction, but about
forbidding such operations for everyone. Anyway, we have a new perl test that
will prevent adding a code that will allow superuser to (somehow) break the
protection of both mine and Jim's patches. Isn't that a sufficient guarantee
that superuser will not bypass checks that it must not bypass?
> The actual problem is that the buffer manager is incapable of dealing
> with other sessions' temp tables, and we need to un-break the buffer
> manager's defense for that implementation restriction. So I feel the
> correct approach is something similar to what I described here:
>
> https://www.postgresql.org/message-id/flat/2736425.1758475979%40sss.pgh.pa.us
>
> I'm not wedded to that specific patch, but that is the implementation
> level where the fix is needed.
>
Handling access to other-temp-tables on the buffer manager level seems to me
like fighting the symptom, not the cause. Protection of other-temp-tables is
kinda "upper-level logical restriction". At the same time, buffer manager is a
lower-level implementation which shouldn't face such upper-level issues.
Am I missing something?
--
Best regards,
Daniil Davydov
