[SPARK-58592][CORE] Redact secrets in Standalone Master RequestMasterState RPC - #57796
[SPARK-58592][CORE] Redact secrets in Standalone Master RequestMasterState RPC#57796wangyum wants to merge 3 commits into
Conversation
ApplicationInfo/DriverInfo are Java-serialized as-is by PersistenceEngine (ZooKeeper/filesystem/RocksDB) for Master HA recovery, so secrets in an app or driver's Command (env vars, -D java opts) end up in plaintext in ZK znodes or the recovery directory. Tag each instance with the SparkConf at creation time and redact via writeReplace so the redacted copy is what actually gets serialized, without changing the in-memory object used by the running Master.
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Redacts secrets (env vars and -D Java options) when ApplicationInfo / DriverInfo are persisted for Standalone Master HA recovery, without changing in-memory behavior.
Changes:
- Tag
ApplicationInfo/DriverInfowithSparkConfat construction inMaster, and usewriteReplace()to serialize a redacted substitute. - Add
redactedCopy()helpers to redactCommandenvironment and Java opts using existingUtilshelpers. - Add unit tests covering redaction and Java serialization behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/scala/org/apache/spark/deploy/master/Master.scala | Ensures newly created app/driver infos are tagged with SparkConf so persistence triggers redaction. |
| core/src/main/scala/org/apache/spark/deploy/master/ApplicationInfo.scala | Adds redactedCopy, stores transient conf, and writeReplace to persist redacted state. |
| core/src/main/scala/org/apache/spark/deploy/master/DriverInfo.scala | Adds redactedCopy, stores transient conf, and writeReplace to persist redacted state. |
| core/src/test/scala/org/apache/spark/deploy/master/PersistenceEngineSuite.scala | Adds filesystem persistence test asserting secrets aren’t present on disk and are redacted after recovery. |
| core/src/test/scala/org/apache/spark/deploy/JsonProtocolSuite.scala | Adds unit tests for redactedCopy and writeReplace serialization redaction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private def writeReplace(): AnyRef = { | ||
| if (_conf == null) this else redactedCopy(_conf) | ||
| } | ||
| } |
There was a problem hiding this comment.
Functional regression in HA recovery for supervised drivers and executors. After a master crash and recovery, the objects deserialized from disk are already-redacted copies: desc.command.environment and desc.command.javaOpts contain Utils.REDACTION_REPLACEMENT_TEXT in place of real secrets.
The recovery path in Master.completeRecovery() calls relaunchDriver(d) for every supervised driver whose worker is gone, and relaunchDriver calls createDriver(driver.desc); where driver.desc is the recovered, permanently-redacted DriverDescription. The resulting DriverInfo (even with withConf(conf) attached) has a redacted command, so the worker receives LaunchDriver with REDACTION_REPLACEMENT_TEXT in env vars. Any driver that requires env-var secrets (e.g. HADOOP_CREDSTORE_PASSWORD, AWS_ACCESS_KEY) will fail to authenticate after master-crash recovery. The same applies to executor re-launches: when a recovered application needs new executors, launchExecutor sends exec.application.desc (the redacted ApplicationDescription) to the worker, which builds the executor process with the redacted environment. The PersistenceEngineSuite test explicitly asserts recoveredApp.desc.command.environment("PASSWORD") == Utils.REDACTION_REPLACEMENT_TEXT, confirming the regression.
The fix should store an out-of-band, separate redacted copy for persistence (e.g., serialize a lightweight ApplicationDescription/DriverDescription snapshot with redacted fields) rather than having the live deserialized object carry permanently-redacted state.
There was a problem hiding this comment.
Thank you @uros-b, I reduced the scope to RPC only, and this only affects Spark Standalone mode. ApplicationInfo/DriverInfo are Standalone/Master-specific classes; YARN and Kubernetes do not use them or have an equivalent RPC.
There was a problem hiding this comment.
May be persistence handling for sensitive fields:
- When spark.authenticate=true: consider encrypting the persisted data instead of redacting it.
- When spark.authenticate=false: no change to existing behavior.
…State RPC Replace writeReplace on ApplicationInfo/DriverInfo with writeReplace on MasterStateResponse only. This redacts secrets (Command.environment, javaOpts) during cross-process RPC serialization via Utils.redact, while leaving PersistenceEngine serialization untouched - avoiding the HA recovery regression where redacted copies would be persisted and deserialized with REDACTION_REPLACEMENT_TEXT instead of real secrets. - Remove writeReplace/withConf/_conf from ApplicationInfo and DriverInfo - Keep redactedCopy on both classes - Add writeReplace/withConf to MasterStateResponse - Call withConf(conf) only in RequestMasterState handler - Revert createApplication/createDriver to not call withConf - Remove PersistenceEngineSuite test (persistence not touched) - Update JsonProtocolSuite tests for RPC serialization path Generated-by: GLM 5.2.
Move the redactedCopy(conf) method down to Command itself, eliminating the same 3-line redaction pattern duplicated in ApplicationInfo, DriverInfo, and JsonProtocol.writeApplicationDescription. Generated-by: GLM 5.2.
wangyum
changed the title
What changes were proposed in this pull request?
This PR redacts secrets in the Standalone Master
RequestMasterStateRPC response.MasterStateResponsegains awriteReplace()hook that fires during cross-process Java serialization (the RPC path) and substitutesApplicationInfo/DriverInfocopies whoseCommand.environmentandCommand.javaOptsare redacted. The Master attaches itsSparkConfto the response viawithConf(conf)in theRequestMasterStatehandler sowriteReplacecan apply the configuredspark.redaction.regex.The redaction logic lives in a new
Command.redactedCopy(conf)method, which is also reused byJsonProtocol.writeApplicationDescription(previously had its own inline copy of the same logic).ApplicationInfoandDriverInfoeach gain a thinredactedCopy(conf)that delegates toCommand.redactedCopy. They do not getwriteReplacethemselves — only the RPC response wrapper does, soPersistenceEngineserialization is unaffected.Why are the changes needed?
Any client that can connect to
spark://master:portcan callRequestMasterStateand receiveApplicationInfo/DriverInfoobjects containingCommand.environmentandCommand.javaOptswith plaintext secrets (e.g.spark.executorEnv.PASSWORD,HADOOP_CREDSTORE_PASSWORD,AWS_SECRET_ACCESS_KEY). Withspark.authenticate=false(the default), there is no authentication on the Master RPC.The web UI JSON path already redacts these (SPARK-57098), but the RPC path does not — this closes that gap.
This only affects Spark Standalone mode.
ApplicationInfo/DriverInfoare Standalone Master-specific classes; YARN and Kubernetes do not use them or have an equivalent RPC.Does this PR introduce any user-facing change?
No. The redacted fields are in the internal RPC response between Spark processes; the Master web UI and REST JSON output were already redacted by SPARK-57098.
How was this patch tested?
Added two tests in
JsonProtocolSuite:SPARK-58592: redactedCopy redacts secrets in ApplicationInfo and DriverInfo— unit test verifying thatredactedCopyreplaces secret values withUtils.REDACTION_REPLACEMENT_TEXTwhile preserving non-sensitive values.SPARK-58592: writeReplace redacts secrets during RPC serialization— end-to-end test that Java-serializes aMasterStateResponse.withConf(conf)containing both anApplicationInfoand aDriverInfowith secrets, deserializes it, and asserts the secrets are redacted. This verifies thatwriteReplaceis actually triggered during the serialization path used by RPC.Was this patch authored or co-authored using generative AI tooling?
Generated-by: GLM 5.2.