From 8eeb2194deee0827d3b1ba6d841af64ea681e824 Mon Sep 17 00:00:00 2001 From: Bjorn Tipling Date: Wed, 29 Jul 2026 02:05:40 +0000 Subject: [PATCH] CE-1007: offline validate fails thrash grants binds Hard-fail bare $N, hybrid $N, and dotted ? on grants queries; dry-run ? expansion against a synthetic parent resource. Co-authored-by: c1-squire-dev[bot] --- pkg/bsql/offline_validate.go | 153 +++++++++++++++++++++++++++- pkg/bsql/offline_validate_test.go | 164 ++++++++++++++++++++++++++++++ 2 files changed, 316 insertions(+), 1 deletion(-) diff --git a/pkg/bsql/offline_validate.go b/pkg/bsql/offline_validate.go index 04f4dad5..d083f2b2 100644 --- a/pkg/bsql/offline_validate.go +++ b/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 also matches \$[0-9]+. + if hybridPlaceholderRegex.MatchString(query) { + return fmt.Errorf("%s: forbidden hybrid $N bind; use vars with a simple name and ? (e.g. vars.group_id: \"resource.ID\" and ?)", 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 ?)", path) + } + if scanBareDollar && bareDollarParamRegex.MatchString(query) { + return fmt.Errorf("%s: do not use raw $N placeholders in grants queries; use vars + ? and let the engine bind (e.g. vars.group_id: \"resource.ID\" and WHERE col = ?)", 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 diff --git a/pkg/bsql/offline_validate_test.go b/pkg/bsql/offline_validate_test.go index 430e0be3..e73f63ff 100644 --- a/pkg/bsql/offline_validate_test.go +++ b/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 = ? + 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 + 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") || 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 = ? + 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 = ? + 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") +}