langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-21 20:15:31 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

[PR #23712] Fix/plugin upload hardening #30367

New Issue
Closed
opened 2026-02-21 20:47:21 -05:00 by yindo · 0 comments
yindo commented 2026-02-21 20:47:21 -05:00
Owner
Copy Link
Copy Source

Original Pull Request: https://github.com/langgenius/dify/pull/23712

State: closed
Merged: No

Fixes #23716

Important

  1. Make sure you have read our contribution guidelines
  2. Ensure there is an associated issue and you have been assigned to it
  3. Use the correct syntax to link this PR: Fixes #<issue number>.

Summary

Harden /files/upload/for-plugin without breaking existing clients.
• Add server-side validation module core/file/safe_upload.py
• Suffix allowlist + dangerous suffix blocking
• Limit excessive compound suffixes
• Lightweight header sniffing (no new deps)
• Env-configurable: PLUGIN_UPLOAD_ALLOWED_EXTS, PLUGIN_UPLOAD_BLOCKED_SUFFIXES
• Keep signature verification unchanged (still uses client-declared mimetype for compatibility)
• Persist/return canonical server mimetype (derived from validated suffix), never trust client mimetype
• Log declared_mimetype vs server_mimetype mismatch for observability
• Minimal controller changes; main flow preserved

Rationale

The endpoint allowed arbitrary file types by trusting client mimetype/filename. This adds strict, configurable server-side checks with near-zero perf cost and no extra dependencies.

Compatibility / Risk
• Signature path unchanged → existing plugins work
• Allowlist is adjustable via env; rollout can be tuned per environment
• No schema or data migrations

Configuration

Add to your environment (or docker-compose, k8s, systemd, etc.):

PLUGIN_UPLOAD_ALLOWED_EXTS=.png,.jpg,.jpeg,.gif,.pdf,.txt,.csv,.json
PLUGIN_UPLOAD_BLOCKED_SUFFIXES=.php,.jsp,.exe,.sh,.bat,.js,.html,.htm

Offline verification (no service needed)
The script below loads safe_upload.py by file path (bypasses app imports), generates sample files, and checks them with the validator.

Set MOD_PATH to the real path of your module (e.g. api/core/file/safe_upload.py).

# === path to safe_upload.py ===
MOD_PATH="api/core/file/safe_upload.py"

# --- common checker (no app deps) ---
check() {
  FILE="$1" MOD_PATH="$MOD_PATH" python3 - <<'PY'
import os, importlib.util, os.path as p
MOD_PATH = os.environ["MOD_PATH"]
spec = importlib.util.spec_from_file_location("safe_upload", MOD_PATH)
m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m)
path = os.environ["FILE"]
name = m.normalize_filename(p.basename(path))
_, suff = m.split_suffixes(name)
ok_suffix = m.is_safe_suffixes(suff)
ext = suff[-1] if suff else ""
with open(path, "rb") as f:
    ok_sniff = m.sniff_ok(f, ext) if ok_suffix else False
print(f"{path} -> suffixes={suff} ok_suffix={ok_suffix} ok_sniff={ok_sniff} mime={m.canonical_mimetype(ext)} RESULT={'OK' if ok_suffix and ok_sniff else 'BLOCK'}")
PY
}

# ===== Samples that SHOULD PASS =====
printf '\xFF\xD8\x00\x00\x00\x00\x00\x00' > /tmp/good.jpg            # JPEG
printf 'GIF89a\x00\x00\x00\x00\x00\x00\x00' > /tmp/good.gif          # GIF89a
printf '%%PDF-1.7\nrest' > /tmp/good.pdf                             # PDF
printf 'hello,world\n' > /tmp/good.txt                               # UTF-8 text
printf 'a,b\n1,2\n' > /tmp/good.csv                                  # CSV
printf '{"x":1}\n' > /tmp/good.json                                  # JSON
check /tmp/good.jpg
check /tmp/good.gif
check /tmp/good.pdf
check /tmp/good.txt
check /tmp/good.csv
check /tmp/good.json

# upper-case suffix & JSON with BOM (still OK)
printf '\xFF\xD8\x00\x00\x00\x00\x00\x00' > /tmp/upper.JPEG
python3 - <<'PY'
open('/tmp/json_bom.json','wb').write(b'\xEF\xBB\xBF' + b'{"a":1}\n')
PY
check /tmp/upper.JPEG
check /tmp/json_bom.json

# ===== Samples that SHOULD BLOCK =====
# Fake PNG/PDF (wrong magic)
printf '<?php echo 1; ?>' > /tmp/evil.png
printf 'not-a-pdf' > /tmp/evil.pdf
# Dangerous double suffix
printf '<?php echo 1; ?>' > /tmp/evil.jpg.php
# Excessive compound suffixes
printf 'dummy' > /tmp/many && cp /tmp/many /tmp/a.report.backup.json.zip && rm /tmp/many
# Non-allowlisted
printf '{"x":1}\n' > /tmp/data.jsonl
# GIF with invalid header
printf 'GIF88a\x00\x00\x00\x00\x00' > /tmp/fake_gif.gif
# No suffix
touch /tmp/noext
# Text file that is not valid UTF-8 (fails text sniff)
printf '\xFF\xFE\x00\x00BINARY' > /tmp/binary.txt
# .tar.gz (not in allowlist)
tmpd=$(mktemp -d); printf 'abc' > "$tmpd/a"; tar -czf /tmp/a.tar.gz -C "$tmpd" a; rm -rf "$tmpd"

check /tmp/evil.png
check /tmp/evil.pdf
check /tmp/evil.jpg.php
check /tmp/a.report.backup.json.zip
check /tmp/data.jsonl
check /tmp/fake_gif.gif
check /tmp/noext
check /tmp/binary.txt
check /tmp/a.tar.gz

# Optional: allow .jsonl temporarily via env for this single run
PLUGIN_UPLOAD_ALLOWED_EXTS=".png,.jpg,.jpeg,.gif,.pdf,.txt,.csv,.json,.jsonl" \
FILE=/tmp/data.jsonl MOD_PATH="$MOD_PATH" python3 - <<'PY'
import os, importlib.util, os.path as p
spec = importlib.util.spec_from_file_location("safe_upload", os.environ["MOD_PATH"])
m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m)
path = os.environ["FILE"]; name = m.normalize_filename(p.basename(path))
_, suff = m.split_suffixes(name); ok = m.is_safe_suffixes(suff); ext = suff[-1] if suff else ""
with open(path,'rb') as f: sniff = m.sniff_ok(f, ext) if ok else False
print("jsonl allowed via env ->", "OK" if ok and sniff else "BLOCK")
PY

Expected: all “GOOD” samples return RESULT=OK; all “BLOCK” samples return RESULT=BLOCK.

close https://github.com/langgenius/dify/issues/23716

Screenshots

