MM-69141: Mask Confluence URL and OAuth credentials in support packets#230
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe plugin manifest's settings schema is extended with three new secret configuration fields: Confluence URL, OAuth client ID, and OAuth client secret. These entries are marked as sensitive and documented as being set during plugin installation and setup. ChangesConfluence OAuth Settings
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
edgarbellot
left a comment
edgarbellot
left a comment
There was a problem hiding this comment.
@nang2049 LGTM, just a suggestion.
| "display_name": "Confluence URL", | ||
| "type": "text", | ||
| "help_text": "Configured via the /confluence install setup command. Declared here so it is redacted from support packets.", | ||
| "secret": true |
There was a problem hiding this comment.
Support packets typically mask sensitive information - like API tokens and secrets - while keeping data useful for troubleshooting issues unmasked. Therefore, it might only be necessary to mask ConfluenceOAuthClientSecret in this case.
This seems to be the standard pattern followed in the sanitized_config.json file generated with support packets. If there are other reasons to mask the URL and client ID, please feel free to ignore my comment :)
3 participants
Summary
Three plugin config fields
ConfluenceURL,ConfluenceOAuthClientID, andConfluenceOAuthClientSecretwere never declared inplugin.json'ssettings_schema. Declaring the three fields in the schema with"secret": truehooks them into the same masking path the other secrets already use.QA
System Console → Reporting → Download Support Packet.
Extract the archive, open sanitized_config.json, locate Plugins["com.mattermost.confluence"].
Confirm all six string fields show ******************************** and only
serverversiongreaterthan9remains visible as a boolean.Ticket Link
MM-69141
Change Impact: 🟢 Low
Reasoning: This is a purely declarative manifest update that adds three configuration fields to the plugin schema and marks them with the existing
"secret": trueflag. The change leverages established masking infrastructure already in place for other sensitive fields, introduces no new code logic, and carries minimal regression risk.Regression Risk: Extremely low. The three fields (ConfluenceURL, ConfluenceOAuthClientID, ConfluenceOAuthClientSecret) already exist in the system and are configured via setup commands; this change simply declares them in the schema to integrate them into the existing support packet masking mechanism. No behavioral changes, no modifications to core logic, no new dependencies introduced.
QA Recommendation: Minimal manual QA is required beyond the provided QA steps. Since the masking mechanism is already tested and working for other secrets in the schema, verification that these three fields are properly masked in the sanitized_config.json (per the provided steps) is sufficient. The risk of skipping manual QA is negligible given the declarative nature of the change and the established precedent of the masking infrastructure.