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.

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

fix / cleanup paths in integration tests #11809

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

valentijnscholten
Copy link
Member

@valentijnscholten valentijnscholten commented Feb 12, 2025

Remove unused variables and fix the error(s) in integration tests caused by Ruff linting PRs.

integration-tests-1  | ======================================================================
integration-tests-1  | ERROR: test_import_same_engagement_tests (__main__.CloseOldDedupeTest.test_import_same_engagement_tests)
integration-tests-1  | ----------------------------------------------------------------------
integration-tests-1  | Traceback (most recent call last):
integration-tests-1  |   File "/app/tests/base_test_class.py", line 24, in wrapper
integration-tests-1  |     return func(self, *args, **kwargs)
integration-tests-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
integration-tests-1  |   File "/app/tests/close_old_findings_dedupe_test.py", line 141, in test_import_same_engagement_tests
integration-tests-1  |     driver.find_element(By.ID, "id_file").send_keys(self.relative_path + "/dedupe_scans/dedupe_endpoint_1.xml")
integration-tests-1  |                                                     ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
integration-tests-1  | TypeError: unsupported operand type(s) for +: 'PosixPath' and 'str'

Copy link

dryrunsecurity bot commented Feb 12, 2025

DryRun Security Summary

The pull request contains minor changes to path handling and logging, with security findings primarily related to insecure test configurations, documentation concerns, and potential vulnerabilities in test workflows, though no critical issues were identified.

Expand for full summary

The pull request introduces minor changes across multiple files, primarily focusing on path handling improvements and debug logging in test and documentation files. Security findings include:

  1. In docker/entrypoint-integration-tests.sh:

    • Curl command uses insecure --insecure flag, bypassing SSL/TLS certificate validation
    • Potential injection risk with environment variable DD_BASE_URL
    • Unauthenticated HTTP requests and dynamic test file selection via environment variables
  2. In docs/content/en/open_source/upgrading/2.44.md:

    • Potential security consideration around Docker command execution context for deduplicating Burp scanner findings
  3. In test files (multiple files):

    • Selenium WebDriver interactions could be vulnerable to timing attacks
    • External repository cloning during tests
    • Potential information disclosure through detailed test workflows

No critical vulnerabilities were identified, but several security considerations and potential improvement areas were noted across the reviewed files.

Code Analysis

We ran 9 analyzers against 8 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

@valentijnscholten
Copy link
Member Author

@manuel-sommer @kiblik Could you take a look? I just converted it to str to make it work, but maybe the official end goal for Ruff is to have some magic done without converting to str but using /?

@kiblik
Copy link
Contributor

kiblik commented Feb 12, 2025

For the processing of paths in Python, it is recommended to use Path and related operations (like / instead of +).
And change it to string as the latest operation: .send_keys(str(Path(...) / "abc.def" ))
But you need to be careful, first character of the string after / should be non-/ (sounds a bit complicated). Example is the best:
.send_keys(self.relative_path + "/dedupe_scans/dedupe_endpoint_1.xml")
should not be
.send_keys(self.relative_path / "/dedupe_scans/dedupe_endpoint_1.xml")
but
.send_keys(self.relative_path / "dedupe_scans" / "dedupe_endpoint_1.xml")

@valentijnscholten
Copy link
Member Author

valentijnscholten commented Feb 13, 2025

@kiblik And will that return a str or a Path? Will send_keys accept it?

Would you be willing to make one more Ruff PR to make it look like you/Ruff wants it to be? I have a bunch of other things first to work on.

@valentijnscholten
Copy link
Member Author

I made some changes to make it work. The path in import_scanner_test is changed into a str as the code is doing some [:5] stuff on it (but this test is disabled anyway).

@valentijnscholten valentijnscholten force-pushed the fix-cleanup-paths-integration-tests branch from 49f6045 to 0e82b47 Compare February 16, 2025 14:43
@github-actions github-actions bot added the docs label Feb 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants