mirror of
https://github.com/langgenius/dify.git
synced 2026-05-09 21:28:25 +08:00
fix(api): keep empty extension whitelist as deny in CUSTOM bucket
Follow-up to the prior fix. The bucket-semantics rewrite changed the extension-whitelist guard from `is not None` to truthiness, which silently widened behavior for the empty-list case (UI never submits it, but DSL / API paths could). Restore the original deny-on-empty posture: when a file falls into the CUSTOM bucket, an explicitly set whitelist (including []) is authoritative. Also tightens _normalize_extension so whitespace-only input returns "" consistent with empty input, and locks two contracts with tests: - empty whitelist + CUSTOM bucket rejects (regression guard for the silent widening) - TokenBufferMemory passes config=None to build_from_message_file (regression guard for the replay-skips-validation contract)
This commit is contained in:
parent
2aa9c69ac4
commit
079fa3e2f9
@ -8,9 +8,9 @@ from graphon.file import FileTransferMethod, FileType, FileUploadConfig
|
||||
|
||||
|
||||
def _normalize_extension(extension: str) -> str:
|
||||
if not extension:
|
||||
return ""
|
||||
s = extension.strip().lower()
|
||||
if not s:
|
||||
return ""
|
||||
return s if s.startswith(".") else "." + s
|
||||
|
||||
|
||||
@ -41,10 +41,14 @@ def is_file_valid_with_config(
|
||||
if not type_allowed and not custom_allowed:
|
||||
return False
|
||||
|
||||
# When the file is in the CUSTOM bucket, the extension whitelist is authoritative.
|
||||
# An explicitly set whitelist (including the empty list) is enforced; empty == deny —
|
||||
# the UI never submits an empty list, so this guards against DSL/API paths that
|
||||
# bypass the UI from accidentally widening the allowlist.
|
||||
in_custom_bucket = input_file_type == FileType.CUSTOM or not type_allowed
|
||||
if (
|
||||
in_custom_bucket
|
||||
and config.allowed_file_extensions
|
||||
and config.allowed_file_extensions is not None
|
||||
and not _extension_matches(file_extension, config.allowed_file_extensions)
|
||||
):
|
||||
return False
|
||||
|
||||
@ -198,6 +198,47 @@ class TestBuildPromptMessageWithFiles:
|
||||
assert isinstance(result.content[-1], TextPromptMessageContent)
|
||||
assert result.content[-1].data == "user text"
|
||||
|
||||
def test_replay_skips_revalidation_by_passing_config_none(self):
|
||||
"""Replay contract: history files were validated on upload, so this
|
||||
path must call build_from_message_file with config=None. Reverting
|
||||
this would re-trigger ENG-244 whenever workflow config drifts."""
|
||||
conv = _make_conversation(AppMode.CHAT)
|
||||
mem = TokenBufferMemory(conversation=conv, model_instance=_make_model_instance())
|
||||
|
||||
mock_file_extra_config = MagicMock()
|
||||
mock_file_extra_config.image_config = None
|
||||
|
||||
real_image_content = ImagePromptMessageContent(
|
||||
url="http://example.com/img.png", format="png", mime_type="image/png"
|
||||
)
|
||||
mock_app_record = MagicMock()
|
||||
mock_app_record.tenant_id = "tenant-1"
|
||||
|
||||
with (
|
||||
patch(
|
||||
"core.memory.token_buffer_memory.FileUploadConfigManager.convert",
|
||||
return_value=mock_file_extra_config,
|
||||
),
|
||||
patch(
|
||||
"core.memory.token_buffer_memory.file_factory.build_from_message_file",
|
||||
return_value=MagicMock(),
|
||||
) as mock_build,
|
||||
patch(
|
||||
"core.memory.token_buffer_memory.file_manager.to_prompt_message_content",
|
||||
return_value=real_image_content,
|
||||
),
|
||||
):
|
||||
mem._build_prompt_message_with_files(
|
||||
message_files=[MagicMock()],
|
||||
text_content="user text",
|
||||
message=_make_message(),
|
||||
app_record=mock_app_record,
|
||||
is_user_message=True,
|
||||
)
|
||||
|
||||
mock_build.assert_called_once()
|
||||
assert mock_build.call_args.kwargs["config"] is None
|
||||
|
||||
@pytest.mark.parametrize("mode", [AppMode.CHAT, AppMode.AGENT_CHAT, AppMode.COMPLETION])
|
||||
def test_chat_mode_with_files_assistant_message(self, mode):
|
||||
"""When files are present, returns AssistantPromptMessage with list content."""
|
||||
|
||||
@ -116,3 +116,27 @@ def test_history_replay_matches_round_1_outcome_under_unchanged_config():
|
||||
)
|
||||
assert _validate(input_file_type="custom", file_extension=".png", config=config) is True
|
||||
assert _validate(input_file_type="image", file_extension=".png", config=config) is True
|
||||
|
||||
|
||||
def test_empty_whitelist_in_custom_bucket_denies_by_default():
|
||||
"""Defensive: when a file lands in the CUSTOM bucket, an empty
|
||||
allowed_file_extensions list rejects. The UI never submits empty;
|
||||
this guards DSL / API paths that bypass the UI from accidentally
|
||||
widening what's accepted."""
|
||||
config = FileUploadConfig(
|
||||
allowed_file_types=[FileType.CUSTOM],
|
||||
allowed_file_extensions=[],
|
||||
)
|
||||
assert _validate(input_file_type="custom", file_extension=".png", config=config) is False
|
||||
assert _validate(input_file_type="image", file_extension=".png", config=config) is False
|
||||
|
||||
|
||||
def test_normalize_handles_whitespace_and_empty_consistently():
|
||||
"""Whitespace-only or empty entries in the whitelist must not match real
|
||||
extensions (regression guard for _normalize_extension edge cases)."""
|
||||
for noisy_entry in ("", " ", "\t"):
|
||||
config = FileUploadConfig(
|
||||
allowed_file_types=[FileType.CUSTOM],
|
||||
allowed_file_extensions=[noisy_entry],
|
||||
)
|
||||
assert _validate(input_file_type="custom", file_extension=".png", config=config) is False
|
||||
|
||||
Loading…
Reference in New Issue
Block a user