Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[🔨 WIP 🔨] Support running on ATS grid #736

Merged
merged 13 commits into from
Dec 13, 2023
Merged

[🔨 WIP 🔨] Support running on ATS grid #736

merged 13 commits into from
Dec 13, 2023

Conversation

nagpalkaran95
Copy link
Collaborator

@nagpalkaran95 nagpalkaran95 requested a review from a team as a code owner November 22, 2023 09:07
@nagpalkaran95 nagpalkaran95 changed the title Support running on ATS grid [🔨 WIP 🔨] Support running on ATS grid Nov 22, 2023
asambstack
asambstack requested changes Dec 6, 2023
View reviewed changes
bin/helpers/atsHelper.js Outdated Show resolved Hide resolved
asambstack
asambstack reviewed Dec 6, 2023
View reviewed changes
bin/commands/runs.js Outdated Show resolved Hide resolved
asambstack
asambstack previously approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@asambstack asambstack left a comment

Choose a reason for hiding this comment

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

LGTM

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 8, 2023
View reviewed changes
bin/helpers/utils.js

if (turboScaleSession) {
// remove unwanted path prefix for turboscale
files = files.map((x) => { return path.join(testFolderPath, x.split(testFolderPath)[1]) })

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal Warning

Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we are just trimming few path strings…removing prefix as we expect trimmed down path…
using path.join to be os agnostic.
no impact

bin/helpers/utils.js

if (turboScaleSession) {
// remove unwanted path prefix for turboscale
files = files.map((x) => { return path.join(testFolderPath, x.split(testFolderPath)[1]) })

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal Warning

Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we are just trimming few path strings…removing prefix as we expect trimmed down path…
using path.join to be os agnostic.

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 12, 2023
View reviewed changes
bin/helpers/atsHelper.js

exports.patchCypressConfigFileContent = (bsConfig) => {
try {
let cypressConfigFileData = fs.readFileSync(path.resolve(bsConfig.run_settings.cypress_config_file)).toString();

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal Warning

Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no impact can be ignored. check above comment

bin/helpers/atsHelper.js
}

exports.atsFileCleanup = (bsConfig) => {
const filePath = path.resolve(bsConfig.run_settings.patched_cypress_config_file);

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal Warning

Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no impact can be ignored. check above comment

@nagpalkaran95
Copy link
Collaborator Author

Semgrep issue can be ignored and force merged as per slack thread - https://browserstack.slack.com/archives/CR42BQ3AQ/p1702374508668739?thread_ts=1702357970.975999&cid=CR42BQ3AQ

@francisf francisf merged commit 89eaf83 into master Dec 13, 2023
1 of 2 checks passed
@francisf francisf deleted the HST_722_ats branch December 13, 2023 11:10
@pranavj1001 pranavj1001 mentioned this pull request Dec 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KrishnaSuravarapu KrishnaSuravarapu KrishnaSuravarapu approved these changes

@asambstack asambstack asambstack approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nagpalkaran95 @KrishnaSuravarapu @asambstack @francisf