This is an automated email from the ASF dual-hosted git repository. cgivre pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/drill-mcp.git
commit b3caedc150162804c25f5dbbb0202ca5e3cdb6b4 Author: cgivre <[email protected]> AuthorDate: Wed Aug 12 16:26:37 2026 -0400 fix: redact secrets embedded inside string values, not just keys redact() only matched sensitive-looking keys, so a secret embedded inside an ordinary-looking value survived untouched -- e.g. {"connection": "s3a://AKIA:secret@bucket"} or a JDBC-style URL with a password= query parameter, both realistic Drill storage-plugin config shapes. Scrub userinfo out of URL-shaped values and redact password/secret/api_key/token query parameters after the key check. Updates test_leaves_innocuous_keys_alone's sibling assertion, which encoded this bug (connection strings without embedded credentials are still left alone; only embedded credentials are now redacted). --- drill_mcp/redact.py | 28 ++++++++++++++++++++++++++++ tests/test_redact.py | 21 +++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/drill_mcp/redact.py b/drill_mcp/redact.py index f6ac0a4..150ea28 100644 --- a/drill_mcp/redact.py +++ b/drill_mcp/redact.py @@ -39,6 +39,32 @@ _SENSITIVE = re.compile( re.IGNORECASE, ) +# The key-based check above only catches secrets that live at a sensitive +# *key*. A storage-plugin config routinely carries secrets embedded inside an +# ordinary-looking *value* instead -- a JDBC/S3-style URL with userinfo +# (`s3a://AKIA:secret@bucket`), or a connection string with a `password=`/ +# `secret=`/`token=` query parameter. Both shapes are real Drill storage +# plugin configs, so string values are scrubbed too, not just keys. +# +# Matches "scheme://user:pass@" and keeps everything else (scheme, host, +# path) intact -- only the credential pair between "//" and "@" is replaced. +_URL_USERINFO = re.compile(r"(?P<scheme>[A-Za-z][A-Za-z0-9+.-]*://)[^/@\s]+:[^/@\s]*@") + +# Matches a `?key=value` or `&key=value` query parameter whose key looks like +# a secret, and replaces only the value -- the `?`/`&` and key name are kept +# so the rest of the string still parses as the same shape of URL. +_QUERY_SECRET = re.compile( + r"(?P<prefix>[?&](?:password|secret|api_?key|token)=)[^&]*", + re.IGNORECASE, +) + + +def _scrub_value(value: str) -> str: + """Strip credentials embedded inside a string value, not just its key.""" + value = _URL_USERINFO.sub(lambda m: f"{m.group('scheme')}{REDACTED}@", value) + value = _QUERY_SECRET.sub(lambda m: f"{m.group('prefix')}{REDACTED}", value) + return value + def redact(value: Any) -> Any: """Return a copy of `value` with sensitive-looking values replaced.""" @@ -51,4 +77,6 @@ def redact(value: Any) -> Any: return [redact(item) for item in value] if isinstance(value, tuple): return tuple(redact(item) for item in value) + if isinstance(value, str): + return _scrub_value(value) return value diff --git a/tests/test_redact.py b/tests/test_redact.py index 7c0c64b..f195a20 100644 --- a/tests/test_redact.py +++ b/tests/test_redact.py @@ -48,6 +48,27 @@ def test_leaves_innocuous_keys_alone(): } +def test_redacts_userinfo_embedded_in_a_url_shaped_value(): + # A secret can live inside an ordinary-looking value, not just behind a + # sensitive key -- e.g. a `connection` string embedding S3 credentials as + # userinfo. This used to survive untouched (the bug this test replaces + # `test_leaves_innocuous_keys_alone`'s old assertion for): the key-based + # check alone let `redact({"connection": "s3a://AKIA:secret@bucket"})` + # pass the secret straight through. + source = {"connection": "s3a://AKIA:supersecret@bucket"} + result = redact(source) + assert "supersecret" not in result["connection"] + assert "AKIA" not in result["connection"] + assert result["connection"] == "s3a://***REDACTED***@bucket" + + +def test_redacts_password_query_parameter_embedded_in_a_url_shaped_value(): + source = {"url": "jdbc:mysql://host/db?user=root&password=hunter2"} + result = redact(source) + assert "hunter2" not in result["url"] + assert result["url"] == "jdbc:mysql://host/db?user=root&password=***REDACTED***" + + def test_recurses_into_nested_dicts(): source = {"config": {"aws": {"awsSecretAccessKey": "s"}}} assert redact(source)["config"]["aws"]["awsSecretAccessKey"] == REDACTED
