Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Ray Core] Redis password got logged when starting the client server #50266

Closed
letaoj opened this issue Feb 5, 2025 · 2 comments · Fixed by #50409
Closed

[Ray Core] Redis password got logged when starting the client server #50266

letaoj opened this issue Feb 5, 2025 · 2 comments · Fixed by #50409
Labels
core Issues that should be addressed in Ray Core enhancement Request for new feature and/or capability good-first-issue Great starter issue for someone just starting to contribute to Ray P1 Issue that should be fixed within a few weeks

Comments

@letaoj
Copy link
Contributor

letaoj commented Feb 5, 2025

Description

logger.info(f"Starting Ray Client server on {hostport}, args {args}")

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

Line 895 in e3680f7

logger.info(f"Starting Ray Client server on {hostport}, args {args}")
If redis password is passed as an argument, it will be logged and could potentially leak the password.

Use case

No response
@letaoj letaoj added enhancement Request for new feature and/or capability triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Feb 5, 2025
@jcotant1 jcotant1 added the core Issues that should be addressed in Ray Core label Feb 6, 2025
@jjyao jjyao added the good-first-issue Great starter issue for someone just starting to contribute to Ray label Feb 10, 2025
@jjyao
Copy link
Collaborator

jjyao commented Feb 10, 2025

@letaoj do you want to create a PR to fix it?

@jjyao jjyao added P1 Issue that should be fixed within a few weeks and removed triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Feb 10, 2025
@letaoj
Copy link
Contributor Author

letaoj commented Feb 11, 2025

Yes, I can take it.

@letaoj letaoj mentioned this issue Feb 11, 2025
Merged
8 tasks
pcmoritz added a commit that referenced this issue Feb 11, 2025
@letaoj @pcmoritz
[core] Redact redis password in log (#50409)
64a2e40 
<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
This change is needed to prevent redis password been logged in the
standard logging. This is a secure vulnerability.

<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number

<!-- For example: "Closes #1234" -->
Closes #50266

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [x] This PR is not tested :(

---------

Signed-off-by: Letao Jiang <letaoj@gmail.com>
Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>
Co-authored-by: Philipp Moritz <pcmoritz@gmail.com>
israbbani pushed a commit that referenced this issue Feb 25, 2025
@letaoj @pcmoritz @israbbani
[core] Redact redis password in log (#50409)
b7c9abd 
<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
This change is needed to prevent redis password been logged in the
standard logging. This is a secure vulnerability.

<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number

<!-- For example: "Closes #1234" -->
Closes #50266

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [x] This PR is not tested :(

---------

Signed-off-by: Letao Jiang <letaoj@gmail.com>
Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>
Co-authored-by: Philipp Moritz <pcmoritz@gmail.com>
xsuler pushed a commit to antgroup/ant-ray that referenced this issue Mar 4, 2025
@letaoj @pcmoritz @xsuler
[core] Redact redis password in log (ray-project#50409)
d62d778 
<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
This change is needed to prevent redis password been logged in the
standard logging. This is a secure vulnerability.

<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number

<!-- For example: "Closes ray-project#1234" -->
Closes ray-project#50266

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [x] This PR is not tested :(

---------

Signed-off-by: Letao Jiang <letaoj@gmail.com>
Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>
Co-authored-by: Philipp Moritz <pcmoritz@gmail.com>
xsuler pushed a commit to antgroup/ant-ray that referenced this issue Mar 4, 2025
@letaoj @pcmoritz @xsuler
[core] Redact redis password in log (ray-project#50409)
ab55b88 
<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
This change is needed to prevent redis password been logged in the
standard logging. This is a secure vulnerability.

<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number

<!-- For example: "Closes ray-project#1234" -->
Closes ray-project#50266

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [x] This PR is not tested :(

---------

Signed-off-by: Letao Jiang <letaoj@gmail.com>
Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>
Co-authored-by: Philipp Moritz <pcmoritz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core Issues that should be addressed in Ray Core enhancement Request for new feature and/or capability good-first-issue Great starter issue for someone just starting to contribute to Ray P1 Issue that should be fixed within a few weeks
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[core] Redact redis password in log letaoj/ray
3 participants
@jjyao @letaoj @jcotant1