Skip to content

[SPARK-58658][CONNECT] Do not return redacted configurations from the Config RPC - #57892

Open
alexandrefimov wants to merge 1 commit into
apache:masterfrom
alexandrefimov:SPARK-58658-config-rpc-redaction
Open

[SPARK-58658][CONNECT] Do not return redacted configurations from the Config RPC#57892
alexandrefimov wants to merge 1 commit into
apache:masterfrom
alexandrefimov:SPARK-58658-config-rpc-redaction

Conversation

@alexandrefimov

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The four read paths of the Config RPC — Get, GetWithDefault, GetOption and GetAll — stop returning configuration entries that match spark.redaction.regex. A withheld entry is reported the way that same operation already reports an unset key: Get fails with SQL_CONF_NOT_FOUND, GetWithDefault returns the caller's default, GetOption returns no value and GetAll omits the entry.

Three details are worth pointing at.

The pattern is read from the SparkConf, not from the session config. The session config is client-writable. requireNonStaticConf does guard spark.redaction.regex, but that guard is itself controlled by spark.sql.legacy.setCommandRejectsSparkCoreConfs, an ordinary non-static SQL conf, so two Set calls would otherwise be enough to clear the filter before reading.

Matching covers the key or the value, following Utils.redact. SetCommand already redacts through SQLConf.redactOptions, so a key-only match would leave the Config RPC laxer than SET over the same configuration — a password inside a JDBC URL is the obvious case. Only spark.redaction.regex is applied; spark.sql.redaction.options.regex is deliberately left out, since it exists for SQL command output and its default would withhold every key with url in its name.

GetAll filters before the requested prefix is stripped. Filtering afterwards would return spark.my.secret.value as value for prefix spark.my.secret., walking straight past the pattern.

IsModifiable is left as it is: whether a key is modifiable is a property of the name and discloses no value.

Why are the changes needed?

The Config RPC discloses anything sensitive that reached SparkConf, whatever put it there. spark.connect.authenticate.token is the one key Spark itself puts there, and it looks benign only because it happens to be the client's own credential.

Apache Kyuubi makes the general shape visible. Kyuubi passes its own configuration to the Spark engine through --conf and prefixes every non-spark. key with spark., so the whole engine configuration ends up in the driver's SparkConf, and SQLConf.mergeSparkConf copies all of it into the session config. Some of those entries are secrets shared across the deployment rather than owned by the connecting user — a ZooKeeper digest, an internal pre-shared secret. That is a plain client-to-server deployment with no proxy anywhere, and a client reads back secrets it never held.

Does this PR introduce any user-facing change?

Yes. A configuration entry whose key or value matches spark.redaction.regex is no longer returned by the Config RPC, and each read operation reports it as if it were unset.

Three consequences I would like reviewers to look at explicitly.

A client can no longer read back an entry it set itself in its own session: after spark.conf.set("spark.sql.catalog.mycat.password", …), reading that key back now fails. Classic Spark still returns it. The filter could be narrowed to keys present in sparkContext.conf, which would keep session-local entries readable; I did not do that, because it would let a secret written into the session config server-side — by a plugin or an extension rather than through SparkConf — stay readable. That trade is a judgement call and I am happy to switch.

A batched Get fails as a whole if any key in the batch is withheld, since that is how a single unset key in a batch already behaves. With the default pattern none of the keys the clients batch internally match, but an operator who widens spark.redaction.regex far enough could turn createDataFrame into an error rather than a hidden value.

An empty spark.redaction.regex matches every key, so setting it to an empty string — a plausible attempt to turn redaction off — now withholds the entire configuration. Utils.redact degrades the same way, so I did not special-case it here, but it is worth a decision.

How was this patch tested?

New SparkConnectConfigHandlerSuite covers the five read operations, the prefix-stripping order, the value match, and that the pattern is not taken from the session config. SparkConnectAuthSuite gains an end-to-end assertion that the authentication token is not readable while a non-sensitive server-side configuration still is.

build/sbt "connect/testOnly org.apache.spark.sql.connect.service.SparkConnectConfigHandlerSuite org.apache.spark.sql.connect.service.SparkConnectAuthSuite"

10 tests, all passing. With the handler reverted to master, 8 of the 10 fail; the two that still pass are the IsModifiable case, whose behaviour is deliberately unchanged, and the pre-existing authentication test.

Was this patch authored or co-authored using generative AI tooling?

Yes — Claude Code (Opus 5) was used for code reading, drafting and review. Every line was reviewed by the author, who takes responsibility for the patch.

… Config RPC

The Config RPC read paths return any configuration that reached SparkConf,
including entries the connecting client never held. Withhold entries matching
spark.redaction.regex, reporting them the way each operation reports an unset
key: Get fails, GetWithDefault returns the caller's default, GetOption returns
no value and GetAll omits the entry.

Matching follows Utils.redact and covers the key or the value, so a secret
carried by an innocuous key, such as a password inside a JDBC URL, is withheld
as well. SET already masks that case through SQLConf.redactOptions.

The pattern is read from the SparkConf rather than from the session config,
which the client can write. GetAll filters on the full key, before the
requested prefix is stripped.
@alexandrefimov

Copy link
Copy Markdown
Contributor Author

cc @viirya

One deviation from the JIRA text: matching follows Utils.redact and covers the value as well as the key. SetCommand already redacts through SQLConf.redactOptions (SetCommand.scala:140,196), so a key-only match would leave the Config RPC laxer than SET over the same configuration.

The description lists three open questions. The one I would most like your call on: should an entry the client wrote into its own session stay readable?

@uros-b

uros-b commented Aug 10, 2026

Copy link
Copy Markdown
Member

Thank you @alexandrefimov!

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.

2 participants