fix: tighten press/click.*hold regex to prevent false positives on normal page content - #1649
Conversation
…rmal page content
There was a problem hiding this comment.
Hey @ssilare-adobe,
Strengths
bot-blocker-detect.js:106-107: The fix directly addresses the root cause. Replacing unbounded.*with\s+(and\s+)?eliminates catastrophic matching across thousands of characters of minified HTML while preserving detection of real challenge copy. The new patterns are also free of ReDoS risk - no nested quantifiers or overlapping alternations, and worst-case backtracking is linear.bot-blocker-detect.test.js:962-975: The regression test reproduces the exact failure scenario from the bug report ("pressure" + "placeholder" proximity) with realistic HTML content and good behavior-level assertions checking bothcrawlableandtype.- Change discipline is excellent. Two patterns changed, one test added, nothing else touched. The PR body explains the mechanism of failure thoroughly with concrete index positions from a real site.
- Follows all documented project conventions: test mirrors src structure, uses describe/it with expect assertions, and the commit message follows conventional commits format.
Issues
Minor (Nice to Have)
-
bot-blocker-detect.js:106-107: The patterns lack word boundaries, so they can match inside compound words. For example,/press\s+(and\s+)?hold/iwould match in content like "Express Holdings" - "press" at the tail of "Express", whitespace, then "hold" at the head of "Holdings". Adding\bat both edges (/\bpress\s+(and\s+)?hold\b/iand/\bclick\s+(and\s+)?hold\b/i) would close this. This is Minor because the false-positive surface is dramatically smaller than before, and the practical likelihood is low. -
bot-blocker-detect.test.js: The new regression test only exercises thepress...holdfalse-positive path. A symmetric negative test forclick...hold(e.g., HTML containing "clickbait" near "household") would make future regressions on either pattern independently detectable.
Recommendations
- The broader pattern (a flat array of regexes run against full HTML documents) has a structural fragility: any regex with unbounded wildcards is one minified page away from the same class of false positive. Worth considering in a future effort: a lightweight validation harness that runs each pattern against a corpus of known-good pages as a CI gate, catching overly broad patterns before they ship.
Assessment
Ready to merge? Yes
Reasoning: This is a well-scoped, minimal fix to a real production false-positive scenario. The tightened patterns correctly model the intended detection semantics, CI passes with all 1155 tests green, and no security or architectural concerns exist.
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 33s | Cost: $2.26 | Commit: 261f994f1820ac1291696b2f1f6a023dcb8c994e
If this code review was useful, please react with 👍. Otherwise, react with 👎.
## [@adobe/spacecat-shared-utils-v1.116.6](https://github.com/adobe/spacecat-shared/compare/@adobe/spacecat-shared-utils-v1.116.5...@adobe/spacecat-shared-utils-v1.116.6) (2026-06-04) ### Bug Fixes * tighten press/click.*hold regex to prevent false positives on normal page content ([#1649](#1649)) ([96e9c83](96e9c83))
|
🎉 This PR is included in version @adobe/spacecat-shared-utils-v1.116.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## Summary - Bumps `@adobe/spacecat-shared-utils` from `1.115.4` to `1.116.6` - `@adobe/spacecat-shared-tokowaka-client` is already at `1.19.0` (landed in #2552) ## Changes - `package.json`: `@adobe/spacecat-shared-utils` `1.115.4` → `1.116.6` Notable in `1.116.6`: tightens the `/press.*hold/` and `/click.*hold/` regexes in `detectBotBlocker` to require adjacent words, preventing false-positive bot-block detections on normal page content (e.g. athenahealth.com). See adobe/spacecat-shared#1649. ## Test plan - [ ] CI passes - [ ] No regression in bot-blocker detection
Summary
CHALLENGE_PATTERNS.generalcontained/press.*hold/iand/click.*hold/iwith unbounded.*wildcardsblood pressure monitor(index 281,995) toplaceholder graphic(index 291,537) on athenahealth.com — causing a false-positivecrawlable: falseresult despite the site returning HTTP 200 with no challenge page/press\s+(and\s+)?hold/i— matching real challenge copy like "Press and hold the button" while rejecting incidental word proximityChanges
src/bot-blocker-detect/bot-blocker-detect.js: narrowpress.*holdandclick.*holdpatternstest/bot-blocker-detect/bot-blocker-detect.test.js: add regression test coveringblood pressure+placeholdercontentTest plan
does not false-positive on "pressure" and "placeholder" in normal page contentpasses