fix: redact any key #3860
fix: redact any key #3860
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughMoves JSON-string redaction out of WithMetrics into package-scoped utilities: introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware as MetricsMiddleware
participant Redactor as redact()
participant Handler as NextHandler
participant Logger
Client->>Middleware: HTTP request (body)
Middleware->>Redactor: redact(request body)
Redactor-->>Middleware: sanitized body
Middleware->>Handler: call next handler with sanitized body
Handler-->>Middleware: response (body)
Middleware->>Redactor: redact(response body)
Redactor-->>Middleware: sanitized response
Middleware->>Logger: record metrics & log sanitized bodies
Middleware-->>Client: HTTP response (sanitized)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
go/pkg/zen/middleware_metrics.go (2)
78-83: Trim whitespace on X-Forwarded-For.Safer to trim the first hop IP; proxies often include spaces.
- ips := strings.Split(s.r.Header.Get("X-Forwarded-For"), ",") - ipAddress := "" - if len(ips) > 0 { - ipAddress = ips[0] - } + ips := strings.Split(s.r.Header.Get("X-Forwarded-For"), ",") + ipAddress := "" + if len(ips) > 0 { + ipAddress = strings.TrimSpace(ips[0]) + }
16-18: Add a doc comment for exported interface EventBuffer.Public types must be documented per guidelines.
type EventBuffer interface { + // BufferApiRequest enqueues a request event for asynchronous export + // (e.g., to ClickHouse). Implementations should be safe for concurrent use. BufferApiRequest(schema.ApiRequestV1) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
go/pkg/zen/middleware_metrics.go(1 hunks)go/pkg/zen/middleware_metrics_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/pkg/zen/middleware_metrics_test.gogo/pkg/zen/middleware_metrics.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Use table-driven tests in Go
Organize Go integration tests with real dependencies
Organize Go tests by HTTP status codes
Files:
go/pkg/zen/middleware_metrics_test.go
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/pkg/zen/middleware_metrics_test.gogo/pkg/zen/middleware_metrics.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
📚 Learning: 2025-01-31T13:50:45.004Z
Learnt from: chronark
PR: unkeyed/unkey#2825
File: apps/dashboard/app/(app)/logs/components/table/logs-table.tsx:100-118
Timestamp: 2025-01-31T13:50:45.004Z
Learning: Sensitive data in logs is masked at the API layer (apps/api/src/pkg/middleware/metrics.ts) before being written to Clickhouse. This includes redacting API keys, plaintext values, and authorization headers using regex replacements.
Applied to files:
go/pkg/zen/middleware_metrics.go
🪛 Gitleaks (8.27.2)
go/pkg/zen/middleware_metrics_test.go
17-17: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.
(stripe-access-token)
298-298: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.
(stripe-access-token)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (2)
go/pkg/zen/middleware_metrics.go (1)
93-97: LGTM: central redaction applied to request/response bodies before logging.This ensures consistent masking in ClickHouse logs.
go/pkg/zen/middleware_metrics_test.go (1)
9-242: Solid coverage and edge cases.Comprehensive table-driven tests for redact() including escapes, malformed input, and idempotence.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
go/pkg/zen/middleware_metrics.go(1 hunks)go/pkg/zen/middleware_metrics_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/pkg/zen/middleware_metrics.gogo/pkg/zen/middleware_metrics_test.go
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/pkg/zen/middleware_metrics.gogo/pkg/zen/middleware_metrics_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Use table-driven tests in Go
Organize Go integration tests with real dependencies
Organize Go tests by HTTP status codes
Files:
go/pkg/zen/middleware_metrics_test.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.283Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
Learnt from: chronark
PR: unkeyed/unkey#3860
File: go/pkg/zen/middleware_metrics_test.go:297-313
Timestamp: 2025-08-27T11:02:17.134Z
Learning: The redact function in go/pkg/zen/middleware_metrics.go is not expected to be called concurrently, so concurrency tests are unnecessary for this function.
📚 Learning: 2025-08-27T11:02:17.134Z
Learnt from: chronark
PR: unkeyed/unkey#3860
File: go/pkg/zen/middleware_metrics_test.go:297-313
Timestamp: 2025-08-27T11:02:17.134Z
Learning: The redact function in go/pkg/zen/middleware_metrics.go is not expected to be called concurrently, so concurrency tests are unnecessary for this function.
Applied to files:
go/pkg/zen/middleware_metrics.gogo/pkg/zen/middleware_metrics_test.go
📚 Learning: 2025-07-16T17:51:57.297Z
Learnt from: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
Applied to files:
go/pkg/zen/middleware_metrics_test.go
🪛 Gitleaks (8.27.2)
go/pkg/zen/middleware_metrics_test.go
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (2)
go/pkg/zen/middleware_metrics.go (1)
25-36: Regexes correctly match generic JSON string values for "key" and "plaintext".Covers escaped quotes and varied whitespace; tests reflect intent. LGTM.
go/pkg/zen/middleware_metrics_test.go (1)
9-249: Solid, table-driven coverage; good edge cases and idempotence.Covers whitespace, escapes, malformed JSON, binary data, and idempotence. Nicely aligned with runtime behavior.
Also applies to: 252-276, 289-338
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
go/pkg/zen/middleware_metrics.go (1)
20-23: Rename field to avoid package shadowing; add brief AIDEV note.
- Rename
regexptoreto avoid confusion with the imported package.- Add a short AIDEV note documenting intent and deterministic ordering (slice).
type redactionRule struct { - regexp *regexp.Regexp - replacement []byte + re *regexp.Regexp + replacement []byte } var redactionRules = []redactionRule{ - // Redact "key" field values - matches JSON-style key fields with various whitespace combinations + // AIDEV-NOTE: RE2 patterns applied in order; using a slice ensures deterministic redaction. + // We match JSON string values without parsing JSON to handle malformed inputs robustly. + // Redact "key" field values - matches JSON-style key fields with various whitespace combinations { - regexp: regexp.MustCompile(`"key"\s*:\s*"[^"\\]*(?:\\.[^"\\]*)*"`), + re: regexp.MustCompile(`"key"\s*:\s*"[^"\\]*(?:\\.[^"\\]*)*"`), replacement: []byte(`"key": "[REDACTED]"`), }, // Redact "plaintext" field values - matches JSON-style plaintext fields with various whitespace combinations { - regexp: regexp.MustCompile(`"plaintext"\s*:\s*"[^"\\]*(?:\\.[^"\\]*)*"`), + re: regexp.MustCompile(`"plaintext"\s*:\s*"[^"\\]*(?:\\.[^"\\]*)*"`), replacement: []byte(`"plaintext": "[REDACTED]"`), }, } func redact(in []byte) []byte { b := in for _, rule := range redactionRules { - b = rule.regexp.ReplaceAll(b, rule.replacement) + b = rule.re.ReplaceAll(b, rule.replacement) } return b }Also applies to: 28-29, 33-34, 41-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
go/pkg/zen/middleware_metrics.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/pkg/zen/middleware_metrics.go
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/pkg/zen/middleware_metrics.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.283Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
Learnt from: chronark
PR: unkeyed/unkey#3860
File: go/pkg/zen/middleware_metrics_test.go:297-313
Timestamp: 2025-08-27T11:02:17.134Z
Learning: The redact function in go/pkg/zen/middleware_metrics.go is not expected to be called concurrently, so concurrency tests are unnecessary for this function.
📚 Learning: 2025-08-27T11:02:17.134Z
Learnt from: chronark
PR: unkeyed/unkey#3860
File: go/pkg/zen/middleware_metrics_test.go:297-313
Timestamp: 2025-08-27T11:02:17.134Z
Learning: The redact function in go/pkg/zen/middleware_metrics.go is not expected to be called concurrently, so concurrency tests are unnecessary for this function.
Applied to files:
go/pkg/zen/middleware_metrics.go
📚 Learning: 2025-01-31T13:50:45.004Z
Learnt from: chronark
PR: unkeyed/unkey#2825
File: apps/dashboard/app/(app)/logs/components/table/logs-table.tsx:100-118
Timestamp: 2025-01-31T13:50:45.004Z
Learning: Sensitive data in logs is masked at the API layer (apps/api/src/pkg/middleware/metrics.ts) before being written to Clickhouse. This includes redacting API keys, plaintext values, and authorization headers using regex replacements.
Applied to files:
go/pkg/zen/middleware_metrics.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
go/pkg/zen/middleware_metrics.go (1)
38-46: LGTM: deterministic, idempotent redaction loop.Simple pass applying precompiled rules in order; matches the non-concurrent usage expectation.
|
👀 |
|
wanna give it a review? @ogzhanolguncu :bugeyes: |
|
Will review it in 10 🫡 🫡 |
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (08/27/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
What does this PR do?
Updates our regex and adds some test to make sure we redact any key and not just the ones that fit our schema.
Type of change
How should this be tested?
Manual testing using verifyKey and changing up the key json structure
Running the tests
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests