optbuilder, sql: fix stable sql api edge cases

The logictests have flushed out a handful of situations in which the
stable sql api gates block unexpectedly. Examples of this include unsafe
access in safe builtin execution, nested delegates, and other similar
use cases. What was required to fix them was to update the way we
unguard the unsafe internals during query planning.

Fixes: cockroachdb#154675
Epic: CRDB-55267
Release note: None

Author: angles-n-daemons

pkg/sql/opt/optbuilder/builder.go

@@ -593,6 +593,11 @@ func (b *Builder) maybeTrackUserDefinedTypeDepsForViews(texpr tree.TypedExpr) {
 // DisableUnsafeInternalCheck is used to disable the check that the
 // prevents external users from accessing unsafe internals.
 func (b *Builder) DisableUnsafeInternalCheck() func() {
+	// Already in the middle of a disabled section.
+	if b.skipUnsafeInternalsCheck {
+		return func() {}
+	}
+
 	b.skipUnsafeInternalsCheck = true
 	var cleanup func()
 	if b.catalog != nil {

pkg/sql/opt/optbuilder/routine.go

@@ -51,6 +51,11 @@ func (b *Builder) buildUDF(
 		))
 	}
 
+	// builtins should have access to unsafe internals
+	if o.Type == tree.BuiltinRoutine {
+		defer b.DisableUnsafeInternalCheck()()
+	}
+
 	// Check for execution privileges for user-defined overloads. Built-in
 	// overloads do not need to be checked.
 	if o.Type == tree.UDFRoutine {

pkg/sql/opt_catalog.go

@@ -742,13 +742,9 @@ func (oc *optCatalog) codec() keys.SQLCodec {
 	return oc.planner.ExecCfg().Codec
 }
 
-// DisableUnsafeInternalCheck sets the planners skipUnsafeInternalsCheck
-// to true, and returns a function which reverses it to false.
+// DisableUnsafeInternalCheck forwards the call to the planner.
 func (oc *optCatalog) DisableUnsafeInternalCheck() func() {
-	oc.planner.skipUnsafeInternalsCheck = true
-	return func() {
-		oc.planner.skipUnsafeInternalsCheck = false
-	}
+	return oc.planner.DisableUnsafeInternalsCheck()
 }
 
 // optView is a wrapper around catalog.TableDescriptor that implements

pkg/sql/planner.go

@@ -667,6 +667,19 @@ func (p *planner) InternalSQLTxn() descs.Txn {
 	return &p.internalSQLTxn
 }
 
+// DisableUnsafeInternalCheck sets the skipUnsafeInternalsCheck property
+// to true, and returns a function which reverses it to false.
+func (p *planner) DisableUnsafeInternalsCheck() func() {
+	if p.skipUnsafeInternalsCheck {
+		return func() {}
+	}
+
+	p.skipUnsafeInternalsCheck = true
+	return func() {
+		p.skipUnsafeInternalsCheck = false
+	}
+}
+
 func (p *planner) regionsProvider() *regions.Provider {
 	if txn := p.InternalSQLTxn(); txn != nil {
 		_ = txn.Regions() // force initialization

pkg/sql/unsafesql/unsafesql_test.go

@@ -231,6 +231,16 @@ func TestAccessCheckServer(t *testing.T) {
 			Passes:     false,
 			LogsDenied: true,
 		},
+		{
+			// test nested delegates
+			Query:  "SHOW JOBS WHEN COMPLETE SELECT job_id FROM [SHOW JOBS]",
+			Passes: true,
+		},
+		{
+			// unexpected unsafe builtin access
+			Query:  "SELECT col_description(0, 0)",
+			Passes: true,
+		},
 	} {
 		t.Run(fmt.Sprintf("query=%s,internal=%t,allowUnsafe=%t", test.Query, test.Internal, test.AllowUnsafeInternals), func(t *testing.T) {
 			accessedSpy.Reset()