feat: include SHA-256 hash prefix in redacted secrets (FR-9)#12
Conversation
RedactSecrets() now outputs [REDACTED:{hash[:8]}] instead of plain
[REDACTED], using the same SHA-256 hash derivation as ExtractSecrets().
This allows correlating redacted text with vault entries without
exposing the secret value.
Added TestRedactSecretsHashMatchesExtract to verify hash consistency
between RedactSecrets and ExtractSecrets across all secret patterns.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the privacy module by introducing a mechanism to correlate redacted secrets with their original extracted forms. By embedding a SHA-256 hash prefix into the redaction marker, the system can now uniquely identify and link redacted values to their corresponding vault entries, which greatly aids in debugging and auditing without compromising sensitive data. This change improves the traceability of secrets throughout the system. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughДобавлена общая функция deriveHashPrefix для вычисления 8‑символьного SHA‑256 префикса из значения секрета; ExtractSecrets и RedactSecrets перешли на её использование. RedactSecrets теперь вставляет маркеры вида Changes
Sequence Diagram(s)(нет диаграмм — изменения не вводят новую многокомпонентную последовательность взаимодействий) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Поэма
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances secret redaction by including a SHA-256 hash prefix, allowing for cross-referencing with extracted secrets. The implementation is straightforward and includes a valuable new test to ensure the hashes match between ExtractSecrets and RedactSecrets. My review includes a couple of suggestions to improve code maintainability and test robustness.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/privacy/secrets.go (1)
131-137: Вынесите вычислениеhashPrefixв общий helper дляExtractSecretsиRedactSecrets.Сейчас одна и та же деривация хеша продублирована в двух местах; это риск расхождения поведения при следующих правках.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/privacy/secrets.go` around lines 131 - 137, The hash prefix derivation for secrets is duplicated; create a single helper (e.g., deriveHashPrefix or computeHashPrefix) that takes the secret string (after using extractSecretValue(match) or the original match) and returns the 8-char SHA-256 hex prefix, then replace the inline logic in both ExtractSecrets and RedactSecrets to call that helper so both functions share identical hashing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/privacy/secrets_test.go`:
- Line 160: The test calls t.Run(input[:20], ...) which can panic for inputs
shorter than 20; change the test to compute a safe sub-name before calling t.Run
by checking len(input) and using input[:20] only when len(input) > 20 (e.g.,
name := input; if len(name) > 20 { name = name[:20] }; then t.Run(name, func(t
*testing.T) { ... } ) ), ensuring the slice operation on input is protected;
update the occurrence of the raw slice inside the t.Run call (the fragment
input[:20]) accordingly.
In `@internal/privacy/secrets.go`:
- Around line 140-145: The current logic checks for "=" before ":" which can
pick an "=" inside the secret value (e.g., password: "a=b") and leak part of the
secret; update the code in internal/privacy/secrets.go that handles the local
variable match to find the first key/value separator by computing both indexes
(idxEq := strings.Index(match, "=") and idxColon := strings.Index(match, ":"))
and then choosing the smallest non-negative index (prefer the earlier separator
in the string) to redact from match[:idx+1] + "[REDACTED:"+hashPrefix+"]";
replace the two separate if-blocks with this single selection so you always cut
at the actual key/value separator rather than the first "=" encountered inside a
value.
---
Nitpick comments:
In `@internal/privacy/secrets.go`:
- Around line 131-137: The hash prefix derivation for secrets is duplicated;
create a single helper (e.g., deriveHashPrefix or computeHashPrefix) that takes
the secret string (after using extractSecretValue(match) or the original match)
and returns the 8-char SHA-256 hex prefix, then replace the inline logic in both
ExtractSecrets and RedactSecrets to call that helper so both functions share
identical hashing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31d59518-b722-44e9-9543-cf6ed32d18e7
📒 Files selected for processing (2)
internal/privacy/secrets.gointernal/privacy/secrets_test.go
|
@codex review |
…ash derivation - Extract deriveHashPrefix() helper to eliminate duplicated sha256+hex logic between ExtractSecrets and RedactSecrets; both functions now share a single consistent hash derivation path. - Fix CRITICAL separator-selection bug in RedactSecrets: previous logic always checked '=' before ':', so a value like password: "a=b" would use the '=' inside the value as the cut point, leaking part of the secret. New logic picks min(idxEq, idxColon) to always cut at the first separator. - Fix test fragility: t.Run(input[:20], ...) panics for inputs shorter than 20 chars; replaced with safe length-guarded name computation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/privacy/secrets.go`:
- Around line 58-61: При нормализации переменной value в secrets.go избегать
использования extractSecretValue для случаев с ":"-присваиванием, потому что
extractSecretValue отдаёт приоритет "=", что обрезает значения, содержащие "="
внутри секрета; вместо этого, если match содержит ":" (т.е. присваивание через
двоеточие), присвоить value как часть справа от первого ':' (с нужной обрезкой
пробелов/кавычек), иначе вызывать extractSecretValue(match); сохранять текущую
fallback-логику (если value == "" — установить value = match). Используйте
идентификаторы match, value и функцию extractSecretValue при правке.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: daf2275b-2c23-43c6-bafc-0013a7193fee
📒 Files selected for processing (2)
internal/privacy/secrets.gointernal/privacy/secrets_test.go
| value := extractSecretValue(match) | ||
| if value == "" { | ||
| value = match | ||
| } |
There was a problem hiding this comment.
Неверная нормализация value для hash при :-присваивании со знаком = внутри секрета
Сейчас hash может считаться по усечённому хвосту значения (например, password: "abc=def..."), потому что extractSecretValue приоритетно ищет "=". Это ломает стабильность auto:{hash} и может дать ложную дедупликацию разных секретов.
Предлагаемое исправление
+func firstKVSeparatorIndex(match string) int {
+ idxEq := strings.Index(match, "=")
+ idxColon := strings.Index(match, ":")
+ switch {
+ case idxEq == -1:
+ return idxColon
+ case idxColon == -1:
+ return idxEq
+ case idxEq < idxColon:
+ return idxEq
+ default:
+ return idxColon
+ }
+}
+
func extractSecretValue(match string) string {
- for _, sep := range []string{"=", ":"} {
- if idx := strings.Index(match, sep); idx != -1 {
- val := strings.TrimSpace(match[idx+1:])
- val = strings.Trim(val, `'"`)
- if val != "" {
- return val
- }
- }
- }
+ idx := firstKVSeparatorIndex(match)
+ if idx == -1 {
+ return ""
+ }
+ val := strings.TrimSpace(match[idx+1:])
+ val = strings.Trim(val, `'"`)
+ if val != "" {
+ return val
+ }
return ""
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| value := extractSecretValue(match) | |
| if value == "" { | |
| value = match | |
| } | |
| func firstKVSeparatorIndex(match string) int { | |
| idxEq := strings.Index(match, "=") | |
| idxColon := strings.Index(match, ":") | |
| switch { | |
| case idxEq == -1: | |
| return idxColon | |
| case idxColon == -1: | |
| return idxEq | |
| case idxEq < idxColon: | |
| return idxEq | |
| default: | |
| return idxColon | |
| } | |
| } | |
| func extractSecretValue(match string) string { | |
| - for _, sep := range []string{"=", ":"} { | |
| - if idx := strings.Index(match, sep); idx != -1 { | |
| - val := strings.TrimSpace(match[idx+1:]) | |
| - val = strings.Trim(val, `'"`) | |
| - if val != "" { | |
| - return val | |
| - } | |
| - } | |
| idx := firstKVSeparatorIndex(match) | |
| if idx == -1 { | |
| return "" | |
| } | |
| val := strings.TrimSpace(match[idx+1:]) | |
| val = strings.Trim(val, `'"`) | |
| if val != "" { | |
| return val | |
| } | |
| return "" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/privacy/secrets.go` around lines 58 - 61, При нормализации
переменной value в secrets.go избегать использования extractSecretValue для
случаев с ":"-присваиванием, потому что extractSecretValue отдаёт приоритет "=",
что обрезает значения, содержащие "=" внутри секрета; вместо этого, если match
содержит ":" (т.е. присваивание через двоеточие), присвоить value как часть
справа от первого ':' (с нужной обрезкой пробелов/кавычек), иначе вызывать
extractSecretValue(match); сохранять текущую fallback-логику (если value == "" —
установить value = match). Используйте идентификаторы match, value и функцию
extractSecretValue при правке.
1 participant
Summary
RedactSecrets()now outputs[REDACTED:{hash[:8]}]instead of[REDACTED]auto:{hash}vault entry name fromExtractSecrets()Test plan
RedactSecrets("sk-abc123...")→sk-a...[REDACTED:c99e03e4]RedactSecrets("api_key=abc123...")→api_key=[REDACTED:586f23e7][REDACTED:xxx]matchesauto:xxxfromExtractSecrets()Summary by CodeRabbit
Примечания к выпуску
Исправления ошибок
Тесты