fix: add usedforsecurity=False to hashlib md5/sha1 in skills_hub, skills_sync, web_server

[AlexFucuson9]

Summary

hashlib.md5() and hashlib.sha1() without usedforsecurity=False crash on FIPS-enabled systems (RHEL 8/9, Ubuntu FIPS) with ValueError: EVP_DigestInit_ex disabled for FIPS.

All affected calls are non-security uses (cache keys, deduplication, file hashing). SHA-256 is FIPS-approved and does not need this parameter.

Prior PRs (#56715, #56716, #56719) fixed the same pattern in agent/, gateway/, and tools/ — this PR covers the remaining instances in tools/skills_* and hermes_cli/web_server.py.

Changes

File	Calls fixed	Hash	Purpose
tools/skills_sync.py	1	md5	Directory content hash for change detection
tools/skills_hub.py	5	md5	Cache key generation for search/index/detail
hermes_cli/web_server.py	2	sha1	Action name slug deduplication

Total: 3 files, 8 calls.

Test Plan

• ruff check passes
• No behavioral change — identical hash output, only the FIPS flag differs

[teknium1]

Thanks for the focused FIPS compatibility fix. I verified that current GitHub main still has all eight targeted unflagged calls: tools/skills_hub.py:1270,1535,1762,2256,2362, tools/skills_sync.py:234, and hermes_cli/web_server.py:11140,12079. The changed helpers use the hashes for cache/change-detection/action-name derivation, and the patch applies cleanly against the checkout.

Suggested changes

• Consider a focused regression test for the usedforsecurity=False argument. Existing tests exercise surrounding behavior, but not FIPS-mode constructor compatibility.
• The body says related fixes already landed, but fix: hashlib.md5 FIPS crash in context_compressor.py #56715, fix: hashlib.sha1 FIPS crash in codex_responses_adapter.py #56716, and fix: hashlib FIPS crash in gateway and tools modules #56719 remain open and their target sites still lack the flag on current GitHub main; describe this as a complementary slice instead.

Automated hermes-sweeper review.

[egilewski]

looks mergeable

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

The patch only opts cache-key, dashboard action-name, and skills-sync provenance hashes out of security enforcement. It preserves digest values, leaves validators and sinks unchanged, and has no source-backed security finding. Focused skill-hub, ClawHub, and skill-sync tests pass.

Security evidence:

• trust boundary: Dashboard skill and MCP identifiers plus remote catalog responses are untrusted inputs. They reach cache-key construction and detached-action naming; no changed call feeds authentication, authorization, signature verification, URL validation, or content-integrity enforcement. The skills-sync logic hashes bundled and user trees for update/provenance comparison.
• source/sink/invariant: The five changed MD5 calls in the skill-hub module produce only cache-key strings consumed by index-cache readers and writers. The two changed SHA-1 calls in the dashboard web server produce only action and log-map keys. The skills-sync fallback hash is used only for manifest/provenance comparison. usedforsecurity=False preserves digest bytes and changes only provider policy; surrounding response, URL, and validation guards remain unchanged.
• current-main reproduction: Current main contains the same eight MD5/SHA-1 call sites with default constructors, and each is a non-security cache, action-name, or provenance use. Digest values remain equal between default and flagged constructors, preserving current-main keys and provenance values while changing only provider policy.
• PR-head or patch-replay validation: The reviewed patch is a coherent replay on current main; the diff contains only eight hashlib constructor changes across the dashboard web server, skill-hub, and skills-sync modules. Changed sources compile, and the focused skill-hub, ClawHub, and skill-sync suites pass.
• positive/negative cases: Positive cases covered action-name generation, cache/provenance hashing, cache hit/miss and response-shape handling, ClawHub listing fallback, and skill-sync update and user-modification handling. Negative cases covered digest equality between default and flagged constructors, changed directory contents changing the sync digest, malformed/non-list responses, and private URL rejection in the existing focused tests.
• residual bypass search: A repository-wide search of Python hashlib MD5/SHA-1 uses found no remaining unflagged MD5/SHA-1 calls in the three changed modules; other uses remain outside this patch and are unchanged. The content guard continues to provide the primary SHA-256 content hash; MD5 is only the documented resilient fallback in skills-sync.
• reviewer validation: CodeRabbit review found no findings.

Not checked:

• Combined focused test run
• Dashboard profile action test

Signed: GPT-5.6-luna-max in Codex

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix: add usedforsecurity=False to hashlib md5/sha1 in skills_hub, skills_sync, web_server

1. Scoping is correct: all eight changed sites are non-security uses (index/search cache keys, detached action names, change-detection/provenance digests), and usedforsecurity=False preserves digest bytes, so no cache keys or action names change. usedforsecurity is accepted since CPython 3.9 and requires-python is >=3.11, so no version concern.
2. If FIPS compatibility is the goal, note that other unflagged hashlib.md5()/sha1() call sites remain in the tree and would equally raise under FIPS — e.g. agent/context_compressor.py:3210, agent/codex_responses_adapter.py:333, gateway/platforms/weixin.py:1453/2157, gateway/platforms/yuanbao_media.py:110/331, gateway/platforms/qqbot/chunked_upload.py:369/561-563, tui_gateway/server.py:12121. A follow-up sweep would be valuable — but audit each site first: some (notably qqbot chunked_upload's file-integrity md5) may be genuine integrity/security uses where usedforsecurity=False is NOT appropriate.
3. No regression test covers the new constructor argument (the sweeper review raised the same point). A small test asserting the affected helpers (e.g. _mcp_install_action_name, _hub_action_name, _dir_hash) produce stable digests and don't crash would guard against accidental revert — though it cannot prove FIPS-mode behavior on a non-FIPS runner, it locks the argument in place.