Code of Conduct
Search before asking
Describe the feature
Add an opt-in, runtime enforcement mode to the Spark authz plugin that makes unrecognized plan nodes fail closed (i.e. throw an authz exception) instead of falling through the match and silently contributing no access request.
Plus, at build time, catch classification drift by checking that every plan node class found in the classpath is one of:
- Explicitly allow-listed as "known harmless"
- Durably asserted by FQCN to be checked properly, in each Spark version -- in text files, not easily-ignored tests
- Not a
Command or LeafNode
Any newly introduced plan node type would need to be allow-listed, or manually asserted as safe (hopefully with careful PR review!)
Runtime config:
spark.kyuubi.authz.unclassifiedNode.behavior = allow | warn | deny (default: warn)
allow: today's behavior; violations counted and visible at DEBUG.warn: log once per (class name, violation kind) per JVM and count occurrences. The accretion mode for building an allowlist from real workloads.deny: throw AccessControlException naming the unclassified class and the config key.
The enforced invariant is deliberately narrower than "flag every fallthrough", which would be unusably noisy since intermediate operators legitimately recurse:
Every Command, every LeafNode, every node that can execute or mutate state outside the checked path (e.g. Spark 4's ExecutableDuringAnalysis), and every node whose class name has a spec that dispatch did not consult — including in subtrees pruned by constant-projection elimination — must be either matched by a spec or present on an explicit allowlist. Ordinary non-leaf query operators recurse freely.
Motivation
In the authz plugin, recognition of plan nodes is the first and mandatory step to forming a security boundary, and non-recognition (e.g. a match that falls through) fails open. Privilege building walks the Catalyst plan and matches nodes against the JSON spec files; any node that falls through is implicitly treated as not-authz-relevant and contributes no access request. Spark's plan space is open, meaning that new commands appear in every minor release, and third-party catalogs inject their own nodes at user runtime, so the unrecognized set grows silently.
A green test suite is unfortunately insufficient to bound this risk: every test was written for a node type someone had already classified, so the tests certify that the previously known sample still authorizes correctly, but can't verify nodes that nobody has written a test for yet.
Unfortunately, this is not hypothetical. Running the existing suites in deny mode against master immediately surfaced two real fail-opens, both fixed and regression-tested in the patch:
- Iceberg metadata tables (
SELECT * FROM t.snapshots) were never authorized: the 4-part name() threw a MatchError that fail-open swallowed.
- Iceberg
MERGE INTO: the rewrite embeds an already-planned DataSourceV2ScanRelation for its read of the target table, which buildQuery skipped entirely.
The sharpest structural case is CALL on Spark 4. On Spark 3.x with Iceberg it resolves to o.a.s.sql.catalyst.plans.logical.Call, an Iceberg-injected Command, so dispatch reaches its spec. Spark 4 ships its own class under the identical fully qualified name with a different hierarchy — a UnaryNode implementing ExecutableDuringAnalysis, not a Command.
Dispatch never reached the spec lookup and the procedure executed during analysis, before any optimizer-phase check could run. The spec entry still existed and still named the right class; it was simply unreachable. Neither a class-name check nor the presence of a spec entry detects this.
When the build-time enumeration first ran, it counted 136 unclassified authz-relevant plan classes on Spark 3.5 and 195 on Spark 4.1 — the population fail-open had been hiding.
The gap between "policies are enforced on every operation the plugin recognizes" and "policies are enforced on every operation" is something very few users can audit from the outside.
Describe the solution
Four fail-open layers; the fourth was found during implementation.
- Unknown commands:
buildCommand's fallthrough returned QUERY with zero privilege objects, so RuleAuthorization never called verify.
- Unknown leaf relations: an unmatched leaf (or a known scan node not resolved) fell into the generic recursive arm and contributed nothing.
- Extractor drift on known commands: extraction was wrapped in
catch { case e: Exception => LOG.debug(...); Nil }, so a spec'd command failed open when its extractors broke against a new Spark version.
- Constant-projection pruning:
buildQuery skips the subtree of a constant Project, but the subtree still executes (SELECT 'x' FROM t).
Supporting pieces: an allowlist with a required reason per entry (Commands take a second colocated review); per-command extraction-failure semantics, so single descriptor failures stay expected variance rather than drift; dispatch hardening so a spec'd class routes to buildCommand even when it isn't a Command on this version; verifiedSparkVersions as an explicit enumeration of exact major.minor pairs, never a range — gating for allowlist entries, advisory for specs; and build-time coverage checks that enumerate every concrete LogicalPlan descendant into exactly one of four buckets, failing the build on any class new to the diff.
These do not replace the runtime mode, for two independent reasons: build-time enumeration cannot see third-party catalog plugins loaded only in the user's environment, and the set of dangerous node shapes is open, so the only sound default is "unrecognized ⇒ deny".
Additional context
Implemented and green in deny mode on all four profiles (3.5, 4.0, 4.1, 4.2); backlogs 135/182/195/208. Design doc ships in the patch.
Known limitations stated up front:
- analysis-time execution is unreachable at runtime (
CALL on Spark 4 is an acknowledged gap, not a fix)
- partial extractor drift isn't reported at runtime
- row-filtering and data-masking traversals aren't covered and want the same treatment as a follow-up
- a startup Spark-major assertion is worth considering separately.
Three questions for the community:
- is
warn the right default or should the first release ship allow?;
- should the allowlist ship in-repo or be user-supplied?
- should the per-profile backlog files be committed or generated on demand?
Are you willing to submit PR?
Code of Conduct
Search before asking
Describe the feature
Add an opt-in, runtime enforcement mode to the Spark authz plugin that makes unrecognized plan nodes fail closed (i.e. throw an authz exception) instead of falling through the match and silently contributing no access request.
Plus, at build time, catch classification drift by checking that every plan node class found in the classpath is one of:
CommandorLeafNodeAny newly introduced plan node type would need to be allow-listed, or manually asserted as safe (hopefully with careful PR review!)
Runtime config:
spark.kyuubi.authz.unclassifiedNode.behavior=allow|warn|deny(default: warn)allow: today's behavior; violations counted and visible at DEBUG.warn: log once per (class name, violation kind) per JVM and count occurrences. The accretion mode for building an allowlist from real workloads.deny: throwAccessControlExceptionnaming the unclassified class and the config key.The enforced invariant is deliberately narrower than "flag every fallthrough", which would be unusably noisy since intermediate operators legitimately recurse:
Every
Command, everyLeafNode, every node that can execute or mutate state outside the checked path (e.g. Spark 4'sExecutableDuringAnalysis), and every node whose class name has a spec that dispatch did not consult — including in subtrees pruned by constant-projection elimination — must be either matched by a spec or present on an explicit allowlist. Ordinary non-leaf query operators recurse freely.Motivation
In the authz plugin, recognition of plan nodes is the first and mandatory step to forming a security boundary, and non-recognition (e.g. a
matchthat falls through) fails open. Privilege building walks the Catalyst plan and matches nodes against the JSON spec files; any node that falls through is implicitly treated as not-authz-relevant and contributes no access request. Spark's plan space is open, meaning that new commands appear in every minor release, and third-party catalogs inject their own nodes at user runtime, so the unrecognized set grows silently.A green test suite is unfortunately insufficient to bound this risk: every test was written for a node type someone had already classified, so the tests certify that the previously known sample still authorizes correctly, but can't verify nodes that nobody has written a test for yet.
Unfortunately, this is not hypothetical. Running the existing suites in deny mode against master immediately surfaced two real fail-opens, both fixed and regression-tested in the patch:
SELECT * FROM t.snapshots) were never authorized: the 4-partname()threw aMatchErrorthat fail-open swallowed.MERGE INTO: the rewrite embeds an already-plannedDataSourceV2ScanRelationfor its read of the target table, whichbuildQueryskipped entirely.The sharpest structural case is
CALLon Spark 4. On Spark 3.x with Iceberg it resolves too.a.s.sql.catalyst.plans.logical.Call, an Iceberg-injectedCommand, so dispatch reaches its spec. Spark 4 ships its own class under the identical fully qualified name with a different hierarchy — aUnaryNodeimplementingExecutableDuringAnalysis, not aCommand.Dispatch never reached the spec lookup and the procedure executed during analysis, before any optimizer-phase check could run. The spec entry still existed and still named the right class; it was simply unreachable. Neither a class-name check nor the presence of a spec entry detects this.
When the build-time enumeration first ran, it counted 136 unclassified authz-relevant plan classes on Spark 3.5 and 195 on Spark 4.1 — the population fail-open had been hiding.
The gap between "policies are enforced on every operation the plugin recognizes" and "policies are enforced on every operation" is something very few users can audit from the outside.
Describe the solution
Four fail-open layers; the fourth was found during implementation.
buildCommand's fallthrough returnedQUERYwith zero privilege objects, soRuleAuthorizationnever calledverify.catch { case e: Exception => LOG.debug(...); Nil }, so a spec'd command failed open when its extractors broke against a new Spark version.buildQueryskips the subtree of a constant Project, but the subtree still executes (SELECT 'x' FROM t).Supporting pieces: an allowlist with a required
reasonper entry (Commands take a second colocated review); per-command extraction-failure semantics, so single descriptor failures stay expected variance rather than drift; dispatch hardening so a spec'd class routes to buildCommand even when it isn't aCommandon this version;verifiedSparkVersionsas an explicit enumeration of exact major.minor pairs, never a range — gating for allowlist entries, advisory for specs; and build-time coverage checks that enumerate every concreteLogicalPlandescendant into exactly one of four buckets, failing the build on any class new to the diff.These do not replace the runtime mode, for two independent reasons: build-time enumeration cannot see third-party catalog plugins loaded only in the user's environment, and the set of dangerous node shapes is open, so the only sound default is "unrecognized ⇒ deny".
Additional context
Implemented and green in deny mode on all four profiles (3.5, 4.0, 4.1, 4.2); backlogs 135/182/195/208. Design doc ships in the patch.
Known limitations stated up front:
CALLon Spark 4 is an acknowledged gap, not a fix)Three questions for the community:
warnthe right default or should the first release shipallow?;Are you willing to submit PR?