Skip to content

chore: add dependency security constraints and audits - #1093

Merged
nabinchha merged 8 commits into
mainfrom
codex/dependency-security-constraints
Oct 29, 2026
Merged

nabinchha merged 8 commits into
mainfrom
codex/dependency-security-constraints

Conversation

@nabinchha

@nabinchha nabinchha commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

📋 Summary

Publish optional pip/uv constraints for transitive dependency security floors, generated from the workspace uv configuration. Audit dependencies daily and before dependency-related changes land, without adding failed security checks to unrelated PRs.

🔗 Related Issue

N/A — maintainer-requested security improvements. Overlaps #1091: this PR retains the same published Pillow minimum and matching Colab setup while adding maintained constraints and recurring audits.

🔄 Changes

  • Require PyJWT >=2.15.0 and virtualenv >=21.14.13 in the maintained transitive floors; mirror PyJWT in e2e and regenerate the export so affected older versions cannot satisfy the constraints.
  • Keep direct dependency minimums in package requirements or dependency groups. Reserve constraint-dependencies and generated constraints.txt for transitive floors, removing duplicated direct floors.
  • Generate and validate the export with make update-security-constraints and make check-security-constraints. Declare Python and packaging requirements through inline script metadata; remove extras for pip compatibility while preserving markers and direct URLs.
  • Audit on dependency-related PRs/pushes, daily, and on demand. Generate the ignored e2e lockfile before auditing; audit committed workspace pins unchanged. This path-filtered workflow is not intended as a universally required status check.
  • Apply patched dependency versions and replace the nbconvert source workaround with patched nbconvert/Bleach releases. Keep the image tutorial's generated Colab install compatible with Pillow 13.
  • Preserve pip install data-designer as the main installation command, with constraints in an optional note. Document transitive dependency maintenance, older-release compatibility, and the unresolved Pillow advisory. No Dev Notes changes.

🔍 Attention Areas

  • fix: address Pillow and filelock vulnerabilities #1091 shares the Pillow requirement, Colab compatibility updates, and filelock remediation. Preserve these fixes when reconciling the PRs; this PR also requires newer virtualenv fixes.
  • The upstream Pillow advisory now lists 13.0.0 as patched. Removed the dated unresolved-advisory text from both guides; retain a general note about advisory-database coverage.
  • Existing uv 0.13.4 pins are retained in this workflow. Unifying uv version selection across repository workflows is deferred.

🧪 Testing

  • Exporter and docs suite: 87 passed, 2 skipped. Includes extras, markers, direct URLs, stale/missing exports, CLI behavior, and dependency-path trigger regression coverage.
  • make check-vulnerabilities: no OSV findings in 189 workspace and 92 e2e packages; both lockfile consistency checks passed.
  • Fresh-checkout regression previously verified that a missing e2e lockfile is generated before auditing.
  • Colab install cell matches generator output; notebook schema validated and existing cell IDs preserved.
  • Targeted Ruff and changed-file pre-commit checks passed. Fern: zero errors, one unauthenticated redirect-check warning; internal links checked across 90 pages.
  • Earlier validation: package suites 4,817 passed, 1 skipped; latest-main integration subset 39 passed, 8 skipped; dependency licenses, SPDX headers, and nbconvert HTML/Markdown smoke tests passed. The latest changes do not change resolved package versions.
  • E2E dataset generation: not run.

✅ Checklist

  • Follows commit message conventions
  • Commits signed off (DCO)
  • External-contributor issue requirement: N/A — repository maintainer.
  • Architecture docs: N/A — dependency tooling and documentation changes.

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
@nabinchha
nabinchha requested a review from a team as a code owner October 26, 2026 16:44
@github-actions

Copy link
Copy Markdown
Contributor

Fern preview: https://nvidia-preview-pr-1093.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for putting this together, @nabinchha. Getting security floors to downstream pip users and adding recurring audits is a useful gap to close.

Summary

This PR moves the reviewed security floors into [tool.uv].constraint-dependencies and exports them to a generated constraints.txt with a staleness check, Make targets and tests. It adds a Dependency Security workflow that runs uv audit --locked on both lockfiles. It also swaps the nbconvert git-source pin for the patched PyPI releases of nbconvert 7.17.2 and Bleach 6.4.0, and documents the optional constraints URL and the open Pillow advisory. The implementation matches the PR description. I regenerated the export locally and it matches the committed constraints.txt.

Findings

Suggestions — Take it or leave it

