hash: silence 19 bandit B324 weak-hash warnings (cache keys + protocol fields)

[YonganZhang]

Summary

bandit -r . -x tests,skills/red-teaming,scripts,website,docs --severity-level high --confidence-level high currently reports 19 HIGH-severity B324 hits for hashlib.md5(...) and hashlib.sha1(...) uses. Every one of them is functionally safe — they fall into two distinct categories that the current code does not document:

Category 1 — Non-security content fingerprints / cache keys (11 sites)

File:line	Use
agent/codex_responses_adapter.py:197	Truncated SHA-1 of a seed string as an internal response ID
agent/context_compressor.py:723	Truncated MD5 of compressor input content (cache key)
gateway/platforms/msgraph_webhook.py:334	MS Graph webhook idempotency dedup key
gateway/platforms/weixin.py:1395	Sender-content cache key for WeChat replies
gateway/platforms/yuanbao_media.py:110	md5_hex content fingerprint helper
tools/skills_hub.py:928, 1180, 1316, 1793, 1899	5× cache keys for SkillsHub API responses
tools/skills_sync.py:165	_dir_hash directory-change detection

Fix: add usedforsecurity=False (available since Python 3.9). This both documents intent and lets hashlib skip FIPS-disallowed-algorithm checks in restricted environments. No behavioral change — same digest bytes either way.

Category 2 — Third-party protocol-required digests (8 sites)

File:line	Required by
gateway/platforms/qqbot/chunked_upload.py:369, 561, 562, 563	QQ Bot rich-media upload protocol (MD5/SHA-1 of chunk payload + first-10MB MD5)
gateway/platforms/wecom.py:1161	WeCom media upload protocol — body literally requires an md5 field
gateway/platforms/wecom_crypto.py:63	WeChat enterprise message-encrypt signature; spec uses SHA-1
gateway/platforms/weixin.py:1907	WeChat media upload protocol — field name rawfilemd5
gateway/platforms/yuanbao_media.py:311	Tencent Cloud COS V4 signature requires SHA-1 of HttpString

Switching algorithm here would break interoperability with the upstream service. Annotated each with # nosec B324 -- <protocol reason> so future contributors don't try to "fix" them and so static-analysis CI stays green.

Real behavior proof

After this change:

$ bandit -r . -x './tests,./skills/red-teaming,./scripts,./.git,./website,./docs' \
    --severity-level high --confidence-level high -t B324

  Total issues (by severity):
      Undefined: 0
      Low: 0
      Medium: 0
      High: 0          ← was 19 before this PR

Same digest output on representative inputs (verified inline with Node-equivalent in Python — hashlib.md5(b"x").hexdigest() == hashlib.md5(b"x", usedforsecurity=False).hexdigest() is true by construction since Python 3.9).

Why this matters

• Quiets 19 CI/scanner false-positives without hiding real findings
• Adds protocol-level context next to each "must-be-MD5" call so the next contributor doesn't open a "convert to SHA-256" PR and silently break QQ/WeChat/Tencent gateway integrations
• Zero runtime cost; only metadata

[teknium1]

Thanks for the focused security-scanner cleanup. I verified the premise against current origin/main and this still looks like a real, narrow cleanup rather than something already implemented.

Current main still has the raw weak-hash constructor sites this PR targets, for example:

• agent/codex_responses_adapter.py:236 derives an internal response id with hashlib.sha1(...).
• agent/context_compressor.py:937 uses hashlib.md5(...) as a duplicate-content fingerprint.
• gateway/platforms/qqbot/chunked_upload.py:369, :561, :562, and :563 still compute protocol MD5/SHA-1 values.
• tools/skills_hub.py:1148, :1413, :1640, :2134, and :2240 still use MD5 for cache keys.

I also checked the broader repo surfaces with git grep -En 'hashlib\.(md5|sha1)|from hashlib import sha1|\bmd5\(|\bsha1\(' origin/main -- agent gateway tools optional-skills tests website docs ui-tui tui_gateway acp_adapter hermes_cli run_agent.py cli.py model_tools.py toolsets.py; the extra hits I saw were HMAC digestmod references or docs/test/example material rather than obvious missed direct B324 constructor sites in the same runtime scope.

No correctness issues found in the diff: the PR preserves digest output and adds either usedforsecurity=False for non-security fingerprints/cache keys or # nosec B324 with protocol rationale where the upstream protocol requires MD5/SHA-1.

This is an automated hermes-sweeper review.

[teknium1]

Thanks for the focused scanner cleanup. The original raw constructor sites still exist on current main, including agent/codex_responses_adapter.py:236, agent/context_compressor.py:1411, and gateway/platforms/qqbot/chunked_upload.py:369/:561–:563.

Problems

• GitHub reports this PR as CONFLICTING. The two edited WeCom gateway paths were migrated by 560010547; the live raw constructors are now plugins/platforms/wecom/adapter.py:1210 and plugins/platforms/wecom/wecom_crypto.py:63.
• Current main also has unaddressed direct fingerprints at hermes_cli/web_server.py:11163, hermes_cli/web_server.py:12102, and optional-skills/security/unbroker/scripts/dossier.py:25. As written, the claimed repository-wide zero-hit result would not hold against current main.

Suggested changes

• Carry the WeCom protocol rationale to the plugin paths and classify the current non-security action/identifier fingerprints, or narrow the stated scan scope.
• The timeline-linked open #52783 covers overlapping FIPS hardening; a maintainer may want to consolidate the current-main salvage deliberately.

This is an automated hermes-sweeper review.

[teknium1]

Current main moved this adapter in 560010547; carry this protocol rationale to the live constructor at plugins/platforms/wecom/adapter.py:1210 during salvage.

[teknium1]

This source path was migrated in 560010547; the live callback-signature implementation is now plugins/platforms/wecom/wecom_crypto.py:63 and needs the same treatment.

[GottZ]

This was generated by AI during triage.

Summary

Twenty PRs address or reference this weak-hash/FIPS cluster; complete cached diffs are available for 18 of them. Those 18 diffs cover non-security MD5/SHA-1 annotations, protocol/security-sensitive digest sites requiring separate treatment, a distinct WeCom constant-time-comparison fix, and several overbroad SHA-256 sweeps; #72370 and #73278 can only be characterized from their metadata and discussion, not independently verified diffs.

Related pull requests

• #27368 related — (+19/-19) — salvage as scanner rationale, not as the consolidation head: the diff annotates cache/fingerprint and protocol-required MD5/SHA-1 sites, but it edits pre-relocation WeCom paths and does not cover current-main web-server and other sibling calls. This agrees with the keep_open review only conditionally: rebase, retarget live plugin paths, narrow the claimed scan scope, and add regression coverage.
• #39049 related — (+7/-1) — close in favor of #54617: the diff correctly introduces hmac.compare_digest but edits the retired gateway/platforms/wecom_crypto.py path and has no regression test. Despite the keep_open review on #39049, its own diff does not reach the live verifier, whereas #54617 targets plugins/platforms/wecom/wecom_crypto.py.
• #43937 related — (+44/-1) — close in favor of #54617 while carrying over its stronger spy and None-input tests: the production and test imports still target the retired gateway module. Despite the keep_open review and earlier approvals on #43937, the diff cannot protect the current plugin verifier until relocated.
• #48472 related — (+5/-5) — fold into the canonical FIPS patch: the diff correctly annotates all five Skills Hub MD5 cache-key constructors but supplies no FIPS-constructor regression test. Despite its keep_open review, these exact sites are also covered by #62654, which additionally covers skills_sync and web-server fingerprints.
• #51962 [closed] related — (+10/-10) — closed but relevant as an earlier reference implementation: its diff annotates ten non-security MD5 sites and is substantially subsumed by open #52783, while omitting several current sibling sites and tests.
• #51973 [closed] related — (+48/-48) — keep closed: the diff mechanically marks many SHA-256 operations, including PKCE, credential, integrity, and signature-related paths, as non-security, so it does not isolate the reported weak-algorithm FIPS cause safely. It remains relevant as evidence against an indiscriminate repository-wide replacement.
• #52783 related — (+16/-16) — use as the canonical FIPS consolidation head after revision: the diff covers the main cache/dedup/upload-checksum set, but misses the fifth current Skills Hub cache key, lacks restricted-constructor tests, and incorrectly groups the COS authorization digest with non-security hashes. Its keep_open review supports salvage only after those concrete corrections.
• #52967 [closed] related — (+18/-18) — closed duplicate of #52783 and therefore still relevant as its broader v2 reference: the diff adds the WeCom callback-signature SHA-1 site and the fifth Skills Hub key, but also retains the unresolved COS/security classification problem.
• #54617 related — (+32/-1) — retain as the current-path WeCom timing-hardening PR: the diff reaches plugins/platforms/wecom/wecom_crypto.py and replaces != with hmac.compare_digest, addressing a distinct comparison-side-channel cause. Its keep_open review should be resolved by replacing the redundant accept/reject additions with a spy test proving compare_digest is invoked.
• #56715 related — (+1/-1) — close after folding into #52783: the diff is the single context-compressor MD5 annotation already present in the broader patch and has no constructor regression test. Despite the keep_open review on #56715, the diff adds no independent behavior beyond that audited superset.
• #56719 related — (+13/-13) — fold only its confirmed cache, dedup, and upload-checksum changes into #52783: the diff overlaps the canonical patch and also marks the SHA-1 input to COS HMAC authorization as non-security without establishing protocol/FIPS compatibility. This addresses its keep_open review by excluding that disputed security-sensitive site rather than merging the PR wholesale.
• #62654 related — (+8/-8) — fold into #52783, especially its two web-server SHA-1 action-name fingerprints and the missing complete Skills Hub/skills_sync slice; the diff targets confirmed non-security derivations but lacks a restricted-constructor test. Despite the keep_open review on #62654, consolidation avoids duplicating the same five Skills Hub and skills_sync edits already present in #52783.
• #64062 related — (+8/-8) — do not merge wholesale; carry only QQ Bot checksums, Yuanbao's standalone MD5 helper, and the WeCom upload checksum into #52783. Its keep_open review explicitly identifies the callback-signature SHA-1 and COS authorization SHA-1 as security-sensitive paths for separate protocol decisions, which the diff currently conflates with content checksums.
• #64808 related — (+6/-4) — split before consolidation: its two hash edits exactly address the agent response-ID and context-dedup constructors, but the unrelated assertion changes include an unreachable TUI fallback and miss a sibling Codex assertion. Its keep_open review is best satisfied by folding only the two verified hash substitutions into #52783 and dropping the incomplete assertion sweep.
• #65434 related — (+5/-5) — close after folding its non-security sites into #52783: the diff combines the same two Weixin edits as #66857 with skills_sync and web-server edits already covered by #62654, while omitting all five Skills Hub siblings. Despite the keep_open review, the diff is fully decomposable into those broader, better-audited slices.
• #65561 related — (+11/-10) — do not merge as part of this weak-hash fix: the diff predominantly adds usedforsecurity=False to SHA-256, including PKCE and integrity/fingerprint paths, rather than resolving the MD5/SHA-1 constructors behind the reported FIPS failures. Any genuine SHA-256 runtime incompatibility needs separate reproduced evidence and security-context classification.
• #65570 related — (+20/-19) — do not merge wholesale: the diff marks SHA-256 webhook verification, PKCE, integrity, and authentication-related operations as non-security while leaving the central MD5 cache-key family outside its scope. This follows the keep_open review's requirement to re-scope around verified blockable algorithms and exclude security contexts.
• #66857 related — (+2/-2) — fold into #52783 and then close: the diff precisely annotates the two live Weixin MD5 uses for deduplication and upload integrity, both already present in #52783. Despite the keep_open review confirming the patch is current and exact, consolidation removes a byte-for-byte overlapping slice rather than disputing its correctness.
• #72370 related — (+1/-1) — needs diff retrieval and a focused regression test before disposition: metadata claims a non-security TUI MCP-revision SHA-1 annotation, but no cached diff is available here, so the exact changed symbol and applicability cannot be independently asserted from diff evidence.
• #73278 [closed] related — (+1/-1) — closed but relevant as a discussion-level duplicate candidate for #72370; no cached diff is available, so exact diff identity cannot be independently confirmed. Contributor discussion closed it for missing tests, and a non-contributor further narrowed the alleged symptom from a gateway crash to loss of revision tracking because the exception is caught.

