feat(helpers): add rule-level validate (like test) by Kikobeats · Pull Request #866 · microlinkhq/metascraper

Kikobeats · 2026-08-06T19:07:11Z

Summary

Add optional validate on a rule or rules bundle (same shape as test): after a rule returns a value, validate(value, args) may return falsy or throw to reject that candidate and continue to the next rule.
loadRules / mergeRules copy bundle validate onto each rule; findRule reads rule.validate.
Extract shared withIframe into @metascraper/helpers so audio/video nested iframe probing (and validate) share one path.
Docs in CONTRIBUTING; TypeScript types updated. No scrape-time metascraper({ validate }) option.

rules.validate = async (value, args) => !(await isHostile(value))
// or
Object.assign(ruleFn, { validate: async value => … })

Unblocks Microlink API logo XSS fallthrough (hostile candidate → next rule) by setting validate on logo rule bundles.

Test plan

ava on helpers find-rule, metascraper validate / merge-rules, audio/video iframe
CI green on next
After merge/release, bump api @metascraper/helpers and set rules.validate on logo / logo-favicon / logo-bimi

Note

Medium Risk
Changes core findRule resolution used on every scrape property, but behavior is backward-compatible when validate is omitted and is covered by new unit/integration tests.

Overview
Introduces optional validate on individual rules or whole rule bundles (mirroring test): after a rule returns a value, validate(value, args, debug) can reject it so findRule tries the next rule; if all fail, the property is null. Bundle-level validate is copied onto each rule via loadRules / mergeRules, with TypeScript types and CONTRIBUTING docs.

Moves duplicated iframe / twitter:player probing from metascraper-audio and metascraper-video into shared withIframe in @metascraper/helpers, including propName for debug timing and coverage that validate applies to values found inside iframes.

^{Reviewed by Cursor Bugbot for commit 27f1adc. Bugbot is set up for automated code reviews on this repo. Configure here.}

Allow callers to reject a rule result and continue to the next rule without throwing, so logo XSS fallthrough can skip hostile candidates. Co-authored-by: Cursor <[email protected]>

coderabbitai · 2026-08-06T19:07:31Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The change adds synchronous and asynchronous rule validation, propagates validators through rule metadata, and continues rule evaluation after rejected candidates. A shared withIframe helper now handles normalized iframe and Twitter player extraction for audio and video rules.

Changes

Validation-aware rule resolution

Layer / File(s)	Summary
Validation contract and metadata `packages/metascraper/src/index.d.ts`, `packages/metascraper/src/rules.js`, `packages/metascraper/test/types/*`, `packages/metascraper/test/unit/merge-rules.js`, `CONTRIBUTING.md`	The public types, rule loading, inline rule merging, type tests, documentation, and metadata tests now support bundle-level and per-rule `validate` callbacks.
Validation-aware rule resolution `packages/metascraper-helpers/src/index.js`, `packages/metascraper-helpers/test/find-rule.js`, `packages/metascraper/test/unit/validate.js`	`findRule` accepts optional arguments, evaluates synchronous or asynchronous validators, logs validator errors, rejects invalid candidates, and continues with later rules.
Shared iframe rule evaluation `packages/metascraper-helpers/src/index.js`, `packages/metascraper-audio/src/index.js`, `packages/metascraper-video/src/index.js`, `packages/metascraper-audio/test/iframe.js`, `packages/metascraper-video/test/iframe.js`	`withIframe` centralizes iframe and Twitter player probing for audio and video rules. Tests cover rejected candidates and fallback results.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Metascraper
  participant withIframe
  participant getIframeCached
  participant findRule
  participant Validate
  Metascraper->>findRule: evaluate direct rules
  findRule->>Validate: validate candidate
  Validate-->>findRule: accept or reject
  Metascraper->>withIframe: evaluate nested media rules
  withIframe->>getIframeCached: fetch normalized iframe URL
  getIframeCached-->>withIframe: nested metadata
  withIframe->>findRule: evaluate nested rules
  findRule->>Validate: validate nested candidate
  Validate-->>findRule: accept or reject
  findRule-->>withIframe: result or continue probing

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding rule-level validation support through the helpers package.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch next

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

