From 283cea567e5ffbdea38a74bf394da84bd036bacc Mon Sep 17 00:00:00 2001
From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com>
Date: Sat, 18 Jul 2026 16:39:52 -0700
Subject: [PATCH] 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.
---
 backend/tests/test_support_bundle.py | 144 +++++++++++++++++++++++++++
 scripts/support_bundle.py            |  35 ++++++-
 2 files changed, 175 insertions(+), 4 deletions(-)

diff --git a/backend/tests/test_support_bundle.py b/backend/tests/test_support_bundle.py
index 596d8fd65..b0202ec47 100644
--- a/backend/tests/test_support_bundle.py
+++ b/backend/tests/test_support_bundle.py
@@ -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()
diff --git a/scripts/support_bundle.py b/scripts/support_bundle.py
index b782ba6bb..2eb0247b0 100644
--- a/scripts/support_bundle.py
+++ b/scripts/support_bundle.py
@@ -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)