π‘οΈ Sentinel: [MEDIUM] Fix error message leakage - #241
Conversation
|
π Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a π emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Caution Review failedThe pull request is closed. βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: π Files selected for processing (2)
π WalkthroughSummary by CodeRabbit
WalkthroughChangesHeader validation responses
Estimated code review effort: 2 (Simple) | ~10 minutes π₯ Pre-merge checks | β 8β Passed checks (8 passed)
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 |
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 `@internal/refbackend/openaicodex/server.go`:
- Line 139: Add regression coverage for the generic validation error contract in
both the HTTP endpoint and WebSocket upgrade path: assert the response status is
400 and the exact response body is βinvalid headersβ. Update the existing tests
covering these two request flows, ensuring internal validation details remain
hidden.
πͺ Autofix (Beta)
β Autofix completed
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 65e3f424-c46c-4c02-b98c-84244a6a2a18
π Files selected for processing (1)
internal/refbackend/openaicodex/server.go
π Review details
β° Context from checks skipped due to timeout. (5)
- GitHub Check: Analyze (Go)
- GitHub Check: qa
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Test (macos-latest)
- GitHub Check: Test (windows-latest)
π§° Additional context used
π Path-based instructions (4)
**/*.go
π CodeRabbit inference engine (Custom checks)
**/*.go: For server, CLI, worker, or network Go code, ensurecontext.Contextis propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely.
Do not make accidental public API breaks in Go code: underpkg/**or anywhere exported Go identifiers are changed, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact.
**/*.go: Implement and describe Go behavior only; explicitly label Python-era or future behavior.
The core owns orchestration, routing, failover, and B2BUA continuity; provider semantics remain in adapters/plugins.
Core code must not import provider SDKs or concrete plugins.
Use canonical protocol adapters instead of pairwise protocol translators.
Streaming is primary; non-streaming APIs must collect the canonical stream.
Do not transparently retry or fail over after the first downstream content event.
Fail explicitly on capability mismatches; never silently drop required semantics.
Place request and response mutation behind hooks or extensions, not core branching.
Use explicit construction and registration; do not use DI containers, reflection registries, globals, or Go nativepluginin v1.
Use specs for new features, breaking or architecture changes, protocol/plugin contract changes, routing semantic changes, or unclear requirements; use direct code for small fixes, documentation, narrow tests, and trivial maintenance.
If an active Kiro spec is in scope, do not code before approvedrequirements.mdanddesign.mdare recorded inspec.json.
Prefer the standard library and add dependencies only when they reduce complexity or risk.
Define small interfaces where consumed; constructors should return concrete types unless exposing stable SDK or plugin contracts.
Every I/O boundary must takecontext.Context; never store contexts in structs.
Own goroutines, channels, and cancellation ex...
Files:
internal/refbackend/openaicodex/server.go
βοΈ CodeRabbit configuration file
**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.
Files:
internal/refbackend/openaicodex/server.go
**/*
π CodeRabbit inference engine (Custom checks)
Do not introduce hardcoded credentials, API keys, tokens, private keys, passwords, production secrets, or sensitive internal URLs.
**/*: Do not add changelog,_Updated,_Reason, timestamp, or history footers to steering or agent instructions.
Use TDD by default: write the test or interface before the implementation.
Prefer the smallest correct diff and avoid speculative abstractions.
Never claim success without direct verification evidence.
Preserve user-authored changes and never use destructive git commands unless explicitly requested.
Do not work onmain; create a suitably named local worktree and fix/spec/feat branch based onmainfor each change.
Use forward-slash git pathspecs on Windows.
Reports must state changed files and verification run, and plainly state skipped tests or uncertainty; mention architectural trade-offs only when relevant.
Files:
internal/refbackend/openaicodex/server.go
internal/refbackend/**/*.go
π CodeRabbit inference engine (AGENTS.md)
Keep reference backends, clients, stubs, fixtures, architecture tests, and QA helpers test-only.
Files:
internal/refbackend/openaicodex/server.go
internal/**
βοΈ CodeRabbit configuration file
internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.
Files:
internal/refbackend/openaicodex/server.go
π§ Learnings (1)
π Learning: 2026-07-01T22:57:42.953Z
Learnt from: matdev83
Repo: matdev83/go-llm-interactive-proxy PR: 101
File: pkg/lipsdk/scope/context.go:0-0
Timestamp: 2026-07-01T22:57:42.953Z
Learning: In this repository, when defining unexported Go `context` key constants of type `ctxKey int` (e.g., `const ( kFoo ctxKey = iota + N )`), preserve the `iota + <offset>` pattern and keep the existing `<offset>` values rather than simplifying to plain `iota`. These per-package offsets are part of the repo-wide convention to avoid key collisions across packages, and each such constant set should include a short explanatory comment (for example: βoffset avoids collision with other packages' context keysβ).
Applied to files:
internal/refbackend/openaicodex/server.go
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
π¨ Severity: MEDIUM π‘ Vulnerability: Exposed raw internal error messages in HTTP responses π― Impact: Could leak internal implementation details or unexpected states π§ Fix: Replaced err.Error() with a generic error message β Verification: Ran the existing test suite Co-authored-by: matdev83 <211248003+matdev83@users.noreply.github.com>
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
bed299d to
cf9e5a1
Compare
π¨ Severity: MEDIUM
π‘ Vulnerability: Exposed raw internal error messages in HTTP responses
π― Impact: Could leak internal implementation details or unexpected states
π§ Fix: Replaced err.Error() with a generic error message
β Verification: Ran the existing test suite
PR created automatically by Jules for task 17523149817371763483 started by @matdev83