[PROD] - PS489 #187

kkartunov · 2025-12-18T08:52:55Z

https://topcoder.atlassian.net/browse/PS-489
https://topcoder.atlassian.net/browse/PM-3173

… including reviews)

…rkflows have already been queued. if so, prevent further queueing

…queueing-ai-workflow-runs PS-489 - prevent double queueing ai workflow runs

Lint fix

github-actions · 2025-12-18T08:53:10Z

src/api/submission/submission.controller.ts

    );
-    const authUser: JwtUser = req['user'] as JwtUser;
+    const authUser: JwtUser =
+      (req['user'] as JwtUser) ?? ({ isMachine: false, roles: [] } as JwtUser);


[⚠️ correctness]
Using the nullish coalescing operator (??) to provide a default value for authUser is a good approach. However, ensure that this default value is appropriate for all cases where req['user'] might be null or undefined. If there are specific roles or machine states that should be handled differently, consider refining this default value.

github-actions · 2025-12-18T08:53:15Z

src/api/submission/submission.service.spec.ts

        reviewType: {
          findMany: jest.fn().mockResolvedValue([]),
        },
+        $queryRaw: jest.fn().mockResolvedValue([]),


[❗❗ security]
The use of $queryRaw with jest.fn().mockResolvedValue([]) indicates raw SQL queries are being mocked. Ensure that the raw SQL queries are safe from SQL injection vulnerabilities, especially if any user input is involved.

github-actions · 2025-12-18T08:53:15Z

src/api/submission/submission.service.spec.ts

      prismaMock.submission.findFirst.mockResolvedValue({
        id: 'submission-new',
      });
+      prismaMock.$queryRaw.mockResolvedValue([{ id: 'submission-new' }]);


[❗❗ security]
The use of $queryRaw.mockResolvedValue([{ id: 'submission-new' }]) suggests that raw SQL queries are being executed. Verify that these queries are parameterized to prevent SQL injection attacks.

github-actions · 2025-12-18T08:53:15Z

src/api/submission/submission.service.spec.ts

