feat: params override log - #3285
Merged
Calcium-Ion merged 3 commits intoMar 17, 2026
Merged
Conversation
Contributor
WalkthroughThis pull request adds comprehensive auditing for parameter override operations. The backend captures audit trails for each override operation (set, delete, move, copy, transform, etc.) through a new internal audit recorder mechanism, stores them in RelayInfo.ParamOverrideAudit, and surfaces them in logs. The frontend provides UI components to display and interact with these audit records through a modal interface. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/Request
participant Override as ApplyParamOverride
participant Recorder as Audit Recorder
participant Ops as applyOperations
participant Info as RelayInfo
participant Logger as Log Generator
participant UI as Frontend Modal
Client->>Override: ApplyParamOverrideWithRelayInfo(jsonData, paramOverride)
Override->>Override: Check shouldEnableParamOverrideAudit (debug flag)
alt Debug Mode Enabled
Override->>Recorder: Initialize audit recorder
Override->>Ops: Pass context with recorder
Ops->>Ops: Execute operations (set, copy, move, etc.)
Ops->>Recorder: recordOperation(action, key, value)
Recorder->>Recorder: buildParamOverrideAuditLine()
end
Override->>Info: Attach audit lines to ParamOverrideAudit
Override-->>Client: Return modified JSON + RelayInfo
Client->>Logger: GenerateTextOtherInfo(RelayInfo)
Logger->>Logger: appendParamOverrideInfo()
Logger-->>Client: Return log with "po" field
Client->>UI: Display logs
UI->>UI: User clicks override entry
UI->>UI: parseAuditLine(line) → action + content
UI->>UI: Render ParamOverrideModal with parsed lines
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
web/src/components/table/usage-logs/modals/ParamOverrideModal.jsx (1)
98-103: PreferuseTranslation()locally here too.This modal is another leaf UI component, so pulling
twithuseTranslation()keeps its prop surface smaller and matches the repo’s i18n convention.As per coding guidelines, 'Use
useTranslation()hook and callt('中文key')in components.'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/usage-logs/modals/ParamOverrideModal.jsx` around lines 98 - 103, ParamOverrideModal currently accepts a translation function t as a prop; change it to use the useTranslation() hook inside the component instead: remove t from the ParamOverrideModal parameter list and any callers should stop passing t, import and call useTranslation() inside the ParamOverrideModal implementation to get t, and update any usages of t(...) within the component to use the locally obtained t; ensure PropTypes/TS types and any tests or parent components are updated to stop providing the t prop.web/src/components/table/usage-logs/components/ParamOverrideEntry.jsx (1)
25-49: PreferuseTranslation()inside the component.Passing
tthrough props widens a tiny presentational API for no real gain and drifts from the repo’s i18n pattern.As per coding guidelines, 'Use
useTranslation()hook and callt('中文key')in components.'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/usage-logs/components/ParamOverrideEntry.jsx` around lines 25 - 49, ParamOverrideEntry currently accepts a translation function via the t prop which widens its API and violates the repo pattern; change it to call useTranslation() inside the component instead of receiving t as a prop: remove t from the ParamOverrideEntry signature and any parent-provided t usage, import and call useTranslation() within ParamOverrideEntry to obtain t, and update all internal t('...') calls to use that local t so the component becomes self-contained and conforms to the i18n guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/common/override.go`:
- Around line 198-225: The audit check in shouldEnableParamOverrideAudit only
inspects raw operation.Path and operation.To (and keys) so it misses source
paths and prefixed sync targets like "header:x-request-model" or "json:model";
update shouldEnableParamOverrideAudit (and the similar block around 261-270) to
also examine operation.From (or any source field returned by
tryParseOperations), and to normalize values with prefixes by stripping known
sync prefixes ("json:", "header:", "query:", etc.) before calling
shouldAuditParamPath; also ensure buildLegacyParamOverride key checks strip
those prefixes too so both legacy and parsed operation flows detect touched
tracked fields.
- Around line 178-194: ApplyParamOverride may append audit lines to recorder
even when it returns an error; move the logic that copies recorder.lines into
info.ParamOverrideAudit onto the error path so recorded lines are preserved
before returning. Concretely, after calling ApplyParamOverride(jsonData,
paramOverride, overrideCtx) check err and if err != nil and info != nil and
recorder != nil set info.ParamOverrideAudit = recorder.lines before returning
the error; keep the existing syncRuntimeHeaderOverrideFromContext(info,
overrideCtx) and the non-error path behavior for setting info.ParamOverrideAudit
as-is.
In `@service/log_info_generate.go`:
- Line 77: Redact ParamOverrideAudit contents before writing into other["po"]:
update appendParamOverrideInfo (and any call sites around lines calling it) so
it detects entries of type relay/common/override.ParamOverrideAudit and replaces
sensitive fields with a redacted placeholder (or strips raw values) before
serializing into relayInfo -> other["po"]; ensure the redaction happens inside
appendParamOverrideInfo so both the call at the shown location and the similar
calls at the 81-85 region persist only redacted data to Other (which is later
stored as a plain string and surfaced in the UI).
---
Nitpick comments:
In `@web/src/components/table/usage-logs/components/ParamOverrideEntry.jsx`:
- Around line 25-49: ParamOverrideEntry currently accepts a translation function
via the t prop which widens its API and violates the repo pattern; change it to
call useTranslation() inside the component instead of receiving t as a prop:
remove t from the ParamOverrideEntry signature and any parent-provided t usage,
import and call useTranslation() within ParamOverrideEntry to obtain t, and
update all internal t('...') calls to use that local t so the component becomes
self-contained and conforms to the i18n guideline.
In `@web/src/components/table/usage-logs/modals/ParamOverrideModal.jsx`:
- Around line 98-103: ParamOverrideModal currently accepts a translation
function t as a prop; change it to use the useTranslation() hook inside the
component instead: remove t from the ParamOverrideModal parameter list and any
callers should stop passing t, import and call useTranslation() inside the
ParamOverrideModal implementation to get t, and update any usages of t(...)
within the component to use the locally obtained t; ensure PropTypes/TS types
and any tests or parent components are updated to stop providing the t prop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 486ba1ee-fb88-4ad6-a05e-69f683ff0342
📒 Files selected for processing (8)
relay/common/override.gorelay/common/override_test.gorelay/common/relay_info.goservice/log_info_generate.goweb/src/components/table/usage-logs/components/ParamOverrideEntry.jsxweb/src/components/table/usage-logs/index.jsxweb/src/components/table/usage-logs/modals/ParamOverrideModal.jsxweb/src/hooks/usage-logs/useUsageLogsData.jsx
Comment on lines
+178
to
+194
| var recorder *paramOverrideAuditRecorder | ||
| if shouldEnableParamOverrideAudit(paramOverride) { | ||
| recorder = ¶mOverrideAuditRecorder{} | ||
| overrideCtx[paramOverrideContextAuditRecorder] = recorder | ||
| } | ||
| result, err := ApplyParamOverride(jsonData, paramOverride, overrideCtx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| syncRuntimeHeaderOverrideFromContext(info, overrideCtx) | ||
| if info != nil { | ||
| if recorder != nil { | ||
| info.ParamOverrideAudit = recorder.lines | ||
| } else { | ||
| info.ParamOverrideAudit = nil | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve recorded audit lines on the error path.
ApplyParamOverride can append to recorder before it returns an error (return_error is the obvious case). The early return here skips copying those lines back to info, so the failing request ends up with no param-override audit at all.
Suggested fix
result, err := ApplyParamOverride(jsonData, paramOverride, overrideCtx)
- if err != nil {
- return nil, err
- }
syncRuntimeHeaderOverrideFromContext(info, overrideCtx)
if info != nil {
if recorder != nil {
- info.ParamOverrideAudit = recorder.lines
+ info.ParamOverrideAudit = append([]string(nil), recorder.lines...)
} else {
info.ParamOverrideAudit = nil
}
}
+ if err != nil {
+ return nil, err
+ }
return result, nil📝 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.
Suggested change
| var recorder *paramOverrideAuditRecorder | |
| if shouldEnableParamOverrideAudit(paramOverride) { | |
| recorder = ¶mOverrideAuditRecorder{} | |
| overrideCtx[paramOverrideContextAuditRecorder] = recorder | |
| } | |
| result, err := ApplyParamOverride(jsonData, paramOverride, overrideCtx) | |
| if err != nil { | |
| return nil, err | |
| } | |
| syncRuntimeHeaderOverrideFromContext(info, overrideCtx) | |
| if info != nil { | |
| if recorder != nil { | |
| info.ParamOverrideAudit = recorder.lines | |
| } else { | |
| info.ParamOverrideAudit = nil | |
| } | |
| } | |
| var recorder *paramOverrideAuditRecorder | |
| if shouldEnableParamOverrideAudit(paramOverride) { | |
| recorder = ¶mOverrideAuditRecorder{} | |
| overrideCtx[paramOverrideContextAuditRecorder] = recorder | |
| } | |
| result, err := ApplyParamOverride(jsonData, paramOverride, overrideCtx) | |
| syncRuntimeHeaderOverrideFromContext(info, overrideCtx) | |
| if info != nil { | |
| if recorder != nil { | |
| info.ParamOverrideAudit = append([]string(nil), recorder.lines...) | |
| } else { | |
| info.ParamOverrideAudit = nil | |
| } | |
| } | |
| if err != nil { | |
| return nil, err | |
| } | |
| return result, nil |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/common/override.go` around lines 178 - 194, ApplyParamOverride may
append audit lines to recorder even when it returns an error; move the logic
that copies recorder.lines into info.ParamOverrideAudit onto the error path so
recorded lines are preserved before returning. Concretely, after calling
ApplyParamOverride(jsonData, paramOverride, overrideCtx) check err and if err !=
nil and info != nil and recorder != nil set info.ParamOverrideAudit =
recorder.lines before returning the error; keep the existing
syncRuntimeHeaderOverrideFromContext(info, overrideCtx) and the non-error path
behavior for setting info.ParamOverrideAudit as-is.
Comment on lines
+198
to
+225
| func shouldEnableParamOverrideAudit(paramOverride map[string]interface{}) bool { | ||
| if common.DebugEnabled { | ||
| return true | ||
| } | ||
| if len(paramOverride) == 0 { | ||
| return false | ||
| } | ||
| if operations, ok := tryParseOperations(paramOverride); ok { | ||
| for _, operation := range operations { | ||
| if shouldAuditParamPath(strings.TrimSpace(operation.Path)) || | ||
| shouldAuditParamPath(strings.TrimSpace(operation.To)) { | ||
| return true | ||
| } | ||
| } | ||
| for key := range buildLegacyParamOverride(paramOverride) { | ||
| if shouldAuditParamPath(strings.TrimSpace(key)) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
| for key := range paramOverride { | ||
| if shouldAuditParamPath(strings.TrimSpace(key)) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
Non-debug audit matching misses source paths and json: sync targets.
Both guards only inspect raw path/to strings. That means move model -> meta.backup and sync_fields header:x-request-model -> json:model won’t be audited in non-debug mode, even though a tracked field is being touched.
Also applies to: 261-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/common/override.go` around lines 198 - 225, The audit check in
shouldEnableParamOverrideAudit only inspects raw operation.Path and operation.To
(and keys) so it misses source paths and prefixed sync targets like
"header:x-request-model" or "json:model"; update shouldEnableParamOverrideAudit
(and the similar block around 261-270) to also examine operation.From (or any
source field returned by tryParseOperations), and to normalize values with
prefixes by stripping known sync prefixes ("json:", "header:", "query:", etc.)
before calling shouldAuditParamPath; also ensure buildLegacyParamOverride key
checks strip those prefixes too so both legacy and parsed operation flows detect
touched tracked fields.
| appendRequestPath(ctx, relayInfo, other) | ||
| appendRequestConversionChain(relayInfo, other) | ||
| appendBillingInfo(relayInfo, other) | ||
| appendParamOverrideInfo(relayInfo, other) |
Contributor
There was a problem hiding this comment.
Redact ParamOverrideAudit before writing it into other["po"].
relay/common/override.go can emit raw values for set/set_header, and this helper persists them verbatim. Since model/log.go:19-40 stores Other as a plain string and web/src/hooks/usage-logs/useUsageLogsData.jsx:604-618 surfaces po in the normal usage-log UI, debug-mode audit lines can turn sensitive override values into durable user-visible data.
Also applies to: 81-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/log_info_generate.go` at line 77, Redact ParamOverrideAudit contents
before writing into other["po"]: update appendParamOverrideInfo (and any call
sites around lines calling it) so it detects entries of type
relay/common/override.ParamOverrideAudit and replaces sensitive fields with a
redacted placeholder (or strips raw values) before serializing into relayInfo ->
other["po"]; ensure the redaction happens inside appendParamOverrideInfo so both
the call at the shown location and the similar calls at the 81-85 region persist
only redacted data to Other (which is later stored as a plain string and
surfaced in the UI).
This was referenced Apr 4, 2026
11 tasks
11 tasks
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.
Summary by CodeRabbit
Release Notes