Skip to content

Allow schema-qualified type names in Grant routine args - #436

Open
aravinthAS wants to merge 1 commit into
crossplane-contrib:masterfrom
aravinthAS:fix/grant-routine-args-allow-schema-qualified-type
Open

Allow schema-qualified type names in Grant routine args#436
aravinthAS wants to merge 1 commit into
crossplane-contrib:masterfrom
aravinthAS:fix/grant-routine-args-allow-schema-qualified-type

Conversation

@aravinthAS

Copy link
Copy Markdown

Description of your changes

Grant.spec.forProvider.routines[].args validates each argument
against ^[a-zA-Z_][a-zA-Z0-9_$]*$, which rejects any dot. This blocks
granting EXECUTE on routines whose signature includes a
schema-qualified composite type -- e.g. AWS RDS's
aws_s3.table_import_from_s3:

table_import_from_s3(text, text, text, aws_commons._s3_uri_1, aws_commons._aws_credentials_1)

Applying such a Grant fails at admission with:

spec.forProvider.routines[0].args[3]: Invalid value: "aws_commons._s3_uri_1": should match '^[a-zA-Z_][a-zA-Z0-9_$]*$'

There's no way to express this grant today.

Root cause / why this isn't just "widen the regex"

The pattern isn't only shape validation -- it's the SQL-injection guard
for this field. quotedSignatures() in the grant reconciler splices
each argument unquoted directly into the GRANT/REVOKE ... ON ROUTINE SQL string:

// Safe because the CRD restricts arguments to identifier characters:
// +kubebuilder:validation:items:Pattern:=^[a-zA-Z_][a-zA-Z0-9_$]*$
args[j] = strings.ToLower(arg)

So the CRD pattern is the only thing standing between this field and
arbitrary SQL. This PR widens it to allow exactly one optional
schema.type qualification, instead of opening up punctuation
generally:

^[a-zA-Z_][a-zA-Z0-9_$]*(\.[a-zA-Z_][a-zA-Z0-9_$]*)?$

This still anchors start/end and only permits a single well-formed
identifier.identifier shape -- injection payloads (; DROP TABLE ..., embedded quotes/parens, extra dots, etc.) still fail to match.

Changes

  • Updated the pattern on Routine.Arguments in both the cluster-scoped
    (apis/cluster/postgresql/v1alpha1/grant_types.go) and namespaced
    (apis/namespaced/postgresql/v1alpha1/grant_types.go) Grant APIs,
    which duplicate this type.
  • Updated the safety comment above quotedSignatures() in both
    reconcilers to document the widened invariant.
  • Regenerated CRDs via go generate (package/crds/postgresql.sql.crossplane.io_grants.yaml,
    package/crds/postgresql.sql.m.crossplane.io_grants.yaml) -- only
    the routines[].args field changed.
  • Added TestRoutineArgumentPattern (new grant_types_test.go in both
    API packages) asserting the pattern accepts schema-qualified
    identifiers and still rejects malformed/injection input.
  • Added RoutineArgumentsAllowSchemaQualifiedCompositeTypes to the
    existing TestGrantSQL table (both cluster and namespaced
    reconciler_test.go), asserting the generated GRANT/REVOKE SQL for
    aws_s3.table_import_from_s3 with aws_commons._s3_uri_1 /
    aws_commons._aws_credentials_1 args.

How has this code been tested

  • go build ./...
  • go test $(go list ./... | grep -v /test/) -- all packages pass
  • go vet ./...
  • gofmt -l on all changed files -- no output
  • Manually verified go generate regenerated only the two Grant CRD
    files, with only the args pattern/description changed

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review. (ran go build/go test/go vet/gofmt and go generate directly; make reviewable itself wasn't runnable in this environment, see note below)

Grant.spec.forProvider.routines[].args validated each argument against
^[a-zA-Z_][a-zA-Z0-9_$]*$, which rejects any dot. This blocks granting
EXECUTE on routines whose signature includes a schema-qualified
composite type, e.g. AWS RDS's aws_s3.table_import_from_s3(text, text,
text, aws_commons._s3_uri_1, aws_commons._aws_credentials_1).

The pattern isn't just shape validation: quotedSignatures() in the
grant reconciler splices each argument unquoted into the GRANT/REVOKE
SQL string, so the pattern is the only thing preventing arbitrary SQL
via this field. Widen it to allow exactly one optional schema
qualification (^[a-zA-Z_][a-zA-Z0-9_$]*(\.[a-zA-Z_][a-zA-Z0-9_$]*)?$)
rather than opening up punctuation generally, so injection payloads
still fail to match.

Applied identically to both the cluster-scoped and namespaced Grant
APIs, which duplicate this type and reconciler. CRDs regenerated via
go generate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant