Skip to content

Fix WithRedactionKeys validation inconsistency#44

Closed
Copilot wants to merge 3 commits intofeat/secretsfrom
copilot/sub-pr-42-again
Closed

Fix WithRedactionKeys validation inconsistency#44
Copilot wants to merge 3 commits intofeat/secretsfrom
copilot/sub-pr-42-again

Conversation

Copy link
Contributor

Copilot AI commented Jan 12, 2026

The WithRedactionKeys validation exhibited inconsistent behavior: passing all-whitespace keys would fail if no default keys existed, but succeed if default keys were present. This created unpredictable behavior based on initialization state.

Changes

  • Track addedCount of normalized keys actually added, validate against this instead of total key count
  • Update documentation to reflect consistent validation behavior regardless of existing keys

Example

// Previously: succeeds because default keys exist in cfg.keys
r, err := NewRedactor(WithRedactionKeys("   ", "\t", "\n"))

// Now: consistently fails since no valid keys were added
r, err := NewRedactor(WithRedactionKeys("   ", "\t", "\n"))
// err == ErrInvalidRedactorConfig

Test coverage added for edge cases: all-whitespace keys, mixed valid/invalid keys.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits January 12, 2026 12:39
Co-authored-by: hyp3rd <62474964+hyp3rd@users.noreply.github.com>
Co-authored-by: hyp3rd <62474964+hyp3rd@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issues in secrets detection and redaction package based on review Fix WithRedactionKeys validation inconsistency Jan 12, 2026
Copilot AI requested a review from hyp3rd January 12, 2026 12:42
@hyp3rd hyp3rd marked this pull request as ready for review January 12, 2026 13:08
Copilot AI review requested due to automatic review settings January 12, 2026 13:08
Copy link
Contributor

Copilot AI left a 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 PR fixes a validation inconsistency in WithRedactionKeys where all-whitespace keys would fail validation only when no default keys existed, but succeed when default keys were present. The fix tracks the count of valid keys actually added from the input and validates against this count, ensuring consistent behavior regardless of the initialization state.

Changes:

  • Modified WithRedactionKeys to track addedCount of normalized keys that were actually added, validating against this instead of the total keys in the configuration
  • Updated documentation to clarify that validation is consistent regardless of existing keys
  • Added comprehensive test coverage for edge cases including all-whitespace keys, empty keys, valid keys, and mixed valid/invalid keys

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/secrets/redact.go Fixed validation logic to use addedCount instead of len(cfg.keys) and updated documentation to reflect consistent validation behavior
pkg/secrets/secrets_test.go Added TestWithRedactionKeysValidation with comprehensive test cases for the validation edge cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hyp3rd hyp3rd closed this Jan 12, 2026
@hyp3rd hyp3rd deleted the copilot/sub-pr-42-again branch January 12, 2026 13:27
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.

2 participants