fix: use constant-time comparison for API key validation by xr843 · Pull Request #33986 · langgenius/dify

xr843 · 2026-03-24T00:07:43Z

Summary

Replace == with hmac.compare_digest() for API key and secret comparisons to prevent timing attacks
Add tenant_id check in DataSourceOauthBinding queries to prevent IDOR vulnerabilities
Add test to verify tenant_id scoping in data source patch operations

Rebased on latest main to resolve CI failures from #33858 (which were caused by upstream test issues, now fixed in #33896 and #33899).

Supersedes #33858.

gemini-code-assist · 2026-03-24T00:07:48Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

github-actions · 2026-03-24T00:09:10Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-03-24 00:08:55.984484758 +0000
+++ /tmp/pyrefly_pr.txt	2026-03-24 00:08:47.136528613 +0000
@@ -21,9 +21,9 @@
 ERROR Object of class `NoneType` has no attribute `id` [missing-attribute]
    --> controllers/console/human_input_form.py:190:70
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

github-actions · 2026-03-24T00:11:23Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-03-24 00:11:06.123580148 +0000
+++ /tmp/pyrefly_pr.txt	2026-03-24 00:10:55.856481113 +0000
@@ -21,9 +21,9 @@
 ERROR Object of class `NoneType` has no attribute `id` [missing-attribute]
    --> controllers/console/human_input_form.py:190:70
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

github-actions · 2026-03-28T07:16:58Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-03-28 07:16:39.949555767 +0000
+++ /tmp/pyrefly_pr.txt	2026-03-28 07:16:29.880518904 +0000
@@ -11,9 +11,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:78:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

xr843 · 2026-03-28T23:29:29Z

Hi team, gentle ping on this PR. It replaces direct string comparison of API keys with hmac.compare_digest(), which prevents timing side-channel attacks that could allow API key enumeration. Happy to make any adjustments if needed. Thanks for your time!

asukaminato0721 · 2026-04-01T03:45:03Z

fix conflict

- data_source.py: scope patch() query by tenant_id to prevent cross-tenant data source binding access (IDOR vulnerability) - test_auth_wraps.py: update mock to match db.session.query() API instead of db.session.get() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

xr843 · 2026-04-02T23:19:35Z

Rebased on latest main to resolve the merge conflict. CI is green — would appreciate a review when you have a chance. 🙏

This is a straightforward security improvement: replacing direct string comparison of API keys with hmac.compare_digest() to prevent timing side-channel attacks.

github-actions · 2026-04-02T23:20:49Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-02 23:20:35.914689118 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-02 23:20:27.219668118 +0000
@@ -3,9 +3,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:78:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

The test_patch_binding_scoped_to_current_tenant test was mocking 'Session' which no longer exists in the source module. Updated to mock 'sessionmaker' with the correct .begin() context manager chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-03T02:12:07Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-03 02:11:54.132962680 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-03 02:11:45.354934748 +0000
@@ -3,9 +3,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:78:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

xr843 · 2026-04-06T05:28:03Z

Hi maintainers, this PR has been passing all CI checks for a while now. Could someone take a look when you have a moment? The change adds constant-time comparison for API key validation to prevent timing attacks. Thanks! 🙏

Copilot

Pull request overview

This PR hardens authentication and tenant-scoped lookups by switching sensitive token comparisons to constant-time checks and tightening a data source binding lookup to prevent cross-tenant access.

Changes:

Replace == / != with hmac.compare_digest() for API key / secret comparisons in multiple auth-related code paths.
Scope DataSourceOauthBinding patch lookup by tenant_id to mitigate IDOR risk.
Add/adjust tests to validate the tenant scoping and updated inner-api user lookup mocking.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
api/controllers/console/admin.py	Constant-time comparison for admin API key validation.
api/controllers/console/auth/oauth_server.py	Constant-time comparison + None guards for OAuth client secret validation.
api/controllers/console/extension.py	Constant-time comparison when deciding whether to overwrite masked API key value.
api/controllers/console/init_validate.py	Constant-time comparison for init password validation with missing-env guard.
api/controllers/inner_api/wraps.py	Constant-time comparison for inner API keys and enterprise HMAC signature; user fetch logic adjusted.
api/extensions/ext_login.py	Constant-time comparison for admin API key auth in request loader.
api/libs/token.py	Constant-time comparison for admin API key bypass in CSRF check.
api/services/tools/mcp_tools_manage_service.py	Constant-time comparisons when preserving masked client credentials.
api/controllers/console/datasets/data_source.py	Patch query filters `DataSourceOauthBinding` by `tenant_id` (IDOR mitigation).
api/tests/test_containers_integration_tests/controllers/console/datasets/test_data_source.py	Adds test asserting tenant scoping on patch binding lookup.
api/tests/unit_tests/controllers/inner_api/test_auth_wraps.py	Updates mocking to match new session query-based user retrieval.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

api/tests/test_containers_integration_tests/controllers/console/datasets/test_data_source.py

api/controllers/inner_api/wraps.py

xr843 · 2026-04-06T23:49:34Z

Friendly bump — this PR adds constant-time comparison for API key validation and prevents an IDOR issue. Happy to address any feedback. Thanks!

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

github-actions · 2026-04-07T00:28:17Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-07 00:28:00.539381436 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-07 00:27:52.618393031 +0000
@@ -3,9 +3,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:78:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

api/tests/unit_tests/controllers/inner_api/test_auth_wraps.py

Update test mock to match wraps.py which uses db.session.get(EndUser, user_id) instead of the query chain pattern. Session.get() is the correct approach for primary key lookups. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-04-07T13:15:59Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-07 13:15:43.859141686 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-07 13:15:34.164063189 +0000
@@ -3,9 +3,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:78:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

github-actions · 2026-04-07T13:16:02Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-07 13:15:45.366217631 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-07 13:15:36.912218122 +0000
@@ -3,9 +3,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:78:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

…ing matching Address Copilot review feedback: check for tenant_id in WHERE clause structure rather than matching exact compiled SQL text, which can vary across SQLAlchemy versions/dialects. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-12T13:47:19Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-12 13:47:04.764903142 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-12 13:46:55.637879231 +0000
@@ -3,9 +3,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:75:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

github-actions · 2026-04-12T13:49:02Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-12 13:48:48.114881733 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-12 13:48:40.179998387 +0000
@@ -3,9 +3,9 @@
 ERROR Argument `str | None` is not assignable to parameter `language` with type `str` in function `services.account_service.AccountService.send_email_register_email` [bad-argument-type]
   --> controllers/console/auth/email_register.py:75:108
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
-  --> controllers/console/init_validate.py:31:2
+  --> controllers/console/init_validate.py:32:2
 ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
-  --> controllers/console/init_validate.py:44:2
+  --> controllers/console/init_validate.py:45:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
   --> controllers/console/ping.py:10:2
 ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]

xr843 requested review from Nov1c444, QuantumGhost and laipz8200 as code owners March 24, 2026 00:07

dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 24, 2026

xr843 mentioned this pull request Mar 24, 2026

fix: use constant-time comparison for API key validation #33858

Closed

4 tasks

xr843 mentioned this pull request Mar 24, 2026

fix(security): prevent IDOR in DataSourceOauthBinding by adding tenant_id check #33840

Closed

2 tasks

xr843 requested a review from JohnJyong as a code owner March 28, 2026 07:15

xr843 and others added 4 commits April 3, 2026 07:18

fix: use constant-time comparison for API key validation

25587e0

test(data_source): strengthen tenant_id assertion to verify exact value

7f138f5

[autofix.ci] apply automated fixes

bd3edc3

xr843 force-pushed the fix/constant-time-api-key-compare-v3 branch from d07f74d to 8ccdba9 Compare April 2, 2026 23:19

asukaminato0721 requested a review from Copilot April 6, 2026 05:29

Copilot started reviewing on behalf of asukaminato0721 April 6, 2026 05:30 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

api/tests/test_containers_integration_tests/controllers/console/datasets/test_data_source.py Outdated Show resolved Hide resolved

api/controllers/inner_api/wraps.py Outdated Show resolved Hide resolved

Update api/controllers/inner_api/wraps.py

b938201

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

asukaminato0721 enabled auto-merge April 7, 2026 00:27

asukaminato0721 previously approved these changes Apr 7, 2026

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 7, 2026

asukaminato0721 requested a review from Copilot April 7, 2026 00:28

Copilot started reviewing on behalf of asukaminato0721 April 7, 2026 00:28 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

api/tests/unit_tests/controllers/inner_api/test_auth_wraps.py Outdated Show resolved Hide resolved

auto-merge was automatically disabled April 7, 2026 13:14
Head branch was pushed to by a user without write access

xr843 dismissed asukaminato0721’s stale review via d071e77 April 7, 2026 13:14

[autofix.ci] apply automated fixes

832bccb

Conversation

xr843 commented Mar 24, 2026

Summary

Uh oh!

gemini-code-assist bot commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Pyrefly Diff

Uh oh!

github-actions bot commented Mar 24, 2026

Pyrefly Diff

Uh oh!

github-actions bot commented Mar 28, 2026

Pyrefly Diff

Uh oh!

xr843 commented Mar 28, 2026

Uh oh!

asukaminato0721 commented Apr 1, 2026

Uh oh!

xr843 commented Apr 2, 2026

Uh oh!

github-actions bot commented Apr 2, 2026

Pyrefly Diff

Uh oh!

github-actions bot commented Apr 3, 2026

Pyrefly Diff

Uh oh!

xr843 commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

xr843 commented Apr 6, 2026

Uh oh!

github-actions bot commented Apr 7, 2026

Pyrefly Diff

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions bot commented Apr 7, 2026

Pyrefly Diff

Uh oh!

github-actions bot commented Apr 7, 2026

Pyrefly Diff

Uh oh!

github-actions bot commented Apr 12, 2026

Pyrefly Diff

Uh oh!

github-actions bot commented Apr 12, 2026

Pyrefly Diff

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants