Update testing improve poetry by dreadn0ught · Pull Request #45 · ossprey/ossprey-python-client

dreadn0ught · 2026-02-10T14:41:12Z

High-level PR Summary

This PR refactors the testing infrastructure by introducing comprehensive smoke tests, reorganizing test packages into a dedicated test/test_packages/ directory, and improving Poetry dependency handling. The changes include creating a new smoke test suite that exercises end-to-end CLI scenarios for Python, npm, and GitHub repositories, updating the pre-push hooks to use the new smoke tests, adding graceful fallback from Poetry to pipenv when Poetry scans fail, and restructuring the scan logic for better modularity. All existing test references have been updated to point to the new test package location.

⏱️ Estimated Review Time: 30-90 minutes

💡 Review Order Suggestion

Order	File Path
1	`test/smoke/conftest.py`
2	`test/smoke/test_smoke.py`
3	`.githooks/pre-push`
4	`.github/workflows/test.yml`
5	`ossprey/scan.py`
6	`ossprey/sbom_python.py`
7	`ossprey/test_main.py`
8	`ossprey/test_scan.py`
9	`ossprey/test_mode_fs.py`
10	`ossprey/test_sbom_python.py`
11	`ossprey/test_virtualenv.py`
12	`test/test_packages/python_simple_math/setup.py`
13	`test/test_packages/python_simple_math/simple_math/__init__.py`
14	`test/test_packages/python_simple_math/simple_math/operations.py`
15	`test/test_packages/poetry_broken_simple_math/pyproject.toml`
16	`test/test_packages/poetry_simple_math/.gitignore`
17	`test/test_packages/poetry_simple_math/poetry_simple_math/__init__.py`

recurseml

Review by RecurseML

🔍 Review performed on 647cae9..56fa816

Severity	Location	Issue	Delete
	ossprey/sbom_python.py:137	Missing exception handling causes sys.path corruption

✅ Files analyzed, no issues (14)

• .githooks/pre-push
• .github/workflows/test.yml
• ossprey/scan.py
• ossprey/test_main.py
• ossprey/test_mode_fs.py
• ossprey/test_sbom_python.py
• ossprey/test_scan.py
• ossprey/test_virtualenv.py
• test/smoke/conftest.py
• test/smoke/test_smoke.py
• test/test_packages/poetry_broken_simple_math/pyproject.toml
• test/test_packages/poetry_simple_math/.gitignore
• test/test_packages/python_simple_math/setup.py
• test/test_packages/python_simple_math/simple_math/operations.py

⏭️ Files skipped (43)

Locations
`test/test_packages/docker_js_simple_math/Dockerfile`
`test/test_packages/docker_js_simple_math/build.sh`
`test/test_packages/docker_js_simple_math/javascript/.gitignore`
`test/test_packages/docker_js_simple_math/javascript/README.md`
`test/test_packages/docker_js_simple_math/javascript/package-lock.json`
`test/test_packages/docker_js_simple_math/javascript/package.json`
`test/test_packages/docker_js_simple_math/javascript/src/index.js`
`test/test_packages/docker_js_simple_math/javascript/test/simpleMath.test.js`
`test/test_packages/docker_js_simple_math/javascript/yarn.lock`
`test/test_packages/docker_py_simple_math/Dockerfile`
`test/test_packages/docker_py_simple_math/build.sh`
`test/test_packages/docker_py_simple_math/python/README.md`
`test/test_packages/docker_py_simple_math/python/main.py`
`test/test_packages/docker_py_simple_math/python/requirements.txt`
`test/test_packages/npm_simple_math/.gitignore`
`test/test_packages/npm_simple_math/README.md`
`test/test_packages/npm_simple_math/package-lock.json`
`test/test_packages/npm_simple_math/package.json`
`test/test_packages/npm_simple_math/src/index.js`
`test/test_packages/npm_simple_math/test/simpleMath.test.js`
`test/test_packages/poetry_broken_simple_math/.gitignore`
`test/test_packages/poetry_broken_simple_math/README.md`
`test/test_packages/poetry_broken_simple_math/poetry_simple_math/__init__.py`
`test/test_packages/poetry_broken_simple_math/poetry_simple_math/operations.py`
`test/test_packages/poetry_broken_simple_math/setup.py`
`test/test_packages/poetry_simple_math/README.md`
`test/test_packages/poetry_simple_math/poetry_simple_math/__init__.py`
`test/test_packages/poetry_simple_math/poetry_simple_math/operations.py`
`test/test_packages/poetry_simple_math/pyproject.toml`
`test/test_packages/python_simple_math/requirements.txt`
`test/test_packages/python_simple_math/simple_math/__init__.py`
`test/test_packages/yarn_massive_math/.gitignore`
`test/test_packages/yarn_massive_math/README.md`
`test/test_packages/yarn_massive_math/package.json`
`test/test_packages/yarn_massive_math/src/index.js`
`test/test_packages/yarn_massive_math/test/simpleMath.test.js`
`test/test_packages/yarn_massive_math/yarn.lock`
`test/test_packages/yarn_simple_math/.gitignore`
`test/test_packages/yarn_simple_math/README.md`
`test/test_packages/yarn_simple_math/package.json`
`test/test_packages/yarn_simple_math/src/index.js`
`test/test_packages/yarn_simple_math/test/simpleMath.test.js`
`test/test_packages/yarn_simple_math/yarn.lock`

recurseml · 2026-02-10T14:44:23Z

+    return ossbom
+
+
+def update_sbom_from_virtualenv(ossbom: OSSBOM, package_name: str) -> OSSBOM:


Resource leak and sys.path corruption vulnerability. The function creates a VirtualEnv and calls venv.enter() which modifies sys.path globally (line 140). If any exception occurs during venv.install_package() (line 142), venv.create_requirements_file_from_env() (line 143), or update_sbom_from_requirements() (line 145), the venv.exit() call on line 147 will never execute. This leaves sys.path in a corrupted state for the remainder of the process execution.

While VirtualEnv has a del method that eventually calls exit(), garbage collection is not immediate. Until the venv object is garbage collected, any subsequent code in the same process will have the wrong sys.path, potentially importing packages from the temporary virtual environment instead of the correct environment. This could lead to:

Incorrect package imports in subsequent scan operations

Hard-to-debug runtime errors where the wrong version of a package is imported

Security issues if malicious packages in the temp venv are imported instead of trusted packages

The function is called from scan.py line 45 during pipenv mode scanning. If a pipenv scan fails (e.g., package installation fails, which is common with broken packages), sys.path corruption affects all subsequent operations in the same scan() execution.

Fix: Wrap the venv operations in a try/finally block to ensure venv.exit() is always called, or use VirtualEnv as a context manager.

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

@copilot can you address the suggestion provided here

I'd be happy to help address this suggestion! However, I need to see the actual code being discussed to provide specific guidance.

From the comment, I can see that the issue is about a resource leak in a function that:

Creates a VirtualEnv object

Calls venv.enter() (modifying sys.path)

Performs some operations (install_package, create_requirements_file, etc.)

Calls venv.exit() to clean up

The problem is that if an exception occurs during the operations, venv.exit() never gets called, leaving sys.path corrupted.

Recommended fix:

If VirtualEnv supports being used as a context manager, the cleanest solution would be:

with VirtualEnv(...) as venv: venv.install_package(...) venv.create_requirements_file_from_env(...) update_sbom_from_requirements(...)

If it doesn't support context manager protocol, use try/finally:

venv = VirtualEnv(...) venv.enter() try: venv.install_package(...) venv.create_requirements_file_from_env(...) update_sbom_from_requirements(...) finally: venv.exit()

Could you share the relevant code snippet so I can provide a more specific solution for your situation?

Copilot · 2026-02-10T14:49:04Z

@dreadn0ught I've opened a new pull request, #46, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

This PR refactors the test/fixture layout and adds an end-to-end smoke test suite to validate ossprey CLI scanning across Python (requirements/poetry/pip install), npm/yarn, and GitHub repo scenarios, while improving Poetry scan robustness via a fallback path.

Changes:

Added test/smoke/ subprocess-based CLI smoke tests and pytest markers/fixtures.
Moved/added scan fixtures under test/test_packages/ (Python, Poetry, npm/yarn, Docker-based fixtures).
Refactored scan flow to modularize per-ecosystem scanning and added Poetry→virtualenv fallback behavior.

Reviewed changes