packages/metascraper/test/unit/validate.js (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test the validator context contract.

This test only verifies the value argument. Record the second validator argument. Assert propName, rule, and args match the documented contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/metascraper/test/unit/validate.js` around lines 24 - 31, Update the
scraper test’s validate callback to capture its second context argument, then
assert that context.propName, context.rule, and context.args match the
documented validator contract. Keep the existing value filtering and logo
assertions unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/metascraper/test/unit/validate.js`:
- Around line 24-31: Update the scraper test’s validate callback to capture its
second context argument, then assert that context.propName, context.rule, and
context.args match the documented validator contract. Keep the existing value
filtering and logo assertions unchanged.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 87e95a1d-e3ea-47c0-86a1-5b3829da3434

📥 Commits

Reviewing files that changed from the base of the PR and between 9c5e77b and 753228e.

📒 Files selected for processing (2)

packages/metascraper-helpers/src/index.js
packages/metascraper/test/unit/validate.js

`validate` applied to every property, so a validator written for one field silently nulled unrelated ones. It now accepts an object keyed by property name; the function form still runs for every property. A throwing validator aborted the whole property through get-data's catch. It now rejects only the candidate that triggered it and the search resumes at the next rule, with the error logged under DEBUG=metascraper:find-rule. metascraper-video and metascraper-audio rebuilt the args object for their nested findRule calls, so validate never reached rules behind an iframe and per-candidate fallthrough did not happen there. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/metascraper-audio/test/iframe.js (1)
71-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover property-specific and Twitter-player validation propagation.

The current tests use global validators and generic iframe sources only. A regression that removes the explicit property name, or drops validation arguments in the Twitter-player path, can still pass.

packages/metascraper-audio/test/iframe.js#L71-L89: Add a test with { audio: validator }. Add coverage for meta[name="twitter:player"].

packages/metascraper-video/test/iframe.js#L72-L91: Add a test with { video: validator }. Add coverage for meta[name="twitter:player"].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/metascraper-audio/test/iframe.js` around lines 71 - 89, Add
regression coverage in packages/metascraper-audio/test/iframe.js:71-89 by
changing the iframe validation test to use an explicit { audio: validator }
configuration and include meta[name="twitter:player"] validation. Apply the same
coverage in packages/metascraper-video/test/iframe.js:72-91 using { video:
validator }, including the Twitter-player metadata path; ensure both tests
verify hostile values are rejected and safe values are retained.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 363-364: Update the validate example to check that value is a
string before calling startsWith, while preserving rejection of string values
beginning with "data:" and accepting non-string values without throwing.

---

Nitpick comments:
In `@packages/metascraper-audio/test/iframe.js`:
- Around line 71-89: Add regression coverage in
packages/metascraper-audio/test/iframe.js:71-89 by changing the iframe
validation test to use an explicit { audio: validator } configuration and
include meta[name="twitter:player"] validation. Apply the same coverage in
packages/metascraper-video/test/iframe.js:72-91 using { video: validator },
including the Twitter-player metadata path; ensure both tests verify hostile
values are rejected and safe values are retained.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 635fdb3f-8a82-4638-84e8-272753d16804

📥 Commits

Reviewing files that changed from the base of the PR and between 753228e and 390cb2c.

📒 Files selected for processing (10)

README.md
packages/metascraper-audio/src/index.js
packages/metascraper-audio/test/iframe.js
packages/metascraper-helpers/src/index.js
packages/metascraper-helpers/test/find-rule.js
packages/metascraper-video/src/index.js
packages/metascraper-video/test/iframe.js
packages/metascraper/src/index.d.ts
packages/metascraper/test/types/index.test-d.ts
packages/metascraper/test/unit/validate.js

🚧 Files skipped from review as they are similar to previous changes (1)

packages/metascraper-helpers/src/index.js

Deduplicate nested iframe probing so validate keeps one code path, and tighten docs/tests from review feedback. Co-authored-by: Cursor <[email protected]>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/metascraper-helpers/src/index.js`:
- Around line 661-665: Update the iframe wrapper flow around findRule so
candidates already validated by the nested findRule call are not validated again
by the outer findRule. Preserve the nested validation context, including rule
and htmlDom, by carrying an internal validated-result state through the wrapper
return path. Add coverage asserting an accepted iframe candidate invokes its
validator exactly once.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: eed0265c-0a0f-4a1a-a611-495d2f22cab4

📥 Commits

Reviewing files that changed from the base of the PR and between 3502697 and cd84e8c.

📒 Files selected for processing (7)

README.md
packages/metascraper-audio/src/index.js
packages/metascraper-audio/test/iframe.js
packages/metascraper-helpers/src/index.js
packages/metascraper-video/src/index.js
packages/metascraper-video/test/iframe.js
packages/metascraper/src/index.d.ts

🚧 Files skipped from review as they are similar to previous changes (3)

README.md
packages/metascraper-video/src/index.js
packages/metascraper/src/index.d.ts

Scrape-time validate mixed a second control plane into the call. Bundle/rule validate matches test and needs no args forwarding. Co-authored-by: Cursor <[email protected]>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/metascraper/src/rules.js`:
- Around line 65-70: Update mergeRules so pkgName is retained when destructuring
each rule and ruleSet, then pass the parent package name to attachRuleMeta for
every processed inline rule. Preserve the existing test and validate metadata
while ensuring inline rules keep their originating package name.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f52a48f-f20e-4281-9123-64435eaba283

📥 Commits

Reviewing files that changed from the base of the PR and between cd84e8c and a48e8d9.

📒 Files selected for processing (10)

CONTRIBUTING.md
packages/metascraper-audio/test/iframe.js
packages/metascraper-helpers/src/index.js
packages/metascraper-helpers/test/find-rule.js
packages/metascraper-video/test/iframe.js
packages/metascraper/src/index.d.ts
packages/metascraper/src/rules.js
packages/metascraper/test/types/index.test-d.ts
packages/metascraper/test/unit/merge-rules.js
packages/metascraper/test/unit/validate.js

🚧 Files skipped from review as they are similar to previous changes (4)

packages/metascraper-helpers/test/find-rule.js
packages/metascraper-audio/test/iframe.js
packages/metascraper-video/test/iframe.js
packages/metascraper-helpers/src/index.js

loadRules already copied bundle pkgName; mergeRules dropped it so inline bundles lost debug names and pkgName could become a prop key. Co-authored-by: Cursor <[email protected]>

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

feat(helpers): add per-call validate hook to findRule

753228e

Allow callers to reject a rule result and continue to the next rule without throwing, so logo XSS fallthrough can skip hostile candidates. Co-authored-by: Cursor <[email protected]>

coderabbitai Bot reviewed Aug 6, 2026

View reviewed changes

Kikobeats and others added 2 commits August 6, 2026 21:19

refactor(video,audio): keep param destructuring in iframe rules

3502697

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

coderabbitai Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread README.md Resolved

refactor(helpers): share withIframe for audio/video

cd84e8c

Deduplicate nested iframe probing so validate keeps one code path, and tighten docs/tests from review feedback. Co-authored-by: Cursor <[email protected]>

coderabbitai Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread packages/metascraper-helpers/src/index.js Resolved

refactor(helpers): move validate onto rules like test

a48e8d9

Scrape-time validate mixed a second control plane into the call. Bundle/rule validate matches test and needs no args forwarding. Co-authored-by: Cursor <[email protected]>

coderabbitai Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread packages/metascraper/src/rules.js Resolved

Kikobeats changed the title ~~feat(helpers): add per-call validate hook to findRule~~ feat(helpers): add rule-level validate (like test) Aug 6, 2026

Kikobeats and others added 5 commits August 6, 2026 22:34

fix(metascraper): keep pkgName on inline mergeRules

49b7f51

loadRules already copied bundle pkgName; mergeRules dropped it so inline bundles lost debug names and pkgName could become a prop key. Co-authored-by: Cursor <[email protected]>

feat(helpers): pass find-rule debug logger to validate

83f2483

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

refactor: simplify validate plumbing and its tests

3d8cd41

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

refactor: drop redundant meta guard and duplicated iframe tests

4f88ea6

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

test(helpers): require package entry like sibling tests

27f1adc

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VSEyFT3dw16tyvhRGVAWrx

Kikobeats merged commit d40856b into master Aug 7, 2026
85 of 89 checks passed

Kikobeats deleted the next branch August 7, 2026 08:03

This was referenced Aug 7, 2026

refactor!: move validate to scrape-level findRule hook #867

Merged

feat(helpers): pass propName to validate #868

Merged

Navigation Menu

Repository navigation

feat(helpers): add rule-level validate (like test) - #866

feat(helpers): add rule-level validate (like test)#866
Kikobeats merged 10 commits into
masterfrom
next

Kikobeats commented Aug 6, 2026 •

edited by cursor Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Kikobeats commented Aug 6, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

coderabbitai Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Kikobeats commented Aug 6, 2026 •

edited by cursor Bot

Loading

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading