dev to master #10
dev to master #10
Conversation
Submission and Final Fix for Update Code with Postgres Schema Part 2
add stats and history endpoints
migrate dynamo data
…history add subTrackId in DataScienceHistory schema
update trait data list item type
distribution fix
…n a few places in WM and system-admin app
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| install_deploysuite: &install_deploysuite | ||
| name: Installation of install_deploysuite. | ||
| command: | | ||
| git clone --branch v1.4.19 https://github.com/topcoder-platform/tc-deploy-scripts ../buildscript |
There was a problem hiding this comment.
[❗❗ security]
Cloning a specific branch from a public GitHub repository without verifying the integrity of the scripts can introduce security risks. Consider verifying the integrity of the scripts, for example, by checking a hash or using a trusted source.
| source awsenvconf | ||
| ./psvar-processor.sh -t appenv -p /config/${APPNAME}/deployvar | ||
| source deployvar_env | ||
| ./master_deploy.sh -d ECS -e $DEPLOY_ENV -t latest -j /config/${APPNAME}/appvar,/config/common/global-appvar -i ${APPNAME} -p FARGATE |
There was a problem hiding this comment.
[❗❗ security]
Using environment variables directly in scripts can lead to security issues if they are not properly sanitized or if they contain sensitive information. Ensure that sensitive data is handled securely.
| jobs: | ||
| # Build & Deploy against development backend | ||
| "build-dev": | ||
| !!merge <<: *defaults |
There was a problem hiding this comment.
[correctness]
The use of !!merge is not a standard YAML feature and may cause issues with some parsers. Consider using a more standard approach to merge YAML anchors.
| steps: *builddeploy_steps | ||
|
|
||
| "build-prod": | ||
| !!merge <<: *defaults |
There was a problem hiding this comment.
[correctness]
The use of !!merge is not a standard YAML feature and may cause issues with some parsers. Consider using a more standard approach to merge YAML anchors.
| with: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # The GITHUB_TOKEN is there by default so you just need to keep it like it is and not necessarily need to add it as secret as it will throw an error. [More Details](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret) | ||
| LAB45_API_KEY: ${{ secrets.LAB45_API_KEY }} | ||
| exclude: '**/*.json, **/*.md, **/*.jpg, **/*.png, **/*.jpeg, **/*.bmp, **/*.webp' # Optional: exclude patterns separated by commas No newline at end of file |
There was a problem hiding this comment.
[💡 style]
Consider adding a newline at the end of the file to improve compatibility with various tools and editors, which often expect files to end with a newline character.
| @@ -0,0 +1 @@ | |||
| 21.6 No newline at end of file | |||
There was a problem hiding this comment.
[💡 style]
Consider adding a newline at the end of the file to adhere to POSIX standards, which can improve compatibility with various tools and systems.
| Joi.perPage = () => Joi.number().integer().min(1).max(100).default(50) | ||
| Joi.size = () => Joi.number().integer().min(1).max(1000).default(500) | ||
| Joi.sort = () => Joi.string().default('asc') | ||
| Joi.positive = () => Joi.number().integer().min(0) |
There was a problem hiding this comment.
[💡 readability]
Consider renaming Joi.positive to Joi.nonNegative to more accurately reflect that the minimum value is 0, which includes zero as a valid value.
| app.set('port', config.PORT) | ||
|
|
||
| // Swagger / OpenAPI documentation | ||
| const swaggerDocument = YAML.load(path.join(__dirname, 'docs', 'swagger.yaml')) |
There was a problem hiding this comment.
[❗❗ security]
Loading the Swagger document using YAML.load can be a potential security risk if the YAML file contains untrusted content. Consider using a safer method or validating the content after loading.
|
|
||
| // Swagger / OpenAPI documentation | ||
| const swaggerDocument = YAML.load(path.join(__dirname, 'docs', 'swagger.yaml')) | ||
| swaggerDocument.basePath = `/${config.API_VERSION}` |
There was a problem hiding this comment.
[maintainability]
Directly modifying the basePath of the Swagger document might lead to unexpected behavior if the document is used elsewhere. Ensure that this change is intended and does not affect other parts of the application.
| @@ -0,0 +1,23 @@ | |||
| #!/bin/bash | |||
| set -eo pipefail | |||
| APP_NAME=$1 | |||
There was a problem hiding this comment.
[❗❗ correctness]
Consider adding a check to ensure APP_NAME is not empty before proceeding. This will prevent potential errors when the script is executed without an argument.
| mv package-lock.json old-package-lock.json | ||
| docker cp app:/$APP_NAME/package-lock.json package-lock.json | ||
| set +eo pipefail | ||
| UPDATE_CACHE=$(cmp package-lock.json old-package-lock.json) |
There was a problem hiding this comment.
[❗❗ correctness]
The use of cmp to compare package-lock.json files will result in UPDATE_CACHE being empty if the files are identical. This could lead to unexpected behavior in the subsequent conditional check. Consider using a more explicit comparison method that sets UPDATE_CACHE to a clear boolean value.
| if [ "$UPDATE_CACHE" == 1 ] | ||
| then | ||
| docker cp app:/$APP_NAME/node_modules . | ||
| fi No newline at end of file |
There was a problem hiding this comment.
[💡 style]
Add a newline at the end of the file to adhere to POSIX standards and avoid potential issues with certain text processing tools.
| // Gamification | ||
| MAMBO_PUBLIC_KEY: process.env.MAMBO_PUBLIC_KEY, | ||
| MAMBO_PRIVATE_KEY: process.env.MAMBO_PRIVATE_KEY, | ||
| MAMBO_DOMAIN_URL: process.env.MAMBO_DOMAIN_URL, |
There was a problem hiding this comment.
[❗❗ correctness]
The removal of the LOOKER configuration block could impact functionality if any part of the application relies on Looker API access. Ensure that this removal is intentional and that no dependent code will break due to missing configuration.
| FROM node:21.6.0 | ||
|
|
||
| # Copy the current directory into the Docker image | ||
| COPY . /member-api-v6 |
There was a problem hiding this comment.
[security]
Copying the entire current directory into the Docker image can lead to larger image sizes and potential security risks if sensitive files are included. Consider using a .dockerignore file to exclude unnecessary files and directories.
| WORKDIR /member-api-v6 | ||
|
|
||
| # Install the dependencies from package.json | ||
| RUN yarn |
There was a problem hiding this comment.
[correctness]
Using yarn without specifying --frozen-lockfile can lead to inconsistent dependencies being installed if the yarn.lock file is not up-to-date. Consider using RUN yarn install --frozen-lockfile to ensure consistency.
| # Install the dependencies from package.json | ||
| RUN yarn | ||
|
|
||
| CMD node app.js No newline at end of file |
There was a problem hiding this comment.
[💡 style]
There is no newline at the end of the file. While this is not a functional issue, it is a common convention to end files with a newline to avoid potential issues with concatenation or certain text processing tools.
| winston.info('Received Auth0 request') | ||
| // return config/test.js#M2M_FULL_ACCESS_TOKEN | ||
| res.status(200).json({ access_token: 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJodHRwczovL3RvcGNvZGVyLWRldi5hdXRoMC5jb20vIiwic3ViIjoiZW5qdzE4MTBlRHozWFR3U08yUm4yWTljUVRyc3BuM0JAY2xpZW50cyIsImF1ZCI6Imh0dHBzOi8vbTJtLnRvcGNvZGVyLWRldi5jb20vIiwiaWF0IjoxNTUwOTA2Mzg4LCJleHAiOjE2ODA5OTI3ODgsImF6cCI6ImVuancxODEwZUR6M1hUd1NPMlJuMlk5Y1FUcnNwbjNCIiwic2NvcGUiOiJhbGw6bWVtYmVycyIsImd0eSI6ImNsaWVudC1jcmVkZW50aWFscyJ9.Eo_cyyPBQfpWp_8-NSFuJI5MvkEV3UJZ3ONLcFZedoA' }) | ||
| res.status(200).json({ access_token: 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJodHRwczovL3RvcGNvZGVyLWRldi5hdXRoMC5jb20vIiwic3ViIjoiZW5qdzE4MTBlRHozWFR3U08yUm4yWTljUVRyc3BuM0JAY2xpZW50cyIsImF1ZCI6Imh0dHBzOi8vbTJtLnRvcGNvZGVyLWRldi5jb20vIiwiaWF0IjoxNTUwOTA2Mzg4LCJleHAiOjE2ODA5OTI3ODgsImF6cCI6ImVuancxODEwZUR6M1hUd1NPMlJuMlk5Y1FUcnNwbjNCIiwic2NvcGUiOiJhbGw6bWVtYmVycyIsImd0eSI6ImNsaWVudC1jcmVkZW50aWFscyJ9.Eo_cyyPBQfpWp_8-NSFuJI5MvkEV3UJZ3ONLcFZedoA' }) |
There was a problem hiding this comment.
[❗❗ security]
The hardcoded JWT token in the response could pose a security risk if this code is ever used in a production environment. Consider using environment variables or a configuration file to manage sensitive information.
| res.status(200).json({}); | ||
| }); | ||
| winston.info('Received bus events') | ||
| winston.info(JSON.stringify(req.body, null, 2)) |
There was a problem hiding this comment.
[❗❗ security]
Logging the entire request body with winston.info(JSON.stringify(req.body, null, 2)) could expose sensitive information in the logs. Ensure that no sensitive data is logged, or sanitize the data before logging.
| if (allGroups.includes(groupId)) { | ||
| res.json({ id: groupId }) | ||
| } else { | ||
| res.status(404).json(null) |
There was a problem hiding this comment.
[💡 readability]
Returning null in the JSON response for a 404 status might not be informative for the client. Consider returning a more descriptive error message, such as { "error": "Group not found" }.
No description provided.