Skip to content

refactor(api): replace json.loads with Pydantic validation in security and tools layers#34380

Merged
asukaminato0721 merged 1 commit intolanggenius:mainfrom
eureka0928:refactor/replace-json-loads-pydantic-security-tools
Apr 7, 2026
Merged

refactor(api): replace json.loads with Pydantic validation in security and tools layers#34380
asukaminato0721 merged 1 commit intolanggenius:mainfrom
eureka0928:refactor/replace-json-loads-pydantic-security-tools

Conversation

@eureka0928
Copy link
Copy Markdown
Contributor

Summary

Replaces raw json.loads calls with Pydantic TypeAdapter validation in security-critical and tools subsystem code (Phases 2+3 of #33092, continuing from #33704 and #34277).

  • Security-critical (provider_manager.py): 3 encrypted credential parsing calls now use _credentials_adapter.validate_json() with proper (ValueError, JSONDecodeError) error handling. Removed unused import json and cast.
  • Token parsing (helper.py): Added _TokenData(TypedDict) with known fields (account_id, email, token_type, code, old_email) for Redis token data validation.
  • Tools icon parsing (tool_manager.py): Added _emoji_icon_adapter using existing EmojiIconDict TypedDict. Fixed TypedDict import to use typing_extensions for Python <3.12 compatibility with Pydantic.
  • Tools services: Local _emoji_icon_adapter instances in tools_transform_service.py and workflow_tools_manage_service.py to avoid circular imports with tool_manager.py.

Files intentionally skipped

  • tool_engine.py — LLM-generated parameters, contextlib.suppress(Exception), polymorphic
  • custom_tool/tool.py — OpenAPI schema-driven, dict | list return
  • mcp_tool/tool.py — MCP response content, polymorphic JSON

Test plan

  • 303 tools unit tests pass
  • 26 provider_manager unit tests pass
  • ruff check clean on all modified files

🤖 Generated with Claude Code

@eureka0928 eureka0928 requested a review from QuantumGhost as a code owner April 1, 2026 06:18
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. refactor labels Apr 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 06:19:43.797698191 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 06:19:33.239666412 +0000
@@ -68,6 +68,8 @@
   --> core/plugin/backwards_invocation/base.py:10:33
 ERROR Runtime checkable protocol `Generator` has an unsafe overlap with type `Generator[LLMResultChunkWithStructuredOutput] | LLMResultWithStructuredOutput` [unsafe-overlap]
    --> core/plugin/backwards_invocation/model.py:140:33
+ERROR Argument `Any | None` is not assignable to parameter `token` with type `str` in function `core.helper.encrypter.decrypt_token_with_decoding` [bad-argument-type]
+    --> core/provider_manager.py:1185:45
 ERROR Object of class `NoneType` has no attribute `data_source_type` [missing-attribute]
    --> core/rag/datasource/keyword/jieba/jieba.py:143:36
 ERROR Object of class `NoneType` has no attribute `keyword_table` [missing-attribute]
@@ -362,6 +364,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR Returned type `_TokenData` is not assignable to declared return type `dict[str, Any] | None` [bad-return]
+   --> libs/helper.py:459:16
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 06:21:37.187607992 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 06:21:26.311678457 +0000
@@ -68,6 +68,8 @@
   --> core/plugin/backwards_invocation/base.py:10:33
 ERROR Runtime checkable protocol `Generator` has an unsafe overlap with type `Generator[LLMResultChunkWithStructuredOutput] | LLMResultWithStructuredOutput` [unsafe-overlap]
    --> core/plugin/backwards_invocation/model.py:140:33
+ERROR Argument `Any | None` is not assignable to parameter `token` with type `str` in function `core.helper.encrypter.decrypt_token_with_decoding` [bad-argument-type]
+    --> core/provider_manager.py:1183:45
 ERROR Object of class `NoneType` has no attribute `data_source_type` [missing-attribute]
    --> core/rag/datasource/keyword/jieba/jieba.py:143:36
 ERROR Object of class `NoneType` has no attribute `keyword_table` [missing-attribute]
@@ -362,6 +364,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR Returned type `_TokenData` is not assignable to declared return type `dict[str, Any] | None` [bad-return]
+   --> libs/helper.py:459:16
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from 8ef359b to f719b17 Compare April 1, 2026 06:50
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 06:51:59.963928584 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 06:51:49.485952166 +0000
@@ -68,6 +68,8 @@
   --> core/plugin/backwards_invocation/base.py:10:33
 ERROR Runtime checkable protocol `Generator` has an unsafe overlap with type `Generator[LLMResultChunkWithStructuredOutput] | LLMResultWithStructuredOutput` [unsafe-overlap]
    --> core/plugin/backwards_invocation/model.py:140:33
+ERROR Argument `Any | None` is not assignable to parameter `token` with type `str` in function `core.helper.encrypter.decrypt_token_with_decoding` [bad-argument-type]
+    --> core/provider_manager.py:1185:45
 ERROR Object of class `NoneType` has no attribute `data_source_type` [missing-attribute]
    --> core/rag/datasource/keyword/jieba/jieba.py:143:36
 ERROR Object of class `NoneType` has no attribute `keyword_table` [missing-attribute]
@@ -362,6 +364,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR Returned type `_TokenData` is not assignable to declared return type `dict[str, Any] | None` [bad-return]
+   --> libs/helper.py:459:16
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 06:54:04.987355984 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 06:53:54.764288080 +0000
@@ -68,6 +68,8 @@
   --> core/plugin/backwards_invocation/base.py:10:33
 ERROR Runtime checkable protocol `Generator` has an unsafe overlap with type `Generator[LLMResultChunkWithStructuredOutput] | LLMResultWithStructuredOutput` [unsafe-overlap]
    --> core/plugin/backwards_invocation/model.py:140:33
+ERROR Argument `Any | None` is not assignable to parameter `token` with type `str` in function `core.helper.encrypter.decrypt_token_with_decoding` [bad-argument-type]
+    --> core/provider_manager.py:1183:45
 ERROR Object of class `NoneType` has no attribute `data_source_type` [missing-attribute]
    --> core/rag/datasource/keyword/jieba/jieba.py:143:36
 ERROR Object of class `NoneType` has no attribute `keyword_table` [missing-attribute]
@@ -362,6 +364,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR Returned type `_TokenData` is not assignable to declared return type `dict[str, Any] | None` [bad-return]
+   --> libs/helper.py:459:16
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from f93f708 to c7a71ca Compare April 1, 2026 07:03
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 07:04:53.289811414 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 07:04:43.193951398 +0000
@@ -362,6 +362,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR `_TokenData` is not assignable to `dict[str, Any]` [bad-assignment]
+   --> libs/helper.py:458:38
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 07:07:07.687843393 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 07:06:56.242836188 +0000
@@ -362,6 +362,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR `_TokenData` is not assignable to `dict[str, Any]` [bad-assignment]
+   --> libs/helper.py:458:38
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]

