[PROD] - Security, clean up & bug fixes #1368

kkartunov · 2025-12-11T07:59:44Z

Related JIRA Ticket:

https://topcoder.atlassian.net/browse/PM-2611
clean up from @vas3a

What's in this PR?

fix(PM-3141): allow only admins and reviewer to respond to appeals

fix(PM-2662): hide other submitters submissions if challenge is configured to hide it

…orecard-ui Cleanup old code for the review scorecard ui

…to hide submissions

fix(PM-2662): Show only users submission in review tab if challenge is configured to hide submissions

github-actions · 2025-12-11T07:59:58Z

src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentSubmissions.tsx

+        )
+    }, [challengeInfo])
+
+    const isSubmissionsViewable = useMemo(() => {


[❗❗ correctness]
The isSubmissionsViewable function uses challengeInfo.metadata.some(...) which could potentially throw an error if challengeInfo.metadata is not an array. Consider adding a check to ensure challengeInfo.metadata is an array before calling .some().

github-actions · 2025-12-11T07:59:58Z

src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentSubmissions.tsx

    const filteredSubmissions = useMemo<BackendSubmission[]>(
        () => {
+
+            const filterFunc = (submissions: BackendSubmission[]): BackendSubmission[] => submissions


[💡 maintainability]
The filterFunc is defined inside the useMemo hook but is used twice within the same scope. Consider defining filterFunc outside of useMemo to avoid redefining it unnecessarily, which could improve readability and maintainability.

github-actions · 2025-12-11T08:00:03Z

...s/Scorecard/ScorecardViewer/ScorecardQuestion/ReviewResponse/ReviewComment/ReviewComment.tsx

        addAppeal,
        isSavingAppeal,
    }: ScorecardViewerContextValue = useScorecardViewerContext()
+    const { isAdmin, hasReviewerRole }: UseRolePermissionsResult = useRolePermissions()


[❗❗ security]
The useRolePermissions hook is used to determine role-based permissions. Ensure that this hook is correctly implemented and returns accurate role information, as incorrect role data could lead to unauthorized access or actions.

github-actions · 2025-12-11T08:00:03Z

...s/Scorecard/ScorecardViewer/ScorecardQuestion/ReviewResponse/ReviewComment/ReviewComment.tsx

                        reviewItem={props.reviewItem}
                        scorecardQuestion={props.question}
-                        canRespondToAppeal={isReviewerRole}
+                        canRespondToAppeal={isAdmin || hasReviewerRole}


[❗❗ correctness]
The condition isAdmin || hasReviewerRole is used to determine if a user can respond to an appeal. Ensure that this logic aligns with the intended business rules, as incorrect permission checks could lead to unauthorized actions.

github-actions · 2025-12-11T08:00:50Z

src/libs/ui/lib/components/form/form-groups/form-input/input-date-picker/InputDatePicker.tsx

            type='text'
            className={classNames(props.className, styles.container)}
        >
+            {/* @ts-expect-error: TS2786: DatePicker cannot be used as a JSX component */}


[⚠️ maintainability]
Using @ts-expect-error to suppress TypeScript errors can hide potential issues. Investigate why DatePicker cannot be used as a JSX component and address the root cause instead of suppressing the error.

github-actions · 2025-12-11T08:00:54Z

src/libs/ui/lib/components/form/form-groups/form-input/input-wrapper/InputWrapper.tsx

    readonly forceFocusStyle?: boolean
 }

+// @ts-expect-error: TS2322: Type mismatch for ForwardRefExoticComponent


[⚠️ maintainability]
Using @ts-expect-error to suppress TypeScript errors can hide potential issues. Consider addressing the underlying type mismatch instead of suppressing it.

github-actions · 2025-12-11T08:00:57Z

src/libs/ui/lib/components/page-title/PageTitle.tsx

 }

 const PageTitle: FC<PageTitleProps> = (props: PageTitleProps) => (
+    // @ts-expect-error: TS2786: Helmet cannot be used as a JSX component


[⚠️ maintainability]
Using @ts-expect-error to suppress TypeScript errors should be avoided if possible, as it can hide potential issues. Consider investigating the cause of the error TS2786 and resolving it, or documenting why this suppression is necessary and safe.

github-actions · 2025-12-11T08:01:00Z

src/libs/ui/lib/components/portal/Portal.tsx

    portalRef?: MutableRefObject<HTMLElement>,
 }

+// @ts-expect-error: TS2322: Type mismatch for FC return type


[⚠️ maintainability]
Using // @ts-expect-error to suppress TypeScript errors can hide potential issues. Investigate the root cause of the type mismatch and address it instead of suppressing the error. This will improve type safety and maintainability.

more security fixes - high -> dev

drop ssl serts

fix(PM-3141): Respond to appeals

github-actions · 2025-12-11T09:46:10Z

src/apps/review/src/lib/hooks/useFetchSubmissionReviews.ts

        error: fetchAppealsError,
    }: SWRResponse<AppealInfo[], Error> = useSWR<AppealInfo[], Error>(
-        `EnvironmentConfig.API.V6/appeals/resourceId/${resourceId}`,
+        `${EnvironmentConfig.API.V6}/appeals/resourceId/${resourceId}`,


[⚠️ security]
The URL string is constructed using template literals, which is correct. However, ensure that resourceId is always a valid and sanitized string to prevent potential injection attacks or malformed URLs.

github-actions · 2025-12-11T09:46:10Z

src/apps/review/src/lib/hooks/useFetchSubmissionReviews.ts

+                    try {
+                        if (reviewId) {
+                            // re-fetch review data
+                            mutate(`EnvironmentConfig.API.V6/reviews/${reviewId}`)


[⚠️ security]
The mutate function is used to revalidate the SWR cache for the review data. Ensure that reviewId is always a valid and sanitized string to prevent potential issues with cache keys.

github-actions · 2025-12-11T09:46:13Z

src/apps/review/src/lib/utils/routes.ts

                : `/${match[1]}/${match[2]}`

-            return `${basePath}/reviews/${submissionId}?reviewId=${encodedReviewId}`
+            return `${basePath}/reviews/${submissionId}?reviewId=${encodedReviewId}&respondToAppeals=true`


[❗❗ correctness]
Adding the respondToAppeals=true query parameter changes the behavior of the route. Ensure that this change is intentional and that all parts of the application that rely on this route are updated accordingly to handle the new parameter.

github-actions · 2025-12-11T09:46:18Z

src/apps/review/src/pages/reviews/components/ReviewViewer/ReviewViewer.tsx

 const ReviewViewer: FC = () => {
    const navigate = useAppNavigate()
+    const [searchParams] = useSearchParams()
+    const [respondToAppeals, setRespondToAppeals] = useState(searchParams.get('respondToAppeals') === 'true')


[⚠️ performance]
Using useState with searchParams.get('respondToAppeals') directly ties the initial state to the URL parameter. Consider using useMemo to derive the initial state from searchParams to avoid unnecessary re-renders if searchParams changes but respondToAppeals does not.

github-actions · 2025-12-11T09:46:18Z

src/apps/review/src/pages/reviews/components/ReviewViewer/ReviewViewer.tsx

+            setRespondToAppeals(false)
        }
-    }, [canEditScorecard, isManagerEdit])
+    }, [canEditScorecard, isManagerEdit, respondToAppeals])


