Skip to content

Improve secret filter#144

Merged
AlphaOne1 merged 13 commits into
masterfrom
improve_secret_filter
Jan 29, 2026
Merged

Improve secret filter#144
AlphaOne1 merged 13 commits into
masterfrom
improve_secret_filter

Conversation

@AlphaOne1

@AlphaOne1 AlphaOne1 commented Jan 18, 2026

Copy link
Copy Markdown
Owner

The detection of secrets can now be configured on a per-instance basis rather than globally.

Summary by CodeRabbit

  • New Features

    • Per-instance and global configurable secret-detection regex for masking secrets.
  • Documentation

    • README and changelog updated with guidance, examples, and a new release note for 0.9.0.
  • Tests

    • Added and updated tests for configurable secret-detection and nil handling.
  • Chores

    • Updated copyright years, refreshed indirect Go dependencies, and reformatted CI workflow files (no behavior change).

✏️ Tip: You can customize this high-level summary in your review settings.

…stency.

Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
@AlphaOne1 AlphaOne1 self-assigned this Jan 18, 2026
@coderabbitai

coderabbitai Bot commented Jan 18, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Add per-instance secret-detection regexp to Config, propagate it through overlay/merge and secret-hiding, change HideSecrets to accept a regexp, add tests and docs, update CI workflow YAML formatting and indirect go.mod deps, and bump SPDX header years across many files.

Changes