Duplicates

Diff-backed overlap: #39049 and #43937 implement the same WeCom comparison hardening on the retired path, with #54617 as the live-path successor; #48472 is the five-site Skills Hub slice also present in #62654 and largely in #52783; #51962, #52967, #56715, #56719, #62654, #64062, #64808, #65434, and #66857 overlap substantial subsets of #52783. Discussion alleges #73278 duplicates #72370, but neither cached diff is available here, so exact identity is not independently diff-verified.

Suggested consolidation

Merge #52783 only after rebasing and turning it into the tested canonical non-security MD5/SHA-1 patch: add the missing Skills Hub and current web-server/TUI sibling sites after live verification, retain protocol-required checksums only with explicit rationale, exclude the COS authorization and WeCom callback-signature security contexts, and add mocked restricted-constructor tests. Then close the overlapping FIPS slices #27368, #48472, #56715, #56719, #62654, #64062, #64808, #65434, and #66857 as superseded; keep #51962 and #52967 closed. Separately merge #54617 after adding a compare_digest spy test, and close #39049 and #43937 as obsolete-path variants. Do not reopen #73278; assess #72370 only after obtaining its diff and testing the caught-error revision-tracking behavior.

Cross-PR triage: Reviewed 20 pull requests and 0 issues in this complex. Diffs were read for 18 of 20 PRs (rest unavailable); Assessment working set: 110 kB of PR diffs, 28 kB of issue/PR text, 25 kB of discussion (31 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[GottZ]

This was generated by AI during triage.

Delta since our previous triage comment

@teknium1’s review confirms that #72370 targets the live bare SHA-1 constructor and clarifies the actual failure mode: the exception is caught, so FIPS hosts lose MCP revision-based reload gating rather than crashing the gateway. The now-available diff also confirms the exact one-line change, while the review identifies the missing regression because the existing fixture replaces _compute_mcp_rev() entirely.

Changed pull requests

• #72370 related — (+1/-1) — keep open pending test and description correction: the diff correctly passes usedforsecurity=False to the non-security SHA-1 revision hash, but it needs a regression invoking the real _compute_mcp_rev() with a rejecting SHA-1 stub. This agrees with @teknium1’s keep_open review; the PR should describe restoration of revision tracking on FIPS hosts, not an uncaught gateway crash.

Suggested consolidation

The previous consolidation recommendation is unchanged; retain #72370 for the focused test-backed MCP revision fix.

Complex graph unchanged since our previous triage comment.

Cross-PR triage: Reviewed 20 pull requests and 0 issues in this complex. Diffs were read for 19 of 20 PRs (rest unavailable); Assessment working set: 111 kB of PR diffs, 28 kB of issue/PR text, 26 kB of discussion (33 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.