Copilot reviewed 15 out of 58 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
test/smoke/conftest.py	Adds smoke-test fixtures and registers pytest markers.
test/smoke/test_smoke.py	Adds CLI-driven end-to-end smoke tests (includes network/slow cases).
.githooks/pre-push	Switches pre-push runtime checks to the new smoke tests and updates a malicious-scan path.
.github/workflows/test.yml	Adds smoke-test execution to CI workflow.
ossprey/scan.py	Refactors scanning into python/javascript/fs helpers and adds Poetry failure fallback to virtualenv install scan.
ossprey/sbom_python.py	Improves subprocess invocation formatting; adds `update_sbom_from_virtualenv`; triggers Poetry lock generation via Poetry.
ossprey/test_main.py	Updates fixture paths; adds test for broken Poetry fallback behavior.
ossprey/test_scan.py	Updates fixture paths and adjusts assertions around missing package paths.
ossprey/test_mode_fs.py	Updates docker fixture paths.
ossprey/test_sbom_python.py	Formatting/path updates for SBOM python tests.
ossprey/test_virtualenv.py	Updates fixture paths and normalizes string quoting.
test/test_packages/python_simple_math/setup.py	Adds a minimal installable Python fixture package definition.
test/test_packages/python_simple_math/requirements.txt	Adds Python requirements fixture file.
test/test_packages/python_simple_math/simple_math/operations.py	Adds simple arithmetic module used by Python fixture package.
test/test_packages/python_simple_math/simple_math/init.py	Package marker for python fixture module.
test/test_packages/poetry_simple_math/pyproject.toml	Adds Poetry/PEP621-based Python fixture.
test/test_packages/poetry_simple_math/poetry_simple_math/operations.py	Adds module for Poetry fixture package.
test/test_packages/poetry_simple_math/poetry_simple_math/init.py	Package marker for Poetry fixture module.
test/test_packages/poetry_simple_math/.gitignore	Ignores Poetry lockfile for the Poetry fixture.
test/test_packages/poetry_simple_math/README.md	Placeholder/readme for Poetry fixture package.
test/test_packages/poetry_broken_simple_math/pyproject.toml	Adds intentionally “broken” Poetry config to exercise fallback behavior.
test/test_packages/poetry_broken_simple_math/setup.py	Adds setup.py so the broken Poetry fixture is still pip-installable.
test/test_packages/poetry_broken_simple_math/poetry_simple_math/operations.py	Adds module for broken Poetry fixture package.
test/test_packages/poetry_broken_simple_math/poetry_simple_math/init.py	Package marker for broken Poetry fixture module.
test/test_packages/poetry_broken_simple_math/.gitignore	Ignores Poetry lockfile for broken Poetry fixture.
test/test_packages/poetry_broken_simple_math/README.md	Placeholder/readme for broken Poetry fixture package.
test/test_packages/npm_simple_math/package.json	Adds npm fixture package manifest.
test/test_packages/npm_simple_math/src/index.js	Adds simple arithmetic module for npm fixture.
test/test_packages/npm_simple_math/test/simpleMath.test.js	Adds unit tests for npm fixture package.
test/test_packages/npm_simple_math/.gitignore	Ignores `node_modules/` for npm fixture.
test/test_packages/npm_simple_math/README.md	Adds documentation for npm fixture.
test/test_packages/yarn_simple_math/package.json	Adds yarn fixture package manifest.
test/test_packages/yarn_simple_math/src/index.js	Adds simple arithmetic module for yarn fixture.
test/test_packages/yarn_simple_math/test/simpleMath.test.js	Adds unit tests for yarn fixture package.
test/test_packages/yarn_simple_math/.gitignore	Ignores `node_modules/` for yarn fixture.
test/test_packages/yarn_simple_math/README.md	Adds documentation for yarn fixture.
test/test_packages/yarn_massive_math/package.json	Adds a large-dependency yarn fixture for scale testing.
test/test_packages/yarn_massive_math/src/index.js	Adds simple arithmetic module for large yarn fixture.
test/test_packages/yarn_massive_math/test/simpleMath.test.js	Adds unit tests for large yarn fixture package.
test/test_packages/yarn_massive_math/.gitignore	Ignores `node_modules/` for large yarn fixture.
test/test_packages/yarn_massive_math/README.md	Adds documentation for large yarn fixture.
test/test_packages/docker_py_simple_math/Dockerfile	Adds Docker-based Python fixture to test filesystem scanning/SBOM export.
test/test_packages/docker_py_simple_math/build.sh	Adds helper script to build/export SBOM from docker fixture.
test/test_packages/docker_py_simple_math/python/main.py	Adds sample FastAPI app used in docker python fixture.
test/test_packages/docker_py_simple_math/python/requirements.txt	Adds python requirements for docker python fixture (includes a Git dependency).
test/test_packages/docker_py_simple_math/python/README.md	Documents docker python fixture behavior and env vars.
test/test_packages/docker_js_simple_math/Dockerfile	Adds Docker-based JS fixture with scanner stage exporting SBOM.
test/test_packages/docker_js_simple_math/build.sh	Adds helper script to build/export SBOM from docker JS fixture.
test/test_packages/docker_js_simple_math/javascript/package.json	Adds JS manifest for docker JS fixture (includes a Git dependency).
test/test_packages/docker_js_simple_math/javascript/src/index.js	Adds simple arithmetic module for docker JS fixture.
test/test_packages/docker_js_simple_math/javascript/test/simpleMath.test.js	Adds unit tests for docker JS fixture package.
test/test_packages/docker_js_simple_math/javascript/yarn.lock	Adds yarn lockfile for docker JS fixture.
test/test_packages/docker_js_simple_math/javascript/.gitignore	Ignores `node_modules/` for docker JS fixture.
test/test_packages/docker_js_simple_math/javascript/README.md	Adds documentation for docker JS fixture.

