Skip to content

Fix audit-identity gaps: updated_by, org membership, UTC timestamps#2651

Merged
renuka-fernando merged 3 commits into
wso2:mainfrom
ashera96:audit-bug-fix
Jul 15, 2026
Merged

Fix audit-identity gaps: updated_by, org membership, UTC timestamps#2651
renuka-fernando merged 3 commits into
wso2:mainfrom
ashera96:audit-bug-fix

Conversation

@ashera96

Copy link
Copy Markdown
Contributor

Purpose

Follow-up to #2370 / #2371, which moved all audit-identity columns (created_by, updated_by, revoked_by, performed_by) to internal UUIDs resolved via user_idp_references. Retesting those flows end-to-end (UI → REST API → DB) surfaced several gaps:

  1. updated_by was never set on create for rest_apis, llm_providers, llm_proxies, and api_keys — it stayed NULL until the first edit. api_keys update/revoke also touched only updated_at, never updated_by.
  2. GET /organizations ignored user_organization_mappings entirely and returned every organization to every caller, unfiltered.
  3. Stale schema comments claimed user_organization_mappings has no ON DELETE CASCADE, contradicting the schema (which always had it).
  4. ChoreoUserContext parsed the paginated {count, list, pagination} org envelope as a single object, so id/name came out undefined; a second, uncalled copy of the same bug lived in platformApis.getOrganization().
  5. deleted_user fallback resolution (for a removed/repointed user) had no test coverage.
  6. Code review of the above surfaced a related gap: several timestamps (gateway associations, gateway updated_at, deployment performed_at) were computed with local-server time in the service layer instead of UTC, diverging from the sibling UTC columns in the same rows on any non-UTC host.

Goals

  • Audit Resolution: Ensure updated_by == created_by immediately after creating a REST API, LLM provider, LLM proxy, or API key. Ensure updated_by accurately reflects the acting user on every subsequent update/revoke.
  • Org Membership: Ensure GET /organizations returns only the organizations the caller is a member of; callers with ap:organization:manage keep list-all behavior. A caller's membership in a pre-existing/seeded org is healed lazily on first list.
  • Schema Accuracy: Match schema comments to actual FK behavior (ON DELETE CASCADE kept, SQLite FK enforcement hardened via ?_foreign_keys=on).
  • UI Parsing: Ensure ai-workspace correctly parses the organizations list, drops dead code, and surfaces createdBy/updatedBy on relevant detail views without ever round-tripping them back in update requests.
  • Timestamp Normalization: Ensure all timestamps that share a row with an existing UTC column are now computed/normalized in UTC.

Approach

  • Audit columns: Added UpdatedBy to the create paths for the four affected models; APIKeyRepository.Revoke now takes an updatedBy actor (signature change, all callers/mocks updated).
  • Org membership: OrganizationRepository gained ListOrganizationsForUser/CountOrganizationsForUser. The handler branches on middleware.HasEffectiveScope(r, mode, "ap:organization:manage") to pick list-all vs. membership-filtered, with a best-effort AddMembership heal (failures logged at Warn, not fatal) before listing.
  • UTC timestamps: Centralized the fix at the repository layer where possible (APIRepo.CreateAPIAssociation, GatewayRepo.UpdateGateway now compute time.Now().UTC() themselves, matching every other repo Create), which fixes every caller (REST API, WebBroker, WebSub) in one place; performedAt in the REST/LLM/MCP/WebBroker/WebSub deployment flows switched to time.Now().UTC().
  • Identity resolution: Batch ResolveIdentityFields wired into every list endpoint (not just orgs) to avoid one identity lookup per row.
  • UI: Fixed the org envelope parser, removed the dead getOrganization(), added "Created by" to the LLM Provider/Proxy, MCP External Server, and Provider Template overview pages, and stripped updatedBy from every update-payload builder now that the field exists on those types.

Full decision record available in tmp/PLAN-audit-followup.md (local, gitignored).


User stories

  • As a non-admin user, GET /organizations shows only orgs I belong to, not every org on the platform.
  • As an operator, I can see who created and who last updated a REST API, LLM provider/proxy, or API key immediately, not just after a first edit.
  • As a security reviewer, I can tell who revoked or rotated an API key.
  • As a user whose IdP account was removed, resources I created still show a sensible "deleted_user" audit trail instead of erroring or leaking an internal UUID.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a514f25c-a28b-4c2f-ac65-3cb8ffce9475

📥 Commits

Reviewing files that changed from the base of the PR and between c667b7b and 9cc2fa9.

📒 Files selected for processing (1)
  • platform-api/internal/repository/deployment.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • platform-api/internal/repository/deployment.go

📝 Walkthrough

Walkthrough

The changes add audit-field persistence and retrieval, normalize timestamps to UTC, restrict organization listings by authorization and membership, batch identity resolution in service responses, and update workspace organization retrieval and audit metadata handling.

Changes

Platform schema, audit persistence, and UTC handling

Layer / File(s) Summary
Schema and connection normalization
platform-api/internal/database/*
SQLite connections use UTC location settings, and timestamp defaults/types are aligned across database schemas.
Repository audit and timestamp flows
platform-api/internal/model/*, platform-api/internal/repository/*, platform-api/plugins/eventgateway/repository/*
Audit actors are persisted and retrieved for APIs, API keys, LLM resources, associations, and event-gateway resources; repository timestamps use UTC.
Persistence validation
platform-api/internal/repository/*_test.go, platform-api/internal/integration/*
Tests cover audit actors, UTC normalization, identity mappings, memberships, cascade cleanup, and API-key revocation.

Organization access control

Layer / File(s) Summary
Membership-scoped retrieval
platform-api/internal/repository/organization.go, platform-api/internal/repository/interfaces.go, platform-api/internal/service/organization.go
Organization listing and counting support membership filtering and membership healing.
Authorization-aware handlers
platform-api/internal/handler/organization.go, platform-api/internal/middleware/*, platform-api/internal/server/server.go
Organization requests select all organizations or membership-filtered results based on effective scope.
Organization tests
platform-api/internal/handler/organization_integration_test.go, platform-api/internal/service/organization_test.go, platform-api/internal/repository/user_organization_mapping_test.go
Tests cover scope filtering, membership creation, pagination, foreign-key failures, healing, and deletion cleanup.

Service and workspace integration

Layer / File(s) Summary
Batch identity and deployment mapping
platform-api/internal/service/*, platform-api/plugins/eventgateway/service/*
List responses batch-resolve creator identities, deleted mappings use the deleted-user marker, and deployment timestamps use UTC with millisecond precision.
Workspace organization and audit contracts
portals/ai-workspace/src/apis/*, portals/ai-workspace/src/contexts/*, portals/ai-workspace/src/utils/types.ts, portals/ai-workspace/src/pages/appShell/appShellPages/*
Workspace organization loading consumes paginated results, audit fields are represented in resource types, update payloads omit read-only metadata, and creator information is displayed conditionally.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant OrganizationHandler
  participant Authorization
  participant OrganizationService
  participant OrganizationRepository

  Client->>OrganizationHandler: GET /organizations
  OrganizationHandler->>Authorization: HasEffectiveScope(request, authzMode, manage scope)
  alt manage scope present
    OrganizationHandler->>OrganizationService: ListOrganizations(pagination)
    OrganizationService->>OrganizationRepository: List all organizations
  else no manage scope
    OrganizationHandler->>OrganizationService: ListOrganizationsForUser(actor, resolved organization, pagination)
    OrganizationService->>OrganizationRepository: Count and list memberships
    OrganizationService->>OrganizationRepository: AddMembership when resolved organization is missing
  end
  OrganizationRepository-->>OrganizationService: Organizations and count
  OrganizationService-->>OrganizationHandler: Organization response
  OrganizationHandler-->>Client: Paginated JSON response
Loading

Possibly related PRs

Suggested reviewers: pubudu538, malinthaprasan, virajsalaka, tharindu1st, renuka-fernando

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR also includes non-audit work such as org membership filtering and UI parsing, which are not covered by linked issue #2559. Move unrelated org/UI changes to a separate PR or link matching issues so this one stays focused on the audit fix.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.07% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, specific, and matches the PR's main audit-identity and timestamp changes.
Description check ✅ Passed The description covers purpose, goals, approach, and user stories, but omits several template sections like docs, tests, security, samples, related PRs, and environment.
Linked Issues check ✅ Passed The changes address #2559's audit-storage/retrieval fix, including updated_by persistence, UTC normalization, and identity-resolution coverage.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
platform-api/internal/repository/llm.go (1)

479-491: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Record updated_by for template rename and enable operations.

RenameFamily and SetEnabled update the entity and updated_at, but neither receives an actor nor writes updated_by. After these mutations, audit reads can report the previous user as the last editor. Thread the actor through the service/repository APIs and include updated_by in both UPDATE statements.

Based on the PR objective to initialize and update audit identity for LLM providers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform-api/internal/repository/llm.go` around lines 479 - 491, Update
RenameFamily and SetEnabled to accept an actor identity, thread it through their
service and repository callers, and write it to updated_by alongside the
existing updated_at mutation. Preserve the current update filters and
enabled/display_name behavior while ensuring both operations record the acting
user.
🧹 Nitpick comments (3)
platform-api/internal/handler/organization.go (1)

166-183: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Actor resolution unnecessarily gates the privileged list-all path.

performedBy is only consumed in the else branch (ListOrganizationsForUser); the ap:organization:manage branch doesn't use it at all. Resolving it unconditionally before the scope check means a transient identity-resolution failure blocks organization managers from listing all orgs, even though that path has no dependency on it.

♻️ Proposed refactor
 func (h *OrganizationHandler) ListOrganizations(w http.ResponseWriter, r *http.Request) error {
 	limit, offset := parsePagination(r)
 
-	// resolveActorErr also creates the caller's user_idp_references row on first
-	// use, which is what makes the membership heal below FK-safe.
-	performedBy, err := resolveActorErr(r, h.identity, "list organizations")
-	if err != nil {
-		return err
-	}
-
 	var orgs []api.Organization
 	var total int
+	var err error
 	if middleware.HasEffectiveScope(r, h.authzMode, "ap:organization:manage") {
 		orgs, total, err = h.orgService.ListOrganizations(limit, offset)
 	} else {
+		// resolveActorErr also creates the caller's user_idp_references row on
+		// first use, which is what makes the membership heal below FK-safe.
+		performedBy, actorErr := resolveActorErr(r, h.identity, "list organizations")
+		if actorErr != nil {
+			return actorErr
+		}
 		resolvedOrgUUID, _ := middleware.GetOrganizationFromRequest(r)
 		orgs, total, err = h.orgService.ListOrganizationsForUser(performedBy, resolvedOrgUUID, limit, offset)
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform-api/internal/handler/organization.go` around lines 166 - 183, Move
resolveActorErr in ListOrganizations inside the non-privileged else branch,
immediately before ListOrganizationsForUser, and declare performedBy there. Keep
the privileged ap:organization:manage path independent of actor resolution while
preserving the existing error handling for user-scoped listing.
platform-api/internal/repository/llm_audit_test.go (1)

31-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding update-path tests for LLM provider and proxy updated_by.

The Create+Read path is well covered, but there's no test verifying that Update changes UpdatedBy to the new actor while preserving CreatedBy — the API key repo has TestAPIKeyRepo_UpdateAndRevoke_SetUpdatedByWithoutTouchingCreatedBy for this pattern. Adding equivalent tests for LLM providers and proxies would close the coverage gap.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform-api/internal/repository/llm_audit_test.go` around lines 31 - 163,
Add update-path tests alongside TestLLMProviderRepo_CreateAndRead_SetsUpdatedBy
and TestLLMProxyRepo_CreateAndRead_SetsUpdatedBy that update each record with a
different actor, then verify UpdatedBy changes to that actor while CreatedBy
remains unchanged. Follow the existing API-key test pattern and validate the
persisted values through the repository read path.
platform-api/internal/service/api.go (1)

304-320: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Consider batching project-handle resolution too.

Identity resolution is now batched for the list response, but modelToRESTAPIUnresolved still does a per-item s.projectRepo.GetProjectByUUID(...) call for every API in the page (N+1). Not a regression from this diff — the same per-item lookup existed via the old modelToRESTAPI — but since the batching pattern for identity was just introduced here, extending it to project handles would close the remaining N+1 gap on this list endpoint.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform-api/internal/service/api.go` around lines 304 - 320, Extend the
list-response flow around modelToRESTAPIUnresolved to batch-resolve project
handles instead of calling projectRepo.GetProjectByUUID per API. Collect the
required project UUIDs while processing apiModels, resolve them in one
repository operation, and apply the results to the corresponding responses while
preserving existing missing-project and error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@platform-api/internal/handler/organization_integration_test.go`:
- Around line 53-59: Configure SQLite foreign-key enforcement for the entire
sqlDB pool in the test setup, rather than relying on the single-connection
PRAGMA Exec call. Update the sql.Open configuration around sqlDB to use the
supported DSN foreign-keys parameter, or set the pool to one connection with
SetMaxOpenConns(1); remove the per-connection PRAGMA call if the chosen
configuration makes it unnecessary.

In `@platform-api/internal/repository/api.go`:
- Around line 62-63: Update the timestamp initialization in the API creation
flow to capture one UTC timestamp and assign it to both CreatedAt and UpdatedAt,
preserving the equal-on-creation invariant used by CreateAPIAssociation and
APIKeyRepo.Create.

In `@platform-api/internal/service/organization_test.go`:
- Around line 42-48: Update the SQLite setup around sql.Open so foreign-key
enforcement applies consistently to every connection: add _foreign_keys=on to
the DSN, or constrain the sqlDB pool to a single connection. Remove the
connection-specific PRAGMA setup if the DSN option is used, while preserving the
existing failure handling and FK assertions.

In `@portals/ai-workspace/src/contexts/ChoreoUserContext.tsx`:
- Around line 90-94: Update the organization-fetching flow around the single
res.json() call to follow the pagination metadata and request subsequent pages
until pagination.total organizations are collected, merging each response’s list
rather than retaining only the first page. Preserve the existing organization
mapping behavior, and add a regression test covering multiple paginated
responses.

---

Outside diff comments:
In `@platform-api/internal/repository/llm.go`:
- Around line 479-491: Update RenameFamily and SetEnabled to accept an actor
identity, thread it through their service and repository callers, and write it
to updated_by alongside the existing updated_at mutation. Preserve the current
update filters and enabled/display_name behavior while ensuring both operations
record the acting user.

---

Nitpick comments:
In `@platform-api/internal/handler/organization.go`:
- Around line 166-183: Move resolveActorErr in ListOrganizations inside the
non-privileged else branch, immediately before ListOrganizationsForUser, and
declare performedBy there. Keep the privileged ap:organization:manage path
independent of actor resolution while preserving the existing error handling for
user-scoped listing.

In `@platform-api/internal/repository/llm_audit_test.go`:
- Around line 31-163: Add update-path tests alongside
TestLLMProviderRepo_CreateAndRead_SetsUpdatedBy and
TestLLMProxyRepo_CreateAndRead_SetsUpdatedBy that update each record with a
different actor, then verify UpdatedBy changes to that actor while CreatedBy
remains unchanged. Follow the existing API-key test pattern and validate the
persisted values through the repository read path.

In `@platform-api/internal/service/api.go`:
- Around line 304-320: Extend the list-response flow around
modelToRESTAPIUnresolved to batch-resolve project handles instead of calling
projectRepo.GetProjectByUUID per API. Collect the required project UUIDs while
processing apiModels, resolve them in one repository operation, and apply the
results to the corresponding responses while preserving existing missing-project
and error behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fd1547a0-7aeb-4ce5-93d9-4d566d50f33e

📥 Commits

Reviewing files that changed from the base of the PR and between d16ad96 and 3307e27.

📒 Files selected for processing (76)
  • platform-api/internal/database/schema.postgres.sql
  • platform-api/internal/database/schema.sql
  • platform-api/internal/database/schema.sqlite.sql
  • platform-api/internal/handler/gateway_internal.go
  • platform-api/internal/handler/organization.go
  • platform-api/internal/handler/organization_integration_test.go
  • platform-api/internal/handler/secret.go
  • platform-api/internal/integration/audit_identity_it_test.go
  • platform-api/internal/integration/crud_test.go
  • platform-api/internal/middleware/auth.go
  • platform-api/internal/middleware/authorization.go
  • platform-api/internal/model/apikey.go
  • platform-api/internal/model/user_organization_mapping.go
  • platform-api/internal/repository/api.go
  • platform-api/internal/repository/api_test.go
  • platform-api/internal/repository/apikey.go
  • platform-api/internal/repository/apikey_test.go
  • platform-api/internal/repository/application.go
  • platform-api/internal/repository/audit.go
  • platform-api/internal/repository/custom_policy.go
  • platform-api/internal/repository/deployment.go
  • platform-api/internal/repository/gateway.go
  • platform-api/internal/repository/gateway_test.go
  • platform-api/internal/repository/interfaces.go
  • platform-api/internal/repository/llm.go
  • platform-api/internal/repository/llm_audit_test.go
  • platform-api/internal/repository/organization.go
  • platform-api/internal/repository/project.go
  • platform-api/internal/repository/secret.go
  • platform-api/internal/repository/secret_test.go
  • platform-api/internal/repository/subscription_plan_repository.go
  • platform-api/internal/repository/subscription_repository.go
  • platform-api/internal/repository/user_identity_mapping.go
  • platform-api/internal/repository/user_organization_mapping.go
  • platform-api/internal/repository/user_organization_mapping_test.go
  • platform-api/internal/server/server.go
  • platform-api/internal/service/api.go
  • platform-api/internal/service/apikey.go
  • platform-api/internal/service/apikey_user.go
  • platform-api/internal/service/application.go
  • platform-api/internal/service/artifact_import.go
  • platform-api/internal/service/deployment.go
  • platform-api/internal/service/deployment_test.go
  • platform-api/internal/service/gateway.go
  • platform-api/internal/service/identity_deleted_user_test.go
  • platform-api/internal/service/llm.go
  • platform-api/internal/service/llm_apikey.go
  • platform-api/internal/service/llm_deployment.go
  • platform-api/internal/service/llm_proxy_apikey.go
  • platform-api/internal/service/mcp.go
  • platform-api/internal/service/mcp_deployment.go
  • platform-api/internal/service/organization.go
  • platform-api/internal/service/organization_test.go
  • platform-api/internal/service/project.go
  • platform-api/plugins/eventgateway/repository/webbroker_api.go
  • platform-api/plugins/eventgateway/repository/websub_api.go
  • platform-api/plugins/eventgateway/service/webbroker_api.go
  • platform-api/plugins/eventgateway/service/webbroker_api_deployment.go
  • platform-api/plugins/eventgateway/service/websub_api.go
  • platform-api/plugins/eventgateway/service/websub_api_deployment.go
  • portals/ai-workspace/src/apis/platformApis.ts
  • portals/ai-workspace/src/contexts/ChoreoUserContext.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/EditProviderTemplate.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/proxies/EditLLMProxy.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/EditServiceProvider.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderGuardrailsTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderModelsTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderRateLimitingTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderResourcesTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderSecurityTab.tsx
  • portals/ai-workspace/src/utils/types.ts
💤 Files with no reviewable changes (2)
  • platform-api/internal/service/artifact_import.go
  • portals/ai-workspace/src/apis/platformApis.ts

Comment thread platform-api/internal/handler/organization_integration_test.go Outdated
Comment thread platform-api/internal/repository/api.go Outdated
Comment thread platform-api/internal/service/organization_test.go Outdated
Comment thread portals/ai-workspace/src/contexts/ChoreoUserContext.tsx Outdated
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 13, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Validation Results

Dependency name: github.com/gorilla/websocket
Version: v1.5.4-0.20250319132907-e064f32e3674 (was v1.5.3)
Allowed range: >=v1.5.3
Approved: ✅ Yes

Dependency name: golang.org/x/crypto
Version: v0.54.0 (was v0.50.0)
Allowed range: >=v0.31.0
Approved: ✅ Yes

⚠️ Please verify the scope of the dependencies usage is necessary

@renuka-fernando

Copy link
Copy Markdown
Contributor

The SQLite connection opens without ?_loc=UTC:
db, err = sql.Open(DriverSQLite, cfg.Path)

Fix would be: sql.Open(DriverSQLite, cfg.Path+"?_loc=UTC")

Let's do that in a separate PR.

@ashera96

Copy link
Copy Markdown
Contributor Author

The SQLite connection opens without ?_loc=UTC: db, err = sql.Open(DriverSQLite, cfg.Path)

Fix would be: sql.Open(DriverSQLite, cfg.Path+"?_loc=UTC")

Let's do that in a separate PR.

Fixed via c667b7b. cc: @renuka-fernando

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
platform-api/internal/service/project.go (1)

189-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid taking addresses of slice elements during an append loop.

Although this is currently memory-safe because projects is pre-allocated with a capacity of len(projectModels) and will not reallocate, taking the address of a slice element (&projects[len(projects)-1].CreatedBy) during an append loop is a fragile pattern. If future code changes cause the slice to grow beyond its capacity and reallocate, createdByFields will silently hold pointers to the old, abandoned array. The identity resolver would then mutate the old array, and the resolved values would be lost in the returned slice.

Consider populating the pointers in a separate loop after all elements have been appended to guarantee they point to the final array.

♻️ Proposed refactor
 	projects := make([]api.Project, 0, len(projectModels))
-	createdByFields := make([]**string, 0, len(projectModels))
 	for _, projectModel := range projectModels {
 		apiProj, err := s.modelToAPIUnresolved(projectModel, org.Handle)
 		if err != nil {
 			return nil, 0, err
 		}
 		if apiProj == nil {
 			s.slogger.Warn("Failed to convert project model to API", "organizationId", organizationID)
 			continue
 		}
 		// updatedBy is detail-only; omit it from list responses.
 		apiProj.UpdatedBy = nil
 		projects = append(projects, *apiProj)
-		createdByFields = append(createdByFields, &projects[len(projects)-1].CreatedBy)
 	}
+
+	createdByFields := make([]**string, 0, len(projects))
+	for i := range projects {
+		createdByFields = append(createdByFields, &projects[i].CreatedBy)
+	}
+
 	if err := s.identity.ResolveIdentityFields(createdByFields); err != nil {
 		return nil, 0, err
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform-api/internal/service/project.go` around lines 189 - 207, Refactor
the project conversion loop in the relevant service method so it only appends
converted projects; after all appends complete, populate createdByFields in a
separate loop using addresses of elements in the finalized projects slice. Keep
the existing model conversion, UpdatedBy omission, warning, and identity
resolution behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@platform-api/internal/repository/deployment.go`:
- Around line 67-71: The deployment creation flow must normalize non-zero
deployment.CreatedAt values to UTC before persistence. In
platform-api/internal/repository/deployment.go lines 67-71, update the CreatedAt
handling to preserve caller-provided timestamps while converting them with
UTC(); in lines 343-350, ensure the corresponding deployment insert path
receives the normalized value, with no direct change needed if it is corrected
by the root-cause normalization.

---

Nitpick comments:
In `@platform-api/internal/service/project.go`:
- Around line 189-207: Refactor the project conversion loop in the relevant
service method so it only appends converted projects; after all appends
complete, populate createdByFields in a separate loop using addresses of
elements in the finalized projects slice. Keep the existing model conversion,
UpdatedBy omission, warning, and identity resolution behavior unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f63684aa-ecf3-4183-bf7f-96fc15aad24d

📥 Commits

Reviewing files that changed from the base of the PR and between d5c80c5 and c667b7b.

📒 Files selected for processing (81)
  • platform-api/internal/database/connection.go
  • platform-api/internal/database/schema.postgres.sql
  • platform-api/internal/database/schema.sql
  • platform-api/internal/database/schema.sqlite.sql
  • platform-api/internal/handler/api.go
  • platform-api/internal/handler/gateway_internal.go
  • platform-api/internal/handler/llm_deployment.go
  • platform-api/internal/handler/organization.go
  • platform-api/internal/handler/organization_integration_test.go
  • platform-api/internal/handler/secret.go
  • platform-api/internal/integration/audit_identity_it_test.go
  • platform-api/internal/integration/crud_test.go
  • platform-api/internal/middleware/auth.go
  • platform-api/internal/middleware/authorization.go
  • platform-api/internal/model/api.go
  • platform-api/internal/model/apikey.go
  • platform-api/internal/model/user_organization_mapping.go
  • platform-api/internal/repository/api.go
  • platform-api/internal/repository/api_test.go
  • platform-api/internal/repository/apikey.go
  • platform-api/internal/repository/apikey_test.go
  • platform-api/internal/repository/application.go
  • platform-api/internal/repository/audit.go
  • platform-api/internal/repository/custom_policy.go
  • platform-api/internal/repository/deployment.go
  • platform-api/internal/repository/gateway.go
  • platform-api/internal/repository/gateway_test.go
  • platform-api/internal/repository/interfaces.go
  • platform-api/internal/repository/llm.go
  • platform-api/internal/repository/llm_audit_test.go
  • platform-api/internal/repository/mcp.go
  • platform-api/internal/repository/organization.go
  • platform-api/internal/repository/project.go
  • platform-api/internal/repository/secret.go
  • platform-api/internal/repository/secret_test.go
  • platform-api/internal/repository/subscription_plan_repository.go
  • platform-api/internal/repository/subscription_repository.go
  • platform-api/internal/repository/user_identity_mapping.go
  • platform-api/internal/repository/user_organization_mapping.go
  • platform-api/internal/repository/user_organization_mapping_test.go
  • platform-api/internal/server/server.go
  • platform-api/internal/service/api.go
  • platform-api/internal/service/apikey.go
  • platform-api/internal/service/apikey_user.go
  • platform-api/internal/service/application.go
  • platform-api/internal/service/artifact_import.go
  • platform-api/internal/service/deployment.go
  • platform-api/internal/service/deployment_test.go
  • platform-api/internal/service/gateway.go
  • platform-api/internal/service/identity_deleted_user_test.go
  • platform-api/internal/service/llm.go
  • platform-api/internal/service/llm_apikey.go
  • platform-api/internal/service/llm_deployment.go
  • platform-api/internal/service/llm_proxy_apikey.go
  • platform-api/internal/service/mcp.go
  • platform-api/internal/service/mcp_deployment.go
  • platform-api/internal/service/organization.go
  • platform-api/internal/service/organization_test.go
  • platform-api/internal/service/project.go
  • platform-api/plugins/eventgateway/repository/webbroker_api.go
  • platform-api/plugins/eventgateway/repository/websub_api.go
  • platform-api/plugins/eventgateway/service/webbroker_api.go
  • platform-api/plugins/eventgateway/service/webbroker_api_deployment.go
  • platform-api/plugins/eventgateway/service/websub_api.go
  • platform-api/plugins/eventgateway/service/websub_api_deployment.go
  • portals/ai-workspace/src/apis/platformApis.ts
  • portals/ai-workspace/src/contexts/ChoreoUserContext.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/EditProviderTemplate.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/proxies/EditLLMProxy.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/EditServiceProvider.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderGuardrailsTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderModelsTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderOverview.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderRateLimitingTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderResourcesTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderSecurityTab.tsx
  • portals/ai-workspace/src/utils/types.ts
💤 Files with no reviewable changes (1)
  • portals/ai-workspace/src/apis/platformApis.ts
🚧 Files skipped from review as they are similar to previous changes (71)
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderSecurityTab.tsx
  • platform-api/internal/handler/api.go
  • platform-api/internal/repository/gateway_test.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsx
  • platform-api/internal/service/llm_proxy_apikey.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/EditServiceProvider.tsx
  • platform-api/internal/model/api.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/proxies/EditLLMProxy.tsx
  • platform-api/internal/repository/custom_policy.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderModelsTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyOverview.tsx
  • platform-api/internal/service/llm_apikey.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx
  • platform-api/internal/middleware/authorization.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderRateLimitingTab.tsx
  • platform-api/internal/service/apikey.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderOverview.tsx
  • platform-api/internal/handler/secret.go
  • platform-api/internal/handler/gateway_internal.go
  • platform-api/internal/repository/apikey_test.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderGuardrailsTab.tsx
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderResourcesTab.tsx
  • platform-api/internal/repository/user_organization_mapping.go
  • platform-api/internal/repository/secret_test.go
  • platform-api/plugins/eventgateway/repository/websub_api.go
  • platform-api/internal/model/user_organization_mapping.go
  • platform-api/internal/handler/organization_integration_test.go
  • platform-api/internal/middleware/auth.go
  • platform-api/internal/model/apikey.go
  • platform-api/internal/repository/subscription_plan_repository.go
  • platform-api/internal/repository/subscription_repository.go
  • platform-api/internal/handler/llm_deployment.go
  • platform-api/internal/handler/organization.go
  • platform-api/internal/service/mcp_deployment.go
  • platform-api/internal/database/schema.sql
  • platform-api/internal/service/apikey_user.go
  • platform-api/internal/repository/organization.go
  • platform-api/internal/repository/user_identity_mapping.go
  • platform-api/internal/repository/mcp.go
  • platform-api/internal/service/organization_test.go
  • platform-api/plugins/eventgateway/service/webbroker_api_deployment.go
  • platform-api/internal/repository/project.go
  • platform-api/internal/service/mcp.go
  • platform-api/internal/repository/llm_audit_test.go
  • platform-api/internal/server/server.go
  • platform-api/internal/service/deployment_test.go
  • platform-api/plugins/eventgateway/service/websub_api_deployment.go
  • platform-api/plugins/eventgateway/service/websub_api.go
  • platform-api/internal/service/application.go
  • platform-api/plugins/eventgateway/service/webbroker_api.go
  • platform-api/internal/repository/secret.go
  • platform-api/internal/repository/application.go
  • portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx
  • platform-api/internal/repository/api.go
  • platform-api/internal/database/schema.postgres.sql
  • platform-api/internal/service/artifact_import.go
  • platform-api/internal/database/schema.sqlite.sql
  • platform-api/internal/service/identity_deleted_user_test.go
  • platform-api/internal/service/deployment.go
  • platform-api/internal/service/gateway.go
  • platform-api/internal/repository/gateway.go
  • platform-api/internal/repository/user_organization_mapping_test.go
  • platform-api/internal/service/llm_deployment.go
  • platform-api/internal/service/organization.go
  • platform-api/internal/repository/interfaces.go
  • portals/ai-workspace/src/utils/types.ts
  • platform-api/internal/integration/audit_identity_it_test.go
  • platform-api/internal/service/llm.go
  • platform-api/internal/repository/apikey.go
  • platform-api/internal/service/api.go
  • platform-api/internal/repository/llm.go

Comment thread platform-api/internal/repository/deployment.go
…loyment repo

- In CreateWithLimitEnforcement, add else branch to call .UTC() on
  caller-provided CreatedAt values; the existing fix only covered the
  zero-value (default to now) path, leaving import-flow timestamps
  potentially in local time and breaking the eviction ORDER BY
  created_at ASC ordering on non-UTC hosts.
- In SetCurrentWithDetails, call .UTC() on the dereferenced performedAt
  pointer so the concurrency token is always UTC-normalized, matching
  the updatedAt column already written as time.Now().UTC().
@Thushani-Jayasekera

Copy link
Copy Markdown
Contributor

@CodeRabbit approve

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Comments resolved and changes approved.

@renuka-fernando
renuka-fernando merged commit 33f23ef into wso2:main Jul 15, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task]: Analyse and fix audit related data storage and retrieval in the platform-api

3 participants