Cohort / File(s) Summary
Core secret customization
config.go, secrets.go
Add secretRE *regexp.Regexp to Config[T]; add SecretRE() and SetSecretRE() and ErrNoSecretRegexp; thread secretRE through overlay/merge/ToSecretsHidden; change HideSecrets/hideSecrets signatures to accept a per-instance secretRE.
Tests for secret feature
config_test.go, secrets_test.go
Add TestSetSecretRE and TestSetSecretRENil; extend TestHideSecrets cases to pass secretRE; update SPDX years in tests.
Docs & release notes
CHANGELOG.md, README.md
Add Release 0.9.0 entry and README example documenting per-instance secret regex and SetSecretRE.
CI workflow formatting
.github/workflows/...
.../codeql.yml, .../compliance.yml, .../release.yml, .../security.yml
Reformat/reindent workflows; convert branch arrays to block lists and adjust whitespace only — no behavior changes.
Dependency updates
go.mod
Bump indirect golang.org/x/* module versions (x/crypto, x/mod, x/sync, x/sys, x/term, x/text, x/tools).
Copyright/year bumps
*.go, examples/..., *_test.go (many files)
Update SPDX/header years from 2025 → 2026 across source, tests, and examples; no logic changes.
Examples (no behavior changes)
examples/...
Only header year updates in example sources and tests; no functional edits.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant Config as Config
    participant Merge as MergeLogic
    participant Hider as HideSecrets
    participant Detector as SecretDetector

    User->>Config: SetSecretRE(customRE)
    Config-->>Config: store secretRE

    User->>Config: Overlay(otherConfig)
    Config->>Merge: pass secretRE
    Merge-->>Config: merged Config (with secretRE)

    User->>Config: ToSecretsHidden(node)
    Config->>Hider: HideSecrets(node, hideStructure, secretRE)
    Hider->>Detector: match secrets using secretRE
    Detector-->>Hider: matches
    Hider-->>Config: node with secrets hidden
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I tuck a regex deep in each Config's sleeve,

I hop through merges and secrets I weave.
With a twitch and a sniff I quietly hide,
Tokens and keys safe, no leaks on my stride. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Improve secret filter' directly aligns with the main objective: enabling per-instance secret detection configuration rather than global-only configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@config_test.go`:
- Line 1: Add a unit test that covers the new SetSecretRE method: create or
reuse a config instance, call SetSecretRE with a specific regex (e.g., a pattern
matching "MYSECRET" or a custom key), then exercise the same secret-hiding logic
used by TestSecretsHidden/TestSecretsHiddenStructured to assert that values
matching the custom regex are masked while non-matching values are unchanged;
name the new test (e.g., TestSetSecretRE) and mirror the existing test structure
and assertions so it verifies SetSecretRE actually alters the masking behavior.

In `@README.md`:
- Around line 430-433: The code example has a missing assignment operator: when
calling templig.FromFile[Config] you must capture the returned value(s) using
:=; update the example so the variable c is declared via short assignment from
templig.FromFile[Config] (and still call c.SetSecretRE with
regexp.MustCompile(templig.SecretDefaultRE + "|token")), ensuring the call site
matches the templig.FromFile[Config] return signature.

In `@secrets.go`:
- Around line 27-29: The comment for HideSecrets incorrectly references
[SecretRE] even though the regex is now passed in; update the doc comment to say
that secrets are identified by the provided regexp parameter (secretRE) supplied
by the caller. Also make HideSecrets nil-safe by checking if secretRE is nil
before any use (e.g., before calling secretRE.MatchString in the function body)
and handle nil by either returning early or treating it as "no matches" to avoid
a panic; ensure references to the function name HideSecrets and the parameter
secretRE are used to locate where to add the nil-check and doc update.
🧹 Nitpick comments (3)
CHANGELOG.md (1)

7-9: Minor: Inconsistent terminology (regex vs regexp).

Line 7 uses "regex" while line 9 uses "regexp". Consider using consistent terminology throughout.

📝 Suggested fix
-- introduced `Config.SetSecretRE` to allow users to define their own secret regex
+- introduced `Config.SetSecretRE` to allow users to define their own secret regexp
   per instance

Or alternatively, change "regexp" to "regex" on line 9.

secrets_test.go (1)

272-272: Tests correctly updated for new signature; consider adding custom regex test coverage.

The test updates correctly pass templig.SecretRE to match the new HideSecrets signature. However, the PR introduces per-instance secret regex customization, but no tests exercise this feature with a custom regex pattern.

Consider adding a test case that verifies HideSecrets works correctly with a non-default regex pattern to ensure the per-instance functionality is properly covered.

Also applies to: 293-293, 318-318

config.go (1)

314-318: Consider adding nil-safety or documentation for SetSecretRE.

The SetSecretRE method allows setting nil, which would cause a panic when HideSecrets is later called (since secretRE.MatchString would be invoked on nil). Consider either:

  1. Adding a nil check that falls back to the default SecretRE, or
  2. Documenting that nil is not a valid input
Option 1: Add nil-safety
 // SetSecretRE sets the regular expression to be used for hiding secrets of that specific instance.
+// If re is nil, the default [SecretRE] is used.
 func (c *Config[T]) SetSecretRE(re *regexp.Regexp) {
+	if re == nil {
+		re = SecretRE
+	}
 	c.secretRE = re
 }

Comment thread config_test.go
Comment thread README.md
Comment thread secrets.go Outdated
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
…ult logic.

Signed-off-by: Alexander Adam <alphaone23@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@config.go`:
- Around line 322-328: SecretRE on Config[T] dereferences c.secretRE without
guarding for nil which can panic for zero-valued or uninitialized configs;
modify the SecretRE method to check if c.secretRE == nil and either return nil
(or initialize a sensible default regexp) instead of dereferencing, and update
callers or the method's docstring to reflect the nil-return behavior (or that it
lazily initializes) so callers like any code invoking Config.SecretRE know to
handle a nil result.
🧹 Nitpick comments (2)
secrets.go (1)

27-39: Consider reusing the pre-compiled SecretRE instead of compiling a new regex.

When secretRE is nil, the code compiles a new regex from SecretDefaultRE. However, SecretRE (line 20) is already a pre-compiled version of this same pattern. Reusing it avoids redundant compilation overhead.

♻️ Suggested optimization
 	if secretRE == nil {
-		secretRE = regexp.MustCompile(SecretDefaultRE)
+		secretRE = SecretRE
 	}
config.go (1)

188-190: Documentation refers to global [SecretRE] but per-instance regex is now used.

The doc comment mentions hiding secrets "using the [SecretRE]", but the implementation now uses the per-instance c.secretRE. Consider updating the documentation to reflect this change (e.g., "using the configured secret regex").

…ng behavior for unset values.

Signed-off-by: Alexander Adam <alphaone23@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@config.go`:
- Around line 323-333: The SecretRE method currently creates a value copy of
c.secretRE (tmp := *c.secretRE), which duplicates the regexp's internal mutex;
update SecretRE to return a safe copy by calling c.secretRE.Copy() instead,
i.e., replace the value-copy logic in the SecretRE method to use
c.secretRE.Copy() and return that result (handling the nil case as before) to
avoid mutex duplication and deadlocks.

Comment thread config.go
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
Signed-off-by: Alexander Adam <alphaone23@gmail.com>
@AlphaOne1 AlphaOne1 merged commit 74aeca0 into master Jan 29, 2026
19 checks passed
@AlphaOne1 AlphaOne1 deleted the improve_secret_filter branch January 29, 2026 19:04
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