Skip to content

Conversation

@Joao-vi
Copy link
Collaborator

@Joao-vi Joao-vi commented Jan 31, 2026

fixes #

PR checklist:

  • Update READ.me ?
  • Update API documentation ?

QA checklist:

  • Smoke test the functionality described in the issue
  • Test for side effects
  • UI responsiveness
  • Cross browser testing
  • Code review

Copilot AI review requested due to automatic review settings January 31, 2026 13:42
@Joao-vi Joao-vi self-assigned this Jan 31, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new UpdateEntity application use case and wires it into the layered architecture, moving entity updates (and, conditionally, creates) off the legacy saveEntity path and onto v2 controllers, use cases, and listeners.

Changes:

  • Add a new UpdateEntityUseCase with rich tests, backed by MongoMultiLanguageEntityDataSource.getById/update, and expose it via UpdateEntityUseCaseFactory and UpdateEntityController.
  • Extend the entity domain model (Entity, EntityTranslation, new EntityUpdatedEvent, EntityDoesNotExistError) to support update semantics (icon, generated TOC, template migration, editDate refresh) and emit events consumed by new listeners.
  • Wire the /api/entities route and server startup to use the new controller and listeners, registering them in the queue system for denormalization and relationship processing.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
app/server.js Adds a side-effect import of api/core/infrastructure/listeners/Listeners so event listeners are registered when the server starts, enabling the new entity update event flow.
app/queueRegistry.ts Registers DenormalizeEntityUpdatedListener and ProcessRelationshipAfterEntityUpdatedListener as queue jobs so EntityUpdatedEvent emissions are processed asynchronously.
app/api/entities/routes.js Replaces the legacy saveEntity/withTransaction path for non-v2-create requests with UpdateEntityController, while keeping the v2 create path for feature-flagged new entities; also still handles upload middleware selection.
app/api/entities.v2/database/MongoMultiLanguageEntityDataSource.ts Adds getById and update methods to work with the new use case, updates the aggregation pipeline to group by sharedId before template lookup, and continues to integrate with search indexing via modifiedSharedIds.
app/api/entities.v2/contracts/MultiLanguageEntitiesDataSource.ts Extends the datasource contract with update(entity) and getById(sharedId) returning ResultType<Entity, EntityDoesNotExistError>, so use cases can depend on these operations.
app/api/core/libs/eventEmitter/Event.ts Changes EventPayload to only require userId, affecting how payloads for event-based queue jobs are typed and constructed.
app/api/core/libs/DependenciesContext.ts Adds an attachContext helper that wraps a use case’s execute method in an AsyncLocalStorage context, wiring transactionManager, jobsDispatcher, eventEmitter, and idGenerator access for events and jobs.
app/api/core/infrastructure/listeners/DenormalizeEntityUpdatedListener.ts New listener for EntityUpdatedEvent that recreates before/after entities, maps them to DB objects, and calls denormalizeRelated for the target language, with basic error handling for missing translations/templates.
app/api/core/infrastructure/listeners/ProcessRelationshipAfterEntityUpdatedListener.ts New listener that, on EntityUpdatedEvent, recomputes relationship-based references via relationships.saveEntityBasedReferences for the updated entity/language.
app/api/core/infrastructure/listeners/Listeners.ts Centralizes side-effect imports so all relevant listeners (including the new entity-updated ones) are registered when Listeners is imported.
app/api/core/infrastructure/factories/UpdateEntityUseCaseFactory.ts Factory that builds UpdateEntityUseCase with the appropriate datasources, file service, property assignment strategy, and emitter, pulling transactionManager from DependenciesContext.
app/api/core/infrastructure/express/entity/UpdateEntityController.ts New express controller that parses the update DTO (including multipart form payloads), maps it to UpdateEntityUseCaseInput, executes the use case, and returns the updated language-specific entity DBO while emitting documentProcessed.
app/api/core/infrastructure/express/entity/Schemas.ts Refactors entity request validation into a shared MutateEntitySchema, defines CreateEntitySchema and a new UpdateEntitySchema (with _id, sharedId, and language), and exports corresponding DTO types.
app/api/core/infrastructure/express/entity/ExpressEntityMapper.ts Extends the mapper to support toEntityUpdateInput, translating the update DTO (title, metadata, template, icon, uploaded files and URL attachments) into UpdateEntityUseCaseInput.
app/api/core/domain/entity/EntityTranslation.ts Adds mergeMetadata for template changes and refreshEditDate to centralize editDate handling when metadata is modified.
app/api/core/domain/entity/Entity.ts Enhances the entity domain model with generatedToc, an update method, template-change logic that migrates metadata via EntityTranslation.mergeMetadata, an editDate refresh helper, and exposes prevState/getProps for event emission.
app/api/core/domain/entity/errors.ts Introduces EntityDoesNotExistError, a domain error used when a requested entity sharedId is not found.
app/api/core/domain/entity/EntityUpdatedEvent.ts Defines EntityUpdatedEvent with before/after payloads and factory method create, intended to snapshot entity state around updates for listeners.
app/api/core/domain/entity/specs/Entity.spec.ts Adds tests covering the new update behavior (icon, generatedToc, editDate) and a detailed scenario for template changes and metadata migration across languages.
app/api/core/application/UpdateEntity.ts Implements UpdateEntityUseCase, orchestrating entity lookup, optional template change, property assignment updates, file storage, and emission of EntityUpdatedEvent inside a transaction.
app/api/core/application/specs/UpdateEntity.spec.ts Comprehensive test suite for UpdateEntityUseCase, covering basic field updates, metadata clearing and mutation, file upload/linking behavior, event emission, and template changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const dbos = MongoEntityMapper.toDBO(entity);

