mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-20 09:45:47 +00:00
fix(scripts): broaden support bundle secret-key redaction denylist (#4242)
* fix(scripts): broaden support bundle secret-key redaction denylist SECRET_KEY_RE only matched a fixed keyword allowlist, so a secret stored under an unanticipated key name inside an open-ended config dict (e.g. guardrails.provider.config, an arbitrary provider-kwargs dict) was emitted verbatim into config-summary.json even though manifest.json claims redacted_secret_fields=true. This gap was flagged on PR #3886's review before merge but not fully addressed. Broaden the key-name match to mirror env_policy.py's wildcard denylist (*KEY*/*SECRET*/*TOKEN*/*PASS*/*CREDENTIAL*/*DSN*) already used for sandbox env-scrubbing, plus its no-flag credential exact names (GH_PAT/GITHUB_PAT/ REDIS_AUTH/REDISCLI_AUTH/PGSERVICEFILE). The new bare key/pass/dsn alternatives are boundary-guarded so they match only their own delimited token, not an unrelated word that starts with the same letters (routing "keywords", guardrails "passport"). * fix(scripts): stop the pass token boundary from missing passphrase/passcode SECRET_KEY_RE's bare "pass" alternative, (?<![a-zA-Z])pass(?![a-zA-Z]), excludes any key where "pass" is followed by another letter. That correctly keeps "passport" out of the redaction set, but it also excludes genuine secret-bearing key names like "passphrase" and "passcode" -- both of which env_policy.py's *PASS* substring denylist does catch, so a secret stored under either name in an open-ended config dict (e.g. guardrails.provider.config) would still leak into config-summary.json. Narrow the lookahead to only exclude a trailing "port" -- pass(?!port) -- so passphrase/passcode/pass/db_pass all match while passport stays excluded; compass/bypass stay excluded via the existing leading-letter lookbehind, independent of the lookahead. Added a regression test covering both the newly-caught names and the still-excluded ones in one place. Reverting to the old lookahead reproduces the exact leak (passphrase left unredacted); with the fix, tests/test_support_bundle.py (30 tests) is green, and ruff check/format are clean.
This commit is contained in:
Daoyuan Li
@@ -190,6 +190,150 @@ def test_redact_data_masks_broadened_secret_key_names():
|
||||
assert redacted["signing_private_key"] == "<redacted>"
|
||||
|
||||
|
||||
def test_redact_data_masks_secret_shaped_keys_in_arbitrary_provider_config():
|
||||
"""Guards the gap flagged on PR #3886's review: a fixed keyword allowlist
|
||||
misses secrets stored under an unanticipated key name inside an
|
||||
open-ended config dict, e.g. guardrails.provider.config (GuardrailProviderConfig.config
|
||||
is an arbitrary dict of provider-specific kwargs)."""
|
||||
data = {
|
||||
"guardrails": {
|
||||
"enabled": True,
|
||||
"provider": {
|
||||
"use": "my_org.guardrails:CustomProvider",
|
||||
"config": {
|
||||
"db_pass": "hunter2-literal",
|
||||
"encryption_key": "0123456789abcdef-literal",
|
||||
"redis_pass": "redis-literal-secret",
|
||||
"webhook_signing_key": "whsec_literal_secret",
|
||||
"SUPABASE_SERVICE_ROLE_KEY": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.no-env-wrapper.sig",
|
||||
"gh_pat": "ghp_literalPatValue",
|
||||
"endpoint": "https://policy.internal/v1",
|
||||
"timeout_seconds": 30,
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
redacted = support_bundle.redact_data(data)
|
||||
config = redacted["guardrails"]["provider"]["config"]
|
||||
|
||||
assert config["db_pass"] == "<redacted>"
|
||||
assert config["encryption_key"] == "<redacted>"
|
||||
assert config["redis_pass"] == "<redacted>"
|
||||
assert config["webhook_signing_key"] == "<redacted>"
|
||||
assert config["SUPABASE_SERVICE_ROLE_KEY"] == "<redacted>"
|
||||
assert config["gh_pat"] == "<redacted>"
|
||||
# Legitimate, non-secret fields in the same open-ended dict must survive.
|
||||
assert config["endpoint"] == "https://policy.internal/v1"
|
||||
assert config["timeout_seconds"] == 30
|
||||
|
||||
dumped = json.dumps(redacted)
|
||||
for secret in (
|
||||
"hunter2-literal",
|
||||
"0123456789abcdef-literal",
|
||||
"redis-literal-secret",
|
||||
"whsec_literal_secret",
|
||||
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9",
|
||||
"ghp_literalPatValue",
|
||||
):
|
||||
assert secret not in dumped
|
||||
|
||||
|
||||
def test_redact_data_does_not_over_redact_lookalike_non_secret_keys():
|
||||
"""Broadening the key-name match must not catch fields that merely start
|
||||
with the same letters as a secret keyword: MCP routing "keywords" hints
|
||||
(extensions_config.json -> mcpServers.*.routing.keywords) and the
|
||||
guardrails "passport" path/ID are real, non-secret fields."""
|
||||
data = {
|
||||
"routing": {"mode": "prefer", "priority": 50, "keywords": ["database", "SQL", "table"]},
|
||||
"guardrails": {"passport": "/etc/deer-flow/passport.json"},
|
||||
}
|
||||
|
||||
redacted = support_bundle.redact_data(data)
|
||||
|
||||
assert redacted["routing"]["keywords"] == ["database", "SQL", "table"]
|
||||
assert redacted["routing"]["priority"] == 50
|
||||
assert redacted["guardrails"]["passport"] == "/etc/deer-flow/passport.json"
|
||||
|
||||
|
||||
def test_redact_data_masks_passphrase_and_passcode_without_over_redacting_passport():
|
||||
"""Guards the gap flagged on this PR's review: the original token-boundary
|
||||
`pass` match, (?<![a-zA-Z])pass(?![a-zA-Z]), correctly excludes "passport"
|
||||
(a real, non-secret field, per the test above) but its blanket
|
||||
not-followed-by-any-letter lookahead also excluded genuine secret-bearing
|
||||
key names like "passphrase" and "passcode" -- both of which
|
||||
env_policy.py's *PASS* substring match does catch, so they'd still leak
|
||||
into config-summary.json. Narrowing the lookahead to only exclude a
|
||||
trailing "port" (pass(?!port)) closes that gap while leaving passport,
|
||||
compass, and bypass alone (those stay excluded via the leading-letter
|
||||
lookbehind, independent of the lookahead)."""
|
||||
data = {
|
||||
"guardrails": {
|
||||
"provider": {
|
||||
"config": {
|
||||
"passphrase": "hunter2-literal",
|
||||
"passcode": "0000-literal",
|
||||
"passport": "/etc/deer-flow/passport.json",
|
||||
"compass_bearing": 42,
|
||||
"bypass_reason": "maintenance window",
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
redacted = support_bundle.redact_data(data)
|
||||
config = redacted["guardrails"]["provider"]["config"]
|
||||
|
||||
assert config["passphrase"] == "<redacted>"
|
||||
assert config["passcode"] == "<redacted>"
|
||||
assert config["passport"] == "/etc/deer-flow/passport.json"
|
||||
assert config["compass_bearing"] == 42
|
||||
assert config["bypass_reason"] == "maintenance window"
|
||||
|
||||
|
||||
def test_create_support_bundle_masks_provider_config_secret_shaped_keys(tmp_path):
|
||||
"""End-to-end: an open-ended guardrails.provider.config block in config.yaml
|
||||
must not leak into config-summary.json even though manifest.json declares
|
||||
redacted_secret_fields=true."""
|
||||
project_root = tmp_path / "project"
|
||||
project_root.mkdir()
|
||||
(project_root / "config.yaml").write_text(
|
||||
"config_version: 26\n"
|
||||
"models:\n - name: default\n"
|
||||
"guardrails:\n"
|
||||
" enabled: true\n"
|
||||
" provider:\n"
|
||||
" use: my_org.guardrails:CustomProvider\n"
|
||||
" config:\n"
|
||||
" db_pass: hunter2-literal\n"
|
||||
" encryption_key: 0123456789abcdef-literal\n"
|
||||
" redis_pass: redis-literal-secret\n"
|
||||
" webhook_signing_key: whsec_literal_secret\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
output_path = tmp_path / "support.zip"
|
||||
support_bundle.create_support_bundle(
|
||||
project_root=project_root,
|
||||
out_path=output_path,
|
||||
include_doctor=False,
|
||||
)
|
||||
|
||||
config_summary = json.loads(_zip_text(output_path, "config-summary.json"))
|
||||
provider_config = config_summary["guardrails"]["provider"]["config"]
|
||||
assert provider_config["db_pass"] == "<redacted>"
|
||||
assert provider_config["encryption_key"] == "<redacted>"
|
||||
assert provider_config["redis_pass"] == "<redacted>"
|
||||
assert provider_config["webhook_signing_key"] == "<redacted>"
|
||||
|
||||
manifest = json.loads(_zip_text(output_path, "manifest.json"))
|
||||
assert manifest["privacy"]["redacted_secret_fields"] is True
|
||||
|
||||
all_text = "\n".join(_zip_text(output_path, name) for name in zipfile.ZipFile(output_path).namelist())
|
||||
for secret in ("hunter2-literal", "0123456789abcdef-literal", "redis-literal-secret", "whsec_literal_secret"):
|
||||
assert secret not in all_text
|
||||
|
||||
|
||||
def test_create_support_bundle_masks_hardcoded_env_secret(tmp_path):
|
||||
project_root = tmp_path / "project"
|
||||
project_root.mkdir()
|
||||
|
||||
@@ -21,9 +21,35 @@ except Exception: # pragma: no cover - exercised only in broken environments
|
||||
|
||||
|
||||
SECRET_KEY_RE = re.compile(
|
||||
r"(api[_-]?key|access[_-]?key|token|secret|password|passwd|pwd|authorization|cookie|credential|private[_-]?key)",
|
||||
r"(api[_-]?key|access[_-]?key|private[_-]?key|(?<![a-zA-Z])key(?![a-zA-Z])"
|
||||
r"|token|secret|password|passwd|pwd|(?<![a-zA-Z])pass(?!port)"
|
||||
r"|authorization|cookie|credential|(?<![a-zA-Z])dsn(?![a-zA-Z]))",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
# Bare-word coverage above mirrors env_policy.py's *KEY*/*SECRET*/*TOKEN*/*PASS*/
|
||||
# *CREDENTIAL*/*DSN* sandbox-env denylist (backend/packages/harness/deerflow/sandbox/env_policy.py):
|
||||
# a fixed keyword allowlist misses a secret stored under an unanticipated key name
|
||||
# inside an open-ended config dict (e.g. guardrails.provider.config, which is an
|
||||
# arbitrary dict of provider-specific kwargs). The api_key/access_key/private_key/
|
||||
# password/passwd/pwd forms predate this and stay for their glued-compound coverage
|
||||
# (e.g. "apikey" with no separator); the new bare key/pass/dsn alternatives are
|
||||
# boundary-guarded so they match only their own delimited token and not an
|
||||
# unrelated word that merely starts with the same letters (keywords, keyboard).
|
||||
# `pass` only excludes a trailing "port" (passport) rather than any trailing
|
||||
# letter: excluding any trailing letter also missed genuine secret-bearing
|
||||
# names like passphrase/passcode, which env_policy.py's *PASS* substring match
|
||||
# does catch -- `compass`/`bypass` stay excluded via the leading-letter
|
||||
# lookbehind regardless of the lookahead.
|
||||
#
|
||||
# Case-insensitive exact key names that carry a bare credential with no
|
||||
# distinguishing keyword substring, mirroring env_policy.py's no-flag credential
|
||||
# sources (GH_PAT/GITHUB_PAT/REDIS_AUTH/REDISCLI_AUTH/PGSERVICEFILE). Matched as a
|
||||
# full key name, not a substring: a bare "pat"/"auth" wildcard would false-positive
|
||||
# on unrelated fields (author, authenticated, compatible, pattern, ...). Connection
|
||||
# strings (DATABASE_URL, REDIS_URL, ...) are deliberately not in this set -- their
|
||||
# embedded credentials are already stripped in place by URL_USERINFO_RE, which
|
||||
# preserves the host/port/db-name diagnostic value instead of blanking the field.
|
||||
NO_FLAG_CREDENTIAL_KEY_NAMES = frozenset({"gh_pat", "github_pat", "redis_auth", "rediscli_auth", "pgservicefile"})
|
||||
ENV_KEY_RE = re.compile(r"(?i)^env$")
|
||||
VAR_REFERENCE_RE = re.compile(r"^\$\{?[A-Za-z_][A-Za-z0-9_]*\}?$")
|
||||
ENV_SECRET_RE = re.compile(r"(?im)^([A-Z0-9_]*(?:API[_-]?KEY|TOKEN|SECRET|PASSWORD|PASSWD|AUTHORIZATION|COOKIE|CREDENTIAL)[A-Z0-9_]*\s*=\s*)(.+)$")
|
||||
@@ -104,11 +130,12 @@ def redact_data(value: Any) -> Any:
|
||||
if isinstance(value, dict):
|
||||
redacted: dict[Any, Any] = {}
|
||||
for key, item in value.items():
|
||||
if SECRET_KEY_RE.search(str(key)):
|
||||
key_str = str(key)
|
||||
if SECRET_KEY_RE.search(key_str) or key_str.lower() in NO_FLAG_CREDENTIAL_KEY_NAMES:
|
||||
redacted[key] = "<redacted>"
|
||||
elif ENV_KEY_RE.fullmatch(str(key)) and isinstance(item, dict):
|
||||
elif ENV_KEY_RE.fullmatch(key_str) and isinstance(item, dict):
|
||||
redacted[key] = {k: _redact_env_value(v) for k, v in item.items()}
|
||||
elif HEADER_KEY_RE.search(str(key)) and isinstance(item, dict):
|
||||
elif HEADER_KEY_RE.search(key_str) and isinstance(item, dict):
|
||||
redacted[key] = {k: "<redacted>" for k in item}
|
||||
else:
|
||||
redacted[key] = redact_data(item)
|
||||
|
||||
Reference in New Issue
Block a user