Skip to content

fix(deps): override limacharlie's pins to clear the cryptography and Pygments advisories - #95

Merged
ubercylon8 merged 3 commits into
mainfrom
deps/security-alerts
Aug 9, 2026
Merged

fix(deps): override limacharlie's pins to clear the cryptography and Pygments advisories#95
ubercylon8 merged 3 commits into
mainfrom
deps/security-alerts

Conversation

@ubercylon8

Copy link
Copy Markdown
Owner

What & why

Clears the repository's two open Dependabot alerts:

#4  cryptography 48.0.1  HIGH  PKCS#7 EnvelopedData decryption exposes a
                               Bleichenbacher oracle · patched 50.0.0
#3  Pygments     2.19.1  LOW   ReDoS via inefficient GUID regex · patched 2.20.0

cryptography reaches seven servers — defender, entra, intune, limacharlie,
purview, and both projectachilles servers — through pyjwt[crypto] and the
limacharlie SDK.

Why an ordinary bump does not work

uv lock --upgrade-package cryptography is a no-op, and uv explains why when
pushed:

limacharlie>=5.6.1 cannot be used.
And because f0-limacharlie-mcp depends on limacharlie>=5.6.1 [...]
your workspace's requirements are unsatisfiable.

The limacharlie SDK exact-pins ten of its dependencies:

requests==2.33.0   passlib==1.7.4    pyyaml==6.0.2     tabulate==0.9.0
termcolor==2.5.0   pygments==2.19.1  rich==13.9.4      cryptography==48.0.1
click==8.1.8       jmespath==1.1.0

No released version relaxes this — 5.5.2, 5.5.3, 5.5.4, 5.5.5, 5.6.0 and 5.6.1
(latest) all carry cryptography==48.0.1. So the repository is downstream of a
vendor's pinning policy for ten packages, two of which currently carry
advisories.

Why overriding is safe here

Not optimism — a specific check. The installed SDK imports neither package:

$ grep -rn "cryptography\|pygments" .venv/…/site-packages/limacharlie
(no output)

It pins what it does not use. The packages that genuinely consume cryptography
are permissive: PyJWT >=3.4.0 under its crypto extra, and passlib
unpinned. Nothing but a vendor declaration was holding 48.0.1 in place.

Separately, no repository code imports cryptography at all, and nothing
references PKCS#7, EnvelopedData, or S/MIME — so the specific vulnerable API was
not being called from here either. That lowers the urgency; it does not remove
the exposure, since pyjwt[crypto] links the library in regardless.

The override, and its exit condition

[tool.uv] override-dependencies in the root pyproject.toml. Each entry names
the advisory it exists for, so it can be removed when the SDK relaxes its
pin rather than quietly outliving its reason — an override nobody remembers is
how a project ends up resolving something nobody chose.

Worth raising upstream with LimaCharlie: exact-pinning transitives in a library
propagates every one of their advisories to every consumer, and it is why alerts
#1 and #2 (requests) could only close when the SDK itself moved.

Checklist (mirrors the Critical Rules in CLAUDE.md)

  • Scope-first / Irreversible actions gated / Secrets never reach the
    model
    / Returns Finding(s) / Safety logic stays in core/ /
    Small-model-safe — n/a: dependency resolution only, no source touched.
  • Generated trees rebuilt, not hand-edited — n/a.
  • No engagement data, credentials, or eval results staged.
  • uv run ruff check . and uv run mypy . pass; suite unchanged (see below).

Testing

uv lock                 # cryptography 48.0.1 -> 50.0.0, pygments 2.19.1 -> 2.20.0
uv sync --all-packages
uv run pytest           # 815 passed, 1 skipped, 2 failed  (see note)
uv run ruff check .     # clean
uv run mypy .           # clean, 60 source files

Functional smoke of the upgraded stack, since a cryptography major is the risk here:

