-
Notifications
You must be signed in to change notification settings - Fork 12
chore: add redact package to not print sensitive data #270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a centralized redaction utility (agent/redact package) to mask sensitive information in logs throughout the agent codebase. The new package provides two main functions: SensitiveData for recursively redacting sensitive fields in data structures, and Args for masking sensitive command-line arguments. The implementation replaces previous ad-hoc masking logic with a consistent, reusable approach.
Changes:
- Added new
agent/redactpackage with comprehensive test coverage for masking sensitive data in logs - Updated all backend Start methods (worker, snmpdiscovery, networkdiscovery, devicediscovery) to use
redact.Argsinstead of manual secret swapping - Updated configuration logging in agent.go, cmd/main.go, and fleet config manager to use
redact.SensitiveData
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| agent/redact/redact.go | New package implementing sensitive data redaction with reflection-based deep copying and pattern-based field detection |
| agent/redact/redact_test.go | Comprehensive test suite covering edge cases, different data types, and real config structures |
| agent/agent.go | Updated backend logging to use redact.SensitiveData for configuration objects |
| cmd/main.go | Updated backend logging to use redact.SensitiveData for configuration objects |
| agent/configmgr/fleet.go | Replaced manual ClientSecret masking with redact.SensitiveData for full config redaction |
| agent/configmgr/fleet/auth.go | Updated authentication request logging to use redact.SensitiveData |
| agent/backend/worker/worker.go | Removed maskedSecret constant and manual swapping; now uses redact.Args for cleaner implementation |
| agent/backend/snmpdiscovery/snmp_discovery.go | Removed maskedSecret constant and manual swapping; now uses redact.Args for cleaner implementation |
| agent/backend/networkdiscovery/network_discovery.go | Removed maskedSecret constant and manual swapping; now uses redact.Args for cleaner implementation |
| agent/backend/devicediscovery/device_discovery.go | Removed maskedSecret constant and manual swapping; now uses redact.Args for cleaner implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab088e80c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Go test coverage
Total coverage: 60.1% |
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecf5a0ca73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c71539338d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This pull request introduces a new
redactpackage and refactors logging throughout the agent to ensure sensitive information (such as secrets, tokens, and passwords) is properly masked in logs. The changes replace previous ad-hoc secret masking with a consistent, reusable approach for redacting sensitive data in both structured data and command-line arguments. This improves security and maintainability.Key changes:
Sensitive Data Redaction (core implementation):
agent/redact/redact.gopackage, providingSensitiveDataandArgsfunctions to recursively mask sensitive fields in structs, maps, and CLI arguments. The package defines patterns and suffixes for identifying sensitive data and ensures all redacted output uses a consistent mask.Agent and Backend Logging Updates:
agent.goand all backendStartmethods (device_discovery.go,network_discovery.go,snmp_discovery.go,worker.go) to useredact.SensitiveDataandredact.Argsfor masking secrets in configuration and CLI arguments, replacing previous manual masking and in-place value swapping. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]Configuration Management Improvements:
fleetConfigManagerinconfigmgr/fleet.goto useredact.SensitiveDatafor masking sensitive fields (likeclient_secret) in configuration logs, replacing the previous manual masking logic. [1] [2]AuthTokenManagerinconfigmgr/fleet/auth.goto mask sensitive data in debug logs for token requests. [1] [2]Testing Adjustments:
fleet_test.goto expect the new masked secret value (********) in YAML output, matching the new redaction standard. [1] [2]Code Clean-up:
maskedSecretconstants from backend files, since masking is handled centrally by the new redaction package. [1] [2] [3] [4]These changes provide a robust and consistent way to prevent sensitive information from leaking into logs, improving the overall security posture of the agent.