Skip to content

[Core] fix TaskLifecycleEvent.node_id populated with emitting node instead of just executor node #61478

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

Merged
MengjinYan merged 2 commits into from
Mar 4, 2026
Merged

[Core] fix TaskLifecycleEvent.node_id populated with emitting node instead of just executor node #61478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28ebb7a
[Core] fix TaskLifecycleEvent.node_id populated with emitting node in…
Mar 4, 2026
27f971a
address comment
Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/ray/core_worker/task_event_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ void TaskStatusEvent::PopulateRpcRayTaskLifecycleEvent(
std::move(state_transition);
}

lifecycle_event_data.set_node_id(node_id_.Binary());
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The TaskLifecycleEvent.node_id field was being populated with the node ID of the worker emitting the event (node_id_) by default. In Ray, TaskLifecycleEvent is intended to track the lifecycle of a task, and its node_id field should represent the node where the task is executing (the executor). Populating it with the submitter's node ID by default is incorrect and leads to the leakage of the submitter's node ID to any consumer of these events (e.g., via the Export API or the event aggregator). This information could be used by an attacker to map the internal cluster topology or target specific nodes. The pull request correctly addresses this issue by removing the default assignment of the submitter's node ID to the lifecycle_event_data.node_id field.

lifecycle_event_data.set_job_id(job_id_.Binary());

// Task property updates
Expand Down
59 changes: 59 additions & 0 deletions src/ray/core_worker/tests/task_event_buffer_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,65 @@ TEST_F(TaskEventBufferTest, TestTaskLogInfoInLifecycleEvent) {
EXPECT_EQ(log_info.stderr_end(), 50);
}

// Tests that TaskLifecycleEvent.node_id is only set to the executor's node ID
// (available at SUBMITTED_TO_WORKER), never the submitter's node ID.
TEST_F(TaskEventBufferTest, TestTaskLifecycleEventNodeId) {
auto task_id = RandomTaskId();
NodeID submitter_node_id = NodeID::FromRandom();
NodeID executor_node_id = NodeID::FromRandom();

// For SUBMITTED_TO_WORKER, node_id should be set to the executor's node (not the
// submitter's).
{
TaskStatusEvent::TaskStateUpdate submitted_state_update(executor_node_id,
WorkerID::FromRandom());
auto submitted_event =
std::make_unique<TaskStatusEvent>(task_id,
JobID::FromInt(0),
/*attempt_number=*/0,
rpc::TaskStatus::SUBMITTED_TO_WORKER,
/*timestamp=*/1,
/*is_actor_task_event=*/false,
"test_session_name",
submitter_node_id,
nullptr,
submitted_state_update);

RayEventsTuple submitted_ray_events;
submitted_event->ToRpcRayEvents(submitted_ray_events);
ASSERT_TRUE(submitted_ray_events.task_lifecycle_event.has_value());
const auto &submitted_lifecycle =
submitted_ray_events.task_lifecycle_event->task_lifecycle_event();
EXPECT_EQ(submitted_lifecycle.node_id(), executor_node_id.Binary())
<< "node_id should be the executor's node for SUBMITTED_TO_WORKER";
EXPECT_NE(submitted_lifecycle.node_id(), submitter_node_id.Binary())
<< "node_id should not be the submitter's node";
}

// For RUNNING (no state_update node_id), node_id should NOT be set — the submitter's
// node must not leak into the lifecycle event's node_id field.
{
auto task_event = std::make_unique<TaskStatusEvent>(task_id,
JobID::FromInt(0),
/*attempt_number=*/0,
rpc::TaskStatus::RUNNING,
/*timestamp=*/2,
/*is_actor_task_event=*/false,
"test_session_name",
submitter_node_id,
nullptr,
/*state_update=*/absl::nullopt);

RayEventsTuple ray_events_tuple;
task_event->ToRpcRayEvents(ray_events_tuple);
ASSERT_TRUE(ray_events_tuple.task_lifecycle_event.has_value());
const auto &lifecycle_event =
ray_events_tuple.task_lifecycle_event->task_lifecycle_event();
EXPECT_TRUE(lifecycle_event.node_id().empty())
<< "node_id should not be set to the submitter's node in lifecycle event";
}
}

TEST_F(TaskEventBufferTest, TestGracefulDestruction) {
delete task_event_buffer_.release();
}
Expand Down