Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- CreateIndex
CREATE INDEX "appeal_response_appeal_resource_idx" ON "appealResponse"("appealId", "resourceId");

-- CreateIndex
CREATE INDEX "review_resource_status_phase_idx" ON "review"("resourceId", "status", "phaseId");

-- Clean up orphaned reviewSummations before enforcing FK
UPDATE "reviewSummation" rs
SET "scorecardId" = NULL
WHERE "scorecardId" IS NOT NULL
AND NOT EXISTS (
SELECT 1
FROM "scorecard" sc
WHERE sc."id" = rs."scorecardId"
);

-- AddForeignKey
ALTER TABLE "reviewSummation" ADD CONSTRAINT "reviewSummation_scorecardId_fkey" FOREIGN KEY ("scorecardId") REFERENCES "scorecard"("id") ON DELETE CASCADE ON UPDATE CASCADE;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- CreateIndex
CREATE INDEX "appeal_comment_resource_idx" ON "appeal"("reviewItemCommentId", "resourceId");
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-- Add composite indexes to improve My Reviews query performance

CREATE INDEX IF NOT EXISTS "review_resource_status_phase_idx"
ON "reviews"."review"("resourceId", "status", "phaseId");

CREATE INDEX IF NOT EXISTS "appeal_response_appeal_resource_idx"
ON "reviews"."appealResponse"("appealId", "resourceId");

CREATE INDEX IF NOT EXISTS "appeal_comment_resource_idx"
ON "reviews"."appeal"("reviewItemCommentId", "resourceId");
3 changes: 3 additions & 0 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ model review {
@@index([status]) // Index for filtering by review status
@@index([status, phaseId])
@@index([resourceId, status])
@@index([resourceId, status, phaseId], map: "review_resource_status_phase_idx") // Supports incomplete review lookups that also consider phase ordering
@@unique([resourceId, submissionId, scorecardId])
}

Expand Down Expand Up @@ -284,6 +285,7 @@ model appeal {
@@index([resourceId]) // Index for resource ID
@@index([id]) // Index for direct ID lookups
@@index([reviewItemCommentId]) // Index for joining with reviewItemComment table
@@index([reviewItemCommentId, resourceId], map: "appeal_comment_resource_idx") // Supports filtered appeal lookups by comment and resource
}

model appealResponse {
Expand All @@ -303,6 +305,7 @@ model appealResponse {
@@index([id]) // Index for direct ID lookups
@@index([appealId]) // Index for joining with appeal table
@@index([resourceId]) // Index for filtering by resource (responder)
@@index([appealId, resourceId], map: "appeal_response_appeal_resource_idx") // Supports lookups for pending appeal responses by appeal and resource
}

model challengeResult {
Expand Down
40 changes: 34 additions & 6 deletions src/api/my-review/myReview.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ export class MyReviewService {
}

const baseJoins: Prisma.Sql[] = [];
const countJoins: Prisma.Sql[] = [];
const countExtras: Prisma.Sql[] = [];
const rowExtras: Prisma.Sql[] = [];

if (!adminUser) {
if (!normalizedUserId) {
Expand All @@ -175,7 +178,17 @@ export class MyReviewService {
`,
);

whereFragments.push(Prisma.sql`r."challengeId" IS NOT NULL`);
rowExtras.push(Prisma.sql`r."challengeId" IS NOT NULL`);
countExtras.push(
Prisma.sql`
EXISTS (
SELECT 1
FROM resources."Resource" r
WHERE r."challengeId" = c.id
AND r."memberId" = ${normalizedUserId}
)
`,
);
} else {
baseJoins.push(
Prisma.sql`
Expand All @@ -193,6 +206,11 @@ export class MyReviewService {
LEFT JOIN challenges."ChallengeType" ct ON ct.id = c."typeId"
`,
);
countJoins.push(
Prisma.sql`
LEFT JOIN challenges."ChallengeType" ct ON ct.id = c."typeId"
`,
);

const metricJoins: Prisma.Sql[] = [
Prisma.sql`
Expand Down Expand Up @@ -318,7 +336,10 @@ export class MyReviewService {
[...baseJoins, ...metricJoins],
Prisma.sql``,
);
const countJoinClause = joinSqlFragments(baseJoins, Prisma.sql``);
const countJoinClause = joinSqlFragments(countJoins, Prisma.sql``);

const rowWhereFragments = [...whereFragments, ...rowExtras];
const countWhereFragments = [...whereFragments, ...countExtras];

if (challengeTypeId) {
whereFragments.push(Prisma.sql`c."typeId" = ${challengeTypeId}`);
Expand All @@ -340,7 +361,14 @@ export class MyReviewService {
);
}

const whereClause = joinSqlFragments(whereFragments, Prisma.sql` AND `);
const rowWhereClause = joinSqlFragments(
rowWhereFragments,
Prisma.sql` AND `,
);
const countWhereClause = joinSqlFragments(
countWhereFragments,
Prisma.sql` AND `,
);

const phaseEndExpression = Prisma.sql`
COALESCE(cp."actualEndDate", cp."scheduledEndDate")
Expand Down Expand Up @@ -416,10 +444,10 @@ export class MyReviewService {
const orderClause = joinSqlFragments(orderFragments, Prisma.sql`, `);

const countQuery = Prisma.sql`
SELECT COUNT(DISTINCT c.id) AS "total"
SELECT COUNT(*) AS "total"
FROM challenges."Challenge" c
${countJoinClause}
WHERE ${whereClause}
WHERE ${countWhereClause}
`;

const countQueryDetails = countQuery.inspect();
Expand Down Expand Up @@ -470,7 +498,7 @@ export class MyReviewService {
c.status AS "status"
FROM challenges."Challenge" c
${joinClause}
WHERE ${whereClause}
WHERE ${rowWhereClause}
ORDER BY ${orderClause}
LIMIT ${perPage}
OFFSET ${offset}
Expand Down
43 changes: 33 additions & 10 deletions src/api/scorecard/scorecard.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,22 +307,45 @@ export class ScoreCardService {
*/
async viewScorecard(id: string): Promise<ScorecardWithGroupResponseDto> {
try {
const data = await this.prisma.scorecard.findUniqueOrThrow({
where: { id },
include: {
scorecardGroups: {
include: {
sections: {
include: {
questions: true,
},
const include = {
scorecardGroups: {
include: {
sections: {
include: {
questions: true,
},
},
},
},
};

const scorecardById = await this.prisma.scorecard.findUnique({
where: { id },
include,
});

if (scorecardById) {
return scorecardById as ScorecardWithGroupResponseDto;
}

const scorecardByLegacyId = await this.prisma.scorecard.findFirst({
where: { legacyId: id },
include,
});
return data as ScorecardWithGroupResponseDto;

if (!scorecardByLegacyId) {
throw new NotFoundException({
message: `Scorecard with ID ${id} not found. Please check the ID and try again.`,
details: { scorecardId: id },
});
}

return scorecardByLegacyId as ScorecardWithGroupResponseDto;
} catch (error) {
if (error instanceof NotFoundException) {
throw error;
}

const errorResponse = this.prismaErrorService.handleError(
error,
`viewing scorecard with ID: ${id}`,
Expand Down
118 changes: 117 additions & 1 deletion src/api/submission/submission.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,23 @@
url: string | null;
};

interface TopgearSubmissionEventPayload {
submissionId: string;
challengeId: string;
submissionUrl: string;
memberHandle: string;
memberId: string;
submittedDate: string;
}

type TopgearSubmissionRecord = {
id: string;
challengeId: string | null;
memberId: string | null;
url: string | null;
createdAt: Date;
};

type SubmissionBusPayloadSource = Prisma.submissionGetPayload<{
select: {
id: true;
Expand Down Expand Up @@ -1399,6 +1416,95 @@
);
}

private async publishTopgearSubmissionEventIfEligible(
submission: TopgearSubmissionRecord,
): Promise<void> {
if (!submission.challengeId) {
this.logger.log(
`Submission ${submission.id} missing challengeId. Skipping Topgear event publish.`,
);
return;
}

const challenge = await this.challengeApiService.getChallengeDetail(
submission.challengeId,
);

if (!this.isTopgearTaskChallenge(challenge?.type)) {
this.logger.log(
`Challenge ${submission.challengeId} is not Topgear Task. Skipping immediate Topgear event for submission ${submission.id}.`,
);
return;
}

if (!submission.url) {
throw new InternalServerErrorException({
message:
'Updated submission does not contain a URL required for Topgear event payload.',
code: 'TOPGEAR_SUBMISSION_URL_MISSING',
details: { submissionId: submission.id },
});
}

if (!submission.memberId) {
throw new InternalServerErrorException({
message:
'Submission is missing memberId. Cannot publish Topgear event.',
code: 'TOPGEAR_SUBMISSION_MEMBER_MISSING',
details: { submissionId: submission.id },
});
}

const memberHandle = await this.lookupMemberHandle(
submission.challengeId,
submission.memberId,
);

if (!memberHandle) {
throw new InternalServerErrorException({
message: 'Unable to locate member handle for Topgear event payload.',
code: 'TOPGEAR_MEMBER_HANDLE_MISSING',
details: {
submissionId: submission.id,
challengeId: submission.challengeId,
memberId: submission.memberId,
},
});
}

const payload: TopgearSubmissionEventPayload = {
submissionId: submission.id,
challengeId: submission.challengeId,
submissionUrl: submission.url,
memberHandle,
memberId: submission.memberId,
submittedDate: submission.createdAt.toISOString(),
};

await this.eventBusService.publish('topgear.submission.received', payload);
this.logger.log(
`Published topgear.submission.received event for submission ${submission.id} immediately after creation.`,
);
}

private isTopgearTaskChallenge(typeName?: string): boolean {
return (typeName ?? '').trim().toLowerCase() === 'topgear task';
}

private async lookupMemberHandle(
challengeId: string,
memberId: string,
): Promise<string | null> {
const resource = await this.resourcePrisma.resource.findFirst({
where: {
challengeId,
memberId,
},
});

return resource?.memberHandle ?? null;
}

async createSubmission(
authUser: JwtUser,
body: SubmissionRequestDto,
Expand Down Expand Up @@ -1543,7 +1649,10 @@
!!file &&
((typeof file.size === 'number' && file.size > 0) ||
(file.buffer && file.buffer.length > 0));
const isFileSubmission = hasUploadedFile;
const hasS3Url =
typeof body.url === 'string' &&
body.url.includes('https://s3.amazonaws.com');

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

'
https://s3.amazonaws.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix

AI 14 days ago

The fix is to properly parse body.url and check its host rather than searching for a substring. Use the standard Node.js URL class to parse the URL. Then, check whether the host is one of the allowed hosts for S3 (e.g., s3.amazonaws.com or region-specific variants like s3.<region>.amazonaws.com).

In this code, to replace the substring check, introduce host checks such as:

  • host exactly equals s3.amazonaws.com
  • host ends with .s3.amazonaws.com
  • host matches common region patterns (e.g., my-bucket.s3.eu-west-1.amazonaws.com)

To implement this:

  • Replace line 1654 with: check that body.url is a string, parse it with new URL(body.url), and test the host.
  • Add try/catch to guard against invalid URLs.
  • Use a helper function or inline code for host matching; avoid changing broader logic outside this region.

No external packages are needed, as the built-in URL class suffices.

Suggested changeset 1
src/api/submission/submission.service.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/src/api/submission/submission.service.ts b/src/api/submission/submission.service.ts
--- a/src/api/submission/submission.service.ts
+++ b/src/api/submission/submission.service.ts
@@ -1649,9 +1649,25 @@
         !!file &&
         ((typeof file.size === 'number' && file.size > 0) ||
           (file.buffer && file.buffer.length > 0));
-      const hasS3Url =
-        typeof body.url === 'string' &&
-        body.url.includes('https://s3.amazonaws.com');
+      let hasS3Url = false;
+      if (typeof body.url === 'string') {
+        try {
+          const urlObj = new URL(body.url);
+          // Accept s3.amazonaws.com and any subdomain of s3.amazonaws.com
+          const s3Hosts = [
+            's3.amazonaws.com',
+          ];
+          // Accept region pattern: *.s3.amazonaws.com or *.s3.<region>.amazonaws.com
+          const host = urlObj.host;
+          hasS3Url =
+            s3Hosts.includes(host) ||
+            host.endsWith('.s3.amazonaws.com') ||
+            /^s3\.[a-z0-9-]+\.amazonaws\.com$/.test(host) ||
+            /^[^\.]+\.s3\.[a-z0-9-]+\.amazonaws\.com$/.test(host);
+        } catch (e) {
+          hasS3Url = false;
+        }
+      }
       const isFileSubmission = hasUploadedFile || hasS3Url;
 
       // Derive common metadata if available
EOF
@@ -1649,9 +1649,25 @@
!!file &&
((typeof file.size === 'number' && file.size > 0) ||
(file.buffer && file.buffer.length > 0));
const hasS3Url =
typeof body.url === 'string' &&
body.url.includes('https://s3.amazonaws.com');
let hasS3Url = false;
if (typeof body.url === 'string') {
try {
const urlObj = new URL(body.url);
// Accept s3.amazonaws.com and any subdomain of s3.amazonaws.com
const s3Hosts = [
's3.amazonaws.com',
];
// Accept region pattern: *.s3.amazonaws.com or *.s3.<region>.amazonaws.com
const host = urlObj.host;
hasS3Url =
s3Hosts.includes(host) ||
host.endsWith('.s3.amazonaws.com') ||
/^s3\.[a-z0-9-]+\.amazonaws\.com$/.test(host) ||
/^[^\.]+\.s3\.[a-z0-9-]+\.amazonaws\.com$/.test(host);
} catch (e) {
hasS3Url = false;
}
}
const isFileSubmission = hasUploadedFile || hasS3Url;

// Derive common metadata if available
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
const isFileSubmission = hasUploadedFile || hasS3Url;

// Derive common metadata if available
let systemFileName: string | undefined;
Expand Down Expand Up @@ -1588,6 +1697,13 @@
this.logger.log(
`Skipping AV scan event for submission ${data.id} because it is not a file-based submission.`,
);
await this.publishTopgearSubmissionEventIfEligible({
id: data.id,
challengeId: data.challengeId,
memberId: data.memberId,
url: data.url,
createdAt: data.createdAt,
});
}
// Increment challenge submission counters if challengeId present
if (body.challengeId) {
Expand Down