Skip to content

fix: allow configured file endpoints through agent proxy - #39839

Open
zyssyz123 wants to merge 1 commit into
mainfrom
fix/39733-agent-file-domain-main
Open

fix: allow configured file endpoints through agent proxy#39839
zyssyz123 wants to merge 1 commit into
mainfrom
fix/39733-agent-file-domain-main

Conversation

@zyssyz123

Copy link
Copy Markdown
Contributor

Summary

  • resolve INTERNAL_FILES_URL with explicit precedence over SERVER_CONSOLE_API_URL and FILES_URL, including the common Docker case where the variable is present but empty
  • pass the resolved file origin to agent_ssrf_proxy and generate a narrow allow rule for the configured host while preserving private-network denial for other destinations
  • document the HTTPS CONNECT trust boundary and add coverage for configuration precedence, private file hosts, blocked non-file paths, metadata protection, and HTTPS tunneling

Fixes #39733

Screenshots

Not applicable; this is a backend and Docker proxy fix.

Validation

  • 30 configuration unit tests passed
  • 18 file-signing and file-request service tests passed
  • Ruff format and lint passed
  • Pyrefly passed
  • Docker Compose generation and configuration validation passed
  • Agent Squid integration suite passed locally and on Linux/amd64, including private target denial and HTTPS CONNECT coverage
  • deployed revision c52c2da0ee3df64f35f86ea23fd7986cfb442369 to the dev environment
  • dev upload request, upload, download request, and byte-for-byte download round trip passed against agent-upload.dify.dev
  • API, workers, agent backend, and local sandbox remained healthy with zero restarts after deployment

Checklist

  • I understand that this PR may be closed in case there was no previous discussion or issues.
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the Docker documentation accordingly.
  • I ran the full repository-wide make lint && make type-check; focused backend lint and type checks passed.

From Codex

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jul 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 56.86% 56.86% +0.00%
Strict coverage 56.38% 56.38% +0.00%
Typed symbols 37,167 37,170 +3
Untyped symbols 28,439 28,441 +2
Modules 3092 3092 0

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.18%. Comparing base (676dff0) to head (47b98d8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #39839   +/-   ##
=======================================
  Coverage   86.18%   86.18%           
=======================================
  Files        5086     5086           
  Lines      287691   287697    +6     
  Branches    57356    57357    +1     
=======================================
+ Hits       247950   247958    +8     
+ Misses      34938    34936    -2     
  Partials     4803     4803           
Flag Coverage Δ
api 86.37% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


AGENT_FILE_TARGETS_CONF=/etc/squid/dify_agent_file_targets.conf

extract_url_host() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function discards the scheme and port. For example, a user-configured https://files.internal:8443 would be parsed as files.internal, resulting in https://files.internal:443 being forwarded. Is this expected?

acl dst_dify_api dstdomain api
acl dst_agent_backend dstdomain -n agent_backend
acl dst_dify_api dstdomain -n api
acl path_files urlpath_regex -i ^/files/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please confirm if the attacker could exploit URL normalization in Nginx to bypass this pattern matching, such as using /files/../console/api/...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

external file domain blocked by agent squid proxy

2 participants