chore: security updates#324
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR restricts GitHub Actions workflow token permissions, adds a SECURITY.md policy, hardens secret redaction in logger/sanitize.go (SHA-256 hashing, stricter URL gating, first-delimiter splitting) applied through Tracef and HTTP logging paths, refactors HTTP tests to use a local TLS server, and bumps Go module dependencies. ChangesSecret redaction hardening
HTTP test infrastructure improvements
CI workflow permissions and security policy
Go module dependency updates
Compact metadata: 16 files changed across workflows, security policy, HTTP client/logging, logger sanitization, and Go module dependencies. Related issues: None linked in the provided information. Related PRs: None linked in the provided information. Suggested labels: security, dependencies, ci Suggested reviewers: None specified. 🐰 A rabbit hops through logs so neat, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
Gavel summary
Totals: 418 passed · 5 failed · 4 skipped · 16.1s Failing testsgithub.com/flanksource/commons/http — TestToCurl/auth_headers_are_included_unredactedgithub.com/flanksource/commons/http — TestToCurlgithub.com/flanksource/commons/logger — TestSanitizegithub.com/flanksource/commons/logger — TestSanitize/Leave_non-sensitive_headers_intactFailing lintersgolangci-lint — error |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@logger/sanitize.go`:
- Around line 132-134: The URL fast-path in sanitize logic is only redacting
userinfo and still leaves sensitive query parameters exposed, so update the
handling in the URL parsing branch to redact or sanitize RawQuery before
returning. Use the sanitize path around url.Parse/text checks in sanitize.go,
and ensure the fast-path applies the same sensitive-key protection as the
IsSensitiveKey loop so absolute URLs with secrets in query strings are not
logged verbatim.
🪄 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: e1ddf3ab-fc92-4d1c-9063-1190a6349768
⛔ Files ignored due to path filters (2)
cmd/hx/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
.github/workflows/hx-test.yml.github/workflows/lint.yml.github/workflows/release.yml.github/workflows/test.ymlSECURITY.mdcmd/hx/go.modgo.modhttp/client.gohttp/curl.gohttp/digest.gohttp/middlewares/logger.gologger/default.gologger/sanitize.gologger/sanitize_fuzz_test.go
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
logger/sanitize_test.go (1)
88-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew test correctly validates URL query redaction.
Verified against
StripSecrets's URL fast-path (query key redaction +uri.Redacted()password masking): the assertions correctly hold givenPrintableSecret's output format for a 12-char secret and Go's URL redaction behavior.Minor stylistic note: assertions use substring
Containschecks rather than an exact-match comparison (likecmp.Diffused inTestSanitizeabove). This is sufficient to catch the leak scenario being tested, but an exact expected-string comparison would be a more precise regression guard against future changes to query encoding order or redaction format.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@logger/sanitize_test.go` around lines 88 - 102, The new TestStripSecretsURLQuery in StripSecrets should use an exact expected-string assertion instead of only Contains checks so it more precisely guards the URL redaction behavior. Update the test to compare the full sanitized output from StripSecrets against the expected redacted URL, while still validating the query-key redaction and password masking behavior exercised by the URL fast-path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@logger/sanitize_test.go`:
- Around line 88-102: The new TestStripSecretsURLQuery in StripSecrets should
use an exact expected-string assertion instead of only Contains checks so it
more precisely guards the URL redaction behavior. Update the test to compare the
full sanitized output from StripSecrets against the expected redacted URL, while
still validating the query-key redaction and password masking behavior exercised
by the URL fast-path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed0e3540-1977-4752-8c16-fed0b8068b89
⛔ Files ignored due to path filters (1)
cmd/hx/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
cmd/hx/go.modhttp/http_test.gologger/sanitize.gologger/sanitize_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- logger/sanitize.go
- cmd/hx/go.mod
2 participants
Summary by CodeRabbit
Bug Fixes
Documentation
Tests
Chores