Skip to content

Conversation

@kkartunov
Copy link
Contributor

No description provided.

)
{/* Always show Review link for v5 challenges using v5 ID */}
<>
<a href={reviewUrl} target={'_blank'} rel='noopener noreferrer' onClick={onClick}>Review</a>

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML Medium

DOM text
is reinterpreted as HTML without escaping meta-characters.
DOM text
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix

AI 19 days ago

To fix this issue, we should ensure that the value used for the challenge.id in the URL is strictly sanitized and/or type-checked as a valid non-malicious identifier before using it in the anchor's href attribute.

The best fix is to:

  • Coerce challenge.id to a string containing only safe characters appropriate for an identifier. Since IDs are typically numbers, we should ensure only numeric data is passed through, e.g. using Number(challenge.id) or a more robust parsing such as parseInt.
  • Alternatively, explicitly encode the URI component portion containing challenge.id via encodeURIComponent, which ensures any unexpected input is safely escaped for use in the URL.
  • Apply this fix within src/components/LegacyLinks/index.js, at the point where reviewUrl is constructed.

No new methods are needed; you can use JavaScript's built-in encodeURIComponent.


Suggested changeset 1
src/components/LegacyLinks/index.js

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/components/LegacyLinks/index.js b/src/components/LegacyLinks/index.js
--- a/src/components/LegacyLinks/index.js
+++ b/src/components/LegacyLinks/index.js
@@ -12,7 +12,7 @@
     e.stopPropagation()
   }, [])
 
-  const reviewUrl = `${REVIEW_APP_URL}/active-challenges/${challenge.id}/challenge-details`
+  const reviewUrl = `${REVIEW_APP_URL}/active-challenges/${encodeURIComponent(challenge.id)}/challenge-details`
   return (
     <div className={styles.container}>
       {/* Always show Review link for v5 challenges using v5 ID */}
EOF
@@ -12,7 +12,7 @@
e.stopPropagation()
}, [])

const reviewUrl = `${REVIEW_APP_URL}/active-challenges/${challenge.id}/challenge-details`
const reviewUrl = `${REVIEW_APP_URL}/active-challenges/${encodeURIComponent(challenge.id)}/challenge-details`
return (
<div className={styles.container}>
{/* Always show Review link for v5 challenges using v5 ID */}
Copilot is powered by AI and may make mistakes. Always verify output.
name: Installation of install_deploysuite.
command: |
git clone --branch v1.4.15 https://github.com/topcoder-platform/tc-deploy-scripts ../buildscript
git clone --branch v1.4.17 https://github.com/topcoder-platform/tc-deploy-scripts ../buildscript
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The branch version for tc-deploy-scripts has been updated from v1.4.15 to v1.4.17. Ensure that this new version is compatible with the rest of the deployment scripts and does not introduce breaking changes.

./awsconfiguration.sh $DEPLOY_ENV
./buildenv.sh -e $DEPLOY_ENV -b ${LOGICAL_ENV}-${APPNAME}-buildvar
#./buildenv.sh -e $DEPLOY_ENV -b ${LOGICAL_ENV}-${APPNAME}-buildvar
source awsenvconf
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The buildenv.sh script has been commented out and replaced with psvar-processor.sh. Verify that psvar-processor.sh provides all necessary environment configurations previously handled by buildenv.sh to avoid missing configurations.

./master_deploy.sh -d ECS -e $DEPLOY_ENV -t latest -s ${LOGICAL_ENV}-global-appvar,${LOGICAL_ENV}-${APPNAME}-appvar -i ${APPNAME} -p FARGATE
#./buildenv.sh -e $DEPLOY_ENV -b ${LOGICAL_ENV}-${APPNAME}-deployvar
#source buildenvvar
./psvar-processor.sh -t appenv -p /config/${APPNAME}/deployvar
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The buildenv.sh script has been commented out and replaced with psvar-processor.sh. Ensure that psvar-processor.sh correctly sets up the environment variables needed for deployment, as this change could affect the deployment process.

./psvar-processor.sh -t appenv -p /config/${APPNAME}/deployvar
source deployvar_env
#./master_deploy.sh -d ECS -e $DEPLOY_ENV -t latest -s ${LOGICAL_ENV}-global-appvar,${LOGICAL_ENV}-${APPNAME}-appvar -i ${APPNAME} -p FARGATE
./master_deploy.sh -d ECS -e $DEPLOY_ENV -t latest -j /config/common/global-appvar,/config/${APPNAME}/appvar -i ${APPNAME} -p FARGATE
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The master_deploy.sh command now uses a different set of parameters for the -j option. Confirm that the new paths /config/common/global-appvar,/config/${APPNAME}/appvar are correct and accessible during deployment.

./awsconfiguration.sh $DEPLOY_ENV
./buildenv.sh -e $DEPLOY_ENV -b ${LOGICAL_ENV}-${APPNAME}-buildvar
#./buildenv.sh -e $DEPLOY_ENV -b ${LOGICAL_ENV}-${APPNAME}-buildvar
./psvar-processor.sh -t appenv -p /config/${APPNAME}/buildvar
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The buildenv.sh script has been commented out in the smoke testing configuration. Ensure that psvar-processor.sh provides all necessary environment configurations previously handled by buildenv.sh to avoid missing configurations during smoke tests.

filters: &filters-dev
branches:
only: ["develop", "PM-803_wm-regression-fixes", "PM-902_show-all-projects-on-challenge-page", "pm-1365"]
only: ["develop"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The branch filter for development builds has been changed to only include the develop branch. Ensure this aligns with the intended workflow and that no other branches should trigger development builds.


permissions:
contents: read
security-events: write
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ security]
The security-events: write permission is quite broad. Ensure that this level of access is necessary, as it could pose a security risk if not properly justified.

jobs:
trivy-scan:
name: Use Trivy
runs-on: ubuntu-24.04
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
Consider using a stable or LTS version of Ubuntu (e.g., ubuntu-latest or ubuntu-22.04) to ensure long-term support and stability for the workflow.

ignore-unfixed: true
format: "sarif"
output: "trivy-results.sarif"
severity: "CRITICAL,HIGH,UNKNOWN"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The severity levels include UNKNOWN, which might lead to unexpected results. Ensure this is intentional and aligns with your security policy.

output: "trivy-results.sarif"
severity: "CRITICAL,HIGH,UNKNOWN"
scanners: vuln,secret,misconfig,license
github-pat: ${{ secrets.GITHUB_TOKEN }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ security]
Ensure that the GITHUB_TOKEN has the necessary permissions and is securely managed, as it is used to authenticate the Trivy scanner.

*.env
*.pem
*.vscode
*.pem
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
The entry *.pem is duplicated. Consider removing the duplicate to maintain clarity and avoid confusion.

*.pem
*.vscode
*.pem
*.vscode
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
The entry *.vscode is duplicated. Consider removing the duplicate to maintain clarity and avoid confusion.

@@ -1,5 +1,5 @@
# Use the base image with Node.js
FROM node:12
# Use Node.js 22 base image

Check notice

Code scanning / Trivy

No HEALTHCHECK defined Low

Artifact: docker/Dockerfile
Type: dockerfile
Vulnerability DS026
Severity: LOW
Message: Add HEALTHCHECK instruction in your Dockerfile
Link: DS026
else
# Compare lockfile from container with local copy
cp "$LOCK_FILE_NAME" ".old-$LOCK_FILE_NAME"
docker cp "app:/$APP_NAME/$LOCK_FILE_NAME" "$LOCK_FILE_NAME"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
Consider checking the exit status of docker cp to ensure the lockfile is copied successfully. This will help avoid silent failures if the copy operation fails.

fi
if [ "$UPDATE_CACHE" -eq 1 ]; then
echo "Lockfile changed or node_modules missing; refreshing local node_modules from container..."
rm -rf node_modules
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ security]
The use of rm -rf node_modules is potentially dangerous as it will forcefully remove the directory without confirmation. Ensure that this is the intended behavior and that there are no unintended side effects.

CHALLENGE_PHASES_URL: `${DEV_API_HOSTNAME}/v6/challenge-phases`,
CHALLENGE_TIMELINES_URL: `${DEV_API_HOSTNAME}/v6/challenge-timelines`,
COPILOTS_URL: 'https://copilots.topcoder-dev.com',
PROJECT_API_URL: `${DEV_API_HOSTNAME}/v5/projects`,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The PROJECT_API_URL is still using the /v5 endpoint, while other similar URLs have been updated to /v6. Verify if this is intentional or if it should be updated to maintain consistency.

PROJECT_API_URL: `${DEV_API_HOSTNAME}/v5/projects`,
GROUPS_API_URL: `${DEV_API_HOSTNAME}/v5/groups`,
GROUPS_API_URL: `${DEV_API_HOSTNAME}/v6/groups`,
TERMS_API_URL: `${DEV_API_HOSTNAME}/v5/terms`,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The TERMS_API_URL is still using the /v5 endpoint, while other similar URLs have been updated to /v6. Verify if this is intentional or if it should be updated to maintain consistency.

@@ -1,10 +1,18 @@
module.exports = (() => {
let env = process.env.NODE_ENV || 'development'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
Consider using const instead of let for env since its value is not reassigned after initialization. This can prevent accidental reassignment and improve code clarity.

}

// Support explicit local environment (mirrors platform-ui local setup)
const hostEnv = process.env.HOST_ENV
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
Consider using const instead of let for hostEnv since its value is not reassigned after initialization. This can prevent accidental reassignment and improve code clarity.

const API_V5 = `${DEV_API_HOSTNAME}/v5`

// Local service endpoints mirror platform-ui local.env.ts overrides
const LOCAL_CHALLENGE_API = 'http://localhost:3000/v6'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ security]
Consider using https instead of http for local service endpoints to ensure encrypted communication, even in local development environments.

CP_TRACK_ID: '9d6e0de8-df14-4c76-ba0a-a9a8cb03a4ea',
CHALLENGE_TYPE_ID: '927abff4-7af9-4145-8ba1-577c16e64e2e',
MARATHON_TYPE_ID: '929bc408-9cf2-4b3e-ba71-adfbf693046c',
SEGMENT_API_KEY: 'QBtLgV8vCiuRX1lDikbMjcoe9aCHkF6n',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ security]
The SEGMENT_API_KEY is hardcoded in the file. Consider using environment variables to store sensitive information like API keys to avoid exposing them in the codebase.

SUBMISSIONS_API_URL: `${PROD_API_HOSTNAME}/v5/submissions`,
SUBMISSIONS_API_URL: `${PROD_API_HOSTNAME}/v6/submissions`,
REVIEW_TYPE_API_URL: `${PROD_API_HOSTNAME}/v5/reviewTypes`,
REVIEWS_API_URL: `${PROD_API_HOSTNAME}/v5/reviews`, //update to use v6
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The comment //update to use v6 suggests an intention to update REVIEWS_API_URL to use version 6, but the URL is still pointing to v5. Ensure the URL is updated to ${PROD_API_HOSTNAME}/v6/reviews if the API version change is intended.

SUBMISSIONS_API_URL: `${PROD_API_HOSTNAME}/v6/submissions`,
REVIEW_TYPE_API_URL: `${PROD_API_HOSTNAME}/v5/reviewTypes`,
REVIEWS_API_URL: `${PROD_API_HOSTNAME}/v5/reviews`, //update to use v6
SCORECARDS_API_URL: `${PROD_API_HOSTNAME}/v5/scorecards`, // update to use v6
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The comment // update to use v6 indicates an intention to update SCORECARDS_API_URL to version 6, but the URL remains at v5. Verify and update the URL to ${PROD_API_HOSTNAME}/v6/scorecards if the change is required.

REVIEW_TYPE_API_URL: `${PROD_API_HOSTNAME}/v5/reviewTypes`,
REVIEWS_API_URL: `${PROD_API_HOSTNAME}/v5/reviews`, //update to use v6
SCORECARDS_API_URL: `${PROD_API_HOSTNAME}/v5/scorecards`, // update to use v6
WORKFLOWS_API_URL: `${PROD_API_HOSTNAME}/v5/workflows`, // update to use v6
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The comment // update to use v6 suggests WORKFLOWS_API_URL should be updated to v6, but it still points to v5. Confirm and update the URL to ${PROD_API_HOSTNAME}/v6/workflows if necessary.

// Add /* filename */ comments to generated require()s in the output.
pathinfo: isEnvDevelopment,
// Ensure compatible hashing algorithm on modern Node versions
hashFunction: 'xxhash64',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ performance]
The change to use xxhash64 for hashFunction is beneficial for performance, but ensure compatibility across environments where the build will be deployed.

