EA-2327 Group-on-ClickHouse hardening (EA-2322/2323/2324 + adversarial fixes)

[wfieldx]

Consolidated hardening of the Group-on-ClickHouse path. The socket fix (EA-2320) ships separately in #2282 (cross-cutting - it also helps the AuditEvent write path).

Integrates + adversarially verifies

• EA-2322 - Mongo/ClickHouse split-brain compensation on write failure (+ a now-genuinely-running failure-injection test).
• EA-2323 - fail-closed + admin-exempt tenant filtering (403 for unscoped, matching SecurityTagManager); content/version-derived idempotency (deterministic event_id + argMax convergence; dropped the inert dedup token on the plain MergeTree); retry+jitter on the Group insert; surface ClickHouse read failures (no silent quantity:0). ADR docs/adr/0003.
• EA-2324 - strict concurrency assertions (verified no lost writes), real failure injection, activated the previously-commented-out MAX_GROUP_MEMBERS_PER_PUT guardrail (behavior: oversized PUT -> 400 too-costly), fixed stale perf wiring + added a CI perf smoke.

Adversarial integration testing (EA-2327) found + fixed

• BUG-1 (blocker): EA-2323's event_time = meta.lastUpdated (a Date) hit .replace() in the formatter -> every Group create/PUT/$merge 500'd. Fixed (normalize event_time). This is why EA-2323 Group ClickHouse hardening: idempotency, fail-closed tenancy, retry+jitter, surface read failures #2284 was red in isolation; EA-2322's compensation was masking it.
• BUG-2/3: EA-2322's split-brain test never actually ran (jest not imported under injectGlobals:false, wrong Mongo lookup). Fixed; now genuinely passes 3/3.
• Scoped coverage 64.6%->73.5% statements, 50.9%->61.5% branches; added a 41-test adversarial suite.

Known follow-up (NOT in this PR)

BUG-4 (EA-2326) - PUT/versioned member removal can be lost via a same-timestamp argMax tie (content-hash event_id). Repros committed as skipped; needs a causal-ordering fix + its own ADR.

Verified locally green (24 suites, 199 pass, 2 skipped BUG-4 repros); CI runs the full container suite. Latest main merged in.

Supersedes #2283, #2284, #2285. Refs EA-2322, EA-2323, EA-2324, EA-2327, EA-2326; epic DCON-3433.

[aikido-pr-checks]

restoreStrippedMembersInMongo was added to a pre-save helper and performs direct Mongo updates; move compensation/DB-repair logic to a dedicated compensation/service layer.

Details

✨ AI Reasoning
​clickHouseGroupPreSave previously handled in-memory pre-save transformations (tagging and stripping members). The diff adds restoreStrippedMembersInMongo which performs an actual MongoDB updateOne call to restore members as compensation for failed ClickHouse writes. This introduces a persistence/compensation responsibility into a module that was previously an in-memory pre-save helper, mixing transformation logic with side-effecting DB repair logic.

🔧 How do I fix it?
Split classes that handle database, HTTP, and UI concerns into separate, focused classes.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info}

[aikido-pr-checks]

Added a multi-getter ClickHouse configuration block (clickHouseIdleSocketTtl, clickHouseSendProgressInHttpHeaders). Consider extracting ClickHouse settings into a dedicated config module to reduce ConfigManager size and improve maintainability.

Details

✨ AI Reasoning
​A large configuration class has been expanded with a multi-property ClickHouse section (idle socket TTL and HTTP-header progress toggles). This adds several getters and explanatory comments into an already long, multi-concern module, increasing cognitive load and navigation difficulty. The new code mixes ClickHouse-specific connection tuning into a broad ConfigManager that already contains many unrelated settings, which makes the file harder to maintain and discover configuration related to ClickHouse in isolation. Splitting or extracting a ClickHouse-specific config helper would localize these responsibilities and keep ConfigManager focused.

🔧 How do I fix it?
Split large files into smaller, focused modules. Each file should have a single responsibility.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info}

[gecko-security]

Gecko PR Review

No vulnerabilities found, LGTM.

Summary

This PR hardens the ClickHouse Group membership dual-write path by fixing two critical security and reliability bugs: (1) correcting a fail-closed tenant isolation filter that incorrectly denied wildcard admins in mongoWithClickHouseStorageProvider.js and queryBuilder.js, and (2) removing reliance on ineffective ClickHouse deduplication tokens and implementing proper idempotency via content-derived insertion checks in groupMemberRepository.js and related handlers. The changes span query building, bulk write execution, pre/post-save handlers, and comprehensive test coverage for adversarial scenarios, concurrent writes, and compensation logic.

Important Files Changed

9 important files out of 34 changed

File Path	Why it matters
src/dataLayer/providers/mongoWithClickHouse/queryBuilder.js	Removes incorrect 1 = 0 deny clause that blocked legitimate wildcard admin (access/*.*) queries by misinterpreting empty security tags as authorization failure rather than full-access signal.
src/dataLayer/repositories/groupMemberRepository.js	Replaces ineffective ClickHouse insert_deduplication_token with application-level idempotency checks since MergeTree engine ignores dedup tokens; ensures safe retries on insert failures.
src/dataLayer/providers/mongoWithClickHouseStorageProvider.js	Coordinates MongoDB Group writes with ClickHouse member event appends, enforcing tenant isolation at query boundaries and applying compensating writes on partial failures.
src/dataLayer/postSaveHandlers/clickHouseGroupHandler.js	Implements async post-save compensation logic to reconcile MongoDB and ClickHouse state after transient failures, with retry and logging for observability.
src/dataLayer/bulkWriteExecutors/mongoBulkWriteExecutor.js	Handles bulk write failures and triggers ClickHouse compensation flow when MongoDB writes partially fail, maintaining consistency across dual stores.
docs/adr/0003-clickhouse-group-tenant-isolation-and-idempotency.md	Documents the design decisions correcting both tenant isolation and idempotency failures, including tracing of security tag propagation and ClickHouse engine limitations.
src/tests/group/group_clickhouse_adversarial.unit.test.js	Tests tenant isolation against adversarial queries (e.g., wildcard admins, empty tags, cross-tenant filters) to prevent regression of the fail-closed bug.
src/tests/group/group_clickhouse_compensation.unit.test.js	Validates compensation logic when MongoDB writes succeed but ClickHouse appends fail, ensuring eventual consistency and idempotency on retry.
src/utils/retryWithBackoff.js	Provides exponential backoff retry mechanism used by post-save handlers and groupMemberRepository to safely recover from transient ClickHouse failures.

Omitted 25 lower-signal changed files.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Controller
    participant MongoWithClickHouse
    participant MongoDB
    participant ClickHouse
    participant PostSaveHandler

    Client->>Controller: PATCH Group (add member)
    Controller->>MongoWithClickHouse: Update
    MongoWithClickHouse->>MongoDB: Write Group metadata
    MongoDB-->>MongoWithClickHouse: Success
    MongoWithClickHouse->>ClickHouse: Append member event
    alt ClickHouse append succeeds
        ClickHouse-->>MongoWithClickHouse: Dedup check + insert
        MongoWithClickHouse-->>Controller: Success
    else ClickHouse append fails (transient)
        ClickHouse-->>MongoWithClickHouse: Error
        MongoWithClickHouse->>PostSaveHandler: Enqueue compensation
        MongoWithClickHouse-->>Controller: 500
        PostSaveHandler->>ClickHouse: Retry append with backoff
        ClickHouse-->>PostSaveHandler: Success (idempotent)
    end
    Controller->>Client: Response

Loading

_{Edit Gecko PR Settings | Reviewed commit: 178fab9}