Conversation
…aratMLStack into feat/horizon_code_sync
* Added switch between cac and env * Added open source functions for some of the internal calls * created stub files for some functions and moved code to interal * added changes * updated the import * added more changes' * Removed FSV1 Client, Zk Connectivity, Pricing Client * Moved dev toggle for golang to parent and configured it to run for multi repo with interactive modes --------- Co-authored-by: Aditya Garg <aditya.garg@meesho.com>
…ack into feat/horizon_code_sync
…ted the horizon configs
…ack into feat/horizon_code_sync
…onfig options for better display and handling
…o feat/auth-and-permission
…nd authentication improvements - Introduced `ENV_VARIABLES.md` to document required and optional environment variables for backend and frontend, including JWT and SSO configurations. - Added `BACKEND_AUTH_IMPROVEMENTS.md` detailing fixes for hardcoded values, security issues, and code quality enhancements in the authentication system. - Created `BACKEND_CODE_QUALITY_ISSUES.md` to outline identified code quality issues and recommendations for improvement. - Established `BACKEND_FINAL_CHECKLIST.md` for a review of completed improvements and remaining issues. - Added `BACKEND_SECURITY_RECOMMENDATIONS.md` to provide security best practices and critical issues to address. - Implemented a new constants file to centralize configuration values for roles, providers, error messages, and more. - Enhanced OAuth configuration management in `oauth.go` for better handling of environment variables.
…o feat/auth-and-permission
…BharatMLStack into feat/auth-and-permission
f6eea88 to
03b33d4
Compare
WalkthroughThis pull request adds comprehensive single sign-on (Google OAuth), permission-based access control, and metadata management capabilities to both the Horizon backend and TruffleBox UI frontend. It includes new database migrations, repository layers, authentication handlers, permission/metadata controllers, and frontend components for managing SSO, tokens, permissions, and user roles. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TruffleBox as TruffleBox UI
participant Horizon as Horizon Backend
participant Google as Google OAuth
participant DB as Database
User->>TruffleBox: Click "Login with Google"
TruffleBox->>Horizon: GET /auth/initiate-google-oauth
Horizon->>Horizon: Generate CSRF state<br/>Store state + expiry
Horizon-->>TruffleBox: Return auth_url + state
TruffleBox->>TruffleBox: Store state in sessionStorage<br/>Redirect to Google auth_url
User->>Google: Authenticate & authorize
Google->>TruffleBox: Redirect to callback<br/>with code + state
TruffleBox->>TruffleBox: Retrieve stored state<br/>Validate match
TruffleBox->>Horizon: POST /auth/google-callback<br/>code + state
Horizon->>Horizon: Validate state<br/>Check expiry & delete
Horizon->>Google: POST token endpoint<br/>Exchange code for tokens
Google-->>Horizon: Return access_token +<br/>refresh_token (Google)
Horizon->>Google: GET userinfo endpoint<br/>Bearer access_token
Google-->>Horizon: Return user profile<br/>(email, name, etc.)
Horizon->>DB: Check user by email<br/>& auth_provider='google'
alt User exists
DB-->>Horizon: Return user record
else User not found
Horizon->>DB: Create user<br/>auth_provider='google'
DB-->>Horizon: Return new user
end
Horizon->>Horizon: Generate JWT access_token<br/>Generate refresh_token
Horizon->>DB: Save refresh_token<br/>with expiry
Horizon-->>TruffleBox: Return access_token +<br/>refresh_token + user info
TruffleBox->>TruffleBox: Store tokens<br/>in localStorage<br/>Schedule refresh
TruffleBox->>TruffleBox: Redirect to dashboard
TruffleBox-->>User: Dashboard loaded
sequenceDiagram
participant User
participant TruffleBox as TruffleBox UI
participant Horizon as Horizon Backend
participant DB as Database
Note over User: Token nearing expiry<br/>Scheduled refresh triggered
TruffleBox->>TruffleBox: Retrieve refresh_token<br/>from localStorage
TruffleBox->>Horizon: POST /auth/refresh-token<br/>refresh_token in body
Horizon->>DB: Query refresh_token<br/>by token value
alt Token exists & not expired
DB-->>Horizon: Return token record
Horizon->>DB: Invalidate old<br/>refresh_token
Horizon->>Horizon: Generate new<br/>access_token +<br/>refresh_token pair
Horizon->>DB: Save new refresh_token
Horizon-->>TruffleBox: Return new tokens
TruffleBox->>TruffleBox: Update localStorage<br/>with new tokens<br/>Reschedule refresh
else Token invalid/expired
DB-->>Horizon: Not found or expired
Horizon-->>TruffleBox: Return 401/400 error
TruffleBox->>TruffleBox: Clear auth state<br/>Redirect to login
TruffleBox-->>User: Re-authenticate required
end
sequenceDiagram
participant SuperAdmin as Super Admin User
participant TruffleBox as TruffleBox UI
participant Horizon as Horizon Backend
participant DB as Database
SuperAdmin->>TruffleBox: Navigate to<br/>Permission Management
TruffleBox->>TruffleBox: Check role == 'super_admin'
alt Not super_admin
TruffleBox-->>SuperAdmin: Show Access Denied
else Super admin
TruffleBox->>Horizon: GET /permissions
Horizon->>DB: Query all permissions
DB-->>Horizon: Return permissions
Horizon-->>TruffleBox: Return permissions list
TruffleBox->>Horizon: GET /services<br/>GET /screen-types<br/>GET /actions
Horizon->>DB: Query metadata
DB-->>Horizon: Return services,<br/>screen-types, actions
Horizon-->>TruffleBox: Return metadata
TruffleBox->>TruffleBox: Render permission grid<br/>with filters & tables
SuperAdmin->>TruffleBox: Select role + service<br/>+ screen-type +<br/>allowed actions
TruffleBox->>TruffleBox: Build permission request
TruffleBox->>Horizon: POST /permissions<br/>permission object
Horizon->>DB: Validate service active<br/>Validate screen-type<br/>exists & belongs to service<br/>Validate all action IDs exist
alt Validation passes
DB-->>Horizon: Metadata valid
Horizon->>DB: Insert permission
DB-->>Horizon: Return created ID
Horizon-->>TruffleBox: Return created permission
TruffleBox->>TruffleBox: Refresh permission list
TruffleBox-->>SuperAdmin: Show success
else Validation fails
DB-->>Horizon: Error
Horizon-->>TruffleBox: Return 400 error
TruffleBox-->>SuperAdmin: Show error message
end
end
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
…nd security recommendations
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
dev-toggle-go.sh (1)
135-211:⚠️ Potential issue | 🔴 CriticalCritical: Unresolved Git merge conflict markers render the script non-functional.
The script contains unresolved merge conflicts at multiple locations. The shell interpreter will fail to parse/execute this file.
Conflict locations:
Lines Description 135-211 select_internal_repo_branch()function vs modifiedprint_usage()237-243 Branch argument documentation 248-252 Usage examples 385-462 Branch handling in clone_or_update_internal_repo()471-478 Clone with/without -b "$target_branch"519-538 File patterns ( .goonly vs.go+ configs)565-569 findcommand execution578-654 go.modprocessing (replace only vs require+replace)678-686 Horizon test instructions 836-861 Main script branch selection Resolve all conflict markers (
<<<<<<< HEAD,=======,>>>>>>> f6eea88...) before merging.Also applies to: 237-243, 248-252, 385-462, 471-478, 519-538, 565-569, 578-654, 678-686, 836-861
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-toggle-go.sh` around lines 135 - 211, The file contains unresolved git conflict markers that break the shell script; remove all conflict markers and reconcile the differing branches by choosing the intended logic for each conflicted area (notably the select_internal_repo_branch function, print_usage declaration, clone_or_update_internal_repo behavior, branch handling for git clone (-b usage), file pattern choices (.go vs .go + configs), the find invocation, go.mod processing (require+replace vs replace-only), and the Horizon/test instructions), then ensure the final code compiles by keeping only one coherent implementation for each symbol (select_internal_repo_branch, print_usage, clone_or_update_internal_repo, the go.mod processing block, and the main branch selection) and running a shellcheck/syntax check to validate there are no leftover <<<<<<<, =======, >>>>>>> markers.trufflebox-ui/src/pages/Header/index.jsx (1)
391-394:⚠️ Potential issue | 🟡 MinorInconsistent route path usage.
Line 393 uses a hardcoded
'/login'path while other parts of the codebase useAPP_ROUTES.LOGIN. This should be consistent.🔧 Proposed fix
+import { APP_ROUTES } from '../../constants/authConstants'; // ... const handleLogout = () => { logout(); - navigate('/login'); + navigate(APP_ROUTES.LOGIN); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@trufflebox-ui/src/pages/Header/index.jsx` around lines 391 - 394, handleLogout uses a hardcoded route string; replace the literal '/login' with the shared route constant to be consistent. In the handleLogout function (which calls logout() and navigate(...)), change the navigate argument from '/login' to APP_ROUTES.LOGIN and ensure APP_ROUTES is imported/available in this module so the function uses the central route constant.quick-start/db-init/scripts/init-etcd.sh (1)
147-155:⚠️ Potential issue | 🟡 MinorDuplicate catalog configuration will cause redundant writes.
Lines 147-155 duplicate the exact same etcd writes as lines 15-22 (catalog component configuration). This appears to be accidentally left in after refactoring.
🧹 Proposed fix - remove duplicate block
echo " ✅ etcd configuration key '/config/onfs' created successfully" else echo " ❌ Failed to create etcd configuration key '/config/onfs'" exit 1 fi -echo " 📋 Creating /config/horizon/inferflow/inferflow-components/catalog configuration key..." -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/component-id "catalog_id" -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/composite-id "false" -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/execution-dependency "feature_initializer" -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/fs-flatten-res-keys/0 "catalog_id" -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/fs-id-schema-to-value-columns/0/data-type "FP32" -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/fs-id-schema-to-value-columns/0/schema "catalog_id" -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/fs-id-schema-to-value-columns/0/value-col "catalog_id" -etcdctl --endpoints=http://etcd:2379 put /config/horizon/inferflow/inferflow-components/catalog/override-component/reel '{"component-id":"reel:derived_int32:reel__hero_catalog_id"}' - echo " ✅ etcd initialization completed successfully"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quick-start/db-init/scripts/init-etcd.sh` around lines 147 - 155, Remove the duplicated etcdctl block that re-writes the catalog component config: the second occurrence that echoes "Creating /config/horizon/inferflow/inferflow-components/catalog configuration key..." and the subsequent etcdctl --endpoints=... put commands for keys under /config/horizon/inferflow/inferflow-components/catalog (including component-id, composite-id, execution-dependency, fs-flatten-res-keys/0, fs-id-schema-to-value-columns/0/* and override-component/reel) should be deleted so the catalog configuration is only written once.horizon/internal/middleware/resolver/predator_resolver.go (1)
4-102:⚠️ Potential issue | 🔴 CriticalUnresolved merge conflict - file will not compile.
This file contains unresolved merge conflict markers (
<<<<<<< HEAD,=======,>>>>>>>) that must be resolved before merging. The conflict appears to be between two different resolver implementations - one with deployable/model/model-approval screen types and another with a more extensive set of resolvers.Please resolve the merge conflict by choosing the appropriate implementation or combining both as needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/middleware/resolver/predator_resolver.go` around lines 4 - 102, Remove the unresolved git conflict markers and produce a single coherent implementation for the Predator resolver: delete the <<<<<<<, =======, >>>>>>> blocks and either pick/merge the two implementations so that one consistent type (either predatorResolver or PredatorResolver) is defined, NewPredatorServiceResolver returns that type, and all referenced constants (e.g., screenTypePredatorDeployable, screenTypeModel, screenTypeApproval, moduleView, moduleOnboard, moduleApprove, moduleTest, servicePredator) and resolver name constants (e.g., resolverPredatorDeployableDiscovery, resolverModelDiscovery, resolverModelRequestApprove, etc.) are present and used in the GetResolvers map; ensure GetResolvers returns a single map[string]Func with no duplicate keys and that StaticResolver calls use the correct module strings (e.g., "upload" vs moduleUpload/review) to match the chosen implementation.horizon/internal/auth/controller/controller.go (1)
96-105:⚠️ Potential issue | 🟠 MajorAllow
super_adminthrough this authorization check.This PR adds
super_admin, butGetAllUsersstill rejects every non-adminrole. That leaves the new top-level admin role unable to load the user-management page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/controller/controller.go` around lines 96 - 105, The authorization check in AuthController.GetAllUsers currently only allows role == "admin" and rejects others; update the check that inspects the role (returned from controller.ParseAuthenticationHeader) to permit both "admin" and "super_admin" (e.g., change the if condition that sets the error when role != "admin" to instead error only when role is neither "admin" nor "super_admin") so super_admin can access the endpoint; keep the existing error handling (ctx.Error, ctx.JSON) intact.quick-start/db-init/scripts/init-mysql.sh (2)
13-25:⚠️ Potential issue | 🔴 CriticalRemove the duplicated columns from these table definitions.
entity,feature_group,features, andjobeach repeat one or more column names in the sameCREATE TABLE. MySQL will stop on the first duplicate-column error, so the bootstrap never gets past schema creation.Also applies to: 35-45, 55-65, 74-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quick-start/db-init/scripts/init-mysql.sh` around lines 13 - 25, Several CREATE TABLE definitions (entity, feature_group, features, job) contain duplicated column declarations (e.g., in entity the columns request_type, service and reject_reason are declared twice); remove the repeated column definitions so each column name appears only once per table. Locate the CREATE TABLE blocks for entity, feature_group, features and job in init-mysql.sh, delete the duplicate column lines (keeping one definition of each unique column such as request_type, service, reject_reason, etc.), and ensure the remaining column list is syntactically valid (commas and NOT NULL qualifiers preserved) so MySQL will not error on duplicate columns during schema creation.
900-924:⚠️ Potential issue | 🟠 MajorKeep seeded service names lowercase here as well.
This row seeds
service='INFERFLOW', while the rest of the PR and the Horizon service metadata use lowercase names. That inconsistency is easy to miss and can break comparisons/lookups once the value leaves MySQL.Based on learnings: Within the BharatMLStack horizon codebase, ensure that service names (inferflow, predator, numerix) are stored in lowercase in database tables and are consistently used in lowercase throughout the code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quick-start/db-init/scripts/init-mysql.sh` around lines 900 - 924, The seeded row in service_connection_config uses an uppercase service value ('INFERFLOW') which must be lowercase to match the rest of the code; update the VALUES clause for id 3 in the INSERT IGNORE into service_connection_config so the service column is 'inferflow' (and verify other seed entries like predator/numerix follow lowercase), ensuring consistency with any code paths that compare the service field (e.g., service_connection_config.service lookups).horizon/internal/middleware/resolver/inferflow_resolver.go (1)
4-109:⚠️ Potential issue | 🔴 CriticalCritical: Unresolved merge conflict markers in source file.
This file contains unresolved Git merge conflict markers (
<<<<<<< HEAD,=======,>>>>>>>). The code will not compile. The conflict must be resolved by choosing between:
- HEAD version (lines 4-52): New approach with
mp-config,deployable,connection-configscreen types andinferflowResolver(unexported)- Incoming version (lines 54-108): Previous approach with
inferflow-configscreen types andInferflowResolver(exported)Both versions correctly use lowercase
"inferflow"for the service name, which is compliant with repository conventions. Based on learnings, service names should be stored in lowercase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/middleware/resolver/inferflow_resolver.go` around lines 4 - 109, The file contains unresolved Git conflict markers; remove the markers and reconcile which implementation to keep (either the HEAD variant using screenTypeInferFlowDeployable/connection-config/mp-config constants and inferflowResolver with NewInferFlowServiceResolver/resolverInferFlow* names, or the incoming variant using screenTypeInferflowConfig*/InferflowResolver with NewInferflowServiceResolver/resolverInferflow* names). Pick the correct variant for this service (ensure only one set of screenType/resolver constants and a single resolver struct and constructor remain), delete the other block and the conflict markers, and verify GetResolvers returns the matching resolver constant names and module constants for the chosen implementation. Ensure the serviceInferflow/serviceInferFlow identifier casing matches repository convention (lowercase "inferflow") across the kept code.horizon/internal/middleware/middleware.go (1)
116-132:⚠️ Potential issue | 🔴 CriticalCritical: Malformed control flow in public route bypass logic.
The
if isPublicRoute {block starting at line 124 is missing a closing brace andreturnstatement before the duplicate condition check on lines 126-129. This causes the subsequent auth logic to execute even for public routes.🐛 Proposed fix
if isPublicRoute { - // Bypass authentication for login, register, and specific routes - if strings.HasPrefix(c.Request.URL.Path, "/login") || - strings.HasPrefix(c.Request.URL.Path, "/register") || - strings.HasPrefix(c.Request.URL.Path, "/health") || - strings.HasPrefix(c.Request.URL.Path, "/api/1.0/fs-config") { c.Next() return }The duplicate condition (lines 126-129) is redundant since
PublicRoutesalready includes/login,/register,/health, and/api/1.0/fs-config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/middleware/middleware.go` around lines 116 - 132, The public-route bypass logic in middleware.go incorrectly leaves the if isPublicRoute { block open and then rechecks specific paths redundantly; fix the control flow in the middleware by closing the isPublicRoute branch and immediately calling c.Next() and return when isPublicRoute is true (so requests matched via constants.PublicRoutes skip auth), and remove the duplicated explicit path checks (the /login, /register, /health, /api/1.0/fs-config conditional block) to avoid redundant/erroneous execution; locate the logic around the isPublicRoute variable, constants.PublicRoutes loop, and the c.Next()/return handling to apply this change.horizon/internal/auth/handler/auth.go (1)
17-17:⚠️ Potential issue | 🟠 MajorMigrate from deprecated dgrijalva/jwt-go to golang-jwt/jwt.
github.com/dgrijalva/jwt-gois unmaintained and has known security vulnerabilities (CVE-2020-26160: CVSS 7.5 High access restriction bypass in audience validation). Migrate github.com/golang-jwt/jwt/v5(or v4 as a drop-in replacement). Update the import path and rungo get github.com/golang-jwt/jwt/v5 && go mod tidy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/handler/auth.go` at line 17, The import of the unmaintained "github.com/dgrijalva/jwt-go" must be replaced with the maintained module "github.com/golang-jwt/jwt/v5"; update the import in auth.go and any other files using jwt, then run "go get github.com/golang-jwt/jwt/v5 && go mod tidy" and adjust any API calls if needed (search for usages like jwt.Parse, jwt.NewWithClaims, jwt.SigningMethod*, jwt.MapClaims) to match the v5 function signatures/types.
🟠 Major comments (17)
horizon/go.mod-39-48 (1)
39-48:⚠️ Potential issue | 🟠 MajorRemove duplicated
requireentries; this complicates dependency management and may cause confusion with tools likego listandgo mod tidy.Several identical module/version pairs are declared twice in the same
requireblock. While Go's module parser handles duplicates correctly (by using the last occurrence), having duplicate entries is still a maintainability issue that can lead to confusion and inconsistency in dependency resolution.Suggested fix (keep one copy of each module in the second
requireblock)require ( - cel.dev/expr v0.24.0 // indirect - cloud.google.com/go v0.121.6 // indirect - cloud.google.com/go/auth v0.17.0 // indirect - cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect - cloud.google.com/go/compute/metadata v0.9.0 // indirect - cloud.google.com/go/iam v1.5.2 // indirect - cloud.google.com/go/monitoring v1.24.2 // indirect - github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.29.0 // indirect - github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.53.0 // indirect - github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.53.0 // indirect @@ - github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect - github.com/envoyproxy/go-control-plane/envoy v1.32.4 // indirect - github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect - github.com/felixge/httpsnoop v1.0.4 // indirect - github.com/fsnotify/fsnotify v1.9.0 // indirect - github.com/gabriel-vasile/mimetype v1.4.9 // indirect - github.com/gin-contrib/sse v1.1.0 // indirect - github.com/go-jose/go-jose/v4 v4.1.2 // indirect - github.com/go-logr/logr v1.4.3 // indirect - github.com/go-logr/stdr v1.2.2 // indirect @@ - github.com/go-playground/validator/v10 v10.27.0 // indirect @@ - github.com/go-viper/mapstructure/v2 v2.4.0 // indirect - github.com/goccy/go-json v0.10.5 // indirect - github.com/goccy/go-yaml v1.18.0 // indirect @@ - github.com/klauspost/cpuid/v2 v2.3.0 // indirect @@ - github.com/mattn/go-colorable v0.1.14 // indirect @@ - github.com/pelletier/go-toml/v2 v2.2.4 // indirect - github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect - github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/quic-go/qpack v0.5.1 // indirect - github.com/quic-go/quic-go v0.54.0 // indirect - github.com/sagikazarmark/locafero v0.9.0 // indirect - github.com/sourcegraph/conc v0.3.0 // indirect - github.com/spf13/afero v1.14.0 // indirect - github.com/spf13/cast v1.9.2 // indirect - github.com/spf13/pflag v1.0.6 // indirect - github.com/spiffe/go-spiffe/v2 v2.5.0 // indirect @@ - github.com/ugorji/go/codec v1.3.0 // indirect - github.com/zeebo/errs v1.4.0 // indirect - go.etcd.io/etcd/api/v3 v3.5.12 // indirect - go.etcd.io/etcd/client/pkg/v3 v3.5.12 // indirect - go.opentelemetry.io/auto/sdk v1.1.0 // indirect - go.opentelemetry.io/contrib/detectors/gcp v1.36.0 // indirect - go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.62.0 // indirect - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.62.0 // indirect - go.opentelemetry.io/otel v1.38.0 // indirect - go.opentelemetry.io/otel/metric v1.38.0 // indirect - go.opentelemetry.io/otel/sdk v1.37.0 // indirect - go.opentelemetry.io/otel/sdk/metric v1.37.0 // indirect - go.opentelemetry.io/otel/trace v1.38.0 // indirect @@ - go.uber.org/mock v0.5.0 // indirect - go.uber.org/multierr v1.11.0 // indirect )Affected lines: 39–48, 79–88, 92, 97–99, 114, 117, 132–142, 161–173, 177–178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/go.mod` around lines 39 - 48, The go.mod has duplicated require entries (e.g., cel.dev/expr v0.24.0, cloud.google.com/go v0.121.6, cloud.google.com/go/auth v0.17.0, github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.53.0, etc.) across multiple require blocks; remove the duplicate lines so each module/version pair appears only once (prefer keeping the canonical copy in the second require block per the suggestion), scanning for duplicates in the require blocks and deleting the earlier/extra occurrences to leave one authoritative entry for symbols like cel.dev/expr, cloud.google.com/go, cloud.google.com/go/auth, cloud.google.com/go/compute/metadata, cloud.google.com/go/iam, cloud.google.com/go/monitoring, and github.com/GoogleCloudPlatform/opentelemetry-operations-go/*.quick-start/local-start.sh-409-410 (1)
409-410:⚠️ Potential issue | 🟠 MajorGo version check should be conditional on local build mode.
Line 409 currently blocks even remote-image startup for users without Go. This should only run when
--localor--all-localis active.✅ Proposed fix
echo "🚀 Starting BharatML Stack Quick Start..." -check_go_version setup_workspace @@ if [[ "$ENABLE_LOCAL_BUILD" = true ]]; then + check_go_version echo "🔨 Local build mode enabled"Also applies to: 447-449
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quick-start/local-start.sh` around lines 409 - 410, The script currently calls check_go_version unconditionally (seen near the sequence check_go_version / setup_workspace), which blocks remote-image startup for users without Go; modify the startup flow so check_go_version is executed only when local build flags are set (detect presence of --local or --all-local in the script's parsed flags) and leave setup_workspace and other steps untouched for remote-image modes; apply the same conditional guard around the other unconditional check_go_version invocation identified around lines 447-449 so both checks run only in local build scenarios.horizon/internal/repositories/sql/auth/repository.go-18-19 (1)
18-19:⚠️ Potential issue | 🟠 Major
updatedByis accepted but ignored in persistence.The new method signatures imply audit tracking, but Lines 107 and 113 only update
role/is_active. This drops updater metadata and weakens auditability.🛠️ Proposed fix
func (auth *Auth) UpdateUserRole(id uint, role string, updatedBy uint) error { - result := auth.db.Model(&User{}).Where("id = ?", id).Update("role", role) + result := auth.db.Model(&User{}).Where("id = ?", id).Updates(map[string]interface{}{ + "role": role, + "updated_by": updatedBy, + }) return result.Error } @@ func (auth *Auth) UpdateUserStatus(id uint, isActive bool, updatedBy uint) error { - result := auth.db.Model(&User{}).Where("id = ?", id).Update("is_active", isActive) + result := auth.db.Model(&User{}).Where("id = ?", id).Updates(map[string]interface{}{ + "is_active": isActive, + "updated_by": updatedBy, + }) return result.Error }Also applies to: 106-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/repositories/sql/auth/repository.go` around lines 18 - 19, UpdateUserRole and UpdateUserStatus accept updatedBy but the repository updates only role/is_active and drops updater metadata; modify the persistence updates in UpdateUserRole and UpdateUserStatus to set the audit fields (e.g., UpdatedBy and UpdatedAt) together with role/is_active: include updatedBy in the DB UPDATE statement or ORM call and set UpdatedAt to time.Now() (or use your existing timestamp helper) so both UpdateUserRole(id uint, role string, updatedBy uint) and UpdateUserStatus(id uint, isActive bool, updatedBy uint) persist the updater and timestamp along with the other changed fields.horizon/internal/middleware/resolver/embedding_platform_resolver.go-6-10 (1)
6-10:⚠️ Potential issue | 🟠 MajorMissing resolver mapping for
hierarchical-discoveryscreen type.
App.jsregisters an embedding-platform route withscreenType="hierarchical-discovery", but this resolver file does not expose a corresponding key. That creates an authorization resolution gap for that page.Suggested patch
const ( // Embedding Platform screen types screenTypeEmbeddingStoreDiscovery = "store-discovery" + screenTypeEmbeddingHierarchicalDiscovery = "hierarchical-discovery" screenTypeEmbeddingEntityDiscovery = "entity-discovery" @@ // Discovery resolvers resolverEmbeddingStoreDiscovery = "EmbeddingPlatformStoreDiscoveryResolver" + resolverEmbeddingHierarchicalDiscovery = "EmbeddingPlatformHierarchicalDiscoveryResolver" resolverEmbeddingEntityDiscovery = "EmbeddingPlatformEntityDiscoveryResolver" @@ return map[string]Func{ // Discovery - View resolverEmbeddingStoreDiscovery: StaticResolver(screenTypeEmbeddingStoreDiscovery, moduleView, serviceEmbeddingPlatform), + resolverEmbeddingHierarchicalDiscovery: StaticResolver(screenTypeEmbeddingHierarchicalDiscovery, moduleView, serviceEmbeddingPlatform), resolverEmbeddingEntityDiscovery: StaticResolver(screenTypeEmbeddingEntityDiscovery, moduleView, serviceEmbeddingPlatform),Also applies to: 67-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/middleware/resolver/embedding_platform_resolver.go` around lines 6 - 10, The resolver is missing a constant and mapping for the "hierarchical-discovery" screen type, causing authorization to skip that page; add a new constant (e.g. screenTypeEmbeddingHierarchicalDiscovery = "hierarchical-discovery") alongside the other screenTypeEmbedding* constants and include it in the resolver's mapping/place where screen types are exposed (the same block containing screenTypeEmbeddingEntityDiscovery, screenTypeEmbeddingModelDiscovery, screenTypeEmbeddingVariantDiscovery, screenTypeEmbeddingFilterDiscovery, screenTypeEmbeddingJobFrequencyDiscovery) so the hierarchical-discovery route from App.js is handled by the embedding platform resolver.horizon/internal/middleware/resolver/resolver_registry.go-15-21 (1)
15-21:⚠️ Potential issue | 🟠 MajorResolver registry contains duplicate and inconsistent InferFlow/Predator constructor entries.
NewPredatorServiceResolveris listed twice (lines 15 and 19), and bothNewInferFlowServiceResolver(line 16) andNewInferflowServiceResolver(line 21) are registered. The inconsistent casing and duplicate entries can cause key overwrite issues and stale symbol usage in the registry map.Suggested patch
resolverList := []func() (ServiceResolver, error){ - NewPredatorServiceResolver, - NewInferFlowServiceResolver, - NewEmbeddingPlatformServiceResolver, - NewOnlineFeatureStoreResolver, NewPredatorServiceResolver, + NewInferFlowServiceResolver, + NewEmbeddingPlatformServiceResolver, + NewOnlineFeatureStoreResolver, NewDeployableServiceResolver, - NewInferflowServiceResolver, NewNumerixServiceResolver, NewApplicationServiceResolver, NewSkyeServiceResolver, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/middleware/resolver/resolver_registry.go` around lines 15 - 21, The registry currently contains duplicate and inconsistent resolver constructors (NewPredatorServiceResolver appears twice and both NewInferFlowServiceResolver and NewInferflowServiceResolver are registered); remove the duplicate NewPredatorServiceResolver entry and normalize to a single, correctly-cased InferFlow constructor (choose and keep NewInferFlowServiceResolver or NewInferflowServiceResolver consistently across the registry), ensuring each resolver key is unique; update any surrounding code/imports to use the chosen symbol (e.g., NewInferFlowServiceResolver) and verify NewOnlineFeatureStoreResolver and NewDeployableServiceResolver remain present once.horizon/internal/repositories/sql/permissions/table.go-21-22 (1)
21-22:⚠️ Potential issue | 🟠 MajorRemove the
foreignKeytag from theCreatedByandUpdatedByfields.The
foreignKeytag in GORM is meant for association fields (e.g.,Creator *User) to specify which field serves as the foreign key, not for the FK fields themselves. These plainuintfields should simply be:CreatedBy uint `gorm:"not null"` UpdatedBy uint `gorm:"not null"`If
CreatedByandUpdatedByare meant to reference auserstable via association, define association fields with theforeignKeytag instead:Creator *User `gorm:"foreignKey:CreatedBy"` Updater *User `gorm:"foreignKey:UpdatedBy"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/repositories/sql/permissions/table.go` around lines 21 - 22, Remove the incorrect `foreignKey` GORM tags on the uint fields CreatedBy and UpdatedBy in the struct (they should be plain FK scalar fields with `not null`), and if you intend to create associations add pointer fields (e.g., Creator and Updater) that use `foreignKey:CreatedBy` and `foreignKey:UpdatedBy` respectively; update the struct by deleting `foreignKey:CreatedBy`/`foreignKey:UpdatedBy` from the CreatedBy and UpdatedBy field tags and optionally add association fields named Creator and Updater with the proper `foreignKey` tag.horizon/internal/repositories/sql/permissions/table.go-31-39 (1)
31-39:⚠️ Potential issue | 🟠 MajorGORM hooks require pointer receivers.
The
BeforeCreateandBeforeUpdatehooks must use pointer receivers(*Permission)instead of value receivers(Permission). Per official GORM documentation, value receivers prevent proper hook invocation and can cause hooks to not fire or result in runtime panics due to GORM's reflection-based callback mechanism expecting addressable pointer instances.🐛 Proposed fix
-func (Permission) BeforeCreate(tx *gorm.DB) (err error) { +func (p *Permission) BeforeCreate(tx *gorm.DB) (err error) { tx.Statement.SetColumn(createdAt, time.Now()) return } -func (Permission) BeforeUpdate(tx *gorm.DB) (err error) { +func (p *Permission) BeforeUpdate(tx *gorm.DB) (err error) { tx.Statement.SetColumn(updatedAt, time.Now()) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/repositories/sql/permissions/table.go` around lines 31 - 39, Change the GORM hook method receivers from value to pointer on the Permission type so hooks are invoked correctly: update the method signatures for BeforeCreate and BeforeUpdate to use (*Permission) (keeping the same parameters and bodies that call tx.Statement.SetColumn with createdAt and updatedAt respectively) so GORM's reflection can find and call these hooks on *Permission instances.horizon/internal/auth/config/oauth.go-34-34 (1)
34-34:⚠️ Potential issue | 🟠 MajorSSO enabled by default may cause unintended behavior.
SSOEnableddefaults totrue, meaning SSO will be active even if no Google OAuth credentials are configured. This could lead to confusion or broken authentication flows in environments where SSO wasn't intended to be enabled. Consider defaulting tofalseto require explicit opt-in.🔒 Proposed fix
- SSOEnabled: getEnvOrViperBool("SSO_ENABLED", true), + SSOEnabled: getEnvOrViperBool("SSO_ENABLED", false),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/config/oauth.go` at line 34, The SSOEnabled config field is defaulting to true which can enable SSO without credentials; update the initialization in oauth.go so SSOEnabled uses getEnvOrViperBool("SSO_ENABLED", false) (i.e., default to false) and add a brief inline comment near the SSOEnabled assignment to indicate SSO must be explicitly opted-in and requires proper Google OAuth credentials; ensure references to SSOEnabled elsewhere (auth config consumers) continue to work with the new default.trufflebox-ui/src/constants/authConstants.js-35-56 (1)
35-56:⚠️ Potential issue | 🟠 MajorAlign the frontend action vocabulary with the backend permission model.
ALL_ACTIONSbecomes the UI source of truth here, but this list omits action names that the backend still seeds/uses in this PR (review,register,fetch). That makes permission editing lossy and prevents the UI from representing the full backend permission set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@trufflebox-ui/src/constants/authConstants.js` around lines 35 - 56, The frontend permission constants are missing backend actions causing a mismatch: add the missing action keys ('review', 'register', 'fetch') into the PERMISSION_ACTIONS object so that ALL_ACTIONS (which is Object.values(PERMISSION_ACTIONS)) includes them; update the PERMISSION_ACTIONS entries (alongside existing symbols like VIEW, EDIT, ONBOARD, etc.) to include REVIEW: 'review', REGISTER: 'register', and FETCH: 'fetch' so the UI can represent and edit the full backend permission set.horizon/internal/auth/controller/controller.go-216-237 (1)
216-237:⚠️ Potential issue | 🟠 MajorDon't return a fake success from
TrackSession.This endpoint accepts arbitrary session data, persists nothing, and still responds with
"Session tracked successfully". That will silently drop audit/session telemetry while making the frontend believe tracking worked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/controller/controller.go` around lines 216 - 237, The TrackSession handler currently accepts arbitrary JSON into the local request struct and always returns success; instead validate the parsed fields, generate or use request.UserID/request.UserID, persist the session record via the appropriate session store/service (e.g., call a SessionStore/AuthService method such as TrackSession or CreateSession) including timestamp and UserAgent, handle and log persistence errors (returning 4xx/5xx on failure) and on success return the persisted session_id and created timestamp; ensure you replace the fake success path and do not respond 200 unless persistence succeeds, and keep references to the existing TrackSession method, the request struct, ctx.BindJSON and ctx.GetString("session_id") when wiring the real storage call.horizon/internal/repositories/sql/permissions/repository.go-113-128 (1)
113-128:⚠️ Potential issue | 🟠 Major
BulkUpdatePermissionsByRolelacks transactional safety.The delete-then-create pattern without a transaction is risky. If any
Createcall fails after theDelete, the role loses all permissions with no way to recover. This could result in users being locked out.🐛 Proposed fix using GORM transaction
func (p *PermissionRepository) BulkUpdatePermissionsByRole(role string, permissionList []Permission) error { - // Delete existing permissions for the role - if err := p.db.Where("role = ?", role).Delete(&Permission{}).Error; err != nil { - return err - } - - // Create new permissions - for _, permission := range permissionList { - permission.Role = role - if err := p.db.Create(&permission).Error; err != nil { - return err + return p.db.Transaction(func(tx *gorm.DB) error { + // Delete existing permissions for the role + if err := tx.Where("role = ?", role).Delete(&Permission{}).Error; err != nil { + return err } - } - - return nil + + // Create new permissions + for i := range permissionList { + permissionList[i].Role = role + if err := tx.Create(&permissionList[i]).Error; err != nil { + return err + } + } + + return nil + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/repositories/sql/permissions/repository.go` around lines 113 - 128, BulkUpdatePermissionsByRole currently deletes then creates without a transaction; wrap the whole delete-and-create sequence in a GORM transaction so failure rolls back. Start a tx via tx := p.db.Begin() (or p.db.Transaction with a closure), perform tx.Where("role = ?", role).Delete(&Permission{}) and then loop using tx.Create(&permission) after setting permission.Role, and if any operation errors return and rollback; finally commit the tx (or return the error from the Transaction closure). Use unique symbols PermissionRepository, BulkUpdatePermissionsByRole, p.db (or tx) to locate and replace the current implementation.horizon/internal/auth/constants/constants.go-40-43 (1)
40-43:⚠️ Potential issue | 🟠 MajorSecurity concern: Hardcoded default JWT secret.
Having a default JWT secret in source code is risky, even with warning comments. If the environment variable isn't set, the application may silently use this weak, publicly-known secret. Consider failing fast at startup if
JWT_SECRET_KEYis not set, rather than falling back to a default.♻️ Suggested approach
Remove the default and enforce environment variable:
- // Default JWT secret (should NEVER be used in production) - // This is only for development - production MUST set JWT_SECRET_KEY env var - DefaultJWTSecret = "horizon-admin-secret"In the initialization code, panic or return an error if the secret is not configured:
jwtSecret := os.Getenv("JWT_SECRET_KEY") if jwtSecret == "" { log.Fatal("JWT_SECRET_KEY environment variable is required") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/constants/constants.go` around lines 40 - 43, Remove the hardcoded DefaultJWTSecret constant and stop falling back to a default; instead require the JWT secret be provided via the JWT_SECRET_KEY environment variable and fail fast at startup if it's missing. Replace references that read DefaultJWTSecret to read the env var (os.Getenv("JWT_SECRET_KEY")) and, in your application initialization (where JWT is configured/initialized), add a check that logs an error and exits (e.g., log.Fatal or panic) when the env value is empty so the process will not start with a weak/default secret; update any codepaths that assume DefaultJWTSecret to use the required env variable instead.horizon/internal/auth/constants/constants.go-131-143 (1)
131-143:⚠️ Potential issue | 🟠 MajorReview online-feature-store endpoints for potential authorization bypass.
The
/auth/refreshendpoint is correctly designed—it's public but requires a refresh token in the request body and validates it against the database.However, two of the three online-feature-store endpoints lack authorization checks:
/api/v1/online-feature-store/get-source-mappingis safe; it validates thejobTokenparameter against the database before returning data/api/v1/online-feature-store/get-online-features-mappingaccepts a request body but performs no token/authorization validation before returning feature mappings/api/v1/online-feature-store/retrieve-feature-groupsaccepts an entity parameter but performs no access control before returning feature group metadataConsider whether these endpoints should require authentication or implement authorization checks before making feature store configuration publicly accessible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/constants/constants.go` around lines 131 - 143, PublicRoutes currently exposes two online-feature-store endpoints without authorization; remove the endpoints from the PublicRoutes slice or add authorization checks in their handlers: add token/permission validation in the handlers that serve /api/v1/online-feature-store/get-online-features-mapping and /api/v1/online-feature-store/retrieve-feature-groups (mirror the validation used by /api/v1/online-feature-store/get-source-mapping or /auth/refresh: extract the jobToken/refresh token from request body/header, verify it against the database/session store, and return 401/403 if invalid). Update the PublicRoutes constant (variable PublicRoutes) or implement the checks in the corresponding handler functions (getOnlineFeaturesMapping, retrieveFeatureGroups) so feature store configuration is not returned without authorization.horizon/internal/externalcall/ring_master_client.go-132-145 (1)
132-145:⚠️ Potential issue | 🟠 MajorMissing validation for environment format can cause panic.
The environment string is split by
"_"andparts[1]is accessed without checking if the split produced enough parts. If the environment string doesn't contain an underscore, this will panic with an index out of range error. OnlyRestartDeployableandGetResourceDetailvalidate this.Proposed fix
func (r *ringmasterClientImpl) GetConfig(serviceName, workflowID, runID string) Config { parts := strings.Split(r.Environment, "_") + if len(parts) < 2 { + log.Error().Msgf("invalid environment format: %s", r.Environment) + return Config{ + MinReplica: "0", + MaxReplica: "0", + RunningStatus: "false", + } + } url := fmt.Sprintf("%s/api/v1/mlp/application/resource/%s-%s/HorizontalPodAutoscaler?workingEnv=%s", r.BaseURL, parts[1], serviceName, r.Environment)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/externalcall/ring_master_client.go` around lines 132 - 145, In GetConfig, r.Environment is split into parts and parts[1] is used without validation which can panic; add a guard after parts := strings.Split(r.Environment, "_") to check len(parts) > 1 (or otherwise handle unexpected format) and if the format is invalid return the default Config (same shape as the current early-return) or construct the URL using a safe fallback (e.g., parts[0] or the whole r.Environment), ensuring no index out of range occurs when accessing parts[1]; update the function GetConfig and any related logging to surface the invalid environment format.horizon/internal/auth/handler/permissions.go-212-249 (1)
212-249:⚠️ Potential issue | 🟠 MajorBulkUpdatePermissionsByRole has weaker validation than single create/update.
Unlike
CreatePermissionandUpdatePermission, the bulk update doesn't validate that services, screen types, and actions are active. This inconsistency could allow creating permissions referencing inactive metadata.Proposed fix to add isActive validation
for i, req := range permissionList { // Validate each permission - _, err := p.metadataRepo.GetServiceByID(req.ServiceID) + service, err := p.metadataRepo.GetServiceByID(req.ServiceID) if err != nil { return fmt.Errorf("invalid service_id for permission %d: %w", i, err) } + if !service.IsActive { + return fmt.Errorf("service for permission %d is not active", i) + } - _, err = p.metadataRepo.GetScreenTypeByID(req.ScreenTypeID) + screenType, err := p.metadataRepo.GetScreenTypeByID(req.ScreenTypeID) if err != nil { return fmt.Errorf("invalid screen_type_id for permission %d: %w", i, err) } + if !screenType.IsActive { + return fmt.Errorf("screen type for permission %d is not active", i) + } - _, err = p.metadataRepo.GetActionsByIDs(req.AllowedActions) + actions, err := p.metadataRepo.GetActionsByIDs(req.AllowedActions) if err != nil { return fmt.Errorf("invalid action IDs for permission %d: %w", i, err) } + for _, action := range actions { + if !action.IsActive { + return fmt.Errorf("action '%s' for permission %d is not active", action.Name, i) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/handler/permissions.go` around lines 212 - 249, BulkUpdatePermissionsByRole currently only checks existence of service/screen/actions via metadataRepo.GetServiceByID, GetScreenTypeByID and GetActionsByIDs but does not verify the metadata are active; update the validation to mirror CreatePermission/UpdatePermission by checking the returned service and screenType objects' IsActive flags and ensuring every action returned by GetActionsByIDs is active (iterate the actions slice and fail if any inactive), and return descriptive fmt.Errorf messages (e.g. "service is inactive for permission %d", "screen type is inactive for permission %d", "action id X is inactive for permission %d") before marshalling and constructing permissions; keep using the same metadataRepo methods and the PermissionRequest index i to produce consistent error messaging.trufflebox-ui/src/pages/Auth/AuthContext.jsx-194-257 (1)
194-257:⚠️ Potential issue | 🟠 MajorMissing
logoutin dependency array causes stale closure.The
refreshTokencallback useslogout()(line 219, 229, 253) but doesn't include it in the dependency array (line 257). This can cause stale closure bugs where an outdatedlogoutreference is called.Proposed fix
} finally { isRefreshingRef.current = false; } - }, []); + }, [logout]);Note: This will require reordering the function definitions or using
useRefto avoid circular dependency, sincelogoutalso depends onrefreshTokenindirectly through the cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@trufflebox-ui/src/pages/Auth/AuthContext.jsx` around lines 194 - 257, The refreshToken useCallback closes over logout but doesn't list it in its dependency array, causing a stale closure; fix by ensuring logout is a stable reference (wrap logout in useCallback and include any of its dependencies) and add logout to refreshToken's dependency array, or alternatively store logout in a ref (e.g., logoutRef.current) and read that in refreshToken to avoid circular deps with scheduleTokenRefresh/logout; update references to use the chosen stable logout (logout or logoutRef.current) and adjust scheduleTokenRefresh/cleanup ordering accordingly so there are no circular dependency issues.horizon/internal/auth/handler/handler.go-16-34 (1)
16-34:⚠️ Potential issue | 🟠 MajorPackage-level initialization may use default JWT key unexpectedly.
JwtKeyis assigned viagetJWTKey()at package initialization time (beforemain()runs). If viper configuration is loaded later during application startup, this will always fall back toconstants.DefaultJWTSecreteven when the environment is properly configured.Consider lazy initialization or ensuring the JWT key is retrieved after configuration is loaded.
Proposed fix using lazy initialization
var ( authOnce sync.Once authenticator Authenticator - JwtKey = getJWTKey() + jwtKeyOnce sync.Once + jwtKey []byte ) +// GetJWTKey returns the JWT signing key, initializing it on first call +func GetJWTKey() []byte { + jwtKeyOnce.Do(func() { + jwtKey = loadJWTKey() + }) + return jwtKey +} + // getJWTKey retrieves JWT secret key from environment variable or uses default -func getJWTKey() []byte { +func loadJWTKey() []byte {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@horizon/internal/auth/handler/handler.go` around lines 16 - 34, The package-level JwtKey is being set at init time by getJWTKey(), which can run before viper is configured; change to lazy initialization: remove the immediate assignment to JwtKey and implement a GetJWTKey() accessor that uses a sync.Once or checks a package-scoped jwtKey variable and calls getJWTKey() on first access (or after viper is loaded) so the value reflects configuration loaded at startup; update call sites to use GetJWTKey() instead of the JwtKey variable and keep getJWTKey() as the loader function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f56ec3dd-0941-4f08-8cb0-e76bd0c42d1c
⛔ Files ignored due to path filters (1)
horizon/go.sumis excluded by!**/*.sum
📒 Files selected for processing (71)
ENV_VARIABLES.mddev-toggle-go.shhorizon/cmd/horizon/main.gohorizon/docs/BACKEND_AUTH_IMPROVEMENTS.mdhorizon/docs/BACKEND_CODE_QUALITY_ISSUES.mdhorizon/docs/BACKEND_FINAL_CHECKLIST.mdhorizon/docs/BACKEND_REVIEW_SUMMARY.mdhorizon/docs/BACKEND_SECURITY_RECOMMENDATIONS.mdhorizon/env.examplehorizon/go.modhorizon/internal/auth/config/oauth.gohorizon/internal/auth/constants/constants.gohorizon/internal/auth/controller/controller.gohorizon/internal/auth/controller/metadata.gohorizon/internal/auth/controller/permissions.gohorizon/internal/auth/handler/auth.gohorizon/internal/auth/handler/handler.gohorizon/internal/auth/handler/metadata.gohorizon/internal/auth/handler/models.gohorizon/internal/auth/handler/oauth.gohorizon/internal/auth/handler/permissions.gohorizon/internal/auth/router/router.gohorizon/internal/externalcall/ring_master_client.gohorizon/internal/externalcall/ring_master_client_stub.gohorizon/internal/inferflow/handler/adaptor.gohorizon/internal/jobs/bulkdeletestrategy/predator_service.gohorizon/internal/jobs/bulkdeletestrategy/strategy_selector.gohorizon/internal/middleware/middleware.gohorizon/internal/middleware/resolver/config.gohorizon/internal/middleware/resolver/embedding_platform_resolver.gohorizon/internal/middleware/resolver/inferflow_resolver.gohorizon/internal/middleware/resolver/numerix_resolver.gohorizon/internal/middleware/resolver/online_feature_store_resolver.gohorizon/internal/middleware/resolver/predator_resolver.gohorizon/internal/middleware/resolver/resolver_registry.gohorizon/internal/online-feature-store/handler/online_feature_store.gohorizon/internal/predator/init.gohorizon/internal/repositories/sql/auth/repository.gohorizon/internal/repositories/sql/metadata/repository.gohorizon/internal/repositories/sql/metadata/table.gohorizon/internal/repositories/sql/permissions/repository.gohorizon/internal/repositories/sql/permissions/table.gohorizon/internal/repositories/sql/predatorconfig/table.gohorizon/internal/repositories/sql/token/repository.gohorizon/internal/repositories/sql/token/table.gohorizon/migrations/001_add_sso_fields.sqlquick-start/db-init/scripts/init-etcd.shquick-start/db-init/scripts/init-mysql.shquick-start/docker-compose.ymlquick-start/local-start.shtrufflebox-ui/README.updated.mdtrufflebox-ui/build/asset-manifest.jsontrufflebox-ui/build/env.jstrufflebox-ui/env.exampletrufflebox-ui/package.jsontrufflebox-ui/src/App.jstrufflebox-ui/src/config.jstrufflebox-ui/src/constants/authConstants.jstrufflebox-ui/src/constants/serviceMapping.jstrufflebox-ui/src/docs/AUTH_IMPROVEMENTS_SUMMARY.mdtrufflebox-ui/src/docs/AUTH_SECURITY.mdtrufflebox-ui/src/docs/HARDCODED_VALUES_FIXES.mdtrufflebox-ui/src/pages/Auth/AuthContext.jsxtrufflebox-ui/src/pages/Auth/Login.jsxtrufflebox-ui/src/pages/Auth/ProtectedRoute.jsxtrufflebox-ui/src/pages/Auth/Unauthorized.jsxtrufflebox-ui/src/pages/Header/index.jsxtrufflebox-ui/src/pages/PermissionManagement/index.jsxtrufflebox-ui/src/pages/UserManagement/index.jsxtrufflebox-ui/src/services/httpInterceptor.jstrufflebox-ui/src/services/ssoService.js
💤 Files with no reviewable changes (1)
- horizon/internal/inferflow/handler/adaptor.go
| horizonConfig "github.com/Meesho/BharatMLStack/horizon/internal" | ||
| applicationRouter "github.com/Meesho/BharatMLStack/horizon/internal/application/route" | ||
| horizonConfig "github.com/Meesho/BharatMLStack/horizon/internal" | ||
| applicationRouter "github.com/Meesho/BharatMLStack/horizon/internal/application/route" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
sed -n '1,90p' horizon/cmd/horizon/main.go
python - <<'PY'
from pathlib import Path
text = Path("horizon/cmd/horizon/main.go").read_text()
checks = {
'horizonConfig import alias occurrences': text.count('horizonConfig "github.com/Meesho/BharatMLStack/horizon/internal"'),
'applicationRouter import alias occurrences': text.count('applicationRouter "github.com/Meesho/BharatMLStack/horizon/internal/application/route"'),
'type AppConfig struct occurrences': text.count('type AppConfig struct'),
'GetStaticConfig occurrences': text.count('func (cfg *AppConfig) GetStaticConfig() interface{} {'),
'GetDynamicConfig occurrences': text.count('func (cfg *AppConfig) GetDynamicConfig() interface{} {'),
}
for name, count in checks.items():
print(f"{name}: {count}")
var_block = text.split('var (', 1)[1].split(')', 1)[0]
print('raw import path inside var block:', '"github.com/Meesho/BharatMLStack/horizon/pkg/scheduler"' in var_block)
PYRepository: Meesho/BharatMLStack
Length of output: 4338
Remove duplicate imports, type declarations, and fix the var block.
The file contains three compile-blocking errors:
- Duplicate imports (lines 6–9):
horizonConfigandapplicationRouterare imported twice. - Duplicate type and methods (around lines 41–65):
AppConfigstruct and its methodsGetStaticConfig()andGetDynamicConfig()are declared twice. - Invalid var block (line 50): Raw string literal
"github.com/Meesho/BharatMLStack/horizon/pkg/scheduler"is not valid Go syntax.
Delete the duplicate declarations and correct the var block to contain only valid variable declarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@horizon/cmd/horizon/main.go` around lines 6 - 9, Remove the duplicated import
entries for horizonConfig and applicationRouter so each package is imported only
once, delete the duplicated AppConfig struct definition and its duplicate
methods GetStaticConfig and GetDynamicConfig (keep a single correct definition
and implementation), and fix the malformed var block that currently contains a
raw string literal (e.g., remove the stray raw string or replace it with a valid
variable declaration or proper import) so the var block only contains valid Go
variable declarations; search for symbols AppConfig, GetStaticConfig,
GetDynamicConfig, horizonConfig, applicationRouter, and the var block to locate
and correct the duplicates.
| func NewPermissionController() *PermissionController { | ||
| return &PermissionController{ | ||
| PermissionHandler: handler.InitPermissionHandler(), | ||
| Authenticator: handler.InitAuthHandler(), // Inject authenticator to avoid hacky pattern | ||
| } | ||
| } |
There was a problem hiding this comment.
Nil handler checks missing after initialization.
Both InitPermissionHandler() and InitAuthHandler() can return nil on error. Using nil handlers will cause panics when methods are called.
Proposed fix
func NewPermissionController() *PermissionController {
+ permHandler := handler.InitPermissionHandler()
+ authHandler := handler.InitAuthHandler()
+ if permHandler == nil || authHandler == nil {
+ log.Error().Msg("Failed to initialize permission controller handlers")
+ return nil
+ }
return &PermissionController{
- PermissionHandler: handler.InitPermissionHandler(),
- Authenticator: handler.InitAuthHandler(), // Inject authenticator to avoid hacky pattern
+ PermissionHandler: permHandler,
+ Authenticator: authHandler,
}
}📝 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.
| func NewPermissionController() *PermissionController { | |
| return &PermissionController{ | |
| PermissionHandler: handler.InitPermissionHandler(), | |
| Authenticator: handler.InitAuthHandler(), // Inject authenticator to avoid hacky pattern | |
| } | |
| } | |
| func NewPermissionController() *PermissionController { | |
| permHandler := handler.InitPermissionHandler() | |
| authHandler := handler.InitAuthHandler() | |
| if permHandler == nil || authHandler == nil { | |
| log.Error().Msg("Failed to initialize permission controller handlers") | |
| return nil | |
| } | |
| return &PermissionController{ | |
| PermissionHandler: permHandler, | |
| Authenticator: authHandler, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@horizon/internal/auth/controller/permissions.go` around lines 19 - 24,
NewPermissionController currently assumes handler.InitPermissionHandler() and
handler.InitAuthHandler() never return nil; change its signature to return
(*PermissionController, error), call both InitPermissionHandler and
InitAuthHandler into local variables, check each for nil and return a
descriptive error (e.g., fmt.Errorf("InitPermissionHandler returned nil") or
wrap underlying error) if any are nil, and only then construct and return the
&PermissionController{PermissionHandler: ..., Authenticator: ...}; update
callers to handle the returned error.
| func InitMetadataHandler() *MetadataHandler { | ||
| connection, _ := infra.SQL.GetConnection() | ||
| sqlConn := connection.(*infra.SQLConnection) | ||
| metadataRepo, err := metadata.NewRepository(sqlConn) | ||
| if err != nil { | ||
| log.Error().Msgf("Error in creating metadata repository: %v", err) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Ignoring SQL connection error could cause nil pointer dereference.
Same issue as in permissions.go: the error from infra.SQL.GetConnection() is ignored, which can lead to a panic if the connection fails.
Proposed fix
func InitMetadataHandler() *MetadataHandler {
- connection, _ := infra.SQL.GetConnection()
+ connection, err := infra.SQL.GetConnection()
+ if err != nil {
+ log.Error().Msgf("Error getting SQL connection: %v", err)
+ return nil
+ }
sqlConn := connection.(*infra.SQLConnection)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@horizon/internal/auth/handler/metadata.go` around lines 15 - 22,
InitMetadataHandler currently ignores the error returned by
infra.SQL.GetConnection(), risking a nil pointer dereference; update the
function to capture the returned error from infra.SQL.GetConnection() (e.g.,
conn, err := infra.SQL.GetConnection()), check err and if non-nil log the error
with context using log.Error().Msgf and return nil, and only then assert the
connection to *infra.SQLConnection and proceed to call metadata.NewRepository;
ensure existing error handling around metadata.NewRepository remains intact
(referencing InitMetadataHandler, infra.SQL.GetConnection, infra.SQLConnection,
metadata.NewRepository, and log.Error().Msgf).
| func InitPermissionHandler() *PermissionHandler { | ||
| connection, _ := infra.SQL.GetConnection() | ||
| sqlConn := connection.(*infra.SQLConnection) | ||
| permissionRepo, err := permissions.NewRepository(sqlConn) | ||
| if err != nil { | ||
| log.Error().Msgf("Error in creating permission repository: %v", err) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Ignoring SQL connection error could cause nil pointer dereference.
The error from infra.SQL.GetConnection() is silently ignored. If the connection fails, connection will be nil, and the type assertion on line 21 will panic.
Proposed fix
func InitPermissionHandler() *PermissionHandler {
- connection, _ := infra.SQL.GetConnection()
+ connection, err := infra.SQL.GetConnection()
+ if err != nil {
+ log.Error().Msgf("Error getting SQL connection: %v", err)
+ return nil
+ }
sqlConn := connection.(*infra.SQLConnection)📝 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.
| func InitPermissionHandler() *PermissionHandler { | |
| connection, _ := infra.SQL.GetConnection() | |
| sqlConn := connection.(*infra.SQLConnection) | |
| permissionRepo, err := permissions.NewRepository(sqlConn) | |
| if err != nil { | |
| log.Error().Msgf("Error in creating permission repository: %v", err) | |
| return nil | |
| } | |
| func InitPermissionHandler() *PermissionHandler { | |
| connection, err := infra.SQL.GetConnection() | |
| if err != nil { | |
| log.Error().Msgf("Error getting SQL connection: %v", err) | |
| return nil | |
| } | |
| sqlConn := connection.(*infra.SQLConnection) | |
| permissionRepo, err := permissions.NewRepository(sqlConn) | |
| if err != nil { | |
| log.Error().Msgf("Error in creating permission repository: %v", err) | |
| return nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@horizon/internal/auth/handler/permissions.go` around lines 19 - 26,
InitPermissionHandler currently ignores the error from
infra.SQL.GetConnection(), which can leave connection nil and cause the type
assertion to panic; update InitPermissionHandler to capture and check the error
returned by infra.SQL.GetConnection(), log the error (using log.Error().Msgf
with the error) and return nil (or otherwise abort initialization) if it fails,
and only perform the type assertion to *infra.SQLConnection and call
permissions.NewRepository when the connection is non-nil and err is nil; also
handle or propagate any error from permissions.NewRepository consistently
(permissionRepo, err) to avoid nil dereferences.
| type Config struct { | ||
| MinReplica string `json:"min_replica"` | ||
| MaxReplica string `json:"max_replica"` | ||
| RunningStatus string `json:"running_status"` | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all Config declarations in the externalcall package
ast-grep --pattern 'type Config struct {
$$$
}'
# Also check for any other ringmaster client files
fd -e go . horizon/internal/externalcall --exec grep -l "type Config"Repository: Meesho/BharatMLStack
Length of output: 19588
🏁 Script executed:
# Check build tags in the externalcall package files
for file in horizon/internal/externalcall/ring_master_client_stub.go horizon/internal/externalcall/ring_master_client.go horizon/internal/externalcall/config.go; do
echo "=== $file ==="
head -20 "$file" | grep -E "^//\s*\+build|^package"
doneRepository: Meesho/BharatMLStack
Length of output: 300
🏁 Script executed:
# Check the actual content and build tags
echo "=== ring_master_client_stub.go (first 40 lines) ==="
head -40 horizon/internal/externalcall/ring_master_client_stub.go
echo -e "\n=== ring_master_client.go (first 60 lines) ==="
head -60 horizon/internal/externalcall/ring_master_client.go
echo -e "\n=== config.go (full file) ==="
cat horizon/internal/externalcall/config.goRepository: Meesho/BharatMLStack
Length of output: 3476
Remove Config struct declaration from stub and implementation files to avoid redeclaration.
The Config struct is defined in three places within the externalcall package: config.go (always compiled), ring_master_client.go (meesho build), and ring_master_client_stub.go (non-meesho build). This causes a redeclaration error in both build variants. The struct should only be declared once in config.go and imported by both the stub and real implementation files where needed.
🧰 Tools
🪛 GitHub Actions: Horizon CI
[error] 28-28: Go build failed during tests: Config redeclared in this block.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@horizon/internal/externalcall/ring_master_client_stub.go` around lines 28 -
32, Remove the duplicate Config struct declaration from the stub and real
ring-master client files: the type named Config (fields MinReplica, MaxReplica,
RunningStatus) should only be declared once in config.go; delete the redundant
struct in ring_master_client_stub.go (and the real implementation if present),
and update any references to use the existing Config type from config.go (no
code changes needed beyond removing the duplicate type declarations and ensuring
the package import/usage remains unchanged).
| INSERT IGNORE INTO users (first_name, last_name, email, password_hash, role, is_active, auth_provider) | ||
| VALUES ('admin', 'admin', 'admin@admin.com', '\$2a\$10\$kYoMds9IsbvPNhJasKHO7.fTSosfbPhSAf7ElNQJ9pIa0iWBOt97e', 'super_admin', true, 'password'); |
There was a problem hiding this comment.
Do not ship a hardcoded super_admin credential.
This creates a predictable bootstrap super-admin with a repository-known password hash. Anyone who knows the default password can take over fresh environments.
Safer direction
- INSERT IGNORE INTO users (first_name, last_name, email, password_hash, role, is_active, auth_provider)
- VALUES ('admin', 'admin', 'admin@admin.com', '...', 'super_admin', true, 'password');
+ -- Read bootstrap admin identity/secret from environment or a one-time secret source.
+ -- If no bootstrap secret is provided, skip creating the default super_admin.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@quick-start/db-init/scripts/init-mysql.sh` around lines 625 - 626, The INSERT
IGNORE INTO users statement inserts a repo-known super_admin account; remove
this hardcoded bootstrap credential and replace with a secure bootstrapping
mechanism: stop inserting the default ('admin','admin',...,'super_admin') row,
and instead gate admin creation behind a runtime/CI-provided secret or explicit
migration step (e.g., require an environment variable or an initialization
script that generates a random password and outputs it once), or document an
interactive setup to create the first super_admin; locate the SQL INSERT IGNORE
INTO users ... VALUES (...) and remove or replace that line so no predictable
super_admin account is created at deploy time.
| # Step 1: Insert services | ||
| echo " 📦 Creating services metadata..." | ||
| mysql -hmysql -uroot -proot --skip-ssl testdb -e " | ||
| INSERT INTO services (name, display_name, description, created_by, updated_by) VALUES | ||
| ('predator', 'Predator', 'Model registry and deployment management service', $ADMIN_ID, $ADMIN_ID), | ||
| ('inferflow', 'InferFlow', 'Model proxy and inference flow management service', $ADMIN_ID, $ADMIN_ID), | ||
| ('numerix', 'Numerix', 'Matrix operations and numerical computing service', $ADMIN_ID, $ADMIN_ID), | ||
| ('embedding_platform', 'Embedding Platform', 'Vector database and embedding management platform', $ADMIN_ID, $ADMIN_ID), | ||
| ('online_feature_store', 'Online Feature Store', 'High-performance feature serving platform', $ADMIN_ID, $ADMIN_ID); | ||
| " | ||
|
|
||
| # Step 2: Insert screen types for each service | ||
| echo " 📺 Creating screen types metadata..." | ||
| mysql -hmysql -uroot -proot --skip-ssl testdb -e " | ||
| -- Predator screen types | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'deployable', 'Deployable', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'predator'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'model', 'Model Management', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'predator'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'model-approval', 'Model Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'predator'; | ||
|
|
||
| -- InferFlow screen types | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'deployable', 'Deployable', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'inferflow'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'connection-config', 'Connection Configuration', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'inferflow'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'mp-config', 'InferFlow Configuration', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'inferflow'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'mp-config-approval', 'Configuration Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'inferflow'; | ||
|
|
||
| -- Numerix screen types | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'numerix-config', 'Numerix Configuration', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'numerix'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'numerix-config-approval', 'Configuration Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'numerix'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'numerix-config-testing', 'Numerix Configuration Testing', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'numerix'; | ||
|
|
||
| -- Embedding Platform screen types | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'store-discovery', 'Store Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'entity-discovery', 'Entity Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'model-discovery', 'Model Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'variant-discovery', 'Variant Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'filter-discovery', 'Filter Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'job-frequency-discovery', 'Job Frequency Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'store-registry', 'Store Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'entity-registry', 'Entity Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'model-registry', 'Model Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'variant-registry', 'Variant Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'filter-registry', 'Filter Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'job-frequency-registry', 'Job Frequency Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'store-approval', 'Store Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'entity-approval', 'Entity Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'model-approval', 'Model Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'variant-approval', 'Variant Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'filter-approval', 'Filter Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'job-frequency-approval', 'Job Frequency Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'deployment-operations', 'Deployment Operations', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'onboard-variant-to-db', 'Onboard Variant to DB', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'onboard-variant-approval', 'Onboard Variant Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'embedding_platform'; | ||
|
|
||
| -- Online Feature Store screen types | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'feature-discovery', 'Feature Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'store-discovery', 'Store Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'job-discovery', 'Job Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'client-discovery', 'Client Discovery', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'store-registry', 'Store Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'entity-registry', 'Entity Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'feature-group-registry', 'Feature Group Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'feature-registry', 'Feature Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'job-registry', 'Job Registry', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| INSERT INTO screen_types (service_id, name, display_name, created_by, updated_by) | ||
| SELECT id, 'feature-approval', 'Feature Approval', $ADMIN_ID, $ADMIN_ID FROM services WHERE name = 'online_feature_store'; | ||
| " | ||
|
|
||
| # Step 3: Insert actions | ||
| echo " ⚡ Creating actions metadata..." | ||
| mysql -hmysql -uroot -proot --skip-ssl testdb -e " | ||
| INSERT INTO actions (name, display_name, category, created_by, updated_by) VALUES | ||
| ('view', 'View', 'crud', $ADMIN_ID, $ADMIN_ID), | ||
| ('edit', 'Edit', 'crud', $ADMIN_ID, $ADMIN_ID), | ||
| ('onboard', 'Create/Onboard', 'crud', $ADMIN_ID, $ADMIN_ID), | ||
| ('delete', 'Delete', 'crud', $ADMIN_ID, $ADMIN_ID), | ||
| ('clone', 'Clone', 'management', $ADMIN_ID, $ADMIN_ID), | ||
| ('upload', 'Upload', 'management', $ADMIN_ID, $ADMIN_ID), | ||
| ('upload_edit', 'Upload Edit', 'management', $ADMIN_ID, $ADMIN_ID), | ||
| ('upload_partial', 'Upload Partial', 'management', $ADMIN_ID, $ADMIN_ID), | ||
| ('promote', 'Promote', 'management', $ADMIN_ID, $ADMIN_ID), | ||
| ('scale_up', 'Scale Up', 'management', $ADMIN_ID, $ADMIN_ID), | ||
| ('validate', 'Validate', 'approval', $ADMIN_ID, $ADMIN_ID), | ||
| ('approve', 'Approve', 'approval', $ADMIN_ID, $ADMIN_ID), | ||
| ('reject', 'Reject', 'approval', $ADMIN_ID, $ADMIN_ID), | ||
| ('cancel', 'Cancel', 'approval', $ADMIN_ID, $ADMIN_ID), | ||
| ('test', 'Test', 'testing', $ADMIN_ID, $ADMIN_ID), | ||
| ('load_test', 'Load Test', 'testing', $ADMIN_ID, $ADMIN_ID), | ||
| ('deactivate', 'Deactivate', 'management', $ADMIN_ID, $ADMIN_ID); | ||
| " | ||
|
|
||
| # Step 4: Create default permissions for super_admin role using IDs | ||
| echo " 🔐 Creating default permissions for super_admin role..." | ||
| # Get all action IDs as JSON array | ||
| ALL_ACTION_IDS=$(mysql -hmysql -uroot -proot --skip-ssl testdb -sN -e "SELECT JSON_ARRAYAGG(id) FROM actions;") | ||
|
|
||
| mysql -hmysql -uroot -proot --skip-ssl testdb -e " | ||
| -- Predator permissions | ||
| INSERT INTO permissions (role, service_id, screen_type_id, allowed_actions, created_by, updated_by) | ||
| SELECT 'super_admin', s.id, st.id, '$ALL_ACTION_IDS', $ADMIN_ID, $ADMIN_ID | ||
| FROM services s | ||
| CROSS JOIN screen_types st | ||
| WHERE s.name = 'predator' AND st.service_id = s.id; | ||
|
|
||
| -- InferFlow permissions | ||
| INSERT INTO permissions (role, service_id, screen_type_id, allowed_actions, created_by, updated_by) | ||
| SELECT 'super_admin', s.id, st.id, '$ALL_ACTION_IDS', $ADMIN_ID, $ADMIN_ID | ||
| FROM services s | ||
| CROSS JOIN screen_types st | ||
| WHERE s.name = 'inferflow' AND st.service_id = s.id; | ||
|
|
||
| -- Numerix permissions | ||
| INSERT INTO permissions (role, service_id, screen_type_id, allowed_actions, created_by, updated_by) | ||
| SELECT 'super_admin', s.id, st.id, '$ALL_ACTION_IDS', $ADMIN_ID, $ADMIN_ID | ||
| FROM services s | ||
| CROSS JOIN screen_types st | ||
| WHERE s.name = 'numerix' AND st.service_id = s.id; | ||
|
|
||
| -- Embedding Platform permissions | ||
| INSERT INTO permissions (role, service_id, screen_type_id, allowed_actions, created_by, updated_by) | ||
| SELECT 'super_admin', s.id, st.id, '$ALL_ACTION_IDS', $ADMIN_ID, $ADMIN_ID | ||
| FROM services s | ||
| CROSS JOIN screen_types st | ||
| WHERE s.name = 'embedding_platform' AND st.service_id = s.id; | ||
|
|
||
| -- Online Feature Store permissions (if screen types exist) | ||
| INSERT INTO permissions (role, service_id, screen_type_id, allowed_actions, created_by, updated_by) | ||
| SELECT 'super_admin', s.id, st.id, '$ALL_ACTION_IDS', $ADMIN_ID, $ADMIN_ID | ||
| FROM services s | ||
| CROSS JOIN screen_types st | ||
| WHERE s.name = 'online_feature_store' AND st.service_id = s.id; |
There was a problem hiding this comment.
Create the metadata tables before seeding them.
This script seeds services, screen_types, actions, and permissions, but their DDL is not present in the schema section above. On a fresh quick-start database, the first insert here will fail because the tables do not exist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@quick-start/db-init/scripts/init-mysql.sh` around lines 632 - 801, The script
attempts to INSERT into services, screen_types, actions, and permissions before
those tables are guaranteed to exist; add or invoke the schema creation step so
the DDL for tables services, screen_types, actions, and permissions is created
prior to the INSERTs (either add CREATE TABLE statements for those tables at the
top of this script or call the existing schema SQL that defines them), and
ensure the SELECT/JSON aggregation that sets ALL_ACTION_IDS runs after actions
is created and populated so the permissions INSERTs use valid IDs.
| if [[ $START_NUMERIX == true ]]; then | ||
| perl -i.bak -pe 's|image: numerix:\$\{NUMERIX_VERSION:-latest\}|build:\n context: ../../numerix\n dockerfile: Dockerfile|' "$compose" | ||
| fi |
There was a problem hiding this comment.
Numerix local-build substitution pattern does not match compose image name.
Line 469 targets image: numerix:..., but compose uses image: ghcr.io/meesho/numerix:...; local build mode won’t rewrite Numerix.
✅ Proposed fix
- perl -i.bak -pe 's|image: numerix:\$\{NUMERIX_VERSION:-latest\}|build:\n context: ../../numerix\n dockerfile: Dockerfile|' "$compose"
+ perl -i.bak -pe 's|image: ghcr.io/meesho/numerix:\$\{NUMERIX_VERSION:-latest\}|build:\n context: ../../numerix\n dockerfile: Dockerfile|' "$compose"📝 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.
| if [[ $START_NUMERIX == true ]]; then | |
| perl -i.bak -pe 's|image: numerix:\$\{NUMERIX_VERSION:-latest\}|build:\n context: ../../numerix\n dockerfile: Dockerfile|' "$compose" | |
| fi | |
| if [[ $START_NUMERIX == true ]]; then | |
| perl -i.bak -pe 's|image: ghcr.io/meesho/numerix:\$\{NUMERIX_VERSION:-latest\}|build:\n context: ../../numerix\n dockerfile: Dockerfile|' "$compose" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@quick-start/local-start.sh` around lines 468 - 470, The perl substitution
invoked when START_NUMERIX is true currently matches only "image: numerix:...",
so update the regex in the perl -pe call to also accept the registry-prefixed
image name used in compose (e.g., "ghcr.io/meesho/numerix:...") by making the
registry prefix optional (for example using a non-capturing optional prefix like
(?:ghcr.io/meesho/)?). Modify the substitution invoked in the START_NUMERIX
block (the perl -i.bak -pe line) to match both plain "numerix" and
"ghcr.io/meesho/numerix" and keep the same replacement block (build: context:
../../numerix dockerfile: Dockerfile).
| window.env = { | ||
| // Add your environment variables here | ||
| API_URL: 'http://localhost:3000', | ||
| // Add other configuration as needed |
There was a problem hiding this comment.
API_URL points to the UI port, not the backend API.
Line 3 routes API traffic to localhost:3000 (frontend), which can break auth/permission API calls. It should target Horizon (for quick-start, localhost:8082).
💡 Proposed fix
window.env = {
// Add your environment variables here
- API_URL: 'http://localhost:3000',
+ API_URL: 'http://localhost:8082',
// Add other configuration as needed
}; 📝 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.
| window.env = { | |
| // Add your environment variables here | |
| API_URL: 'http://localhost:3000', | |
| // Add other configuration as needed | |
| window.env = { | |
| // Add your environment variables here | |
| API_URL: 'http://localhost:8082', | |
| // Add other configuration as needed | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@trufflebox-ui/build/env.js` around lines 1 - 4, window.env currently sets
API_URL to the frontend port (API_URL: 'http://localhost:3000'), causing API
calls to hit the UI instead of the backend; update the API_URL value in the
window.env object to point to the Horizon backend (for quick-start use
'http://localhost:8082') so backend/auth requests are routed correctly (modify
the API_URL entry in the env.js file).
| 'FeatureDiscovery': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'feature-discovery', requiredParentKey: 'FeatureStore' }, | ||
| 'StoreDiscovery': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'store-discovery', requiredParentKey: 'FeatureStore' }, | ||
| 'JobDiscovery': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'job-discovery', requiredParentKey: 'FeatureStore' }, | ||
| 'ClientDiscovery': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'client-discovery', requiredParentKey: 'FeatureStore' }, | ||
| 'StoreRegistry': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'store-registry', requiredParentKey: 'FeatureStore' }, | ||
| 'JobRegistry': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'job-registry', requiredParentKey: 'FeatureStore' }, | ||
| 'EntityRegistry': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'entity-registry', requiredParentKey: 'FeatureStore' }, | ||
| 'FeatureGroupRegistry': { service: SERVICES.ONLINE_FEATURE_STORE, screenType: 'feature-group-registry', requiredParentKey: 'FeatureStore' }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: detect duplicate keys inside MENU_PERMISSION_MAP
python - <<'PY'
import re, collections, pathlib
p = pathlib.Path("trufflebox-ui/src/constants/serviceMapping.js")
text = p.read_text()
m = re.search(r'export const MENU_PERMISSION_MAP = \{(.*?)\n\};', text, re.S)
block = m.group(1) if m else ""
keys = re.findall(r"'([^']+)'\s*:", block)
dups = [k for k,v in collections.Counter(keys).items() if v > 1]
print("Duplicate keys:", dups)
PYRepository: Meesho/BharatMLStack
Length of output: 134
🏁 Script executed:
cat -n trufflebox-ui/src/constants/serviceMapping.jsRepository: Meesho/BharatMLStack
Length of output: 6348
Duplicate keys prevent Embedding Platform from accessing its own permission mappings.
The keys StoreDiscovery (line 46), StoreRegistry (line 49), and EntityRegistry (line 51) override their earlier definitions at lines 26, 30, and 31. In a JavaScript object literal, the last definition wins, so the Embedding Platform entries with requiredParentKey: 'EmbeddingPlatform' are permanently lost.
When requiresPermissionCheck() or getPermissionInfo() lookup these keys, they return only the Online Feature Store variant with requiredParentKey: 'FeatureStore'. If any Embedding Platform code attempts to use these menu items with parent 'EmbeddingPlatform', the permission check will fail because the parent key will not match.
The comment on lines 24-25 acknowledges the intent to support both services, but the data structure does not allow a single key to map to multiple service contexts. This design must be refactored: either rename the duplicate keys to be unique per service (e.g., 'EmbeddingStoreDiscovery', 'FeatureStoreDiscovery') or restructure MENU_PERMISSION_MAP to support multi-context lookups.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@trufflebox-ui/src/constants/serviceMapping.js` around lines 45 - 52,
MENU_PERMISSION_MAP currently uses duplicate keys (StoreDiscovery,
StoreRegistry, EntityRegistry) which get overwritten by the Online Feature Store
entries; update the map so both contexts exist by renaming the Embedding
Platform variants to unique keys (e.g., EmbeddingStoreDiscovery,
EmbeddingStoreRegistry, EmbeddingEntityRegistry) or conversely rename the
Feature Store ones (e.g., FeatureStoreDiscovery, FeatureStoreRegistry,
FeatureEntityRegistry) and keep their existing service/requiredParentKey values;
then update any call sites that reference the old keys in
requiresPermissionCheck() and getPermissionInfo() to use the new unique key
names so lookups can distinguish EmbeddingPlatform vs FeatureStore entries.
🔁 Pull Request Template – BharatMLStack
📌 Summary
📂 Modules Affected
horizon(Real-time systems / networking)online-feature-store(Feature serving infra)trufflebox-ui(Admin panel / UI)infra(Docker, CI/CD, GCP/AWS setup)docs(Documentation updates)___________✅ Type of Change
___________📊 Benchmark / Metrics (if applicable)
Summary by CodeRabbit
Release Notes
New Features
Documentation