Before After
... ...

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • 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 documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
**Original Pull Request:** https://github.com/langgenius/dify/pull/23712 **State:** closed **Merged:** No --- Fixes #23716 > [!IMPORTANT] > > 1. Make sure you have read our [contribution guidelines](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) > 2. Ensure there is an associated issue and you have been assigned to it > 3. Use the correct syntax to link this PR: `Fixes #<issue number>`. ## Summary Harden /files/upload/for-plugin without breaking existing clients. • Add server-side validation module core/file/safe_upload.py • Suffix allowlist + dangerous suffix blocking • Limit excessive compound suffixes • Lightweight header sniffing (no new deps) • Env-configurable: PLUGIN_UPLOAD_ALLOWED_EXTS, PLUGIN_UPLOAD_BLOCKED_SUFFIXES • Keep signature verification unchanged (still uses client-declared mimetype for compatibility) • Persist/return canonical server mimetype (derived from validated suffix), never trust client mimetype • Log declared_mimetype vs server_mimetype mismatch for observability • Minimal controller changes; main flow preserved Rationale The endpoint allowed arbitrary file types by trusting client mimetype/filename. This adds strict, configurable server-side checks with near-zero perf cost and no extra dependencies. Compatibility / Risk • Signature path unchanged → existing plugins work • Allowlist is adjustable via env; rollout can be tuned per environment • No schema or data migrations Configuration Add to your environment (or docker-compose, k8s, systemd, etc.): ```yaml PLUGIN_UPLOAD_ALLOWED_EXTS=.png,.jpg,.jpeg,.gif,.pdf,.txt,.csv,.json PLUGIN_UPLOAD_BLOCKED_SUFFIXES=.php,.jsp,.exe,.sh,.bat,.js,.html,.htm ``` Offline verification (no service needed) The script below loads safe_upload.py by file path (bypasses app imports), generates sample files, and checks them with the validator. Set MOD_PATH to the real path of your module (e.g. api/core/file/safe_upload.py). ```shell # === path to safe_upload.py === MOD_PATH="api/core/file/safe_upload.py" # --- common checker (no app deps) --- check() { FILE="$1" MOD_PATH="$MOD_PATH" python3 - <<'PY' import os, importlib.util, os.path as p MOD_PATH = os.environ["MOD_PATH"] spec = importlib.util.spec_from_file_location("safe_upload", MOD_PATH) m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m) path = os.environ["FILE"] name = m.normalize_filename(p.basename(path)) _, suff = m.split_suffixes(name) ok_suffix = m.is_safe_suffixes(suff) ext = suff[-1] if suff else "" with open(path, "rb") as f: ok_sniff = m.sniff_ok(f, ext) if ok_suffix else False print(f"{path} -> suffixes={suff} ok_suffix={ok_suffix} ok_sniff={ok_sniff} mime={m.canonical_mimetype(ext)} RESULT={'OK' if ok_suffix and ok_sniff else 'BLOCK'}") PY } # ===== Samples that SHOULD PASS ===== printf '\xFF\xD8\x00\x00\x00\x00\x00\x00' > /tmp/good.jpg # JPEG printf 'GIF89a\x00\x00\x00\x00\x00\x00\x00' > /tmp/good.gif # GIF89a printf '%%PDF-1.7\nrest' > /tmp/good.pdf # PDF printf 'hello,world\n' > /tmp/good.txt # UTF-8 text printf 'a,b\n1,2\n' > /tmp/good.csv # CSV printf '{"x":1}\n' > /tmp/good.json # JSON check /tmp/good.jpg check /tmp/good.gif check /tmp/good.pdf check /tmp/good.txt check /tmp/good.csv check /tmp/good.json # upper-case suffix & JSON with BOM (still OK) printf '\xFF\xD8\x00\x00\x00\x00\x00\x00' > /tmp/upper.JPEG python3 - <<'PY' open('/tmp/json_bom.json','wb').write(b'\xEF\xBB\xBF' + b'{"a":1}\n') PY check /tmp/upper.JPEG check /tmp/json_bom.json # ===== Samples that SHOULD BLOCK ===== # Fake PNG/PDF (wrong magic) printf '<?php echo 1; ?>' > /tmp/evil.png printf 'not-a-pdf' > /tmp/evil.pdf # Dangerous double suffix printf '<?php echo 1; ?>' > /tmp/evil.jpg.php # Excessive compound suffixes printf 'dummy' > /tmp/many && cp /tmp/many /tmp/a.report.backup.json.zip && rm /tmp/many # Non-allowlisted printf '{"x":1}\n' > /tmp/data.jsonl # GIF with invalid header printf 'GIF88a\x00\x00\x00\x00\x00' > /tmp/fake_gif.gif # No suffix touch /tmp/noext # Text file that is not valid UTF-8 (fails text sniff) printf '\xFF\xFE\x00\x00BINARY' > /tmp/binary.txt # .tar.gz (not in allowlist) tmpd=$(mktemp -d); printf 'abc' > "$tmpd/a"; tar -czf /tmp/a.tar.gz -C "$tmpd" a; rm -rf "$tmpd" check /tmp/evil.png check /tmp/evil.pdf check /tmp/evil.jpg.php check /tmp/a.report.backup.json.zip check /tmp/data.jsonl check /tmp/fake_gif.gif check /tmp/noext check /tmp/binary.txt check /tmp/a.tar.gz # Optional: allow .jsonl temporarily via env for this single run PLUGIN_UPLOAD_ALLOWED_EXTS=".png,.jpg,.jpeg,.gif,.pdf,.txt,.csv,.json,.jsonl" \ FILE=/tmp/data.jsonl MOD_PATH="$MOD_PATH" python3 - <<'PY' import os, importlib.util, os.path as p spec = importlib.util.spec_from_file_location("safe_upload", os.environ["MOD_PATH"]) m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m) path = os.environ["FILE"]; name = m.normalize_filename(p.basename(path)) _, suff = m.split_suffixes(name); ok = m.is_safe_suffixes(suff); ext = suff[-1] if suff else "" with open(path,'rb') as f: sniff = m.sniff_ok(f, ext) if ok else False print("jsonl allowed via env ->", "OK" if ok and sniff else "BLOCK") PY ``` Expected: all “GOOD” samples return RESULT=OK; all “BLOCK” samples return RESULT=BLOCK. close https://github.com/langgenius/dify/issues/23716 ## Screenshots | Before | After | |--------|-------| | ... | ... | ## Checklist - [ ] This change requires a documentation update, included: [Dify Document](https://github.com/langgenius/dify-docs) - [x] I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!) - [x] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change. - [x] I've updated the documentation accordingly. - [x] I ran `dev/reformat`(backend) and `cd web && npx lint-staged`(frontend) to appease the lint gods
yindo added the pull-request label 2026-02-21 20:47:21 -05:00
yindo closed this issue 2026-02-21 20:47:21 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#30367
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?