fix(security): serialize activity log hash chain writes (#542)

[2witstudios]

Summary

• Adds pg_advisory_xact_lock to logActivity() and logActivityWithTx() to serialize hash chain writes, preventing concurrent fork vulnerability
• Uses lock key 5829174063 (distinct from security audit's 8370291546) to avoid cross-chain contention
• Moves broadcast and workflow triggers outside the transaction so they don't hold the lock or emit events for rolled-back writes
• Reorders createPageVersion() before logActivityWithTx() in page-mutation-service and page-service so the advisory lock is not held across disk I/O (compression + fs.writeFile)
• Removes dead getLatestLogHash() (replaced by getLatestLogHashWithTx)
• Fixes compliance test mock to include db.transaction and sql so all 30 tests actually execute inserts

Context

The security audit chain was fixed with advisory locking in commit 9a2856e (#541-544), but the activity log chain was missed. Concurrent calls to logActivity() could read the same previousHash and both insert entries pointing to it, forking the chain.

Test plan

• 3 new tests in describe('hash chain serialization (#542)') verify lock acquisition, key value, and logActivityWithTx lock behavior
• All 95 activity-logger tests pass
• All 30 compliance tests pass (with inserts actually executing)
• All 343 monitoring tests pass (8 test files)
• TypeScript typecheck clean (lib)
• Verify advisory lock serialization under concurrent load in staging

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
pagespace-master-plan	Ready	Preview, Comment	Apr 10, 2026 2:29am

[coderabbitai]

Warning

Rate limit exceeded

@2witstudios has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 21 minutes and 28 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 21 minutes and 28 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 71850d15-855c-4c71-9165-0a4beeaf09a2

📥 Commits

Reviewing files that changed from the base of the PR and between ed11d35 and 357d36e.

📒 Files selected for processing (5)

• apps/web/src/services/api/page-mutation-service.ts
• apps/web/src/services/api/page-service.ts
• packages/lib/src/monitoring/__tests__/activity-logger-compliance.test.ts
• packages/lib/src/monitoring/__tests__/activity-logger.test.ts
• packages/lib/src/monitoring/activity-logger.ts

📝 Walkthrough

Walkthrough

The PR refactors activity logging to exclude PII from hash-chain computations, introduces advisory-lock-based serialization to prevent concurrent hash-chain mutations, reorders createPageVersion calls earlier in transactions, and adds a migration script to reset existing hash chains.

Changes

Cohort / File(s)	Summary
Page Service Transaction Reordering apps/web/src/services/api/page-service.ts, apps/web/src/services/api/page-mutation-service.ts	Moved createPageVersion(...) execution to occur immediately after inserting pages into the database and before logActivityWithTx(...) calls, instead of after activity logging.
Activity Logger Core & Exports packages/lib/src/monitoring/activity-logger.ts	Added ACTIVITY_CHAIN_LOCK_KEY constant and pg_advisory_xact_lock calls to serialize hash-chain writes. Removed exported getLatestLogHash() function. Excluded PII fields (userId, actorEmail) from hash computation inputs. Moved hash-chain operations inside locked transactions.
Hash Chain Verification packages/lib/src/monitoring/hash-chain-verifier.ts	Updated hash computation to exclude PII fields (userId, actorEmail) from input objects passed to computeLogHash.
Activity Logger Tests packages/lib/src/monitoring/__tests__/activity-logger.test.ts, packages/lib/src/monitoring/__tests__/activity-logger-compliance.test.ts	Extended mocks to include transaction and advisory lock behavior. Updated serialization expectations to exclude PII while validating non-PII fields. Added test coverage for GDPR-safe hash chaining and advisory-lock-based serialization.
Hash Chain Verifier Tests packages/lib/src/monitoring/__tests__/hash-chain-verifier.test.ts, packages/lib/src/monitoring/__tests__/hash-chain-verifier-full.test.ts	Refactored hash construction to compute hashes from reduced "hashData" objects omitting PII, then merge those fields into stored entry data. Updated tampering tests to mutate non-PII fields affecting hash input.
Activity Log Reset Migration scripts/rehash-activity-logs.ts	New executable script supporting --dry-run mode to reset hash-chain fields (logHash, previousLogHash, chainSeed) across existing activity log rows.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Logger as Activity Logger
    participant Lock as Advisory Lock<br/>(DB)
    participant TX as Transaction<br/>(DB)
    participant Chain as Hash Chain<br/>(DB)

    rect rgba(100, 150, 200, 0.5)
    Note over Client,Chain: OLD FLOW (Non-serialized)
    Client->>Logger: logActivity(data)
    Logger->>TX: Read latest hash
    Logger->>Logger: Compute new hash (with PII)
    Logger->>TX: Insert log entry
    TX->>Chain: Write to activity_logs
    TX-->>Logger: Commit
    Logger->>Client: Return (possible race conditions)
    end

    rect rgba(150, 200, 100, 0.5)
    Note over Client,Chain: NEW FLOW (Serialized, PII-excluded)
    Client->>Logger: logActivity(data)
    Logger->>Lock: Acquire pg_advisory_xact_lock
    Lock-->>Logger: Lock acquired
    Logger->>TX: Begin transaction
    Logger->>Chain: Read latest hash
    Logger->>Logger: Compute new hash (PII excluded)
    Logger->>TX: Insert log entry
    TX->>Chain: Write to activity_logs
    TX-->>Logger: Commit
    Logger->>Lock: Release lock
    Lock-->>Logger: Lock released
    Logger->>Client: Return (serialized, safe)
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• [Audit] Hash-chain concurrent write race — activity_logs can fork #542: The PR directly implements advisory-lock-based serialization of hash-chain writes using pg_advisory_xact_lock and the ACTIVITY_CHAIN_LOCK_KEY constant to prevent concurrent readers/writers from forking the chain, addressing the core issue proposal.

Possibly related PRs

• fix(activity-logger): handle FK constraint violation for deleted pages #211: Modifies logActivity and logActivityWithTx flows in the same activity-logger module, indicating direct overlap in transactional logging implementation.
• Advanced audit logging with hash chain integrity and SIEM integration #155: Updates hash-chain logic, advisory lock constants, and related test infrastructure for activity logs across the monitoring module, sharing multiple code-level changes.
• fix: Rollback system improvements - transaction safety, idempotency, UI #123: Modifies the activity-logger implementation module; the main PR reworks hash-chain locking and exports while this PR extends related function signatures, creating potential conflicts.

Poem

🐰 A lock upon the chain so bright,
PII hides from hashing sight,
Serially we write with care,
No fork can form in this affair! ✨
Hash and bloom, both safe and sound,
The rabbit's crypto's homeward bound! 🔐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: adding advisory locking to serialize activity log hash-chain writes for security/concurrency purposes, which is the core objective of the PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pu/fix-activity-chain-lock

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}