tests_e2e/pyproject.toml:34 — The e2e floor mirror has no drift check

  • What: DEVELOPMENT.md asks maintainers to "mirror relevant runtime floors" by hand into tests_e2e/pyproject.toml. Nothing checks that the two lists stay in sync. The same goes for the per-group minimums in the root pyproject.toml: for example, jupyter-server>=2.20.0 in the notebooks group sits next to the new jupyter-server>=2.21.0 constraint.
  • Why: The e2e uv audit will still catch a vulnerable locked version. But a floor that someone forgets to mirror can let a later uv lock --project tests_e2e re-resolve below it without any signal until the next advisory.
  • Suggestion: Could generate_security_constraints.py --check also confirm that each e2e floor is at least as high as the matching workspace floor for packages listed in both files? That would be a cheap guard. Alternatively, a short comment in tests_e2e/pyproject.toml pointing back to the root list would help.

.github/workflows/dependency-security.yml:7 — Unrelated PRs fail when an advisory is published

  • What: The audit runs on every pull_request with no path filter. A newly disclosed CVE in any locked package will turn every open PR red until someone lands a floor bump. Separately, a failure of the daily schedule run only notifies the person who last edited the cron.
  • Why: This is probably the behavior we want, since the failure forces a timely bump. It is still worth stating explicitly, so contributors who see an unrelated audit failure know what to do. If this check becomes a required status check, that state will block merges.
  • Suggestion: Consider adding a sentence to DEVELOPMENT.md on what to do when the audit fails on an unrelated PR. As a follow-up, the scheduled run could open or update an issue so main-branch failures are visible. I'm not suggesting edits to the workflow file in this PR.

fern/versions/latest/pages/concepts/security.mdx:17 — The main constraints URL vs. older releases

  • What: Every doc points to .../main/constraints.txt. A future floor might fall outside an older release's upper bounds; for example, pillow>=14 would conflict with data-designer-config's pillow>=12.3.0,<14. A user pinned to that older release who applies the main constraints would then hit a resolution failure.
  • Why: The docs already tell readers to keep a reviewed copy for reproducible deployments, so this is low impact. Still, "upgrade data-designer" is the implicit remedy, and the docs don't say so.
  • Suggestion: Consider a one-liner that says "the main constraints target the latest release; for pinned older versions, use the file from the matching release tag (.../v<version>/constraints.txt)". Release tags already contain the file, so this costs nothing extra.

scripts/generate_security_constraints.py:37 — description=__doc__ is None

  • What: The script has no module docstring, so argparse.ArgumentParser(description=__doc__) passes None and --help shows no description.
  • Suggestion: Add a one-line module docstring, or pass a literal description string.

Makefile:480 — New targets sit under the LICENSE CHECKS banner

  • What: security-constraints, check-security-constraints and check-vulnerabilities were added inside the license-checks section.
  • Suggestion: A small # SECURITY CHECKS banner would keep the Makefile easy to navigate. This is a minor nit.

What Looks Good

  • One source of truth with a drift check. constraints.txt is generated from pyproject.toml, and CI rejects a stale export. The tests cover markers, missing and stale files, no-write-on-check, and the CLI exit code. All 5 tests pass locally.
  • The nbconvert simplification. Dropping the git-source pin and the HATCH_BUILD_NO_HOOKS build override for the patched PyPI release removes a fragile workaround. install-notebook-css still works as a verify-and-restore step.
  • Honest handling of the open Pillow advisory. The docs say plainly that a clean OSV scan does not close it, and they give users a concrete mitigation (avoid Arrow export with PILLOW_ALIGNMENT > 1). The workflow also pins actions by SHA with contents: read only, and --locked stops the audit from quietly re-resolving.

Residual Risk

  • I could not independently verify the specific advisory IDs, affected Pillow ranges or patched versions cited in the docs and in the floor comments. I relied on the PR description for those.
  • uv.lock changes reviewed at version level only: the floor-driven bumps (e.g. filelock 3.25 → 4.2, virtualenv 21.2 → 21.14, json-repair 0.58 → 0.64, pydantic-settings 2.13 → 2.16, setuptools 83 → 84.2) plus new bleach/tinycss2/webencodings and removal of nh3. The filelock major bump overlaps fix: address Pillow and filelock vulnerabilities #1091, as the PR notes.
  • Linting: ruff check and ruff format --check pass on the changed Python files.

Structural Impact

(graphify 0.9.92, 8.8s)

Risk: LOW (localized change)

  • 2 Python files, 0 AST entities, 0/193 clusters

Verdict

Ship it (with nits). I found nothing blocking. The suggestions above are optional hardening and docs clarity.


This review was generated by an AI assistant.

@greptile-apps

greptile-apps Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issue was found.

Summary

Adds optional dependency constraints and recurring dependency audits.

  • Users can install Data Designer with maintained security version floors.
  • Dependency changes and daily runs trigger workspace and end-to-end security audits.
  • The engine and notebook environment require newer patched dependency releases.

The earlier fresh-checkout issue is fixed: both audit paths generate the ignored e2e lockfile before auditing it. No new actionable issue was established.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["Workspace security floors"] --> B["Generate constraints.txt"]
  B --> C["Check committed export"]
  D["Dependency changes, daily schedule, or manual run"] --> C
  D --> E["Audit committed workspace lock"]
  D --> F["Generate e2e lock"]
  F --> G["Audit generated e2e lock"]
Loading

Reviews (7) · Last reviewed commit: "Merge branch 'main' into codex/dependenc..." · Reviewed by Greptile

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
@nabinchha

Copy link
Copy Markdown
Contributor Author

Addressed the suggestions in this review in ae2f715:

  • Shared floors: used the suggested comment alternative in tests_e2e/pyproject.toml, pointing maintainers to the root list and requiring shared-floor updates in both places. Also aligned the notebook group's aiohttp and jupyter-server minimums with the security floors. No automated e2e-floor drift check was added.
  • Unrelated PR audit failures: documented the intended behavior and remediation in DEVELOPMENT.md: coordinate a floor update, bring the fix into the branch, and rerun the audit. Automatic issue creation is left for a follow-up.
  • Older releases: clarified that main constraints target the latest release and may conflict with older bounds. Documented matching release-tag files where available and reviewed application constraints for releases predating this file.
  • CLI help: added a module docstring so --help includes a description.
  • Makefile organization: renamed the section to SECURITY AND LICENSE CHECKS.

Also fixed the missing-e2e-lockfile finding from the inline review by resolving e2e dependencies before auditing. Validation: 11 focused tests passed, fresh-checkout audit regression passed, all three security CI checks passed, and Fern/internal-link validation passed. The documented unpatched Pillow advisory remains open despite the passing OSV scans.

Comment thread DEVELOPMENT.md Resolved
Comment thread Makefile Resolved
Comment thread pyproject.toml Resolved

@binaryaaron binaryaaron 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.

A few suggestions on the constraints export and the new workflow; none blocking.

Comment thread Makefile Resolved
Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
Retain virtualenv 21.14.13 to satisfy the security floor.

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
Comment thread pyproject.toml Outdated
"filelock>=4.2.1",
"idna>=3.15",
"pydantic-settings>=2.14.2",
"PyJWT>=2.14.0",

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.

These floors still allow PyJWT 2.14.0 and virtualenv 21.14.12, which have known vulnerabilities fixed in PyJWT 2.15.0 and virtualenv 21.14.13. A downstream app pinned to either affected version can apply our constraints and retain it. The lockfile audits miss this because they check newer versions. Let's raise both floors, mirror the PyJWT change in the e2e manifest, and regenerate constraints.txt.

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.

Fixed in 3543112. Raised the workspace floors to PyJWT>=2.15.0 and virtualenv>=21.14.13, mirrored PyJWT in the e2e manifest, and regenerated constraints.txt and lock metadata. The locked versions were already newer, so no resolved package versions changed.

Verified the exported specifiers reject PyJWT 2.14.0 and virtualenv 21.14.12 while accepting the patched floors. Both audits and 87 focused tests passed (2 skipped).


### Known Unresolved Advisory

As of October 26, 2026, [CVE-2026-114501 / GHSA-654x-cwwv-5j9c](https://github.com/python-pillow/Pillow/security/advisories/GHSA-654x-cwwv-5j9c) affects Pillow 11.2.1 through 13.0.0, and upstream lists no patched release. It can expose uninitialized memory when exporting images to Arrow with `PILLOW_ALIGNMENT` set above 1. Avoid that combination while the advisory remains unresolved.

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.

The Pillow advisory changed on October 27 and now lists 13.0.0 as patched. Could we refresh this section and the matching paragraph in DEVELOPMENT.md? Both still describe 13.0.0 as affected with no patched release available.

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.

Confirmed against the updated upstream advisory: it now lists affected versions as >=11.2.1,<13.0.0 and 13.0.0 as patched.

Fixed in 3543112 by removing the dated unresolved-advisory section and the matching DEVELOPMENT.md paragraph. Both guides now keep a general caveat about advisory-database coverage instead of a static CVE-status entry that can become stale. The existing Pillow >=13 package requirement remains in place, and I corrected the PR description too. Fern and internal-link checks passed.

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
@nabinchha
nabinchha merged commit 9b0702b into main Oct 29, 2026
139 of 157 checks passed
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.

4 participants