Add extra validations for endpoints that do not use principal to get the resources by MarceloRGonc · Pull Request #2112 · openops-cloud/openops

MarceloRGonc · 2026-03-11T15:40:25Z

Fixes OPS-3895.

packages/server/api/src/app/flows/flow/flow-version.controller.ts

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

linear · 2026-03-11T17:01:43Z

OPS-3895 Add extra validations for endpoints that do not use principal to get the resources

packages/server/api/src/app/flows/common/flow-validations.ts

bigfluffycookie · 2026-03-11T17:09:50Z

packages/server/api/src/app/flows/common/flow-validations.ts

+  flowVersion: FlowVersion,
+  projectId: string,
+): Promise<void> {
+  const flow = await flowService.getOne({


hm so, if the flowservice does not return a flow for the version it explicitly means that the flow version does not belong to the project?

I didn't add logic; I just moved the method and renamed it. But I think it makes sense that we have a FlowVersion object with a flowId if we search the DB for the flowId, and a projectId. If we don't have any results, we can assume it's because of the project. It's debatable, but it's not new code.

Ahh gotcha, i missed the collapsed deleted part

packages/server/api/src/app/flows/flow/flow-version.controller.ts

sonarqubecloud · 2026-03-11T17:33:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

This PR strengthens project-scoped authorization for flow-related endpoints that previously fetched resources without constraining them to the principal’s project, aligning error handling with the global ApplicationError -> { code, params } response shape.

Changes:

Introduces shared flow/flow-version project ownership assertions (assertFlowBelongsToProject, assertFlowVersionBelongsToProject) and replaces ad-hoc validation/reply logic.
Adds project scoping to endpoints that were missing it (form retrieval by flowId, flow version test-output routes, flow-run retry).
Updates/introduces unit + integration tests to validate the new authorization/error response behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
packages/server/api/src/app/flows/common/flow-validations.ts	Adds shared ownership assertion helpers that throw `ApplicationError` for consistent error handling.
packages/server/api/src/app/flows/common/flow-version-validation.ts	Removes old reply-based validation helper.
packages/server/api/src/app/flows/test/test.controller.ts	Switches to assertion-based validation for test endpoints.
packages/server/api/src/app/flows/flow/form/form.service.ts	Adds projectId parameter and asserts flow ownership before returning form config.
packages/server/api/src/app/flows/flow/form/form.controller.ts	Passes `principal.projectId` into form service to enable ownership validation.
packages/server/api/src/app/flows/flow/flow-version.controller.ts	Adds ownership assertions for additional routes and relies on global `ApplicationError` handler.
packages/server/api/src/app/flows/flow-run/flow-run-service.ts	Requires `projectId` for retry and scopes flow-run lookup accordingly.
packages/server/api/src/app/flows/flow-run/flow-run-controller.ts	Passes `principal.projectId` into retry call.
packages/server/api/src/app/flows/flow/flow.controller.ts	Updates delete-flow permission to `Permission.DELETE_FLOW`.
packages/server/api/test/unit/flow/flow-validations.test.ts	Adds unit tests for the new assertion helpers.
packages/server/api/test/integration/ce/flows/flow-version.test.ts	Updates integration assertions to match `{ code, params }` error responses.

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

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T17:41:19Z

packages/server/api/test/unit/flow/flow-validations.test.ts

+      expect(() => assertFlowBelongsToProject(flow, projectId)).toThrow(
+        expect.objectContaining({
+          error: expect.objectContaining({
+            code: ErrorCode.AUTHORIZATION,


Similarly here, toThrow(expect.objectContaining(...)) is unlikely to validate the thrown ApplicationError.error contents (it mostly matches the error message/type). Use toThrow(new ApplicationError(...)) or wrap the call and assert with toMatchObject on the caught error to ensure the code/params are actually checked.

Copilot · 2026-03-11T17:41:20Z

packages/server/api/src/app/flows/flow/form/form.service.ts

    }
+
+    assertFlowBelongsToProject(flow, projectId);
    logger.info(flow.version.trigger.settings);


logger.info(flow.version.trigger.settings) logs the full trigger settings on every form fetch. This can be noisy in production and may log user-supplied form configuration. Consider removing this log or downgrading it to debug/trace with a minimal, non-sensitive summary (e.g., flowId/triggerName).

Suggested change

logger.info(flow.version.trigger.settings);

logger.debug(

{ flowId: flow.id, triggerName: flow.version.trigger.settings.triggerName },

'Fetched form trigger settings',

);

Copilot · 2026-03-11T17:41:20Z

packages/server/api/src/app/flows/test/test.controller.ts

      const flowVersion = await flowVersionService.getOneOrThrow(flowVersionId);

-      const isValid = await validateFlowVersionBelongsToProject(
-        flowVersion,
-        projectId,
-        reply,
-      );
+      await assertFlowVersionBelongsToProject(flowVersion, projectId);



assertFlowVersionBelongsToProject throws an ApplicationError (mapped to 403 by the global error handler). Because this call is inside the surrounding try/catch, that ApplicationError will be caught and turned into a 400 later in the handler. Update the catch block to re-throw ApplicationError (or move this call outside the try) so auth failures keep the correct status code/shape.

Copilot · 2026-03-11T17:41:20Z

packages/server/api/src/app/flows/flow/flow-version.controller.ts

        if (flowVersion.flowId !== updateData.flowId) {
          await reply.status(StatusCodes.BAD_REQUEST).send({
            success: false,
            message:
              'It is not possible to update the flowId of a flow version',
          });
        }


After sending an error response (e.g., when flowVersion.flowId !== updateData.flowId), the handler doesn't return, so execution continues and may attempt to send additional responses (locked/conflict/success), causing "Reply was already sent" or unintended updates. Add return after each reply.status(...).send(...) in these validation branches (or restructure with else).

Copilot · 2026-03-11T17:41:20Z

packages/server/api/test/unit/flow/flow-validations.test.ts

+      ).rejects.toThrow(
+        expect.objectContaining({
+          error: expect.objectContaining({
+            code: ErrorCode.AUTHORIZATION,
+            params: {
+              message:
+                'The flow and version are not associated with the project',
+            },
+          }),
+        }),
+      );


These assertions use rejects.toThrow(expect.objectContaining(...)). toThrow primarily matches against the thrown error’s message/class, so this pattern is brittle and may not assert the ApplicationError.error payload as intended. Prefer rejects.toMatchObject({ error: { code: ..., params: ... } }) or rejects.toThrow(new ApplicationError(...)) (as used in other tests).

Suggested change

).rejects.toThrow(

expect.objectContaining({

error: expect.objectContaining({

code: ErrorCode.AUTHORIZATION,

params: {

message:

'The flow and version are not associated with the project',

},

}),

}),

);

).rejects.toMatchObject({

error: {

code: ErrorCode.AUTHORIZATION,

params: {

message:

'The flow and version are not associated with the project',

},

},

});

MarceloRGonc added 4 commits March 11, 2026 15:27

WIP

76e344b

Add unit tests

0daf239

Fix lint

20f599c

Fix tests

244ffe7

github-code-quality bot found potential problems Mar 11, 2026

View reviewed changes

packages/server/api/src/app/flows/flow/flow-version.controller.ts Fixed Show fixed Hide fixed

MarceloRGonc and others added 3 commits March 11, 2026 16:57

Potential fix for pull request finding 'Unreachable statement'

0b4f932

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

Merge branch 'main' into mg/add-validations

ca6c7d1

WIP

4278fb8

MarceloRGonc changed the title ~~Mg/add validations~~ Add extra validations for endpoints that do not use principal to get the resources Mar 11, 2026

MarceloRGonc requested a review from bigfluffycookie March 11, 2026 17:04

Fix delete

b2ce2be

bigfluffycookie reviewed Mar 11, 2026

View reviewed changes

MarceloRGonc added 2 commits March 11, 2026 17:12

Fix lint

83a58fd

Suggestions

304826b

MarceloRGonc marked this pull request as ready for review March 11, 2026 17:35

Copilot AI review requested due to automatic review settings March 11, 2026 17:35

Copilot started reviewing on behalf of MarceloRGonc March 11, 2026 17:35 View session

bigfluffycookie approved these changes Mar 11, 2026

View reviewed changes

MarceloRGonc merged commit f2963e0 into main Mar 11, 2026
23 checks passed

MarceloRGonc deleted the mg/add-validations branch March 11, 2026 17:37

Copilot AI reviewed Mar 11, 2026

View reviewed changes

-    logger.info(flow.version.trigger.settings);
+    logger.debug(
+      { flowId: flow.id, triggerName: flow.version.trigger.settings.triggerName },
+      'Fetched form trigger settings',
+    );

Conversation

MarceloRGonc commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

linear bot commented Mar 11, 2026

Uh oh!

Uh oh!

bigfluffycookie Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

MarceloRGonc Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

bigfluffycookie Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 11, 2026

Quality Gate passed

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MarceloRGonc commented Mar 11, 2026 •

edited

Loading