fix: Add ephemeral file upload and credential env fallback (#12333)

Co-authored-by: vjgit96 <vijay.katuri@ibm.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
fix: replace grep -oP with sed for Node.js version extraction in Docker images (#12330)
fix as PR #12309 on release-1.9.0.
This commit is contained in:
Cristhian Zanforlin Lousa
2026-03-25 16:48:54 -03:00
committed by GitHub
parent 63ad0ec05c
commit 48c5d429e4
11 changed files with 934 additions and 389 deletions

View File

@ -88,12 +88,11 @@ RUN ARCH=$(dpkg --print-architecture) \
elif [ "$ARCH" = "arm64" ]; then NODE_ARCH="arm64"; \
else NODE_ARCH="$ARCH"; fi \
&& NODE_VERSION=$(curl -fsSL https://nodejs.org/dist/latest-v22.x/ \
| grep -oP "node-v\K[0-9]+\.[0-9]+\.[0-9]+(?=-linux-${NODE_ARCH}\.tar\.xz)" \
| sed -nE "s/.*node-v([0-9]+\.[0-9]+\.[0-9]+)-linux-${NODE_ARCH}\.tar\.xz.*/\1/p" \
| head -1) \
&& if [ -z "$NODE_VERSION" ]; then echo "ERROR: Could not determine Node.js version" && exit 1; fi \
&& curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz" \
| tar -xJ -C /usr/local --strip-components=1 \
&& npm install -g npm@latest \
&& npm cache clean --force
| tar -xJ -C /usr/local --strip-components=1
RUN useradd user -u 1000 -g 0 --no-create-home --home-dir /app/data
COPY --from=builder --chown=1000 /app/.venv /app/.venv

View File

@ -63,12 +63,11 @@ RUN ARCH=$(dpkg --print-architecture) \
elif [ "$ARCH" = "arm64" ]; then NODE_ARCH="arm64"; \
else NODE_ARCH="$ARCH"; fi \
&& NODE_VERSION=$(curl -fsSL https://nodejs.org/dist/latest-v22.x/ \
| grep -oP "node-v\K[0-9]+\.[0-9]+\.[0-9]+(?=-linux-${NODE_ARCH}\.tar\.xz)" \
| sed -nE "s/.*node-v([0-9]+\.[0-9]+\.[0-9]+)-linux-${NODE_ARCH}\.tar\.xz.*/\1/p" \
| head -1) \
&& if [ -z "$NODE_VERSION" ]; then echo "ERROR: Could not determine Node.js version" && exit 1; fi \
&& curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz" \
| tar -xJ -C /usr/local --strip-components=1 \
&& npm install -g npm@latest \
&& npm cache clean --force
| tar -xJ -C /usr/local --strip-components=1
# Create non-root user
RUN useradd --uid 1000 --gid 0 --no-create-home --home-dir /app/data user

View File

@ -89,12 +89,11 @@ RUN ARCH=$(dpkg --print-architecture) \
elif [ "$ARCH" = "arm64" ]; then NODE_ARCH="arm64"; \
else NODE_ARCH="$ARCH"; fi \
&& NODE_VERSION=$(curl -fsSL https://nodejs.org/dist/latest-v22.x/ \
| grep -oP "node-v\K[0-9]+\.[0-9]+\.[0-9]+(?=-linux-${NODE_ARCH}\.tar\.xz)" \
| sed -nE "s/.*node-v([0-9]+\.[0-9]+\.[0-9]+)-linux-${NODE_ARCH}\.tar\.xz.*/\1/p" \
| head -1) \
&& if [ -z "$NODE_VERSION" ]; then echo "ERROR: Could not determine Node.js version" && exit 1; fi \
&& curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz" \
| tar -xJ -C /usr/local --strip-components=1 \
&& npm install -g npm@latest \
&& npm cache clean --force
| tar -xJ -C /usr/local --strip-components=1
RUN useradd user -u 1000 -g 0 --no-create-home --home-dir /app/data
COPY --from=builder --chown=1000 /app/.venv /app/.venv

View File

@ -84,12 +84,11 @@ RUN ARCH=$(dpkg --print-architecture) \
elif [ "$ARCH" = "arm64" ]; then NODE_ARCH="arm64"; \
else NODE_ARCH="$ARCH"; fi \
&& NODE_VERSION=$(curl -fsSL https://nodejs.org/dist/latest-v22.x/ \
| grep -oP "node-v\K[0-9]+\.[0-9]+\.[0-9]+(?=-linux-${NODE_ARCH}\.tar\.xz)" \
| sed -nE "s/.*node-v([0-9]+\.[0-9]+\.[0-9]+)-linux-${NODE_ARCH}\.tar\.xz.*/\1/p" \
| head -1) \
&& if [ -z "$NODE_VERSION" ]; then echo "ERROR: Could not determine Node.js version" && exit 1; fi \
&& curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz" \
| tar -xJ -C /usr/local --strip-components=1 \
&& npm install -g npm@latest \
&& npm cache clean --force
| tar -xJ -C /usr/local --strip-components=1
RUN useradd user -u 1000 -g 0 --no-create-home --home-dir /app/data
COPY --from=builder --chown=1000 /app/.venv /app/.venv

View File

@ -85,12 +85,11 @@ RUN ARCH=$(dpkg --print-architecture) \
elif [ "$ARCH" = "arm64" ]; then NODE_ARCH="arm64"; \
else NODE_ARCH="$ARCH"; fi \
&& NODE_VERSION=$(curl -fsSL https://nodejs.org/dist/latest-v22.x/ \
| grep -oP "node-v\K[0-9]+\.[0-9]+\.[0-9]+(?=-linux-${NODE_ARCH}\.tar\.xz)" \
| sed -nE "s/.*node-v([0-9]+\.[0-9]+\.[0-9]+)-linux-${NODE_ARCH}\.tar\.xz.*/\1/p" \
| head -1) \
&& if [ -z "$NODE_VERSION" ]; then echo "ERROR: Could not determine Node.js version" && exit 1; fi \
&& curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz" \
| tar -xJ -C /usr/local --strip-components=1 \
&& npm install -g npm@latest \
&& npm cache clean --force
| tar -xJ -C /usr/local --strip-components=1
RUN useradd user -u 1000 -g 0 --no-create-home --home-dir /app/data
COPY --from=builder --chown=1000 /app/.venv /app/.venv

View File

@ -137,6 +137,7 @@ async def upload_user_file(
settings_service: Annotated[SettingsService, Depends(get_settings_service)],
*,
append: bool = False,
ephemeral: bool = False,
) -> UploadFileResponse:
"""Upload a file for the current user and track it in the database."""
# Get the max allowed file size from settings (in MB)
@ -232,6 +233,12 @@ async def upload_user_file(
# General error saving file or getting file size
raise HTTPException(status_code=500, detail=f"Error accessing file: {e}") from e
if ephemeral:
# Ephemeral uploads: file is saved to storage (servable for chat history)
# but no UserFile record is created (won't appear in "My Files")
file_path = f"{current_user.id}/{stored_file_name}"
return UploadFileResponse(id=file_id, name=root_filename, path=file_path, size=file_size)
if append and existing_file:
existing_file.size = file_size
session.add(existing_file)

View File

@ -149,6 +149,77 @@ async def test_upload_file(files_client, files_created_api_key):
assert "id" in response_json
async def test_should_not_persist_in_my_files_when_upload_is_ephemeral(files_client, files_created_api_key):
"""Ephemeral uploads save the file to storage but do NOT create a UserFile DB record.
This is the expected behavior for chat playground uploads in Desktop,
where the file must be servable (for chat history) but should not
appear in the user's 'My Files' list.
"""
headers = {"x-api-key": files_created_api_key.api_key}
# Upload with ephemeral=true
response = await files_client.post(
"api/v2/files",
files={"file": ("playground_image.png", b"fake image content")},
params={"ephemeral": "true"},
headers=headers,
)
assert response.status_code == 201, f"Expected 201, got {response.status_code}: {response.text}"
upload_response = response.json()
assert "path" in upload_response
# The file must NOT appear in the user's file list
list_response = await files_client.get("api/v2/files", headers=headers)
assert list_response.status_code == 200
file_names = [f["name"] for f in list_response.json()]
assert "playground_image" not in file_names, (
f"Ephemeral file should not appear in My Files, but found: {file_names}"
)
# The file is saved in storage and the response includes a valid path
file_path = upload_response["path"]
assert file_path, "Ephemeral upload should return a non-empty path"
# Path format: {user_id}/{stored_file_name}
parts = file_path.split("/")
assert len(parts) == 2, f"Expected path format 'user_id/filename', got: {file_path}"
async def test_should_return_path_with_forward_slashes_when_uploading_file(files_client, files_created_api_key):
"""Upload response path must use forward slashes on all platforms.
On Windows, pathlib.Path serializes with backslashes, but the GET list
endpoint returns the raw DB string with forward slashes. If the POST
response uses backslashes, the frontend cannot match them with
selectedFiles.includes(file.path), leaving checkboxes unchecked.
"""
headers = {"x-api-key": files_created_api_key.api_key}
response = await files_client.post(
"api/v2/files",
files={"file": ("test_path.txt", b"path test content")},
headers=headers,
)
assert response.status_code == 201
upload_path = response.json()["path"]
assert "\\" not in upload_path, (
f"Upload response path contains backslashes: '{upload_path}'. "
"Path must use forward slashes on all platforms for frontend compatibility."
)
# Verify the upload path matches what GET /files returns
list_response = await files_client.get("api/v2/files", headers=headers)
assert list_response.status_code == 200
listed_paths = [f["path"] for f in list_response.json()]
assert upload_path in listed_paths, (
f"Upload path '{upload_path}' not found in listed paths {listed_paths}. "
"POST and GET must return identical path strings."
)
async def test_download_file(files_client, files_created_api_key):
headers = {"x-api-key": files_created_api_key.api_key}

View File

@ -0,0 +1,102 @@
"""Tests for credential resolution in the unified models system.
Bug: Desktop Global Variable OPENAI_API_KEY not injected at runtime.
When api_key parameter is None (Agent component default), get_api_key_for_provider
only attempts DB lookup but has no os.getenv() fallback and no error handling
for ValueError from get_variable(). This causes failures in Desktop where the
env var is not set and the DB lookup is the only path.
"""
from unittest.mock import patch
from uuid import uuid4
class TestGetApiKeyForProviderDbFallback:
"""Tests for get_api_key_for_provider when api_key param is None (second path)."""
@patch("lfx.base.models.unified_models.get_model_provider_variable_mapping")
@patch("lfx.base.models.unified_models.run_until_complete")
def test_should_fallback_to_env_when_db_lookup_raises_value_error(self, mock_run, mock_mapping, monkeypatch):
"""When variable_service.get_variable raises ValueError (variable not found in DB).
get_api_key_for_provider should fall back to os.getenv() instead of returning None.
"""
from lfx.base.models.unified_models import get_api_key_for_provider
user_id = str(uuid4())
mock_mapping.return_value = {"OpenAI": "OPENAI_API_KEY"}
mock_run.side_effect = ValueError("OPENAI_API_KEY variable not found.")
monkeypatch.setenv("OPENAI_API_KEY", "sk-test-env-key")
result = get_api_key_for_provider(user_id, "OpenAI", None)
assert result == "sk-test-env-key"
@patch("lfx.base.models.unified_models.get_model_provider_variable_mapping")
@patch("lfx.base.models.unified_models.run_until_complete")
def test_should_fallback_to_env_when_db_lookup_returns_empty_string(self, mock_run, mock_mapping, monkeypatch):
"""When decryption fails, get_variable returns empty string.
get_api_key_for_provider should fall back to os.getenv().
"""
from lfx.base.models.unified_models import get_api_key_for_provider
user_id = str(uuid4())
mock_mapping.return_value = {"OpenAI": "OPENAI_API_KEY"}
mock_run.return_value = ""
monkeypatch.setenv("OPENAI_API_KEY", "sk-test-env-key")
result = get_api_key_for_provider(user_id, "OpenAI", None)
assert result == "sk-test-env-key"
@patch("lfx.base.models.unified_models.get_model_provider_variable_mapping")
@patch("lfx.base.models.unified_models.run_until_complete")
def test_should_fallback_to_env_when_variable_service_is_none(self, mock_run, mock_mapping, monkeypatch):
"""When variable_service is None (service not available in thread context).
get_api_key_for_provider should fall back to os.getenv().
"""
from lfx.base.models.unified_models import get_api_key_for_provider
user_id = str(uuid4())
mock_mapping.return_value = {"OpenAI": "OPENAI_API_KEY"}
mock_run.return_value = None
monkeypatch.setenv("OPENAI_API_KEY", "sk-test-env-key")
result = get_api_key_for_provider(user_id, "OpenAI", None)
assert result == "sk-test-env-key"
@patch("lfx.base.models.unified_models.get_model_provider_variable_mapping")
@patch("lfx.base.models.unified_models.run_until_complete")
def test_should_return_none_when_both_db_and_env_unavailable(self, mock_run, mock_mapping, monkeypatch):
"""When both DB lookup and env var are unavailable, should return None."""
from lfx.base.models.unified_models import get_api_key_for_provider
user_id = str(uuid4())
mock_mapping.return_value = {"OpenAI": "OPENAI_API_KEY"}
mock_run.return_value = None
monkeypatch.delenv("OPENAI_API_KEY", raising=False)
result = get_api_key_for_provider(user_id, "OpenAI", None)
assert result is None
@patch("lfx.base.models.unified_models.get_model_provider_variable_mapping")
@patch("lfx.base.models.unified_models.run_until_complete")
def test_should_return_db_value_when_db_lookup_succeeds(self, mock_run, mock_mapping):
"""When DB lookup succeeds, should return the DB value (no env fallback needed)."""
from lfx.base.models.unified_models import get_api_key_for_provider
user_id = str(uuid4())
mock_mapping.return_value = {"OpenAI": "OPENAI_API_KEY"}
mock_run.return_value = "sk-from-database"
result = get_api_key_for_provider(user_id, "OpenAI", None)
assert result == "sk-from-database"

View File

@ -118191,6 +118191,6 @@
"num_components": 359,
"num_modules": 97
},
"sha256": "c2c714d9bec52ae89ed53341db3be1653da036cb8259013f204df4707bdbf757",
"version": "0.3.1"
"sha256": "e1d9ffffbffc29b15303caf157393d6cf1489763c218a0a8f5187eb395085593",
"version": "0.3.2"
}

File diff suppressed because it is too large Load Diff

View File

@ -458,20 +458,31 @@ def get_api_key_for_provider(user_id: UUID | str | None, provider: str, api_key:
if not variable_name:
return None
# Try to get from global variables
# Try to get from global variables, fall back to environment
async def _get_variable():
async with session_scope() as session:
variable_service = get_variable_service()
if variable_service is None:
return None
return await variable_service.get_variable(
user_id=UUID(user_id) if isinstance(user_id, str) else user_id,
name=variable_name,
field="",
session=session,
)
try:
return await variable_service.get_variable(
user_id=UUID(user_id) if isinstance(user_id, str) else user_id,
name=variable_name,
field="",
session=session,
)
except ValueError:
return None
return run_until_complete(_get_variable())
try:
api_key = run_until_complete(_get_variable())
except (ValueError, Exception): # noqa: BLE001
api_key = None
if api_key:
return api_key
return os.getenv(variable_name)
def get_all_variables_for_provider(user_id: UUID | str | None, provider: str) -> dict[str, str]: