fix: hashlib FIPS crash in skills_sync, weixin, web_server (usedforsecurity=False)

[AlexFucuson9]

Problem

3 files still call hashlib.md5() / hashlib.sha1() without usedforsecurity=False. On FIPS-enabled systems (RHEL 9, Ubuntu 22.04 FIPS, AWS AL2023 FIPS), these raise ValueError: [digital envelope routines] unsupported, crashing the affected code paths.

Files Fixed

File	Line	Usage
tools/skills_sync.py	234	_dir_hash() content fingerprinting
gateway/platforms/weixin.py	1419	Text message dedup hash
gateway/platforms/weixin.py	2118	File upload checksum
hermes_cli/web_server.py	11493	MCP install action name
hermes_cli/web_server.py	12433	Skill action name

All are non-security use cases (content hashing, dedup, naming).

Not Changed (intentionally)

plugins/platforms/sms/adapter.py:257 — HMAC-SHA1 for Twilio webhook signature verification. This is a cryptographic security use case and must NOT use usedforsecurity=False.

Related

Follows pattern from PRs #56715, #64062, #64808 which fixed the same issue in other files.

[teknium1]

Thanks for the focused FIPS compatibility fix. Current main still has every target expression from this PR: tools/skills_sync.py:234, gateway/platforms/weixin.py:1420 and :2119, and hermes_cli/web_server.py:11760 and :12700.

Problems

• The claimed remaining-file audit is incomplete: tools/skills_hub.py:1363, :1648, :1875, :2369, and :2475 create MD5 cache keys without usedforsecurity=False. These are cache-key paths and retain the same FIPS incompatibility.
• No regression test checks the FIPS-specific constructor argument. Existing _dir_hash behavior tests are in tests/tools/test_skills_sync.py:91-109, while a repository-wide search found no usedforsecurity coverage.

Suggested changes

• Include the clearly non-security tools/skills_hub.py cache-key call sites in this audit, while retaining the PR's semantic caution around signature/protocol hashes.
• Add a focused mocked-FIPS test for _dir_hash() passing usedforsecurity=False.

This is an automated hermes-sweeper review.

[egilewski]

suggesting changes

The patch correctly annotates the five changed MD5/SHA-1 constructors as non-security uses, preserving digest values while avoiding the reported FIPS-mode constructor failure in Weixin deduplication/upload checksums, skill synchronization, and dashboard action identifiers. Focused tests, a FIPS-style constructor probe, Ruff, and compilation checks passed. A repository-wide residual search still finds analogous non-security weak-hash constructors in skills-hub cache keys, QQ/WeCom/Yuanbao upload identifiers, context compression, webhook receipt IDs, response IDs, and the TUI revision key, so the claimed remaining-call cleanup is incomplete.

• [P2] FIPS-unsafe weak-hash constructors remain in non-security paths (tools/skills_hub.py:1381)
  The patch fixes five constructors, but analogous non-security uses still call hashlib.md5 or hashlib.sha1 without usedforsecurity=False: tools/skills_hub.py at lines 1381, 1666, 1893, 2391, and 2517; gateway/platforms/qqbot/chunked_upload.py at lines 369, 561, 562, and 563; gateway/platforms/yuanbao_media.py:110; gateway/platforms/msgraph_webhook.py:390; plugins/platforms/wecom/adapter.py:1247; agent/context_compressor.py:3211; agent/codex_responses_adapter.py:333; and tui_gateway/server.py:12784. On a FIPS-enabled OpenSSL runtime these feature paths can raise the same ValueError before producing their cache, checksum, receipt, or identifier output. Add the non-security annotation to those call sites, or explicitly justify any protocol-security use and provide an approved algorithm path, before treating this cleanup as complete.
  Remediation: Annotate every remaining non-security MD5/SHA-1 constructor with usedforsecurity=False and add a FIPS-mode regression test covering the cache, upload, compression, receipt-ID, and revision-key paths. Keep protocol-signature hashes separately reviewed rather than weakening their security semantics.

Security evidence:

• trust boundary: The relevant boundary is the Python/OpenSSL hashing runtime, including FIPS-enabled deployments. Inputs are inbound message text, outbound file bytes, skill-directory contents, and user or catalog names. The changed digests feed deduplication, protocol metadata, change detection, and in-memory action identifiers; source inspection shows they are not authentication or authorization decisions.
• source/sink/invariant: Each changed call keeps the original input bytes and encoding and adds only usedforsecurity=False. The resulting MD5/SHA-1 bytes and lengths are unchanged, so Weixin protocol fields and deduplication keys remain compatible while the constructors are explicitly marked as non-security uses. Dashboard action names and skill-directory fingerprints retain their existing formatting and comparison behavior.
• current-main reproduction: Current-main source contains the default security-tagged constructors at the five changed call sites. A FIPS-style constructor probe that rejects omitted usedforsecurity reproduces the old failure shape, while the changed call sites pass the keyword and preserve digest behavior. The focused directory-hash test and Weixin duplicate-content test passed on the changed sources, and Ruff plus compilation checks completed successfully.
• PR-head or patch-replay validation: The patch is a coherent five-line change on top of current main; source comparison shows no unrelated edits in the three touched files. The changed functions were exercised where deterministic local tests exist, and a focused probe covered the two dashboard action-name helpers and directory hashing under constructor rejection.
• positive/negative cases: Positive cases: directory fingerprints, action identifiers, and Weixin content deduplication complete when weak-hash constructors require an explicit non-security annotation. Negative cases: the FIPS-style probe rejects the old default constructor shape, and the existing directory-hash tests continue to distinguish changed contents while preserving the 32-character output contract.
• residual bypass search: A repository-wide search and source review found unsuffixed weak-hash constructors in skills-hub cache keys, QQ, WeCom, and Yuanbao media identifiers, Microsoft Graph receipt IDs, context-compression content fingerprints, Codex response IDs, and the TUI revision key. These are analogous non-security availability paths and remain outside this patch; protocol-signature uses were not grouped with them.
• reviewer validation: Reviewed every changed call and its consumer, checked the Python version floor, compared the diff with the reviewed patch, and adjudicated all guard-surface rows against their source observations. The focused tests, FIPS-style probe, Ruff check, and compile check all passed.

Not checked:

• Full test suite
• Native FIPS/OpenSSL runtime

Signed: GPT-5.6-luna-max in Codex

[Enough1122]

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

PR #65434: fix: hashlib FIPS crash in skills_sync, weixin, web_server (usedforsecurity=False)

• Audit is incomplete for the same bug class: the same FIPS-incompatible weak-hash constructors remain in clearly non-security paths — tools/skills_hub.py (MD5 cache keys, e.g. ~line 1381/1666/1893/2391/2517), agent/context_compressor.py:3211, tui_gateway/server.py:12784, gateway/platforms/qqbot/chunked_upload.py, gateway/platforms/yuanbao_media.py:110, gateway/platforms/msgraph_webhook.py:390, plugins/platforms/wecom/adapter.py:1247. These will hit the same ValueError on a FIPS-enabled OpenSSL runtime. Either extend the fix to those call sites or explicitly document why they're out of scope.
• No regression test pins the usedforsecurity keyword itself: the existing behavior tests for _dir_hash() / dedup pass whether or not the keyword is present, so they can't distinguish the fixed from the unfixed code. A mocked-FIPS constructor (one that raises when the flag is omitted) would lock the fix in.
• Minor consistency: gateway/platforms/weixin.py:1419 passes usedforsecurity=False positionally while the other sites use the keyword form — harmless on Python 3.9+, but the keyword form reads better and is uniform.