feat(auth): add API key path scoping and workflow auth node styling#197
Merged
SantiagoDePolonia merged 3 commits intomainfrom Mar 31, 2026
Merged
feat(auth): add API key path scoping and workflow auth node styling#197SantiagoDePolonia merged 3 commits intomainfrom
SantiagoDePolonia merged 3 commits intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds optional per-auth-key user_path: UI input, validation/normalization, persistence across stores, propagation through authentication middleware into request context and audit logs; execution-plan middleware ordering adjusted so managed key user_path is applied before plan resolution; tests updated accordingly. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Server
participant AuthService as Auth Service
participant Context as Context Helpers
participant Audit as Audit Log
participant ExecPlan as Execution Planner
Client->>Server: HTTP request (Authorization: Bearer <token>, User-Path header)
Server->>AuthService: Authenticate(token)
AuthService-->>Server: AuthenticationResult { ID, UserPath }
Server->>Context: WithAuthKeyID(ctx, ID)
Server->>Context: WithEffectiveUserPath(ctx, UserPath)
Server->>Server: RequestSnapshot.WithUserPath(UserPath)
Server->>Audit: EnrichEntryWithUserPath(UserPath)
Server->>ExecPlan: ExecutionPlanningWithResolverAndPolicy(ctx with effective UserPath)
ExecPlan-->>Server: Plan resolved using effective UserPath
Server->>Client: Handler response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/admin/dashboard/templates/execution-plan-chart.html (1)
14-20: 🧹 Nitpick | 🔵 TrivialAuth and Cache nodes use identical icons and styling—consider distinct visual treatment for clarity.
Both the Auth and Cache nodes use the exact same SVG (database/cylinder icon) and share identical CSS styling (same
--accentcolor, border, background). This makes them visually indistinguishable except for their text labels, which could confuse users when scanning the execution pipeline visually.Since these represent functionally different workflow stages, they should have distinct visual identities. Consider using a unique icon for Auth (e.g., lock/key or security-related symbol) to improve visual differentiation and pipeline readability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/dashboard/templates/execution-plan-chart.html` around lines 14 - 20, The Auth node currently reuses the same database SVG and styling as Cache (ep-node-auth uses authNodeClass/authNodeSublabel); update the template and styles so Auth has a distinct visual identity: replace the current cylinder SVG in the ep-node-auth block with a security-themed SVG (lock/key) and assign a distinct accent color and border/background variables (e.g., set --accent to a different color) for the ep-node-auth class so it no longer matches Cache—ensure you update any style rules that reference ep-node-auth or authNodeClass so the new icon and color are applied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/authkeys/store_postgresql.go`:
- Around line 46-48: Replace the inline ALTER TABLE Exec with the same
"migrations array" pattern used in auditlog: create a migrations []string
containing the ALTER TABLE statement (`ALTER TABLE auth_keys ADD COLUMN IF NOT
EXISTS user_path TEXT`) and then iterate over migrations calling pool.Exec(ctx,
migration) and returning a wrapped error (as currently done with fmt.Errorf) on
failure; this keeps consistency with the auditlog store and makes future
migrations clearer—locate the current pool.Exec call and replace it with the
migrations slice + loop, preserving use of pool.Exec, ctx, and fmt.Errorf for
errors.
In `@internal/authkeys/store_sqlite.go`:
- Around line 41-43: The inline ALTER TABLE call that adds user_path should be
converted to the same migrations-array pattern used in
internal/auditlog/store_sqlite.go and internal/usage/store_sqlite.go: create a
migrations []string (or []migration) entry containing "ALTER TABLE auth_keys ADD
COLUMN user_path TEXT", append it to the existing migrations list, and run it
through the module's migration executor so migrations are visible and auditable;
when applying this migration ensure you preserve the current error handling (use
isSQLiteDuplicateColumnError to ignore already-present column errors) so
behavior doesn't change.
---
Outside diff comments:
In `@internal/admin/dashboard/templates/execution-plan-chart.html`:
- Around line 14-20: The Auth node currently reuses the same database SVG and
styling as Cache (ep-node-auth uses authNodeClass/authNodeSublabel); update the
template and styles so Auth has a distinct visual identity: replace the current
cylinder SVG in the ep-node-auth block with a security-themed SVG (lock/key) and
assign a distinct accent color and border/background variables (e.g., set
--accent to a different color) for the ep-node-auth class so it no longer
matches Cache—ensure you update any style rules that reference ep-node-auth or
authNodeClass so the new icon and color are applied.
🪄 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: 943e5c87-538a-437d-bb84-99693e9f4e62
📒 Files selected for processing (27)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/modules/auth-keys.jsinternal/admin/dashboard/static/js/modules/auth-keys.test.jsinternal/admin/dashboard/static/js/modules/dashboard-layout.test.jsinternal/admin/dashboard/static/js/modules/execution-plans-layout.test.jsinternal/admin/dashboard/templates/execution-plan-chart.htmlinternal/admin/dashboard/templates/index.htmlinternal/admin/handler.gointernal/admin/handler_authkeys_test.gointernal/auditlog/auditlog_test.gointernal/auditlog/middleware.gointernal/authkeys/service.gointernal/authkeys/service_test.gointernal/authkeys/store.gointernal/authkeys/store_mongodb.gointernal/authkeys/store_postgresql.gointernal/authkeys/store_sqlite.gointernal/authkeys/types.gointernal/core/context.gointernal/core/request_snapshot.gointernal/core/request_snapshot_test.gointernal/core/user_path.gointernal/core/user_path_test.gointernal/server/auth.gointernal/server/auth_test.gointernal/server/http.gointernal/server/http_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/authkeys/store_postgresql.go`:
- Around line 163-176: The MongoDB store is missing the store-level
normalization present in PostgreSQL (pgNullableString/derefTrimmedString);
update the MongoDB write and read paths so they mirror PostgreSQL behavior by
trimming whitespace and converting empty strings to nil-equivalent values: in
the MongoDB Create method (where UserPath is assigned) apply a
trim-and-empty-to-nil normalization (same semantics as pgNullableString) before
persisting, and in authKeyFromMongo apply a trimmed-default behavior (same
semantics as derefTrimmedString) when constructing the returned auth key; this
keeps store-level behavior consistent with core.NormalizeUserPath and PostgreSQL
defenses.
In `@internal/authkeys/store_sqlite.go`:
- Around line 150-156: The error-detection is fragile because it relies on
message text; update the isSQLiteDuplicateColumnError function by adding a
concise comment documenting which SQLite library/driver and versions you tested
against and the exact error message variants observed (e.g., "duplicate column",
"already exists"), and expand the string checks to include likely variants such
as "duplicate column name" and patterns like "already exists:" to be more
robust; reference the function name isSQLiteDuplicateColumnError and ensure the
comment explains that these heuristics are used when a structured error code is
unavailable.
🪄 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: 9cd1961a-d58f-4e30-867c-1c8fb8d2df54
📒 Files selected for processing (2)
internal/authkeys/store_postgresql.gointernal/authkeys/store_sqlite.go
Comment on lines
+163
to
+176
| func pgNullableString(value string) any { | ||
| value = strings.TrimSpace(value) | ||
| if value == "" { | ||
| return nil | ||
| } | ||
| return value | ||
| } | ||
|
|
||
| func derefTrimmedString(value *string) string { | ||
| if value == nil { | ||
| return "" | ||
| } | ||
| return strings.TrimSpace(*value) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
❓ Verification inconclusive
Script executed:
#!/bin/bash
# Verify MongoDB store handling of UserPath
rg -n -A3 -B3 'UserPath' internal/authkeys/store_mongodb.goRepository: ENTERPILOT/GoModel
Repository: ENTERPILOT/GoModel
Exit code: 0
stdout:
14- ID string `bson:"_id"`
15- Name string `bson:"name"`
16- Description string `bson:"description,omitempty"`
17: UserPath string `bson:"user_path,omitempty"`
18- RedactedValue string `bson:"redacted_value"`
19- SecretHash string `bson:"secret_hash"`
20- Enabled bool `bson:"enabled"`
--
79- ID: key.ID,
80- Name: key.Name,
81- Description: key.Description,
82: UserPath: key.UserPath,
83- RedactedValue: key.RedactedValue,
84- SecretHash: key.SecretHash,
85- Enabled: key.Enabled,
--
124- ID: doc.ID,
125- Name: doc.Name,
126- Description: doc.Description,
127: UserPath: doc.UserPath,
128- RedactedValue: doc.RedactedValue,
129- SecretHash: doc.SecretHash,
130- Enabled: doc.Enabled,
Script executed:
#!/bin/bash
# Check the PostgreSQL store to verify pgNullableString/derefTrimmedString usage
cat -n internal/authkeys/store_postgresql.go | head -200Repository: ENTERPILOT/GoModel
Repository: ENTERPILOT/GoModel
Exit code: 0
stdout:
1 package authkeys
2
3 import (
4 "context"
5 "errors"
6 "fmt"
7 "strings"
8 "time"
9
10 "github.com/jackc/pgx/v5"
11 "github.com/jackc/pgx/v5/pgxpool"
12 )
13
14 // PostgreSQLStore stores auth keys in PostgreSQL.
15 type PostgreSQLStore struct {
16 pool *pgxpool.Pool
17 }
18
19 // NewPostgreSQLStore creates the auth_keys table and indexes if needed.
20 func NewPostgreSQLStore(ctx context.Context, pool *pgxpool.Pool) (*PostgreSQLStore, error) {
21 if ctx == nil {
22 return nil, fmt.Errorf("context is required")
23 }
24 if pool == nil {
25 return nil, fmt.Errorf("connection pool is required")
26 }
27
28 _, err := pool.Exec(ctx, `
29 CREATE TABLE IF NOT EXISTS auth_keys (
30 id TEXT PRIMARY KEY,
31 name TEXT NOT NULL,
32 description TEXT NOT NULL DEFAULT '',
33 user_path TEXT,
34 redacted_value TEXT NOT NULL,
35 secret_hash TEXT NOT NULL UNIQUE,
36 enabled BOOLEAN NOT NULL DEFAULT TRUE,
37 expires_at BIGINT,
38 deactivated_at BIGINT,
39 created_at BIGINT NOT NULL,
40 updated_at BIGINT NOT NULL
41 )
42 `)
43 if err != nil {
44 return nil, fmt.Errorf("failed to create auth_keys table: %w", err)
45 }
46
47 migrations := []string{
48 `ALTER TABLE auth_keys ADD COLUMN IF NOT EXISTS user_path TEXT`,
49 }
50 for _, migration := range migrations {
51 if _, err := pool.Exec(ctx, migration); err != nil {
52 return nil, fmt.Errorf("failed to run migration %q: %w", migration, err)
53 }
54 }
55 for _, index := range []string{
56 `CREATE INDEX IF NOT EXISTS idx_auth_keys_enabled ON auth_keys(enabled)`,
57 `CREATE INDEX IF NOT EXISTS idx_auth_keys_created_at ON auth_keys(created_at DESC)`,
58 } {
59 if _, err := pool.Exec(ctx, index); err != nil {
60 return nil, fmt.Errorf("failed to create auth_keys index: %w", err)
61 }
62 }
63 return &PostgreSQLStore{pool: pool}, nil
64 }
65
66 func (s *PostgreSQLStore) List(ctx context.Context) ([]AuthKey, error) {
67 rows, err := s.pool.Query(ctx, `
68 SELECT id, name, description, user_path, redacted_value, secret_hash, enabled, expires_at, deactivated_at, created_at, updated_at
69 FROM auth_keys
70 ORDER BY created_at DESC, id ASC
71 `)
72 if err != nil {
73 return nil, fmt.Errorf("list auth keys: %w", err)
74 }
75 defer rows.Close()
76 result, err := collectAuthKeys(rows, scanPostgreSQLAuthKey)
77 if err != nil {
78 return nil, fmt.Errorf("iterate auth keys: %w", err)
79 }
80 return result, nil
81 }
82
83 func (s *PostgreSQLStore) Create(ctx context.Context, key AuthKey) error {
84 _, err := s.pool.Exec(ctx, `
85 INSERT INTO auth_keys (id, name, description, user_path, redacted_value, secret_hash, enabled, expires_at, deactivated_at, created_at, updated_at)
86 VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
87 `, key.ID, key.Name, key.Description, pgNullableString(key.UserPath), key.RedactedValue, key.SecretHash, key.Enabled, pgUnixOrNil(key.ExpiresAt), pgUnixOrNil(key.DeactivatedAt), key.CreatedAt.Unix(), key.UpdatedAt.Unix())
88 if err != nil {
89 return fmt.Errorf("create auth key: %w", err)
90 }
91 return nil
92 }
93
94 func (s *PostgreSQLStore) Deactivate(ctx context.Context, id string, now time.Time) error {
95 cmd, err := s.pool.Exec(ctx, `
96 UPDATE auth_keys
97 SET enabled = FALSE,
98 deactivated_at = COALESCE(deactivated_at, $1),
99 updated_at = $2
100 WHERE id = $3
101 `, now.Unix(), now.Unix(), normalizeID(id))
102 if err != nil {
103 return fmt.Errorf("deactivate auth key: %w", err)
104 }
105 if cmd.RowsAffected() == 0 {
106 return ErrNotFound
107 }
108 return nil
109 }
110
111 func (s *PostgreSQLStore) Close() error {
112 return nil
113 }
114
115 func scanPostgreSQLAuthKey(scanner authKeyScanner) (AuthKey, error) {
116 var key AuthKey
117 var userPath *string
118 var expiresAt *int64
119 var deactivatedAt *int64
120 var createdAt int64
121 var updatedAt int64
122 if err := scanner.Scan(
123 &key.ID,
124 &key.Name,
125 &key.Description,
126 &userPath,
127 &key.RedactedValue,
128 &key.SecretHash,
129 &key.Enabled,
130 &expiresAt,
131 &deactivatedAt,
132 &createdAt,
133 &updatedAt,
134 ); err != nil {
135 if errors.Is(err, pgx.ErrNoRows) {
136 return AuthKey{}, ErrNotFound
137 }
138 return AuthKey{}, err
139 }
140 key.UserPath = derefTrimmedString(userPath)
141 key.ExpiresAt = int64PtrToTime(expiresAt)
142 key.DeactivatedAt = int64PtrToTime(deactivatedAt)
143 key.CreatedAt = time.Unix(createdAt, 0).UTC()
144 key.UpdatedAt = time.Unix(updatedAt, 0).UTC()
145 return key, nil
146 }
147
148 func pgUnixOrNil(value *time.Time) any {
149 if value == nil {
150 return nil
151 }
152 return value.UTC().Unix()
153 }
154
155 func int64PtrToTime(value *int64) *time.Time {
156 if value == nil {
157 return nil
158 }
159 t := time.Unix(*value, 0).UTC()
160 return &t
161 }
162
163 func pgNullableString(value string) any {
164 value = strings.TrimSpace(value)
165 if value == "" {
166 return nil
167 }
168 return value
169 }
170
171 func derefTrimmedString(value *string) string {
172 if value == nil {
173 return ""
174 }
175 return strings.TrimSpace(*value)
176 }
Script executed:
#!/bin/bash
# Get the full MongoDB store to see the actual structure and line numbers
cat -n internal/authkeys/store_mongodb.go | head -150Repository: ENTERPILOT/GoModel
Repository: ENTERPILOT/GoModel
Exit code: 0
stdout:
1 package authkeys
2
3 import (
4 "context"
5 "fmt"
6 "time"
7
8 "go.mongodb.org/mongo-driver/v2/bson"
9 "go.mongodb.org/mongo-driver/v2/mongo"
10 "go.mongodb.org/mongo-driver/v2/mongo/options"
11 )
12
13 type mongoAuthKeyDocument struct {
14 ID string `bson:"_id"`
15 Name string `bson:"name"`
16 Description string `bson:"description,omitempty"`
17 UserPath string `bson:"user_path,omitempty"`
18 RedactedValue string `bson:"redacted_value"`
19 SecretHash string `bson:"secret_hash"`
20 Enabled bool `bson:"enabled"`
21 ExpiresAt *time.Time `bson:"expires_at,omitempty"`
22 DeactivatedAt *time.Time `bson:"deactivated_at,omitempty"`
23 CreatedAt time.Time `bson:"created_at"`
24 UpdatedAt time.Time `bson:"updated_at"`
25 }
26
27 type mongoAuthKeyIDFilter struct {
28 ID string `bson:"_id"`
29 }
30
31 // MongoDBStore stores auth keys in MongoDB.
32 type MongoDBStore struct {
33 collection *mongo.Collection
34 }
35
36 // NewMongoDBStore creates collection indexes if needed.
37 func NewMongoDBStore(database *mongo.Database) (*MongoDBStore, error) {
38 if database == nil {
39 return nil, fmt.Errorf("database is required")
40 }
41 coll := database.Collection("auth_keys")
42 ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
43 defer cancel()
44
45 indexes := []mongo.IndexModel{
46 {Keys: bson.D{{Key: "secret_hash", Value: 1}}, Options: options.Index().SetUnique(true)},
47 {Keys: bson.D{{Key: "enabled", Value: 1}}},
48 {Keys: bson.D{{Key: "created_at", Value: -1}}},
49 }
50 if _, err := coll.Indexes().CreateMany(ctx, indexes); err != nil {
51 return nil, fmt.Errorf("create auth_keys indexes: %w", err)
52 }
53 return &MongoDBStore{collection: coll}, nil
54 }
55
56 func (s *MongoDBStore) List(ctx context.Context) ([]AuthKey, error) {
57 cursor, err := s.collection.Find(ctx, bson.M{}, options.Find().SetSort(bson.D{{Key: "created_at", Value: -1}, {Key: "_id", Value: 1}}))
58 if err != nil {
59 return nil, fmt.Errorf("list auth keys: %w", err)
60 }
61 defer cursor.Close(ctx)
62
63 result := make([]AuthKey, 0)
64 for cursor.Next(ctx) {
65 var doc mongoAuthKeyDocument
66 if err := cursor.Decode(&doc); err != nil {
67 return nil, fmt.Errorf("decode auth key: %w", err)
68 }
69 result = append(result, authKeyFromMongo(doc))
70 }
71 if err := cursor.Err(); err != nil {
72 return nil, fmt.Errorf("iterate auth keys: %w", err)
73 }
74 return result, nil
75 }
76
77 func (s *MongoDBStore) Create(ctx context.Context, key AuthKey) error {
78 _, err := s.collection.InsertOne(ctx, mongoAuthKeyDocument{
79 ID: key.ID,
80 Name: key.Name,
81 Description: key.Description,
82 UserPath: key.UserPath,
83 RedactedValue: key.RedactedValue,
84 SecretHash: key.SecretHash,
85 Enabled: key.Enabled,
86 ExpiresAt: key.ExpiresAt,
87 DeactivatedAt: key.DeactivatedAt,
88 CreatedAt: key.CreatedAt.UTC(),
89 UpdatedAt: key.UpdatedAt.UTC(),
90 })
91 if err != nil {
92 return fmt.Errorf("create auth key: %w", err)
93 }
94 return nil
95 }
96
97 func (s *MongoDBStore) Deactivate(ctx context.Context, id string, now time.Time) error {
98 now = now.UTC()
99 result, err := s.collection.UpdateOne(ctx, mongoAuthKeyIDFilter{ID: normalizeID(id)}, mongo.Pipeline{
100 {{
101 Key: "$set",
102 Value: bson.D{
103 {Key: "enabled", Value: false},
104 {Key: "updated_at", Value: now},
105 {Key: "deactivated_at", Value: bson.D{{Key: "$ifNull", Value: bson.A{"$deactivated_at", now}}}},
106 },
107 }},
108 })
109 if err != nil {
110 return fmt.Errorf("deactivate auth key: %w", err)
111 }
112 if result.MatchedCount == 0 {
113 return ErrNotFound
114 }
115 return nil
116 }
117
118 func (s *MongoDBStore) Close() error {
119 return nil
120 }
121
122 func authKeyFromMongo(doc mongoAuthKeyDocument) AuthKey {
123 return AuthKey{
124 ID: doc.ID,
125 Name: doc.Name,
126 Description: doc.Description,
127 UserPath: doc.UserPath,
128 RedactedValue: doc.RedactedValue,
129 SecretHash: doc.SecretHash,
130 Enabled: doc.Enabled,
131 ExpiresAt: timePtrUTC(doc.ExpiresAt),
132 DeactivatedAt: timePtrUTC(doc.DeactivatedAt),
133 CreatedAt: doc.CreatedAt.UTC(),
134 UpdatedAt: doc.UpdatedAt.UTC(),
135 }
136 }
137
138 func timePtrUTC(value *time.Time) *time.Time {
139 if value == nil {
140 return nil
141 }
142 t := value.UTC()
143 return &t
144 }
Script executed:
#!/bin/bash
# Check service layer normalization of UserPath
rg -n 'NormalizeUserPath' --type goRepository: ENTERPILOT/GoModel
Repository: ENTERPILOT/GoModel
Exit code: 0
stdout:
internal/authkeys/store.go:77: userPath, err := core.NormalizeUserPath(input.UserPath)
internal/usage/user_path_filter.go:11: userPath, err := core.NormalizeUserPath(raw)
internal/usage/stream_observer.go:28: if normalized, err := core.NormalizeUserPath(userPath[0]); err == nil {
internal/server/request_snapshot.go:34: userPath, err := core.NormalizeUserPath(req.Header.Get(core.UserPathHeader))
internal/executionplans/types.go:95: userPath, err := core.NormalizeUserPath(scope.UserPath)
internal/core/execution_plan.go:85: if normalized, err := NormalizeUserPath(userPath[0]); err == nil {
internal/core/user_path_test.go:8:func TestNormalizeUserPath(t *testing.T) {
internal/core/user_path_test.go:30: got, err := NormalizeUserPath(tt.raw)
internal/core/user_path_test.go:33: t.Fatal("NormalizeUserPath() error = nil, want error")
internal/core/user_path_test.go:38: t.Fatalf("NormalizeUserPath() error = %v", err)
internal/core/user_path_test.go:41: t.Fatalf("NormalizeUserPath() = %q, want %q", got, tt.want)
internal/core/user_path.go:11:// NormalizeUserPath canonicalizes one user hierarchy path from request ingress.
internal/core/user_path.go:12:func NormalizeUserPath(raw string) (string, error) {
internal/core/user_path.go:46: path, err := NormalizeUserPath(path)
internal/admin/handler.go:174: userPath, err := core.NormalizeUserPath(raw)
internal/auditlog/user_path_filter.go:11: userPath, err := core.NormalizeUserPath(raw)
Script executed:
#!/bin/bash
# Check auth.go for trimming operations on read
cat -n internal/server/auth.go | sed -n '75,95p'Repository: ENTERPILOT/GoModel
Repository: ENTERPILOT/GoModel
Exit code: 0
stdout:
75 }
76
77 if authenticator != nil && authenticator.Enabled() {
78 auditlog.EnrichEntryWithAuthMethod(c, auditlog.AuthMethodAPIKey)
79 authResult, err := authenticator.Authenticate(c.Request().Context(), token)
80 if err == nil {
81 ctx := core.WithAuthKeyID(c.Request().Context(), authResult.ID)
82 if userPath := strings.TrimSpace(authResult.UserPath); userPath != "" {
83 ctx = core.WithEffectiveUserPath(ctx, userPath)
84 if snapshot := core.GetRequestSnapshot(ctx); snapshot != nil {
85 ctx = core.WithRequestSnapshot(ctx, snapshot.WithUserPath(userPath))
86 }
87 c.Request().Header.Set(core.UserPathHeader, userPath)
88 auditlog.EnrichEntryWithUserPath(c, userPath)
89 }
90 c.SetRequest(c.Request().WithContext(ctx))
91 auditlog.EnrichEntryWithAuthKeyID(c, authResult.ID)
92 return next(c)
93 }
94
95 authErr := authenticationErrorWithAudit(c, authFailureMessage(err), "authentication failed")
Store-level normalization inconsistency between PostgreSQL and MongoDB.
The PostgreSQL store applies pgNullableString during writes (converts empty/whitespace to null) and derefTrimmedString during reads (trims whitespace). MongoDB's Create method (line 82) and authKeyFromMongo (line 127) assign UserPath directly without any trimming or empty-to-null conversion.
While the service layer normalizes via core.NormalizeUserPath before calling Create, and the auth handler trims on read (auth.go:82), the store-level inconsistency creates a behavioral gap. PostgreSQL's defensive store-level normalization would handle edge cases like direct database manipulation or future code paths that bypass service normalization; MongoDB does not.
Consider either adding equivalent trimming to MongoDB's write/read paths or documenting that store-level normalization is optional and the service layer is the authoritative normalization point.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/authkeys/store_postgresql.go` around lines 163 - 176, The MongoDB
store is missing the store-level normalization present in PostgreSQL
(pgNullableString/derefTrimmedString); update the MongoDB write and read paths
so they mirror PostgreSQL behavior by trimming whitespace and converting empty
strings to nil-equivalent values: in the MongoDB Create method (where UserPath
is assigned) apply a trim-and-empty-to-nil normalization (same semantics as
pgNullableString) before persisting, and in authKeyFromMongo apply a
trimmed-default behavior (same semantics as derefTrimmedString) when
constructing the returned auth key; this keeps store-level behavior consistent
with core.NormalizeUserPath and PostgreSQL defenses.
Comment on lines
+150
to
+156
| func isSQLiteDuplicateColumnError(err error) bool { | ||
| if err == nil { | ||
| return false | ||
| } | ||
| message := strings.ToLower(err.Error()) | ||
| return strings.Contains(message, "duplicate column") || strings.Contains(message, "already exists") | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Error detection logic is reasonable but could be fragile.
The isSQLiteDuplicateColumnError function checks for "duplicate column" or "already exists" substrings in lowercased error messages. This works for common SQLite versions but relies on error message text which could vary across SQLite versions or drivers.
Consider documenting the expected error messages or adding a comment noting which SQLite/driver versions this was tested against.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/authkeys/store_sqlite.go` around lines 150 - 156, The
error-detection is fragile because it relies on message text; update the
isSQLiteDuplicateColumnError function by adding a concise comment documenting
which SQLite library/driver and versions you tested against and the exact error
message variants observed (e.g., "duplicate column", "already exists"), and
expand the string checks to include likely variants such as "duplicate column
name" and patterns like "already exists:" to be more robust; reference the
function name isSQLiteDuplicateColumnError and ensure the comment explains that
these heuristics are used when a structured error code is unavailable.
This was referenced Apr 3, 2026
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
1 participant
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
Testing
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Style