await this.getCollection().bulkWrite(
dbos.map(dbo => ({ updateOne: { filter: { _id: dbo._id }, update: { $set: dbo } } })),
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

In update, you're using $set: dbo where dbo includes the _id field (lines 47-51). MongoDB does not allow updating _id, so this bulkWrite will fail with a "Mod on _id not allowed" error when this path is exercised. To avoid this, either switch to a replaceOne operation like bulkUpdate does, or build the $set object without the _id field (and any other immutable fields).

Suggested change
dbos.map(dbo => ({ updateOne: { filter: { _id: dbo._id }, update: { $set: dbo } } })),
dbos.map(dbo => ({
replaceOne: {
filter: { _id: dbo._id },
replacement: dbo,
},
})),

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +27
const sanitized = filename
.replace(/\.\.[\\\/]/g, '')

Check failure

Code scanning / CodeQL

Incomplete multi-character sanitization High

This string may still contain
../
, which may cause a path injection vulnerability.

Copilot Autofix

AI about 13 hours ago

General approach: ensure that any occurrence or emergent occurrence of directory traversal or path separators is fully removed. We can do this either by (a) repeatedly applying the same replacement until no further change occurs, or (b) strengthening the regexes so that any number of dots before a separator are removed in one go, or (c) both, for defense in depth. We also want to keep the rest of the logic (trimming, length checks, etc.) unchanged.

Best concrete fix here without changing external behavior is to harden sanitizeFilename:

  1. First, remove any sequence of 2 or more dots immediately followed by a path separator: use /\.{2,}[\\\/]/g. This removes ../, ..../, .../, ..\\, etc., in one pass without reintroducing traversal patterns.
  2. Then remove all path separators everywhere with /[\\\/]/g, and null bytes with /\0/g as before.
  3. Optionally add a simple loop to reapply the \.{2,}[\\\/] removal until the string stabilizes, to be fully robust against crafted corner cases.

This change is entirely local to sanitizeFilename in app/api/core/domain/files/BaseFile.ts. No new imports are required; we stay within standard JavaScript/TypeScript capabilities and keep the same function signature and uses (the zod schema transformations).

Suggested changeset 1
app/api/core/domain/files/BaseFile.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/api/core/domain/files/BaseFile.ts b/app/api/core/domain/files/BaseFile.ts
--- a/app/api/core/domain/files/BaseFile.ts
+++ b/app/api/core/domain/files/BaseFile.ts
@@ -23,11 +23,23 @@
 
 const sanitizeFilename = (filename: string) => {
   // Remove path traversal attempts, path separators, and null bytes
-  const sanitized = filename
-    .replace(/\.\.[\\\/]/g, '')
-    .replace(/[\\\/]/g, '')
-    .replace(/\0/g, '');
+  let sanitized = filename;
 
+  // Repeatedly strip any sequence of two or more dots followed by a path separator,
+  // to ensure patterns like ".../", "..../", "../", and "..\\" are fully removed
+  // without leaving behind new "../" segments.
+  let previous: string;
+  do {
+    previous = sanitized;
+    sanitized = sanitized.replace(/\.{2,}[\\\/]/g, '');
+  } while (sanitized !== previous);
+
+  // Remove any remaining path separators
+  sanitized = sanitized.replace(/[\\\/]/g, '');
+
+  // Remove null bytes
+  sanitized = sanitized.replace(/\0/g, '');
+
   return sanitized;
 };
 
EOF
@@ -23,11 +23,23 @@

const sanitizeFilename = (filename: string) => {
// Remove path traversal attempts, path separators, and null bytes
const sanitized = filename
.replace(/\.\.[\\\/]/g, '')
.replace(/[\\\/]/g, '')
.replace(/\0/g, '');
let sanitized = filename;

// Repeatedly strip any sequence of two or more dots followed by a path separator,
// to ensure patterns like ".../", "..../", "../", and "..\\" are fully removed
// without leaving behind new "../" segments.
let previous: string;
do {
previous = sanitized;
sanitized = sanitized.replace(/\.{2,}[\\\/]/g, '');
} while (sanitized !== previous);

// Remove any remaining path separators
sanitized = sanitized.replace(/[\\\/]/g, '');

// Remove null bytes
sanitized = sanitized.replace(/\0/g, '');

return sanitized;
};

Copilot is powered by AI and may make mistakes. Always verify output.
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