Skip to content

Commit f5ab9ff

Browse files
SantiagoDePoloniaSantiagoDePolonia
committed
fix(tracking): align root user-path fallbacks
1 parent a8a46f8 commit f5ab9ff

18 files changed

Lines changed: 377 additions & 23 deletions

‎internal/admin/handler.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func parseUsageParams(c *echo.Context) (usage.UsageQueryParams, error) {
161161
params.Interval = "daily"
162162
}
163163

164-
userPath, err := normalizeUserPathQueryParam(c.QueryParam("user_path"))
164+
userPath, err := normalizeUserPathQueryParam("user_path", c.QueryParam("user_path"))
165165
if err != nil {
166166
return params, err
167167
}
@@ -170,10 +170,10 @@ func parseUsageParams(c *echo.Context) (usage.UsageQueryParams, error) {
170170
return params, nil
171171
}
172172

173-
func normalizeUserPathQueryParam(raw string) (string, error) {
173+
func normalizeUserPathQueryParam(fieldName, raw string) (string, error) {
174174
userPath, err := core.NormalizeUserPath(raw)
175175
if err != nil {
176-
return "", core.NewInvalidRequestError("invalid user_path: "+err.Error(), err)
176+
return "", core.NewInvalidRequestError("invalid "+fieldName+": "+err.Error(), err)
177177
}
178178
return userPath, nil
179179
}
@@ -454,7 +454,7 @@ func (h *Handler) AuditLog(c *echo.Context) error {
454454
if err != nil {
455455
return handleError(c, err)
456456
}
457-
userPath, err := normalizeUserPathQueryParam(c.QueryParam("user_path"))
457+
userPath, err := normalizeUserPathQueryParam("user_path", c.QueryParam("user_path"))
458458
if err != nil {
459459
return handleError(c, err)
460460
}
@@ -911,7 +911,7 @@ func (h *Handler) CreateExecutionPlan(c *echo.Context) error {
911911
return handleError(c, core.NewInvalidRequestError("invalid request body: "+err.Error(), err))
912912
}
913913

914-
scopeUserPath, err := normalizeUserPathQueryParam(req.ScopeUserPath)
914+
scopeUserPath, err := normalizeUserPathQueryParam("scope_user_path", req.ScopeUserPath)
915915
if err != nil {
916916
return handleError(c, err)
917917
}

‎internal/admin/handler_executionplans_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,56 @@ func TestCreateExecutionPlanRejectsUnknownProviderOrModelScope(t *testing.T) {
792792
}
793793
}
794794

795+
func TestCreateExecutionPlan_UsesScopeUserPathInValidationErrors(t *testing.T) {
796+
store := &executionPlanTestStore{
797+
versions: []executionplans.Version{
798+
{
799+
ID: "global-plan",
800+
Scope: executionplans.Scope{},
801+
ScopeKey: "global",
802+
Version: 1,
803+
Active: true,
804+
Name: "global",
805+
Payload: executionplans.Payload{
806+
SchemaVersion: 1,
807+
Features: executionplans.FeatureFlags{Cache: true, Audit: true, Usage: true, Guardrails: false},
808+
},
809+
PlanHash: "hash-global",
810+
},
811+
},
812+
}
813+
h := newExecutionPlanHandler(t, store, nil)
814+
e := echo.New()
815+
816+
req := httptest.NewRequest(http.MethodPost, "/admin/api/v1/execution-plans", bytes.NewBufferString(`{
817+
"scope_user_path":"/team/../alpha",
818+
"name":"invalid path",
819+
"plan_payload":{
820+
"schema_version":1,
821+
"features":{"cache":true,"audit":true,"usage":true,"guardrails":false},
822+
"guardrails":[]
823+
}
824+
}`))
825+
req.Header.Set("Content-Type", "application/json")
826+
rec := httptest.NewRecorder()
827+
c := e.NewContext(req, rec)
828+
829+
if err := h.CreateExecutionPlan(c); err != nil {
830+
t.Fatalf("CreateExecutionPlan() error = %v", err)
831+
}
832+
if rec.Code != http.StatusBadRequest {
833+
t.Fatalf("status = %d, want 400", rec.Code)
834+
}
835+
836+
body := decodeExecutionPlanErrorEnvelope(t, rec.Body.Bytes())
837+
if body.Error.Type != "invalid_request_error" {
838+
t.Fatalf("error type = %q, want invalid_request_error", body.Error.Type)
839+
}
840+
if body.Error.Message != `invalid scope_user_path: user path cannot contain '.' or '..' segments` {
841+
t.Fatalf("error message = %q, want invalid scope_user_path message", body.Error.Message)
842+
}
843+
}
844+
795845
func TestExecutionPlanViewReflectsFeatureCaps(t *testing.T) {
796846
store := &executionPlanTestStore{
797847
versions: []executionplans.Version{

‎internal/admin/handler_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,25 @@ func TestParseUsageParams_InvalidEndDate(t *testing.T) {
14571457
}
14581458
}
14591459

1460+
func TestParseUsageParams_InvalidUserPath(t *testing.T) {
1461+
c := newContext("user_path=/team/../alpha")
1462+
_, err := parseUsageParams(c)
1463+
if err == nil {
1464+
t.Fatal("expected error for invalid user_path, got nil")
1465+
}
1466+
1467+
var gatewayErr *core.GatewayError
1468+
if !errors.As(err, &gatewayErr) {
1469+
t.Fatalf("expected GatewayError, got %T", err)
1470+
}
1471+
if gatewayErr.Message != `invalid user_path: user path cannot contain '.' or '..' segments` {
1472+
t.Fatalf("message = %q, want invalid user_path message", gatewayErr.Message)
1473+
}
1474+
if gatewayErr.HTTPStatusCode() != http.StatusBadRequest {
1475+
t.Errorf("expected status 400, got %d", gatewayErr.HTTPStatusCode())
1476+
}
1477+
}
1478+
14601479
func TestParseUsageParams_IntervalWeekly(t *testing.T) {
14611480
c := newContext("interval=weekly")
14621481
params, err := parseUsageParams(c)

‎internal/auditlog/auditlog_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,31 @@ func TestMiddleware_StoresAuthKeyIDFromContext(t *testing.T) {
699699
}
700700
}
701701

702+
func TestMiddleware_DefaultsMissingUserPathToRoot(t *testing.T) {
703+
logger := &capturingLogger{cfg: Config{Enabled: true}}
704+
middleware := Middleware(logger)
705+
e := echo.New()
706+
707+
req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{"model":"gpt-4o-mini"}`))
708+
req.Header.Set("Content-Type", "application/json")
709+
rec := httptest.NewRecorder()
710+
c := e.NewContext(req, rec)
711+
712+
handler := middleware(func(c *echo.Context) error {
713+
return c.NoContent(http.StatusOK)
714+
})
715+
716+
if err := handler(c); err != nil {
717+
t.Fatalf("handler() error = %v", err)
718+
}
719+
if len(logger.entries) != 1 {
720+
t.Fatalf("logger.entries len = %d, want 1", len(logger.entries))
721+
}
722+
if got := logger.entries[0].UserPath; got != "/" {
723+
t.Fatalf("UserPath = %q, want /", got)
724+
}
725+
}
726+
702727
func TestMiddleware_SkipsWriteWhenExecutionPlanDisablesAudit(t *testing.T) {
703728
e := echo.New()
704729
logger := &capturingLogger{

‎internal/auditlog/middleware.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ func Middleware(logger LoggerInterface) echo.MiddlewareFunc {
5757

5858
// Read request ID (always set by the request ID middleware in http.go)
5959
requestID := req.Header.Get("X-Request-ID")
60+
userPath := core.UserPathFromContext(req.Context())
61+
if userPath == "" {
62+
userPath = "/"
63+
}
6064

6165
// Create initial log entry
6266
entry := &LogEntry{
@@ -66,7 +70,7 @@ func Middleware(logger LoggerInterface) echo.MiddlewareFunc {
6670
ClientIP: c.RealIP(),
6771
Method: req.Method,
6872
Path: req.URL.Path,
69-
UserPath: core.UserPathFromContext(req.Context()),
73+
UserPath: userPath,
7074
Data: &LogData{
7175
UserAgent: req.UserAgent(),
7276
},

‎internal/auditlog/reader_mongodb.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"regexp"
77
"time"
88

9+
"gomodel/internal/core"
10+
911
"go.mongodb.org/mongo-driver/v2/bson"
1012
"go.mongodb.org/mongo-driver/v2/mongo"
1113
"go.mongodb.org/mongo-driver/v2/mongo/options"
@@ -120,7 +122,7 @@ func (r *MongoDBReader) GetLogs(ctx context.Context, params LogQueryParams) (*Lo
120122
})
121123
}
122124
if userPath, err := normalizeAuditUserPathFilter(params.UserPath); err != nil {
123-
return nil, err
125+
return nil, core.NewInvalidRequestError(err.Error(), err)
124126
} else if userPath != "" {
125127
matchFilters = append(matchFilters, bson.E{
126128
Key: "user_path",

‎internal/auditlog/reader_mongodb_test.go‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
package auditlog
22

3-
import "testing"
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
"gomodel/internal/core"
9+
)
410

511
func TestSanitizeLogDataRedactsHeaders(t *testing.T) {
612
original := &LogData{
@@ -64,3 +70,20 @@ func TestMongoLogRowToLogEntryPreservesCacheType(t *testing.T) {
6470
t.Fatalf("CacheType = %q, want %q", entry.CacheType, CacheTypeSemantic)
6571
}
6672
}
73+
74+
func TestMongoDBReader_GetLogsInvalidUserPathReturnsGatewayError(t *testing.T) {
75+
reader := &MongoDBReader{}
76+
77+
_, err := reader.GetLogs(context.Background(), LogQueryParams{UserPath: "/team/../alpha"})
78+
if err == nil {
79+
t.Fatal("expected error, got nil")
80+
}
81+
82+
var gatewayErr *core.GatewayError
83+
if !errors.As(err, &gatewayErr) {
84+
t.Fatalf("expected GatewayError, got %T", err)
85+
}
86+
if gatewayErr.Type != core.ErrorTypeInvalidRequest {
87+
t.Fatalf("gatewayErr.Type = %q, want %q", gatewayErr.Type, core.ErrorTypeInvalidRequest)
88+
}
89+
}

‎internal/auditlog/reader_postgresql.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ func (r *PostgreSQLReader) GetLogs(ctx context.Context, params LogQueryParams) (
5353
argIdx++
5454
}
5555
if userPath != "" {
56-
conditions = append(conditions, fmt.Sprintf("(user_path = $%d OR user_path LIKE $%d ESCAPE '\\')", argIdx, argIdx+1))
56+
conditions = append(conditions, auditUserPathSQLPredicate(
57+
userPath,
58+
fmt.Sprintf("user_path = $%d", argIdx),
59+
fmt.Sprintf("user_path LIKE $%d ESCAPE '\\'", argIdx+1),
60+
))
5761
args = append(args, userPath, auditUserPathSubtreePattern(userPath))
5862
argIdx += 2
5963
}

‎internal/auditlog/reader_sqlite.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (r *SQLiteReader) GetLogs(ctx context.Context, params LogQueryParams) (*Log
5252
args = append(args, "%"+escapeLikeWildcards(params.Path)+"%")
5353
}
5454
if userPath != "" {
55-
conditions = append(conditions, "(user_path = ? OR user_path LIKE ? ESCAPE '\\')")
55+
conditions = append(conditions, auditUserPathSQLPredicate(userPath, "user_path = ?", "user_path LIKE ? ESCAPE '\\'"))
5656
args = append(args, userPath, auditUserPathSubtreePattern(userPath))
5757
}
5858
if params.ErrorType != "" {

‎internal/auditlog/store_sqlite_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,78 @@ func TestSQLiteReader_GetLogsFiltersByUserPathSubtree(t *testing.T) {
415415
}
416416
}
417417

418+
func TestSQLiteReader_GetLogsRootUserPathIncludesLegacyNullRows(t *testing.T) {
419+
db := createTestDB(t)
420+
defer db.Close()
421+
422+
store, err := NewSQLiteStore(db, 0)
423+
if err != nil {
424+
t.Fatalf("failed to create store: %v", err)
425+
}
426+
defer store.Close()
427+
428+
now := time.Now().UTC().Format(time.RFC3339Nano)
429+
_, err = db.Exec(`
430+
INSERT INTO audit_logs (
431+
id, timestamp, duration_ns, model, resolved_model, provider, alias_used, execution_plan_version_id,
432+
status_code, request_id, client_ip, method, path, user_path, stream, error_type, data
433+
) VALUES
434+
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?),
435+
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
436+
`,
437+
"legacy-null",
438+
now,
439+
0,
440+
"gpt-4",
441+
"",
442+
"openai",
443+
0,
444+
nil,
445+
200,
446+
"req-legacy",
447+
"127.0.0.1",
448+
"POST",
449+
"/v1/chat/completions",
450+
nil,
451+
0,
452+
"",
453+
nil,
454+
"root-explicit",
455+
now,
456+
0,
457+
"gpt-4",
458+
"",
459+
"openai",
460+
0,
461+
nil,
462+
200,
463+
"req-root",
464+
"127.0.0.1",
465+
"POST",
466+
"/v1/chat/completions",
467+
"/",
468+
0,
469+
"",
470+
nil,
471+
)
472+
if err != nil {
473+
t.Fatalf("failed to insert audit log rows: %v", err)
474+
}
475+
476+
reader, err := NewSQLiteReader(db)
477+
if err != nil {
478+
t.Fatalf("failed to create reader: %v", err)
479+
}
480+
481+
logs, err := reader.GetLogs(context.Background(), LogQueryParams{UserPath: "/", Limit: 10})
482+
if err != nil {
483+
t.Fatalf("GetLogs failed: %v", err)
484+
}
485+
if len(logs.Entries) != 2 {
486+
t.Fatalf("len(entries) = %d, want 2", len(logs.Entries))
487+
}
488+
}
489+
418490
func TestSQLiteStoreAndReader_PreserveCacheType(t *testing.T) {
419491
db := createTestDB(t)
420492
defer db.Close()

0 commit comments

Comments
 (0)