[dashboard] mask password arguments in get_entrypoint_name() to prevent password exposure

[pedrojeronim0]

Description

Mask any process argument matching --[\w-]*password=<value> in get_entrypoint_name()
to prevent passwords from being exposed in the Ray dashboard.

• Verified with ruff linter.
• Added test to verify password values are not exposed in the output of get_entrypoint_name().
• Verified that test_job.py which contains test_get_entrypoint passes.

Related issues

Fixes #56927

Additional information

The entrypoint was being sent to the GCS with sensitive arguments exposed.
This was then reflected in the Ray dashboard, leaking password values.

[gemini-code-assist]

Code Review

This pull request introduces a security enhancement to mask password arguments in get_entrypoint_name(), preventing them from being exposed in the Ray dashboard. The implementation correctly handles arguments in the format --key=value. I've suggested an improvement to also handle the --key value format for more comprehensive protection. Additionally, I've recommended adding a corresponding test case to ensure both formats are properly masked.

[gemini-code-assist]

The current implementation only masks passwords provided in the --key=value format. It would be more robust to also handle passwords passed in the --key value format (e.g., --password mysecret). This change enhances security by covering another common way of passing sensitive information via command-line arguments.

Suggested change

	sanitized = [
	re.sub(r"(--[\w-]*password=).*", r"\1****", arg) for arg in curr.cmdline()
	]
	return prefix + list2cmdline(sanitized)
	cmd_line = curr.cmdline()
	sanitized = []
	i = 0
	while i < len(cmd_line):
	arg = cmd_line[i]
	# Mask arguments in the form --key=value
	new_arg = re.sub(r"(--[\w-]*password=).*", r"\1****", arg)
	if new_arg != arg:
	sanitized.append(new_arg)
	i += 1
	# Mask arguments in the form --key value
	elif re.match(r"--[\w-]*password$", arg) and i + 1 < len(cmd_line) and not cmd_line[i + 1].startswith("-"):
	sanitized.append(arg)
	sanitized.append("****")
	i += 2
	else:
	sanitized.append(arg)
	i += 1
	return prefix + list2cmdline(sanitized)

[gemini-code-assist]

To ensure the password masking is robust, it's a good idea to add a test case for passwords passed in the --key value format. This will verify that both common argument styles are handled correctly.

Suggested change

	assert not any("secret123" in line for line in outputs)
	assert not any("secret123" in line for line in outputs)
	# Test that password arguments with a space are also masked.
	outputs = execute_driver([sys.executable, str(fp), "--another-password", "secret456"])
	assert line_exists(
	outputs, f"result: {sys.executable} {fp} --another-password \*\*\*\*"
	)
	assert not any("secret456" in line for line in outputs)

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[edoakes]

This solution looks quite brittle. Could we instead just avoid passing the password as a CLI argument? It could be passed as an environment variable instead, for example.

[pedrojeronim0]

This solution looks quite brittle. Could we instead just avoid passing the password as a CLI argument? It could be passed as an environment variable instead, for example.

Thanks for the feedback, @edoakes ! You're right, passing the Redis password via an environment variable is a cleaner and more robust solution. I'll update the implementation accordingly and drop the masking approach.

I'll keep the CLI argument option for retro-compatibility, marking it as deprecated. If provided, it will set the environment variable early (if not already set), so it doesn't get propagated to subprocesses as an argument, showing up in places like get_entrypoint_name(). Does that sound reasonable, or would you prefer we remove the CLI argument entirely?

[edoakes]

I'll keep the CLI argument option for retro-compatibility, marking it as deprecated. If provided, it will set the environment variable early (if not already set), so it doesn't get propagated to subprocesses as an argument, showing up in places like get_entrypoint_name(). Does that sound reasonable, or would you prefer we remove the CLI argument entirely?

Directly removing the argument should be fine as this is an internal API (I don't expect anyone to be starting this server on their own manually). Here is the entrypoint:

ray/python/ray/util/client/server/server.py

Line 876 in 3ee7c9d

parser.add_argument(

[pedrojeronim0]

I'll keep the CLI argument option for retro-compatibility, marking it as deprecated. If provided, it will set the environment variable early (if not already set), so it doesn't get propagated to subprocesses as an argument, showing up in places like get_entrypoint_name(). Does that sound reasonable, or would you prefer we remove the CLI argument entirely?

Directly removing the argument should be fine as this is an internal API (I don't expect anyone to be starting this server on their own manually). Here is the entrypoint:

ray/python/ray/util/client/server/server.py

Line 876 in 3ee7c9d

parser.add_argument(

Ah got it, thanks for the clarification @edoakes ! I was initially thinking about a more general change at the ray start level, but your suggestion makes sense and is more targeted.

I'll go ahead and remove the argument from the client server entrypoint and rely on the environment variable there instead.

[pedrojeronim0]

Hey @edoakes ! I just committed the discussed changes. Let me know what you think!

Now the redis password no longer appears in the job entrypoint.

[edoakes]

Looks good, minor comments

[pedrojeronim0]

Hey @edoakes !
I noticed auto-merge was enabled for my PR last week, but it still hasn’t been merged.

I saw that the buildkite/premerge is currently failing due to a dependency resolution error. I looked into it briefly, and it doesn’t seem related to my changes.

Just wanted to check if there’s anything I should do on my side. Thanks!

[edoakes]

@pedrojeronim0 just merged it. Thanks for the contribution and the reminder!

[anhpham1509]

Hi @edoakes , I saw the PR is already merged. Are you planning to release this soon in the next version?

[edoakes]

@anhpham1509 we are cutting a release branch imminently. The release should go out next week.

[anhpham1509]

@edoakes Hello, just want to catch up on the release progress. It's few week drifted and I haven't seen a new release.