Copy link
Copy Markdown
Contributor

@xr843 xr843 left a comment

Choose a reason for hiding this comment

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

Review Summary

Overall direction is solid — replacing raw json.loads with Pydantic TypeAdapter brings type safety to security-critical credential parsing. The phased approach and clear documentation of intentionally skipped files show good engineering judgment.

Specific Observations

1. _TokenData type narrowing removes None return path (helper.py)

The original code had:

token_data: dict[str, Any] | None = json.loads(token_data_json)

The new code changes the annotation to dict[str, Any] (no None). This is actually correct since json.loads on a non-None string never returns None either, so the old annotation was misleading. Good cleanup.

However, _token_data_adapter.validate_json() will raise ValidationError on malformed input, whereas the old json.loads would raise JSONDecodeError. The caller get_token_data doesn't catch either — if Redis returns corrupted data, this will now raise pydantic.ValidationError instead of json.JSONDecodeError. Depending on upstream error handling, this could change behavior. Worth confirming that callers handle this gracefully (or that Redis token data is trusted enough that this is acceptable).

2. _TokenData fields are quite open-ended

class _TokenData(TypedDict, total=False):
    account_id: str | None
    email: str
    token_type: str
    code: str
    old_email: str

With total=False, all fields are optional, and TypeAdapter[_TokenData] won't reject extra keys. This means validation is very permissive — essentially equivalent to dict[str, Any] in practice. This is fine for a non-breaking refactor, but a future follow-up could tighten this (e.g., making account_id required for certain token types).

3. Inconsistent _emoji_icon_adapter types across files

  • tool_manager.py: TypeAdapter[EmojiIconDict] where EmojiIconDict = TypedDict("EmojiIconDict", background=str, content=str)
  • tools_transform_service.py: TypeAdapter[dict[str, str]]
  • workflow_tools_manage_service.py: TypeAdapter[dict[str, str]]

The services use dict[str, str] instead of EmojiIconDict to avoid circular imports, which is a reasonable trade-off. But this means the services won't validate that background and content keys are present — they'll accept any dict[str, str]. Consider defining a lightweight local TypedDict (or re-exporting EmojiIconDict from a shared types module) to keep validation consistent across all three files.

4. Good: defensive or "" fix (provider_manager.py L1180)

provider_model_credentials.get(variable) or "",

This fixes a potential None being passed to decrypt_token_with_decoding. Nice catch — this is a genuine bug fix bundled with the refactor.

5. Good: typing_extensions.TypedDict for Pydantic compatibility

Using typing_extensions.TypedDict instead of typing.TypedDict is the right call for Pydantic + Python <3.12 compatibility. Well-researched.

Minor Nits

  • The PR body mentions "303 tools unit tests pass" and "26 provider_manager unit tests pass" — it would be great to also confirm that helper.py's token-related tests pass (if they exist), since that's where the behavioral change around error types is most visible.

Verdict

Clean, well-scoped refactor with good documentation of what was intentionally skipped. The inconsistent emoji icon adapter types across files is the main thing I'd suggest addressing. Everything else looks solid. 👍

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 08:09:56.986126760 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 08:09:46.218142567 +0000
@@ -362,6 +362,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR `_TokenData` is not assignable to `dict[str, Any]` [bad-assignment]
+   --> libs/helper.py:458:38
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]
@@ -420,6 +422,8 @@
    --> services/summary_index_service.py:409:24
 ERROR Argument `datetime | object` is not assignable to parameter `value` with type `SQLCoreOperations[datetime] | datetime` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
    --> services/summary_index_service.py:410:53
+ERROR Returned type `_EmojiIcon` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:64:28
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Runtime checkable protocol `Generator` has an unsafe overlap with type `Generator[Mapping[str, Any] | str, Any] | Mapping[str, Any]` [unsafe-overlap]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-01 08:11:58.615783583 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-01 08:11:46.705786657 +0000
@@ -362,6 +362,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR `_TokenData` is not assignable to `dict[str, Any]` [bad-assignment]
+   --> libs/helper.py:458:38
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]
@@ -420,6 +422,8 @@
    --> services/summary_index_service.py:409:24
 ERROR Argument `datetime | object` is not assignable to parameter `value` with type `SQLCoreOperations[datetime] | datetime` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
    --> services/summary_index_service.py:410:53
+ERROR Returned type `_EmojiIcon` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:64:28
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Runtime checkable protocol `Generator` has an unsafe overlap with type `Generator[Mapping[str, Any] | str, Any] | Mapping[str, Any]` [unsafe-overlap]

@eureka0928
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review @xr843!

Addressed point #3 — both service files now use a local _EmojiIcon(TypedDict) with explicit background: str and content: str fields, matching the EmojiIconDict structure in tool_manager.py. This ensures consistent schema validation across all three files.

On point #1 (error type change): callers of get_token_data don't catch JSONDecodeError — they check for None return. Since Redis token data is written by our own generate_token with json.dumps, malformed data would indicate a serious Redis corruption issue, so letting ValidationError propagate is acceptable.

@asukaminato0721 this PR is ready for review when you have a chance — covers Phases 2+3 (security-critical + tools subsystem) of #33092.

@eureka0928 eureka0928 requested a review from xr843 April 1, 2026 08:22
@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from 04497e7 to ebf1854 Compare April 4, 2026 06:52
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-04 06:54:03.188793108 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-04 06:53:54.410768659 +0000
@@ -313,6 +313,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR `_TokenData` is not assignable to `dict[str, Any]` [bad-assignment]
+   --> libs/helper.py:458:38
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]
@@ -343,6 +345,8 @@
   --> services/document_indexing_proxy/duplicate_document_indexing_task_proxy.py:15:5
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:81:34
+ERROR Returned type `_EmojiIcon` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:64:28
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-04 06:55:54.881561496 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-04 06:55:46.763548343 +0000
@@ -313,6 +313,8 @@
   --> extensions/storage/huawei_obs_storage.py:31:20
 ERROR Argument `Module[Crypto.Hash.SHA1] | Unknown` is not assignable to parameter with type `Hash | HashModule` [bad-argument-type]
   --> libs/gmpy2_pkcs10aep_cipher.py:73:49
+ERROR `_TokenData` is not assignable to `dict[str, Any]` [bad-assignment]
+   --> libs/helper.py:458:38
 ERROR No matching overload found for function `redis.client.Redis.__init__` called with arguments: (host=int | str | Unknown, port=int | str | Unknown, password=int | str | Unknown | None, db=int, ssl=bool, ssl_ca_certs=str | None, ssl_cert_reqs=Any | None, ssl_certfile=Any | None, ssl_keyfile=Any | None, socket_timeout=Literal[5], socket_connect_timeout=Literal[5], health_check_interval=Literal[30]) [no-matching-overload]
   --> schedule/queue_monitor_task.py:14:21
 ERROR Object of class `Tenant` has no attribute `role` [missing-attribute]
@@ -343,6 +345,8 @@
   --> services/document_indexing_proxy/duplicate_document_indexing_task_proxy.py:15:5
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:81:34
+ERROR Returned type `_EmojiIcon` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:64:28
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from 9158858 to 364dcec Compare April 4, 2026 07:08
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-04 07:09:33.288630334 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-04 07:09:24.533701159 +0000
@@ -343,6 +343,8 @@
   --> services/document_indexing_proxy/duplicate_document_indexing_task_proxy.py:15:5
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:81:34
+ERROR No matching overload found for function `dict.__init__` called with arguments: (_EmojiIcon) [no-matching-overload]
+  --> services/tools/tools_transform_service.py:64:32
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-04 07:11:48.975544468 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-04 07:11:38.147661406 +0000
@@ -343,6 +343,8 @@
   --> services/document_indexing_proxy/duplicate_document_indexing_task_proxy.py:15:5
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:81:34
+ERROR No matching overload found for function `dict.__init__` called with arguments: (_EmojiIcon) [no-matching-overload]
+  --> services/tools/tools_transform_service.py:64:32
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]

@eureka0928
Copy link
Copy Markdown
Contributor Author

@asukaminato0721 can you review please?

Comment thread api/services/tools/tools_transform_service.py Outdated
@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from abfc9eb to 846d6df Compare April 5, 2026 02:00
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-05 02:01:55.459706304 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-05 02:01:46.366658046 +0000
@@ -343,6 +343,8 @@
   --> services/document_indexing_proxy/duplicate_document_indexing_task_proxy.py:15:5
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:81:34
+ERROR No matching overload found for function `dict.__init__` called with arguments: (_EmojiIcon) [no-matching-overload]
+  --> services/tools/tools_transform_service.py:64:32
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]
@@ -6712,7 +6714,7 @@
     --> tests/unit_tests/services/test_workflow_service.py:2763:65
 ERROR Argument `Literal['api_key']` is not assignable to parameter `credential_type` with type `CredentialType` in function `services.tools.builtin_tools_manage_service.BuiltinToolManageService.list_builtin_provider_credentials_schema` [bad-argument-type]
   --> tests/unit_tests/services/tools/test_builtin_tools_manage_service.py:84:89
-ERROR Object of class `Mapping` has no attribute `startswith` [missing-attribute]
+ERROR Object of class `dict` has no attribute `startswith` [missing-attribute]
    --> tests/unit_tests/services/tools/test_tools_transform_service.py:464:16
 ERROR Could not import `DatasetDocument` from `models.dataset` [missing-module-attribute]
    --> tests/unit_tests/services/vector_service.py:126:49

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-05 02:03:39.665782298 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-05 02:03:31.023949536 +0000
@@ -343,6 +343,8 @@
   --> services/document_indexing_proxy/duplicate_document_indexing_task_proxy.py:15:5
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:81:34
+ERROR No matching overload found for function `dict.__init__` called with arguments: (_EmojiIcon) [no-matching-overload]
+  --> services/tools/tools_transform_service.py:64:32
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]
@@ -6712,7 +6714,7 @@
     --> tests/unit_tests/services/test_workflow_service.py:2763:65
 ERROR Argument `Literal['api_key']` is not assignable to parameter `credential_type` with type `CredentialType` in function `services.tools.builtin_tools_manage_service.BuiltinToolManageService.list_builtin_provider_credentials_schema` [bad-argument-type]
   --> tests/unit_tests/services/tools/test_builtin_tools_manage_service.py:84:89
-ERROR Object of class `Mapping` has no attribute `startswith` [missing-attribute]
+ERROR Object of class `dict` has no attribute `startswith` [missing-attribute]
    --> tests/unit_tests/services/tools/test_tools_transform_service.py:464:16
 ERROR Could not import `DatasetDocument` from `models.dataset` [missing-module-attribute]
    --> tests/unit_tests/services/vector_service.py:126:49

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from 27d318a to 1ee354d Compare April 5, 2026 02:18
@asukaminato0721
Copy link
Copy Markdown
Contributor

+ERROR `str | EmojiIconDict` is not assignable to attribute `icon` with type `Mapping[str, str] | str` [bad-assignment]
+  --> services/tools/tools_transform_service.py:88:33
+ERROR `str | EmojiIconDict` is not assignable to attribute `icon_dark` with type `Mapping[str, str] | str` [bad-assignment]
+  --> services/tools/tools_transform_service.py:92:42

we can fix it in another pr.

xr843 added a commit to xr843/dify that referenced this pull request Apr 5, 2026
…onDict

Pyrefly flags type incompatibility at services/tools/tools_transform_service.py:88/92
where get_tool_provider_icon_url returns str | EmojiIconDict but
ToolProviderApiEntity.icon/icon_dark only accept str | Mapping[str, str].

EmojiIconDict is a TypedDict with specific keys (background, content) which
pyrefly does not structurally treat as Mapping[str, str].

Move EmojiIconDict from tool_manager.py to common_entities.py so it can be
reused by api_entities.py without introducing a circular import, then widen
the icon/icon_dark fields to accept EmojiIconDict.

Follow-up to langgenius#34380 (per reviewer suggestion).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@eureka0928
Copy link
Copy Markdown
Contributor Author

Hi as @xr843 put the PR, @asukaminato0721 would you merge my PR?
Thank you

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-05 11:43:25.251237990 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-05 11:43:16.034196208 +0000
@@ -343,6 +343,10 @@
   --> services/document_indexing_proxy/duplicate_document_indexing_task_proxy.py:15:5
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:81:34
+ERROR `str | EmojiIconDict` is not assignable to attribute `icon` with type `Mapping[str, str] | str` [bad-assignment]
+  --> services/tools/tools_transform_service.py:88:33
+ERROR `str | EmojiIconDict` is not assignable to attribute `icon_dark` with type `Mapping[str, str] | str` [bad-assignment]
+  --> services/tools/tools_transform_service.py:92:42
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]
@@ -6712,7 +6716,7 @@
     --> tests/unit_tests/services/test_workflow_service.py:2763:65
 ERROR Argument `Literal['api_key']` is not assignable to parameter `credential_type` with type `CredentialType` in function `services.tools.builtin_tools_manage_service.BuiltinToolManageService.list_builtin_provider_credentials_schema` [bad-argument-type]
   --> tests/unit_tests/services/tools/test_builtin_tools_manage_service.py:84:89
-ERROR Object of class `Mapping` has no attribute `startswith` [missing-attribute]
+ERROR Object of class `EmojiIconDict` has no attribute `startswith` [missing-attribute]
    --> tests/unit_tests/services/tools/test_tools_transform_service.py:464:16
 ERROR Could not import `DatasetDocument` from `models.dataset` [missing-module-attribute]
    --> tests/unit_tests/services/vector_service.py:126:49

@eureka0928
Copy link
Copy Markdown
Contributor Author

@asukaminato0721 can you merge this PR so that I can put the new?

@asukaminato0721
Copy link
Copy Markdown
Contributor

fix ci

auto-merge was automatically disabled April 7, 2026 09:33

Head branch was pushed to by a user without write access

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from a13be26 to 0fda3c7 Compare April 7, 2026 09:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-07 09:34:32.984521233 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-07 09:34:24.232438049 +0000
@@ -347,6 +347,14 @@
   --> services/hit_testing_service.py:94:21
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:91:34
+ERROR Returned type `EmojiIconDict` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:57:28
+ERROR Returned type `EmojiIconDict` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:58:24
+ERROR Returned type `EmojiIconDict` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:60:24
+ERROR Returned type `EmojiIconDict` is not assignable to declared return type `Mapping[str, str] | str` [bad-return]
+  --> services/tools/tools_transform_service.py:63:24
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from 0fda3c7 to b26c498 Compare April 7, 2026 11:01
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-07 11:02:08.240529927 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-07 11:01:59.509541163 +0000
@@ -347,6 +347,8 @@
   --> services/hit_testing_service.py:94:21
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:91:34
+ERROR No matching overload found for function `dict.__init__` called with arguments: (EmojiIconDict) [no-matching-overload]
+  --> services/tools/tools_transform_service.py:56:32
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from b26c498 to 58bb755 Compare April 7, 2026 11:30
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Pyrefly Diff