Copilot · 2026-02-10T14:56:51Z

+EXISTING_PACKAGES = [
+    "python_simple_math",
+    "poetry_simple_math",
+    "poetry_broken_simple_math",
+    "npm_simple_math",
+    "yarn_simple_math",
+    "yarn_massive_math",
+]
+
+
+@pytest.mark.smoke
+@pytest.mark.parametrize("pkg_name", EXISTING_PACKAGES)
+def test_existing_test_packages(pkg_name, test_packages_dir, tmp_path):
+    """Scan each non-Docker test package and verify a clean SBOM is produced."""
+    pkg_dir = test_packages_dir / pkg_name
+    assert pkg_dir.is_dir(), f"Test package not found: {pkg_dir}"
+
+    sbom_file = tmp_path / "sbom.json"
+    result = run_ossprey(pkg_dir, output_file=sbom_file)
+
+    # Verify stdout reports clean
+    assert "No malware found" in result.stdout
+
+    # Verify SBOM file has components
+    assert_sbom_has_components(sbom_file)
+
+    # Optionally exercise real API
+    maybe_run_api_scan(pkg_dir)


test_existing_test_packages scans poetry_simple_math / poetry_broken_simple_math in auto mode, which will trigger poetry install when poetry.lock is absent (network + potentially slow). However this test is not marked network/slow, so it will still run when users/CI filter those markers out. Consider either checking in a poetry.lock for the poetry fixture(s), or marking the relevant parametrized cases as network/slow so marker-based selection behaves as expected.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-10T14:56:52Z

+# Run Smoke tests
+poetry run pytest test/smoke/test_smoke.py


The pre-push hook runs the full smoke suite (pytest test/smoke/test_smoke.py) which includes network and slow tests (npm installs, git clones). This can make pre-push unexpectedly slow/flaky and also fails for contributors without those tools. Consider defaulting pre-push to a non-network subset (e.g. -m "smoke and not network and not slow") and leaving the full suite as an explicit opt-in.

@copilot open a new pull request to apply changes based on this feedback

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-02-10T15:10:31Z

@dreadn0ught I've opened a new pull request, #47, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-02-10T15:11:58Z

@dreadn0ught I've opened a new pull request, #48, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * Update pre-push hook to run only fast, non-network smoke tests Co-authored-by: dreadn0ught <31293924+dreadn0ught@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dreadn0ught <31293924+dreadn0ught@users.noreply.github.com>

dreadn0ught added 5 commits February 10, 2026 10:27

Improved testing and fixed poetry failure handling

99f04d5

add smoke tests

4c39dc5

Add smoke tests to cicd

7eba469

Update pre-push tests

41b152b

fix tests

56fa816

recurseml Bot reviewed Feb 10, 2026

View reviewed changes

dreadn0ught requested a review from Copilot February 10, 2026 14:48

Copilot started reviewing on behalf of dreadn0ught February 10, 2026 14:48 View session

Copilot AI mentioned this pull request Feb 10, 2026

[WIP] Update testing based on review feedback for poetry #46

Closed

7 tasks

dreadn0ught added 2 commits February 10, 2026 14:51

update runtime test paths

8049efa

fix venv issue

755353f

Copilot AI reviewed Feb 10, 2026

View reviewed changes

Update .github/workflows/test.yml

3b08b90

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Feb 10, 2026

Reduce pre-push hook to fast, non-network smoke tests #47

Merged

dreadn0ught and others added 2 commits February 10, 2026 15:10

Update ossprey/sbom_python.py

1963c43

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update test/smoke/test_smoke.py

78239e9

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Feb 10, 2026

Check in poetry.lock for poetry_simple_math and mark poetry_broken_simple_math with network/slow #48

Closed

Copilot AI and others added 2 commits February 10, 2026 15:55

bug fix

ef3e2ed

dreadn0ught merged commit d26939c into main Feb 10, 2026
1 check passed

dreadn0ught deleted the update_testing_improve_poetry branch February 10, 2026 16:48

		return ossbom


		def update_sbom_from_virtualenv(ossbom: OSSBOM, package_name: str) -> OSSBOM:

Uh oh!

Conversation

dreadn0ught commented Feb 10, 2026 • edited by recurseml Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High-level PR Summary

Uh oh!

recurseml Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by RecurseML

Uh oh!

recurseml Bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

dreadn0ught Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

recurseml Bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

dreadn0ught Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

dreadn0ught Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Feb 10, 2026

Uh oh!

Copilot AI commented Feb 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dreadn0ught commented Feb 10, 2026 •

edited by recurseml Bot

Loading

recurseml Bot left a comment •

edited

Loading