CE-1007: offline validate fails thrash grants binds

pkg/bsql/offline_validate.go

@@ -1,10 +1,24 @@
 package bsql
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"net/url"
+	"regexp"
 	"strings"
+
+	v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"
+	"github.com/conductorone/baton-sql/pkg/bcel"
+	"github.com/conductorone/baton-sql/pkg/database"
+)
+
+// Thrash / illegal parent-bind patterns produced by assist models. Checked offline
+// so validate_baton_sql_config fails closed before Apply/sync.
+var (
+	hybridPlaceholderRegex = regexp.MustCompile(`\$\d+<`)
+	dottedPlaceholderRegex = regexp.MustCompile(`\?<[^>\n]*\.[^>\n]*>`)
+	bareDollarParamRegex   = regexp.MustCompile(`\$[0-9]+`)
 )
 
 // OfflineValidate performs YAML-level structural checks without opening a DB or
@@ -33,7 +47,144 @@ func OfflineValidate(cfg *Config) error {
 			return fmt.Errorf("resource_types.%s.list: %w", name, err)
 		}
 	}
-	return RejectNonV1ProductFeatures(cfg)
+	if err := RejectNonV1ProductFeatures(cfg); err != nil {
+		return err
+	}
+	if err := validateQueryPlaceholderLegality(cfg); err != nil {
+		return err
+	}
+	return OfflineValidateGrantBinds(cfg)
+}
+
+// validateQueryPlaceholderLegality walks list/entitlements/grants query strings
+// for illegal thrash forms. Bare $N hard-fail is grants-only for this slice.
+func validateQueryPlaceholderLegality(cfg *Config) error {
+	for name, rt := range cfg.ResourceTypes {
+		if rt.List != nil {
+			if err := checkQueryPlaceholderLegality(
+				fmt.Sprintf("resource_types.%s.list.query", name),
+				rt.List.Query,
+				false,
+			); err != nil {
+				return err
+			}
+		}
+		if rt.Entitlements != nil {
+			if err := checkQueryPlaceholderLegality(
+				fmt.Sprintf("resource_types.%s.entitlements.query", name),
+				rt.Entitlements.Query,
+				false,
+			); err != nil {
+				return err
+			}
+		}
+		for i, g := range rt.Grants {
+			if g == nil {
+				continue
+			}
+			if err := checkQueryPlaceholderLegality(
+				fmt.Sprintf("resource_types.%s.grants[%d].query", name, i),
+				g.Query,
+				true,
+			); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func checkQueryPlaceholderLegality(path, query string, scanBareDollar bool) error {
+	if strings.TrimSpace(query) == "" {
+		return nil
+	}
+	// Hybrid before bare: $1<path> also matches \$[0-9]+.
+	if hybridPlaceholderRegex.MatchString(query) {
+		return fmt.Errorf("%s: forbidden hybrid $N<path> bind; use vars with a simple name and ?<name> (e.g. vars.group_id: \"resource.ID\" and ?<group_id>)", path)
+	}
+	if dottedPlaceholderRegex.MatchString(query) {
+		return fmt.Errorf("%s: placeholder keys may only be [A-Za-z0-9_]; cannot contain '.'; use vars with CEL resource.ID (e.g. vars.group_id: \"resource.ID\" then ?<group_id>)", path)
+	}
+	if scanBareDollar && bareDollarParamRegex.MatchString(query) {
+		return fmt.Errorf("%s: do not use raw $N placeholders in grants queries; use vars + ?<name> and let the engine bind (e.g. vars.group_id: \"resource.ID\" and WHERE col = ?<group_id>)", path)
+	}
+	return nil
+}
+
+// OfflineValidateGrantBinds dry-runs grants ?<...> expansion against a synthetic
+// parent resource without opening a database. Unknown tokens or unbound params fail.
+func OfflineValidateGrantBinds(cfg *Config) error {
+	if cfg == nil {
+		return errors.New("config is nil")
+	}
+	ctx := context.Background()
+	env, err := bcel.NewEnv(ctx)
+	if err != nil {
+		return fmt.Errorf("offline grants bind: cel env: %w", err)
+	}
+	s := &SQLSyncer{
+		dbEngine: database.PostgreSQL,
+		env:      env,
+	}
+	pCtx := &paginationContext{}
+
+	for rtName, rt := range cfg.ResourceTypes {
+		for i, g := range rt.Grants {
+			if g == nil {
+				continue
+			}
+			path := fmt.Sprintf("resource_types.%s.grants[%d]", rtName, i)
+			if err := offlineValidateOneGrantBind(ctx, s, pCtx, rtName, path, g); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func offlineValidateOneGrantBind(ctx context.Context, s *SQLSyncer, pCtx *paginationContext, rtName, path string, g *GrantsQuery) error {
+	tokens, err := s.queryVars(g.Query)
+	if err != nil {
+		return fmt.Errorf("%s.query: %w", path, err)
+	}
+	hasNonPagination := false
+	for _, tok := range tokens {
+		switch tok {
+		case limitKey, offsetKey, cursorKey:
+			// pagination tokens are engine-supplied
+		default:
+			hasNonPagination = true
+		}
+	}
+	if !hasNonPagination {
+		return nil
+	}
+
+	resource := &v2.Resource{
+		Id: &v2.ResourceId{
+			ResourceType: rtName,
+			Resource:     "g2",
+		},
+		DisplayName: "offline-validate-parent",
+	}
+	inputs := s.env.SyncInputsWithResource(nil, resource)
+	queryVars, err := s.PrepareQueryVars(ctx, inputs, g.Vars)
+	if err != nil {
+		return fmt.Errorf("%s.vars: %w", path, err)
+	}
+	// parseToken lowercases placeholder keys; align vars map keys for lookup.
+	normalized := make(map[string]any, len(queryVars))
+	for k, v := range queryVars {
+		normalized[strings.ToLower(k)] = v
+	}
+
+	updated, qArgs, _, err := s.parseQueryOpts(pCtx, g.Query, normalized)
+	if err != nil {
+		return fmt.Errorf("%s.query: %w", path, err)
+	}
+	_ = updated
+	_ = qArgs
+	return nil
 }
 
 // RejectNonV1ProductFeatures fails configs that are outside the C1 agent-authored

pkg/bsql/offline_validate_test.go

@@ -4,6 +4,9 @@ import (
 	"strings"
 	"testing"
 
+	v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"
+	"github.com/conductorone/baton-sql/pkg/bcel"
+	"github.com/conductorone/baton-sql/pkg/database"
 	"github.com/stretchr/testify/require"
 )
 
@@ -164,3 +167,164 @@ resource_types:
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "app_name")
 }
+
+func withGroupResourceAndGrants(grantsBlock string) string {
+	return `
+app_name: Golden Postgres
+app_description: grants thrash fixture
+connect:
+  scheme: postgres
+  host: ${DB_HOST}
+  port: ${DB_PORT}
+  database: ${DB_DATABASE}
+  user: ${DB_USER}
+  password: ${DB_PASSWORD}
+  params:
+    sslmode: disable
+resource_types:
+  user:
+    name: User
+    list:
+      query: |
+        SELECT id FROM public.c1_smoke_users
+      map:
+        id: ".id"
+        display_name: ".id"
+  group:
+    name: Group
+    list:
+      query: |
+        SELECT id FROM public.c1_smoke_groups
+      map:
+        id: ".id"
+        display_name: ".id"
+    static_entitlements:
+      - id: member
+        display_name: "'Member'"
+        purpose: assignment
+        grantable_to:
+          - user
+` + grantsBlock
+}
+
+func TestOfflineValidate_RejectsBareDollarGrant(t *testing.T) {
+	yaml := withGroupResourceAndGrants(`
+    grants:
+      - query: |
+          SELECT user_id FROM public.c1_smoke_group_members WHERE group_id = $1
+        map:
+          - principal_id: ".user_id"
+            principal_type: user
+            entitlement_id: member
+`)
+	err := ValidateYAML([]byte(yaml))
+	require.Error(t, err)
+	require.True(t,
+		strings.Contains(err.Error(), "raw $") || strings.Contains(err.Error(), "do not use raw"),
+		"error=%v", err)
+}
+
+func TestOfflineValidate_RejectsDottedPlaceholder(t *testing.T) {
+	yaml := withGroupResourceAndGrants(`
+    grants:
+      - query: |
+          SELECT user_id FROM public.c1_smoke_group_members WHERE group_id = ?<ResourceID.Resource>
+        map:
+          - principal_id: ".user_id"
+            principal_type: user
+            entitlement_id: member
+`)
+	err := ValidateYAML([]byte(yaml))
+	require.Error(t, err)
+	require.True(t,
+		strings.Contains(err.Error(), "placeholder keys may only") || strings.Contains(err.Error(), "cannot contain '.'"),
+		"error=%v", err)
+}
+
+func TestOfflineValidate_RejectsHybridDollarPath(t *testing.T) {
+	yaml := withGroupResourceAndGrants(`
+    grants:
+      - query: |
+          SELECT user_id FROM public.c1_smoke_group_members WHERE group_id = $1<ResourceID.Resource>
+        map:
+          - principal_id: ".user_id"
+            principal_type: user
+            entitlement_id: member
+`)
+	err := ValidateYAML([]byte(yaml))
+	require.Error(t, err)
+	require.True(t,
+		strings.Contains(err.Error(), "$N<path>") || strings.Contains(strings.ToLower(err.Error()), "hybrid"),
+		"error=%v", err)
+}
+
+func TestOfflineValidate_AcceptsD1MembershipGrants(t *testing.T) {
+	yaml := withGroupResourceAndGrants(`
+    grants:
+      - vars:
+          group_id: "resource.ID"
+        query: |
+          SELECT gm.user_id
+          FROM public.c1_smoke_group_members gm
+          WHERE gm.group_id = ?<group_id>
+          ORDER BY gm.user_id
+        map:
+          - principal_id: ".user_id"
+            principal_type: user
+            entitlement_id: member
+`)
+	require.NoError(t, ValidateYAML([]byte(yaml)))
+
+	cfg, err := Parse([]byte(yaml))
+	require.NoError(t, err)
+	// Dry-run assert: postgres rewrite binds synthetic parent id g2.
+	ctx := t.Context()
+	env, err := bcel.NewEnv(ctx)
+	require.NoError(t, err)
+	s := &SQLSyncer{dbEngine: database.PostgreSQL, env: env}
+	g := cfg.ResourceTypes["group"].Grants[0]
+	resource := &v2.Resource{
+		Id: &v2.ResourceId{ResourceType: "group", Resource: "g2"},
+	}
+	inputs := s.env.SyncInputsWithResource(nil, resource)
+	queryVars, err := s.PrepareQueryVars(ctx, inputs, g.Vars)
+	require.NoError(t, err)
+	normalized := map[string]any{}
+	for k, v := range queryVars {
+		normalized[strings.ToLower(k)] = v
+	}
+	updated, qArgs, _, err := s.parseQueryOpts(&paginationContext{}, g.Query, normalized)
+	require.NoError(t, err)
+	require.Contains(t, updated, "$1")
+	require.Len(t, qArgs, 1)
+	require.Equal(t, "g2", qArgs[0])
+}
+
+func TestOfflineValidate_AcceptsD2SkipIfOnly(t *testing.T) {
+	yaml := withGroupResourceAndGrants(`
+    grants:
+      - query: |
+          SELECT group_id, user_id FROM public.c1_smoke_group_members
+        map:
+          - skip_if: "cols.group_id != resource.ID"
+            principal_id: ".user_id"
+            principal_type: user
+            entitlement_id: member
+`)
+	require.NoError(t, ValidateYAML([]byte(yaml)))
+}
+
+func TestOfflineValidate_UnknownGrantToken(t *testing.T) {
+	yaml := withGroupResourceAndGrants(`
+    grants:
+      - query: |
+          SELECT user_id FROM public.c1_smoke_group_members WHERE group_id = ?<group_id>
+        map:
+          - principal_id: ".user_id"
+            principal_type: user
+            entitlement_id: member
+`)
+	err := ValidateYAML([]byte(yaml))
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unknown token")
+}