Skip to content

chore(deps): fix moderate/low Dependabot alerts in sandbox - #287

Merged
cploujoux merged 3 commits into
mainfrom
cploujoux/devin/dependabot-moderate-low-sandbox
Aug 13, 2026
Merged

chore(deps): fix moderate/low Dependabot alerts in sandbox#287
cploujoux merged 3 commits into
mainfrom
cploujoux/devin/dependabot-moderate-low-sandbox

Conversation

@cploujoux

@cploujoux cploujoux commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Closes all 3 moderate Dependabot alerts: pytest, and requests in both requirements.txt files.

3 closed / 0 skipped / 0 blocked.

Moderate/low PR, lighter evidence bar: advisory cleared and verified in both manifests, existing tests green. A new discriminating security test was added on top, so the bump is not resting on the install alone.

Both packages install to the exact target version, and the advisory is cleared in both requirements.txt files rather than only the one Dependabot named.


Open in Devin Review

Note

Follow-up commit adds defensive guards around the _pytest.tmpdir.TempPathFactory internal API usage: wraps the import in try/except and the constructor call in a TypeError catch, converting breakage from future pytest internals changes into informative skips rather than hard CI failures.

Written by Mendral for commit 0751830.

cploujoux and others added 2 commits August 12, 2026 08:54
pytest 8.3.5 -> 9.0.3 (GHSA-6w46-j5rx-g56g, CVE-2025-71176): the base temp
directory /tmp/pytest-of-{user} was resolved without checking whether it
was a symlink, letting a local attacker redirect another user's pytest
temp artifacts via a TOCTOU symlink swap. Fixed in pytest-dev/pytest#14343.
Major bump; reviewed the 8.4.0 -> 9.0.3 changelog for breaking changes
(Python 3.9 support dropped -- this image is on 3.12; PytestRemovedIn9
deprecations now error by default; overlapping-path CLI arg handling
changed) -- none touch how this image invokes pytest (`pytest tests/ -v`,
one path argument, no deprecated APIs in tests/). Full existing test suite
re-run clean against the new version.

requests 2.32.4 -> 2.33.0 (GHSA-gc5v-m9x4-r6x2, CVE-2026-25645): predictable
temp-file reuse in requests.utils.extract_zipped_paths(); upstream advisory
states standard usage of the library is unaffected. Neither requirements.txt
file's requests import is reached from our own server code (server/*.py
uses httpx) or calls extract_zipped_paths -- shipped for user code inside
the sandbox. urllib3==2.7.0 (pinned separately) already satisfies 2.33.0's
`urllib3<3,>=1.26` requirement, so no companion bump was forced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reproduces pytest's own upstream regression test for the fix
(testing/test_tmpdir.py::test_tmp_path_factory_doesnt_follow_symlinks) via
the internal _pytest.tmpdir.TempPathFactory API and the
PYTEST_DEBUG_TEMPROOT escape hatch: replaces the pytest-of-{user} base
directory with a symlink to an attacker-controlled directory and asserts
getbasetemp() now raises OSError instead of silently following it.

Revert-checked against pytest==8.3.5: the same test fails with
"DID NOT RAISE <class 'OSError'>", confirming it actually discriminates
the fix rather than passing regardless of version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

Comment on lines +67 to +76
tmp_factory = TempPathFactory(None, 3, "all", lambda *args: None, _ispytest=True)
pytest_of_user = tmp_factory.getbasetemp().parent
assert "pytest-of-" in str(pytest_of_user)
shutil.rmtree(pytest_of_user)
pytest_of_user.symlink_to(attacker_controlled)

# A fresh factory must now refuse to use it.
tmp_factory = TempPathFactory(None, 3, "all", lambda *args: None, _ispytest=True)
with pytest.raises(OSError, match=r"temporary directory .* is a symbolic link"):
tmp_factory.getbasetemp()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 New security test depends on an internal pytest API and could break on future bumps

The test constructs _pytest.tmpdir.TempPathFactory(None, 3, "all", lambda *args: None, _ispytest=True) — an internal, positional-argument-sensitive API. Any future pytest release reordering/renaming these constructor parameters (or changing the OSError message matched by the pytest.raises(..., match=r"temporary directory .* is a symbolic link") regex) will make the CI gate in .github/workflows/dependency-verification.yml:55 fail on a perfectly healthy dependency bump. The docstring acknowledges this, but the failure surfaces as a red required-ish job on future dependency PRs, so it's worth deciding whether this test should be marked xfail-on-import or version-guarded.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid, fixed in 0751830. Guarded the import and the constructor call behind a helper that pytest.skip()s with an explanatory message on ImportError/TypeError instead of failing, so a future pytest release moving or reshaping _pytest.tmpdir.TempPathFactory degrades to a skip on an unrelated future dependency PR rather than a hard CI failure. Left the actual security assertion (the OSError-on-symlinked-basetemp check) as a hard failure -- that one failing for real would mean the property regressed, not that pytest's internals moved. Verified both guard paths trigger a skip by monkeypatching the module's TempPathFactory to None and to a signature-mismatched stub, and reran the full suite (still 11 passed).

plotly==6.9.0
kaleido==1.3.0
pytest==8.3.5
pytest==9.0.3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Major pytest version jump (8.x -> 9.x) affects user code inside the image, not just this test

pytest==9.0.3 is a major-version bump shipped into the jupyter-server image (hub/jupyter-server/requirements.txt:27), which users' notebook/test code runs against. pytest 9 removes several long-deprecated behaviors (e.g. nose-style setup/teardown remnants, older config options). The repo's own tests only exercise tests/ in CI, so no regression signal exists for user code; worth confirming no template docs or sample notebooks rely on pytest 8 behavior.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checked: this repo ships no sample notebooks, templates, or repo-owned pytest config that a user's test code would inherit (grep -rln pytest --include=*.ipynb --include=*.md hub/ -> empty; no pytest.ini/pyproject/setup.cfg under hub/jupyter-server; hub/jupyter-server/template.json is just marketplace metadata, unrelated). No nose-style setup/teardown or other pytest-9-removed API appears anywhere in the repo either. So there's nothing in this repo itself pytest 9 could regress beyond what's already covered in the report's changelog review (Python 3.9 drop n/a on this 3.12 image, PytestRemovedIn9Warning-as-error verified against the repo's own tests/, CLI path-arg handling unaffected). What I can't verify is arbitrary user code written after the bump ships, inside a running sandbox -- that's an inherent, accepted residual risk of shipping any major version of a general-purpose dev tool into a runtime users write their own code against, not something a repo-side check can close. Noted in the report as an accepted risk rather than left silent.

mendral-app[bot]

This comment was marked as outdated.

… API moves

Flagged by devin-ai-integration: _pytest.tmpdir.TempPathFactory is internal
and undocumented, so a future pytest release reordering/renaming its
constructor args (or removing it) would turn this test into a hard CI
failure on an unrelated, otherwise-healthy future dependency PR. Guard the
import and construction behind a helper that pytest.skip()s with an
explanatory message on ImportError/TypeError instead, leaving the actual
security assertion (OSError on a symlinked basetemp) as a hard failure --
that one failing would mean the property genuinely regressed.

Verified both guard paths trigger a skip (not a failure) by monkeypatching
the module's TempPathFactory to None and to a signature-mismatched stub;
reran the full suite (11 passed) to confirm the happy path is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mendral-app

mendral-app Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🧪 Testing Guide

What this PR addresses

Fixes 3 moderate/low Dependabot security alerts by bumping:

  • pytest 8.3.5 → 9.0.3 (fixes CVE-2025-71176 / GHSA-6w46-j5rx-g56g — symlink-following TOCTOU in tmp_path)
  • requests 2.32.4 → 2.33.0 (in both hub/jupyter-server/requirements.txt and hub/jupyter-server/server/requirements.txt)

A new test (test_pytest_tmpdir_security.py) is added to verify the pytest security fix is active.

Steps to reproduce the original issue

  1. Check Dependabot alerts on this repo — there should be 3 open moderate/low alerts for pytest and requests.
  2. For the pytest CVE specifically: on pytest 8.3.5, if /tmp/pytest-of-{user} is replaced with a symlink to an attacker-controlled directory, pytest silently follows it — allowing test artifacts to be redirected.

What to verify (expected behavior)

  1. Dependency versions resolve correctly:
    • pip install -r hub/jupyter-server/requirements.txt installs pytest>=9.0.3 and requests>=2.33.0
    • pip install -r hub/jupyter-server/server/requirements.txt installs requests>=2.33.0
  2. New security test passes:
    cd hub/jupyter-server && pytest tests/test_pytest_tmpdir_security.py -v
    • test_tmp_path_factory_rejects_symlinked_basetemp — verifies pytest raises OSError when base temp is a symlink
    • test_pytest_version_is_patched — asserts pytest version is ≥ 9.0
  3. No regressions: existing tests in the repo continue to pass (CI should confirm this).
  4. Dependabot alerts cleared: after merge, the 3 moderate/low alerts should auto-close.

Note

Posted by PR Testing Guide · Tag @mendral-app with feedback.

@mendral-app mendral-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

The new commit directly addresses the fragility I noted in my previous review. The skip-on-ImportError/TypeError pattern is well-structured: internal API breakage skips gracefully while actual security regression (OSError not raised) remains a hard failure. No new issues.

Tag @mendral-app with feedback or questions. View session

@cploujoux
cploujoux merged commit 337640d into main Aug 13, 2026
6 checks passed
@cploujoux
cploujoux deleted the cploujoux/devin/dependabot-moderate-low-sandbox branch August 13, 2026 16:09
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.

1 participant