fix(isms): serialize approve/decline to prevent concurrent double-publish (CS-701)#3366
Merged
Conversation
…lish (CS-701) Flagged on the production deploy PR (#3364): assertPendingApprovalBy runs before the transaction, so two racing approve() calls could both pass the guard. Root cause (verified): under READ COMMITTED a plain in-transaction status re-read does NOT serialize — the loser reads the still-committed `needs_review`. The @@unique([documentId, version]) constraint only turns the perfectly-simultaneous case into an error; the staggered case silently creates a second published version and overwrites currentVersionId. Fix: - approve() now takes a per-document advisory lock and then claims the approval atomically via updateMany({ where: status='needs_review' + approverId }) inside the transaction, aborting when count !== 1. Exactly one caller freezes a version; losers abort before any derivation/version creation. The pre-transaction guard is kept for the normal-case specific error messages. - decline() gets the same atomic claim, so an approve/decline race can no longer leave a "declined" document that still owns a live currentVersionId. - Renamed the shared advisory-lock helper lockDocumentForPositions -> lockDocument (it now serializes approval too, not just register-row position allocation); updated the four register-create callers. Adversarially audited the whole approve/publish/version/export flow for related concurrency + IDOR issues; getVersionExport org-scoping, S3 keys, snapshot parsing, currentVersionId integrity and the migration paths verified safe. The narrow edit-during-approve window for register update/delete is low-severity/self-healing and left as a follow-up. Tests: 321 api ISMS tests passing (incl. new approve + decline race tests); typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This was referenced Jul 7, 2026
claudfuen
pushed a commit
that referenced
this pull request
Jul 7, 2026
# [3.99.0](v3.98.1...v3.99.0) (2026-07-07) ### Bug Fixes * **integrations:** don't persist unverified GitHub installation_id ([#3370](#3370)) ([9c6f6fa](9c6f6fa)) * **isms:** lock before writing control links so edits serialize with approve ([#3369](#3369)) ([a21f866](a21f866)) * **isms:** production-deploy review follow-ups (CS-701) ([#3365](#3365)) ([d756d4a](d756d4a)), closes [#3364](#3364) * **isms:** serialize approve against ALL register/content edits (central lock) ([#3367](#3367)) ([5c88a3d](5c88a3d)) * **isms:** serialize approve/decline to prevent concurrent double-publish (CS-701) ([#3366](#3366)) ([b158bc9](b158bc9)), closes [#3364](#3364) ### Features * **integrations:** add read-only GitHub App integration (CS-710) ([#3363](#3363)) ([a5a9243](a5a9243)) * **isms:** document versioning and history (CS-701) ([#3361](#3361)) ([d95dc2e](d95dc2e))
Contributor
|
🎉 This PR is included in version 3.99.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The production-deploy PR #3364 flagged a concurrent-approve race in
isms.service.ts(CS-701, already on main). This PR fixes it — plus a closely-related decline race found while auditing. Base ismain; it reachesreleaseon the next deploy.Root cause (verified)
assertPendingApprovalByruns beforedb.$transaction. Under READ COMMITTED, a plain in-transaction status re-read does not serialize — the losing approval reads the still-committedneeds_review.@@unique([documentId, version])only converts the perfectly simultaneous interleaving into an error; the staggered interleaving silently creates a second published version and overwritescurrentVersionId, orphaning the first.Fix
approve(): takes a per-document advisory lock, then atomically claims the approval viaupdateMany({ where: { status: 'needs_review', approverId } })inside the transaction, aborting whencount !== 1. Exactly one caller freezes a version; losers abort before any derivation/version creation. The pre-transaction guard is kept for normal-case error messages.decline(): same atomic claim, so an approve/decline race can no longer leave a declined document that still owns a livecurrentVersionId.lockDocumentForPositions→lockDocument(it now serializes approval too), updating the four register-create callers.Audit
I ran an adversarial audit of the whole approve/publish/version/export flow (concurrency, data-integrity, and cross-org/IDOR lenses, each independently verified). Confirmed-safe:
getVersionExportorg-scoping, S3 key derivation (no IDOR), snapshot parsing,currentVersionIdintegrity, and every migrated-doc read path. The only residual is a narrow, self-healing edit-during-approve window for register update/delete — low severity, noted as a follow-up rather than plumbing advisory locks through every edit path in a production hotfix.Testing
Flagged in #3364. Follows #3361 (CS-701) and #3365.
🤖 Generated with Claude Code
https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp
Summary by cubic
Prevents double-publish by serializing ISMS approve/decline with a per-document advisory lock and an in-transaction atomic claim. Ensures CS-701 versioning keeps exactly one published version and consistent history.
Bug Fixes
approve()takes a per-document advisory lock and atomically claims the approval withupdateMany(...)inside the transaction; only one caller proceeds, others abort before version creation.decline()uses the same atomic claim to avoid approve/decline races that could leave a declined doc with a livecurrentVersionId.Refactors
lockDocumentForPositionstolockDocumentand updated register-create callers; the lock now also serializes approvals.Written for commit 68df7f4. Summary will update on new commits.