minimizer: [
// This is only used in production mode
new TerserPlugin({
parallel: true,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ performance]
Enabling parallel for TerserPlugin is a good performance improvement, but verify that it doesn't cause issues in environments with limited CPU resources.

// This is only used in production mode
new TerserPlugin({
parallel: true,
extractComments: false,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
Disabling extractComments in TerserPlugin may affect legal compliance if the build includes third-party code with required license comments. Ensure this is acceptable for your use case.

inject: true,
template: paths.appHtml
// Use raw template content to avoid child compilation issues
templateContent: fs.readFileSync(paths.appHtml, 'utf8')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
Switching to templateContent for HtmlWebpackPlugin may lead to issues if the HTML template contains dynamic content or requires processing. Ensure this change doesn't affect the expected output.

}),
// Merge our app constants with the standard env variables into a single DefinePlugin
(() => {
const envMap = env.stringified && env.stringified['process.env']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The merging of environment variables and constants into a single DefinePlugin instance is a good approach for maintainability, but ensure that there are no conflicts between the keys in envMap and constMap.

chunkFilename: 'static/css/[name].[contenthash:8].chunk.css'
}),
// Polyfill select Node core modules for browser bundles (Webpack 5)
new NodePolyfillPlugin(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ performance]
Adding NodePolyfillPlugin is necessary for Webpack 5, but verify that it doesn't introduce unnecessary polyfills that could increase bundle size.

performance: false
performance: false,
// When debugging, surface more internal logs from webpack itself
infrastructureLogging: WM_DEBUG ? { level: 'verbose' } : { level: 'warn' }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
The use of WM_DEBUG for infrastructureLogging is a useful debugging feature, but ensure it's properly documented so developers know how to enable verbose logging.


const protocol = process.env.HTTPS === 'true' ? 'https' : 'http'
const host = process.env.HOST || '0.0.0.0'
const WM_DEBUG = /^(1|true|on|yes)$/i.test(String(process.env.WM_DEBUG || ''))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The regular expression used to parse WM_DEBUG environment variable is case-insensitive and checks for multiple truthy values. Ensure this aligns with the intended use case, as it may lead to unexpected behavior if the environment variable is set to a non-standard truthy value.

}
// Add /health endpoint in dev to mirror production server.js
try {
const healthCheck = require('topcoder-healthcheck-dropin')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The require statement for topcoder-healthcheck-dropin is inside a try block. If this module is not installed, it will fail silently unless WM_DEBUG is enabled. Consider handling this more explicitly to avoid missing the health check functionality in development.

# Try to fetch the JSON from S3; fall back to local file if AWS CLI is missing or S3 copy fails
JSON_CONTENT=""
if command -v aws >/dev/null 2>&1; then
if JSON_CONTENT=$(aws s3 cp "${APPVAR_S3_URI}" - 2>/dev/null); then
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The use of aws s3 cp without specifying a region or profile might lead to unexpected behavior if the default AWS configuration is not set correctly. Consider explicitly setting the region and profile if applicable.


if command -v nvm >/dev/null 2>&1; then
echo "[work-manager] Using Node via nvm..."
nvm use
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The command nvm use without specifying a version can lead to using an unintended Node.js version if multiple versions are installed. Consider specifying a version to ensure consistency.

-n "${PROXY_HOST}" \
-s "${PROXY_SOURCE_PORT}" \
-t "${PROXY_TARGET_PORT}" \
>"${PROXY_LOG_FILE}" 2>&1 & echo $! > "${PROXY_PID_FILE}" )
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The background process for local-ssl-proxy is started without checking if it starts successfully. Consider adding a check to ensure the proxy starts correctly and handle any errors.

fi

echo "[work-manager] Starting app: ${RUNNER} run start:dev (PORT=${PORT})"
"${RUNNER}" run start:dev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The command ${RUNNER} run start:dev assumes that the start:dev script is defined in the package.json. Ensure that this script exists to prevent runtime errors.

# Use the base image with Node.js
FROM node:12
# Use Node.js 22 base image
FROM node:22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The change from Node.js version 12 to 22 is significant. Ensure that all dependencies and the application itself are compatible with Node.js 22, as there may be breaking changes between these major versions.

RUN chown -R appuser:appuser /challenge-engine-ui

# Enable corepack and prepare pnpm for installs (run as root)
RUN corepack enable && corepack prepare pnpm@9.12.0 --activate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The command corepack prepare pnpm@9.12.0 --activate is used to activate a specific version of pnpm. Verify that this version is compatible with your project dependencies and that no breaking changes have been introduced.

RUN echo "BABEL ENV in Docker: $BABEL_ENV"
RUN npm install
RUN npm run lint
RUN pnpm install
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
Switching from npm install to pnpm install can lead to differences in how dependencies are resolved and installed. Ensure that the project dependencies are correctly handled by pnpm, especially if there are any specific npm scripts or configurations that might not be compatible.

@kkartunov kkartunov merged commit eac06e0 into master Nov 2, 2025
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants