Skip to content

improvement(api): add config for sending audit logs to clickhouse#5458

Merged
maidul98 merged 4 commits intomainfrom
ENG-4567-send-audit-logs-to-clickhouse
Feb 12, 2026
Merged

improvement(api): add config for sending audit logs to clickhouse#5458
maidul98 merged 4 commits intomainfrom
ENG-4567-send-audit-logs-to-clickhouse

Conversation

@fangpenlin
Copy link
Contributor

@fangpenlin fangpenlin commented Feb 11, 2026

Context

https://linear.app/infisical/issue/ENG-4567/update-infisical-platform-backend-code-to-allow-sending-audit-logs-to

Screenshots

Steps to verify the change

  1. ClickHouse is added to .dev docker compose file
  2. add CLICKHOUSE_URL=http://default:clickhouse@clickhouse:8123 to your .env to enable clickhouse
  3. Restart your docker compose
  4. Do anything that will emit an audit log (literary anything)
  5. Check the audit_logs table in both ClickHouse and PostgreSQL, you should see the same id appears in both table

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Read the contributing guide

@linear
Copy link

linear bot commented Feb 11, 2026

@maidul98
Copy link
Collaborator

maidul98 commented Feb 11, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@fangpenlin fangpenlin force-pushed the ENG-4567-send-audit-logs-to-clickhouse branch 2 times, most recently from 4016f8c to 86e64c0 Compare February 11, 2026 22:39
@fangpenlin fangpenlin force-pushed the ENG-4567-send-audit-logs-to-clickhouse branch from 70008d7 to 424b1d6 Compare February 11, 2026 22:51
@fangpenlin fangpenlin marked this pull request as ready for review February 11, 2026 22:56
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 11, 2026

Greptile Overview

Greptile Summary

This PR adds ClickHouse as an optional audit log storage backend alongside the existing Postgres implementation. When configured via CLICKHOUSE_URL, audit logs are inserted into both databases in parallel.

Key Changes:

  • Added @clickhouse/client dependency and configuration through environment variables
  • Created ClickHouse schema migration that sets up an audit_logs table with ReplacingMergeTree engine
  • Modified audit log queue to insert into both Postgres and ClickHouse using Promise.allSettled
  • Added ClickHouse service to docker-compose for local development

Critical Issues Found:

  • Breaks existing functionality: The async function in Promise.allSettled is not invoked, preventing ALL Postgres audit log inserts from executing
  • Syntax error: Trailing comma in ClickHouse table schema will cause migration failures
  • SSRF vulnerability: CLICKHOUSE_URL accepts any URL without validation, allowing access to cloud metadata endpoints and internal services
  • SQL injection: Table name and engine parameters are directly interpolated into SQL without validation
  • Missing error handling: JSON parsing of insert settings can crash application startup

Documentation: No documentation added for this feature. Customers will need guidance on when to use ClickHouse, how to configure it, and what the trade-offs are versus Postgres-only audit logs.

Confidence Score: 0/5

  • This PR has critical bugs that will break audit log functionality and contains multiple security vulnerabilities
  • Score reflects multiple critical issues: (1) Promise.allSettled bug that breaks all Postgres audit log inserts, (2) SQL syntax error that will fail ClickHouse table creation, (3) SSRF vulnerability allowing access to internal services, (4) SQL injection vulnerability in table creation, and (5) missing error handling that could crash the application
  • Critical attention required for backend/src/ee/services/audit-log/audit-log-queue.ts (breaks Postgres inserts), backend/src/db/clickhouse-migration-runner.ts (syntax error + SQL injection), and backend/src/lib/config/clickhouse.ts (SSRF vulnerability)

Important Files Changed

Filename Overview
backend/src/db/clickhouse-migration-runner.ts Creates ClickHouse audit_logs table schema. Has critical syntax error (trailing comma) and SQL injection vulnerability via unsanitized table/engine parameters.
backend/src/lib/config/clickhouse.ts Creates ClickHouse client from config. Has SSRF vulnerability - URL is not validated and could point to internal services or metadata endpoints.
backend/src/ee/services/audit-log/audit-log-queue.ts Adds parallel ClickHouse insertion. Has critical bug - async function not invoked in Promise.allSettled, breaking Postgres inserts. Also has ID mismatch between databases.
backend/src/lib/config/env.ts Adds ClickHouse environment variables. JSON.parse lacks error handling and could crash startup with malformed input.
backend/src/auto-start-migrations.ts Integrates ClickHouse schema setup into startup migrations. Properly handles optional ClickHouse client and exits on error.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

15 files reviewed, 6 comments

Edit Code Review Agent Settings | Greptile

@fangpenlin fangpenlin requested a review from maidul98 February 12, 2026 00:07
@maidul98 maidul98 merged commit bd29b3c into main Feb 12, 2026
13 of 14 checks passed
@fangpenlin fangpenlin deleted the ENG-4567-send-audit-logs-to-clickhouse branch February 12, 2026 01:14
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.

2 participants