cryptography 50.0.0 · pygments 2.20.0
pyjwt RS256 sign/verify round-trip through the new cryptography: OK
limacharlie imports cleanly with its pin overridden

Note on the 2 failures — they are pre-existing on main, not caused by this
PR.
I verified by stashing this change, re-syncing, and running them against a
clean tree: same two, same 815 passed.

servers/intune-mcp/tests/test_tools.py::test_list_stale_devices_filters_by_cutoff
integrations/test_integrations_valid.py::test_every_server_wired_into_opencode_config

Neither is dependency-related — one asserts device filtering returns OLD but
not FRESH, the other asserts opencode.json carries no model/provider key.
They want fixing, but in their own change rather than folded in here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GWp9quiV9aBepvRyjA92Br

…ts GHSAs

Two open Dependabot alerts could not be fixed by an ordinary lockfile bump:

  #4  cryptography 48.0.1  HIGH  PKCS#7 EnvelopedData Bleichenbacher oracle
  #3  Pygments     2.19.1  LOW   ReDoS in GUID matching

Both were held open by limacharlie, which exact-pins ten of its dependencies —
cryptography==48.0.1, pygments==2.19.1, requests==, pyyaml==, rich==, click==,
and more. `uv lock --upgrade-package cryptography` is a no-op against an `==`
constraint, and no released SDK relaxes it: 5.5.2 through 5.6.1 (latest) all
carry the same pin.

Overriding is safe here for a specific reason, not an optimistic one: the
installed SDK imports neither package. grep over site-packages/limacharlie for
cryptography and pygments returns nothing, so the pins constrain a dependency
the vendor does not use. The packages that genuinely consume cryptography are
permissive — PyJWT >=3.4.0 under its crypto extra, passlib unpinned.

Verified: cryptography 50.0.0 and pygments 2.20.0 resolve, limacharlie still
imports, and a PyJWT RS256 sign/verify round-trip through the new cryptography
succeeds. Suite is unchanged at 815 passed — the 2 failures present are
pre-existing on main and unrelated (confirmed by running them on a stashed tree).

Each override names the advisory it exists for, so it can be removed once the
SDK relaxes the pin rather than outliving its reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GWp9quiV9aBepvRyjA92Br
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

Claude finished @ubercylon8's task in 2m 26s —— View job


Security review — PR #95

  • Gather context (PR diff, CLAUDE.md Critical Rules)
  • Review pyproject.toml override-dependencies block
  • Review uv.lock diff for consistency with the override
  • Verify the PR's factual claims (limacharlie doesn't import cryptography/pygments, no repo code imports cryptography)
  • Check for read-only/gating/secrets/findings-schema/small-model-safe relevance
  • Check CI status
  • Post final review

Scope: dependency-only change (pyproject.toml + uv.lock), no source files touched. Confirmed via uv.lock diff that exactly two package versions changed (cryptography 48.0.1→50.0.0, pygments 2.19.1→2.20.0) — no collateral drift in any other package. None of the Critical Rules around gating, secret handling, the findings schema, or small-model-safe tool design apply here (no tools, servers, or core/ code touched).

Claims verified:

  • git log confirms neither servers/intune-mcp/tests/test_tools.py nor integrations/test_integrations_valid.py (the two tests called out as pre-existing failures) were touched by this PR.
  • The current CI run (test-lint job) does show 1 failed, 816 passed, 1 skipped — the single failure is test_list_stale_devices_filters_by_cutoff in intune-mcp, matching the PR's disclosure. Not caused by this change (no intune code or logic touched), but ci is a required/hard-gate check per CLAUDE.md, so it's worth confirming that failure gets fixed (in its own PR, as already proposed) before/around merging this one.
  • No repo code (core/, any servers/*) imports jwt/cryptography directly — the only consumer path is mcp (the MCP SDK itself pins pyjwt[crypto], per uv.lock:1001) plus whatever OAuth libraries the Entra/Defender/Intune/Purview servers use transitively. This actually makes cryptography a bit more central than "nothing in the workspace calls it" — it backs the MCP transport's own auth path, not just an unused transitive of limacharlie — which if anything strengthens the case for keeping it patched, and the PR's RS256 sign/verify smoke test is a reasonable (if narrow) correctness check for that path.
  • pip-audit/Dependency audit, Semgrep, CodeQL, and Secret scan workflows are all green on this PR.

One finding (suggestion, not blocking):

pyproject.toml (new override-dependencies block, ~line 32) pins both overrides as open-ended lower bounds:

override-dependencies = [
    "cryptography>=50.0.0",
    "pygments>=2.20.0",
]

Because this override exists specifically to force a security-relevant package past a vendor's stale pin, an unbounded >= means any future uv lock --upgrade (or a routine re-lock) can silently float cryptography/pygments to whatever major is newest at that moment — with no override-text change to trigger review, and no signal that a new major (which can carry breaking API changes, e.g. cffi/OpenSSL ABI shifts in cryptography) was pulled in. The PR's own rationale for why the override should be removable — "an override nobody remembers is how a project ends up resolving something nobody chose" — applies symmetrically to letting it silently drift upward. Consider an exact pin (cryptography==50.0.0, pygments==2.20.0) or a capped range (>=50.0.0,<51), matching the specificity of the advisory-tagged comments already in place, so a future major bump goes through the same deliberate review as this one.

Fix this → — tightens the override bound and re-locks.

Summary: No security issues in the change itself — it correctly closes both Dependabot alerts, is scoped to exactly the two flagged packages, and includes a documented exit condition. The only suggestion is to bound the override specifiers so the fix doesn't quietly become a standing "always take whatever's newest" policy for a crypto-relevant dependency.

From the PR's own review: unbounded `>=` on a security-motivated override lets a
routine `uv lock --upgrade` float cryptography across a major boundary with no
override-text change to review. That is the same "resolved something nobody
chose" failure the block exists to prevent, pointed the other way.

Now floor-and-cap, matching the repo's dependency policy: >=50.0.0,<51 and
>=2.20.0,<3. Patch and minor releases still arrive without an edit, so a 50.0.x
security fix needs no manual bump; only a major does. Resolution is unchanged —
cryptography 50.0.0, pygments 2.20.0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GWp9quiV9aBepvRyjA92Br
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

Claude finished @ubercylon8's task in 59s —— View job


Security review — PR #95 (updated after latest push)

  • Gather context (PR diff since last review, CLAUDE.md Critical Rules)
  • Review pyproject.toml override-dependencies block
  • Review uv.lock diff for consistency
  • Check whether prior finding (unbounded >= override) was addressed
  • Check CI status
  • Post final review

What changed since my last pass: commit a2ef633 addresses the one suggestion from my earlier review. The overrides are now capped ranges instead of open-ended lower bounds:

override-dependencies = [
    "cryptography>=50.0.0,<51",
    "pygments>=2.20.0,<3",
]

This closes the gap I flagged — a future uv lock --upgrade can no longer silently float cryptography (or pygments) across a major version boundary with no override-text change to trigger review, while patch/minor security fixes within 50.x/2.x still resolve automatically. The added comment block correctly explains both the "why override" and "why capped" rationale, matching the specificity the advisory-tagged entries already had. No further action needed here.

Re-verified scope: git diff origin/main...HEAD -- uv.lock shows exactly two version = changes in the whole lockfile (cryptography 48.0.1→50.0.0, pygments 2.19.1→2.20.0), and the new overrides block in uv.lock's workspace metadata matches pyproject.toml exactly. No collateral dependency drift, no source files touched.

CLAUDE.md Critical Rules: still not applicable — no tools, servers, core/ code, secrets handling, or the findings schema are touched by this change; it's dependency resolution only.

CI: test-lint (CI) shows 1 failed, 816 passed, 1 skipped. The single failure, servers/intune-mcp/tests/test_tools.py::test_list_stale_devices_filters_by_cutoff, is pre-existing and unrelated — confirmed via git log that this PR never touches servers/intune-mcp/, and the assertion failure ('OLD' in [...] and 'FRESH' not in [...]) is a device-filtering logic issue, not a dependency-resolution one. pip-audit/Dependency audit, Semgrep, CodeQL, Secret scan, and Link check are all green. CI is a required hard gate per CLAUDE.md, so this pre-existing failure should still get fixed (in its own PR, as the PR description already proposes) before/around merge — it just isn't something this PR introduced or should absorb.

Summary: No security issues in the change itself or in this update. The override is scoped to exactly the two advisory-flagged packages, is now bounded below the next major on both floor and ceiling, and includes documented exit conditions. Nothing further to flag.

@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

Claude finished @ubercylon8's task in 59s —— View job


Security review — PR #95

  • Gather context (diff vs origin/main, CLAUDE.md Critical Rules)
  • Review pyproject.toml override-dependencies block
  • Review uv.lock diff for consistency
  • Check read-only/gating/secrets/findings-schema/small-model-safe applicability
  • Check correctness/test coverage of changed logic
  • Post final review

Scope: pyproject.toml (+30) and uv.lock (+55/-54) only. Confirmed via git diff origin/main...HEAD that exactly two packages change version — cryptography 48.0.1 → 50.0.0 and pygments 2.19.1 → 2.20.0 — with no other collateral dependency drift. No core/, server, skill, or tool code touched.

CLAUDE.md Critical Rules: none apply directly — no tools, gated-write logic, secret handling, or findings-schema code is touched. cryptography does back the OAuth/JWT path used transitively by several servers (via pyjwt[crypto]) even though no repo code imports it directly, so keeping it patched is still worthwhile from a supply-chain standpoint, but this PR doesn't change how secrets are handled — only which version of a transitive dependency is resolved.

The override itself (pyproject.toml, [tool.uv] override-dependencies):

override-dependencies = [
    "cryptography>=50.0.0,<51",
    "pygments>=2.20.0,<3",
]
  • Correctly scoped to only the two advisory-flagged packages.
  • Bounded below the next major on both floor and cap, so a future uv lock --upgrade can't silently cross a major version boundary (e.g. a cryptography ABI/API break) without a review-triggering edit to this block — this addresses the one gap flagged in my earlier pass on this PR (unbounded >=), fixed in a2ef633.
  • Each entry documents which GHSA/alert it exists for and the removal condition (once the limacharlie SDK relaxes its own pin), so the override has a clear exit condition rather than becoming permanent silent drift.
  • The PR's claim that the installed limacharlie SDK imports neither cryptography nor pygments directly is consistent with why this override is safe: it only forces versions past a vendor's unused exact-pins, it doesn't paper over an actual runtime incompatibility.

uv.lock consistency: the new overrides block in the lockfile's workspace metadata matches pyproject.toml exactly, and the per-package version =/sdist =/wheel-hash entries for both packages are internally consistent with the new resolved versions. No stray hash/version mismatches.

Correctness / test coverage: this is a pure dependency-resolution change; there's no new source logic to unit-test. The PR description documents a manual JWT sign/verify smoke test against the new cryptography major and confirms the two previously-failing tests were pre-existing on main. CI is now fully green on this PR (CI, Dependency audit, Semgrep, CodeQL, Secret scan, Link check all passing) — the intune test failure noted in earlier review rounds has since been resolved via the merge from main (PR #96) reflected in this branch.

Summary: No security issues. The change correctly closes both open Dependabot alerts, is scoped to exactly the two flagged packages, is bounded to avoid unreviewed future major bumps, and documents its own exit condition. Nothing further to flag.

@ubercylon8
ubercylon8 merged commit 7da689a into main Aug 9, 2026
8 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.

1 participant