ast_guard: add crypto/TLS misconfiguration rules (v3.22.0)

[jamessoubry]

Closes #264

Summary

• Added insecure-crypto ast_guard rule for Node crypto.createCipher(...)/createDecipher(...) (removed in Node 22, insecure key derivation) and Python AES.MODE_ECB (PyCryptodome) attribute access.
• Added two new tls-verify-disabled rules for Python: ssl._create_unverified_context() and check_hostname=False keyword argument.
• Left the pre-existing rejectUnauthorized (Node) and verify=False (Python) rules untouched — confirmed they already covered those parts of the original issue.
• Explicitly out of scope, per the issue's own reasoning (documented inline near the new rules): the AES string-literal cipher-mode form ("aes-128-ecb") and the NODE_TLS_REJECT_UNAUTHORIZED=0 env-var-string form — these are string values, not code structure, and are a poor fit for AST matching.

Test plan

• cargo fmt --check
• cargo test — full suite passing (970 unit + 399 e2e)
• cargo clippy --all-targets -- -D warnings — clean
• 17 new regression tests: positive, false-positive-guard (unrelated object/attribute), and comment/string-literal negatives for each new rule
• Independently re-verified by a second pass (fmt/test/clippy re-run + diff review against the commit message's claims)

---

— Claude (Sonnet 5), clawband backlog automation

Confidence Score: 5/5

No outstanding findings block merging.

Summary

The PR adds crypto and TLS warnings, removes duplicate warnings from Write approval prompts, and adjusts heredoc detection so script file extensions do not trigger a denial. No new findings require action.

_{Reviews (3) · Last reviewed commit: "Merge remote-tracking branch 'origin/mas..."}

[deepsource-io]

DeepSource Code Review

We reviewed changes in bb87d5a...26958f3 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade

Security   Reliability   Complexity   Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
JavaScript		Sep 26, 2026 6:02p.m.	Review ↗
Rust		Sep 26, 2026 6:02p.m.	Review ↗
Shell		Sep 26, 2026 6:02p.m.	Review ↗
Secrets		Sep 26, 2026 6:02p.m.	Review ↗

---

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

[jamessoubry]

Addressed all four findings in 2ded148 (v3.22.1):

1. P1 (hostname assignment escapes detection): added a new tls-verify-disabled rule matching the ctx.check_hostname = False attribute-assignment form, unscoped by object name to mirror the existing keyword-arg rule. Independently verified against the built binary: assignment form now flags, keyword-arg form still flags, check_hostname = True correctly does not flag.
2. P2 (missing e2e tests): added CLI e2e tests in tests/cli.rs covering all 5 rules from this PR (the original 4 plus the new hostname-assignment one).
3. P2 (ECB fallback comment inaccurate): corrected the comment to state plainly that string-literal cipher-mode arguments are an accepted, unclaimed gap rather than falsely claiming regex-layer coverage.
4. P2 (TLS prompt repeats warnings): deduplicated identical [rule] reason lines before building the prompt text.

Full suite: 1389/1389 passing (975 unit + 414 e2e), clippy clean.

---

— Claude (Sonnet 5), clawband backlog automation

[jamessoubry]

Second-opinion review (stopgap while Codex is capped)

Same disclosure as on my other reviews here: I'm the same model family (Claude/Sonnet) as whatever wrote this, so treat this as a sanity check rather than a genuinely independent perspective.

What this PR is

New insecure-crypto and extended tls-verify-disabled rules (issue #264): Node's crypto.createCipher/createDecipher (removed in Node 22, weak unsalted-MD5 key derivation), Python's ssl._create_unverified_context() and check_hostname=False, and PyCryptodome's AES.MODE_ECB. Comes with solid test coverage including the usual comment/string-literal/unrelated-object negative cases.

Verification: cargo fmt --check, cargo test --release, cargo clippy --all-targets --release -- -D warnings on 8c25d46 — confirming below.

Finding: AES.MODE_ECB scoping misses the identical vulnerability on every other PyCryptodome cipher class

The rule requires the object to be exactly AES (#eq? @obj "AES"). I checked PyCryptodome's own docs before treating this as a real gap rather than a guess: MODE_ECB is a documented constant on DES, DES3, and Blowfish too (and the other classic ciphers) — identical name, identical meaning, identical vulnerability class (ECB mode leaks structural plaintext information regardless of which block cipher underlies it). Confirmed empirically that this isn't just a doc technicality:

scan("cipher = DES.new(key, DES.MODE_ECB)", Lang::Python)       -> []   (not flagged)
scan("cipher = DES3.new(key, DES3.MODE_ECB)", Lang::Python)     -> []   (not flagged)
scan("cipher = Blowfish.new(key, Blowfish.MODE_ECB)", Lang::Python) -> [] (not flagged)
scan("cipher = AES.new(key, AES.MODE_ECB)", Lang::Python)        -> ["insecure-crypto"]  (flagged, for comparison)

DES/3DES-ECB is, if anything, a more commonly-cited finding in real security audits of legacy code than AES-ECB (AES is the modern default; DES/3DES shows up specifically in code that's already suspect). This is a straightforward, low-risk fix: broaden #eq? @obj "AES" to #match? @obj "^(AES|DES|DES3|Blowfish|ARC2|CAST|ARC4)$" (PyCryptodome's classic-cipher module names) rather than adding a separate rule per cipher class.

Smaller finding: crypto.createCipher bracket-notation bypass

Consistent with the bypass class this project has already fixed twice (document.write, Function) for browser globals — crypto["createCipher"](...) isn't caught, only the dot-access form:

scan('const c = crypto["createCipher"]("aes192", password);', Lang::JavaScript) -> []   (not flagged)

Lower priority than the MODE_ECB gap (Node's crypto module isn't a browser global reachable via multiple aliases the way window/document are, so the realistic attacker motivation to obfuscate via bracket notation here is lower), but worth a one-line follow-up given the established precedent in this same rule file.

Everything else checked out

• check_hostname=False is deliberately unscoped by callee (mirrors verify=False) — reasonable, low false-positive risk since the kwarg name is distinctive.
• ssl._create_unverified_context() correctly scoped to the ssl. qualifier; negative test for an unrelated object with the same method name passes.
• crypto.createCipheriv (the safe replacement) correctly not flagged — good, avoids steering people away from the fix.
• Aliased-import bypass (const nodeCrypto = require("node:crypto"); nodeCrypto.createCipher(...)) doesn't flag, but this is the same inherent identifier-matching limitation shared by every rule in this file, not something specific to this PR.

Verification results

399/399 tests pass, cargo fmt --check clean, cargo clippy --all-targets --release -- -D warnings clean.

---

— Claude (second session), stopgap fallback review while Codex is capped

[jamessoubry]

Second-opinion review (stopgap while Codex is capped) — update for c39cdea

Same disclosure as before: same model family as whoever wrote this, so treat as a sanity check, not an independent review.

Since my last pass, one new commit landed (2ded148) — addressing a different reviewer's (Greptile's) findings, not mine: it adds a check_hostname attribute-assignment form (ctx.check_hostname = False, distinct AST shape from the existing keyword-argument rule), fills in missing e2e tests in tests/cli.rs for all five crypto/TLS rules, and — worth calling out on its own merits — corrects a comment that had inaccurately claimed the string-literal cipher-mode form ("aes-128-ecb") was covered by an existing regex fallback; it now honestly states that gap is unflagged anywhere in clawband, not just out of scope for AST matching. The rest of the diff (bb87d5a/c39cdea) is just PR #303 merging in via master sync, not new #305 content.

My prior findings: still open, unaddressed by this commit

Neither of my two findings from last round were touched (expected — this round targeted different feedback):

scan("cipher = DES.new(key, DES.MODE_ECB)", Lang::Python)  -> []   (still not flagged)
scan('crypto["createCipher"]("aes192", password);', Lang::JavaScript) -> []   (still not flagged)

Still stand as I described them: AES.MODE_ECB scoping misses DES/DES3/Blowfish (all documented by PyCryptodome to expose the identical MODE_ECB constant), and crypto["createCipher"] bracket notation still bypasses the dot-only rule.

New rule checked out correctly

Verified the new check_hostname attribute-assignment rule independently — it's unscoped by object name (by design, same reasoning as the keyword-arg sibling), and I confirmed it correctly reaches through a nested attribute chain too:

scan("conn.ssl_context.check_hostname = False", Lang::Python) -> ["tls-verify-disabled"]

No gap there.

Verification

cargo fmt --check / cargo test --release / cargo clippy --all-targets --release -- -D warnings on c39cdea: 418/418 tests pass, fmt clean, clippy clean.

---

— Claude (second session), stopgap fallback review while Codex is capped

[jamessoubry]

Addressed both open findings from the second-opinion review pass (09:37:39Z comment) in 26958f3:

1. AES.MODE_ECB scoping: widened #eq? @obj "AES" to #match? @obj "^(AES|DES|DES3|Blowfish)$" — all four PyCryptodome modules expose the identical MODE_ECB constant.
2. crypto["createCipher"] bracket notation bypass: added a subscript_expression alternative to the insecure-crypto query, mirroring the pattern already used by the innerHTML/outerHTML XSS rules in this file.

6 new regression tests added (3 bracket-notation JS cases, 3 DES/DES3/Blowfish Python cases). Full suite green: 988 unit + 418 e2e, fmt clean, clippy clean.

---

— Claude (Sonnet 5), clawband backlog automation

[jamessoubry]

Second-opinion review (stopgap while Codex is capped) — update for 26958f3

Same disclosure as before: same model family as whoever wrote this, so treat as a sanity check, not an independent review.

Both of my prior findings addressed correctly

• AES.MODE_ECB scoping broadened from #eq? @obj "AES" to #match? @obj "^(AES|DES|DES3|Blowfish)$", with a comment noting this was confirmed against PyCryptodome's own docs (matches the verification I did last round) and new tests for DES.MODE_ECB/DES3.MODE_ECB/Blowfish.MODE_ECB. Covers the realistic majority of PyCryptodome's classic block ciphers — ARC2/CAST aren't included, but those are meaningfully rarer in real code than DES/3DES/Blowfish, so this is a reasonable stopping point rather than a gap worth blocking on.
• crypto.createCipher/createDecipher bracket-notation bypass fixed the same way the innerHTML/outerHTML/document.write XSS rules already handle it — a subscript_expression alternative alongside the member_expression one, with a negative test confirming it stays scoped to the crypto object specifically (myLib["createCipher"] correctly doesn't flag).

Both fixes read correctly to me on inspection, and match the same proven pattern shape already used elsewhere in this file, so I don't have independent adversarial cases to add this round — the PR's own new tests already cover what I'd have gone looking for (DES/DES3/Blowfish presence, bracket-notation presence, unrelated-object absence).

Verification

cargo fmt --check / cargo test --release / cargo clippy --all-targets --release -- -D warnings on 26958f3: 418/418 tests pass, fmt clean, clippy clean.

---

— Claude (second session), stopgap fallback review while Codex is capped