+        },
+      ]);
+
+      const result = await listService.listSubmission(


[❗❗ security]
The test case exposes submitter identity but strips reviews for anonymous challenge queries is handling sensitive data. Ensure that the logic for stripping reviews and URLs is correctly implemented in the actual service to prevent accidental data exposure.

github-actions · 2025-12-18T08:53:19Z

src/dto/reviewApplication.dto.ts

 export const ReviewApplicationRoleOpportunityTypeMap: Record<
  ReviewApplicationRole,
-  ReviewOpportunityType
+  PrismaReviewOpportunityType


[❗❗ correctness]
The change from ReviewOpportunityType to PrismaReviewOpportunityType in the ReviewApplicationRoleOpportunityTypeMap may introduce a mismatch if the enum values in PrismaReviewOpportunityType do not align with those in ReviewOpportunityType. Ensure that the values are compatible or provide a mapping if necessary.

github-actions · 2025-12-18T08:53:23Z

src/shared/guards/tokenRoles.guard.spec.ts

    method: string;
    query: Record<string, unknown>;
-    user: {
+    user?: {


[❗❗ correctness]
Making the user property optional in TestRequest could lead to runtime errors if the code assumes user is always present. Ensure that all usages of request.user handle the case where user is undefined.

github-actions · 2025-12-18T08:53:23Z

src/shared/guards/tokenRoles.guard.spec.ts

      expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
    });
+
+    it('allows anonymous access when challengeId is provided', () => {


[❗❗ security]
The test case allows anonymous access when challengeId is provided introduces a scenario where anonymous access is allowed based on the presence of challengeId. Ensure that this behavior is intentional and does not introduce a security vulnerability by allowing unauthorized access.

github-actions · 2025-12-18T08:53:26Z

src/shared/guards/tokenRoles.guard.ts

    return true;
  }

+  private allowAnonymousSubmissionListByChallenge(


[⚠️ maintainability]
The allowAnonymousSubmissionListByChallenge method duplicates logic from allowSubmissionListByChallenge. Consider refactoring to avoid code duplication and improve maintainability.

github-actions · 2025-12-18T08:53:26Z

src/shared/guards/tokenRoles.guard.ts

+      return false;
+    }
+
+    const challengeId = request?.query?.challengeId;


[❗❗ security]
The challengeId is extracted from request.query without validation. Ensure that challengeId is sanitized to prevent injection attacks.

github-actions · 2025-12-18T08:53:31Z

src/shared/modules/global/workflow-queue.handler.ts

  }

+  async hasQueuedWorkflowRuns(submissionId: string): Promise<boolean> {
+    if (!submissionId) return false;


[⚠️ correctness]
The check for !submissionId should be more explicit. Consider using typeof submissionId !== 'string' || submissionId.trim() === '' to ensure that submissionId is a non-empty string. This will prevent potential issues with falsy values like 0 or false being passed as valid submission IDs.

Copilot

Pull request overview

This PR implements anonymous access to submission listings when filtered by challenge ID and adds duplicate workflow run queueing prevention. The changes support PS-489 (per the JIRA ticket reference).

Key changes:

Adds anonymous GET access to submissions list endpoint when challengeId is provided
Implements duplicate workflow run prevention by checking for existing runs before queueing
Exposes submitter identity (handle, rating) to anonymous users viewing challenge submissions while protecting review details and URLs

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/shared/modules/global/workflow-queue.handler.ts	Adds `hasQueuedWorkflowRuns` method to check for existing workflow runs before queueing new ones
src/shared/modules/global/submission-scan-complete.orchestrator.ts	Uses the new duplicate check to prevent queueing workflow runs multiple times for the same submission
src/shared/guards/tokenRoles.guard.ts	Adds `allowAnonymousSubmissionListByChallenge` method to permit unauthenticated requests to list submissions when challengeId is provided
src/shared/guards/tokenRoles.guard.spec.ts	Adds test coverage for anonymous access with challengeId and updates user type to be optional
src/dto/reviewApplication.dto.ts	Changes type import to use Prisma-generated type for better type safety and removes now-unnecessary eslint disable comment
src/api/submission/submission.service.ts	Adds support for anonymous users by defaulting authUser, always enriching submitter identity for challenge queries, handling anonymous users in visibility context, and stripping reviews/URLs for anonymous access
src/api/submission/submission.service.spec.ts	Adds test case for anonymous challenge queries and updates mock setup to support the new behavior
src/api/submission/submission.controller.ts	Provides a default authUser object when request user is undefined to enable anonymous access
src/api/review-application/reviewApplication.service.ts	Removes obsolete eslint-disable comment now that type is properly aligned
pnpm-lock.yaml	Updates various dependencies to their latest patch/minor versions

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

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

Copilot · 2025-12-18T08:57:54Z

src/api/submission/submission.service.ts

+      const shouldEnrichSubmitter =
+        submissions.length > 0 &&
+        (queryDto.challengeId
+          ? true
+          : await this.canViewSubmitterIdentity(
+              authUser,
+              queryDto.challengeId,
+            ));


The conditional logic for enriching submitter identity has a bug. When queryDto.challengeId is provided, the code always enriches submitter data (true), bypassing the canViewSubmitterIdentity check entirely. This means anonymous users querying by challengeId will have submissions enriched with submitter handles and ratings, which may be intentional for this feature. However, the fallback path passes queryDto.challengeId to canViewSubmitterIdentity, which will always be undefined in that branch (since if it were defined, the ternary would have taken the true path). This likely should pass undefined explicitly or restructure the logic to be clearer.

Suggested change

const shouldEnrichSubmitter =

submissions.length > 0 &&

(queryDto.challengeId

? true

: await this.canViewSubmitterIdentity(

authUser,

queryDto.challengeId,

));

let shouldEnrichSubmitter = false;

if (submissions.length > 0) {

if (queryDto.challengeId) {

// For challenge listings, always enrich submitter data

shouldEnrichSubmitter = true;

} else {

shouldEnrichSubmitter = await this.canViewSubmitterIdentity(

authUser,

undefined,

);

}

}

Copilot · 2025-12-18T08:57:54Z

src/shared/modules/global/workflow-queue.handler.ts

+  async hasQueuedWorkflowRuns(submissionId: string): Promise<boolean> {
+    if (!submissionId) return false;
+
+    const existing = await this.prisma.aiWorkflowRun.findFirst({
+      where: {
+        submissionId,
+      },
+    });
+
+    return !!existing;
+  }


The hasQueuedWorkflowRuns method only checks if any workflow run exists for the submission but doesn't verify the status. Based on the context where it's used to prevent duplicate queueing, this check should probably verify that there are workflow runs with status 'INIT', 'QUEUED', or 'DISPATCHED' rather than just any workflow run. A completed or failed workflow run shouldn't prevent queueing new runs.

Copilot · 2025-12-18T08:57:55Z

src/api/submission/submission.service.ts

    if (!uid) {
-      return emptyContext;
+      return {
+        ...emptyContext,
+        requesterUserId,
+      };
    }


Unreachable code detected. The condition if (!uid) at line 2456 will never be true because it's checked after a previous condition at line 2430 that handles the case where !isPrivilegedRequester && !requesterUserId. Since uid is assigned from requesterUserId at line 2454, and privileged requesters return early at line 2447-2452, the only way to reach line 2456 is if requesterUserId has a value (making uid truthy).

jmgasper and others added 9 commits December 15, 2025 12:24

Allow all users, including anonymous, to pull submission details (not…

371cf2c

… including reviews)

Merge

e1b55ad

Lint

fd3c185

Build fix

ef2d5db

Lint

ba865b2

PS-489 - when receiving SubmissionScanCompleteHandler, check if ai wo…

0bf1311

…rkflows have already been queued. if so, prevent further queueing

Merge pull request #185 from topcoder-platform/PS-489_prevent-double-…

6dab29b

…queueing-ai-workflow-runs PS-489 - prevent double queueing ai workflow runs

Lint fix

f94c2ce

Merge pull request #186 from topcoder-platform/lint-fix

2d38b73

Lint fix

github-actions bot reviewed Dec 18, 2025

View reviewed changes

kkartunov requested review from Copilot and jmgasper December 18, 2025 08:53

Copilot started reviewing on behalf of kkartunov December 18, 2025 08:54 View session

Copilot AI reviewed Dec 18, 2025

View reviewed changes

kkartunov merged commit cc58a2d into master Dec 18, 2025
14 of 15 checks passed

-      const shouldEnrichSubmitter =
-        submissions.length > 0 &&
-        (queryDto.challengeId
-          ? true
-          : await this.canViewSubmitterIdentity(
-              authUser,
-              queryDto.challengeId,
-            ));
+      let shouldEnrichSubmitter = false;
+      if (submissions.length > 0) {
+        if (queryDto.challengeId) {
+          // For challenge listings, always enrich submitter data
+          shouldEnrichSubmitter = true;
+        } else {
+          shouldEnrichSubmitter = await this.canViewSubmitterIdentity(
+            authUser,
+            undefined,
+          );
+        }
+      }

[PROD] - PS489 #187

[PROD] - PS489 #187

Uh oh!

Conversation

kkartunov commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kkartunov commented Dec 18, 2025 •

edited

Loading