[❗❗ correctness]
The dependency array for this useEffect includes respondToAppeals, which is being set to false inside the effect. This could lead to an infinite loop if not handled carefully. Ensure that the state update logic is correct and doesn't cause unnecessary re-renders.

fix(PM-3141): edit mode for copilot users

github-actions · 2025-12-11T14:30:13Z

src/apps/review/src/pages/reviews/components/ReviewViewer/ReviewViewer.tsx


    useEffect(() => {
-        if (!canEditScorecard && isManagerEdit) {
+        if (!canEditScorecard && isManagerEdit && !respondToAppeals) {


[⚠️ correctness]
The condition !respondToAppeals was added to the if statement. Ensure that this change aligns with the intended logic, as it alters when setIsManagerEdit(false) is called. Verify that this does not introduce any unintended behavior, especially if respondToAppeals is expected to influence the manager edit state.

fix(PM-2662): Show only current users winning submission in winners tab if challenge is private

github-actions · 2025-12-11T14:30:36Z

src/apps/review/src/lib/components/TableWinners/TableWinners.tsx

+            .toLowerCase() === 'true')
+    }, [challengeInfo])
+
+    const canViewSubmissions = useMemo(() => {


[💡 readability]
The canViewSubmissions logic might be clearer if the isCompletedDesignChallenge and isSubmissionsViewable checks were combined into a single condition. This could improve readability by reducing the number of nested conditions.

github-actions · 2025-12-11T14:30:36Z

src/apps/review/src/lib/components/TableWinners/TableWinners.tsx

+        return true
+    }, [isCompletedDesignChallenge, isSubmissionsViewable, canViewAllSubmissions])
+
+    const filterFunc = useCallback((submissions: ProjectResult[]): ProjectResult[] => submissions


[⚠️ performance]
The filterFunc function uses useCallback, but its dependencies include canViewSubmissions and loginUserInfo?.userId, which are derived from context and could change. Ensure these dependencies are stable or consider using useMemo if the function doesn't need to be recreated.

fix(PM-2573): show only submissions with passed screening score

github-actions · 2025-12-11T21:52:06Z

src/apps/review/src/lib/components/TableReview/TableReview.tsx

-        })) as SubmissionRow[]
+        const rows = aggregatedSubmissionRows
+            .filter(aggregated => {
+                const reviews = aggregated.reviews ?? []


[⚠️ performance]
The filter function is used to find submissions with a specific review detail. However, it might be more efficient to use some instead of find within the filter callback to avoid unnecessary iterations once a match is found.

github-actions · 2025-12-11T21:52:06Z

src/apps/review/src/lib/components/TableReview/TableReview.tsx

+                return !!myReviewDetail?.reviewId
+            })
+            .map(aggregated => ({
+                ...(aggregated.submission ?? {}),


[💡 maintainability]
The spread operator is used twice on aggregated.submission. This is redundant and could be simplified to a single spread operation.

jmgasper

Added a few comments for clarification - nothing major.

jmgasper · 2025-12-11T22:45:57Z

src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentSubmissions.tsx

+            .toLowerCase() : ''
+        const status = challengeInfo.status ? String(challengeInfo.status)
+            .toLowerCase() : ''
+        return type === 'design' && (


Do we need to check the challenge type here as well? This will match F2Fs in addition to challenges.

jmgasper · 2025-12-11T22:46:58Z

...s/Scorecard/ScorecardViewer/ScorecardQuestion/ReviewResponse/ReviewComment/ReviewComment.tsx

                        reviewItem={props.reviewItem}
                        scorecardQuestion={props.question}
-                        canRespondToAppeal={isReviewerRole}
+                        canRespondToAppeal={isAdmin || hasReviewerRole}


What is this change for? Are we now allowing admins to respond to appeals? I'm not sure that's something we've done in the past?

jmgasper · 2025-12-11T22:48:25Z

src/apps/review/src/lib/components/TableWinners/TableWinners.tsx

        }
    }, [challengeInfo?.type?.abbreviation, challengeInfo?.type?.name])

+    const isCompletedDesignChallenge = useMemo(() => {


Should we move this out so that we aren't duplicating it multiple places? We added the same exact functionality in src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentSubmissions.tsx

Master -> dev

vas3a and others added 27 commits December 4, 2025 13:24

Cleanup old code for the review scorecard ui

5ce15aa

fix: allow only admins and reviewer to respond to appeals

3b1193a

fix: lint

0361d8c

Merge pull request #1361 from topcoder-platform/pm-3141

985e16a

fix(PM-3141): allow only admins and reviewer to respond to appeals

fix: filter submissions to design challenges if its configured that way

1468bd4

fix: can view submissions

57b48d3

fix: lint

6528746

fix: lint

d37b511

fix: lint

195226c

Merge pull request #1364 from topcoder-platform/pm-2662_1

d088964

fix(PM-2662): hide other submitters submissions if challenge is configured to hide it

Merge pull request #1359 from topcoder-platform/cleanup-old-review-sc…

951de7d

…orecard-ui Cleanup old code for the review scorecard ui

point tc deps to master & npm minor update

c3baec0

ci: deploy to dev env

65eec2d

back to auth-lib-v2

d6462df

fix: directly open scorecard in edit mode

da81018

fix: revalidate appeals and response

5a25ed2

fix: revalidate appeals and response

d216fde

fix: lint

b111246

fix: lint

548b0c4

fix: restrict reviews in review tab for design challenge with config …

7fd1d25

…to hide submissions

fix: lint

b6ed3ed

fix: lint

995d81e

update yarn.lock

75664d7

Merge pull request #1367 from topcoder-platform/pm-2662_2

2e1fc7d

fix(PM-2662): Show only users submission in review tab if challenge is configured to hide submissions

fix trivy alerts

0563cf0

Merge branch 'dev' into security

b8d39fa

ci: remove from dev

48816c5

kkartunov requested a review from jmgasper as a code owner December 11, 2025 07:59

github-actions bot reviewed Dec 11, 2025

View reviewed changes

kkartunov and others added 5 commits December 11, 2025 10:24

security fixes - high

61ad75b

Merge pull request #1369 from topcoder-platform/security

e564eed

more security fixes - high -> dev

drop ssl serts

5b01f5f

Merge pull request #1370 from topcoder-platform/security

72028ad

drop ssl serts

Merge pull request #1366 from topcoder-platform/pm-3141_1

1b0df87

fix(PM-3141): Respond to appeals

github-actions bot reviewed Dec 11, 2025

View reviewed changes

hentrymartin added 7 commits December 11, 2025 12:45

fix: repond to appeals

21226f2

fix: repond to appeals

12c4c64

fix: respond to appeals

d0a5e6d

fix: lint

86b03c8

fix: show only my winning results

4eb2201

fix: lint

8fac84b

Merge pull request #1371 from topcoder-platform/pm-3141_3

5163510

fix(PM-3141): edit mode for copilot users

github-actions bot reviewed Dec 11, 2025

View reviewed changes

Merge pull request #1372 from topcoder-platform/pm-2662_3

1ced8a1

fix(PM-2662): Show only current users winning submission in winners tab if challenge is private

github-actions bot reviewed Dec 11, 2025

View reviewed changes

hentrymartin and others added 4 commits December 11, 2025 15:58

fix: show only submissions with passed screening score

e9d4976

fix: lint

6ae3ec0

Tweak this path to address 401 errors in the service

d1cf5c6

Merge pull request #1373 from topcoder-platform/pm-2573

23a62d1

fix(PM-2573): show only submissions with passed screening score

github-actions bot reviewed Dec 11, 2025

View reviewed changes

jmgasper reviewed Dec 11, 2025

View reviewed changes

Merge pull request #1375 from topcoder-platform/master

9ed7544

Master -> dev

kkartunov merged commit 9a94084 into master Dec 12, 2025
9 of 10 checks passed

[PROD] - Security, clean up & bug fixes #1368

[PROD] - Security, clean up & bug fixes #1368

Uh oh!

Conversation

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

Uh oh!

Related JIRA Ticket:

What's in this PR?

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

jmgasper left a comment

Choose a reason for hiding this comment

Uh oh!

jmgasper Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

jmgasper Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

jmgasper Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

kkartunov commented Dec 11, 2025 •

edited

Loading