No changes detected.

@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from 58bb755 to acf0c3f Compare April 7, 2026 11:32
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-07 11:33:28.324571766 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-07 11:33:18.814551231 +0000
@@ -347,6 +347,10 @@
   --> services/hit_testing_service.py:94:21
 ERROR `handled_tenant_count` was assigned in the current scope before the nonlocal declaration [unknown-name]
   --> services/plugin/plugin_migration.py:91:34
+ERROR `str | EmojiIconDict` is not assignable to attribute `icon` with type `Mapping[str, str] | str` [bad-assignment]
+  --> services/tools/tools_transform_service.py:88:33
+ERROR `str | EmojiIconDict` is not assignable to attribute `icon_dark` with type `Mapping[str, str] | str` [bad-assignment]
+  --> services/tools/tools_transform_service.py:92:42
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Object of class `dict` has no attribute `encode`
 ERROR Returned type `EndUser | Unknown | None` is not assignable to declared return type `Account | EndUser` [bad-return]
@@ -6688,7 +6692,7 @@
     --> tests/unit_tests/services/test_workflow_service.py:2763:65
 ERROR Argument `Literal['api_key']` is not assignable to parameter `credential_type` with type `CredentialType` in function `services.tools.builtin_tools_manage_service.BuiltinToolManageService.list_builtin_provider_credentials_schema` [bad-argument-type]
   --> tests/unit_tests/services/tools/test_builtin_tools_manage_service.py:84:89
-ERROR Object of class `Mapping` has no attribute `startswith` [missing-attribute]
+ERROR Object of class `EmojiIconDict` has no attribute `startswith` [missing-attribute]
    --> tests/unit_tests/services/tools/test_tools_transform_service.py:464:16
 ERROR Could not import `DatasetDocument` from `models.dataset` [missing-module-attribute]
    --> tests/unit_tests/services/vector_service.py:126:49

…y and tools layers

Replace raw json.loads calls with Pydantic TypeAdapter validation in
provider credential parsing and tools icon/credential parsing
(Phases 2+3 of langgenius#33092).

- provider_manager.py: validate encrypted credentials with TypeAdapter
- helper.py: add _TokenData TypedDict for token data validation
- tool_entities.py: add shared EmojiIconDict and emoji_icon_adapter
- tool_manager.py: use typed EmojiIconDict adapter for icon parsing
- tools_transform_service.py: use emoji_icon_adapter with plain dict returns
- workflow_tools_manage_service.py: use shared emoji_icon_adapter
@eureka0928 eureka0928 force-pushed the refactor/replace-json-loads-pydantic-security-tools branch from acf0c3f to 11e21ca Compare April 7, 2026 11:49
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Pyrefly Diff

No changes detected.

@eureka0928
Copy link
Copy Markdown
Contributor Author

@asukaminato0721 All code-related CI checks are passing (unit tests, pyrefly, mypy, superlinter). The two remaining failures are unrelated to this PR:

  • Integration Tests: Sandbox service returning 502 (CodeExecutionError: sandbox service is unavailable) — CI infra issue in test_code_javascript.py
  • API Tests gate: Auto-fails because integration tests failed

Ready for merge when you get a chance.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 7, 2026
@asukaminato0721 asukaminato0721 added this pull request to the merge queue Apr 7, 2026
Merged via the queue into langgenius:main with commit 89ce61c Apr 7, 2026
46 of 49 checks passed
xr843 added a commit to xr843/dify that referenced this pull request Apr 8, 2026
…onDict

Pyrefly flags type incompatibility at services/tools/tools_transform_service.py:88/92
where get_tool_provider_icon_url returns str | EmojiIconDict but
ToolProviderApiEntity.icon/icon_dark only accept str | Mapping[str, str].

EmojiIconDict is a TypedDict with specific keys (background, content) which
pyrefly does not structurally treat as Mapping[str, str].

Move EmojiIconDict from tool_manager.py to common_entities.py so it can be
reused by api_entities.py without introducing a circular import, then widen
the icon/icon_dark fields to accept EmojiIconDict.

Follow-up to langgenius#34380 (per reviewer suggestion).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HanqingZ pushed a commit to HanqingZ/dify that referenced this pull request Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer refactor size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants