feat(server): stop CE startup when database was previously EE#41906
Open
salevine wants to merge 4 commits into
Open
feat(server): stop CE startup when database was previously EE#41906salevine wants to merge 4 commits into
salevine wants to merge 4 commits into
Conversation
Running Appsmith Community Edition (CE) against a MongoDB that was previously operated by Appsmith Enterprise Edition (EE) is an unsupported downgrade: CE migrations and CE code then run against EE-shaped data and can corrupt it. Until now this happened silently. Add a guard that runs inside MongoConfig.mongockInitializingBeanRunner, before buildInitializingBeanRunner() — i.e. before any Mongock migration executes. It queries the Mongock audit collection (mongockChangeLog) for a record whose changeLogClass starts with the EE migration package prefix (com.appsmith.server.migrations.db.ee.). Such a record proves EE ran against this database. The match is: - anchored to the package prefix (built with Pattern.quote) so a CE class that merely contains the substring cannot trip it; - state-agnostic — a FAILED/partial EE migration still proves EE ran. On detection the guard logs a clear ERROR banner (carrying the stable token APPSMITH-EE-DOWNGRADE-BLOCKED, with changeId/changeLogClass passed as CR/LF-stripped SLF4J parameters to avoid log forging) and then throws to abort startup, matching the existing throw-to-abort convention in the same bean method (checkForbiddenIds). Failure semantics are split deliberately: - detection (EE found) -> fail-closed (abort startup); - query/infrastructure error -> fail-open (log a warning and proceed), in a catch that does not wrap the abort, so a flaky/locked-down Mongo never bricks a healthy CE instance. The check is CE-only via the standard 4-part CE/EE seam (EnterpriseDowngradeGuardCE / ...CEImpl / EnterpriseDowngradeGuard / EnterpriseDowngradeGuardImpl); the EE overlay replaces EnterpriseDowngradeGuardImpl with a no-op so EE never blocks itself. Tests (embedded Mongo + a fast pure-Mockito unit test) cover: EE record present -> abort; CE-only and legacy changelog -> pass; empty and missing collection -> pass; FAILED-state EE record -> abort; anchored-prefix near-misses -> pass; and the fail-open path on query error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughIntroduces an ChangesEnterprise Downgrade Guard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImplTest.java`:
- Around line 111-113: The test setup code is silently swallowing all errors
when creating the MONGOCK_CHANGE_LOG collection using onErrorResume(e ->
Mono.empty()), which can hide genuine Mongo test setup failures and allow tests
to pass despite broken setup. Replace the blanket error suppression with a more
targeted approach that only ignores the specific expected error (such as when
the collection already exists) while allowing other errors to propagate so
actual setup issues are caught and cause the test to fail appropriately.
🪄 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: 61fd7e46-0b87-4c37-aa93-d12ab7f5d962
📒 Files selected for processing (7)
app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/MongoConfig.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/EnterpriseDowngradeGuard.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/EnterpriseDowngradeGuardImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCE.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImpl.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImplTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImplUnitTest.java
The guard shipped with a wrong detection signal and never fired. It assumed EE migrations are recorded under the package "com.appsmith.server.migrations.db.ee." and matched changeLogClass with that prefix. That package does not exist, so a CE build started against an EE database without stopping (false negative — the dangerous direction). Verified against a live EE-then-CE database (mongockChangeLog, 233 docs) and the appsmith-ee repo: EE records its 69 @ChangeUnit migrations in the PARENT package "com.appsmith.server.migrations.db." with an EE token in the class name (e.g. Migration003EE01..., Migration042EE01AddWorkflowPlugin), plus a legacy "com.appsmith.server.migrations.DatabaseChangelogEE". CE migrations all live in the "com.appsmith.server.migrations.db.ce." subpackage, and CE has zero classes directly in the parent db package. Change detection to: a changeLogClass under "...migrations.db." that is NOT under "...migrations.db.ce.", OR the legacy DatabaseChangelogEE class. This relies on the package layout (the same split Mongock scans) rather than an "EE" name token, so a future CE class such as db.ce.EeThing is not misread as EE. Built with Filters.or(and(regex parent, not(regex ce)), eq legacy); prefixes are anchored and Pattern.quote-escaped. Against the live DB the new rule matches 84 EE records and 0 CE (db.ce.) records; the old rule matched 0. Tests updated to use real EE class names and a mixed CE+EE database case; guard suite is 8/8 + 2/2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImplTest.java (1)
127-129:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid swallowing all setup errors in this test case.
onErrorResume(e -> Mono.empty())can hide broken Mongo test setup and still let the test pass, which weakens coverage for the "empty collection" path.Suggested change
- Mono.from(mongoClient.getDatabase(databaseName).createCollection(MONGOCK_CHANGE_LOG)) - .onErrorResume(e -> Mono.empty()) - .block(); - clearChangeLog(); + // Create the collection explicitly, then empty it without masking setup failures. + Mono.from(changeLogCollection().insertOne(new Document("_id", "seed-empty-case"))) + .block(); + clearChangeLog();🤖 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 `@app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImplTest.java` around lines 127 - 129, The test setup code in the MongoDB collection creation block swallows all errors with onErrorResume(e -> Mono.empty()), which can hide broken test setup and allow tests to pass when they should fail. Remove or replace the onErrorResume(e -> Mono.empty()) call in the Mono.from(mongoClient.getDatabase(databaseName).createCollection(MONGOCK_CHANGE_LOG)) chain to ensure that actual setup errors are not silently ignored, allowing the test to properly detect when the "empty collection" path is broken.
🤖 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.
Duplicate comments:
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImplTest.java`:
- Around line 127-129: The test setup code in the MongoDB collection creation
block swallows all errors with onErrorResume(e -> Mono.empty()), which can hide
broken test setup and allow tests to pass when they should fail. Remove or
replace the onErrorResume(e -> Mono.empty()) call in the
Mono.from(mongoClient.getDatabase(databaseName).createCollection(MONGOCK_CHANGE_LOG))
chain to ensure that actual setup errors are not silently ignored, allowing the
test to properly detect when the "empty collection" path is broken.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 748f7cee-1e8c-448f-bd74-95361030fe9d
📒 Files selected for processing (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImpl.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImplTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/EnterpriseDowngradeGuardCEImpl.java
The guard inspects the database's Mongock changelog, not the running edition, so once this code syncs to EE an EE build would self-abort against its own (legitimate) EE database. Gate the guard call in MongoConfig on the running edition: invoke it only when DeploymentProperties.getEdition() == "CE". An EE build already overrides getEdition() to return "EE", so EE skips the guard automatically — no EE-repo override is needed and EE can never self-block. This is also the semantically correct condition: the guard exists to stop a CE build from running on EE data. Edition is binary-determined (DeploymentPropertiesCE.getEdition() returns a hardcoded "CE"; it is not read from infra.json or any external config), so a CE deployment cannot be misconfigured into skipping the guard. Extract the CE edition value to DeploymentPropertiesCE.EDITION_CE and reference it from both getEdition() and the MongoConfig gate so the two cannot drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…aceExists The empty-changelog case swallowed every error from createCollection via onErrorResume(e -> Mono.empty()), masking genuine Mongo setup failures. Limit the suppression to the expected "collection already exists" error (code 48) so real setup breakage fails the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
subrata71
approved these changes
Jun 18, 2026
salevine
added
ok-to-test
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.
Summary
Running Appsmith Community Edition (CE) against a MongoDB that was previously operated by Appsmith Enterprise Edition (EE) is an unsupported EE→CE downgrade — CE migrations and CE code then run against EE-shaped data and can corrupt it. Today this happens silently.
This PR adds a startup guard that detects the condition before any Mongock migration runs and hard-stops CE startup with a clear, operator-facing log message.
How it works
MongoConfig.mongockInitializingBeanRunner, immediately beforebuildInitializingBeanRunner()— guaranteed to run before any migration, and where the existingcheckForbiddenIdsalready throws-to-abort.DeploymentProperties.getEdition() == "CE". An EE build already reports edition"EE"(hardcoded, binary-determined — not read from config), so EE skips the guard automatically and can never self-block against its own database. The edition value is shared viaDeploymentPropertiesCE.EDITION_CE.mongockChangeLog. EE records its@ChangeUnitmigrations in the parent packagecom.appsmith.server.migrations.db.with anEEtoken in the class name (e.g.Migration003EE01...,Migration042EE01AddWorkflowPlugin), plus a legacycom.appsmith.server.migrations.DatabaseChangelogEE. CE migrations live in thecom.appsmith.server.migrations.db.ce.subpackage, and CE has zero classes directly in the parentdbpackage. So the DB was used by EE iffmongockChangeLogholds achangeLogClassunder...migrations.db.that is not under...migrations.db.ce., or the legacyDatabaseChangelogEE. This keys on the package layout (the same split Mongock scans), not anEEname token, so a CE class likedb.ce.EeThingis not misread as EE. Prefixes are anchored andPattern.quote-escaped.APPSMITH-EE-DOWNGRADE-BLOCKED, withchangeId/changeLogClasspassed as CR/LF-stripped SLF4J parameters — then throw to abort startup.Testing
EnterpriseDowngradeGuardCEImplTest(embedded Mongo) — 8/8: real EE class present → abort; legacyDatabaseChangelogEE→ abort; CE-only (db.ce + legacyDatabaseChangelog0/2) → pass; empty collection → pass; missing collection → pass;FAILED-state EE record → abort; CE-package near-misses (db.ce.EeThing) → pass; mixed CE+EE database → abort.EnterpriseDowngradeGuardCEImplUnitTest(Mockito) — 2/2: fail-open when the query throws.spotless:checkandtest-compileclean. Verified live: CE startup against an EE database now aborts.Follow-ups (not in this PR)
...migrations.db.ce.(never directly in the parentdbpackage), since the guard treats any parent-dbclass as EE.APPSMITH-EE-DOWNGRADE-BLOCKEDtoken (// TODO(docs)).DeploymentProperties.getEdition() == "EE"so an accidentally-dropped override is caught in CI rather than at startup.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/27768419055
Commit: 57f2657
Cypress dashboard.
Tags:
@tag.AllSpec:
Thu, 18 Jun 2026 15:59:26 UTC
Automation
/ok-to-test tags="@tag.All"