Skip to content

Add SecretMasking capability#172

Draft
DouweM wants to merge 2 commits intomainfrom
capability/secret-masking
Draft

Add SecretMasking capability#172
DouweM wants to merge 2 commits intomainfrom
capability/secret-masking

Conversation

@DouweM
Copy link
Copy Markdown
Contributor

@DouweM DouweM commented Apr 10, 2026

Summary

  • Adds SecretMasking capability that redacts secrets, API keys, and sensitive data from tool outputs and model responses
  • Uses after_tool_execute to scrub tool return values and after_model_request to scrub model response TextParts
  • Built-in pattern categories: api_keys (OpenAI, Anthropic, AWS, GitHub, Slack, Google, generic), tokens (Bearer, JWT), connection_strings (password-in-URL, database URIs), private_keys (RSA, EC, OpenSSH)
  • Configurable: categories, custom_patterns, replacement string (default [REDACTED])

Closes #78

Test plan

  • 45 tests covering all pattern categories, both hooks, edge cases (empty string, None, non-string results, non-text parts)
  • 100% code coverage
  • Passes ruff check, ruff format, pyright strict mode

Generated with Claude Code

DouweM and others added 2 commits April 2, 2026 05:32
…and model responses

Implements regex-based detection and masking of API keys, Bearer tokens, JWTs,
connection strings, and private keys using `after_tool_execute` and
`after_model_request` hooks. Configurable pattern categories, custom patterns,
and replacement string.

Closes #78

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…rubbing, and partial masking

Addresses audit findings from PR #157:
- Add patterns for Azure subscription keys, Stripe, SendGrid, Twilio, GCP service account keys
- Add `env_file` category to detect KEY=value lines in .env-style content
- Add `before_tool_execute` hook to scrub secrets from tool call args before execution
- Add `partial_mask` option to keep first 4 chars visible (e.g. `sk-a****`)
- Fix `after_model_request` signature to use `ModelRequestContext` instead of `Any`
- Fix `after_tool_execute` args type to use `ValidatedToolArgs` instead of `dict[str, Any]`

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment on lines +91 to +103
for key, value in d.items():
if isinstance(value, str):
if partial:
result[key] = _partial_mask_text(value, patterns, visible_chars)
else:
result[key] = _mask_text(value, patterns, replacement)
elif isinstance(value, dict):
result[key] = _mask_dict_values(
cast(dict[str, Any], value), patterns, replacement, partial=partial, visible_chars=visible_chars
)
else:
result[key] = value
return result
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 _mask_dict_values doesn't recurse into list values, allowing secrets in list-typed tool args to bypass masking

The _mask_dict_values function handles str and dict values recursively but passes all other types (including list) through unchanged (secret_masking.py:101-102). Since before_tool_execute delegates entirely to _mask_dict_values (secret_masking.py:180-185), tool arguments like {'keys': ['sk-abc123def456ghi789jkl012mno']} or {'configs': [{'token': 'sk-ant-secret...'}]} will have their secrets pass through unmasked. This is inconsistent with the nested-dict handling (which does recurse) and creates a security gap in a security-critical code path.

Example of unmasked secrets in list args

Given tool args:

args = {'items': ['sk-abc123def456ghi789jkl012mno'], 'config': {'key': 'sk-abc123def456ghi789jkl012mno'}}

After _mask_dict_values, args['config']['key'] is correctly [REDACTED], but args['items'][0] still contains the raw secret.

Suggested change
for key, value in d.items():
if isinstance(value, str):
if partial:
result[key] = _partial_mask_text(value, patterns, visible_chars)
else:
result[key] = _mask_text(value, patterns, replacement)
elif isinstance(value, dict):
result[key] = _mask_dict_values(
cast(dict[str, Any], value), patterns, replacement, partial=partial, visible_chars=visible_chars
)
else:
result[key] = value
return result
result: dict[str, Any] = {}
for key, value in d.items():
if isinstance(value, str):
if partial:
result[key] = _partial_mask_text(value, patterns, visible_chars)
else:
result[key] = _mask_text(value, patterns, replacement)
elif isinstance(value, dict):
result[key] = _mask_dict_values(
cast(dict[str, Any], value), patterns, replacement, partial=partial, visible_chars=visible_chars
)
elif isinstance(value, list):
result[key] = _mask_list_values(value, patterns, replacement, partial=partial, visible_chars=visible_chars)
else:
result[key] = value
return result
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +48 to +50
_ENV_FILE_PATTERNS: dict[str, re.Pattern[str]] = {
'env_key_value': re.compile(r'(?m)^[A-Z][A-Z0-9_]+=.+$'),
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 env_file category enabled by default is very aggressive and may cause false positives

The env_key_value pattern (?m)^[A-Z][A-Z0-9_]+=.+$ matches any line that looks like UPPER_CASE_VAR=value. When SecretMasking() is instantiated with defaults (categories=None), this category is enabled alongside all others. This will redact innocuous tool outputs or model responses containing lines like PATH=/usr/bin, DEBUG=true, or HOME=/home/user. This is a design choice but could lead to surprising over-redaction in production. Consider whether env_file should be opt-in rather than included in the default set.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@DouweM
Copy link
Copy Markdown
Contributor Author

DouweM commented Apr 10, 2026

Originally posted by @DouweM in #157 comment (PR closed due to history rewrite)

Audit vs prior art: SecretMasking

Worth adding now:

  • More key patterns: Azure, GCP, Stripe, SendGrid, Twilio
  • .env content detection (KEY=value lines)
  • Mask in tool call args too (via before_tool_execute)
  • Partial masking option (sk-**** instead of [REDACTED])

Follow-up opportunities:

  • Encrypted secret registry, env-var blocking

@DouweM DouweM marked this pull request as draft April 10, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Secret Management / Credential Masking in tool outputs

1 participant