mirror of
https://github.com/langgenius/dify.git
synced 2026-10-04 13:47:55 +08:00
fix(dify-builder): scrub only real credential keys and exact URL hosts
Keys are credentials by their last segment (api_key, access_token, Authorization), not by containing "token" or "password", so password_policy / token_limit defaults survive. URL hosts stop at the first non-URL character and drop trailing punctuation, so a goal that names a host at the end of a sentence or next to CJK text is recognised. Placeholder tokens like <…> only count under a credential key or an auth scheme. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
be24b00a2a
commit
a73c443ecd
4 files changed
+180
-26
No files matched your search
@@ -59,22 +59,22 @@ def analyze_goal(
|
||||
return {"fields": form_schema.reconcile_form_fields(fields, scrubbed_values), "values": scrubbed_values}
|
||||
|
||||
|
||||
# A dict key naming a credential -- checked against ``is_user_supplied_secret``
|
||||
# rather than the plain placeholder regex, so a realistic-looking but
|
||||
# never-stated secret (e.g. a made-up ``sk-live-...`` string) is still caught
|
||||
# even though it doesn't match any of CREDENTIAL_PLACEHOLDER_RE's shapes.
|
||||
_CREDENTIAL_KEY_RE = re.compile(r"(?i)(authorization|api[_-]?key|token|secret|password)")
|
||||
|
||||
# Mirrors the prefix ``user_supplied.is_user_supplied_secret`` strips, so a
|
||||
# literal is checked against CREDENTIAL_PLACEHOLDER_RE the same way regardless
|
||||
# of which of the two credential checks below is doing the looking.
|
||||
_AUTH_SCHEME_PREFIX_RE = re.compile(r"^(?:bearer|basic|token)\s+", re.IGNORECASE)
|
||||
|
||||
|
||||
def _is_invented_literal(value: Any, key: str | None, goal_text: str, trusted_hosts: set[str]) -> bool:
|
||||
"""True when ``value`` (found under ``key``, possibly nested inside a
|
||||
field's dict/list default) is a URL or credential the LLM invented
|
||||
rather than one the goal actually states."""
|
||||
rather than one the goal actually states.
|
||||
|
||||
An off-goal URL blanks the field regardless of key. A credential-shaped
|
||||
placeholder (``<...>``, ``YOUR_API_KEY``, ...) only blanks it when the
|
||||
value is under a credential-named key (``user_supplied.is_credential_key``,
|
||||
decided by the key's LAST segment -- not by containing "token" or
|
||||
"password" anywhere) or the value itself starts with an auth scheme
|
||||
(``Bearer ``/``Basic ``/``Token ``); otherwise an ordinary default like
|
||||
``"Dear <customer_name>,"`` would be blanked for no reason. A
|
||||
credential-keyed value that isn't placeholder-shaped but also never
|
||||
appears in the goal (e.g. an invented ``sk-live-...`` string) is still
|
||||
caught via ``is_user_supplied_secret``.
|
||||
"""
|
||||
if isinstance(value, dict):
|
||||
return any(_is_invented_literal(v, k, goal_text, trusted_hosts) for k, v in value.items())
|
||||
if isinstance(value, list):
|
||||
@@ -83,14 +83,11 @@ def _is_invented_literal(value: Any, key: str | None, goal_text: str, trusted_ho
|
||||
return False
|
||||
if user_supplied.url_hosts(value) - trusted_hosts:
|
||||
return True
|
||||
stripped = _AUTH_SCHEME_PREFIX_RE.sub("", value, count=1)
|
||||
if user_supplied.CREDENTIAL_PLACEHOLDER_RE.search(stripped):
|
||||
credential_key = key is not None and user_supplied.is_credential_key(key)
|
||||
stripped = user_supplied.strip_auth_scheme(value)
|
||||
if (credential_key or stripped != value) and user_supplied.CREDENTIAL_PLACEHOLDER_RE.search(stripped):
|
||||
return True
|
||||
if (
|
||||
key is not None
|
||||
and _CREDENTIAL_KEY_RE.search(key)
|
||||
and not user_supplied.is_user_supplied_secret(value, goal_text)
|
||||
):
|
||||
if credential_key and not user_supplied.is_user_supplied_secret(value, goal_text):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
@@ -19,10 +19,17 @@ from collections.abc import Mapping
|
||||
from typing import Any
|
||||
from urllib.parse import urlsplit
|
||||
|
||||
# A URL literal as it appears embedded in freeform prose -- stops at
|
||||
# whitespace or a quoting/bracketing character so "https://a.com/x." at the
|
||||
# end of a sentence doesn't pull in trailing punctuation.
|
||||
_URL_RE = re.compile(r"https?://[^\s\"'<>]+", re.IGNORECASE)
|
||||
# A URL literal as it appears embedded in freeform prose. Restricted to the
|
||||
# ASCII characters RFC 3986 allows in a URI, so it stops before CJK or other
|
||||
# non-URL text glued onto the URL with no separating space (e.g. a Chinese
|
||||
# sentence with no space before or after the URL). Trailing sentence
|
||||
# punctuation this still picks up (a period ending a sentence, say) is
|
||||
# stripped separately below, since ``.`` is itself a valid URI character.
|
||||
_URL_RE = re.compile(r"https?://[A-Za-z0-9._~:/?#\[\]@!$&'()*+,;=%-]+", re.IGNORECASE)
|
||||
|
||||
# Punctuation that ends a sentence or closes a bracket/quote rather than
|
||||
# belonging to the URL itself, stripped from the end of each regex match.
|
||||
_URL_TRAILING_PUNCTUATION = ".,;:!?)]'\""
|
||||
|
||||
# A Dify workflow template reference, e.g. ``{{#node1.url#}}``. It names a
|
||||
# variable, not a literal value, so it can never be "invented" the way a
|
||||
@@ -44,6 +51,21 @@ CREDENTIAL_PLACEHOLDER_RE = re.compile(
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
# A dict/field key's last word-segment that, by itself, names a credential
|
||||
# regardless of what comes before it (so "access_token" and "auth" both
|
||||
# qualify, but "token_limit" does not -- "limit" is the last segment).
|
||||
_CREDENTIAL_LAST_SEGMENTS = {"authorization", "auth", "token", "secret", "password", "passwd"}
|
||||
|
||||
# A last segment of "key" only counts as a credential when the segment (or
|
||||
# merged word) before it names what kind of key it is -- otherwise ordinary
|
||||
# fields like "key_points" would match on "key" alone.
|
||||
_CREDENTIAL_KEY_PREFIXES = {"api", "access", "secret", "private", "auth"}
|
||||
|
||||
# Splits an identifier into camelCase words: an uppercase run immediately
|
||||
# followed by "Xy" (e.g. the "API" in "APIKey"), or an optional leading
|
||||
# capital plus a run of lowercase/digits (e.g. "api", "Key", "3").
|
||||
_CAMEL_SEGMENT_RE = re.compile(r"[A-Z]+(?=[A-Z][a-z])|[A-Z]?[a-z0-9]+|[A-Z]+|[0-9]+")
|
||||
|
||||
|
||||
def url_hosts(text: str) -> set[str]:
|
||||
"""Lowercase hostnames of every ``http(s)://…`` URL literally present in
|
||||
@@ -51,8 +73,9 @@ def url_hosts(text: str) -> set[str]:
|
||||
text actually names."""
|
||||
hosts: set[str] = set()
|
||||
for raw_url in _URL_RE.findall(text or ""):
|
||||
url = raw_url.rstrip(_URL_TRAILING_PUNCTUATION)
|
||||
try:
|
||||
host = urlsplit(raw_url).hostname
|
||||
host = urlsplit(url).hostname
|
||||
except ValueError:
|
||||
continue
|
||||
if host:
|
||||
@@ -80,12 +103,19 @@ def is_user_supplied_url(url: str, trusted_text: str) -> bool:
|
||||
return host.lower() in url_hosts(trusted_text)
|
||||
|
||||
|
||||
def strip_auth_scheme(value: str) -> str:
|
||||
"""Strip a leading ``Bearer ``/``Basic ``/``Token `` auth-scheme prefix
|
||||
(case-insensitive) from ``value``, if present; otherwise return it
|
||||
unchanged. Shared so every credential check strips the same way."""
|
||||
return _AUTH_SCHEME_PREFIX_RE.sub("", value or "", count=1)
|
||||
|
||||
|
||||
def is_user_supplied_secret(value: str, trusted_text: str) -> bool:
|
||||
"""True iff ``value`` -- after stripping a leading ``Bearer ``/``Basic
|
||||
``/``Token `` auth-scheme prefix -- is a literal the user actually
|
||||
wrote, rather than an LLM's placeholder for a secret it does not have.
|
||||
"""
|
||||
remainder = _AUTH_SCHEME_PREFIX_RE.sub("", value or "", count=1)
|
||||
remainder = strip_auth_scheme(value)
|
||||
if not remainder:
|
||||
return False
|
||||
if _TEMPLATE_MARKER in remainder:
|
||||
@@ -95,6 +125,34 @@ def is_user_supplied_secret(value: str, trusted_text: str) -> bool:
|
||||
return remainder in (trusted_text or "")
|
||||
|
||||
|
||||
def _key_segments(key: str) -> list[str]:
|
||||
"""Lowercase word-segments of a dict/field key, split on ``_``, ``-``,
|
||||
``.``, whitespace, and camelCase boundaries -- so ``x-api-key``,
|
||||
``apiKey`` and ``api_key`` all segment to ``["api", "key"]``."""
|
||||
segments: list[str] = []
|
||||
for part in re.split(r"[_\-.\s]+", key or ""):
|
||||
if not part:
|
||||
continue
|
||||
segments.extend(match.lower() for match in _CAMEL_SEGMENT_RE.findall(part))
|
||||
return segments
|
||||
|
||||
|
||||
def is_credential_key(key: str) -> bool:
|
||||
"""True iff ``key`` names a credential, decided by its LAST segment --
|
||||
not by containing a credential-ish substring anywhere. So
|
||||
``password_policy``, ``token_limit``, ``session_token_expiry`` and
|
||||
``max_tokens`` are NOT credential keys, but ``api_key``, ``apiKey``,
|
||||
``x-api-key``, ``access_token`` and ``Authorization`` are.
|
||||
"""
|
||||
segments = _key_segments(key)
|
||||
if not segments:
|
||||
return False
|
||||
last = segments[-1]
|
||||
if last in _CREDENTIAL_LAST_SEGMENTS or last == "apikey":
|
||||
return True
|
||||
return last == "key" and len(segments) >= 2 and segments[-2] in _CREDENTIAL_KEY_PREFIXES
|
||||
|
||||
|
||||
def trusted_text_for(goal_text: str, requirements: Mapping[str, Any]) -> str:
|
||||
"""The text a candidate URL/credential is checked against: what the user
|
||||
actually submitted -- the goal prose plus the requirements form they
|
||||
|
||||
@@ -143,6 +143,49 @@ def test_scrub_invented_defaults_blanks_a_credential_the_goal_does_not_state():
|
||||
assert out["api_key"] == ""
|
||||
|
||||
|
||||
def test_scrub_invented_defaults_keeps_lookalike_keys_that_are_not_credentials():
|
||||
# ESQ1-302 fix round 1: an unanchored credential-key regex blanked these
|
||||
# (token_limit contains "token", password_policy contains "password",
|
||||
# session_token_expiry contains "token") even though none of them holds
|
||||
# a secret. Credential-ness is decided by the key's LAST segment.
|
||||
values = {
|
||||
"password_policy": "strong",
|
||||
"token_limit": "unlimited",
|
||||
"session_token_expiry": "30 minutes",
|
||||
}
|
||||
|
||||
out = build._scrub_invented_defaults(values, _S5B_GOAL)
|
||||
|
||||
assert out == values
|
||||
|
||||
|
||||
def test_scrub_invented_defaults_keeps_a_non_credential_placeholder_looking_string():
|
||||
# ESQ1-302 fix round 1: CREDENTIAL_PLACEHOLDER_RE's "<...>" shape must
|
||||
# not blank an ordinary templated-greeting default that happens to use
|
||||
# angle brackets.
|
||||
values = {"greeting_template": "Dear <customer_name>,"}
|
||||
|
||||
out = build._scrub_invented_defaults(values, _S5B_GOAL)
|
||||
|
||||
assert out == values
|
||||
|
||||
|
||||
def test_scrub_invented_defaults_blanks_a_bearer_placeholder_under_a_non_credential_key():
|
||||
values = {"x": "Bearer YOUR_API_KEY"}
|
||||
|
||||
out = build._scrub_invented_defaults(values, _S5B_GOAL)
|
||||
|
||||
assert out["x"] == ""
|
||||
|
||||
|
||||
def test_scrub_invented_defaults_blanks_a_placeholder_under_a_credential_key():
|
||||
values = {"api_key": "<your key>"}
|
||||
|
||||
out = build._scrub_invented_defaults(values, _S5B_GOAL)
|
||||
|
||||
assert out["api_key"] == ""
|
||||
|
||||
|
||||
def test_analyze_goal_scrubs_invented_defaults_end_to_end(monkeypatch):
|
||||
captured: dict[str, str] = {}
|
||||
raw = {
|
||||
|
||||
@@ -95,3 +95,59 @@ def test_trusted_text_for_combines_goal_and_requirements_json():
|
||||
|
||||
def test_trusted_text_for_empty_requirements():
|
||||
assert us.trusted_text_for("goal only", {}) == "goal only\n{}"
|
||||
|
||||
|
||||
# --- Fix round 1: is_credential_key decides on the key's LAST segment -----
|
||||
|
||||
|
||||
def test_is_credential_key_true_for_real_credential_keys():
|
||||
for key in ("api_key", "apiKey", "x-api-key", "access_token", "Authorization"):
|
||||
assert us.is_credential_key(key), key
|
||||
|
||||
|
||||
def test_is_credential_key_false_for_lookalike_keys():
|
||||
for key in ("password_policy", "token_limit", "session_token_expiry", "max_tokens", "key_points"):
|
||||
assert not us.is_credential_key(key), key
|
||||
|
||||
|
||||
def test_is_credential_key_false_for_empty_key():
|
||||
assert us.is_credential_key("") is False
|
||||
|
||||
|
||||
# --- Fix round 1: url_hosts stops at the first non-URL character ----------
|
||||
|
||||
|
||||
def test_url_hosts_drops_trailing_sentence_punctuation():
|
||||
text = "… to https://render.acme.internal. Thanks."
|
||||
assert us.url_hosts(text) == {"render.acme.internal"}
|
||||
|
||||
|
||||
def test_url_hosts_stops_before_cjk_text():
|
||||
text = "请调用https://api.x.com接口完成任务"
|
||||
assert us.url_hosts(text) == {"api.x.com"}
|
||||
|
||||
|
||||
def test_is_user_supplied_url_true_with_url_at_end_of_sentence():
|
||||
trusted = "… to https://render.acme.internal. Thanks."
|
||||
assert us.is_user_supplied_url("https://render.acme.internal/v2/pptx", trusted) is True
|
||||
|
||||
|
||||
def test_url_hosts_lowercases_host_with_port():
|
||||
assert us.url_hosts("https://API.Example.com:8443/x") == {"api.example.com"}
|
||||
|
||||
|
||||
# --- Fix round 1: strip_auth_scheme is a shared, exported helper ----------
|
||||
|
||||
|
||||
def test_strip_auth_scheme_strips_known_prefixes_case_insensitively():
|
||||
assert us.strip_auth_scheme("Bearer sk-live-abc123") == "sk-live-abc123"
|
||||
assert us.strip_auth_scheme("basic sk-live-abc123") == "sk-live-abc123"
|
||||
assert us.strip_auth_scheme("TOKEN sk-live-abc123") == "sk-live-abc123"
|
||||
|
||||
|
||||
def test_strip_auth_scheme_leaves_a_plain_value_unchanged():
|
||||
assert us.strip_auth_scheme("sk-live-abc123") == "sk-live-abc123"
|
||||
|
||||
|
||||
def test_strip_auth_scheme_handles_empty_value():
|
||||
assert us.strip_auth_scheme("") == ""
|
||||
Reference in new issue
Block a user