Skip to content

fix: lock releases for after the CCA keys leak problem - #967

Merged
jdlcdl merged 2 commits into
selfcustody:mainfrom
joaozinhom:pin_version
Nov 3, 2026
Merged

jdlcdl merged 2 commits into
selfcustody:mainfrom
joaozinhom:pin_version

Conversation

@joaozinhom

Copy link
Copy Markdown
Member

This should avoid developers and contributors to have old possible compromise versions of our package manager related dependencies

What is this PR for?

Changes made to:

  • Code
  • Tests
  • Docs
  • CHANGELOG

Did you build the code and tested on device?

  • Yes, build and tested on

What is the purpose of this pull request?

  • Bug fix
  • New feature
  • Docs update
  • Other

@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.59%. Comparing base (9b3a431) to head (e1961d0).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #967   +/-   ##
=======================================
  Coverage   97.59%   97.59%           
=======================================
  Files          84       84           
  Lines       10977    10977           
=======================================
  Hits        10713    10713           
  Misses        264      264           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This should avoid developers and contributors to have old possible
comprimissed vversions of our package manager related dependencies
@jdlcdl

jdlcdl commented Oct 21, 2026

Copy link
Copy Markdown
Member

I've done "uv python install 3.12.13" within my own environment. I don't know if that needed to be done (or mentioned in readme).

uv sync --frozen indeed updated certifi for me.

I had previously updated to latest astral-uv by uninstalling via snap manager and re-installing latest in user-space.

Thank you for this @joaozinhom.

@joaozinhom
joaozinhom marked this pull request as ready for review October 21, 2026 17:03
@qlrd

qlrd commented Oct 21, 2026

Copy link
Copy Markdown
Member

cACK, i will do a proper review offline

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

Concept ACK, but breaks docker build + lock is stale, see inline.

not blocking:

  • actions pinned by tag, codecov/gh-pages get secrets. pin to SHAs
  • psf/black@stable is a moving tag, deps unpinned
  • Dockerfile: astor unpinned, pyserial==3.4 no hash
  • develop has no uv pin, main not merged back

Comment thread Dockerfile Resolved
Comment thread pyproject.toml Resolved

@qlrd qlrd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just nits, and i have more doubts on pin first to main then back to develop:

Comment thread README.md Resolved
Comment thread pyproject.toml Resolved
Comment thread .github/workflows/docs.yml Resolved
@joaozinhom
joaozinhom requested review from notTanveer and qlrd October 24, 2026 16:34

@notTanveer notTanveer 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 questions

  • uv lock --check fails on this branch. requires-python in the lock header is still >=3.11.5 and there's no [manifest] constraints entry for certifi. did you run uv lock? ci uses --frozen so it won't catch it
  • is requires-python >=3.11.17 meant to be a hard install gate? anyone on 3.11.5-3.11.16 (distro python) gets an error now. or should only ci pin the patch version?
  • why constraint-dependencies for certifi instead of just bumping the lock? it makes uv.lock depend on a pyproject.toml entry we'd have to remember to drop later
  • the dockerfile fetches wheels from files.pythonhosted.org/packages/py3/p/.... it works (hashes match), but it's a legacy redirect and not a documented api. would pip install --require-hashes -r with a requirements file work instead?

@joaozinhom

Copy link
Copy Markdown
Member Author

a few questions

* uv lock --check fails on this branch. requires-python in the lock header is still >=3.11.5 and there's no [manifest] constraints entry for certifi. did you run uv lock? ci uses --frozen so it won't catch it

* is requires-python >=3.11.17 meant to be a hard install gate? anyone on 3.11.5-3.11.16 (distro python) gets an error now. or should only ci pin the patch version?

* why constraint-dependencies for certifi instead of just bumping the lock? it makes uv.lock depend on a pyproject.toml entry we'd have to remember to drop later

* the dockerfile fetches wheels from files.pythonhosted.org/packages/py3/p/.... it works (hashes match), but it's a legacy redirect and not a documented api. would pip install --require-hashes -r with a requirements file work instead?

Thanks, I pushed updates addressing these points:

  • I regenerated uv.lock with uv 0.13.4. The lock now has the correct [manifest] constraint for certifi, and uv lock --check passes.
  • I restored requires-python to >=3.11.5, <3.13. The secure patch releases remain pinned in CI, but they are no longer a hard installation gate for distro Python packages.
  • I kept certifi>=2026.10.19 as a resolver constraint rather than only bumping the lock. This prevents a future re-lock from selecting a vulnerable older release, while still allowing newer certifi versions, so the constraint should not need to be removed later.
  • I replaced the redirect URLs with the exact artifact URLs from the PyPI release metadata. I kept the external download, SHA-256 verification, and --no-index installation because using pip with a requirements file to fetch its own replacement would make the existing pip part of the bootstrap download path.

I also ran the lock check, lint, formatting check, and test suite. Everything passes with 1193 tests passed and the existing expected xfail.

@joaozinhom
joaozinhom requested a review from notTanveer October 26, 2026 12:51
@joaozinhom

Copy link
Copy Markdown
Member Author

@jdlcdl @odudex thoughts or suggestions down here?

@odudex

odudex commented Oct 28, 2026

Copy link
Copy Markdown
Member

@odudex thoughts or suggestions down here?

I don't have a strong opinion. Locking should reduce some risks, but adds complexity and complicates maintenance.
Hard to tell the right dose of ossification and conciseness that better address security and the dynamism demanded by this moment.

@joaozinhom

Copy link
Copy Markdown
Member Author

@odudex thoughts or suggestions down here?

I don't have a strong opinion. Locking should reduce some risks, but adds complexity and complicates maintenance. Hard to tell the right dose of ossification and conciseness that better address security and the dynamism demanded by this moment.

We already implement version pinning for cases such as Python versions prior to 3.10 (due to breaking changes, such as f-strings and new methods like startswith and endswith). I believe we are simply moving this pinning forward to prevent a serious issue—like the data leak that occurred in the CCA—thereby implementing a change that, while breaking compatibility, is necessary for security reasons. I consider this a relatively common practice in projects; we aren’t introducing a new constraint, but rather bringing forward one that already exists.
As always please correct me if i am wrong, or if you disagree, is just how i see this situation.

@qlrd qlrd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No block on those, but IMO is good to update

Comment thread .github/workflows/tests.yml Resolved
Comment thread .github/workflows/tests.yml Resolved
Comment thread .github/workflows/tests.yml Resolved
Comment thread .github/workflows/tests.yml Resolved
Comment thread pyproject.toml Resolved
@jdlcdl

jdlcdl commented Nov 2, 2026

Copy link
Copy Markdown
Member

ACK on these changes to be merged into "main", and later into "develop".

@jdlcdl
jdlcdl merged commit 28d14c2 into selfcustody:main Nov 3, 2026
12 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.

5 participants