Commit 9f506dd7 by CaIon

refactor(logs): simplify LogOther projection and dedupe sensitive keys

- Drop the unreachable user-visibility branch in LogOther.toMap and the
  receiver-mutating normalizeLegacyAdminFields; JSONString/Snapshot/
  MarshalJSON now share one full serialization
- Define legacySensitiveLogOtherKeys once and reference it from both
  SetPublic rejection and the user read-side projection
- Return the original JSON for every role when formatLogOtherJSON
  removed nothing, avoiding a re-marshal on the user log list path
- Log rejected OtherRatios keys in taskBillingOther instead of dropping
  them silently
- Use Snapshot() with typed assertions in service tests
parent 219c9e06
...@@ -229,4 +229,16 @@ func TestLogFormattingPreservesLargeIntegerLexemes(t *testing.T) { ...@@ -229,4 +229,16 @@ func TestLogFormattingPreservesLargeIntegerLexemes(t *testing.T) {
assert.Equal(t, other, logs[0].Other) assert.Equal(t, other, logs[0].Other)
}) })
t.Run("unprivileged", func(t *testing.T) {
const unprivileged = `{"public_id":9007199254740993,"model_price":0.004}`
userLogs := []*Log{{Other: unprivileged}}
formatUserLogs(userLogs, 0)
assert.Equal(t, unprivileged, userLogs[0].Other)
adminLogs := []*Log{{Other: unprivileged}}
FormatAdminLogs(adminLogs)
assert.Equal(t, unprivileged, adminLogs[0].Other)
})
} }
...@@ -3,6 +3,7 @@ package model ...@@ -3,6 +3,7 @@ package model
import ( import (
"encoding/json" "encoding/json"
"maps" "maps"
"slices"
"github.com/QuantumNous/new-api/common" "github.com/QuantumNous/new-api/common"
) )
...@@ -13,6 +14,15 @@ const ( ...@@ -13,6 +14,15 @@ const (
logOtherAuditInfoKey = "audit_info" logOtherAuditInfoKey = "audit_info"
) )
// legacySensitiveLogOtherKeys are historical top-level fields that must never
// be written via SetPublic and must be stripped from user-visible projections.
var legacySensitiveLogOtherKeys = []string{
"channel_id",
"channel_name",
"channel_type",
"reject_reason",
}
type logOtherVisibility int type logOtherVisibility int
const ( const (
...@@ -37,11 +47,10 @@ func NewLogOther() *LogOther { ...@@ -37,11 +47,10 @@ func NewLogOther() *LogOther {
func isReservedLogOtherKey(key string) bool { func isReservedLogOtherKey(key string) bool {
switch key { switch key {
case logOtherAdminInfoKey, logOtherRootInfoKey, logOtherAuditInfoKey, case logOtherAdminInfoKey, logOtherRootInfoKey, logOtherAuditInfoKey:
"channel_id", "channel_name", "channel_type", "reject_reason":
return true return true
default: default:
return false return slices.Contains(legacySensitiveLogOtherKeys, key)
} }
} }
...@@ -125,55 +134,32 @@ func copyLogOtherMap(values map[string]any) map[string]any { ...@@ -125,55 +134,32 @@ func copyLogOtherMap(values map[string]any) map[string]any {
return copyValues return copyValues
} }
func (o *LogOther) normalizeLegacyAdminFields() { func (o *LogOther) toMap() map[string]any {
if o == nil || o.public == nil {
return
}
if rejectReason, ok := o.public["reject_reason"]; ok {
if _, exists := o.adminInfo["reject_reason"]; !exists {
o.SetAdmin("reject_reason", rejectReason)
}
delete(o.public, "reject_reason")
}
}
func (o *LogOther) toMap(visibility logOtherVisibility) map[string]any {
result := make(map[string]any) result := make(map[string]any)
if o == nil { if o == nil {
return result return result
} }
for key, value := range o.public { for key, value := range o.public {
if visibility == logOtherVisibilityUser {
switch key {
case "channel_id", "channel_name", "channel_type", "reject_reason":
continue
}
}
result[key] = value result[key] = value
} }
if visibility >= logOtherVisibilityAdmin { if adminInfo := copyLogOtherMap(o.adminInfo); len(adminInfo) > 0 {
if adminInfo := copyLogOtherMap(o.adminInfo); len(adminInfo) > 0 { result[logOtherAdminInfoKey] = adminInfo
result[logOtherAdminInfoKey] = adminInfo
}
if auditInfo := copyLogOtherMap(o.auditInfo); len(auditInfo) > 0 {
result[logOtherAuditInfoKey] = auditInfo
}
} }
if visibility == logOtherVisibilityRoot { if auditInfo := copyLogOtherMap(o.auditInfo); len(auditInfo) > 0 {
if rootInfo := copyLogOtherMap(o.rootInfo); len(rootInfo) > 0 { result[logOtherAuditInfoKey] = auditInfo
result[logOtherRootInfoKey] = rootInfo }
} if rootInfo := copyLogOtherMap(o.rootInfo); len(rootInfo) > 0 {
result[logOtherRootInfoKey] = rootInfo
} }
return result return result
} }
func (o *LogOther) jsonString(visibility logOtherVisibility) string { func (o *LogOther) jsonString() string {
if o == nil { if o == nil {
return "" return ""
} }
o.normalizeLegacyAdminFields() data, err := common.Marshal(o.toMap())
data, err := common.Marshal(o.toMap(visibility))
if err != nil { if err != nil {
common.SysError("failed to marshal log other: " + err.Error()) common.SysError("failed to marshal log other: " + err.Error())
return "" return ""
...@@ -184,7 +170,7 @@ func (o *LogOther) jsonString(visibility logOtherVisibility) string { ...@@ -184,7 +170,7 @@ func (o *LogOther) jsonString(visibility logOtherVisibility) string {
// JSONString returns the complete stored representation, including all // JSONString returns the complete stored representation, including all
// privileged scopes. API projections must use the role-specific formatter. // privileged scopes. API projections must use the role-specific formatter.
func (o *LogOther) JSONString() string { func (o *LogOther) JSONString() string {
return o.jsonString(logOtherVisibilityRoot) return o.jsonString()
} }
// Snapshot returns a detached top-level view for tests and read-only // Snapshot returns a detached top-level view for tests and read-only
...@@ -193,11 +179,11 @@ func (o *LogOther) Snapshot() map[string]any { ...@@ -193,11 +179,11 @@ func (o *LogOther) Snapshot() map[string]any {
if o == nil { if o == nil {
return nil return nil
} }
return o.toMap(logOtherVisibilityRoot) return o.toMap()
} }
func (o *LogOther) MarshalJSON() ([]byte, error) { func (o *LogOther) MarshalJSON() ([]byte, error) {
return common.Marshal(o.toMap(logOtherVisibilityRoot)) return common.Marshal(o.toMap())
} }
func normalizeLegacyRejectReason(values map[string]json.RawMessage) bool { func normalizeLegacyRejectReason(values map[string]json.RawMessage) bool {
...@@ -243,15 +229,13 @@ func formatLogOtherJSON(value string, visibility logOtherVisibility) string { ...@@ -243,15 +229,13 @@ func formatLogOtherJSON(value string, visibility logOtherVisibility) string {
changed := false changed := false
if visibility == logOtherVisibilityUser { if visibility == logOtherVisibilityUser {
for _, key := range []string{ for _, key := range []string{logOtherAdminInfoKey, logOtherRootInfoKey, logOtherAuditInfoKey} {
logOtherAdminInfoKey, if _, exists := values[key]; exists {
logOtherRootInfoKey, delete(values, key)
logOtherAuditInfoKey, changed = true
"channel_id", }
"channel_name", }
"channel_type", for _, key := range legacySensitiveLogOtherKeys {
"reject_reason",
} {
if _, exists := values[key]; exists { if _, exists := values[key]; exists {
delete(values, key) delete(values, key)
changed = true changed = true
...@@ -267,7 +251,7 @@ func formatLogOtherJSON(value string, visibility logOtherVisibility) string { ...@@ -267,7 +251,7 @@ func formatLogOtherJSON(value string, visibility logOtherVisibility) string {
} }
} }
if visibility == logOtherVisibilityRoot && !changed { if !changed {
return value return value
} }
formatted, err := common.Marshal(values) formatted, err := common.Marshal(values)
......
...@@ -68,3 +68,17 @@ func TestLogOtherRejectsSensitivePublicFields(t *testing.T) { ...@@ -68,3 +68,17 @@ func TestLogOtherRejectsSensitivePublicFields(t *testing.T) {
require.JSONEq(t, `{"request_path":"/v1/responses"}`, other.JSONString()) require.JSONEq(t, `{"request_path":"/v1/responses"}`, other.JSONString())
require.JSONEq(t, `{}`, NewLogOther().JSONString()) require.JSONEq(t, `{}`, NewLogOther().JSONString())
} }
func TestLogOtherJSONStringDoesNotMutateReceiver(t *testing.T) {
other := NewLogOther()
require.True(t, other.SetPublic("request_path", "/v1/chat/completions"))
require.True(t, other.SetAdmin("rejected", false))
before := other.Snapshot()
first := other.JSONString()
after := other.Snapshot()
second := other.JSONString()
require.Equal(t, before, after)
require.Equal(t, first, second)
}
...@@ -144,7 +144,9 @@ func taskBillingOther(task *model.Task) *model.LogOther { ...@@ -144,7 +144,9 @@ func taskBillingOther(task *model.Task) *model.LogOther {
other.SetPublic("group_ratio", bc.GroupRatio) other.SetPublic("group_ratio", bc.GroupRatio)
if priceData := taskBillingContextPriceData(bc); priceData != nil { if priceData := taskBillingContextPriceData(bc); priceData != nil {
for k, v := range priceData.OtherRatios() { for k, v := range priceData.OtherRatios() {
other.SetPublic(k, v) if !other.SetPublic(k, v) {
common.SysError("task billing other ratio key rejected: " + k)
}
} }
} }
if snap := bc.TieredSnapshot; snap != nil { if snap := bc.TieredSnapshot; snap != nil {
......
...@@ -163,13 +163,6 @@ func makeTask(userId, channelId, quota, tokenId int, billingSource string, subsc ...@@ -163,13 +163,6 @@ func makeTask(userId, channelId, quota, tokenId int, billingSource string, subsc
} }
} }
func taskBillingOtherMap(t *testing.T, other *model.LogOther) map[string]interface{} {
t.Helper()
var values map[string]interface{}
require.NoError(t, common.UnmarshalJsonStr(other.JSONString(), &values))
return values
}
func TestPriceDataOtherRatiosFilterAndSnapshot(t *testing.T) { func TestPriceDataOtherRatiosFilterAndSnapshot(t *testing.T) {
priceData := types.PriceData{} priceData := types.PriceData{}
...@@ -234,7 +227,7 @@ func TestTaskBillingOtherFiltersHistoricalOtherRatios(t *testing.T) { ...@@ -234,7 +227,7 @@ func TestTaskBillingOtherFiltersHistoricalOtherRatios(t *testing.T) {
"inf": math.Inf(1), "inf": math.Inf(1),
} }
other := taskBillingOtherMap(t, taskBillingOther(task)) other := taskBillingOther(task).Snapshot()
assert.Equal(t, 2.0, other["seconds"]) assert.Equal(t, 2.0, other["seconds"])
assert.Equal(t, 1.0, other["identity"]) assert.Equal(t, 1.0, other["identity"])
...@@ -260,7 +253,7 @@ func TestTaskBillingOtherIncludesTieredSnapshotAndKeepsUsageFactsNested(t *testi ...@@ -260,7 +253,7 @@ func TestTaskBillingOtherIncludesTieredSnapshotAndKeepsUsageFactsNested(t *testi
}, },
} }
other := taskBillingOtherMap(t, taskBillingOther(task)) other := taskBillingOther(task).Snapshot()
assert.Equal(t, "tiered_expr", other["billing_mode"]) assert.Equal(t, "tiered_expr", other["billing_mode"])
assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(expression)), other["expr_b64"]) assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(expression)), other["expr_b64"])
...@@ -269,7 +262,7 @@ func TestTaskBillingOtherIncludesTieredSnapshotAndKeepsUsageFactsNested(t *testi ...@@ -269,7 +262,7 @@ func TestTaskBillingOtherIncludesTieredSnapshotAndKeepsUsageFactsNested(t *testi
require.True(t, ok) require.True(t, ok)
assert.Equal(t, map[string]any{ assert.Equal(t, map[string]any{
"resolution": "720P", "resolution": "720P",
"seconds": float64(5), "seconds": 5,
}, facts) }, facts)
assert.NotContains(t, other, "resolution") assert.NotContains(t, other, "resolution")
assert.NotContains(t, other, "seconds") assert.NotContains(t, other, "seconds")
...@@ -284,7 +277,7 @@ func TestTaskBillingOtherOmitsEmptyUsageFacts(t *testing.T) { ...@@ -284,7 +277,7 @@ func TestTaskBillingOtherOmitsEmptyUsageFacts(t *testing.T) {
UsageFacts: map[string]any{}, UsageFacts: map[string]any{},
} }
other := taskBillingOtherMap(t, taskBillingOther(task)) other := taskBillingOther(task).Snapshot()
assert.Equal(t, "tiered_expr", other["billing_mode"]) assert.Equal(t, "tiered_expr", other["billing_mode"])
assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(expression)), other["expr_b64"]) assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(expression)), other["expr_b64"])
...@@ -408,7 +401,7 @@ func TestTaskBillingOtherSeparatesPluginAndRootDiagnostics(t *testing.T) { ...@@ -408,7 +401,7 @@ func TestTaskBillingOtherSeparatesPluginAndRootDiagnostics(t *testing.T) {
}, },
} }
other := taskBillingOtherMap(t, taskBillingOther(task)) other := taskBillingOther(task).Snapshot()
assert.Equal(t, "task_public", other["task_id"]) assert.Equal(t, "task_public", other["task_id"])
adminInfo, ok := other["admin_info"].(map[string]interface{}) adminInfo, ok := other["admin_info"].(map[string]interface{})
...@@ -428,7 +421,7 @@ func TestTaskBillingOtherSeparatesPluginAndRootDiagnostics(t *testing.T) { ...@@ -428,7 +421,7 @@ func TestTaskBillingOtherSeparatesPluginAndRootDiagnostics(t *testing.T) {
assert.Equal(t, "node-a", rootInfo["node_name"]) assert.Equal(t, "node-a", rootInfo["node_name"])
runtimeInfo, ok := rootInfo["task_plugin"].(map[string]interface{}) runtimeInfo, ok := rootInfo["task_plugin"].(map[string]interface{})
require.True(t, ok) require.True(t, ok)
assert.Equal(t, float64(42), runtimeInfo["generation"]) assert.Equal(t, uint64(42), runtimeInfo["generation"])
assert.NotContains(t, runtimeInfo, "author") assert.NotContains(t, runtimeInfo, "author")
} }
......
...@@ -451,16 +451,13 @@ func TestAppendUsageBillingPathForLogWritesAdminInfo(t *testing.T) { ...@@ -451,16 +451,13 @@ func TestAppendUsageBillingPathForLogWritesAdminInfo(t *testing.T) {
BillingUsage: dto.NewClaudeMessagesBillingUsage(&dto.ClaudeUsage{InputTokens: 1}), BillingUsage: dto.NewClaudeMessagesBillingUsage(&dto.ClaudeUsage{InputTokens: 1}),
}) })
var values map[string]interface{} adminInfo, ok := other.Snapshot()["admin_info"].(map[string]interface{})
require.NoError(t, common.UnmarshalJsonStr(other.JSONString(), &values))
adminInfo, ok := values["admin_info"].(map[string]interface{})
require.True(t, ok) require.True(t, ok)
require.Equal(t, usageBillingPathAnthropic, adminInfo["usage_billing_path"]) require.Equal(t, usageBillingPathAnthropic, adminInfo["usage_billing_path"])
other = model.NewLogOther() other = model.NewLogOther()
appendUsageBillingPathForLog(other, true, nil) appendUsageBillingPathForLog(other, true, nil)
require.NoError(t, common.UnmarshalJsonStr(other.JSONString(), &values)) adminInfo, ok = other.Snapshot()["admin_info"].(map[string]interface{})
adminInfo, ok = values["admin_info"].(map[string]interface{})
require.True(t, ok) require.True(t, ok)
require.Equal(t, usageBillingPathLocal, adminInfo["usage_billing_path"]) require.Equal(t, usageBillingPathLocal, adminInfo["usage_billing_path"])
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment