fix(api-service): Workflow sync environment check

[scopsy]

What changed? Why was the change needed?

This PR introduces an environment ID validation check for the targetEnvironmentId in the PUT /v2/workflows/:workflowId/sync endpoint.

This change was needed to prevent potential cross-organization environment access, ensuring that the targetEnvironmentId belongs to the user's organization. This aligns with the security enhancement introduced in PR #10063, which added similar validation for the environmentId in the getWorkflow endpoint.

Screenshots

N/A

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

The validation logic mirrors the environmentId check implemented in PR #10063.

---

Slack Thread

 

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[netlify]

✅ Deploy Preview for dashboard-v2-novu-staging canceled.

Name	Link
🔨 Latest commit	0e3ea9a
🔍 Latest deploy log	https://app.netlify.com/projects/dashboard-v2-novu-staging/deploys/69973471ee194d00082e3d4a

[github-actions]

Hey there and thank you for opening this pull request! 👋

We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted.

Your PR title is: fix(api-service): Workflow sync environment check

Requirements:

1. Follow the Conventional Commits specification
2. As a team member, include Linear ticket ID at the end: fixes TICKET-ID or include it in your branch name

Expected format: feat(scope): Add fancy new feature fixes NOV-123

Details:

PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name

[coderabbitai]

Walkthrough

SyncToEnvironmentUseCase now injects EnvironmentRepository and imports NotFoundException. A new private method, validateTargetEnvironment(targetEnvironmentId: string, organizationId: string): Promise, was added to verify the target environment and throw NotFoundException if missing. The execute method awaits this validation before continuing. End-to-end tests in workflow.controller.e2e.ts were reordered and updated to cover a "target environment not found" 404 case, add a production promotion path with a non-existent-workflow 404 assertion, and adjust test flow setup and ordering.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: adding environment validation for workflow sync operations.
Description check	✅ Passed	The description is related to the changeset, explaining what changed (environment ID validation), why it was needed (security), and referencing a similar PR.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts`:
- Around line 149-163: The current validateTargetEnvironment method swallows all
errors and converts them to NotFoundException; instead, call
environmentRepository.findByIdAndOrganization(...) without a broad try/catch,
then if the returned environment is falsy throw new
NotFoundException(`Environment ${targetEnvironmentId} not found`); only catch
and translate a Mongoose CastError (or the specific repository validation error
class) if you must handle non-ObjectId inputs, e.g. catch error instanceof
CastError and throw NotFoundException, but let all other exceptions (DB
connection/timeouts, etc.) propagate unchanged; remove the redundant try/catch
that rethrows NotFoundException and reference the validateTargetEnvironment and
environmentRepository.findByIdAndOrganization symbols when making the change.

[coderabbitai]

🧹 Nitpick comments (1)

apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts (1)

152-158: Optional: deduplicate the repeated error message literal.

Environment ${targetEnvironmentId} not found appears on both lines 152 and 158. Extracting it to a local constant removes the duplication and makes future message changes a one-line edit.

♻️ Proposed refactor

  private async validateTargetEnvironment(targetEnvironmentId: string, organizationId: string): Promise<void> {
+   const notFoundMessage = `Environment ${targetEnvironmentId} not found`;
+
    if (!BaseRepository.isInternalId(targetEnvironmentId)) {
-     throw new NotFoundException(`Environment ${targetEnvironmentId} not found`);
+     throw new NotFoundException(notFoundMessage);
    }

    const environment = await this.environmentRepository.findByIdAndOrganization(targetEnvironmentId, organizationId);

    if (!environment) {
-     throw new NotFoundException(`Environment ${targetEnvironmentId} not found`);
+     throw new NotFoundException(notFoundMessage);
    }
  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts`
around lines 152 - 158, The NotFoundException message literal is duplicated;
inside the sync-to-environment use case (e.g., the method that calls
environmentRepository.findByIdAndOrganization and throws NotFoundException with
`Environment ${targetEnvironmentId} not found`), extract that string into a
local constant (e.g., const notFoundMsg = `Environment ${targetEnvironmentId}
not found`) and use the constant in both throw statements so the message is only
defined once and easier to change later.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts`:
- Around line 150-160: The validateTargetEnvironment method is implemented
correctly: it first uses BaseRepository.isInternalId to guard against invalid
IDs and then calls environmentRepository.findByIdAndOrganization to enforce org
scoping and throw NotFoundException if missing; no try-catch conversion is
needed—leave validateTargetEnvironment as-is (keep the two-step format and the
NotFoundException throws) and ensure any future changes maintain the
isInternalId check and the org-scoped lookup in validateTargetEnvironment.

---

Nitpick comments:
In
`@apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts`:
- Around line 152-158: The NotFoundException message literal is duplicated;
inside the sync-to-environment use case (e.g., the method that calls
environmentRepository.findByIdAndOrganization and throws NotFoundException with
`Environment ${targetEnvironmentId} not found`), extract that string into a
local constant (e.g., const notFoundMsg = `Environment ${targetEnvironmentId}
not found`) and use the constant in both throw statements so the message is only
defined once and easier to change later.