Skip to content

hide clusters from dashboard when API server SKYPILOT_DEBUG is set #8339

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

Open
cg505 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

hide clusters from dashboard when API server SKYPILOT_DEBUG is set #8339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 0 deletions sky/dashboard/src/data/connectors/clusters.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export async function getClusters({ clusterNames = null } = {}) {
include_credentials: false,
include_handle: false,
summary_response: clusterNames == null,
from_dashboard: true,
});

const clusterData = clusters.map((cluster) => {
Expand Down
11 changes: 11 additions & 0 deletions sky/server/requests/payloads.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,17 @@ class RequestBody(BasePayload):

def __init__(self, **data):
data['env_vars'] = data.get('env_vars', request_body_env_vars())
if data.get('from_dashboard', False):
Copy link
Collaborator

@aylei aylei Dec 18, 2025

Choose a reason for hiding this comment

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

IIUC, the js client will not set env_vars in json body so it get populated with the server's env var in https://github.com/skypilot-org/skypilot/pull/8339/changes#diff-5ca880f8b68852cc1918d53ec44bc5741ee58013ed94ab3ed71266c5af1f916bL151

The dashboard does have no way to access ordinary env vars, but I think we may reuse the env_vars field in dashboard request to reflect client infos like USAGE_RUN_ID. Initiate a request to include a no-op env var (SKYPILOT_IS_FROM_DASHBOARD: true) might be a good beginning I think

# Limit the env vars to avoid leaking other API server env vars like
# SKYPILOT_DEBUG to the request.
data['env_vars'] = {
constants.USER_ID_ENV_VAR: data['env_vars']
[constants.USER_ID_ENV_VAR],
constants.USER_ENV_VAR: data['env_vars']
[constants.USER_ENV_VAR],
usage_constants.USAGE_RUN_ID_ENV_VAR: data['env_vars'][
usage_constants.USAGE_RUN_ID_ENV_VAR],
}
Comment on lines +152 to +162
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 18, 2025

Choose a reason for hiding this comment

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

security-high high

The current implementation for filtering env_vars is not fully robust. It directly accesses keys from data['env_vars'] using bracket notation ([]), which will raise a KeyError if a key is missing. It will also raise a TypeError if a client sends a non-dictionary value (like null) for env_vars. This could lead to an unhandled exception and a 500 server error, creating a potential denial-of-service vulnerability from a crafted request.

To make this more robust, it's safer to avoid assumptions about the structure of data['env_vars']. The suggested change uses a more defensive approach to safely handle missing keys or incorrect types, making the logic more resilient.

Suggested change
if data.get('from_dashboard', False):
# Limit the env vars to avoid leaking other API server env vars like
# SKYPILOT_DEBUG to the request.
data['env_vars'] = {
constants.USER_ID_ENV_VAR: data['env_vars']
[constants.USER_ID_ENV_VAR],
constants.USER_ENV_VAR: data['env_vars']
[constants.USER_ENV_VAR],
usage_constants.USAGE_RUN_ID_ENV_VAR: data['env_vars'][
usage_constants.USAGE_RUN_ID_ENV_VAR],
}
if data.get('from_dashboard', False):
# Limit the env vars to avoid leaking other API server env vars like
# SKYPILOT_DEBUG to the request.
old_env_vars = data.get('env_vars') or {}
allowed_keys = {
constants.USER_ID_ENV_VAR,
constants.USER_ENV_VAR,
usage_constants.USAGE_RUN_ID_ENV_VAR,
}
data['env_vars'] = {
k: v for k, v in old_env_vars.items() if k in allowed_keys
}

usage_lib_entrypoint = usage_lib.messages.usage.entrypoint
if usage_lib_entrypoint is None:
usage_lib_entrypoint = ''
Expand Down
Loading