chore(deps): update cryptography requirement from >=42.0.0 to >=50.0.1 - #72
chore(deps): update cryptography requirement from >=42.0.0 to >=50.0.1#72dependabot[bot] wants to merge 1 commit into
Conversation
Updates the requirements on [cryptography](https://github.com/pyca/cryptography) to permit the latest version. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](pyca/cryptography@42.0.0...50.0.1) --- updated-dependencies: - dependency-name: cryptography dependency-version: 50.0.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
srpatcha
left a comment
There was a problem hiding this comment.
Review — eDB#72 "chore(deps): update cryptography requirement from >=42.0.0 to >=50.0.1"
head: 2200af1 author: app/dependabot ci: pass (CodeQL only — the test matrix did not run)
Verdict: This is the one of the three eDB bumps that touches a runtime security
dependency, and I verified it works: src/edb/security/encryption.py round-trips
identically under 42.0.0 and 50.0.1. The problem is what sits behind it — that module has
no tests at all, so the round-trip I ran locally is currently the only evidence that
exists either way.
What was measured
Cloned origin/master (0457c0a) and exercised the sole consumer of this dependency
under both floors, Python 3.12.14:
cryptography 42.0.0 -> round-trip OK, both key paths
cryptography 50.0.1 -> round-trip OK, both key paths
covering EncryptionManager('pw') (PBKDF2-derived key) and EncryptionManager()
(generated key), encrypt → decrypt. cryptography==50.0.1 exists on PyPI with
requires_python = "!=3.9.0,!=3.9.1,>=3.9", so it installs on every entry of the CI
matrix and satisfies pyproject.toml:11's requires-python = ">=3.11".
Findings
| # | Severity | File:line | Finding | Recommended fix |
|---|---|---|---|---|
| 1 | High | src/edb/security/encryption.py (whole file) |
The only module that imports cryptography — AES-256-GCM at rest, PBKDF2HMAC key derivation — has no test coverage. grep -rln "encryption|AESGCM|encrypt" tests/ returns nothing; tests/unit/ contains only test_unit_core.py and test_unit_kv.py. So an eight-major-version bump of a cryptographic library lands with nothing to catch a behaviour change, and .ai/security.md is explicit that "the firmware building proves nothing about its security — every claim here needs a check that was actually run". |
Add tests/unit/test_unit_encryption.py before merging: encrypt/decrypt round-trip on both key paths, wrong-key rejection, truncated and zero-length ciphertext, tampered tag, and a fixed known-answer vector so a future library change cannot silently alter the wire format of data already at rest. |
| 2 | High | .github/workflows/ci.yml:33 |
The test job installs from requirements.txt, a file that does not exist in this repository. The most recent CI — eDB push run on master concluded failure with ERROR: Could not open requirements file: [Errno 2] No such file or directory: 'requirements.txt'. Line 34 then installs only pytest pytest-cov pytest-benchmark mypy ruff — never the project. Nothing in CI installs cryptography at all, so the floor this PR raises is read by no check. |
Replace ci.yml:33-34 with pip install -e ".[dev]". This is the change that makes the runtime dependency actually resolve in CI, and it fixes a job that currently cannot pass. |
| 3 | Medium | pyproject.toml:33 |
Raising a floor is not a pin, and this repo has no lockfile. >=50.0.1 still resolves to whatever is newest at install time, and it does nothing for anyone who already resolved 42.x. It also excludes consumers on distribution-packaged cryptography, which is a real cost for an embedded-adjacent project — 50.x was published recently. There is no advisory cited in the PR body, so I cannot tell whether this is a CVE response or routine version drift; the title says chore(deps), which suggests the latter. |
Merge it, but add a lockfile or constraints file for the tested configuration so "what we tested" and "what a user gets" are the same thing. If it is advisory-driven, say which advisory in the commit message — that is the difference between a floor a maintainer may lower and one they must not. |
| 4 | Medium | (repo-wide) | .github/dependabot.yml was removed from master by eaf4e1c (PR #68): "Dependabot is disabled org-wide: config removed here, and alerts plus automated security fixes turned off via the API. 90 open Dependabot PRs". This matters more here than on the other two eDB PRs: there is now no automated channel that will tell this repo about the next cryptography advisory. .github/STANDARDS.md:74-77 claims SPDX/CycloneDX SBOMs and NTIA minimum elements, and §14.1 of the master design requires vulnerability reporting and advisory processes — but those cover EmbeddedOS's own disclosures outward, not consumption of third-party advisories inward. This PR is an orphan of that decommissioned automation: nothing will rebase or supersede it. |
Merge or close it deliberately, and put a replacement in place — GitHub's dependency review action, pip-audit in CI, or osv-scanner. I have appended an architecture proposal on this gap to .ai/autoreview/proposals/2026-09.md. |
| 5 | Medium | .github/workflows/ci.yml:8 (at base 5e436bd6) |
The Test (Python …) matrix did not run on this PR; the only checks are Analyze (Python)/CodeQL and assign. Root cause, verified: at base 5e436bd6 the trigger was pull_request: branches: [main] while this PR targets master. Fixed on master by #70 (6d53641); this PR is 3 commits behind. The green tick is CodeQL, not tests. |
Rebase onto origin/master. |
| 6 | Medium | repo setting: branches/master/protection |
required_status_checks: null on master — reviews required, no check required. CI — eDB failing on master blocks nothing, which is how finding 2 has survived. |
Set required status checks once finding 2 makes the job passable. |
Architecture conformance
Conforms.
- §5.1 architectural law.
cryptographyis a third-party library beneath eDB, not a
higher-level EmbeddedOS product. Nothing points up a tier. - §14.1 security principles. "Use reviewed cryptographic libraries; do not invent
cryptographic primitives."src/edb/security/encryption.py:10-12imports AESGCM and
PBKDF2HMAC fromcryptographyrather than hand-rolling them, and this PR keeps that
library current. Directionally correct. - §21 tier placement. eDB is Tier 3 — Advanced. A runtime dependency of eDB belongs in
eDB'spyproject.toml. §21.1 untouched. - Gap, not a deviation.
.ai/security.mdrequires key management to be reviewed as
one system across eBoot, eSec, eOTA and release signing (§14.1). eDB's
EncryptionManagerderives and holds a data-at-rest key entirely on its own, outside
that system. That is a pre-existing structural question well beyond a dependency bump —
raising it here only so it is on the record, not as a finding against this PR.
Proposed changes
- Fix
ci.yml:33-34→pip install -e ".[dev]"(finding 2). Without it nothing in CI
installscryptographyat any version. - Add
tests/unit/test_unit_encryption.py(finding 1). This is the one item I would not
merge without — the bump is almost certainly fine, but "almost certainly" is what the
test is for. - Merge this PR.
- Add
pip-auditorosv-scannerto CI to replace the disabled Dependabot alerts
(finding 4). - Set required status checks on
master(finding 6).
Not checked
- The changelogs for cryptography 43 through 50. I measured behaviour on this
codebase's two code paths rather than reading eight major releases. A change this
module does not exercise would not appear in my result. - Whether an advisory motivates this bump. The PR body cites none and it is titled
chore(deps); I did not search the advisory databases forcryptographyin the
42.x–50.x range. If one exists, my Medium on finding 3 is too low. - The rest of
src/edb/security/. Onlyencryption.pywas read, because it is the
only file that importscryptography. JWT handling,bcryptusage and the audit path
were not reviewed. - Key management and storage. Where
EDB_ENCRYPTION_KEYcomes from in a deployment,
and what happens on rotation, were not examined. Note that importing the config with no
key set prints "No EDB_ENCRYPTION_KEY configured! A random secret was generated for this
session" and continues — a fail-open default worth its own review, outside this diff. - Platform coverage. All runs were Python 3.12.14 on Linux;
cryptographyships
compiled wheels per platform and I exercised only one.
Automated architecture review of 2200af170b91 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.
Updates the requirements on cryptography to permit the latest version.
Changelog
Sourced from cryptography's changelog.
... (truncated)
Commits
ffde75abump for 50.0.1 + changelog (#15520)dcb7050Prepare for 50.0.0 release (#15372)53fccd9Don't leak how PKCS#7 encryptedKey decryption failed (#15369)d472f97Addfrom __future__ import annotationsto all src/ Python files (#15371)908773dBump downstream dependencies in CI (#15368)2cc07ccBump BoringSSL, OpenSSL, AWS-LC in CI (#15367)c94ede9chore(deps): bump ruff from 0.16.0 to 0.16.1 (#15366)67a8308chore(deps): bump virtualenv from 21.7.0 to 21.7.1 (#15365)95018ffRelease the GIL in one-shot AEAD encrypt/decrypt (#15361)6954733Release the GIL during DH and DSA parameter generation (#15364)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)