Skip to content

[medium] Redact config field with placeholder instead of deleting it - #38

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/41-redact-placeholder
Open

[medium] Redact config field with placeholder instead of deleting it#38
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/41-redact-placeholder

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026

Copy link
Copy Markdown
Member

BLUF — Redacting a config key deletes it outright, so legitimate module result fields vanish without trace.

  • Problemredact_config_keys() in bin/cli.py recursively deletes every dict key named config from module responses. That is needed because modules such as passivetotal echo API credentials back under that key, but a module returning a legitimate result field named config loses it with no trace it existed.
  • Fix — Replaces the value with the string <redacted> instead of removing the key.
  • Effect — Credentials stay hidden, and any suppressed field is now visible in the output rather than silently vanishing.

Finding 41 (Low) — bin/cli.py:65-74

Problem

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
Called redact_config_keys({"config": {"apikey":"secret"}, "results": [{"config": "legit-data-field", "other":1}]}) -> {'results': [{'other': 1}]} -- the legitimate "config": "legit-data-field" result field vanished with no trace.
After
Same input -> {'config': '<redacted>', 'results': [{'config': '<redacted>', 'other': 1}]} -- credentials still hidden, but the key and its redaction are now visible instead of silently disappearing.

python bin/cli.py --help exits 0 and the module still imports cleanly. Verification was performed offline against the pure functions — no running misp-modules instance 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

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.
@elhoim elhoim changed the title Redact config field with placeholder instead of deleting it [medium] Redact config field with placeholder instead of deleting it Sep 3, 2026
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.

1 participant