fix: CxONE findings#178
Open
cb80 wants to merge 1 commit into
Open
Conversation
WalkthroughThe PR adds security hardening to the HTTP server by wrapping it with a middleware that applies Content-Security-Policy headers, and updates the linter configuration to disable the ChangesSecurity Configuration and Headers
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Signed-off-by: Chris Burkert <burkert.chris@gmail.com>
![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
🤖 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 @.golangci.yaml:
- Line 31: Re-enable gomodguard in .golangci.yaml (remove or undo the entry that
disables "gomodguard") unless you can add a clear, documented justification for
disabling it; if you choose to keep it disabled instead, add an alternative
dependency allow/block enforcement and document it in the repo (for example: a
COMPANY_DEPENDENCY_POLICY.md describing how dependencies are approved, or a CI
check that validates an allowlist) and reference the specific dependency usage
in internal/business/server/http_server.go (middleware.SecurityHeadersMiddleware
from github.com/openkcm/common-sdk) to show why the exception is needed. Ensure
the change either re-enables "gomodguard" or commits the justification and
alternative enforcement so supply-chain dependency validation is preserved.
In `@internal/business/server/http_server.go`:
- Around line 55-57: Add unit tests that exercise the
SecurityHeadersMiddleware-wrapped handler: create a server via
createHTTPServer(context.Background(), cfg), send an httptest request to an
existing endpoint (e.g., "/health") using server.Handler.ServeHTTP, and assert
that the "Content-Security-Policy" header exists and equals "default-src 'none';
frame-ancestors 'none'; base-uri 'none'; form-action 'none';"; also assert the
original handler behavior (status code/body) is unchanged to ensure middleware
does not interfere. Use a test file (e.g., http_server_test.go) and reference
middleware.SecurityHeadersMiddleware, createHTTPServer, and server.Handler in
the test.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed49b9d9-a64e-49ae-b8dc-53a5ba24f5c8
📒 Files selected for processing (2)
.golangci.yamlinternal/business/server/http_server.go
| - gocritic | ||
| - godot | ||
| - godox | ||
| - gomodguard |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for gomodguard configuration and verify the middleware package
# Look for gomodguard configuration files
echo "=== Searching for gomodguard configuration ==="
fd -t f -e yaml -e yml -e toml | xargs rg -l "gomodguard" 2>/dev/null || echo "No gomodguard config found"
# Check if common-sdk is referenced in any allow/block lists
echo -e "\n=== Checking for common-sdk in config files ==="
rg -i "common-sdk|openkcm" -g "*.{yaml,yml,toml}" -C 3
# Verify the middleware package is actually used
echo -e "\n=== Verifying middleware.SecurityHeadersMiddleware usage ==="
rg -n "SecurityHeadersMiddleware" --type go
# Check go.mod for the dependency version
echo -e "\n=== Checking go.mod for common-sdk version ==="
rg "github.com/openkcm/common-sdk" go.modRepository: openkcm/checker
Length of output: 2735
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== .golangci.yaml lines 1-80 ==="
nl -ba .golangci.yaml | sed -n '1,80p'
echo -e "\n=== Search for gomodguard in .golangci.yaml ==="
rg -n "gomodguard" .golangci.yaml
echo -e "\n=== Any other gomodguard config files (yaml/yml/toml) ==="
fd -t f -e yaml -e yml -e toml . | xargs -r rg -l "gomodguard" || true
echo -e "\n=== Confirm common-sdk + middleware usage ==="
rg -n "openkcm/common-sdk|pkg/middleware|SecurityHeadersMiddleware" --glob '*.go'
echo -e "\n=== go.mod entry for common-sdk ==="
rg -n "github.com/openkcm/common-sdk" go.modRepository: openkcm/checker
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== .golangci.yaml (first 120 lines with line numbers) ==="
cat -n .golangci.yaml | sed -n '1,120p'
echo -e "\n=== Search for gomodguard in .golangci.yaml ==="
rg -n "gomodguard" .golangci.yaml || true
echo -e "\n=== Any other gomodguard config files (yaml/yml/toml) ==="
fd -t f -e yaml -e yml -e toml . | xargs -r rg -l "gomodguard" || true
echo -e "\n=== Confirm common-sdk + middleware usage ==="
rg -n "openkcm/common-sdk|pkg/middleware|SecurityHeadersMiddleware" --glob '*.go' || true
echo -e "\n=== go.mod entry for common-sdk ==="
rg -n "github.com/openkcm/common-sdk" go.mod || trueRepository: openkcm/checker
Length of output: 4032
Re-enable (or justify disabling) gomodguard to keep dependency allow/block enforcement
.golangci.yaml disables gomodguard (linters.disable: - gomodguard), and no other repo gomodguard configuration was found—so allow/block dependency validation won’t run. This removes supply-chain controls for the github.com/openkcm/common-sdk dependency used via middleware.SecurityHeadersMiddleware in internal/business/server/http_server.go.
Provide a concrete justification for disabling gomodguard and/or add an alternative dependency enforcement mechanism; otherwise re-enable it.
🤖 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 @.golangci.yaml at line 31, Re-enable gomodguard in .golangci.yaml (remove or
undo the entry that disables "gomodguard") unless you can add a clear,
documented justification for disabling it; if you choose to keep it disabled
instead, add an alternative dependency allow/block enforcement and document it
in the repo (for example: a COMPANY_DEPENDENCY_POLICY.md describing how
dependencies are approved, or a CI check that validates an allowlist) and
reference the specific dependency usage in
internal/business/server/http_server.go (middleware.SecurityHeadersMiddleware
from github.com/openkcm/common-sdk) to show why the exception is needed. Ensure
the change either re-enables "gomodguard" or commits the justification and
alternative enforcement so supply-chain dependency validation is preserved.
Comment on lines
+55
to
+57
| Handler: middleware.SecurityHeadersMiddleware(mux, map[string]string{ | ||
| "Content-Security-Policy": "default-src 'none'; frame-ancestors 'none'; base-uri 'none'; form-action 'none';", | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add tests to verify security headers are applied correctly.
The security middleware integration lacks test coverage. Add tests to verify:
- The CSP header is present in all responses
- The header value matches the expected policy
- The middleware doesn't interfere with existing handler behavior
🧪 Example test structure
Add to your test file (e.g., http_server_test.go):
func TestSecurityHeadersApplied(t *testing.T) {
cfg := &config.Config{
Server: config.ServerConfig{Address: ":8080"},
Healthcheck: config.HealthcheckConfig{Enabled: true, Endpoint: "/health"},
}
server := createHTTPServer(context.Background(), cfg)
req := httptest.NewRequest("GET", "/health", nil)
w := httptest.NewRecorder()
server.Handler.ServeHTTP(w, req)
csp := w.Header().Get("Content-Security-Policy")
expectedCSP := "default-src 'none'; frame-ancestors 'none'; base-uri 'none'; form-action 'none';"
if csp != expectedCSP {
t.Errorf("Expected CSP %q, got %q", expectedCSP, csp)
}
}🤖 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 `@internal/business/server/http_server.go` around lines 55 - 57, Add unit tests
that exercise the SecurityHeadersMiddleware-wrapped handler: create a server via
createHTTPServer(context.Background(), cfg), send an httptest request to an
existing endpoint (e.g., "/health") using server.Handler.ServeHTTP, and assert
that the "Content-Security-Policy" header exists and equals "default-src 'none';
frame-ancestors 'none'; base-uri 'none'; form-action 'none';"; also assert the
original handler behavior (status code/body) is unchanged to ensure middleware
does not interfere. Use a test file (e.g., http_server_test.go) and reference
middleware.SecurityHeadersMiddleware, createHTTPServer, and server.Handler in
the test.
bkieling
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Special notes for your reviewer:
Release note:
Summary by CodeRabbit
Security Improvements
Chores