feat(guardrails): support outbound header modification#535
feat(guardrails): support outbound header modification#535SantiagoDePolonia wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds ChangesHeader modification guardrails
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![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
🤖 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 `@cmd/gomodel/docs/docs.go`:
- Line 6279: Update the source annotation that generates the RequestRevisions
description to describe these as request-processing changes rather than ingress
changes, while preserving the existing explanation of body rewrites and outbound
header modifications. Then regenerate the Swagger output so the generated
documentation matches the updated annotation.
In `@internal/admin/dashboard/static/css/dashboard.css`:
- Around line 5272-5278: Add a .guardrail-action-row modifier next to
.guardrail-header-row in internal/admin/dashboard/static/css/dashboard.css,
swapping the first two grid column sizing rules for the action-row DOM order.
Apply this modifier alongside the existing class on the action row container in
internal/admin/dashboard/templates/page-guardrails.html at line 152.
In `@internal/server/header_modification.go`:
- Around line 9-10: Replace the incorrect gomodel import prefix with
github.com/enterpilot/gomodel in internal/server/header_modification.go (lines
9-10) and internal/server/header_modification_test.go (lines 12-13). Make no
direct change to internal/workflows/service.go (lines 344-353); after updating
the imports, check internal/guardrails/header_modification.go for any remaining
gomodel/internal imports and re-run the build.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 57d344be-bca8-4dbd-86a6-9cdd2518bc12
📒 Files selected for processing (32)
CLAUDE.mdcmd/gomodel/docs/docs.goconfig/config.example.yamlconfig/guardrails.godocs/advanced/guardrails.mdxdocs/openapi.jsoninternal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/modules/audit-list.jsinternal/admin/dashboard/static/js/modules/audit-list.test.cjsinternal/admin/dashboard/static/js/modules/guardrails.jsinternal/admin/dashboard/static/js/modules/guardrails.test.cjsinternal/admin/dashboard/templates/page-guardrails.htmlinternal/app/app.gointernal/app/app_test.gointernal/auditlog/auditlog.gointernal/core/header_mutation.gointernal/core/header_mutation_test.gointernal/guardrails/definitions.gointernal/guardrails/header_modification.gointernal/guardrails/header_modification_test.gointernal/llmclient/client.gointernal/llmclient/client_test.gointernal/responsecache/semantic.gointernal/responsecache/semantic_test.gointernal/server/header_modification.gointernal/server/header_modification_test.gointernal/server/http.gointernal/server/internal_chat_completion_executor.gointernal/server/internal_chat_completion_executor_test.gointernal/server/passthrough_support.gointernal/server/realtime_service_test.gointernal/workflows/service.go
| }, | ||
| "request_revisions": { | ||
| "description": "RequestRevisions captures the ingress request-rewrite chain: one entry\nper registered rewriter that changed the body, in application order.\nRequestBody always remains the original client request; the last\nrevision is what was forwarded downstream.", | ||
| "description": "RequestRevisions captures intended ingress changes in application order:\nbody rewrites and outbound header modifications. RequestBody always\nremains the original client request. A revision records what would be\napplied if execution reaches provider egress; it is not proof of egress.", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Describe these as request-processing changes, not ingress changes.
This description groups outbound provider-header mutations under “ingress changes,” which is directionally misleading and can confuse audit consumers. Update the generator/source annotation and regenerate the Swagger file.
Proposed wording
- "description": "RequestRevisions captures intended ingress changes in application order:\nbody rewrites and outbound header modifications. RequestBody always\nremains the original client request. A revision records what would be\napplied if execution reaches provider egress; it is not proof of egress.",
+ "description": "RequestRevisions captures intended request-processing changes in application order:\nbody rewrites and outbound header modifications. RequestBody always\nremains the original client request. A revision records what would be\napplied if execution reaches provider egress; it is not proof of egress.",📝 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.
| "description": "RequestRevisions captures intended ingress changes in application order:\nbody rewrites and outbound header modifications. RequestBody always\nremains the original client request. A revision records what would be\napplied if execution reaches provider egress; it is not proof of egress.", | |
| "description": "RequestRevisions captures intended request-processing changes in application order:\nbody rewrites and outbound header modifications. RequestBody always\nremains the original client request. A revision records what would be\napplied if execution reaches provider egress; it is not proof of egress.", |
🤖 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 `@cmd/gomodel/docs/docs.go` at line 6279, Update the source annotation that
generates the RequestRevisions description to describe these as
request-processing changes rather than ingress changes, while preserving the
existing explanation of body rewrites and outbound header modifications. Then
regenerate the Swagger output so the generated documentation matches the updated
annotation.
| .guardrail-header-row { | ||
| display: grid; | ||
| grid-template-columns: minmax(0, 1.1fr) minmax(120px, auto) minmax(0, 1fr) auto; | ||
| gap: 8px; | ||
| align-items: center; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix visual layout glitch on action rows.
The CSS grid defined in .guardrail-header-row is designed for the DOM order of the condition row ([Header input] [Select] [Value input]). Because the action row has a different element order ([Select] [Header input] [Value input]), reusing the same unmodified class causes the Select dropdown to stretch excessively (1.1fr) while the Header input becomes rigidly constrained (minmax(120px, auto)).
internal/admin/dashboard/static/css/dashboard.css#L5272-L5278: Add a.guardrail-action-rowmodifier class with swapped column sizes.internal/admin/dashboard/templates/page-guardrails.html#L152-L152: Apply the modifier class to the action row container.
✨ Proposed CSS and HTML fix
In internal/admin/dashboard/static/css/dashboard.css:
.guardrail-header-row {
display: grid;
grid-template-columns: minmax(0, 1.1fr) minmax(120px, auto) minmax(0, 1fr) auto;
gap: 8px;
align-items: center;
width: 100%;
}
+
+.guardrail-action-row {
+ grid-template-columns: minmax(120px, auto) minmax(0, 1.1fr) minmax(0, 1fr) auto;
+}In internal/admin/dashboard/templates/page-guardrails.html:
<template x-if="field.input === 'header_actions'">
<div class="guardrail-header-rows">
<template x-for="(row, index) in guardrailHeaderRows(field)" :key="field.key + '-' + index">
- <div class="guardrail-header-row">
+ <div class="guardrail-header-row guardrail-action-row">
<select class="form-select settings-select" :aria-label="'Action ' + (index + 1) + ' kind'" `@change`="setGuardrailActionMode(field, index, $event.target.value)">📝 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.
| .guardrail-header-row { | |
| display: grid; | |
| grid-template-columns: minmax(0, 1.1fr) minmax(120px, auto) minmax(0, 1fr) auto; | |
| gap: 8px; | |
| align-items: center; | |
| width: 100%; | |
| } | |
| .guardrail-header-row { | |
| display: grid; | |
| grid-template-columns: minmax(0, 1.1fr) minmax(120px, auto) minmax(0, 1fr) auto; | |
| gap: 8px; | |
| align-items: center; | |
| width: 100%; | |
| } | |
| .guardrail-action-row { | |
| grid-template-columns: minmax(120px, auto) minmax(0, 1.1fr) minmax(0, 1fr) auto; | |
| } |
📍 Affects 2 files
internal/admin/dashboard/static/css/dashboard.css#L5272-L5278(this comment)internal/admin/dashboard/templates/page-guardrails.html#L152-L152
🤖 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/admin/dashboard/static/css/dashboard.css` around lines 5272 - 5278,
Add a .guardrail-action-row modifier next to .guardrail-header-row in
internal/admin/dashboard/static/css/dashboard.css, swapping the first two grid
column sizing rules for the action-row DOM order. Apply this modifier alongside
the existing class on the action row container in
internal/admin/dashboard/templates/page-guardrails.html at line 152.
| "gomodel/internal/auditlog" | ||
| "gomodel/internal/core" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Wrong module import prefix (gomodel/...) breaks the build across the header-modification feature. CI directly fails on the two files that import "gomodel/internal/auditlog" / "gomodel/internal/core"; the real module path is github.com/enterpilot/gomodel (per the Docker image enterpilot/gomodel, the ENTERPILOT/GoModel repo, and the github.com/enterpilot/gomodel/internal/core reference in the workflows/service.go build error). The workflows/service.go failure looks like a downstream symptom of the same wrong-prefix import existing in a dependency (most likely internal/guardrails/header_modification.go, which defines Pipeline.HeaderMutators() and is not included in this review batch).
internal/server/header_modification.go#L9-L10: change"gomodel/internal/auditlog"and"gomodel/internal/core"to"github.com/enterpilot/gomodel/internal/auditlog"and"github.com/enterpilot/gomodel/internal/core".internal/server/header_modification_test.go#L12-L13: apply the same import-path fix.internal/workflows/service.go#L344-L353: no code change needed here; re-run the build after fixing the imports above and additionally checkinternal/guardrails/header_modification.go's imports for the samegomodel/...prefix, since that is the most likely source of the[]invalid typemismatch reported at this site.
🐛 Verify remaining occurrences of the wrong import prefix
#!/bin/bash
rg -n '"gomodel/internal' --type=go
rg -n '"github.com/enterpilot/gomodel/internal' --type=go | head -5
cat go.mod | head -3🧰 Tools
🪛 GitHub Check: Build
[failure] 9-9:
package gomodel/internal/auditlog is not in std (/opt/hostedtoolcache/go/1.26.5/x64/src/gomodel/internal/auditlog)
🪛 GitHub Check: Vulnerability Scan
[failure] 10-10:
could not import gomodel/internal/core (invalid package name: "")
[failure] 9-9:
could not import gomodel/internal/auditlog (invalid package name: "")
[failure] 9-9:
package gomodel/internal/auditlog is not in std (/opt/hostedtoolcache/go/1.26.5/x64/src/gomodel/internal/auditlog)
📍 Affects 3 files
internal/server/header_modification.go#L9-L10(this comment)internal/server/header_modification_test.go#L12-L13internal/workflows/service.go#L344-L353
🤖 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/server/header_modification.go` around lines 9 - 10, Replace the
incorrect gomodel import prefix with github.com/enterpilot/gomodel in
internal/server/header_modification.go (lines 9-10) and
internal/server/header_modification_test.go (lines 12-13). Make no direct change
to internal/workflows/service.go (lines 344-353); after updating the imports,
check internal/guardrails/header_modification.go for any remaining
gomodel/internal imports and re-run the build.
Source: Linters/SAST tools
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Audit & Documentation