Redact config field with placeholder instead of deleting it - #38
Open
elhoim wants to merge 1 commit into
Open
Conversation
redact_config_keys() dropped any dict key named 'config' entirely. This is needed because some modules (e.g. passivetotal) echo API credentials back in a 'config' field, but it meant any module returning a legitimate result field named 'config' would have it silently vanish with no trace. Replace the value with the string '<redacted>' instead of deleting the key, so the credential data is still hidden but the data loss is visible in the output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finding 41 (Low) —
bin/cli.py:65-74Problem
redact_config_keys recursively DELETES any dict key named 'config' from the module response. It is currently protective (passivetotal echoes credentials back) so it must stay - but a module returning a legitimate result field named config would have it silently removed with no placeholder.
Fix
redact_config_keys() in bin/cli.py recursively deleted any dict key literally named "config" from module responses. This is needed because some modules (e.g. passivetotal) echo back API credentials under a "config" key, but the blanket deletion meant a module returning a legitimate result field also named "config" would silently lose that data with zero trace it ever existed. Fixed by replacing the value with the string "" instead of removing the key, preserving the protective redaction while making any data loss visible in the output.
Verification
Reproduced against the unmodified code at
9b8c605, then re-checked after the change.Before
After
python bin/cli.py --helpexits 0 and the module still imports cleanly. Verification was performed offline against the pure functions — no runningmisp-modulesinstance is required.Branched from
9b8c605. This PR addresses only this finding; the other findings from the same review are in separate PRs, so they will need rebasing against each other as they merge.🤖 Generated with Claude Code
https://claude.ai/code/session_01DYX4TKA5inzByJ4qGWKjqh